From 87876a3e36dbf067245ee727beaed7829a5b00c1 Mon Sep 17 00:00:00 2001 From: Tak Hoffman <781889+Takhoffman@users.noreply.github.com> Date: Wed, 11 Mar 2026 10:21:35 -0500 Subject: [PATCH] Fix env proxy bootstrap for model traffic (#43248) * Fix env proxy bootstrap for model traffic * Address proxy dispatcher review followups * Fix proxy env precedence for empty lowercase vars --- .../run/attempt.spawn-workspace.test.ts | 1 + src/agents/pi-embedded-runner/run/attempt.ts | 8 ++- src/infra/net/proxy-env.test.ts | 42 +++++++++++ src/infra/net/proxy-env.ts | 37 ++++++++++ src/infra/net/proxy-fetch.test.ts | 6 -- src/infra/net/proxy-fetch.ts | 8 +-- .../net/undici-global-dispatcher.test.ts | 70 +++++++++++++++++++ src/infra/net/undici-global-dispatcher.ts | 62 ++++++++++++---- src/telegram/fetch.ts | 9 +-- 9 files changed, 209 insertions(+), 34 deletions(-) create mode 100644 src/infra/net/proxy-env.test.ts diff --git a/src/agents/pi-embedded-runner/run/attempt.spawn-workspace.test.ts b/src/agents/pi-embedded-runner/run/attempt.spawn-workspace.test.ts index 0341ee97587..3801231f1f2 100644 --- a/src/agents/pi-embedded-runner/run/attempt.spawn-workspace.test.ts +++ b/src/agents/pi-embedded-runner/run/attempt.spawn-workspace.test.ts @@ -79,6 +79,7 @@ vi.mock("../../../infra/machine-name.js", () => ({ })); vi.mock("../../../infra/net/undici-global-dispatcher.js", () => ({ + ensureGlobalUndiciEnvProxyDispatcher: () => {}, ensureGlobalUndiciStreamTimeouts: () => {}, })); diff --git a/src/agents/pi-embedded-runner/run/attempt.ts b/src/agents/pi-embedded-runner/run/attempt.ts index 084a6d39746..0014475a880 100644 --- a/src/agents/pi-embedded-runner/run/attempt.ts +++ b/src/agents/pi-embedded-runner/run/attempt.ts @@ -11,7 +11,10 @@ import { resolveHeartbeatPrompt } from "../../../auto-reply/heartbeat.js"; import { resolveChannelCapabilities } from "../../../config/channel-capabilities.js"; import type { OpenClawConfig } from "../../../config/config.js"; import { getMachineDisplayName } from "../../../infra/machine-name.js"; -import { ensureGlobalUndiciStreamTimeouts } from "../../../infra/net/undici-global-dispatcher.js"; +import { + ensureGlobalUndiciEnvProxyDispatcher, + ensureGlobalUndiciStreamTimeouts, +} from "../../../infra/net/undici-global-dispatcher.js"; import { MAX_IMAGE_BYTES } from "../../../media/constants.js"; import { getGlobalHookRunner } from "../../../plugins/hook-runner-global.js"; import type { @@ -749,6 +752,9 @@ export async function runEmbeddedAttempt( const resolvedWorkspace = resolveUserPath(params.workspaceDir); const prevCwd = process.cwd(); const runAbortController = new AbortController(); + // Proxy bootstrap must happen before timeout tuning so the timeouts wrap the + // active EnvHttpProxyAgent instead of being replaced by a bare proxy dispatcher. + ensureGlobalUndiciEnvProxyDispatcher(); ensureGlobalUndiciStreamTimeouts(); log.debug( diff --git a/src/infra/net/proxy-env.test.ts b/src/infra/net/proxy-env.test.ts new file mode 100644 index 00000000000..37b910f1769 --- /dev/null +++ b/src/infra/net/proxy-env.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it } from "vitest"; +import { hasEnvHttpProxyConfigured, resolveEnvHttpProxyUrl } from "./proxy-env.js"; + +describe("resolveEnvHttpProxyUrl", () => { + it("uses lower-case https_proxy before upper-case HTTPS_PROXY", () => { + const env = { + https_proxy: "http://lower.test:8080", + HTTPS_PROXY: "http://upper.test:8080", + } as NodeJS.ProcessEnv; + + expect(resolveEnvHttpProxyUrl("https", env)).toBe("http://lower.test:8080"); + }); + + it("treats empty lower-case https_proxy as authoritative over upper-case HTTPS_PROXY", () => { + const env = { + https_proxy: "", + HTTPS_PROXY: "http://upper.test:8080", + } as NodeJS.ProcessEnv; + + expect(resolveEnvHttpProxyUrl("https", env)).toBeUndefined(); + expect(hasEnvHttpProxyConfigured("https", env)).toBe(false); + }); + + it("treats empty lower-case http_proxy as authoritative over upper-case HTTP_PROXY", () => { + const env = { + http_proxy: " ", + HTTP_PROXY: "http://upper-http.test:8080", + } as NodeJS.ProcessEnv; + + expect(resolveEnvHttpProxyUrl("http", env)).toBeUndefined(); + expect(hasEnvHttpProxyConfigured("http", env)).toBe(false); + }); + + it("falls back from HTTPS proxy vars to HTTP proxy vars for https requests", () => { + const env = { + HTTP_PROXY: "http://upper-http.test:8080", + } as NodeJS.ProcessEnv; + + expect(resolveEnvHttpProxyUrl("https", env)).toBe("http://upper-http.test:8080"); + expect(hasEnvHttpProxyConfigured("https", env)).toBe(true); + }); +}); diff --git a/src/infra/net/proxy-env.ts b/src/infra/net/proxy-env.ts index 01401074678..c0c332c7301 100644 --- a/src/infra/net/proxy-env.ts +++ b/src/infra/net/proxy-env.ts @@ -16,3 +16,40 @@ export function hasProxyEnvConfigured(env: NodeJS.ProcessEnv = process.env): boo } return false; } + +function normalizeProxyEnvValue(value: string | undefined): string | null | undefined { + if (typeof value !== "string") { + return undefined; + } + const trimmed = value.trim(); + return trimmed.length > 0 ? trimmed : null; +} + +/** + * Match undici EnvHttpProxyAgent semantics for env-based HTTP/S proxy selection: + * - lower-case vars take precedence over upper-case + * - HTTPS requests prefer https_proxy/HTTPS_PROXY, then fall back to http_proxy/HTTP_PROXY + * - ALL_PROXY is ignored by EnvHttpProxyAgent + */ +export function resolveEnvHttpProxyUrl( + protocol: "http" | "https", + env: NodeJS.ProcessEnv = process.env, +): string | undefined { + const lowerHttpProxy = normalizeProxyEnvValue(env.http_proxy); + const lowerHttpsProxy = normalizeProxyEnvValue(env.https_proxy); + const httpProxy = + lowerHttpProxy !== undefined ? lowerHttpProxy : normalizeProxyEnvValue(env.HTTP_PROXY); + const httpsProxy = + lowerHttpsProxy !== undefined ? lowerHttpsProxy : normalizeProxyEnvValue(env.HTTPS_PROXY); + if (protocol === "https") { + return httpsProxy ?? httpProxy ?? undefined; + } + return httpProxy ?? undefined; +} + +export function hasEnvHttpProxyConfigured( + protocol: "http" | "https" = "https", + env: NodeJS.ProcessEnv = process.env, +): boolean { + return resolveEnvHttpProxyUrl(protocol, env) !== undefined; +} diff --git a/src/infra/net/proxy-fetch.test.ts b/src/infra/net/proxy-fetch.test.ts index a10c83d1a07..331cd1ac6ea 100644 --- a/src/infra/net/proxy-fetch.test.ts +++ b/src/infra/net/proxy-fetch.test.ts @@ -73,11 +73,7 @@ describe("resolveProxyFetchFromEnv", () => { }); it("returns proxy fetch using EnvHttpProxyAgent when HTTPS_PROXY is set", async () => { - // Stub empty vars first — on Windows, process.env is case-insensitive so - // HTTPS_PROXY and https_proxy share the same slot. Value must be set LAST. vi.stubEnv("HTTP_PROXY", ""); - vi.stubEnv("https_proxy", ""); - vi.stubEnv("http_proxy", ""); vi.stubEnv("HTTPS_PROXY", "http://proxy.test:8080"); undiciFetch.mockResolvedValue({ ok: true }); @@ -94,8 +90,6 @@ describe("resolveProxyFetchFromEnv", () => { it("returns proxy fetch when HTTP_PROXY is set", () => { vi.stubEnv("HTTPS_PROXY", ""); - vi.stubEnv("https_proxy", ""); - vi.stubEnv("http_proxy", ""); vi.stubEnv("HTTP_PROXY", "http://fallback.test:3128"); const fetchFn = resolveProxyFetchFromEnv(); diff --git a/src/infra/net/proxy-fetch.ts b/src/infra/net/proxy-fetch.ts index 391387f3cca..7305cbfcc5c 100644 --- a/src/infra/net/proxy-fetch.ts +++ b/src/infra/net/proxy-fetch.ts @@ -1,5 +1,6 @@ import { EnvHttpProxyAgent, ProxyAgent, fetch as undiciFetch } from "undici"; import { logWarn } from "../../logger.js"; +import { hasEnvHttpProxyConfigured } from "./proxy-env.js"; export const PROXY_FETCH_PROXY_URL = Symbol.for("openclaw.proxyFetch.proxyUrl"); type ProxyFetchWithMetadata = typeof fetch & { @@ -51,12 +52,7 @@ export function getProxyUrlFromFetch(fetchImpl?: typeof fetch): string | undefin * Gracefully returns undefined if the proxy URL is malformed. */ export function resolveProxyFetchFromEnv(): typeof fetch | undefined { - const proxyUrl = - process.env.HTTPS_PROXY || - process.env.HTTP_PROXY || - process.env.https_proxy || - process.env.http_proxy; - if (!proxyUrl?.trim()) { + if (!hasEnvHttpProxyConfigured("https")) { return undefined; } try { diff --git a/src/infra/net/undici-global-dispatcher.test.ts b/src/infra/net/undici-global-dispatcher.test.ts index 0c4d5793b57..8b14c4084fc 100644 --- a/src/infra/net/undici-global-dispatcher.test.ts +++ b/src/infra/net/undici-global-dispatcher.test.ts @@ -57,8 +57,14 @@ vi.mock("node:net", () => ({ getDefaultAutoSelectFamily, })); +vi.mock("./proxy-env.js", () => ({ + hasEnvHttpProxyConfigured: vi.fn(() => false), +})); + +import { hasEnvHttpProxyConfigured } from "./proxy-env.js"; import { DEFAULT_UNDICI_STREAM_TIMEOUT_MS, + ensureGlobalUndiciEnvProxyDispatcher, ensureGlobalUndiciStreamTimeouts, resetGlobalUndiciStreamTimeoutsForTests, } from "./undici-global-dispatcher.js"; @@ -69,6 +75,7 @@ describe("ensureGlobalUndiciStreamTimeouts", () => { resetGlobalUndiciStreamTimeoutsForTests(); setCurrentDispatcher(new Agent()); getDefaultAutoSelectFamily.mockReturnValue(undefined); + vi.mocked(hasEnvHttpProxyConfigured).mockReturnValue(false); }); it("replaces default Agent dispatcher with extended stream timeouts", () => { @@ -136,3 +143,66 @@ describe("ensureGlobalUndiciStreamTimeouts", () => { }); }); }); + +describe("ensureGlobalUndiciEnvProxyDispatcher", () => { + beforeEach(() => { + vi.clearAllMocks(); + resetGlobalUndiciStreamTimeoutsForTests(); + setCurrentDispatcher(new Agent()); + vi.mocked(hasEnvHttpProxyConfigured).mockReturnValue(false); + }); + + it("installs EnvHttpProxyAgent when env HTTP proxy is configured on a default Agent", () => { + vi.mocked(hasEnvHttpProxyConfigured).mockReturnValue(true); + + ensureGlobalUndiciEnvProxyDispatcher(); + + expect(setGlobalDispatcher).toHaveBeenCalledTimes(1); + expect(getCurrentDispatcher()).toBeInstanceOf(EnvHttpProxyAgent); + }); + + it("does not override unsupported custom proxy dispatcher types", () => { + vi.mocked(hasEnvHttpProxyConfigured).mockReturnValue(true); + setCurrentDispatcher(new ProxyAgent("http://proxy.test:8080")); + + ensureGlobalUndiciEnvProxyDispatcher(); + + expect(setGlobalDispatcher).not.toHaveBeenCalled(); + }); + + it("retries proxy bootstrap after an unsupported dispatcher later becomes a default Agent", () => { + vi.mocked(hasEnvHttpProxyConfigured).mockReturnValue(true); + setCurrentDispatcher(new ProxyAgent("http://proxy.test:8080")); + + ensureGlobalUndiciEnvProxyDispatcher(); + expect(setGlobalDispatcher).not.toHaveBeenCalled(); + + setCurrentDispatcher(new Agent()); + ensureGlobalUndiciEnvProxyDispatcher(); + + expect(setGlobalDispatcher).toHaveBeenCalledTimes(1); + expect(getCurrentDispatcher()).toBeInstanceOf(EnvHttpProxyAgent); + }); + + it("is idempotent after proxy bootstrap succeeds", () => { + vi.mocked(hasEnvHttpProxyConfigured).mockReturnValue(true); + + ensureGlobalUndiciEnvProxyDispatcher(); + ensureGlobalUndiciEnvProxyDispatcher(); + + expect(setGlobalDispatcher).toHaveBeenCalledTimes(1); + }); + + it("reinstalls env proxy if an external change later reverts the dispatcher to Agent", () => { + vi.mocked(hasEnvHttpProxyConfigured).mockReturnValue(true); + + ensureGlobalUndiciEnvProxyDispatcher(); + expect(setGlobalDispatcher).toHaveBeenCalledTimes(1); + + setCurrentDispatcher(new Agent()); + ensureGlobalUndiciEnvProxyDispatcher(); + + expect(setGlobalDispatcher).toHaveBeenCalledTimes(2); + expect(getCurrentDispatcher()).toBeInstanceOf(EnvHttpProxyAgent); + }); +}); diff --git a/src/infra/net/undici-global-dispatcher.ts b/src/infra/net/undici-global-dispatcher.ts index b63ff5688bb..994af564777 100644 --- a/src/infra/net/undici-global-dispatcher.ts +++ b/src/infra/net/undici-global-dispatcher.ts @@ -1,11 +1,13 @@ import * as net from "node:net"; import { Agent, EnvHttpProxyAgent, getGlobalDispatcher, setGlobalDispatcher } from "undici"; +import { hasEnvHttpProxyConfigured } from "./proxy-env.js"; export const DEFAULT_UNDICI_STREAM_TIMEOUT_MS = 30 * 60 * 1000; const AUTO_SELECT_FAMILY_ATTEMPT_TIMEOUT_MS = 300; -let lastAppliedDispatcherKey: string | null = null; +let lastAppliedTimeoutKey: string | null = null; +let lastAppliedProxyBootstrap = false; type DispatcherKind = "agent" | "env-proxy" | "unsupported"; @@ -59,28 +61,59 @@ function resolveDispatcherKey(params: { return `${params.kind}:${params.timeoutMs}:${autoSelectToken}`; } +function resolveCurrentDispatcherKind(): DispatcherKind | null { + let dispatcher: unknown; + try { + dispatcher = getGlobalDispatcher(); + } catch { + return null; + } + + const currentKind = resolveDispatcherKind(dispatcher); + return currentKind === "unsupported" ? null : currentKind; +} + +export function ensureGlobalUndiciEnvProxyDispatcher(): void { + const shouldUseEnvProxy = hasEnvHttpProxyConfigured("https"); + if (!shouldUseEnvProxy) { + return; + } + if (lastAppliedProxyBootstrap) { + if (resolveCurrentDispatcherKind() === "env-proxy") { + return; + } + lastAppliedProxyBootstrap = false; + } + const currentKind = resolveCurrentDispatcherKind(); + if (currentKind === null) { + return; + } + if (currentKind === "env-proxy") { + lastAppliedProxyBootstrap = true; + return; + } + try { + setGlobalDispatcher(new EnvHttpProxyAgent()); + lastAppliedProxyBootstrap = true; + } catch { + // Best-effort bootstrap only. + } +} + export function ensureGlobalUndiciStreamTimeouts(opts?: { timeoutMs?: number }): void { const timeoutMsRaw = opts?.timeoutMs ?? DEFAULT_UNDICI_STREAM_TIMEOUT_MS; const timeoutMs = Math.max(1, Math.floor(timeoutMsRaw)); if (!Number.isFinite(timeoutMsRaw)) { return; } - - let dispatcher: unknown; - try { - dispatcher = getGlobalDispatcher(); - } catch { - return; - } - - const kind = resolveDispatcherKind(dispatcher); - if (kind === "unsupported") { + const kind = resolveCurrentDispatcherKind(); + if (kind === null) { return; } const autoSelectFamily = resolveAutoSelectFamily(); const nextKey = resolveDispatcherKey({ kind, timeoutMs, autoSelectFamily }); - if (lastAppliedDispatcherKey === nextKey) { + if (lastAppliedTimeoutKey === nextKey) { return; } @@ -102,12 +135,13 @@ export function ensureGlobalUndiciStreamTimeouts(opts?: { timeoutMs?: number }): }), ); } - lastAppliedDispatcherKey = nextKey; + lastAppliedTimeoutKey = nextKey; } catch { // Best-effort hardening only. } } export function resetGlobalUndiciStreamTimeoutsForTests(): void { - lastAppliedDispatcherKey = null; + lastAppliedTimeoutKey = null; + lastAppliedProxyBootstrap = false; } diff --git a/src/telegram/fetch.ts b/src/telegram/fetch.ts index 3934c10c391..a6b2cec4810 100644 --- a/src/telegram/fetch.ts +++ b/src/telegram/fetch.ts @@ -2,6 +2,7 @@ import * as dns from "node:dns"; import { Agent, EnvHttpProxyAgent, ProxyAgent, fetch as undiciFetch } from "undici"; import type { TelegramNetworkConfig } from "../config/types.telegram.js"; import { resolveFetch } from "../infra/fetch.js"; +import { hasEnvHttpProxyConfigured } from "../infra/net/proxy-env.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; import { resolveTelegramAutoSelectFamilyDecision, @@ -177,13 +178,7 @@ function shouldBypassEnvProxyForTelegramApi(env: NodeJS.ProcessEnv = process.env } function hasEnvHttpProxyForTelegramApi(env: NodeJS.ProcessEnv = process.env): boolean { - // Match EnvHttpProxyAgent behavior (undici) for HTTPS requests: - // - lower-case env vars take precedence over upper-case - // - HTTPS requests use https_proxy/HTTPS_PROXY first, then fall back to http_proxy/HTTP_PROXY - // - ALL_PROXY is ignored by EnvHttpProxyAgent - const httpProxy = env.http_proxy ?? env.HTTP_PROXY; - const httpsProxy = env.https_proxy ?? env.HTTPS_PROXY; - return Boolean(httpProxy) || Boolean(httpsProxy); + return hasEnvHttpProxyConfigured("https", env); } function createTelegramDispatcher(params: {