diff --git a/.github/workflows/free-tests.yml b/.github/workflows/free-tests.yml index b364c500a..948b597ee 100644 --- a/.github/workflows/free-tests.yml +++ b/.github/workflows/free-tests.yml @@ -146,7 +146,7 @@ jobs: resolved="$(realpath "$tmp")" echo "temp root: $tmp -> $resolved" test "$resolved" != "${tmp%/}" - files=(test/gstack-design-detect.test.ts browse/test/chromium-profile-isolation.test.ts test/cso-git-hardening.test.ts test/plan-count-file-permission.test.ts test/spec-quality-gate-secret-sink.test.ts test/gstack-memory-ingest.test.ts test/regression-issue3024-zsh-sourced-helpers.test.ts test/codex-probe-subcommands.test.ts browse/test/proxy-env-daemon.test.ts) + files=(test/gstack-design-detect.test.ts browse/test/chromium-profile-isolation.test.ts test/cso-git-hardening.test.ts test/plan-count-file-permission.test.ts test/spec-quality-gate-secret-sink.test.ts test/gstack-memory-ingest.test.ts test/regression-issue3024-zsh-sourced-helpers.test.ts test/codex-probe-subcommands.test.ts browse/test/proxy-env-daemon.test.ts browse/test/proxy-direct-launch-args.test.ts) for file in "${files[@]}"; do test -f "$file"; done bun test "${files[@]}" diff --git a/BROWSER.md b/BROWSER.md index a7fac4a37..c9077f518 100644 --- a/BROWSER.md +++ b/BROWSER.md @@ -970,7 +970,8 @@ first one set wins: 3. `HTTPS_PROXY`, then `https_proxy` 4. `HTTP_PROXY`, then `http_proxy` 5. `ALL_PROXY`, then `all_proxy` -6. no proxy (a direct connection) +6. no proxy configured by `$B` (a direct connection on Linux; the system proxy + settings on macOS and Windows) Whatever `$B` resolves is what Chromium gets, explicitly: a proxy is passed as `--proxy-server=`, and a direct connection as @@ -985,8 +986,9 @@ never reads those variables; it follows the system proxy settings (PAC and WPAD included), and step 6 leaves it to them, so GStack Browser.app behind a corporate proxy keeps working. `BROWSE_PROXY_URL=direct` bypasses the system settings too, on every platform; on macOS and Windows it is part of the -daemon's settings, so a running daemon started without it is not reused (the -refusal below names `BROWSE_PROXY_URL`) and a crash restart keeps it. The +daemon's settings, so a running daemon started without it is not reused (for +a headless daemon the refusal below names `BROWSE_PROXY_URL`; a `connect` +daemon gets the generic flags refusal) and a crash restart keeps it. The daemon log (`browse-daemon.log` next to the state file) records the effective mode each time the daemon starts, as `[proxy] using from `, `[proxy] none: direct connection (--proxy-server=direct://; diff --git a/CHANGELOG.md b/CHANGELOG.md index afe3c29e1..89ba6c915 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ ### Fixed - **`browse/src/browser-manager.ts`**: every launch path (headless `launch`, the `BROWSE_EXTENSIONS_DIR` persistent context, `launchHeaded`, `handoff`) adds `--proxy-server=direct://` when no proxy is configured and `forcesDirect()` says Chromium would otherwise pick one browse did not resolve, from one `proxyLaunchArgs()`. A configured proxy was already explicit (Playwright passes it as `--proxy-server`), so the environment can no longer win silently in either mode. `--no-proxy-server` was measured and rejected: chrome-headless-shell, the default headless build, ignores it and still took `HTTPS_PROXY`; the full Chromium honors both switches, and `direct://` works on both. - **`browse/src/proxy-config.ts`**: `forcesDirect(env, platform)` is true on Linux always, where the leak lives, and on macOS and Windows only with `BROWSE_PROXY_URL=direct`. Chromium there never read the proxy variables and follows the system proxy settings (PAC and WPAD included), so a user of GStack Browser.app behind a corporate proxy keeps that connectivity, and a PAC cannot be named through `BROWSE_PROXY_URL`. An explicit `direct` bypasses the system settings on every platform. -- **`browse/src/cli.ts`, `browse/src/server.ts`**: on macOS and Windows the explicit `direct` is part of the daemon config hash (`explicitDirect()`), is recorded in the daemon state as `proxyDirect` and is re-emitted on a crash auto-restart, so a running daemon started on the system proxy is refused rather than reused by a session that opted out (the refusal names `BROWSE_PROXY_URL` and offers `browse stop` or unsetting it), and a restart triggered from a shell without the variable stays direct. On Linux the opt-out hashes like no proxy, as before, because every direct connection there already carries the switch. +- **`browse/src/cli.ts`, `browse/src/server.ts`**: on macOS and Windows the explicit `direct` is part of the daemon config hash (`explicitDirect()`), is recorded in the daemon state as `proxyDirect` and is re-emitted on a crash auto-restart, so a running daemon started on the system proxy is refused rather than reused by a session that opted out (the refusal names `BROWSE_PROXY_URL` and offers `browse stop` or unsetting it; a daemon started before this release meets that refusal once), and a restart triggered from a shell without the variable stays direct. On Linux the opt-out hashes like no proxy, as before, because every direct connection there already carries the switch. - **`browse/src/server.ts`** logs the effective proxy mode each time the daemon starts, beside the existing `[proxy] using from ` line: `[browse] [proxy] none: direct connection (--proxy-server=direct://; ignoring HTTPS_PROXY)` on Linux, naming each standard variable that was set and ignored; `(...; ignoring the system proxy settings)` for an explicit direct on macOS and Windows; and `[browse] [proxy] none: system proxy settings (Chromium's default on ; BROWSE_PROXY_URL=direct connects directly)` when those platforms are left to the system. - **BROWSER.md, "Proxy from environment"**: says that whatever `$B` resolves is passed to Chromium explicitly, that on Linux a set proxy variable is ignored in direct mode, that macOS and Windows follow the system proxy settings unless `BROWSE_PROXY_URL=direct` is set (and that the opt-out is part of the daemon's settings there), and where the daemon log records the effective mode. The eMudhra/TLS trust section is unchanged. diff --git a/browse/src/cli.ts b/browse/src/cli.ts index e99fd709d..321198c43 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -552,7 +552,7 @@ export function buildRestartEnv( * the same port, watchdog setting, proxy and config hash. Pure + exported for tests. */ export function buildHeadedServerEnv( - globalFlags: Pick & Partial>, + globalFlags: Pick & Partial>, ): Record { return { BROWSE_HEADED: '1', diff --git a/browse/src/proxy-config.ts b/browse/src/proxy-config.ts index dfc25d9f1..877960d9e 100644 --- a/browse/src/proxy-config.ts +++ b/browse/src/proxy-config.ts @@ -235,12 +235,12 @@ export function withLoopbackBypass(rules: string[]): string { * caller other than the CLI set only BROWSE_PROXY_URL), and whether an * explicit direct bypasses the system proxy settings here (explicitDirect). */ -export function serverProxyEnv(env: NodeJS.ProcessEnv): { url: string | undefined; source: string; bypass: string; direct: boolean } { +export function serverProxyEnv(env: NodeJS.ProcessEnv, platform: NodeJS.Platform = process.platform): { url: string | undefined; source: string; bypass: string; direct: boolean } { return { url: env.BROWSE_PROXY_URL === 'direct' ? undefined : env.BROWSE_PROXY_URL || undefined, source: env.BROWSE_PROXY_SOURCE || 'BROWSE_PROXY_URL', bypass: env.BROWSE_PROXY_BYPASS || withLoopbackBypass(noProxyToChromiumRules(env.NO_PROXY || env.no_proxy)), - direct: explicitDirect(env), + direct: explicitDirect(env, platform), }; } diff --git a/browse/test/cli-supervisor.test.ts b/browse/test/cli-supervisor.test.ts index a689a025a..eed733983 100644 --- a/browse/test/cli-supervisor.test.ts +++ b/browse/test/cli-supervisor.test.ts @@ -92,7 +92,7 @@ function harness(opts: { let clock = 1_000_000, sleeps = 0, tick = 0, exiting = false, starts = 0; const calls = { startEnv: [] as Record[], agents: [] as number[], log: [] as string[], warn: [] as string[], error: [] as string[] }; const deps: HeadedSupervisorDeps = { - env: buildHeadedServerEnv({ proxyUrl: 'socks5://127.0.0.1:9050', configHash: 'abc123' }), + env: buildHeadedServerEnv({ proxyUrl: 'socks5://127.0.0.1:9050', configHash: 'abc123', explicitDirect: false }), tickMs: opts.tickMs ?? 30_000, backoffMs: [1000, 2000, 4000, 8000, 30000], daemonLog: '/state/browse-daemon.log', @@ -181,7 +181,7 @@ describe('runHeadedSupervisor (behavior)', () => { describe('buildHeadedServerEnv', () => { test('omits proxy and config hash when this invocation has none', () => { - expect(buildHeadedServerEnv({ proxyUrl: null, configHash: '' })).toEqual({ BROWSE_HEADED: '1', BROWSE_PORT: '34567', BROWSE_PARENT_PID: '0' }); + expect(buildHeadedServerEnv({ proxyUrl: null, configHash: '', explicitDirect: false })).toEqual({ BROWSE_HEADED: '1', BROWSE_PORT: '34567', BROWSE_PARENT_PID: '0' }); }); }); diff --git a/browse/test/proxy-config.test.ts b/browse/test/proxy-config.test.ts index 858f13fd1..bc2725c8f 100644 --- a/browse/test/proxy-config.test.ts +++ b/browse/test/proxy-config.test.ts @@ -220,9 +220,12 @@ describe('explicitDirect (#3440 item 8)', () => { .toBe(computeConfigHash({ proxyUrl: 'http://p:1', headed: false })); }); - test('the daemon reads the opt-out from its own environment for the state file', () => { - expect(serverProxyEnv({ BROWSE_PROXY_URL: 'direct' }).direct).toBe(process.platform !== 'linux'); - expect(serverProxyEnv({}).direct).toBe(false); + test('the daemon reads the opt-out from its own environment for the state file, on the platforms where it matters', () => { + expect(serverProxyEnv({ BROWSE_PROXY_URL: 'direct' }, 'darwin').direct).toBe(true); + expect(serverProxyEnv({ BROWSE_PROXY_URL: 'direct' }, 'win32').direct).toBe(true); + expect(serverProxyEnv({ BROWSE_PROXY_URL: 'direct' }, 'linux').direct).toBe(false); + expect(serverProxyEnv({}, 'darwin').direct).toBe(false); + expect(serverProxyEnv({ BROWSE_PROXY_URL: 'http://p:1' }, 'darwin').direct).toBe(false); }); }); diff --git a/browse/test/server-auth.test.ts b/browse/test/server-auth.test.ts index 2220be27c..f52914a76 100644 --- a/browse/test/server-auth.test.ts +++ b/browse/test/server-auth.test.ts @@ -514,7 +514,7 @@ describe('Server auth security', () => { // parent-PID watchdog. const connectBlock = sliceBetween(CLI_SRC, 'Launching headed Chromium', 'Terminal agent started'); expect(connectBlock).toContain('startServer(buildHeadedServerEnv(globalFlags))'); - expect(buildHeadedServerEnv({ proxyUrl: null, configHash: '' }).BROWSE_PARENT_PID).toBe('0'); + expect(buildHeadedServerEnv({ proxyUrl: null, configHash: '', explicitDirect: false }).BROWSE_PARENT_PID).toBe('0'); }); // Regression: newtab returned 403 for scoped tokens because the tab ownership