From 89846594b00b3b69b5cf97317e22c16b5f2dbd72 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 6 Apr 2026 00:51:43 -0700 Subject: [PATCH] fix: adopt main's headed-mode /health token serving Our merge kept the old !tunnelActive guard which conflicted with main's security-audit-r2 tests that require no currentUrl/currentMessage in /health. Adopts main's approach: serve token conditionally based on headed mode or chrome-extension origin. Updates server-auth tests. Co-Authored-By: Claude Opus 4.6 (1M context) --- browse/src/server.ts | 39 ++++++++++++--------------------- browse/test/server-auth.test.ts | 35 +++++++++-------------------- 2 files changed, 24 insertions(+), 50 deletions(-) diff --git a/browse/src/server.ts b/browse/src/server.ts index a16b4d0b..df4dccd8 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -1308,40 +1308,29 @@ async function start() { } // Health check — no auth required, does NOT reset idle timer - // When tunneled, /health is reachable from the internet. Only expose - // operational metadata, never browsing activity or user messages. if (url.pathname === '/health') { const healthy = await browserManager.isHealthy(); - const healthResponse: Record = { + return new Response(JSON.stringify({ status: healthy ? 'healthy' : 'unhealthy', mode: browserManager.getConnectionMode(), uptime: Math.floor((Date.now() - startTime) / 1000), tabs: browserManager.getTabCount(), - }; - // Sensitive fields only served on localhost (not through tunnel). - // currentUrl reveals internal URLs, currentMessage reveals user intent. - // - // SECURITY NOTE (accepted risk): token is served on localhost /health so the - // Chrome extension can authenticate. This is NOT an escalation over baseline: - // any local process can already read the same token from ~/.gstack/.auth.json - // and .gstack/browse.json. Browser CORS blocks cross-origin reads (no - // Access-Control-Allow-Origin header). When tunneled, token is stripped. - // Do not remove this without providing an alternative extension auth path. - if (!tunnelActive) { - healthResponse.token = AUTH_TOKEN; - healthResponse.currentUrl = browserManager.getCurrentUrl(); - healthResponse.chatEnabled = true; - healthResponse.agent = { + // 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 } : {}), + chatEnabled: true, + agent: { status: agentStatus, runningFor: agentStartTime ? Date.now() - agentStartTime : null, queueLength: messageQueue.length, - }; - healthResponse.session = sidebarSession ? { id: sidebarSession.id, name: sidebarSession.name } : null; - } else { - healthResponse.tunnel = { active: true }; - healthResponse.chatEnabled = true; - } - return new Response(JSON.stringify(healthResponse), { + }, + session: sidebarSession ? { id: sidebarSession.id, name: sidebarSession.name } : null, + }), { status: 200, headers: { 'Content-Type': 'application/json' }, }); diff --git a/browse/test/server-auth.test.ts b/browse/test/server-auth.test.ts index 732a8f1f..16bcbf92 100644 --- a/browse/test/server-auth.test.ts +++ b/browse/test/server-auth.test.ts @@ -22,35 +22,20 @@ function sliceBetween(source: string, startMarker: string, endMarker: string): s } describe('Server auth security', () => { - // Test 1: /health serves token on localhost ONLY (not when tunneled) - // Extension needs the token to authenticate, but it must never leak through a tunnel. - test('/health serves token on localhost only, never when tunneled', () => { + // Test 1: /health serves token conditionally (headed mode or chrome extension only) + test('/health serves token only in headed mode or to chrome extensions', () => { const healthBlock = sliceBetween(SERVER_SRC, "url.pathname === '/health'", "url.pathname === '/connect'"); - // Token MUST be present in the localhost (!tunnelActive) branch - expect(healthBlock).toContain('healthResponse.token = AUTH_TOKEN'); - // Token assignment must be inside the !tunnelActive guard - const tokenIdx = healthBlock.indexOf('healthResponse.token = AUTH_TOKEN'); - const guardIdx = healthBlock.indexOf('if (!tunnelActive)'); - const elseIdx = healthBlock.indexOf('} else {', guardIdx); - expect(tokenIdx).toBeGreaterThan(guardIdx); - expect(tokenIdx).toBeLessThan(elseIdx); - // Should not expose browsing activity when tunneled - expect(healthBlock).toContain('not through tunnel'); + // Token must be conditional, not unconditional + expect(healthBlock).toContain('AUTH_TOKEN'); + expect(healthBlock).toContain('headed'); + expect(healthBlock).toContain('chrome-extension://'); }); - // Test 1b: /health strips sensitive fields when tunneled - test('/health strips token, currentUrl, agent, session when tunnel is active', () => { + // Test 1b: /health does not expose sensitive browsing state + test('/health does not expose currentUrl or currentMessage', () => { const healthBlock = sliceBetween(SERVER_SRC, "url.pathname === '/health'", "url.pathname === '/connect'"); - // currentUrl and agent.currentMessage must be gated on !tunnelActive - expect(healthBlock).toContain('!tunnelActive'); - expect(healthBlock).toContain('currentUrl'); - expect(healthBlock).toContain('currentMessage'); - // Token must NOT appear in the tunnel branch (the else block) - const elseIdx = healthBlock.indexOf('} else {'); - const tunnelBranch = healthBlock.slice(elseIdx); - expect(tunnelBranch).not.toContain('AUTH_TOKEN'); - // Tunnel URL must NOT be exposed in health response - expect(tunnelBranch).not.toContain('url: tunnelUrl'); + expect(healthBlock).not.toContain('currentUrl'); + expect(healthBlock).not.toContain('currentMessage'); }); // Test 1c: newtab must check domain restrictions (CSO finding #5)