refactor(net): unify proxy env checks and guarded fetch modes

This commit is contained in:
Peter Steinberger
2026-03-02 16:24:20 +00:00
parent a229ae6c3e
commit c973b053a5
12 changed files with 129 additions and 117 deletions

View File

@@ -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<string, unknown>) => ({
...params,
mode: GUARDED_FETCH_MODE.STRICT,
}),
withTrustedEnvProxyGuardedFetchMode: (params: Record<string, unknown>) => ({
...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);
});
});

View File

@@ -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<GuardedFetchResult> {
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<T>(

View File

@@ -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]";

View File

@@ -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",
);

View File

@@ -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);

View File

@@ -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<Response>;
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<void>;
};
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<G
typeof params.maxRedirects === "number" && Number.isFinite(params.maxRedirects)
? Math.max(0, Math.floor(params.maxRedirects))
: DEFAULT_MAX_REDIRECTS;
const mode = resolveGuardedFetchMode(params);
const { signal, cleanup } = buildAbortSignal({
timeoutMs: params.timeoutMs,
@@ -162,8 +179,9 @@ export async function fetchWithSsrFGuard(params: GuardedFetchOptions): Promise<G
lookupFn: params.lookupFn,
policy: params.policy,
});
const hasEnvProxy = params.proxy === "env" && hasEnvProxyConfigured();
if (hasEnvProxy && params.dangerouslyAllowEnvProxyWithoutPinnedDns === true) {
const canUseTrustedEnvProxy =
mode === GUARDED_FETCH_MODE.TRUSTED_ENV_PROXY && hasProxyEnvConfigured();
if (canUseTrustedEnvProxy) {
dispatcher = new EnvHttpProxyAgent();
} else if (params.pinDns !== false) {
dispatcher = createPinnedDispatcher(pinned);

View File

@@ -0,0 +1,18 @@
export const PROXY_ENV_KEYS = [
"HTTP_PROXY",
"HTTPS_PROXY",
"ALL_PROXY",
"http_proxy",
"https_proxy",
"all_proxy",
] as const;
export function hasProxyEnvConfigured(env: NodeJS.ProcessEnv = process.env): boolean {
for (const key of PROXY_ENV_KEYS) {
const value = env[key];
if (typeof value === "string" && value.trim().length > 0) {
return true;
}
}
return false;
}

View File

@@ -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);

View File

@@ -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<Fetc
let finalUrl = url;
let release: (() => Promise<void>) | 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;

View File

@@ -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}`);

View File

@@ -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<string, unknown>) => ({
...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",
}),
);

View File

@@ -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");