Generalize vMCP session dial-control into a per-workload resolver

[tgrunnagle]

Summary

PR #6547 added session.WithDialControl / backend.WithDialControl: a single, address-blind net.Dialer.Control hook applied to every backend's session-init dials (the MCP handshake plus capability listing). Because that hook receives only (network, address, RawConn), an embedder cannot vary the dial policy per backend — every backend gets the same hook. vMCP needs a per-backend dial policy at session initialize (some backends opt into private-IP dialing, most don't), which the single hook cannot express.

Since #6547 is merged but not yet in any release tag, this generalizes that option in place rather than adding a second one alongside — we ship one option, not two.

• Replace session.WithDialControl with session.WithDialControlResolver, a func(workloadID string) func(network, address string, c syscall.RawConn) error, mirroring the sibling WithRevisionLookup / WithRequestTimeoutResolver options in the same file. The resolver is called per backend and returns the hook to install for that workload (or nil).
• Replace backend.WithDialControl (HTTPConnectorOption) likewise. The connector resolves the per-workload hook in NewHTTPConnector's closure — where the target (and thus target.WorkloadID) is known — and threads the already-resolved hook through mcpClientParams, so createMCPClient and the backendBaseTransport / networking.CloneDefaultTransportWithDialControl primitive are unchanged.
• Preserve the no-hook invariant exactly: a nil resolver, or a resolver that returns nil for a given workload, leaves that backend's transport on http.DefaultTransport — byte-for-byte identical to the no-hook path (no clone).

Type of change

• Refactoring (no behavior change)

The default (no resolver) path is unchanged; this reshapes an opt-in embedder API that is not yet released.

Test plan

• Unit tests (task test)
• Linting (task lint-fix)

Ran the affected packages (pkg/networking, pkg/vmcp/session, pkg/vmcp/session/internal/backend, pkg/vmcp/client) with the Taskfile flags — all green; golangci-lint --fix reports 0 issues. Coverage kept and extended:

• Per-backend keying (new): at the connector level, one connector resolves a deny-all hook for one workload ID and nil for another — the denied workload's server is never reached (hook asserted to have fired) while the allowed workload's dial proceeds and reaches its server. At the factory level, a two-backend session whose resolver denies one workload ID proves the allowed backend connects and the denied one is excluded.
• Deny-all resolver yields zero backend requests during session initialize for both the streamable-http and sse transports, with the hook asserted to have actually fired.
• The resolver also guards the RestoreSession path.
• A nil dial control still returns http.DefaultTransport unchanged (TestBackendBaseTransport_NilHookReturnsDefault, unchanged).

Changes

File	Change
pkg/vmcp/session/factory.go	Replace WithDialControl with WithDialControlResolver; hold the resolver on the factory and thread it into the HTTP connector.
pkg/vmcp/session/factory_dialcontrol_test.go	Update to the resolver signature; re-key the per-backend test off workloadID instead of address.
pkg/vmcp/session/internal/backend/mcp_session.go	Replace WithDialControl with WithDialControlResolver; resolve the per-workload hook in the connector closure and thread the resolved hook through mcpClientParams. createMCPClient and backendBaseTransport unchanged.
pkg/vmcp/session/internal/backend/mcp_session_dialcontrol_test.go	Update the guard test to the resolver; add a per-workload resolution test.

Does this introduce a user-facing change?

No end-user behavior changes. This reshapes an opt-in, not-yet-released embedder API from a single address-blind dial-control hook into a per-workload resolver, so a deployment can enforce a per-backend dial policy on the connections opened at vMCP session initialization.

Special notes for reviewers

• The no-hook path is intentionally byte-for-byte identical to before: backendBaseTransport(nil) returns http.DefaultTransport directly rather than a clone, and the connector installs no hook when the resolver is nil or returns nil.
• networking.CloneDefaultTransportWithDialControl is the right primitive and is untouched — only the option shape above it changes.
• The resolver is called from the per-backend init goroutines (up to maxConcurrency), so it is documented as requiring concurrency-safety.
• No production caller wired the previous option (the Add dial-control option to vMCP session factory #6547 cli/serve.go follow-up was deferred), so this is an internal-plumbing + test-surface change with no call site to migrate.

Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.79%. Comparing base (71e8e50) to head (2813311).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6567      +/-   ##
==========================================
+ Coverage   78.72%   78.79%   +0.07%     
==========================================
  Files         777      778       +1     
  Lines       77213    77500     +287     
==========================================
+ Hits        60784    61067     +283     
- Misses      16424    16428       +4     
  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: concurrency-reviewer, security-reviewer, test-coverage-reviewer, general-quality-reviewer

Consensus Summary

#	Finding	Consensus	Severity	Action
1	Doc comment doesn't warn that a workloadID-only resolver defeats the SSRF/DNS-rebinding defense	8/10	MEDIUM	Fix
2	RestoreSession path never tests per-backend resolver keying, only blanket deny-all	8/10	MEDIUM	Fix
3	No test coverage for a resolver that panics for a given workload	7/10	MEDIUM	Fix
4	Doc comments overstate "twin" parity with client.WithDialControl, now asymmetric	7/10	MEDIUM	Fix
5	Documented concurrent-invocation contract for the resolver isn't exercised by any race test	8/10	LOW	Fix
6	Resolver's parameter named resolve, diverging from sibling WithRequestTimeoutResolver's resolver	7/10	LOW	Fix

Overall

This is a small, self-contained refactor that generalizes an unreleased, opt-in embedder option (WithDialControl) into a per-workload resolver (WithDialControlResolver), fixing a real limitation of the single address-blind hook from #6547. The approach is sound — it mirrors the two resolver-style options already on the same struct (WithRevisionLookup, WithRequestTimeoutResolver) rather than inventing a new shape, and the no-hook default path is proven byte-for-byte unchanged.

All findings below are polish and coverage gaps, not correctness bugs — nothing here should block merge. The two most worth addressing before merge: the RestoreSession path only has a blanket deny-all test (no per-backend-keying proof, unlike the MakeSessionWithID path), and the security-limitations doc doesn't warn that a resolver deciding purely on workloadID without inspecting the dialed address gives zero SSRF protection despite looking like a per-backend guard.

Documentation

• mcp_session.go (mirrored in factory.go): add an explicit caveat to "Security limitations embedders must understand" that the resolver must select a hook which itself inspects the resolved address — gating purely on workloadID defeats the SSRF/DNS-rebinding defense (see inline comment).
• Both files' WithDialControlResolver doc comments: the "twin"/"counterpart" framing with client.WithDialControl should acknowledge that the two option shapes are now asymmetric (see inline comment).

---

Generated with Claude Code

[jhrozek]

Reviewed the diff (factory.go, mcp_session.go, + tests) across Go quality, security, and ToolHive conventions/test-coverage angles.

• Clean, narrow refactor of the session-init dial-control option from a single static hook into a per-workload resolver, mirroring the existing WithRevisionLookup/WithRequestTimeoutResolver pattern.
• Panic-recovery in resolveDialControl is fail-closed: a panicking resolver excludes that backend rather than silently leaving the hook absent.
• No regression in the SSRF/DNS-rebinding guarantee; the new caveat that the resolver only selects a hook (the returned hook must itself check the address) is documented prominently.
• Test coverage is thorough: per-backend resolution on both init and restore, panic isolation, and a real barrier-based concurrency test.
• Checked the API-removal concern raised elsewhere: WithDialControl was added in #6547 and never shipped in a release (last tag v0.47.1 predates it) and has no in-tree caller, so the rename is not a compatibility break.

No blocking issues found.