Mirrors the API-side row-colour whitelist at the database layer so any out-of-band write path (manual psql, future bulk-import jobs, restored backups) cannot smuggle an unrenderable colour past the Zod guard introduced in #288. Changes: - lib/db/src/schema/executive-meetings.ts: export new shared constant EXECUTIVE_MEETING_ROW_COLOR_KEYS (red/amber/green/blue/violet/gray) + ExecutiveMeetingRowColor union type. Add a Drizzle check() constraint named `executive_meetings_row_color_palette_check` that allows NULL or any of the six keys. The CHECK uses sql.raw to inline the palette as quoted SQL literals (PG CHECK definitions are DDL and reject parameter placeholders); safe because the keys come from a hardcoded compile-time constant of single-word identifiers, never user input. Generating the literal list from the same constant guarantees the API guard and the DB constraint stay in sync. - artifacts/api-server/src/routes/executive-meetings.ts: import EXECUTIVE_MEETING_ROW_COLOR_KEYS from @workspace/db and rewire rowColorSchema to z.enum(EXECUTIVE_MEETING_ROW_COLOR_KEYS).nullable(). Deletes the local ROW_COLOR_KEYS const so the palette has exactly one source of truth. No behaviour change at runtime. - artifacts/api-server/tests/executive-meetings-row-color.test.mjs: append a focused defense-in-depth test that bypasses the API and asserts (a) NULL is allowed, (b) each of the six palette keys round-trips on raw UPDATE, (c) off-palette UPDATEs reject with PG SQLSTATE 23514 referencing the constraint by name, (d) the row keeps its previous colour after the rejected updates, and (e) raw INSERT with an off-palette value is rejected the same way. Migration applied via pnpm --filter @workspace/db push (no destructive prompts; existing rows are NULL or valid keys from #288 so the constraint installs cleanly). All 7 tests in the row-color file pass. Architect review: APPROVED, no critical findings. The single pre-existing Reorder test failure in the wider suite is unrelated to this change (Reorder does not touch row_color).
This commit is contained in:
@@ -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");
|
||||
|
||||
|
||||
@@ -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", "سجل لون الصف");
|
||||
|
||||
|
||||
@@ -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(", "),
|
||||
)})`,
|
||||
),
|
||||
}),
|
||||
);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user