From c0ddffa3e2de7856d27e3fe2c14d04b076e39c2f Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 26 Oct 2026 04:31:36 +0000 Subject: [PATCH] fix: pre-landing review fixes Ship adversarial pass: a crash restart re-emits BROWSE_PROXY_URL=direct from the daemon state only when the invocation has no flags, so the env it hands the new daemon never contradicts the config hash it also hands over; an explicit-direct session is refused against a daemon with no recorded config hash, like --proxy and --headed already were; the unparseable-variable hint says that the opt-out also bypasses the system proxy settings on macOS and Windows; BROWSER.md notes that a managed Chromium proxy policy still overrides the switch; CHANGELOG names both files that joined the macOS named-regressions job. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- BROWSER.md | 8 ++++++-- CHANGELOG.md | 6 +++--- browse/src/cli.ts | 6 +++--- browse/src/proxy-config.ts | 2 +- browse/test/restart-env.test.ts | 7 ++++--- 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/BROWSER.md b/BROWSER.md index c9077f518..27f6de3a0 100644 --- a/BROWSER.md +++ b/BROWSER.md @@ -993,7 +993,10 @@ 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://; ignoring HTTPS_PROXY)` (on macOS and Windows, `ignoring the system proxy -settings`), or `[proxy] none: system proxy settings`. +settings`), or `[proxy] none: system proxy settings`. A managed Chromium +proxy policy (`ProxyMode`/`ProxySettings`) still overrides the switch; `$B` +cannot see it, so on a managed machine the log line describes what `$B` +asked for, not what the policy enforces. Values from the environment follow the same credential policy as `--proxy`. `socks5h://` is treated as `socks5://`, because Chromium's SOCKS5 already @@ -1009,7 +1012,8 @@ where it came from, with credentials redacted. unsupported scheme, or a bad port. `$B` connects directly (on Linux, Chromium is told so and does not read the variable itself) instead of failing every command. Fix: `export HTTPS_PROXY=http://host:port` (or `socks5://`), - or set `BROWSE_PROXY_URL=direct` to silence the warning. A bad `--proxy` or + or set `BROWSE_PROXY_URL=direct` to silence the warning (on macOS and + Windows that also makes Chromium bypass the system proxy settings). A bad `--proxy` or `BROWSE_PROXY_URL` is still an error, because those are explicit browse settings and a silent direct connection could expose your real IP address. - **`the running browse daemon was started with different proxy settings ( differs), so it was not reused.`** diff --git a/CHANGELOG.md b/CHANGELOG.md index 89ba6c915..62450fe2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,16 +7,16 @@ ### 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; 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/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 daemon with no recorded config hash is refused like one started without `--proxy`), and a restart with no invocation flags re-emits the opt-out from the daemon state; a restart driven by a session's own flags emits only what that session's hash says. The unparseable-variable warning's hint now says that `BROWSE_PROXY_URL=direct` also bypasses the system proxy settings on macOS and Windows. 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. +- **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), where the daemon log records the effective mode, and that a managed Chromium proxy policy still overrides the switch. The eMudhra/TLS trust section is unchanged. ### Tests - `browse/test/proxy-direct-launch.test.ts` (Linux): two logging CONNECT proxies on loopback (`browse/test/helpers/connect-proxy.ts`) and a target name that does not resolve. With `HTTPS_PROXY` set and `BROWSE_PROXY_URL=direct`, the goto fails at DNS, no CONNECT reaches the proxy, the daemon log names the ignored variable and the state file has no proxy; this case fails on the unfixed tree with `ERR_TUNNEL_CONNECTION_FAILED` and a logged CONNECT. An unparseable `HTTPS_PROXY` (curl-style `host:port`) warns and still forces direct, so no CONNECT reaches the host Chromium itself would have parsed. Two controls: `HTTPS_PROXY` alone is the configured proxy and receives the CONNECT; `--proxy` beside `HTTPS_PROXY` sends the CONNECT only to the configured proxy. Each case asserts on the daemon-log slice its own invocation wrote. - `browse/test/proxy-direct-launch-paths.test.ts` (Linux, real Chromium under Xvfb): the `BROWSE_EXTENSIONS_DIR` persistent context, `launchHeaded()` and the `handoff()` relaunch each ignore an inherited `HTTPS_PROXY` (no CONNECT, `ERR_NAME_NOT_RESOLVED`), and a headed control with a configured proxy still sends the CONNECT to it. - `browse/test/proxy-config.test.ts`: `forcesDirect` per platform, and `directProxyLogLine` for the Linux variable list, the explicit-direct line on macOS/Windows, and the system-proxy line. - `browse/test/proxy-env.test.ts`, `browse/test/restart-env.test.ts`: the config hash separates an explicit direct from no proxy on macOS and Windows and not on Linux (`extractGlobalFlags` takes the platform); the mismatch refusal for both directions (explicit session against a system-proxy daemon, default session against an explicit-direct daemon); `buildRestartEnv` re-emits `BROWSE_PROXY_URL=direct` from the invocation or the persisted state, and a proxied invocation wins over it. -- `browse/test/proxy-env-daemon.test.ts`: through the real CLI and daemon, `BROWSE_PROXY_URL=direct` is written to the daemon state as `proxyDirect` where it bypasses the system proxy and a default session is then refused with the `BROWSE_PROXY_URL differs` lines; on Linux the daemon is reused. The file joins the macOS named-regressions CI job so the refusal arm runs there. `buildHeadedServerEnv` carries the opt-out to the headed server, and BROWSER.md's section is checked against the real switch and log literals. +- `browse/test/proxy-env-daemon.test.ts`: through the real CLI and daemon, `BROWSE_PROXY_URL=direct` is written to the daemon state as `proxyDirect` where it bypasses the system proxy and a default session is then refused with the `BROWSE_PROXY_URL differs` lines; on Linux the daemon is reused. This file and `proxy-direct-launch-args.test.ts` join the macOS named-regressions CI job, so the refusal arm and the "no switch without an opt-out" arm run on macOS. `buildHeadedServerEnv` carries the opt-out to the headed server, and BROWSER.md's section is checked against the real switch and log literals. - `browse/test/proxy-direct-launch-args.test.ts`: reads the real command line of the headless Chromium `BrowserManager` launched (from the pid it records) and asserts the switch is present on Linux and absent elsewhere without an opt-out, and present on every platform with `BROWSE_PROXY_URL=direct`; the per-platform expectations are written out, not read back from `forcesDirect`. Skipped on Windows, where no command line reader exists. ## [1.91.226.0] - 2026-10-26 diff --git a/browse/src/cli.ts b/browse/src/cli.ts index 321198c43..dff8ff441 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -539,7 +539,7 @@ export function buildRestartEnv( oldState: ServerState | null, ): Record { const env: Record = proxyServerEnv(globalFlags); - if (!globalFlags?.proxyUrl && oldState?.proxyDirect) env.BROWSE_PROXY_URL = 'direct'; + if (!globalFlags && oldState?.proxyDirect) env.BROWSE_PROXY_URL = 'direct'; if (globalFlags?.headed || oldState?.mode === 'headed') env.BROWSE_HEADED = '1'; const configHash = globalFlags?.configHash || oldState?.configHash; if (configHash) env.BROWSE_CONFIG_HASH = configHash; @@ -943,8 +943,8 @@ async function ensureServer(flags?: GlobalFlags): Promise { // Same path: existing daemon is plain (no flags) but caller passes // --proxy/--headed. Refuse for the same reason — apply explicitly via // disconnect+reconnect. - if (desiredHash && !state.configHash && (flags?.proxyUrl || flags?.headed)) { - console.error(`[browse] existing daemon was started without --proxy/--headed.`); + if (desiredHash && !state.configHash && (flags?.proxyUrl || flags?.headed || flags?.explicitDirect)) { + console.error(`[browse] existing daemon was started without --proxy/--headed${flags?.explicitDirect ? ' or BROWSE_PROXY_URL=direct' : ''}.`); console.error(`[browse] run 'browse disconnect' first to apply new flags.`); process.exit(1); } diff --git a/browse/src/proxy-config.ts b/browse/src/proxy-config.ts index 877960d9e..f31ebb9f9 100644 --- a/browse/src/proxy-config.ts +++ b/browse/src/proxy-config.ts @@ -319,7 +319,7 @@ export function resolveProxy(flagUrl: string | null, env: NodeJS.ProcessEnv): Re } catch (err) { if (!(err instanceof ProxyConfigError)) throw err; return direct(`${source} is not a proxy URL browse can use (${err.message}; ${err.hint}), so browse connects directly. ` + - `fix: export ${source}=http://host:port (or socks5://), or set BROWSE_PROXY_URL=direct to silence this. see ${PROXY_TROUBLESHOOTING}`); + `fix: export ${source}=http://host:port (or socks5://), or set BROWSE_PROXY_URL=direct to silence this (on macOS and Windows that also makes Chromium bypass the system proxy settings). see ${PROXY_TROUBLESHOOTING}`); } } const parsed = parseProxyConfig({ proxyUrl: raw, envUser: env.BROWSE_PROXY_USER, envPass: env.BROWSE_PROXY_PASS, source }); diff --git a/browse/test/restart-env.test.ts b/browse/test/restart-env.test.ts index 794452663..85403bacb 100644 --- a/browse/test/restart-env.test.ts +++ b/browse/test/restart-env.test.ts @@ -38,9 +38,10 @@ describe("buildRestartEnv (#1781 headed persistence)", () => { expect(env.BROWSE_CONFIG_HASH).toBe("d"); }); - test("explicit direct falls back to persisted state, so a plain command's restart never flips to the system proxy", () => { - expect(buildRestartEnv({} as any, { ...launchedState, proxyDirect: true } as any).BROWSE_PROXY_URL).toBe("direct"); - expect(buildRestartEnv({} as any, launchedState as any).BROWSE_PROXY_URL).toBeUndefined(); + test("explicit direct falls back to persisted state only when no invocation flags exist, so the env never contradicts the hash", () => { + expect(buildRestartEnv(null, { ...launchedState, proxyDirect: true } as any).BROWSE_PROXY_URL).toBe("direct"); + expect(buildRestartEnv(null, launchedState as any).BROWSE_PROXY_URL).toBeUndefined(); + expect(buildRestartEnv({ explicitDirect: false, configHash: "plain" } as any, { ...launchedState, proxyDirect: true } as any).BROWSE_PROXY_URL).toBeUndefined(); }); test("a proxied invocation wins over a persisted explicit direct", () => {