Improve sanitization for meeting details to prevent malicious input

Introduce a new function `stripTagsToPlainTextOrNull` to sanitize location, meeting URL, and notes fields, ensuring HTML tags are removed while preserving special characters for proper URL and text rendering. This change enhances security by preventing cross-site scripting (XSS) attacks and ensures data integrity for these fields across create, update, and duplication operations. Replit-Commit-Author: Agent Replit-Commit-Session-Id: 77cfe984-2c65-4152-bb7a-0df28274fe66 Replit-Commit-Checkpoint-Type: full_checkpoint Replit-Commit-Event-Id: cee83569-351b-48ea-ade1-9ebfdd9d85eb Replit-Commit-Screenshot-Url: https://storage.googleapis.com/screenshot-production-us-central1/c3c252e4-c83d-40ca-9fff-99a3ea60701e/77cfe984-2c65-4152-bb7a-0df28274fe66/dYJU04s Replit-Helium-Checkpoint-Created: true
2026-04-30 20:58:11 +00:00
parent ed618f3e3f
commit ff220770bb
3 changed files with 350 additions and 13 deletions
@@ -91,3 +91,44 @@ export function sanitizePlainTextOrNull(input: unknown): string | null {
  const s = sanitizePlainText(input);
  return s === "" ? null : s;
 }
+
+/**
+ * Strip ALL HTML tags but PRESERVE stray special characters (`&`, `<`,
+ * `>`, quotes) as their literal plain-text form. Use for free-form
+ * fields that are NOT interpolated into HTML — URLs, location strings,
+ * and note bodies — where entity-encoding would corrupt valid input
+ * (e.g. `?a=1&b=2` → `?a=1&amp;b=2`).
+ *
+ * Behavior:
+ * - `<script>alert(1)</script>` → "" (tag and content discarded)
+ * - `https://x.test?a=1&b=2#frag` → unchanged
+ * - `5 < 10 and 20 > 5` → unchanged
+ *
+ * Empty / null / undefined input becomes "".
+ */
+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, "`");
+}
+
+/**
+ * Same as {@link stripTagsToPlainText} but preserves null for nullable
+ * DB columns (location, meetingUrl, notes).
+ */
+export function stripTagsToPlainTextOrNull(input: unknown): string | null {
+  if (input === null || input === undefined) return null;
+  const s = stripTagsToPlainText(input);
+  return s === "" ? null : s;
+}
@@ -33,7 +33,11 @@ import {
  requireExecutiveAccess,
  getEffectiveRoleIds,
 } from "../middlewares/auth";
-import { sanitizePlainTextOrNull, sanitizeRichText } from "../lib/sanitize";
+import {
+  sanitizePlainTextOrNull,
+  sanitizeRichText,
+  stripTagsToPlainTextOrNull,
+} from "../lib/sanitize";
 import {
  emitExecutiveMeetingsDayChanged,
  emitExecutiveMeetingsDaysChanged,
@@ -673,12 +677,16 @@ router.post(
          dailyNumber,
          startTime: data.startTime ?? null,
          endTime: data.endTime ?? null,
-          location: data.location ?? null,
-          meetingUrl: data.meetingUrl ?? null,
+          // location/meetingUrl/notes are plain-text fields in the UI but
+          // can receive sneaky markup from a paste; strip tags at the API
+          // boundary. Use stripTagsToPlainTextOrNull so URLs with `&` and
+          // notes with stray `<`/`>` round-trip without entity-encoding.
+          location: stripTagsToPlainTextOrNull(data.location),
+          meetingUrl: stripTagsToPlainTextOrNull(data.meetingUrl),
          platform: data.platform ?? "none",
          status: data.status ?? "scheduled",
          isHighlighted: data.isHighlighted ?? 0,
-          notes: data.notes ?? null,
+          notes: stripTagsToPlainTextOrNull(data.notes),
          createdBy: userId,
          updatedBy: userId,
        };
@@ -783,13 +791,19 @@ router.patch(
          updateValues.dailyNumber = data.dailyNumber;
        if (data.startTime !== undefined) updateValues.startTime = data.startTime;
        if (data.endTime !== undefined) updateValues.endTime = data.endTime;
-        if (data.location !== undefined) updateValues.location = data.location;
-        if (data.meetingUrl !== undefined) updateValues.meetingUrl = data.meetingUrl;
+        // Plain-text fields: strip any HTML at the API boundary so a
+        // PATCH cannot smuggle markup that the POST path already rejects.
+        // Mirrors the create handler above and the attendee.title path.
+        if (data.location !== undefined)
+          updateValues.location = stripTagsToPlainTextOrNull(data.location);
+        if (data.meetingUrl !== undefined)
+          updateValues.meetingUrl = stripTagsToPlainTextOrNull(data.meetingUrl);
        if (data.platform !== undefined) updateValues.platform = data.platform;
        if (data.status !== undefined) updateValues.status = data.status;
        if (data.isHighlighted !== undefined)
          updateValues.isHighlighted = data.isHighlighted;
-        if (data.notes !== undefined) updateValues.notes = data.notes;
+        if (data.notes !== undefined)
+          updateValues.notes = stripTagsToPlainTextOrNull(data.notes);
        if (data.merge !== undefined) {
          if (data.merge === null) {
            updateValues.mergeStartColumn = null;
@@ -984,12 +998,14 @@ router.post(
          meetingDate: data.targetDate,
          startTime: source.startTime,
          endTime: source.endTime,
-          location: source.location,
-          meetingUrl: source.meetingUrl,
+          // Re-sanitize plain-text fields on duplicate for the same
+          // defense-in-depth reason as the rich-text fields above.
+          location: stripTagsToPlainTextOrNull(source.location),
+          meetingUrl: stripTagsToPlainTextOrNull(source.meetingUrl),
          platform: source.platform,
          status: "scheduled",
          isHighlighted: 0,
-          notes: source.notes,
+          notes: stripTagsToPlainTextOrNull(source.notes),
          attachments: source.attachments,
          createdBy: userId,
          updatedBy: userId,
@@ -1221,10 +1237,17 @@ async function applyApprovedRequest(
      break;
    }
    case "change_location": {
+      // Same plain-text sanitization boundary as the direct meeting
+      // POST/PATCH paths so an approved request cannot smuggle markup
+      // into location/meetingUrl.
      if (typeof details.location === "string")
-        update.location = details.location.slice(0, 300);
+        update.location = stripTagsToPlainTextOrNull(
+          details.location.slice(0, 300),
+        );
      if (typeof details.meetingUrl === "string")
-        update.meetingUrl = details.meetingUrl.slice(0, 1000);
+        update.meetingUrl = stripTagsToPlainTextOrNull(
+          details.meetingUrl.slice(0, 1000),
+        );
      if (
        typeof details.platform === "string" &&
        (PLATFORMS as readonly string[]).includes(details.platform)
@@ -1242,7 +1265,13 @@ async function applyApprovedRequest(
      update.isHighlighted = 0;
      break;
    case "note": {
-      const newNote = typeof details.note === "string" ? details.note.slice(0, 5000) : "";
+      // Same plain-text sanitization as the direct notes write paths so
+      // an approved request cannot smuggle markup into the appended note.
+      const sanitized =
+        typeof details.note === "string"
+          ? stripTagsToPlainTextOrNull(details.note.slice(0, 5000))
+          : null;
+      const newNote = sanitized ?? "";
      const prev = existing.notes ?? "";
      update.notes = prev ? `${prev}\n${newNote}` : newNote;
      break;
@@ -418,6 +418,273 @@ test("Meetings: POST /duplicate clones a meeting onto another date", async () =>
  assert.ok(dayBody.meetings.some((m) => m.id === newRow.id));
 });

+// --- Plain-text sanitization for location / meetingUrl / notes (#189) ----
+// Mirrors the existing attendee-title sanitization so a meeting payload
+// cannot smuggle <script> / <img onerror=...> markup into fields that
+// templates (PDF renderer, audit-log popovers) interpolate as HTML.
+test("Sanitization: POST strips HTML from location, meetingUrl, and notes", async () => {
+  const create = await api(adminCookie, "POST", "/api/executive-meetings", {
+    titleAr: "ل",
+    titleEn: "L",
+    meetingDate: today,
+    location: '<script>alert("xss")</script>Conference Room A',
+    // URL with query params + fragment + ampersand — must round-trip
+    // unchanged after tag removal so `meet?a=1&b=2` doesn't get stored
+    // as `meet?a=1&amp;b=2`.
+    meetingUrl:
+      '<img src=x onerror=alert(1)>https://example.com/meet?a=1&b=2#room',
+    // Notes with stray `<` (math-style) plus a real script tag — the
+    // script must be stripped, the `5 < 10` text must survive intact.
+    notes: '<b>bold</b> note: 5 < 10 & ok <script>bad()</script>',
+    attendees: [],
+  });
+  assert.equal(create.status, 201);
+  const meeting = await create.json();
+  created.meetingIds.push(meeting.id);
+
+  assert.ok(
+    !/<script/i.test(meeting.location ?? ""),
+    `location should not contain <script>, got: ${meeting.location}`,
+  );
+  assert.ok(
+    (meeting.location ?? "").includes("Conference Room A"),
+    `location should preserve plain text, got: ${meeting.location}`,
+  );
+  assert.ok(
+    !/<img|onerror/i.test(meeting.meetingUrl ?? ""),
+    `meetingUrl should not contain <img/onerror, got: ${meeting.meetingUrl}`,
+  );
+  // Round-trip: `&` must NOT be entity-encoded.
+  assert.equal(
+    meeting.meetingUrl,
+    "https://example.com/meet?a=1&b=2#room",
+    `meetingUrl must round-trip ampersands and fragments, got: ${meeting.meetingUrl}`,
+  );
+  assert.ok(
+    !/<script|<b>/i.test(meeting.notes ?? ""),
+    `notes should strip both <b> and <script>, got: ${meeting.notes}`,
+  );
+  // Round-trip: stray `<` and `&` in plain prose must survive as literals.
+  assert.ok(
+    (meeting.notes ?? "").includes("5 < 10"),
+    `notes must preserve literal '<', got: ${meeting.notes}`,
+  );
+  assert.ok(
+    (meeting.notes ?? "").includes("& ok"),
+    `notes must preserve literal '&', got: ${meeting.notes}`,
+  );
+  assert.ok(
+    !/&amp;|&lt;|&gt;/.test(meeting.notes ?? ""),
+    `notes must NOT be HTML-entity-encoded, got: ${meeting.notes}`,
+  );
+});
+
+test("Sanitization: PATCH strips HTML from location, meetingUrl, and notes", async () => {
+  const create = await api(adminCookie, "POST", "/api/executive-meetings", {
+    titleAr: "م",
+    titleEn: "M",
+    meetingDate: today,
+    location: "Original",
+    notes: "Original note",
+    attendees: [],
+  });
+  assert.equal(create.status, 201);
+  const meeting = await create.json();
+  created.meetingIds.push(meeting.id);
+
+  const patch = await api(
+    adminCookie,
+    "PATCH",
+    `/api/executive-meetings/${meeting.id}`,
+    {
+      location: '<script>steal()</script>Updated Room',
+      meetingUrl: '<a href="javascript:bad()">https://meet.example.com</a>',
+      notes: '<iframe src="x"></iframe>Updated note',
+    },
+  );
+  assert.equal(patch.status, 200);
+
+  const fetched = await api(
+    adminCookie,
+    "GET",
+    `/api/executive-meetings/${meeting.id}`,
+  );
+  const body = await fetched.json();
+  assert.ok(!/<script/i.test(body.location ?? ""));
+  assert.ok((body.location ?? "").includes("Updated Room"));
+  assert.ok(!/<a |<iframe|javascript:/i.test(body.meetingUrl ?? ""));
+  assert.ok((body.meetingUrl ?? "").includes("https://meet.example.com"));
+  assert.ok(!/<iframe/i.test(body.notes ?? ""));
+  assert.ok((body.notes ?? "").includes("Updated note"));
+});
+
+// --- EditableCell column independence (#202) -----------------------------
+// The schedule's inline title editor decides which DB column to write
+// based on the active i18n locale (titleAr in ar, titleEn in en). The
+// server contract that powers this must let either column be patched
+// independently — patching titleEn must not clobber titleAr (and vice
+// versa). This test locks down that contract so a future server-side
+// regression that writes both columns from one payload fails loudly.
+test("EditableCell contract: PATCH titleEn alone leaves titleAr untouched (and vice versa)", async () => {
+  const create = await api(adminCookie, "POST", "/api/executive-meetings", {
+    titleAr: "العنوان الأصلي",
+    titleEn: "Original Title",
+    meetingDate: today,
+    attendees: [],
+  });
+  assert.equal(create.status, 201);
+  const meeting = await create.json();
+  created.meetingIds.push(meeting.id);
+
+  // Mimic an `ar`-pref admin who switched UI to `en` and edited the
+  // English title cell — saveTitle sends only titleEn.
+  const patchEn = await api(
+    adminCookie,
+    "PATCH",
+    `/api/executive-meetings/${meeting.id}`,
+    { titleEn: "<p>Edited English</p>" },
+  );
+  assert.equal(patchEn.status, 200);
+
+  let fetched = await api(
+    adminCookie,
+    "GET",
+    `/api/executive-meetings/${meeting.id}`,
+  );
+  let body = await fetched.json();
+  assert.ok(
+    body.titleEn.includes("Edited English"),
+    `titleEn should be updated, got: ${body.titleEn}`,
+  );
+  assert.ok(
+    body.titleAr.includes("العنوان الأصلي"),
+    `titleAr must NOT be clobbered by a titleEn-only PATCH, got: ${body.titleAr}`,
+  );
+
+  // Mirror: editing the Arabic cell must not clobber the (just-edited)
+  // English title.
+  const patchAr = await api(
+    adminCookie,
+    "PATCH",
+    `/api/executive-meetings/${meeting.id}`,
+    { titleAr: "<p>عنوان معدل</p>" },
+  );
+  assert.equal(patchAr.status, 200);
+
+  fetched = await api(
+    adminCookie,
+    "GET",
+    `/api/executive-meetings/${meeting.id}`,
+  );
+  body = await fetched.json();
+  assert.ok(body.titleAr.includes("عنوان معدل"));
+  assert.ok(
+    body.titleEn.includes("Edited English"),
+    `titleEn must NOT be clobbered by a titleAr-only PATCH, got: ${body.titleEn}`,
+  );
+});
+
+// Locks down the two indirect write paths for plain-text fields so a
+// future regression in either path (forgetting the sanitizer, or
+// swapping it back to one that entity-encodes) fails loudly.
+test("Sanitization: duplicate path re-sanitizes location/meetingUrl/notes and preserves URL ampersands", async () => {
+  const create = await api(adminCookie, "POST", "/api/executive-meetings", {
+    titleAr: "د",
+    titleEn: "D",
+    meetingDate: today,
+    location: "Boardroom 7",
+    meetingUrl: "https://meet.example.com/x?room=42&token=abc",
+    notes: "Bring slides & notes",
+    attendees: [],
+  });
+  assert.equal(create.status, 201);
+  const original = await create.json();
+  created.meetingIds.push(original.id);
+
+  const dup = await api(
+    adminCookie,
+    "POST",
+    `/api/executive-meetings/${original.id}/duplicate`,
+    { targetDate: tomorrow },
+  );
+  assert.equal(dup.status, 201);
+  const cloned = await dup.json();
+  created.meetingIds.push(cloned.id);
+
+  assert.equal(
+    cloned.meetingUrl,
+    "https://meet.example.com/x?room=42&token=abc",
+    `duplicate must round-trip URL ampersands, got: ${cloned.meetingUrl}`,
+  );
+  assert.ok(
+    (cloned.notes ?? "").includes("&"),
+    `duplicate must preserve literal '&' in notes, got: ${cloned.notes}`,
+  );
+  assert.ok(
+    !/&amp;/.test(cloned.meetingUrl ?? "") && !/&amp;/.test(cloned.notes ?? ""),
+    `duplicate must not entity-encode ampersands`,
+  );
+});
+
+test("Sanitization: change_location approved request round-trips URL ampersands and strips tags", async () => {
+  const create = await api(adminCookie, "POST", "/api/executive-meetings", {
+    titleAr: "ج",
+    titleEn: "J",
+    meetingDate: today,
+    location: "Old Room",
+    meetingUrl: null,
+    attendees: [],
+  });
+  assert.equal(create.status, 201);
+  const meeting = await create.json();
+  created.meetingIds.push(meeting.id);
+
+  const reqRes = await api(
+    adminCookie,
+    "POST",
+    "/api/executive-meetings/requests",
+    {
+      requestType: "change_location",
+      meetingId: meeting.id,
+      requestDetails: {
+        location: '<script>x()</script>New Room (Bldg A & B)',
+        meetingUrl: 'https://x.test/join?id=99&utm=ar',
+      },
+    },
+  );
+  assert.equal(reqRes.status, 201);
+  const reqRow = await reqRes.json();
+  created.requestIds.push(reqRow.id);
+
+  const approve = await api(
+    adminCookie,
+    "PATCH",
+    `/api/executive-meetings/requests/${reqRow.id}`,
+    { status: "approved", reviewNotes: "ok" },
+  );
+  assert.equal(approve.status, 200);
+
+  const fetched = await api(
+    adminCookie,
+    "GET",
+    `/api/executive-meetings/${meeting.id}`,
+  );
+  const body = await fetched.json();
+  assert.ok(
+    !/<script/i.test(body.location ?? ""),
+    `change_location must strip <script>, got: ${body.location}`,
+  );
+  assert.ok(
+    (body.location ?? "").includes("New Room (Bldg A & B)"),
+    `change_location must preserve literal '&' in location, got: ${body.location}`,
+  );
+  assert.equal(
+    body.meetingUrl,
+    "https://x.test/join?id=99&utm=ar",
+    `change_location must round-trip URL ampersands, got: ${body.meetingUrl}`,
+  );
+});
+
 test("Requests: POST → admin can list", async () => {
  const create = await api(
    adminCookie,