Skip to content

fix: allow configured HTTP proxy on private IPs in SSRF transport#2864

Merged
dgageot merged 1 commit into
mainfrom
board/30a60fd708e6c7e6
May 21, 2026
Merged

fix: allow configured HTTP proxy on private IPs in SSRF transport#2864
dgageot merged 1 commit into
mainfrom
board/30a60fd708e6c7e6

Conversation

@dgageot
Copy link
Copy Markdown
Member

@dgageot dgageot commented May 21, 2026

Inside the docker-agent sandbox, all egress traffic must route through a network-policy proxy on a private IP (e.g. http://172.17.0.0:3128). The pkg/httpclient.NewSSRFSafeTransport function installs a dial-time SSRF check that rejects every non-public address. When the HTTP client follows the HTTP_PROXY environment variable, it dials the proxy itself first—and our check saw the private IP proxy address, rejected it, and every outbound request failed. This broke the aqua-registry HTTP fetch that auto-installs gopls, so the binary never landed on PATH and the MCP toolset couldn't start.

NewSSRFSafeTransport now snapshots HTTP_PROXY, HTTPS_PROXY, and ALL_PROXY (and lowercase variants) at construction time, parses each proxy spec into one or more host:port entries, and bypasses the SSRF dial control for dials whose post-resolution address matches. The underlying SSRFDialControl function is unchanged, so DNS-rebinding defenses continue to apply to every other dial. Allowlisting an operator-configured proxy doesn't widen the SSRF threat model: refusing to dial it provides zero protection (the proxy itself enforces destination policy), and breaking every request inside the sandbox is not acceptable.

Three new tests pin the behavior: TestProxyHostPorts covers URL/bare-host parsing and default port assignment for http, https, and socks5 schemes; TestNewSSRFSafeTransport_AllowsConfiguredProxy is the regression test verifying private-IP proxies bypass SSRF rejection; TestNewSSRFSafeTransport_AllowlistFrozenAtConstruction documents that the allowlist is captured at construction time (which is correct since proxy env vars are set at process start).

@dgageot dgageot requested a review from a team as a code owner May 21, 2026 16:59
melmennaoui
melmennaoui previously approved these changes May 21, 2026
docker-agent

This comment was marked as outdated.

@dgageot dgageot merged commit 1386b54 into main May 21, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants