fix(auth): serialize setup routes via in-process mutex#401
Conversation
Two setup requests arriving concurrently can both pass the
`needsSetup()` check and both create the first admin user:
- `auth:setup` calls `createAdmin()` first
- `auth:setup-no-auth` previously set `authMode` first, then
called `createOrGetNoAuthAdmin()` — so a race that reached the
no-auth path could persist `authMode: 'none'` while the user
was actually created via the `required` path (or vice versa).
Wrap both routes in a shared `setupMutex.run(SETUP_LOCK_KEY, ...)`
so the second request blocks until the first has fully released
the lock, and reorder `auth:setup-no-auth` to create the user
first and write the `authMode` setting only after that succeeds.
The mutex is per-key and in-process — Clopen runs as a single
Bun process, so the race is in-process by construction. A future
multi-process deployment would still need a DB-level unique
constraint on the users table to prevent duplicate admins.
Tests:
- backend/auth/setup-mutex.test.ts (new): serialization, key
isolation, throw-release, sync return, 25-call fan-out.
- bun test --isolate (full): 163/163 pass, no regressions.
|
Hi @DeryFerd, thanks for digging into this — the mutex implementation is clean, and the The premise is that two concurrent Tracing the actual path, Bun/JS runs synchronous code to completion. No other request can interleave into that check-then-create block, no matter how many requests arrive at once. I reproduced the exact yield-point shape in an isolated script and raced two calls with
One thing worth keeping regardless: reordering What would change my mind: a real Reopen here anytime if either of those shows up — I'd rather walk through it together than close the door. |
Summary
The two initial-setup routes,
auth:setupandauth:setup-no-auth, both checkauthQueries.countUsers() === 0to decide whether to create the first admin. The check and the create are two separate database operations with nothing between them.If two requests arrive in the same microtask, both can pass the gate and both can create a user. There is no unique constraint on the first admin, so the second insert succeeds. You end up with two admins and (worse) two active session tokens that the same client can use interchangeably.
auth:setup-no-authalso writes theauthModesystem setting before it callscreateOrGetNoAuthAdmin(). A race that reaches the no-auth path can persistauthMode: 'none'while the user was actually created via therequiredpath. No single route wrote that combination on purpose.What this PR changes
Adds an in-process mutex around both setup routes and reorders the no-auth route so the user-create happens before the settings write. New file
backend/auth/setup-mutex.tsexports thesetupMutexsingleton.backend/ws/auth/login.tswraps each route's critical section insetupMutex.run(SETUP_LOCK_KEY, ...).This is an in-process problem. Clopen runs as a single Bun/Elysia process, both routes share the same event loop, and an in-process lock is enough. A future multi-process deployment would need a unique constraint on the
userstable to prevent duplicate admins in that scenario. That is out of scope here.The mutex
setupMutex.run(key, fn)appends to a tail of a promise chain keyed bykey. The caller waits for the previous holder'srelease()before itsfnruns. The return value or rejection offnpropagates unchanged. Rejections from a previous holder are absorbed so a thrownfndoes not poison the chain for the next caller. The map is eagerly cleaned when the last caller for a key finishes, so there is no leak across the lifetime of a single process.Two routes share
SETUP_LOCK_KEY = 'setup', so any setup request blocks until the previous one has fully released. Different keys are independent, so unrelated operations are not affected.Route changes
auth:setup(login.ts:33-67) keeps its existing order. Create user first, then write settings, then audit log, then set auth. The whole section now runs inside the mutex.auth:setup-no-auth(login.ts:70-106) is reordered.createOrGetNoAuthAdmin()now runs beforesettings.set('system:settings', ...). The user-create call is what actually catches the duplicate. If it throwsSetup already completed., the authMode write never happens. The previous order would have writtenauthMode: 'none'even when the user-create failed, which is the bug.auth:auto-login-no-authis not changed. By the time a client calls it,getAuthMode() !== 'none'is the only gate, and the existing check is sufficient. If a future audit shows this path racing with setup, the same mutex applies with no interface change.Failure modes
auth:setupcalls in parallelauth:setupandauth:setup-no-authin parallelSetup already completed.Validation
Targeted:
bun test --isolate backend/auth/setup-mutex.test.ts: 5/5 pass. Covers serialization, key isolation, throw-release, sync return, and a 25-call fan-out.bun test --isolate backend/ws/auth/login.test.ts: 5/5 pass. No regression in the existing audit-logging assertions.bunx eslint backend/auth/setup-mutex.ts backend/auth/setup-mutex.test.ts backend/ws/auth/login.ts: clean.bun run checkis a svelte-check that exceeds the 60-second budget on this codebase independent of this change. Type safety for the new code is enforced by Bun's loader at runtime and by the test suite.Diff stat