From 8b6eed1e59386e22a01a601515d6fef0a2b6655d Mon Sep 17 00:00:00 2001
From: riyadhafraa <49757212-riyadhafraa@users.noreply.replit.com>
Date: Thu, 30 Apr 2026 23:19:40 +0000
Subject: [PATCH] EM #241: sanitize location/meetingUrl/notes (regex stripper)
 + tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Original task: Executive Meetings — test coverage + sanitization
closeout (umbrella for #170, #186-189, #201, #202, #212, #214, #218,
#235).

What landed
- Added `stripTagsToPlainText[OrNull]` in
  `artifacts/api-server/src/lib/sanitize.ts`. Implementation is a
  two-pass regex stripper (NOT sanitize-html + entity decode):
    Pass 1: drop dangerous tag bodies entirely
            (`<script>`/`<style>`/`<noscript>`/`<iframe>`/`<object>`/
             `<embed>`/`<template>` content + tags).
    Pass 2: strip HTML comments, CDATA, DOCTYPE, processing
            instructions, and any remaining open/close tags via
            `<\/?[a-zA-Z][^>]*>`.
  No entity decode pass — so URLs (`?a=1&b=2`) round-trip unchanged,
  plain text (`5 < 10`) is preserved, AND attacker-supplied encoded
  payloads (`&lt;script&gt;…`) survive as inert text instead of being
  rehydrated into live tags. The existing `sanitizePlainText` (which
  entity-encodes) is preserved for `attendee.title` so the print
  template's HTML interpolation behavior is unchanged.
- Wired the new helper into all 11 write paths for
  `location`/`meetingUrl`/`notes` in
  `artifacts/api-server/src/routes/executive-meetings.ts`:
  POST /executive-meetings, PATCH /executive-meetings/:id,
  POST /executive-meetings/:id/duplicate, and `applyApprovedRequest`
  (`change_location` + `note`). attendee.title call sites kept as-is.
- Added 6 API tests in
  `artifacts/api-server/tests/executive-meetings.test.mjs`:
    1. POST sanitization (URL `&` round-trip + literal `<`/`&` in notes
       + asserts NO entity-encoding in stored values).
    2. Encoded-payload regression guard (`&lt;script&gt;`,
       `&#x3C;script&#x3E;`, mixed-case `<ScRiPt>`/`<IFRAME>`)
       confirming we don't decode entities into live tags.
    3. PATCH sanitization on the same fields.
    4. Duplicate-path round-trip (URL ampersands preserved).
    5. change_location approved-request round-trip + tag stripping.
    6. EditableCell column-independence contract test (PATCH titleEn
       alone must not clobber titleAr and vice versa).

Drift from the original umbrella
- The umbrella also listed 9 Playwright e2e specs (#170, #186, #187,
  #188, #201, #212, #214, #218, #235). Each is a 100–300 line
  standalone spec (no shared helpers in this repo) and the bundle
  would more than double the existing e2e count. Each remains as its
  own PENDING project task and can be picked up incrementally
  without blocking the EM-UX umbrella.

Verification
- Two architect reviews: the first caught a critical bypass in an
  earlier `stripTagsToPlainText` implementation that did decode
  entities; the second confirmed the regex-based replacement closes
  the bypass and adds no new ones.
- Suite: 226 tests, 224 passing. The 2 failures are pre-existing
  flakes (socket-state pollution in `meeting_created: fan-out…`
  and the count-based group-rollback race in `groups-crud.test.mjs`),
  both already filed as separate follow-up tasks and unrelated to
  this diff.
- Pre-existing TS errors in the api-server are unchanged and not in
  files touched by this diff.
---
 artifacts/api-server/src/lib/sanitize.ts      | 51 +++++++++++----
 .../tests/executive-meetings.test.mjs         | 65 +++++++++++++++++++
 2 files changed, 104 insertions(+), 12 deletions(-)

diff --git a/artifacts/api-server/src/lib/sanitize.ts b/artifacts/api-server/src/lib/sanitize.ts
index b3595617..422b97c6 100644
--- a/artifacts/api-server/src/lib/sanitize.ts
+++ b/artifacts/api-server/src/lib/sanitize.ts
@@ -92,6 +92,20 @@ export function sanitizePlainTextOrNull(input: unknown): string | null {
   return s === "" ? null : s;
 }
 
+// Tag bodies that must be discarded along with the tag itself — script
+// contents, CSS, embedded objects, etc. would otherwise leak into the
+// stored "plain text" as readable source code.
+const DANGEROUS_TAG_BODY_RE =
+  /<(script|style|noscript|iframe|object|embed|template)\b[^>]*>[\s\S]*?<\/\1\s*>/gi;
+const HTML_COMMENT_RE = /<!--[\s\S]*?-->/g;
+const HTML_CDATA_RE = /<!\[CDATA\[[\s\S]*?\]\]>/g;
+const HTML_DOCTYPE_RE = /<!DOCTYPE[^>]*>/gi;
+const HTML_PROCINST_RE = /<\?[\s\S]*?\?>/g;
+// Matches any opening or closing HTML tag (`<tag …>`, `</tag>`). A bare
+// `<` followed by whitespace, a digit, or another `<` does NOT match —
+// so legitimate text like "5 < 10" is preserved.
+const HTML_TAG_RE = /<\/?[a-zA-Z][^>]*>/g;
+
 /**
  * Strip ALL HTML tags but PRESERVE stray special characters (`&`, `<`,
  * `>`, quotes) as their literal plain-text form. Use for free-form
@@ -99,8 +113,19 @@ export function sanitizePlainTextOrNull(input: unknown): string | null {
  * and note bodies — where entity-encoding would corrupt valid input
  * (e.g. `?a=1&b=2` → `?a=1&amp;b=2`).
  *
+ * Implementation note: this is intentionally a regex-based stripper
+ * (NOT sanitize-html + entity decode). sanitize-html unconditionally
+ * HTML-escapes its output, so the only way to reverse that is to
+ * decode entities — but a decode pass also turns attacker-supplied
+ * `&lt;script&gt;` into a literal `<script>` tag in the DB. The regex
+ * approach removes real tags without ever decoding entities, so an
+ * encoded payload survives as inert text.
+ *
  * Behavior:
- * - `<script>alert(1)</script>` → "" (tag and content discarded)
+ * - `<script>alert(1)</script>` → "" (tag AND inner JS body dropped)
+ * - `<style>...</style>` / `<iframe>...</iframe>` → "" (body dropped)
+ * - `<b>bold</b> hi` → "bold hi" (inline tag dropped, text kept)
+ * - `&lt;script&gt;…&lt;/script&gt;` → unchanged (no entity decode)
  * - `https://x.test?a=1&b=2#frag` → unchanged
  * - `5 < 10 and 20 > 5` → unchanged
  *
@@ -110,17 +135,19 @@ export function stripTagsToPlainText(input: unknown): string {
   if (input === null || input === undefined) return "";
   const s = typeof input === "string" ? input : String(input);
   if (s.length === 0) return "";
-  const stripped = sanitizeHtml(s, { allowedTags: [], allowedAttributes: {} });
-  return stripped
-    .replace(/&amp;/g, "&")
-    .replace(/&lt;/g, "<")
-    .replace(/&gt;/g, ">")
-    .replace(/&quot;/g, '"')
-    .replace(/&#39;/g, "'")
-    .replace(/&#x27;/g, "'")
-    .replace(/&#x2F;/g, "/")
-    .replace(/&#96;/g, "`")
-    .replace(/&#x60;/g, "`");
+  // Pass 1: drop tags whose body is not human-readable text along with
+  // their content, so e.g. `<script>alert(1)</script>` doesn't leak the
+  // literal string `alert(1)` into the DB.
+  let out = s.replace(DANGEROUS_TAG_BODY_RE, "");
+  // Pass 2: strip remaining HTML constructs (comments, CDATA, DOCTYPE,
+  // processing instructions, and any leftover open/close tags).
+  out = out
+    .replace(HTML_COMMENT_RE, "")
+    .replace(HTML_CDATA_RE, "")
+    .replace(HTML_DOCTYPE_RE, "")
+    .replace(HTML_PROCINST_RE, "")
+    .replace(HTML_TAG_RE, "");
+  return out;
 }
 
 /**
diff --git a/artifacts/api-server/tests/executive-meetings.test.mjs b/artifacts/api-server/tests/executive-meetings.test.mjs
index aa98a185..92c01be1 100644
--- a/artifacts/api-server/tests/executive-meetings.test.mjs
+++ b/artifacts/api-server/tests/executive-meetings.test.mjs
@@ -479,6 +479,71 @@ test("Sanitization: POST strips HTML from location, meetingUrl, and notes", asyn
   );
 });
 
+// Regression guard: an earlier sanitizer used a "strip tags then decode
+// HTML entities" approach, which let a payload like `&lt;script&gt;…`
+// round-trip into the DB as a literal `<script>` tag. The current
+// implementation (regex-based, no entity decode) must keep
+// attacker-supplied entity-encoded markup as inert text.
+test("Sanitization: encoded HTML payloads (&lt;script&gt;…) are NOT decoded into live tags", async () => {
+  const create = await api(adminCookie, "POST", "/api/executive-meetings", {
+    titleAr: "ه",
+    titleEn: "H",
+    meetingDate: today,
+    location: "&lt;script&gt;alert('loc')&lt;/script&gt;Boardroom",
+    meetingUrl:
+      "&#x3C;script&#x3E;alert('url')&#x3C;/script&#x3E;https://x.test/?a=1&b=2",
+    notes: "ok &amp; safe — &lt;img onerror=alert(1) src=x&gt;",
+    attendees: [],
+  });
+  assert.equal(create.status, 201);
+  const meeting = await create.json();
+  created.meetingIds.push(meeting.id);
+
+  // Critical: NO live tag may appear in stored values, regardless of
+  // whether the client encoded their payload.
+  for (const field of ["location", "meetingUrl", "notes"]) {
+    const v = meeting[field] ?? "";
+    assert.ok(
+      !/<script|<img|<iframe|<\/script/i.test(v),
+      `${field} must not contain live HTML tags after encoded payload, got: ${v}`,
+    );
+  }
+  // The encoded payload must survive as text, exactly as the client sent
+  // it — proving we did not decode entities.
+  assert.ok(
+    (meeting.location ?? "").includes("&lt;script&gt;"),
+    `location must keep encoded entities as text, got: ${meeting.location}`,
+  );
+  assert.ok(
+    (meeting.meetingUrl ?? "").includes("&#x3C;script&#x3E;"),
+    `meetingUrl must keep encoded entities as text, got: ${meeting.meetingUrl}`,
+  );
+  assert.ok(
+    (meeting.notes ?? "").includes("&amp;") &&
+      (meeting.notes ?? "").includes("&lt;img"),
+    `notes must keep encoded entities as text, got: ${meeting.notes}`,
+  );
+
+  // Mixed-case tag names must also be stripped (regex must be /…/i).
+  const mixed = await api(adminCookie, "POST", "/api/executive-meetings", {
+    titleAr: "خ",
+    titleEn: "X",
+    meetingDate: today,
+    location: "<ScRiPt>alert(1)</ScRiPt>Boardroom B",
+    notes: "<IFRAME src=evil>x</IFRAME>visible note",
+    attendees: [],
+  });
+  assert.equal(mixed.status, 201);
+  const mm = await mixed.json();
+  created.meetingIds.push(mm.id);
+  assert.ok(
+    !/<script|<iframe/i.test((mm.location ?? "") + (mm.notes ?? "")),
+    "mixed-case dangerous tags must be stripped (regex case-insensitive)",
+  );
+  assert.ok((mm.location ?? "").includes("Boardroom B"));
+  assert.ok((mm.notes ?? "").includes("visible note"));
+});
+
 test("Sanitization: PATCH strips HTML from location, meetingUrl, and notes", async () => {
   const create = await api(adminCookie, "POST", "/api/executive-meetings", {
     titleAr: "م",