Skip to content

Commit c973b05

Browse files
steipetesteipete
committed
refactor(net): unify proxy env checks and guarded fetch modes
1 parent a229ae6 commit c973b05

12 files changed

Lines changed: 129 additions & 117 deletions

File tree

‎src/agents/tools/web-guarded-fetch.test.ts‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,25 @@
11
import { afterEach, describe, expect, it, vi } from "vitest";
2-
import { fetchWithSsrFGuard } from "../../infra/net/fetch-guard.js";
2+
import { fetchWithSsrFGuard, GUARDED_FETCH_MODE } from "../../infra/net/fetch-guard.js";
33
import { withStrictWebToolsEndpoint, withTrustedWebToolsEndpoint } from "./web-guarded-fetch.js";
44

5-
vi.mock("../../infra/net/fetch-guard.js", () => ({
6-
fetchWithSsrFGuard: vi.fn(),
7-
}));
5+
vi.mock("../../infra/net/fetch-guard.js", () => {
6+
const GUARDED_FETCH_MODE = {
7+
STRICT: "strict",
8+
TRUSTED_ENV_PROXY: "trusted_env_proxy",
9+
} as const;
10+
return {
11+
GUARDED_FETCH_MODE,
12+
fetchWithSsrFGuard: vi.fn(),
13+
withStrictGuardedFetchMode: (params: Record<string, unknown>) => ({
14+
...params,
15+
mode: GUARDED_FETCH_MODE.STRICT,
16+
}),
17+
withTrustedEnvProxyGuardedFetchMode: (params: Record<string, unknown>) => ({
18+
...params,
19+
mode: GUARDED_FETCH_MODE.TRUSTED_ENV_PROXY,
20+
}),
21+
};
22+
});
823

924
describe("web-guarded-fetch", () => {
1025
afterEach(() => {
@@ -27,8 +42,7 @@ describe("web-guarded-fetch", () => {
2742
dangerouslyAllowPrivateNetwork: true,
2843
allowRfc2544BenchmarkRange: true,
2944
}),
30-
proxy: "env",
31-
dangerouslyAllowEnvProxyWithoutPinnedDns: true,
45+
mode: GUARDED_FETCH_MODE.TRUSTED_ENV_PROXY,
3246
}),
3347
);
3448
});
@@ -49,7 +63,6 @@ describe("web-guarded-fetch", () => {
4963
);
5064
const call = vi.mocked(fetchWithSsrFGuard).mock.calls[0]?.[0];
5165
expect(call?.policy).toBeUndefined();
52-
expect(call?.proxy).toBeUndefined();
53-
expect(call?.dangerouslyAllowEnvProxyWithoutPinnedDns).toBeUndefined();
66+
expect(call?.mode).toBe(GUARDED_FETCH_MODE.STRICT);
5467
});
5568
});

‎src/agents/tools/web-guarded-fetch.ts‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import {
22
fetchWithSsrFGuard,
33
type GuardedFetchOptions,
44
type GuardedFetchResult,
5+
withStrictGuardedFetchMode,
6+
withTrustedEnvProxyGuardedFetchMode,
57
} from "../../infra/net/fetch-guard.js";
68
import type { SsrFPolicy } from "../../infra/net/ssrf.js";
79

@@ -12,7 +14,7 @@ const WEB_TOOLS_TRUSTED_NETWORK_SSRF_POLICY: SsrFPolicy = {
1214

1315
type WebToolGuardedFetchOptions = Omit<
1416
GuardedFetchOptions,
15-
"proxy" | "dangerouslyAllowEnvProxyWithoutPinnedDns"
17+
"mode" | "proxy" | "dangerouslyAllowEnvProxyWithoutPinnedDns"
1618
> & {
1719
timeoutSeconds?: number;
1820
useEnvProxy?: boolean;
@@ -36,16 +38,15 @@ export async function fetchWithWebToolsNetworkGuard(
3638
params: WebToolGuardedFetchOptions,
3739
): Promise<GuardedFetchResult> {
3840
const { timeoutSeconds, useEnvProxy, ...rest } = params;
39-
return fetchWithSsrFGuard({
41+
const resolved = {
4042
...rest,
4143
timeoutMs: resolveTimeoutMs({ timeoutMs: rest.timeoutMs, timeoutSeconds }),
42-
...(useEnvProxy
43-
? {
44-
proxy: "env",
45-
dangerouslyAllowEnvProxyWithoutPinnedDns: true,
46-
}
47-
: {}),
48-
});
44+
};
45+
return fetchWithSsrFGuard(
46+
useEnvProxy
47+
? withTrustedEnvProxyGuardedFetchMode(resolved)
48+
: withStrictGuardedFetchMode(resolved),
49+
);
4950
}
5051

5152
async function withWebToolsNetworkGuard<T>(

‎src/browser/cdp-proxy-bypass.ts‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import http from "node:http";
1111
import https from "node:https";
1212
import { isLoopbackHost } from "../gateway/net.js";
13+
import { hasProxyEnvConfigured } from "../infra/net/proxy-env.js";
1314

1415
/** HTTP agent that never uses a proxy — for localhost CDP connections. */
1516
const directHttpAgent = new http.Agent();
@@ -39,15 +40,7 @@ export function getDirectAgentForCdp(url: string): http.Agent | https.Agent | un
3940
* interfere with loopback connections.
4041
*/
4142
export function hasProxyEnv(): boolean {
42-
const env = process.env;
43-
return Boolean(
44-
env.HTTP_PROXY ||
45-
env.http_proxy ||
46-
env.HTTPS_PROXY ||
47-
env.https_proxy ||
48-
env.ALL_PROXY ||
49-
env.all_proxy,
50-
);
43+
return hasProxyEnvConfigured();
5144
}
5245

5346
const LOOPBACK_ENTRIES = "localhost,127.0.0.1,[::1]";

‎src/browser/navigation-guard.ts‎

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,13 @@
1+
import { hasProxyEnvConfigured } from "../infra/net/proxy-env.js";
12
import {
3+
isPrivateNetworkAllowedByPolicy,
24
resolvePinnedHostnameWithPolicy,
35
type LookupFn,
46
type SsrFPolicy,
57
} from "../infra/net/ssrf.js";
68

79
const NETWORK_NAVIGATION_PROTOCOLS = new Set(["http:", "https:"]);
810
const SAFE_NON_NETWORK_URLS = new Set(["about:blank"]);
9-
const ENV_PROXY_KEYS = [
10-
"HTTP_PROXY",
11-
"HTTPS_PROXY",
12-
"ALL_PROXY",
13-
"http_proxy",
14-
"https_proxy",
15-
"all_proxy",
16-
] as const;
17-
18-
function hasEnvProxyConfigured(): boolean {
19-
for (const key of ENV_PROXY_KEYS) {
20-
const value = process.env[key];
21-
if (typeof value === "string" && value.trim().length > 0) {
22-
return true;
23-
}
24-
}
25-
return false;
26-
}
27-
28-
function allowsPrivateNetworkNavigation(policy?: SsrFPolicy): boolean {
29-
return policy?.dangerouslyAllowPrivateNetwork === true || policy?.allowPrivateNetwork === true;
30-
}
3111

3212
function isAllowedNonNetworkNavigationUrl(parsed: URL): boolean {
3313
// Keep non-network navigation explicit; about:blank is the only allowed bootstrap URL.
@@ -82,7 +62,7 @@ export async function assertBrowserNavigationAllowed(
8262
// can bypass strict destination-binding intent from pre-navigation DNS checks.
8363
// In strict mode, fail closed unless private-network navigation is explicitly
8464
// enabled by policy.
85-
if (hasEnvProxyConfigured() && !allowsPrivateNetworkNavigation(opts.ssrfPolicy)) {
65+
if (hasProxyEnvConfigured() && !isPrivateNetworkAllowedByPolicy(opts.ssrfPolicy)) {
8666
throw new InvalidBrowserNavigationUrlError(
8767
"Navigation blocked: strict browser SSRF policy cannot be enforced while env proxy variables are set",
8868
);

‎src/infra/net/fetch-guard.ssrf.test.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { EnvHttpProxyAgent } from "undici";
22
import { afterEach, describe, expect, it, vi } from "vitest";
3-
import { fetchWithSsrFGuard } from "./fetch-guard.js";
3+
import { fetchWithSsrFGuard, GUARDED_FETCH_MODE } from "./fetch-guard.js";
44

55
function redirectResponse(location: string): Response {
66
return new Response(null, {
@@ -180,7 +180,7 @@ describe("fetchWithSsrFGuard hardening", () => {
180180
url: "https://public.example/resource",
181181
fetchImpl,
182182
lookupFn,
183-
proxy: "env",
183+
mode: GUARDED_FETCH_MODE.STRICT,
184184
});
185185

186186
expect(fetchImpl).toHaveBeenCalledTimes(1);
@@ -202,8 +202,7 @@ describe("fetchWithSsrFGuard hardening", () => {
202202
url: "https://public.example/resource",
203203
fetchImpl,
204204
lookupFn,
205-
proxy: "env",
206-
dangerouslyAllowEnvProxyWithoutPinnedDns: true,
205+
mode: GUARDED_FETCH_MODE.TRUSTED_ENV_PROXY,
207206
});
208207

209208
expect(fetchImpl).toHaveBeenCalledTimes(1);

‎src/infra/net/fetch-guard.ts‎

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { EnvHttpProxyAgent, type Dispatcher } from "undici";
22
import { logWarn } from "../../logger.js";
33
import { bindAbortRelay } from "../../utils/fetch-timeout.js";
4+
import { hasProxyEnvConfigured } from "./proxy-env.js";
45
import {
56
closeDispatcher,
67
createPinnedDispatcher,
@@ -12,6 +13,13 @@ import {
1213

1314
type FetchLike = (input: RequestInfo | URL, init?: RequestInit) => Promise<Response>;
1415

16+
export const GUARDED_FETCH_MODE = {
17+
STRICT: "strict",
18+
TRUSTED_ENV_PROXY: "trusted_env_proxy",
19+
} as const;
20+
21+
export type GuardedFetchMode = (typeof GUARDED_FETCH_MODE)[keyof typeof GUARDED_FETCH_MODE];
22+
1523
export type GuardedFetchOptions = {
1624
url: string;
1725
fetchImpl?: FetchLike;
@@ -21,11 +29,12 @@ export type GuardedFetchOptions = {
2129
signal?: AbortSignal;
2230
policy?: SsrFPolicy;
2331
lookupFn?: LookupFn;
32+
mode?: GuardedFetchMode;
2433
pinDns?: boolean;
34+
/** @deprecated use `mode: "trusted_env_proxy"` for trusted/operator-controlled URLs. */
2535
proxy?: "env";
2636
/**
27-
* Env proxies can break destination binding between SSRF pre-check and connect-time target.
28-
* Keep this off for untrusted URLs; enable only for trusted/operator-controlled endpoints.
37+
* @deprecated use `mode: "trusted_env_proxy"` instead.
2938
*/
3039
dangerouslyAllowEnvProxyWithoutPinnedDns?: boolean;
3140
auditContext?: string;
@@ -37,30 +46,37 @@ export type GuardedFetchResult = {
3746
release: () => Promise<void>;
3847
};
3948

49+
type GuardedFetchPresetOptions = Omit<
50+
GuardedFetchOptions,
51+
"mode" | "proxy" | "dangerouslyAllowEnvProxyWithoutPinnedDns"
52+
>;
53+
4054
const DEFAULT_MAX_REDIRECTS = 3;
41-
const ENV_PROXY_KEYS = [
42-
"HTTP_PROXY",
43-
"HTTPS_PROXY",
44-
"ALL_PROXY",
45-
"http_proxy",
46-
"https_proxy",
47-
"all_proxy",
48-
] as const;
4955
const CROSS_ORIGIN_REDIRECT_SENSITIVE_HEADERS = [
5056
"authorization",
5157
"proxy-authorization",
5258
"cookie",
5359
"cookie2",
5460
];
5561

56-
function hasEnvProxyConfigured(): boolean {
57-
for (const key of ENV_PROXY_KEYS) {
58-
const value = process.env[key];
59-
if (typeof value === "string" && value.trim()) {
60-
return true;
61-
}
62+
export function withStrictGuardedFetchMode(params: GuardedFetchPresetOptions): GuardedFetchOptions {
63+
return { ...params, mode: GUARDED_FETCH_MODE.STRICT };
64+
}
65+
66+
export function withTrustedEnvProxyGuardedFetchMode(
67+
params: GuardedFetchPresetOptions,
68+
): GuardedFetchOptions {
69+
return { ...params, mode: GUARDED_FETCH_MODE.TRUSTED_ENV_PROXY };
70+
}
71+
72+
function resolveGuardedFetchMode(params: GuardedFetchOptions): GuardedFetchMode {
73+
if (params.mode) {
74+
return params.mode;
75+
}
76+
if (params.proxy === "env" && params.dangerouslyAllowEnvProxyWithoutPinnedDns === true) {
77+
return GUARDED_FETCH_MODE.TRUSTED_ENV_PROXY;
6278
}
63-
return false;
79+
return GUARDED_FETCH_MODE.STRICT;
6480
}
6581

6682
function isRedirectStatus(status: number): boolean {
@@ -122,6 +138,7 @@ export async function fetchWithSsrFGuard(params: GuardedFetchOptions): Promise<G
122138
typeof params.maxRedirects === "number" && Number.isFinite(params.maxRedirects)
123139
? Math.max(0, Math.floor(params.maxRedirects))
124140
: DEFAULT_MAX_REDIRECTS;
141+
const mode = resolveGuardedFetchMode(params);
125142

126143
const { signal, cleanup } = buildAbortSignal({
127144
timeoutMs: params.timeoutMs,
@@ -162,8 +179,9 @@ export async function fetchWithSsrFGuard(params: GuardedFetchOptions): Promise<G
162179
lookupFn: params.lookupFn,
163180
policy: params.policy,
164181
});
165-
const hasEnvProxy = params.proxy === "env" && hasEnvProxyConfigured();
166-
if (hasEnvProxy && params.dangerouslyAllowEnvProxyWithoutPinnedDns === true) {
182+
const canUseTrustedEnvProxy =
183+
mode === GUARDED_FETCH_MODE.TRUSTED_ENV_PROXY && hasProxyEnvConfigured();
184+
if (canUseTrustedEnvProxy) {
167185
dispatcher = new EnvHttpProxyAgent();
168186
} else if (params.pinDns !== false) {
169187
dispatcher = createPinnedDispatcher(pinned);

‎src/infra/net/proxy-env.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
export const PROXY_ENV_KEYS = [
2+
"HTTP_PROXY",
3+
"HTTPS_PROXY",
4+
"ALL_PROXY",
5+
"http_proxy",
6+
"https_proxy",
7+
"all_proxy",
8+
] as const;
9+
10+
export function hasProxyEnvConfigured(env: NodeJS.ProcessEnv = process.env): boolean {
11+
for (const key of PROXY_ENV_KEYS) {
12+
const value = env[key];
13+
if (typeof value === "string" && value.trim().length > 0) {
14+
return true;
15+
}
16+
}
17+
return false;
18+
}

‎src/infra/net/ssrf.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ function normalizeHostnameAllowlist(values?: string[]): string[] {
6363
);
6464
}
6565

66-
function resolveAllowPrivateNetwork(policy?: SsrFPolicy): boolean {
66+
export function isPrivateNetworkAllowedByPolicy(policy?: SsrFPolicy): boolean {
6767
return policy?.dangerouslyAllowPrivateNetwork === true || policy?.allowPrivateNetwork === true;
6868
}
6969

@@ -282,7 +282,7 @@ export async function resolvePinnedHostnameWithPolicy(
282282
throw new Error("Invalid hostname");
283283
}
284284

285-
const allowPrivateNetwork = resolveAllowPrivateNetwork(params.policy);
285+
const allowPrivateNetwork = isPrivateNetworkAllowedByPolicy(params.policy);
286286
const allowedHostnames = normalizeHostnameSet(params.policy?.allowedHostnames);
287287
const hostnameAllowlist = normalizeHostnameAllowlist(params.policy?.hostnameAllowlist);
288288
const isExplicitAllowed = allowedHostnames.has(normalized);

‎src/media/fetch.ts‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import path from "node:path";
2-
import { fetchWithSsrFGuard } from "../infra/net/fetch-guard.js";
2+
import { fetchWithSsrFGuard, withStrictGuardedFetchMode } from "../infra/net/fetch-guard.js";
33
import type { LookupFn, SsrFPolicy } from "../infra/net/ssrf.js";
44
import { detectMime, extensionForMime } from "./mime.js";
55
import { readResponseWithLimit } from "./read-response-with-limit.js";
@@ -95,14 +95,16 @@ export async function fetchRemoteMedia(options: FetchMediaOptions): Promise<Fetc
9595
let finalUrl = url;
9696
let release: (() => Promise<void>) | null = null;
9797
try {
98-
const result = await fetchWithSsrFGuard({
99-
url,
100-
fetchImpl,
101-
init: requestInit,
102-
maxRedirects,
103-
policy: ssrfPolicy,
104-
lookupFn,
105-
});
98+
const result = await fetchWithSsrFGuard(
99+
withStrictGuardedFetchMode({
100+
url,
101+
fetchImpl,
102+
init: requestInit,
103+
maxRedirects,
104+
policy: ssrfPolicy,
105+
lookupFn,
106+
}),
107+
);
106108
res = result.response;
107109
finalUrl = result.finalUrl;
108110
release = result.release;

0 commit comments

Comments
 (0)