diff --git a/CHANGELOG.md b/CHANGELOG.md index 53cb335cb..5654e7f10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ # Changelog +## [Unreleased] + +### Fixed + +- **Blood pressure saved without its diastolic value.** The iPhone app (up to + 1.0.3, and TestFlight 1.1.0) sends the two halves of a manual reading as two + requests under one `Idempotency-Key`, and the server answered the second + with the first one's cached response instead of saving it. The replay cache + now stores a SHA-256 fingerprint of the request body (migration 0371, + `idempotency_keys.request_fingerprint`) and replays only the same body; a + different body under the same key is saved normally. Readings saved before + this fix stay incomplete: delete the incomplete reading and enter it again + with both values and the original time. Operators can list affected + readings with the query in `docs/ops/blood-pressure-missing-diastolic.md`. + ## [1.39.9] — 2026-10-03 Workouts can be logged by hand, the body fat tile opens its own page, and diff --git a/docs/CONTRIBUTING-CLIENTS.md b/docs/CONTRIBUTING-CLIENTS.md index 2f5660022..e2f627e8c 100644 --- a/docs/CONTRIBUTING-CLIENTS.md +++ b/docs/CONTRIBUTING-CLIENTS.md @@ -19,7 +19,7 @@ Every JSON route answers the same shape: `{ data, error }`, with `meta` on the e - Refresh tokens rotate per device and are one-time-use. Reusing an old one revokes that device's whole token family, on purpose. Persist the newest pair atomically and never retry a refresh with a token that already succeeded once. - Bearer token scopes are fail-closed. A token minted for one purpose does not open unrelated routes, and a route that does not declare a scope refuses narrow tokens outright. Expect `403` to be a normal, recoverable answer, not an anomaly. - Admin surfaces are cookie-session only, by construction. A native client cannot be an admin client, whatever scopes its token carries. Do not build admin features. -- Mutations accept an `Idempotency-Key` header. Replays answer with the original result plus `X-Idempotent-Replay`, so retrying on a dropped connection is safe. Use it for anything a user would notice twice. +- Mutations accept an `Idempotency-Key` header. A repeat of the same key with the same body answers with the original result plus `X-Idempotent-Replay`, so retrying on a dropped connection is safe. A different body under the same key is treated as a new request, not a replay; still, give each distinct write its own key. Use it for anything a user would notice twice. ## Data semantics worth knowing before you sync diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 25431a0bd..50a2926cb 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -162,12 +162,14 @@ paths: pattern: ^[A-Za-z0-9_\-:.]{8,128}$ minLength: 8 maxLength: 128 - description: "Makes this write safe to retry. Send the same key again and the first response is replayed rather than the - work repeated, marked with `X-Idempotent-Replay: true`. A key that does not match the pattern is IGNORED - rather than refused — the write then proceeds unprotected and looks exactly like a request that carried no - key, so validate the key on your side. Scoped per user and per (method, path): the same key on a different - route is a different key. While a first request under this key is still in flight, a second is refused with - 409 rather than queued." + description: "Makes this write safe to retry. Send the same key with the same body again and the first response is + replayed rather than the work repeated, marked with `X-Idempotent-Replay: true`. The body is compared as + JSON with key order ignored; a different body under the same key is not a retry and runs as a new write, + while the first response stays stored for retries of the first request. A key that does not match the + pattern is IGNORED rather than refused — the write then proceeds unprotected and looks exactly like a + request that carried no key, so validate the key on your side. Scoped per user and per (method, path): the + same key on a different route is a different key. While a first request under this key is still in flight, a + second with the same body is refused with 409 rather than queued." requestBody: required: true content: diff --git a/docs/integrations/home-assistant.md b/docs/integrations/home-assistant.md index c521a8c74..71d679ccc 100644 --- a/docs/integrations/home-assistant.md +++ b/docs/integrations/home-assistant.md @@ -171,8 +171,9 @@ tells you which rows to retry. ## Retries and duplicates Send an `Idempotency-Key` header with a value your automation can reproduce for -the same reading. A retry with the same key returns the original response -instead of writing a second row: +the same reading. A retry with the same key and the same body returns the +original response instead of writing a second row. A different body under the +same key is a different reading and is written: ```yaml headers: diff --git a/docs/ops/blood-pressure-missing-diastolic.md b/docs/ops/blood-pressure-missing-diastolic.md new file mode 100644 index 000000000..1509a6ade --- /dev/null +++ b/docs/ops/blood-pressure-missing-diastolic.md @@ -0,0 +1,60 @@ +# Blood pressure readings missing their diastolic half + +## What happened + +The iPhone app (every App Store build up to 1.0.3 and TestFlight 1.1.0) +saves a manually entered blood pressure as two requests to +`POST /api/measurements`: one `BLOOD_PRESSURE_SYS`, one +`BLOOD_PRESSURE_DIA`, both carrying the same `Idempotency-Key`. Until the +replay cache learned to compare request bodies, the server treated the second +request as a retry of the first. It answered with the first request's cached +`201` (header `X-Idempotent-Replay: true`) and never wrote the diastolic +value. The app showed success, so nobody noticed. + +The fix makes the cache replay only a request with the same body. A different +body under the same key now runs normally. Nothing recovers the values that +were never sent to the database: they exist only on the person's phone, if at +all. + +## Finding the affected readings + +Read-only. Run it against the HealthLog database (with the bundled compose +file: `docker compose exec db psql -U healthlog healthlog`). It lists every live systolic reading that has no +live diastolic reading for the same account at the same instant. + + + +```sql +SELECT u.username, + m.id, + m.measured_at, + m.value AS systolic, + m.source +FROM measurements m +JOIN users u ON u.id = m.user_id +WHERE m.type = 'BLOOD_PRESSURE_SYS' + AND m.deleted_at IS NULL + AND NOT EXISTS ( + SELECT 1 + FROM measurements d + WHERE d.user_id = m.user_id + AND d.measured_at = m.measured_at + AND d.type = 'BLOOD_PRESSURE_DIA' + AND d.deleted_at IS NULL + ) +ORDER BY u.username, m.measured_at; +``` + + + +Readings from the phone app carry `source = 'MANUAL'`. Rows from a synced +source (Withings, Apple Health) that show up here are a different gap, not +this one: those sources write both halves themselves. + +## What to tell users + +There is nothing the server can repair: the diastolic value never reached +it. A person whose reading appears in the list can re-enter it: delete the +incomplete reading, then add the blood pressure again with both values and +the original date and time. From this release on, both halves are saved on +the first try, from any app version. diff --git a/prisma/migrations/0371_idempotency_request_fingerprint/migration.sql b/prisma/migrations/0371_idempotency_request_fingerprint/migration.sql new file mode 100644 index 000000000..dcb360852 --- /dev/null +++ b/prisma/migrations/0371_idempotency_request_fingerprint/migration.sql @@ -0,0 +1,8 @@ +-- What request a replay-cache cell answered, as a SHA-256 of the canonical +-- request body scoped by method and path. A request that reuses a key with a +-- different body is executed instead of being handed the first body's cached +-- response. The body itself is never stored. Nullable: rows written before +-- this release carry none, and a null never matches, so they stop replaying +-- for anything but their own retries already in flight (they expire within +-- 24 hours either way). +ALTER TABLE "idempotency_keys" ADD COLUMN IF NOT EXISTS "request_fingerprint" TEXT; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 6a05a5659..9250aef3f 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -5545,15 +5545,19 @@ model ProviderHealth { /// (userId, key, method, path) so retries within the TTL replay the /// original outcome instead of producing a duplicate side-effect. model IdempotencyKey { - id String @id @default(cuid()) - userId String @map("user_id") - key String - method String - path String - responseStatus Int @map("response_status") - responseBody String @map("response_body") // serialized JSON envelope - expiresAt DateTime @map("expires_at") - createdAt DateTime @default(now()) @map("created_at") + id String @id @default(cuid()) + userId String @map("user_id") + key String + method String + path String + responseStatus Int @map("response_status") + responseBody String @map("response_body") // serialized JSON envelope + /// SHA-256 of the canonical request body, scoped by method and path. A + /// lookup replays only when it matches; a different body under the same key + /// runs normally. Null on rows written before it existed, which never match. + requestFingerprint String? @map("request_fingerprint") + expiresAt DateTime @map("expires_at") + createdAt DateTime @default(now()) @map("created_at") user User @relation(fields: [userId], references: [id], onDelete: Cascade) diff --git a/src/app/api/anamnesis/facts/__tests__/idempotency-conflict.test.ts b/src/app/api/anamnesis/facts/__tests__/idempotency-conflict.test.ts index adb31bbdc..dc93fd8e2 100644 --- a/src/app/api/anamnesis/facts/__tests__/idempotency-conflict.test.ts +++ b/src/app/api/anamnesis/facts/__tests__/idempotency-conflict.test.ts @@ -58,10 +58,13 @@ vi.mock("@/lib/profile/health-facts", () => ({ import { NextRequest } from "next/server"; import { POST } from "../route"; +import { requestFingerprint } from "@/lib/idempotency"; /** The pending sentinel `findCached` recognises as "another call is running". */ const PENDING_STATUS = 0; +const BODY = JSON.stringify({ kind: "SMOKING_STATUS", value: "FORMER" }); + function request(): NextRequest { return new NextRequest("http://localhost/api/anamnesis/facts", { method: "POST", @@ -69,7 +72,7 @@ function request(): NextRequest { "content-type": "application/json", "idempotency-key": "key-in-flight", }, - body: JSON.stringify({ kind: "SMOKING_STATUS", value: "FORMER" }), + body: BODY, }); } @@ -99,6 +102,12 @@ describe("idempotency 409 envelope, over the real route", () => { it("serves `error` as a string when a request is already in flight", async () => { mocks.findUnique.mockResolvedValue({ id: "idem-1", + // The same request in flight: a different body would run instead. + requestFingerprint: requestFingerprint( + "POST", + "/api/anamnesis/facts", + new TextEncoder().encode(BODY), + ), responseStatus: PENDING_STATUS, responseBody: "", expiresAt: new Date(Date.now() + 60_000), diff --git a/src/lib/__tests__/idempotency.test.ts b/src/lib/__tests__/idempotency.test.ts index 61494372e..a93a23ee1 100644 --- a/src/lib/__tests__/idempotency.test.ts +++ b/src/lib/__tests__/idempotency.test.ts @@ -49,6 +49,8 @@ import { withIdempotency, defaultUserIdResolver, isCachableStatus, + canonicalJson, + requestFingerprint, } from "../idempotency"; import { prisma } from "@/lib/db"; import { getSession } from "@/lib/auth/session"; @@ -81,6 +83,13 @@ function makeRequest( }); } +/** The fingerprint `makeRequest("POST", …)` produces. */ +const OK_BODY_FINGERPRINT = requestFingerprint( + "POST", + "/api/example", + new TextEncoder().encode(JSON.stringify({ ok: true })), +); + beforeEach(() => { vi.resetAllMocks(); vi.mocked(prisma.idempotencyKey.findUnique).mockResolvedValue(null); @@ -144,6 +153,11 @@ describe("withIdempotency", () => { data: { responseBody: string; responseStatus: number }; } ).data; + const claimedFingerprint = ( + vi.mocked(prisma.idempotencyKey.create).mock.calls[0][0] as { + data: { requestFingerprint: string }; + } + ).data.requestFingerprint; // Second call: cache hit returns persisted envelope. vi.mocked(prisma.idempotencyKey.findUnique).mockResolvedValueOnce({ @@ -152,6 +166,7 @@ describe("withIdempotency", () => { key: "abc-12345678", method: "POST", path: "/api/example", + requestFingerprint: claimedFingerprint, responseStatus: persistedBody.responseStatus, responseBody: persistedBody.responseBody, expiresAt: new Date(Date.now() + 86_400_000), @@ -296,6 +311,7 @@ describe("withIdempotency concurrency claim (A3)", () => { key: "abc-12345678", method: "POST", path: "/api/example", + requestFingerprint: OK_BODY_FINGERPRINT, responseStatus: 0, responseBody: "", expiresAt: new Date(Date.now() + 60_000), @@ -928,3 +944,195 @@ describe("the record-session fence sits above the replay cache", () => { expect(handler).toHaveBeenCalledTimes(1); }); }); + +describe("request fingerprint", () => { + const enc = (text: string) => new TextEncoder().encode(text); + + it("sorts object keys recursively and keeps array order", () => { + expect(canonicalJson({ b: 1, a: { d: [3, 1], c: null } })).toBe( + '{"a":{"c":null,"d":[3,1]},"b":1}', + ); + expect(canonicalJson([{ y: 1, x: 2 }, "s"])).toBe('[{"x":2,"y":1},"s"]'); + }); + + it("treats the same JSON object in a different key order as one request", () => { + const a = requestFingerprint("POST", "/p", enc('{"type":"X","value":1}')); + const b = requestFingerprint( + "POST", + "/p", + enc('{ "value": 1, "type": "X" }'), + ); + expect(a).toBe(b); + expect(a).toMatch(/^[0-9a-f]{64}$/); + }); + + it("tells nested values and array order apart", () => { + const base = requestFingerprint("POST", "/p", enc('{"a":{"b":[1,2]}}')); + expect(requestFingerprint("POST", "/p", enc('{"a":{"b":[2,1]}}'))).not.toBe( + base, + ); + expect(requestFingerprint("POST", "/p", enc('{"a":{"b":[1,3]}}'))).not.toBe( + base, + ); + }); + + it("tells the two halves of a blood pressure apart", () => { + const sys = requestFingerprint( + "POST", + "/api/measurements", + enc('{"type":"BLOOD_PRESSURE_SYS","value":120}'), + ); + const dia = requestFingerprint( + "POST", + "/api/measurements", + enc('{"type":"BLOOD_PRESSURE_DIA","value":80}'), + ); + expect(sys).not.toBe(dia); + }); + + it("hashes a non-JSON or empty body as its raw bytes", () => { + const text = requestFingerprint("POST", "/p", enc("not json")); + expect(text).toBe(requestFingerprint("POST", "/p", enc("not json"))); + expect(text).not.toBe(requestFingerprint("POST", "/p", enc("not json!"))); + const empty = requestFingerprint("DELETE", "/p", new Uint8Array()); + expect(empty).toBe(requestFingerprint("DELETE", "/p", new Uint8Array())); + expect(empty).not.toBe(text); + // Invalid UTF-8 is raw bytes, not an error. + expect(() => + requestFingerprint("POST", "/p", new Uint8Array([0xff, 0xfe])), + ).not.toThrow(); + }); + + it("is scoped by method and path", () => { + const body = enc('{"a":1}'); + const base = requestFingerprint("POST", "/p", body); + expect(requestFingerprint("PUT", "/p", body)).not.toBe(base); + expect(requestFingerprint("POST", "/q", body)).not.toBe(base); + }); +}); + +describe("withIdempotency with a different body under one key", () => { + function postWith(body: unknown): NextRequest { + return new NextRequest("http://localhost/api/example", { + method: "POST", + headers: { + "content-type": "application/json", + "idempotency-key": "abc-12345678", + }, + body: JSON.stringify(body), + }); + } + + function liveRow(fingerprint: string | null, responseStatus: number) { + return { + id: "idem-1", + userId: "u-1", + key: "abc-12345678", + method: "POST", + path: "/api/example", + requestFingerprint: fingerprint, + responseStatus, + responseBody: "", + expiresAt: new Date(Date.now() + 60_000), + createdAt: new Date(), + } as never; + } + + it.each([ + ["a completed row for another body", "f".repeat(64), 201], + ["an in-flight claim for another body", "f".repeat(64), 0], + ["a legacy row without a fingerprint", null, 201], + ])( + "runs the handler without claiming or caching over %s", + async (_label, fingerprint, status) => { + vi.mocked(prisma.idempotencyKey.findUnique).mockResolvedValueOnce( + liveRow(fingerprint, status), + ); + const handler = vi.fn(async () => + NextResponse.json({ data: "dia", error: null }, { status: 201 }), + ); + const wrapped = withIdempotency<[NextRequest]>( + handler, + async () => "u-1", + ); + const res = await wrapped(postWith({ type: "BLOOD_PRESSURE_DIA" })); + + expect(res.status).toBe(201); + expect(res.headers.get("X-Idempotent-Replay")).toBeNull(); + expect(handler).toHaveBeenCalledTimes(1); + expect(prisma.idempotencyKey.create).not.toHaveBeenCalled(); + expect(prisma.idempotencyKey.updateMany).not.toHaveBeenCalled(); + expect(prisma.idempotencyKey.deleteMany).not.toHaveBeenCalled(); + expect(prisma.idempotencyKey.delete).not.toHaveBeenCalled(); + }, + ); + + it("runs the handler when a racing claim for another body wins the insert", async () => { + vi.mocked(prisma.idempotencyKey.create).mockRejectedValueOnce( + Object.assign(new Error("unique"), { code: "P2002" }), + ); + // First lookup: nothing yet. Second, after the collision: the other body. + vi.mocked(prisma.idempotencyKey.findUnique) + .mockResolvedValueOnce(null) + .mockResolvedValueOnce(liveRow("f".repeat(64), 0)); + const handler = vi.fn(async () => + NextResponse.json({ data: "dia", error: null }, { status: 201 }), + ); + const wrapped = withIdempotency<[NextRequest]>(handler, async () => "u-1"); + const res = await wrapped(postWith({ type: "BLOOD_PRESSURE_DIA" })); + + expect(res.status).toBe(201); + expect(handler).toHaveBeenCalledTimes(1); + expect(prisma.idempotencyKey.updateMany).not.toHaveBeenCalled(); + }); + + it("stores the fingerprint, never the body, on the claim", async () => { + const handler = vi.fn(async () => + NextResponse.json({ data: "ok", error: null }, { status: 201 }), + ); + const wrapped = withIdempotency<[NextRequest]>(handler, async () => "u-1"); + await wrapped(postWith({ type: "BLOOD_PRESSURE_SYS", value: 123 })); + + const claim = ( + vi.mocked(prisma.idempotencyKey.create).mock.calls[0][0] as { + data: Record; + } + ).data; + expect(claim.requestFingerprint).toBe( + requestFingerprint( + "POST", + "/api/example", + new TextEncoder().encode( + JSON.stringify({ type: "BLOOD_PRESSURE_SYS", value: 123 }), + ), + ), + ); + expect(JSON.stringify(claim)).not.toContain("BLOOD_PRESSURE_SYS"); + }); + + it("leaves the body readable for the handler", async () => { + const handler = vi.fn(async (req: NextRequest) => + NextResponse.json({ data: await req.json(), error: null }), + ); + const wrapped = withIdempotency<[NextRequest]>(handler, async () => "u-1"); + const res = await wrapped(postWith({ type: "WEIGHT", value: 70 })); + expect((await res.json()).data).toEqual({ type: "WEIGHT", value: 70 }); + }); + + it("reads at most a bounded prefix of a large body and still hands the whole body on", async () => { + const big = "x".repeat(3 * 1024 * 1024); + const handler = vi.fn(async (req: NextRequest) => + NextResponse.json({ data: (await req.text()).length, error: null }), + ); + const wrapped = withIdempotency<[NextRequest]>(handler, async () => "u-1"); + const res = await wrapped( + new NextRequest("http://localhost/api/example", { + method: "POST", + headers: { "idempotency-key": "abc-12345678" }, + body: big, + }), + ); + expect((await res.json()).data).toBe(big.length); + expect(prisma.idempotencyKey.create).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/lib/idempotency.ts b/src/lib/idempotency.ts index 70e2ddcbc..82d2cc0b6 100644 --- a/src/lib/idempotency.ts +++ b/src/lib/idempotency.ts @@ -11,6 +11,19 @@ * 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. + * + * A cell also remembers WHICH request it answered: a SHA-256 of the canonical + * request body (`requestFingerprint`). A key names one request, not a slot, and + * a client that reuses a key for a second, different request must not be + * handed the first request's response. Native clients up to 1.1.0 sent the two + * halves of a manual blood pressure as two POSTs under one key; the second half + * replayed the first's 201 and was never written. A lookup therefore replays, + * or reports in-flight, only for the same body. A different body — or a row + * written before fingerprints existed — runs the handler normally without + * claiming or caching anything, and the existing cell stays for its own + * retries. This is deliberately not the IETF draft's 422: refusing would turn + * every affected client's second write into a visible error with no way for + * the person to recover the value. */ import type { NextRequest } from "next/server"; import { headers } from "next/headers"; @@ -109,6 +122,122 @@ export interface IdempotencyContext { key: string; method: string; path: string; + /** See `requestFingerprint`. Never the body itself. */ + fingerprint: string; +} + +/** + * How much of a body the fingerprint reads before it stops. The wrapper runs + * before the handler's own bounded readers and upload slots, so it must not + * buffer an arbitrarily large body on their behalf: reading the clone holds + * what it has read in the original's queue until the handler consumes it. + * Every JSON write the cache fronts is far below this; the one multipart route + * (document upload) is above it, and there the prefix plus the declared length + * identifies a retry of the same upload, with the route's own content hash as + * the duplicate check behind it. + */ +const FINGERPRINT_MAX_BYTES = 1024 * 1024; + +/** + * JSON with every object's keys sorted, recursively. Arrays keep their order — + * it is part of the request. Exported for its unit tests. + */ +export function canonicalJson(value: unknown): string { + if (Array.isArray(value)) { + return `[${value.map((v) => canonicalJson(v)).join(",")}]`; + } + if (value !== null && typeof value === "object") { + const entries = Object.keys(value) + .sort() + .map( + (k) => + `${JSON.stringify(k)}:${canonicalJson((value as Record)[k])}`, + ); + return `{${entries.join(",")}}`; + } + return JSON.stringify(value); +} + +/** + * The fingerprint of a request: SHA-256 hex over method, path and the body in + * canonical form. A body that parses as JSON is hashed as `canonicalJson`, so + * the same object serialised with a different key order is the same request; + * anything else (multipart, plain text, an empty body) is hashed as its raw + * bytes. `truncatedAt` marks a body cut at `FINGERPRINT_MAX_BYTES`, hashed as + * that raw prefix together with the declared length. Exported for unit tests. + */ +export function requestFingerprint( + method: string, + path: string, + body: Uint8Array, + truncatedAt: string | null = null, +): string { + const hash = createHash("sha256") + .update(method) + .update("\0") + .update(path) + .update("\0"); + if (truncatedAt !== null) { + return hash.update(`partial:${truncatedAt}\0`).update(body).digest("hex"); + } + if (body.length > 0) { + try { + const text = new TextDecoder("utf-8", { fatal: true }).decode(body); + return hash + .update("json:") + .update(canonicalJson(JSON.parse(text))) + .digest("hex"); + } catch { + // Not UTF-8 or not JSON — fall through to the raw bytes. + } + } + return hash.update("raw:").update(body).digest("hex"); +} + +/** + * Read the request's body from a clone, at most `FINGERPRINT_MAX_BYTES`, and + * fingerprint it. The original request keeps its body for the handler. + */ +async function fingerprintRequest( + request: Request | NextRequest, + path: string, +): Promise { + const stream = request.clone().body; + if (!stream) + return requestFingerprint(request.method, path, new Uint8Array()); + const reader = stream.getReader(); + const chunks: Uint8Array[] = []; + let total = 0; + let truncated = false; + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + if (total + value.byteLength > FINGERPRINT_MAX_BYTES) { + chunks.push(value.subarray(0, FINGERPRINT_MAX_BYTES - total)); + total = FINGERPRINT_MAX_BYTES; + truncated = true; + // Cancelling this branch of the tee stops it from queueing the rest of + // the body, and leaves the handler's branch intact. Not awaited: a tee + // branch's cancel settles only once BOTH branches are done, which is + // after the handler has run. + void reader.cancel().catch(() => {}); + break; + } + chunks.push(value); + total += value.byteLength; + } + const body = new Uint8Array(total); + let offset = 0; + for (const chunk of chunks) { + body.set(chunk, offset); + offset += chunk.byteLength; + } + return requestFingerprint( + request.method, + path, + body, + truncated ? (request.headers.get("content-length") ?? "unknown") : null, + ); } function getIdempotencyKey(request: Request | NextRequest): string | null { @@ -125,10 +254,16 @@ function getIdempotencyKey(request: Request | NextRequest): string | null { * - `{ kind: "pending" }` — another request holds an in-flight claim * for this key; the caller must NOT run the * handler again. + * - `{ kind: "mismatch" }` — the live row answered a different body (or + * predates fingerprints); the caller runs the + * handler without claiming or caching. * - `null` — no live row; the caller may claim + run. */ type CacheLookup = - { kind: "replay"; response: NextResponse } | { kind: "pending" } | null; + | { kind: "replay"; response: NextResponse } + | { kind: "pending" } + | { kind: "mismatch" } + | null; /** * Look up the state of a (userId, key, method, path) tuple. Distinguishes @@ -157,6 +292,13 @@ async function findCached(ctx: IdempotencyContext): Promise { return null; } + // Checked before both the pending and the replay arm: a different request + // under the same key is neither in flight nor answered. A legacy row's null + // never equals a fingerprint. + if (row.requestFingerprint !== ctx.fingerprint) { + return { kind: "mismatch" }; + } + if (row.responseStatus === PENDING_STATUS) { // A concurrent request claimed this key and is still running its // side-effect. Signal the caller to refuse rather than double-execute. @@ -208,6 +350,7 @@ async function claimKey(ctx: IdempotencyContext): Promise { key: ctx.key, method: ctx.method, path: ctx.path, + requestFingerprint: ctx.fingerprint, responseStatus: PENDING_STATUS, responseBody: "", expiresAt: new Date(Date.now() + PENDING_TTL_MS), @@ -233,6 +376,7 @@ async function releaseClaim(ctx: IdempotencyContext): Promise { key: ctx.key, method: ctx.method, path: ctx.path, + requestFingerprint: ctx.fingerprint, responseStatus: PENDING_STATUS, }, }) @@ -312,6 +456,7 @@ async function persistCached( key: ctx.key, method: ctx.method, path: ctx.path, + requestFingerprint: ctx.fingerprint, }, data: { responseStatus: response.status, @@ -502,6 +647,23 @@ async function narrowTokenFacet(): Promise { return apiToken.permissions.includes("*") ? null : `t:${apiToken.id}`; } +/** + * Run a request whose key is already taken by a different body: the handler + * runs exactly as it would without a key, and nothing is claimed or cached, so + * the existing cell keeps answering its own retries. + */ +function runUncached( + ctx: IdempotencyContext, + handler: (...args: Args) => Promise, + args: Args, +): Promise { + annotate({ + action: { name: "idempotency.key.body_mismatch" }, + meta: { method: ctx.method, path: ctx.path }, + }); + return handler(...args); +} + export function withIdempotency< Args extends [Request | NextRequest, ...unknown[]], >( @@ -589,14 +751,27 @@ export function withIdempotency< if (tokenFacet !== null) facets.push(tokenFacet); const url = new URL(request.url); + const fingerprint = await fingerprintRequest(request, url.pathname).catch( + () => null, + ); + if (fingerprint === null) { + // The body could not be read for the fingerprint (already consumed, or + // the stream failed). Without knowing which request this is, no cell + // may answer for it; the handler meets the body problem itself. + return handler(...args); + } const ctx: IdempotencyContext = { userId, key: cellKey(key, claimedRecord, facets), method: request.method, path: url.pathname, + fingerprint, }; const cached = await findCached(ctx); + if (cached?.kind === "mismatch") { + return runUncached(ctx, handler, args); + } if (cached?.kind === "replay") { return cached.response; } @@ -617,6 +792,13 @@ export function withIdempotency< // one caller may execute the side-effect for a given key. const won = await claimKey(ctx); if (!won) { + // The racing claim may be a different request under the same key — + // the two halves of one reading posted together. That one is not a + // duplicate of this one and must still run. + const raced = await findCached(ctx); + if (raced?.kind === "mismatch") { + return runUncached(ctx, handler, args); + } annotate({ action: { name: "idempotency.inflight_conflict" }, meta: { method: ctx.method, path: ctx.path }, diff --git a/src/lib/openapi/routes/shared.ts b/src/lib/openapi/routes/shared.ts index c96613bc0..831933630 100644 --- a/src/lib/openapi/routes/shared.ts +++ b/src/lib/openapi/routes/shared.ts @@ -391,7 +391,8 @@ export const profileEmailRateLimitResponse = { * * Scope, so nobody reads more into it than is there: only POST, PUT, PATCH and * DELETE are covered (`SUPPORTED_METHODS`), the key is scoped per user and per - * `(method, path)`, and `isCachableStatus` refuses to cache 5xx or the + * `(method, path)`, a replay needs the same body (a different body under the + * same key runs as a new write and is neither replayed nor refused), and `isCachableStatus` refuses to cache 5xx or the * transient 401 / 403 / 408 / 429 — a retry after one of those runs for real. * * `src/__tests__/idempotency-contract-publication.test.ts` holds this to the @@ -410,7 +411,7 @@ export const idempotencyKeyParameter = { maxLength: 128, }, description: - "Makes this write safe to retry. Send the same key again and the first response is replayed rather than the work repeated, marked with `X-Idempotent-Replay: true`. A key that does not match the pattern is IGNORED rather than refused — the write then proceeds unprotected and looks exactly like a request that carried no key, so validate the key on your side. Scoped per user and per (method, path): the same key on a different route is a different key. While a first request under this key is still in flight, a second is refused with 409 rather than queued.", + "Makes this write safe to retry. Send the same key with the same body again and the first response is replayed rather than the work repeated, marked with `X-Idempotent-Replay: true`. The body is compared as JSON with key order ignored; a different body under the same key is not a retry and runs as a new write, while the first response stays stored for retries of the first request. A key that does not match the pattern is IGNORED rather than refused — the write then proceeds unprotected and looks exactly like a request that carried no key, so validate the key on your side. Scoped per user and per (method, path): the same key on a different route is a different key. While a first request under this key is still in flight, a second with the same body is refused with 409 rather than queued.", }; /** diff --git a/tests/integration/idempotency-body-fingerprint.test.ts b/tests/integration/idempotency-body-fingerprint.test.ts new file mode 100644 index 000000000..c0d04d93c --- /dev/null +++ b/tests/integration/idempotency-body-fingerprint.test.ts @@ -0,0 +1,249 @@ +/** + * One `Idempotency-Key`, two different requests. + * + * Native clients up to 1.1.0 post a manual blood pressure as two + * `POST /api/measurements` calls — the systolic half, then the diastolic half — + * under the same key. While the replay cell was keyed on + * `(user, key, method, path)` alone, the second call was answered with the + * first call's cached 201 and the diastolic value was never written. The cell + * now carries a fingerprint of the request body: only the same body replays. + * + * Everything here is real — the shipped route, the shipped wrapper, the + * session resolver and the `idempotency_keys` unique index. + */ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +import { NextRequest } from "next/server"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { POST as postMeasurement } from "@/app/api/measurements/route"; + +import { cookieJar, headerJar } from "./mock-next-headers"; +import { getPrismaClient, truncateAllTables } from "./setup"; + +process.env.ENCRYPTION_KEY ??= + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + +vi.mock("next/headers", async () => { + const { cookieJar, headerJar } = await import("./mock-next-headers"); + return { + headers: vi.fn(async () => ({ + get: (name: string) => headerJar.get(name.toLowerCase()) ?? null, + })), + cookies: vi.fn(async () => ({ + get: (name: string) => { + const value = cookieJar.get(name); + return value ? { name, value } : undefined; + }, + set: (name: string, value: string) => { + cookieJar.set(name, value); + }, + delete: (name: string) => { + cookieJar.delete(name); + }, + })), + }; +}); + +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +const USER_A = "idem-fingerprint-a"; +const USER_B = "idem-fingerprint-b"; +const KEY = "bp-entry-5c1e2a7f"; +const MEASURED_AT = "2026-09-30T07:15:00.000Z"; + +const SYS = { type: "BLOOD_PRESSURE_SYS", value: 128, measuredAt: MEASURED_AT }; +const DIA = { type: "BLOOD_PRESSURE_DIA", value: 84, measuredAt: MEASURED_AT }; + +const sessions = new Map(); + +function post(body: unknown, key = KEY): Promise { + return postMeasurement( + new NextRequest("http://localhost/api/measurements", { + method: "POST", + headers: { + "content-type": "application/json", + "idempotency-key": key, + }, + body: JSON.stringify(body), + }), + ); +} + +function signInAs(userId: string) { + cookieJar.set("healthlog_session", sessions.get(userId)!); +} + +async function rowsOf(userId: string) { + const rows = await getPrismaClient().measurement.findMany({ + where: { userId, deletedAt: null }, + select: { type: true, value: true }, + }); + // By name, not by the enum's declaration order. + return rows.sort((a, b) => a.type.localeCompare(b.type)); +} + +beforeEach(async () => { + const prisma = getPrismaClient(); + await truncateAllTables(prisma); + cookieJar.clear(); + headerJar.clear(); + sessions.clear(); + for (const id of [USER_A, USER_B]) { + await prisma.user.create({ + data: { + id, + username: id, + email: `${id}@example.test`, + timezone: "UTC", + }, + }); + const session = await prisma.session.create({ + data: { userId: id, expiresAt: new Date(Date.now() + 60 * 60 * 1000) }, + }); + sessions.set(id, session.id); + } + signInAs(USER_A); +}); + +describe("Idempotency-Key reused for a different body (real Postgres)", () => { + it("writes both halves of a blood pressure posted under one key", async () => { + const first = await post(SYS); + expect(first.status).toBe(201); + + const second = await post(DIA); + expect(second.status).toBe(201); + expect(second.headers.get("X-Idempotent-Replay")).toBeNull(); + + expect(await rowsOf(USER_A)).toEqual([ + { type: "BLOOD_PRESSURE_DIA", value: 84 }, + { type: "BLOOD_PRESSURE_SYS", value: 128 }, + ]); + }); + + it("still replays a retry of the same body and writes one row", async () => { + const first = await post(SYS); + expect(first.status).toBe(201); + const firstBody = await first.json(); + + // Same object, different key order: the same request. + const retry = await post({ + measuredAt: MEASURED_AT, + value: 128, + type: "BLOOD_PRESSURE_SYS", + }); + expect(retry.status).toBe(201); + expect(retry.headers.get("X-Idempotent-Replay")).toBe("true"); + expect(await retry.json()).toEqual(firstBody); + + expect(await rowsOf(USER_A)).toEqual([ + { type: "BLOOD_PRESSURE_SYS", value: 128 }, + ]); + }); + + it("keeps the first cell answering its own retries after a different body ran", async () => { + await post(SYS); + await post(DIA); + const retry = await post(SYS); + expect(retry.headers.get("X-Idempotent-Replay")).toBe("true"); + expect(await rowsOf(USER_A)).toHaveLength(2); + const cells = await getPrismaClient().idempotencyKey.findMany({ + where: { userId: USER_A }, + }); + expect(cells).toHaveLength(1); + expect(cells[0]?.requestFingerprint).toMatch(/^[0-9a-f]{64}$/); + }); + + it("writes both halves when they are in flight at the same time", async () => { + const [a, b] = await Promise.all([post(SYS), post(DIA)]); + expect([a.status, b.status]).toEqual([201, 201]); + expect(await rowsOf(USER_A)).toEqual([ + { type: "BLOOD_PRESSURE_DIA", value: 84 }, + { type: "BLOOD_PRESSURE_SYS", value: 128 }, + ]); + }); + + it("still lets only one of two identical bodies in flight run", async () => { + const [a, b] = await Promise.all([post(SYS), post(SYS)]); + expect([a.status, b.status].sort()).toEqual([201, 409]); + expect(await rowsOf(USER_A)).toHaveLength(1); + }); + + it("does not replay a row written before fingerprints existed", async () => { + await getPrismaClient().idempotencyKey.create({ + data: { + userId: USER_A, + key: KEY, + method: "POST", + path: "/api/measurements", + responseStatus: 201, + responseBody: JSON.stringify({ data: { id: "legacy" }, error: null }), + expiresAt: new Date(Date.now() + 60 * 60 * 1000), + }, + }); + + const res = await post(DIA); + expect(res.status).toBe(201); + expect(res.headers.get("X-Idempotent-Replay")).toBeNull(); + expect(await rowsOf(USER_A)).toEqual([ + { type: "BLOOD_PRESSURE_DIA", value: 84 }, + ]); + // The legacy row is left alone, not claimed over. + const cell = await getPrismaClient().idempotencyKey.findFirstOrThrow({ + where: { userId: USER_A, key: KEY }, + }); + expect(cell.requestFingerprint).toBeNull(); + }); + + it("never hands one user's cached response to another using the same key", async () => { + const first = await post(SYS); + const firstBody = await first.json(); + + signInAs(USER_B); + const other = await post(SYS); + expect(other.status).toBe(201); + expect(other.headers.get("X-Idempotent-Replay")).toBeNull(); + expect(await other.json()).not.toEqual(firstBody); + + expect(await rowsOf(USER_A)).toHaveLength(1); + expect(await rowsOf(USER_B)).toHaveLength(1); + }); + + it("the documented operator query finds a systolic reading left without its diastolic half", async () => { + const doc = readFileSync( + join(process.cwd(), "docs/ops/blood-pressure-missing-diastolic.md"), + "utf8", + ); + const block = + /([\s\S]*?)/.exec( + doc, + ); + const sql = block?.[1] + .replace(/```sql|```/g, "") + .trim() + .replace(/;$/, ""); + expect(sql).toBeTruthy(); + + // A complete reading through the fixed path, and the damage the old + // cache left behind: a systolic half on its own. + await post(SYS); + await post(DIA); + const lone = await getPrismaClient().measurement.create({ + data: { + userId: USER_A, + type: "BLOOD_PRESSURE_SYS", + value: 141, + unit: "mmHg", + measuredAt: new Date("2026-09-29T19:40:00.000Z"), + }, + }); + + const found = await getPrismaClient().$queryRawUnsafe< + Array<{ id: string; systolic: number }> + >(sql!); + expect(found.map((r) => [r.id, r.systolic])).toEqual([[lone.id, 141]]); + }); +});