fix: pre-landing review fixes

A backslash is excluded from the scheme-less proxy shape and parseProxyConfig
refuses any proxy URL with a path, query or fragment, for every source: WHATWG
parsing ends an http authority at a backslash, so a DOMAIN\user login or a /
in a password landed in the path, which the redactor does not mask, and was
printed in the clear on the start line, in the daemon log and in the state
file. Such values now warn (standard variables) or error (--proxy,
BROWSE_PROXY_URL) without echoing the credential. Docs updated.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

BROWSER.md

@@ -1015,7 +1015,11 @@ where it came from, with credentials redacted.
 
 - **`warning: HTTPS_PROXY is not a proxy URL browse can use (...)`.** The
   variable doesn't parse even as `http://` plus the value: an unsupported
-  scheme (`ftp://`), a bad port, or a missing host. `$B` connects directly (on Linux,
+  scheme (`ftp://`), a bad port, a missing host, a path, query or fragment
+  (where a `DOMAIN\user` login or a `/` in a password ends up after URL
+  parsing, so such values are refused for every source rather than printed
+  unredacted; percent-encode `/ \ ? # @` in credentials), or credentials
+  that are not valid percent-encoding. `$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 (on macOS and

CHANGELOG.md

@@ -6,7 +6,7 @@
 
 ### Fixed
 - **`browse/src/browser-manager.ts`**: each of `BrowserManager`'s four Chromium launch sites (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. The cookie-import worker (`cookie-import-native-worker.ts`), which launches the user's own browser only to read its cookie store and never navigates, is unchanged.
-- **`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. A standard proxy variable without a scheme (curl-style `host:port` or `user:pass@host:port`) is now read as an `http://` proxy (port 80 when none is given), the way Chromium's own environment reader took it: before this change browse rejected it with a warning and Chromium on Linux read the variable anyway, so that session was proxied; with the launch now explicit, reading it the same way keeps those sessions on their proxy instead of sending them direct. On macOS and Windows the same value is now a configured proxy too, where it used to be ignored in favour of the system proxy settings, so a daemon started under the old warning meets the mismatch refusal once. Only the plain `[user:pass@]host[:port]` shape is read that way; a value with a scheme, a path, a `?` or `#` in the credentials, or a bare number goes to the URL parser as written, so a typo such as `http:/host:3128` keeps its diagnostic and no part of a password can land outside the redacted userinfo. Credentials that are not valid percent-encoding (a literal `%`) are a proxy configuration error instead of a crash, for every source. A value that does not parse even as `http://` (an unsupported scheme, a bad port, a missing host) still warns and connects directly; `--proxy` and `BROWSE_PROXY_URL` still require the scheme.
+- **`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. A standard proxy variable without a scheme (curl-style `host:port` or `user:pass@host:port`) is now read as an `http://` proxy (port 80 when none is given), the way Chromium's own environment reader took it: before this change browse rejected it with a warning and Chromium on Linux read the variable anyway, so that session was proxied; with the launch now explicit, reading it the same way keeps those sessions on their proxy instead of sending them direct. On macOS and Windows the same value is now a configured proxy too, where it used to be ignored in favour of the system proxy settings, so a daemon started under the old warning meets the mismatch refusal once. Only the plain `[user:pass@]host[:port]` shape is read that way; a value with a scheme, a path, a `/`, `\\`, `?` or `#` in the credentials, or a bare number goes to the URL parser as written, so a typo such as `http:/host:3128` keeps its diagnostic. A proxy URL with a path, query or fragment is now refused for every source (`--proxy` included), because that is where a `DOMAIN\\user` login or a `/` in a password lands after URL parsing and only the userinfo is redacted; percent-encode those characters. Credentials that are not valid percent-encoding (a literal `%`) are a proxy configuration error instead of a crash, for every source. For a standard variable each of these is a warning and a direct connection, like any other unusable value. A value that does not parse even as `http://` (an unsupported scheme, a bad port, a missing host) still warns and connects directly; `--proxy` and `BROWSE_PROXY_URL` still require the scheme.
 - **`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 from the session's own flags, 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`; the mismatch refusal offers `BROWSE_PROXY_URL=direct` as a remedy only where it would match the running daemon, on Linux or against a daemon started with it, and otherwise says to unset the variable that differs). 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 <url> from <source>` 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 <platform>; 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), 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.
@@ -15,7 +15,7 @@
 - `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. A scheme-less `HTTPS_PROXY` (`host:port`) is the configured proxy and the CONNECT reaches it (this case fails on the tree before the lenient parse); an `HTTPS_PROXY` that does not parse even as `http://` (`ftp://`) warns and still forces direct, so no CONNECT reaches the proxy it named. 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) and the remedy an env-proxied session gets against a system-proxy daemon (unset the variable, not `BROWSE_PROXY_URL=direct`, which that daemon would refuse); `buildRestartEnv` re-emits `BROWSE_PROXY_URL=direct` from the invocation's flags only, never from the persisted state, and a proxied invocation wins over it; `resolveProxy` reads scheme-less standard variables as `http://` (with and without `user:pass@`, IPv6, no port, surrounding whitespace), leaves a one-slash scheme and a bare number to the URL parser, gives `h:3128` and `http://h:3128` the same config hash, warns for an unsupported scheme, a bad port, a missing host, a credential that would spill past the userinfo or one that is not percent-encoded, without echoing credentials, and still fails closed for `--proxy` and `BROWSE_PROXY_URL`.
+- `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) and the remedy an env-proxied session gets against a system-proxy daemon (unset the variable, not `BROWSE_PROXY_URL=direct`, which that daemon would refuse); `buildRestartEnv` re-emits `BROWSE_PROXY_URL=direct` from the invocation's flags only, never from the persisted state, and a proxied invocation wins over it; `resolveProxy` reads scheme-less standard variables as `http://` (with and without `user:pass@`, IPv6, no port, surrounding whitespace), leaves a one-slash scheme and a bare number to the URL parser, gives `h:3128` and `http://h:3128` the same config hash, warns for an unsupported scheme, a bad port, a missing host, a credential that would spill past the userinfo (`/`, `\\`, `?`, `#`, including a `DOMAIN\\user` login) or one that is not percent-encoded, without echoing credentials, refuses a proxy URL with a path, query or fragment for every source, and still fails closed for `--proxy` and `BROWSE_PROXY_URL`.
 - `browse/test/proxy-env-daemon.test.ts`: through the real CLI and daemon, a scheme-less `HTTPS_PROXY` with credentials starts the daemon on that `http://` proxy, redacted in the start line and the state, and an `ftp://` value warns and falls back to direct; `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.
 

browse/src/proxy-config.ts

@@ -85,6 +85,17 @@ export function parseProxyConfig(opts: {
     );
   }
 
+  // A proxy address has no path, query or fragment. Anything the URL parser
+  // put there came from a value that was not a proxy URL (a backslash in a
+  // DOMAIN\user login, a '/' in a password) and would otherwise be printed
+  // in the clear, because only the userinfo is redacted.
+  if ((url.pathname !== '' && url.pathname !== '/') || url.search || url.hash) {
+    throw new ProxyConfigError(
+      'expected scheme://[user:pass@]host:port with no path; percent-encode any / \\ ? # @ in the credentials',
+      `invalid proxy URL — unexpected path, query or fragment`,
+    );
+  }
+
   const port = url.port
     ? parseInt(url.port, 10)
     : (scheme === 'http' ? 80 : scheme === 'https' ? 443 : 1080);
@@ -317,11 +328,12 @@ export function directProxyLogLine(env: NodeJS.ProcessEnv, platform: NodeJS.Plat
  * The scheme-less proxy shapes curl and Chromium's environment reader take as
  * an HTTP proxy: `[user:pass@]host[:port]` with a name, dotted address or
  * bracketed IPv6 host. Anything else (a scheme, a path, query or fragment
- * character, whitespace inside, a bare number) goes to the URL parser as
- * written, so a typo such as `http:/host:3128` keeps the diagnostic it had
- * and no part of a password can land outside the redacted userinfo.
+ * character, a backslash, whitespace inside, a bare number) goes to the URL
+ * parser as written, so a typo such as `http:/host:3128` keeps the
+ * diagnostic it had; parseProxyConfig then rejects any path, query or
+ * fragment, so no part of a password can land outside the redacted userinfo.
  */
-const SCHEMELESS_PROXY_RE = /^(?:[^/?#@\s]+@)?(?:\[[0-9a-f:.]+\]|(?=[^:]*[a-z.])[^/?#@\s:[\]]+)(?::\d+)?$/i;
+const SCHEMELESS_PROXY_RE = /^(?:[^/\\?#@\s]+@)?(?:\[[0-9a-f:.]+\]|(?=[^:]*[a-z.])[^/\\?#@\s:[\]]+)(?::\d+)?$/i;
 
 /**
  * Resolve the proxy for this invocation (#915). Env URLs go through

browse/test/proxy-env.test.ts

@@ -86,7 +86,7 @@ describe('resolveProxy precedence', () => {
   });
 
   test('a scheme-less password that would spill past the userinfo, or that is not percent-encoded, warns instead of leaking or crashing', () => {
-    for (const value of ['bob:1234?x@proxy.corp:3128', 'bob:1234#x@proxy.corp:3128', 'bob:1234/x@proxy.corp:3128']) {
+    for (const value of ['bob:1234?x@proxy.corp:3128', 'bob:1234#x@proxy.corp:3128', 'bob:1234/x@proxy.corp:3128', 'CORP\\jdoe:1234@proxy.corp:3128', 'bob:\\1234@proxy.corp:3128', 'bob:1234\\abc@proxy.corp:3128']) {
       const resolved = resolveProxy(null, { HTTPS_PROXY: value });
       expect(resolved.url, value).toBeNull();
       expect(resolved.warning, value).toStartWith('HTTPS_PROXY is not a proxy URL browse can use (');
@@ -99,6 +99,17 @@ describe('resolveProxy precedence', () => {
     expect(() => resolveProxy(['http://bob', '100%@proxy.corp:3128'].join(':'), {})).toThrow(/percent-encoding/);
   });
 
+  test('a proxy URL with a path, query or fragment is refused for every source, so a DOMAIN\\user login never prints its password', () => {
+    for (const value of [['http://CORP\\jdoe', '1234@proxy.corp:3128'].join(':'), 'http://proxy.corp:3128/path', 'http://proxy.corp:3128?x=1', 'socks5://proxy.corp:1080#f']) {
+      expect(() => resolveProxy(value, {}), value).toThrow(/unexpected path, query or fragment/);
+      const resolved = resolveProxy(null, { HTTPS_PROXY: value });
+      expect(resolved.url, value).toBeNull();
+      expect(resolved.warning, value).not.toContain('1234');
+    }
+    expect(resolveProxy('http://proxy.corp:3128/', {}).url).toBe('http://proxy.corp:3128/');
+    expect(resolveProxy('socks5://proxy.corp:1080', {}).url).toBe('socks5://proxy.corp:1080');
+  });
+
   test('scheme-less and http:// spellings of the same proxy share a config hash, so either shell reuses the daemon', () => {
     const a = extractGlobalFlags(['status'], { HTTPS_PROXY: 'h:3128' }, 'linux');
     const b = extractGlobalFlags(['status'], { HTTPS_PROXY: 'http://h:3128' }, 'linux');