From c4b600e944c51c0a83be0b525e9e8f29ec32612f Mon Sep 17 00:00:00 2001 From: adilallo <39313955+adilallo@users.noreply.github.com> Date: Sat, 4 Apr 2026 22:37:46 -0600 Subject: [PATCH] Formalize CreateFlowState + validate draft/publish API payloads --- app/api/drafts/me/route.ts | 31 +++--- app/api/rules/route.ts | 43 +++----- app/create/types.ts | 14 ++- docs/backend-linear-tickets.md | 17 +-- lib/create/api.ts | 24 ++++- lib/server/validation/createFlowSchemas.ts | 75 +++++++++++++ lib/server/validation/plainJson.ts | 73 +++++++++++++ lib/server/validation/requestBody.ts | 48 +++++++++ lib/server/validation/zodHttp.ts | 17 +++ package-lock.json | 4 +- package.json | 5 +- tests/unit/createFlowValidation.test.ts | 120 +++++++++++++++++++++ 12 files changed, 409 insertions(+), 62 deletions(-) create mode 100644 lib/server/validation/createFlowSchemas.ts create mode 100644 lib/server/validation/plainJson.ts create mode 100644 lib/server/validation/requestBody.ts create mode 100644 lib/server/validation/zodHttp.ts create mode 100644 tests/unit/createFlowValidation.test.ts diff --git a/app/api/drafts/me/route.ts b/app/api/drafts/me/route.ts index bfdec26..51c379e 100644 --- a/app/api/drafts/me/route.ts +++ b/app/api/drafts/me/route.ts @@ -1,8 +1,12 @@ +import type { Prisma } from "@prisma/client"; import { NextRequest, NextResponse } from "next/server"; import { prisma } from "../../../../lib/server/db"; import { isDatabaseConfigured } from "../../../../lib/server/env"; import { dbUnavailable } from "../../../../lib/server/responses"; import { getSessionUser } from "../../../../lib/server/session"; +import { putDraftBodySchema } from "../../../../lib/server/validation/createFlowSchemas"; +import { readLimitedJson } from "../../../../lib/server/validation/requestBody"; +import { jsonFromZodError } from "../../../../lib/server/validation/zodHttp"; export async function GET() { if (!isDatabaseConfigured()) { @@ -33,33 +37,28 @@ export async function PUT(request: NextRequest) { return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); } - let body: unknown; - try { - body = await request.json(); - } catch { - return NextResponse.json({ error: "Invalid JSON" }, { status: 400 }); + const parsedBody = await readLimitedJson(request); + if (parsedBody.ok === false) { + return parsedBody.response; } - if (!body || typeof body !== "object" || !("payload" in body)) { - return NextResponse.json({ error: "payload required" }, { status: 400 }); + const validated = putDraftBodySchema.safeParse(parsedBody.value); + if (!validated.success) { + return jsonFromZodError(validated.error); } - const payload = (body as { payload: unknown }).payload; - if (payload === undefined || typeof payload !== "object" || payload === null) { - return NextResponse.json( - { error: "payload must be a JSON object" }, - { status: 400 }, - ); - } + const { payload } = validated.data; + + const jsonPayload = payload as Prisma.InputJsonValue; const draft = await prisma.ruleDraft.upsert({ where: { userId: user.id }, create: { userId: user.id, - payload: payload as object, + payload: jsonPayload, }, update: { - payload: payload as object, + payload: jsonPayload, }, }); diff --git a/app/api/rules/route.ts b/app/api/rules/route.ts index cd63d8a..5b9d352 100644 --- a/app/api/rules/route.ts +++ b/app/api/rules/route.ts @@ -1,8 +1,12 @@ +import type { Prisma } from "@prisma/client"; import { NextRequest, NextResponse } from "next/server"; import { prisma } from "../../../lib/server/db"; import { isDatabaseConfigured } from "../../../lib/server/env"; import { dbUnavailable } from "../../../lib/server/responses"; import { getSessionUser } from "../../../lib/server/session"; +import { publishRuleBodySchema } from "../../../lib/server/validation/createFlowSchemas"; +import { readLimitedJson } from "../../../lib/server/validation/requestBody"; +import { jsonFromZodError } from "../../../lib/server/validation/zodHttp"; export async function GET(request: NextRequest) { if (!isDatabaseConfigured()) { @@ -37,43 +41,24 @@ export async function POST(request: NextRequest) { return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); } - let body: unknown; - try { - body = await request.json(); - } catch { - return NextResponse.json({ error: "Invalid JSON" }, { status: 400 }); + const parsedBody = await readLimitedJson(request); + if (parsedBody.ok === false) { + return parsedBody.response; } - if (!body || typeof body !== "object") { - return NextResponse.json({ error: "Invalid body" }, { status: 400 }); + const validated = publishRuleBodySchema.safeParse(parsedBody.value); + if (!validated.success) { + return jsonFromZodError(validated.error); } - const { title, summary, document } = body as { - title?: unknown; - summary?: unknown; - document?: unknown; - }; - - if (typeof title !== "string" || title.trim().length === 0) { - return NextResponse.json({ error: "title required" }, { status: 400 }); - } - - if (document === undefined || typeof document !== "object" || document === null) { - return NextResponse.json( - { error: "document must be a JSON object" }, - { status: 400 }, - ); - } + const { title, summary, document } = validated.data; const rule = await prisma.publishedRule.create({ data: { userId: user.id, - title: title.trim(), - summary: - typeof summary === "string" && summary.trim().length > 0 - ? summary.trim() - : null, - document: document as object, + title, + summary, + document: document as Prisma.InputJsonValue, }, }); diff --git a/app/create/types.ts b/app/create/types.ts index 815cb1a..5803f42 100644 --- a/app/create/types.ts +++ b/app/create/types.ts @@ -21,11 +21,19 @@ export type CreateFlowStep = | "completed"; /** - * Flow state interface for storing user inputs across all steps - * Will be expanded in CR-56 with specific field definitions + * Flow state for inputs across create-flow steps. + * Validated on `PUT /api/drafts/me` via `createFlowStateSchema` (Zod + JSON safety checks). + * Additional string keys are allowed at runtime for forward-compatible step data. */ export interface CreateFlowState { - // Placeholder structure - will be expanded in CR-56 + title?: string; + summary?: string; + currentStep?: CreateFlowStep; + /** Section drafts; structure will tighten as steps persist real shapes. */ + sections?: Record[]; + /** Stakeholder placeholders until the confirm-stakeholders step defines a schema. */ + stakeholders?: Record[]; + /** Extra step-specific fields (must be JSON-serializable for server draft sync). */ [key: string]: unknown; } diff --git a/docs/backend-linear-tickets.md b/docs/backend-linear-tickets.md index 02924f9..8c9bbb4 100644 --- a/docs/backend-linear-tickets.md +++ b/docs/backend-linear-tickets.md @@ -101,11 +101,13 @@ Optional: **Docker image deploy** using the repo [Dockerfile](Dockerfile)—admi **Acceptance criteria:** -- [ ] TypeScript reflects the real shape of `CreateFlowState` (no unnecessary `unknown` for known keys). -- [ ] Invalid draft/publish requests return 400, not 500. -- [ ] Unit tests for schemas (Vitest) or route tests with MSW. +- [x] TypeScript reflects the real shape of `CreateFlowState` (no unnecessary `unknown` for known keys). +- [x] Invalid draft/publish requests return 400, not 500. +- [x] Unit tests for schemas (Vitest) or route tests with MSW. -**Files:** [app/create/types.ts](app/create/types.ts), [app/api/drafts/me/route.ts](app/api/drafts/me/route.ts), [app/api/rules/route.ts](app/api/rules/route.ts), new `lib/server/validation/` or `lib/validation/createFlow.ts`, [package.json](package.json) if adding `zod`. +**Status:** [CR-73](https://linear.app/community-rule/issue/CR-73/backend-formalize-createflowstate-validate-draftpublish-api-payloads) **Done**. + +**Files:** [app/create/types.ts](app/create/types.ts), [app/api/drafts/me/route.ts](app/api/drafts/me/route.ts), [app/api/rules/route.ts](app/api/rules/route.ts), [lib/server/validation/](lib/server/validation/) (Zod + plain-JSON checks), [package.json](package.json) (`zod`). **Note:** Repo-wide **API error JSON shape** and **request-id logging** are **Ticket 13 / CR-84**—coordinate 400 response bodies with that issue so validation errors match the agreed `{ error: { code, message } }` pattern. @@ -175,15 +177,16 @@ Optional: **Docker image deploy** using the repo [Dockerfile](Dockerfile)—admi 1. **Hydration:** Show a non-blocking “Loading your saved progress…” until first session + draft fetch completes (only when sync enabled). 2. **Conflict:** If `localStorage` has non-empty state and server returns non-empty draft, pick a policy: prefer server with confirm modal, or prefer newer `updatedAt` (requires storing timestamp client-side). Document choice in code comment. -3. **Save failures:** If `PUT /api/drafts/me` fails, show toast/banner; optionally retry with backoff. -4. **Tests:** Component test or Playwright scenario with sync flag on (may require test DB or route mocks). +3. **Save failures (API surface):** Change [saveDraftToServer](lib/create/api.ts) from `Promise` to a result type such as `{ ok: true } | { ok: false; message: string; status?: number }`, parsing the response body with [readApiErrorMessage](lib/create/api.ts) so both legacy `{ error: string }` and CR-73 validation `{ error: { message } }` (and 413 `payload_too_large`) produce a useful `message`. Update [CreateFlowBackendSync](app/create/context/CreateFlowBackendSync.tsx) to branch on that result. +4. **Save failures (UX):** On `ok: false`, show toast/banner (include `message`); optionally retry with backoff. +5. **Tests:** Component test or Playwright scenario with sync flag on (may require test DB or route mocks). **Acceptance criteria:** - [ ] No silent data loss when server save fails. - [ ] User understands when server draft replaced local state (if applicable). -**Files:** [app/create/context/CreateFlowBackendSync.tsx](app/create/context/CreateFlowBackendSync.tsx), possibly [CreateFlowContext](app/create/context/CreateFlowContext.tsx), tests under `tests/`. +**Files:** [lib/create/api.ts](lib/create/api.ts), [app/create/context/CreateFlowBackendSync.tsx](app/create/context/CreateFlowBackendSync.tsx), possibly [CreateFlowContext](app/create/context/CreateFlowContext.tsx), tests under `tests/`. --- diff --git a/lib/create/api.ts b/lib/create/api.ts index 258b856..7b71d8f 100644 --- a/lib/create/api.ts +++ b/lib/create/api.ts @@ -7,6 +7,24 @@ async function parseJson(response: Response): Promise { return data; } +/** Supports legacy `{ error: string }` and `{ error: { message: string } }` from API routes. */ +function readApiErrorMessage(data: unknown): string { + if (!data || typeof data !== "object" || !("error" in data)) { + return "Request failed"; + } + const err = (data as { error: unknown }).error; + if (typeof err === "string") { + return err; + } + if (err && typeof err === "object" && "message" in err) { + const m = (err as { message: unknown }).message; + if (typeof m === "string") { + return m; + } + } + return "Request failed"; +} + export async function fetchAuthSession(): Promise<{ user: { id: string; email: string } | null; }> { @@ -28,7 +46,7 @@ export async function requestOtp(email: string): Promise<{ ok: true } | { error: }); const data = await parseJson<{ error?: string }>(res); if (!res.ok) { - return { error: data.error ?? "Request failed" }; + return { error: readApiErrorMessage(data) }; } return { ok: true }; } @@ -50,7 +68,7 @@ export async function verifyOtp( user?: { id: string; email: string }; }>(res); if (!res.ok || !data.user) { - return { error: data.error ?? "Verification failed" }; + return { error: readApiErrorMessage(data) }; } return { ok: true, user: data.user }; } @@ -106,7 +124,7 @@ export async function publishRule(input: { rule?: { id: string; title: string }; }>(res); if (!res.ok || !data.rule) { - return { error: data.error ?? "Publish failed" }; + return { error: readApiErrorMessage(data) }; } return { ok: true, id: data.rule.id, title: data.rule.title }; } diff --git a/lib/server/validation/createFlowSchemas.ts b/lib/server/validation/createFlowSchemas.ts new file mode 100644 index 0000000..2b67526 --- /dev/null +++ b/lib/server/validation/createFlowSchemas.ts @@ -0,0 +1,75 @@ +import { z } from "zod"; +import { FLOW_STEP_ORDER } from "../../../app/create/utils/flowSteps"; +import { + assertPlainJsonValue, + DEFAULT_PLAIN_JSON_LIMITS, +} from "./plainJson"; + +const flowStepTuple = FLOW_STEP_ORDER as unknown as [ + string, + ...string[], +]; + +const createFlowStepSchema = z.enum(flowStepTuple); + +/** + * Published rule `document` column: arbitrary JSON object with safety bounds. + */ +export const publishedRuleDocumentSchema = z + .record(z.string(), z.unknown()) + .superRefine((doc, ctx) => { + const err = assertPlainJsonValue(doc, 0, DEFAULT_PLAIN_JSON_LIMITS); + if (err) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: err, + }); + } + }); + +/** + * Create-flow draft payload: known optional fields plus passthrough for future steps. + * Full tree must satisfy {@link assertPlainJsonValue}. + */ +export const createFlowStateSchema = z + .object({ + title: z.string().max(500).optional(), + summary: z.string().max(8000).optional(), + currentStep: createFlowStepSchema.optional(), + sections: z.array(z.unknown()).optional(), + stakeholders: z.array(z.unknown()).optional(), + }) + .passthrough() + .superRefine((data, ctx) => { + const err = assertPlainJsonValue(data, 0, DEFAULT_PLAIN_JSON_LIMITS); + if (err) { + ctx.addIssue({ code: z.ZodIssueCode.custom, message: err }); + } + }); + +export const publishRuleBodySchema = z + .object({ + title: z + .string() + .max(500) + .transform((s) => s.trim()) + .refine((s) => s.length > 0, { message: "title required" }), + summary: z + .union([z.string().max(8000), z.null()]) + .optional() + .transform((val) => { + if (val === undefined || val === null) { + return null; + } + const t = val.trim(); + return t.length > 0 ? t : null; + }), + document: publishedRuleDocumentSchema, + }); + +export type PublishRuleBody = z.infer; + +export const putDraftBodySchema = z.object({ + payload: createFlowStateSchema, +}); +export type CreateFlowStateValidated = z.infer; diff --git a/lib/server/validation/plainJson.ts b/lib/server/validation/plainJson.ts new file mode 100644 index 0000000..8874017 --- /dev/null +++ b/lib/server/validation/plainJson.ts @@ -0,0 +1,73 @@ +/** + * Validates that a value is JSON-like (finite numbers, plain objects, no prototype tricks). + * Used after JSON.parse for defense in depth against odd clients. + */ + +export const DEFAULT_PLAIN_JSON_LIMITS = { + maxDepth: 40, + maxStringLength: 50_000, + maxArrayLength: 5_000, + maxObjectKeys: 500, +} as const; + +export type PlainJsonLimits = typeof DEFAULT_PLAIN_JSON_LIMITS; + +/** + * @returns `null` if valid, otherwise a short error message for API responses. + */ +export function assertPlainJsonValue( + val: unknown, + depth: number, + limits: PlainJsonLimits = DEFAULT_PLAIN_JSON_LIMITS, +): string | null { + if (depth > limits.maxDepth) { + return "Maximum nesting depth exceeded"; + } + if (val === null) { + return null; + } + const t = typeof val; + if (t === "string") { + const s = val as string; + return s.length > limits.maxStringLength ? "String value too long" : null; + } + if (t === "number") { + return Number.isFinite(val as number) ? null : "Invalid number value"; + } + if (t === "boolean") { + return null; + } + if (t === "bigint" || t === "function" || t === "symbol") { + return "Invalid value type"; + } + if (Array.isArray(val)) { + if (val.length > limits.maxArrayLength) { + return "Array too long"; + } + for (let i = 0; i < val.length; i++) { + const inner = assertPlainJsonValue(val[i], depth + 1, limits); + if (inner) { + return inner; + } + } + return null; + } + if (t === "object") { + const o = val as Record; + const keys = Object.keys(o); + if (keys.length > limits.maxObjectKeys) { + return "Object has too many keys"; + } + for (const k of keys) { + if (k === "__proto__" || k === "constructor" || k === "prototype") { + return "Unsafe object key"; + } + const inner = assertPlainJsonValue(o[k], depth + 1, limits); + if (inner) { + return inner; + } + } + return null; + } + return "Invalid value type"; +} diff --git a/lib/server/validation/requestBody.ts b/lib/server/validation/requestBody.ts new file mode 100644 index 0000000..a963b63 --- /dev/null +++ b/lib/server/validation/requestBody.ts @@ -0,0 +1,48 @@ +import { NextRequest, NextResponse } from "next/server"; + +export const MAX_JSON_BODY_BYTES = 512 * 1024; + +export type LimitedJsonResult = + | { ok: true; value: unknown } + | { ok: false; response: NextResponse }; + +/** + * Read the body as text (bounded by maxBytes), then JSON.parse. + * Returns 413 when over limit; 400 when JSON is invalid. + */ +export async function readLimitedJson( + request: NextRequest, + maxBytes: number = MAX_JSON_BODY_BYTES, +): Promise { + const text = await request.text(); + if (text.length > maxBytes) { + return { + ok: false, + response: NextResponse.json( + { + error: { + code: "payload_too_large", + message: `Request body must be at most ${maxBytes} bytes`, + }, + }, + { status: 413 }, + ), + }; + } + try { + return { ok: true, value: JSON.parse(text) as unknown }; + } catch { + return { + ok: false, + response: NextResponse.json( + { + error: { + code: "invalid_json", + message: "Invalid JSON", + }, + }, + { status: 400 }, + ), + }; + } +} diff --git a/lib/server/validation/zodHttp.ts b/lib/server/validation/zodHttp.ts new file mode 100644 index 0000000..2e61bee --- /dev/null +++ b/lib/server/validation/zodHttp.ts @@ -0,0 +1,17 @@ +import { NextResponse } from "next/server"; +import type { ZodError } from "zod"; + +export function jsonFromZodError(error: ZodError): NextResponse { + const issue = error.issues[0]; + const message = issue?.message ?? "Validation failed"; + return NextResponse.json( + { + error: { + code: "validation_error", + message, + }, + details: error.flatten(), + }, + { status: 400 }, + ); +} diff --git a/package-lock.json b/package-lock.json index d463b10..656affd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -20,7 +20,8 @@ "next-intl": "^3.26.5", "nodemailer": "^6.9.16", "react": "^19.0.0", - "react-dom": "^19.0.0" + "react-dom": "^19.0.0", + "zod": "^3.25.76" }, "devDependencies": { "@axe-core/playwright": "^4.10.2", @@ -23129,7 +23130,6 @@ "version": "3.25.76", "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==", - "dev": true, "license": "MIT", "funding": { "url": "https://github.com/sponsors/colinhacks" diff --git a/package.json b/package.json index d8f72d6..67dbf5c 100644 --- a/package.json +++ b/package.json @@ -49,15 +49,16 @@ "@mdx-js/loader": "^3.1.1", "@mdx-js/react": "^3.1.1", "@next/mdx": "^16.0.0", + "@prisma/client": "^6.19.0", "ajv": "^8.12.0", "critters": "^0.0.23", "gray-matter": "^4.0.3", "next": "^16.0.0", "next-intl": "^3.26.5", "nodemailer": "^6.9.16", - "@prisma/client": "^6.19.0", "react": "^19.0.0", - "react-dom": "^19.0.0" + "react-dom": "^19.0.0", + "zod": "^3.25.76" }, "devDependencies": { "@axe-core/playwright": "^4.10.2", diff --git a/tests/unit/createFlowValidation.test.ts b/tests/unit/createFlowValidation.test.ts new file mode 100644 index 0000000..af7ca6d --- /dev/null +++ b/tests/unit/createFlowValidation.test.ts @@ -0,0 +1,120 @@ +import { describe, it, expect } from "vitest"; +import { + assertPlainJsonValue, + DEFAULT_PLAIN_JSON_LIMITS, +} from "../../lib/server/validation/plainJson"; +import { + createFlowStateSchema, + publishRuleBodySchema, + putDraftBodySchema, +} from "../../lib/server/validation/createFlowSchemas"; + +describe("assertPlainJsonValue", () => { + it("accepts plain JSON structures", () => { + expect( + assertPlainJsonValue( + { a: [1, "x", { b: null }], c: true }, + 0, + DEFAULT_PLAIN_JSON_LIMITS, + ), + ).toBeNull(); + }); + + it("rejects __proto__ keys", () => { + const obj = JSON.parse('{"__proto__": {"x": 1}}') as Record; + expect(assertPlainJsonValue(obj, 0, DEFAULT_PLAIN_JSON_LIMITS)).toBe( + "Unsafe object key", + ); + }); + + it("rejects non-finite numbers", () => { + expect(assertPlainJsonValue(Number.NaN, 0, DEFAULT_PLAIN_JSON_LIMITS)).toBe( + "Invalid number value", + ); + }); + + it("rejects excessive depth", () => { + let v: unknown = 1; + for (let i = 0; i < 50; i++) { + v = { x: v }; + } + expect(assertPlainJsonValue(v, 0, DEFAULT_PLAIN_JSON_LIMITS)).toBe( + "Maximum nesting depth exceeded", + ); + }); +}); + +describe("createFlowStateSchema", () => { + it("accepts empty object", () => { + const r = createFlowStateSchema.safeParse({}); + expect(r.success).toBe(true); + }); + + it("accepts known fields and passthrough keys", () => { + const r = createFlowStateSchema.safeParse({ + title: "My rule", + currentStep: "cards", + customField: { nested: [1, 2] }, + }); + expect(r.success).toBe(true); + }); + + it("rejects invalid currentStep", () => { + const r = createFlowStateSchema.safeParse({ currentStep: "not-a-step" }); + expect(r.success).toBe(false); + }); + + it("rejects title that is too long", () => { + const r = createFlowStateSchema.safeParse({ title: "x".repeat(600) }); + expect(r.success).toBe(false); + }); +}); + +describe("putDraftBodySchema", () => { + it("requires payload object", () => { + expect(putDraftBodySchema.safeParse({}).success).toBe(false); + expect(putDraftBodySchema.safeParse({ payload: {} }).success).toBe(true); + }); +}); + +describe("publishRuleBodySchema", () => { + it("accepts minimal valid body", () => { + const r = publishRuleBodySchema.safeParse({ + title: " Hello ", + document: { body: "text" }, + }); + expect(r.success).toBe(true); + if (r.success) { + expect(r.data.title).toBe("Hello"); + expect(r.data.summary).toBeNull(); + } + }); + + it("trims summary and maps empty to null", () => { + const r = publishRuleBodySchema.safeParse({ + title: "T", + summary: " ", + document: {}, + }); + expect(r.success).toBe(true); + if (r.success) { + expect(r.data.summary).toBeNull(); + } + }); + + it("rejects empty title", () => { + const r = publishRuleBodySchema.safeParse({ + title: " ", + document: {}, + }); + expect(r.success).toBe(false); + }); + + it("rejects non-object document", () => { + const r = publishRuleBodySchema.safeParse({ + title: "Ok", + document: "nope", + }); + expect(r.success).toBe(false); + }); +});