Refine text sanitization and update test descriptions

Improve text sanitization logic and update comments in test files to be more concise and informative.

Replit-Commit-Author: Agent
Replit-Commit-Session-Id: 77cfe984-2c65-4152-bb7a-0df28274fe66
Replit-Commit-Checkpoint-Type: full_checkpoint
Replit-Commit-Event-Id: 316e780f-e839-41c5-9826-be64a0fe9d70
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
This commit is contained in:
riyadhafraa
2026-04-30 23:31:57 +00:00
parent ca727c3759
commit 3b35c29b9f
4 changed files with 36 additions and 178 deletions
@@ -418,23 +418,15 @@ 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.
// #189: plain-text sanitization for location / meetingUrl / notes.
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: [],
});
@@ -442,48 +434,17 @@ test("Sanitization: POST strips HTML from location, meetingUrl, and notes", asyn
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}`,
);
assert.ok(!/<script/i.test(meeting.location ?? ""));
assert.ok((meeting.location ?? "").includes("Conference Room A"));
assert.ok(!/<img|onerror/i.test(meeting.meetingUrl ?? ""));
assert.equal(meeting.meetingUrl, "https://example.com/meet?a=1&b=2#room");
assert.ok(!/<script|<b>/i.test(meeting.notes ?? ""));
assert.ok((meeting.notes ?? "").includes("5 < 10"));
assert.ok((meeting.notes ?? "").includes("& ok"));
assert.ok(!/&amp;|&lt;|&gt;/.test(meeting.notes ?? ""));
});
// 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.
// Regression: encoded payloads must NOT be decoded back into live tags.
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: "ه",
@@ -499,32 +460,17 @@ test("Sanitization: encoded HTML payloads (&lt;script&gt;…) are NOT decoded in
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}`,
);
assert.ok(!/<script|<img|<iframe|<\/script/i.test(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.location ?? "").includes("&lt;script&gt;"));
assert.ok((meeting.meetingUrl ?? "").includes("&#x3C;script&#x3E;"));
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",
@@ -536,10 +482,7 @@ test("Sanitization: encoded HTML payloads (&lt;script&gt;…) are NOT decoded in
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(!/<script|<iframe/i.test((mm.location ?? "") + (mm.notes ?? "")));
assert.ok((mm.location ?? "").includes("Boardroom B"));
assert.ok((mm.notes ?? "").includes("visible note"));
});
@@ -583,13 +526,9 @@ test("Sanitization: PATCH strips HTML from location, meetingUrl, and notes", asy
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.
// #202: titleEn / titleAr are independent columns; saveTitle in
// executive-meetings.tsx routes by visible i18n.language, so a PATCH
// for one column must never clobber the other.
test("EditableCell contract: PATCH titleEn alone leaves titleAr untouched (and vice versa)", async () => {
const create = await api(adminCookie, "POST", "/api/executive-meetings", {
titleAr: "العنوان الأصلي",
@@ -601,8 +540,6 @@ test("EditableCell contract: PATCH titleEn alone leaves titleAr untouched (and v
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",
@@ -617,17 +554,9 @@ test("EditableCell contract: PATCH titleEn alone leaves titleAr untouched (and v
`/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}`,
);
assert.ok(body.titleEn.includes("Edited English"));
assert.ok(body.titleAr.includes("العنوان الأصلي"));
// Mirror: editing the Arabic cell must not clobber the (just-edited)
// English title.
const patchAr = await api(
adminCookie,
"PATCH",
@@ -643,15 +572,9 @@ test("EditableCell contract: PATCH titleEn alone leaves titleAr untouched (and v
);
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}`,
);
assert.ok(body.titleEn.includes("Edited English"));
});
// 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: "د",
@@ -676,18 +599,10 @@ test("Sanitization: duplicate path re-sanitizes location/meetingUrl/notes and pr
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.equal(cloned.meetingUrl, "https://meet.example.com/x?room=42&token=abc");
assert.ok((cloned.notes ?? "").includes("&"));
assert.ok(
!/&amp;/.test(cloned.meetingUrl ?? "") && !/&amp;/.test(cloned.notes ?? ""),
`duplicate must not entity-encode ampersands`,
);
});
@@ -735,19 +650,9 @@ test("Sanitization: change_location approved request round-trips URL ampersands
`/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}`,
);
assert.ok(!/<script/i.test(body.location ?? ""));
assert.ok((body.location ?? "").includes("New Room (Bldg A & B)"));
assert.equal(body.meetingUrl, "https://x.test/join?id=99&utm=ar");
});
test("Requests: POST → admin can list", async () => {