From d68859d68b780cd38503fd02e9ae94d06f3b3b6c Mon Sep 17 00:00:00 2001
From: Ole-Morten Duesund <olemd@glemt.net>
Date: Mon, 25 May 2026 14:00:26 +0200
Subject: [PATCH] fix(auth): race-proof username uniqueness in PATCH /profile
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Problem: the profile-update handler pre-checked username uniqueness
with a SELECT followed by an UPDATE outside any transaction. Two
concurrent PATCHes setting the same slug would both pass the SELECT
(no conflict yet), then one of the UPDATEs would hit the underlying
UNIQUE constraint and surface as an unhandled SqliteError → 500.

Fix: drop the racy pre-check entirely. The UNIQUE constraint on
users.username (column-level on fresh DBs, partial unique index on
migrated DBs) is the source of truth. Wrap the UPDATE in try/catch
and convert SQLITE_CONSTRAINT_UNIQUE into a clean 409 username_taken
response. Same "push the invariant down to the database" pattern as
the recent first-user-auto-admin race fix.

Surfaced by the username-uniqueness review.
---
 server/auth.ts | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/server/auth.ts b/server/auth.ts
index 2cc5afa..09e90c4 100644
--- a/server/auth.ts
+++ b/server/auth.ts
@@ -306,14 +306,11 @@ authRoutes.patch('/profile', requireAuth, async (c) => {
         detail: 'lowercase a-z, 0-9, _ or -; 2-31 characters; must start with a letter or digit',
       }, 400);
     }
-    if (next !== null) {
-      // Pre-check uniqueness so we can return a clear 409 instead of the
-      // SQLite UNIQUE-constraint error bubbling up as a 500.
-      const taken = db.prepare(
-        'SELECT 1 FROM users WHERE username = ? AND id <> ?',
-      ).get(next, userId);
-      if (taken) return c.json({ error: 'username_taken' }, 409);
-    }
+    // Uniqueness is enforced by the UNIQUE constraint on users.username
+    // (column-level for fresh DBs, partial unique index for migrated DBs).
+    // We rely on it directly rather than pre-checking — a pre-check would
+    // be racy (two concurrent PATCHes could both pass it). The constraint
+    // catch below converts SQLITE_CONSTRAINT_UNIQUE into a clean 409.
     updates.push('username = ?');
     params.push(next);
   }
@@ -333,7 +330,18 @@ authRoutes.patch('/profile', requireAuth, async (c) => {
   }
 
   params.push(userId);
-  db.prepare(`UPDATE users SET ${updates.join(', ')} WHERE id = ?`).run(...params);
+  try {
+    db.prepare(`UPDATE users SET ${updates.join(', ')} WHERE id = ?`).run(...params);
+  } catch (err) {
+    // SQLite throws an SqliteError with a message containing "UNIQUE
+    // constraint failed: users.username" (column-level constraint) or
+    // ".users_username_idx" (the partial unique index used by migrated DBs).
+    // Either way the user-facing meaning is the same — slug taken.
+    if (err instanceof Error && /UNIQUE constraint failed.*username/i.test(err.message)) {
+      return c.json({ error: 'username_taken' }, 409);
+    }
+    throw err;
+  }
 
   const me = loadMe(userId);
   if (!me) return c.json({ error: 'internal_error' }, 500);