Wire backend dial-control policy into vMCP serve.go

[tgrunnagle]

Closes #6564

Summary

The vMCP backend dial paths both exposed a WithDialControl option to install an SSRF / DNS-rebinding guard, but neither was connected to a real policy in production — pkg/vmcp/cli/serve.go built the per-call backend client and the session factory without passing a control. The capability existed and was tested, but the shipped vMCP binary gained no private-address protection on backend dials. This PR builds a private-IP-blocking dial control at the serve.go composition root and wires it into both call sites, with a config toggle to opt out for in-cluster / development use.

Changes Made

pkg/networking

• Exported protectedDialerControl as ProtectedDialerControl so serve.go can install the private/loopback/link-local-blocking net.Dialer.Control hook. Internal callers updated to the exported name.

pkg/vmcp/config

• Added the BackendAllowPrivateIP config field (backendAllowPrivateIp), following the existing *AllowPrivateIP precedent. Defaults to false (guarded).

pkg/vmcp/cli/serve.go

• Added backendDialControl, the single policy source shared by both production dial paths so they cannot drift. It returns the guarding hook by default and nil (no guard) when BackendAllowPrivateIP is enabled.
• Passed the control to both dial paths:
  • vmcpclient.NewHTTPBackendClient (per-call dials) via client.WithDialControl — a single address-blind hook.
  • vmcpsession.NewSessionFactory (session-init dials) via session.WithDialControlResolver — the per-workload resolver introduced by Generalize vMCP session dial-control into a per-workload resolver #6567 (see note below). Because the policy is global today, the resolver returns the same hook for every workload; the resolver shape leaves room for a future per-backend policy without re-wiring.
• Log a warning at startup when the guard is disabled.

Generated artifacts

• Regenerated the VirtualMCPServer CRD manifests and docs/operator/crd-api.md for the new spec.config field.

pkg/authserver/runner (drive-by build fix)

• Added the missing github.com/alicebob/miniredis/v2 import to embeddedauthserver_test.go. Support no-auth Redis for the embedded auth server and vMCP sessions #6551 added a miniredis.RunT(t) call but omitted the import (the dependency is already in go.mod), leaving the test package non-compiling on main. See the scope note below.

Implementation Details

• The dial-control hook fires on the resolved peer IP before the TCP handshake, so it refuses dials into private ranges even when a hostname passed an earlier host-based check — this is what defends against DNS rebinding.
• A single shared policy source (backendDialControl) guarantees the per-call client and the session factory always apply the same rule.
• Default behavior is the safe, guarded one; opting out is explicit and surfaced via a startup warning.

Testing

• TestProtectedDialerControl (pkg/networking): verifies the exported hook blocks loopback (IPv4/IPv6), RFC 1918, and link-local peers while allowing a public address.
• TestBackendDialControl (pkg/vmcp/cli): covers both the guarded default (nil config, empty config, and BackendAllowPrivateIP: false all install a guard that refuses private and permits public dials) and the opt-out path (BackendAllowPrivateIP: true returns no control).
• task test and task lint-fix pass across the full tree.

Additional Notes

• Configuration: the new backendAllowPrivateIp field defaults to the safe guarded behavior; no action is required for existing deployments. Enable it only for in-cluster / development setups where backends legitimately resolve to private addresses.
• Follow-up to Add dial-control option to vMCP session factory #6547; closes the SSRF / DNS-rebinding gap left by vMCP: session-init backend dials cannot install a dial-control (SSRF) guard #6545 and Add WithDialControl option to vMCP backend client #5551 at the production composition root.

Special notes for reviewers

• Adapted to Generalize vMCP session dial-control into a per-workload resolver #6567: that PR generalized session.WithDialControl into the per-workload session.WithDialControlResolver after this work began. The session-factory call site was updated to the resolver shape; the per-call client.WithDialControl was not changed by Generalize vMCP session dial-control into a per-workload resolver #6567 and is used as-is.
• Cross-scope import fix: the pkg/authserver/runner change is unrelated to the dial-control feature — it only restores the build after a missing import landed on main via Support no-auth Redis for the embedded auth server and vMCP sessions #6551. It is included here as a one-line unblocker; happy to split it into a dedicated hotfix PR against main if preferred.

Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 52.94118% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.90%. Comparing base (c25e508) to head (d0f85c6).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/vmcp/cli/serve.go	46.66%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6566      +/-   ##
==========================================
- Coverage   78.94%   78.90%   -0.05%     
==========================================
  Files         781      781              
  Lines       77784    77893     +109     
==========================================
+ Hits        61410    61459      +49     
- Misses      16369    16429      +60     
  Partials        5        5              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tgrunnagle]

Multi-Agent Consensus Review

Agents consulted: security, architecture, test-coverage, general-quality

Consensus Summary

#	Finding	Consensus	Severity	Action
1	Default guard breaks backend connectivity in both deployment modes	9/10	HIGH	Fix
2	Proxy env vars silently defeat the SSRF guard	7/10	MEDIUM	Fix
3	No test exercises the actual wiring at either production call site	9/10	MEDIUM	Fix
4	Doc comment references a nonexistent pkg/vmcp/session.WithDialControl	7/10	MEDIUM	Fix
5	PR body omits required "Type of change" / "Test plan" sections	8/10	HIGH	Fix
6	PR body omits "API Compatibility" section despite CRD change	8/10	HIGH	Fix
7	PR left marked DRAFT while under review	7/10	MEDIUM	Fix
8	Unrelated miniredis import commit bundled into this PR	8/10	HIGH	Fix

Overall

This wires an existing, already-tested SSRF/DNS-rebinding dial-control hook into the two production backend-dial paths in pkg/vmcp/cli/serve.go (closes #6564), using a single shared policy function so the session factory and per-call client can't drift apart. The design is sound, but the default it ships is unconditionally hostile to ToolHive's actual backend topology: Kubernetes backend URLs resolve to *.svc.cluster.local (private ClusterIP ranges — cmd/thv-operator/controllers/mcpserver_controller.go:568-570) and local/CLI-mode backends are Docker-proxied to 127.0.0.1 (pkg/workloads/manager.go:325, pkg/container/docker/client.go:1723). No CRD default, chart, or operator code sets backendAllowPrivateIp: true, so this default appears to break backend connectivity for essentially every existing deployment — the opposite of the PR's own claim that "no action is required for existing deployments." That needs a design call (narrow the guard's scope vs. change the default) before merge, backed by a test that actually dials through Serve()/discoverBackends() rather than only the extracted helper.

Separately, HTTP_PROXY/HTTPS_PROXY env vars can silently defeat the guard entirely (a caveat already documented elsewhere in the codebase but unaddressed here), and one unrelated commit (an auth-server test import fix) is bundled into this diff.

Documentation

• pkg/networking/http_client.go:111 — the ProtectedDialerControl doc comment claims it can be "passed directly to the WithDialControl options in pkg/vmcp/client and pkg/vmcp/session," but pkg/vmcp/session has no WithDialControl — only WithDialControlResolver, which needs a wrapping resolver closure.
• pkg/vmcp/config/config.go's BackendAllowPrivateIP doc comment (and the generated CRD description) should note the proxy-bypass caveat rather than unconditionally calling the default "the safe production behavior."

PR Process (not mappable to a diff line)

• Type of change / Test plan sections missing: the PR body doesn't follow .github/pull_request_template.md — no "Type of change" box and no "Test plan" box are checked, both required.
• API Compatibility section missing: this PR changes the VirtualMCPServer CRD schema, which is exactly the case the template's "API Compatibility" section exists for; the section is absent entirely.
• Draft status: the PR body opens with "# DRAFT - not ready for review" — worth resolving (or converting to an actual GitHub draft) before this goes through full review.

---

Generated with Claude Code

[tgrunnagle]

[HIGH] Default guard breaks backend connectivity in both deployment modes (Consensus: 9/10)

backendDialControl defaults to the guarding hook whenever BackendAllowPrivateIP is false — but ToolHive's own backend addresses are private by construction: K8s backends resolve to *.svc.cluster.local (ClusterIP range, see cmd/thv-operator/controllers/mcpserver_controller.go:568-570), and local/CLI backends are Docker-proxied to 127.0.0.1 (pkg/workloads/manager.go:325). Nothing in cmd/thv-operator/ or deploy/ sets backendAllowPrivateIp: true, so this default appears to break backend connectivity for real deployments in both topologies — contradicting the PR body's claim that no action is required for existing deployments.

Consider scoping the guard to the actual SSRF risk (redirects / re-resolution after the initial connect) rather than the operator's own initially-configured target — see the existing NewHostScopedClientBuilder pattern that already special-cases loopback/operator-configured-private hosts (pkg/networking/http_client.go:352) — or default the opt-out to true for the operator/local topologies.

Raised by: security

[tgrunnagle]

[MEDIUM] Proxy env vars silently defeat the SSRF guard (Consensus: 7/10)

CloneDefaultTransportWithDialControl clones http.DefaultTransport, which carries Proxy: http.ProxyFromEnvironment. When HTTP_PROXY/HTTPS_PROXY is set, the dial target becomes the proxy's address, so this guard validates the proxy's IP, not the backend's — a documented caveat on WithDialControl/WithDialControlResolver that this composition root doesn't address.

Consider disabling proxying on the guarded transport (Proxy: nil) when the guard is active, or at minimum warn at startup (alongside the existing backendAllowPrivateIp warning) when a proxy env var is set.

Raised by: security

[tgrunnagle]

[MEDIUM] No test exercises the actual wiring at either production call site (Consensus: 9/10)

TestBackendDialControl only unit-tests the standalone backendDialControl(cfg) helper. Nothing calls Serve()/discoverBackends() and asserts the hook actually reaches WithDialControlResolver here or vmcpclient.WithDialControl in discoverBackends. The closure that adapts the helper into a per-workload resolver has no direct test either. This is exactly the class of gap that would let the connectivity-breaking default above ship unnoticed.

Consider extracting a backendDialControlResolver(cfg) helper (mirroring backendRequestTimeoutResolver's shape) that can be unit-tested directly, and adding a test that builds the client/session factory through the real Serve/discoverBackends path against a loopback backend.

Raised by: security, architecture, test-coverage

[tgrunnagle]

[MEDIUM] Doc comment references a nonexistent pkg/vmcp/session.WithDialControl (Consensus: 7/10)

pkg/vmcp/session has no WithDialControl — only WithDialControlResolver, which takes a resolver function, not the hook directly. ProtectedDialerControl must be wrapped in a resolver closure to be used there (exactly as this PR's own serve.go does).

Suggested change

	// The signature matches net.Dialer.Control exactly, so it can be passed
	// directly to the WithDialControl options in pkg/vmcp/client and
	// pkg/vmcp/session, or installed on a net.Dialer.
	// The signature matches net.Dialer.Control exactly, so it can be passed
	// directly to pkg/vmcp/client's WithDialControl option, or wrapped in a
	// workload-invariant resolver for pkg/vmcp/session's WithDialControlResolver
	// option (see pkg/vmcp/cli/serve.go's backendDialControl for an example).

Raised by: architecture

[tgrunnagle]

[HIGH] Unrelated miniredis import commit bundled into this PR (Consensus: 8/10)

This PR's own description covers only pkg/networking, pkg/vmcp/config, pkg/vmcp/cli/serve.go, and generated CRD artifacts — nothing about the auth server. This import fixes a pre-existing compile error for a no-auth-Redis test, unrelated to SSRF/dial-control wiring. Per this repo's PR-scope rule, each PR should contain only related changes.

Consider rebasing this commit out onto its own PR against main (or dropping it here if it's already landed elsewhere).

Raised by: general-quality, test-coverage

[tgrunnagle]

Intentional and kept here as a build fix. #6551 added a miniredis.RunT(t) call to this test but omitted the github.com/alicebob/miniredis/v2 import (the dependency is already in go.mod), so pkg/authserver/runner's test package does not compile on current main — go vet/go test/golangci-lint over ./... all fail with undefined: miniredis (go build ./... passes because it skips test files, which is why it slipped through). The one-line import add restores the build, which this PR needs in order to run task test/task lint-fix at all. Agree it's out of scope for the dial-control feature; happy to split it into a standalone hotfix PR against main if you'd prefer it not ride along here.

[tgrunnagle]

Abandoning this PR. Per the discussion on #6564, a guarded-by-default backend dial-control is a mismatch for vMCP's architecture — backends are private by construction (K8s ClusterIP, local 127.0.0.1) and their URLs are operator-managed rather than attacker/request-influenceable, so the guard would block legitimate traffic without closing a threat that applies to this path. #6564 is closed as not planned with the full rationale.

The embedder options (session.WithDialControlResolver, client.WithDialControl) remain available for library consumers and any future per-backend opt-in.

Note: this branch also carried an unrelated one-line build fix — the missing github.com/alicebob/miniredis/v2 import that #6551 omitted, which currently leaves pkg/authserver/runner's test package non-compiling on main. That fix is worth keeping; I'll propose it as a standalone hotfix PR against main rather than losing it here.