diff --git a/artifacts/api-server/src/routes/executive-meetings.ts b/artifacts/api-server/src/routes/executive-meetings.ts index 7f57586d..99e5ffff 100644 --- a/artifacts/api-server/src/routes/executive-meetings.ts +++ b/artifacts/api-server/src/routes/executive-meetings.ts @@ -25,6 +25,7 @@ import { executiveMeetingAlertStateTable, rolesTable, usersTable, + EXECUTIVE_MEETING_ROW_COLOR_KEYS, type ExecutiveMeetingAttendee, } from "@workspace/db"; import { @@ -159,18 +160,11 @@ const requireAdminAudit = makeRequireRoles(ADMIN_AUDIT_ROLES); // Allowed values for the row-highlight overlay on the daily schedule. // Mirrors the ROW_COLOR_OPTIONS palette in the frontend ( // artifacts/tx-os/src/pages/executive-meetings.tsx). NULL/omitted on the -// wire = "default" (no tint). Kept as a literal whitelist (not a string -// max-length check) so an unknown key is rejected with 400 instead of -// silently persisting a colour the UI cannot render. -const ROW_COLOR_KEYS = [ - "red", - "amber", - "green", - "blue", - "violet", - "gray", -] as const; -const rowColorSchema = z.enum(ROW_COLOR_KEYS).nullable(); +// wire = "default" (no tint). The whitelist itself is exported from +// the schema package (#293) so the API guard, the Drizzle CHECK +// constraint, and any future caller share one source of truth — change +// the palette in one place and both layers stay in sync. +const rowColorSchema = z.enum(EXECUTIVE_MEETING_ROW_COLOR_KEYS).nullable(); const dateSchema = z.string().regex(/^\d{4}-\d{2}-\d{2}$/, "expected YYYY-MM-DD"); diff --git a/artifacts/api-server/tests/executive-meetings-row-color.test.mjs b/artifacts/api-server/tests/executive-meetings-row-color.test.mjs index a0fdb793..9625426a 100644 --- a/artifacts/api-server/tests/executive-meetings-row-color.test.mjs +++ b/artifacts/api-server/tests/executive-meetings-row-color.test.mjs @@ -369,6 +369,104 @@ test("PATCH rowColor: the realtime executive_meetings_changed event fires for th } }); +test("DB CHECK constraint rejects off-palette rowColor on raw UPDATE that bypasses the API (#293)", async () => { + // The API's Zod whitelist is the primary guard, but #293 adds a + // matching CHECK constraint on `executive_meetings.row_color` so + // any out-of-band write path (manual psql, future bulk jobs, + // restored backups) cannot smuggle an unrenderable colour into the + // table either. This test bypasses the route entirely and writes + // straight to the DB to prove the constraint is doing its job. + const meeting = await createMeeting( + "RowColor DB check", + "قيد التحقق على مستوى قاعدة البيانات", + ); + + // Sanity: a NULL (default / no tint) update must still succeed — + // the constraint allows NULL to preserve the "no colour" case. + await pool.query( + `UPDATE executive_meetings SET row_color = NULL WHERE id = $1`, + [meeting.id], + ); + + // Each of the six palette keys must round-trip cleanly so we know + // the literal-list inside the CHECK matches the API whitelist. + for (const good of ["red", "amber", "green", "blue", "violet", "gray"]) { + await pool.query( + `UPDATE executive_meetings SET row_color = $1 WHERE id = $2`, + [good, meeting.id], + ); + } + + // Anything outside the palette must be rejected with PG's + // check_violation (SQLSTATE 23514) and must NOT change the stored + // colour. Cover a few realistic mistakes a future bulk-import + // script might introduce: an unsupported key, a casing slip, an + // empty string, and the magic string "default" the UI uses for + // the "no tint" radio option (which on the wire is supposed to be + // null, not the literal "default"). + for (const bad of ["fuchsia", "RED", "", "default", "red ", "blue;--"]) { + await assert.rejects( + pool.query( + `UPDATE executive_meetings SET row_color = $1 WHERE id = $2`, + [bad, meeting.id], + ), + (err) => { + assert.equal( + err.code, + "23514", + `expected PG check_violation for "${bad}", got ${err.code}: ${err.message}`, + ); + assert.match( + err.constraint ?? "", + /executive_meetings_row_color_palette_check/, + `error should reference the row-colour palette CHECK constraint, got "${err.constraint}"`, + ); + return true; + }, + ); + } + + // Last good colour was "gray" — confirm the rejected updates did + // not silently mutate the row. + const { rows } = await pool.query( + `SELECT row_color FROM executive_meetings WHERE id = $1`, + [meeting.id], + ); + assert.equal( + rows[0]?.row_color, + "gray", + "rejected updates must leave the previous colour intact", + ); + + // Belt-and-braces: also exercise INSERT, since a future bulk-import + // job is more likely to create rows than to UPDATE in place. The + // constraint must reject the off-palette value at row creation too, + // not only on subsequent updates. + await assert.rejects( + pool.query( + `INSERT INTO executive_meetings (title_ar, title_en, meeting_date, daily_number, row_color) + VALUES ($1, $2, $3, $4, $5)`, + [ + "INSERT-check probe", + "INSERT-check probe", + today, + // High daily_number to dodge the unique (date, daily_number) + // index against meetings created earlier in this file. + 99000 + Math.floor(Math.random() * 999), + "fuchsia", + ], + ), + (err) => { + assert.equal(err.code, "23514"); + assert.match( + err.constraint ?? "", + /executive_meetings_row_color_palette_check/, + ); + return true; + }, + ); +}); + test("PATCH rowColor: each successful change writes an audit row attributed to the acting user", async () => { const meeting = await createMeeting("RowColor audit", "سجل لون الصف"); diff --git a/lib/db/src/schema/executive-meetings.ts b/lib/db/src/schema/executive-meetings.ts index 5f309b1b..0f670b5e 100644 --- a/lib/db/src/schema/executive-meetings.ts +++ b/lib/db/src/schema/executive-meetings.ts @@ -11,9 +11,28 @@ import { boolean, index, uniqueIndex, + check, } from "drizzle-orm/pg-core"; +import { sql } from "drizzle-orm"; import { usersTable } from "./users"; +// Single source of truth for the row-highlight palette used by the +// daily schedule overlay (#288). Imported by the API route's Zod +// whitelist AND pinned at the database layer via the CHECK constraint +// below (#293), so the same invariant survives any out-of-band write +// path (manual SQL, future bulk jobs, restored backups). NULL means +// "no tint" / default; any other value must be one of these keys. +export const EXECUTIVE_MEETING_ROW_COLOR_KEYS = [ + "red", + "amber", + "green", + "blue", + "violet", + "gray", +] as const; +export type ExecutiveMeetingRowColor = + (typeof EXECUTIVE_MEETING_ROW_COLOR_KEYS)[number]; + export const executiveMeetingsTable = pgTable( "executive_meetings", { @@ -72,6 +91,25 @@ export const executiveMeetingsTable = pgTable( t.meetingDate, t.dailyNumber, ), + // Defense-in-depth (#293): mirror the API-side ROW_COLOR_KEYS + // whitelist at the DB layer so any out-of-band write path + // (manual SQL, future bulk jobs, restored backups) cannot smuggle + // an unrenderable colour past the Zod guard. NULL is the "no tint" + // sentinel, the six keys are the supported palette. Any other + // value raises a check_violation (PG SQLSTATE 23514) that the + // caller must explicitly handle. + // Literal-SQL form: Postgres CHECK constraint definitions are + // parsed as DDL and reject parameter placeholders ($1, $2, …), + // so the palette keys are inlined as quoted literals via + // `sql.raw`. This is safe because the keys come from a hardcoded + // compile-time constant of single-word identifiers (no user + // input ever reaches this string). + rowColorPaletteCheck: check( + "executive_meetings_row_color_palette_check", + sql`${t.rowColor} IS NULL OR ${t.rowColor} IN (${sql.raw( + EXECUTIVE_MEETING_ROW_COLOR_KEYS.map((k) => `'${k}'`).join(", "), + )})`, + ), }), );