From 24bdae29ca489f37e33403f71fa98d25899d3c26 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Sat, 8 Aug 2026 10:35:18 -0300 Subject: [PATCH] =?UTF-8?q?fix(api):=20validate=20request=20bodies=20with?= =?UTF-8?q?=20Zod=20in=204=20routes=20=E2=80=94=20restores=20the=20t06=20g?= =?UTF-8?q?ate=20(#9779)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The release-green verdict (#9737) lists check:route-validation:t06 as a HARD failure and it is STILL red on the current tip: four routes call request.json() and hand-roll `typeof x === "string"` checks instead of using Zod, which Hard Rule #7 requires and the gate enforces (it scans source and has no allowlist). - src/app/api/plugins/marketplace/install (#9445): InstallBodySchema; the 400 'Missing or invalid name field' response is preserved verbatim. - src/app/api/services/dario/admin/accounts (#8523): DeleteAccountBodySchema for the optional { alias } DELETE body; query-param path untouched. - src/app/api/services/dario/admin/login-start (#8523): LoginStartBodySchema; trimming now happens in the schema, so the forward body is unchanged. - src/app/api/services/dario/admin/import-from-omniroute (#8523): ImportBodySchema for connectionId/alias; invalid shapes fall back to the same 'connectionId is required' 400 as before. All four keep their exact status codes and messages — this is a validation mechanism swap, not a contract change (plugins route suite still 33/33). Adds tests/unit/route-body-validation-t06.test.ts, which runs the gate's own rule inside the unit suite so the next such route fails on ITS OWN PR instead of surfacing weeks later in a base-red sweep. Guard verified by mutation: renaming .safeParse( in one route makes it fail (1 fail), restored from a pre-probe copy. Gates: route-validation:t06, file-size, test-discovery, mutation-test-coverage, dead-code exit 0; typecheck:core clean; eslint clean. Refs #9737 Co-authored-by: diegosouzapw --- .../fixes/9737-route-body-validation.md | 1 + .../api/plugins/marketplace/install/route.ts | 12 +++-- .../services/dario/admin/accounts/route.ts | 12 +++-- .../admin/import-from-omniroute/route.ts | 20 ++++--- .../services/dario/admin/login-start/route.ts | 14 +++-- tests/unit/route-body-validation-t06.test.ts | 54 +++++++++++++++++++ 6 files changed, 94 insertions(+), 19 deletions(-) create mode 100644 changelog.d/fixes/9737-route-body-validation.md create mode 100644 tests/unit/route-body-validation-t06.test.ts diff --git a/changelog.d/fixes/9737-route-body-validation.md b/changelog.d/fixes/9737-route-body-validation.md new file mode 100644 index 0000000000..3c9f16a39c --- /dev/null +++ b/changelog.d/fixes/9737-route-body-validation.md @@ -0,0 +1 @@ +- Replaced hand-rolled body type checks with Zod validation in the plugins marketplace install route and the three Dario admin routes, restoring the `t06:route-validation` gate (Hard Rule #7). diff --git a/src/app/api/plugins/marketplace/install/route.ts b/src/app/api/plugins/marketplace/install/route.ts index f2c1b24e5b..4410fa013e 100644 --- a/src/app/api/plugins/marketplace/install/route.ts +++ b/src/app/api/plugins/marketplace/install/route.ts @@ -1,9 +1,14 @@ import { NextRequest, NextResponse } from "next/server"; +import { z } from "zod"; import { CORS_HEADERS, handleCorsOptions } from "@/shared/utils/cors"; import { buildErrorBody } from "@omniroute/open-sse/utils/error"; import { installMarketplacePlugin } from "@/lib/plugins/marketplace"; import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; +const InstallBodySchema = z.object({ + name: z.string().trim().min(1), +}); + export async function OPTIONS() { return handleCorsOptions(); } @@ -15,15 +20,14 @@ export async function POST(request: NextRequest) { const authError = await requireManagementAuth(request); if (authError) return authError; try { - const body = await request.json(); - const { name } = body as { name?: string }; - if (!name || typeof name !== "string") { + const parsed = InstallBodySchema.safeParse(await request.json()); + if (!parsed.success) { return NextResponse.json(buildErrorBody(400, "Missing or invalid 'name' field"), { status: 400, headers: CORS_HEADERS, }); } - const result = await installMarketplacePlugin(name); + const result = await installMarketplacePlugin(parsed.data.name); return NextResponse.json(result, { status: 201, headers: CORS_HEADERS }); } catch (err: unknown) { const msg = err instanceof Error ? err.message : "Failed to install marketplace plugin"; diff --git a/src/app/api/services/dario/admin/accounts/route.ts b/src/app/api/services/dario/admin/accounts/route.ts index 4b6865a5ce..50cdc3bcd9 100644 --- a/src/app/api/services/dario/admin/accounts/route.ts +++ b/src/app/api/services/dario/admin/accounts/route.ts @@ -11,9 +11,15 @@ * and never reaches the browser. */ +import { z } from "zod"; + import { forwardToDarioAdmin, requireAdminAuth } from "../_lib"; import { createErrorResponse } from "@/lib/api/errorResponse"; +const DeleteAccountBodySchema = z.object({ + alias: z.string().trim().min(1).optional(), +}); + export async function GET(request: Request): Promise { const authResponse = await requireAdminAuth(request); if (authResponse) return authResponse; @@ -29,9 +35,9 @@ export async function DELETE(request: Request): Promise { if (!alias && request.body !== null) { try { - const parsed = await request.json(); - if (parsed && typeof parsed === "object" && typeof (parsed as { alias?: unknown }).alias === "string") { - alias = (parsed as { alias: string }).alias.trim(); + const parsed = DeleteAccountBodySchema.safeParse(await request.json()); + if (parsed.success && parsed.data.alias) { + alias = parsed.data.alias; } } catch { /* fall through to the missing-alias error below */ diff --git a/src/app/api/services/dario/admin/import-from-omniroute/route.ts b/src/app/api/services/dario/admin/import-from-omniroute/route.ts index 2340b7532b..fd627b29af 100644 --- a/src/app/api/services/dario/admin/import-from-omniroute/route.ts +++ b/src/app/api/services/dario/admin/import-from-omniroute/route.ts @@ -28,6 +28,7 @@ * pickup, rather than relying on any undocumented hot-reload behavior. */ +import { z } from "zod"; import { NextResponse } from "next/server"; import fs from "node:fs"; import path from "node:path"; @@ -39,6 +40,11 @@ import { getDarioHomeDir } from "@/lib/services/installers/dario"; import { createErrorResponse } from "@/lib/api/errorResponse"; import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error"; +const ImportBodySchema = z.object({ + connectionId: z.string().trim().min(1).optional(), + alias: z.string().trim().min(1).optional(), +}); + const ALIAS_PATTERN = /^[A-Za-z0-9][A-Za-z0-9_\-.]{0,63}$/; function safeAliasFromSource(email: string | null | undefined, connectionId: string): string { @@ -82,15 +88,16 @@ export async function POST(request: Request): Promise { const authResponse = await requireAdminAuth(request); if (authResponse) return authResponse; - let body: unknown; + let raw: unknown; try { - body = await request.json(); + raw = await request.json(); } catch { return createErrorResponse({ status: 400, message: "Invalid JSON body" }); } - const b = (body || {}) as Record; - const connectionId = typeof b.connectionId === "string" ? b.connectionId : null; + const parsed = ImportBodySchema.safeParse(raw ?? {}); + const b = parsed.success ? parsed.data : {}; + const connectionId = b.connectionId ?? null; if (!connectionId) { return createErrorResponse({ status: 400, message: "connectionId is required" }); } @@ -112,10 +119,7 @@ export async function POST(request: Request): Promise { }); } - let alias = - typeof b.alias === "string" && b.alias.trim() - ? b.alias.trim() - : safeAliasFromSource(conn.email as string | null, connectionId); + let alias = b.alias || safeAliasFromSource(conn.email as string | null, connectionId); if (!ALIAS_PATTERN.test(alias)) { alias = safeAliasFromSource(conn.email as string | null, connectionId); } diff --git a/src/app/api/services/dario/admin/login-start/route.ts b/src/app/api/services/dario/admin/login-start/route.ts index 3920a3393c..55cf2678ec 100644 --- a/src/app/api/services/dario/admin/login-start/route.ts +++ b/src/app/api/services/dario/admin/login-start/route.ts @@ -8,23 +8,29 @@ * posts the displayed code to /login-complete. */ +import { z } from "zod"; import { forwardToDarioAdmin, requireAdminAuth } from "../_lib"; import { createErrorResponse } from "@/lib/api/errorResponse"; +const LoginStartBodySchema = z.object({ + alias: z.string().trim().min(1).optional(), +}); +type LoginStartBody = z.infer; + export async function POST(request: Request): Promise { const authResponse = await requireAdminAuth(request); if (authResponse) return authResponse; - let body: { alias?: string } = {}; + let body: LoginStartBody = {}; try { if (request.body !== null) { - const parsed = await request.json(); - if (parsed && typeof parsed === "object") body = parsed as { alias?: string }; + const parsed = LoginStartBodySchema.safeParse(await request.json()); + if (parsed.success) body = parsed.data; } } catch { return createErrorResponse({ status: 400, message: "Invalid JSON body" }); } - const forwardBody = typeof body.alias === "string" && body.alias.trim() ? { alias: body.alias.trim() } : {}; + const forwardBody = body.alias ? { alias: body.alias } : {}; return forwardToDarioAdmin({ method: "POST", path: "/admin/login/start", body: forwardBody }); } diff --git a/tests/unit/route-body-validation-t06.test.ts b/tests/unit/route-body-validation-t06.test.ts new file mode 100644 index 0000000000..8aaed20065 --- /dev/null +++ b/tests/unit/route-body-validation-t06.test.ts @@ -0,0 +1,54 @@ +/** + * Guard for the t06:route-validation gate (Hard Rule #7 — always validate inputs + * with Zod schemas). + * + * The gate (scripts/check/check-route-validation.mjs) is a source scan: any + * `route.ts` under src/app/api that calls `request.json()` must also call + * `validateBody()` or `.safeParse()`. It has NO allowlist, so a route that + * hand-rolls `typeof x === "string"` checks passes review but fails CI — which + * is exactly how four routes (#9445 marketplace install, #8523's three Dario + * admin routes) landed on release/v3.8.50 and kept the branch out of + * release-green (#9737). + * + * This test runs the same rule inside the unit suite so the violation surfaces + * on the PR that introduces it, instead of on the next base-red sweep. + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; + +const REPO_ROOT = path.resolve(import.meta.dirname, "..", ".."); +const API_ROOT = path.join(REPO_ROOT, "src", "app", "api"); + +function collectRouteFiles(dir: string): string[] { + const files: string[] = []; + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + files.push(...collectRouteFiles(full)); + } else if (entry.isFile() && entry.name === "route.ts") { + files.push(full); + } + } + return files; +} + +test("every API route reading request.json() validates it with Zod (t06)", () => { + const offenders: string[] = []; + + for (const file of collectRouteFiles(API_ROOT)) { + const source = fs.readFileSync(file, "utf8"); + if (!/request\.json\s*\(/.test(source)) continue; + if (/\bvalidateBody\s*\(/.test(source) || /\.safeParse\s*\(/.test(source)) continue; + offenders.push(path.relative(REPO_ROOT, file)); + } + + assert.deepEqual( + offenders, + [], + `routes call request.json() without validateBody()/.safeParse() — hand-rolled ` + + `typeof checks do not satisfy Hard Rule #7 and fail the t06 CI gate:\n ` + + offenders.join("\n ") + ); +});