refactor(withings): differentiate callback failure reasons + tighten delete warning

The atomic-delete landing of v1.4.48 collapsed five historical delete
sites into one, but the post-delete redirect target ?reason=state
now covers three semantically different conditions (CSRF leg 1 mismatch,
P2025 replay, post-delete validity fail). Operators reading the
audit trail cannot tell them apart. This release splits into four
distinct reason tags (csrf1, replay, expired, cross_user) and emits
a wide-event annotation carrying the same.

The delete-catch warning template now interpolates err.name rather
than ${err}, defensively guarding against Prisma error messages
echoing offending values into the wide-event log.

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

@@ -12,6 +12,10 @@
  * Co-tests M7 — non-P2025 delete failures (DB-connection drop /
  * integrity violation) surface via Wide-Event `addWarning` instead
  * of being silently swallowed.
+ *
+ * v1.4.49 — assertions updated for the four-way reason-tag split
+ * (`csrf1`, `replay`, `expired`, `cross_user`) and the QA-2
+ * err.name-only warning template.
  */
 import { describe, it, expect, vi, beforeEach } from "vitest";
 import { NextRequest } from "next/server";
@@ -67,8 +71,9 @@ vi.mock("@/lib/integrations/status", () => ({
 }));
 
 const addWarning = vi.fn();
+const annotateMock = vi.hoisted(() => vi.fn());
 vi.mock("@/lib/logging/context", () => ({
-  annotate: vi.fn(),
+  annotate: annotateMock,
   getEvent: vi.fn(() => ({ addWarning, setError: vi.fn() })),
 }));
 
@@ -149,7 +154,7 @@ describe("withings/callback atomic nonce consumption (M3)", () => {
     expect(deleteMock).toHaveBeenCalledTimes(2);
 
     // Exactly one leg landed on `withings=connected`, exactly one
-    // landed on the replay branch (`withings=error&reason=state`).
+    // landed on the replay branch (`withings=error&reason=replay`).
     const locations = [resA, resB]
       .map((r) => r.headers.get("location"))
       .sort();
@@ -158,20 +163,34 @@ describe("withings/callback atomic nonce consumption (M3)", () => {
       locations.some((l) => l?.includes("withings=connected")),
     ).toBe(true);
     expect(
-      locations.some((l) => l?.includes("withings=error&reason=state")),
+      locations.some((l) => l?.includes("withings=error&reason=replay")),
     ).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();
+
+    // Wide-Event annotation carries the same reason so operators can
+    // grep without DB-shell.
+    expect(annotateMock).toHaveBeenCalledWith({ meta: { reason: "replay" } });
   });
 
-  it("non-P2025 delete failure (e.g. connection drop) surfaces a Wide-Event warning and bounces to the error page", async () => {
+  it("non-P2025 delete failure (e.g. connection drop) surfaces a Wide-Event warning carrying err.name only, 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");
+    //
+    // v1.4.49 QA-2 — the warning template now interpolates `err.name`
+    // rather than `${err}`. A Prisma error whose `message` echoes the
+    // offending value (e.g. the raw nonce) into the wide-event log
+    // would otherwise leak it.
+    class TimeoutError extends Error {
+      constructor() {
+        super("connection terminated unexpectedly nonce=abc123");
+        this.name = "TimeoutError";
+      }
+    }
     vi.mocked(prisma.withingsOAuthState.delete).mockRejectedValueOnce(
-      infraError,
+      new TimeoutError(),
     );
 
     const req = callbackRequest(NONCE, NONCE);
@@ -182,19 +201,25 @@ describe("withings/callback atomic nonce consumption (M3)", () => {
       "withings=error&reason=state",
     );
 
-    // The warning carries the error string so the audit trail can
+    // The warning carries the error NAME so the audit trail can
     // distinguish "row was gone" (P2025, no warning) from "DB unhappy"
-    // (warning, infra signal).
+    // (warning, infra signal) without ever interpolating the message
+    // body.
     expect(addWarning).toHaveBeenCalledTimes(1);
     expect(addWarning).toHaveBeenCalledWith(
-      expect.stringContaining("oauth-state-delete failed"),
+      "oauth-state-delete failed: TimeoutError",
     );
-    expect(addWarning).toHaveBeenCalledWith(
-      expect.stringContaining("connection terminated unexpectedly"),
+    // Defensive: the message body (which Prisma may use to echo the
+    // offending value) must not appear in the warning.
+    expect(addWarning).not.toHaveBeenCalledWith(
+      expect.stringContaining("nonce=abc123"),
+    );
+    expect(addWarning).not.toHaveBeenCalledWith(
+      expect.stringContaining("connection terminated"),
     );
   });
 
-  it("delete succeeds but row has expired — validity check trips, no token exchange", async () => {
+  it("delete succeeds but row has expired — validity check trips with reason=expired, no token exchange", async () => {
     const expiredRow = {
       nonce: NONCE,
       userId: "user-1",
@@ -212,13 +237,14 @@ describe("withings/callback atomic nonce consumption (M3)", () => {
 
     expect(res.status).toBe(307);
     expect(res.headers.get("location")).toContain(
-      "withings=error&reason=state",
+      "withings=error&reason=expired",
     );
     // Single atomic round-trip consumed the row; no token exchange.
     expect(exchangeCode).not.toHaveBeenCalled();
+    expect(annotateMock).toHaveBeenCalledWith({ meta: { reason: "expired" } });
   });
 
-  it("delete succeeds but row's userId mismatches session — validity check trips, no token exchange", async () => {
+  it("delete succeeds but row's userId mismatches session — validity check trips with reason=cross_user, no token exchange", async () => {
     const crossUserRow = {
       nonce: NONCE,
       userId: "user-OTHER",
@@ -236,12 +262,36 @@ describe("withings/callback atomic nonce consumption (M3)", () => {
 
     expect(res.status).toBe(307);
     expect(res.headers.get("location")).toContain(
-      "withings=error&reason=state",
+      "withings=error&reason=cross_user",
     );
     expect(exchangeCode).not.toHaveBeenCalled();
+    expect(annotateMock).toHaveBeenCalledWith({ meta: { reason: "cross_user" } });
   });
 
-  it("URL state vs cookie state mismatch short-circuits BEFORE the atomic delete fires", async () => {
+  it("non-P2025 infra error branch annotates reason=state on the Wide Event (legacy fallback for unclassified DB failures)", async () => {
+    // v1.4.49 — the four named reasons (csrf1, replay, expired,
+    // cross_user) cover every "we know what went wrong" branch. A
+    // raw infra failure is intentionally still tagged `state` —
+    // operators reading the wide event will see the err.name in the
+    // warning and the `reason=state` annotation, which is enough to
+    // distinguish it from the four classified branches.
+    class TimeoutError extends Error {
+      constructor() {
+        super("connection terminated");
+        this.name = "TimeoutError";
+      }
+    }
+    vi.mocked(prisma.withingsOAuthState.delete).mockRejectedValueOnce(
+      new TimeoutError(),
+    );
+
+    const req = callbackRequest(NONCE, NONCE);
+    await GET(req);
+
+    expect(annotateMock).toHaveBeenCalledWith({ meta: { reason: "state" } });
+  });
+
+  it("URL state vs cookie state mismatch short-circuits with reason=csrf1 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.
@@ -250,8 +300,9 @@ describe("withings/callback atomic nonce consumption (M3)", () => {
 
     expect(res.status).toBe(307);
     expect(res.headers.get("location")).toContain(
-      "withings=error&reason=state",
+      "withings=error&reason=csrf1",
     );
     expect(prisma.withingsOAuthState.delete).not.toHaveBeenCalled();
+    expect(annotateMock).toHaveBeenCalledWith({ meta: { reason: "csrf1" } });
   });
 });

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

@@ -55,15 +55,24 @@ export const GET = apiHandler(async (request: NextRequest) => {
   // probe with arbitrary state values can't grief legitimate ledger
   // rows by triggering deletes for nonces it doesn't legitimately
   // hold the cookie for.
+  //
+  // v1.4.49 — reason tags now distinguish the four post-delete branches
+  // so operators reading the audit trail can tell `csrf1` (URL/cookie
+  // mismatch, short-circuit before delete), `replay` (P2025 — nonce
+  // already consumed), `expired` (valid row but TTL elapsed), and
+  // `cross_user` (valid row but session userId mismatch) apart. A
+  // matching `meta.reason` Wide-Event annotation carries the same so
+  // operators can grep without DB-shell.
   if (
     !state ||
     !storedState ||
     state.length !== storedState.length ||
     !timingSafeEqual(Buffer.from(state), Buffer.from(storedState))
   ) {
+    annotate({ meta: { reason: "csrf1" } });
     return NextResponse.redirect(
       new URL(
-        "/settings/integrations?withings=error&reason=state",
+        "/settings/integrations?withings=error&reason=csrf1",
         process.env.NEXT_PUBLIC_APP_URL!,
       ),
     );
@@ -88,9 +97,10 @@ export const GET = apiHandler(async (request: NextRequest) => {
       err.code === "P2025"
     ) {
       // Row already consumed (replay) or never existed.
+      annotate({ meta: { reason: "replay" } });
       return NextResponse.redirect(
         new URL(
-          "/settings/integrations?withings=error&reason=state",
+          "/settings/integrations?withings=error&reason=replay",
           process.env.NEXT_PUBLIC_APP_URL!,
         ),
       );
@@ -100,7 +110,13 @@ export const GET = apiHandler(async (request: NextRequest) => {
     // 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}`);
+    //
+    // v1.4.49 QA-2 — the warning interpolates `err.name` rather than
+    // `${err}` so a Prisma error message echoing the offending value
+    // (e.g. the raw nonce) into the wide-event log cannot leak it.
+    const errName = err instanceof Error ? err.name : "unknown";
+    getEvent()?.addWarning(`oauth-state-delete failed: ${errName}`);
+    annotate({ meta: { reason: "state" } });
     return NextResponse.redirect(
       new URL(
         "/settings/integrations?withings=error&reason=state",
@@ -113,10 +129,20 @@ export const GET = apiHandler(async (request: NextRequest) => {
   // 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) {
+  if (stateRow.expiresAt <= new Date()) {
+    annotate({ meta: { reason: "expired" } });
     return NextResponse.redirect(
       new URL(
-        "/settings/integrations?withings=error&reason=state",
+        "/settings/integrations?withings=error&reason=expired",
+        process.env.NEXT_PUBLIC_APP_URL!,
+      ),
+    );
+  }
+  if (stateRow.userId !== user.id) {
+    annotate({ meta: { reason: "cross_user" } });
+    return NextResponse.redirect(
+      new URL(
+        "/settings/integrations?withings=error&reason=cross_user",
         process.env.NEXT_PUBLIC_APP_URL!,
       ),
     );