fix: preserve dns pinning for strict web SSRF fetches

This commit is contained in:
Peter Steinberger
2026-03-02 15:54:18 +00:00
parent a3d2021eea
commit 345abf0b20
9 changed files with 83 additions and 9 deletions

View File

@@ -46,6 +46,7 @@ Docs: https://docs.openclaw.ai
- Zalo/Pairing auth tests: add webhook regression coverage asserting DM pairing-store reads/writes remain account-scoped, preventing cross-account authorization bleed in multi-account setups. (#26121) Thanks @bmendonca3.
- Zalouser/Pairing auth tests: add account-scoped DM pairing-store regression coverage (`monitor.account-scope.test.ts`) to prevent cross-account allowlist bleed in multi-account setups. (#26672) Thanks @bmendonca3.
- Security/Web tools SSRF guard: keep DNS pinning for untrusted `web_fetch` and citation-redirect URL checks when proxy env vars are set, and require explicit dangerous opt-in before env-proxy routing can bypass pinned dispatch for trusted/operator-controlled endpoints. Thanks @tdjackey for reporting.
- Agents/Sessions list transcript paths: handle missing/non-string/relative `sessions.list.path` values and per-agent `{agentId}` templates when deriving `transcriptPath`, so cross-agent session listings resolve to concrete agent session files instead of workspace-relative paths. (#24775) Thanks @martinfrancois.
- macOS/PeekabooBridge: add compatibility socket symlinks for legacy `clawdbot`, `clawdis`, and `moltbot` Application Support socket paths so pre-rename clients can still connect. (#6033) Thanks @lumpinif and @vincentkoc.
- Webchat/Feishu session continuation: preserve routable `OriginatingChannel`/`OriginatingTo` metadata from session delivery context in `chat.send`, and prefer provider-normalized channel when deciding cross-channel route dispatch so Webchat replies continue on the selected Feishu session instead of falling back to main/internal session routing. (#31573)

View File

@@ -27,6 +27,8 @@ describe("web-guarded-fetch", () => {
dangerouslyAllowPrivateNetwork: true,
allowRfc2544BenchmarkRange: true,
}),
proxy: "env",
dangerouslyAllowEnvProxyWithoutPinnedDns: true,
}),
);
});
@@ -47,5 +49,7 @@ 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();
});
});

View File

@@ -10,10 +10,14 @@ const WEB_TOOLS_TRUSTED_NETWORK_SSRF_POLICY: SsrFPolicy = {
allowRfc2544BenchmarkRange: true,
};
type WebToolGuardedFetchOptions = Omit<GuardedFetchOptions, "proxy"> & {
type WebToolGuardedFetchOptions = Omit<
GuardedFetchOptions,
"proxy" | "dangerouslyAllowEnvProxyWithoutPinnedDns"
> & {
timeoutSeconds?: number;
useEnvProxy?: boolean;
};
type WebToolEndpointFetchOptions = Omit<WebToolGuardedFetchOptions, "policy">;
type WebToolEndpointFetchOptions = Omit<WebToolGuardedFetchOptions, "policy" | "useEnvProxy">;
function resolveTimeoutMs(params: {
timeoutMs?: number;
@@ -31,11 +35,16 @@ function resolveTimeoutMs(params: {
export async function fetchWithWebToolsNetworkGuard(
params: WebToolGuardedFetchOptions,
): Promise<GuardedFetchResult> {
const { timeoutSeconds, ...rest } = params;
const { timeoutSeconds, useEnvProxy, ...rest } = params;
return fetchWithSsrFGuard({
...rest,
timeoutMs: resolveTimeoutMs({ timeoutMs: rest.timeoutMs, timeoutSeconds }),
proxy: "env",
...(useEnvProxy
? {
proxy: "env",
dangerouslyAllowEnvProxyWithoutPinnedDns: true,
}
: {}),
});
}
@@ -59,6 +68,7 @@ export async function withTrustedWebToolsEndpoint<T>(
{
...params,
policy: WEB_TOOLS_TRUSTED_NETWORK_SSRF_POLICY,
useEnvProxy: true,
},
run,
);

View File

@@ -32,9 +32,9 @@ describe("web_search redirect resolution hardening", () => {
url: "https://example.com/start",
timeoutMs: 5000,
init: { method: "HEAD" },
proxy: "env",
}),
);
expect(fetchWithSsrFGuardMock.mock.calls[0]?.[0]?.proxy).toBeUndefined();
expect(fetchWithSsrFGuardMock.mock.calls[0]?.[0]?.policy).toBeUndefined();
expect(release).toHaveBeenCalledTimes(1);
});

View File

@@ -258,7 +258,7 @@ describe("web_fetch extraction fallbacks", () => {
expect(details?.warning).toContain("Response body truncated");
});
it("uses proxy-aware dispatcher when HTTP_PROXY is configured", async () => {
it("keeps DNS pinning for untrusted web_fetch URLs even when HTTP_PROXY is configured", async () => {
vi.stubEnv("HTTP_PROXY", "http://127.0.0.1:7890");
const mockFetch = installMockFetch((input: RequestInfo | URL) =>
Promise.resolve({
@@ -276,7 +276,8 @@ describe("web_fetch extraction fallbacks", () => {
const requestInit = mockFetch.mock.calls[0]?.[1] as
| (RequestInit & { dispatcher?: unknown })
| undefined;
expect(requestInit?.dispatcher).toBeInstanceOf(EnvHttpProxyAgent);
expect(requestInit?.dispatcher).toBeDefined();
expect(requestInit?.dispatcher).not.toBeInstanceOf(EnvHttpProxyAgent);
});
// NOTE: Test for wrapping url/finalUrl/warning fields requires DNS mocking.

View File

@@ -1,4 +1,5 @@
import { describe, expect, it, vi } from "vitest";
import { EnvHttpProxyAgent } from "undici";
import { afterEach, describe, expect, it, vi } from "vitest";
import { fetchWithSsrFGuard } from "./fetch-guard.js";
function redirectResponse(location: string): Response {
@@ -14,6 +15,9 @@ function okResponse(body = "ok"): Response {
describe("fetchWithSsrFGuard hardening", () => {
type LookupFn = NonNullable<Parameters<typeof fetchWithSsrFGuard>[0]["lookupFn"]>;
afterEach(() => {
vi.unstubAllEnvs();
});
it("blocks private and legacy loopback literals before fetch", async () => {
const blockedUrls = [
@@ -159,4 +163,50 @@ describe("fetchWithSsrFGuard hardening", () => {
expect(headers.get("authorization")).toBe("Bearer secret");
await result.release();
});
it("ignores env proxy by default to preserve DNS-pinned destination binding", async () => {
vi.stubEnv("HTTP_PROXY", "http://127.0.0.1:7890");
const lookupFn = vi.fn(async () => [
{ address: "93.184.216.34", family: 4 },
]) as unknown as LookupFn;
const fetchImpl = vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => {
const requestInit = init as RequestInit & { dispatcher?: unknown };
expect(requestInit.dispatcher).toBeDefined();
expect(requestInit.dispatcher).not.toBeInstanceOf(EnvHttpProxyAgent);
return okResponse();
});
const result = await fetchWithSsrFGuard({
url: "https://public.example/resource",
fetchImpl,
lookupFn,
proxy: "env",
});
expect(fetchImpl).toHaveBeenCalledTimes(1);
await result.release();
});
it("uses env proxy only when dangerous proxy bypass is explicitly enabled", async () => {
vi.stubEnv("HTTP_PROXY", "http://127.0.0.1:7890");
const lookupFn = vi.fn(async () => [
{ address: "93.184.216.34", family: 4 },
]) as unknown as LookupFn;
const fetchImpl = vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => {
const requestInit = init as RequestInit & { dispatcher?: unknown };
expect(requestInit.dispatcher).toBeInstanceOf(EnvHttpProxyAgent);
return okResponse();
});
const result = await fetchWithSsrFGuard({
url: "https://public.example/resource",
fetchImpl,
lookupFn,
proxy: "env",
dangerouslyAllowEnvProxyWithoutPinnedDns: true,
});
expect(fetchImpl).toHaveBeenCalledTimes(1);
await result.release();
});
});

View File

@@ -23,6 +23,11 @@ export type GuardedFetchOptions = {
lookupFn?: LookupFn;
pinDns?: boolean;
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.
*/
dangerouslyAllowEnvProxyWithoutPinnedDns?: boolean;
auditContext?: string;
};
@@ -157,7 +162,8 @@ export async function fetchWithSsrFGuard(params: GuardedFetchOptions): Promise<G
lookupFn: params.lookupFn,
policy: params.policy,
});
if (params.proxy === "env" && hasEnvProxyConfigured()) {
const hasEnvProxy = params.proxy === "env" && hasEnvProxyConfigured();
if (hasEnvProxy && params.dangerouslyAllowEnvProxyWithoutPinnedDns === true) {
dispatcher = new EnvHttpProxyAgent();
} else if (params.pinDns !== false) {
dispatcher = createPinnedDispatcher(pinned);

View File

@@ -220,6 +220,7 @@ async function uploadSlackFile(params: {
},
policy: SLACK_UPLOAD_SSRF_POLICY,
proxy: "env",
dangerouslyAllowEnvProxyWithoutPinnedDns: true,
auditContext: "slack-upload-file",
});
try {

View File

@@ -168,6 +168,7 @@ describe("sendMessageSlack file upload with user IDs", () => {
expect.objectContaining({
url: "https://uploads.slack.test/upload",
proxy: "env",
dangerouslyAllowEnvProxyWithoutPinnedDns: true,
auditContext: "slack-upload-file",
}),
);