From 19b69dee21018823fec1021274ef4813b9048034 Mon Sep 17 00:00:00 2001 From: Antiheld86 <4201585+Antiheld86@users.noreply.github.com> Date: Mon, 31 Aug 2026 11:57:26 +0000 Subject: [PATCH] Fix the B8 delegation cases to actually claim a record Three failures in the integration run, all mine, none in the product. The two delegation cases set the account selector as a header on the NextRequest. Nothing reads it there. `readSelectorHeader()` resolves through `next/headers`, which this suite mocks against the shared `headerJar`, so the carrier came back `none` and both cases quietly exercised the ordinary own-record path instead of the one they name. The scoped case wrote to its own record and returned 201; the wildcard control wrote to the caller rather than the owner. Neither was testing delegation at all. That is worse than a broken test. The refusal these two cover is the regression this change could most easily have shipped in silence, and the tests standing in front of it were not looking at it. They now set the selector on the jar, through the exported ACCOUNT_SELECTOR_HEADER rather than a literal, so a rename cannot reintroduce the same gap. The third failure is unrelated and smaller: the read-leg case read the audit row once, racing a fire-and-forget write, and reported `undefined` for a row about to exist. It now polls, matching B1, which carries a comment about this exact hazard that I did not follow. Not verified here: this environment has no Docker daemon, so the integration suite cannot be run against a real Postgres. The diagnosis is from the mock and the resolver, not from a green run. --- .../bearer-scope-enforcement.test.ts | 44 ++++++++++++++----- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/tests/integration/bearer-scope-enforcement.test.ts b/tests/integration/bearer-scope-enforcement.test.ts index 3677027f0..08c759e24 100644 --- a/tests/integration/bearer-scope-enforcement.test.ts +++ b/tests/integration/bearer-scope-enforcement.test.ts @@ -25,6 +25,10 @@ process.env.API_TOKEN_HMAC_KEY ??= "test-hmac-key-bearer-scope-enforcement-32-bytes-min-0987654321"; const { hashToken } = await import("@/lib/auth/hmac"); +// The canonical name rather than a literal, so a rename cannot leave these +// cases setting a header nothing reads — which is the exact failure the +// delegation cases below already shipped once. +const { ACCOUNT_SELECTOR_HEADER } = await import("@/lib/auth/acting-carrier"); const USER_ID = "user-bearer-scope-test"; const MCP_RESOURCE = "https://health.example/mcp"; @@ -475,15 +479,27 @@ describe("B8 — the measurement-ingest scope reaches its two routes and no othe }); } - function postMeasurement(selector?: string): NextRequest { - const headers: Record = { - authorization: headerJar.get("authorization")!, - "content-type": "application/json", - }; - if (selector) headers["x-healthlog-account"] = selector; + /** + * Claim a record for the next request. + * + * It goes in the `headerJar`, NOT on the `NextRequest`, and the difference + * is the whole reason the first version of these cases passed for the wrong + * reason. `readSelectorHeader()` resolves through `next/headers`, which this + * suite mocks against the jar — a header set on the request object is never + * read by the resolver, so the carrier came back `none` and the delegation + * cases were silently exercising the ordinary own-record path. + */ + function claimRecord(accountId: string): void { + headerJar.set(ACCOUNT_SELECTOR_HEADER, accountId); + } + + function postMeasurement(): NextRequest { return new NextRequest("https://health.example/api/measurements", { method: "POST", - headers, + headers: { + authorization: headerJar.get("authorization")!, + "content-type": "application/json", + }, body: measurementBody(), } as never); } @@ -602,7 +618,13 @@ describe("B8 — the measurement-ingest scope reaches its two routes and no othe const res = await GET(req("/api/measurements")); expect(res.status).toBe(403); - expect(await lastBearerFailureReason()).toBe("undeclared_scope"); + // Polled, not read once: the audit write is fire-and-forget, so a bare + // read races the request and reports `undefined` for a row that is about + // to exist. Same posture as B1 above — on timeout the poll asserts against + // whatever WAS written rather than passing by default. + await expect + .poll(() => lastBearerFailureReason(), { timeout: 5_000, interval: 100 }) + .toBe("undeclared_scope"); }); it("is refused on the manage legs and on an unrelated surface", async () => { @@ -689,9 +711,10 @@ describe("B8 — the measurement-ingest scope reaches its two routes and no othe it("is refused a shared record its own holder may genuinely write to", async () => { const ownerId = await seedSharedOwner(); await armToken(["measurements:write"], "mwrite9"); + claimRecord(ownerId); const { POST } = await import("@/app/api/measurements/route"); - const res = await POST(postMeasurement(ownerId)); + const res = await POST(postMeasurement()); expect(res.status).toBe(403); // Nothing landed on the owner, and nothing quietly landed on the holder @@ -704,9 +727,10 @@ describe("B8 — the measurement-ingest scope reaches its two routes and no othe // outright would make that 403 look like the scope working. const ownerId = await seedSharedOwner(); await armToken(["*"], "wildshared"); + claimRecord(ownerId); const { POST } = await import("@/app/api/measurements/route"); - const res = await POST(postMeasurement(ownerId)); + const res = await POST(postMeasurement()); expect(res.status).toBe(201); const row = await getPrismaClient().measurement.findFirstOrThrow({