From 023d4e2b9e4faf62af7268ad77f57784f7fc98b1 Mon Sep 17 00:00:00 2001 From: JnHu Date: Sat, 3 Oct 2026 00:01:28 +0800 Subject: [PATCH 1/4] fix(rich): align structural preflight with authoritative content limits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit preflightContentDocumentStructure counted every raw JSON value (objects, arrays, scalar strings) against PREFLIGHT_NODE_BUDGET = totalNodes * 2 + 64 (4064), while CONTENT_LIMITS.totalNodes counts grammar nodes: a marked text run costs ~6 raw units, so legal documents at ~30% of the serialized budget and well under 2000 grammar nodes were rejected before the schema parse (#673 C1 / PC-F02, evidence #686 — 7/7 shape families HIDDEN_STRICTER_SET). Remove the raw-node and fan-out budgets: the serialized-size gate already bounds the walk (every raw JSON value serializes to >= 1 char), and raw nesting depth stays bounded by PREFLIGHT_RAW_DEPTH_BUDGET, which is what actually protects the recursive parser. Grammar-node budgets remain the post-parse CONTENT_LIMITS authority, so preflight no longer defines a hidden stricter legal set (RC-04). --- .../domain/src/content/contentDocument.ts | 36 ++++++++----------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/packages/domain/src/content/contentDocument.ts b/packages/domain/src/content/contentDocument.ts index 957c4f851..f66539d47 100644 --- a/packages/domain/src/content/contentDocument.ts +++ b/packages/domain/src/content/contentDocument.ts @@ -279,12 +279,18 @@ function pushBlockViolations( // The wire schema (ContentDocumentV1Schema) is recursive (z.lazy over the // list/table grammar). A hostile payload nested thousands of levels deep can // overflow the RECURSIVE parser before the post-parse limit check ever runs. -// This preflight runs BEFORE any schema parse: it is iterative (explicit -// stack, no recursion → no stack overflow) and bounded, so the recursive -// parser never sees a structure deeper/larger than these budgets. +// This preflight runs BEFORE any schema parse and owns exactly the budgets +// the recursive parser needs to be safe (RC-04): +// - non-JSON / cycle rejection (JSON.stringify); +// - serialized size, which is also the traversal bound: every raw JSON +// value serializes to at least one character, so the raw walk below can +// never visit more than CONTENT_LIMITS.serializedChars units; +// - raw nesting depth — nested arrays cost ~1 character per level, so +// serialization size alone cannot bound recursion. +// Grammar-node budgets (node count, per-run text length, …) belong to +// CONTENT_LIMITS and are enforced after the parse: preflight must not define +// a smaller legal document set than the contract it fronts (#673 C1). -/** Raw-walk node budget. Grammar-valid documents stay far below it (arrays and envelope included). */ -const PREFLIGHT_NODE_BUDGET = CONTENT_LIMITS.totalNodes * 2 + 64; /** * Raw nesting budget. The raw walk counts the ARRAY levels interleaved * between the object levels, so a document at grammar depth g peaks at raw @@ -299,7 +305,7 @@ const PREFLIGHT_RAW_DEPTH_BUDGET = (CONTENT_LIMITS.depth + 2) * 2 + 3; * to enter the recursive ContentDocumentV1Schema parse. Returns every * violation (empty array = safe to parse). Pure; does not replace the schema * — it only makes sure the recursive parser never receives a structure that - * is dangerously deep, oversized, or cyclic. + * is dangerously deep or oversized. */ export function preflightContentDocumentStructure(value: unknown): string[] { const violations: string[] = []; @@ -339,20 +345,13 @@ export function preflightContentDocumentStructure(value: unknown): string[] { // Iterative DFS over the RAW value (every object/array, regardless of node // type): the recursive Zod parser's depth is driven by raw JSON nesting, - // not by grammar-correct nesting, so the bound must apply to both. + // not by grammar-correct nesting. Only depth is checked here — the + // traversal cost is already bounded by the serialized-size gate above. const stack: Array<{ node: unknown; depth: number }> = [ { node: value, depth: 0 }, ]; - let nodes = 0; while (stack.length > 0) { const { node, depth } = stack.pop()!; - nodes += 1; - if (nodes > PREFLIGHT_NODE_BUDGET) { - violations.push( - `document exceeds ${PREFLIGHT_NODE_BUDGET} structural nodes`, - ); - return violations; - } if (depth > PREFLIGHT_RAW_DEPTH_BUDGET) { violations.push( `document nesting exceeds ${PREFLIGHT_RAW_DEPTH_BUDGET} levels`, @@ -360,13 +359,6 @@ export function preflightContentDocumentStructure(value: unknown): string[] { return violations; } if (Array.isArray(node)) { - // Fan-out bound: reject a huge array without iterating its elements. - if (node.length > PREFLIGHT_NODE_BUDGET) { - violations.push( - `document array fan-out exceeds ${PREFLIGHT_NODE_BUDGET} elements`, - ); - return violations; - } for (const child of node) { stack.push({ node: child, depth: depth + 1 }); } From 2b20516531aca9bcc2e7f4ffa78788787ee4044c Mon Sep 17 00:00:00 2001 From: JnHu Date: Sat, 3 Oct 2026 00:01:41 +0800 Subject: [PATCH 2/4] fix(rich): enforce canonical closure at durable acceptance boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Normalization can change the measured shape (adjacent same-mark text runs merge, duplicate marks dedup), so a schema-valid input could normalize to a canonical value outside CONTENT_LIMITS — accepted on write, rejected by the same authority on every later re-validation (B-F01 / PC-F01, RC-03; the 20001-char merge class, evidence #686). Both durable Rich writers shared the hole: the answer canonicalizer and the question/option content seam. Add canonicalizeContentDocument to @exam/contracts — normalize + re-validate the canonical output through the one Rich authority (ContentDocumentV1Schema = grammar + CONTENT_LIMITS) — and route both writers through it. Legality is now decided on the representation that is actually persisted. SaveAnswer maps the rejection to its existing INVALID_ANSWER category; authoring maps it to ValidationError (400). No second limit table, schema, or normalizer. --- apps/api/src/lib/validateAnswerForQuestion.ts | 28 +++++++++++---- apps/api/src/routes/questionContent.ts | 33 +++++++++++------ packages/contracts/src/contentDocument.ts | 35 ++++++++++++++++++- 3 files changed, 77 insertions(+), 19 deletions(-) diff --git a/apps/api/src/lib/validateAnswerForQuestion.ts b/apps/api/src/lib/validateAnswerForQuestion.ts index 2e71e5f07..1b30c7cc0 100644 --- a/apps/api/src/lib/validateAnswerForQuestion.ts +++ b/apps/api/src/lib/validateAnswerForQuestion.ts @@ -1,6 +1,8 @@ -import { ContentDocumentV1Schema } from "@exam/contracts"; import { - normalizeContentDocument, + ContentDocumentV1Schema, + canonicalizeContentDocument, +} from "@exam/contracts"; +import { preflightContentDocumentStructure, type QuestionSnapshot, } from "@exam/domain"; @@ -14,9 +16,11 @@ import { * FROZEN QuestionSnapshot the attempt carries — never the live question row. * * For rich text_response answers this is also the CANONICALIZATION seam: the - * returned value is the normalized ContentDocumentV1, so every downstream - * consumer (answersEqual idempotency, draft persistence, submit freeze, - * grading workset) only ever sees canonical documents (#301 §22). + * returned value is the canonical ContentDocumentV1, re-validated after + * normalization so no accepted answer can persist a canonical form outside + * schema/limits (RC-03 closure). Every downstream consumer (answersEqual + * idempotency, draft persistence, submit freeze, grading workset) only ever + * sees canonical documents (#301 §22). * * `null` remains valid for every type — it is the protocol's "cleared" * answer (buildSubmittedAnswersSnapshot normalizes it for unanswered @@ -140,8 +144,18 @@ export function validateAnswerForQuestion( reason: "rich text_response answer must be a valid ContentDocumentV1", }; } - // Canonicalize BEFORE equality/idempotency/persistence (#301 §22). - return { ok: true, value: normalizeContentDocument(parsed.data) }; + // Canonical closure (RC-03, #669 Phase D1): normalization can merge + // adjacent same-mark runs, so legality is re-decided on the canonical + // value that will actually be persisted. Rejection keeps the existing + // INVALID_ANSWER-mapped category. + const canonical = canonicalizeContentDocument(parsed.data); + if (!canonical.ok) { + return { + ok: false, + reason: "rich text_response answer must be a valid ContentDocumentV1", + }; + } + return { ok: true, value: canonical.value }; } } } diff --git a/apps/api/src/routes/questionContent.ts b/apps/api/src/routes/questionContent.ts index 4012132b5..6625917c2 100644 --- a/apps/api/src/routes/questionContent.ts +++ b/apps/api/src/routes/questionContent.ts @@ -1,10 +1,10 @@ import { - normalizeContentDocument, plainTextProjection, type ContentDocumentV1, type ContentMode, type QuestionType, } from "@exam/domain"; +import { canonicalizeContentDocument } from "@exam/contracts"; import { ValidationError } from "@exam/domain"; /** @@ -12,10 +12,11 @@ import { ValidationError } from "@exam/domain"; * * Every question/option content write — create, update, and the merged * update re-validation — resolves through this seam. For a Rich write the - * document is normalized into canonical form and `content` is DERIVED as its - * plain-text projection; the client's `content`, if any, is never trusted. - * For a Plain write `content_document` is persisted as NULL so legacy and - * plain rows stay indistinguishable. + * document is canonicalized (normalized AND re-validated on the canonical + * form, so no write can persist a document outside schema/limits — RC-03) + * and `content` is DERIVED as its plain-text projection; the client's + * `content`, if any, is never trusted. For a Plain write `content_document` + * is persisted as NULL so legacy and plain rows stay indistinguishable. * * Hostile-depth protection is a schema-level closure: `ContentDocumentV1Schema` * — which types every rich slot in the create/update request schemas — runs @@ -50,11 +51,16 @@ export interface ResolvedQuestionContent { function resolveOption(option: OptionWriteInput): ResolvedOption { if (option.contentDocument != null) { - const document = normalizeContentDocument(option.contentDocument); + const canonical = canonicalizeContentDocument(option.contentDocument); + if (!canonical.ok) { + throw new ValidationError( + `option ${option.id} contentDocument violates canonical limits: ${canonical.reason}`, + ); + } return { id: option.id, - content: plainTextProjection(document), - contentDocument: document, + content: plainTextProjection(canonical.value), + contentDocument: canonical.value, ...(option.isCorrect !== undefined ? { isCorrect: option.isCorrect } : {}), @@ -88,10 +94,15 @@ export function resolveQuestionContentWrite(input: { const options = (input.options ?? []).map(resolveOption); if (input.contentDocument != null) { - const document = normalizeContentDocument(input.contentDocument); + const canonical = canonicalizeContentDocument(input.contentDocument); + if (!canonical.ok) { + throw new ValidationError( + `contentDocument violates canonical limits: ${canonical.reason}`, + ); + } return { - content: plainTextProjection(document), - contentDocument: document, + content: plainTextProjection(canonical.value), + contentDocument: canonical.value, answerMode: input.answerMode ?? null, options, }; diff --git a/packages/contracts/src/contentDocument.ts b/packages/contracts/src/contentDocument.ts index 9dd181533..53ddffc2c 100644 --- a/packages/contracts/src/contentDocument.ts +++ b/packages/contracts/src/contentDocument.ts @@ -4,6 +4,7 @@ import { CONTENT_DOC_VERSION, CONTENT_LIMITS, checkContentDocumentLimits, + normalizeContentDocument, preflightContentDocumentStructure, type ContentBlockMath, type ContentBlock, @@ -16,8 +17,8 @@ import { type ContentOrderedList, type ContentParagraph, type ContentTable, - type ContentTableCell, type ContentTableRow, + type ContentTableCell, type ContentTextRun, } from "@exam/domain"; @@ -230,6 +231,38 @@ export const ContentDocumentV1Schema = RawPreflightSchema.pipe( RecursiveContentDocumentV1Schema, ); +/** + * Canonicalization closure seam (RC-03, #669 Phase D1): normalizes a + * schema-valid document and re-validates the canonical output through the + * one Rich authority (ContentDocumentV1Schema = grammar + CONTENT_LIMITS). + * + * Normalization can change the measured shape — adjacent same-mark text + * runs merge, duplicate marks dedup — so a schema-valid input can normalize + * to a canonical value outside the contract's limits. Every durable Rich + * write boundary (answer canonicalization, question/option content writes) + * must persist THIS function's accepted value and nothing else: legality is + * decided on the representation that will actually be stored. + * + * Outer protocols own the wire/error mapping: SaveAnswer turns `ok: false` + * into its INVALID_ANSWER category; authoring turns it into a + * ValidationError. + */ +export function canonicalizeContentDocument( + doc: ContentDocumentV1, +): { ok: true; value: ContentDocumentV1 } | { ok: false; reason: string } { + const canonical = normalizeContentDocument(doc); + const parsed = ContentDocumentV1Schema.safeParse(canonical); + if (!parsed.success) { + return { + ok: false, + reason: + parsed.error.issues[0]?.message ?? + "document is not a valid ContentDocumentV1", + }; + } + return { ok: true, value: parsed.data }; +} + /** Type identity between the wire schema and the domain grammar. */ type AssertExact = [A] extends [B] ? [B] extends [A] From 5c7c04a2745898e3d51a17c821ae678fc75c1901 Mon Sep 17 00:00:00 2001 From: JnHu Date: Sat, 3 Oct 2026 00:01:56 +0800 Subject: [PATCH 3/4] test(rich): add phase-D1 canonical closure regressions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit R1 PC-F01 merge-class seed (20000+1 unmarked runs) rejected at the answer canonicalizer, the authoring seam (400), and the save-answer route (INVALID_ANSWER) — the Phase-C L3 gap driven end to end. R2 Canonical textRun boundary closed from both sides (T-1+1 accepted and schema-legal; T-1+2 rejected), derived from CONTENT_LIMITS. R3 Legality decided on the canonical form: duplicate-mark dedup + merge, hardBreak as a non-merge barrier, RC-03 fixed point on accepted values. R4 The seven Phase-C preflight-shrink families (plain runs, marked runs, duplicate marks, empty paragraphs, marked tables) pass preflight and the public parse entry at their measured failure scales. R5 Genuine violations still rejected: hostile nesting, oversized serialization, and node-count overflow now named by the CONTENT_LIMITS walker instead of a preflight budget. Plus a bounded seeded property (300 cases, seed 0x66900001): every accepted answer stays schema-legal after canonicalization. Production functions only — no duplicated oracle. Evidence provenance: #686 (research branch, not a parent of this branch). #669 #673 --- .../src/lib/validateAnswerForQuestion.test.ts | 212 ++++++++++++++- apps/api/src/routes/answerRichClosure.test.ts | 253 ++++++++++++++++++ .../src/routes/questionRichContent.test.ts | 62 +++++ .../src/__tests__/contentDocument.test.ts | 80 +++++- .../src/content/contentDocument.test.ts | 108 +++++++- 5 files changed, 707 insertions(+), 8 deletions(-) create mode 100644 apps/api/src/routes/answerRichClosure.test.ts diff --git a/apps/api/src/lib/validateAnswerForQuestion.test.ts b/apps/api/src/lib/validateAnswerForQuestion.test.ts index bdf32d49f..ac5fc8b39 100644 --- a/apps/api/src/lib/validateAnswerForQuestion.test.ts +++ b/apps/api/src/lib/validateAnswerForQuestion.test.ts @@ -1,5 +1,14 @@ import { describe, expect, it } from "vitest"; -import type { QuestionSnapshot } from "@exam/domain"; +import { + ContentDocumentV1Schema, + canonicalizeContentDocument, +} from "@exam/contracts"; +import { + CONTENT_LIMITS, + normalizeContentDocument, + type ContentDocumentV1, + type QuestionSnapshot, +} from "@exam/domain"; import { validateAnswerForQuestion } from "./validateAnswerForQuestion.js"; function snapshot(overrides: Partial): QuestionSnapshot { @@ -159,3 +168,204 @@ describe("validateAnswerForQuestion (#301 §21/§44)", () => { } }); }); + +describe("rich canonical closure (RC-03, #669 Phase D1)", () => { + const RICH = snapshot({ + type: "text_response", + answerMode: "rich", + options: [], + standardAnswer: null, + }); + const T = CONTENT_LIMITS.textRun; + + /** One-paragraph document from text runs; runs merge when marks match. */ + function runsDocument( + runs: Array<{ text: string; marks?: string[] }>, + ): ContentDocumentV1 { + return { + docVersion: 1, + type: "doc", + content: [ + { + type: "paragraph", + content: runs.map((run) => ({ + type: "text" as const, + text: run.text, + ...(run.marks ? { marks: run.marks as never[] } : {}), + })), + }, + ], + }; + } + + it("rejects the PC-F01 merge-class seed before durable acceptance (#669/#686)", () => { + // The Phase-C minimized counterexample: two adjacent unmarked runs, + // each within textRun, whose canonical merge is a 20001-char run that + // ContentDocumentV1Schema rejects. Durable acceptance must decide on + // that canonical form, so the seed must be rejected, not persisted. + const seed = runsDocument([{ text: "a".repeat(T) }, { text: "b" }]); + expect( + ContentDocumentV1Schema.safeParse(normalizeContentDocument(seed)).success, + ).toBe(false); + expect(validateAnswerForQuestion(RICH, seed).ok).toBe(false); + }); + + it("keeps the canonical textRun boundary closed from both sides", () => { + // Largest accepted canonical form: two merged runs totalling exactly T. + const atLimit = validateAnswerForQuestion( + RICH, + runsDocument([{ text: "a".repeat(T - 1) }, { text: "b" }]), + ); + expect(atLimit.ok).toBe(true); + if (atLimit.ok) { + expect(atLimit.value).toEqual( + runsDocument([{ text: "a".repeat(T - 1) + "b" }]), + ); + expect(ContentDocumentV1Schema.safeParse(atLimit.value).success).toBe( + true, + ); + } + + // First rejected canonical boundary: one char past the limit exists + // only after the merge, never in any input run. + expect( + validateAnswerForQuestion( + RICH, + runsDocument([{ text: "a".repeat(T - 1) }, { text: "bc" }]), + ).ok, + ).toBe(false); + + // A single run at the limit stays legal; one char more is not. + expect( + validateAnswerForQuestion(RICH, runsDocument([{ text: "a".repeat(T) }])) + .ok, + ).toBe(true); + expect( + validateAnswerForQuestion( + RICH, + runsDocument([{ text: "a".repeat(T + 1) }]), + ).ok, + ).toBe(false); + }); + + it("decides legality on the canonical form normalization produces", () => { + // Duplicate marks are schema-legal; canonicalization dedups them to + // ["bold"] AND merges adjacent runs, so the canonical text length is + // the sum. Exactly-at-limit merges stay legal; one char over is not. + const dup = (n: number) => Array(n).fill("bold"); + const atLimit = validateAnswerForQuestion( + RICH, + runsDocument([ + { text: "a".repeat(T - 1), marks: dup(3) }, + { text: "b", marks: dup(2) }, + ]), + ); + expect(atLimit.ok).toBe(true); + if (atLimit.ok) { + expect(atLimit.value).toEqual( + runsDocument([{ text: "a".repeat(T - 1) + "b", marks: ["bold"] }]), + ); + } + expect( + validateAnswerForQuestion( + RICH, + runsDocument([ + { text: "a".repeat(T - 1), marks: dup(3) }, + { text: "bc", marks: dup(2) }, + ]), + ).ok, + ).toBe(false); + }); + + it("merge bridges only adjacent identical runs; hardBreak still separates (fixed point holds)", () => { + const separated: ContentDocumentV1 = { + docVersion: 1, + type: "doc", + content: [ + { + type: "paragraph", + content: [ + { type: "text", text: "a".repeat(T) }, + { type: "hardBreak" }, + { type: "text", text: "b" }, + ], + }, + ], + }; + const result = validateAnswerForQuestion(RICH, separated); + expect(result.ok).toBe(true); + if (result.ok) { + const parsed = ContentDocumentV1Schema.safeParse(result.value); + expect(parsed.success).toBe(true); + if (parsed.success) { + // RC-03 fixed point: canonicalizing an accepted canonical value + // again succeeds and changes nothing. + expect(normalizeContentDocument(parsed.data)).toEqual(parsed.data); + } + } + }); + + it("property: every accepted answer stays schema-legal after canonicalization (seeded, bounded)", () => { + // Generator only constructs candidates; acceptance and canonical + // legality are decided exclusively by production functions. Text-run + // lengths are boundary-biased so same-mark neighbours land around the + // merge limit. Seed 0x66900001 from the Phase-C campaign lineage. + let a = 0x66900001 >>> 0; + const rand = () => { + a = (a + 0x6d2b79f5) | 0; + let t = Math.imul(a ^ (a >>> 15), 1 | a); + t = (t + Math.imul(t ^ (t >>> 7), 61 | t)) ^ t; + return ((t ^ (t >>> 14)) >>> 0) / 4294967296; + }; + const intBetween = (lo: number, hi: number) => + lo + Math.floor(rand() * (hi - lo + 1)); + const pick = (xs: readonly T[]): T => + xs[Math.floor(rand() * xs.length)]!; + + for (let i = 0; i < 300; i++) { + const candidate: ContentDocumentV1 = { + docVersion: 1, + type: "doc", + content: Array.from({ length: intBetween(1, 3) }, () => ({ + type: "paragraph" as const, + content: Array.from({ length: intBetween(1, 4) }, () => { + const kind = pick([ + "text", + "text", + "text", + "hardBreak", + "math", + ] as const); + if (kind === "hardBreak") return { type: "hardBreak" as const }; + if (kind === "math") + return { type: "inlineMath" as const, latex: "x" }; + const len = + rand() < 0.34 + ? Math.max(1, Math.round(T / 2) + intBetween(-60, 60)) + : intBetween(1, 40); + const marks = pick([ + undefined, + ["bold"], + ["bold", "italic"], + ["bold", "bold"], + ["underline", "underline", "underline"], + ] as const); + return { + type: "text" as const, + text: "x".repeat(len), + ...(marks ? { marks: [...marks] } : {}), + }; + }), + })), + }; + const result = validateAnswerForQuestion(RICH, candidate); + if (result.ok) { + const parsed = ContentDocumentV1Schema.safeParse(result.value); + expect( + parsed.success, + `accepted candidate #${i} must stay legal after canonicalization`, + ).toBe(true); + } + } + }); +}); diff --git a/apps/api/src/routes/answerRichClosure.test.ts b/apps/api/src/routes/answerRichClosure.test.ts new file mode 100644 index 000000000..1cb494e63 --- /dev/null +++ b/apps/api/src/routes/answerRichClosure.test.ts @@ -0,0 +1,253 @@ +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { eq } from "drizzle-orm"; +import { schema } from "@exam/db/src/schema/pg.js"; +import { ContentDocumentV1Schema } from "@exam/contracts"; +import type { TestContext } from "./testHelpers.js"; +import { buildTestApp, uniquePrefix } from "./testHelpers.js"; +import courseRoutes from "./course.js"; +import questionRoutes from "./question.js"; +import examRoutes from "./exam.js"; +import attemptRoutes from "./attempts.js"; + +/** + * Rich canonical closure at the SaveAnswer wire (#669 Phase D1, RC-03). + * + * Phase C proved the merge class (PC-F01) at the engine seam and inferred + * route reachability; these tests drive the real route: a legal input whose + * canonical form violates CONTENT_LIMITS must be rejected before durable + * acceptance (INVALID_ANSWER), while the exactly-at-limit canonical form is + * accepted, replayed, and served back as a schema-legal canonical value. + */ +describe("rich answer canonical closure (save-answer route)", () => { + let ctx: Awaited>; + let courseId: string; + let questionId: string; + let candidateProfileId: string; + let attemptId: string; + + const MERGE_SEED = { + docVersion: 1, + type: "doc", + content: [ + { + type: "paragraph", + content: [ + { type: "text", text: "a".repeat(20000) }, + { type: "text", text: "b" }, + ], + }, + ], + }; + + const AT_LIMIT_SEED = { + docVersion: 1, + type: "doc", + content: [ + { + type: "paragraph", + content: [ + { type: "text", text: "a".repeat(19999) }, + { type: "text", text: "b" }, + ], + }, + ], + }; + + beforeAll(async () => { + ctx = await buildTestApp(async (fastify) => { + await fastify.register(courseRoutes, { prefix: "" }); + await fastify.register(questionRoutes, { prefix: "" }); + await fastify.register(examRoutes, { prefix: "" }); + await fastify.register(attemptRoutes, { prefix: "" }); + }); + + const courseRes = await ctx.app.inject({ + method: "POST", + url: "/api/courses", + payload: { + name: "Closure Course", + code: `CL-${uniquePrefix()}`, + description: "", + }, + cookies: { "auth-token": ctx.adminToken }, + }); + courseId = courseRes.json().id as string; + + const questionRes = await ctx.app.inject({ + method: "POST", + url: "/api/questions", + payload: { + courseId, + type: "text_response", + answerMode: "rich", + contentDocument: { + docVersion: 1, + type: "doc", + content: [ + { + type: "paragraph", + content: [{ type: "text", text: "Describe in detail." }], + }, + ], + }, + options: [], + standardAnswer: null, + score: 10, + difficulty: 1, + rubric: "r", + }, + cookies: { "auth-token": ctx.adminToken }, + }); + questionId = questionRes.json().id as string; + + const existing = await ctx.db + .select({ id: schema.candidateProfiles.id }) + .from(schema.candidateProfiles) + .where(eq(schema.candidateProfiles.userId, ctx.candidate.id)); + candidateProfileId = existing[0]?.id ?? crypto.randomUUID(); + if (!existing[0]) { + await ctx.db.insert(schema.candidateProfiles).values({ + id: candidateProfileId, + organizationId: ctx.org.id, + userId: ctx.candidate.id, + fields: {}, + createdAt: new Date(), + updatedAt: new Date(), + }); + } + + const examRes = await ctx.app.inject({ + method: "POST", + url: "/api/exams", + payload: { + title: "Rich Closure Exam", + description: "", + courseId, + timingMode: "timed_window", + durationMinutes: 60, + openAt: new Date(Date.now() - 3600000).toISOString(), + closeAt: new Date(Date.now() + 86400000).toISOString(), + passingScore: 6, + totalScore: 10, + questionSelectionMode: "manual", + questionIds: [questionId], + controlFlags: { + shuffleQuestions: false, + shuffleOptions: false, + detectTabSwitch: false, + disableCopyPaste: false, + requireQueue: false, + batchSize: 10, + batchInterval: 3, + restrictIp: false, + requireLockdown: false, + showResultImmediately: true, + }, + retakePolicy: "unlimited", + scoreStrategy: "highest", + maxAttempts: 3, + }, + cookies: { "auth-token": ctx.adminToken }, + }); + const examId = examRes.json().id as string; + await ctx.app.inject({ + method: "POST", + url: `/api/exams/${examId}/publish`, + cookies: { "auth-token": ctx.adminToken }, + }); + await ctx.app.inject({ + method: "POST", + url: `/api/exams/${examId}/enrollments`, + payload: { candidateIds: [candidateProfileId] }, + cookies: { "auth-token": ctx.adminToken }, + }); + const startRes = await ctx.app.inject({ + method: "POST", + url: `/api/attempts/${examId}/start`, + cookies: { "auth-token": ctx.candidateToken }, + }); + attemptId = startRes.json().id as string; + }); + + afterAll(async () => { + await ctx.cleanup(); + }); + + function saveAnswer(answer: unknown, clientSeq: number, baseVersion = 0) { + return ctx.app.inject({ + method: "POST", + url: `/api/attempts/${attemptId}/answers/${questionId}`, + payload: { + attemptId, + questionId, + answer, + clientSeq, + clientSavedAt: new Date().toISOString(), + baseVersion, + }, + cookies: { "auth-token": ctx.candidateToken }, + }); + } + + it("rejects the merge-class seed at the wire with INVALID_ANSWER (PC-F01)", async () => { + const res = await saveAnswer(MERGE_SEED, 1); + expect(res.statusCode).toBe(200); + expect(res.json()).toMatchObject({ + accepted: false, + reason: "INVALID_ANSWER", + }); + }); + + it("accepts the exactly-at-limit canonical form, replays it, and serves it back schema-legal", async () => { + const first = await saveAnswer(AT_LIMIT_SEED, 10); + expect(first.statusCode, first.body).toBe(200); + const accepted = first.json(); + expect(accepted, first.body).toMatchObject({ + accepted: true, + serverVersion: 1, + }); + + // §12 replay: same clientSeq + same canonical identity → same prior + // acknowledgement, zero new write (serverVersion unchanged). + const replay = await saveAnswer(AT_LIMIT_SEED, 10); + expect(replay.statusCode).toBe(200); + expect(replay.json()).toMatchObject({ + accepted: true, + serverVersion: accepted.serverVersion, + }); + + // The served canonical answer (stale-version probe) is the merged + // at-limit form and passes the schema it must satisfy on every read. + const probe = await saveAnswer( + { + docVersion: 1, + type: "doc", + content: [ + { type: "paragraph", content: [{ type: "text", text: "new" }] }, + ], + }, + 11, + 0, + ); + expect(probe.statusCode).toBe(200); + expect(probe.json()).toMatchObject({ + accepted: false, + reason: "STALE_VERSION", + }); + const served = probe.json().details?.serverAnswer; + expect( + ContentDocumentV1Schema.safeParse(served).success, + JSON.stringify(served)?.slice(0, 200), + ).toBe(true); + expect(served).toEqual({ + docVersion: 1, + type: "doc", + content: [ + { + type: "paragraph", + content: [{ type: "text", text: "a".repeat(19999) + "b" }], + }, + ], + }); + }); +}); diff --git a/apps/api/src/routes/questionRichContent.test.ts b/apps/api/src/routes/questionRichContent.test.ts index caf1c1d46..20b9bb7ee 100644 --- a/apps/api/src/routes/questionRichContent.test.ts +++ b/apps/api/src/routes/questionRichContent.test.ts @@ -314,4 +314,66 @@ describe("rich content write authority", () => { }); expect(res.statusCode, res.body).toBe(201); }); + + it("rejects a rich prompt whose canonical form violates the text-run limit (PC-F01 closure at the authoring seam)", async () => { + // The Phase-C merge-class seed: two adjacent unmarked runs, each within + // textRun, whose normalized merge is a 20001-char run. The persisted + // document is the canonical form, so the write must fail instead of + // persisting a document the schema rejects on read-back. + const mergeSeed = { + docVersion: 1, + type: "doc", + content: [ + { + type: "paragraph", + content: [ + { type: "text", text: "a".repeat(20000) }, + { type: "text", text: "b" }, + ], + }, + ], + }; + const res = await createQuestion({ + type: "text_response", + contentDocument: mergeSeed, + options: [], + standardAnswer: null, + rubric: "r", + }); + expect(res.statusCode).toBe(400); + }); + + it("keeps a rich prompt at the exact canonical text-run boundary creatable", async () => { + const atLimit = { + docVersion: 1, + type: "doc", + content: [ + { + type: "paragraph", + content: [ + { type: "text", text: "a".repeat(19999) }, + { type: "text", text: "b" }, + ], + }, + ], + }; + const res = await createQuestion({ + type: "text_response", + contentDocument: atLimit, + options: [], + standardAnswer: null, + rubric: "r", + }); + expect(res.statusCode, res.body).toBe(201); + expect(res.json().contentDocument).toEqual({ + docVersion: 1, + type: "doc", + content: [ + { + type: "paragraph", + content: [{ type: "text", text: "a".repeat(19999) + "b" }], + }, + ], + }); + }); }); diff --git a/packages/contracts/src/__tests__/contentDocument.test.ts b/packages/contracts/src/__tests__/contentDocument.test.ts index 428a460eb..a650846c2 100644 --- a/packages/contracts/src/__tests__/contentDocument.test.ts +++ b/packages/contracts/src/__tests__/contentDocument.test.ts @@ -1,11 +1,19 @@ import { describe, expect, it } from "vitest"; -import { AnswerModeEnum, ContentDocumentV1Schema } from "../contentDocument.js"; +import { + AnswerModeEnum, + ContentDocumentV1Schema, + canonicalizeContentDocument, +} from "../contentDocument.js"; import { CreateQuestionRequestSchema, UpdateQuestionRequestSchema, } from "../question.js"; import { QuestionSnapshotSchema } from "../attempt.js"; -import type { ContentDocumentV1 } from "@exam/domain"; +import { + CONTENT_LIMITS, + normalizeContentDocument, + type ContentDocumentV1, +} from "@exam/domain"; function doc(content: unknown[]): ContentDocumentV1 { return { docVersion: 1, type: "doc", content } as ContentDocumentV1; @@ -419,6 +427,74 @@ describe("ContentDocumentV1Schema — preflight-safe parse entry", () => { expect(message).toMatch(/nesting exceeds|depth exceeds|structural/); }); + it("accepts a within-limits document the removed raw-node budget used to reject (#673 C1 / PC-F02)", () => { + // 677 plain paragraphs = 1354 grammar nodes < totalNodes(2000), ~41k + // serialized chars < serializedChars: the measured minimal failure of + // the Phase-C raw-node-budget campaign, now asserted at the PUBLIC + // parse entry shared by route validation, read classification, and + // client guards. Within-limit documents must never fail here. + const legal = { + docVersion: 1, + type: "doc", + content: Array.from({ length: 677 }, () => ({ + type: "paragraph", + content: [{ type: "text", text: "0123456789".repeat(3) }], + })), + }; + const parsed = ContentDocumentV1Schema.safeParse(legal); + expect(parsed.success).toBe(true); + }); + + describe("canonicalizeContentDocument (RC-03 closure seam)", () => { + function runsDocument( + runs: Array<{ text: string; marks?: string[] }>, + ): ContentDocumentV1 { + return { + docVersion: 1, + type: "doc", + content: [ + { + type: "paragraph", + content: runs.map((run) => ({ + type: "text" as const, + text: run.text, + ...(run.marks ? { marks: run.marks as never[] } : {}), + })), + }, + ], + }; + } + + it("returns the RC-03 fixed point for legal input", () => { + const legal = canonicalizeContentDocument( + runsDocument([{ text: "x" }, { text: "y" }]), + ); + expect(legal.ok).toBe(true); + if (legal.ok) { + expect(legal.value).toEqual(runsDocument([{ text: "xy" }])); + expect(normalizeContentDocument(legal.value)).toEqual(legal.value); + } + }); + + it("rejects the merge class on the canonical form with the limit violation", () => { + // Two schema-valid unmarked runs whose merge is a 20001-char run: + // rejected on the representation that would be persisted. + const overMerge = canonicalizeContentDocument( + runsDocument([ + { text: "a".repeat(CONTENT_LIMITS.textRun) }, + { text: "b" }, + ]), + ); + expect(overMerge.ok).toBe(false); + if (!overMerge.ok) { + // The first canonical-limit issue names the textRun ceiling — via + // the zod max message or the limits walker, both driven by + // CONTENT_LIMITS.textRun. + expect(overMerge.reason).toContain(String(CONTENT_LIMITS.textRun)); + } + }); + }); + it("still accepts the deepest legal grammar document (7 nested lists = tree depth 16)", () => { let block: Record = { type: "paragraph", diff --git a/packages/domain/src/content/contentDocument.test.ts b/packages/domain/src/content/contentDocument.test.ts index 32edd97a1..18ac20566 100644 --- a/packages/domain/src/content/contentDocument.test.ts +++ b/packages/domain/src/content/contentDocument.test.ts @@ -344,16 +344,17 @@ describe("preflightContentDocumentStructure", () => { expect(violations.some((v) => v.includes("nesting exceeds"))).toBe(true); }); - it("rejects a huge array fan-out without iterating every element", () => { - // 200k elements is ~100× the preflight node budget (4064): large enough - // that iterating it would be the measurable cost, small enough that the - // fixture itself allocates fast under parallel test load. + it("rejects a serialized-oversized array fan-out at the serialization gate", () => { + // 200k empty objects serialize to ~600k chars — past serializedChars — + // so the value is rejected before the raw walk runs. The walk itself is + // inherently bounded: every raw JSON value costs at least one serialized + // character, so a payload that passes the size gate cannot make the walk + // visit more than CONTENT_LIMITS.serializedChars units. const hostile = { docVersion: 1, type: "doc", content: new Array(200_000).fill(0).map(() => ({})), }; - // Budget-bounded: must return long before touching all elements. const violations = preflightContentDocumentStructure(hostile); expect(violations.length).toBeGreaterThan(0); }); @@ -386,4 +387,101 @@ describe("preflightContentDocumentStructure", () => { expect(checkContentDocumentLimits(legal)).toEqual([]); expect(preflightContentDocumentStructure(legal)).toEqual([]); }); + + // ── RC-04 / PC-F02 regressions (#669 Phase D1; evidence #686) ───── + // + // Phase C (#673 C1) proved the preflight raw-walk node budget rejected + // documents CONTENT_LIMITS accepts: every object, array, and scalar + // string counted as one raw unit, so legal documents at ~30% of the + // serialized budget and well under totalNodes failed with "document + // exceeds 4064 structural nodes". The fixtures below are the measured + // failure scales of that campaign, kept fixed and deterministic. The + // invariant: within authoritative CONTENT_LIMITS ⇒ preflight accepts. + + it("accepts within-limits documents the raw-node budget used to reject (#673 C1 / PC-F02)", () => { + // Plain runs: 700 paragraphs = 1400 grammar nodes < totalNodes. + const plainRuns = doc( + ...Array.from({ length: 700 }, () => + paragraph("012345678901234567890123456789"), + ), + ); + expect(checkContentDocumentLimits(plainRuns)).toEqual([]); + expect(preflightContentDocumentStructure(plainRuns)).toEqual([]); + + // Fully marked runs: 500 paragraphs = 1000 grammar nodes; every mark + // string is a raw unit the old budget charged against the document. + const markedRuns = doc( + ...Array.from({ length: 500 }, () => + paragraph("01234567890123456789", [ + "bold", + "italic", + "underline", + ] as never[]), + ), + ); + expect(checkContentDocumentLimits(markedRuns)).toEqual([]); + expect(preflightContentDocumentStructure(markedRuns)).toEqual([]); + + // Duplicate marks: 12 repeated "bold" marks per run pass the schema + // (only inlineCode exclusivity is restricted) and normalization dedups + // them — 300 paragraphs of this shape are grammar-legal and were + // rejected purely by raw-unit accounting. + const duplicateMarks = doc( + ...Array.from({ length: 300 }, () => + paragraph("01234567890123456789", Array(12).fill("bold") as never[]), + ), + ); + expect(checkContentDocumentLimits(duplicateMarks)).toEqual([]); + expect(preflightContentDocumentStructure(duplicateMarks)).toEqual([]); + + // Empty paragraphs: pure container overhead, one third of a raw unit + // per grammar node. + const emptyParagraphs = doc( + ...Array.from({ length: 1400 }, () => ({ + type: "paragraph" as const, + content: [], + })), + ); + expect(checkContentDocumentLimits(emptyParagraphs)).toEqual([]); + expect(preflightContentDocumentStructure(emptyParagraphs)).toEqual([]); + + // Marked table: 16 rows × 20 cells = 320 cells ≤ tableCells, 977 + // grammar nodes < totalNodes. + const markedTable = doc({ + type: "table", + content: Array.from({ length: 16 }, () => ({ + type: "tableRow" as const, + content: Array.from({ length: 20 }, () => ({ + type: "tableCell" as const, + content: [ + paragraph("0123456789", ["bold", "italic", "underline"] as never[]), + ], + })), + })), + }); + expect(checkContentDocumentLimits(markedTable)).toEqual([]); + expect(preflightContentDocumentStructure(markedTable)).toEqual([]); + }); + + it("leaves node-count rejection to the limits authority, not preflight (RC-04 boundary)", () => { + // 1100 paragraphs × 2 runs = 2200 grammar nodes > totalNodes(2000), + // ~105k serialized chars < serializedChars: the only authority that + // rejects this document is CONTENT_LIMITS — preflight must stay clean + // so the schema's own limit walker produces the violation. + const overNodeBudget = doc( + ...Array.from({ length: 1100 }, () => ({ + type: "paragraph" as const, + content: [ + { type: "text" as const, text: "abcde" }, + { type: "text" as const, text: "fghij" }, + ], + })), + ); + expect(preflightContentDocumentStructure(overNodeBudget)).toEqual([]); + expect( + checkContentDocumentLimits(overNodeBudget).some((violation) => + violation.includes("nodes"), + ), + ).toBe(true); + }); }); From 38b6b735997127e4185dbb9d9ae067481ac343d3 Mon Sep 17 00:00:00 2001 From: JnHu Date: Sat, 3 Oct 2026 01:23:24 +0800 Subject: [PATCH 4/4] fix(rich): short-circuit structural preflight at the serialization gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F1 review finding (#687): an oversized payload had its verdict recorded, but the raw DFS still traversed the hostile structure — the serialized budget was not actually the traversal bound the section comment claimed. An oversized payload is now rejected before the walk; envelope diagnostics (O(1) property reads, existing error contract) are kept. The regression test pairs oversize with beyond-raw-depth nesting and asserts the size violation is the ONLY output, proving the walk never ran. --- packages/domain/src/content/contentDocument.test.ts | 13 +++++++++++++ packages/domain/src/content/contentDocument.ts | 12 +++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/packages/domain/src/content/contentDocument.test.ts b/packages/domain/src/content/contentDocument.test.ts index 18ac20566..af73fdee6 100644 --- a/packages/domain/src/content/contentDocument.test.ts +++ b/packages/domain/src/content/contentDocument.test.ts @@ -359,6 +359,19 @@ describe("preflightContentDocumentStructure", () => { expect(violations.length).toBeGreaterThan(0); }); + it("stops at the serialization gate — an oversized payload never enters the raw walk", () => { + // Oversized AND deeper than the raw depth budget: if the walk ran, it + // would append a "nesting exceeds" violation on top of the size one. + // The gate's verdict is final, so the serialized-size violation must be + // the ONLY one — proving the hostile structure was never traversed. + let bomb: unknown = "x".repeat(200_000); + for (let i = 0; i < 50; i++) bomb = [bomb]; + const hostile = { docVersion: 1, type: "doc", content: bomb }; + expect(preflightContentDocumentStructure(hostile)).toEqual([ + `serialized document exceeds ${CONTENT_LIMITS.serializedChars} chars`, + ]); + }); + it("rejects a cyclic structure instead of throwing", () => { const cyclic: Record = { docVersion: 1, type: "doc" }; cyclic["self"] = cyclic; diff --git a/packages/domain/src/content/contentDocument.ts b/packages/domain/src/content/contentDocument.ts index f66539d47..61883adb7 100644 --- a/packages/domain/src/content/contentDocument.ts +++ b/packages/domain/src/content/contentDocument.ts @@ -313,6 +313,7 @@ export function preflightContentDocumentStructure(value: unknown): string[] { // Serialized size. JSON.stringify also detects cycles — a cyclic value can // never be a document and would break downstream serialization anyway. let serialized: string | undefined; + let oversized = false; try { serialized = JSON.stringify(value); } catch { @@ -321,7 +322,8 @@ export function preflightContentDocumentStructure(value: unknown): string[] { ); } if (serialized !== undefined) { - if (serialized.length > CONTENT_LIMITS.serializedChars) { + oversized = serialized.length > CONTENT_LIMITS.serializedChars; + if (oversized) { violations.push( `serialized document exceeds ${CONTENT_LIMITS.serializedChars} chars`, ); @@ -343,6 +345,14 @@ export function preflightContentDocumentStructure(value: unknown): string[] { violations.push("content must be an array"); } + // An oversized payload is already rejected — its verdict is final, so the + // hostile structure is never traversed (F1 review finding). Everything + // below only ever sees a payload inside the serialized budget, which is + // what bounds the walk: every raw JSON value costs at least one character. + if (oversized) { + return violations; + } + // Iterative DFS over the RAW value (every object/array, regardless of node // type): the recursive Zod parser's depth is driven by raw JSON nesting, // not by grammar-correct nesting. Only depth is checked here — the