/api/auth/setup reaches the KDF unguarded before first-run completes #162

New issue

Open

opened 2026-09-15 21:17:36 +02:00 by lz · 0 comments

lz commented

2026-09-15 21:17:36 +02:00

Owner

Surfaced by PR #160 (rate-limiting /api/auth/unlock) and deliberately scoped out of it. Belongs to the #110 auth-hardening family rather than to the MCP milestone — it blocks nothing there.

The gap

/api/auth/setup is unauthenticated by necessity and does reach the KDF, deriving a key with no lock around isSetupComplete. PR #160 added an attempt limiter to /api/auth/unlock but not here, so on an instance that has not completed first-run, /api/auth/setup is an unmetered Argon2id trigger — the same CPU-exhaustion shape #149 was filed for, on a different route.

It also corrected a claim that had been written on that branch: "the only unauthenticated route that reaches the KDF" is false for a pre-setup instance. Both statements now say "once setup is complete", which is accurate and which is what leaves this gap visible.

Why it is lower priority than #149 was

An instance that has not been set up holds no secret. Anyone who can reach it can simply claim it outright by completing setup themselves — so CPU burn is not the marginal risk in that window, and a rate limiter would not be the control that matters.

The more interesting question is arguably the race: isSetupComplete is checked without a lock, and /api/auth/setup is unauthenticated. PR #146 already hardened the vault side of this by splitting writePrincipal into insertPrincipal / upsertPrincipal, so a concurrent setup now fails loudly rather than silently replacing the winner's wrapper. Worth confirming that the route-level behaviour is equally sane, and that the loser of the race gets a comprehensible error rather than a 500.

Work

Decide whether the exposure warrants an attempt limiter at all, given the above. "No, and here is why" is a legitimate outcome — record it rather than leaving the asymmetry unexplained.
If yes, reuse UnlockLimiter rather than growing a second mechanism; it is a single global counter with no attacker-controlled key.
Either way, cover the unauthenticated concurrent-setup race with a test, and check the loser's error is meaningful.
Whatever is decided, state it where the next person will look — the vault documentation is the natural home, since that is where the setup/unlock asymmetry is already explained.

Surfaced by PR #160 (rate-limiting `/api/auth/unlock`) and deliberately scoped out of it. Belongs to the #110 auth-hardening family rather than to the MCP milestone — it blocks nothing there. ## The gap `/api/auth/setup` is unauthenticated by necessity and **does reach the KDF**, deriving a key with no lock around `isSetupComplete`. PR #160 added an attempt limiter to `/api/auth/unlock` but not here, so on an instance that has not completed first-run, `/api/auth/setup` is an unmetered Argon2id trigger — the same CPU-exhaustion shape #149 was filed for, on a different route. It also corrected a claim that had been written on that branch: "the only unauthenticated route that reaches the KDF" is **false for a pre-setup instance**. Both statements now say "once setup is complete", which is accurate and which is what leaves this gap visible. ## Why it is lower priority than #149 was An instance that has not been set up **holds no secret**. Anyone who can reach it can simply claim it outright by completing setup themselves — so CPU burn is not the marginal risk in that window, and a rate limiter would not be the control that matters. The more interesting question is arguably the **race**: `isSetupComplete` is checked without a lock, and `/api/auth/setup` is unauthenticated. PR #146 already hardened the vault side of this by splitting `writePrincipal` into `insertPrincipal` / `upsertPrincipal`, so a concurrent setup now fails loudly rather than silently replacing the winner's wrapper. Worth confirming that the route-level behaviour is equally sane, and that the loser of the race gets a comprehensible error rather than a 500. ## Work - Decide whether the exposure warrants an attempt limiter at all, given the above. "No, and here is why" is a legitimate outcome — record it rather than leaving the asymmetry unexplained. - If yes, reuse `UnlockLimiter` rather than growing a second mechanism; it is a single global counter with no attacker-controlled key. - Either way, cover the unauthenticated concurrent-setup race with a test, and check the loser's error is meaningful. - Whatever is decided, state it where the next person will look — the vault documentation is the natural home, since that is where the setup/unlock asymmetry is already explained.