refactor(withings): atomic callback nonce consumption + surfaced delete failures

The callback previously read the ledger row then deleted it in two
round-trips, contradicting its own docstring's atomicity promise.
Concurrent callbacks with the same nonce could both pass findUnique
before either issued delete. Replace with delete-first + catch P2025
for the replay branch — atomic at the Postgres level. Silent
.catch(() => {}) on delete failures now surfaces via Wide-Event
warning. Closes M3 + M7 in v1.4.48 backlog.

src/app/api/withings/callback/__tests__/route.test.ts

@@ -0,0 +1,257 @@
+/**
+ * v1.4.48 M3 — unit-level coverage for the atomic nonce-consumption
+ * branch.
+ *
+ * The integration tests in `tests/integration/withings-oauth*.test.ts`
+ * already pin the full handshake against real Postgres (happy path,
+ * replay, expired, cross-user). This file specifically targets the
+ * delete-first race window the M3 refactor closed: two concurrent
+ * callbacks with the same nonce — the first wins, the second hits
+ * Prisma P2025 and lands on the replay branch.
+ *
+ * Co-tests M7 — non-P2025 delete failures (DB-connection drop /
+ * integrity violation) surface via Wide-Event `addWarning` instead
+ * of being silently swallowed.
+ */
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { NextRequest } from "next/server";
+
+vi.mock("@/lib/api-handler", () => ({
+  apiHandler: <T extends (...args: unknown[]) => unknown>(fn: T) => fn,
+  requireAuth: vi.fn(async () => ({
+    user: { id: "user-1" },
+    session: { id: "sess-1" },
+  })),
+  HttpError: class HttpError extends Error {
+    constructor(
+      public status: number,
+      message: string,
+    ) {
+      super(message);
+    }
+  },
+}));
+
+vi.mock("@/lib/db", () => ({
+  prisma: {
+    withingsOAuthState: {
+      delete: vi.fn(),
+    },
+    withingsConnection: {
+      upsert: vi.fn(),
+    },
+  },
+}));
+
+vi.mock("@/lib/auth/audit", () => ({ auditLog: vi.fn() }));
+
+vi.mock("@/lib/crypto", () => ({
+  encrypt: vi.fn((s: string) => `enc(${s})`),
+}));
+
+vi.mock("@/lib/withings/client", () => ({
+  exchangeCode: vi.fn(),
+  WITHINGS_OAUTH_SCOPE: "user.metrics,user.activity",
+}));
+
+vi.mock("@/lib/withings/credentials", () => ({
+  getUserWithingsCredentials: vi.fn(),
+}));
+
+vi.mock("@/lib/withings/sync", () => ({
+  setupWebhook: vi.fn(() => Promise.resolve()),
+}));
+
+vi.mock("@/lib/integrations/status", () => ({
+  markReconnected: vi.fn(),
+}));
+
+const addWarning = vi.fn();
+vi.mock("@/lib/logging/context", () => ({
+  annotate: vi.fn(),
+  getEvent: vi.fn(() => ({ addWarning, setError: vi.fn() })),
+}));
+
+import { GET } from "../route";
+import { prisma } from "@/lib/db";
+import { Prisma } from "@/generated/prisma/client";
+
+const NONCE = "abcdefghijklmnopqrstuv"; // 22-char base64url
+const APP_URL = "http://localhost:3000";
+
+function callbackRequest(state: string, cookieState: string): NextRequest {
+  return new NextRequest(
+    `http://localhost/api/withings/callback?code=auth-code-1&state=${encodeURIComponent(state)}`,
+    {
+      method: "GET",
+      headers: {
+        cookie: `withings_state=${cookieState}`,
+      },
+    },
+  );
+}
+
+beforeEach(() => {
+  vi.clearAllMocks();
+  process.env.NEXT_PUBLIC_APP_URL = APP_URL;
+});
+
+describe("withings/callback atomic nonce consumption (M3)", () => {
+  it("concurrent callbacks with the same nonce — only one succeeds, the second hits the P2025 replay branch", async () => {
+    // First callback: delete returns the row (win the race).
+    // Second callback: delete throws P2025 (row already consumed).
+    const winningRow = {
+      nonce: NONCE,
+      userId: "user-1",
+      expiresAt: new Date(Date.now() + 10 * 60 * 1000),
+      createdAt: new Date(),
+    };
+    const p2025 = new Prisma.PrismaClientKnownRequestError(
+      "An operation failed because it depends on one or more records that were required but not found.",
+      { code: "P2025", clientVersion: "x" },
+    );
+
+    const deleteMock = vi.mocked(prisma.withingsOAuthState.delete);
+    deleteMock
+      .mockResolvedValueOnce(winningRow as never)
+      .mockRejectedValueOnce(p2025);
+
+    // Stub the rest of the happy path for the winning request — code
+    // exchange returns plausible tokens so the upsert + redirect run.
+    const { getUserWithingsCredentials } = await import(
+      "@/lib/withings/credentials"
+    );
+    vi.mocked(getUserWithingsCredentials).mockResolvedValue({
+      clientId: "cid",
+      clientSecret: "csec",
+    } as never);
+    const { exchangeCode } = await import("@/lib/withings/client");
+    vi.mocked(exchangeCode).mockResolvedValue({
+      userid: "wuid",
+      access_token: "atok",
+      refresh_token: "rtok",
+      expires_in: 3600,
+    } as never);
+
+    // Fire both legs concurrently. The mock's call ordering is FIFO,
+    // so whichever `await` hits the Prisma mock first gets the
+    // success result. Both legs use the same nonce + cookie value.
+    const reqA = callbackRequest(NONCE, NONCE);
+    const reqB = callbackRequest(NONCE, NONCE);
+    const [resA, resB] = await Promise.all([GET(reqA), GET(reqB)]);
+
+    // Both responses are 307 redirects.
+    expect(resA.status).toBe(307);
+    expect(resB.status).toBe(307);
+
+    // Exactly one delete-attempt per leg — the atomic shape collapses
+    // findUnique + delete into a single round-trip.
+    expect(deleteMock).toHaveBeenCalledTimes(2);
+
+    // Exactly one leg landed on `withings=connected`, exactly one
+    // landed on the replay branch (`withings=error&reason=state`).
+    const locations = [resA, resB]
+      .map((r) => r.headers.get("location"))
+      .sort();
+    expect(locations).toHaveLength(2);
+    expect(
+      locations.some((l) => l?.includes("withings=connected")),
+    ).toBe(true);
+    expect(
+      locations.some((l) => l?.includes("withings=error&reason=state")),
+    ).toBe(true);
+
+    // P2025 must NOT surface as a warning — it's the expected
+    // replay-branch signal, not a real infra problem.
+    expect(addWarning).not.toHaveBeenCalled();
+  });
+
+  it("non-P2025 delete failure (e.g. connection drop) surfaces a Wide-Event warning and bounces to the error page", async () => {
+    // M7 — a real infra failure on the consume-delete must not be
+    // silently swallowed.
+    const infraError = new Error("connection terminated unexpectedly");
+    vi.mocked(prisma.withingsOAuthState.delete).mockRejectedValueOnce(
+      infraError,
+    );
+
+    const req = callbackRequest(NONCE, NONCE);
+    const res = await GET(req);
+
+    expect(res.status).toBe(307);
+    expect(res.headers.get("location")).toContain(
+      "withings=error&reason=state",
+    );
+
+    // The warning carries the error string so the audit trail can
+    // distinguish "row was gone" (P2025, no warning) from "DB unhappy"
+    // (warning, infra signal).
+    expect(addWarning).toHaveBeenCalledTimes(1);
+    expect(addWarning).toHaveBeenCalledWith(
+      expect.stringContaining("oauth-state-delete failed"),
+    );
+    expect(addWarning).toHaveBeenCalledWith(
+      expect.stringContaining("connection terminated unexpectedly"),
+    );
+  });
+
+  it("delete succeeds but row has expired — validity check trips, no token exchange", async () => {
+    const expiredRow = {
+      nonce: NONCE,
+      userId: "user-1",
+      expiresAt: new Date(Date.now() - 60_000),
+      createdAt: new Date(),
+    };
+    vi.mocked(prisma.withingsOAuthState.delete).mockResolvedValueOnce(
+      expiredRow as never,
+    );
+
+    const { exchangeCode } = await import("@/lib/withings/client");
+
+    const req = callbackRequest(NONCE, NONCE);
+    const res = await GET(req);
+
+    expect(res.status).toBe(307);
+    expect(res.headers.get("location")).toContain(
+      "withings=error&reason=state",
+    );
+    // Single atomic round-trip consumed the row; no token exchange.
+    expect(exchangeCode).not.toHaveBeenCalled();
+  });
+
+  it("delete succeeds but row's userId mismatches session — validity check trips, no token exchange", async () => {
+    const crossUserRow = {
+      nonce: NONCE,
+      userId: "user-OTHER",
+      expiresAt: new Date(Date.now() + 10 * 60 * 1000),
+      createdAt: new Date(),
+    };
+    vi.mocked(prisma.withingsOAuthState.delete).mockResolvedValueOnce(
+      crossUserRow as never,
+    );
+
+    const { exchangeCode } = await import("@/lib/withings/client");
+
+    const req = callbackRequest(NONCE, NONCE);
+    const res = await GET(req);
+
+    expect(res.status).toBe(307);
+    expect(res.headers.get("location")).toContain(
+      "withings=error&reason=state",
+    );
+    expect(exchangeCode).not.toHaveBeenCalled();
+  });
+
+  it("URL state vs cookie state mismatch short-circuits BEFORE the atomic delete fires", async () => {
+    // CSRF leg 1 — must reject without touching the ledger so a
+    // probe can't grief legitimate rows by issuing deletes for
+    // nonces it doesn't legitimately hold the cookie for.
+    const req = callbackRequest("aaaaaaaaaaaaaaaaaaaaaa", "bbbbbbbbbbbbbbbbbbbbbb");
+    const res = await GET(req);
+
+    expect(res.status).toBe(307);
+    expect(res.headers.get("location")).toContain(
+      "withings=error&reason=state",
+    );
+    expect(prisma.withingsOAuthState.delete).not.toHaveBeenCalled();
+  });
+});

src/app/api/withings/callback/route.ts

@@ -8,6 +8,7 @@ import { getUserWithingsCredentials } from "@/lib/withings/credentials";
 import { WITHINGS_OAUTH_STATE_COOKIE } from "@/lib/withings/oauth-state";
 import { setupWebhook } from "@/lib/withings/sync";
 import { markReconnected } from "@/lib/integrations/status";
+import { Prisma } from "@/generated/prisma/client";
 import { NextRequest, NextResponse } from "next/server";
 import { timingSafeEqual } from "node:crypto";
 
@@ -23,6 +24,17 @@ import { timingSafeEqual } from "node:crypto";
  * runs. The row is consumed (deleted) on every exit branch so a
  * replay of the same nonce fails the second time.
  *
+ * v1.4.48 M3 — the row is now consumed via a single delete-first
+ * round-trip. Prisma `delete` returns the row on success, so the
+ * post-delete validity checks (`expiresAt`, `userId`) still run
+ * against the consumed payload. Two concurrent callbacks with the
+ * same nonce now race at the Postgres row level — only one delete
+ * wins, the other hits P2025 ("record not found") and is treated as
+ * a replay, bounced to the error page. Previously the `findUnique +
+ * separate delete` shape left a tiny RTT window where both legs
+ * could pass the lookup before either issued the delete, contrary
+ * to the docstring's atomicity promise.
+ *
  * Closes the v1.4.43 security audit L-1 finding: the cookie now
  * carries only the opaque nonce, so a future refactor flipping it to
  * non-httpOnly cannot leak the user id into request logs / network
@@ -39,7 +51,10 @@ export const GET = apiHandler(async (request: NextRequest) => {
 
   // CSRF check, leg 1: URL state must match the cookie state byte-for-byte.
   // Timing-safe comparison so a probe can't recover the cookie bytes via
-  // response-time analysis.
+  // response-time analysis. This runs BEFORE the atomic delete so a
+  // probe with arbitrary state values can't grief legitimate ledger
+  // rows by triggering deletes for nonces it doesn't legitimately
+  // hold the cookie for.
   if (
     !state ||
     !storedState ||
@@ -54,28 +69,51 @@ export const GET = apiHandler(async (request: NextRequest) => {
     );
   }
 
-  // CSRF check, leg 2: the nonce must resolve to a live, unexpired
-  // ledger row whose `userId` matches the authenticated session.
-  // Resolving the user from the row (rather than parsing the cookie
-  // value) is the v1.4.43 audit recommendation — a future refactor
-  // flipping the cookie to non-httpOnly cannot leak the user id this
-  // way.
-  const stateRow = await prisma.withingsOAuthState.findUnique({
-    where: { nonce: state },
-  });
-  if (
-    !stateRow ||
-    stateRow.expiresAt <= new Date() ||
-    stateRow.userId !== user.id
-  ) {
-    // Single-use: stamp out the row whether the expiry tripped, the
-    // user mismatched, or we found nothing. The user will retry from
-    // `withings/connect` with a fresh nonce.
-    if (stateRow) {
-      await prisma.withingsOAuthState
-        .delete({ where: { nonce: state } })
-        .catch(() => {});
+  // CSRF check, leg 2: atomically consume the ledger row. Prisma
+  // `delete` returns the row on success so `expiresAt` + `userId`
+  // checks still run against the consumed payload. P2025 ("record
+  // not found") means the nonce was already consumed by a prior
+  // callback (a replay) or never existed — both land on the same
+  // error redirect. Atomic at the Postgres row level: two concurrent
+  // callbacks with the same nonce can no longer both pass the
+  // lookup before either issues the delete.
+  let stateRow: { userId: string; expiresAt: Date } | null = null;
+  try {
+    stateRow = await prisma.withingsOAuthState.delete({
+      where: { nonce: state },
+    });
+  } catch (err) {
+    if (
+      err instanceof Prisma.PrismaClientKnownRequestError &&
+      err.code === "P2025"
+    ) {
+      // Row already consumed (replay) or never existed.
+      return NextResponse.redirect(
+        new URL(
+          "/settings/integrations?withings=error&reason=state",
+          process.env.NEXT_PUBLIC_APP_URL!,
+        ),
+      );
     }
+    // A real infra problem (connection drop, integrity violation)
+    // shouldn't be silently swallowed. Surface it to the Wide Event
+    // so the audit trail captures it, then bounce to the error
+    // page. Re-throwing would land on the apiHandler's 500 path and
+    // strand the user without a clean redirect.
+    getEvent()?.addWarning(`oauth-state-delete failed: ${err}`);
+    return NextResponse.redirect(
+      new URL(
+        "/settings/integrations?withings=error&reason=state",
+        process.env.NEXT_PUBLIC_APP_URL!,
+      ),
+    );
+  }
+
+  // Validity checks against the consumed row. The row is already
+  // gone from the ledger by this point — single-use semantics —
+  // whether the expiry tripped, the userId mismatched, or
+  // everything looked good.
+  if (stateRow.expiresAt <= new Date() || stateRow.userId !== user.id) {
     return NextResponse.redirect(
       new URL(
         "/settings/integrations?withings=error&reason=state",
@@ -85,11 +123,6 @@ export const GET = apiHandler(async (request: NextRequest) => {
   }
 
   if (!code) {
-    // Consume the row even on the no-code path so the nonce can't
-    // be replayed by an attacker who scraped the redirect URL.
-    await prisma.withingsOAuthState
-      .delete({ where: { nonce: state } })
-      .catch(() => {});
     return NextResponse.redirect(
       new URL(
         "/settings/integrations?withings=error&reason=nocode",
@@ -101,9 +134,6 @@ export const GET = apiHandler(async (request: NextRequest) => {
   try {
     const creds = await getUserWithingsCredentials(user.id);
     if (!creds) {
-      await prisma.withingsOAuthState
-        .delete({ where: { nonce: state } })
-        .catch(() => {});
       return NextResponse.redirect(
         new URL(
           "/settings/integrations?withings=error&reason=nocreds",
@@ -156,13 +186,6 @@ export const GET = apiHandler(async (request: NextRequest) => {
     // don't write a fresh success-time — that's the next sync's job.
     await markReconnected(user.id, "withings");
 
-    // Consume the ledger row — single-use semantics. From this point
-    // on a replay of the same `state` lands on the not-found branch
-    // above and redirects to the error page.
-    await prisma.withingsOAuthState
-      .delete({ where: { nonce: state } })
-      .catch(() => {});
-
     const response = NextResponse.redirect(
       new URL(
         "/settings/integrations?withings=connected",
@@ -173,12 +196,6 @@ export const GET = apiHandler(async (request: NextRequest) => {
     return response;
   } catch (err) {
     getEvent()?.setError(err);
-    // Consume the row on the failure path too — the user is going to
-    // retry from `withings/connect` which mints a fresh nonce, and a
-    // stranded row buys us nothing.
-    await prisma.withingsOAuthState
-      .delete({ where: { nonce: state } })
-      .catch(() => {});
     return NextResponse.redirect(
       new URL(
         "/settings/integrations?withings=error&reason=token",