diff --git a/src/lib/proxy.test.ts b/src/lib/proxy.test.ts index 5c686af0..4cb3ccbe 100644 --- a/src/lib/proxy.test.ts +++ b/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(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> = []; + 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'); + }, + ); + }); }); diff --git a/src/lib/proxy.ts b/src/lib/proxy.ts index 04b625e6..fe452f2f 100644 --- a/src/lib/proxy.ts +++ b/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 diff --git a/test/proxy.subprocess.test.ts b/test/proxy.subprocess.test.ts new file mode 100644 index 00000000..ee306f23 --- /dev/null +++ b/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(); + +function listen(server: Server): Promise { + 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, +): 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); +});