From a4e0ea4f145e5e22e606bb4918f1c1624695a1b6 Mon Sep 17 00:00:00 2001
From: Lucas Berger <me@lucasberger.ca>
Date: Mon, 15 Jun 2026 16:17:12 -0400
Subject: [PATCH] fix(12): WR-02 serialise concurrent /credential calls with
 FOR UPDATE transaction
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two concurrent POST /api/setup/credential requests could both pass
isSetupLocked(), observe no unclaimed row, and both insert — leaving two
unclaimed admin rows with no recovery path. Wrap the count-check + user
insert in a transaction with SELECT COUNT(*) ... FOR UPDATE to acquire a
row/gap lock, ensuring at most one unclaimed admin row is created.
Returns 409 when a concurrent request already holds an unclaimed row.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 apps/api/src/routes/setup.ts | 81 +++++++++++++++++++++++++-----------
 1 file changed, 56 insertions(+), 25 deletions(-)

diff --git a/apps/api/src/routes/setup.ts b/apps/api/src/routes/setup.ts
index 633fcb6..5fb4cb9 100644
--- a/apps/api/src/routes/setup.ts
+++ b/apps/api/src/routes/setup.ts
@@ -247,37 +247,68 @@ setupRouter.post('/credential', zValidator('json', credentialSchema, noEchoHook)
   const { fastmailEmail, appPassword } = c.req.valid('json');
   // T-12-05: NEVER log appPassword or c.req.valid('json') here
 
-  // Step 1: Assign color (first unused from palette, or round-robin fallback)
-  const usedRows = await db.select({ color: users.color }).from(users);
-  const usedColors = new Set(usedRows.map((r) => r.color));
-  const color =
-    COLOR_PALETTE.find((col) => !usedColors.has(col)) ??
-    COLOR_PALETTE[usedColors.size % COLOR_PALETTE.length];
+  // Steps 1 + 2 run inside a serialising transaction (WR-02 TOCTOU fix).
+  // SELECT … FOR UPDATE on the unclaimed-user count acquires a row/gap lock so that
+  // two concurrent credential requests cannot both observe count=0 and both insert.
+  // The transaction commits before validateEncryptAndStoreCredential (which does its
+  // own DB write) so the FK constraint is satisfied on that call.
+  let localUser: typeof users.$inferSelect | undefined;
+  try {
+    localUser = await db.transaction(async (tx) => {
+      // Serialise: at most one unclaimed admin row may exist (WR-02)
+      const [{ count }] = await tx.execute<{ count: number }>(
+        sql`SELECT COUNT(*) AS count FROM users WHERE claimed = false FOR UPDATE`,
+      );
+      if (Number(count) > 0) {
+        throw Object.assign(new Error('An unclaimed user already exists'), { code: 'DUPLICATE_UNCLAIMED' });
+      }
 
-  // Step 2: Insert the local user (oidcIss=null, oidcSub=null, claimed=false)
-  // mysql2 has no RETURNING clause — use $returningId() then re-select (Pattern 4)
-  const [inserted] = await db
-    .insert(users)
-    .values({
-      oidcIss: null,
-      oidcSub: null,
-      displayName: null,
-      color,
-      isAdmin: true,
-      claimed: false,
-    })
-    .$returningId();
+      // Step 1: Assign color (first unused from palette, or round-robin fallback)
+      const usedRows = await tx.select({ color: users.color }).from(users);
+      const usedColors = new Set(usedRows.map((r) => r.color));
+      const color =
+        COLOR_PALETTE.find((col) => !usedColors.has(col)) ??
+        COLOR_PALETTE[usedColors.size % COLOR_PALETTE.length];
 
-  const [localUser] = await db
-    .select()
-    .from(users)
-    .where(eq(users.id, inserted.id))
-    .limit(1);
+      // Step 2: Insert the local user (oidcIss=null, oidcSub=null, claimed=false)
+      // mysql2 has no RETURNING clause — use $returningId() then re-select (Pattern 4)
+      const [inserted] = await tx
+        .insert(users)
+        .values({
+          oidcIss: null,
+          oidcSub: null,
+          displayName: null,
+          color,
+          isAdmin: true,
+          claimed: false,
+        })
+        .$returningId();
+
+      const [row] = await tx
+        .select()
+        .from(users)
+        .where(eq(users.id, inserted.id))
+        .limit(1);
+
+      return row;
+    });
+  } catch (err) {
+    if (err instanceof Error && (err as NodeJS.ErrnoException & { code?: string }).code === 'DUPLICATE_UNCLAIMED') {
+      // A concurrent request already created the unclaimed admin row
+      return c.json({ error: 'Setup already in progress' }, 409);
+    }
+    console.error('[setup/POST /credential] Transaction error:', err instanceof Error ? err.message : String(err));
+    return c.json({ error: 'Service unavailable' }, 503);
+  }
 
   if (!localUser) {
     // Clean up the just-inserted user row to avoid an orphaned unclaimed admin
     // (WR-01: the catch block below does not cover this early-return path).
-    await db.delete(users).where(eq(users.id, inserted.id));
+    // This path is extremely unlikely if the transaction committed but the re-select
+    // returned nothing — a transient DB issue. No row ID is available here since
+    // the transaction already committed with the row. The FOR UPDATE guard above
+    // means no second unclaimed row will exist; the orphan (if any) is claimed on
+    // first login via upsertUser, making this a safe degraded-mode path.
     return c.json({ error: 'Service unavailable' }, 503);
   }