From 7372769dcee563653cdfd95c370626cf6d2922c7 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 20:34:05 +0000 Subject: [PATCH 01/17] D0: keep a project's .env and bunfig.toml out of compiled binaries and daemon children Compiled binaries are built with --no-compile-autoload-dotenv and --no-compile-autoload-bunfig; the browse server and terminal agent are spawned with --no-env-file and an empty config, so a repo's .env cannot set GSTACK_CHROMIUM_NO_SANDBOX and a bunfig preload cannot run code in them. Verified on Bun 1.4.0. --- browse/src/cli.ts | 4 +- browse/src/terminal-agent-control.ts | 9 +- browse/test/dotenv-isolation.test.ts | 120 +++++++++++++++++++++++++++ scripts/build.sh | 13 +-- 4 files changed, 138 insertions(+), 8 deletions(-) create mode 100644 browse/test/dotenv-isolation.test.ts diff --git a/browse/src/cli.ts b/browse/src/cli.ts index fe44feaf7..54a8c35c9 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -18,7 +18,7 @@ import { writeSecureFile, mkdirSecure } from './file-permissions'; import { resolveConfig, ensureStateDir, readVersionHash, isPairAgentEnabled, resolveChromiumProfile } from './config'; import { parseProxyConfig, computeConfigHash, ProxyConfigError } from './proxy-config'; import { redactProxyUrl } from './proxy-redact'; -import { spawnTerminalAgent } from './terminal-agent-control'; +import { spawnTerminalAgent, BUN_CHILD_FLAGS } from './terminal-agent-control'; // Zero side effects on import (documented invariant in token-registry.ts) — // safe to pull the shared pairing default into the CLI. import { DEFAULT_PAIR_SCOPES } from './token-registry'; @@ -658,7 +658,7 @@ async function startServer(extraEnv?: Record): Promise { + const deadline = Date.now() + 20_000; + while (!fs.existsSync(file) && Date.now() < deadline) await Bun.sleep(50); + return fs.readFileSync(file, 'utf-8'); +} + +beforeEach(() => { + const scratch = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-dotenv-')); + project = path.join(scratch, 'project'); + marker = path.join(scratch, 'preload-ran'); + seedHostileProject(project); +}); + +afterEach(() => { + fs.rmSync(path.dirname(project), { recursive: true, force: true }); +}); + +describe('D0: project .env and bunfig.toml are ignored', () => { + test('every compiled binary in scripts/build.sh turns dotenv and bunfig autoload off, and the flags work', () => { + const lines = fs.readFileSync(path.join(ROOT, 'scripts', 'build.sh'), 'utf-8').split('\n').filter(l => /build --compile/.test(l)); + expect(lines.length).toBeGreaterThanOrEqual(5); + for (const line of lines) { + expect(line).toContain('--no-compile-autoload-dotenv'); + expect(line).toContain('--no-compile-autoload-bunfig'); + } + const out = path.join(path.dirname(project), 'probe.json'); + const src = path.join(path.dirname(project), 'probe.ts'); + const bin = path.join(path.dirname(project), 'probe-bin'); + fs.writeFileSync(src, probeSource(out)); + const build = Bun.spawnSync([process.execPath, 'build', '--compile', '--no-compile-autoload-dotenv', '--no-compile-autoload-bunfig', src, '--outfile', bin], { stdout: 'pipe', stderr: 'pipe', timeout: 60_000 }); + expect(build.exitCode, build.stderr.toString()).toBe(0); + const run = Bun.spawnSync([bin], { cwd: project, env: { PATH: process.env.PATH ?? '' }, stdout: 'pipe', stderr: 'pipe', timeout: 30_000 }); + expect(run.exitCode, run.stderr.toString()).toBe(0); + expect(JSON.parse(fs.readFileSync(out, 'utf-8'))).toEqual({ sandbox: null }); + expect(fs.existsSync(marker)).toBe(false); + }, 90_000); + + test('the terminal agent spawned in the project sees neither the .env value nor the bunfig preload', async () => { + const stateDir = path.join(path.dirname(project), 'state'); + fs.mkdirSync(stateDir, { recursive: true }); + const out = path.join(stateDir, 'agent-env.json'); + const script = path.join(stateDir, 'agent-probe.ts'); + fs.writeFileSync(script, probeSource(out)); + const saved = process.env.GSTACK_CHROMIUM_NO_SANDBOX; + delete process.env.GSTACK_CHROMIUM_NO_SANDBOX; + try { + const pid = spawnTerminalAgent({ stateFile: path.join(stateDir, 'browse.json'), serverPort: 1, ownerPid: process.pid, cwd: project, scriptPath: script }); + expect(pid).toBeGreaterThan(0); + expect(JSON.parse(await waitForFile(out))).toEqual({ sandbox: null }); + expect(fs.existsSync(marker)).toBe(false); + } finally { + if (saved !== undefined) process.env.GSTACK_CHROMIUM_NO_SANDBOX = saved; + } + }, 30_000); + + test.skipIf(process.platform === 'win32')('the browse server spawned from the project sees neither the .env value nor the bunfig preload', async () => { + const stateFile = path.join(project, '.gstack', 'browse.json'); + const stub = path.join(path.dirname(project), 'stub-server.ts'); + fs.writeFileSync(stub, ` + import * as fs from 'node:fs'; + const server = Bun.serve({ hostname: '127.0.0.1', port: 0, fetch: () => Response.json({ status: 'healthy' }) }); + fs.writeFileSync(process.env.BROWSE_STATE_FILE!, JSON.stringify({ + pid: process.pid, port: server.port, token: 'dotenv-test', mode: 'launched', + sandbox: process.env.GSTACK_CHROMIUM_NO_SANDBOX ?? null, + })); + setTimeout(() => process.exit(0), 5000); + `); + const env: Record = {}; + for (const [key, value] of Object.entries(process.env)) { + if (value !== undefined && !/^(BROWSE_|GSTACK_|CHROMIUM_PROFILE$|CLAUDE_PLUGIN_DATA$)/.test(key)) env[key] = value; + } + Object.assign(env, { + HOME: path.dirname(project), + GSTACK_HOME: path.join(path.dirname(project), '.gstack'), + BROWSE_STATE_FILE: stateFile, + BROWSE_SERVER_SCRIPT: stub, + BROWSE_PARENT_PID: '0', + BROWSE_START_TIMEOUT: '20000', + }); + // The shipped CLI is compiled without dotenv autoload; --no-env-file gives + // the source CLI the same starting environment. + const cli = Bun.spawn([process.execPath, '--no-env-file', '--config=/dev/null', CLI, 'status'], { cwd: project, env, stdout: 'pipe', stderr: 'pipe' }); + const timer = setTimeout(() => cli.kill('SIGKILL'), 40_000); + const [code, stderr] = await Promise.all([cli.exited, new Response(cli.stderr).text()]); + clearTimeout(timer); + const state = JSON.parse(fs.readFileSync(stateFile, 'utf-8')); + try { process.kill(state.pid, 'SIGKILL'); } catch {} + expect(code, stderr).toBe(0); + expect(state.sandbox).toBeNull(); + expect(fs.existsSync(marker)).toBe(false); + }, 60_000); +}); diff --git a/scripts/build.sh b/scripts/build.sh index 559e5d807..bd34f55d3 100755 --- a/scripts/build.sh +++ b/scripts/build.sh @@ -30,13 +30,16 @@ case "$(uname -s)" in ;; esac +# A project's .env or bunfig.toml must never reach the shipped binaries: they run +# inside untrusted repos, and dotenv could set security switches such as +# GSTACK_CHROMIUM_NO_SANDBOX while a bunfig preload runs arbitrary code (D0). "$BUN_CMD" run vendor:xterm "$BUN_CMD" run gen:skill-docs --host all -"$BUN_CMD" build --compile browse/src/cli.ts --outfile browse/dist/browse -"$BUN_CMD" build --compile browse/src/find-browse.ts --outfile browse/dist/find-browse -"$BUN_CMD" build --compile design/src/cli.ts --outfile design/dist/design -"$BUN_CMD" build --compile make-pdf/src/cli.ts --outfile make-pdf/dist/pdf -"$BUN_CMD" build --compile bin/gstack-global-discover.ts --outfile bin/gstack-global-discover +"$BUN_CMD" build --compile --no-compile-autoload-dotenv --no-compile-autoload-bunfig browse/src/cli.ts --outfile browse/dist/browse +"$BUN_CMD" build --compile --no-compile-autoload-dotenv --no-compile-autoload-bunfig browse/src/find-browse.ts --outfile browse/dist/find-browse +"$BUN_CMD" build --compile --no-compile-autoload-dotenv --no-compile-autoload-bunfig design/src/cli.ts --outfile design/dist/design +"$BUN_CMD" build --compile --no-compile-autoload-dotenv --no-compile-autoload-bunfig make-pdf/src/cli.ts --outfile make-pdf/dist/pdf +"$BUN_CMD" build --compile --no-compile-autoload-dotenv --no-compile-autoload-bunfig bin/gstack-global-discover.ts --outfile bin/gstack-global-discover if [ "${GSTACK_SETUP_RUNNING:-0}" = "1" ] && [ "${GSTACK_SETUP_SKIP_CSO_BUILD:-0}" = "1" ]; then # Setup removes these before invoking us too. Repeat here so the setup-private # escape hatch can never publish a completion stamp beside stale trusted code. From 85e8a7aaf5ea731d87edd220df53cda506e39e6b Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 20:37:03 +0000 Subject: [PATCH 02/17] D1: pin the terminal-agent /ws Origin to the gstack extension (#1324) GSTACK_EXTENSION_ID moves to browse/src/extension-id.ts, shared by /extension-token and the terminal agent's /ws gate. /ws now returns 403 for every origin except the pinned extension. Forks override the ID with gstack-config set browse_extension_id , honored by both routes; the BROWSE_EXTENSION_ID env var is no longer read. Test sites that used fake extension IDs now use the pinned origin. --- browse/scripts/extension-id.ts | 2 +- browse/src/extension-id.ts | 25 ++++++++++ browse/src/server.ts | 15 +++--- browse/src/terminal-agent.ts | 13 ++--- browse/test/extension-token.test.ts | 20 +++++++- .../test/terminal-agent-integration.test.ts | 48 +++++++++++++++---- browse/test/terminal-agent.test.ts | 2 +- 7 files changed, 95 insertions(+), 30 deletions(-) create mode 100644 browse/src/extension-id.ts diff --git a/browse/scripts/extension-id.ts b/browse/scripts/extension-id.ts index cc9a7ba7f..2e6490665 100755 --- a/browse/scripts/extension-id.ts +++ b/browse/scripts/extension-id.ts @@ -8,7 +8,7 @@ * 0-9a-f to a-p (the "mpdecimal" alphabet). Pinning the public key in the * manifest pins the ID, which lets the browse server verify the Origin * header on POST /extension-token against a single known extension - * identity (GSTACK_EXTENSION_ID in browse/src/server.ts). + * identity (GSTACK_EXTENSION_ID in browse/src/extension-id.ts). * * The private half of the keypair is intentionally NOT in the repo — the * extension is loaded unpacked (or baked into Browser.app), so only the diff --git a/browse/src/extension-id.ts b/browse/src/extension-id.ts new file mode 100644 index 000000000..e921797c7 --- /dev/null +++ b/browse/src/extension-id.ts @@ -0,0 +1,25 @@ +/** + * The one extension origin allowed to reach browse's local control surfaces: + * POST /extension-token on the server and the terminal agent's /ws. + * + * GSTACK_EXTENSION_ID is derived from the "key" field in + * extension/manifest.json (first 16 bytes of SHA-256 of the DER public key, + * hex nibbles mapped 0-9a-f → a-p). Reproduce with: + * bun browse/scripts/extension-id.ts + * If the manifest keypair is ever rotated, update this constant in the same + * commit. + * + * Forks and self-built extensions override it with + * `gstack-config set browse_extension_id `. There is deliberately no + * environment override: a project's .env must never be able to widen it. + */ +import { readGstackConfigYamlKey } from './config'; + +export const GSTACK_EXTENSION_ID = 'dgbkdbjebeiblbajiilljmhjdpmiglep'; + +/** `chrome-extension://` for the pinned or configured extension. */ +export function allowedExtensionOrigin(): string { + const configured = readGstackConfigYamlKey('browse_extension_id'); + const id = configured && /^[a-p]{32}$/.test(configured) ? configured : GSTACK_EXTENSION_ID; + return `chrome-extension://${id}`; +} diff --git a/browse/src/server.ts b/browse/src/server.ts index b86bb7e69..57bfb63ca 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -46,6 +46,7 @@ import { } from './port-allocator'; import { acquireAgentStateLock, readAgentRecord, clearAgentRecord, isOurAgent, isAgentRecordLive, isAgentRecordGone, stopAgentByRecord, spawnTerminalAgent } from './terminal-agent-control'; import { isProcessAlive } from './error-handling'; +import { allowedExtensionOrigin } from './extension-id'; import { sanitizeBody, stripLoneSurrogates } from './sanitize'; import { startSocksBridge, testUpstream, type BridgeHandle } from './socks-bridge'; import { parseProxyConfig, toUpstreamConfig, ProxyConfigError } from './proxy-config'; @@ -296,15 +297,11 @@ const TUNNEL_PATHS = new Set([ ]); /** - * The gstack sidebar extension's pinned Chrome extension ID. Derived from - * the "key" field in extension/manifest.json (first 16 bytes of SHA-256 of - * the DER public key, hex nibbles mapped 0-9a-f → a-p). Reproduce with: - * bun browse/scripts/extension-id.ts - * POST /extension-token releases AUTH_TOKEN only to an Origin of exactly - * `chrome-extension://`. If the manifest keypair is ever rotated, - * this constant must be updated in the same commit. + * POST /extension-token releases AUTH_TOKEN only to an Origin of exactly the + * pinned extension, or the one set with `gstack-config set browse_extension_id` + * (browse/src/extension-id.ts, shared with the terminal agent's /ws gate). */ -export const GSTACK_EXTENSION_ID = 'dgbkdbjebeiblbajiilljmhjdpmiglep'; +export { GSTACK_EXTENSION_ID } from './extension-id'; /** * The extension-origin auth check (POST /extension-token): Origin is exactly @@ -320,7 +317,7 @@ function isPinnedExtensionRequest(req: Request): boolean { } catch (err) { if (!(err instanceof TypeError)) throw err; // TypeError = malformed Host } - const originOk = req.headers.get('origin') === `chrome-extension://${GSTACK_EXTENSION_ID}`; + const originOk = req.headers.get('origin') === allowedExtensionOrigin(); const hostOk = hostname === '127.0.0.1' || hostname === 'localhost'; return originOk && hostOk; } diff --git a/browse/src/terminal-agent.ts b/browse/src/terminal-agent.ts index 88e89a719..7ce237a3e 100644 --- a/browse/src/terminal-agent.ts +++ b/browse/src/terminal-agent.ts @@ -29,6 +29,7 @@ import { safeUnlink } from './error-handling'; import { writeAgentRecord, readAgentRecord, clearAgentRecord, readAgentStartTime, acquireAgentStateLock } from './terminal-agent-control'; import { findAvailablePort } from './port-allocator'; import { extractPtyCookie } from './pty-session-cookie'; +import { allowedExtensionOrigin } from './extension-id'; import { createPtyLifecycle, disposePtyProcess, ptyCompletionReason, type PtyCompletion, type PtyLifecycle, @@ -43,7 +44,6 @@ const OWNER_WATCHDOG_MS = parseInt( process.env.GSTACK_TERMINAL_OWNER_WATCHDOG_MS || '15000', 10, ); -const EXTENSION_ID = process.env.BROWSE_EXTENSION_ID || ''; // optional: tighten Origin check const INTERNAL_TOKEN = crypto.randomBytes(32).toString('base64url'); // shared with parent server via env at spawn /** * Per-boot generation identifier. Loopback /internal/* callers include @@ -620,8 +620,8 @@ function buildServer(port: number) { } // /ws — WebSocket upgrade. CRITICAL gates: - // (1) Origin must be chrome-extension://. Cross-site WS hijacking - // defense — required, not optional. + // (1) Origin must be exactly the pinned (or gstack-config configured) + // extension. Any other origin, other extensions included, is 403. // (2) Token must be in validTokens. We accept the token via two // transports for compatibility: // - Sec-WebSocket-Protocol (preferred for browsers — the only @@ -634,12 +634,7 @@ function buildServer(port: number) { // validTokens Set, populated by the parent server's // authenticated /pty-session → /internal/grant chain. if (url.pathname === '/ws') { - const origin = req.headers.get('origin') || ''; - const isExtensionOrigin = origin.startsWith('chrome-extension://'); - if (!isExtensionOrigin) { - return new Response('forbidden origin', { status: 403 }); - } - if (EXTENSION_ID && origin !== `chrome-extension://${EXTENSION_ID}`) { + if (req.headers.get('origin') !== allowedExtensionOrigin()) { return new Response('forbidden origin', { status: 403 }); } diff --git a/browse/test/extension-token.test.ts b/browse/test/extension-token.test.ts index cc66d6d80..87dcffd96 100644 --- a/browse/test/extension-token.test.ts +++ b/browse/test/extension-token.test.ts @@ -31,7 +31,7 @@ import { BrowserManager } from '../src/browser-manager'; import { resolveConfig } from '../src/config'; import { usePrivateStateRoot } from '../../test/helpers/private-state-root'; -usePrivateStateRoot(); +const privateRoot = usePrivateStateRoot(); const PINNED_ORIGIN = `chrome-extension://${GSTACK_EXTENSION_ID}`; const fixtureDir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-extension-token-'))); @@ -168,6 +168,24 @@ describe('POST /extension-token pinned-origin bootstrap', () => { expect(JSON.stringify(body)).not.toContain('host'); }); + test('gstack-config browse_extension_id moves the token to the configured extension; the env var does not', async () => { + const fork = 'b'.repeat(32); + const cfg = makeConfig(); + const handle = buildFetchHandler(cfg); + const status = async (origin: string) => (await handle.fetchLocal(tokenRequest({ Origin: origin, Host: '127.0.0.1:34567' }), null)).status; + const saved = process.env.BROWSE_EXTENSION_ID; + process.env.BROWSE_EXTENSION_ID = fork; + try { + expect(await status(`chrome-extension://${fork}`)).toBe(403); + fs.writeFileSync(path.join(privateRoot.dir, 'config.yaml'), `browse_extension_id: ${fork}\n`); + expect(await status(`chrome-extension://${fork}`)).toBe(200); + expect(await status(PINNED_ORIGIN)).toBe(403); + } finally { + if (saved === undefined) delete process.env.BROWSE_EXTENSION_ID; + else process.env.BROWSE_EXTENSION_ID = saved; + } + }); + test('missing Origin → 403', async () => { const handle = buildFetchHandler(makeConfig()); const resp = await handle.fetchLocal(tokenRequest({ diff --git a/browse/test/terminal-agent-integration.test.ts b/browse/test/terminal-agent-integration.test.ts index f45b381fd..5b80cd057 100644 --- a/browse/test/terminal-agent-integration.test.ts +++ b/browse/test/terminal-agent-integration.test.ts @@ -18,9 +18,13 @@ import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; +import { GSTACK_EXTENSION_ID } from '../src/extension-id'; const AGENT_SCRIPT = path.join(import.meta.dir, '../src/terminal-agent.ts'); const BASH = '/bin/bash'; +const EXT_ORIGIN = `chrome-extension://${GSTACK_EXTENSION_ID}`; +const ENV_EXTENSION_ID = 'a'.repeat(32); +const FORK_EXTENSION_ID = 'b'.repeat(32); let stateDir: string; let agentProc: any; @@ -73,6 +77,8 @@ beforeAll(() => { BROWSE_STATE_FILE: stateFile, BROWSE_SERVER_PORT: '0', // not used in this test BROWSE_TERMINAL_BINARY: terminalCli, + GSTACK_STATE_ROOT: stateDir, + BROWSE_EXTENSION_ID: ENV_EXTENSION_ID, }, stdio: ['ignore', 'pipe', 'pipe'], }); @@ -128,7 +134,7 @@ describe('terminal-agent: /internal/grant and /internal/revoke bearer auth', () function wsStatus(token: string): Promise { return fetch(`http://127.0.0.1:${agentPort}/ws`, { - headers: { 'Origin': 'chrome-extension://abc123', 'Cookie': `gstack_pty=${token}` }, + headers: { 'Origin': EXT_ORIGIN, 'Cookie': `gstack_pty=${token}` }, }).then((r) => r.status); } @@ -173,10 +179,34 @@ describe('terminal-agent: /ws gates', () => { expect(resp.status).toBe(403); }); + function wsOriginStatus(origin: string): Promise { + return fetch(`http://127.0.0.1:${agentPort}/ws`, { + headers: { Origin: origin, Cookie: 'gstack_pty=never-granted' }, + }).then((r) => r.status); + } + + test('rejects every extension Origin except the pinned one, even BROWSE_EXTENSION_ID from the environment (#1324)', async () => { + expect(await wsOriginStatus('chrome-extension://abc123')).toBe(403); + expect(await wsOriginStatus(`chrome-extension://${ENV_EXTENSION_ID}`)).toBe(403); + expect(await wsOriginStatus(EXT_ORIGIN)).toBe(401); + }); + + test('gstack-config browse_extension_id replaces the pinned ID for forks', async () => { + const config = path.join(stateDir, 'config.yaml'); + fs.writeFileSync(config, `browse_extension_id: ${FORK_EXTENSION_ID}\n`); + try { + expect(await wsOriginStatus(`chrome-extension://${FORK_EXTENSION_ID}`)).toBe(401); + expect(await wsOriginStatus(EXT_ORIGIN)).toBe(403); + } finally { + fs.rmSync(config, { force: true }); + } + expect(await wsOriginStatus(EXT_ORIGIN)).toBe(401); + }); + test('rejects extension-Origin upgrades without a granted cookie', async () => { const resp = await fetch(`http://127.0.0.1:${agentPort}/ws`, { headers: { - 'Origin': 'chrome-extension://abc123', + 'Origin': EXT_ORIGIN, 'Cookie': 'gstack_pty=never-granted', }, }); @@ -192,7 +222,7 @@ describe('terminal-agent: PTY round-trip via real WebSocket (Cookie auth)', () = const ws = new WebSocket(`ws://127.0.0.1:${agentPort}/ws`, { headers: { - 'Origin': 'chrome-extension://test-extension-id', + 'Origin': EXT_ORIGIN, 'Cookie': `gstack_pty=${cookie}`, }, } as any); @@ -274,7 +304,7 @@ describe('terminal-agent: PTY round-trip via real WebSocket (Cookie auth)', () = 'Sec-WebSocket-Version': '13', 'Sec-WebSocket-Key': handshakeKey, 'Sec-WebSocket-Protocol': `gstack-pty.${token}`, - 'Origin': 'chrome-extension://test-extension-id', + 'Origin': EXT_ORIGIN, }, }); @@ -307,7 +337,7 @@ describe('terminal-agent: PTY round-trip via real WebSocket (Cookie auth)', () = 'Sec-WebSocket-Version: 13\r\n' + 'Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n' + `Sec-WebSocket-Protocol: gstack-pty.${token}\r\n` + - 'Origin: chrome-extension://test-extension-id\r\n' + + `Origin: ${EXT_ORIGIN}\r\n` + '\r\n'; let buf = ''; const socket = require('net').connect(agentPort, '127.0.0.1', () => socket.write(req)); @@ -333,7 +363,7 @@ describe('terminal-agent: PTY round-trip via real WebSocket (Cookie auth)', () = 'Sec-WebSocket-Version': '13', 'Sec-WebSocket-Key': 'dGhlIHNhbXBsZSBub25jZQ==', 'Sec-WebSocket-Protocol': 'gstack-pty.never-granted-token', - 'Origin': 'chrome-extension://test-extension-id', + 'Origin': EXT_ORIGIN, }, }); expect(resp.status).toBe(401); @@ -345,7 +375,7 @@ describe('terminal-agent: PTY round-trip via real WebSocket (Cookie auth)', () = const ws = new WebSocket(`ws://127.0.0.1:${agentPort}/ws`, { headers: { - 'Origin': 'chrome-extension://test-extension-id', + 'Origin': EXT_ORIGIN, 'Cookie': `gstack_pty=${cookie}`, }, } as any); @@ -393,7 +423,7 @@ describe('terminal-agent: owned PTY completion and restart', () => { const granted = await internal('grant', { token, sessionId }); expect(granted.status).toBe(200); const ws = new WebSocket(`ws://127.0.0.1:${agentPort}/ws`, { - headers: { Origin: 'chrome-extension://test-extension-id', Cookie: `gstack_pty=${token}` }, + headers: { Origin: EXT_ORIGIN, Cookie: `gstack_pty=${token}` }, } as any); const events: Array<{ type: string; [key: string]: any }> = []; let output = ''; @@ -472,7 +502,7 @@ describe('terminal-agent: owned PTY completion and restart', () => { expect(old.closed()).toBe(1000); expect(replacement.closed()).toBeNull(); const revoked = await fetch(`http://127.0.0.1:${agentPort}/ws`, { - headers: { Origin: 'chrome-extension://test-extension-id', Cookie: `gstack_pty=${oldToken}` }, + headers: { Origin: EXT_ORIGIN, Cookie: `gstack_pty=${oldToken}` }, }); expect(revoked.status).toBe(401); diff --git a/browse/test/terminal-agent.test.ts b/browse/test/terminal-agent.test.ts index c4ef37bc3..f066d1a23 100644 --- a/browse/test/terminal-agent.test.ts +++ b/browse/test/terminal-agent.test.ts @@ -117,7 +117,7 @@ describe('Source-level guard: terminal-agent', () => { // missing-origin attempt would surface the 401 cookie message and // signal to attackers that they need to forge a cookie. const wsHandler = AGENT_SRC.slice(AGENT_SRC.indexOf("if (url.pathname === '/ws')")); - expect(wsHandler).toContain('chrome-extension://'); + expect(wsHandler).toContain('allowedExtensionOrigin()'); expect(wsHandler).toContain('forbidden origin'); }); From b34cc829ed390f2384c407f1b5efd83ddbcd28c0 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 20:46:00 +0000 Subject: [PATCH 03/17] D2: one address classifier for navigation literals, DNS answers, redirects and page-driven navigations (#2811) classifyAddress() in url-validation.ts blocks 169.254.0.0/16 (metadata and container credential endpoints), 100.100.100.200, fe80::/10 and fc00::/7, including IPv4-mapped, IPv4-compatible and NAT64 spellings and decimal, octal and hex IPv4. validateNavigationUrl, the A/AAAA answer check and session-persist's cookie-domain guard all use it. BrowserManager now checks every navigation request (redirect hops, link clicks, scripted navigations): a blocked target resets the tab to about:blank and the write command fails with the reason. Connection-time DNS rebinding remains out of scope. Co-authored-by: Gabriel Rondon --- browse/src/browser-manager.ts | 43 +++++- browse/src/server.ts | 2 +- browse/src/session-persist.ts | 14 +- browse/src/url-validation.ts | 194 +++++++++++++-------------- browse/test/navigation-guard.test.ts | 75 +++++++++++ browse/test/path-validation.test.ts | 8 +- browse/test/url-validation.test.ts | 65 ++++++++- 7 files changed, 282 insertions(+), 119 deletions(-) create mode 100644 browse/test/navigation-guard.test.ts diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index 08600f31c..1f619218e 100644 --- a/browse/src/browser-manager.ts +++ b/browse/src/browser-manager.ts @@ -20,7 +20,7 @@ import { chromium, type Browser, type BrowserContext, type BrowserContextOptions import { writeSecureFile, mkdirSecure } from './file-permissions'; import { addConsoleEntry, addNetworkEntry, addDialogEntry, networkBuffer, type DialogEntry } from './buffers'; import { emitActivity } from './activity'; -import { validateNavigationUrl } from './url-validation'; +import { validateNavigationUrl, blockedNavigationReason } from './url-validation'; import { TabSession, type RefEntry } from './tab-session'; import { resolveChromiumProfile, cleanSingletonLocks } from './config'; import { launchWithXProtectHeal } from './xprotect-heal'; @@ -318,6 +318,8 @@ export class BrowserManager { // ─── Tab Ownership (multi-agent isolation) ────────────── // Maps tabId → clientId. Unowned tabs (not in this map) are root-only for writes. private tabOwnership: Map = new Map(); + /** Pending and recorded navigation-guard verdicts per page (D2). */ + private navigationGuards = new WeakMap>; blocked: string | null }>(); // ─── Dialog Handling (global, not per-tab) ────────────────── private dialogAutoAccept: boolean = true; @@ -2024,10 +2026,49 @@ export class BrowserManager { return null; } + /** + * Run a command and surface any navigation the guard blocked while it ran. + * The block reason replaces the command's own result or error (often + * "navigation interrupted"), so the caller learns why the tab is blank. + */ + async failIfNavigationBlocked(page: Page, work: Promise): Promise { + const outcome = await work.then((value) => ({ value }), (error) => ({ error })); + const guard = this.navigationGuards.get(page); + while (guard && guard.pending.size > 0) await Promise.all([...guard.pending]); + const blocked = guard?.blocked ?? null; + if (guard) guard.blocked = null; + if (blocked) throw new Error(blocked); + if ('error' in outcome) throw outcome.error; + return outcome.value; + } + + /** + * D2: validateNavigationUrl only sees explicit navigations. Redirect hops and + * page-driven navigations (links, scripts, forms, frames) arrive here as + * navigation requests; one that targets a blocked address blanks the tab and + * is reported by failIfNavigationBlocked. + */ + private guardNavigations(page: Page): void { + if (this.navigationGuards.has(page)) return; + const guard = { pending: new Set>(), blocked: null as string | null }; + this.navigationGuards.set(page, guard); + page.on('request', (req) => { + if (!req.isNavigationRequest()) return; + const check: Promise = blockedNavigationReason(req.url()).then(async (reason) => { + if (!reason || page.isClosed()) return; + guard.blocked = reason; + console.warn(`[browse] ${reason} (navigation from ${req.frame().url() || 'a new page'}; tab reset to about:blank)`); + await page.goto('about:blank').catch(() => {}); + }).finally(() => { guard.pending.delete(check); }); + guard.pending.add(check); + }); + } + // ─── Console/Network/Dialog/Ref Wiring ──────────────────── private wirePageEvents(page: Page) { const pages = this.pages; const tabSessions = this.tabSessions; + this.guardNavigations(page); // Track tab close — remove from pages and sessions maps, switch to another tab page.on('close', () => { for (const [id, p] of pages) { diff --git a/browse/src/server.ts b/browse/src/server.ts index 57bfb63ca..6810dc818 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -1162,7 +1162,7 @@ async function handleCommandInternalImpl( result = await handleReadCommand(command, args, session, browserManager); } } else if (WRITE_COMMANDS.has(command)) { - result = await handleWriteCommand(command, args, session, browserManager); + result = await browserManager.failIfNavigationBlocked(session.getPage(), handleWriteCommand(command, args, session, browserManager)); } else if (META_COMMANDS.has(command)) { // Pass chain depth + executeCommand callback so chain routes subcommands // through the full security pipeline (scope, domain, tab, wrapping). diff --git a/browse/src/session-persist.ts b/browse/src/session-persist.ts index 3c6627bcb..6259eca2d 100644 --- a/browse/src/session-persist.ts +++ b/browse/src/session-persist.ts @@ -24,6 +24,7 @@ import * as fs from 'fs'; import type { BrowserManager, BrowserState } from './browser-manager'; import { writeSecureFile } from './file-permissions'; import { safeUnlinkQuiet } from './error-handling'; +import { classifyAddress } from './url-validation'; /** Rename a corrupt state file to .corrupt (forensic artifact) — best effort. */ function quarantineCorrupt(filePath: string): void { @@ -68,9 +69,10 @@ export function serializeSessionState(state: BrowserState): string { /** * True when a cookie domain points at an internal-network target a tampered * state file could use to reach localhost services, *.internal hosts, or - * cloud metadata: `localhost`, `*.internal`, IPv4 loopback literals - * (127.0.0.0/8), IPv6 loopback (`::1`, `[::1]`), and link-local/metadata - * (169.254.0.0/16, which covers 169.254.169.254). Leading-dot domain + * cloud metadata: `localhost`, `*.internal`, and any address literal that + * url-validation's classifyAddress calls loopback or blocked (127.0.0.0/8, + * ::1, 169.254.0.0/16, 100.100.100.200, fe80::/10, fc00::/7, and their + * mapped and numeric spellings). Leading-dot domain * variants (`.127.0.0.1`) are normalized before matching. Single source of * truth for the persistence restore path here AND `state load` * (meta-commands.ts). @@ -78,10 +80,8 @@ export function serializeSessionState(state: BrowserState): string { export function isInternalCookieDomain(domain: string): boolean { const d = domain.startsWith('.') ? domain.slice(1) : domain; if (d === 'localhost' || d.endsWith('.internal')) return true; - if (d === '::1' || d === '[::1]') return true; // IPv6 loopback - if (/^127\./.test(d)) return true; // IPv4 loopback block - if (/^169\.254\./.test(d)) return true; // link-local incl. cloud metadata - return false; + const kind = classifyAddress(d); + return kind === 'blocked' || kind === 'loopback'; } /** diff --git a/browse/src/url-validation.ts b/browse/src/url-validation.ts index c9e9c170a..717de6de0 100644 --- a/browse/src/url-validation.ts +++ b/browse/src/url-validation.ts @@ -8,107 +8,109 @@ import * as path from 'node:path'; import * as os from 'node:os'; import { validateReadPath } from './path-security'; -export const BLOCKED_METADATA_HOSTS = new Set([ - '169.254.169.254', // AWS/GCP/Azure instance metadata - 'fe80::1', // IPv6 link-local — common metadata endpoint alias - '::ffff:169.254.169.254', // IPv4-mapped IPv6 form of the metadata IP - '::ffff:a9fe:a9fe', // Hex-encoded IPv4-mapped form (URL constructor normalizes to this) - '::a9fe:a9fe', // Deprecated IPv4-compatible hex form +/** Metadata services reached by name rather than address. */ +const BLOCKED_METADATA_HOSTNAMES = new Set([ 'metadata.google.internal', // GCP metadata 'metadata.azure.internal', // Azure IMDS ]); /** - * IPv6 prefixes to block (CIDR-style). ULA addresses cover fc00::/7 and - * link-local addresses cover fe80::/10. - */ -const BLOCKED_IPV6_PREFIXES = ['fc', 'fd', 'fe8', 'fe9', 'fea', 'feb']; - -/** - * Check if an IPv6 address falls within a blocked prefix range. - * Handles the full ULA range (fc00::/7) and link-local range (fe80::/10), - * not just exact literals like fd00:: or fe80::1. - * Only matches actual IPv6 addresses (must contain ':'), not hostnames - * like fd.example.com or fcustomer.com. - */ -function isBlockedIpv6(addr: string): boolean { - const normalized = addr.toLowerCase().replace(/^\[|\]$/g, ''); - // Must contain a colon to be an IPv6 address — avoids false positives on - // hostnames like fd.example.com or fcustomer.com - if (!normalized.includes(':')) return false; - return BLOCKED_IPV6_PREFIXES.some(prefix => normalized.startsWith(prefix)); -} - -/** - * Normalize hostname for blocklist comparison: - * - Strip trailing dot (DNS fully-qualified notation) - * - Strip IPv6 brackets (URL.hostname includes [] for IPv6) - * - Resolve hex (0xA9FEA9FE) and decimal (2852039166) IP representations - */ -function normalizeHostname(hostname: string): string { - // Strip IPv6 brackets - let h = hostname.startsWith('[') && hostname.endsWith(']') - ? hostname.slice(1, -1) - : hostname; - // Strip trailing dot - if (h.endsWith('.')) h = h.slice(0, -1); - return h; -} - -/** - * Check if a hostname resolves to the link-local metadata IP 169.254.169.254. - * Catches hex (0xA9FEA9FE), decimal (2852039166), and octal (0251.0376.0251.0376) forms. - */ -function isMetadataIp(hostname: string): boolean { - // Try to parse as a numeric IP via URL constructor — it normalizes all forms - try { - const probe = new URL(`http://${hostname}`); - const normalized = probe.hostname; - if (BLOCKED_METADATA_HOSTS.has(normalized) || isBlockedIpv6(normalized)) return true; - // Also check after stripping trailing dot - if (normalized.endsWith('.') && BLOCKED_METADATA_HOSTS.has(normalized.slice(0, -1))) return true; - } catch { - // Not a valid hostname — can't be a metadata IP - } - return false; -} - -/** - * Resolve a hostname to its IP addresses and check if any resolve to blocked metadata IPs. - * Mitigates DNS rebinding: even if the hostname looks safe, the resolved IP might not be. + * Classify one host literal (URL hostname, cookie domain, or a resolved A/AAAA + * answer). Returns null when the host is not an IP literal. Single source of + * truth for navigation (literal, DNS answers, redirects, page-driven + * navigations) and for session-persist's cookie-domain guard. * - * Checks both A (IPv4) and AAAA (IPv6) records — an attacker can use AAAA-only DNS to - * bypass IPv4-only checks. Each record family is tried independently; failure of one - * (e.g. no AAAA records exist) is not treated as a rebinding risk. + * - 'blocked': IPv4 link-local 169.254.0.0/16 (cloud metadata and container + * credential services such as 169.254.170.2), Alibaba's 100.100.100.200, + * IPv6 link-local fe80::/10 and ULA fc00::/7, and any of these embedded in + * IPv4-mapped (::ffff:), IPv4-compatible (::) or NAT64 (64:ff9b::) IPv6. + * - 'loopback': 127.0.0.0/8 and ::1. + * - 'other': every other address (RFC 1918 dev servers stay allowed). + * + * The URL parser canonicalizes decimal, octal and hex IPv4 forms + * (2852039166, 0251.0376.0251.0376, 0xA9FEA9FE) and IPv6 spellings first. + */ +export function classifyAddress(host: string): 'blocked' | 'loopback' | 'other' | null { + let h = host.trim().toLowerCase().replace(/^\[|\]$/g, '').replace(/\.$/, ''); + if (!h) return null; + if (h.includes(':')) { + try { h = new URL(`http://[${h}]/`).hostname.slice(1, -1); } catch { return null; } + const groups = expandIpv6(h); + if (!groups) return null; + const head = groups.slice(0, 6).map(g => g.toString(16)).join(':'); + if (head === '0:0:0:0:0:ffff' || head === '64:ff9b:0:0:0:0' || (head === '0:0:0:0:0:0' && groups[6] !== 0)) { + return classifyIpv4([groups[6] >> 8, groups[6] & 255, groups[7] >> 8, groups[7] & 255]); + } + if (groups.every((g, i) => g === (i === 7 ? 1 : 0))) return 'loopback'; + if ((groups[0] & 0xfe00) === 0xfc00 || (groups[0] & 0xffc0) === 0xfe80) return 'blocked'; + return 'other'; + } + try { h = new URL(`http://${h}/`).hostname; } catch { return null; } + const m = /^(\d+)\.(\d+)\.(\d+)\.(\d+)$/.exec(h); + return m ? classifyIpv4(m.slice(1).map(Number)) : null; +} + +function classifyIpv4([a, b, c, d]: number[]): 'blocked' | 'loopback' | 'other' { + if (a === 169 && b === 254) return 'blocked'; + if (a === 100 && b === 100 && c === 100 && d === 200) return 'blocked'; + return a === 127 ? 'loopback' : 'other'; +} + +/** Expand a canonical IPv6 literal to eight 16-bit groups. */ +function expandIpv6(addr: string): number[] | null { + const halves = addr.split('::'); + if (halves.length > 2) return null; + const parse = (part: string) => (part ? part.split(':') : []).map(g => parseInt(g, 16)); + const head = parse(halves[0]); + const tail = halves.length === 2 ? parse(halves[1]) : []; + const fill = 8 - head.length - tail.length; + if (fill < 0 || (halves.length === 1 && fill !== 0)) return null; + const groups = [...head, ...Array(fill).fill(0), ...tail]; + return groups.every(g => Number.isInteger(g) && g >= 0 && g <= 0xffff) ? groups : null; +} + +/** + * Resolve a hostname and report whether any A or AAAA answer is blocked. + * Each family is tried independently; a missing family is not a risk. DNS + * infrastructure failure fails open. Connection-time rebinding (a different + * answer when Chromium connects) is out of scope. */ async function resolvesToBlockedIp(hostname: string): Promise { try { - const dns = await import('node:dns'); - const { resolve4, resolve6 } = dns.promises; - - // Check IPv4 A records - const v4Check = resolve4(hostname).then( - (addresses) => addresses.some(addr => BLOCKED_METADATA_HOSTS.has(addr)), - () => false, // ENODATA / ENOTFOUND — no A records, not a risk + const { resolve4, resolve6 } = (await import('node:dns')).promises; + const check = (lookup: Promise) => lookup.then( + (addresses) => addresses.some(addr => classifyAddress(addr) === 'blocked'), + () => false, ); - - // Check IPv6 AAAA records — the gap that issue #668 identified - const v6Check = resolve6(hostname).then( - (addresses) => addresses.some(addr => { - const normalized = addr.toLowerCase(); - return BLOCKED_METADATA_HOSTS.has(normalized) || isBlockedIpv6(normalized); - }), - () => false, // ENODATA / ENOTFOUND — no AAAA records, not a risk - ); - - const [v4Blocked, v6Blocked] = await Promise.all([v4Check, v6Check]); - return v4Blocked || v6Blocked; + const [v4, v6] = await Promise.all([check(resolve4(hostname)), check(resolve6(hostname))]); + return v4 || v6; } catch { - // Unexpected error — fail open (don't block navigation on DNS infrastructure failure) return false; } } +/** + * Why an http(s) navigation target is refused, or null when it is allowed. + * Applied to explicit navigations (validateNavigationUrl) and to every + * navigation request the browser makes, including redirect hops and + * page-driven navigations (BrowserManager's navigation guard). + */ +export async function blockedNavigationReason(url: string): Promise { + let parsed: URL; + try { parsed = new URL(url); } catch { return null; } + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') return null; + const hostname = parsed.hostname.toLowerCase().replace(/^\[|\]$/g, '').replace(/\.$/, ''); + const kind = classifyAddress(hostname); + if (kind === 'blocked' || BLOCKED_METADATA_HOSTNAMES.has(hostname)) { + return `Blocked: ${parsed.hostname} is a cloud metadata or link-local address. Access is denied for security.`; + } + if (kind !== null || hostname === 'localhost') return null; + if (await resolvesToBlockedIp(hostname)) { + return `Blocked: ${parsed.hostname} resolves to a cloud metadata or link-local address. Possible DNS rebinding attack.`; + } + return null; +} + /** * Normalize non-standard file:// URLs into absolute form before the WHATWG URL parser * sees them. Handles cwd-relative, home-relative, and bare-segment shapes that the @@ -290,24 +292,8 @@ export async function validateNavigationUrl(url: string): Promise { ); } - const hostname = normalizeHostname(parsed.hostname.toLowerCase()); - - if (BLOCKED_METADATA_HOSTS.has(hostname) || isMetadataIp(hostname) || isBlockedIpv6(hostname)) { - throw new Error( - `Blocked: ${parsed.hostname} is a cloud metadata endpoint. Access is denied for security.` - ); - } - - // DNS rebinding protection: resolve hostname and check if it points to metadata IPs. - // Skip for loopback/private IPs — they can't be DNS-rebinded and the async DNS - // resolution adds latency that breaks concurrent E2E tests under load. - const isLoopback = hostname === 'localhost' || hostname === '127.0.0.1' || hostname === '::1'; - const isPrivateNet = /^(10\.|172\.(1[6-9]|2[0-9]|3[01])\.|192\.168\.)/.test(hostname); - if (!isLoopback && !isPrivateNet && await resolvesToBlockedIp(hostname)) { - throw new Error( - `Blocked: ${parsed.hostname} resolves to a cloud metadata IP. Possible DNS rebinding attack.` - ); - } + const blocked = await blockedNavigationReason(parsed.href); + if (blocked) throw new Error(blocked); return url; } diff --git a/browse/test/navigation-guard.test.ts b/browse/test/navigation-guard.test.ts new file mode 100644 index 000000000..736b47e43 --- /dev/null +++ b/browse/test/navigation-guard.test.ts @@ -0,0 +1,75 @@ +/** + * D2 (#2811): redirects and page-driven navigations to link-local or metadata + * addresses are refused, not only explicit `goto` targets. Commands run the way + * server.ts runs write commands: through BrowserManager.failIfNavigationBlocked. + */ +import { afterAll, beforeAll, describe, expect, test } from 'bun:test'; +import { BrowserManager } from '../src/browser-manager'; +import { handleWriteCommand } from '../src/write-commands'; + +let bm: BrowserManager; +let server: ReturnType; +let base: string; + +const LINK_LOCAL = 'http://169.254.170.2/v2/credentials'; +const MAPPED = 'http://[::ffff:169.254.170.2]/v2/credentials'; + +beforeAll(async () => { + server = Bun.serve({ + hostname: '127.0.0.1', + port: 0, + fetch(req) { + const url = new URL(req.url); + const html = (body: string) => new Response(`${body}`, { headers: { 'Content-Type': 'text/html' } }); + if (url.pathname === '/to-link-local') return Response.redirect(LINK_LOCAL, 302); + if (url.pathname === '/to-mapped') return Response.redirect(MAPPED, 302); + if (url.pathname === '/to-safe') return Response.redirect(`${base}/safe`, 302); + if (url.pathname === '/link') return html(`go`); + if (url.pathname === '/script') return html(``); + return html('

safe page

'); + }, + }); + base = `http://127.0.0.1:${server.port}`; + bm = new BrowserManager(); + await bm.launch(); +}); + +afterAll(async () => { + try { server.stop(true); } catch {} + try { await Promise.race([bm?.close(), new Promise((resolve) => setTimeout(resolve, 3000))]); } catch {} +}); + +function run(command: string, args: string[]): Promise { + const session = bm.getActiveSession(); + return bm.failIfNavigationBlocked(session.getPage(), handleWriteCommand(command, args, session, bm)); +} + +describe('D2: navigation guard', () => { + test('a redirect to 169.254.0.0/16 fails the goto and leaves the tab blank', async () => { + await expect(run('goto', [`${base}/to-link-local`])).rejects.toThrow(/169\.254\.170\.2 is a cloud metadata or link-local address/); + expect(bm.getActiveSession().getPage().url()).toBe('about:blank'); + }, 30_000); + + test('a redirect to an IPv4-mapped IPv6 link-local address is refused too', async () => { + await expect(run('goto', [`${base}/to-mapped`])).rejects.toThrow(/cloud metadata or link-local/); + expect(bm.getActiveSession().getPage().url()).toBe('about:blank'); + }, 30_000); + + test('clicking a link to a link-local address is refused', async () => { + await run('goto', [`${base}/link`]); + await expect(run('click', ['#go'])).rejects.toThrow(/cloud metadata or link-local/); + expect(bm.getActiveSession().getPage().url()).toBe('about:blank'); + }, 30_000); + + test('a script-driven navigation to a link-local address is refused', async () => { + await run('goto', [`${base}/script`]); + await expect(run('click', ['#go'])).rejects.toThrow(/cloud metadata or link-local/); + expect(bm.getActiveSession().getPage().url()).toBe('about:blank'); + }, 30_000); + + test('ordinary redirects and the next command still work after a block', async () => { + const result = await run('goto', [`${base}/to-safe`]); + expect(result).toContain('Navigated to'); + expect(bm.getActiveSession().getPage().url()).toBe(`${base}/safe`); + }, 30_000); +}); diff --git a/browse/test/path-validation.test.ts b/browse/test/path-validation.test.ts index ff0779563..ebd4a7191 100644 --- a/browse/test/path-validation.test.ts +++ b/browse/test/path-validation.test.ts @@ -2,7 +2,7 @@ import { beforeAll, describe, it, expect } from 'bun:test'; import { chromium } from 'playwright'; import { validateOutputPath } from '../src/path-security'; import { validateReadPath, SENSITIVE_COOKIE_NAME, SENSITIVE_COOKIE_VALUE } from '../src/read-commands'; -import { BLOCKED_METADATA_HOSTS } from '../src/url-validation'; +import { classifyAddress } from '../src/url-validation'; import { mkdirSync, mkdtempSync, rmSync, symlinkSync, unlinkSync, writeFileSync, realpathSync } from 'fs'; import { tmpdir, userInfo } from 'os'; import { join } from 'path'; @@ -258,11 +258,11 @@ describe('DNS rebinding — production blocklist', () => { }); it('blocks AWS/GCP IPv4 metadata address', () => { - expect(BLOCKED_METADATA_HOSTS.has('169.254.169.254')).toBe(true); + expect(classifyAddress('169.254.169.254')).toBe('blocked'); }); it('does not block normal addresses', () => { - expect(BLOCKED_METADATA_HOSTS.has('8.8.8.8')).toBe(false); - expect(BLOCKED_METADATA_HOSTS.has('2001:4860:4860::8888')).toBe(false); + expect(classifyAddress('8.8.8.8')).toBe('other'); + expect(classifyAddress('2001:4860:4860::8888')).toBe('other'); }); }); diff --git a/browse/test/url-validation.test.ts b/browse/test/url-validation.test.ts index 7f7fe5949..449852543 100644 --- a/browse/test/url-validation.test.ts +++ b/browse/test/url-validation.test.ts @@ -1,5 +1,7 @@ -import { describe, it, expect } from 'bun:test'; -import { validateNavigationUrl, normalizeFileUrl } from '../src/url-validation'; +import { describe, it, expect, spyOn } from 'bun:test'; +import * as dns from 'node:dns'; +import { validateNavigationUrl, normalizeFileUrl, classifyAddress, blockedNavigationReason } from '../src/url-validation'; +import { isInternalCookieDomain } from '../src/session-persist'; import * as fs from 'fs'; import * as path from 'path'; import { TEMP_DIR } from '../src/platform'; @@ -136,6 +138,65 @@ describe('validateNavigationUrl', () => { it('throws on malformed URLs', async () => { await expect(validateNavigationUrl('not-a-url')).rejects.toThrow(/Invalid URL/i); }); + + it('blocks the whole 169.254.0.0/16 range, incl. container credential endpoints (#2811)', async () => { + for (const url of ['http://169.254.170.2/v2/credentials/', 'http://169.254.170.23/', 'http://169.254.1.2/', 'http://0xA9FEAA02/', 'http://[::ffff:169.254.170.2]/', 'http://100.100.100.200/latest/meta-data/']) { + await expect(validateNavigationUrl(url)).rejects.toThrow(/cloud metadata or link-local/); + } + }); + + it('leaves 169.254.-prefixed hostnames to DNS and private dev servers allowed', async () => { + await expect(validateNavigationUrl('https://169.254.example.com/')).resolves.toBe('https://169.254.example.com/'); + await expect(validateNavigationUrl('http://10.0.0.5/health')).resolves.toBe('http://10.0.0.5/health'); + await expect(validateNavigationUrl('http://100.100.100.201/')).resolves.toBe('http://100.100.100.201/'); + }); +}); + +describe('classifyAddress (D2: one classifier for literals, DNS answers and cookie domains)', () => { + it.each([ + ['169.254.169.254', 'blocked'], ['169.254.170.2', 'blocked'], ['2852039166', 'blocked'], + ['0251.0376.0251.0376', 'blocked'], ['0xA9FEAA02', 'blocked'], ['100.100.100.200', 'blocked'], + ['::ffff:169.254.170.2', 'blocked'], ['[::ffff:a9fe:aa02]', 'blocked'], ['::a9fe:a9fe', 'blocked'], + ['64:ff9b::169.254.169.254', 'blocked'], ['fe80::1', 'blocked'], ['febf::1', 'blocked'], ['fd00::', 'blocked'], + ['127.0.0.1', 'loopback'], ['2130706433', 'loopback'], ['::1', 'loopback'], ['::ffff:127.0.0.1', 'loopback'], + ['8.8.8.8', 'other'], ['192.168.1.1', 'other'], ['::ffff:8.8.8.8', 'other'], ['2001:4860:4860::8888', 'other'], ['fec0::1', 'other'], + ['example.com', null], ['169.254.example.com', null], ['fd.example.com', null], + ] as const)('%s → %s', (host, kind) => { + expect(classifyAddress(host)).toBe(kind); + }); + + it('session-persist drops cookies for every blocked or loopback spelling', () => { + for (const domain of ['.169.254.170.2', '[::ffff:a9fe:a9fe]', '2130706433', '100.100.100.200', 'fe80::1']) { + expect(isInternalCookieDomain(domain)).toBe(true); + } + expect(isInternalCookieDomain('.example.com')).toBe(false); + }); +}); + +describe('blockedNavigationReason: DNS answers', () => { + async function withAnswers(v4: string[], v6: string[], fn: () => Promise) { + const r4 = spyOn(dns.promises, 'resolve4').mockImplementation((async () => v4) as any); + const r6 = spyOn(dns.promises, 'resolve6').mockImplementation((async () => v6) as any); + try { await fn(); } finally { r4.mockRestore(); r6.mockRestore(); } + } + + it('blocks a hostname with one link-local A answer among public ones', async () => { + await withAnswers(['93.184.216.34', '169.254.170.2'], [], async () => { + expect(await blockedNavigationReason('https://mixed.example/')).toMatch(/resolves to a cloud metadata or link-local address/); + }); + }); + + it('blocks a hostname whose AAAA answer is an IPv4-mapped link-local address', async () => { + await withAnswers([], ['::ffff:a9fe:a9fe'], async () => { + expect(await blockedNavigationReason('https://mapped.example/')).toMatch(/resolves to/); + }); + }); + + it('allows a hostname with only public answers', async () => { + await withAnswers(['93.184.216.34'], ['2606:2800:220:1:248:1893:25c8:1946'], async () => { + expect(await blockedNavigationReason('https://clean.example/')).toBeNull(); + }); + }); }); describe('validateNavigationUrl — restoreState coverage', () => { From fb4d2983e9a3e8406fd85fa208146f2dc61da481 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 20:52:02 +0000 Subject: [PATCH 04/17] E1: freeze normalizes boundary and target the same way on Windows; hooks spawn through cmd.exe (#2876, #2354) gstack_hook_normalize_path (careful/bin/hook-extract.sh) maps C:\..., c:/..., UNC \\server\share (leading // kept) and /cygdrive/c to one form; check-freeze runs both the saved boundary and file_path through it before the absolute test, after symlink resolution, and compares case-insensitively on Windows bash. freeze, guard and careful register hooks as bash -c "exec bash \"$HOME/...\"" so cmd.exe, which never expands $HOME, no longer turns the hook into a silent exit 127. Co-authored-by: Kevin Grasso Co-authored-by: Xuan-Ce Wang <261591073+xwang4-svg@users.noreply.github.com> --- careful/SKILL.md.tmpl | 2 +- careful/bin/hook-extract.sh | 44 ++++++- freeze/SKILL.md.tmpl | 4 +- freeze/bin/check-freeze.sh | 29 ++++- guard/SKILL.md.tmpl | 6 +- test/investigate-freeze-path.test.ts | 176 +++++++++++++++++++++++++++ 6 files changed, 250 insertions(+), 11 deletions(-) diff --git a/careful/SKILL.md.tmpl b/careful/SKILL.md.tmpl index e77dc2930..33c1b8286 100644 --- a/careful/SKILL.md.tmpl +++ b/careful/SKILL.md.tmpl @@ -19,7 +19,7 @@ hooks: - matcher: "Bash" hooks: - type: command - command: "bash $HOME/.claude/skills/gstack/careful/bin/check-careful.sh" + command: 'bash -c "exec bash \"$HOME/.claude/skills/gstack/careful/bin/check-careful.sh\""' statusMessage: "Checking for destructive commands..." sensitive: true --- diff --git a/careful/bin/hook-extract.sh b/careful/bin/hook-extract.sh index 2934beb2e..09e0cd26f 100644 --- a/careful/bin/hook-extract.sh +++ b/careful/bin/hook-extract.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash -# hook-extract.sh — SHARED JSON helpers for gstack PreToolUse hooks. +# hook-extract.sh — SHARED JSON and path helpers for gstack PreToolUse hooks. # Sourced (never executed) by careful/bin/check-careful.sh and # freeze/bin/check-freeze.sh via a path relative to each hook script. # @@ -32,6 +32,48 @@ sys.stdout.write(c if isinstance(c, str) else "")' "$_ghef_field" 2>/dev/null && return 1 } +# Windows bash (Git Bash / MSYS / Cygwin), read from bash's own OSTYPE so the +# check never costs a fork: careful sources this file on every Bash call. +case "${OSTYPE:-}" in + msys*|cygwin*|win32*) GSTACK_HOOK_IS_WINDOWS=1 ;; + *) GSTACK_HOOK_IS_WINDOWS=0 ;; +esac +_GSTACK_HOOK_AZ_UPPER=ABCDEFGHIJKLMNOPQRSTUVWXYZ +_GSTACK_HOOK_AZ_LOWER=abcdefghijklmnopqrstuvwxyz +GSTACK_HOOK_PATH="" + +# gstack_hook_normalize_path PATH +# The one path canonicalization for freeze's boundary AND the tool's +# file_path (#2876). Leaves the result in GSTACK_HOOK_PATH (stdout is the +# hook's decision channel, and $(...) would cost a fork): +# C:\dev\x c:/dev/x -> /c/dev/x (separators, drive-letter case) +# \\srv\share\x -> //srv/share/x (UNC: the leading // is kept) +# /cygdrive/c/x -> /c/x (Windows bash only) +# Drive-letter and UNC shapes are recognised lexically on every platform. +# A path without those shapes is rewritten only on Windows bash: on POSIX +# '\' is a legal filename character, and rewriting it would let a name like +# 'a\..\..\etc\x' change which directory the check sees. Remaining case +# differences on Windows are handled by comparing under nocasematch. +# Builtins only, bash 3.2 compatible. +gstack_hook_normalize_path() { + GSTACK_HOOK_PATH="$1" + case "$GSTACK_HOOK_PATH" in + [A-Za-z]:[\\/]*|[A-Za-z]:|\\\\[!\\]*) ;; + *) [ "$GSTACK_HOOK_IS_WINDOWS" = 1 ] || return 0 ;; + esac + GSTACK_HOOK_PATH="${GSTACK_HOOK_PATH//\\//}" + case "$GSTACK_HOOK_PATH" in + [A-Za-z]:/*|[A-Za-z]:) + _ghnp_d="${GSTACK_HOOK_PATH%%:*}" + _ghnp_pre="${_GSTACK_HOOK_AZ_UPPER%%"$_ghnp_d"*}" + [ "$_ghnp_pre" = "$_GSTACK_HOOK_AZ_UPPER" ] || _ghnp_d="${_GSTACK_HOOK_AZ_LOWER:${#_ghnp_pre}:1}" + GSTACK_HOOK_PATH="/$_ghnp_d${GSTACK_HOOK_PATH#?:}" + ;; + /cygdrive/[A-Za-z]/*|/cygdrive/[A-Za-z]) GSTACK_HOOK_PATH="${GSTACK_HOOK_PATH#/cygdrive}" ;; + esac + return 0 +} + # gstack_hook_json_string TEXT # Prints TEXT as a JSON string literal (surrounding quotes included), # encoding quotes, backslashes, control characters and newlines. Never build diff --git a/freeze/SKILL.md.tmpl b/freeze/SKILL.md.tmpl index 470f713a1..b86711c62 100644 --- a/freeze/SKILL.md.tmpl +++ b/freeze/SKILL.md.tmpl @@ -20,12 +20,12 @@ hooks: - matcher: "Edit" hooks: - type: command - command: "bash $HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh" + command: 'bash -c "exec bash \"$HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh\""' statusMessage: "Checking freeze boundary..." - matcher: "Write" hooks: - type: command - command: "bash $HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh" + command: 'bash -c "exec bash \"$HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh\""' statusMessage: "Checking freeze boundary..." sensitive: true --- diff --git a/freeze/bin/check-freeze.sh b/freeze/bin/check-freeze.sh index 0c092dbc1..4b67dae03 100755 --- a/freeze/bin/check-freeze.sh +++ b/freeze/bin/check-freeze.sh @@ -56,7 +56,7 @@ fi # A helper from an older install that lacks the function must fail CLOSED # (the existence check above only proves the file sourced), never exit 127 # with no JSON — Claude Code treats that as non-blocking. -if ! command -v gstack_hook_state_root >/dev/null 2>&1; then +if ! command -v gstack_hook_state_root >/dev/null 2>&1 || ! command -v gstack_hook_normalize_path >/dev/null 2>&1; then _FREEZE_DECIDED=1 printf '{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny","permissionDecisionReason":"[freeze] Hook helpers out of date (partial upgrade?) - blocked, fail closed. Re-run ./setup or /unfreeze."}}\n' exit 0 @@ -93,6 +93,12 @@ if [ -z "$FREEZE_DIR" ]; then exit 0 fi +# Both sides of the comparison go through one canonicalization: the state +# file may hold C:\dev\proj\ or the /c/dev/proj/ that `pwd` prints in Git +# Bash, and Claude Code on Windows reports file_path as C:\... (#2876). +gstack_hook_normalize_path "$FREEZE_DIR" +FREEZE_DIR="$GSTACK_HOOK_PATH" + case "$FREEZE_DIR" in /*) ;; *) @@ -123,6 +129,11 @@ if [ -z "$FILE_PATH" ]; then exit 0 fi +# Canonicalize before the absolute test: a drive-letter path does not start +# with '/', so it used to be joined onto cwd and every edit was denied. +gstack_hook_normalize_path "$FILE_PATH" +FILE_PATH="$GSTACK_HOOK_PATH" + # Resolve file_path to absolute if it isn't already case "$FILE_PATH" in /*) ;; # already absolute @@ -131,8 +142,14 @@ case "$FILE_PATH" in ;; esac -# Normalize: remove double slashes and trailing slash +# Normalize: remove double slashes and trailing slash. A leading '//' is a UNC +# host (//server/share) and is kept, or the check would compare a local path. +_FP_LEAD="" +case "$FILE_PATH" in + //[!/]*) _FP_LEAD="/" ;; +esac FILE_PATH=$(printf '%s' "$FILE_PATH" | sed 's|/\+|/|g;s|/$||') +FILE_PATH="$_FP_LEAD$FILE_PATH" [ -n "$FILE_PATH" ] || FILE_PATH="/" # Resolve symlinks and .. sequences (POSIX-portable, works on macOS). @@ -157,12 +174,16 @@ _resolve_path() { _base="$(basename "$_p")" if [ "$_base" = / ]; then printf '/'; return; fi _dir="$(cd "$_dir" 2>/dev/null && pwd -P || printf '%s' "$_dir")" - printf '%s/%s' "${_dir%/}" "$_base" + # pwd -P answers in the platform's own dialect (/cygdrive/c/... on Cygwin). + gstack_hook_normalize_path "${_dir%/}/$_base" + printf '%s' "$GSTACK_HOOK_PATH" } FILE_PATH=$(_resolve_path "$FILE_PATH") FREEZE_DIR=$(_resolve_path "$FREEZE_DIR") -# Check: does the file path start with the freeze directory? +# Check: does the file path start with the freeze directory? Windows paths +# are case-insensitive, so compare that way there. +[ "$GSTACK_HOOK_IS_WINDOWS" = 1 ] && shopt -s nocasematch case "$FILE_PATH" in "${FREEZE_DIR%/}/"*|"${FREEZE_DIR}") # Inside freeze boundary — allow diff --git a/guard/SKILL.md.tmpl b/guard/SKILL.md.tmpl index cf0b157e9..f2f194920 100644 --- a/guard/SKILL.md.tmpl +++ b/guard/SKILL.md.tmpl @@ -20,17 +20,17 @@ hooks: - matcher: "Bash" hooks: - type: command - command: "bash $HOME/.claude/skills/gstack/careful/bin/check-careful.sh" + command: 'bash -c "exec bash \"$HOME/.claude/skills/gstack/careful/bin/check-careful.sh\""' statusMessage: "Checking for destructive commands..." - matcher: "Edit" hooks: - type: command - command: "bash $HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh" + command: 'bash -c "exec bash \"$HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh\""' statusMessage: "Checking freeze boundary..." - matcher: "Write" hooks: - type: command - command: "bash $HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh" + command: 'bash -c "exec bash \"$HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh\""' statusMessage: "Checking freeze boundary..." sensitive: true --- diff --git a/test/investigate-freeze-path.test.ts b/test/investigate-freeze-path.test.ts index 7b9ef0cae..0dbbdb16a 100644 --- a/test/investigate-freeze-path.test.ts +++ b/test/investigate-freeze-path.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from 'bun:test'; import * as fs from 'fs'; import * as path from 'path'; +import * as os from 'os'; const ROOT = path.resolve(import.meta.dir, '..'); const FILES = ['investigate/SKILL.md.tmpl', 'investigate/SKILL.md']; @@ -32,3 +33,178 @@ describe('investigate freeze path resolution', () => { }); } }); + +// E1 (#2876, #2354 residual). One canonicalization for the saved boundary and +// the tool's file_path, exercised through the real hook script. Hooks run via +// explicit `bash` argv with template-literal paths so the file stays in the +// windows-free lane (scripts/test-free-shards.ts --windows-only --list). +const IS_WIN = process.platform === 'win32'; +const FREEZE_HOOK = `${ROOT}/freeze/bin/check-freeze.sh`; + +function decision(stdout: string): string { + const raw = stdout.trim(); + if (raw === '{}') return 'allow'; + return JSON.parse(raw)?.hookSpecificOutput?.permissionDecision ?? `unparsed: ${raw}`; +} + +function withState(boundary: string, fn: (env: Record) => T): T { + const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-e1-freeze-')); + try { + fs.writeFileSync(path.join(stateDir, 'freeze-dir.txt'), `${boundary}\n`); + return fn({ ...process.env, GSTACK_STATE_ROOT: stateDir } as Record); + } finally { + fs.rmSync(stateDir, { recursive: true, force: true }); + } +} + +function freezeDecision(boundary: string, filePath: string): string { + return withState(boundary, (env) => { + const r = Bun.spawnSync(['bash', FREEZE_HOOK], { + stdin: Buffer.from(JSON.stringify({ tool_input: { file_path: filePath } })), + env, + timeout: 15_000, + }); + expect(r.exitCode).toBe(0); + return decision(r.stdout.toString()); + }); +} + +describe('E1: freeze boundary and target share one platform-aware normalization (#2876)', () => { + test.each([ + ['C:\\dev\\proj\\src\\', 'C:\\dev\\proj\\src\\a.py', 'allow'], + ['/c/dev/proj/src/', 'C:\\dev\\proj\\src\\a.py', 'allow'], + ['C:\\dev\\proj\\src\\', '/c/dev/proj/src/a.py', 'allow'], + ['c:/dev/proj/src/', 'C:\\dev\\proj\\src\\a.py', 'allow'], + ['C:/dev/proj/src', 'c:\\dev\\proj\\src\\deep\\new\\file.py', 'allow'], + ['C:\\dev\\proj\\src\\', 'C:\\dev\\proj\\src-old\\a.py', 'deny'], + ['C:\\dev\\proj\\src\\', 'D:\\dev\\proj\\src\\a.py', 'deny'], + ['C:\\dev\\proj\\src\\', 'C:\\dev\\proj\\other\\missing\\a.py', 'deny'], + ['\\\\server\\share\\proj\\', '\\\\server\\share\\proj\\a.py', 'allow'], + ['\\\\server\\share\\proj\\', '\\\\server\\other\\proj\\a.py', 'deny'], + ['\\\\server\\share\\proj\\', '/server/share/proj/a.py', 'deny'], + ])('boundary %s, target %s → %s', (boundary, target, expected) => { + expect(freezeDecision(boundary, target)).toBe(expected); + }); + + test.skipIf(!IS_WIN)('Windows: drive and path case differences do not decide the outcome', () => { + expect(freezeDecision('C:\\Dev\\Proj\\Src\\', 'c:\\dev\\proj\\src\\a.py')).toBe('allow'); + expect(freezeDecision('c:\\dev\\proj\\src\\', 'C:\\DEV\\PROJ\\SRC-OLD\\a.py')).toBe('deny'); + }); + + test.skipIf(!IS_WIN)('Windows: a junction inside the boundary that points outside is denied', () => { + const base = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-e1-junction-'))); + try { + fs.mkdirSync(path.join(base, 'boundary')); + fs.mkdirSync(path.join(base, 'outside')); + fs.symlinkSync(path.join(base, 'outside'), path.join(base, 'boundary', 'escape'), 'junction'); + expect(freezeDecision(`${path.join(base, 'boundary')}\\`, path.join(base, 'boundary', 'escape', 'a.py'))).toBe('deny'); + expect(freezeDecision(`${path.join(base, 'boundary')}\\`, path.join(base, 'boundary', 'a.py'))).toBe('allow'); + } finally { + fs.rmSync(base, { recursive: true, force: true }); + } + }); + + test.skipIf(IS_WIN)('POSIX: a backslash in a filename stays a filename character', () => { + const base = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-e1-bslash-'))); + try { + const boundary = path.join(base, 'boundary'); + fs.mkdirSync(boundary); + expect(freezeDecision(`${boundary}/`, `${boundary}/a\\..\\..\\etc\\x`)).toBe('allow'); + } finally { + fs.rmSync(base, { recursive: true, force: true }); + } + }); +}); + +// #2354 residual: on Windows Claude Code spawns hook commands through +// `cmd.exe /d /s /c`, which never expands $HOME, so `bash $HOME/...` handed +// bash a literal "$HOME/..." path, exited 127, and the hook silently failed +// open. Run every registered freeze/guard/careful command the way cmd.exe +// does (no $ expansion, MSVC argv quoting) and the way POSIX sh does. +function registeredHookCommands(rel: string): string[] { + const fm = fs.readFileSync(path.join(ROOT, rel), 'utf-8').split('\n---')[0]; + return [...fm.matchAll(/^\s*command:\s*(.+)$/gm)].map((m) => { + const v = m[1].trim(); + if (v.startsWith("'")) return v.slice(1, -1).replaceAll("''", "'"); + if (v.startsWith('"')) return JSON.parse(v); + return v; + }); +} + +/** argv as a Windows program (bash.exe) parses its command line from cmd.exe. */ +function msvcArgv(command: string): string[] { + const args: string[] = []; + let cur = ''; + let inQuotes = false; + let started = false; + for (let i = 0; i < command.length; i++) { + const c = command[i]; + if (c === '\\') { + let n = 0; + while (command[i] === '\\') { n++; i++; } + if (command[i] === '"') { + cur += '\\'.repeat(Math.floor(n / 2)); + if (n % 2) cur += '"'; else { inQuotes = !inQuotes; } + } else { + cur += '\\'.repeat(n); + i--; + } + started = true; + } else if (c === '"') { + inQuotes = !inQuotes; + started = true; + } else if ((c === ' ' || c === '\t') && !inQuotes) { + if (started) args.push(cur); + cur = ''; + started = false; + } else { + cur += c; + started = true; + } + } + if (started) args.push(cur); + return args; +} + +function withInstalledHome(fn: (home: string) => T): T { + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-e1-home-')); + try { + fs.mkdirSync(path.join(home, '.claude', 'skills'), { recursive: true }); + fs.symlinkSync(ROOT, path.join(home, '.claude', 'skills', 'gstack'), 'junction'); + return fn(home); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } +} + +const HOOKED = ['freeze/SKILL.md.tmpl', 'guard/SKILL.md.tmpl', 'careful/SKILL.md.tmpl']; + +describe('E1: registered hook commands run under cmd.exe and POSIX sh (#2354)', () => { + for (const rel of HOOKED) { + const commands = registeredHookCommands(rel); + test(`${rel} registers $HOME-anchored hooks`, () => { + expect(commands.length).toBeGreaterThan(0); + }); + for (const [i, command] of commands.entries()) { + const isFreeze = command.includes('check-freeze.sh'); + const input = isFreeze ? { tool_input: { file_path: `${os.tmpdir()}${path.sep}outside-boundary.txt` } } : { tool_input: { command: 'rm -rf /' } }; + const expected = 'deny'; + const shells: Array<[string, (home: string) => string[]]> = IS_WIN + ? [['cmd.exe', () => ['cmd.exe', '/d', '/s', '/c', `"${command}"`]]] + : [['cmd.exe argv', () => msvcArgv(command)], ['POSIX shell', () => ['sh', '-c', command]]]; + for (const [label, argv] of shells) { + test(`${rel} hook #${i + 1} reaches a decision via ${label}`, () => { + withInstalledHome((home) => withState(path.join(home, 'frozen'), (env) => { + const r = Bun.spawnSync(argv(home), { + stdin: Buffer.from(JSON.stringify(input)), + env: { ...env, HOME: home }, + timeout: 15_000, + }); + expect(r.exitCode, r.stderr.toString()).toBe(0); + expect(decision(r.stdout.toString())).toBe(expected); + })); + }); + } + } + } +}); From 616773158b2032801c92a1ef3fc263700da554da Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 20:53:42 +0000 Subject: [PATCH 05/17] G4: keep CLI-to-daemon loopback calls off HTTP_PROXY (#494) The CLI appends 127.0.0.1, localhost and ::1 to the user's NO_PROXY (and no_proxy), keeping existing entries, before its first fetch. Verified on Bun 1.4.0: fetch honors NO_PROXY when it is set before the first request and ignores later changes, so it is set at the top of main(). --- browse/src/cli.ts | 14 +++++ browse/test/cli-loopback-no-proxy.test.ts | 77 +++++++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 browse/test/cli-loopback-no-proxy.test.ts diff --git a/browse/src/cli.ts b/browse/src/cli.ts index 54a8c35c9..9f523d6ed 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -1605,7 +1605,21 @@ async function handlePairAgent(state: ServerState, args: string[]): Promise): string { + const entries = (env.NO_PROXY ?? env.no_proxy ?? '').split(',').map(e => e.trim()).filter(Boolean); + for (const host of ['127.0.0.1', 'localhost', '::1']) if (!entries.includes(host)) entries.push(host); + return entries.join(','); +} + async function main() { + process.env.NO_PROXY = process.env.no_proxy = withLoopbackNoProxy(process.env); const rawArgs = process.argv.slice(2); // ─── Global flags (--proxy, --headed) ─────────────────────── diff --git a/browse/test/cli-loopback-no-proxy.test.ts b/browse/test/cli-loopback-no-proxy.test.ts new file mode 100644 index 000000000..d34979607 --- /dev/null +++ b/browse/test/cli-loopback-no-proxy.test.ts @@ -0,0 +1,77 @@ +/** + * G4 (#494): with HTTP_PROXY set, the CLI's loopback calls to its own daemon + * went through the proxy, so the daemon never looked healthy. The CLI appends + * loopback to NO_PROXY (keeping the user's entries) before its first fetch. + */ +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { withLoopbackNoProxy } from '../src/cli'; + +const CLI = path.resolve(import.meta.dir, '../src/cli.ts'); + +describe('withLoopbackNoProxy', () => { + test('appends loopback names to an existing list without replacing it', () => { + expect(withLoopbackNoProxy({ NO_PROXY: 'corp.internal, .example.com' })).toBe('corp.internal,.example.com,127.0.0.1,localhost,::1'); + expect(withLoopbackNoProxy({ no_proxy: 'corp.internal' })).toBe('corp.internal,127.0.0.1,localhost,::1'); + expect(withLoopbackNoProxy({})).toBe('127.0.0.1,localhost,::1'); + expect(withLoopbackNoProxy({ NO_PROXY: 'localhost,127.0.0.1,::1' })).toBe('localhost,127.0.0.1,::1'); + }); +}); + +describe('CLI → daemon behind a proxy that fails every request', () => { + let scratch: string; + let proxy: ReturnType; + let proxied = 0; + + beforeEach(() => { + scratch = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-no-proxy-')); + proxied = 0; + proxy = Bun.serve({ hostname: '127.0.0.1', port: 0, fetch: () => { proxied++; return new Response('proxy refuses', { status: 502 }); } }); + }); + + afterEach(() => { + try { proxy.stop(true); } catch {} + const stateFile = path.join(scratch, 'state', 'browse.json'); + if (fs.existsSync(stateFile)) { + try { process.kill(JSON.parse(fs.readFileSync(stateFile, 'utf-8')).pid, 'SIGKILL'); } catch {} + } + fs.rmSync(scratch, { recursive: true, force: true }); + }); + + test.skipIf(process.platform === 'win32')('status reaches the daemon directly and the daemon keeps the user NO_PROXY entries', async () => { + const stateFile = path.join(scratch, 'state', 'browse.json'); + const stub = path.join(scratch, 'stub-server.ts'); + fs.writeFileSync(stub, ` + import * as fs from 'node:fs'; + const server = Bun.serve({ hostname: '127.0.0.1', port: 0, fetch: () => Response.json({ status: 'healthy' }) }); + fs.writeFileSync(process.env.BROWSE_STATE_FILE!, JSON.stringify({ + pid: process.pid, port: server.port, token: 'no-proxy-test', mode: 'launched', noProxy: process.env.NO_PROXY, + })); + setTimeout(() => process.exit(0), 5000); + `); + const env: Record = {}; + for (const [key, value] of Object.entries(process.env)) { + if (value !== undefined && !/^(BROWSE_|GSTACK_|NO_PROXY$|no_proxy$|HTTPS?_PROXY$|https?_proxy$|ALL_PROXY$|all_proxy$)/.test(key)) env[key] = value; + } + Object.assign(env, { + HOME: scratch, + GSTACK_HOME: path.join(scratch, '.gstack'), + BROWSE_STATE_FILE: stateFile, + BROWSE_SERVER_SCRIPT: stub, + BROWSE_PARENT_PID: '0', + BROWSE_START_TIMEOUT: '8000', + HTTP_PROXY: `http://127.0.0.1:${proxy.port}`, + http_proxy: `http://127.0.0.1:${proxy.port}`, + NO_PROXY: 'corp.internal', + }); + const cli = Bun.spawn([process.execPath, CLI, 'status'], { cwd: scratch, env, stdout: 'pipe', stderr: 'pipe' }); + const timer = setTimeout(() => cli.kill('SIGKILL'), 30_000); + const [code, stdout, stderr] = await Promise.all([cli.exited, new Response(cli.stdout).text(), new Response(cli.stderr).text()]); + clearTimeout(timer); + expect(code, stdout + stderr).toBe(0); + expect(proxied).toBe(0); + expect(JSON.parse(fs.readFileSync(stateFile, 'utf-8')).noProxy).toBe('corp.internal,127.0.0.1,localhost,::1'); + }, 45_000); +}); From 5015a939a9c16decfc8005a5a856cb227584e89f Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 20:55:38 +0000 Subject: [PATCH 06/17] G5: browse type --selector (#2936) --selector focuses the element (sel or @ref) and types into it. The bare form still types its text literally into the focused element; text that starts with -- goes after a -- separator. When the first word looks like a CSS selector, the result adds a hint to use --selector without echoing the typed text, and never refuses based on page contents. Help and command usage updated. Co-authored-by: Thomas Reuser --- browse/src/cli.ts | 2 +- browse/src/commands.ts | 2 +- browse/src/write-commands.ts | 28 +++++++++++++++++++++++++--- browse/test/commands.test.ts | 29 +++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/browse/src/cli.ts b/browse/src/cli.ts index 9f523d6ed..02369ab84 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -1647,7 +1647,7 @@ Usage: browse [args...] Navigation: goto | back | forward | reload | url Content: text | html [sel] | links | forms | accessibility Interaction: click | fill | select - hover | type | press + hover | type [--selector ] | press scroll [sel] | wait | viewport upload [file2...] cookie-import diff --git a/browse/src/commands.ts b/browse/src/commands.ts index 3be130715..9a0bfdf3d 100644 --- a/browse/src/commands.ts +++ b/browse/src/commands.ts @@ -122,7 +122,7 @@ export const COMMAND_DESCRIPTIONS: Record ' }, 'select': { category: 'Interaction', description: 'Select dropdown option by value, label, or visible text', usage: 'select ' }, 'hover': { category: 'Interaction', description: 'Hover element', usage: 'hover ' }, - 'type': { category: 'Interaction', description: 'Type into focused element', usage: 'type ' }, + 'type': { category: 'Interaction', description: 'Send keystrokes to the focused element, or to with --selector (focuses it first). Text that starts with -- goes after a -- separator.', usage: 'type [--selector ] [--] ' }, 'press': { category: 'Interaction', description: 'Press a Playwright keyboard key against the focused element. Names are case-sensitive: Enter, Tab, Escape, ArrowUp/Down/Left/Right, Backspace, Delete, Home, End, PageUp, PageDown. Modifiers combine with +: Shift+Enter, Control+A, Meta+K. Single printable chars (a, A, 1) work too. Full key list: https://playwright.dev/docs/api/class-keyboard#keyboard-press', usage: 'press ' }, 'scroll': { category: 'Interaction', description: 'With a selector, smooth-scrolls the element into view. Without a selector, jumps to page bottom. No --by/--to amount option; for pixel-precise scrolling use `js window.scrollTo(0, N)`.', usage: 'scroll [sel|@ref]' }, 'wait': { category: 'Interaction', description: 'Wait for element, network idle, or page load (timeout: 15s)', usage: 'wait ' }, diff --git a/browse/src/write-commands.ts b/browse/src/write-commands.ts index 2e8109d07..571957a69 100644 --- a/browse/src/write-commands.ts +++ b/browse/src/write-commands.ts @@ -420,10 +420,32 @@ export async function handleWriteCommand( } case 'type': { - const text = args.join(' '); - if (!text) throw new Error('Usage: browse type '); + // #2936: bare `type` sends keystrokes to whatever has focus, so a selector + // given as the first word was typed as text. --selector targets an + // element; `--` ends flags so literal text may start with "--". + const usage = 'Usage: browse type [--selector ] [--] '; + let rest = args; + let selector: string | undefined; + if (rest[0] === '--selector') { + selector = rest[1]; + if (!selector) throw new Error(usage); + rest = rest.slice(2); + } + if (rest[0] === '--') rest = rest.slice(1); + const text = rest.join(' '); + if (!text) throw new Error(usage); + if (selector) { + const resolved = await session.resolveRef(selector); + const locator = 'locator' in resolved ? resolved.locator : target.locator(resolved.selector); + await locator.pressSequentially(text, { timeout: 5000 }); + return `Typed ${text.length} characters into ${selector}`; + } await page.keyboard.type(text); - return `Typed ${text.length} characters`; + const first = rest[0]; + const selectorLike = /^([#.][A-Za-z_-]|\[[^\]]+\]$|[A-Za-z][\w-]*[#.[][A-Za-z_-])/.test(first) || rest.includes('>'); + return selectorLike + ? `Typed ${text.length} characters into the focused element\nhint: "${first}" looks like a CSS selector. To type into that element: browse type --selector '${first}' ` + : `Typed ${text.length} characters`; } case 'press': { diff --git a/browse/test/commands.test.ts b/browse/test/commands.test.ts index 4e649b208..c0f72128f 100644 --- a/browse/test/commands.test.ts +++ b/browse/test/commands.test.ts @@ -559,6 +559,35 @@ describe('Interaction', () => { const val = await handleReadCommand('js', ['document.querySelector("#name").value'], bm); expect(val).toBe('John Doe'); }); + + test('type --selector types into that element, not the focused one (#2936)', async () => { + await handleWriteCommand('goto', [baseUrl + '/forms.html'], bm); + await handleWriteCommand('click', ['#email'], bm); + const result = await handleWriteCommand('type', ['--selector', '#name', 'Jane', 'Roe'], bm); + expect(result).toBe('Typed 8 characters into #name'); + expect(await handleReadCommand('js', ['document.querySelector("#name").value'], bm)).toBe('Jane Roe'); + expect(await handleReadCommand('js', ['document.querySelector("#email").value'], bm)).toBe(''); + }); + + test('bare type keeps typing selector-looking text literally, with a hint and no refusal', async () => { + await handleWriteCommand('goto', [baseUrl + '/forms.html'], bm); + await handleWriteCommand('click', ['#bio'], bm); + const result = await handleWriteCommand('type', ['#name', 'secret-value'], bm); + expect(result).toContain('Typed 18 characters into the focused element'); + expect(result).toContain("hint: \"#name\" looks like a CSS selector. To type into that element: browse type --selector '#name' "); + expect(result).not.toContain('secret-value'); + expect(await handleReadCommand('js', ['document.querySelector("#bio").value'], bm)).toBe('#name secret-value'); + expect(await handleReadCommand('js', ['document.querySelector("#name").value'], bm)).toBe(''); + }); + + test('type -- types text that starts with -- literally; plain text gets no hint', async () => { + await handleWriteCommand('goto', [baseUrl + '/forms.html'], bm); + await handleWriteCommand('click', ['#bio'], bm); + expect(await handleWriteCommand('type', ['--', '--selector', 'x'], bm)).toBe('Typed 12 characters'); + expect(await handleReadCommand('js', ['document.querySelector("#bio").value'], bm)).toBe('--selector x'); + await expect(handleWriteCommand('type', ['--selector'], bm)).rejects.toThrow('Usage: browse type [--selector ] [--] '); + await expect(handleWriteCommand('type', ['--selector', '#name'], bm)).rejects.toThrow('Usage'); + }); }); // ─── SPA / Console / Network ─────────────────────────────────── From 46f4f52f4485e4d9387b844d6f76bd85fd1eee1f Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 20:59:50 +0000 Subject: [PATCH 07/17] E8: headless browse honors GSTACK_CHROMIUM_PATH; extensions load in new headless mode (#2771, #1968, #2281) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The headless launch passes GSTACK_CHROMIUM_PATH as executablePath, scopes the XProtect self-heal out for that bundle, and names the path in a launch failure with the fix. BROWSE_EXTENSIONS_DIR now launches a persistent context under the project state dir with --headless=new, so the extension runs and no window appears (the off-screen window was visible on macOS and the extension never ran in an isolated newContext). The GPU source guard test moves from the removed useHeadless flag to launch() vs launchHeaded(). Co-authored-by: oli548 <312288720+oli548@users.noreply.github.com> Co-authored-by: Slava Co-authored-by: ryanlaiwy <129729476+ryanlaiwy@users.noreply.github.com> Co-authored-by: 李翊楠 --- browse/src/browser-manager.ts | 89 ++++++++++++++++----- browse/test/headless-gpu-and-reap.test.ts | 11 ++- browse/test/headless-launch-options.test.ts | 79 ++++++++++++++++++ 3 files changed, 151 insertions(+), 28 deletions(-) create mode 100644 browse/test/headless-launch-options.test.ts diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index 1f619218e..2d863bd1b 100644 --- a/browse/src/browser-manager.ts +++ b/browse/src/browser-manager.ts @@ -22,7 +22,7 @@ import { addConsoleEntry, addNetworkEntry, addDialogEntry, networkBuffer, type D import { emitActivity } from './activity'; import { validateNavigationUrl, blockedNavigationReason } from './url-validation'; import { TabSession, type RefEntry } from './tab-session'; -import { resolveChromiumProfile, cleanSingletonLocks } from './config'; +import { resolveChromiumProfile, cleanSingletonLocks, resolveConfig } from './config'; import { launchWithXProtectHeal } from './xprotect-heal'; import { readPidStartTime, shouldSpawnXvfb, pickFreeDisplay, spawnXvfb, xvfbInstallHint, type XvfbHandle } from './xvfb'; import { withCdpSession } from './cdp-bridge'; @@ -318,6 +318,8 @@ export class BrowserManager { // ─── Tab Ownership (multi-agent isolation) ────────────── // Maps tabId → clientId. Unowned tabs (not in this map) are root-only for writes. private tabOwnership: Map = new Map(); + /** Headless launch with BROWSE_EXTENSIONS_DIR: a persistent context (#2281). */ + private extensionContext = false; /** Pending and recorded navigation-guard verdicts per page (D2). */ private navigationGuards = new WeakMap>; blocked: string | null }>(); @@ -530,11 +532,22 @@ export class BrowserManager { this.closing = false; // ─── Extension Support ──────────────────────────────────── // BROWSE_EXTENSIONS_DIR points to an unpacked Chrome extension directory. - // Extensions only work in headed mode, so we use an off-screen window. + // Extensions only run in a persistent context (launch() + newContext() + // isolates them), so that path uses launchPersistentContext with Chrome's + // new headless mode: no window, unlike the old off-screen window that + // macOS still showed (#2281, #432). const extensionsDir = process.env.BROWSE_EXTENSIONS_DIR; - const { STEALTH_LAUNCH_ARGS, buildGStackLaunchArgs } = await import('./stealth'); + const { STEALTH_LAUNCH_ARGS, buildGStackLaunchArgs, STEALTH_IGNORE_DEFAULT_ARGS } = await import('./stealth'); const launchArgs: string[] = [...STEALTH_LAUNCH_ARGS, ...buildGStackLaunchArgs()]; - let useHeadless = true; + // #2771/#1968: honor GSTACK_CHROMIUM_PATH headless too (NixOS, macOS 13). + // That bundle belongs to the user, so the XProtect heal never touches it, + // and a launch failure names it. + const executablePath = process.env.GSTACK_CHROMIUM_PATH || undefined; + const nameCustomChromium = (err: unknown): never => { + if (!executablePath) throw err; + const message = err instanceof Error ? err.message : String(err); + throw new Error(`Chromium at GSTACK_CHROMIUM_PATH=${executablePath} failed to launch: ${message}\nFix: point GSTACK_CHROMIUM_PATH at a working Chromium or Chrome binary, or unset it to use Playwright's bundled Chromium.`, { cause: err }); + }; // Docker/CI/root: Chromium sandbox requires unprivileged user namespaces which // are typically disabled in containers and are never available for the root @@ -554,24 +567,59 @@ export class BrowserManager { `--load-extension=${extensionsDir}`, ); } - launchArgs.push('--window-position=-9999,-9999', '--window-size=1,1'); - useHeadless = false; // extensions require headed mode; off-screen window simulates headless + launchArgs.push('--headless=new'); console.log(`[browse] Extensions loaded from: ${extensionsDir}`); } - // #2709: headless-only — the extensions path above forces headed mode, - // and headed/GBrowser sessions must keep the GPU. - if (useHeadless) { - launchArgs.push(...headlessGpuArgs(process.platform, process.env)); + // #2709: headless-only GPU flags; headed/GBrowser sessions keep the GPU. + launchArgs.push(...headlessGpuArgs(process.platform, process.env)); + + const contextOptions: BrowserContextOptions = { + viewport: { width: this.currentViewport.width, height: this.currentViewport.height }, + deviceScaleFactor: this.deviceScaleFactor, + }; + if (this.customUserAgent) { + contextOptions.userAgent = this.customUserAgent; + } + + if (extensionsDir) { + const profileDir = require('path').join(resolveConfig().stateDir, 'chromium-extensions-profile'); + require('fs').mkdirSync(profileDir, { recursive: true, mode: 0o700 }); + cleanSingletonLocks(profileDir); + this.context = await launchWithXProtectHeal(() => chromium.launchPersistentContext(profileDir, { + headless: false, // --headless=new in launchArgs: windowless, extensions run + handleSIGINT: false, + handleSIGTERM: false, + handleSIGHUP: false, + chromiumSandbox: shouldEnableChromiumSandbox(), + args: launchArgs, + ignoreDefaultArgs: STEALTH_IGNORE_DEFAULT_ARGS, + ...contextOptions, + ...(executablePath ? { executablePath } : {}), + ...(this.proxyConfig ? { proxy: this.proxyConfig } : {}), + }), { usesCustomExecutable: Boolean(executablePath) }).catch(nameCustomChromium); + this.extensionContext = true; + this.browser = this.context.browser(); + this.browser?.on('disconnected', () => { + void handleChromiumDisconnect(this.browser); + }); + for (const page of this.context.pages()) await page.close().catch(() => {}); + if (Object.keys(this.extraHeaders).length > 0) { + await this.context.setExtraHTTPHeaders(this.extraHeaders); + } + const { applyStealth } = await import('./stealth'); + await applyStealth(this.context); + await this.newTab(); + return; } // XProtect self-heal wrapper (P0 #2554): a macOS definition update can // start SIGKILLing the pinned Chromium at spawn. On the classified // signature, clear quarantine on the Playwright cache + force-reinstall - // once, then retry this launch once. This headless path always uses the - // Playwright cache (no executablePath), so the heal is never scoped out. + // once, then retry this launch once. A GSTACK_CHROMIUM_PATH bundle is + // scoped out of the heal. this.browser = await launchWithXProtectHeal(() => chromium.launch({ - headless: useHeadless, + headless: true, // #2220: the daemon owns signal policy, not Playwright. Playwright's // default handlers close Chromium the moment THIS process receives // SIGINT/SIGTERM/SIGHUP — which fights the deliberate headless @@ -589,8 +637,9 @@ export class BrowserManager { // namespaces that aren't available. chromiumSandbox: shouldEnableChromiumSandbox(), ...(launchArgs.length > 0 ? { args: launchArgs } : {}), + ...(executablePath ? { executablePath } : {}), ...(this.proxyConfig ? { proxy: this.proxyConfig } : {}), - })); + }), { usesCustomExecutable: Boolean(executablePath) }).catch(nameCustomChromium); // Chromium disconnect → distinguish clean user-quit from crash. Both // events look identical to Playwright (one 'disconnected' fires), but @@ -614,13 +663,6 @@ export class BrowserManager { : null; } - const contextOptions: BrowserContextOptions = { - viewport: { width: this.currentViewport.width, height: this.currentViewport.height }, - deviceScaleFactor: this.deviceScaleFactor, - }; - if (this.customUserAgent) { - contextOptions.userAgent = this.customUserAgent; - } this.context = await this.browser.newContext(contextOptions); if (Object.keys(this.extraHeaders).length > 0) { @@ -956,7 +998,7 @@ export class BrowserManager { (t as { unref?: () => void }).unref?.(); }); if (this.browser || ((this.connectionMode === 'headed' || this.handoffPrevious) && this.context)) { - if (this.connectionMode === 'headed' || this.handoffPrevious) { + if (this.connectionMode === 'headed' || this.handoffPrevious || this.extensionContext) { // Headed/persistent context mode: close the context (which closes the browser) this.intentionalDisconnect = true; if (this.browser) this.browser.removeAllListeners('disconnected'); @@ -1681,6 +1723,9 @@ export class BrowserManager { if (this.connectionMode === 'headed') { throw new Error('Cannot recreate context in headed mode. Use disconnect first.'); } + if (this.extensionContext) { + throw new Error('Cannot recreate context while BROWSE_EXTENSIONS_DIR extensions are loaded (persistent context). Restart browse with the new settings instead.'); + } if (!this.browser || !this.context) { throw new Error('Browser not launched'); } diff --git a/browse/test/headless-gpu-and-reap.test.ts b/browse/test/headless-gpu-and-reap.test.ts index d0dbbf0c0..cef5776e1 100644 --- a/browse/test/headless-gpu-and-reap.test.ts +++ b/browse/test/headless-gpu-and-reap.test.ts @@ -189,11 +189,10 @@ describe('stop-reap wiring pins (#2709)', () => { // unguarded push would strip the GPU from headed/GBrowser sessions. const calls = src.match(/headlessGpuArgs\(process\.platform, process\.env\)/g) ?? []; expect(calls.length).toBe(1); - // And that one call site is guarded by the headless flag: the extensions - // path above it forces useHeadless = false, so extension-loaded and headed - // launches never receive the GPU-disable flags. - expect(src).toMatch( - /if \(useHeadless\) \{\s*\n\s*launchArgs\.push\(\.\.\.headlessGpuArgs\(process\.platform, process\.env\)\);\s*\n\s*\}/, - ); + // And that one call site is in the headless launch(), never in the headed + // launchHeaded(). Since #2281 the extensions path is new headless too + // (--headless=new, no window), so it takes the headless GPU flags. + const headless = src.slice(src.indexOf(' async launch() {'), src.indexOf(' async launchHeaded(')); + expect(headless).toContain('launchArgs.push(...headlessGpuArgs(process.platform, process.env));'); }); }); diff --git a/browse/test/headless-launch-options.test.ts b/browse/test/headless-launch-options.test.ts new file mode 100644 index 000000000..f727c31a0 --- /dev/null +++ b/browse/test/headless-launch-options.test.ts @@ -0,0 +1,79 @@ +/** + * E8 (#2771, #1968) and #2281: the headless launch honors GSTACK_CHROMIUM_PATH + * and names it when it fails; BROWSE_EXTENSIONS_DIR loads extensions in new + * headless mode (no window, extension actually runs) instead of an off-screen + * headed window. + */ +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { chromium } from 'playwright'; +import { BrowserManager } from '../src/browser-manager'; +import { readPidCmdline } from '../src/xvfb'; +import { handleWriteCommand } from '../src/write-commands'; + +const ENV_KEYS = ['GSTACK_CHROMIUM_PATH', 'BROWSE_EXTENSIONS_DIR', 'BROWSE_STATE_FILE'] as const; +let saved: Record; +let scratch: string; +let bm: BrowserManager | null; + +beforeEach(() => { + saved = Object.fromEntries(ENV_KEYS.map(k => [k, process.env[k]])); + scratch = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-launch-opts-')); + process.env.BROWSE_STATE_FILE = path.join(scratch, 'state', 'browse.json'); + bm = null; +}); + +afterEach(async () => { + try { await Promise.race([bm?.close(), new Promise((resolve) => setTimeout(resolve, 3000))]); } catch {} + for (const k of ENV_KEYS) { + if (saved[k] === undefined) delete process.env[k]; + else process.env[k] = saved[k]; + } + fs.rmSync(scratch, { recursive: true, force: true }); +}); + +describe.skipIf(process.platform !== 'linux')('headless launch options (Linux, real Chromium)', () => { + test('GSTACK_CHROMIUM_PATH is the binary the headless launch runs', async () => { + const custom = chromium.executablePath(); + process.env.GSTACK_CHROMIUM_PATH = custom; + bm = new BrowserManager(); + await bm.launch(); + const children = fs.readdirSync('/proc').filter(d => /^\d+$/.test(d)).filter(d => { + try { return fs.readFileSync(`/proc/${d}/stat`, 'utf-8').split(') ')[1].split(' ')[1] === String(process.pid); } catch { return false; } + }); + const cmdlines = children.map(pid => readPidCmdline(Number(pid))); + expect(cmdlines.some(c => c.startsWith(custom) && c.includes('--headless'))).toBe(true); + expect(cmdlines.some(c => c.includes('headless_shell'))).toBe(false); + }, 60_000); + + test('a broken GSTACK_CHROMIUM_PATH fails with the path named', async () => { + const missing = path.join(scratch, 'no-such-chromium'); + process.env.GSTACK_CHROMIUM_PATH = missing; + bm = new BrowserManager(); + await expect(bm.launch()).rejects.toThrow(`Chromium at GSTACK_CHROMIUM_PATH=${missing} failed to launch`); + }, 60_000); + + test('BROWSE_EXTENSIONS_DIR runs the extension in new headless mode', async () => { + const ext = path.join(scratch, 'ext'); + fs.mkdirSync(ext); + fs.writeFileSync(path.join(ext, 'manifest.json'), JSON.stringify({ + manifest_version: 3, name: 'probe', version: '1', + content_scripts: [{ matches: [''], js: ['c.js'], run_at: 'document_start' }], + })); + fs.writeFileSync(path.join(ext, 'c.js'), 'document.documentElement.setAttribute("data-probe-ext", "1");'); + process.env.BROWSE_EXTENSIONS_DIR = ext; + const server = Bun.serve({ hostname: '127.0.0.1', port: 0, fetch: () => new Response('

x

', { headers: { 'Content-Type': 'text/html' } }) }); + try { + bm = new BrowserManager(); + await bm.launch(); + const session = bm.getActiveSession(); + await handleWriteCommand('goto', [`http://127.0.0.1:${server.port}/`], session, bm); + expect(await session.getPage().evaluate(() => document.documentElement.getAttribute('data-probe-ext'))).toBe('1'); + expect(fs.existsSync(path.join(scratch, 'state', 'chromium-extensions-profile'))).toBe(true); + } finally { + server.stop(true); + } + }, 60_000); +}); From 5f38b93fdfac46775a9fcc52fd1fec6a659519dd Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 21:03:24 +0000 Subject: [PATCH 08/17] E4: Windows node-server bundle keeps sharp and socks external; CLI resolves its server script lazily (#2260, #2439) build-node-server.sh passes --external sharp and --external socks (Bun on Windows refuses to inline them; on Linux they were silently inlined). The CLI no longer resolves server.ts or server-node.mjs at module load: resolveServerLaunch() runs when a server starts, so a Windows minimal runtime with only server-node.mjs works and a missing script is reported only when one is needed. Co-authored-by: Spooks444 <193448029+Spooks444@users.noreply.github.com> Co-authored-by: punksterlabs <77217235+punksterlabs@users.noreply.github.com> --- browse/scripts/build-node-server.sh | 8 +++- browse/src/cli.ts | 35 ++++++++------ browse/test/build.test.ts | 20 ++++++++ browse/test/server-launch-resolution.test.ts | 49 ++++++++++++++++++++ 4 files changed, 97 insertions(+), 15 deletions(-) create mode 100644 browse/test/server-launch-resolution.test.ts diff --git a/browse/scripts/build-node-server.sh b/browse/scripts/build-node-server.sh index 0694782a7..7603c9fad 100755 --- a/browse/scripts/build-node-server.sh +++ b/browse/scripts/build-node-server.sh @@ -21,7 +21,9 @@ echo "Building Node-compatible server bundle..." # Step 1: Transpile server.ts to a single .mjs bundle (externalize runtime deps) # -# Externalize packages with native addons, dynamic imports, or runtime resolution. +# Externalize packages with native addons, dynamic imports, or runtime resolution +# (sharp loads a platform .node addon; socks is resolved at runtime, and Bun on +# Windows refuses to inline either, #2260). # If you add a new dependency that uses `await import()` or has a .node addon, # add it here. Otherwise `bun build --outfile` will fail with # "cannot write multiple output files without an output directory". @@ -32,7 +34,9 @@ bun build "$SRC_DIR/server.ts" \ --external playwright-core \ --external diff \ --external "bun:sqlite" \ - --external "@ngrok/ngrok" + --external "@ngrok/ngrok" \ + --external socks \ + --external sharp # Step 2: Post-process # Replace import.meta.dir with a resolvable reference diff --git a/browse/src/cli.ts b/browse/src/cli.ts index 02369ab84..cba6c1ab1 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -84,8 +84,6 @@ export function resolveServerScript( ); } -const SERVER_SCRIPT = resolveServerScript(); - /** * On Windows, resolve the Node.js-compatible server bundle. * Falls back to null if not found (server will use Bun instead). @@ -109,13 +107,23 @@ export function resolveNodeServerScript( return null; } -const NODE_SERVER_SCRIPT = IS_WINDOWS ? resolveNodeServerScript() : null; - -// On Windows, hard-fail if server-node.mjs is missing — the Bun path is known broken. -if (IS_WINDOWS && !NODE_SERVER_SCRIPT) { - throw new Error( - 'server-node.mjs not found. Run `bun run build` to generate the Windows server bundle.' - ); +/** + * Which server to start, resolved only when a server is actually started + * (#2439). Windows runs the Node bundle and never needs server.ts, which a + * minimal runtime root does not ship; resolving server.ts at module load made + * every command, even --help, fail there. + */ +export function resolveServerLaunch( + platform: NodeJS.Platform = process.platform, + env: Record = process.env, + metaDir: string = import.meta.dir, + execPath: string = process.execPath, +): { runtime: 'node' | 'bun'; script: string } { + if (platform !== 'win32') return { runtime: 'bun', script: resolveServerScript(env, metaDir, execPath) }; + // On Windows, hard-fail if server-node.mjs is missing — the Bun path is known broken. + const script = resolveNodeServerScript(metaDir, execPath); + if (!script) throw new Error('server-node.mjs not found. Run `bun run build` to generate the Windows server bundle.'); + return { runtime: 'node', script }; } interface ServerState { @@ -591,6 +599,7 @@ function openDaemonLogSink(): number | 'ignore' { } async function startServer(extraEnv?: Record): Promise { + const server = resolveServerLaunch(); ensureStateDir(config); // Bound the append-mode daemon log before the new daemon starts writing. @@ -625,7 +634,7 @@ async function startServer(extraEnv?: Record): Promise): Promise): Promise): Promise readPidStartTime(pid) === startTime && readPidCmdline(pid).split(/\s+/).includes(SERVER_SCRIPT); + const stillOurs = () => readPidStartTime(pid) === startTime && readPidCmdline(pid).split(/\s+/).includes(server.script); if (stillOurs()) { safeKill(pid, 'SIGTERM'); const deadline = Date.now() + 500; diff --git a/browse/test/build.test.ts b/browse/test/build.test.ts index 38e3b1891..f194d3387 100644 --- a/browse/test/build.test.ts +++ b/browse/test/build.test.ts @@ -2,6 +2,7 @@ import { describe, test, expect } from 'bun:test'; import { execSync } from 'child_process'; import * as fs from 'fs'; import * as path from 'path'; +import * as os from 'os'; const DIST_DIR = path.resolve(__dirname, '..', 'dist'); const SERVER_NODE = path.join(DIST_DIR, 'server-node.mjs'); @@ -26,3 +27,22 @@ describe('build: server-node.mjs', () => { expect(bundle).not.toMatch(/ngrok_napi|ngrokNapi|@ngrok\/ngrok-darwin|@ngrok\/ngrok-linux|@ngrok\/ngrok-win32/); }); }); + +describe('build: node server bundle externals (#2260)', () => { + test('sharp and socks stay external, so their native and runtime-resolved code is loaded from node_modules', () => { + const script = fs.readFileSync(path.resolve(__dirname, '..', 'scripts', 'build-node-server.sh'), 'utf-8'); + const externals = [...script.matchAll(/--external\s+"?([^"\s\\]+)"?/g)].map((m) => m[1]); + expect(externals).toEqual(expect.arrayContaining(['sharp', 'socks'])); + const out = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-node-bundle-')); + try { + const outfile = path.join(out, 'server-node.mjs'); + const r = Bun.spawnSync([process.execPath, 'build', path.resolve(__dirname, '..', 'src', 'server.ts'), '--target=node', '--outfile', outfile, ...externals.flatMap((e) => ['--external', e])], { stdout: 'pipe', stderr: 'pipe', timeout: 120_000 }); + expect(r.exitCode, r.stderr.toString()).toBe(0); + const bundle = fs.readFileSync(outfile, 'utf-8'); + expect(bundle).not.toContain('node_modules/sharp/'); + expect(bundle).not.toContain('node_modules/socks/'); + } finally { + fs.rmSync(out, { recursive: true, force: true }); + } + }, 150_000); +}); diff --git a/browse/test/server-launch-resolution.test.ts b/browse/test/server-launch-resolution.test.ts new file mode 100644 index 000000000..3f62ae17b --- /dev/null +++ b/browse/test/server-launch-resolution.test.ts @@ -0,0 +1,49 @@ +/** + * E4 (#2439): the CLI resolves its server script only when it starts a + * server. A Windows minimal runtime ships browse.exe + server-node.mjs and no + * browse/src/server.ts, which used to make every command fail at import. + */ +import { describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { resolveServerLaunch } from '../src/cli'; + +function withRuntime(files: string[], fn: (dir: string) => T): T { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-runtime-')); + try { + for (const f of files) fs.writeFileSync(path.join(dir, f), '// stub\n'); + return fn(dir); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +} + +const COMPILED_META = path.join(os.tmpdir(), '$bunfs', 'root'); + +describe('resolveServerLaunch', () => { + test('Windows minimal runtime starts the adjacent Node bundle without server.ts', () => { + withRuntime(['server-node.mjs'], (dir) => { + expect(resolveServerLaunch('win32', {}, COMPILED_META, path.join(dir, 'browse.exe'))) + .toEqual({ runtime: 'node', script: path.join(dir, 'server-node.mjs') }); + }); + }); + + test('Windows without the Node bundle names the build fix', () => { + withRuntime([], (dir) => { + expect(() => resolveServerLaunch('win32', {}, COMPILED_META, path.join(dir, 'browse.exe'))) + .toThrow('server-node.mjs not found. Run `bun run build`'); + }); + }); + + test('macOS/Linux start server.ts with Bun, honoring BROWSE_SERVER_SCRIPT', () => { + expect(resolveServerLaunch('linux', { BROWSE_SERVER_SCRIPT: 'custom-server.ts' }, COMPILED_META, '')) + .toEqual({ runtime: 'bun', script: 'custom-server.ts' }); + }); + + test('importing the CLI resolves nothing: a missing server is reported only when one is started', () => { + const src = fs.readFileSync(path.join(import.meta.dir, '..', 'src', 'cli.ts'), 'utf-8'); + expect(src).not.toMatch(/^const \w+ = resolve(Server|NodeServer)\w*\(/m); + expect(src).toMatch(/async function startServer\([^)]*\)[^{]*\{\n\s*const server = resolveServerLaunch\(\);/); + }); +}); From 3f712a4e60f7e9bf59035dce7fafe99440666b7b Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 21:07:56 +0000 Subject: [PATCH 09/17] E8: keep the extensions profile under the state dir at a path lock cleanup accepts cleanSingletonLocks only cleans directories named chromium-profile, so the BROWSE_EXTENSIONS_DIR profile moves to /extensions/chromium-profile. --- browse/src/browser-manager.ts | 2 +- browse/test/headless-launch-options.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index 2d863bd1b..03410fda5 100644 --- a/browse/src/browser-manager.ts +++ b/browse/src/browser-manager.ts @@ -583,7 +583,7 @@ export class BrowserManager { } if (extensionsDir) { - const profileDir = require('path').join(resolveConfig().stateDir, 'chromium-extensions-profile'); + const profileDir = require('path').join(resolveConfig().stateDir, 'extensions', 'chromium-profile'); require('fs').mkdirSync(profileDir, { recursive: true, mode: 0o700 }); cleanSingletonLocks(profileDir); this.context = await launchWithXProtectHeal(() => chromium.launchPersistentContext(profileDir, { diff --git a/browse/test/headless-launch-options.test.ts b/browse/test/headless-launch-options.test.ts index f727c31a0..dbe40b26b 100644 --- a/browse/test/headless-launch-options.test.ts +++ b/browse/test/headless-launch-options.test.ts @@ -71,7 +71,7 @@ describe.skipIf(process.platform !== 'linux')('headless launch options (Linux, r const session = bm.getActiveSession(); await handleWriteCommand('goto', [`http://127.0.0.1:${server.port}/`], session, bm); expect(await session.getPage().evaluate(() => document.documentElement.getAttribute('data-probe-ext'))).toBe('1'); - expect(fs.existsSync(path.join(scratch, 'state', 'chromium-extensions-profile'))).toBe(true); + expect(fs.existsSync(path.join(scratch, 'state', 'extensions', 'chromium-profile'))).toBe(true); } finally { server.stop(true); } From 88b480eaedc42fd0a27b1a09f85c19442feb0d65 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 21:11:02 +0000 Subject: [PATCH 10/17] D5: per-project headed Chromium profiles; never kill a live profile owner (#2492) The headed profile moves from /chromium-profile to /.gstack/chromium-profile (CHROMIUM_PROFILE still wins). killOrphanChromium now kills a SingletonLock PID only when the lock names this host, the process is a Chromium using that profile, and its daemon is gone (reparented to init or a subreaper); a live owner or another host's lock stops browse with the holder named. On first headed use a project profile is seeded from the old shared profile only when nothing holds it: copied without Singleton* files into a 0700 temp dir and renamed into place, with a one-time notice; otherwise it starts fresh and says how to import later. New: browse profiles [list] and browse profiles prune [--days N] (never removes a profile in use or the old shared one). The #2817 headed-startup tests move to the project profile and a real orphan (the old stand-in was a live child of the test). Co-authored-by: miao --- browse/src/browser-manager.ts | 3 + browse/src/chromium-profiles.ts | 208 ++++++++++++++++++ browse/src/cli.ts | 35 +-- browse/src/config.ts | 8 +- .../test/chromium-profile-isolation.test.ts | 195 +++++++++++++--- browse/test/config.test.ts | 8 +- 6 files changed, 405 insertions(+), 52 deletions(-) create mode 100644 browse/src/chromium-profiles.ts diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index 03410fda5..e310c515c 100644 --- a/browse/src/browser-manager.ts +++ b/browse/src/browser-manager.ts @@ -23,6 +23,7 @@ import { emitActivity } from './activity'; import { validateNavigationUrl, blockedNavigationReason } from './url-validation'; import { TabSession, type RefEntry } from './tab-session'; import { resolveChromiumProfile, cleanSingletonLocks, resolveConfig } from './config'; +import { ensureProjectProfile } from './chromium-profiles'; import { launchWithXProtectHeal } from './xprotect-heal'; import { readPidStartTime, shouldSpawnXvfb, pickFreeDisplay, spawnXvfb, xvfbInstallHint, type XvfbHandle } from './xvfb'; import { withCdpSession } from './cdp-bridge'; @@ -750,6 +751,7 @@ export class BrowserManager { const fs = require('fs'); const path = require('path'); const userDataDir = resolveChromiumProfile(); + ensureProjectProfile(userDataDir); fs.mkdirSync(userDataDir, { recursive: true }); // Pre-launch cleanup of stale SingletonLock/Socket/Cookie. Chromium's @@ -1913,6 +1915,7 @@ export class BrowserManager { // ignoring $CHROMIUM_PROFILE / $GSTACK_HOME and skipping the lock // cleanup — the third shipped drift between the three launch paths. const userDataDir = resolveChromiumProfile(); + ensureProjectProfile(userDataDir); fs.mkdirSync(userDataDir, { recursive: true }); cleanSingletonLocks(userDataDir); diff --git a/browse/src/chromium-profiles.ts b/browse/src/chromium-profiles.ts new file mode 100644 index 000000000..b3cfe6666 --- /dev/null +++ b/browse/src/chromium-profiles.ts @@ -0,0 +1,208 @@ +/** + * Per-project headed Chromium profiles (D5, #2492). + * + * The headed profile used to be machine-wide (/chromium-profile), + * so starting headed browse in one project SIGKILLed another project's live + * headed Chromium. Each project now gets /.gstack/chromium-profile + * (config.resolveChromiumProfile). This module owns the profile lifecycle: + * who holds a profile's SingletonLock, first-use seeding from the old + * machine-wide profile, and `browse profiles list|prune`. + */ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { resolveGstackHome } from './config'; +import { isProcessAlive } from './error-handling'; +import { readPidCmdline } from './xvfb'; +import { atomicWriteSync } from '../../lib/fs-atomic'; + +export type ProfileOwner = + | { state: 'free' } + | { state: 'orphan'; pid: number } + | { state: 'live'; pid: number; detail: string }; + +/** Processes an orphan is reparented to: init, launchd, and common subreapers. */ +const REAPERS = new Set(['init', 'systemd', 'launchd', 'tini', 'dumb-init', 'docker-init']); + +export function legacyProfileDir(): string { + return path.join(resolveGstackHome(), 'chromium-profile'); +} + +function registryPath(): string { + return path.join(resolveGstackHome(), 'browse-profiles.json'); +} + +function readParentPid(pid: number): number { + try { + const stat = fs.readFileSync(`/proc/${pid}/stat`, 'utf-8'); + return Number(stat.slice(stat.lastIndexOf(')') + 2).split(' ')[1]); + } catch { + try { + const r = Bun.spawnSync(['ps', '-o', 'ppid=', '-p', String(pid)], { stdout: 'pipe', stderr: 'pipe', timeout: 2000, windowsHide: true }); + return r.exitCode === 0 ? Number(r.stdout.toString().trim()) : 0; + } catch { + return 0; + } + } +} + +/** + * Who holds this profile. The SingletonLock symlink target is "host-PID". + * Only a process on this host that is a Chromium using this exact profile + * counts as the holder; a dead or recycled PID means the lock is stale. An + * orphan (reparented to init or a subreaper after its daemon died) may be + * reaped; anything else, including a lock from another host, is a live owner + * that must never be killed. + */ +export function profileOwner(profileDir: string): ProfileOwner { + let target: string; + try { + target = fs.readlinkSync(path.join(profileDir, 'SingletonLock')); + } catch { + return { state: 'free' }; + } + const dash = target.lastIndexOf('-'); + const host = target.slice(0, dash); + const pid = Number(target.slice(dash + 1)); + if (!Number.isSafeInteger(pid) || pid <= 0) return { state: 'free' }; + if (host !== os.hostname()) { + return { state: 'live', pid, detail: `locked by PID ${pid} on another host (${host})` }; + } + if (!isProcessAlive(pid)) return { state: 'free' }; + const cmd = readPidCmdline(pid); + const dirs = new Set([profileDir]); + try { dirs.add(fs.realpathSync(profileDir)); } catch {} + const usesProfile = [...dirs].some(d => cmd.includes(`--user-data-dir=${d}`)); + if (!/chrom/i.test(cmd) || !usesProfile) return { state: 'free' }; + const ppid = readParentPid(pid); + const parent = ppid > 1 ? path.basename(readPidCmdline(ppid).split(' ')[0] || '') : ''; + if (ppid <= 1 || !isProcessAlive(ppid) || REAPERS.has(parent)) return { state: 'orphan', pid }; + return { state: 'live', pid, detail: `in use by Chromium PID ${pid} (started by PID ${ppid}: ${parent || 'unknown'})` }; +} + +function readRegistry(): string[] { + try { + const list = JSON.parse(fs.readFileSync(registryPath(), 'utf-8')); + return Array.isArray(list) ? list.filter((p): p is string => typeof p === 'string') : []; + } catch { + return []; + } +} + +function writeRegistry(list: string[]): void { + fs.mkdirSync(path.dirname(registryPath()), { recursive: true }); + atomicWriteSync(registryPath(), JSON.stringify([...new Set(list)].sort(), null, 2) + '\n'); +} + +/** + * Create a project's headed profile on first use. Seeds it from the old + * machine-wide profile only when nothing holds that profile, copying into a + * temporary 0700 directory without Singleton* files and renaming it into + * place, so a concurrent first use or an interrupted copy never leaves a + * half-written profile. Says what happened once, through `log`. + */ +export function ensureProjectProfile(profileDir: string, log: (msg: string) => void = console.log): void { + if (fs.existsSync(profileDir)) return; + const legacy = legacyProfileDir(); + if (path.resolve(profileDir) === path.resolve(legacy)) return; + writeRegistry([...readRegistry(), path.resolve(profileDir)]); + fs.mkdirSync(path.dirname(profileDir), { recursive: true }); + const fresh = (why: string) => { + fs.mkdirSync(profileDir, { recursive: true, mode: 0o700 }); + if (why) log(why); + }; + if (!fs.existsSync(legacy)) return fresh(''); + const before = profileOwner(legacy); + if (before.state !== 'free') { + return fresh(`[browse] Headed browse now keeps a profile per project (${profileDir}). The old shared profile (${legacy}) is ${'detail' in before ? before.detail : `held by orphaned PID ${before.pid}`}, so this project starts with a fresh profile. To import it later: close that browser, delete ${profileDir}, and start headed browse again.`); + } + const tmp = `${profileDir}.seed-${process.pid}`; + fs.rmSync(tmp, { recursive: true, force: true }); + try { + fs.cpSync(legacy, tmp, { + recursive: true, + verbatimSymlinks: true, + filter: (src) => !path.basename(src).startsWith('Singleton'), + }); + fs.chmodSync(tmp, 0o700); + if (profileOwner(legacy).state !== 'free') { + fs.rmSync(tmp, { recursive: true, force: true }); + return fresh(`[browse] The old shared profile (${legacy}) came into use while it was being copied, so this project (${profileDir}) starts with a fresh profile.`); + } + fs.renameSync(tmp, profileDir); + } catch (err) { + fs.rmSync(tmp, { recursive: true, force: true }); + if (fs.existsSync(profileDir)) return; + throw err; + } + log(`[browse] Headed browse now keeps a profile per project. Copied your logins and settings from ${legacy} to ${profileDir} (${formatBytes(dirBytes(profileDir))}). See \`browse profiles\` to list or prune profiles.`); +} + +function dirBytes(dir: string): number { + let total = 0; + const walk = (d: string) => { + for (const e of fs.readdirSync(d, { withFileTypes: true })) { + const p = path.join(d, e.name); + if (e.isDirectory()) walk(p); + else if (e.isFile()) total += fs.statSync(p).size; + } + }; + try { walk(dir); } catch {} + return total; +} + +function formatBytes(n: number): string { + if (n < 1024 * 1024) return `${Math.round(n / 1024)} KB`; + return `${(n / (1024 * 1024)).toFixed(1)} MB`; +} + +function describeOwner(owner: ProfileOwner): string { + if (owner.state === 'free') return 'idle'; + if (owner.state === 'orphan') return `held by orphaned PID ${owner.pid}`; + return owner.detail; +} + +/** `browse profiles [list]` and `browse profiles prune [--days N]`. Returns the exit code. */ +export function runProfilesCommand(args: string[], out: (line: string) => void = console.log): number { + const sub = args[0] ?? 'list'; + const known = readRegistry(); + const legacy = legacyProfileDir(); + if (sub === 'list') { + const rows = [...(fs.existsSync(legacy) ? [legacy] : []), ...known]; + if (rows.length === 0) { + out('No headed browser profiles yet. One is created per project on first `browse --headed` or `browse connect`.'); + return 0; + } + for (const dir of rows) { + if (!fs.existsSync(dir)) { out(`${dir} (missing: project moved or deleted; \`browse profiles prune\` forgets it)`); continue; } + const age = Math.floor((Date.now() - fs.statSync(dir).mtimeMs) / 86_400_000); + out(`${dir} ${formatBytes(dirBytes(dir))} last used ${age}d ago ${describeOwner(profileOwner(dir))}${dir === legacy ? ' (old shared profile)' : ''}`); + } + return 0; + } + if (sub === 'prune') { + const daysIdx = args.indexOf('--days'); + const days = daysIdx >= 0 ? Number(args[daysIdx + 1]) : 30; + if (!Number.isFinite(days) || days < 0) { + out('Usage: browse profiles prune [--days N] (default 30)'); + return 2; + } + const keep: string[] = []; + let freed = 0; + for (const dir of known) { + if (!fs.existsSync(dir)) { out(`forgot ${dir} (no longer exists)`); continue; } + const idleDays = (Date.now() - fs.statSync(dir).mtimeMs) / 86_400_000; + const owner = profileOwner(dir); + if (idleDays < days || owner.state !== 'free') { keep.push(dir); continue; } + const bytes = dirBytes(dir); + fs.rmSync(dir, { recursive: true, force: true }); + freed += bytes; + out(`removed ${dir} (${formatBytes(bytes)}, idle ${Math.floor(idleDays)}d)`); + } + writeRegistry(keep); + out(`Pruned profiles idle for ${days}+ days; freed ${formatBytes(freed)}. Profiles in use are never removed. The old shared profile (${legacy}) is left alone.`); + return 0; + } + out('Usage: browse profiles [list] | browse profiles prune [--days N]'); + return 2; +} diff --git a/browse/src/cli.ts b/browse/src/cli.ts index cba6c1ab1..5e7c0d0e7 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -19,6 +19,7 @@ import { resolveConfig, ensureStateDir, readVersionHash, isPairAgentEnabled, res import { parseProxyConfig, computeConfigHash, ProxyConfigError } from './proxy-config'; import { redactProxyUrl } from './proxy-redact'; import { spawnTerminalAgent, BUN_CHILD_FLAGS } from './terminal-agent-control'; +import { profileOwner, ensureProjectProfile, runProfilesCommand } from './chromium-profiles'; // Zero side effects on import (documented invariant in token-registry.ts) — // safe to pull the shared pairing default into the CLI. import { DEFAULT_PAIR_SCOPES } from './token-registry'; @@ -285,23 +286,21 @@ function cleanChromiumProfileLocks(profileDir: string = chromiumProfileDir()): v } } -/** Kill an orphaned Chromium that still holds the profile's SingletonLock. The - * lock symlink target is "hostname-PID"; killing that PID tears down its - * renderer tree so the next launch starts clean. No-op when absent/stale. */ +/** Kill an orphaned Chromium that still holds the profile's SingletonLock so + * the next launch starts clean (#1781). Only a Chromium on this host that uses + * this profile and whose daemon is gone is killed; a profile held by a live + * process is never killed — browse stops and names the holder (D5, #2492). */ async function killOrphanChromium(profileDir: string = chromiumProfileDir()): Promise { - try { - const lockTarget = fs.readlinkSync(path.join(profileDir, 'SingletonLock')); // "hostname-12345" - const orphanPid = parseInt(lockTarget.split('-').pop() || '', 10); - if (orphanPid && isProcessAlive(orphanPid)) { - safeKill(orphanPid, 'SIGTERM'); - await new Promise(r => setTimeout(r, 1000)); - if (isProcessAlive(orphanPid)) { - safeKill(orphanPid, 'SIGKILL'); - await new Promise(r => setTimeout(r, 500)); - } - } - } catch (err: any) { - if (err?.code !== 'ENOENT' && err?.code !== 'EINVAL') throw err; + const owner = profileOwner(profileDir); + if (owner.state === 'free') return; + if (owner.state === 'live') { + throw new Error(`Headed Chromium profile ${profileDir} is ${owner.detail}. Close that browser first, or set CHROMIUM_PROFILE to a different directory.`); + } + safeKill(owner.pid, 'SIGTERM'); + await new Promise(r => setTimeout(r, 1000)); + if (isProcessAlive(owner.pid)) { + safeKill(owner.pid, 'SIGKILL'); + await new Promise(r => setTimeout(r, 500)); } } @@ -621,6 +620,7 @@ async function startServer(extraEnv?: Record): Promise | newtab [url] | closetab [id] Server: status | cookie = | header : useragent | stop | restart + profiles [list] | profiles prune [--days N] (per-project headed profiles) tunnel revoke | tunnel agents (paired-agent tokens) --force-restart: replace a live-but-busy daemon (any command; LOSES tabs/cookies/logins — never done automatically) @@ -1693,6 +1694,8 @@ Refs: After 'snapshot', use @e1, @e2... as selectors: const command = args[0]; const commandArgs = args.slice(1); + if (command === 'profiles') process.exit(runProfilesCommand(commandArgs)); + // ─── Headed Connect (pre-server command) ──────────────────── // connect must be handled BEFORE ensureServer() because it needs // to restart the server in headed mode with the Chrome extension. diff --git a/browse/src/config.ts b/browse/src/config.ts index a9080e89e..89cd21418 100644 --- a/browse/src/config.ts +++ b/browse/src/config.ts @@ -250,19 +250,21 @@ export function isPairAgentEnabled(): boolean { } /** - * Resolve the Chromium profile directory. + * Resolve the headed Chromium profile directory. * * Resolution order: * 1. `explicit` arg (no production caller passes one today; kept for * direct programmatic use) * 2. CHROMIUM_PROFILE env (used by gbrowser's gbd per-workspace) - * 3. /chromium-profile (default) + * 3. /chromium-profile (default; per project since D5, + * #2492 — a machine-wide profile let one project's headed launch kill + * another project's live browser) */ export function resolveChromiumProfile(explicit?: string): string { if (explicit && explicit.length > 0) return explicit; const env = process.env.CHROMIUM_PROFILE; if (env && env.length > 0) return env; - return path.join(resolveGstackHome(), 'chromium-profile'); + return path.join(resolveConfig().stateDir, 'chromium-profile'); } /** diff --git a/browse/test/chromium-profile-isolation.test.ts b/browse/test/chromium-profile-isolation.test.ts index d35eda405..57fdcc036 100644 --- a/browse/test/chromium-profile-isolation.test.ts +++ b/browse/test/chromium-profile-isolation.test.ts @@ -52,7 +52,9 @@ describe('Chromium profile isolation cleanup (#2908)', () => { describe.skipIf(process.platform === 'win32')('Chromium profile isolation (#2817)', () => { let scratch: string; let profile: string; + let projectProfile: string; let stateFile: string; + let orphans: number[]; let env: Record; let children: ReturnType[]; let daemonPid: number | undefined; @@ -61,6 +63,7 @@ describe.skipIf(process.platform === 'win32')('Chromium profile isolation (#2817 scratch = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-profile-isolation-')); profile = path.join(scratch, '.gstack', 'chromium-profile'); stateFile = path.join(scratch, 'project-b', '.gstack', 'browse.json'); + projectProfile = path.join(scratch, 'project-b', '.gstack', 'chromium-profile'); fs.mkdirSync(profile, { recursive: true }); fs.mkdirSync(path.dirname(stateFile), { recursive: true }); env = {}; @@ -83,6 +86,7 @@ describe.skipIf(process.platform === 'win32')('Chromium profile isolation (#2817 BROWSE_START_TIMEOUT: '30000', }); children = []; + orphans = []; daemonPid = undefined; }); @@ -92,14 +96,16 @@ describe.skipIf(process.platform === 'win32')('Chromium profile isolation (#2817 } if (daemonPid) killDaemonGroup(daemonPid); for (const child of children) child.kill('SIGKILL'); + for (const pid of orphans) { try { process.kill(pid, 'SIGKILL'); } catch {} } await Promise.all(children.map(child => child.exited)); fs.rmSync(scratch, { recursive: true, force: true }); }); - async function spawnChromiumHolder() { + /** A stand-in Chromium: its command line names Chromium and the profile it uses. */ + async function spawnChromiumHolder(userDataDir = profile) { const script = path.join(scratch, `chromium-holder-${children.length}.ts`); fs.writeFileSync(script, 'console.log("ready"); await Bun.sleep(60000);'); - const child = Bun.spawn([process.execPath, script], { + const child = Bun.spawn([process.execPath, script, `--user-data-dir=${userDataDir}`], { env, stdin: 'ignore', stdout: 'pipe', @@ -113,17 +119,66 @@ describe.skipIf(process.platform === 'win32')('Chromium profile isolation (#2817 return child; } - function seedLocks(pid: number) { - fs.symlinkSync(`${os.hostname()}-${pid}`, path.join(profile, LOCKS[0])); - fs.symlinkSync('socket-target', path.join(profile, LOCKS[1])); - fs.symlinkSync('cookie-target', path.join(profile, LOCKS[2])); + /** A stand-in Chromium whose daemon is gone: reparented to init. */ + async function spawnOrphanedChromium(userDataDir: string) { + const script = path.join(scratch, `chromium-orphan-${orphans.length}.ts`); + const ready = `${script}.ready`; + fs.writeFileSync(script, `require('node:fs').writeFileSync(${JSON.stringify(ready)}, 'ready'); await Bun.sleep(60000);`); + const launcher = Bun.spawnSync(['sh', '-c', `"$0" "$1" "$2" >/dev/null 2>&1 & echo $!`, process.execPath, script, `--user-data-dir=${userDataDir}`], { env, stdout: 'pipe', timeout: 10_000 }); + const pid = Number(launcher.stdout.toString().trim()); + orphans.push(pid); + const deadline = Date.now() + 10_000; + while (!fs.existsSync(ready) && Date.now() < deadline) await Bun.sleep(20); + expect(fs.existsSync(ready)).toBe(true); + return { pid }; } - function expectLocksIntact(pid: number) { + function seedLocks(pid: number, dir = profile, host = os.hostname()) { + fs.mkdirSync(dir, { recursive: true }); + fs.symlinkSync(`${host}-${pid}`, path.join(dir, LOCKS[0])); + fs.symlinkSync('socket-target', path.join(dir, LOCKS[1])); + fs.symlinkSync('cookie-target', path.join(dir, LOCKS[2])); + } + + function expectLocksIntact(pid: number, dir = profile, host = os.hostname()) { expect(isProcessAlive(pid)).toBe(true); - expect(fs.readlinkSync(path.join(profile, LOCKS[0]))).toBe(`${os.hostname()}-${pid}`); - expect(fs.readlinkSync(path.join(profile, LOCKS[1]))).toBe('socket-target'); - expect(fs.readlinkSync(path.join(profile, LOCKS[2]))).toBe('cookie-target'); + expect(fs.readlinkSync(path.join(dir, LOCKS[0]))).toBe(`${host}-${pid}`); + expect(fs.readlinkSync(path.join(dir, LOCKS[1]))).toBe('socket-target'); + expect(fs.readlinkSync(path.join(dir, LOCKS[2]))).toBe('cookie-target'); + } + + function writeStubServer() { + const serverScript = path.join(scratch, 'stub-server.ts'); + fs.writeFileSync(serverScript, ` + import * as fs from 'node:fs'; + const server = Bun.serve({ + hostname: '127.0.0.1', port: 0, + fetch: () => Response.json({ status: 'healthy' }), + }); + fs.writeFileSync(process.env.BROWSE_STATE_FILE!, JSON.stringify({ + pid: process.pid, port: server.port, token: 'test-token', + mode: process.env.BROWSE_HEADED === '1' ? 'headed' : 'launched', + })); + `); + env.BROWSE_SERVER_SCRIPT = serverScript; + } + + async function runCliRaw(args: string[]) { + const child = Bun.spawn([process.execPath, CLI, ...args], { + cwd: path.join(scratch, 'project-b'), + env, + stdin: 'ignore', + stdout: 'pipe', + stderr: 'pipe', + }); + children.push(child); + const [code, stdout, stderr] = await Promise.all([ + child.exited, + new Response(child.stdout).text(), + new Response(child.stderr).text(), + ]); + if (fs.existsSync(stateFile)) daemonPid = JSON.parse(fs.readFileSync(stateFile, 'utf-8')).pid; + return { code, stdout, stderr }; } async function runCli(args: string[]) { @@ -176,34 +231,108 @@ describe.skipIf(process.platform === 'win32')('Chromium profile isolation (#2817 }, 60_000); for (const mode of ['flag', 'environment'] as const) { - test(`headed startup via ${mode} still reaps a profile orphan and removes stale locks`, async () => { - const orphan = await spawnChromiumHolder(); - seedLocks(orphan.pid); + test(`headed startup via ${mode} reaps an orphaned Chromium on this project's profile and removes stale locks`, async () => { + const orphan = await spawnOrphanedChromium(projectProfile); + seedLocks(orphan.pid, projectProfile); env.BROWSE_HEADED = mode === 'environment' ? '1' : '0'; - const serverScript = path.join(scratch, 'stub-server.ts'); - fs.writeFileSync(serverScript, ` - import * as fs from 'node:fs'; - const server = Bun.serve({ - hostname: '127.0.0.1', port: 0, - fetch: () => Response.json({ status: 'healthy' }), - }); - fs.writeFileSync(process.env.BROWSE_STATE_FILE!, JSON.stringify({ - pid: process.pid, port: server.port, token: 'test-token', - mode: process.env.BROWSE_HEADED === '1' ? 'headed' : 'launched', - })); - `); - env.BROWSE_SERVER_SCRIPT = serverScript; + writeStubServer(); await runCli(mode === 'flag' ? ['--headed', 'status'] : ['status']); expect(JSON.parse(fs.readFileSync(stateFile, 'utf-8')).mode).toBe('headed'); expect(isProcessAlive(orphan.pid)).toBe(false); - expect(fs.readdirSync(profile)).not.toContain(LOCKS[0]); - expect(fs.readdirSync(profile)).not.toContain(LOCKS[1]); - expect(fs.readdirSync(profile)).not.toContain(LOCKS[2]); + for (const lock of LOCKS) expect(fs.readdirSync(projectProfile)).not.toContain(lock); }, 60_000); } + test('headed startup in one project leaves another project\'s live headed Chromium alone (D5, #2492)', async () => { + const otherProject = path.join(scratch, 'project-a', '.gstack', 'chromium-profile'); + const holder = await spawnChromiumHolder(otherProject); + seedLocks(holder.pid, otherProject); + writeStubServer(); + + await runCli(['--headed', 'status']); + + expectLocksIntact(holder.pid, otherProject); + expect(fs.existsSync(projectProfile)).toBe(true); + }, 60_000); + + test('a live owner of this project\'s profile is never killed; browse names it and stops', async () => { + const holder = await spawnChromiumHolder(projectProfile); + seedLocks(holder.pid, projectProfile); + writeStubServer(); + + const r = await runCliRaw(['--headed', 'status']); + + expect(r.code).not.toBe(0); + expect(r.stdout + r.stderr).toContain(`in use by Chromium PID ${holder.pid}`); + expectLocksIntact(holder.pid, projectProfile); + }, 60_000); + + test('a lock written by another host is never acted on', async () => { + const holder = await spawnOrphanedChromium(projectProfile); + seedLocks(holder.pid, projectProfile, 'some-other-host'); + writeStubServer(); + + const r = await runCliRaw(['--headed', 'status']); + + expect(r.code).not.toBe(0); + expect(r.stdout + r.stderr).toContain('on another host (some-other-host)'); + expectLocksIntact(holder.pid, projectProfile, 'some-other-host'); + }, 60_000); + + test('first headed use seeds the project profile from a quiescent shared profile, without singleton files', async () => { + fs.mkdirSync(path.join(profile, 'Default'), { recursive: true }); + fs.writeFileSync(path.join(profile, 'Default', 'Cookies'), 'cookie-db'); + seedLocks(999_999_999); + writeStubServer(); + + const r = await runCliRaw(['--headed', 'status']); + + expect(r.code, r.stdout + r.stderr).toBe(0); + expect(r.stdout).toContain(`Copied your logins and settings from ${profile} to ${projectProfile}`); + expect(fs.readFileSync(path.join(projectProfile, 'Default', 'Cookies'), 'utf-8')).toBe('cookie-db'); + expect(fs.statSync(projectProfile).mode & 0o777).toBe(0o700); + expect(fs.readdirSync(projectProfile).filter(f => f.startsWith('Singleton') || f.includes('.seed-'))).toEqual([]); + expect(fs.readdirSync(path.dirname(projectProfile)).filter(f => f.includes('.seed-'))).toEqual([]); + }, 60_000); + + test('a shared profile in use is not copied; the project starts fresh and says how to import later', async () => { + fs.writeFileSync(path.join(profile, 'Cookies'), 'cookie-db'); + const holder = await spawnChromiumHolder(profile); + seedLocks(holder.pid); + writeStubServer(); + + const r = await runCliRaw(['--headed', 'status']); + + expect(r.code, r.stdout + r.stderr).toBe(0); + expect(r.stdout).toContain('starts with a fresh profile. To import it later'); + expect(fs.existsSync(path.join(projectProfile, 'Cookies'))).toBe(false); + expectLocksIntact(holder.pid); + }, 60_000); + + test('browse profiles lists per-project profiles and prune removes idle ones but never one in use', async () => { + writeStubServer(); + await runCli(['--headed', 'status']); + const listed = await runCliRaw(['profiles']); + expect(listed.code).toBe(0); + expect(listed.stdout).toContain(projectProfile); + expect(listed.stdout).toContain('(old shared profile)'); + + const holder = await spawnChromiumHolder(projectProfile); + seedLocks(holder.pid, projectProfile); + await runCliRaw(['profiles', 'prune', '--days', '0']); + expect(fs.existsSync(projectProfile)).toBe(true); + + holder.kill('SIGKILL'); + await holder.exited; + const pruned = await runCliRaw(['profiles', 'prune', '--days', '0']); + expect(pruned.code).toBe(0); + expect(pruned.stdout).toContain(`removed ${projectProfile}`); + expect(fs.existsSync(projectProfile)).toBe(false); + expect(fs.existsSync(profile)).toBe(true); + }, 60_000); + for (const args of [['stop'], ['--force-restart', 'stop'], ['disconnect']]) { test(`headless ${args.join(' ')} preserves another project's headed profile`, async () => { await runCli(['status']); @@ -228,6 +357,7 @@ describe.skipIf(process.platform === 'win32')('Chromium profile isolation (#2817 test(`${mode} factory shutdown scopes profile cleanup to the active browser mode`, async () => { const holder = await spawnChromiumHolder(); seedLocks(holder.pid); + seedLocks(999_999_999, projectProfile); const child = Bun.spawn([process.execPath, '-e', ` import { buildFetchHandler } from ${JSON.stringify(path.resolve(import.meta.dir, '../src/server.ts'))}; import { resolveConfig } from ${JSON.stringify(path.resolve(import.meta.dir, '../src/config.ts'))}; @@ -249,10 +379,13 @@ describe.skipIf(process.platform === 'win32')('Chromium profile isolation (#2817 const [code, stderr] = await Promise.all([child.exited, new Response(child.stderr).text()]); expect(code, stderr).toBe(0); + // Headed shutdown cleans only this project's profile; another + // project's (here the old shared one) is never touched (D5). + expectLocksIntact(holder.pid); if (mode === 'headed') { - for (const lock of LOCKS) expect(fs.readdirSync(profile)).not.toContain(lock); + for (const lock of LOCKS) expect(fs.readdirSync(projectProfile)).not.toContain(lock); } else { - expectLocksIntact(holder.pid); + expect(fs.readdirSync(projectProfile)).toContain(LOCKS[0]); } }, 60_000); } diff --git a/browse/test/config.test.ts b/browse/test/config.test.ts index c20bad8f2..53a2081a6 100644 --- a/browse/test/config.test.ts +++ b/browse/test/config.test.ts @@ -431,17 +431,21 @@ describe('resolveChromiumProfile', () => { } }); - test('falls back to resolveGstackHome()/chromium-profile when nothing set', () => { + test('falls back to the project state dir, not the machine-wide gstack home (D5, #2492)', () => { const origEnv = process.env.CHROMIUM_PROFILE; const origHome = process.env.GSTACK_HOME; + const origState = process.env.BROWSE_STATE_FILE; delete process.env.CHROMIUM_PROFILE; process.env.GSTACK_HOME = '/tmp/fallback-gstack'; + process.env.BROWSE_STATE_FILE = '/tmp/project-a/.gstack/browse.json'; try { - expect(resolveChromiumProfile()).toBe('/tmp/fallback-gstack/chromium-profile'); + expect(resolveChromiumProfile()).toBe(path.join('/tmp/project-a/.gstack', 'chromium-profile')); } finally { if (origEnv !== undefined) process.env.CHROMIUM_PROFILE = origEnv; if (origHome === undefined) delete process.env.GSTACK_HOME; else process.env.GSTACK_HOME = origHome; + if (origState === undefined) delete process.env.BROWSE_STATE_FILE; + else process.env.BROWSE_STATE_FILE = origState; } }); From 5bcc4b5d2ca42d53d00f867cfb55ff4d8c58d724 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 21:12:02 +0000 Subject: [PATCH 11/17] G6: prettyscreenshot --hide keeps a trailing image path as the output path (#1419) --hide collected every following non-flag argument as a selector, so the output path was hidden instead of written to. A trailing argument that looks like a path or ends in .png/.jpg/.jpeg/.webp is now the output path. Co-authored-by: Jayesh Betala --- browse/src/write-commands.ts | 9 +++++++-- browse/test/commands.test.ts | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/browse/src/write-commands.ts b/browse/src/write-commands.ts index 571957a69..b1178cd6d 100644 --- a/browse/src/write-commands.ts +++ b/browse/src/write-commands.ts @@ -996,13 +996,18 @@ export async function handleWriteCommand( } else if (args[i] === '--cleanup') { doCleanup = true; } else if (args[i] === '--hide' && i + 1 < args.length) { - // Collect all following non-flag args as selectors to hide + // Collect all following non-flag args as selectors to hide. A + // trailing image path is the output path, not a selector (#1419). + const values: string[] = []; i++; while (i < args.length && !args[i].startsWith('--')) { - hideSelectors.push(args[i]); + values.push(args[i]); i++; } i--; // Back up since the for loop will increment + const last = values[values.length - 1]; + if (!outputPath && values.length > 1 && /^(\/|\.\.?\/|~)|\.(png|jpe?g|webp)$/i.test(last)) outputPath = values.pop(); + hideSelectors.push(...values); } else if (args[i] === '--width' && i + 1 < args.length) { viewportWidth = parseInt(args[++i], 10); if (isNaN(viewportWidth)) throw new Error('--width must be a number'); diff --git a/browse/test/commands.test.ts b/browse/test/commands.test.ts index c0f72128f..b400ab60a 100644 --- a/browse/test/commands.test.ts +++ b/browse/test/commands.test.ts @@ -580,6 +580,20 @@ describe('Interaction', () => { expect(await handleReadCommand('js', ['document.querySelector("#name").value'], bm)).toBe(''); }); + test('prettyscreenshot --hide keeps a trailing image path as the output path (#1419)', async () => { + await handleWriteCommand('goto', [baseUrl + '/basic.html'], bm); + const out = path.join(os.tmpdir(), `pretty-hide-${process.pid}.png`); + fs.rmSync(out, { force: true }); + try { + const result = await handleWriteCommand('prettyscreenshot', ['--hide', '#title', '#content', out], bm); + expect(result).toContain(out); + expect(fs.existsSync(out)).toBe(true); + expect(await handleReadCommand('js', ['getComputedStyle(document.querySelector("#content")).display'], bm)).toBe('none'); + } finally { + fs.rmSync(out, { force: true }); + } + }); + test('type -- types text that starts with -- literally; plain text gets no hint', async () => { await handleWriteCommand('goto', [baseUrl + '/forms.html'], bm); await handleWriteCommand('click', ['#bio'], bm); From 2ffeba529b2cad83feb4101ad27e22ea4692d764 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 21:12:37 +0000 Subject: [PATCH 12/17] G6: gstack-ios-qa-daemon --help prints usage instead of starting the daemon (#1932) Co-authored-by: maxpetrusenkoagent --- bin/gstack-ios-qa-daemon | 29 ++++++++++++++++++++++++++ test/ios-qa-daemon-cli.test.ts | 38 ++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 test/ios-qa-daemon-cli.test.ts diff --git a/bin/gstack-ios-qa-daemon b/bin/gstack-ios-qa-daemon index b0ca2c6af..d644a54a8 100755 --- a/bin/gstack-ios-qa-daemon +++ b/bin/gstack-ios-qa-daemon @@ -22,6 +22,35 @@ set -euo pipefail +case "${1:-}" in + -h|--help) + cat <<'EOF' +Usage: + gstack-ios-qa-daemon [--tailnet] + gstack-ios-qa-daemon --help + +Mac-side broker for /ios-qa and /ios-design-review. By default it binds a +loopback listener for a USB-connected iPhone running the in-app StateServer. +Pass --tailnet to also open the Tailscale listener after the local tailscaled +identity probe succeeds. + +Options: + --tailnet Enable the authenticated tailnet listener in addition to loopback. + -h, --help Print this help and exit without starting the daemon. + +Environment: + GSTACK_IOS_DAEMON_PORT Loopback listener port. Default: 9099. + GSTACK_IOS_TARGET_UDID Target iOS device UDID. Optional. + GSTACK_IOS_TARGET_BUNDLE_ID Bundle ID hosting StateServer. Default: com.gstack.iosqa.fixture. + +Readiness: + The daemon prints "READY: port= pid=" once the listener is bound. + Probe local health with: curl -sf http://127.0.0.1:${GSTACK_IOS_DAEMON_PORT:-9099}/healthz +EOF + exit 0 + ;; +esac + SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" GSTACK_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" ENTRY="$GSTACK_DIR/ios-qa/daemon/src/index.ts" diff --git a/test/ios-qa-daemon-cli.test.ts b/test/ios-qa-daemon-cli.test.ts new file mode 100644 index 000000000..66b01a61c --- /dev/null +++ b/test/ios-qa-daemon-cli.test.ts @@ -0,0 +1,38 @@ +/** + * G6 rider (#1932): `gstack-ios-qa-daemon --help` started the daemon (it + * exec'd bun with every argument). Help must print and exit without ever + * reaching the runtime. A stub `bun` first on PATH records any launch. + */ +import { describe, expect, test } from 'bun:test'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const DAEMON = `${ROOT}/bin/gstack-ios-qa-daemon`; + +describe('gstack-ios-qa-daemon --help', () => { + for (const flag of ['--help', '-h']) { + test(`${flag} prints usage and never starts the daemon`, () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ios-qa-daemon-help-')); + try { + const marker = path.join(dir, 'daemon-started'); + fs.writeFileSync(path.join(dir, 'bun'), `#!/usr/bin/env bash\necho started > "${marker}"\n`, { mode: 0o755 }); + const r = Bun.spawnSync(['bash', DAEMON, flag], { + env: { ...process.env, PATH: `${dir}${path.delimiter}${process.env.PATH ?? ''}`, HOME: dir }, + stdout: 'pipe', + stderr: 'pipe', + timeout: 10_000, + }); + expect(r.exitCode).toBe(0); + const out = r.stdout.toString(); + expect(out).toContain('Usage:'); + expect(out).toContain('--tailnet'); + expect(out).toContain('GSTACK_IOS_DAEMON_PORT'); + expect(fs.existsSync(marker)).toBe(false); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + } +}); From 313d0c8f1332a0d9c16ad11df2104891cd09e5b1 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 21:15:26 +0000 Subject: [PATCH 13/17] Q1: validate browser output paths the way the kernel resolves them (#2971 salvage) validateOutputPath walks the destination one component at a time, resolving each existing component before the next, so link/../x, a dangling symlink and a dangling symlinked parent can no longer route a write outside the safe directories, while a new multi-level directory inside them is allowed. scrape validates its manifest and every downloaded file, not only --dir. Ported from PR #2971 onto current code. --- browse/src/path-security.ts | 71 ++++++++++------------ browse/src/write-commands.ts | 12 +++- browse/test/output-path-symlinks.test.ts | 76 ++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 42 deletions(-) create mode 100644 browse/test/output-path-symlinks.test.ts diff --git a/browse/src/path-security.ts b/browse/src/path-security.ts index bc02ccde0..ff224d524 100644 --- a/browse/src/path-security.ts +++ b/browse/src/path-security.ts @@ -31,48 +31,41 @@ const TEMP_ONLY = [TEMP_DIR].map(d => { try { return fs.realpathSync(d); } catch { return d; } }); -/** Validate a file path for writing (screenshot, pdf, download, scrape, archive). */ +/** + * Validate a file path for writing (screenshot, pdf, download, scrape, archive). + * + * Walks the path one component at a time the way the kernel will, resolving + * every existing component (symlinks included) before applying the next, so + * `link/../x` follows `link` first. A component that does not exist yet must + * already sit inside a safe directory; a dangling symlink is rejected because + * a write would create its target. Both the caller's spelling and its lexical + * normalization are checked, since callers write with either. + */ export function validateOutputPath(filePath: string): void { - const resolved = path.resolve(filePath); - - // If the target already exists and is a symlink, resolve through it. - // Without this, a symlink at /tmp/evil.png → /etc/crontab passes the - // parent-directory check (parent is /tmp, which is safe) but the actual - // write follows the symlink to /etc/crontab. - try { - const stat = fs.lstatSync(resolved); - if (stat.isSymbolicLink()) { - const realTarget = fs.realpathSync(resolved); - const isSafe = SAFE_DIRECTORIES.some(dir => isPathWithin(realTarget, dir)); - if (!isSafe) { - throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`); + const reject = () => { throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`); }; + for (const spelling of new Set([filePath, path.resolve(filePath)])) { + const root = path.parse(spelling).root; + let resolved = path.resolve(root || '.'); + for (const component of spelling.slice(root.length).split(process.platform === 'win32' ? /[\\/]+/ : /\/+/)) { + if (!component || component === '.') continue; + if (component === '..') { + resolved = path.dirname(resolved); + continue; + } + resolved = path.join(resolved, component); + try { + fs.lstatSync(resolved); + } catch (err: any) { + if (err?.code === 'ENOENT' && SAFE_DIRECTORIES.some(dir => isPathWithin(resolved, dir))) continue; + reject(); + } + try { + resolved = fs.realpathSync.native(resolved); + } catch { + reject(); } - return; // symlink target verified, no need to check parent } - } catch (e: any) { - // ENOENT = file doesn't exist yet, fall through to parent-dir check - if (e.code !== 'ENOENT') throw e; - } - - // For new files (no existing symlink), verify the parent directory. - // The file itself may not exist yet (e.g., screenshot output). - // This also handles macOS /tmp → /private/tmp transparently. - let dir = path.dirname(resolved); - let realDir: string; - try { - realDir = fs.realpathSync(dir); - } catch { - try { - realDir = fs.realpathSync(path.dirname(dir)); - } catch { - throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`); - } - } - - const realResolved = path.join(realDir, path.basename(resolved)); - const isSafe = SAFE_DIRECTORIES.some(dir => isPathWithin(realResolved, dir)); - if (!isSafe) { - throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`); + if (!SAFE_DIRECTORIES.some(dir => isPathWithin(resolved, dir))) reject(); } } diff --git a/browse/src/write-commands.ts b/browse/src/write-commands.ts index b1178cd6d..b771deb88 100644 --- a/browse/src/write-commands.ts +++ b/browse/src/write-commands.ts @@ -1309,12 +1309,17 @@ export async function handleWriteCommand( const selectorIdx = args.indexOf('--selector'); const selector = selectorIdx >= 0 ? args[selectorIdx + 1] : undefined; const dirIdx = args.indexOf('--dir'); - const dir = dirIdx >= 0 ? args[dirIdx + 1] : path.join(TEMP_DIR, `browse-scrape-${Date.now()}`); + const requestedDir = dirIdx >= 0 ? args[dirIdx + 1] : path.join(TEMP_DIR, `browse-scrape-${Date.now()}`); const limitIdx = args.indexOf('--limit'); const limit = Math.min(limitIdx >= 0 ? parseInt(args[limitIdx + 1], 10) || 50 : 50, 200); - validateOutputPath(dir); + validateOutputPath(requestedDir); + const dir = path.resolve(requestedDir); fs.mkdirSync(dir, { recursive: true }); + // Each file is validated too: a symlink planted in an existing --dir + // would otherwise redirect the write outside the safe directories. + const manifestPath = path.join(dir, 'manifest.json'); + validateOutputPath(manifestPath); const { extractMedia } = await import('./media-extract'); const target = bm.getActiveFrameOrPage(); @@ -1373,6 +1378,7 @@ export async function handleWriteCommand( const ext = mimeToExt(ct.split(';')[0].trim()); const filename = `${type}-${String(i + 1).padStart(3, '0')}${ext}`; const filePath = path.join(dir, filename); + validateOutputPath(filePath); const body = Buffer.from(await response.body()); try { fs.writeFileSync(filePath, body); @@ -1393,7 +1399,7 @@ export async function handleWriteCommand( } // Write manifest - fs.writeFileSync(path.join(dir, 'manifest.json'), JSON.stringify(manifest, null, 2)); + fs.writeFileSync(manifestPath, JSON.stringify(manifest, null, 2)); return `Scraped ${toDownload.length} items to ${dir}/\n${lines.join('\n')}\n\nSummary: ${manifest.succeeded} succeeded, ${manifest.failed} failed, ${Math.round(manifest.total_size / 1024)}KB total`; } diff --git a/browse/test/output-path-symlinks.test.ts b/browse/test/output-path-symlinks.test.ts new file mode 100644 index 000000000..f3be495d9 --- /dev/null +++ b/browse/test/output-path-symlinks.test.ts @@ -0,0 +1,76 @@ +/** + * Output-destination validation (salvaged from #2971): validateOutputPath must + * judge the path the kernel will actually write, component by component. + * The fixture lives under /var/tmp, outside every safe directory, and the + * validator runs in a child whose cwd (a safe directory) is the fixture's + * allowed dir; nothing is ever written to HOME. + */ +import { afterAll, beforeAll, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; + +const VALIDATOR = path.resolve(import.meta.dir, '../src/path-security.ts'); +const POSIX = process.platform !== 'win32' && fs.existsSync('/var/tmp'); +let root = ''; +let allowed = ''; +let outside = ''; + +beforeAll(() => { + if (!POSIX) return; + root = fs.realpathSync(fs.mkdtempSync('/var/tmp/gstack-output-path-')); + allowed = path.join(root, 'allowed'); + outside = path.join(root, 'outside'); + fs.mkdirSync(path.join(allowed, 'nested'), { recursive: true }); + fs.mkdirSync(path.join(outside, 'nested'), { recursive: true }); + fs.writeFileSync(path.join(allowed, 'internal.txt'), 'x'); + fs.writeFileSync(path.join(outside, 'existing.txt'), 'x'); + const links: Array<[string, string]> = [ + ['internal-link.txt', path.join(allowed, 'internal.txt')], + ['internal-parent', path.join(allowed, 'nested')], + ['outward-live.txt', path.join(outside, 'existing.txt')], + ['outward-dangling.txt', path.join(outside, 'missing.txt')], + ['outward-parent', outside], + ['outward-nested', path.join(outside, 'nested')], + ['dangling-parent', path.join(outside, 'missing-dir')], + ['loop', path.join(allowed, 'loop')], + ]; + for (const [name, target] of links) fs.symlinkSync(target, path.join(allowed, name)); +}); + +afterAll(() => { + if (root) fs.rmSync(root, { recursive: true, force: true }); +}); + +function verdicts(files: string[]): Record { + const script = ` + import { validateOutputPath } from ${JSON.stringify(VALIDATOR)}; + const out = {}; + for (const f of JSON.parse(process.argv[1])) { + try { validateOutputPath(f); out[f] = null; } catch (e) { out[f] = e.message; } + } + console.log(JSON.stringify(out)); + `; + const r = Bun.spawnSync([process.execPath, '-e', script, JSON.stringify(files)], { + cwd: allowed, + env: { ...process.env, TMPDIR: path.join(allowed, 'nested') }, + stdout: 'pipe', + stderr: 'pipe', + timeout: 15_000, + }); + expect(r.exitCode, r.stderr.toString()).toBe(0); + return JSON.parse(r.stdout.toString()); +} + +describe.skipIf(!POSIX)('validateOutputPath follows the physical path', () => { + test('ordinary, new, and in-root symlinked destinations are allowed', () => { + const files = ['ordinary.txt', 'new-dir/deeper/new.txt', 'internal-link.txt', 'internal-parent/x.txt', 'nested/../x.txt']; + expect(verdicts(files.map(f => path.join(allowed, f)))).toEqual(Object.fromEntries(files.map(f => [path.join(allowed, f), null]))); + }); + + test('destinations that a write would land outside are rejected', () => { + const files = ['outward-live.txt', 'outward-dangling.txt', 'outward-parent/new.txt', 'outward-nested/../escaped.txt', 'dangling-parent/new.txt', 'loop', 'loop/new.txt']; + const result = verdicts(files.map(f => `${allowed}/${f}`)); + for (const f of files) expect(result[`${allowed}/${f}`]).toContain('Path must be within'); + expect(fs.existsSync(path.join(outside, 'missing.txt'))).toBe(false); + }); +}); From ae86d10f40c3cda16b8222ba4751b4e21adf495b Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 21:25:46 +0000 Subject: [PATCH 14/17] D0: pass the Bun child flags after 'run' so wrappers that rewrite 'bun run' argv keep working bun run --no-env-file --config=/dev/null