Skip to content

Block private and loopback dials in webhook HTTP client#5190

Open
JAORMX wants to merge 1 commit intomainfrom
pr/webhook-ssrf-fix
Open

Block private and loopback dials in webhook HTTP client#5190
JAORMX wants to merge 1 commit intomainfrom
pr/webhook-ssrf-fix

Conversation

@JAORMX
Copy link
Copy Markdown
Collaborator

@JAORMX JAORMX commented May 5, 2026

Summary

Follow-up to the round-2 review of #4564 (item #5). A tenant with rights to create MCPWebhookConfig could previously point url at any HTTPS endpoint, including 169.254.169.254 (cloud metadata), 127.0.0.1, RFC1918 ranges, and IPv6 loopback / link-local / ULA addresses. The webhook HTTP transport built a bare http.Transport with no DialContext; the wrapping networking.ValidatingTransport only checks the URL scheme — it does not validate the resolved peer address — so cross-tenant access to cloud metadata or in-cluster services was unblocked.

This PR wires networking.ProtectedDialerControl into the inner transport's DialContext so private, loopback, and link-local destinations are rejected at dial time, regardless of whether ValidatingTransport's HTTPS check is bypassed via InsecureAllowHTTP. The hook is held in an atomic.Pointer so test overrides remain race-free if a future test introduces t.Parallel().

protectedDialerControl is exported as ProtectedDialerControl in pkg/networking so the webhook package can install it without duplicating the body. Cross-package test injection (SetDialerControlForTesting, SetDialerControlForTestMain, AllowAnyDialerControl) is added so the existing httptest-based suite in pkg/webhook and its subpackages keeps working; subpackage TestMain functions install the permissive override at suite startup.

Type of change

  • Bug fix

Test plan

  • Unit tests (task testpkg/webhook/... and pkg/networking/... pass with -race)
  • Linting (scoped golangci-lint reports 0 issues)
  • New TestClientSSRFGuardBlocksPrivateAddress covers IPv4 loopback, RFC1918, and 169.254.169.254, plus IPv6 loopback (::1), link-local (fe80::1), and ULA (fc00::1).
  • New TestBuildTransportInstallsDialerGuard asserts DialContext is wired on the inner transport.

Special notes for reviewers

  • The three exported test helpers in pkg/webhook/dialer_testing.go add public surface area in service of cross-package test injection. A reviewer raised the option of moving them to a pkg/webhook/internal/dialer subpackage; that's a viable follow-up but feels like scope creep here. Each helper has a Production code MUST NOT call this function deterrent in its godoc and the testing.TB argument signals test-scoped intent.
  • Item Figure out ergonomics for exposing directories #6 (InsecureSkipVerify conflates cert-skip with plaintext-HTTP) and item Implement secret injection #5's broader IPv6 coverage in pkg/networking itself are tracked separately.

🤖 Generated with Claude Code

@github-actions github-actions Bot added the size/S Small PR: 100-299 lines changed label May 5, 2026
A tenant with rights to create MCPWebhookConfig could previously point
url at any HTTPS endpoint, including 169.254.169.254, 127.0.0.1, RFC1918
ranges, and IPv6 loopback or link-local addresses. The webhook HTTP
transport built a bare http.Transport with no DialContext; the wrapping
networking.ValidatingTransport only checks the URL scheme, not the
resolved peer address, so cross-tenant access to cloud metadata or
in-cluster services was unblocked.

Wire networking.ProtectedDialerControl into the inner transport's
DialContext so private, loopback, and link-local destinations are
rejected at dial time, regardless of whether ValidatingTransport's
HTTPS check is bypassed via InsecureAllowHTTP. The hook is held in an
atomic.Pointer so test overrides remain race-free if a future test
introduces t.Parallel().

Export protectedDialerControl as ProtectedDialerControl in pkg/networking
so the webhook package can install it without duplicating the body.

Add cross-package test injection (SetDialerControlForTesting,
SetDialerControlForTestMain, AllowAnyDialerControl) so existing
httptest-based tests in pkg/webhook and its subpackages continue to
work; subpackage TestMain functions install the permissive override
at suite startup.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@JAORMX JAORMX force-pushed the pr/webhook-ssrf-fix branch from f1ad404 to a48d30f Compare May 5, 2026 10:16
@github-actions github-actions Bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels May 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Small PR: 100-299 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant