olemd/vinterliste
1
0
Fork 0
Code Issues 1 Pull requests Projects Releases Packages Wiki Activity Actions

fix(auth): close first-user-auto-admin race

Browse source 
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).
This commit is contained in:
Ole-Morten Duesund 2026-05-25 13:52:57 +02:00
commit b16d06e651
1 changed files with 9 additions and 8 deletions
Download patch file Download diff file Expand all files Collapse all files

17
server/auth.ts
View file

@ -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,
);