feat(tools): add proxy support for web_fetch tool#5831
feat(tools): add proxy support for web_fetch tool#5831bobcyw wants to merge 4 commits intoopenclaw:mainfrom
Conversation
src/agents/tools/web-fetch.ts
Outdated
| // Use proxy dispatcher if available, otherwise use pinned dispatcher for SSRF protection | ||
| let dispatcher: Dispatcher; | ||
| if (proxyDispatcher) { | ||
| dispatcher = proxyDispatcher; | ||
| } else { | ||
| const pinned = await resolvePinnedHostname(parsedUrl.hostname); | ||
| dispatcher = createPinnedDispatcher(pinned); | ||
| } |
There was a problem hiding this comment.
[P0] Proxy support bypasses SSRF pinning
When params.proxy is set, this skips resolvePinnedHostname()/createPinnedDispatcher() entirely and uses ProxyAgent directly, which effectively disables the repo’s SSRF protection for web_fetch in proxied mode. This becomes a real security issue if proxy support is used in environments where the proxy can reach internal networks or can be influenced/controlled by an attacker.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/tools/web-fetch.ts
Line: 206:213
Comment:
[P0] Proxy support bypasses SSRF pinning
When `params.proxy` is set, this skips `resolvePinnedHostname()`/`createPinnedDispatcher()` entirely and uses `ProxyAgent` directly, which effectively disables the repo’s SSRF protection for web_fetch in proxied mode. This becomes a real security issue if proxy support is used in environments where the proxy can reach internal networks or can be influenced/controlled by an attacker.
How can I resolve this? If you propose a fix, please make it concise.
src/agents/tools/web-fetch.ts
Outdated
| }): Promise<{ response: Response; finalUrl: string; dispatcher: Dispatcher }> { | ||
| const signal = withTimeout(undefined, params.timeoutSeconds * 1000); | ||
| const visited = new Set<string>(); | ||
| let currentUrl = params.url; | ||
| let redirectCount = 0; | ||
|
|
||
| // Create proxy dispatcher if configured | ||
| const proxyDispatcher = params.proxy ? new ProxyAgent(params.proxy) : null; |
There was a problem hiding this comment.
[P1] ProxyAgent lifecycle likely closed too early
fetchWithRedirects() returns { dispatcher } even when that dispatcher is proxyDispatcher, and runWebFetch() always does await closeDispatcher(dispatcher) in finally. That means the proxy dispatcher is likely closed after every request despite the comment that it’s “reused”, and it’s also never explicitly closed on the success path inside fetchWithRedirects(). This can lead to confusing behavior or resource churn depending on how undici’s ProxyAgent manages connections.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/tools/web-fetch.ts
Line: 184:191
Comment:
[P1] ProxyAgent lifecycle likely closed too early
`fetchWithRedirects()` returns `{ dispatcher }` even when that dispatcher is `proxyDispatcher`, and `runWebFetch()` always does `await closeDispatcher(dispatcher)` in `finally`. That means the proxy dispatcher is likely closed after every request despite the comment that it’s “reused”, and it’s also never explicitly closed on the success path inside `fetchWithRedirects()`. This can lead to confusing behavior or resource churn depending on how undici’s `ProxyAgent` manages connections.
How can I resolve this? If you propose a fix, please make it concise.| it("uses proxy dispatcher when proxy is configured", async () => { | ||
| let capturedDispatcher: unknown = null; | ||
| const mockFetch = vi.fn((input: RequestInfo, init?: RequestInit) => { | ||
| capturedDispatcher = (init as { dispatcher?: unknown })?.dispatcher; | ||
| return Promise.resolve( | ||
| htmlResponse("<html><body><p>Hello via proxy</p></body></html>", requestUrl(input)), |
There was a problem hiding this comment.
[P2] Proxy tests don't actually assert proxy behavior
Both proxy-related tests only assert that a dispatcher was passed to fetch, but they never verify it's a ProxyAgent when proxy is configured, nor that SSRF pinning was skipped/called appropriately. As written, these tests would still pass if the dispatcher wiring were wrong (e.g., always using a pinned dispatcher).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/tools/web-tools.fetch.test.ts
Line: 295:300
Comment:
[P2] Proxy tests don't actually assert proxy behavior
Both proxy-related tests only assert that a `dispatcher` was passed to fetch, but they never verify it's a `ProxyAgent` when proxy is configured, nor that SSRF pinning was skipped/called appropriately. As written, these tests would still pass if the dispatcher wiring were wrong (e.g., always using a pinned dispatcher).
How can I resolve this? If you propose a fix, please make it concise.1ea6a71 to
5390624
Compare
Add HTTP proxy configuration option for the web_fetch tool, allowing requests to be routed through a proxy server. Configuration example: tools.web.fetch.proxy: "http://127.0.0.1:7890" Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- P0: Add comments clarifying SSRF bypass is intentional when proxy is configured - P1: Fix dispatcher lifecycle by tracking isProxyDispatcher flag - P2: Strengthen tests to verify ProxyAgent is used and SSRF is bypassed Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
45a7ce2 to
e615e56
Compare
|
CLAWDINATOR FIELD REPORT // PR Closure I am CLAWDINATOR — cybernetic crustacean, maintainer triage bot for OpenClaw. I was sent from the future to keep this repo shipping clean code. Context: OpenClaw serves ~1M users. Provider integrations and platform additions are not being accepted without prior discussion. The queue is too large to review unsolicited features. If this integration is critical for users, open a GitHub issue first to discuss with maintainers. TERMINATED. 🤖 This is an automated message from CLAWDINATOR, the OpenClaw maintainer bot. |
|
Why closed? |
Add HTTP proxy configuration option for the web_fetch tool, allowing requests to be routed through a proxy server.
Configuration example:
tools.web.fetch.proxy: "http://127.0.0.1:7890"
Greptile Overview
Greptile Summary
This PR adds a new
tools.web.fetch.proxyconfig option and wires it throughcreateWebFetchTool()into the fetch path, using undici’sProxyAgentsoweb_fetchrequests can be routed via an HTTP proxy. It also updates the config UI hints/types/runtime zod schema and adds tests intended to cover the new proxy behavior.The main concern is that the implementation switches from the existing pinned-dispatcher SSRF protection to a raw proxy dispatcher when proxy is enabled, which changes the security model of
web_fetchin a way that can allow fetching internal resources if the proxy can reach them. Additionally, the proxy agent lifecycle/closing behavior looks inconsistent with the “reused” intent, and the new tests don’t currently validate that proxy mode is actually being exercised.Confidence Score: 2/5
(4/5) You can add custom instructions or style guidelines for the agent here!
Context used:
dashboard- CLAUDE.md (source)dashboard- AGENTS.md (source)