From b16d06e6513a4a7cccad22ecea206378a861459a Mon Sep 17 00:00:00 2001
From: Ole-Morten Duesund <olemd@glemt.net>
Date: Mon, 25 May 2026 13:52:57 +0200
Subject: [PATCH] fix(auth): close first-user-auto-admin race
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Problem: the signup handler counted users with a SELECT issued BEFORE the
transaction opened. Two parallel signups against an empty DB could both
observe count == 0 and both be promoted to admin — a violation of the
"exactly one first user" invariant. Not a confidentiality breach (both
rows passed the same signup checks), but real.

Fix: drop the JS-side count entirely. The is_admin column in the INSERT
is now populated via a subquery:

    (SELECT CASE WHEN COUNT(*) = 0 THEN 1 ELSE 0 END FROM users)

SQLite's WAL serializes writers, so the second concurrent INSERT runs
after the first has committed. Its subquery sees count = 1 and returns
0; the new row is not admin. No transaction-mode tweak required.

Surfaced by /audit security (auth & session lens).
---
 server/auth.ts | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/server/auth.ts b/server/auth.ts
index 82f63aa..2cc5afa 100644
--- a/server/auth.ts
+++ b/server/auth.ts
@@ -135,12 +135,6 @@ authRoutes.post('/signup', async (c) => {
     return c.json({ error: 'signup_closed' }, 403);
   }
 
-  // First-user-auto-admin: count rows *before* the insert. If this is the
-  // first user, they become admin so the deployment is never stranded
-  // without one. (Documented in README.)
-  const userCount = (db.prepare('SELECT COUNT(*) AS n FROM users').get() as { n: number }).n;
-  const isFirstUser = userCount === 0;
-
   // Server-side hash of the client-derived verifier. The verifier is already
   // expensive to brute-force (Argon2id-MODERATE), so Bun.password adds a second
   // hardening layer in case the DB leaks but the verifier salt is still public.
@@ -157,6 +151,12 @@ authRoutes.post('/signup', async (c) => {
   // The user-insert and the invite-claim must commit or fail together: a
   // claimed invite pointing at a nonexistent user (or a user without their
   // claimed invite recorded) would be an audit-trail inconsistency.
+  //
+  // First-user-auto-admin is also resolved here: the `is_admin` column is
+  // populated via a subquery that returns 1 only when the users table is
+  // empty at INSERT time. Two parallel signups against an empty DB can't both
+  // become admin — SQLite's WAL serializes writers, so the second INSERT's
+  // subquery sees the first row already present and returns 0.
   let invitedBy: string | null = null;
   try {
     db.transaction(() => {
@@ -174,7 +174,9 @@ authRoutes.post('/signup', async (c) => {
           (id, email, auth_salt, auth_verifier_hash, kek_salt,
            wrapped_dek_pw, dek_pw_nonce, wrapped_dek_rec, rec_salt, dek_rec_nonce,
            rec_auth_salt, rec_auth_verifier_hash, is_admin, invited_by, created_at)
-        VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
+        VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?,
+                (SELECT CASE WHEN COUNT(*) = 0 THEN 1 ELSE 0 END FROM users),
+                ?, ?)
       `).run(
         id,
         email,
@@ -188,7 +190,6 @@ authRoutes.post('/signup', async (c) => {
         b64ToBuffer(body.dek_rec_nonce),
         b64ToBuffer(body.rec_auth_salt),
         recVerifierHash,
-        isFirstUser ? 1 : 0,
         invitedBy,
         now,
       );