MCPServerEntry.spec.headerForward is accepted by CRD but never sent with requests by VirtualMCPServer

[lorr1]

Summary

MCPServerEntry.spec.headerForward.addPlaintextHeaders (and addHeadersFromSecret) is defined in the CRD and accepted at admission, but the values are never actually sent on outbound requests to remoteUrl when the entry is consumed as a static backend of a VirtualMCPServer. Only MCPRemoteProxy currently wires this field through.

Reproducer

apiVersion: toolhive.stacklok.dev/v1beta1
kind: MCPServerEntry
metadata:
  name: github-copilot-projects
  namespace: stacklok-tunnel
spec:
  remoteUrl: https://api.githubcopilot.com/mcp/
  transport: streamable-http
  groupRef: { name: stacklok-group }
  externalAuthConfigRef: { name: github-oauth-upstream-inject }
  headerForward:
    addPlaintextHeaders:
      X-MCP-Toolsets: "projects,issues,pull_requests,users,context"

GitHub's remote MCP server uses X-MCP-Toolsets to combine multiple toolsets (the ?toolsets= query string is ignored and the URL path only accepts one toolset). Requests from the vMCP arrive at api.githubcopilot.com without this header, so only the default toolset's tools surface.

Evidence

HeaderForward is consumed only by MCPRemoteProxy:

• cmd/thv-operator/controllers/mcpremoteproxy_runconfig.go
• cmd/thv-operator/controllers/mcpremoteproxy_deployment.go
• cmd/thv-operator/pkg/controllerutil/externalauth.go

For MCPServerEntry, the only reference is the type definition at cmd/thv-operator/api/v1beta1/mcpserverentry_types.go. No controller/runtime path feeds MCPServerEntry.spec.headerForward into the vMCP's outbound HTTP client for static backends.

Expected

When a VirtualMCPServer loads a backend from an MCPServerEntry, the runtime HTTP client used to reach remoteUrl should inject the addPlaintextHeaders and addHeadersFromSecret values on every outbound request (health checks, listTools, tool invocations).

Workaround

Use the URL path (/mcp/x/<toolset> or /mcp/x/all) since the GitHub upstream is path-addressable. Trades granularity for functionality.

Context

Discovered wiring github-copilot-projects behind the vMCP for the Stacklok Anthropic tunnel — see stacklok/infra#4485 and stacklok/infra branch fix/github-copilot-toolsets-header.

[lorr1]

Fix #4996: Wire MCPServerEntry.spec.headerForward through vMCP outbound requests

Context

MCPServerEntry.spec.headerForward.addPlaintextHeaders (and addHeadersFromSecret) is defined on the CRD and accepted at admission, but the values are silently dropped on the vMCP outbound path. When a VirtualMCPServer resolves an MCPServerEntry backend, requests to remoteUrl go out without the configured headers.

Confirmed:

• Field declared at cmd/thv-operator/api/v1beta1/mcpserverentry_types.go:41. Zero reads of entry.Spec.HeaderForward in the tree.
• pkg/vmcp/workloads/k8s.go mcpServerEntryToBackend (line 503) populates URL, transport, CA bundle, auth, metadata — but not HeaderForward.
• vmcp.Backend (pkg/vmcp/types.go:281) and vmcp.BackendTarget (pkg/vmcp/types.go:20) have no field for forwarded headers.
• Outbound HTTP client (pkg/vmcp/client/client.go:264-340) builds a RoundTripper chain for CA / auth / identity / trace — no per-backend header injection.
• Only MCPRemoteProxy wires HeaderForward through (via the runner's env-var-based secret resolution + inbound middleware at pkg/runner/middleware.go:211). That model doesn't apply to vMCP (shared aggregator Pod, no per-entry Secret mounts).

Motivating case: GitHub's remote MCP server uses X-MCP-Toolsets to combine toolsets (stacklok/infra#4485). vMCP never sends it, so only the default toolset surfaces.

Intended outcome: when an MCPServerEntry is consumed by a VirtualMCPServer (dynamic mode via groupRef), every outbound HTTP request from vMCP to remoteUrl carries the configured HeaderForward headers — plaintext inlined, Secret-backed resolved at discovery time.

Success Criteria

Each item is a user-observable outcome.

The headline flow (what the issue is asking for)

• SC1 — Plaintext header reaches the upstream: An operator sets headerForward.addPlaintextHeaders on an MCPServerEntry and wires it to a VirtualMCPServer. Every request the vMCP sends to that catalog entry's remoteUrl carries that header — the initial handshake, the tool list, every tool call, and the session teardown. No special step, no restart; configuring the entry is enough.
• SC2 — Secret-backed header reaches the upstream: Same as SC1 but the header value comes from a Kubernetes Secret. The operator never has to handle the secret value themselves; it shows up on outbound requests with the value currently in the Secret.
• SC3 — Plaintext and Secret-backed headers together: An entry that uses both forms works — both values appear on the same outbound request.
• SC4 — The end-user bug is fixed: Pointing an entry at api.githubcopilot.com/mcp/ with X-MCP-Toolsets configured, tools from all the toolsets the operator listed appear in the vMCP's aggregated tool list. (Today only the default toolset surfaces; see stacklok/infra#4485.)

Day-2 operations

• SC5 — Secret rotation is live: An operator updates the Secret value (or the operator's secret rotator rotates it). The next tool call from vMCP to that backend uses the new value. No pod restart, no manual re-sync.
• SC6 — Headers stay present when the upstream session reconnects: When vMCP re-establishes its session with the backend (new session, reconnect after drop, health check), the headers keep being applied — not just on the first request.

Misconfiguration is loud, not silent

• SC7 — Forbidden header is rejected at admission time: If an operator tries to configure a header that has no business being set by a user (host-level, content-length-style, hop-by-hop), they see the MCPServerEntry move to a failed state on kubectl get/describe with a clear message naming the offending header. No upstream call is made with a half-configured header set.
• SC8 — Missing Secret is called out: If the operator references a Secret or key that doesn't exist, the entry shows a failed state with a message naming the missing Secret. The backend does not appear as healthy in the aggregated catalog, and no empty-header fallback request is made.
• SC9 — Conflicting names are called out: If an operator declares the same header name both as plaintext and from a Secret, the entry fails with a message identifying the conflict — no silent pick-a-winner behavior.

Auth interaction is predictable

• SC10 — Operator intent wins over discovered auth: If an MCPServerEntry has an auth strategy that would set Authorization AND the operator also sets Authorization via headerForward, the operator's configured value reaches the upstream. (Mirrors how MCPRemoteProxy's header-forward step runs last today.) A warning is logged so an operator can see in the vMCP logs that they are overriding auth-layer credentials — without the credential value leaking into the log.

Nothing else moves

• SC11 — MCPRemoteProxy header forwarding is unchanged: Operators already using MCPRemoteProxy.spec.headerForward see the exact same behavior they had before.
• SC12 — Catalog entries without headerForward behave exactly as before: No new failure modes, no new Secret fetches, no new log noise.

Approach

Follow vMCP's existing "resolve at discovery time, carry on the Backend, apply in the outbound RoundTripper" pattern (same pattern used for CA bundles in dynamic mode and auth). Do not mirror MCPRemoteProxy's env-var-based runner pattern.

Phased Execution (Red → Green)

Follow a strict red-green workflow. Each phase ends with a named gate that must be verified before moving on.

Phase 0 — Baseline

• Capture task test pass/fail baseline on the starting branch.
• Gate: all tests currently green (or record any pre-existing flakes so they aren't conflated with new failures).

Phase 1 — Write failing unit tests (RED)

No production code yet. Write tests targeting the future APIs exactly as described in "Implementation" below. Each test should fail with a clear "undefined" or "zero value" error — a failure that proves the bug.

Test files to add/extend (all tests must compile but fail on current code):

1. pkg/vmcp/workloads/k8s_test.go — table test for mcpServerEntryToBackend:
   • plaintext-only entry → resolved headers populated (covers SC1, SC12)
   • secret-only entry (fake K8s client seeded with Secret) → resolved headers populated (covers SC2)
   • plaintext + secret mixed (SC3)
   • forbidden header → backend discovery fails (SC7)
   • missing Secret → backend discovery fails (SC8)
   • plaintext/secret name collision → backend discovery fails (SC9)
   • entry with no headerForward → behaves exactly as before (SC12)
2. pkg/vmcp/registry_test.go — BackendToTarget copies resolved headers onto the target (supports SC1-SC3).
3. pkg/vmcp/client/client_test.go — new tests for the outbound header-injection round-tripper:
   • Configured headers appear on every outbound request, including session teardown / reconnect paths (SC1, SC6).
   • When auth pre-sets Authorization and the entry also sets it via headerForward, the entry's value reaches the wire (SC10).
   • The caller's *http.Request is not mutated (internal invariant — prevents flakes in the other tests).
   • No-op when the target has no configured headers (SC12).
   • Warning log emitted when Authorization is among configured headers, without leaking the value (SC10).
4. cmd/thv-operator/controllers/mcpserverentry_controller_test.go — validation cases:
   • Forbidden header → entry Phase=Failed, condition message names the header (SC7).
   • Missing Secret/key → Phase=Failed, condition names the Secret (SC8).
   • Plaintext/secret name collision → Phase=Failed, condition names the colliding header (SC9).
   • Happy path → Phase=Valid (SC1-SC3).
5. pkg/vmcp/k8s/backend_reconciler_test.go — Secret-change reconciliation:
   • Update the Secret referenced by an entry's headerForward.addHeadersFromSecret → the entry re-reconciles → registry's target reflects the new value (SC5).

Gate: task test run — every new/modified test must be RED (failing, not errored-skipping). Capture the failure output as the red-baseline record.

Phase 2 — Implementation (make tests GREEN)

Implement in the order below; after each file, re-run task test and confirm the affected tests flip to green.

2.1 pkg/vmcp/types.go

Add HeaderForward map[string]string to Backend (line ~281) and BackendTarget (line ~20). Values are merged + resolved + canonicalized headers. Doc the invariant.

2.2 pkg/vmcp/registry.go

Extend BackendToTarget (line 361) to copy HeaderForward.

2.3 pkg/vmcp/workloads/k8s.go

Add private resolveHeaderForward(ctx, entry):

• Nil HeaderForward → nil, nil.
• Canonicalize names, reject middleware.RestrictedHeaders.
• Merge AddPlaintextHeaders; iterate AddHeadersFromSecret, fetch Secret via existing K8s client (mirror fetchCABundleData/discoverAuthConfigFromRef).
• Error on plaintext/secret name collision.
• Return merged, canonical map.

Call it from mcpServerEntryToBackend after the CA-bundle block. On helper error, log + return nil from mcpServerEntryToBackend (match existing CA-bundle-fatal pattern, line 570-577). Assign result to backend.HeaderForward.

2.4 pkg/vmcp/client/client.go

Add headerForwardRoundTripper (template: authRoundTripper, line 221) — clones request, reqClone.Header.Set(name, value) for each configured header, delegates. No-op on empty.

Insert in defaultClientFactory (line 270-297) between httpTransport and authRoundTripper, so it's innermost around httpTransport and runs AFTER the auth layer. This matches MCPRemoteProxy's runner middleware ordering (pkg/runner/middleware.go:211-218: "added near the end so it executes closest to the backend").

Because it wraps inside identityPropagatingRoundTripper, the health-check DELETE path still carries headers.

Log a warning at client-creation time if Authorization is among configured headers (mirrors header_forward.go:117).

2.5 cmd/thv-operator/controllers/mcpserverentry_controller.go

In Reconcile, after existing validation, validate Spec.HeaderForward:

• Reject restricted header names.
• Reject plaintext/secret name collisions.
• For each AddHeadersFromSecret: verify Secret+key exist; on failure set InvalidHeaderForward condition, phase Failed.

No Secret watch on this controller — the backend reconciler owns that.

2.6 pkg/vmcp/k8s/backend_reconciler.go

• Add field index headerForwardSecretIndex on MCPServerEntry (mirror caBundleConfigMapIndex, line 25-90).
• Add Watches(&corev1.Secret{}, handler) in SetupWithManager: Secret event → reconcile every MCPServerEntry referencing that Secret. Drives DynamicRegistry refresh so running vMCP Pods pick up rotated values without restart.

Gate after Phase 2: task test fully green, including the formerly-red tests from Phase 1. task lint-fix clean.

Phase 3 — Integration tests (GREEN from the start)

Add to cmd/thv-operator/test-integration/virtualmcp/:

• Stand up an in-test HTTP echo backend.
• Apply an MCPServerEntry with plaintext + Secret HeaderForward.
• Create a VirtualMCPServer using groupRef.
• Issue a real MCP request through vMCP; assert the echo server saw the expected headers (SC1, SC2, SC3).
• Mutate the referenced Secret; assert subsequent requests carry the new value (SC5).

Gate: task test-e2e passes.

Phase 4 — Manual verification against the issue reproducer

1. Build the operator image from this branch into a kind cluster.
2. Apply the user's YAML from the issue (X-MCP-Toolsets: "projects,issues,pull_requests,users,context").
3. Point remoteUrl at a local echo server first; confirm X-MCP-Toolsets present.
4. Swap to api.githubcopilot.com/mcp/; confirm tools from all configured toolsets surface (resolves stacklok/infra#4485).

Gate: issue reproducer produces expected behavior.

Phase 5 — Regression sweep

• Run cmd/thv-operator/test-integration/mcp-remote-proxy suite unchanged (SC11).
• task test-all clean.
• Review diff for unexpected renames / accidental changes to mcpServerEntryToBackend happy path.

Gate: full regression suite green.

Files to modify

• pkg/vmcp/types.go
• pkg/vmcp/registry.go
• pkg/vmcp/workloads/k8s.go
• pkg/vmcp/client/client.go
• pkg/vmcp/k8s/backend_reconciler.go
• cmd/thv-operator/controllers/mcpserverentry_controller.go
• Corresponding test files (Phase 1 list).

Reusable utilities

• pkg/transport/middleware/header_forward.go:22 RestrictedHeaders — reuse blocklist. Set is symmetric between inbound forwarding and outbound injection (blocks Host, Content-Length, and other hop-by-hop / transport-managed headers that either side should not set).
• pkg/vmcp/workloads/k8s.go fetchCABundleData — pattern for fetching referenced K8s objects at discovery time.
• pkg/vmcp/workloads/k8s.go discoverAuthConfigFromRef — pattern for Secret resolution at discovery time.
• pkg/vmcp/k8s/backend_reconciler.go caBundleConfigMapIndex + Watches(...) — pattern for propagating referenced-object changes.
• pkg/vmcp/client/client.go authRoundTripper (line 221) — template for headerForwardRoundTripper.

Out of scope

• pkg/vmcp/config/config.go StaticBackendConfig (line 278): no HeaderForward field today. Adding it is a separate feature.
• VirtualMCPServer.spec.backends[*].headerForward: not on the CRD today. Out of scope.
• Templated / per-request header derivation. Headers are static per backend.
• Unifying the MCPRemoteProxy runner secret-resolution model with vMCP's discovery-time model.

Edge cases explicitly handled

• Redirects: headerForwardRoundTripper runs per RoundTrip, so headers re-apply on each hop.
• Health-check DELETE: wrapped inside identityPropagatingRoundTripper — still covered.
• CA bundle + HeaderForward: independent layers.
• Session reconnect: same transport chain per-backend-client; headers re-applied on each RoundTrip.

RBAC

• Backend reconciler needs list/watch on Secrets (likely already has it for ExternalAuthConfig resolution) — audit cmd/thv-operator/controllers/virtualmcpserver_deployment.go Role/ClusterRole; only add if missing.

[ChrisJBurns]

Looking at this right now