Scope the idempotency replay cell to the record a request claims

A delegate retrying with one `Idempotency-Key` across two people's records met
itself in one cell. The cache tuple was `(caller, key, method, path)`, so the
second record's request replayed the first record's cached 201: the handler
never ran, no row was written for the second person, and the caller was told it
had been — with the other person's id in the body. Nothing errored and nothing
logged a conflict, and five of the write verbs about to become delegable are
wrapped here.

Fold the claimed record into the `key` component, centrally, once. Not into
`userId`: that column is a foreign key to `users` with a cascade behind it and
cannot hold a composite. It keeps naming the caller, which is what makes two
delegates on one record two cells for free. The separator is a byte the key
validator refuses, so a caller cannot hand-craft a key into a cell belonging to
a request they are not making.

A request with no acting account keys byte-for-byte as it did before, so a
client that never heard of sharing cannot be affected.

Accepted and pinned by test: the fold uses the CLAIM, before the grant check,
so within the 24h window a delegate whose access has ended can still replay a
key they had already completed. They receive bytes they already had, nothing
new is written, and a fresh key from them is refused because that request
reaches the handler.

Both frozen surfaces move in this diff. The claim reader gains a guard leg
freezing its consumers, since an unchecked claim is exactly the value a future
caller must not mistake for permission.
This commit is contained in:
Marc-André Bombeck committed 2026-08-03 10:18:27 +02:00
1 parent f24ae6deb8
commit c7960d726d
7 files changed
+563 -8

No files matched your search

@@ -74,6 +74,9 @@ describe("the acting-account carrier is read in one place", () => {
const CARRIER_ALLOWLIST = [
// Projects it off the session row it already loaded.
"lib/auth/session.ts",
// Reads it. The one statement of "what does this request say it is acting
// as", for both transports and for every caller that has to ask.
"lib/auth/acting-carrier.ts",
// The resolver. The only thing that acts on the value.
"lib/api-handler.ts",
// v1.36.0 — the only thing that WRITES it: the switch endpoint sets it
@@ -117,6 +120,28 @@ describe("the acting-account carrier is read in one place", () => {
"lib/openapi/routes/account-sharing.ts",
]);
});
/**
* Who may ask "which record does this request claim" without an auth
* context in hand.
*
* `readClaimedActingAccount()` answers before any grant check has run, so a
* caller that treats its answer as permission has skipped the only thing
* that grants any. Exactly one caller exists, beside the file that declares
* it: the idempotency wrapper, which uses the claim to pick a cache cell and
* leaves the check to the handler it wraps. Everything else that needs the
* acting account has an auth context by then and goes through
* `requireRecordAuth`, which checks a live grant before returning. A second
* caller here is a design question, not a patch.
*/
it("the unchecked claim has exactly two consumers", () => {
const consumers = filesMatching(/readClaimedActingAccount/);
expect(consumers.length).toBeGreaterThan(0);
expect(consumers).toEqual([
"lib/auth/acting-carrier.ts",
"lib/idempotency.ts",
]);
});
});
describe("the cookie-only helpers cannot see the acting account", () => {
@@ -65,9 +65,13 @@ describe("T1 — the Bearer resolution set is frozen", () => {
// `defaultUserIdResolver` — resolves the Bearer token to a user id so an
// idempotency key files under its owner. It authorises nothing: the
// handler still runs its own `requireAuth()`, and this resolver only
// decides which user's cache cell a replay lands in. Narrow in reach and
// deliberate, but it is a raw Bearer value becoming a user id, so it
// belongs on this list rather than beside it.
// decides which cache cell a replay lands in. Since delegated writes it
// decides that from two values — the caller resolved here, and the record
// the request claims, read through the one acting-account carrier reader
// and folded into the key. Both are unchecked selectors; neither widens
// what the handler will do. Narrow in reach and deliberate, but it is a
// raw Bearer value becoming a user id, so it belongs on this list rather
// than beside it.
"lib/idempotency.ts",
].sort();
@@ -75,7 +75,16 @@ function request(): NextRequest {
beforeEach(() => {
vi.clearAllMocks();
// The whole shape `getSession()` returns. The session half carries the
// acting-account carrier, which the idempotency wrapper reads to decide
// whose record a cell belongs to — a fake that omitted it made the wrapper
// fail on a field production always has.
mocks.getSession.mockResolvedValue({
session: {
id: "session-1",
expiresAt: new Date(Date.now() + 60_000),
actingAsUserId: null,
},
user: { id: "user-1", role: "USER" },
});
mocks.findUnique.mockResolvedValue(null);
+138 -1
View File
@@ -219,7 +219,11 @@ describe("withIdempotency", () => {
);
const wrapped = withIdempotency<[NextRequest]>(handler);
await wrapped(makeRequest("POST", { "idempotency-key": "abc-12345678" }));
expect(getSession).toHaveBeenCalledTimes(1);
// Twice: once to resolve who is calling, once to read which record they
// say they are acting on. The second read is what a delegated write costs
// here, and it is a single indexed lookup on a request that is about to
// write anyway.
expect(getSession).toHaveBeenCalledTimes(2);
expect(prisma.idempotencyKey.create).toHaveBeenCalledTimes(1);
const persisted = vi.mocked(prisma.idempotencyKey.create).mock
.calls[0][0] as { data: { userId: string } };
@@ -536,3 +540,136 @@ describe("isCachableStatus do-not-cache rules (V3 audit)", () => {
expect(stored.length).toBeGreaterThan(0);
});
});
/**
* The cell is scoped to the record the request claims, not only to the caller.
*
* `tests/integration/idempotency-record-scoped-cell.test.ts` proves the rows.
* These prove the key the wrapper composes, which is what decides whether two
* requests meet in one cell — including the two cases a row-level test cannot
* show, because both end in no row at all.
*/
describe("withIdempotency — record-scoped cells", () => {
const CLIENT_KEY = "abc-12345678";
/** Present these headers to the wrapper for the next call. */
function present(values: Record<string, string>): void {
vi.mocked(headers).mockResolvedValue({
get: (name: string) => values[name.toLowerCase()] ?? null,
} as unknown as ReturnType<typeof headers> extends Promise<infer T>
? T
: never);
}
/** Sign the caller in over the cookie transport, optionally switched. */
function cookieSession(actingAsUserId: string | null): void {
vi.mocked(getSession).mockResolvedValue({
session: {
id: "s-1",
expiresAt: new Date(Date.now() + 60_000),
actingAsUserId,
},
user: { id: "u-1", role: "USER" },
} as Awaited<ReturnType<typeof getSession>>);
}
async function run(key = CLIENT_KEY): Promise<void> {
const handler = vi.fn(async () =>
NextResponse.json({ data: { ok: true }, error: null }, { status: 201 }),
);
const wrapped = withIdempotency<[NextRequest]>(handler, async () => "u-1");
await wrapped(makeRequest("POST", { "idempotency-key": key }));
}
/** The key the wrapper looked the cell up under. */
function lookedUpKey(): string {
const call = vi.mocked(prisma.idempotencyKey.findUnique).mock
.calls[0][0] as {
where: { userId_key_method_path: { userId: string; key: string } };
};
return call.where.userId_key_method_path.key;
}
/** The row the wrapper claimed. */
function claimed(): { userId: string; key: string } {
return (
vi.mocked(prisma.idempotencyKey.create).mock.calls[0][0] as {
data: { userId: string; key: string };
}
).data;
}
it("folds the account named by the Bearer selector into the key", async () => {
vi.mocked(getSession).mockResolvedValue(null);
present({ "x-healthlog-account": "owner-9" });
await run();
expect(lookedUpKey()).toBe(`owner-9|${CLIENT_KEY}`);
// The owner column stays the ACTOR. It is a foreign key to `users` with a
// cascade behind it, so it can only ever hold a real account id — which is
// why the record moved into the key and not into here.
expect(claimed()).toMatchObject({
userId: "u-1",
key: `owner-9|${CLIENT_KEY}`,
});
});
it("folds the account the cookie session is switched to", async () => {
cookieSession("owner-7");
present({});
await run();
expect(claimed().key).toBe(`owner-7|${CLIENT_KEY}`);
});
it("keys a request with no acting account byte-for-byte as the client sent it", async () => {
cookieSession(null);
present({});
await run();
expect(lookedUpKey()).toBe(CLIENT_KEY);
expect(claimed()).toMatchObject({ userId: "u-1", key: CLIENT_KEY });
});
it("ignores a selector sent over the cookie transport", async () => {
// The resolver refuses that request outright (`misplaced_selector`). The
// cell must not pretend it named a record either — a claim the request was
// never going to be served must not move where its answer is filed.
cookieSession(null);
present({ "x-healthlog-account": "owner-9" });
await run();
expect(claimed().key).toBe(CLIENT_KEY);
});
it("refuses a client key that carries the separator", async () => {
// The one way a caller could aim at somebody else's cell: send
// `owner-9|abc-12345678` as the key from their own un-switched session. The
// key validator does not admit the byte, so the header is malformed and the
// wrapper does not cache at all.
vi.mocked(getSession).mockResolvedValue(null);
present({});
await run(`owner-9|${CLIENT_KEY}`);
expect(prisma.idempotencyKey.findUnique).not.toHaveBeenCalled();
expect(prisma.idempotencyKey.create).not.toHaveBeenCalled();
});
it("skips the cache when the claimed account names nothing that could exist", async () => {
// Longer than any account id. The request is refused downstream; caching it
// would let a caller write an arbitrarily long key into the table on the
// way to a 403.
vi.mocked(getSession).mockResolvedValue(null);
present({ "x-healthlog-account": "x".repeat(65) });
await run();
expect(prisma.idempotencyKey.findUnique).not.toHaveBeenCalled();
expect(prisma.idempotencyKey.create).not.toHaveBeenCalled();
});
});
+52 -3
View File
@@ -1,9 +1,14 @@
/**
* What a request says it is acting as.
*
* It lives apart from `requireRecordAuth`, which acts on the answer, because
* asking the question and deciding what it permits are different jobs and the
* asking is about to have more than one caller.
* Two things in the app have to answer that question, and they have to answer
* it the same way:
*
* * `requireRecordAuth` (`src/lib/api-handler.ts`), which turns the claim
* into a data scope after checking a live grant.
* * `withIdempotency` (`src/lib/idempotency.ts`), which files the replay cell
* under the record the request claims — before any handler runs, so it
* cannot wait for the grant check the handler performs.
*
* The claim is a SELECTOR and never an authorisation. Nothing in this file
* looks at a grant, and no caller may treat a carrier as permission: the
@@ -25,6 +30,8 @@
*/
import { headers } from "next/headers";
import { getSession } from "@/lib/auth/session";
/**
* The per-request account selector, on the Bearer transport only.
*
@@ -103,3 +110,45 @@ export function carrierTarget(carrier: ActingCarrier): string | null {
export function selectorNamesAnAccount(value: string): boolean {
return value.length > 0 && value.length <= MAX_SELECTOR_LENGTH;
}
/**
* The account this request CLAIMS, resolved without an auth context in hand.
*
* For callers that run BEFORE authentication has been resolved into a context —
* today that is the idempotency wrapper, which has to know which record a
* request is aimed at before it decides whether the answer is already cached.
* It resolves the session itself for exactly one reason: the transport decides
* which carrier counts, and "there is a valid session" is what the transport
* is. Guessing it from the presence of a cookie or an `Authorization` header
* would be a second, weaker answer to a question this file exists to answer
* once — and the case it gets wrong (a stale session cookie beside a live
* Bearer token) is precisely a delegated write landing in the wrong cell.
*
* The cost is one indexed session lookup on a request that is already about to
* write. It is not shared with the caller's own auth resolution because the
* resolver a route passes to `withIdempotency` is pluggable, and threading a
* session through that seam would make every custom resolver responsible for a
* question none of them asks.
*
* Returns the CLAIMED account, unverified and unchecked. The grant check
* happens later, inside the handler, on every request.
*/
export async function readClaimedActingAccount(): Promise<string | null> {
const header = await readSelectorHeader();
let session: Awaited<ReturnType<typeof getSession>> = null;
try {
session = await getSession();
} catch {
// A session that cannot be resolved is not a switched one. The request
// still has to be authenticated by whatever wraps this call, and an
// unauthenticated request claims nothing.
session = null;
}
return carrierTarget(
decideActingCarrier({
transport: session ? "cookie" : "bearer",
stamped: session?.session.actingAsUserId ?? null,
header,
}),
);
}
+65 -1
View File
@@ -6,12 +6,21 @@
* retry within the TTL (24h) for the same `(userId, key, method, path)`
* tuple returns the cached envelope as a 200 (carrying the original
* status inside the JSON if needed) — no second side-effect.
*
* `userId` is the CALLER. Since a caller can act on somebody else's record,
* the record the request claims is folded into `key` (see `cellKey`), so the
* tuple identifies a request to one person's record rather than a request by
* one person. A caller acting only as themselves is unaffected, byte for byte.
*/
import type { NextRequest } from "next/server";
import { headers } from "next/headers";
import { NextResponse } from "next/server";
import { prisma } from "@/lib/db";
import { getSession } from "@/lib/auth/session";
import {
readClaimedActingAccount,
selectorNamesAnAccount,
} from "@/lib/auth/acting-carrier";
import { hashToken } from "@/lib/auth/hmac";
import { annotate } from "@/lib/logging/context";
import { isP2002 } from "@/lib/prisma-errors";
@@ -37,6 +46,34 @@ const PENDING_STATUS = 0;
const KEY_REGEX = /^[A-Za-z0-9_\-:.]{8,128}$/;
/**
* Separates the record from the client's key inside a cell's `key` column.
*
* `KEY_REGEX` above does not admit it, which is the whole reason it was
* chosen: a caller cannot hand-craft a key that lands in the cell of a request
* they are not making. Changing either one without the other reopens that.
*/
const RECORD_SEPARATOR = "|";
/**
* The `key` column for a request, given the record it claims to act on.
*
* Why the key and not the owner column: `IdempotencyKey.userId` is a foreign
* key to `users` with a cascade behind it (`prisma/schema.prisma`), so it can
* hold one real account id and nothing composite. It keeps holding the ACTOR —
* which is what makes "two delegates, one owner, one key" two cells without
* anything extra — and the record joins the other half of the composite unique
* index instead.
*
* No carrier means the key the client sent, byte for byte. Every request that
* is nobody's delegate therefore files exactly where it filed before, and the
* fold cannot change the behaviour of a client that never heard of sharing.
*/
function cellKey(clientKey: string, actingAccountId: string | null): string {
if (actingAccountId === null) return clientKey;
return `${actingAccountId}${RECORD_SEPARATOR}${clientKey}`;
}
export interface IdempotencyContext {
userId: string;
key: string;
@@ -288,6 +325,11 @@ export function isCachableStatus(status: number): boolean {
* /external clients — without it, every Bearer-authed retry was running
* the handler again and creating duplicate measurements (audit C-4).
*
* It resolves the CALLER and only the caller — which record they are aiming at
* is a separate question, answered by `readClaimedActingAccount()` above and
* folded into the key. Neither answer authorises anything: the handler runs
* its own `requireAuth()` / `requireRecordAuth()` either way.
*
* Exported for unit testing; production callers should let
* `withIdempotency()` pick this up automatically via its default arg.
*/
@@ -374,10 +416,32 @@ export function withIdempotency<
const userId = await userIdResolver(...args);
if (!userId) return handler(...args);
// Which RECORD this request is aimed at, as claimed by the transport and
// not yet checked against a grant — the check belongs to the handler, and
// this runs before it. One delegate posting the same key first for one
// person and then for another must not meet themselves in a single cell:
// that replays the first person's response, writes nothing to the second
// person's record, and reports success. Nothing errors and nothing logs a
// conflict, which is why it is folded here rather than left to call sites.
//
// The consequence of using the CLAIM: inside the 24h window a delegate
// whose grant has since ended can still replay a key they already
// completed, without the grant check running. They receive bytes they
// already had and nothing new is written — a fresh key from them is
// refused, because that request reaches the handler.
const claimedRecord = await readClaimedActingAccount();
if (claimedRecord !== null && !selectorNamesAnAccount(claimedRecord)) {
// A claim that names no account that could exist. The request is refused
// downstream; skipping the cache keeps a caller from writing an
// arbitrarily long key into the table on the way to that refusal, and
// never lets one fall back onto the un-delegated cell.
return handler(...args);
}
const url = new URL(request.url);
const ctx: IdempotencyContext = {
userId,
key,
key: cellKey(key, claimedRecord),
method: request.method,
path: url.pathname,
};