Sanitize attendee titles at the API boundary (task #130)
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.
Replit-Task-Id: 837863ea-23a1-4d26-8261-f0b7ef6e5b0f
This commit is contained in:
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user