diff --git a/docs/architecture.md b/docs/architecture.md index 2ef59d7f7c..a22130e211 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -499,7 +499,7 @@ The daemon and its plugin processes honor `HTTPS_PROXY`, `HTTP_PROXY`, and `NO_P (including lowercase forms, which take precedence). Fetch and relay/Hub WebSockets share this policy. Loopback (`localhost`, `127.0.0.1`, `::1`) always goes direct. A proxy without a scheme uses HTTP. Invalid values and unsupported proxy schemes -are ignored, with one warning naming the variable in `daemon.log`. +are treated as unset, with one warning naming the variable in `daemon.log`. Agent and terminal environments are unchanged. System proxy settings and SOCKS-only `ALL_PROXY` configurations are not supported. diff --git a/packages/server/src/server/network/index.ts b/packages/server/src/server/network/index.ts index a33a4c1352..03ce03c159 100644 --- a/packages/server/src/server/network/index.ts +++ b/packages/server/src/server/network/index.ts @@ -1,5 +1,5 @@ import type { Agent } from "node:http"; -import { Agent as DirectAgent, Dispatcher, EnvHttpProxyAgent, setGlobalDispatcher } from "undici"; +import { EnvHttpProxyAgent, setGlobalDispatcher } from "undici"; import { HttpsProxyAgent } from "https-proxy-agent"; interface ProxyEnvironment { @@ -15,11 +15,10 @@ function readEnvironment(env: NodeJS.ProcessEnv): { ignored: string[]; } { const ignored: string[] = []; - function proxy(lower: string, upper: string): string | undefined { - const variable = env[lower] !== undefined ? lower : upper; + function proxy(variable: string): string | undefined { const value = env[variable]; if (value === undefined) return undefined; - if (!value.trim()) return ""; + if (!value.trim()) return undefined; try { const url = new URL(value.includes("://") ? value : `http://${value}`); if (url.protocol !== "http:" && url.protocol !== "https:") @@ -29,14 +28,14 @@ function readEnvironment(env: NodeJS.ProcessEnv): { return url.href; } catch { ignored.push(variable); - return ""; + return undefined; } } - const httpProxy = proxy("http_proxy", "HTTP_PROXY") ?? ""; + const httpProxy = proxy("http_proxy") ?? proxy("HTTP_PROXY") ?? ""; return { environment: { httpProxy, - httpsProxy: proxy("https_proxy", "HTTPS_PROXY") ?? httpProxy, + httpsProxy: proxy("https_proxy") ?? proxy("HTTPS_PROXY") ?? httpProxy, noProxy: env.no_proxy ?? env.NO_PROXY ?? "", }, ignored, @@ -47,38 +46,12 @@ function bypassList(noProxy: string): string { return noProxy.trim() === "*" ? "*" : `${noProxy},localhost,127.0.0.1,[::1],::1`; } -// undici otherwise falls back to HTTP_PROXY when an explicitly ignored HTTPS proxy is empty. -class EnvironmentProxyAgent extends Dispatcher { - private readonly proxy: EnvHttpProxyAgent; - private readonly direct = new DirectAgent(); - private readonly options: ProxyEnvironment; - constructor(options: ProxyEnvironment) { - super(); - this.options = options; - this.proxy = new EnvHttpProxyAgent(options); - } - override dispatch( - options: Parameters[0], - handler: Parameters[1], - ): boolean { - if (!this.options.httpsProxy && String(options.origin).startsWith("https:")) - return this.direct.dispatch(options, handler); - return this.proxy.dispatch(options, handler); - } - override async close(): Promise { - await Promise.all([this.proxy.close(), this.direct.close()]); - } - override async destroy(): Promise { - await Promise.all([this.proxy.destroy(), this.direct.destroy()]); - } -} - /** Install once at startup; return ignored variable names for logging without exposing credentials. */ export function initializeEnvironmentProxy(env: NodeJS.ProcessEnv = process.env): string[] { const resolved = readEnvironment(env); environment = resolved.environment; setGlobalDispatcher( - new EnvironmentProxyAgent({ + new EnvHttpProxyAgent({ httpProxy: environment.httpProxy, httpsProxy: environment.httpsProxy, noProxy: bypassList(environment.noProxy), diff --git a/packages/server/src/server/network/network.test.ts b/packages/server/src/server/network/network.test.ts index 436c32358e..fc7ef28e00 100644 --- a/packages/server/src/server/network/network.test.ts +++ b/packages/server/src/server/network/network.test.ts @@ -123,11 +123,15 @@ test.each([ expect(initializeEnvironmentProxy({ HTTP_PROXY: recorder.url, https_proxy: value })).toEqual([ "https_proxy", ]); - expect(webSocketProxyAgent("wss://vendor.invalid")).toBeUndefined(); + expect(webSocketProxyAgent("wss://vendor.invalid")).toBeDefined(); await expect( fetch("https://vendor.invalid", { signal: AbortSignal.timeout(500) }), ).rejects.toThrow(); - expect(recorder.tunnels).toEqual([]); + const socket = new WebSocket("wss://vendor.invalid", { + agent: webSocketProxyAgent("wss://vendor.invalid"), + }); + await once(socket, "error"); + expect(recorder.tunnels).toEqual(["vendor.invalid:443", "vendor.invalid:443"]); }); test("daemon starts with an ignored proxy and writes one redacted warning to daemon.log", async () => { @@ -184,3 +188,21 @@ test("accepts a scheme-less corporate proxy hostname", () => { expect(initializeEnvironmentProxy({ https_proxy: "proxy.corp:8080" })).toEqual([]); expect(webSocketProxyAgent("wss://vendor.invalid")).toBeDefined(); }); + +test("an ignored lowercase variable falls back to the uppercase variable", async () => { + const recorder = await proxy(); + expect( + initializeEnvironmentProxy({ + https_proxy: "socks5://proxy.invalid", + HTTPS_PROXY: recorder.url, + }), + ).toEqual(["https_proxy"]); + await expect( + fetch("https://vendor.invalid", { signal: AbortSignal.timeout(500) }), + ).rejects.toThrow(); + const socket = new WebSocket("wss://vendor.invalid", { + agent: webSocketProxyAgent("wss://vendor.invalid"), + }); + await once(socket, "error"); + expect(recorder.tunnels).toEqual(["vendor.invalid:443", "vendor.invalid:443"]); +});