mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-13 14:32:03 +00:00
A `z.object()` without `.strict()` strips an unknown key and parses
clean, so a caller that misspells a field name gets a 200 and loses the
value with nothing to tell it. The worst of them was the Coach
question-dismiss endpoint, where an omitted `question` means "dismiss
all": a typo in that key wiped every pending question and answered as
though one had been dismissed.
Twenty-four inline schemas across twenty-two endpoints are strict now —
the settings toggles, the restore and bulk-delete endpoints, message
feedback, the admin user update, the onboarding acknowledgements, the
provider chain, the chart overlay preferences, the medication extractor
and the two client-side reporters.
The strip stays where it carries weight. Sync ingest, device
registration and the layout PUTs are built out of fields that shipped as
additive and optional, on the promise that a server which does not know
one ignores it; an app running ahead of the install it talks to is
routine here rather than exceptional, and refusing would turn a dropped
field into a refused batch — a year of history lost to one row, a
dashboard that will not save, a phone that quietly stops registering for
push. A read filter discards nothing when a stray query parameter rides
along, the OAuth endpoints are required by RFC 6749 and RFC 7591 to
ignore parameters they do not recognise, and an import file comes from
another version or another tool by definition. Each is registered with
its reason in a new inventory test, so a schema cannot join them without
someone making the call. That test parses the tree rather than grepping
it: fourteen of these schemas are written `z\n .object({…})` and a
`z.object(` sweep never saw them.
`sanitiseZodIssues` renders `unrecognized_keys` instead of passing it
through. Zod leaves `path` empty and puts the names in `issue.keys`, so
the sanitised issue named nothing at all and the caller was told only
that its body had failed. The names now ride a bounded `keys` field and
the message is rebuilt from them, capped at ten keys of 64 characters
and restricted to identifier characters, so a caller-supplied key cannot
echo back verbatim through the error path or into the audit ledger. The
names survive `stripValuesFromMessage` because a field name is
structure, not user-typed content.
181 lines
5.8 KiB
TypeScript
181 lines
5.8 KiB
TypeScript
/**
|
|
* 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"],
|
|
},
|
|
]);
|
|
});
|
|
});
|