fix(executive-meetings): lock down attendee save payload (#221)

Why:
Task #220 added a client-only `_sid` field on attendee rows so React
DnD can identify rows. The two client save sites already enumerate
wire fields explicitly and never serialize `_sid`, but nothing
prevents a future refactor from accidentally leaking it. The server
was zod-default lenient (silently strips unknowns), so a regression
would either be silently absorbed (bad — silent contract drift) or
land in a future JSONB metadata column without anyone noticing.

What changed:
- `attendeeSchema` in artifacts/api-server/src/routes/executive-meetings.ts
  is now `.strict()`. Any unknown attendee key (including `_sid` or
  any future client-only field) is rejected with HTTP 400 instead of
  being silently stripped. The schema is reused by all three
  attendee-bearing endpoints (POST /executive-meetings,
  PATCH /executive-meetings/:id, PUT /executive-meetings/:id/attendees),
  so all three are covered by one change.

Tests:
- Added three API tests in
  artifacts/api-server/tests/executive-meetings.test.mjs:
  1. POST /executive-meetings with attendee carrying `_sid` returns
     400 and the error mentions the rejected key.
  2. PATCH /executive-meetings/:id with attendees carrying `_sid`
     returns 400 AND the meeting's existing attendee list is
     preserved (no partial mutation).
  3. PUT /executive-meetings/:id/attendees with attendee carrying
     `_sid` returns 400 AND the seeded attendee is unchanged.

Verification:
- Full API test suite: 207/207 green (was 204/204 before; +3 new).
- No client-side change needed: existing `saveAttendeeName` (~L943
  in artifacts/tx-os/src/pages/executive-meetings.tsx) and the
  manage-dialog save (~L4004) already project to the documented
  wire shape (`name, title, attendanceType, sortOrder, kind`).
- Architect review: addressed the one gap (PATCH coverage) by
  adding test #2 above; verdict resolved.

Out of scope cleanup:
- Marked the descriptions of stale tasks #172 and #179 as STALE
  (both PDF tests pass and PDF export works in current main).
  Final cancellation left to the user.
This commit is contained in:
riyadhafraa
2026-04-30 19:16:00 +00:00
parent fb2d75ecd7
commit 1b40d7dc00
2 changed files with 142 additions and 12 deletions
@@ -203,18 +203,25 @@ const positiveIntSchema = z.number().int().positive();
// output). The byte count grows quickly with inline <span style=...> wrappers,
// so we allow up to ~10k bytes for titles and ~5k for attendee names. The
// sanitizer strips disallowed tags/attrs before persistence.
const attendeeSchema = z.object({
// For persons this is sanitized rich-text HTML (Tiptap output).
// For subheadings this is the user's free-text label, also passed
// through sanitizeRichText to strip any sneaky markup. Both kinds
// require at least one non-whitespace character; an empty string is
// a delete-row gesture in the inline editor and must never reach the DB.
name: z.string().trim().min(1).max(5000),
title: z.string().trim().max(200).nullable().optional(),
attendanceType: z.enum(ATTENDANCE_TYPES).default("internal"),
sortOrder: z.number().int().min(0).optional(),
kind: z.enum(ATTENDEE_KINDS).default("person"),
});
const attendeeSchema = z
.object({
// For persons this is sanitized rich-text HTML (Tiptap output).
// For subheadings this is the user's free-text label, also passed
// through sanitizeRichText to strip any sneaky markup. Both kinds
// require at least one non-whitespace character; an empty string is
// a delete-row gesture in the inline editor and must never reach the DB.
name: z.string().trim().min(1).max(5000),
title: z.string().trim().max(200).nullable().optional(),
attendanceType: z.enum(ATTENDANCE_TYPES).default("internal"),
sortOrder: z.number().int().min(0).optional(),
kind: z.enum(ATTENDEE_KINDS).default("person"),
})
// `.strict()` so any client-only field (e.g. the React `_sid` row id used
// by the drag-and-drop editor) that accidentally leaks into the wire
// payload is rejected loudly with 400 instead of being silently stripped.
// The browser always projects to the documented wire shape; this is the
// belt-and-suspenders guard against a future refactor regression.
.strict();
const meetingBaseFields = {
titleAr: z.string().trim().min(1).max(10000),
@@ -264,6 +264,129 @@ test("Meetings: PUT /attendees replaces the attendee list", async () => {
assert.ok(!body.attendees.find((a) => a.name === "Old One"));
});
// --- Attendee payload contract (Task #221) ---------------------------------
// Server is `.strict()` on the attendee schema so client-only fields like
// `_sid` (the React row id used by the drag-and-drop editor) are rejected
// loudly with 400 instead of being silently stripped. This locks down the
// wire contract so a future client refactor that accidentally serializes
// `_sid` (or any other internal field) breaks tests immediately rather
// than being absorbed by lenient parsing.
test("Attendee payload contract: POST rejects unknown fields like `_sid`", async () => {
const create = await api(adminCookie, "POST", "/api/executive-meetings", {
titleAr: "س",
titleEn: "S",
meetingDate: today,
attendees: [
{
name: "Leaky One",
attendanceType: "internal",
sortOrder: 0,
_sid: "client-only-row-id",
},
],
});
assert.equal(
create.status,
400,
`expected 400 when attendee carries _sid, got ${create.status}`,
);
const body = await create.json();
// Zod surfaces the rejected key in the error path.
const serialized = JSON.stringify(body);
assert.ok(
serialized.includes("_sid") || serialized.toLowerCase().includes("unrecognized"),
`expected error to mention the rejected key, got: ${serialized}`,
);
});
test("Attendee payload contract: PATCH /:id rejects unknown attendee fields like `_sid`", async () => {
const create = await api(adminCookie, "POST", "/api/executive-meetings", {
titleAr: "ت",
titleEn: "T",
meetingDate: today,
attendees: [{ name: "Patch Seed", attendanceType: "internal", sortOrder: 0 }],
});
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}`,
{
attendees: [
{
name: "Leaky Three",
attendanceType: "internal",
sortOrder: 0,
_sid: "client-only-row-id",
},
],
},
);
assert.equal(
patch.status,
400,
`expected 400 when PATCH attendee carries _sid, got ${patch.status}`,
);
// The seed attendee must still be the only one stored — the rejected
// PATCH must not have replaced the attendee list.
const fetched = await api(
adminCookie,
"GET",
`/api/executive-meetings/${meeting.id}`,
);
const body = await fetched.json();
assert.equal(body.attendees.length, 1);
assert.equal(body.attendees[0].name, "Patch Seed");
});
test("Attendee payload contract: PUT /attendees rejects unknown fields like `_sid`", async () => {
const create = await api(adminCookie, "POST", "/api/executive-meetings", {
titleAr: "ش",
titleEn: "Sh",
meetingDate: today,
attendees: [{ name: "Seed", attendanceType: "internal", sortOrder: 0 }],
});
assert.equal(create.status, 201);
const meeting = await create.json();
created.meetingIds.push(meeting.id);
const replace = await api(
adminCookie,
"PUT",
`/api/executive-meetings/${meeting.id}/attendees`,
{
attendees: [
{
name: "Leaky Two",
attendanceType: "internal",
sortOrder: 0,
_sid: "client-only-row-id",
},
],
},
);
assert.equal(
replace.status,
400,
`expected 400 when PUT attendee carries _sid, got ${replace.status}`,
);
// And the meeting's attendee list must not have changed (the seed row
// still wins, the leaky row never persists).
const fetched = await api(
adminCookie,
"GET",
`/api/executive-meetings/${meeting.id}`,
);
const body = await fetched.json();
assert.equal(body.attendees.length, 1);
assert.equal(body.attendees[0].name, "Seed");
});
test("Meetings: POST /duplicate clones a meeting onto another date", async () => {
const create = await api(adminCookie, "POST", "/api/executive-meetings", {
titleAr: "ب",