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
@aheritier aheritier added area/security Authentication, authorization, secrets, vulnerabilities area/tools For features/issues/fixes related to the usage of built-in and MCP tools kind/fix PR fixes a bug (maps to fix: commit prefix) labels May 22, 2026
@rumpl rumpl deleted the board/30a60fd708e6c7e6 branch May 22, 2026 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/security Authentication, authorization, secrets, vulnerabilities area/tools For features/issues/fixes related to the usage of built-in and MCP tools kind/fix PR fixes a bug (maps to fix: commit prefix)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants