fix(http): accept proxy variables written without a scheme

src/lib/proxy.test.ts

@@ -1,6 +1,23 @@
-import { describe, expect, it, vi } from 'vitest';
+import { createServer, type Server } from 'node:http';
+import type { AddressInfo } from 'node:net';
+import type { Dispatcher } from 'undici';
+import { fetch } from 'undici';
+import { afterEach, describe, expect, it, vi } from 'vitest';
 import { maybeInstallProxyAgent } from './proxy.js';
 
+// A local proxy that records each CONNECT target and refuses the tunnel, so a
+// test can see which proxy a request went to without any external network.
+async function startRecordingProxy(): Promise<{ server: Server; port: number; seen: string[] }> {
+  const seen: string[] = [];
+  const server = createServer((_req, res) => res.writeHead(502).end());
+  server.on('connect', (req, socket) => {
+    seen.push(req.url ?? '');
+    socket.end('HTTP/1.1 502 Bad Gateway\r\n\r\n');
+  });
+  await new Promise<void>(resolve => server.listen(0, '127.0.0.1', resolve));
+  return { server, port: (server.address() as AddressInfo).port, seen };
+}
+
 describe('maybeInstallProxyAgent', () => {
   it('installs an agent when HTTPS_PROXY is set', () => {
     const install = vi.fn();
@@ -56,4 +73,103 @@ describe('maybeInstallProxyAgent', () => {
     expect(installed).toBe(true);
     expect(install).toHaveBeenCalledTimes(1);
   });
+
+  describe('proxy values written without a scheme', () => {
+    const cleanups: Array<() => Promise<unknown>> = [];
+    afterEach(async () => {
+      vi.unstubAllEnvs();
+      await Promise.all(cleanups.splice(0).map(cleanup => cleanup()));
+    });
+
+    async function routeThrough(env: NodeJS.ProcessEnv, target: string) {
+      const proxy = await startRecordingProxy();
+      cleanups.push(() => new Promise(resolve => proxy.server.close(resolve)));
+      const errs: string[] = [];
+      let agent: Dispatcher | undefined;
+      const installed = maybeInstallProxyAgent({
+        env: Object.fromEntries(
+          Object.entries(env).map(([k, v]) => [k, v?.replace('PORT', String(proxy.port))]),
+        ),
+        install: a => {
+          agent = a;
+          cleanups.push(() => a.close());
+        },
+        stderr: line => errs.push(line),
+      });
+      const url = target.replace('PORT', String(proxy.port));
+      if (agent) await fetch(url, { dispatcher: agent }).catch(() => undefined);
+      return { installed, errs, seen: proxy.seen, tunnel: new URL(url).host };
+    }
+
+    it.each([
+      ['HTTPS_PROXY', 'https://127.0.0.1:PORT/me'],
+      ['http_proxy', 'http://127.0.0.1:PORT/me'],
+    ] as const)('routes through a host:port %s value as http://', async (name, target) => {
+      const result = await routeThrough({ [name]: '127.0.0.1:PORT' }, target);
+      expect(result.installed).toBe(true);
+      expect(result.errs).toEqual([]);
+      expect(result.seen).toEqual([result.tunnel]);
+    });
+
+    it('keeps HTTPS falling back to HTTP_PROXY when only HTTP_PROXY is set', async () => {
+      const result = await routeThrough(
+        { HTTP_PROXY: '127.0.0.1:PORT' },
+        'https://127.0.0.1:PORT/me',
+      );
+      expect(result.seen).toEqual([result.tunnel]);
+    });
+
+    it('prefers the lower-case spelling, as undici does', async () => {
+      const result = await routeThrough(
+        { https_proxy: '127.0.0.1:PORT', HTTPS_PROXY: 'ftp://127.0.0.1:1' },
+        'https://127.0.0.1:PORT/me',
+      );
+      expect(result.errs).toEqual([]);
+      expect(result.seen).toEqual([result.tunnel]);
+    });
+
+    it('leaves full http:// values unchanged', async () => {
+      const result = await routeThrough(
+        { HTTPS_PROXY: 'http://127.0.0.1:PORT' },
+        'https://127.0.0.1:PORT/me',
+      );
+      expect(result.errs).toEqual([]);
+      expect(result.seen).toEqual([result.tunnel]);
+    });
+
+    it('still lets NO_PROXY send a matching host direct', async () => {
+      const result = await routeThrough(
+        { HTTPS_PROXY: '127.0.0.1:PORT', no_proxy: '127.0.0.1' },
+        'https://127.0.0.1:PORT/me',
+      );
+      expect(result.installed).toBe(true);
+      expect(result.seen).toEqual([]);
+    });
+
+    it('reads NO_PROXY from the same env as the proxy, not process.env', async () => {
+      vi.stubEnv('no_proxy', '127.0.0.1');
+      vi.stubEnv('NO_PROXY', '127.0.0.1');
+      const result = await routeThrough(
+        { HTTPS_PROXY: '127.0.0.1:PORT' },
+        'https://127.0.0.1:PORT/me',
+      );
+      expect(result.seen).toEqual([result.tunnel]);
+    });
+
+    it.each(['socks5h://127.0.0.1:1080', 'ftp://127.0.0.1:2121'])(
+      'still warns and falls back on an unsupported scheme (%s)',
+      value => {
+        const errs: string[] = [];
+        const install = vi.fn();
+        const installed = maybeInstallProxyAgent({
+          env: { HTTPS_PROXY: value },
+          install,
+          stderr: line => errs.push(line),
+        });
+        expect(installed).toBe(false);
+        expect(install).not.toHaveBeenCalled();
+        expect(errs.join('\n')).toContain('ignoring proxy environment');
+      },
+    );
+  });
 });

src/lib/proxy.ts

@@ -36,15 +36,25 @@ export function maybeInstallProxyAgent(deps: ProxyDeps = {}): boolean {
   );
   if (!hasProxy) return false;
   const install = deps.install ?? setGlobalDispatcher;
-  // EnvHttpProxyAgent reads HTTPS_PROXY/HTTP_PROXY/NO_PROXY itself, per
-  // request, so NO_PROXY exemptions apply without extra plumbing here.
+  // The proxy URLs are read here, in undici's own order (lower-case first;
+  // undici itself falls back from HTTPS to the HTTP proxy), so a `host:port`
+  // value can get the `http://` curl assumes; undici alone rejects it.
+  // NO_PROXY comes from the same env, and `''` stops undici from re-reading
+  // process.env, so the agent has one source. Matching NO_PROXY entries is
+  // left to undici.
   //
-  // A malformed or unsupported proxy value (e.g. `socks5://...`) makes the
-  // agent throw. Because this runs at startup, an unguarded throw would abort
-  // every command before the CLI's own error handling — so fall back to the
-  // default (proxy-less) dispatcher and warn instead of crashing.
+  // A malformed or unsupported proxy value (e.g. `socks5h://` or `ftp://`)
+  // makes the agent throw. Because this runs at startup, an unguarded throw
+  // would abort every command before the CLI's own error handling — so fall
+  // back to the default (proxy-less) dispatcher and warn instead of crashing.
   try {
-    install(new EnvHttpProxyAgent());
+    install(
+      new EnvHttpProxyAgent({
+        httpProxy: withScheme(env.http_proxy ?? env.HTTP_PROXY),
+        httpsProxy: withScheme(env.https_proxy ?? env.HTTPS_PROXY),
+        noProxy: env.no_proxy ?? env.NO_PROXY ?? '',
+      }),
+    );
     return true;
   } catch (error) {
     const stderr = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`));
@@ -57,6 +67,11 @@ export function maybeInstallProxyAgent(deps: ProxyDeps = {}): boolean {
   }
 }
 
+function withScheme(value: string | undefined): string {
+  if (!value) return '';
+  return /^[a-z][a-z\d+.-]*:\/\//i.test(value) ? value : `http://${value}`;
+}
+
 /**
  * Whether the currently-installed global dispatcher is the proxy-aware
  * agent `maybeInstallProxyAgent` installs — i.e., whether a proxy is

test/proxy.subprocess.test.ts

@@ -0,0 +1,136 @@
+/**
+ * Subprocess test for proxy variables written as `host:port` (no scheme), the
+ * way many corporate machines and CI runners set them. The API URL points at
+ * a closed local port that only the proxy can "reach": the in-test proxy
+ * answers every CONNECT by tunnelling to the local API server, so the command
+ * succeeds only if the built CLI really sends its traffic through the proxy.
+ */
+
+import { spawn } from 'node:child_process';
+import { mkdtempSync, rmSync } from 'node:fs';
+import { createServer, type Server } from 'node:http';
+import { connect, type AddressInfo, type Socket } from 'node:net';
+import { tmpdir } from 'node:os';
+import { dirname, join, resolve } from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { afterAll, beforeAll, describe, expect, it } from 'vitest';
+import { assertFreshBuild } from './helpers/assertFreshBuild.js';
+
+const __dirname = dirname(fileURLToPath(import.meta.url));
+const REPO_ROOT = resolve(__dirname, '..');
+const BIN_PATH = join(REPO_ROOT, 'dist', 'index.js');
+
+const ME_BODY = {
+  userId: 'u-proxy',
+  keyId: 'k-proxy',
+  scopes: ['read:projects'],
+  env: 'development',
+};
+
+let api: Server;
+let proxy: Server;
+let proxyPort: number;
+let unreachableApiUrl: string;
+let tmpHome: string;
+const tunnels: string[] = [];
+const sockets = new Set<Socket>();
+
+function listen(server: Server): Promise<number> {
+  return new Promise(resolveListen => {
+    server.listen(0, '127.0.0.1', () => resolveListen((server.address() as AddressInfo).port));
+  });
+}
+
+beforeAll(async () => {
+  assertFreshBuild(REPO_ROOT, BIN_PATH);
+  api = createServer((req, res) => {
+    if (req.url === '/api/cli/v1/me') {
+      res.writeHead(200, { 'content-type': 'application/json' });
+      res.end(JSON.stringify(ME_BODY));
+      return;
+    }
+    res.writeHead(404).end();
+  });
+  const apiPort = await listen(api);
+
+  proxy = createServer((_req, res) => res.writeHead(502).end());
+  proxy.on('connect', (req, client: Socket, head: Buffer) => {
+    tunnels.push(req.url ?? '');
+    const upstream = connect(apiPort, '127.0.0.1', () => {
+      client.write('HTTP/1.1 200 Connection Established\r\n\r\n');
+      upstream.write(head);
+      upstream.pipe(client);
+      client.pipe(upstream);
+    });
+    for (const socket of [client, upstream]) {
+      sockets.add(socket);
+      socket.on('error', () => socket.destroy());
+      socket.on('close', () => sockets.delete(socket));
+    }
+  });
+  proxyPort = await listen(proxy);
+
+  const closed = createServer();
+  const closedPort = await listen(closed);
+  await new Promise(resolveClose => closed.close(resolveClose));
+  unreachableApiUrl = `http://127.0.0.1:${closedPort}`;
+
+  tmpHome = mkdtempSync(join(tmpdir(), 'testsprite-proxy-'));
+});
+
+afterAll(async () => {
+  for (const socket of sockets) socket.destroy();
+  await Promise.all(
+    [api, proxy].map(server =>
+      server ? new Promise(resolveClose => server.close(resolveClose)) : undefined,
+    ),
+  );
+  if (tmpHome) rmSync(tmpHome, { recursive: true, force: true });
+});
+
+function runCli(
+  args: string[],
+  envOverrides: Record<string, string>,
+): Promise<{ exitCode: number; stdout: string; stderr: string }> {
+  return new Promise((resolveResult, rejectResult) => {
+    const child = spawn(process.execPath, [BIN_PATH, ...args], {
+      cwd: REPO_ROOT,
+      env: {
+        ...process.env,
+        HOME: tmpHome,
+        USERPROFILE: tmpHome,
+        HTTP_PROXY: undefined,
+        HTTPS_PROXY: undefined,
+        http_proxy: undefined,
+        https_proxy: undefined,
+        NO_PROXY: undefined,
+        no_proxy: undefined,
+        TESTSPRITE_NO_UPDATE_NOTIFIER: '1',
+        TESTSPRITE_NO_TELEMETRY: '1',
+        NO_COLOR: '1',
+        ...envOverrides,
+      } as NodeJS.ProcessEnv,
+    });
+    let stdout = '';
+    let stderr = '';
+    child.stdout.on('data', chunk => (stdout += chunk.toString()));
+    child.stderr.on('data', chunk => (stderr += chunk.toString()));
+    child.on('error', rejectResult);
+    child.on('close', code => resolveResult({ exitCode: code ?? -1, stdout, stderr }));
+  });
+}
+
+describe('proxy variables without a scheme (built CLI)', () => {
+  it('sends auth status through a host:port proxy and exits 0', async () => {
+    const result = await runCli(['auth', 'status', '--output', 'json'], {
+      TESTSPRITE_API_KEY: 'sk-user-proxy',
+      TESTSPRITE_API_URL: unreachableApiUrl,
+      HTTP_PROXY: `127.0.0.1:${proxyPort}`,
+      HTTPS_PROXY: `127.0.0.1:${proxyPort}`,
+    });
+    expect(result.stderr).not.toContain('ignoring proxy environment');
+    expect(result.exitCode).toBe(0);
+    expect(JSON.parse(result.stdout)).toEqual(ME_BODY);
+    expect(tunnels).toContain(new URL(unreachableApiUrl).host);
+  }, 30_000);
+});