From abc5c863fd7795ad421fb66c9adedfc29d4a69e8 Mon Sep 17 00:00:00 2001 From: Joey Yakimowich-Payne Date: Mon, 20 Apr 2026 17:53:22 -0600 Subject: [PATCH] fix(modifiers): close client schema/server schema drift + add parity test (Q4.2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The T3 audit flagged the Zod v3 / v4 hand-mirrored schemas as a silent-drift risk: a bug where the client accepts what the server rejects (or vice versa) would silently degrade gameplay rather than fail loudly. New cross-package parity test at packages/server/src/custom-modifier-wire-parity.test.ts imports BOTH schemas and asserts they agree on an accept/reject matrix of 17 cases (valid minimal, valid rich, valid optional-field combinations + 12 rejection cases covering type/version/name/description/uiForm/source literals, bounds, empty id, oversized primitives/description, and primitive-node empty-kind). A final round-trip case parses on the client, JSON-serializes, and parses on the server — catching stringification edge cases too. First run surfaced a real drift: the client schema was missing the primitives.max(50) cap that the server schema enforces. A malicious or buggy client could construct an oversized descriptor, get past local validation, then hit the server's rejection. Fixed by adding matching caps (primitives.max(50) + targetAttrs.max(32) + author.max(80)) to the client schema. Barrel export: chess/src/index.ts now re-exports CustomModifierDescriptorSchema + EffectPrimitiveNodeSchema + the parse/serialize helpers so the server parity test can import them without reaching into subpaths. 1400 → 1417 unit tests. --- packages/chess/src/index.ts | 12 ++ packages/chess/src/modifiers/custom/schema.ts | 10 +- .../src/custom-modifier-wire-parity.test.ts | 174 ++++++++++++++++++ 3 files changed, 193 insertions(+), 3 deletions(-) create mode 100644 packages/server/src/custom-modifier-wire-parity.test.ts diff --git a/packages/chess/src/index.ts b/packages/chess/src/index.ts index 6a708c9..d01c11b 100644 --- a/packages/chess/src/index.ts +++ b/packages/chess/src/index.ts @@ -87,3 +87,15 @@ export type { // new ModifierProfile to a live session at a turn boundary without // reaching into engine internals. export { reconcileProfileSwap } from "./modifiers/reconcile.js"; + +// T3 custom modifier schema + parser. Exported so the server +// package can run the cross-package wire-shape parity test (Q4.2) +// — it imports both the v4 client schema and the v3 server schema +// and asserts they agree on the accept/reject matrix. +export { + CustomModifierDescriptorSchema, + EffectPrimitiveNodeSchema, + parseCustomModifierDescriptor, + safeParseCustomModifierDescriptor, + serializeCustomModifierDescriptor, +} from "./modifiers/custom/schema.js"; diff --git a/packages/chess/src/modifiers/custom/schema.ts b/packages/chess/src/modifiers/custom/schema.ts index 3e899ef..eae07b6 100644 --- a/packages/chess/src/modifiers/custom/schema.ts +++ b/packages/chess/src/modifiers/custom/schema.ts @@ -53,11 +53,15 @@ export const CustomModifierDescriptorSchema = z.object({ name: z.string().min(1).max(40), description: z.string().max(200), version: z.literal(1), - primitives: z.array(EffectPrimitiveNodeSchema), - targetAttrs: z.array(z.string()), + // Cap matches the server-side wire schema's .max(50) and the + // validator's MAX_PRIMITIVE_COUNT. Drift between the three caps + // is caught by the cross-package parity test (Q4.2). + primitives: z.array(EffectPrimitiveNodeSchema).max(50), + // Also bounded for parity with the server schema. + targetAttrs: z.array(z.string()).max(32), uiForm: z.literal("primitive-composer"), source: z.literal("custom"), - author: z.string().optional(), + author: z.string().max(80).optional(), createdAt: z.number().int().nonnegative().optional(), }); diff --git a/packages/server/src/custom-modifier-wire-parity.test.ts b/packages/server/src/custom-modifier-wire-parity.test.ts new file mode 100644 index 0000000..4a94f89 --- /dev/null +++ b/packages/server/src/custom-modifier-wire-parity.test.ts @@ -0,0 +1,174 @@ +/** + * Cross-package wire-shape parity test (T3 audit Q4.2). + * + * The chess package (Zod v4) and server package (Zod v3) carry + * hand-mirrored schemas for CustomModifierDescriptor. Drift between + * them manifests as "client accepts a descriptor the server + * rejects" or vice-versa — a silent class of forwards-compat bug. + * + * This test parses a matrix of descriptors through BOTH schemas and + * asserts agreement on accept/reject. It uses the actual chess + * package's v4 schema (imported) so any upstream schema change + * surfaces here as a test failure. + * + * NOT a full property-based fuzz — the matrix is hand-picked cases + * covering the known-risky shapes (optional fields, literal + * discriminators, recursive primitive nodes, bounds on name / + * description, ids). A future fuzzer can grow this matrix. + */ +import { describe, expect, it } from "vitest"; +import { + CustomModifierDescriptorSchema as ClientSchema, + parseCustomModifierDescriptor, +} from "@paratype/chess"; +import { CustomModifierDescriptorSchema as ServerSchema } from "./protocol.js"; + +function checkAgreement(descriptor: unknown): { + client: boolean; + server: boolean; +} { + return { + client: ClientSchema.safeParse(descriptor).success, + server: ServerSchema.safeParse(descriptor).success, + }; +} + +describe("wire-shape parity: client v4 ↔ server v3 (Q4.2)", () => { + const validMinimal = { + type: "data", + id: "custom:parity-min", + name: "Parity Min", + description: "", + version: 1, + primitives: [], + targetAttrs: [], + uiForm: "primitive-composer", + source: "custom", + }; + + const validRich = { + ...validMinimal, + id: "custom:parity-rich", + name: "Parity Rich", + description: "Covers optional fields and nested primitives.", + primitives: [ + { kind: "seed-attribute", params: { attr: "HpBonus", value: 3 } }, + { + kind: "on-turn-start", + params: { + primitives: [ + { + kind: "add-to-attribute", + params: { attr: "HpBonus", delta: 1 }, + }, + ], + }, + }, + ], + targetAttrs: ["HpBonus"], + author: "test-author", + createdAt: 1700000000000, + }; + + describe("both schemas accept", () => { + it("minimal valid descriptor", () => { + const { client, server } = checkAgreement(validMinimal); + expect(client).toBe(true); + expect(server).toBe(true); + }); + + it("rich descriptor with optional author + createdAt + nested trigger", () => { + const { client, server } = checkAgreement(validRich); + expect(client).toBe(true); + expect(server).toBe(true); + }); + + it("descriptor with only author (no createdAt)", () => { + const desc = { ...validMinimal, author: "only-author" }; + const { client, server } = checkAgreement(desc); + expect(client).toBe(true); + expect(server).toBe(true); + }); + + it("descriptor with only createdAt (no author)", () => { + const desc = { ...validMinimal, createdAt: 1234567890 }; + const { client, server } = checkAgreement(desc); + expect(client).toBe(true); + expect(server).toBe(true); + }); + }); + + describe("both schemas reject", () => { + const rejectionCases: Array<[string, unknown]> = [ + ["type !== 'data'", { ...validMinimal, type: "scripted" }], + ["version !== 1", { ...validMinimal, version: 2 }], + ["missing id", (() => { + const { id, ...rest } = validMinimal; + void id; + return rest; + })()], + ["empty id", { ...validMinimal, id: "" }], + [ + "name too long (41 chars)", + { ...validMinimal, name: "x".repeat(41) }, + ], + ["empty name", { ...validMinimal, name: "" }], + [ + "description too long (201 chars)", + { ...validMinimal, description: "y".repeat(201) }, + ], + ["uiForm !== 'primitive-composer'", { ...validMinimal, uiForm: "json" }], + ["source !== 'custom'", { ...validMinimal, source: "premade" }], + ["negative createdAt", { ...validMinimal, createdAt: -1 }], + [ + "primitive node with empty kind", + { + ...validMinimal, + primitives: [{ kind: "", params: {} }], + }, + ], + [ + "51 primitives (exceeds server .max(50))", + { + ...validMinimal, + primitives: Array.from({ length: 51 }, () => ({ + kind: "seed-attribute", + params: { attr: "HpBonus", value: 1 }, + })), + }, + ], + ]; + + for (const [label, desc] of rejectionCases) { + it(label, () => { + const { client, server } = checkAgreement(desc); + // Both must reject. If only ONE rejects, that's drift — the + // whole point of this test is to catch that divergence. + expect(client).toBe(false); + expect(server).toBe(false); + }); + } + }); + + describe("round-trip", () => { + it("parse on client, serialize, parse on server — shape preserved", () => { + // The chess-side parse returns a typed CustomModifierDescriptor. + // We re-serialize via JSON.stringify / JSON.parse (simulating + // the actual wire trip) and feed through the server schema. + const parsed = parseCustomModifierDescriptor(validRich); + const wire = JSON.parse(JSON.stringify(parsed)) as unknown; + const serverParse = ServerSchema.safeParse(wire); + expect(serverParse.success).toBe(true); + if (!serverParse.success) return; + // Key invariants preserved across the trip: + expect((serverParse.data as { id: string }).id).toBe( + "custom:parity-rich", + ); + expect((serverParse.data as { type: string }).type).toBe("data"); + expect((serverParse.data as { version: number }).version).toBe(1); + expect( + (serverParse.data as { primitives: unknown[] }).primitives, + ).toHaveLength(2); + }); + }); +});