From f22bec5a130d72d42093aaadd41258905eaa054e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ullrich=20Sch=C3=A4fer?= <hi+github@ullrich.is>
Date: Sun, 24 May 2026 12:17:42 +0200
Subject: [PATCH] fix(journal/auth): atomic magic-token consume to close TOCTOU
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

verifyLoginCode, verifyMagicToken, and verifyEmailChange previously did
SELECT WHERE used_at IS NULL → UPDATE … SET used_at = now. Two concurrent
verifications could both pass the SELECT and both succeed, accepting the
same single-use token twice.

Collapsed each to a single UPDATE … WHERE … RETURNING * statement.
Postgres serializes row-level locks within an UPDATE, so exactly one
concurrent caller observes a returned row; the rest see an empty array
and get \"Invalid or expired\". The token is also marked used as part of
the same statement — no second write needed.

verifyEmailChange's tertiary email-availability check now runs *after*
the consume; we keep the original semantics where the token is burned
on a clash (the previous code explicitly did the same with a separate
UPDATE).

No behavior change on the happy path. Closes a credential-reuse
window that mattered most for the 6-digit login codes (small search
space, more likely to race).

Full repo: pnpm typecheck, pnpm lint, pnpm test all green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 apps/journal/app/lib/auth.server.ts | 49 ++++++++++++++---------------
 1 file changed, 23 insertions(+), 26 deletions(-)

diff --git a/apps/journal/app/lib/auth.server.ts b/apps/journal/app/lib/auth.server.ts
index 8e79f9b..04d7500 100644
--- a/apps/journal/app/lib/auth.server.ts
+++ b/apps/journal/app/lib/auth.server.ts
@@ -279,9 +279,13 @@ export async function createMagicToken(email: string): Promise<{ token: string;
 export async function verifyLoginCode(email: string, code: string): Promise<string> {
   const db = getDb();
 
+  // Consume atomically: a single UPDATE…RETURNING ensures only one
+  // concurrent request wins. The old select-then-update sequence was a
+  // TOCTOU window where two clients could both see `used_at IS NULL`
+  // and both succeed.
   const [record] = await db
-    .select()
-    .from(magicTokens)
+    .update(magicTokens)
+    .set({ usedAt: new Date() })
     .where(
       and(
         eq(magicTokens.email, email),
@@ -290,15 +294,11 @@ export async function verifyLoginCode(email: string, code: string): Promise<stri
         gt(magicTokens.expiresAt, new Date()),
         isNull(magicTokens.usedAt),
       ),
-    );
+    )
+    .returning();
 
   if (!record) throw new Error("Invalid or expired code");
 
-  await db
-    .update(magicTokens)
-    .set({ usedAt: new Date() })
-    .where(eq(magicTokens.id, record.id));
-
   const [user] = await db.select().from(users).where(eq(users.email, record.email));
   if (!user) throw new Error("User not found");
 
@@ -332,9 +332,10 @@ export async function initiateEmailChange(userId: string, newEmail: string): Pro
 export async function verifyMagicToken(token: string): Promise<string> {
   const db = getDb();
 
+  // Atomic consume — see verifyLoginCode for rationale.
   const [record] = await db
-    .select()
-    .from(magicTokens)
+    .update(magicTokens)
+    .set({ usedAt: new Date() })
     .where(
       and(
         eq(magicTokens.token, token),
@@ -342,15 +343,11 @@ export async function verifyMagicToken(token: string): Promise<string> {
         gt(magicTokens.expiresAt, new Date()),
         isNull(magicTokens.usedAt),
       ),
-    );
+    )
+    .returning();
 
   if (!record) throw new Error("Invalid or expired magic link");
 
-  await db
-    .update(magicTokens)
-    .set({ usedAt: new Date() })
-    .where(eq(magicTokens.id, record.id));
-
   const [user] = await db.select().from(users).where(eq(users.email, record.email));
   if (!user) throw new Error("User not found");
 
@@ -360,9 +357,14 @@ export async function verifyMagicToken(token: string): Promise<string> {
 export async function verifyEmailChange(token: string, userId: string): Promise<string> {
   const db = getDb();
 
+  // Atomic consume — see verifyLoginCode for rationale. The token is
+  // marked used regardless of whether the email is still available;
+  // re-checking availability after the consume keeps the token
+  // single-use even if the new email was claimed in between (the
+  // original implementation also marked it used in that case).
   const [record] = await db
-    .select()
-    .from(magicTokens)
+    .update(magicTokens)
+    .set({ usedAt: new Date() })
     .where(
       and(
         eq(magicTokens.token, token),
@@ -370,25 +372,20 @@ export async function verifyEmailChange(token: string, userId: string): Promise<
         gt(magicTokens.expiresAt, new Date()),
         isNull(magicTokens.usedAt),
       ),
-    );
+    )
+    .returning();
 
   if (!record) throw new Error("Invalid or expired verification link");
 
   const newEmail = record.email;
 
   // Re-check email availability at verification time — someone may have
-  // registered with this email between initiation and verification
+  // registered with this email between initiation and verification.
   const [existing] = await db.select().from(users).where(eq(users.email, newEmail));
   if (existing) {
-    await db.update(magicTokens).set({ usedAt: new Date() }).where(eq(magicTokens.id, record.id));
     throw new Error("This email is now in use by another account");
   }
 
-  await db
-    .update(magicTokens)
-    .set({ usedAt: new Date() })
-    .where(eq(magicTokens.id, record.id));
-
   await db
     .update(users)
     .set({ email: newEmail })