diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 34ac2cfe8..8d65be753 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -7773,7 +7773,8 @@ paths: - Auth summary: Replace the owner's saved doctor-report selection description: Replace, not merge — the body states the whole scope or it is not a scope; a leaf omitted from the body is - a leaf removed from the saved profile. An unknown leaf id is refused with a 422 that names it. The saved + a leaf removed from the saved profile. An unknown leaf id is refused with a 422 carrying `meta.errorCode` + `report-selection.leaves.unknown` and `meta.unknownLeaves`, the ids this build does not know. The saved selection is also what the FHIR REST face and the MCP doctor-visit surfaces replay for a caller that cannot ask a human, so every write is audit-logged. Auth via cookie or Bearer. requestBody: @@ -29574,19 +29575,20 @@ components: onboarding.step.rateLimited, onboarding.tour.invalid, onboarding.user.notFound; personal_records — personal_records.invalid_query; practitioner — practitioner.invalid; profile — profile.update.emailInUse, profile.update.invalidBody, profile.update.nothingSaved; report-selection — - report-selection.body.invalid_json, report-selection.body.invalid_shape; route — route.retired; - share-link — share-link.document.decryptFailed, share-link.document.notFound, - share-link.document.rateLimited, share-link.documents.invalid, share-link.report.notFound, - share-link.report.rateLimited, share-link.selection.forbidden_leaf, share-link.selection.unknown_leaf; - sharing — sharing.accept.expired, sharing.accept.not_found, sharing.accept.not_pending, - sharing.accept.revoked, sharing.access.denied, sharing.invite.duplicate, sharing.invite.invalid, - sharing.invite.invalid_scope, sharing.invite.manage_browser_only, sharing.invite.refused, - sharing.invite.self, sharing.not_permitted, sharing.renounce.already_ended, sharing.renounce.not_found, - sharing.revoke.already_ended, sharing.revoke.not_found, sharing.session.changed, sharing.switch.invalid, - sharing.switch.wrong_transport; source-priority — source-priority.body.invalid_json, - source-priority.body.invalid_shape; tokens — tokens.measurements.ceiling_reached; vaccination — - vaccination.booster-invalid, vaccination.booster-no-antigen, vaccination.encounter-not-found, - vaccination.identity-required, vaccination.invalid, vaccination.link.invalid, vaccination.not-found, + report-selection.body.invalid_json, report-selection.body.invalid_shape, + report-selection.leaves.unknown; route — route.retired; share-link — share-link.document.decryptFailed, + share-link.document.notFound, share-link.document.rateLimited, share-link.documents.invalid, + share-link.report.notFound, share-link.report.rateLimited, share-link.selection.forbidden_leaf, + share-link.selection.unknown_leaf; sharing — sharing.accept.expired, sharing.accept.not_found, + sharing.accept.not_pending, sharing.accept.revoked, sharing.access.denied, sharing.invite.duplicate, + sharing.invite.invalid, sharing.invite.invalid_scope, sharing.invite.manage_browser_only, + sharing.invite.refused, sharing.invite.self, sharing.not_permitted, sharing.renounce.already_ended, + sharing.renounce.not_found, sharing.revoke.already_ended, sharing.revoke.not_found, + sharing.session.changed, sharing.switch.invalid, sharing.switch.wrong_transport; source-priority — + source-priority.body.invalid_json, source-priority.body.invalid_shape; tokens — + tokens.measurements.ceiling_reached; vaccination — vaccination.booster-invalid, + vaccination.booster-no-antigen, vaccination.encounter-not-found, vaccination.identity-required, + vaccination.invalid, vaccination.link.invalid, vaccination.not-found, vaccination.practitioner-not-found; workout — workout.batch.invalid, workout.batch.payload_too_large, workout.batch.too_large; (unprefixed) — about_me_conflict, ai_response_truncated, coach_prefs_conflict, dashboard_layout_conflict, empty_file, health_score_config_conflict, insights_layout_conflict, diff --git a/src/__tests__/zod-refusal-issue-list-guard.test.ts b/src/__tests__/zod-refusal-issue-list-guard.test.ts index 00031d521..57426b41b 100644 --- a/src/__tests__/zod-refusal-issue-list-guard.test.ts +++ b/src/__tests__/zod-refusal-issue-list-guard.test.ts @@ -18,13 +18,21 @@ * judged — a 400, a 401 or a 403 raised off a parse result is a different * refusal and is left alone. * + * The second half of the same audit item is the dotted token in `error`, and + * it is checked by its own sweep at the bottom of this file rather than by the + * per-site matcher above: `report-selection.leaves.unknown` was decided by + * `if (!minted.ok)` rather than by a parse result, so the block matcher could + * not see it while the string it answered with was the defect itself. The + * token sweep reads the refused string, not the condition that chose it, so a + * site cannot hide behind the shape of its own `if`. + * * Its limits, stated so nobody reads more into a green run than is there: * a refusal built somewhere other than the `if (!x.success)` block that decided * it — a helper called with the error, a status assembled from a variable — is - * invisible here, and so is a route that hands the `ZodError` to shared code - * outside `src/app/api`. It catches the shape this defect actually took, and - * the count assertion below is what keeps it from passing on a matcher that - * silently stopped matching anything. + * invisible to the issue-list half, and so is a route that hands the `ZodError` + * to shared code outside `src/app/api`. It catches the shape this defect + * actually took, and the count assertions below are what keep it from passing + * on a matcher that silently stopped matching anything. */ import { readFileSync, readdirSync } from "node:fs"; import { join, relative, sep } from "node:path"; @@ -106,11 +114,21 @@ function scanRefusalSites(): RefusalSite[] { )) { if (!parsed.has(m[1])) continue; const block = refusalBlock(src, (m.index ?? 0) + m[0].length); - if (!/\b422\b/.test(block)) continue; + // `returnAllZodIssues` and `apiValidationError` answer 422 unless told + // otherwise, so a block that calls one is a 422 site whether or not the + // number is spelled: a future `refuse422(parsed.error)` helper would + // otherwise be invisible for the same reason. + const carriesIssues = /returnAllZodIssues|apiValidationError/.test(block); + if (!/\b422\b/.test(block) && !carriesIssues) continue; + if ( + /\b(?:400|401|403|409|413|415|429|500|502)\b/.test(block) && + !/\b422\b/.test(block) + ) + continue; sites.push({ route: relative(process.cwd(), file).split(sep).join("/"), line: src.slice(0, m.index).split("\n").length, - carriesIssues: /returnAllZodIssues|apiValidationError/.test(block), + carriesIssues, }); } } @@ -122,9 +140,9 @@ describe("Zod shape refusals carry the issue list", () => { it("finds the refusal sites it is meant to judge", () => { // A matcher that stops matching would otherwise report a clean zero it did - // not earn. The tree carried 59 of these when the guard was written; the - // floor is deliberately loose so ordinary route churn does not trip it. - expect(sites.length).toBeGreaterThan(40); + // not earn. Two hundred and forty-eight sites when this was written, so + // the old floor of 40 would have survived losing five matches in six. + expect(sites.length).toBeGreaterThan(200); }); it("answers every 422 with details.issues", () => { @@ -141,3 +159,107 @@ describe("Zod shape refusals carry the issue list", () => { expect(stale).toEqual([]); }); }); + +/** + * The dotted token in `error`, wherever it is refused from. + * + * `error` is the field a client shows a person. A 422 that puts + * `report-selection.leaves.unknown` there hands the UI a string it must not + * render and gives it nothing else to branch on. That site was decided by + * `if (!minted.ok)`, not by a parse result, so the per-site matcher above ran + * past it — which is the argument for reading the refused string directly: + * whatever chose the refusal, the string is the defect. + * + * The inventory below is frozen and may only shrink. Each entry says why the + * token is still the whole `error` string; a new one fails until somebody + * either writes a sentence or writes the reason. + */ +const TOKEN_IN_ERROR_422: Record = { + "src/app/api/auth/me/coach-prefs/route.ts coach-prefs.body.invalid_shape": + "The status does not move for a shape refusal, so moving the string too would be an unforced second wire change; `meta.errorCode` now carries the same token.", + "src/app/api/auth/me/report-selection/route.ts report-selection.body.invalid_shape": + "Same shape refusal, same reason.", + "src/app/api/auth/me/source-priority/route.ts source-priority.body.invalid_shape": + "Same shape refusal, same reason.", + "src/app/api/insights/chat/messages/[id]/feedback/route.ts feedback.body.invalid": + "Same shape refusal, same reason.", + "src/app/api/insights/chat/route.ts coach.request.invalid": + "Same shape refusal, same reason.", + "src/app/api/auth/me/health-score-config/route.ts health-score-config.body.invalid_json": + "The published 422 description spells this exact body out as what a non-JSON request earns. It is the one operation whose contract promises the token, so the token is what it answers with.", + "src/app/api/coach/reminder-suggestions/route.ts coach.suggestion.unknownCadence": + "Predates this sweep and carries no `meta.errorCode` yet; the Coach reminder surfaces move as one piece or not at all.", + "src/app/api/coach/reminders/route.ts coach.reminder.invalidWhen": + "Predates this sweep, same surface.", + "src/app/api/coach/reminders/[id]/route.ts coach.reminder.invalidWhen": + "Predates this sweep, same surface.", + "src/app/api/coach/suggested-actions/route.ts coach.action.unknownInterval": + "Predates this sweep, same surface.", + "src/app/api/coach/suggested-actions/route.ts coach.reminder.invalidWhen": + "Predates this sweep, same surface.", +}; + +/** A dotted machine token: no spaces, at least one dot, no sentence in sight. */ +const DOTTED_TOKEN = /^[a-z][A-Za-z0-9_-]*(?:\.[A-Za-z0-9_-]+)+$/; + +/** Every `.ts` under `src/app/api`, tests excluded — not only `route.ts`. */ +function apiFiles(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(...apiFiles(full)); + continue; + } + if (entry.name.endsWith(".ts")) found.push(full); + } + return found; +} + +/** Every 422 whose `error` string is a dotted token, keyed ` `. */ +function tokenRefusals(): string[] { + const found: string[] = []; + for (const file of apiFiles(API_ROOT)) { + const src = readFileSync(file, "utf8"); + const route = relative(process.cwd(), file).split(sep).join("/"); + const shapes = [ + /apiError\(\s*"([^"]+)"\s*,\s*422/g, + /apiValidationError\(\s*"([^"]+)"/g, + /new HttpError\(\s*422\s*,\s*"([^"]+)"/g, + ]; + for (const shape of shapes) { + for (const m of src.matchAll(shape)) { + if (!DOTTED_TOKEN.test(m[1])) continue; + const key = `${route} ${m[1]}`; + if (!found.includes(key)) found.push(key); + } + } + } + return found; +} + +describe("a 422 does not answer with a machine token where the sentence goes", () => { + const refusals = tokenRefusals(); + + it("finds the sites it is meant to judge", () => { + // Eleven when this was written, twelve before the report-selection leaf + // refusal was rewritten. A matcher that stopped matching would agree with + // an empty inventory. + expect(refusals.length).toBeGreaterThan(8); + }); + + it("keeps the token in `error` only where a reason is written down", () => { + const unexplained = refusals + .filter((key) => !(key in TOKEN_IN_ERROR_422)) + .sort(); + expect(unexplained).toEqual([]); + }); + + it("keeps no stale inventory entry", () => { + const stale = Object.keys(TOKEN_IN_ERROR_422) + .filter((key) => !refusals.includes(key)) + .sort(); + expect(stale).toEqual([]); + }); +}); diff --git a/src/app/api/auth/me/coach-prefs/route.ts b/src/app/api/auth/me/coach-prefs/route.ts index 7739a4ea2..4e1bf8593 100644 --- a/src/app/api/auth/me/coach-prefs/route.ts +++ b/src/app/api/auth/me/coach-prefs/route.ts @@ -81,9 +81,11 @@ export const PUT = apiHandler(async (req: Request) => { action: { name: "auth.me.coach-prefs.put.invalid" }, meta: { issues: parsed.error.issues.length }, }); - // The dotted token keeps its place in `error` — clients already match on it - // there — while `meta.errorCode` publishes it in the field a machine code - // belongs in, and `details.issues` names the fields that were refused. + // The dotted token keeps its place in `error`: this refusal keeps its 422, + // so moving the string would be a wire change with nothing forcing it — + // the `invalid_json` siblings moved only because their status moved to 400 + // anyway. `meta.errorCode` publishes the same token in the field a machine + // code belongs in, and `details.issues` names the fields that were refused. return apiValidationError( "coach-prefs.body.invalid_shape", sanitiseZodIssues(parsed.error.issues), diff --git a/src/app/api/auth/me/report-selection/route.ts b/src/app/api/auth/me/report-selection/route.ts index e7aeed85b..bbb85b9c2 100644 --- a/src/app/api/auth/me/report-selection/route.ts +++ b/src/app/api/auth/me/report-selection/route.ts @@ -16,7 +16,7 @@ * Replaces `GET`/`PUT /api/auth/me/doctor-report-prefs`, whose column nothing * outside that route ever read. */ -import { apiHandler, requireAuth, HttpError } from "@/lib/api-handler"; +import { apiHandler, requireAuth } from "@/lib/api-handler"; import { apiError, apiSuccess, @@ -65,9 +65,11 @@ export const PUT = apiHandler(async (req: Request) => { action: { name: "auth.me.report-selection.put.invalid" }, meta: { issues: parsed.error.issues.length }, }); - // The dotted token keeps its place in `error` — clients already match on it - // there — while `meta.errorCode` publishes it in the field a machine code - // belongs in, and `details.issues` names the fields that were refused. + // The dotted token keeps its place in `error`: this refusal keeps its 422, + // so moving the string would be a wire change with nothing forcing it — + // the `invalid_json` siblings moved only because their status moved to 400 + // anyway. `meta.errorCode` publishes the same token in the field a machine + // code belongs in, and `details.issues` names the fields that were refused. return apiValidationError( "report-selection.body.invalid_shape", sanitiseZodIssues(parsed.error.issues), @@ -82,7 +84,17 @@ export const PUT = apiHandler(async (req: Request) => { action: { name: "auth.me.report-selection.put.invalid" }, meta: { unknownLeaves: minted.error.unknownLeaves }, }); - throw new HttpError(422, "report-selection.leaves.unknown"); + // Not a Zod refusal, so there is no issue list to carry — but the same + // rule applies: a sentence in `error`, the machine token in + // `meta.errorCode`, and the leaves that were refused named beside it. + return apiError( + "Report selection names leaves this build does not know", + 422, + { + errorCode: "report-selection.leaves.unknown", + unknownLeaves: minted.error.unknownLeaves, + }, + ); } // Persist the canonical ordering rather than the caller's, so two clients diff --git a/src/app/api/auth/me/source-priority/route.ts b/src/app/api/auth/me/source-priority/route.ts index f1c50337e..d5897cd1d 100644 --- a/src/app/api/auth/me/source-priority/route.ts +++ b/src/app/api/auth/me/source-priority/route.ts @@ -65,9 +65,11 @@ export const PUT = apiHandler(async (req: Request) => { action: { name: "auth.me.source-priority.put.invalid" }, meta: { issues: parsed.error.issues.length }, }); - // The dotted token keeps its place in `error` — clients already match on it - // there — while `meta.errorCode` publishes it in the field a machine code - // belongs in, and `details.issues` names the fields that were refused. + // The dotted token keeps its place in `error`: this refusal keeps its 422, + // so moving the string would be a wire change with nothing forcing it — + // the `invalid_json` siblings moved only because their status moved to 400 + // anyway. `meta.errorCode` publishes the same token in the field a machine + // code belongs in, and `details.issues` names the fields that were refused. return apiValidationError( "source-priority.body.invalid_shape", sanitiseZodIssues(parsed.error.issues), diff --git a/src/app/api/insights/chat/messages/[id]/feedback/route.ts b/src/app/api/insights/chat/messages/[id]/feedback/route.ts index 668a06599..4056d7eb9 100644 --- a/src/app/api/insights/chat/messages/[id]/feedback/route.ts +++ b/src/app/api/insights/chat/messages/[id]/feedback/route.ts @@ -93,9 +93,11 @@ async function handlePost(request: NextRequest, ctx: RouteContext) { action: { name: "insights.coach.message.feedback" }, meta: { outcome: "validation_failed" }, }); - // The dotted token keeps its place in `error` — clients already match on it - // there — while `meta.errorCode` publishes it in the field a machine code - // belongs in, and `details.issues` names the fields that were refused. + // The dotted token keeps its place in `error`: this refusal keeps its 422, + // so moving the string would be a wire change with nothing forcing it — + // the `invalid_json` siblings moved only because their status moved to 400 + // anyway. `meta.errorCode` publishes the same token in the field a machine + // code belongs in, and `details.issues` names the fields that were refused. return apiValidationError( "feedback.body.invalid", sanitiseZodIssues(parsed.error.issues), diff --git a/src/app/api/insights/chat/route.ts b/src/app/api/insights/chat/route.ts index e14cdff7e..9dc19d79d 100644 --- a/src/app/api/insights/chat/route.ts +++ b/src/app/api/insights/chat/route.ts @@ -214,9 +214,11 @@ async function handleChatRequest(request: NextRequest): Promise { action: { name: "insights.coach.invalid" }, meta: { issues: parsed.error.issues.length }, }); - // The dotted token keeps its place in `error` — clients already match on it - // there — while `meta.errorCode` publishes it in the field a machine code - // belongs in, and `details.issues` names the fields that were refused. + // The dotted token keeps its place in `error`: this refusal keeps its 422, + // so moving the string would be a wire change with nothing forcing it — + // the `invalid_json` siblings moved only because their status moved to 400 + // anyway. `meta.errorCode` publishes the same token in the field a machine + // code belongs in, and `details.issues` names the fields that were refused. return apiValidationError( "coach.request.invalid", sanitiseZodIssues(parsed.error.issues), diff --git a/src/lib/openapi/error-codes.ts b/src/lib/openapi/error-codes.ts index 9ba5e0a47..67ef9f148 100644 --- a/src/lib/openapi/error-codes.ts +++ b/src/lib/openapi/error-codes.ts @@ -254,6 +254,7 @@ export const ERROR_CODE_CATALOGUE: Readonly> = "report-selection": [ "report-selection.body.invalid_json", "report-selection.body.invalid_shape", + "report-selection.leaves.unknown", ], route: ["route.retired"], "share-link": [ diff --git a/src/lib/openapi/routes/profile.ts b/src/lib/openapi/routes/profile.ts index 5c02cfcb7..2ce21fc07 100644 --- a/src/lib/openapi/routes/profile.ts +++ b/src/lib/openapi/routes/profile.ts @@ -1540,7 +1540,7 @@ export const profilePaths: NonNullable = { tags: ["Auth"], summary: "Replace the owner's saved doctor-report selection", description: - "Replace, not merge — the body states the whole scope or it is not a scope; a leaf omitted from the body is a leaf removed from the saved profile. An unknown leaf id is refused with a 422 that names it. The saved selection is also what the FHIR REST face and the MCP doctor-visit surfaces replay for a caller that cannot ask a human, so every write is audit-logged. Auth via cookie or Bearer.", + "Replace, not merge — the body states the whole scope or it is not a scope; a leaf omitted from the body is a leaf removed from the saved profile. An unknown leaf id is refused with a 422 carrying `meta.errorCode` `report-selection.leaves.unknown` and `meta.unknownLeaves`, the ids this build does not know. The saved selection is also what the FHIR REST face and the MCP doctor-visit surfaces replay for a caller that cannot ask a human, so every write is audit-logged. Auth via cookie or Bearer.", requestBody: { required: true, content: { "application/json": { schema: savedReportProfile } },