mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 21:10:59 +00:00
Merge pull request #912 from MBombeck/fix/strictschemas
fix: refuse an unrecognised request field where the caller can act on it
This commit is contained in:
28 files changed
+1008
-160
No files matched your search
@@ -2,6 +2,33 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- **A misspelled field in a request is no longer accepted and thrown away.**
|
||||
Send `disableCoachh` where the server expects `disableCoach` and the answer
|
||||
used to be 200: the unrecognised key was dropped, whatever else the body
|
||||
carried was saved, and the caller was told its request had been honoured.
|
||||
Twenty-two endpoints now refuse a field they do not recognise, and the
|
||||
422 names the key that was wrong instead of only saying the body failed. The
|
||||
settings toggles, the restore and bulk-delete endpoints, message feedback,
|
||||
the admin user update, the diabetes and Coach switches, the onboarding
|
||||
acknowledgements, the provider chain, the chart overlay preferences and the
|
||||
two client-side reporters are the ones affected. A key name is bounded and
|
||||
stripped of anything that is not a plain identifier before it is echoed, so
|
||||
a hostile one cannot ride the error message back out.
|
||||
- Sync, layout, import and the read filters deliberately keep accepting a field
|
||||
they do not know, and now have a test saying so. The phone app ships optional
|
||||
fields ahead of the servers that will eventually read them, and a self-hosted
|
||||
install runs whichever release its operator last pulled, so a server quietly
|
||||
ignoring a field it has never heard of is what keeps an updated app working
|
||||
against an older install. Refusing there would turn one dropped field into a
|
||||
refused batch: a year of history lost to a single row, a dashboard that will
|
||||
not save, or a phone that silently stops registering for notifications. On a
|
||||
read filter a stray query parameter discards nothing at all, the OAuth
|
||||
endpoints are required by their specifications to ignore parameters they do
|
||||
not recognise, and an import file comes from another version or another tool
|
||||
by definition.
|
||||
|
||||
### Internal
|
||||
|
||||
- The column-reader sweep was wrong in both directions and had been for as long as it existed. It answers "does this column have a consumer" for every column in the schema, and it is the tool this project leans on to catch a schema change that ships its writer and forgets its reader. Its idea of a write was any `field:` key anywhere, so `where: { ticketHash }` counted as writing `ticketHash`: all twelve columns it reported were a lookup key or a cron's discovery predicate being filtered on, every one a false alarm. And because it only ever reported a column that HAD a write, a column with no write at all was the one thing it could not see. It now understands which Prisma argument a key sits in, follows a payload assembled into a variable before the call, credits raw SQL against the mapped column name, and reports a column no code touches at all as its own category. Twelve false alarms became nineteen findings that each hold up, four of them worth acting on. The matcher underneath is pinned by a test that was checked by breaking it three ways.
|
||||
|
||||
@@ -0,0 +1,304 @@
|
||||
import { readdirSync, readFileSync, statSync } from "node:fs";
|
||||
import { join, relative, resolve } from "node:path";
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
import ts from "typescript";
|
||||
|
||||
/**
|
||||
* Inline request-schema strictness inventory.
|
||||
*
|
||||
* A `z.object()` without `.strict()` STRIPS an unknown key and parses
|
||||
* clean. A client that sends `measuredAtt` for `measuredAt` gets a 200
|
||||
* and loses the value; nobody is told. `.strict()` turns that into a 422
|
||||
* that names the key.
|
||||
*
|
||||
* It is not a change to make everywhere. `.strict()` flips accept-and-
|
||||
* strip into reject, and this server is talked to by a native client on
|
||||
* TestFlight and by self-hosted installs that lag the client by an
|
||||
* arbitrary number of releases. On the surfaces where the client adds an
|
||||
* optional field and older servers are expected to ignore it, the strip
|
||||
* IS the compatibility mechanism, and taking it away turns a graceful
|
||||
* degrade into a hard failure — a whole 500-entry sync batch refused
|
||||
* rather than one field dropped.
|
||||
*
|
||||
* So each inline schema is a decision, and this test is where the
|
||||
* decision is written down. Every `z.object(...)` under a route file is
|
||||
* either `.strict()` or accounted for in {@link DELIBERATELY_OPEN} with
|
||||
* the reason it stays open. A new route cannot land without making the
|
||||
* call, and an existing open schema cannot quietly gain a sibling.
|
||||
*
|
||||
* Deliberately an inventory, not a policy: it does not claim strict is
|
||||
* the right default. It claims the choice was made on purpose.
|
||||
*/
|
||||
|
||||
const API_ROOT = resolve(__dirname, "..");
|
||||
|
||||
/**
|
||||
* Route files whose inline schemas stay open, with the count of open
|
||||
* schemas in each and why. `count` is part of the assertion: adding an
|
||||
* open schema to a listed file fails until the entry is updated, so the
|
||||
* reason below always covers everything it claims to cover.
|
||||
*/
|
||||
const DELIBERATELY_OPEN: Record<string, { count: number; reason: string }> = {
|
||||
// ---------------------------------------------------------------
|
||||
// 1. Query schemas fed a hand-built object. The handler names each
|
||||
// param it reads (`{ limit: searchParams.get("limit"), … }`), so an
|
||||
// unknown query param never reaches the schema at all. `.strict()`
|
||||
// here asserts nothing — it would be decoration on a parse that
|
||||
// cannot see the key it is supposed to reject.
|
||||
// ---------------------------------------------------------------
|
||||
"admin/app-logs/route.ts": { count: 1, reason: "hand-built query object" },
|
||||
"admin/audit-log/route.ts": { count: 1, reason: "hand-built query object" },
|
||||
"admin/host-metrics/route.ts": {
|
||||
count: 1,
|
||||
reason: "hand-built query object",
|
||||
},
|
||||
"analytics/range/route.ts": { count: 1, reason: "hand-built query object" },
|
||||
"insights/biomarker-assessment/route.ts": {
|
||||
count: 1,
|
||||
reason: "hand-built query object",
|
||||
},
|
||||
"insights/coach-read/route.ts": {
|
||||
count: 1,
|
||||
reason: "hand-built query object",
|
||||
},
|
||||
"insights/derived/batch/route.ts": {
|
||||
count: 1,
|
||||
reason: "hand-built query object",
|
||||
},
|
||||
"insights/derived/route.ts": { count: 1, reason: "hand-built query object" },
|
||||
"insights/metric-status/route.ts": {
|
||||
count: 1,
|
||||
reason: "hand-built query object",
|
||||
},
|
||||
"insights/narrative/route.ts": {
|
||||
count: 1,
|
||||
reason: "hand-built query object",
|
||||
},
|
||||
"insights/pulse/intraday/route.ts": {
|
||||
count: 1,
|
||||
reason: "hand-built query object",
|
||||
},
|
||||
"medications/[id]/cadence/route.ts": {
|
||||
count: 1,
|
||||
reason: "hand-built query object",
|
||||
},
|
||||
"sync/changes/route.ts": { count: 1, reason: "hand-built query object" },
|
||||
// Same shape, not a query: the range check the edit handler runs
|
||||
// against the row's own type. Its input is `{ value: data.value }`,
|
||||
// built one line above from data the handler already parsed — there is
|
||||
// no caller-supplied key for a strict parse to reject.
|
||||
"measurements/[id]/route.ts": {
|
||||
count: 1,
|
||||
reason: "hand-built parse input",
|
||||
},
|
||||
|
||||
// ---------------------------------------------------------------
|
||||
// 2. Read filters fed the whole `searchParams`. Here an unknown key
|
||||
// DOES reach the schema, and strict would reject it — which is the
|
||||
// wrong trade on a read. A stray param discards nothing: the filter
|
||||
// the caller asked for is still applied and the response is still
|
||||
// correct. Meanwhile these are hot paths (`measurements/series` is
|
||||
// the iOS chart loader), a 422 paints an error banner over a chart
|
||||
// that would otherwise have rendered, and cache-busters and
|
||||
// campaign params ride on URLs for reasons no handler controls.
|
||||
// ---------------------------------------------------------------
|
||||
"measurements/series/route.ts": {
|
||||
count: 1,
|
||||
reason: "read filter over whole searchParams",
|
||||
},
|
||||
"mood/linked-context/route.ts": {
|
||||
count: 1,
|
||||
reason: "read filter over whole searchParams",
|
||||
},
|
||||
"sleep/night/route.ts": {
|
||||
count: 1,
|
||||
reason: "read filter over whole searchParams",
|
||||
},
|
||||
"medications/[id]/dose-history/route.ts": {
|
||||
count: 1,
|
||||
reason: "read filter over whole searchParams",
|
||||
},
|
||||
|
||||
// ---------------------------------------------------------------
|
||||
// 3. Native-client ingest and device registration. This is where the
|
||||
// strip is load-bearing. Every field on these bodies arrived as an
|
||||
// additive optional one — `source`, `deviceType`, `valueMin` /
|
||||
// `valueMax`, `syncTrigger`, `tagKeys`, `liveActivityPushToken` —
|
||||
// and each shipped on the promise that a server which does not know
|
||||
// it stays byte-for-byte unchanged. A client build reaches a server
|
||||
// older than itself as a matter of routine here: the app updates
|
||||
// through TestFlight and the server updates when its operator gets
|
||||
// round to it. Strict would turn that skew into a refused batch,
|
||||
// and the batch endpoints' own contract is the opposite — a bad row
|
||||
// is `skipped` and the rest lands, so a phone draining a year of
|
||||
// history does not lose the year to one row. `devices` is the worst
|
||||
// case of the set: refuse the registration and the account silently
|
||||
// stops receiving push entirely.
|
||||
// ---------------------------------------------------------------
|
||||
"measurements/batch/route.ts": { count: 2, reason: "native ingest skew" },
|
||||
"mood-entries/bulk/route.ts": { count: 4, reason: "native ingest skew" },
|
||||
"medications/intake/bulk/route.ts": {
|
||||
count: 2,
|
||||
reason: "native ingest skew",
|
||||
},
|
||||
"integrations/healthkit/route.ts": { count: 2, reason: "native ingest skew" },
|
||||
"devices/route.ts": { count: 1, reason: "native ingest skew" },
|
||||
|
||||
// ---------------------------------------------------------------
|
||||
// 4. Layout PUTs. Same skew, same mechanism: `comparisonBaseline`,
|
||||
// `chartOverlayPrefs`, `tileVisible`, `sections` all landed as
|
||||
// optional fields at an unchanged `version`, which makes
|
||||
// "add an optional field, older servers ignore it" the documented
|
||||
// compatibility contract for these blobs rather than an accident.
|
||||
// `insights/layout` still accepts `version: 1` specifically because
|
||||
// the live native client sends it. Strict would refuse a Save from
|
||||
// any client ahead of the server and surface as a failed-to-save
|
||||
// toast with no way for the user to act on it.
|
||||
// ---------------------------------------------------------------
|
||||
"dashboard/widgets/route.ts": { count: 3, reason: "layout additive-field" },
|
||||
"insights/layout/route.ts": { count: 3, reason: "layout additive-field" },
|
||||
"medications/layout/route.ts": { count: 1, reason: "layout additive-field" },
|
||||
|
||||
// ---------------------------------------------------------------
|
||||
// 5. OAuth. RFC 6749 §3.1 requires the authorization endpoint to
|
||||
// ignore unrecognised request parameters, and RFC 7591 §2 requires
|
||||
// the registration endpoint to ignore unrecognised metadata — the
|
||||
// register schema already carries an accepted-and-ignored block for
|
||||
// exactly this. Strict would be a spec violation, not a hardening.
|
||||
// ---------------------------------------------------------------
|
||||
"mcp/oauth/authorize/route.ts": { count: 1, reason: "RFC requires ignore" },
|
||||
"mcp/oauth/register/route.ts": { count: 1, reason: "RFC requires ignore" },
|
||||
|
||||
// ---------------------------------------------------------------
|
||||
// 6. Archive and file import. The payload is a document produced
|
||||
// somewhere else — an export from another version of this app, or a
|
||||
// CSV from a third-party tracker whose columns nobody here controls.
|
||||
// Refusing an archive because it carries a column this version does
|
||||
// not read would make a backup unrestorable on the version that has
|
||||
// to restore it, which is the one job a backup has.
|
||||
// ---------------------------------------------------------------
|
||||
"import/route.ts": { count: 3, reason: "foreign archive" },
|
||||
"medications/[id]/intake/import/route.ts": {
|
||||
count: 1,
|
||||
reason: "foreign archive",
|
||||
},
|
||||
};
|
||||
|
||||
function routeFiles(dir: string, acc: string[] = []): string[] {
|
||||
for (const entry of readdirSync(dir)) {
|
||||
const full = join(dir, entry);
|
||||
if (statSync(full).isDirectory()) {
|
||||
routeFiles(full, acc);
|
||||
} else if (entry === "route.ts") {
|
||||
acc.push(full);
|
||||
}
|
||||
}
|
||||
return acc;
|
||||
}
|
||||
|
||||
/** `z.object(...)` and `z.strictObject(...)` calls. */
|
||||
function zodObjectKind(node: ts.Node): "object" | "strictObject" | null {
|
||||
if (!ts.isCallExpression(node)) return null;
|
||||
const callee = node.expression;
|
||||
if (!ts.isPropertyAccessExpression(callee)) return null;
|
||||
const name = callee.name.text;
|
||||
if (name !== "object" && name !== "strictObject") return null;
|
||||
if (!ts.isIdentifier(callee.expression) || callee.expression.text !== "z") {
|
||||
return null;
|
||||
}
|
||||
return name;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a `.strict()` sits on the chain wrapping this call. Walking the
|
||||
* chain rather than matching text is what makes the sweep trustworthy:
|
||||
* a schema written as `z\n .object({…})\n .strict()` is invisible to a
|
||||
* `z.object(` grep, and fourteen of the schemas in this tree are written
|
||||
* exactly that way.
|
||||
*/
|
||||
function hasStrictOnChain(node: ts.Node): boolean {
|
||||
let current: ts.Node = node;
|
||||
while (current.parent) {
|
||||
const parent = current.parent;
|
||||
if (
|
||||
ts.isPropertyAccessExpression(parent) &&
|
||||
parent.expression === current
|
||||
) {
|
||||
if (parent.name.text === "strict") return true;
|
||||
current = parent;
|
||||
continue;
|
||||
}
|
||||
if (ts.isCallExpression(parent) && parent.expression === current) {
|
||||
current = parent;
|
||||
continue;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
function openSchemaCount(file: string): number {
|
||||
const source = ts.createSourceFile(
|
||||
file,
|
||||
readFileSync(file, "utf8"),
|
||||
ts.ScriptTarget.Latest,
|
||||
true,
|
||||
);
|
||||
let open = 0;
|
||||
const walk = (node: ts.Node): void => {
|
||||
const kind = zodObjectKind(node);
|
||||
if (kind === "object" && !hasStrictOnChain(node)) open += 1;
|
||||
ts.forEachChild(node, walk);
|
||||
};
|
||||
walk(source);
|
||||
return open;
|
||||
}
|
||||
|
||||
const inventory = new Map<string, number>();
|
||||
for (const file of routeFiles(API_ROOT)) {
|
||||
const open = openSchemaCount(file);
|
||||
if (open > 0) inventory.set(relative(API_ROOT, file), open);
|
||||
}
|
||||
|
||||
describe("inline request-schema strictness inventory", () => {
|
||||
it("finds the schemas at all", () => {
|
||||
// A sweep that matches nothing passes every other assertion in this
|
||||
// file. Pin a floor so an accidentally-broken matcher fails loudly
|
||||
// instead of reporting a clean tree.
|
||||
expect(inventory.size).toBeGreaterThan(20);
|
||||
});
|
||||
|
||||
it("accounts for every open schema with a written reason", () => {
|
||||
const unaccounted: string[] = [];
|
||||
for (const [file, open] of inventory) {
|
||||
const entry = DELIBERATELY_OPEN[file];
|
||||
if (!entry) {
|
||||
unaccounted.push(
|
||||
`${file}: ${open} schema(s) without .strict() and no entry in DELIBERATELY_OPEN`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
if (entry.count !== open) {
|
||||
unaccounted.push(
|
||||
`${file}: ${open} open schema(s) but DELIBERATELY_OPEN records ${entry.count}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
expect(unaccounted).toEqual([]);
|
||||
});
|
||||
|
||||
it("carries no stale register entries", () => {
|
||||
const stale = Object.keys(DELIBERATELY_OPEN).filter(
|
||||
(file) => !inventory.has(file),
|
||||
);
|
||||
expect(stale).toEqual([]);
|
||||
});
|
||||
|
||||
it("gives every register entry a reason", () => {
|
||||
const empty = Object.entries(DELIBERATELY_OPEN)
|
||||
.filter(([, entry]) => entry.reason.trim().length < 10)
|
||||
.map(([file]) => file);
|
||||
expect(empty).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -12,24 +12,26 @@ import { annotate } from "@/lib/logging/context";
|
||||
import { NextRequest } from "next/server";
|
||||
import { z } from "zod/v4";
|
||||
|
||||
const updateUserSchema = z.object({
|
||||
username: z.string().min(3).max(30).optional(),
|
||||
email: z.string().email().nullable().optional(),
|
||||
role: z.enum(["ADMIN", "USER"]).optional(),
|
||||
// v1.23 — per-user "require a second factor" override. Effective requirement
|
||||
// is the OR of this and the instance-wide AppSettings.mfaRequired policy.
|
||||
mfaEnforced: z.boolean().optional(),
|
||||
// Document vault — per-user storage-quota override in bytes. Null clears
|
||||
// the override (the instance default applies again). Same bounds as the
|
||||
// instance-wide setting.
|
||||
documentQuotaBytes: z
|
||||
.number()
|
||||
.int()
|
||||
.min(10_485_760)
|
||||
.max(1_099_511_627_776)
|
||||
.nullable()
|
||||
.optional(),
|
||||
});
|
||||
const updateUserSchema = z
|
||||
.object({
|
||||
username: z.string().min(3).max(30).optional(),
|
||||
email: z.string().email().nullable().optional(),
|
||||
role: z.enum(["ADMIN", "USER"]).optional(),
|
||||
// v1.23 — per-user "require a second factor" override. Effective requirement
|
||||
// is the OR of this and the instance-wide AppSettings.mfaRequired policy.
|
||||
mfaEnforced: z.boolean().optional(),
|
||||
// Document vault — per-user storage-quota override in bytes. Null clears
|
||||
// the override (the instance default applies again). Same bounds as the
|
||||
// instance-wide setting.
|
||||
documentQuotaBytes: z
|
||||
.number()
|
||||
.int()
|
||||
.min(10_485_760)
|
||||
.max(1_099_511_627_776)
|
||||
.nullable()
|
||||
.optional(),
|
||||
})
|
||||
.strict();
|
||||
|
||||
export const PUT = apiHandler(
|
||||
async (
|
||||
|
||||
@@ -48,9 +48,11 @@ import type { NextRequest } from "next/server";
|
||||
|
||||
export const dynamic = "force-dynamic";
|
||||
|
||||
const bodySchema = z.object({
|
||||
identifier: z.string().trim().min(1).max(254),
|
||||
});
|
||||
const bodySchema = z
|
||||
.object({
|
||||
identifier: z.string().trim().min(1).max(254),
|
||||
})
|
||||
.strict();
|
||||
|
||||
export type CheckUserBranch =
|
||||
"not_found" | "passkey_only" | "email_fallback" | "exists";
|
||||
|
||||
@@ -45,9 +45,11 @@ interface RouteContext {
|
||||
* v1.7.0 — per-device medication-delivery override. NULL clears the
|
||||
* override (the device inherits the user-level roaming default).
|
||||
*/
|
||||
const devicePatchSchema = z.object({
|
||||
medicationDelivery: deviceDeliverySchema,
|
||||
});
|
||||
const devicePatchSchema = z
|
||||
.object({
|
||||
medicationDelivery: deviceDeliverySchema,
|
||||
})
|
||||
.strict();
|
||||
|
||||
export const PATCH = apiHandler(
|
||||
async (request: NextRequest, context: RouteContext) => {
|
||||
|
||||
@@ -32,9 +32,11 @@ import { auditLog } from "@/lib/auth/audit";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { checkRateLimit, rateLimitHeaders } from "@/lib/rate-limit";
|
||||
|
||||
const patchBodySchema = z.object({
|
||||
hasDiabetes: z.boolean(),
|
||||
});
|
||||
const patchBodySchema = z
|
||||
.object({
|
||||
hasDiabetes: z.boolean(),
|
||||
})
|
||||
.strict();
|
||||
|
||||
export const dynamic = "force-dynamic";
|
||||
|
||||
|
||||
@@ -0,0 +1,120 @@
|
||||
/**
|
||||
* The strict half, end to end on a real route.
|
||||
*
|
||||
* `patchBodySchema` is `.strict()`, so a PATCH carrying a key the route
|
||||
* does not declare is refused rather than trimmed down to the keys it
|
||||
* does. Before that, `{ disableCoachh: true }` parsed clean as `{}` —
|
||||
* except `disableCoach` is required, so this particular body already
|
||||
* 422'd on the missing field. The one worth pinning is the body that
|
||||
* used to succeed while doing something other than what it said:
|
||||
* `{ disableCoach: true, disableCoachh: false }`.
|
||||
*
|
||||
* The second assertion is the user-visible half. A 422 that says only
|
||||
* "Validation failed" leaves the caller staring at a body it believes is
|
||||
* correct, so the envelope has to name the key it refused — and name it
|
||||
* without reflecting a hostile one back verbatim.
|
||||
*/
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
vi.mock("@/lib/db", () => ({
|
||||
prisma: { user: { findUnique: vi.fn(), update: vi.fn() } },
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() }));
|
||||
vi.mock("@/lib/auth/audit", () => ({
|
||||
auditLog: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() }));
|
||||
vi.mock("@/lib/db-compat", () => ({
|
||||
ensureDbCompatibility: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock("@/lib/rate-limit", () => ({
|
||||
checkRateLimit: vi.fn().mockResolvedValue({
|
||||
allowed: true,
|
||||
remaining: 59,
|
||||
resetAt: Date.now() + 60_000,
|
||||
}),
|
||||
rateLimitHeaders: () => ({}),
|
||||
}));
|
||||
vi.mock("next/headers", () => ({
|
||||
headers: vi.fn(async () => ({ get: () => null })),
|
||||
cookies: vi.fn(async () => ({
|
||||
get: () => undefined,
|
||||
set: () => {},
|
||||
delete: () => {},
|
||||
})),
|
||||
}));
|
||||
|
||||
import { PATCH } from "../route";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { getSession } from "@/lib/auth/session";
|
||||
|
||||
const SESSION_OK = {
|
||||
session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) },
|
||||
user: { id: "user-1", username: "testuser", role: "USER" as const },
|
||||
};
|
||||
|
||||
function mkPatch(body: unknown): Request {
|
||||
return new Request("http://localhost/api/auth/me/disable-coach", {
|
||||
method: "PATCH",
|
||||
body: JSON.stringify(body),
|
||||
headers: { "Content-Type": "application/json" },
|
||||
});
|
||||
}
|
||||
|
||||
const patch = PATCH as (r: Request) => Promise<Response>;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
vi.mocked(getSession).mockResolvedValue(SESSION_OK as never);
|
||||
vi.mocked(prisma.user.update).mockResolvedValue({
|
||||
disableCoach: true,
|
||||
} as never);
|
||||
});
|
||||
|
||||
describe("PATCH /api/auth/me/disable-coach — unknown keys", () => {
|
||||
it("accepts the declared body", async () => {
|
||||
const res = await patch(mkPatch({ disableCoach: true }));
|
||||
expect(res.status).toBe(200);
|
||||
expect(prisma.user.update).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("refuses a body that carries an undeclared key alongside a valid one", async () => {
|
||||
const res = await patch(
|
||||
mkPatch({ disableCoach: true, disableCoachh: false }),
|
||||
);
|
||||
expect(res.status).toBe(422);
|
||||
// Nothing was written. Before `.strict()` this wrote `disableCoach:
|
||||
// true` and answered 200, so a caller that meant the second field
|
||||
// was told its request had been honoured.
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("names the offending key in the 422", async () => {
|
||||
const res = await patch(
|
||||
mkPatch({ disableCoach: true, disableCoachh: false }),
|
||||
);
|
||||
const body = (await res.json()) as {
|
||||
error: string;
|
||||
details?: {
|
||||
issues: Array<{ code: string; message: string; keys?: string[] }>;
|
||||
};
|
||||
};
|
||||
const issue = body.details?.issues.find(
|
||||
(i) => i.code === "unrecognized_keys",
|
||||
);
|
||||
expect(issue).toBeDefined();
|
||||
expect(issue?.keys).toEqual(["disableCoachh"]);
|
||||
expect(issue?.message).toContain("disableCoachh");
|
||||
});
|
||||
|
||||
it("does not reflect a hostile key name back verbatim", async () => {
|
||||
const res = await patch(
|
||||
mkPatch({ disableCoach: true, "<script>alert(1)</script>": 1 }),
|
||||
);
|
||||
expect(res.status).toBe(422);
|
||||
const raw = await res.text();
|
||||
expect(raw).not.toContain("<script>");
|
||||
expect(raw).toContain("script_alert_1");
|
||||
});
|
||||
});
|
||||
@@ -33,7 +33,7 @@ import { auditLog } from "@/lib/auth/audit";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { checkRateLimit, rateLimitHeaders } from "@/lib/rate-limit";
|
||||
|
||||
const patchBodySchema = z.object({ disableCoach: z.boolean() });
|
||||
const patchBodySchema = z.object({ disableCoach: z.boolean() }).strict();
|
||||
|
||||
export const dynamic = "force-dynamic";
|
||||
|
||||
|
||||
@@ -22,10 +22,12 @@ import {
|
||||
} from "@/lib/ai/coach/about-me";
|
||||
import { requireModuleEnabled } from "@/lib/modules/gate";
|
||||
|
||||
const dismissSchema = z.object({
|
||||
/** Exact question text to dismiss. Omitted = dismiss all. */
|
||||
question: z.string().max(PENDING_QUESTION_MAX_CHARS).optional(),
|
||||
});
|
||||
const dismissSchema = z
|
||||
.object({
|
||||
/** Exact question text to dismiss. Omitted = dismiss all. */
|
||||
question: z.string().max(PENDING_QUESTION_MAX_CHARS).optional(),
|
||||
})
|
||||
.strict();
|
||||
|
||||
export const GET = apiHandler(async () => {
|
||||
const { user } = await requireAuth();
|
||||
|
||||
@@ -30,9 +30,11 @@ import { annotate } from "@/lib/logging/context";
|
||||
|
||||
const MAX_IDS_PER_BATCH = 200;
|
||||
|
||||
const restoreSchema = z.object({
|
||||
ids: z.array(z.string().min(1)).min(1).max(MAX_IDS_PER_BATCH),
|
||||
});
|
||||
const restoreSchema = z
|
||||
.object({
|
||||
ids: z.array(z.string().min(1)).min(1).max(MAX_IDS_PER_BATCH),
|
||||
})
|
||||
.strict();
|
||||
|
||||
type RouteParams = { params: Promise<{ id: string }> };
|
||||
|
||||
|
||||
@@ -35,23 +35,27 @@ import { invalidateUserDashboardWidgets } from "@/lib/cache/invalidate";
|
||||
import { z } from "zod/v4";
|
||||
import type { NextRequest } from "next/server";
|
||||
|
||||
const prefsSchema = z.object({
|
||||
chartKey: z.enum(CHART_OVERLAY_KEYS),
|
||||
prefs: z.object({
|
||||
showTrendIndicator: z.boolean(),
|
||||
showTrendArrow: z.boolean(),
|
||||
showTargetRange: z.boolean(),
|
||||
comparisonBaseline: z
|
||||
.enum(["none", "lastMonth", "lastYear"])
|
||||
.default("none"),
|
||||
// v1.30.1 — persisted range-tab selection (M2 QoL fix). Optional so
|
||||
// a client on an older cached bundle that never sends it doesn't
|
||||
// 422. Literal set mirrors `CHART_RANGE_POINTS` in dashboard-layout.
|
||||
rangePoints: z
|
||||
.union([z.literal(0), z.literal(7), z.literal(30), z.literal(90)])
|
||||
.optional(),
|
||||
}),
|
||||
});
|
||||
const prefsSchema = z
|
||||
.object({
|
||||
chartKey: z.enum(CHART_OVERLAY_KEYS),
|
||||
prefs: z
|
||||
.object({
|
||||
showTrendIndicator: z.boolean(),
|
||||
showTrendArrow: z.boolean(),
|
||||
showTargetRange: z.boolean(),
|
||||
comparisonBaseline: z
|
||||
.enum(["none", "lastMonth", "lastYear"])
|
||||
.default("none"),
|
||||
// v1.30.1 — persisted range-tab selection (M2 QoL fix). Optional so
|
||||
// a client on an older cached bundle that never sends it doesn't
|
||||
// 422. Literal set mirrors `CHART_RANGE_POINTS` in dashboard-layout.
|
||||
rangePoints: z
|
||||
.union([z.literal(0), z.literal(7), z.literal(30), z.literal(90)])
|
||||
.optional(),
|
||||
})
|
||||
.strict(),
|
||||
})
|
||||
.strict();
|
||||
|
||||
export const PUT = apiHandler(async (request: NextRequest) => {
|
||||
const { user } = await requireAuth();
|
||||
|
||||
@@ -45,10 +45,12 @@ import { requireAssistantSurface } from "@/lib/feature-flags";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { checkRateLimit } from "@/lib/rate-limit";
|
||||
|
||||
const coachMessageFeedbackSchema = z.object({
|
||||
rating: z.enum(["helpful", "unhelpful"]),
|
||||
reason: z.string().min(1).max(200).optional(),
|
||||
});
|
||||
const coachMessageFeedbackSchema = z
|
||||
.object({
|
||||
rating: z.enum(["helpful", "unhelpful"]),
|
||||
reason: z.string().min(1).max(200).optional(),
|
||||
})
|
||||
.strict();
|
||||
|
||||
interface RouteContext {
|
||||
params: Promise<{ id: string }>;
|
||||
|
||||
@@ -77,24 +77,28 @@ export const GET = apiHandler(async () => {
|
||||
});
|
||||
});
|
||||
|
||||
const chainEntrySchema = z.object({
|
||||
providerType: z.enum(
|
||||
PROVIDER_CHAIN_TYPES as unknown as [string, ...string[]],
|
||||
),
|
||||
// Priority is recomputed server-side from insertion order so a stale
|
||||
// client cannot persist a chain whose displayed order disagrees with
|
||||
// its priority field. The number is accepted (and may be present) but
|
||||
// ignored on the wire.
|
||||
priority: z.number().int().optional(),
|
||||
enabled: z.boolean(),
|
||||
});
|
||||
const chainEntrySchema = z
|
||||
.object({
|
||||
providerType: z.enum(
|
||||
PROVIDER_CHAIN_TYPES as unknown as [string, ...string[]],
|
||||
),
|
||||
// Priority is recomputed server-side from insertion order so a stale
|
||||
// client cannot persist a chain whose displayed order disagrees with
|
||||
// its priority field. The number is accepted (and may be present) but
|
||||
// ignored on the wire.
|
||||
priority: z.number().int().optional(),
|
||||
enabled: z.boolean(),
|
||||
})
|
||||
.strict();
|
||||
|
||||
const chainBodySchema = z.object({
|
||||
chain: z
|
||||
.array(chainEntrySchema)
|
||||
.min(1, "Chain must contain at least one provider")
|
||||
.max(PROVIDER_CHAIN_TYPES.length, "Too many providers"),
|
||||
});
|
||||
const chainBodySchema = z
|
||||
.object({
|
||||
chain: z
|
||||
.array(chainEntrySchema)
|
||||
.min(1, "Chain must contain at least one provider")
|
||||
.max(PROVIDER_CHAIN_TYPES.length, "Too many providers"),
|
||||
})
|
||||
.strict();
|
||||
|
||||
export const PUT = apiHandler(async (request: NextRequest) => {
|
||||
const { user } = await requireAuth();
|
||||
|
||||
@@ -44,28 +44,30 @@ import { annotate } from "@/lib/logging/context";
|
||||
import { getClientIp } from "@/lib/api-response";
|
||||
import { checkRateLimit, rateLimitHeaders } from "@/lib/rate-limit";
|
||||
|
||||
const webVitalsBodySchema = z.object({
|
||||
// Locked to the web-vitals library's documented metric identifiers.
|
||||
// FCP is included for parity with the iOS WebView client; INP super-
|
||||
// sedes FID under the new Core Web Vitals contract.
|
||||
name: z.enum(["CLS", "LCP", "FID", "INP", "TTFB", "FCP"]),
|
||||
value: z.number().finite(),
|
||||
// Web Vitals client builds `id` from `${navigation-uuid}-${index}`
|
||||
// — bounded length keeps a hostile peer from logging arbitrary text
|
||||
// under the identifier slot.
|
||||
id: z.string().max(50),
|
||||
delta: z.number().finite().optional(),
|
||||
rating: z.enum(["good", "needs-improvement", "poor"]).optional(),
|
||||
navigationType: z
|
||||
.enum([
|
||||
"navigate",
|
||||
"reload",
|
||||
"back-forward",
|
||||
"back-forward-cache",
|
||||
"prerender",
|
||||
])
|
||||
.optional(),
|
||||
});
|
||||
const webVitalsBodySchema = z
|
||||
.object({
|
||||
// Locked to the web-vitals library's documented metric identifiers.
|
||||
// FCP is included for parity with the iOS WebView client; INP super-
|
||||
// sedes FID under the new Core Web Vitals contract.
|
||||
name: z.enum(["CLS", "LCP", "FID", "INP", "TTFB", "FCP"]),
|
||||
value: z.number().finite(),
|
||||
// Web Vitals client builds `id` from `${navigation-uuid}-${index}`
|
||||
// — bounded length keeps a hostile peer from logging arbitrary text
|
||||
// under the identifier slot.
|
||||
id: z.string().max(50),
|
||||
delta: z.number().finite().optional(),
|
||||
rating: z.enum(["good", "needs-improvement", "poor"]).optional(),
|
||||
navigationType: z
|
||||
.enum([
|
||||
"navigate",
|
||||
"reload",
|
||||
"back-forward",
|
||||
"back-forward-cache",
|
||||
"prerender",
|
||||
])
|
||||
.optional(),
|
||||
})
|
||||
.strict();
|
||||
|
||||
function isSameOriginReferer(request: NextRequest): boolean {
|
||||
// When `NEXT_PUBLIC_APP_URL` is unset (test / unconfigured dev) skip
|
||||
|
||||
@@ -32,9 +32,11 @@ const MAX_IDS_PER_BATCH = 200;
|
||||
const RATE_LIMIT_MAX = 60;
|
||||
const RATE_LIMIT_WINDOW_MS = 60 * 1000;
|
||||
|
||||
const restoreSchema = z.object({
|
||||
ids: z.array(z.string().min(1)).min(1).max(MAX_IDS_PER_BATCH),
|
||||
});
|
||||
const restoreSchema = z
|
||||
.object({
|
||||
ids: z.array(z.string().min(1)).min(1).max(MAX_IDS_PER_BATCH),
|
||||
})
|
||||
.strict();
|
||||
|
||||
export const POST = apiHandler(withIdempotency<[NextRequest]>(postRestore));
|
||||
|
||||
|
||||
@@ -37,9 +37,11 @@ const MAX_IDS_PER_BATCH = 200;
|
||||
const RATE_LIMIT_MAX = 60;
|
||||
const RATE_LIMIT_WINDOW_MS = 60 * 1000;
|
||||
|
||||
const bulkDeleteSchema = z.object({
|
||||
ids: z.array(z.string().min(1)).min(1).max(MAX_IDS_PER_BATCH),
|
||||
});
|
||||
const bulkDeleteSchema = z
|
||||
.object({
|
||||
ids: z.array(z.string().min(1)).min(1).max(MAX_IDS_PER_BATCH),
|
||||
})
|
||||
.strict();
|
||||
|
||||
export const POST = apiHandler(withIdempotency<[NextRequest]>(postBulkDelete));
|
||||
|
||||
|
||||
@@ -46,12 +46,14 @@ import { afterMeasurementMutation } from "@/lib/rollups/after-measurement-mutati
|
||||
|
||||
const MAX_BATCH_ENTRIES = 500;
|
||||
|
||||
const payloadSchema = z.object({
|
||||
externalIds: z
|
||||
.array(z.string().min(1).max(120))
|
||||
.min(0)
|
||||
.max(MAX_BATCH_ENTRIES),
|
||||
});
|
||||
const payloadSchema = z
|
||||
.object({
|
||||
externalIds: z
|
||||
.array(z.string().min(1).max(120))
|
||||
.min(0)
|
||||
.max(MAX_BATCH_ENTRIES),
|
||||
})
|
||||
.strict();
|
||||
|
||||
export const DELETE = apiHandler(deleteByExternalIds);
|
||||
|
||||
|
||||
@@ -39,9 +39,11 @@ const MAX_IDS_PER_BATCH = 200;
|
||||
const RATE_LIMIT_MAX = 60;
|
||||
const RATE_LIMIT_WINDOW_MS = 60 * 1000;
|
||||
|
||||
const restoreSchema = z.object({
|
||||
ids: z.array(z.string().min(1)).min(1).max(MAX_IDS_PER_BATCH),
|
||||
});
|
||||
const restoreSchema = z
|
||||
.object({
|
||||
ids: z.array(z.string().min(1)).min(1).max(MAX_IDS_PER_BATCH),
|
||||
})
|
||||
.strict();
|
||||
|
||||
export const POST = apiHandler(withIdempotency<[NextRequest]>(postRestore));
|
||||
|
||||
|
||||
@@ -0,0 +1,103 @@
|
||||
/**
|
||||
* The other half of the strictness decision, asserted rather than
|
||||
* assumed.
|
||||
*
|
||||
* `measurements/series` is one of the read filters left deliberately
|
||||
* open (see `src/app/api/__tests__/inline-schema-strictness-inventory.test.ts`).
|
||||
* It parses the WHOLE `searchParams`, so an unknown param does reach the
|
||||
* schema — and it has to keep being ignored. This is the iOS chart
|
||||
* loader's hot path; a 422 here paints an error banner over a chart that
|
||||
* would otherwise have rendered, and a stray param discards nothing,
|
||||
* because the filter the caller asked for is still applied.
|
||||
*
|
||||
* Without this test, a later "add .strict() everywhere" sweep would take
|
||||
* the tolerance away and the only thing that noticed would be a user.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { NextRequest } from "next/server";
|
||||
|
||||
vi.mock("@/lib/db", () => ({
|
||||
prisma: {
|
||||
measurement: { findMany: vi.fn() },
|
||||
user: { findUnique: vi.fn() },
|
||||
auditLog: { create: vi.fn() },
|
||||
$queryRaw: vi.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() }));
|
||||
|
||||
vi.mock("@/lib/tz/resolver", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("@/lib/tz/resolver")>();
|
||||
return { ...actual, resolveUserTimezone: vi.fn(async () => "UTC") };
|
||||
});
|
||||
|
||||
vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() }));
|
||||
|
||||
vi.mock("@/lib/rollups/measurement-read", () => ({
|
||||
loadUserSourcePriority: vi.fn(async () => null),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/db-compat", () => ({
|
||||
ensureDbCompatibility: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/logging/context", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("@/lib/logging/context")>();
|
||||
return { ...actual, annotate: vi.fn() };
|
||||
});
|
||||
|
||||
vi.mock("next/headers", () => ({
|
||||
headers: vi.fn(async () => ({ get: () => null })),
|
||||
cookies: vi.fn(async () => ({
|
||||
get: () => undefined,
|
||||
set: () => {},
|
||||
delete: () => {},
|
||||
})),
|
||||
}));
|
||||
|
||||
import { GET } from "../route";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { getSession } from "@/lib/auth/session";
|
||||
|
||||
const SESSION_OK = {
|
||||
session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) },
|
||||
user: { id: "user-1", username: "testuser", role: "USER" as const },
|
||||
};
|
||||
|
||||
function req(query: string): NextRequest {
|
||||
return new NextRequest(`http://localhost/api/measurements/series?${query}`);
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
vi.mocked(getSession).mockResolvedValue(SESSION_OK as never);
|
||||
vi.mocked(prisma.measurement.findMany).mockResolvedValue([] as never);
|
||||
vi.mocked(prisma.$queryRaw).mockResolvedValue([] as never);
|
||||
vi.mocked(prisma.auditLog.create).mockResolvedValue({} as never);
|
||||
vi.mocked(prisma.user.findUnique).mockResolvedValue({
|
||||
glucoseUnit: "mg/dL",
|
||||
} as never);
|
||||
});
|
||||
|
||||
describe("GET /api/measurements/series — undeclared query params", () => {
|
||||
it("serves the series when an undeclared param rides along", async () => {
|
||||
const res = await GET(req("kind=weight&days=30&cacheBust=1757000000"));
|
||||
expect(res.status).toBe(200);
|
||||
});
|
||||
|
||||
it("serves the same series with and without the stray param", async () => {
|
||||
const clean = await (await GET(req("kind=weight&days=30"))).json();
|
||||
const noisy = await (
|
||||
await GET(req("kind=weight&days=30&utm_source=mail&t=42"))
|
||||
).json();
|
||||
expect(noisy).toEqual(clean);
|
||||
});
|
||||
|
||||
it("still refuses a param it DOES declare when the value is wrong", async () => {
|
||||
// Tolerating unknown keys is not tolerating unknown values: the
|
||||
// closed `kind` enum keeps rejecting.
|
||||
const res = await GET(req("kind=garbage&stray=1"));
|
||||
expect(res.status).toBe(422);
|
||||
});
|
||||
});
|
||||
@@ -88,21 +88,23 @@ const MAX_TEXT_LENGTH = 2000;
|
||||
*/
|
||||
const MAX_OUTPUT_TOKENS = 600;
|
||||
|
||||
const requestSchema = z.object({
|
||||
text: z.string().min(1).max(MAX_TEXT_LENGTH),
|
||||
/** Optional UI locale; informational hint for the model. */
|
||||
locale: z.enum(["en", "de", "es", "fr", "it", "pl", "ko"]).optional(),
|
||||
/**
|
||||
* Optional override of the reference date used to resolve relative
|
||||
* phrases ("tomorrow", "next Monday"). Defaults to the server's
|
||||
* UTC `YYYY-MM-DD`. Bounded format so a hostile client cannot
|
||||
* inject prose into the prompt body.
|
||||
*/
|
||||
today: z
|
||||
.string()
|
||||
.regex(/^\d{4}-\d{2}-\d{2}$/)
|
||||
.optional(),
|
||||
});
|
||||
const requestSchema = z
|
||||
.object({
|
||||
text: z.string().min(1).max(MAX_TEXT_LENGTH),
|
||||
/** Optional UI locale; informational hint for the model. */
|
||||
locale: z.enum(["en", "de", "es", "fr", "it", "pl", "ko"]).optional(),
|
||||
/**
|
||||
* Optional override of the reference date used to resolve relative
|
||||
* phrases ("tomorrow", "next Monday"). Defaults to the server's
|
||||
* UTC `YYYY-MM-DD`. Bounded format so a hostile client cannot
|
||||
* inject prose into the prompt body.
|
||||
*/
|
||||
today: z
|
||||
.string()
|
||||
.regex(/^\d{4}-\d{2}-\d{2}$/)
|
||||
.optional(),
|
||||
})
|
||||
.strict();
|
||||
|
||||
/**
|
||||
* Strip leading code-fence noise that some chat models wrap around
|
||||
|
||||
@@ -7,14 +7,16 @@ import { sendGlitchtipEvent } from "@/lib/monitoring/glitchtip";
|
||||
import { apiHandler } from "@/lib/api-handler";
|
||||
import { annotate, getEvent } from "@/lib/logging/context";
|
||||
|
||||
const clientErrorSchema = z.object({
|
||||
message: z.string().min(1).max(2000),
|
||||
stack: z.string().max(20000).optional(),
|
||||
level: z.enum(["error", "warning", "info"]).optional(),
|
||||
type: z.string().max(120).optional(),
|
||||
url: z.string().max(2000).optional(),
|
||||
userAgent: z.string().max(1000).optional(),
|
||||
});
|
||||
const clientErrorSchema = z
|
||||
.object({
|
||||
message: z.string().min(1).max(2000),
|
||||
stack: z.string().max(20000).optional(),
|
||||
level: z.enum(["error", "warning", "info"]).optional(),
|
||||
type: z.string().max(120).optional(),
|
||||
url: z.string().max(2000).optional(),
|
||||
userAgent: z.string().max(1000).optional(),
|
||||
})
|
||||
.strict();
|
||||
|
||||
export const POST = apiHandler(async (request: NextRequest) => {
|
||||
annotate({ action: { name: "monitoring.glitchtip.report" } });
|
||||
|
||||
@@ -36,9 +36,11 @@ const MAX_IDS_PER_BATCH = 200;
|
||||
const RATE_LIMIT_MAX = 60;
|
||||
const RATE_LIMIT_WINDOW_MS = 60 * 1000;
|
||||
|
||||
const bulkDeleteSchema = z.object({
|
||||
ids: z.array(z.string().min(1)).min(1).max(MAX_IDS_PER_BATCH),
|
||||
});
|
||||
const bulkDeleteSchema = z
|
||||
.object({
|
||||
ids: z.array(z.string().min(1)).min(1).max(MAX_IDS_PER_BATCH),
|
||||
})
|
||||
.strict();
|
||||
|
||||
export const POST = apiHandler(withIdempotency<[NextRequest]>(postBulkDelete));
|
||||
|
||||
|
||||
@@ -38,9 +38,11 @@ const MAX_IDS_PER_BATCH = 200;
|
||||
const RATE_LIMIT_MAX = 60;
|
||||
const RATE_LIMIT_WINDOW_MS = 60 * 1000;
|
||||
|
||||
const restoreSchema = z.object({
|
||||
ids: z.array(z.string().min(1)).min(1).max(MAX_IDS_PER_BATCH),
|
||||
});
|
||||
const restoreSchema = z
|
||||
.object({
|
||||
ids: z.array(z.string().min(1)).min(1).max(MAX_IDS_PER_BATCH),
|
||||
})
|
||||
.strict();
|
||||
|
||||
export const POST = apiHandler(withIdempotency<[NextRequest]>(postRestore));
|
||||
|
||||
|
||||
@@ -15,9 +15,11 @@ import { z } from "zod/v4";
|
||||
// the same check again at egress time as defence-in-depth.
|
||||
const subscribeSchema = webPushSubscriptionSchema;
|
||||
|
||||
const unsubscribeSchema = z.object({
|
||||
endpoint: z.string().url(),
|
||||
});
|
||||
const unsubscribeSchema = z
|
||||
.object({
|
||||
endpoint: z.string().url(),
|
||||
})
|
||||
.strict();
|
||||
|
||||
/*
|
||||
* Both parses below answer with the multi-issue envelope, like every other
|
||||
|
||||
@@ -19,12 +19,14 @@ import { DISCLAIMER_VERSION } from "@/lib/onboarding/disclaimer";
|
||||
* privacy page. Never auto-set; the onboarding welcome step is the one writer.
|
||||
*/
|
||||
|
||||
const bodySchema = z.object({
|
||||
// Echoed back from the client so a stale shell cannot silently record an
|
||||
// acknowledgment of copy it never rendered. Bounded; the server pins the
|
||||
// canonical version it persists regardless.
|
||||
version: z.string().min(1).max(64),
|
||||
});
|
||||
const bodySchema = z
|
||||
.object({
|
||||
// Echoed back from the client so a stale shell cannot silently record an
|
||||
// acknowledgment of copy it never rendered. Bounded; the server pins the
|
||||
// canonical version it persists regardless.
|
||||
version: z.string().min(1).max(64),
|
||||
})
|
||||
.strict();
|
||||
|
||||
export const POST = apiHandler(async (request) => {
|
||||
const { user } = await requireAuth();
|
||||
|
||||
@@ -41,6 +41,7 @@ const tourBodySchema = z
|
||||
outcome: z.enum(["completed", "skipped"]).optional(),
|
||||
progress: tourProgressSchema.optional(),
|
||||
})
|
||||
.strict()
|
||||
.refine((b) => b.completed !== undefined || b.progress !== undefined, {
|
||||
message: "Provide `completed` and/or `progress`.",
|
||||
});
|
||||
|
||||
@@ -0,0 +1,180 @@
|
||||
/**
|
||||
* The user-visible half of a strict schema.
|
||||
*
|
||||
* A 422 that says only "Validation failed" costs the caller a debugging
|
||||
* session it should not have to run: it knows the body was refused and
|
||||
* not which field did it. Zod puts the offending names in `issue.keys`
|
||||
* and leaves `issue.path` empty, so a sanitiser that reads `path` alone
|
||||
* emits `{ path: "", code: "unrecognized_keys" }` and names nothing.
|
||||
*
|
||||
* The other half is that a key name is caller-controlled input. Zod's
|
||||
* own message echoes it verbatim and unbounded, and that message reaches
|
||||
* the client, the wide event, and — through the `stripValuesFromMessage`
|
||||
* call sites — the audit ledger. So the name is bounded and character-
|
||||
* restricted before it is rendered, and the message is rebuilt from the
|
||||
* sanitised names rather than passed through.
|
||||
*/
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { z } from "zod/v4";
|
||||
|
||||
import { returnAllZodIssues, sanitiseZodIssues } from "../api-response";
|
||||
|
||||
const schema = z
|
||||
.object({
|
||||
measuredAt: z.string(),
|
||||
value: z.number(),
|
||||
})
|
||||
.strict();
|
||||
|
||||
function issuesFor(body: Record<string, unknown>) {
|
||||
const parsed = schema.safeParse(body);
|
||||
if (parsed.success) throw new Error("expected the parse to fail");
|
||||
return parsed.error;
|
||||
}
|
||||
|
||||
describe("sanitiseZodIssues — unrecognized_keys", () => {
|
||||
it("names the offending key in both the message and a structured field", () => {
|
||||
const error = issuesFor({
|
||||
measuredAt: "2026-01-01",
|
||||
value: 80,
|
||||
measuredAtt: "2026-01-01",
|
||||
});
|
||||
|
||||
const [issue] = sanitiseZodIssues(error.issues);
|
||||
expect(issue.code).toBe("unrecognized_keys");
|
||||
expect(issue.keys).toEqual(["measuredAtt"]);
|
||||
expect(issue.message).toBe('Unrecognized key: "measuredAtt"');
|
||||
});
|
||||
|
||||
it("names every offending key when a body carries several", () => {
|
||||
const error = issuesFor({
|
||||
measuredAt: "2026-01-01",
|
||||
value: 80,
|
||||
measuredAtt: "x",
|
||||
unitt: "kg",
|
||||
});
|
||||
|
||||
const [issue] = sanitiseZodIssues(error.issues);
|
||||
expect(issue.keys).toEqual(["measuredAtt", "unitt"]);
|
||||
expect(issue.message).toBe('Unrecognized keys: "measuredAtt", "unitt"');
|
||||
});
|
||||
|
||||
it("does not echo a hostile key name back verbatim", () => {
|
||||
const error = issuesFor({
|
||||
measuredAt: "2026-01-01",
|
||||
value: 80,
|
||||
"<img src=x onerror=alert(1)>": 1,
|
||||
});
|
||||
|
||||
const [issue] = sanitiseZodIssues(error.issues);
|
||||
expect(issue.keys?.[0]).not.toContain("<");
|
||||
expect(issue.keys?.[0]).not.toContain(">");
|
||||
expect(issue.message).not.toContain("<img");
|
||||
// The shape still survives, so the caller can still recognise its
|
||||
// own key — only the characters that could be read as markup go.
|
||||
expect(issue.keys?.[0]).toBe("_img_src_x_onerror_alert_1__");
|
||||
});
|
||||
|
||||
it("bounds the length of a single key name", () => {
|
||||
const error = issuesFor({
|
||||
measuredAt: "2026-01-01",
|
||||
value: 80,
|
||||
[`over${"x".repeat(500)}`]: 1,
|
||||
});
|
||||
|
||||
const [issue] = sanitiseZodIssues(error.issues);
|
||||
expect(issue.keys?.[0]).toHaveLength(64);
|
||||
expect(issue.message.length).toBeLessThan(120);
|
||||
});
|
||||
|
||||
it("bounds how many key names one issue reports", () => {
|
||||
const body: Record<string, unknown> = {
|
||||
measuredAt: "2026-01-01",
|
||||
value: 80,
|
||||
};
|
||||
for (let i = 0; i < 40; i += 1) body[`extra${i}`] = i;
|
||||
|
||||
const [issue] = sanitiseZodIssues(issuesFor(body).issues);
|
||||
expect(issue.keys).toHaveLength(10);
|
||||
});
|
||||
|
||||
it("keeps the key names under stripValuesFromMessage", () => {
|
||||
// The opt-in exists to keep user-typed VALUES out of the audit
|
||||
// ledger. A key name is structure, not a value, so dropping it too
|
||||
// would leave an operator with a row that says a body was refused
|
||||
// and nothing about why.
|
||||
const error = issuesFor({
|
||||
measuredAt: "2026-01-01",
|
||||
value: 80,
|
||||
measuredAtt: "x",
|
||||
});
|
||||
|
||||
const [issue] = sanitiseZodIssues(error.issues, {
|
||||
stripValuesFromMessage: true,
|
||||
});
|
||||
expect(issue).toEqual({
|
||||
path: "",
|
||||
code: "unrecognized_keys",
|
||||
keys: ["measuredAtt"],
|
||||
});
|
||||
expect(issue).not.toHaveProperty("message");
|
||||
});
|
||||
|
||||
it("leaves every other issue code untouched", () => {
|
||||
const parsed = schema.safeParse({ measuredAt: "2026-01-01" });
|
||||
if (parsed.success) throw new Error("expected the parse to fail");
|
||||
|
||||
const [issue] = sanitiseZodIssues(parsed.error.issues);
|
||||
expect(issue.code).toBe("invalid_type");
|
||||
expect(issue.path).toBe("value");
|
||||
expect(issue).not.toHaveProperty("keys");
|
||||
});
|
||||
|
||||
it("reports the nested object's own path when the strict object is nested", () => {
|
||||
const nested = z
|
||||
.object({ prefs: z.object({ enabled: z.boolean() }).strict() })
|
||||
.strict();
|
||||
const parsed = nested.safeParse({ prefs: { enabled: true, enabledd: 1 } });
|
||||
if (parsed.success) throw new Error("expected the parse to fail");
|
||||
|
||||
const [issue] = sanitiseZodIssues(parsed.error.issues);
|
||||
expect(issue.path).toBe("prefs");
|
||||
expect(issue.keys).toEqual(["enabledd"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("returnAllZodIssues — unrecognized_keys over the wire", () => {
|
||||
it("answers 422 and names the key in the response envelope", async () => {
|
||||
const error = issuesFor({
|
||||
measuredAt: "2026-01-01",
|
||||
value: 80,
|
||||
measuredAtt: "x",
|
||||
});
|
||||
|
||||
const response = returnAllZodIssues(error);
|
||||
expect(response.status).toBe(422);
|
||||
|
||||
const body = (await response.json()) as {
|
||||
data: null;
|
||||
error: string;
|
||||
details: {
|
||||
issues: Array<{
|
||||
path: string;
|
||||
code: string;
|
||||
message: string;
|
||||
keys?: string[];
|
||||
}>;
|
||||
};
|
||||
};
|
||||
expect(body.data).toBeNull();
|
||||
expect(body.error).toBe("Validation failed");
|
||||
expect(body.details.issues).toEqual([
|
||||
{
|
||||
path: "",
|
||||
code: "unrecognized_keys",
|
||||
message: 'Unrecognized key: "measuredAtt"',
|
||||
keys: ["measuredAtt"],
|
||||
},
|
||||
]);
|
||||
});
|
||||
});
|
||||
+81
-12
@@ -7,8 +7,9 @@ export function apiSuccess<T>(data: T, status = 200) {
|
||||
}
|
||||
|
||||
/**
|
||||
* Sanitised view of a single Zod issue. We surface `path`, `code` and
|
||||
* `message` only — `issue.params` may echo the offending user input
|
||||
* Sanitised view of a single Zod issue. We surface `path`, `code`,
|
||||
* `message` and — for `unrecognized_keys` only — the bounded `keys`
|
||||
* list; `issue.params` may echo the offending user input
|
||||
* (e.g. a too-long string, a regex source) and we do not want that
|
||||
* round-tripped to mobile callers or persisted into the audit ledger.
|
||||
*/
|
||||
@@ -16,6 +17,58 @@ export interface SanitisedZodIssue {
|
||||
path: string;
|
||||
code: string;
|
||||
message: string;
|
||||
/**
|
||||
* Only set for `unrecognized_keys`: the rejected key names, sanitised
|
||||
* and bounded by {@link sanitiseKeyName}. A key name is structure, not
|
||||
* content, so naming it is what makes the rejection actionable — the
|
||||
* caller learns which field it got wrong instead of only that the body
|
||||
* had a field too many. It survives `stripValuesFromMessage` for the
|
||||
* same reason: it carries no user-typed value.
|
||||
*/
|
||||
keys?: string[];
|
||||
}
|
||||
|
||||
/** Cap on how many rejected key names one issue reports. */
|
||||
const MAX_REPORTED_KEYS = 10;
|
||||
/** Cap on the length of a single reported key name. */
|
||||
const MAX_KEY_NAME_LENGTH = 64;
|
||||
|
||||
/**
|
||||
* A key name arrives straight from the request body, so it is caller-
|
||||
* controlled and unbounded: a client can post a 100 KB key, or one
|
||||
* carrying markup, and Zod's own `unrecognized_keys` message echoes it
|
||||
* verbatim. That message reaches the client, the wide event and — at the
|
||||
* `stripValuesFromMessage` call sites — the audit ledger, so it is bounded
|
||||
* and character-restricted here rather than at each of those boundaries.
|
||||
* Anything outside `[A-Za-z0-9_.-]` collapses to `_`, which keeps a typo
|
||||
* like `measuredAtt` perfectly readable while leaving nothing that could
|
||||
* be mistaken for markup or a control sequence downstream.
|
||||
*/
|
||||
function sanitiseKeyName(key: string): string {
|
||||
return key.slice(0, MAX_KEY_NAME_LENGTH).replace(/[^A-Za-z0-9_.-]/g, "_");
|
||||
}
|
||||
|
||||
/**
|
||||
* Zod reports every unknown key of one object in a single issue whose
|
||||
* `path` is the object itself, so the offending keys live in `issue.keys`
|
||||
* and nowhere in `path`. Without this the sanitised issue read
|
||||
* `{ path: "", code: "unrecognized_keys" }` and named nothing at all.
|
||||
*/
|
||||
function unrecognisedKeys(issue: ZodIssue): string[] | undefined {
|
||||
if (issue.code !== "unrecognized_keys") return undefined;
|
||||
return issue.keys.slice(0, MAX_REPORTED_KEYS).map(sanitiseKeyName);
|
||||
}
|
||||
|
||||
/**
|
||||
* Rebuilt from the sanitised key names rather than passed through from
|
||||
* Zod, so the wording stays the one callers already parse while the
|
||||
* unbounded verbatim echo does not ship.
|
||||
*/
|
||||
function unrecognisedKeysMessage(keys: string[]): string {
|
||||
const rendered = keys.map((key) => `"${key}"`).join(", ");
|
||||
return keys.length === 1
|
||||
? `Unrecognized key: ${rendered}`
|
||||
: `Unrecognized keys: ${rendered}`;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -29,8 +82,9 @@ export interface SanitisedZodIssue {
|
||||
export interface SanitiseZodIssueOptions {
|
||||
/**
|
||||
* When true, drop `issue.message` from the returned shape entirely.
|
||||
* Only `path` + `code` survive — enough for an operator to triage
|
||||
* the rejection without persisting user-typed content. Default is
|
||||
* Only `path`, `code` and (for `unrecognized_keys`) the sanitised
|
||||
* `keys` survive — enough for an operator to triage the rejection
|
||||
* without persisting user-typed content. Default is
|
||||
* `false` so the additive multi-issue envelope keeps its existing
|
||||
* client contract.
|
||||
*/
|
||||
@@ -60,6 +114,13 @@ export interface SanitiseZodIssueOptions {
|
||||
* the audit ledger would leak user content. The opt-in is additive —
|
||||
* the default behaviour is unchanged so existing clients reading
|
||||
* `details.issues[*].message` over the wire keep working.
|
||||
*
|
||||
* `unrecognized_keys` is rendered rather than passed through. A caller
|
||||
* that sends `measuredAtt` for `measuredAt` has to be told which key was
|
||||
* refused, and Zod puts that in `issue.keys` while leaving `path` empty —
|
||||
* so the key names are lifted onto `keys`, bounded and character-
|
||||
* restricted, and the message is rebuilt from them rather than echoing
|
||||
* the raw request key back.
|
||||
*/
|
||||
export function sanitiseZodIssues(
|
||||
issues: readonly ZodIssue[],
|
||||
@@ -73,16 +134,24 @@ export function sanitiseZodIssues(
|
||||
options?: SanitiseZodIssueOptions,
|
||||
): SanitisedZodIssue[] | Array<Omit<SanitisedZodIssue, "message">> {
|
||||
if (options?.stripValuesFromMessage) {
|
||||
return issues.map((issue) => ({
|
||||
return issues.map((issue) => {
|
||||
const keys = unrecognisedKeys(issue);
|
||||
return {
|
||||
path: issue.path.join("."),
|
||||
code: issue.code,
|
||||
...(keys ? { keys } : {}),
|
||||
};
|
||||
});
|
||||
}
|
||||
return issues.map((issue) => {
|
||||
const keys = unrecognisedKeys(issue);
|
||||
return {
|
||||
path: issue.path.join("."),
|
||||
code: issue.code,
|
||||
}));
|
||||
}
|
||||
return issues.map((issue) => ({
|
||||
path: issue.path.join("."),
|
||||
code: issue.code,
|
||||
message: issue.message,
|
||||
}));
|
||||
message: keys ? unrecognisedKeysMessage(keys) : issue.message,
|
||||
...(keys ? { keys } : {}),
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in new issue
Block a user