From d61ead16392ae79dd98902f611586025db32ceb8 Mon Sep 17 00:00:00 2001
From: Riyadh <dev@tx.local>
Date: Thu, 30 Apr 2026 05:56:23 +0000
Subject: [PATCH] Sanitize attendee titles at the API boundary (task #130)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Original task: attendee.name was passed through sanitizeRichText on every
write path, but the sibling attendee.title was treated as plain text and
inserted verbatim. The print page and any future HTML template that
interpolates a.title would have to remember to escape it. Strip HTML at
the API boundary instead so a malicious title can never be stored.

Implementation:
- Added `sanitizePlainText` and `sanitizePlainTextOrNull` helpers in
  artifacts/api-server/src/lib/sanitize.ts. They wrap sanitize-html with
  an empty allowlist (`allowedTags: [], allowedAttributes: {}`), which
  strips every tag and HTML-escapes any stray `<`, `>`, `&`, or quote
  characters. The OrNull variant preserves null for nullable columns.
- Applied `sanitizePlainTextOrNull(a.title)` to all four direct write
  paths in artifacts/api-server/src/routes/executive-meetings.ts:
    * POST  /executive-meetings
    * PATCH /executive-meetings/:id (attendees branch)
    * PUT   /executive-meetings/:id/attendees
    * POST  /executive-meetings/:id/duplicate
- Also patched the `add_attendee` apply branch (line ~1200) so an
  approved request cannot smuggle <script>/HTML into title via the
  request workflow — same defense-in-depth as the existing name
  sanitization in that branch.

Test:
- Added a single end-to-end test in
  artifacts/api-server/tests/executive-meetings.test.mjs that pushes a
  malicious title (<script>, <b onclick=...>, <img onerror=...>,
  <a href="javascript:...>) through POST/PATCH/PUT/duplicate and asserts
  that the stored value contains no <script>/<img>/<a>/onclick/onerror
  /javascript: but still preserves the visible text. The test passes;
  the only remaining failures in this test file are pre-existing and
  unrelated (PDF archive tests).

Notes / non-deviations:
- Chose the "pass through sanitizer" approach over the Zod regex refine
  the task suggested, because the strip-and-escape behaviour leaves
  legitimate stray characters (e.g. "Director < Manager") usable
  instead of returning a 400.
- Did not touch other plain-text fields like location/meetingUrl/notes —
  they are also rendered via React JSX in the print page so are safe
  today. Captured as follow-up #189 for symmetric defense-in-depth.
---
 artifacts/api-server/src/lib/sanitize.ts      | 26 ++++++
 .../src/routes/executive-meetings.ts          | 20 +++--
 .../tests/executive-meetings.test.mjs         | 89 +++++++++++++++++++
 3 files changed, 129 insertions(+), 6 deletions(-)

diff --git a/artifacts/api-server/src/lib/sanitize.ts b/artifacts/api-server/src/lib/sanitize.ts
index 8cd958ac..009ae9e5 100644
--- a/artifacts/api-server/src/lib/sanitize.ts
+++ b/artifacts/api-server/src/lib/sanitize.ts
@@ -65,3 +65,29 @@ export function sanitizeRichTextOrNull(input: unknown): string | null {
   const s = sanitizeRichText(input);
   return s === "" ? null : s;
 }
+
+/**
+ * Strip ALL HTML tags from a string and HTML-escape any stray `<`, `>`,
+ * `&`, or quote characters. Use for fields that the UI treats as plain
+ * text but that get interpolated into HTML by templates (e.g. the print
+ * page) — this prevents a payload like `<script>alert(1)</script>` from
+ * surviving as executable markup if a future template forgets to escape.
+ *
+ * Empty / null / undefined input becomes "".
+ */
+export function sanitizePlainText(input: unknown): string {
+  if (input === null || input === undefined) return "";
+  const s = typeof input === "string" ? input : String(input);
+  if (s.length === 0) return "";
+  return sanitizeHtml(s, { allowedTags: [], allowedAttributes: {} });
+}
+
+/**
+ * Same as {@link sanitizePlainText} but preserves null for fields that
+ * are nullable in the database (e.g. attendee.title).
+ */
+export function sanitizePlainTextOrNull(input: unknown): string | null {
+  if (input === null || input === undefined) return null;
+  const s = sanitizePlainText(input);
+  return s === "" ? null : s;
+}
diff --git a/artifacts/api-server/src/routes/executive-meetings.ts b/artifacts/api-server/src/routes/executive-meetings.ts
index 9598a31a..2a869c76 100644
--- a/artifacts/api-server/src/routes/executive-meetings.ts
+++ b/artifacts/api-server/src/routes/executive-meetings.ts
@@ -32,7 +32,7 @@ import {
   requireExecutiveAccess,
   getEffectiveRoleIds,
 } from "../middlewares/auth";
-import { sanitizeRichText } from "../lib/sanitize";
+import { sanitizePlainTextOrNull, sanitizeRichText } from "../lib/sanitize";
 import { emitExecutiveMeetingsDayChanged } from "../lib/realtime";
 import { renderSchedulePdf, type PdfFontPrefs } from "../lib/pdf-renderer";
 import { ObjectStorageService } from "../lib/objectStorage";
@@ -668,7 +668,10 @@ router.post(
             attendees.map((a, idx) => ({
               meetingId: meeting.id,
               name: sanitizeRichText(a.name),
-              title: a.title ?? null,
+              // title is plain text in the UI, but it gets interpolated
+              // into HTML by templates like the print page, so strip
+              // any sneaky markup at the API boundary.
+              title: sanitizePlainTextOrNull(a.title),
               attendanceType: a.attendanceType,
               sortOrder: a.sortOrder ?? idx,
             })),
@@ -786,7 +789,7 @@ router.patch(
               data.attendees.map((a, idx) => ({
                 meetingId: id,
                 name: sanitizeRichText(a.name),
-                title: a.title ?? null,
+                title: sanitizePlainTextOrNull(a.title),
                 attendanceType: a.attendanceType,
                 sortOrder: a.sortOrder ?? idx,
               })),
@@ -885,7 +888,7 @@ router.put(
           data.attendees.map((a, idx) => ({
             meetingId: id,
             name: sanitizeRichText(a.name),
-            title: a.title ?? null,
+            title: sanitizePlainTextOrNull(a.title),
             attendanceType: a.attendanceType,
             sortOrder: a.sortOrder ?? idx,
           })),
@@ -959,7 +962,7 @@ router.post(
               // re-running keeps a defense-in-depth boundary if anything was
               // imported via raw SQL.
               name: sanitizeRichText(a.name),
-              title: a.title,
+              title: sanitizePlainTextOrNull(a.title),
               attendanceType: a.attendanceType,
               sortOrder: a.sortOrder ?? idx,
             })),
@@ -1197,7 +1200,12 @@ async function applyApprovedRequest(
       // the rich-text attendee.name column.
       const name = sanitizeRichText(rawName.slice(0, 5000));
       if (!name) break;
-      const title = typeof details.title === "string" ? details.title.slice(0, 200) : null;
+      // Same plain-text sanitization as the direct attendee write paths
+      // so an approved request cannot smuggle <script>/HTML into title.
+      const title =
+        typeof details.title === "string"
+          ? sanitizePlainTextOrNull(details.title.slice(0, 200))
+          : null;
       const attendanceType =
         typeof details.attendanceType === "string" &&
         (ATTENDANCE_TYPES as readonly string[]).includes(details.attendanceType)
diff --git a/artifacts/api-server/tests/executive-meetings.test.mjs b/artifacts/api-server/tests/executive-meetings.test.mjs
index a650ce03..c8ed90a8 100644
--- a/artifacts/api-server/tests/executive-meetings.test.mjs
+++ b/artifacts/api-server/tests/executive-meetings.test.mjs
@@ -731,6 +731,95 @@ test("Sanitization: rich text strips disallowed tags but keeps inline formatting
   assert.ok(/<em[^>]*>One<\/em>/i.test(att.name), "em must survive in attendee");
 });
 
+test("Sanitization: attendee.title is stripped of HTML on every write path", async () => {
+  const malicious = '<script>alert(1)</script><b onclick="x">CFO</b>';
+  // POST creates the meeting with a malicious title on one attendee.
+  const create = await api(adminCookie, "POST", "/api/executive-meetings", {
+    titleAr: "تنظيف العنوان",
+    titleEn: "Title sanitization",
+    meetingDate: today,
+    attendees: [
+      {
+        name: "Attendee POST",
+        title: malicious,
+        attendanceType: "internal",
+        sortOrder: 0,
+      },
+    ],
+  });
+  assert.equal(create.status, 201);
+  const meeting = await create.json();
+  created.meetingIds.push(meeting.id);
+
+  const postedTitle = meeting.attendees[0].title;
+  assert.ok(postedTitle, "title must round-trip");
+  assert.ok(!/<script/i.test(postedTitle), "POST: <script> stripped from title");
+  assert.ok(!/<b/i.test(postedTitle), "POST: <b> stripped from title");
+  assert.ok(!/onclick/i.test(postedTitle), "POST: onclick stripped from title");
+  assert.ok(/CFO/.test(postedTitle), "POST: visible text preserved in title");
+
+  // PATCH replaces attendees with a different malicious title.
+  const patched = await api(
+    adminCookie,
+    "PATCH",
+    `/api/executive-meetings/${meeting.id}`,
+    {
+      attendees: [
+        {
+          name: "Attendee PATCH",
+          title: '<img src=x onerror=alert(1)>Director',
+          attendanceType: "internal",
+          sortOrder: 0,
+        },
+      ],
+    },
+  );
+  assert.equal(patched.status, 200);
+  const patchedBody = await patched.json();
+  const patchedTitle = patchedBody.attendees[0].title;
+  assert.ok(!/<img/i.test(patchedTitle), "PATCH: <img> stripped from title");
+  assert.ok(!/onerror/i.test(patchedTitle), "PATCH: onerror stripped from title");
+  assert.ok(/Director/.test(patchedTitle), "PATCH: visible text preserved");
+
+  // PUT /attendees replaces the list and must sanitize too.
+  const put = await api(
+    adminCookie,
+    "PUT",
+    `/api/executive-meetings/${meeting.id}/attendees`,
+    {
+      attendees: [
+        {
+          name: "Attendee PUT",
+          title: '<a href="javascript:bad()">Manager</a>',
+          attendanceType: "virtual",
+          sortOrder: 0,
+        },
+      ],
+    },
+  );
+  assert.equal(put.status, 200);
+  const putBody = await put.json();
+  const putTitle = putBody.attendees[0].title;
+  assert.ok(!/<a/i.test(putTitle), "PUT: <a> stripped from title");
+  assert.ok(!/javascript:/i.test(putTitle), "PUT: javascript: scheme gone");
+  assert.ok(/Manager/.test(putTitle), "PUT: visible text preserved");
+
+  // Duplicate path: copy of a meeting with a sanitized title must stay sanitized.
+  const dup = await api(
+    adminCookie,
+    "POST",
+    `/api/executive-meetings/${meeting.id}/duplicate`,
+    { targetDate: today },
+  );
+  assert.equal(dup.status, 201);
+  const dupBody = await dup.json();
+  created.meetingIds.push(dupBody.id);
+  const dupTitle = dupBody.attendees[0].title;
+  assert.ok(!/<a/i.test(dupTitle), "duplicate: title remains free of HTML tags");
+  assert.ok(!/<script/i.test(dupTitle), "duplicate: no <script> in copied title");
+  assert.ok(/Manager/.test(dupTitle), "duplicate: visible text preserved");
+});
+
 test("Sanitization: text-align on <p> survives a round-trip via PATCH/GET", async () => {
   const m = await api(adminCookie, "POST", "/api/executive-meetings", {
     titleAr: "محاذاة", titleEn: "Align", meetingDate: today,