mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 04:50:59 +00:00
fix(api): answer an unparseable body with 400 everywhere
`safeJson` refuses a body that will not parse with 400, and so does `apiHandler`'s own SyntaxError branch — about two hundred and twenty routes inherit that. Seven `/api/auth/me/*` writes hand-rolled the parse in a try/catch and threw 422 for the identical condition, so a client's "my serializer produced garbage" branch had to accept two statuses on a subset of routes it had no way to predict. All seven answer 400 now, with the sentence `safeJson` already sends and the dotted token each one carried moved to `meta.errorCode`. The 400 is published on those seven operations through a shared response; it is deliberately not folded into the standard set, which is spread onto reads as well and a GET has no body to refuse. `/api/auth/me/health-score-config` keeps its 422: the operation's own description names that status and its exact message, so moving it is a contract decision rather than a cleanup. The guard carries it as an allowlist entry with that reason rather than quietly skipping it. Three route tests pinned the old status; they assert the new one and say why. The guard reads the hand-rolled parses out of the tree and asserts it still finds them, so it cannot pass on a matcher that stopped matching.
This commit is contained in:
1 parent
9ce0bd5027
commit
3e44d68518
15 files changed
+434
-191
No files matched your search
+181
-167
File diff suppressed because it is too large.
Load diff
@@ -0,0 +1,158 @@
|
||||
/**
|
||||
* A body that will not parse is always a 400.
|
||||
*
|
||||
* `safeJson` has answered 400 since it was written and roughly two hundred and
|
||||
* twenty routes go through it; `apiHandler`'s own `SyntaxError` branch answers
|
||||
* 400 as well. Seven `/api/auth/me/*` writes hand-rolled `await req.json()` in
|
||||
* a try/catch and threw 422 for the same condition, so a client's "my
|
||||
* serializer produced garbage" branch had to accept two statuses on a subset of
|
||||
* routes it could not predict — and the subset was invisible from the outside.
|
||||
*
|
||||
* The guard reads every hand-rolled parse out of the route tree rather than
|
||||
* listing them, so a route that grows one later is judged too. The `catch` is
|
||||
* read on its own: a `try` that parses a body and then refuses with anything
|
||||
* other than 400 fails here.
|
||||
*
|
||||
* Its limit: a route that parses a body somewhere other than inside a `try`
|
||||
* whose `catch` builds the refusal — a shared helper, a promise chain — is
|
||||
* invisible to it. `safeJson` remains the documented entry, and the assertion
|
||||
* on the helper below is what pins the status the rest of the tree inherits.
|
||||
*/
|
||||
import { readFileSync, readdirSync } from "node:fs";
|
||||
import { join, relative, sep } from "node:path";
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { safeJson } from "@/lib/api-response";
|
||||
|
||||
const API_ROOT = join(process.cwd(), "src", "app", "api");
|
||||
|
||||
/**
|
||||
* Hand-rolled parses that must keep a different status, with the reason.
|
||||
*
|
||||
* `/api/auth/me/health-score-config` is here because the published contract
|
||||
* names its answer: the operation's own 422 description spells out that a body
|
||||
* which is not JSON at all comes back with the bare message
|
||||
* `health-score-config.body.invalid_json`. Moving it to 400 would change a
|
||||
* promise a shipped client was generated against, so the status stays and the
|
||||
* change is a contract decision of its own rather than a cleanup.
|
||||
*/
|
||||
const ALLOWLIST: Record<string, { status: number; reason: string }> = {
|
||||
"src/app/api/auth/me/health-score-config/route.ts": {
|
||||
status: 422,
|
||||
reason:
|
||||
"The 422 and its exact message are written into the operation's published description.",
|
||||
},
|
||||
};
|
||||
|
||||
/** Every `route.ts` under `src/app/api`, tests excluded. */
|
||||
function routeFiles(dir: string): string[] {
|
||||
const found: string[] = [];
|
||||
for (const entry of readdirSync(dir, { withFileTypes: true })) {
|
||||
const full = join(dir, entry.name);
|
||||
if (entry.isDirectory()) {
|
||||
if (entry.name === "__tests__") continue;
|
||||
found.push(...routeFiles(full));
|
||||
continue;
|
||||
}
|
||||
if (entry.name === "route.ts") found.push(full);
|
||||
}
|
||||
return found;
|
||||
}
|
||||
|
||||
/** The brace-balanced block starting at the `{` at or after `from`. */
|
||||
function block(src: string, from: number): { text: string; end: number } {
|
||||
let i = from;
|
||||
while (i < src.length && src[i] !== "{") i++;
|
||||
let depth = 0;
|
||||
for (let j = i; j < src.length; j++) {
|
||||
if (src[j] === "{") depth++;
|
||||
else if (src[j] === "}") {
|
||||
depth--;
|
||||
if (depth === 0) return { text: src.slice(i, j + 1), end: j + 1 };
|
||||
}
|
||||
}
|
||||
return { text: src.slice(i), end: src.length };
|
||||
}
|
||||
|
||||
interface ParseRefusal {
|
||||
route: string;
|
||||
line: number;
|
||||
status: number | null;
|
||||
}
|
||||
|
||||
/** Every `try { … .json() … } catch { … }` under the route tree. */
|
||||
function scanHandRolledParses(): ParseRefusal[] {
|
||||
const found: ParseRefusal[] = [];
|
||||
for (const file of routeFiles(API_ROOT)) {
|
||||
const src = readFileSync(file, "utf8");
|
||||
for (const m of src.matchAll(/\btry\s*\{/g)) {
|
||||
const start = m.index ?? 0;
|
||||
const tryBlock = block(src, start);
|
||||
if (!/(?:req|request)\w*\.json\(\)/.test(tryBlock.text)) continue;
|
||||
const after = src.slice(tryBlock.end);
|
||||
const catchAt = after.match(/^\s*catch\s*(?:\([^)]*\)\s*)?\{/);
|
||||
if (!catchAt) continue;
|
||||
const catchBlock = block(after, catchAt.index ?? 0);
|
||||
const status = catchBlock.text.match(/\b(4\d\d|5\d\d)\b/);
|
||||
found.push({
|
||||
route: relative(process.cwd(), file).split(sep).join("/"),
|
||||
line: src.slice(0, start).split("\n").length,
|
||||
status: status ? Number(status[1]) : null,
|
||||
});
|
||||
}
|
||||
}
|
||||
return found;
|
||||
}
|
||||
|
||||
describe("malformed JSON answers one status", () => {
|
||||
const parses = scanHandRolledParses();
|
||||
|
||||
it("finds the hand-rolled parses it is meant to judge", () => {
|
||||
// Eight when the guard was written. A matcher that stops matching would
|
||||
// otherwise report a clean pass it did not earn.
|
||||
expect(parses.length).toBeGreaterThanOrEqual(8);
|
||||
});
|
||||
|
||||
it("refuses an unparseable body with 400", () => {
|
||||
const wrong = parses
|
||||
.filter((parse) => {
|
||||
const allowed = ALLOWLIST[parse.route];
|
||||
if (allowed) return parse.status !== allowed.status;
|
||||
return parse.status !== 400;
|
||||
})
|
||||
.map((parse) => `${parse.route}:${parse.line} -> ${parse.status}`);
|
||||
expect(wrong).toEqual([]);
|
||||
});
|
||||
|
||||
it("keeps no stale allowlist entries", () => {
|
||||
const stale = Object.keys(ALLOWLIST).filter(
|
||||
(route) => !parses.some((parse) => parse.route === route),
|
||||
);
|
||||
expect(stale).toEqual([]);
|
||||
});
|
||||
|
||||
it("is the status the shared helper already answers with", async () => {
|
||||
const request = new Request("https://example.test/api/anything", {
|
||||
method: "POST",
|
||||
headers: { "content-type": "application/json" },
|
||||
body: "{ not json",
|
||||
});
|
||||
const result = await safeJson(request);
|
||||
expect(result.error?.status).toBe(400);
|
||||
expect(await result.error?.clone().json()).toMatchObject({
|
||||
data: null,
|
||||
error: "Invalid JSON body",
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps the wrong-content-type refusal separate at 415", async () => {
|
||||
const request = new Request("https://example.test/api/anything", {
|
||||
method: "POST",
|
||||
headers: { "content-type": "text/plain" },
|
||||
body: "{}",
|
||||
});
|
||||
const result = await safeJson(request);
|
||||
expect(result.error?.status).toBe(415);
|
||||
});
|
||||
});
|
||||
@@ -15,8 +15,9 @@
|
||||
* helper. The Coach prompt builder + snapshot builder both read this
|
||||
* row on every turn — there's no caching layer to invalidate.
|
||||
*/
|
||||
import { apiHandler, requireAuth, HttpError } from "@/lib/api-handler";
|
||||
import { apiHandler, requireAuth } from "@/lib/api-handler";
|
||||
import {
|
||||
apiError,
|
||||
apiSuccess,
|
||||
apiValidationError,
|
||||
sanitiseZodIssues,
|
||||
@@ -61,7 +62,9 @@ export const PUT = apiHandler(async (req: Request) => {
|
||||
try {
|
||||
body = await req.json();
|
||||
} catch {
|
||||
throw new HttpError(422, "coach-prefs.body.invalid_json");
|
||||
return apiError("Invalid JSON body", 400, {
|
||||
errorCode: "coach-prefs.body.invalid_json",
|
||||
});
|
||||
}
|
||||
|
||||
// v1.32.22 (M4) — pull the optimistic-concurrency base token off before the
|
||||
|
||||
@@ -65,7 +65,7 @@ export const PATCH = apiHandler(async (req: Request) => {
|
||||
try {
|
||||
body = await req.json();
|
||||
} catch {
|
||||
return apiError("Invalid JSON body", 422, {
|
||||
return apiError("Invalid JSON body", 400, {
|
||||
errorCode: "cycle-prefs.body.invalid_json",
|
||||
});
|
||||
}
|
||||
|
||||
@@ -212,7 +212,11 @@ describe("PATCH /api/auth/me/disable-coach", () => {
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects malformed JSON with 422", async () => {
|
||||
// 400, not the 422 this used to pin. A body that will not parse is a
|
||||
// client-side serialisation fault, and `safeJson` plus roughly two hundred
|
||||
// and twenty other routes have always answered 400 for it; the dotted token
|
||||
// moved to `meta.errorCode`, which is where a machine code belongs.
|
||||
it("rejects malformed JSON with 400", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(SESSION_OK as never);
|
||||
|
||||
const req = new Request("http://localhost/api/auth/me/disable-coach", {
|
||||
@@ -222,7 +226,11 @@ describe("PATCH /api/auth/me/disable-coach", () => {
|
||||
});
|
||||
|
||||
const res = await (PATCH as (r: Request) => Promise<Response>)(req);
|
||||
expect(res.status).toBe(422);
|
||||
expect(res.status).toBe(400);
|
||||
expect(await res.json()).toMatchObject({
|
||||
error: "Invalid JSON body",
|
||||
meta: { errorCode: "disable-coach.body.invalid_json" },
|
||||
});
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
||||
@@ -21,7 +21,7 @@
|
||||
*/
|
||||
import { z } from "zod";
|
||||
|
||||
import { apiHandler, requireAuth, HttpError } from "@/lib/api-handler";
|
||||
import { apiHandler, requireAuth } from "@/lib/api-handler";
|
||||
import {
|
||||
apiError,
|
||||
apiSuccess,
|
||||
@@ -78,7 +78,9 @@ export const PATCH = apiHandler(async (req: Request) => {
|
||||
try {
|
||||
body = await req.json();
|
||||
} catch {
|
||||
throw new HttpError(422, "disable-coach.body.invalid_json");
|
||||
return apiError("Invalid JSON body", 400, {
|
||||
errorCode: "disable-coach.body.invalid_json",
|
||||
});
|
||||
}
|
||||
|
||||
const parsed = patchBodySchema.safeParse(body);
|
||||
|
||||
@@ -285,7 +285,11 @@ describe("PATCH /api/auth/me/modules", () => {
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects malformed JSON with 422", async () => {
|
||||
// 400, not the 422 this used to pin. A body that will not parse is a
|
||||
// client-side serialisation fault, and `safeJson` plus roughly two hundred
|
||||
// and twenty other routes have always answered 400 for it; the dotted token
|
||||
// moved to `meta.errorCode`, which is where a machine code belongs.
|
||||
it("rejects malformed JSON with 400", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(SESSION_OK as never);
|
||||
const req = new Request("http://localhost/api/auth/me/modules", {
|
||||
method: "PATCH",
|
||||
@@ -293,7 +297,11 @@ describe("PATCH /api/auth/me/modules", () => {
|
||||
headers: { "Content-Type": "application/json" },
|
||||
});
|
||||
const res = await (PATCH as (r: Request) => Promise<Response>)(req);
|
||||
expect(res.status).toBe(422);
|
||||
expect(res.status).toBe(400);
|
||||
expect(await res.json()).toMatchObject({
|
||||
error: "Invalid JSON body",
|
||||
meta: { errorCode: "modules.body.invalid_json" },
|
||||
});
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
||||
@@ -25,7 +25,7 @@
|
||||
* carries it. The semantics are a DISABLED allowlist: a key set to
|
||||
* `false` disables that module; `true` (or absence) leaves it enabled.
|
||||
*/
|
||||
import { apiHandler, requireAuth, HttpError } from "@/lib/api-handler";
|
||||
import { apiHandler, requireAuth } from "@/lib/api-handler";
|
||||
import {
|
||||
apiError,
|
||||
apiSuccess,
|
||||
@@ -84,7 +84,9 @@ export const PATCH = apiHandler(async (req: Request) => {
|
||||
try {
|
||||
body = await req.json();
|
||||
} catch {
|
||||
throw new HttpError(422, "modules.body.invalid_json");
|
||||
return apiError("Invalid JSON body", 400, {
|
||||
errorCode: "modules.body.invalid_json",
|
||||
});
|
||||
}
|
||||
|
||||
// v1.32.22 (M3) — strip the optimistic-concurrency base token BEFORE the
|
||||
|
||||
@@ -324,7 +324,11 @@ describe("PATCH /api/auth/me/notification-prefs", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects malformed JSON with 422", async () => {
|
||||
// 400, not the 422 this used to pin. A body that will not parse is a
|
||||
// client-side serialisation fault, and `safeJson` plus roughly two hundred
|
||||
// and twenty other routes have always answered 400 for it; the dotted token
|
||||
// moved to `meta.errorCode`, which is where a machine code belongs.
|
||||
it("rejects malformed JSON with 400", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(SESSION_OK as never);
|
||||
|
||||
const req = new Request("http://localhost/api/auth/me/notification-prefs", {
|
||||
@@ -334,7 +338,11 @@ describe("PATCH /api/auth/me/notification-prefs", () => {
|
||||
});
|
||||
|
||||
const res = await (PATCH as (r: Request) => Promise<Response>)(req);
|
||||
expect(res.status).toBe(422);
|
||||
expect(res.status).toBe(400);
|
||||
expect(await res.json()).toMatchObject({
|
||||
error: "Invalid JSON body",
|
||||
meta: { errorCode: "notification-prefs.body.invalid_json" },
|
||||
});
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
||||
@@ -23,7 +23,7 @@
|
||||
* extra round-trip. Rate-limit is intentionally generous (60/min) —
|
||||
* matches the disable-coach route which is the closest analogue.
|
||||
*/
|
||||
import { apiHandler, requireAuth, HttpError } from "@/lib/api-handler";
|
||||
import { apiHandler, requireAuth } from "@/lib/api-handler";
|
||||
import {
|
||||
apiError,
|
||||
apiSuccess,
|
||||
@@ -86,7 +86,9 @@ export const PATCH = apiHandler(async (req: Request) => {
|
||||
try {
|
||||
body = await req.json();
|
||||
} catch {
|
||||
throw new HttpError(422, "notification-prefs.body.invalid_json");
|
||||
return apiError("Invalid JSON body", 400, {
|
||||
errorCode: "notification-prefs.body.invalid_json",
|
||||
});
|
||||
}
|
||||
|
||||
// v1.32.22 (M1) — pull the optimistic-concurrency base token off the body
|
||||
|
||||
@@ -18,6 +18,7 @@
|
||||
*/
|
||||
import { apiHandler, requireAuth, HttpError } from "@/lib/api-handler";
|
||||
import {
|
||||
apiError,
|
||||
apiSuccess,
|
||||
apiValidationError,
|
||||
getClientIp,
|
||||
@@ -53,7 +54,9 @@ export const PUT = apiHandler(async (req: Request) => {
|
||||
try {
|
||||
body = await req.json();
|
||||
} catch {
|
||||
throw new HttpError(422, "report-selection.body.invalid_json");
|
||||
return apiError("Invalid JSON body", 400, {
|
||||
errorCode: "report-selection.body.invalid_json",
|
||||
});
|
||||
}
|
||||
|
||||
const parsed = savedReportProfileSchema.safeParse(body ?? {});
|
||||
|
||||
@@ -18,8 +18,9 @@
|
||||
* decides which source's rows the cached targets / derived / analytics
|
||||
* payloads were built from.
|
||||
*/
|
||||
import { apiHandler, requireAuth, HttpError } from "@/lib/api-handler";
|
||||
import { apiHandler, requireAuth } from "@/lib/api-handler";
|
||||
import {
|
||||
apiError,
|
||||
apiSuccess,
|
||||
apiValidationError,
|
||||
getClientIp,
|
||||
@@ -53,7 +54,9 @@ export const PUT = apiHandler(async (req: Request) => {
|
||||
try {
|
||||
body = await req.json();
|
||||
} catch {
|
||||
throw new HttpError(422, "source-priority.body.invalid_json");
|
||||
return apiError("Invalid JSON body", 400, {
|
||||
errorCode: "source-priority.body.invalid_json",
|
||||
});
|
||||
}
|
||||
|
||||
const parsed = sourcePrioritySchema.safeParse(body ?? {});
|
||||
|
||||
@@ -33,6 +33,7 @@ import {
|
||||
errorEnvelope,
|
||||
idempotencyKeyParameter,
|
||||
idempotentWrite,
|
||||
malformedJsonResponse,
|
||||
recordRefusal,
|
||||
stdResponses,
|
||||
} from "./shared";
|
||||
@@ -991,6 +992,7 @@ export const cyclePaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
},
|
||||
},
|
||||
...stdResponses,
|
||||
...malformedJsonResponse,
|
||||
},
|
||||
},
|
||||
},
|
||||
|
||||
@@ -38,16 +38,17 @@ import {
|
||||
healthProfileFactKindSchema,
|
||||
} from "@/lib/validations/health-profile-facts";
|
||||
import {
|
||||
coachPrefsSchema,
|
||||
dataEnvelope,
|
||||
errorEnvelope,
|
||||
stdResponses,
|
||||
baseUpdatedAtField,
|
||||
updatedAtTokenField,
|
||||
coachPrefsSchema,
|
||||
conflictResponse409,
|
||||
invalidBaseTokenResponse,
|
||||
dataEnvelope,
|
||||
deviceRevokeResponse,
|
||||
errorEnvelope,
|
||||
invalidBaseTokenResponse,
|
||||
malformedJsonResponse,
|
||||
recordRefusal,
|
||||
stdResponses,
|
||||
updatedAtTokenField,
|
||||
} from "./shared";
|
||||
|
||||
// v1.18.0 — module enable/disable. The PATCH request is the REAL runtime
|
||||
@@ -1063,6 +1064,7 @@ export const profilePaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
},
|
||||
...conflictResponse409("Coach preferences", "coach_prefs_conflict"),
|
||||
...stdResponses,
|
||||
...malformedJsonResponse,
|
||||
...invalidBaseTokenResponse,
|
||||
},
|
||||
},
|
||||
@@ -1108,6 +1110,7 @@ export const profilePaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
},
|
||||
},
|
||||
...stdResponses,
|
||||
...malformedJsonResponse,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -1263,6 +1266,7 @@ export const profilePaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
},
|
||||
...conflictResponse409("Module preferences", "modules_conflict"),
|
||||
...stdResponses,
|
||||
...malformedJsonResponse,
|
||||
...invalidBaseTokenResponse,
|
||||
},
|
||||
},
|
||||
@@ -1310,6 +1314,7 @@ export const profilePaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
},
|
||||
},
|
||||
...stdResponses,
|
||||
...malformedJsonResponse,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -1505,6 +1510,7 @@ export const profilePaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
"notification_prefs_conflict",
|
||||
),
|
||||
...stdResponses,
|
||||
...malformedJsonResponse,
|
||||
...invalidBaseTokenResponse,
|
||||
},
|
||||
},
|
||||
@@ -1552,6 +1558,7 @@ export const profilePaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
},
|
||||
},
|
||||
...stdResponses,
|
||||
...malformedJsonResponse,
|
||||
},
|
||||
},
|
||||
},
|
||||
|
||||
@@ -225,6 +225,29 @@ export const invalidBaseTokenResponse = {
|
||||
},
|
||||
};
|
||||
|
||||
/**
|
||||
* The 400 an unparseable JSON body earns.
|
||||
*
|
||||
* `safeJson` has answered 400 for a body that will not parse since it was
|
||||
* written, and roughly two hundred and twenty routes go through it — but seven
|
||||
* `/api/auth/me/*` writes hand-rolled the parse and answered 422, so a client's
|
||||
* "my serializer produced garbage" branch had to accept two statuses on a
|
||||
* subset of routes it could not predict. They answer 400 now, and this is the
|
||||
* response that says so. The dotted token each route already carried moved to
|
||||
* `meta.errorCode`, where a machine code belongs.
|
||||
*
|
||||
* Spread only on the operations that hand-rolled the parse. It is deliberately
|
||||
* NOT folded into `stdResponses`: that set is spread onto reads as well, and a
|
||||
* GET that accepts no body has no malformed body to refuse.
|
||||
*/
|
||||
export const malformedJsonResponse = {
|
||||
"400": {
|
||||
description:
|
||||
"The request body is not parseable JSON. Nothing was read and nothing was written. `meta.errorCode` names the surface that refused it (`<surface>.body.invalid_json`). This is a client-side serialisation fault, not a validation failure — a body that parses but fails the schema is the 422 beside this.",
|
||||
content: { "application/json": { schema: errorEnvelope } },
|
||||
},
|
||||
};
|
||||
|
||||
// ── Standard 401 / 422 / 429 responses ───────────────────────────────
|
||||
|
||||
export const stdResponses = {
|
||||
|
||||
Reference in new issue
Block a user