diff --git a/CHANGELOG.md b/CHANGELOG.md index 9acdb93bb6f..166abffe2eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/src/agents/tools/web-guarded-fetch.test.ts b/src/agents/tools/web-guarded-fetch.test.ts index b8be25be762..87b0ba9a5ae 100644 --- a/src/agents/tools/web-guarded-fetch.test.ts +++ b/src/agents/tools/web-guarded-fetch.test.ts @@ -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(); }); }); diff --git a/src/agents/tools/web-guarded-fetch.ts b/src/agents/tools/web-guarded-fetch.ts index f427eabcab3..c6c644fb4e2 100644 --- a/src/agents/tools/web-guarded-fetch.ts +++ b/src/agents/tools/web-guarded-fetch.ts @@ -10,10 +10,14 @@ const WEB_TOOLS_TRUSTED_NETWORK_SSRF_POLICY: SsrFPolicy = { allowRfc2544BenchmarkRange: true, }; -type WebToolGuardedFetchOptions = Omit & { +type WebToolGuardedFetchOptions = Omit< + GuardedFetchOptions, + "proxy" | "dangerouslyAllowEnvProxyWithoutPinnedDns" +> & { timeoutSeconds?: number; + useEnvProxy?: boolean; }; -type WebToolEndpointFetchOptions = Omit; +type WebToolEndpointFetchOptions = Omit; function resolveTimeoutMs(params: { timeoutMs?: number; @@ -31,11 +35,16 @@ function resolveTimeoutMs(params: { export async function fetchWithWebToolsNetworkGuard( params: WebToolGuardedFetchOptions, ): Promise { - 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( { ...params, policy: WEB_TOOLS_TRUSTED_NETWORK_SSRF_POLICY, + useEnvProxy: true, }, run, ); diff --git a/src/agents/tools/web-search.redirect.test.ts b/src/agents/tools/web-search.redirect.test.ts index 6578f917a18..cac014d7e9a 100644 --- a/src/agents/tools/web-search.redirect.test.ts +++ b/src/agents/tools/web-search.redirect.test.ts @@ -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); }); diff --git a/src/agents/tools/web-tools.fetch.test.ts b/src/agents/tools/web-tools.fetch.test.ts index 53836b92067..836f2d91c5c 100644 --- a/src/agents/tools/web-tools.fetch.test.ts +++ b/src/agents/tools/web-tools.fetch.test.ts @@ -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. diff --git a/src/infra/net/fetch-guard.ssrf.test.ts b/src/infra/net/fetch-guard.ssrf.test.ts index 223695c1a53..780f2d94d99 100644 --- a/src/infra/net/fetch-guard.ssrf.test.ts +++ b/src/infra/net/fetch-guard.ssrf.test.ts @@ -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[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(); + }); }); diff --git a/src/infra/net/fetch-guard.ts b/src/infra/net/fetch-guard.ts index 77260f474f5..06847f841c1 100644 --- a/src/infra/net/fetch-guard.ts +++ b/src/infra/net/fetch-guard.ts @@ -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 { expect.objectContaining({ url: "https://uploads.slack.test/upload", proxy: "env", + dangerouslyAllowEnvProxyWithoutPinnedDns: true, auditContext: "slack-upload-file", }), );