diff --git a/src/agents/tools/web-guarded-fetch.test.ts b/src/agents/tools/web-guarded-fetch.test.ts index 87b0ba9a5ae..005a94ad3da 100644 --- a/src/agents/tools/web-guarded-fetch.test.ts +++ b/src/agents/tools/web-guarded-fetch.test.ts @@ -1,10 +1,25 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { fetchWithSsrFGuard } from "../../infra/net/fetch-guard.js"; +import { fetchWithSsrFGuard, GUARDED_FETCH_MODE } from "../../infra/net/fetch-guard.js"; import { withStrictWebToolsEndpoint, withTrustedWebToolsEndpoint } from "./web-guarded-fetch.js"; -vi.mock("../../infra/net/fetch-guard.js", () => ({ - fetchWithSsrFGuard: vi.fn(), -})); +vi.mock("../../infra/net/fetch-guard.js", () => { + const GUARDED_FETCH_MODE = { + STRICT: "strict", + TRUSTED_ENV_PROXY: "trusted_env_proxy", + } as const; + return { + GUARDED_FETCH_MODE, + fetchWithSsrFGuard: vi.fn(), + withStrictGuardedFetchMode: (params: Record) => ({ + ...params, + mode: GUARDED_FETCH_MODE.STRICT, + }), + withTrustedEnvProxyGuardedFetchMode: (params: Record) => ({ + ...params, + mode: GUARDED_FETCH_MODE.TRUSTED_ENV_PROXY, + }), + }; +}); describe("web-guarded-fetch", () => { afterEach(() => { @@ -27,8 +42,7 @@ describe("web-guarded-fetch", () => { dangerouslyAllowPrivateNetwork: true, allowRfc2544BenchmarkRange: true, }), - proxy: "env", - dangerouslyAllowEnvProxyWithoutPinnedDns: true, + mode: GUARDED_FETCH_MODE.TRUSTED_ENV_PROXY, }), ); }); @@ -49,7 +63,6 @@ describe("web-guarded-fetch", () => { ); const call = vi.mocked(fetchWithSsrFGuard).mock.calls[0]?.[0]; expect(call?.policy).toBeUndefined(); - expect(call?.proxy).toBeUndefined(); - expect(call?.dangerouslyAllowEnvProxyWithoutPinnedDns).toBeUndefined(); + expect(call?.mode).toBe(GUARDED_FETCH_MODE.STRICT); }); }); diff --git a/src/agents/tools/web-guarded-fetch.ts b/src/agents/tools/web-guarded-fetch.ts index c6c644fb4e2..aa4e8274cf9 100644 --- a/src/agents/tools/web-guarded-fetch.ts +++ b/src/agents/tools/web-guarded-fetch.ts @@ -2,6 +2,8 @@ import { fetchWithSsrFGuard, type GuardedFetchOptions, type GuardedFetchResult, + withStrictGuardedFetchMode, + withTrustedEnvProxyGuardedFetchMode, } from "../../infra/net/fetch-guard.js"; import type { SsrFPolicy } from "../../infra/net/ssrf.js"; @@ -12,7 +14,7 @@ const WEB_TOOLS_TRUSTED_NETWORK_SSRF_POLICY: SsrFPolicy = { type WebToolGuardedFetchOptions = Omit< GuardedFetchOptions, - "proxy" | "dangerouslyAllowEnvProxyWithoutPinnedDns" + "mode" | "proxy" | "dangerouslyAllowEnvProxyWithoutPinnedDns" > & { timeoutSeconds?: number; useEnvProxy?: boolean; @@ -36,16 +38,15 @@ export async function fetchWithWebToolsNetworkGuard( params: WebToolGuardedFetchOptions, ): Promise { const { timeoutSeconds, useEnvProxy, ...rest } = params; - return fetchWithSsrFGuard({ + const resolved = { ...rest, timeoutMs: resolveTimeoutMs({ timeoutMs: rest.timeoutMs, timeoutSeconds }), - ...(useEnvProxy - ? { - proxy: "env", - dangerouslyAllowEnvProxyWithoutPinnedDns: true, - } - : {}), - }); + }; + return fetchWithSsrFGuard( + useEnvProxy + ? withTrustedEnvProxyGuardedFetchMode(resolved) + : withStrictGuardedFetchMode(resolved), + ); } async function withWebToolsNetworkGuard( diff --git a/src/browser/cdp-proxy-bypass.ts b/src/browser/cdp-proxy-bypass.ts index 7f579370de2..8db5276fc51 100644 --- a/src/browser/cdp-proxy-bypass.ts +++ b/src/browser/cdp-proxy-bypass.ts @@ -10,6 +10,7 @@ import http from "node:http"; import https from "node:https"; import { isLoopbackHost } from "../gateway/net.js"; +import { hasProxyEnvConfigured } from "../infra/net/proxy-env.js"; /** HTTP agent that never uses a proxy — for localhost CDP connections. */ const directHttpAgent = new http.Agent(); @@ -39,15 +40,7 @@ export function getDirectAgentForCdp(url: string): http.Agent | https.Agent | un * interfere with loopback connections. */ export function hasProxyEnv(): boolean { - const env = process.env; - return Boolean( - env.HTTP_PROXY || - env.http_proxy || - env.HTTPS_PROXY || - env.https_proxy || - env.ALL_PROXY || - env.all_proxy, - ); + return hasProxyEnvConfigured(); } const LOOPBACK_ENTRIES = "localhost,127.0.0.1,[::1]"; diff --git a/src/browser/navigation-guard.ts b/src/browser/navigation-guard.ts index 73d1b41ba3d..496dee19469 100644 --- a/src/browser/navigation-guard.ts +++ b/src/browser/navigation-guard.ts @@ -1,4 +1,6 @@ +import { hasProxyEnvConfigured } from "../infra/net/proxy-env.js"; import { + isPrivateNetworkAllowedByPolicy, resolvePinnedHostnameWithPolicy, type LookupFn, type SsrFPolicy, @@ -6,28 +8,6 @@ import { const NETWORK_NAVIGATION_PROTOCOLS = new Set(["http:", "https:"]); const SAFE_NON_NETWORK_URLS = new Set(["about:blank"]); -const ENV_PROXY_KEYS = [ - "HTTP_PROXY", - "HTTPS_PROXY", - "ALL_PROXY", - "http_proxy", - "https_proxy", - "all_proxy", -] as const; - -function hasEnvProxyConfigured(): boolean { - for (const key of ENV_PROXY_KEYS) { - const value = process.env[key]; - if (typeof value === "string" && value.trim().length > 0) { - return true; - } - } - return false; -} - -function allowsPrivateNetworkNavigation(policy?: SsrFPolicy): boolean { - return policy?.dangerouslyAllowPrivateNetwork === true || policy?.allowPrivateNetwork === true; -} function isAllowedNonNetworkNavigationUrl(parsed: URL): boolean { // Keep non-network navigation explicit; about:blank is the only allowed bootstrap URL. @@ -82,7 +62,7 @@ export async function assertBrowserNavigationAllowed( // can bypass strict destination-binding intent from pre-navigation DNS checks. // In strict mode, fail closed unless private-network navigation is explicitly // enabled by policy. - if (hasEnvProxyConfigured() && !allowsPrivateNetworkNavigation(opts.ssrfPolicy)) { + if (hasProxyEnvConfigured() && !isPrivateNetworkAllowedByPolicy(opts.ssrfPolicy)) { throw new InvalidBrowserNavigationUrlError( "Navigation blocked: strict browser SSRF policy cannot be enforced while env proxy variables are set", ); diff --git a/src/infra/net/fetch-guard.ssrf.test.ts b/src/infra/net/fetch-guard.ssrf.test.ts index 780f2d94d99..e686f46e075 100644 --- a/src/infra/net/fetch-guard.ssrf.test.ts +++ b/src/infra/net/fetch-guard.ssrf.test.ts @@ -1,6 +1,6 @@ import { EnvHttpProxyAgent } from "undici"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { fetchWithSsrFGuard } from "./fetch-guard.js"; +import { fetchWithSsrFGuard, GUARDED_FETCH_MODE } from "./fetch-guard.js"; function redirectResponse(location: string): Response { return new Response(null, { @@ -180,7 +180,7 @@ describe("fetchWithSsrFGuard hardening", () => { url: "https://public.example/resource", fetchImpl, lookupFn, - proxy: "env", + mode: GUARDED_FETCH_MODE.STRICT, }); expect(fetchImpl).toHaveBeenCalledTimes(1); @@ -202,8 +202,7 @@ describe("fetchWithSsrFGuard hardening", () => { url: "https://public.example/resource", fetchImpl, lookupFn, - proxy: "env", - dangerouslyAllowEnvProxyWithoutPinnedDns: true, + mode: GUARDED_FETCH_MODE.TRUSTED_ENV_PROXY, }); expect(fetchImpl).toHaveBeenCalledTimes(1); diff --git a/src/infra/net/fetch-guard.ts b/src/infra/net/fetch-guard.ts index 06847f841c1..ded0c5fae21 100644 --- a/src/infra/net/fetch-guard.ts +++ b/src/infra/net/fetch-guard.ts @@ -1,6 +1,7 @@ import { EnvHttpProxyAgent, type Dispatcher } from "undici"; import { logWarn } from "../../logger.js"; import { bindAbortRelay } from "../../utils/fetch-timeout.js"; +import { hasProxyEnvConfigured } from "./proxy-env.js"; import { closeDispatcher, createPinnedDispatcher, @@ -12,6 +13,13 @@ import { type FetchLike = (input: RequestInfo | URL, init?: RequestInit) => Promise; +export const GUARDED_FETCH_MODE = { + STRICT: "strict", + TRUSTED_ENV_PROXY: "trusted_env_proxy", +} as const; + +export type GuardedFetchMode = (typeof GUARDED_FETCH_MODE)[keyof typeof GUARDED_FETCH_MODE]; + export type GuardedFetchOptions = { url: string; fetchImpl?: FetchLike; @@ -21,11 +29,12 @@ export type GuardedFetchOptions = { signal?: AbortSignal; policy?: SsrFPolicy; lookupFn?: LookupFn; + mode?: GuardedFetchMode; pinDns?: boolean; + /** @deprecated use `mode: "trusted_env_proxy"` for trusted/operator-controlled URLs. */ proxy?: "env"; /** - * Env proxies can break destination binding between SSRF pre-check and connect-time target. - * Keep this off for untrusted URLs; enable only for trusted/operator-controlled endpoints. + * @deprecated use `mode: "trusted_env_proxy"` instead. */ dangerouslyAllowEnvProxyWithoutPinnedDns?: boolean; auditContext?: string; @@ -37,15 +46,12 @@ export type GuardedFetchResult = { release: () => Promise; }; +type GuardedFetchPresetOptions = Omit< + GuardedFetchOptions, + "mode" | "proxy" | "dangerouslyAllowEnvProxyWithoutPinnedDns" +>; + const DEFAULT_MAX_REDIRECTS = 3; -const ENV_PROXY_KEYS = [ - "HTTP_PROXY", - "HTTPS_PROXY", - "ALL_PROXY", - "http_proxy", - "https_proxy", - "all_proxy", -] as const; const CROSS_ORIGIN_REDIRECT_SENSITIVE_HEADERS = [ "authorization", "proxy-authorization", @@ -53,14 +59,24 @@ const CROSS_ORIGIN_REDIRECT_SENSITIVE_HEADERS = [ "cookie2", ]; -function hasEnvProxyConfigured(): boolean { - for (const key of ENV_PROXY_KEYS) { - const value = process.env[key]; - if (typeof value === "string" && value.trim()) { - return true; - } +export function withStrictGuardedFetchMode(params: GuardedFetchPresetOptions): GuardedFetchOptions { + return { ...params, mode: GUARDED_FETCH_MODE.STRICT }; +} + +export function withTrustedEnvProxyGuardedFetchMode( + params: GuardedFetchPresetOptions, +): GuardedFetchOptions { + return { ...params, mode: GUARDED_FETCH_MODE.TRUSTED_ENV_PROXY }; +} + +function resolveGuardedFetchMode(params: GuardedFetchOptions): GuardedFetchMode { + if (params.mode) { + return params.mode; } - return false; + if (params.proxy === "env" && params.dangerouslyAllowEnvProxyWithoutPinnedDns === true) { + return GUARDED_FETCH_MODE.TRUSTED_ENV_PROXY; + } + return GUARDED_FETCH_MODE.STRICT; } function isRedirectStatus(status: number): boolean { @@ -122,6 +138,7 @@ export async function fetchWithSsrFGuard(params: GuardedFetchOptions): Promise 0) { + return true; + } + } + return false; +} diff --git a/src/infra/net/ssrf.ts b/src/infra/net/ssrf.ts index 7798e5990a4..45fba10fd30 100644 --- a/src/infra/net/ssrf.ts +++ b/src/infra/net/ssrf.ts @@ -63,7 +63,7 @@ function normalizeHostnameAllowlist(values?: string[]): string[] { ); } -function resolveAllowPrivateNetwork(policy?: SsrFPolicy): boolean { +export function isPrivateNetworkAllowedByPolicy(policy?: SsrFPolicy): boolean { return policy?.dangerouslyAllowPrivateNetwork === true || policy?.allowPrivateNetwork === true; } @@ -282,7 +282,7 @@ export async function resolvePinnedHostnameWithPolicy( throw new Error("Invalid hostname"); } - const allowPrivateNetwork = resolveAllowPrivateNetwork(params.policy); + const allowPrivateNetwork = isPrivateNetworkAllowedByPolicy(params.policy); const allowedHostnames = normalizeHostnameSet(params.policy?.allowedHostnames); const hostnameAllowlist = normalizeHostnameAllowlist(params.policy?.hostnameAllowlist); const isExplicitAllowed = allowedHostnames.has(normalized); diff --git a/src/media/fetch.ts b/src/media/fetch.ts index 2991cda5bea..3f2372c0abf 100644 --- a/src/media/fetch.ts +++ b/src/media/fetch.ts @@ -1,5 +1,5 @@ import path from "node:path"; -import { fetchWithSsrFGuard } from "../infra/net/fetch-guard.js"; +import { fetchWithSsrFGuard, withStrictGuardedFetchMode } from "../infra/net/fetch-guard.js"; import type { LookupFn, SsrFPolicy } from "../infra/net/ssrf.js"; import { detectMime, extensionForMime } from "./mime.js"; import { readResponseWithLimit } from "./read-response-with-limit.js"; @@ -95,14 +95,16 @@ export async function fetchRemoteMedia(options: FetchMediaOptions): Promise Promise) | null = null; try { - const result = await fetchWithSsrFGuard({ - url, - fetchImpl, - init: requestInit, - maxRedirects, - policy: ssrfPolicy, - lookupFn, - }); + const result = await fetchWithSsrFGuard( + withStrictGuardedFetchMode({ + url, + fetchImpl, + init: requestInit, + maxRedirects, + policy: ssrfPolicy, + lookupFn, + }), + ); res = result.response; finalUrl = result.finalUrl; release = result.release; diff --git a/src/slack/send.ts b/src/slack/send.ts index 8495ef2ed24..fcfe230f7dc 100644 --- a/src/slack/send.ts +++ b/src/slack/send.ts @@ -8,7 +8,10 @@ import { isSilentReplyText } from "../auto-reply/tokens.js"; import { loadConfig } from "../config/config.js"; import { resolveMarkdownTableMode } from "../config/markdown-tables.js"; import { logVerbose } from "../globals.js"; -import { fetchWithSsrFGuard } from "../infra/net/fetch-guard.js"; +import { + fetchWithSsrFGuard, + withTrustedEnvProxyGuardedFetchMode, +} from "../infra/net/fetch-guard.js"; import { loadWebMedia } from "../web/media.js"; import type { SlackTokenSource } from "./accounts.js"; import { resolveSlackAccount } from "./accounts.js"; @@ -211,18 +214,18 @@ async function uploadSlackFile(params: { // Upload the file content to the presigned URL const uploadBody = new Uint8Array(buffer) as BodyInit; - const { response: uploadResp, release } = await fetchWithSsrFGuard({ - url: uploadUrlResp.upload_url, - init: { - method: "POST", - ...(contentType ? { headers: { "Content-Type": contentType } } : {}), - body: uploadBody, - }, - policy: SLACK_UPLOAD_SSRF_POLICY, - proxy: "env", - dangerouslyAllowEnvProxyWithoutPinnedDns: true, - auditContext: "slack-upload-file", - }); + const { response: uploadResp, release } = await fetchWithSsrFGuard( + withTrustedEnvProxyGuardedFetchMode({ + url: uploadUrlResp.upload_url, + init: { + method: "POST", + ...(contentType ? { headers: { "Content-Type": contentType } } : {}), + body: uploadBody, + }, + policy: SLACK_UPLOAD_SSRF_POLICY, + auditContext: "slack-upload-file", + }), + ); try { if (!uploadResp.ok) { throw new Error(`Failed to upload file: HTTP ${uploadResp.status}`); diff --git a/src/slack/send.upload.test.ts b/src/slack/send.upload.test.ts index a156d1b84bf..7ff05183b6c 100644 --- a/src/slack/send.upload.test.ts +++ b/src/slack/send.upload.test.ts @@ -16,6 +16,10 @@ const fetchWithSsrFGuard = vi.fn( vi.mock("../infra/net/fetch-guard.js", () => ({ fetchWithSsrFGuard: (...args: unknown[]) => fetchWithSsrFGuard(...(args as [params: { url: string; init?: RequestInit }])), + withTrustedEnvProxyGuardedFetchMode: (params: Record) => ({ + ...params, + mode: "trusted_env_proxy", + }), })); vi.mock("../web/media.js", () => ({ @@ -167,8 +171,7 @@ describe("sendMessageSlack file upload with user IDs", () => { expect(fetchWithSsrFGuard).toHaveBeenCalledWith( expect.objectContaining({ url: "https://uploads.slack.test/upload", - proxy: "env", - dangerouslyAllowEnvProxyWithoutPinnedDns: true, + mode: "trusted_env_proxy", auditContext: "slack-upload-file", }), ); diff --git a/src/telegram/fetch.ts b/src/telegram/fetch.ts index 91a5ef9931d..f1e50021e92 100644 --- a/src/telegram/fetch.ts +++ b/src/telegram/fetch.ts @@ -3,6 +3,7 @@ import * as net from "node:net"; import { EnvHttpProxyAgent, getGlobalDispatcher, setGlobalDispatcher } from "undici"; import type { TelegramNetworkConfig } from "../config/types.telegram.js"; import { resolveFetch } from "../infra/fetch.js"; +import { hasProxyEnvConfigured } from "../infra/net/proxy-env.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; import { resolveTelegramAutoSelectFamilyDecision, @@ -13,25 +14,6 @@ let appliedAutoSelectFamily: boolean | null = null; let appliedDnsResultOrder: string | null = null; let appliedGlobalDispatcherAutoSelectFamily: boolean | null = null; const log = createSubsystemLogger("telegram/network"); -const PROXY_ENV_KEYS = [ - "HTTPS_PROXY", - "HTTP_PROXY", - "ALL_PROXY", - "https_proxy", - "http_proxy", - "all_proxy", -] as const; - -function hasProxyEnvConfigured(): boolean { - for (const key of PROXY_ENV_KEYS) { - const value = process.env[key]; - if (typeof value === "string" && value.trim().length > 0) { - return true; - } - } - return false; -} - function isProxyLikeDispatcher(dispatcher: unknown): boolean { const ctorName = (dispatcher as { constructor?: { name?: string } })?.constructor?.name; return typeof ctorName === "string" && ctorName.includes("ProxyAgent");