Webhook hardening follow-ups (post-#4564 review)

[JAORMX]

Tracking issue for the webhook hardening follow-ups identified in the round-2 review of #4564 that are not addressed by #5190 / #5191 / #5192.

Shipped

• Block private and loopback dials in webhook HTTP client #5190 — Block private/loopback/link-local dials in webhook HTTP client (item Implement secret injection #5)
• Redact webhook response body from returned errors #5191 — Redact webhook response body from returned errors (item Start simple hardcoded registry #8)
• Cap webhook middleware request body at 1 MB #5192 — Cap webhook middleware request body at 1 MB (webhook-layer fix for item Use an actual logger #9)

Remaining

Item #6: InsecureSkipVerify conflates cert-skip with plaintext-HTTP

pkg/webhook/types.go ValidateDefinition allows non-HTTPS URLs whenever InsecureSkipVerify=true. pkg/webhook/client.go buildTransport propagates the same flag into ValidatingTransport.InsecureAllowHTTP=true, which (per pkg/networking/http_client.go:55-73) skips ALL request validation when set: cert check, scheme check, and now also short-circuits after the dial-time SSRF guard fires. Flip one knob, lose multiple guarantees.

RFC THV-0017 says HTTPS-only. Suggested fix:

1. Split InsecureSkipVerify into two fields, or remove the plaintext escape hatch entirely.
2. Add a CRD-level CEL rule on MCPWebhookConfig.spec.*.url enforcing self.startsWith('https://') at admission time.

MCP parsing layer needs an inbound body cap

The webhook-layer cap shipped in #5192 is correct for the webhook package's own re-read buffer, but mcp.ParsingMiddleware (pkg/mcp/parser.go:85) reads the inbound HTTP body via unbounded io.ReadAll BEFORE the webhook middleware in the proxy chain. So an attacker can still buffer a 10 GB body into memory upstream of the webhook cap.

pkg/mcp/tool_filter.go:229 has the same exposure. Both should be wrapped with http.MaxBytesReader or an equivalent cap. The cap may need to be configurable per-server (some MCP tools/call payloads can legitimately be larger than the default).

pkg/webhook/internal/dialer subpackage refactor

The three exported test helpers in pkg/webhook/dialer_testing.go (SetDialerControlForTesting, SetDialerControlForTestMain, AllowAnyDialerControl) inflate pkg/webhook's public API surface in service of cross-package test injection. Moving them — and the dialerControl atomic.Pointer itself — into a new pkg/webhook/internal/dialer subpackage would keep them off the project's public API while still letting pkg/webhook, pkg/webhook/validating, and pkg/webhook/mutating import them via Go's internal rule.

An even more principled fix is to refactor webhook.NewClient to accept an optional *http.Client (or a WithHTTPClient(...) option), so production passes the SSRF-protected client and tests pass an unguarded one. The package-level dialerControl then disappears entirely.

Expanded SSRF integration coverage in pkg/networking

pkg/networking/utilities_test.go already covers IPv6 cases at the unit level. The webhook PR (#5190) adds IPv6 cases to TestClientSSRFGuardBlocksPrivateAddress, but it would be worth adding parallel integration coverage for any other consumer of ProtectedDialerControl (the OAuth/discovery clients, etc.).

Webhook response body in client error log fields

#5191 moves the body preview to slog.Debug with a body_preview field. If the project's log policy forbids untrusted bytes flowing into structured log fields at any level (not just info+), the preview should be redacted further or omitted. Re-evaluate when log-injection policy is formalized.

🤖 Generated with Claude Code