security: enforce auth policy for all endpoints in tunnel mode#1002
Open
Hybirdss wants to merge 2 commits intogarrytan:mainfrom
Open
security: enforce auth policy for all endpoints in tunnel mode#1002Hybirdss wants to merge 2 commits intogarrytan:mainfrom
Hybirdss wants to merge 2 commits intogarrytan:mainfrom
Conversation
1367a80 to
9abcc60
Compare
When BROWSE_TUNNEL=1 is set the server is reachable from the public internet despite binding to 127.0.0.1. Three bootstrapping paths that relied on the localhost trust assumption silently became remote attack surfaces: - GET /health returned the root AUTH_TOKEN to any caller that forged an `Origin: chrome-extension://...` header (trivially spoofable). - GET /health also returned the token unconditionally in headed mode, regardless of tunnel state. - GET /cookie-picker served the root token embedded in HTML with no auth gate, readable by anyone with the ngrok URL. - POST /inspector/pick and the inspector SSE endpoint had no auth check at all (localhost-only assumption). Fix: single `enforceTunnelPolicy()` function runs before every route handler. When the tunnel is active it rejects unauthenticated requests unless the path is on an explicit allowlist. The allowlist currently contains only `/connect` (the remote pairing ceremony entry point, which is intentionally pre-auth and rate-limited). Adding any new entry requires deliberate justification — "I am exposing this endpoint to the internet without auth." Per-endpoint hardening provides defense-in-depth: - /health: token delivery gated behind `!tunnelActive`; returns an `extensionUnavailable` hint instead so remote callers know to use the /connect → /token pairing flow. - /cookie-picker: returns 403 in tunnel mode (the endpoint reads local browser databases and cannot function remotely regardless). - /inspector: explicit auth gate covers all sub-paths before any handler runs. /token is intentionally NOT on the allowlist: it has its own `isRootRequest()` guard and is reachable in tunnel mode for callers that already hold the root token. Tests 13a–13h in server-auth.test.ts verify each attack scenario is blocked and guard against future regressions. refine: add /token exclusion note to tunnel allowlist comment ci: trigger re-run with fork secrets configured
e682332 to
7df20ea
Compare
7df20ea to
0b6cedf
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When
BROWSE_TUNNEL=1is set, the server is reachable from the public internet despite binding to127.0.0.1. Several bootstrapping paths were designed for local-only access and had no auth checks — they silently became remote attack surfaces.#904 fixed the cookie-picker token leak (one-time code exchange, no token in HTML). This PR adds the layer above it: a central gate that rejects unauthenticated requests to any endpoint when the tunnel is active — so future endpoints are protected by default, not by per-handler discipline.
Four issues closed by this PR:
1. Root token exposure via forged Origin header
Any HTTP client can set the
Originheader. The chrome-extension check was meaningful only on localhost.2. Root token exposure in headed mode
Same
/healthendpoint — when the server starts in headed mode, the token was returned unconditionally regardless of tunnel state.3. Cookie-picker accessible without auth in tunnel mode
Even after the #904 token-in-HTML fix,
GET /cookie-pickerwas reachable without auth. Cookie picker reads local browser databases — it can't function remotely regardless, so the right response in tunnel mode is an explicit403.4. Inspector endpoints had no auth gate
POST /inspector/pick,GET /inspector/stream, and related paths accepted requests from anyone.Root cause
server.tsroute handlers each had their own ad-hoc auth checks designed for local-only access. When tunnel mode was added, no central enforcement was added — each handler's "localhost = trusted" assumption became a remote attack surface. Per-handler fixes (#904 and others) close individual gaps; this PR closes the class.Fix
Single policy function runs before every route handler:
Adding a new allowlist entry requires deliberate justification — the comment forces the question "am I intentionally exposing this to the internet without auth?"
Per-endpoint defense-in-depth:
/health: token delivery gated behind!tunnelActive; returnsextensionUnavailable: truewith a hint to use--pairinstead/cookie-picker: explicit403in tunnel mode (reads local browser databases — can't work remotely regardless)/inspector: auth gate at the top of the block covers all sub-paths/tokenis intentionally not on the allowlist — it has its ownisRootRequest()guard and works correctly in tunnel mode for callers who already hold the root token.Local mode behavior is unchanged.
enforceTunnelPolicyreturns immediately whentunnelActiveis false.Test plan
Tests 13a–13h added to
browse/test/server-auth.test.ts— same static-analysis pattern as existing tests. Each test documents which attack scenario it guards against and verifies the fix survives future refactors.All 126 tests pass (on latest main, after rebase over #988).