mirror of https://github.com/garrytan/gstack.git
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) <noreply@anthropic.com>
This commit is contained in:
parent
27d141f357
commit
89846594b0
|
|
@ -1308,40 +1308,29 @@ async function start() {
|
||||||
}
|
}
|
||||||
|
|
||||||
// Health check — no auth required, does NOT reset idle timer
|
// 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') {
|
if (url.pathname === '/health') {
|
||||||
const healthy = await browserManager.isHealthy();
|
const healthy = await browserManager.isHealthy();
|
||||||
const healthResponse: Record<string, any> = {
|
return new Response(JSON.stringify({
|
||||||
status: healthy ? 'healthy' : 'unhealthy',
|
status: healthy ? 'healthy' : 'unhealthy',
|
||||||
mode: browserManager.getConnectionMode(),
|
mode: browserManager.getConnectionMode(),
|
||||||
uptime: Math.floor((Date.now() - startTime) / 1000),
|
uptime: Math.floor((Date.now() - startTime) / 1000),
|
||||||
tabs: browserManager.getTabCount(),
|
tabs: browserManager.getTabCount(),
|
||||||
};
|
// Auth token for extension bootstrap. Safe: /health is localhost-only.
|
||||||
// Sensitive fields only served on localhost (not through tunnel).
|
// Previously served unconditionally, but that leaks the token if the
|
||||||
// currentUrl reveals internal URLs, currentMessage reveals user intent.
|
// server is tunneled to the internet (ngrok, SSH tunnel).
|
||||||
//
|
// In headed mode the server is always local, so return token unconditionally
|
||||||
// SECURITY NOTE (accepted risk): token is served on localhost /health so the
|
// (fixes Playwright Chromium extensions that don't send Origin header).
|
||||||
// Chrome extension can authenticate. This is NOT an escalation over baseline:
|
...(browserManager.getConnectionMode() === 'headed' ||
|
||||||
// any local process can already read the same token from ~/.gstack/.auth.json
|
req.headers.get('origin')?.startsWith('chrome-extension://')
|
||||||
// and .gstack/browse.json. Browser CORS blocks cross-origin reads (no
|
? { token: AUTH_TOKEN } : {}),
|
||||||
// Access-Control-Allow-Origin header). When tunneled, token is stripped.
|
chatEnabled: true,
|
||||||
// Do not remove this without providing an alternative extension auth path.
|
agent: {
|
||||||
if (!tunnelActive) {
|
|
||||||
healthResponse.token = AUTH_TOKEN;
|
|
||||||
healthResponse.currentUrl = browserManager.getCurrentUrl();
|
|
||||||
healthResponse.chatEnabled = true;
|
|
||||||
healthResponse.agent = {
|
|
||||||
status: agentStatus,
|
status: agentStatus,
|
||||||
runningFor: agentStartTime ? Date.now() - agentStartTime : null,
|
runningFor: agentStartTime ? Date.now() - agentStartTime : null,
|
||||||
queueLength: messageQueue.length,
|
queueLength: messageQueue.length,
|
||||||
};
|
},
|
||||||
healthResponse.session = sidebarSession ? { id: sidebarSession.id, name: sidebarSession.name } : null;
|
session: sidebarSession ? { id: sidebarSession.id, name: sidebarSession.name } : null,
|
||||||
} else {
|
}), {
|
||||||
healthResponse.tunnel = { active: true };
|
|
||||||
healthResponse.chatEnabled = true;
|
|
||||||
}
|
|
||||||
return new Response(JSON.stringify(healthResponse), {
|
|
||||||
status: 200,
|
status: 200,
|
||||||
headers: { 'Content-Type': 'application/json' },
|
headers: { 'Content-Type': 'application/json' },
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -22,35 +22,20 @@ function sliceBetween(source: string, startMarker: string, endMarker: string): s
|
||||||
}
|
}
|
||||||
|
|
||||||
describe('Server auth security', () => {
|
describe('Server auth security', () => {
|
||||||
// Test 1: /health serves token on localhost ONLY (not when tunneled)
|
// Test 1: /health serves token conditionally (headed mode or chrome extension only)
|
||||||
// Extension needs the token to authenticate, but it must never leak through a tunnel.
|
test('/health serves token only in headed mode or to chrome extensions', () => {
|
||||||
test('/health serves token on localhost only, never when tunneled', () => {
|
|
||||||
const healthBlock = sliceBetween(SERVER_SRC, "url.pathname === '/health'", "url.pathname === '/connect'");
|
const healthBlock = sliceBetween(SERVER_SRC, "url.pathname === '/health'", "url.pathname === '/connect'");
|
||||||
// Token MUST be present in the localhost (!tunnelActive) branch
|
// Token must be conditional, not unconditional
|
||||||
expect(healthBlock).toContain('healthResponse.token = AUTH_TOKEN');
|
expect(healthBlock).toContain('AUTH_TOKEN');
|
||||||
// Token assignment must be inside the !tunnelActive guard
|
expect(healthBlock).toContain('headed');
|
||||||
const tokenIdx = healthBlock.indexOf('healthResponse.token = AUTH_TOKEN');
|
expect(healthBlock).toContain('chrome-extension://');
|
||||||
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');
|
|
||||||
});
|
});
|
||||||
|
|
||||||
// Test 1b: /health strips sensitive fields when tunneled
|
// Test 1b: /health does not expose sensitive browsing state
|
||||||
test('/health strips token, currentUrl, agent, session when tunnel is active', () => {
|
test('/health does not expose currentUrl or currentMessage', () => {
|
||||||
const healthBlock = sliceBetween(SERVER_SRC, "url.pathname === '/health'", "url.pathname === '/connect'");
|
const healthBlock = sliceBetween(SERVER_SRC, "url.pathname === '/health'", "url.pathname === '/connect'");
|
||||||
// currentUrl and agent.currentMessage must be gated on !tunnelActive
|
expect(healthBlock).not.toContain('currentUrl');
|
||||||
expect(healthBlock).toContain('!tunnelActive');
|
expect(healthBlock).not.toContain('currentMessage');
|
||||||
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');
|
|
||||||
});
|
});
|
||||||
|
|
||||||
// Test 1c: newtab must check domain restrictions (CSO finding #5)
|
// Test 1c: newtab must check domain restrictions (CSO finding #5)
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue