From 7ee72d0161110da117c3e41bce56adc8ccdb2cb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 11 Oct 2026 14:21:56 +0200 Subject: [PATCH] fix(search): admit the search POST at read level through a declared, frozen readOverPost --- src/__tests__/read-over-post-guard.test.ts | 34 +++++++++++++++++++ src/app/api/search/__tests__/route.test.ts | 5 ++- src/app/api/search/route.ts | 6 +++- .../__tests__/acting-account-resolver.test.ts | 23 +++++++++++++ src/lib/api-handler.ts | 18 +++++++++- 5 files changed, 83 insertions(+), 3 deletions(-) create mode 100644 src/__tests__/read-over-post-guard.test.ts diff --git a/src/__tests__/read-over-post-guard.test.ts b/src/__tests__/read-over-post-guard.test.ts new file mode 100644 index 000000000..c699a9ad4 --- /dev/null +++ b/src/__tests__/read-over-post-guard.test.ts @@ -0,0 +1,34 @@ +/** + * `readOverPost` lets a delegable route answer a POST at the grant's read + * level. That widens what a read-only grant reaches, so the set of routes + * declaring it is frozen here: adding one is a reviewed diff, not an + * accident. Today it is the record search alone, whose query is health data + * that must stay out of URLs. + * + * The matcher tolerates whitespace and line breaks, and the guard asserts it + * matched something, so a rewrite that hides the declaration from the regex + * fails instead of passing on an empty set. + */ +import { describe, expect, it } from "vitest"; +import { readFileSync } from "node:fs"; +import { globSync } from "node:fs"; +import { join, relative, sep } from "node:path"; + +const ROOT = process.cwd(); + +const FROZEN = ["src/app/api/search/route.ts"]; + +describe("readOverPost declarations", () => { + it("are exactly the frozen set", () => { + const declaring = globSync("src/**/*.ts", { cwd: ROOT }) + .filter((file) => !file.includes(`${sep}__tests__${sep}`)) + .filter((file) => file !== join("src", "lib", "api-handler.ts")) + .filter((file) => + /readOverPost\s*:\s*true/.test(readFileSync(join(ROOT, file), "utf8")), + ) + .map((file) => relative(ROOT, join(ROOT, file)).split(sep).join("/")) + .sort(); + expect(declaring.length).toBeGreaterThan(0); + expect(declaring).toEqual(FROZEN); + }); +}); diff --git a/src/app/api/search/__tests__/route.test.ts b/src/app/api/search/__tests__/route.test.ts index 5f59997e4..479e3598f 100644 --- a/src/app/api/search/__tests__/route.test.ts +++ b/src/app/api/search/__tests__/route.test.ts @@ -111,7 +111,10 @@ beforeEach(() => { describe("POST /api/search", () => { it("admits the whole record only, at read", async () => { await handler(request("q=schulter")); - expect(mocks.requireRecordAuth).toHaveBeenCalledWith("read", "record"); + // A POST that only reads, declared as such. + expect(mocks.requireRecordAuth).toHaveBeenCalledWith("read", "record", { + readOverPost: true, + }); }); it("propagates a refused admission without searching", async () => { diff --git a/src/app/api/search/route.ts b/src/app/api/search/route.ts index 6e79aca2a..8493f39a7 100644 --- a/src/app/api/search/route.ts +++ b/src/app/api/search/route.ts @@ -56,7 +56,11 @@ function displayLocale(value: string | null | undefined): Locale { } export const POST = apiHandler(async (request: NextRequest) => { - const { user, actor, grantId } = await requireRecordAuth("read", "record"); + // A POST that only reads: the query rides the body to stay out of URLs, and + // a read grant on the whole record is what it needs, as it was over GET. + const { user, actor, grantId } = await requireRecordAuth("read", "record", { + readOverPost: true, + }); const rl = await checkRateLimit( `search:${actor.id}`, diff --git a/src/lib/__tests__/acting-account-resolver.test.ts b/src/lib/__tests__/acting-account-resolver.test.ts index 46cb1cf0c..6ed62b275 100644 --- a/src/lib/__tests__/acting-account-resolver.test.ts +++ b/src/lib/__tests__/acting-account-resolver.test.ts @@ -505,6 +505,29 @@ describe("requireRecordAuth — every way a grant can fail", () => { ); }); + it("reads a POST at read level only where the route declares readOverPost", async () => { + signedInCookie(OWNER); + liveGrant(); + const declared = route(() => + requireRecordAuth("read", "record", { readOverPost: true }), + ); + expect((await declared("POST")).status).toBe(200); + // The declaration covers POST alone: every other write method still + // needs a write grant. + for (const method of ["PUT", "PATCH", "DELETE"]) { + expect((await declared(method)).status, method).toBe(403); + } + }); + + it("does not lower a declared write for a readOverPost route", async () => { + signedInCookie(OWNER); + liveGrant(); + const response = await route(() => + requireRecordAuth("write", "record", { readOverPost: true }), + )("POST"); + expect(response.status).toBe(403); + }); + it("refuses a declared write under a read grant", async () => { signedInCookie(OWNER); liveGrant(); diff --git a/src/lib/api-handler.ts b/src/lib/api-handler.ts index 4f00f4b9e..7c6a1603b 100644 --- a/src/lib/api-handler.ts +++ b/src/lib/api-handler.ts @@ -889,6 +889,19 @@ export interface RecordAuthOptions { * else. */ readonly scope?: string; + /** + * v1.42.2 — this route answers a POST without changing anything: the body + * carries a query that must not stand in a URL (the record search, whose + * text is health data a proxy's access log would otherwise keep). Under a + * route declaring `"read"`, a POST is then read at the grant's read level + * instead of being escalated to a write. + * + * Like `scope`, naming it is a WIDENING and is frozen by + * `read-over-post-guard.test.ts`, so it lands in a reviewed diff. It does + * nothing for any other method, and nothing for a route declaring a need + * above `"read"`. + */ + readonly readOverPost?: true; } /** @@ -980,8 +993,11 @@ export async function requireRecordAuth( } const method = (getEvent()?.getHttpMethod() ?? "").toUpperCase(); + const readsOnly = + READ_HTTP_METHODS.has(method) || + (options?.readOverPost === true && method === "POST"); const effectiveNeed: GrantNeed = - need !== "read" || !READ_HTTP_METHODS.has(method) ? escalate(need) : "read"; + need !== "read" || !readsOnly ? escalate(need) : "read"; return resolveSwitchedRecord(auth, carrier, (grant) => { if (!grantAllows(grant, effectiveNeed)) return "insufficient_access";