diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index a755ff24cb..bdc4375e12 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -89,7 +89,15 @@ The HTTP server binds to `localhost`, not `0.0.0.0`. It's not reachable from the Every server session generates a random UUID token, written to the state file with mode 0o600 (owner-only read). Every HTTP request must include `Authorization: Bearer `. If the token doesn't match, the server returns 401. -This prevents other processes on the same machine from talking to your browse server. The cookie picker UI (`/cookie-picker`) and health check (`/health`) are exempt — they're localhost-only and don't execute commands. +This prevents other processes on the same machine from talking to your browse server. In local mode, the cookie picker UI (`/cookie-picker`) and health check (`/health`) are exempt — they're localhost-only and don't execute commands. In tunnel mode these exemptions are removed; see **Tunnel mode auth** below. + +### Tunnel mode auth + +When `BROWSE_TUNNEL=1` is set, the server is internet-reachable via ngrok. `enforceTunnelPolicy()` runs before every route handler: in tunnel mode, all endpoints require a valid bearer token except CORS preflights and `TUNNEL_UNAUTHENTICATED_ALLOWLIST` (currently: `/connect` only). + +`/connect` is the remote pairing endpoint — it must be reachable before a client has a token, is rate-limited, and returns no root secrets. Everything else (`/health`, `/cookie-picker`, all command routes) requires auth. The allowlist is a closed set with a documented reason per entry. + +Why `/health` and `/cookie-picker` lose their local exemptions in tunnel mode: the session carries Keychain-derived cookie decryption keys. Any unauthenticated surface on the internet is a path to those keys. ### Cookie security diff --git a/browse/src/server.ts b/browse/src/server.ts index 98f43af0c9..b1764d377b 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -87,6 +87,50 @@ function isRootRequest(req: Request): boolean { return token !== null && isRootToken(token); } +// ─── Tunnel security policy ────────────────────────────────────── +// +// When a tunnel (ngrok or SSH forward) is active, the server is reachable +// from the public internet despite binding to 127.0.0.1. Any endpoint that +// relies on the "localhost = trusted" assumption becomes a remote attack +// surface. +// +// This function is the single enforcement point: it runs before every route +// handler and, when the tunnel is active, rejects unauthenticated requests +// unless the path is on the explicit allowlist below. +// +// Adding a new allowlist entry requires deliberate intent — "I am exposing +// this endpoint to the internet without auth." Every other endpoint gets +// authentication for free. +// Endpoints that remain unauthenticated in tunnel mode. +// Adding a new entry requires deliberate intent: "I am exposing this to the +// internet without auth." Every other endpoint is protected for free. +// +// /token is intentionally excluded — it has its own isRootRequest() guard. +const TUNNEL_UNAUTHENTICATED_ALLOWLIST = new Set([ + '/connect', // remote pairing ceremony — rate-limited, no root secrets returned +]); + +function enforceTunnelPolicy(req: Request, url: URL): Response | null { + if (!tunnelActive) return null; // local mode — existing behaviour unchanged + + if (TUNNEL_UNAUTHENTICATED_ALLOWLIST.has(url.pathname)) return null; + + // Allow OPTIONS preflight through so CORS headers are sent correctly. + if (req.method === 'OPTIONS') return null; + + if (!validateAuth(req) && !getTokenInfo(req)) { + return new Response( + JSON.stringify({ + error: 'Tunnel mode: authentication required', + hint: 'All endpoints require a Bearer token when the server is tunneled. ' + + 'Pair a remote agent with: gstack browse --pair', + }), + { status: 401, headers: { 'Content-Type': 'application/json' } }, + ); + } + return null; // authenticated — proceed to route handler +} + // ─── Sidebar Model Router ──────────────────────────────────────── // Fast model for navigation/interaction, smart model for reading/analysis. // The delta between sonnet and opus on "click @e24" is 5-10x in latency @@ -1294,8 +1338,28 @@ async function start() { fetch: async (req) => { const url = new URL(req.url); - // Cookie picker routes — HTML page unauthenticated, data/action routes require auth + // Tunnel security gate — must run before any route handler. + // Rejects unauthenticated requests when the server is exposed via ngrok/SSH tunnel. + const tunnelBlock = enforceTunnelPolicy(req, url); + if (tunnelBlock) return tunnelBlock; + + // Cookie picker routes. + // In tunnel mode this feature is explicitly disabled: cookie-picker reads + // local browser DBs (~/Library/.../Chrome, ~/.config/google-chrome, etc.) + // which only make sense when the caller is physically on the server machine. + // Returning a clear 403 is better than serving a broken UI where the embedded + // auth token is missing and every data API call fails silently. if (url.pathname.startsWith('/cookie-picker')) { + if (tunnelActive) { + return new Response( + JSON.stringify({ + error: 'Cookie picker is not available in tunnel mode', + hint: 'Cookie import reads local browser databases and requires direct server access. ' + + 'Disable the tunnel and run the server locally to use this feature.', + }), + { status: 403, headers: { 'Content-Type': 'application/json' } }, + ); + } return handleCookiePickerRoute(url, req, browserManager, AUTH_TOKEN); } @@ -1339,14 +1403,29 @@ async function start() { mode: browserManager.getConnectionMode(), uptime: Math.floor((Date.now() - startTime) / 1000), tabs: browserManager.getTabCount(), - // Auth token for extension bootstrap. Safe: /health is localhost-only. - // Previously served unconditionally, but that leaks the token if the - // server is tunneled to the internet (ngrok, SSH tunnel). - // In headed mode the server is always local, so return token unconditionally - // (fixes Playwright Chromium extensions that don't send Origin header). - ...(browserManager.getConnectionMode() === 'headed' || - req.headers.get('origin')?.startsWith('chrome-extension://') - ? { token: AUTH_TOKEN } : {}), + // Auth token for Chrome extension / headed-mode bootstrap. + // + // Delivery conditions (ALL must hold): + // 1. Tunnel is NOT active — when tunneled, any caller can set + // Origin: chrome-extension://, so the Origin check + // is not a meaningful guard. In tunnel mode the Chrome + // extension is not expected to work; agents should pair via + // the /connect → /token ceremony instead. + // 2. Request is from a local Chrome extension (origin header) + // OR the server is in headed mode (where the process is + // always local and extensions may omit the Origin header). + // + // When tunnel is active and token is withheld, a hint is returned + // so that callers get a clear error rather than a silent omission. + ...(!tunnelActive && ( + browserManager.getConnectionMode() === 'headed' || + req.headers.get('origin')?.startsWith('chrome-extension://') + ) + ? { token: AUTH_TOKEN } + : tunnelActive + ? { extensionUnavailable: true, hint: 'Chrome extension bootstrap is disabled in tunnel mode. Remote agents must pair via /connect → /token. Token location for local use: ~/.gstack/browse.json' } + : {} + ), chatEnabled: true, agent: { status: agentStatus, @@ -2135,7 +2214,21 @@ async function start() { }); } - // ─── Inspector endpoints ────────────────────────────────────── + // ─── Inspector endpoints — auth required on all sub-paths ──────── + // These endpoints expose page DOM snapshots, allow CSS injection, and + // receive element picks from the Chrome extension. They previously + // had no auth gate (localhost-only assumption). Auth is now enforced + // at the top of the block so every path below is covered automatically. + // The tunnel policy gate at the top of the handler already rejects + // unauthenticated tunnel requests, but this explicit guard means the + // inspector is hardened regardless of future changes to the gate. + if (url.pathname.startsWith('/inspector')) { + if (!validateAuth(req) && !getTokenInfo(req)) { + return new Response(JSON.stringify({ error: 'Unauthorized' }), { + status: 401, headers: { 'Content-Type': 'application/json' }, + }); + } + } // POST /inspector/pick — receive element pick from extension, run CDP inspection if (url.pathname === '/inspector/pick' && req.method === 'POST') { diff --git a/browse/test/server-auth.test.ts b/browse/test/server-auth.test.ts index 48c459870a..08cf117845 100644 --- a/browse/test/server-auth.test.ts +++ b/browse/test/server-auth.test.ts @@ -342,3 +342,129 @@ describe('Server auth security', () => { expect(routeSrc).toContain('SameSite=Strict'); }); }); + +// ─── Tunnel security policy tests ────────────────────────────────────────── +// +// These tests verify the fix for: when BROWSE_TUNNEL=1 is set, the server is +// reachable from the public internet despite binding to 127.0.0.1. Any endpoint +// that relied on "localhost = trusted" was exposed as a remote attack surface. +// +// Root cause: three bootstrapping paths (Origin forgery on /health, /cookie-picker +// HTML embedding the token, headed-mode /health check) were designed for the local +// trust context and silently became internet-exposed under the tunnel. +// +// Fix architecture: single enforceTunnelPolicy() function runs before every route +// handler and rejects unauthenticated requests in tunnel mode. A minimal allowlist +// names the only endpoints that legitimately need pre-auth access (/connect, which +// is the remote pairing ceremony). Per-endpoint hardening (cookie-picker 403, +// inspector auth gate) provides defense-in-depth. +// +// Tests 13a–13h verify the fix is in place and document which attack scenario each +// test guards against. + +describe('Tunnel security policy', () => { + // Test 13a: allowlist contains exactly /connect — not /token, not /health, + // not /cookie-picker. /token has its own isRootRequest() internal guard and + // must NOT appear here (whitelist entries must justify internet exposure). + test('TUNNEL_UNAUTHENTICATED_ALLOWLIST contains /connect and nothing else sensitive', () => { + const allowlistBlock = sliceBetween( + SERVER_SRC, + 'TUNNEL_UNAUTHENTICATED_ALLOWLIST = new Set([', + ']);', + ); + expect(allowlistBlock).toContain("'/connect'"); + // /token is root-only via isRootRequest() — it must NOT be in the allowlist + expect(allowlistBlock).not.toContain("'/token'"); + // Health and cookie-picker must never be open to unauthenticated tunnel callers + expect(allowlistBlock).not.toContain("'/health'"); + expect(allowlistBlock).not.toContain("'/cookie-picker'"); + }); + + // Test 13b: enforceTunnelPolicy is invoked before any route handler. + // If it moves below a route, that route becomes unguarded in tunnel mode. + test('enforceTunnelPolicy fires before the first route handler', () => { + const gateIdx = SERVER_SRC.indexOf('const tunnelBlock = enforceTunnelPolicy(req, url)'); + const firstRouteIdx = SERVER_SRC.indexOf("url.pathname === '/health'"); + expect(gateIdx).toBeGreaterThan(0); + expect(firstRouteIdx).toBeGreaterThan(0); + expect(gateIdx).toBeLessThan(firstRouteIdx); + }); + + // Test 13c: enforceTunnelPolicy returns 401 (not 403 or 200) with a + // machine-readable JSON body so agents can detect tunnel auth failure. + test('enforceTunnelPolicy responds 401 with pairing hint', () => { + const policyFn = sliceBetween(SERVER_SRC, 'function enforceTunnelPolicy', 'function wrapError'); + expect(policyFn).toContain('status: 401'); + expect(policyFn).toContain('Tunnel mode: authentication required'); + expect(policyFn).toContain('gstack browse --pair'); + }); + + // Test 13d: Attack scenario — F-1/F-3: Origin forgery or headed-mode flag used + // to extract AUTH_TOKEN from /health over the public tunnel. + // Fix: token delivery is gated behind !tunnelActive. + test('/health delivers AUTH_TOKEN only when the tunnel is NOT active', () => { + const healthBlock = sliceBetween( + SERVER_SRC, + "url.pathname === '/health'", + "url.pathname === '/connect'", + ); + // The !tunnelActive guard must appear in the health block + expect(healthBlock).toContain('!tunnelActive'); + // token: AUTH_TOKEN must appear AFTER the !tunnelActive condition + const tunnelGuardIdx = healthBlock.indexOf('!tunnelActive'); + const tokenIdx = healthBlock.indexOf('token: AUTH_TOKEN'); + expect(tunnelGuardIdx).toBeGreaterThan(-1); + expect(tokenIdx).toBeGreaterThan(-1); + expect(tokenIdx).toBeGreaterThan(tunnelGuardIdx); + }); + + // Test 13e: Attack scenario — F-1 continued: when tunnel is active and a Chrome + // extension Origin is forged, the server must return a hint instead of the token. + test('/health returns extensionUnavailable hint in tunnel mode', () => { + const healthBlock = sliceBetween( + SERVER_SRC, + "url.pathname === '/health'", + "url.pathname === '/connect'", + ); + expect(healthBlock).toContain('extensionUnavailable: true'); + // The hint must direct remote callers to the correct pairing flow + expect(healthBlock).toContain('/connect'); + }); + + // Test 13f: Attack scenario — F-2: GET /cookie-picker had no auth check and + // embedded AUTH_TOKEN in the served HTML, readable by any caller with the + // ngrok URL. In tunnel mode the endpoint must be fully blocked. + test('/cookie-picker returns 403 in tunnel mode', () => { + const cookieBlock = sliceBetween( + SERVER_SRC, + "url.pathname.startsWith('/cookie-picker')", + 'handleCookiePickerRoute', + ); + expect(cookieBlock).toContain('tunnelActive'); + expect(cookieBlock).toContain('status: 403'); + expect(cookieBlock).toContain('Cookie picker is not available in tunnel mode'); + }); + + // Test 13g: /inspector endpoints had no auth gate (localhost-only assumption). + // The auth block must appear before /inspector/pick in source order. + test('/inspector auth gate appears before /inspector/pick handler', () => { + const authGateIdx = SERVER_SRC.indexOf("Inspector endpoints — auth required on all sub-paths"); + const pickHandlerIdx = SERVER_SRC.indexOf("POST /inspector/pick"); + expect(authGateIdx).toBeGreaterThan(0); + expect(pickHandlerIdx).toBeGreaterThan(0); + expect(authGateIdx).toBeLessThan(pickHandlerIdx); + }); + + // Test 13h: /inspector auth gate checks BOTH validateAuth (root token) and + // getTokenInfo (scoped token) so inspector is accessible to legitimate agents. + test('/inspector auth gate accepts both root and scoped tokens', () => { + const inspectorBlock = sliceBetween( + SERVER_SRC, + "Inspector endpoints — auth required on all sub-paths", + "POST /inspector/pick", + ); + expect(inspectorBlock).toContain('validateAuth(req)'); + expect(inspectorBlock).toContain('getTokenInfo(req)'); + expect(inspectorBlock).toContain('Unauthorized'); + }); +});