Auth gate is bypassable by percent-encoding the path #120

New issue

Closed

opened 2026-09-04 20:07:56 +02:00 by lz · 0 comments

lz commented

2026-09-04 20:07:56 +02:00

Owner

Unauthenticated bypass of the entire API auth gate. Pre-existing — not introduced by #109, but found while reviewing it. Verified live against vite dev on current main-equivalent code.

Reproduction

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

%61 is a. No cookie, no token, no credential of any kind.

Not read-only:

PUT /api/settings/instance     ->  401  {"error":"locked"}           (stopped at the gate)
PUT /%61pi/settings/instance   ->  400  {"error":"unknown setting"}  (handler ran, validated the body)

Confirmed variants: /%61pi/…, /a%70i/…, /ap%69/…, /%61%70%69/….

Cause

hooks.server.ts decides admission on event.url.pathname, which SvelteKit leaves percent-encoded exactly as sent. The router matches on the decoded path:

respond.js:254 — resolved_path = decode_pathname(resolved_path), where decode_pathname is pathname.split('%25').map(decodeURI).join('%25') (utils/url.js:50)
respond.js:322 — find_route(resolved_path, …) populates event.route.id
url.pathname is never rewritten. The only pre-handle normalisation is the trailing-slash 308 at respond.js:366, which does not touch percent-encoding.

So path.startsWith('/api/') tests a string the router has already discarded. /%61pi/quota fails the prefix test, hits return resolve(event) unconditionally, and the real /api/quota handler runs.

Versions: SvelteKit 2.59.1.

Blast radius

Every API route that does not separately guard on locals.masterKey — around 30. The ones that matter most, all reachable unauthenticated:

PUT /api/settings/agent-instructions — rewrites the fleet-shared ~/.claude/CLAUDE.md, which every running agent live-reads
POST /api/previews/[previewId]/approve — the operator-consent gate for opening a host port
DELETE /api/workers/[id], POST /api/workers/[id]/stop, POST /api/workers/[id]/sessions
GET /api/workers/[id]/logs, GET …/artifacts/[artifactId]/raw
PUT /api/settings/instance

Config-explorer, connections and files routes survive only because each re-checks masterKey. The defense-in-depth described in that guard's docstring is currently doing the entire job on its own.

Fix

Decide admission on event.route.id — the identifier the router actually resolved — rather than on event.url.pathname. The two cannot then disagree by construction. PUBLIC_API becomes a set of route ids; for the four static routes it contains (/api/state, /api/auth/setup, /api/auth/unlock, /api/auth/lock) the strings are identical, so it is a same-shape change.

A null route.id means the router matched nothing, so there is no handler to protect and the request 404s regardless.

This is the argument tokens/scopes.ts already makes for the scope map — that keying on route.id avoids "the path-normalisation surface". The gate needs to follow its own advice.

Note on exposure

preview_bind_interface and public_url govern how reachable an instance is. Any deployment reachable beyond loopback should treat this as urgent. See also #110.

**Unauthenticated bypass of the entire API auth gate.** Pre-existing — not introduced by #109, but found while reviewing it. Verified live against `vite dev` on current `main`-equivalent code. ## Reproduction ``` GET /api/quota -> 401 {"error":"locked"} GET /%61pi/quota -> 200 {"quota":null,"sessions":[]} ``` `%61` is `a`. No cookie, no token, no credential of any kind. Not read-only: ``` PUT /api/settings/instance -> 401 {"error":"locked"} (stopped at the gate) PUT /%61pi/settings/instance -> 400 {"error":"unknown setting"} (handler ran, validated the body) ``` Confirmed variants: `/%61pi/…`, `/a%70i/…`, `/ap%69/…`, `/%61%70%69/…`. ## Cause `hooks.server.ts` decides admission on `event.url.pathname`, which SvelteKit leaves **percent-encoded exactly as sent**. The router matches on the **decoded** path: - `respond.js:254` — `resolved_path = decode_pathname(resolved_path)`, where `decode_pathname` is `pathname.split('%25').map(decodeURI).join('%25')` (`utils/url.js:50`) - `respond.js:322` — `find_route(resolved_path, …)` populates `event.route.id` - `url.pathname` is never rewritten. The only pre-`handle` normalisation is the trailing-slash 308 at `respond.js:366`, which does not touch percent-encoding. So `path.startsWith('/api/')` tests a string the router has already discarded. `/%61pi/quota` fails the prefix test, hits `return resolve(event)` unconditionally, and the real `/api/quota` handler runs. Versions: SvelteKit 2.59.1. ## Blast radius Every API route that does not *separately* guard on `locals.masterKey` — around 30. The ones that matter most, all reachable unauthenticated: - `PUT /api/settings/agent-instructions` — rewrites the fleet-shared `~/.claude/CLAUDE.md`, which every running agent live-reads - `POST /api/previews/[previewId]/approve` — the operator-consent gate for opening a host port - `DELETE /api/workers/[id]`, `POST /api/workers/[id]/stop`, `POST /api/workers/[id]/sessions` - `GET /api/workers/[id]/logs`, `GET …/artifacts/[artifactId]/raw` - `PUT /api/settings/instance` Config-explorer, connections and files routes survive only because each re-checks `masterKey`. The defense-in-depth described in that guard's docstring is currently doing the entire job on its own. ## Fix Decide admission on `event.route.id` — the identifier the router actually resolved — rather than on `event.url.pathname`. The two cannot then disagree by construction. `PUBLIC_API` becomes a set of route ids; for the four static routes it contains (`/api/state`, `/api/auth/setup`, `/api/auth/unlock`, `/api/auth/lock`) the strings are identical, so it is a same-shape change. A null `route.id` means the router matched nothing, so there is no handler to protect and the request 404s regardless. This is the argument `tokens/scopes.ts` already makes for the scope map — that keying on `route.id` avoids "the path-normalisation surface". The gate needs to follow its own advice. ## Note on exposure `preview_bind_interface` and `public_url` govern how reachable an instance is. Any deployment reachable beyond loopback should treat this as urgent. See also #110.

lz added the

bug

label

2026-09-04 20:07:56 +02:00

lz referenced this issue from a commit

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

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

lz referenced this issue from a pull request that will close it,

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

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

lz closed this issue

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