Scoped API tokens (#109), and a fix for an unauthenticated auth-gate bypass (#120)

lz commented

2026-09-04 23:42:40 +02:00

Owner

Closes #109. Closes #120. Closes #49.

Read this part first

While reviewing #109 I found an unauthenticated bypass of the entire API auth gate, pre-existing on main today:

GET /api/quota      ->  401  {"error":"locked"}
GET /%61pi/quota    ->  200  {"quota":null,"sessions":[]}

%61 is a. No cookie, no token, no credential. Not read-only either — PUT /%61pi/settings/instance reached the handler and validated a body.

Cause: the gate decided admission on event.url.pathname, which SvelteKit leaves percent-encoded exactly as sent, while the router matches the decoded path (respond.js:254 → decode_pathname → find_route). So startsWith('/api/') tested a string the router had already discarded, the gate returned resolve(event), and the real handler ran.

Reachable unauthenticated: ~30 routes — every one that doesn't separately re-check locals.masterKey, including PUT /api/settings/agent-instructions (rewrites the fleet-shared CLAUDE.md that every running agent reads) and POST /api/previews/[previewId]/approve (the operator-consent gate for opening a host port).

Fix: every admission decision now keys on event.route.id — the identifier the router actually resolved — so the string the gate tests and the handler that runs cannot disagree. Verified live: all four encodings and the PUT now return 401. Written up as AGENTS.md fact 23 so it isn't rediscovered.

Two hardening changes came with it: /api/agent/* is an explicit three-name allowlist rather than a prefix (a prefix silently extends unauthenticated passthrough to any route added later), and a remote-function call is denied outright — it resolves no route yet still dispatches, so it's the one shape where a null route.id doesn't mean "nothing runs", which is the assumption the passthrough rests on.

What #109 adds

Mint read-only bearer tokens under Settings → API tokens so a script elsewhere can read Nexus state without holding the master passphrase — which is the vault key.

curl -H "Authorization: Bearer nxs_…" http://your-nexus:3001/api/quota

Two independently grantable scopes: quota:read and workspaces:read.

tokenMay is the authorization model, not the masterKey guards. A token never sets masterKey, which structurally blocks every vault-touching route — but sensitive routes exist that need no vault key (container logs, agent instructions, artifact contents). TOKEN_ROUTES is an allowlist that denies anything unmapped; that's the actual guarantee.
Tokens are GET-only, and it's load-bearing. /api/workers/[id]/sessions POST has no masterKey guard of its own, so the method rule is the only thing between workspaces:read and creating a session.
/api/workers/[id]/health is deliberately excluded. It returns a 30-line container log tail via the same helper as the unmapped /logs route, and fires convergeSessions — so its GET is neither read-only nor free of what the map exists to withhold.
SHA-256, not Argon2id: a 256-bit CSPRNG token is unguessable at any hash speed, so a slow KDF only buys ~100ms on every authenticated request, and a salted hash can't be looked up by digest at all.
Revoke is a hard delete; expiry is optional and filtered at read time.

Verification

1092 tests, lint and typecheck clean.
Driven in a real browser against a built server — the panel has zero test coverage by construction (vitest.config.ts is environment: 'node'; .svelte files aren't compiled), so create/dismiss/re-create, both validation paths, expiry, and revoke-with-confirm were all exercised by hand.
End-to-end API matrix against a live instance: 200 for a held scope, 403 for a missing one, 401 for unmapped routes and for /api/tokens itself, 403 for non-GET, 401 with no credential, and 401 for the /%61pi/ bypass.
Security-critical assertions are mutation-tested — dropping the method check, setting masterKey on the token path, or reverting to url.pathname each turns its own tests red.

Defects found and fixed during review

Worth listing because each is a shape worth recognising — all are "the wrong thing happened and nothing said so", on a credential path:

A typo'd expiry date became NaN, which JSON.stringify writes as null — byte-identical to "no expiry" — silently minting a permanent token. The server can't catch it; by then the NaN is a null.
Microseconds passed where milliseconds were meant is finite, integer, positive and a safe integer, so every guard accepted it: a credential valid until the year 58647. Now bounded at a century.
createToken returned a DTO built from the values it meant to insert, so a coerced write was misreported.
parseScopes silently dropped a stored scope no longer in SCOPES — a rename would strip permissions from live tokens with mystery 403s as the only symptom. It now warns.
Creating a second token overwrote the first one's plaintext while it was still on screen and unrecoverable.

Notes for review

docs/superpowers/specs/2026-09-04-session-expiry-design.md is the design for #108 and is included as context, not as work in this PR. #108 is next.
readJsonBody and .flatten() were converged only in the tokens routes. Five other routes still hand-roll a local copy of the helper — that's pre-existing repo-wide drift and its own change.
The expires_at column has no upper bound in SQL; the ceiling is enforced in createToken, which is also where the finiteness guard lives.
Rate limiting is deliberately out of scope — tracked in #110 along with the rest of the internet-exposure hardening.

Closes #109. Closes #120. Closes #49. ## Read this part first While reviewing #109 I found **an unauthenticated bypass of the entire API auth gate**, pre-existing on `main` today: ``` GET /api/quota -> 401 {"error":"locked"} GET /%61pi/quota -> 200 {"quota":null,"sessions":[]} ``` `%61` is `a`. No cookie, no token, no credential. Not read-only either — `PUT /%61pi/settings/instance` reached the handler and validated a body. **Cause:** the gate decided admission on `event.url.pathname`, which SvelteKit leaves percent-encoded exactly as sent, while the router matches the *decoded* path (`respond.js:254` → `decode_pathname` → `find_route`). So `startsWith('/api/')` tested a string the router had already discarded, the gate returned `resolve(event)`, and the real handler ran. **Reachable unauthenticated:** ~30 routes — every one that doesn't *separately* re-check `locals.masterKey`, including `PUT /api/settings/agent-instructions` (rewrites the fleet-shared `CLAUDE.md` that every running agent reads) and `POST /api/previews/[previewId]/approve` (the operator-consent gate for opening a host port). **Fix:** every admission decision now keys on `event.route.id` — the identifier the router actually resolved — so the string the gate tests and the handler that runs cannot disagree. Verified live: all four encodings and the `PUT` now return 401. Written up as AGENTS.md fact 23 so it isn't rediscovered. Two hardening changes came with it: `/api/agent/*` is an explicit three-name allowlist rather than a prefix (a prefix silently extends unauthenticated passthrough to any route added later), and a remote-function call is denied outright — it resolves no route yet still dispatches, so it's the one shape where a null `route.id` doesn't mean "nothing runs", which is the assumption the passthrough rests on. ## What #109 adds Mint read-only bearer tokens under **Settings → API tokens** so a script elsewhere can read Nexus state without holding the master passphrase — which *is* the vault key. ```bash curl -H "Authorization: Bearer nxs_…" http://your-nexus:3001/api/quota ``` Two independently grantable scopes: `quota:read` and `workspaces:read`. - **`tokenMay` is the authorization model, not the `masterKey` guards.** A token never sets `masterKey`, which structurally blocks every vault-touching route — but sensitive routes exist that need no vault key (container logs, agent instructions, artifact contents). `TOKEN_ROUTES` is an allowlist that **denies anything unmapped**; that's the actual guarantee. - **Tokens are GET-only, and it's load-bearing.** `/api/workers/[id]/sessions` POST has no `masterKey` guard of its own, so the method rule is the only thing between `workspaces:read` and creating a session. - **`/api/workers/[id]/health` is deliberately excluded.** It returns a 30-line container log tail via the same helper as the unmapped `/logs` route, and fires `convergeSessions` — so its GET is neither read-only nor free of what the map exists to withhold. - SHA-256, not Argon2id: a 256-bit CSPRNG token is unguessable at any hash speed, so a slow KDF only buys ~100ms on every authenticated request, and a salted hash can't be looked up by digest at all. - Revoke is a hard delete; expiry is optional and filtered at read time. ## Verification - **1092 tests**, lint and typecheck clean. - **Driven in a real browser** against a built server — the panel has zero test coverage by construction (`vitest.config.ts` is `environment: 'node'`; `.svelte` files aren't compiled), so create/dismiss/re-create, both validation paths, expiry, and revoke-with-confirm were all exercised by hand. - **End-to-end API matrix** against a live instance: 200 for a held scope, 403 for a missing one, 401 for unmapped routes and for `/api/tokens` itself, 403 for non-GET, 401 with no credential, and 401 for the `/%61pi/` bypass. - Security-critical assertions are mutation-tested — dropping the method check, setting `masterKey` on the token path, or reverting to `url.pathname` each turns its own tests red. ## Defects found and fixed during review Worth listing because each is a shape worth recognising — all are "the wrong thing happened and nothing said so", on a credential path: 1. A typo'd expiry date became `NaN`, which `JSON.stringify` writes as `null` — byte-identical to "no expiry" — silently minting a **permanent** token. The server can't catch it; by then the `NaN` is a `null`. 2. Microseconds passed where milliseconds were meant is finite, integer, positive *and* a safe integer, so every guard accepted it: a credential valid until the year **58647**. Now bounded at a century. 3. `createToken` returned a DTO built from the values it *meant* to insert, so a coerced write was misreported. 4. `parseScopes` silently dropped a stored scope no longer in `SCOPES` — a rename would strip permissions from live tokens with mystery 403s as the only symptom. It now warns. 5. Creating a second token overwrote the first one's plaintext while it was still on screen and unrecoverable. ## Notes for review - `docs/superpowers/specs/2026-09-04-session-expiry-design.md` is the design for **#108** and is included as context, not as work in this PR. #108 is next. - `readJsonBody` and `.flatten()` were converged **only** in the tokens routes. Five other routes still hand-roll a local copy of the helper — that's pre-existing repo-wide drift and its own change. - The `expires_at` column has no upper bound in SQL; the ceiling is enforced in `createToken`, which is also where the finiteness guard lives. - Rate limiting is deliberately out of scope — tracked in #110 along with the rest of the internet-exposure hardening.

lz added 35 commits

2026-09-04 23:42:41 +02:00

docs(spec): session expiry — sliding idle timeout with absolute ceiling adadbc0545

Design for #108. Covers the expiry model (sliding idle deadline clamped by a
fixed ceiling), what counts as activity (non-GET requests plus an explicit
user-interaction keepalive, so background polls do not keep a session alive),
decoupling cookie lifetime from session lifetime, and the client-side 401 path
that redirects to /unlock and returns to the original page (closes #49).

docs(spec): scoped API tokens design; note shared auth-gate shape in #108 spec 200b94ee26

Design for #109. SHA-256 rather than Argon2id for token hashing (a 32-byte
CSPRNG token is unguessable at any hash speed, and a salted hash cannot be
looked up), a scope map keyed on event.route.id so an unmapped route denies by
default, and GET-only tokens so workspaces:read cannot reach the POST handlers
that spawn workspaces and create sessions.

Records the five-step hooks.server.ts gate both specs share, so the file is
restructured once.

docs(plan): implementation plan for scoped API tokens (#109 ) ffd16d64dd

Ten tasks, TDD throughout, with complete code in every step. Setup commands,
the vitest thread-pool constraint, and the green baseline (108 files / 1013
tests) were verified in this worktree before writing them down.

feat(tokens): add api_tokens table f470ce3905

docs(plan): fix the vitest thread-cap flag e43eeae941

maxThreads alone throws against this vitest.config.ts's defaults; minThreads
must be passed with it. Found while executing Task 1.

docs(plan): assert every mapped route id is real f59b081448

A mistyped key in TOKEN_ROUTES fails closed — the route becomes unreachable by
any token, forever, with every other test still green. Adds a filesystem parity
test plus its negative control.

feat(tokens): declare scopes and the route map 3f7241a85e

docs: unmap workspace health from workspaces:read 3c40c7c83e

Two independent reasons found in review. checkWorkerHealth returns a 30-line
container log tail via the same tailLogs helper that backs the unmapped
/api/workers/[id]/logs route, so the scope would grant exactly what the map
exists to withhold. It also fires convergeSessions, which relaunches claude
processes and hard-deletes session rows — making it the one candidate route
whose GET is not read-only, which breaks the premise the GET-only rule rests on.

fix(tokens): unmap workspace health and harden tokenMay 7d34d05def

checkWorkerHealth returns a container log tail via the same helper as the
unmapped logs route, and fires convergeSessions — so its GET is neither
read-only nor free of the data the map exists to withhold. Also guards the
prototype chain and a non-array scopes argument, both of which slipped past
the falsy check.

feat(tokens): generate, hash, create and list tokens e6bbbf3ac4

fix(tokens): re-read after insert and surface dropped scopes f37a4bd9c8

createToken returned a DTO built from the values it meant to insert rather
than the stored row, so a coerced write would be reported inaccurately —
better-sqlite3 binds NaN to NULL, which reads back as a token that never
expires. Re-reads after insert like every other service here, rejects a
non-finite expiry outright, and warns when a stored scope is no longer
recognised instead of silently stripping it from a live credential.

docs(tokens): record why createToken re-reads the inserted row 479bc2f6fd

No test pins it: once a non-finite expiry is rejected outright, no remaining
input diverges between what we build and what SQLite stores. The comment is
what stops it being simplified back into the bug it fixes.

test(tokens): pin the createToken re-read 9cf5bf5489

An unpaired UTF-16 surrogate in the label does not survive the trip into
SQLite, which is the one input that still diverges once a non-finite expiry
is rejected. Mutation-checked: building the DTO from the pre-insert values
turns this red. Asserts divergence and agreement with storage rather than the
exact replacement-character count, which is the vendor's business.

docs(plan): keep the scope-drop warning off the auth hot path b80d93177d

verifyToken runs on every authenticated request, so a single token with a
stale scope would warn on every poll. The warning belongs on the list path,
where the operator looking at the tokens page will actually see it.

feat(tokens): verify, revoke, and throttled last-used tracking 487e2953fe

fix(tokens): reject an expired token when now is not finite efd7aad677

verifyToken used `expires_at <= now`, which is false for NaN or -Infinity and
so accepted an expired token — the only fail-open path in the file. Negates
the comparison instead. Also pins the verify path's fail-closed scope parsing,
the hot-path logging silence, and touchToken's swallowed write, none of which
had a test.

feat(tokens): authorize bearer tokens in the request gate 1b135e40af

docs(plan): use a bare vi.fn for the spread-called mock 7b4344d4f2

vi.fn(async () => undefined) infers a zero-argument signature, so the
(...args: unknown[]) wrapper fails strict typecheck. Found while executing
Task 5; Task 6's mocks follow the same pattern and would have hit it too.

docs(tokens): explain the bare vi.fn in the hooks test 2e8bc5cb65

The plan records why an implementation-bearing mock breaks the spread wrapper
under strict TS; the file it describes did not. Without it this reads as an
inconsistency with the sibling mock and gets tidied back into a typecheck error.

fix(auth): gate on the resolved route, not the encoded pathname 9039afaa6f

The auth gate decided admission on event.url.pathname, which SvelteKit leaves
percent-encoded exactly as sent, while the router matches on the decoded path.
So /%61pi/quota failed the '/api/' prefix test and passed straight through to
/api/quota's handler with no credential of any kind — an unauthenticated
bypass of the whole gate, reaching every route that does not separately
re-check locals.masterKey (~30 of them, including the endpoint that rewrites
the fleet-shared CLAUDE.md and the one that approves a host port forward).

Keying on event.route.id means the string the gate tests and the handler that
runs cannot disagree. A null id matched no route, so nothing runs and
SvelteKit 404s on its own.

Verified live: /%61pi/quota, /a%70i/quota, /%61%70%69/quota and a PUT to
/%61pi/settings/instance all returned 200/400 before and 401 after.

Closes #120.

harden(auth): name the agent callbacks, deny remote calls 51f63459c7

The /api/agent/ prefix granted unauthenticated passthrough to any route ever
added beneath it — each of the three does its own bridge-IP check, but a
fourth would have inherited passthrough silently. Names them instead, so
adding one is a deliberate edit to this gate.

Also denies a remote function call. It resolves no route yet still dispatches
(respond.js skips route resolution for it), so it is the one shape where a
null routeId does not mean 'nothing runs' — which is exactly the assumption
the null-route passthrough rests on. Inert today: the feature is off and no
.remote.* files exist.

Both mutation-checked: restoring the prefix, or dropping the remote guard,
each turns its own test red.

feat(tokens): add token management endpoints 8bae4c155a

feat(tokens): client DTOs and API methods 3baa51057c

feat(tokens): add the API tokens settings tab f32324466d

fix(tokens): reject a malformed expiry and protect the shown token 85b56d03e8

An unparseable date became NaN, which JSON.stringify writes as null — the
same bytes as "no expiry" — so a typo silently minted a permanent token with
nothing to tell the operator. The server cannot catch it; by then the NaN is
already a null.

Creating a second token also overwrote the first one's plaintext while it was
still on screen, and the server keeps only a digest, so the first was then
unrecoverable.

fix(tokens): keep the plaintext out of the listed row f246194ae4

Prepending the whole create response left the token in rows[0].token, and
dismissing the card only clears `issued` — so the plaintext stayed reachable
in component state indefinitely, after the operator had been told it was gone.

docs(plan): correct the browser-pass recipe 722b87e056

pnpm dev cannot serve this app — migrations run only from the custom
src/server.ts entry, so the Vite dev server 500s on 'no such table: meta'.
Builds and runs the real server against a scratch DATA_DIR instead, seeds the
two stub files that clear the claude-session gate, and drops the allowedHosts
step (playwright-cli runs in this container, so it reaches 127.0.0.1 directly).

chore: ignore .agent/ browser-pass screenshots da80054ba9

AGENTS.md directs agents to write screenshots to .agent/screenshots/ and
surface them with notify-artifact, so every verification pass was leaving
untracked files behind. .playwright-cli/ was already ignored for the same
reason.

docs: reconcile the record with what was built 36ed638047

Four places where the durable record drifted from the code during execution:

- service.ts said no test pins the re-read; the surrogate test added one
  commit later pins it exactly, and the comment exists to stop that re-read
  being simplified away.
- The plan's browser-pass step lost its whole checklist and status-code matrix
  when the recipe was corrected — restored from what was actually run.
- The spec still said a quota:read token gets 403 on workspace health; that
  route was unmapped, so it denies with 401.
- The spec still described the in-memory throttle Map that was deliberately
  rejected for reading lastUsedAt off the already-fetched row.

feat(tokens): show created, guard a past expiry, confirm revoke 23204e422a

Four things the final review raised, all in the panel:

- A Created column. It was in the spec and in the DTO but never rendered, and
  it answers the one question the other columns cannot — 'last used: never'
  reads the same for a token minted five minutes ago and one minted last year.
- A past expiry is rejected. A date input yields UTC midnight at the START of
  the chosen day, so picking today silently minted a token that was already
  dead. Same class as the NaN case, but reachable through the normal picker.
- Revoke confirms. It hard-deletes a live credential and anything using it
  fails immediately; sibling panels confirm less destructive actions.
- The issued card shows the actual call. The feature exists so a script
  elsewhere can reach this API, and the panel was handing over a secret
  without naming the header or the endpoint.

docs(agents): record the route.id gate and the token model 1660c48e70

Facts 23 and 24. The first is why the auth gate keys on event.route.id and
never on url.pathname — the percent-encoding bypass (#120) cost a live
unauthenticated hole and is not obvious from reading either file alone. The
second is the token model: why SHA-256 rather than Argon2id, why tokenMay
rather than the masterKey guards is the actual authorization, why GET-only is
load-bearing, and why workspace health is excluded.

Both verified against the source before writing: +layout.server.ts does still
test url.pathname, the WS upgrade is a separate listener that never parses
Authorization, and /api/tokens carries both locks.

docs+refactor(tokens): document the feature, name the bad field d246d5f432

README had nothing for the audience this feature exists to serve — a script on
another machine — so it named neither the header nor the endpoints, and the
cleartext caveat lived only in AGENTS.md.

The POST also collapsed six distinct validation failures into one fixed string,
leaving a script author to guess which of label / scopes / expires_at was
wrong; it now returns zod's flatten() like connections/+server.ts does, with a
test that names the field and is mutation-checked. Drops the seventh local copy
of readJsonBody in favour of the shared helper.

Only the tokens routes here — converging the other five hand-rolled copies is
its own change and touches files this branch has no other reason to open.

test(tokens): anchor the GET-export parity check 0483908d44

A bare .includes('export const GET') is satisfied by a comment. Proven:
rename the real export to GETX, leave '// export const GET' above it, and the
check passes over a route with no GET handler — a test that had stopped
protecting the thing it claims to.

Anchored to the start of a line instead. Importing each module and asserting
typeof mod.GET would be stronger, but two mapped routes import
$server/singletons, which opens a real database and Docker client.

refactor(tokens): make the route map a Map, share the parity predicate 992254533f

TOKEN_ROUTES was a Record, so a route id naming a built-in ('constructor',
'toString') returned a truthy inherited value and needed an Object.hasOwn
guard plus five lines explaining why the guard had to precede the narrowing
check. Map.get returns undefined for those, so the class of bug stops existing
rather than being defended against. The prototype test now passes for a
structural reason, which is a better test than one passing because a guard
caught it.

The parity controls re-implemented the dead-mapping filter instead of calling
it — controls on a copy, which stay green while the real filter drifts. One
predicate now backs all three.

Also flattens the bearer block by one nesting level and reads the captured
routeId rather than event.route.id a second time: this file's invariant is
that every admission decision reads one name for that value, and a second
spelling is what survives a refactor as url.pathname.

Re-ran the security mutations against the simplified gate — dropping the
method check, setting masterKey on the token path, and reverting to
url.pathname each still turn their own tests red.

fix(tokens): bound the expiry, correct three wrong comments

ci / nexus (pull_request) Successful in 12m15s

Details

ci / images (pull_request) Successful in 14m24s

Details

pr-image-cleanup / delete-pr-images (pull_request) Successful in 6s

Details

95e962b7c5

A ceiling on expires_at. Microseconds passed where milliseconds were meant is
finite, integer, positive and a safe integer, so every existing guard accepts
it — the client only checks a floor, zod has no max, and createToken only
checked finiteness. It stores exactly and mints a credential valid until the
year 58647. Same family as the NaN case: a plausible mistake producing a
near-permanent credential with nothing to say so.

Three comments were confidently wrong about mechanism while right about the
conclusion, which is the worst kind — they stop the next person checking:

- 'better-sqlite3 binds a non-finite number to NULL' is true only of NaN.
  Measured: +/-Infinity bind as REAL, and -Infinity is always-EXPIRED, the
  opposite of the stated consequence.
- The negated expiry comparison was described as rejecting any non-finite
  now. Measured: it changes the outcome for NaN only; -Infinity is accepted
  either way.
- AGENTS.md said neither mapped POST guards on masterKey. /api/workers POST
  does, at :29. Only the sessions POST is a genuine one-lock case, which is
  the whole point of the GET-only rule and worth stating accurately.

Plus a spec line citation already off by one, and two comment cuts.

lz referenced this pull request

2026-09-05 12:11:03 +02:00

Session expiry: sliding idle timeout with an absolute ceiling (#108) #127

lz merged commit 7b6e5bc445 into main

2026-09-05 12:13:43 +02:00

lz deleted branch fix/session

2026-09-05 12:13:47 +02:00

lz referenced this pull request from a commit

2026-09-05 12:13:50 +02:00

Merge pull request 'Scoped API tokens (#109), and a fix for an unauthenticated auth-gate bypass (#120)' (#123) from fix/session into main

lz referenced this pull request

2026-09-05 17:10:28 +02:00