fix(browse): daemon reads proxy creds once when BROWSE_PROXY_USER/PASS are set (#3457)

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

browse/src/cli.ts

@@ -522,12 +522,16 @@ export function proxyMismatchLines(flags: GlobalFlags, state: ServerState, env:
  * The server env for this invocation's resolved proxy (#915); empty when
  * direct, except BROWSE_PROXY_URL=direct where that bypasses the system proxy
  * settings (#3440), so a daemon start or auto-restart carries the opt-out the
- * session's hash was computed from.
+ * session's hash was computed from. The resolved URL already carries any
+ * BROWSE_PROXY_USER/PASS, so those are blanked for the daemon, which inherits
+ * this environment and would otherwise see the creds twice (#3457).
  */
 function proxyServerEnv(flags: Partial<Pick<GlobalFlags, 'proxyUrl' | 'proxySource' | 'proxyBypass' | 'explicitDirect'>> | null | undefined): Record<string, string> {
   if (!flags?.proxyUrl) return flags?.explicitDirect ? { BROWSE_PROXY_URL: 'direct' } : {};
   return {
     BROWSE_PROXY_URL: flags.proxyUrl,
+    BROWSE_PROXY_USER: '',
+    BROWSE_PROXY_PASS: '',
     ...(flags.proxySource ? { BROWSE_PROXY_SOURCE: flags.proxySource } : {}),
     ...(flags.proxyBypass ? { BROWSE_PROXY_BYPASS: flags.proxyBypass } : {}),
   };

browse/test/cli-supervisor.test.ts

@@ -124,7 +124,7 @@ describe('runHeadedSupervisor (behavior)', () => {
     expect(h.calls.startEnv).toHaveLength(1);
     expect(h.calls.startEnv[0]).toEqual({
       BROWSE_HEADED: '1', BROWSE_PORT: '34567', BROWSE_PARENT_PID: '0',
-      BROWSE_PROXY_URL: 'socks5://127.0.0.1:9050', BROWSE_CONFIG_HASH: 'abc123',
+      BROWSE_PROXY_URL: 'socks5://127.0.0.1:9050', BROWSE_PROXY_USER: '', BROWSE_PROXY_PASS: '', BROWSE_CONFIG_HASH: 'abc123',
     });
     expect(h.calls.startEnv[0]).toBe(h.deps.env);
     expect(h.calls.agents).toEqual([5000]);

browse/test/proxy-env-daemon.test.ts

@@ -14,6 +14,8 @@ import * as path from 'node:path';
 
 const CLI = path.resolve(import.meta.dir, '../src/cli.ts');
 const PROXIED_HOST = 'proxied.gstack-915.test';
+const AUTH_HOST = 'auth.gstack-3457.test';
+const AUTH_HEADER = `Basic ${Buffer.from('proxy-user:proxy-secret').toString('base64')}`;
 
 let scratch = '';
 let stateFile = '';
@@ -27,6 +29,11 @@ beforeAll(async () => {
   stateFile = path.join(scratch, 'state', 'browse.json');
   proxy = http.createServer((req, res) => {
     const host = new URL(req.url ?? '/', 'http://unknown').hostname;
+    if (host === AUTH_HOST && req.headers['proxy-authorization'] !== AUTH_HEADER) {
+      res.writeHead(407, { 'Proxy-Authenticate': 'Basic realm="gstack-3457"' });
+      res.end();
+      return;
+    }
     proxiedHosts.push(host);
     res.writeHead(200, { 'Content-Type': 'text/html' });
     res.end(`<!doctype html><html><body><p>via proxy for ${host}</p></body></html>`);
@@ -177,4 +184,15 @@ describe.skipIf(process.platform === 'win32')('#915 proxy from the environment,
     }
     expect(daemonPid()).toBe(pid);
   }, 120_000);
+
+  test('BROWSE_PROXY_USER/PASS with HTTP_PROXY: the daemon starts and Chromium answers the proxy auth challenge (#3457)', async () => {
+    expect((await browse(['stop'], {})).code).toBe(0);
+    const env = { HTTP_PROXY: proxyUrl, BROWSE_PROXY_USER: 'proxy-user', BROWSE_PROXY_PASS: 'proxy-secret' };
+    const r = await browse(['goto', `http://${AUTH_HOST}/`], env);
+    expect(r.code, r.out).toBe(0);
+    expect(r.stderr).toContain(`Starting server with proxy ${proxyUrl.replace('http://', 'http://***:***@')}/ from HTTP_PROXY...`);
+    expect(r.out).not.toContain('proxy-secret');
+    expect(proxiedHosts).toContain(AUTH_HOST);
+    expect((await browse(['text'], env)).stdout).toContain(`via proxy for ${AUTH_HOST}`);
+  }, 120_000);
 });

browse/test/restart-env.test.ts

@@ -1,5 +1,6 @@
 import { describe, test, expect } from "bun:test";
-import { buildRestartEnv } from "../src/cli";
+import { buildHeadedServerEnv, buildRestartEnv, extractGlobalFlags } from "../src/cli";
+import { parseProxyConfig, serverProxyEnv } from "../src/proxy-config";
 
 // #1781: an auto-restart triggered by a plain command (no --headed flag) must
 // NOT silently downgrade a headed session to headless. buildRestartEnv reapplies
@@ -48,3 +49,36 @@ describe("buildRestartEnv (#1781 headed persistence)", () => {
     expect(env.BROWSE_CONFIG_HASH).toBe("fromstate");
   });
 });
+
+// #3457: the CLI folds BROWSE_PROXY_USER/PASS into the BROWSE_PROXY_URL it hands
+// the daemon, and the daemon inherits the CLI's environment. The daemon parses
+// that URL with the env creds exactly as server.ts does, so it must see one source.
+describe("daemon env carries one proxy credential source (#3457)", () => {
+  const userEnv = { BROWSE_PROXY_USER: "a", BROWSE_PROXY_PASS: "b" };
+  const daemonParse = (env: Record<string, string>) => {
+    const { url, source } = serverProxyEnv(env, "linux");
+    return parseProxyConfig({ proxyUrl: url!, envUser: env.BROWSE_PROXY_USER, envPass: env.BROWSE_PROXY_PASS, source });
+  };
+  const spellings: Array<[string, string[], Record<string, string>]> = [
+    ["--proxy", ["--proxy", "http://127.0.0.1:3128", "status"], {}],
+    ["BROWSE_PROXY_URL", ["status"], { BROWSE_PROXY_URL: "http://127.0.0.1:3128" }],
+    ["HTTPS_PROXY", ["status"], { HTTPS_PROXY: "http://127.0.0.1:3128" }],
+  ];
+
+  for (const [name, args, proxyEnv] of spellings) {
+    test(`${name} with BROWSE_PROXY_USER/PASS: start, restart and headed daemons parse the creds once`, () => {
+      const env = { ...userEnv, ...proxyEnv };
+      const flags = extractGlobalFlags(args, env, "linux");
+      for (const childEnv of [buildRestartEnv(flags, null), buildHeadedServerEnv(flags)]) {
+        const parsed = daemonParse({ ...env, ...childEnv });
+        expect(parsed).toMatchObject({ host: "127.0.0.1", port: 3128, userId: "a", password: "b" });
+      }
+    });
+  }
+
+  test("without a resolved proxy the daemon env leaves BROWSE_PROXY_USER/PASS alone", () => {
+    const env = buildRestartEnv(extractGlobalFlags(["status"], userEnv, "linux"), null);
+    expect(env.BROWSE_PROXY_USER).toBeUndefined();
+    expect(env.BROWSE_PROXY_PASS).toBeUndefined();
+  });
+});

changelog.d/gsta-217-browse-proxy-creds.md

@@ -0,0 +1,9 @@
+**`browse` starts again when proxy credentials come from `BROWSE_PROXY_USER` and `BROWSE_PROXY_PASS`.** With those two variables set beside `--proxy`, `BROWSE_PROXY_URL` or a standard proxy variable such as `HTTPS_PROXY`, the daemon exited at startup with `proxy creds set in both env (BROWSE_PROXY_USER) and URL — pick one source`, and the CLI reported only `Server failed to start within 15s`. The CLI puts those credentials into the proxy URL it hands the daemon, and the daemon also inherited the two variables, so it saw the same credentials twice and refused them. The daemon now gets the variables blanked whenever the CLI passes it a resolved proxy URL, so it reads the credentials once, from the URL. Setting credentials both in the URL and in the variables is still refused, with the same message, before any daemon starts.
+
+### Fixed
+- **`browse/src/cli.ts`**: the daemon environment for a resolved proxy sets `BROWSE_PROXY_USER` and `BROWSE_PROXY_PASS` to empty. This covers a normal start, an auto-restart, a headed `connect` and its supervisor respawn, and the Windows launcher (#3457).
+
+### Tests
+- `browse/test/restart-env.test.ts`: for `--proxy`, `BROWSE_PROXY_URL` and `HTTPS_PROXY` with `BROWSE_PROXY_USER`/`PASS`, the restart and headed daemon environments parse the way `server.ts` parses them and yield the credentials once. All three failed before the fix with the "pick one source" error.
+- `browse/test/proxy-env-daemon.test.ts`: an end-to-end start with `HTTP_PROXY` and `BROWSE_PROXY_USER`/`PASS` against a local proxy that demands Basic auth loads the page through the proxy. It timed out at the 15 s start limit before the fix.
+- `browse/test/cli-supervisor.test.ts`: the expected headed respawn environment includes the two blanked variables.