mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-02 03:35:09 +02:00
fix: chain scope bypass + /health info leak when tunneled
1. Chain command now pre-validates ALL subcommand scopes before executing any. A read+meta token can no longer escalate to admin via chain (eval, js, cookies were dispatched without scope checks). tokenInfo flows through handleMetaCommand into the chain handler. Rejects entire chain if any subcommand fails. 2. /health strips sensitive fields (currentUrl, agent.currentMessage, session) when tunnel is active. Only operational metadata (status, mode, uptime, tabs) exposed to the internet. Previously anyone reaching the ngrok URL could surveil browsing activity. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -7,6 +7,7 @@ import { handleSnapshot } from './snapshot';
|
||||
import { getCleanText } from './read-commands';
|
||||
import { READ_COMMANDS, WRITE_COMMANDS, META_COMMANDS, PAGE_CONTENT_COMMANDS, wrapUntrustedContent } from './commands';
|
||||
import { validateNavigationUrl } from './url-validation';
|
||||
import { checkScope, type TokenInfo } from './token-registry';
|
||||
import * as Diff from 'diff';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
@@ -48,7 +49,8 @@ export async function handleMetaCommand(
|
||||
command: string,
|
||||
args: string[],
|
||||
bm: BrowserManager,
|
||||
shutdown: () => Promise<void> | void
|
||||
shutdown: () => Promise<void> | void,
|
||||
tokenInfo?: TokenInfo | null
|
||||
): Promise<string> {
|
||||
switch (command) {
|
||||
// ─── Tabs ──────────────────────────────────────────
|
||||
@@ -232,6 +234,21 @@ export async function handleMetaCommand(
|
||||
const { handleReadCommand } = await import('./read-commands');
|
||||
const { handleWriteCommand } = await import('./write-commands');
|
||||
|
||||
// Pre-validate ALL subcommands against the token's scope before executing any.
|
||||
// This prevents partial execution where some subcommands succeed before a
|
||||
// scope violation is hit, leaving the browser in an inconsistent state.
|
||||
if (tokenInfo && tokenInfo.clientId !== 'root') {
|
||||
for (const cmd of commands) {
|
||||
const [name] = cmd;
|
||||
if (!checkScope(tokenInfo, name)) {
|
||||
throw new Error(
|
||||
`Chain rejected: subcommand "${name}" not allowed by your token scope (${tokenInfo.scopes.join(', ')}). ` +
|
||||
`All subcommands must be within scope.`
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let lastWasWrite = false;
|
||||
for (const cmd of commands) {
|
||||
const [name, ...cmdArgs] = cmd;
|
||||
@@ -247,7 +264,7 @@ export async function handleMetaCommand(
|
||||
}
|
||||
lastWasWrite = false;
|
||||
} else if (META_COMMANDS.has(name)) {
|
||||
result = await handleMetaCommand(name, cmdArgs, bm, shutdown);
|
||||
result = await handleMetaCommand(name, cmdArgs, bm, shutdown, tokenInfo);
|
||||
lastWasWrite = false;
|
||||
} else {
|
||||
throw new Error(`Unknown command: ${name}`);
|
||||
|
||||
+15
-13
@@ -947,7 +947,7 @@ async function handleCommand(body: any, tokenInfo?: TokenInfo | null): Promise<R
|
||||
} else if (WRITE_COMMANDS.has(command)) {
|
||||
result = await handleWriteCommand(command, args, browserManager);
|
||||
} else if (META_COMMANDS.has(command)) {
|
||||
result = await handleMetaCommand(command, args, browserManager, shutdown);
|
||||
result = await handleMetaCommand(command, args, browserManager, shutdown, tokenInfo);
|
||||
// Start periodic snapshot interval when watch mode begins
|
||||
if (command === 'watch' && args[0] !== 'stop' && browserManager.isWatching()) {
|
||||
const watchInterval = setInterval(async () => {
|
||||
@@ -1203,6 +1203,8 @@ 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<string, any> = {
|
||||
@@ -1210,22 +1212,22 @@ async function start() {
|
||||
mode: browserManager.getConnectionMode(),
|
||||
uptime: Math.floor((Date.now() - startTime) / 1000),
|
||||
tabs: browserManager.getTabCount(),
|
||||
currentUrl: browserManager.getCurrentUrl(),
|
||||
// Auth token NOT served here. Extension reads from ~/.gstack/.auth.json
|
||||
// (written by launchHeaded at browser-manager.ts:243). Serving the token
|
||||
// on an unauthenticated endpoint is unsafe because Origin headers are
|
||||
// trivially spoofable, and ngrok exposes /health to the internet.
|
||||
chatEnabled: true,
|
||||
agent: {
|
||||
};
|
||||
// Sensitive fields only served on localhost (not through tunnel).
|
||||
// currentUrl reveals internal URLs, currentMessage reveals user intent.
|
||||
if (!tunnelActive) {
|
||||
healthResponse.currentUrl = browserManager.getCurrentUrl();
|
||||
healthResponse.chatEnabled = true;
|
||||
healthResponse.agent = {
|
||||
status: agentStatus,
|
||||
runningFor: agentStartTime ? Date.now() - agentStartTime : null,
|
||||
currentMessage,
|
||||
queueLength: messageQueue.length,
|
||||
},
|
||||
session: sidebarSession ? { id: sidebarSession.id, name: sidebarSession.name } : null,
|
||||
};
|
||||
if (tunnelActive) {
|
||||
healthResponse.tunnel = { url: tunnelUrl, active: true };
|
||||
};
|
||||
healthResponse.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,
|
||||
|
||||
@@ -28,14 +28,19 @@ describe('Server auth security', () => {
|
||||
// Token must not appear in the health response construction
|
||||
expect(healthBlock).not.toContain('token: AUTH_TOKEN');
|
||||
expect(healthBlock).not.toContain('token: AUTH');
|
||||
// Should have a comment explaining why
|
||||
expect(healthBlock).toContain('NOT served here');
|
||||
// Should not expose browsing activity when tunneled
|
||||
expect(healthBlock).toContain('not through tunnel');
|
||||
});
|
||||
|
||||
// Test 1b: /health must not use chrome-extension Origin gating (spoofable)
|
||||
test('/health does not use spoofable Origin header for token gating', () => {
|
||||
// Test 1b: /health strips sensitive fields when tunneled
|
||||
test('/health strips currentUrl, agent, session when tunnel is active', () => {
|
||||
const healthBlock = sliceBetween(SERVER_SRC, "url.pathname === '/health'", "url.pathname === '/connect'");
|
||||
expect(healthBlock).not.toContain("chrome-extension://') ? { token");
|
||||
// currentUrl and agent.currentMessage must be gated on !tunnelActive
|
||||
expect(healthBlock).toContain('!tunnelActive');
|
||||
expect(healthBlock).toContain('currentUrl');
|
||||
expect(healthBlock).toContain('currentMessage');
|
||||
// Tunnel URL must NOT be exposed in health response
|
||||
expect(healthBlock).not.toContain('url: tunnelUrl');
|
||||
});
|
||||
|
||||
// Test 1c: newtab must check domain restrictions (CSO finding #5)
|
||||
@@ -139,7 +144,34 @@ describe('Server auth security', () => {
|
||||
expect(handleBlock).toContain('Tab not owned by your agent');
|
||||
});
|
||||
|
||||
// Test 10b: activity attribution includes clientId
|
||||
// Test 10b: chain command pre-validates subcommand scopes
|
||||
test('chain handler checks scope for each subcommand before dispatch', () => {
|
||||
const metaSrc = fs.readFileSync(path.join(import.meta.dir, '../src/meta-commands.ts'), 'utf-8');
|
||||
const chainBlock = metaSrc.slice(
|
||||
metaSrc.indexOf("case 'chain':"),
|
||||
metaSrc.indexOf("case 'diff':")
|
||||
);
|
||||
expect(chainBlock).toContain('checkScope');
|
||||
expect(chainBlock).toContain('Chain rejected');
|
||||
expect(chainBlock).toContain('tokenInfo');
|
||||
});
|
||||
|
||||
// Test 10c: handleMetaCommand accepts tokenInfo parameter
|
||||
test('handleMetaCommand accepts tokenInfo for chain scope checking', () => {
|
||||
const metaSrc = fs.readFileSync(path.join(import.meta.dir, '../src/meta-commands.ts'), 'utf-8');
|
||||
const sig = metaSrc.slice(
|
||||
metaSrc.indexOf('export async function handleMetaCommand'),
|
||||
metaSrc.indexOf('): Promise<string>')
|
||||
);
|
||||
expect(sig).toContain('tokenInfo');
|
||||
});
|
||||
|
||||
// Test 10d: server passes tokenInfo to handleMetaCommand
|
||||
test('server passes tokenInfo to handleMetaCommand', () => {
|
||||
expect(SERVER_SRC).toContain('handleMetaCommand(command, args, browserManager, shutdown, tokenInfo)');
|
||||
});
|
||||
|
||||
// Test 10e: activity attribution includes clientId
|
||||
test('activity events include clientId from token', () => {
|
||||
const commandStartBlock = sliceBetween(SERVER_SRC, "Activity: emit command_start", "try {");
|
||||
expect(commandStartBlock).toContain('clientId: tokenInfo?.clientId');
|
||||
|
||||
Reference in New Issue
Block a user