From 8d8e6d41b323abd98cb7ee0378736ac90336430a Mon Sep 17 00:00:00 2001 From: Chris Burns <29541485+ChrisJBurns@users.noreply.github.com> Date: Wed, 22 Apr 2026 16:52:49 +0100 Subject: [PATCH] Forward MCPServerEntry headerForward to vMCP outbound requests MCPServerEntry.spec.headerForward was accepted by the CRD but silently ignored when the entry was consumed as a static backend of VirtualMCPServer. Only MCPRemoteProxy honored the field, so vMCP requests to remoteUrl arrived without the configured headers (e.g. GitHub Copilot's X-MCP-Toolsets). Mirror the MCPRemoteProxy pattern so header values never enter the ConfigMap: - Surface headerForward on pkg/vmcp.Backend, BackendTarget, and vmcpconfig.StaticBackendConfig so the runtime model carries per-backend header injection alongside auth and CA bundle. - Extract headerForward in mcpServerEntryToBackend and threader the values through buildHeaderForwardMap and convertBackendsToStaticBackends to the ConfigMap. Plaintext is copied verbatim; Secret refs become only env-var identifiers via ctrlutil.GenerateHeaderForwardSecretEnvVarName. - Emit one env var per (entry, header) on the vMCP Deployment with valueFrom.secretKeyRef so the Secret value is injected at pod start. - Insert a new headerForwardRoundTripper in pkg/vmcp/client between identityPropagatingRoundTripper and authRoundTripper. Resolve headers once at client-factory time via secrets.EnvironmentProvider; reject restricted headers via the shared pkg/transport/middleware.RestrictedHeaders set. - Validate referenced Secrets in MCPServerEntry's reconciler and surface the result through a new HeaderSecretRefsValidated condition, matching MCPRemoteProxy's HeaderSecretNotFound reason. - Carry HeaderForward, CABundlePath, and CABundleData into the BackendTarget built by the health monitor so health checks hit backends with the same TLS trust and header injection as list/call. - Generalize GenerateHeaderForwardSecretEnvVarName to accept an owner name rather than a proxy name; the existing MCPRemoteProxy caller is unchanged and MCPServerEntry uses the entry name as the disambiguator. Closes #4996 Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/thv-operator/Taskfile.yml | 2 +- .../api/v1beta1/mcpserverentry_types.go | 13 + .../controllers/mcpserverentry_controller.go | 60 +++++ .../mcpserverentry_controller_test.go | 75 ++++++ .../virtualmcpserver_controller.go | 13 +- .../virtualmcpserver_deployment.go | 75 ++++++ .../virtualmcpserver_deployment_test.go | 205 ++++++++++++++ .../virtualmcpserver_vmcpconfig.go | 100 ++++++- .../virtualmcpserver_vmcpconfig_test.go | 195 +++++++++++++- .../pkg/controllerutil/externalauth.go | 26 +- ...olhive.stacklok.dev_virtualmcpservers.yaml | 48 ++++ ...olhive.stacklok.dev_virtualmcpservers.yaml | 48 ++++ docs/operator/crd-api.md | 86 ++++++ pkg/vmcp/aggregator/discoverer.go | 1 + pkg/vmcp/client/client.go | 17 +- pkg/vmcp/client/header_forward.go | 157 +++++++++++ pkg/vmcp/client/header_forward_test.go | 251 ++++++++++++++++++ pkg/vmcp/config/config.go | 7 + pkg/vmcp/config/zz_generated.deepcopy.go | 6 + pkg/vmcp/doc.go | 2 + pkg/vmcp/health/monitor.go | 8 +- pkg/vmcp/registry.go | 1 + pkg/vmcp/types.go | 34 +++ pkg/vmcp/workloads/k8s.go | 43 +++ pkg/vmcp/workloads/k8s_test.go | 163 ++++++++++++ pkg/vmcp/zz_generated.deepcopy.go | 52 ++++ 26 files changed, 1671 insertions(+), 17 deletions(-) create mode 100644 pkg/vmcp/client/header_forward.go create mode 100644 pkg/vmcp/client/header_forward_test.go create mode 100644 pkg/vmcp/zz_generated.deepcopy.go diff --git a/cmd/thv-operator/Taskfile.yml b/cmd/thv-operator/Taskfile.yml index ce7a823327..c3de2f7704 100644 --- a/cmd/thv-operator/Taskfile.yml +++ b/cmd/thv-operator/Taskfile.yml @@ -168,7 +168,7 @@ tasks: platforms: [windows] ignore_error: true # Windows has no mkdir -p, so just ignore error if it exists - go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.17.3 - - $(go env GOPATH)/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./cmd/thv-operator/..." paths="./pkg/json/..." paths="./pkg/vmcp/config/..." paths="./pkg/vmcp/auth/types/..." paths="./pkg/telemetry/..." paths="./pkg/audit/..." + - $(go env GOPATH)/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./cmd/thv-operator/..." paths="./pkg/json/..." paths="./pkg/vmcp" paths="./pkg/vmcp/config/..." paths="./pkg/vmcp/auth/types/..." paths="./pkg/telemetry/..." paths="./pkg/audit/..." operator-manifests: desc: Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects diff --git a/cmd/thv-operator/api/v1beta1/mcpserverentry_types.go b/cmd/thv-operator/api/v1beta1/mcpserverentry_types.go index 6c5043d6e7..04c989230b 100644 --- a/cmd/thv-operator/api/v1beta1/mcpserverentry_types.go +++ b/cmd/thv-operator/api/v1beta1/mcpserverentry_types.go @@ -101,6 +101,11 @@ const ( // ConditionTypeMCPServerEntryRemoteURLValidated indicates whether the RemoteURL passes // format and SSRF safety checks. ConditionTypeMCPServerEntryRemoteURLValidated = "RemoteURLValidated" + + // ConditionTypeMCPServerEntryHeaderSecretRefsValidated indicates whether every Kubernetes + // Secret referenced by spec.headerForward.addHeadersFromSecret exists. Absent when the + // entry declares no header Secret refs. + ConditionTypeMCPServerEntryHeaderSecretRefsValidated = "HeaderSecretRefsValidated" ) // Condition reasons for MCPServerEntry. @@ -146,6 +151,14 @@ const ( // ConditionReasonMCPServerEntryRemoteURLInvalid indicates the RemoteURL is malformed or // targets a blocked internal/metadata endpoint. ConditionReasonMCPServerEntryRemoteURLInvalid = ConditionReasonRemoteURLInvalid + + // ConditionReasonMCPServerEntryHeaderSecretsValid indicates every referenced header Secret exists. + ConditionReasonMCPServerEntryHeaderSecretsValid = "HeaderSecretsValid" + + // ConditionReasonMCPServerEntryHeaderSecretNotFound indicates a Secret referenced by + // spec.headerForward.addHeadersFromSecret was not found in the entry's namespace. + // Reuses the string value used by MCPRemoteProxy for parity. + ConditionReasonMCPServerEntryHeaderSecretNotFound = ConditionReasonHeaderSecretNotFound ) //+kubebuilder:object:root=true diff --git a/cmd/thv-operator/controllers/mcpserverentry_controller.go b/cmd/thv-operator/controllers/mcpserverentry_controller.go index c34281470f..672180a264 100644 --- a/cmd/thv-operator/controllers/mcpserverentry_controller.go +++ b/cmd/thv-operator/controllers/mcpserverentry_controller.go @@ -85,6 +85,12 @@ func (r *MCPServerEntryReconciler) Reconcile(ctx context.Context, req ctrl.Reque } allValid = valid && allValid + valid, err = r.validateHeaderForwardSecretRefs(ctx, entry) + if err != nil { + return ctrl.Result{}, err + } + allValid = valid && allValid + // Compute overall phase and Valid condition r.updateOverallStatus(entry, allValid) @@ -300,6 +306,60 @@ func (r *MCPServerEntryReconciler) validateCABundleRef( return true, nil } +// validateHeaderForwardSecretRefs checks that every Secret referenced by +// spec.headerForward.addHeadersFromSecret exists in the entry's namespace. +// Skipped (condition removed) when no Secret-backed headers are declared. +// Returns (valid, error) where a non-nil error means a transient API failure +// that should be requeued. +func (r *MCPServerEntryReconciler) validateHeaderForwardSecretRefs( + ctx context.Context, + entry *mcpv1beta1.MCPServerEntry, +) (bool, error) { + ctxLogger := log.FromContext(ctx) + + if entry.Spec.HeaderForward == nil || len(entry.Spec.HeaderForward.AddHeadersFromSecret) == 0 { + meta.RemoveStatusCondition(&entry.Status.Conditions, + mcpv1beta1.ConditionTypeMCPServerEntryHeaderSecretRefsValidated) + return true, nil + } + + for _, ref := range entry.Spec.HeaderForward.AddHeadersFromSecret { + if ref.ValueSecretRef == nil { + continue + } + secret := &corev1.Secret{} + secretKey := types.NamespacedName{ + Namespace: entry.Namespace, + Name: ref.ValueSecretRef.Name, + } + if err := r.Get(ctx, secretKey, secret); err != nil { + if errors.IsNotFound(err) { + meta.SetStatusCondition(&entry.Status.Conditions, metav1.Condition{ + Type: mcpv1beta1.ConditionTypeMCPServerEntryHeaderSecretRefsValidated, + Status: metav1.ConditionFalse, + Reason: mcpv1beta1.ConditionReasonMCPServerEntryHeaderSecretNotFound, + Message: fmt.Sprintf("Secret %q referenced for header %q not found", + ref.ValueSecretRef.Name, ref.HeaderName), + ObservedGeneration: entry.Generation, + }) + return false, nil + } + ctxLogger.Error(err, "Failed to get referenced header Secret", + "secret", ref.ValueSecretRef.Name, "header", ref.HeaderName) + return false, err + } + } + + meta.SetStatusCondition(&entry.Status.Conditions, metav1.Condition{ + Type: mcpv1beta1.ConditionTypeMCPServerEntryHeaderSecretRefsValidated, + Status: metav1.ConditionTrue, + Reason: mcpv1beta1.ConditionReasonMCPServerEntryHeaderSecretsValid, + Message: "All referenced header Secrets exist", + ObservedGeneration: entry.Generation, + }) + return true, nil +} + // validateRemoteURL checks that the RemoteURL is well-formed and does not target // a blocked internal or metadata endpoint (SSRF protection). func (*MCPServerEntryReconciler) validateRemoteURL( diff --git a/cmd/thv-operator/controllers/mcpserverentry_controller_test.go b/cmd/thv-operator/controllers/mcpserverentry_controller_test.go index 97b01014fd..3dbfcb512f 100644 --- a/cmd/thv-operator/controllers/mcpserverentry_controller_test.go +++ b/cmd/thv-operator/controllers/mcpserverentry_controller_test.go @@ -330,6 +330,81 @@ func TestMCPServerEntryReconciler_Reconcile(t *testing.T) { {mcpv1beta1.ConditionTypeMCPServerEntryValid, metav1.ConditionTrue, mcpv1beta1.ConditionReasonMCPServerEntryValid}, }, }, + { + name: "headerForward plaintext only - no secret validation needed", + entry: func() *mcpv1beta1.MCPServerEntry { + e := newMCPServerEntry(testEntryGroupRef, nil, nil) + e.Spec.HeaderForward = &mcpv1beta1.HeaderForwardConfig{ + AddPlaintextHeaders: map[string]string{"X-MCP-Toolsets": "projects,issues"}, + } + return e + }(), + objects: []client.Object{newMCPGroup(mcpv1beta1.MCPGroupPhaseReady)}, + wantPhase: mcpv1beta1.MCPServerEntryPhaseValid, + conditions: []struct { + condType string + status metav1.ConditionStatus + reason string + }{ + {mcpv1beta1.ConditionTypeMCPServerEntryValid, metav1.ConditionTrue, mcpv1beta1.ConditionReasonMCPServerEntryValid}, + }, + }, + { + name: "headerForward secret ref exists", + entry: func() *mcpv1beta1.MCPServerEntry { + e := newMCPServerEntry(testEntryGroupRef, nil, nil) + e.Spec.HeaderForward = &mcpv1beta1.HeaderForwardConfig{ + AddHeadersFromSecret: []mcpv1beta1.HeaderFromSecret{ + { + HeaderName: "X-API-Key", + ValueSecretRef: &mcpv1beta1.SecretKeyRef{Name: "api-key-secret", Key: "token"}, + }, + }, + } + return e + }(), + objects: []client.Object{ + newMCPGroup(mcpv1beta1.MCPGroupPhaseReady), + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "api-key-secret", Namespace: testEntryNS}, + Data: map[string][]byte{"token": []byte("secret-value")}, + }, + }, + wantPhase: mcpv1beta1.MCPServerEntryPhaseValid, + conditions: []struct { + condType string + status metav1.ConditionStatus + reason string + }{ + {mcpv1beta1.ConditionTypeMCPServerEntryHeaderSecretRefsValidated, metav1.ConditionTrue, mcpv1beta1.ConditionReasonMCPServerEntryHeaderSecretsValid}, + {mcpv1beta1.ConditionTypeMCPServerEntryValid, metav1.ConditionTrue, mcpv1beta1.ConditionReasonMCPServerEntryValid}, + }, + }, + { + name: "headerForward secret ref missing flips entry to Failed", + entry: func() *mcpv1beta1.MCPServerEntry { + e := newMCPServerEntry(testEntryGroupRef, nil, nil) + e.Spec.HeaderForward = &mcpv1beta1.HeaderForwardConfig{ + AddHeadersFromSecret: []mcpv1beta1.HeaderFromSecret{ + { + HeaderName: "X-API-Key", + ValueSecretRef: &mcpv1beta1.SecretKeyRef{Name: "missing-secret", Key: "token"}, + }, + }, + } + return e + }(), + objects: []client.Object{newMCPGroup(mcpv1beta1.MCPGroupPhaseReady)}, + wantPhase: mcpv1beta1.MCPServerEntryPhaseFailed, + conditions: []struct { + condType string + status metav1.ConditionStatus + reason string + }{ + {mcpv1beta1.ConditionTypeMCPServerEntryHeaderSecretRefsValidated, metav1.ConditionFalse, mcpv1beta1.ConditionReasonMCPServerEntryHeaderSecretNotFound}, + {mcpv1beta1.ConditionTypeMCPServerEntryValid, metav1.ConditionFalse, mcpv1beta1.ConditionReasonMCPServerEntryInvalid}, + }, + }, } for _, tt := range tests { diff --git a/cmd/thv-operator/controllers/virtualmcpserver_controller.go b/cmd/thv-operator/controllers/virtualmcpserver_controller.go index 3baf42eed9..518265a7d2 100644 --- a/cmd/thv-operator/controllers/virtualmcpserver_controller.go +++ b/cmd/thv-operator/controllers/virtualmcpserver_controller.go @@ -2112,12 +2112,19 @@ func injectSubjectProviderIfNeeded( // convertBackendsToStaticBackends converts Backend objects to StaticBackendConfig for ConfigMap embedding. // Preserves metadata and uses transport types from workload Specs. // Logs warnings when backends are skipped due to missing URL or transport information. -// caBundlePathMap maps backend names to their CA bundle mount paths (populated for MCPServerEntry backends). +// +// caBundlePathMap maps backend names to their CA bundle mount paths (populated for +// MCPServerEntry backends with caBundleRef). +// headerForwardMap maps backend names to per-backend HeaderForwardConfig (populated +// for MCPServerEntry backends with headerForward). Plaintext values are embedded +// as-is; secret-backed headers carry only env-var identifiers that the vMCP runtime +// resolves via secrets.EnvironmentProvider (TOOLHIVE_SECRET_). func convertBackendsToStaticBackends( ctx context.Context, backends []vmcptypes.Backend, transportMap map[string]string, caBundlePathMap map[string]string, + headerForwardMap map[string]*vmcptypes.HeaderForwardConfig, ) []vmcpconfig.StaticBackendConfig { logger := log.FromContext(ctx) static := make([]vmcpconfig.StaticBackendConfig, 0, len(backends)) @@ -2146,6 +2153,10 @@ func convertBackendsToStaticBackends( cfg.CABundlePath = caBundlePath } + if hf, ok := headerForwardMap[backend.Name]; ok && hf != nil { + cfg.HeaderForward = hf + } + static = append(static, cfg) } return static diff --git a/cmd/thv-operator/controllers/virtualmcpserver_deployment.go b/cmd/thv-operator/controllers/virtualmcpserver_deployment.go index c764c494cd..437c6f8016 100644 --- a/cmd/thv-operator/controllers/virtualmcpserver_deployment.go +++ b/cmd/thv-operator/controllers/virtualmcpserver_deployment.go @@ -347,6 +347,16 @@ func (r *VirtualMCPServerReconciler) buildEnvVarsForVmcp( // Mount outgoing auth secrets env = append(env, r.buildOutgoingAuthEnvVars(ctx, vmcp, typedWorkloads)...) + // Mount per-backend header-forward secrets for MCPServerEntry backends. + // Each Secret referenced by MCPServerEntry.spec.headerForward.addHeadersFromSecret + // is exposed as TOOLHIVE_SECRET_ via valueFrom.secretKeyRef. The vMCP + // runtime reads these through secrets.EnvironmentProvider at request time. + headerEnv, err := r.buildHeaderForwardEnvVarsForEntries(ctx, vmcp.Namespace, typedWorkloads) + if err != nil { + return nil, fmt.Errorf("failed to build header forward env vars: %w", err) + } + env = append(env, headerEnv...) + // Always mount HMAC secret for session token binding. env = append(env, r.buildHMACSecretEnvVar(vmcp)) @@ -482,6 +492,71 @@ func (r *VirtualMCPServerReconciler) buildOutgoingAuthEnvVars( return env } +// buildHeaderForwardEnvVarsForEntries iterates entry-type workloads and emits one +// env var per (entry, header) declared in spec.headerForward.addHeadersFromSecret. +// Each env var uses valueFrom.secretKeyRef so the Secret value is injected by +// Kubernetes at pod start and never stored in the vMCP ConfigMap. +// +// The env var name is TOOLHIVE_SECRET_ where is generated by +// ctrlutil.GenerateHeaderForwardSecretEnvVarName(entry.Name, headerName). Scoping +// by entry name prevents collisions when multiple entries in the group declare +// the same header. +// +// Returns (nil, nil) when no entries declare Secret-backed headers — this is the +// common case and produces no env var churn on the Deployment spec. +func (r *VirtualMCPServerReconciler) buildHeaderForwardEnvVarsForEntries( + ctx context.Context, + namespace string, + typedWorkloads []workloads.TypedWorkload, +) ([]corev1.EnvVar, error) { + // Early return if no MCPServerEntry workloads to avoid unnecessary API calls. + hasEntries := false + for _, workload := range typedWorkloads { + if workload.Type == workloads.WorkloadTypeMCPServerEntry { + hasEntries = true + break + } + } + if !hasEntries { + return nil, nil + } + + entryMap, err := r.listMCPServerEntriesAsMap(ctx, namespace) + if err != nil { + return nil, fmt.Errorf("failed to list MCPServerEntries: %w", err) + } + + var envVars []corev1.EnvVar + for _, workload := range typedWorkloads { + if workload.Type != workloads.WorkloadTypeMCPServerEntry { + continue + } + entry, found := entryMap[workload.Name] + if !found || entry.Spec.HeaderForward == nil { + continue + } + for _, ref := range entry.Spec.HeaderForward.AddHeadersFromSecret { + if ref.ValueSecretRef == nil { + continue + } + envVarName, _ := ctrlutil.GenerateHeaderForwardSecretEnvVarName(entry.Name, ref.HeaderName) + envVars = append(envVars, corev1.EnvVar{ + Name: envVarName, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: ref.ValueSecretRef.Name, + }, + Key: ref.ValueSecretRef.Key, + }, + }, + }) + } + } + + return envVars, nil +} + // discoverExternalAuthConfigSecrets discovers ExternalAuthConfigs from workloads in the group // and returns environment variables for their client secrets. This is used for discovered mode. func (r *VirtualMCPServerReconciler) discoverExternalAuthConfigSecrets( diff --git a/cmd/thv-operator/controllers/virtualmcpserver_deployment_test.go b/cmd/thv-operator/controllers/virtualmcpserver_deployment_test.go index 908beb06bb..b7d2f63021 100644 --- a/cmd/thv-operator/controllers/virtualmcpserver_deployment_test.go +++ b/cmd/thv-operator/controllers/virtualmcpserver_deployment_test.go @@ -868,3 +868,208 @@ func TestBuildCABundleVolumesForEntries(t *testing.T) { }) } } + +// TestBuildHeaderForwardEnvVarsForEntries verifies that MCPServerEntry backends +// with headerForward.addHeadersFromSecret produce deterministic env vars on the +// vMCP deployment pointing at the referenced Kubernetes Secrets. +func TestBuildHeaderForwardEnvVarsForEntries(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + entries []mcpv1beta1.MCPServerEntry + workloads []workloads.TypedWorkload + wantNames []string + validate func(t *testing.T, envVars []corev1.EnvVar) + }{ + { + name: "no entry workloads returns nil", + workloads: []workloads.TypedWorkload{ + {Name: "srv", Type: workloads.WorkloadTypeMCPServer}, + }, + wantNames: nil, + }, + { + name: "entry without headerForward emits nothing", + entries: []mcpv1beta1.MCPServerEntry{ + { + ObjectMeta: metav1.ObjectMeta{Name: "entry-x", Namespace: "default"}, + Spec: mcpv1beta1.MCPServerEntrySpec{ + RemoteURL: "https://mcp.example.com", + Transport: "streamable-http", + GroupRef: &mcpv1beta1.MCPGroupRef{Name: "g"}, + }, + }, + }, + workloads: []workloads.TypedWorkload{ + {Name: "entry-x", Type: workloads.WorkloadTypeMCPServerEntry}, + }, + wantNames: nil, + }, + { + name: "entry with plaintext-only headerForward emits nothing (no secrets needed)", + entries: []mcpv1beta1.MCPServerEntry{ + { + ObjectMeta: metav1.ObjectMeta{Name: "entry-p", Namespace: "default"}, + Spec: mcpv1beta1.MCPServerEntrySpec{ + RemoteURL: "https://mcp.example.com", + Transport: "streamable-http", + GroupRef: &mcpv1beta1.MCPGroupRef{Name: "g"}, + HeaderForward: &mcpv1beta1.HeaderForwardConfig{ + AddPlaintextHeaders: map[string]string{"X-A": "1"}, + }, + }, + }, + }, + workloads: []workloads.TypedWorkload{ + {Name: "entry-p", Type: workloads.WorkloadTypeMCPServerEntry}, + }, + wantNames: nil, + }, + { + name: "entry with secret-backed headers emits one env var per header", + entries: []mcpv1beta1.MCPServerEntry{ + { + ObjectMeta: metav1.ObjectMeta{Name: "entry-s", Namespace: "default"}, + Spec: mcpv1beta1.MCPServerEntrySpec{ + RemoteURL: "https://mcp.example.com", + Transport: "streamable-http", + GroupRef: &mcpv1beta1.MCPGroupRef{Name: "g"}, + HeaderForward: &mcpv1beta1.HeaderForwardConfig{ + AddHeadersFromSecret: []mcpv1beta1.HeaderFromSecret{ + { + HeaderName: "X-API-Key", + ValueSecretRef: &mcpv1beta1.SecretKeyRef{Name: "k-secret", Key: "tok"}, + }, + { + HeaderName: "Authorization", + ValueSecretRef: &mcpv1beta1.SecretKeyRef{Name: "auth-secret", Key: "val"}, + }, + }, + }, + }, + }, + }, + workloads: []workloads.TypedWorkload{ + {Name: "entry-s", Type: workloads.WorkloadTypeMCPServerEntry}, + }, + wantNames: []string{ + "TOOLHIVE_SECRET_HEADER_FORWARD_X_API_KEY_ENTRY_S", + "TOOLHIVE_SECRET_HEADER_FORWARD_AUTHORIZATION_ENTRY_S", + }, + validate: func(t *testing.T, envVars []corev1.EnvVar) { + t.Helper() + for _, ev := range envVars { + require.NotNil(t, ev.ValueFrom, "env var %q must use valueFrom.secretKeyRef", ev.Name) + require.NotNil(t, ev.ValueFrom.SecretKeyRef) + assert.Empty(t, ev.Value, "secret-backed env var must not inline a value") + } + }, + }, + { + name: "two entries declaring same header get distinct env var names", + entries: []mcpv1beta1.MCPServerEntry{ + { + ObjectMeta: metav1.ObjectMeta{Name: "entry-a", Namespace: "default"}, + Spec: mcpv1beta1.MCPServerEntrySpec{ + RemoteURL: "https://a.example.com", + Transport: "streamable-http", + GroupRef: &mcpv1beta1.MCPGroupRef{Name: "g"}, + HeaderForward: &mcpv1beta1.HeaderForwardConfig{ + AddHeadersFromSecret: []mcpv1beta1.HeaderFromSecret{ + { + HeaderName: "X-API-Key", + ValueSecretRef: &mcpv1beta1.SecretKeyRef{Name: "sec-a", Key: "v"}, + }, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "entry-b", Namespace: "default"}, + Spec: mcpv1beta1.MCPServerEntrySpec{ + RemoteURL: "https://b.example.com", + Transport: "streamable-http", + GroupRef: &mcpv1beta1.MCPGroupRef{Name: "g"}, + HeaderForward: &mcpv1beta1.HeaderForwardConfig{ + AddHeadersFromSecret: []mcpv1beta1.HeaderFromSecret{ + { + HeaderName: "X-API-Key", + ValueSecretRef: &mcpv1beta1.SecretKeyRef{Name: "sec-b", Key: "v"}, + }, + }, + }, + }, + }, + }, + workloads: []workloads.TypedWorkload{ + {Name: "entry-a", Type: workloads.WorkloadTypeMCPServerEntry}, + {Name: "entry-b", Type: workloads.WorkloadTypeMCPServerEntry}, + }, + wantNames: []string{ + "TOOLHIVE_SECRET_HEADER_FORWARD_X_API_KEY_ENTRY_A", + "TOOLHIVE_SECRET_HEADER_FORWARD_X_API_KEY_ENTRY_B", + }, + }, + { + name: "secret ref with nil ValueSecretRef is skipped", + entries: []mcpv1beta1.MCPServerEntry{ + { + ObjectMeta: metav1.ObjectMeta{Name: "entry-n", Namespace: "default"}, + Spec: mcpv1beta1.MCPServerEntrySpec{ + RemoteURL: "https://mcp.example.com", + Transport: "streamable-http", + GroupRef: &mcpv1beta1.MCPGroupRef{Name: "g"}, + HeaderForward: &mcpv1beta1.HeaderForwardConfig{ + AddHeadersFromSecret: []mcpv1beta1.HeaderFromSecret{ + {HeaderName: "X-Skip", ValueSecretRef: nil}, + }, + }, + }, + }, + }, + workloads: []workloads.TypedWorkload{ + {Name: "entry-n", Type: workloads.WorkloadTypeMCPServerEntry}, + }, + wantNames: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1beta1.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + + objs := make([]client.Object, 0, len(tt.entries)) + for i := range tt.entries { + objs = append(objs, &tt.entries[i]) + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(objs...). + Build() + + r := &VirtualMCPServerReconciler{ + Client: fakeClient, + Scheme: scheme, + } + + envVars, err := r.buildHeaderForwardEnvVarsForEntries(t.Context(), "default", tt.workloads) + require.NoError(t, err) + + gotNames := make([]string, 0, len(envVars)) + for _, ev := range envVars { + gotNames = append(gotNames, ev.Name) + } + assert.ElementsMatch(t, tt.wantNames, gotNames) + + if tt.validate != nil { + tt.validate(t, envVars) + } + }) + } +} diff --git a/cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig.go b/cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig.go index b4c4956371..185ae9f741 100644 --- a/cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig.go +++ b/cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig.go @@ -13,6 +13,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" + ctrlutil "github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil" "github.com/stacklok/toolhive/cmd/thv-operator/pkg/kubernetes/configmaps" "github.com/stacklok/toolhive/cmd/thv-operator/pkg/oidc" "github.com/stacklok/toolhive/cmd/thv-operator/pkg/runconfig/configmap/checksum" @@ -299,6 +300,96 @@ func (r *VirtualMCPServerReconciler) buildTransportMap( return transportMap, nil } +// buildHeaderForwardMap returns a map keyed by backend name (MCPServerEntry name) +// carrying the per-backend HeaderForwardConfig to embed in the vMCP ConfigMap. +// +// Plaintext headers are copied verbatim. Secret-backed headers are mapped to their +// env-var identifiers via ctrlutil.GenerateHeaderForwardSecretEnvVarName so the +// vMCP runtime can resolve them through secrets.EnvironmentProvider +// (TOOLHIVE_SECRET_). Secret values themselves are never read or stored +// here — only identifiers — keeping values out of the ConfigMap. +// +// Only entries with a non-nil headerForward that produces at least one header are +// included. Entries without headerForward (or with only nil-ValueSecretRef entries) +// are omitted; the caller treats absence as "no per-backend header injection". +func (r *VirtualMCPServerReconciler) buildHeaderForwardMap( + ctx context.Context, + namespace string, + typedWorkloads []workloads.TypedWorkload, +) (map[string]*vmcptypes.HeaderForwardConfig, error) { + result := make(map[string]*vmcptypes.HeaderForwardConfig) + + // Early return if no MCPServerEntry workloads to avoid unnecessary API calls. + hasEntries := false + for _, workload := range typedWorkloads { + if workload.Type == workloads.WorkloadTypeMCPServerEntry { + hasEntries = true + break + } + } + if !hasEntries { + return result, nil + } + + mcpServerEntryMap, err := r.listMCPServerEntriesAsMap(ctx, namespace) + if err != nil { + return nil, fmt.Errorf("failed to list MCPServerEntries: %w", err) + } + + for _, workload := range typedWorkloads { + if workload.Type != workloads.WorkloadTypeMCPServerEntry { + continue + } + entry, found := mcpServerEntryMap[workload.Name] + if !found || entry.Spec.HeaderForward == nil { + continue + } + if cfg := staticHeaderForwardFromEntry(entry); cfg != nil { + result[workload.Name] = cfg + } + } + + return result, nil +} + +// staticHeaderForwardFromEntry builds the ConfigMap-ready HeaderForwardConfig for +// a single MCPServerEntry. Returns nil when the entry declares no headers (e.g., +// all AddHeadersFromSecret entries have nil ValueSecretRef). +func staticHeaderForwardFromEntry(entry *mcpv1beta1.MCPServerEntry) *vmcptypes.HeaderForwardConfig { + src := entry.Spec.HeaderForward + if src == nil { + return nil + } + + var plaintext map[string]string + if len(src.AddPlaintextHeaders) > 0 { + plaintext = make(map[string]string, len(src.AddPlaintextHeaders)) + for k, v := range src.AddPlaintextHeaders { + plaintext[k] = v + } + } + + var secretIdents map[string]string + if len(src.AddHeadersFromSecret) > 0 { + secretIdents = make(map[string]string, len(src.AddHeadersFromSecret)) + for _, ref := range src.AddHeadersFromSecret { + if ref.ValueSecretRef == nil { + continue + } + _, identifier := ctrlutil.GenerateHeaderForwardSecretEnvVarName(entry.Name, ref.HeaderName) + secretIdents[ref.HeaderName] = identifier + } + } + + if len(plaintext) == 0 && len(secretIdents) == 0 { + return nil + } + return &vmcptypes.HeaderForwardConfig{ + AddPlaintextHeaders: plaintext, + AddHeadersFromSecret: secretIdents, + } +} + // buildCABundlePathMap builds a map of backend names to CA bundle file paths for MCPServerEntry backends. // Only entries with a caBundleRef are included in the map. func (r *VirtualMCPServerReconciler) buildCABundlePathMap( @@ -437,7 +528,14 @@ func (r *VirtualMCPServerReconciler) processOutgoingAuth( return fmt.Errorf("failed to build CA bundle path map for static mode: %w", err) } - config.Backends = convertBackendsToStaticBackends(ctx, backends, transportMap, caBundlePathMap) + headerForwardMap, err := r.buildHeaderForwardMap(ctx, vmcp.Namespace, typedWorkloads) + if err != nil { + return fmt.Errorf("failed to build header forward map for static mode: %w", err) + } + + config.Backends = convertBackendsToStaticBackends( + ctx, backends, transportMap, caBundlePathMap, headerForwardMap, + ) // Validate at least one backend exists if len(config.Backends) == 0 { diff --git a/cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig_test.go b/cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig_test.go index 5d0fe5efde..51dad9461c 100644 --- a/cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig_test.go +++ b/cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig_test.go @@ -1772,7 +1772,7 @@ func TestConvertBackendsToStaticBackends_SkipsInvalidBackends(t *testing.T) { // "no-transport-backend" intentionally missing } - result := convertBackendsToStaticBackends(ctx, backends, transportMap, nil) + result := convertBackendsToStaticBackends(ctx, backends, transportMap, nil, nil) // Should only include the valid backend assert.Len(t, result, 1, "should only include backends with URL and transport") @@ -2291,7 +2291,7 @@ func TestConvertBackendsToStaticBackends_WithCABundlePathMap(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - result := convertBackendsToStaticBackends(t.Context(), tt.backends, tt.transportMap, tt.caBundlePathMap) + result := convertBackendsToStaticBackends(t.Context(), tt.backends, tt.transportMap, tt.caBundlePathMap, nil) assert.Len(t, result, tt.expectedCount) if tt.validateBackends != nil { @@ -2451,3 +2451,194 @@ func TestBuildCABundlePathMap(t *testing.T) { }) } } + +// TestBuildHeaderForwardMap tests that the header forward map is built correctly +// from MCPServerEntry workloads, and that Secret values never appear in the +// resulting identifiers — only env-var identifiers. +func TestBuildHeaderForwardMap(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + entries []mcpv1beta1.MCPServerEntry + typedWorkloads []workloads.TypedWorkload + expectedMap map[string]*vmcp.HeaderForwardConfig + }{ + { + name: "no MCPServerEntry workloads yields empty map", + typedWorkloads: []workloads.TypedWorkload{ + {Name: "server1", Type: workloads.WorkloadTypeMCPServer}, + }, + expectedMap: map[string]*vmcp.HeaderForwardConfig{}, + }, + { + name: "entry without headerForward is omitted", + entries: []mcpv1beta1.MCPServerEntry{ + { + ObjectMeta: metav1.ObjectMeta{Name: "entry-plain", Namespace: "default"}, + Spec: mcpv1beta1.MCPServerEntrySpec{ + RemoteURL: "https://mcp.example.com", + Transport: "streamable-http", + GroupRef: &mcpv1beta1.MCPGroupRef{Name: "test-group"}, + }, + }, + }, + typedWorkloads: []workloads.TypedWorkload{ + {Name: "entry-plain", Type: workloads.WorkloadTypeMCPServerEntry}, + }, + expectedMap: map[string]*vmcp.HeaderForwardConfig{}, + }, + { + name: "plaintext headers copied verbatim", + entries: []mcpv1beta1.MCPServerEntry{ + { + ObjectMeta: metav1.ObjectMeta{Name: "gh-copilot", Namespace: "default"}, + Spec: mcpv1beta1.MCPServerEntrySpec{ + RemoteURL: "https://api.githubcopilot.com/mcp/", + Transport: "streamable-http", + GroupRef: &mcpv1beta1.MCPGroupRef{Name: "test-group"}, + HeaderForward: &mcpv1beta1.HeaderForwardConfig{ + AddPlaintextHeaders: map[string]string{ + "X-MCP-Toolsets": "projects,issues", + }, + }, + }, + }, + }, + typedWorkloads: []workloads.TypedWorkload{ + {Name: "gh-copilot", Type: workloads.WorkloadTypeMCPServerEntry}, + }, + expectedMap: map[string]*vmcp.HeaderForwardConfig{ + "gh-copilot": { + AddPlaintextHeaders: map[string]string{ + "X-MCP-Toolsets": "projects,issues", + }, + }, + }, + }, + { + name: "secret refs stored as identifiers scoped by entry name", + entries: []mcpv1beta1.MCPServerEntry{ + { + ObjectMeta: metav1.ObjectMeta{Name: "entry-sec", Namespace: "default"}, + Spec: mcpv1beta1.MCPServerEntrySpec{ + RemoteURL: "https://mcp.example.com", + Transport: "streamable-http", + GroupRef: &mcpv1beta1.MCPGroupRef{Name: "test-group"}, + HeaderForward: &mcpv1beta1.HeaderForwardConfig{ + AddHeadersFromSecret: []mcpv1beta1.HeaderFromSecret{ + { + HeaderName: "X-API-Key", + ValueSecretRef: &mcpv1beta1.SecretKeyRef{ + Name: "prod-api-key-secret", + Key: "do-not-leak", + }, + }, + }, + }, + }, + }, + }, + typedWorkloads: []workloads.TypedWorkload{ + {Name: "entry-sec", Type: workloads.WorkloadTypeMCPServerEntry}, + }, + expectedMap: map[string]*vmcp.HeaderForwardConfig{ + "entry-sec": { + AddHeadersFromSecret: map[string]string{ + "X-API-Key": "HEADER_FORWARD_X_API_KEY_ENTRY_SEC", + }, + }, + }, + }, + { + name: "two entries with same header name produce distinct identifiers", + entries: []mcpv1beta1.MCPServerEntry{ + { + ObjectMeta: metav1.ObjectMeta{Name: "entry-a", Namespace: "default"}, + Spec: mcpv1beta1.MCPServerEntrySpec{ + RemoteURL: "https://a.example.com", + Transport: "streamable-http", + GroupRef: &mcpv1beta1.MCPGroupRef{Name: "test-group"}, + HeaderForward: &mcpv1beta1.HeaderForwardConfig{ + AddHeadersFromSecret: []mcpv1beta1.HeaderFromSecret{ + { + HeaderName: "X-API-Key", + ValueSecretRef: &mcpv1beta1.SecretKeyRef{Name: "sec-a", Key: "v"}, + }, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "entry-b", Namespace: "default"}, + Spec: mcpv1beta1.MCPServerEntrySpec{ + RemoteURL: "https://b.example.com", + Transport: "streamable-http", + GroupRef: &mcpv1beta1.MCPGroupRef{Name: "test-group"}, + HeaderForward: &mcpv1beta1.HeaderForwardConfig{ + AddHeadersFromSecret: []mcpv1beta1.HeaderFromSecret{ + { + HeaderName: "X-API-Key", + ValueSecretRef: &mcpv1beta1.SecretKeyRef{Name: "sec-b", Key: "v"}, + }, + }, + }, + }, + }, + }, + typedWorkloads: []workloads.TypedWorkload{ + {Name: "entry-a", Type: workloads.WorkloadTypeMCPServerEntry}, + {Name: "entry-b", Type: workloads.WorkloadTypeMCPServerEntry}, + }, + expectedMap: map[string]*vmcp.HeaderForwardConfig{ + "entry-a": { + AddHeadersFromSecret: map[string]string{ + "X-API-Key": "HEADER_FORWARD_X_API_KEY_ENTRY_A", + }, + }, + "entry-b": { + AddHeadersFromSecret: map[string]string{ + "X-API-Key": "HEADER_FORWARD_X_API_KEY_ENTRY_B", + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1beta1.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + + objs := make([]client.Object, 0, len(tt.entries)) + for i := range tt.entries { + objs = append(objs, &tt.entries[i]) + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(objs...). + Build() + + r := &VirtualMCPServerReconciler{ + Client: fakeClient, + Scheme: scheme, + } + + result, err := r.buildHeaderForwardMap(t.Context(), "default", tt.typedWorkloads) + require.NoError(t, err) + assert.Equal(t, tt.expectedMap, result) + + // Defense-in-depth: no secret values should ever leak into the returned map. + for backendName, hf := range result { + for header, ident := range hf.AddHeadersFromSecret { + assert.NotContains(t, ident, "do-not-leak", + "backend %q header %q leaked Secret key into identifier", backendName, header) + } + } + }) + } +} diff --git a/cmd/thv-operator/pkg/controllerutil/externalauth.go b/cmd/thv-operator/pkg/controllerutil/externalauth.go index 576b38014f..a1e767cd14 100644 --- a/cmd/thv-operator/pkg/controllerutil/externalauth.go +++ b/cmd/thv-operator/pkg/controllerutil/externalauth.go @@ -48,22 +48,28 @@ func GenerateUniqueHeaderInjectionEnvVarName(configName string) string { // The generated name follows the TOOLHIVE_SECRET_ pattern expected by the EnvironmentProvider. // // Parameters: -// - proxyName: The name of the MCPRemoteProxy resource -// - headerName: The HTTP header name (e.g., "X-API-Key") +// - ownerName: The name of the CRD instance that owns the Secret reference. +// For MCPRemoteProxy this is the proxy name; for MCPServerEntry backends +// consumed by a VirtualMCPServer this is the entry name. Scoping the +// identifier by ownerName ensures multiple owners sharing a pod cannot +// collide on env var names. +// - headerName: The HTTP header name (e.g., "X-API-Key"). // -// Returns the full environment variable name (e.g., "TOOLHIVE_SECRET_HEADER_FORWARD_X_API_KEY_MY_PROXY") -// and the secret identifier portion (e.g., "HEADER_FORWARD_X_API_KEY_MY_PROXY") for use in RunConfig. -func GenerateHeaderForwardSecretEnvVarName(proxyName, headerName string) (envVarName, secretIdentifier string) { +// Returns the full environment variable name (e.g., "TOOLHIVE_SECRET_HEADER_FORWARD_X_API_KEY_MY_OWNER") +// and the secret identifier portion (e.g., "HEADER_FORWARD_X_API_KEY_MY_OWNER") for use in RunConfig +// or vMCP StaticBackendConfig. +func GenerateHeaderForwardSecretEnvVarName(ownerName, headerName string) (envVarName, secretIdentifier string) { // Sanitize header name for use in env var (uppercase, replace hyphens with underscore) sanitizedHeader := strings.ToUpper(strings.ReplaceAll(headerName, "-", "_")) sanitizedHeader = envVarSanitizer.ReplaceAllString(sanitizedHeader, "_") - // Sanitize proxy name for use in env var - sanitizedProxy := strings.ToUpper(strings.ReplaceAll(proxyName, "-", "_")) - sanitizedProxy = envVarSanitizer.ReplaceAllString(sanitizedProxy, "_") + // Sanitize owner name for use in env var + sanitizedOwner := strings.ToUpper(strings.ReplaceAll(ownerName, "-", "_")) + sanitizedOwner = envVarSanitizer.ReplaceAllString(sanitizedOwner, "_") - // Build the secret identifier (what gets stored in RunConfig.AddHeadersFromSecret) - secretIdentifier = fmt.Sprintf("HEADER_FORWARD_%s_%s", sanitizedHeader, sanitizedProxy) + // Build the secret identifier (what gets stored in RunConfig.AddHeadersFromSecret + // or StaticBackendConfig.HeaderForward.AddHeadersFromSecret). + secretIdentifier = fmt.Sprintf("HEADER_FORWARD_%s_%s", sanitizedHeader, sanitizedOwner) // Build the full env var name (TOOLHIVE_SECRET_ prefix + identifier) // This follows the pattern expected by secrets.EnvironmentProvider diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml index 411d84cdc3..1857d34734 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -882,6 +882,30 @@ spec: Only valid when Type is "entry". The operator mounts CA bundles at /etc/toolhive/ca-bundles//ca.crt. type: string + headerForward: + description: |- + HeaderForward configures per-backend HTTP headers to inject on outbound requests + to this backend. Only valid when Type is "entry". Populated by the operator from + MCPServerEntry.spec.headerForward; secret values are never stored here, only + identifiers that resolve to TOOLHIVE_SECRET_ env vars in the vMCP pod. + properties: + addHeadersFromSecret: + additionalProperties: + type: string + description: |- + AddHeadersFromSecret maps canonical HTTP header name to secret identifier. + The vMCP runtime resolves each identifier via secrets.EnvironmentProvider, + which reads TOOLHIVE_SECRET_ from the pod environment. + type: object + addPlaintextHeaders: + additionalProperties: + type: string + description: |- + AddPlaintextHeaders is a map of canonical HTTP header name to literal value. + WARNING: values are stored in plaintext in the vMCP ConfigMap. + Use AddHeadersFromSecret for sensitive data like API keys or tokens. + type: object + type: object metadata: additionalProperties: type: string @@ -3169,6 +3193,30 @@ spec: Only valid when Type is "entry". The operator mounts CA bundles at /etc/toolhive/ca-bundles//ca.crt. type: string + headerForward: + description: |- + HeaderForward configures per-backend HTTP headers to inject on outbound requests + to this backend. Only valid when Type is "entry". Populated by the operator from + MCPServerEntry.spec.headerForward; secret values are never stored here, only + identifiers that resolve to TOOLHIVE_SECRET_ env vars in the vMCP pod. + properties: + addHeadersFromSecret: + additionalProperties: + type: string + description: |- + AddHeadersFromSecret maps canonical HTTP header name to secret identifier. + The vMCP runtime resolves each identifier via secrets.EnvironmentProvider, + which reads TOOLHIVE_SECRET_ from the pod environment. + type: object + addPlaintextHeaders: + additionalProperties: + type: string + description: |- + AddPlaintextHeaders is a map of canonical HTTP header name to literal value. + WARNING: values are stored in plaintext in the vMCP ConfigMap. + Use AddHeadersFromSecret for sensitive data like API keys or tokens. + type: object + type: object metadata: additionalProperties: type: string diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml index 9929904d14..8e06254fa7 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -885,6 +885,30 @@ spec: Only valid when Type is "entry". The operator mounts CA bundles at /etc/toolhive/ca-bundles//ca.crt. type: string + headerForward: + description: |- + HeaderForward configures per-backend HTTP headers to inject on outbound requests + to this backend. Only valid when Type is "entry". Populated by the operator from + MCPServerEntry.spec.headerForward; secret values are never stored here, only + identifiers that resolve to TOOLHIVE_SECRET_ env vars in the vMCP pod. + properties: + addHeadersFromSecret: + additionalProperties: + type: string + description: |- + AddHeadersFromSecret maps canonical HTTP header name to secret identifier. + The vMCP runtime resolves each identifier via secrets.EnvironmentProvider, + which reads TOOLHIVE_SECRET_ from the pod environment. + type: object + addPlaintextHeaders: + additionalProperties: + type: string + description: |- + AddPlaintextHeaders is a map of canonical HTTP header name to literal value. + WARNING: values are stored in plaintext in the vMCP ConfigMap. + Use AddHeadersFromSecret for sensitive data like API keys or tokens. + type: object + type: object metadata: additionalProperties: type: string @@ -3172,6 +3196,30 @@ spec: Only valid when Type is "entry". The operator mounts CA bundles at /etc/toolhive/ca-bundles//ca.crt. type: string + headerForward: + description: |- + HeaderForward configures per-backend HTTP headers to inject on outbound requests + to this backend. Only valid when Type is "entry". Populated by the operator from + MCPServerEntry.spec.headerForward; secret values are never stored here, only + identifiers that resolve to TOOLHIVE_SECRET_ env vars in the vMCP pod. + properties: + addHeadersFromSecret: + additionalProperties: + type: string + description: |- + AddHeadersFromSecret maps canonical HTTP header name to secret identifier. + The vMCP runtime resolves each identifier via secrets.EnvironmentProvider, + which reads TOOLHIVE_SECRET_ from the pod environment. + type: object + addPlaintextHeaders: + additionalProperties: + type: string + description: |- + AddPlaintextHeaders is a map of canonical HTTP header name to literal value. + WARNING: values are stored in plaintext in the vMCP ConfigMap. + Use AddHeadersFromSecret for sensitive data like API keys or tokens. + type: object + type: object metadata: additionalProperties: type: string diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md index 0fbc8f22fe..5fda686ef0 100644 --- a/docs/operator/crd-api.md +++ b/docs/operator/crd-api.md @@ -7,6 +7,7 @@ - [toolhive.stacklok.dev/telemetry](#toolhivestacklokdevtelemetry) - [toolhive.stacklok.dev/v1alpha1](#toolhivestacklokdevv1alpha1) - [toolhive.stacklok.dev/v1beta1](#toolhivestacklokdevv1beta1) +- [toolhive.stacklok.dev/vmcp](#toolhivestacklokdevvmcp) ## toolhive.stacklok.dev/audit @@ -542,6 +543,7 @@ _Appears in:_ | `transport` _string_ | Transport is the MCP transport protocol: "sse" or "streamable-http"
Only network transports supported by vMCP client are allowed. | | Enum: [sse streamable-http]
Required: \{\}
| | `type` _string_ | Type is the backend workload type: "entry" for MCPServerEntry backends, or empty
for container/proxy backends. Entry backends connect directly to remote MCP servers. | | Enum: [entry ]
Optional: \{\}
| | `caBundlePath` _string_ | CABundlePath is the file path to a custom CA certificate bundle for TLS verification.
Only valid when Type is "entry". The operator mounts CA bundles at
/etc/toolhive/ca-bundles//ca.crt. | | Optional: \{\}
| +| `headerForward` _[pkg.vmcp.HeaderForwardConfig](#pkgvmcpheaderforwardconfig)_ | HeaderForward configures per-backend HTTP headers to inject on outbound requests
to this backend. Only valid when Type is "entry". Populated by the operator from
MCPServerEntry.spec.headerForward; secret values are never stored here, only
identifiers that resolve to TOOLHIVE_SECRET_ env vars in the vMCP pod. | | Optional: \{\}
| | `metadata` _object (keys:string, values:string)_ | Refer to Kubernetes API documentation for fields of `metadata`. | | Optional: \{\}
| @@ -3454,5 +3456,89 @@ _Appears in:_ | --- | --- | --- | --- | | `kind` _string_ | Kind is the type of workload resource | | Enum: [MCPServer VirtualMCPServer MCPRemoteProxy]
Required: \{\}
| | `name` _string_ | Name is the name of the workload resource | | MinLength: 1
Required: \{\}
| + + + +## toolhive.stacklok.dev/vmcp + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +#### pkg.vmcp.HeaderForwardConfig + + + +HeaderForwardConfig configures HTTP headers injected into outbound requests +from the vMCP runtime to a static backend. AddPlaintextHeaders carries literal +values; AddHeadersFromSecret maps header names to secret identifiers resolved +via secrets.EnvironmentProvider at request time (env var TOOLHIVE_SECRET_). + +Secret values MUST NOT appear in this struct — only identifiers. The operator +injects actual Secret values into the vMCP pod as env vars via +valueFrom.secretKeyRef at Deployment rendering time. + + + +_Appears in:_ +- [vmcp.config.StaticBackendConfig](#vmcpconfigstaticbackendconfig) + +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `addPlaintextHeaders` _object (keys:string, values:string)_ | AddPlaintextHeaders is a map of canonical HTTP header name to literal value.
WARNING: values are stored in plaintext in the vMCP ConfigMap.
Use AddHeadersFromSecret for sensitive data like API keys or tokens. | | Optional: \{\}
| +| `addHeadersFromSecret` _object (keys:string, values:string)_ | AddHeadersFromSecret maps canonical HTTP header name to secret identifier.
The vMCP runtime resolves each identifier via secrets.EnvironmentProvider,
which reads TOOLHIVE_SECRET_ from the pod environment. | | Optional: \{\}
| + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/pkg/vmcp/aggregator/discoverer.go b/pkg/vmcp/aggregator/discoverer.go index 4a26b9937a..20df22f5f0 100644 --- a/pkg/vmcp/aggregator/discoverer.go +++ b/pkg/vmcp/aggregator/discoverer.go @@ -272,6 +272,7 @@ func (d *backendDiscoverer) discoverFromStaticConfig() []vmcp.Backend { Type: vmcp.BackendType(staticBackend.Type), CABundlePath: staticBackend.CABundlePath, HealthStatus: vmcp.BackendHealthy, // Assume healthy, actual health check happens later + HeaderForward: staticBackend.HeaderForward, Metadata: staticBackend.Metadata, } diff --git a/pkg/vmcp/client/client.go b/pkg/vmcp/client/client.go index d84018856a..efb4ef6467 100644 --- a/pkg/vmcp/client/client.go +++ b/pkg/vmcp/client/client.go @@ -27,6 +27,7 @@ import ( "go.opentelemetry.io/otel/propagation" "github.com/stacklok/toolhive/pkg/auth" + "github.com/stacklok/toolhive/pkg/secrets" "github.com/stacklok/toolhive/pkg/versions" "github.com/stacklok/toolhive/pkg/vmcp" vmcpauth "github.com/stacklok/toolhive/pkg/vmcp/auth" @@ -64,6 +65,11 @@ type httpBackendClient struct { // registry manages authentication strategies for outgoing requests to backend MCP servers. // Must not be nil - use UnauthenticatedStrategy for no authentication. registry vmcpauth.OutgoingAuthRegistry + + // secretsProvider resolves TOOLHIVE_SECRET_ env vars at client-creation + // time for per-backend header-forward injection. Nil when no backends declare + // headerForward.AddHeadersFromSecret — plaintext-only backends do not require it. + secretsProvider secrets.Provider } // NewHTTPBackendClient creates a new HTTP-based backend client. @@ -80,7 +86,8 @@ func NewHTTPBackendClient(registry vmcpauth.OutgoingAuthRegistry) (vmcp.BackendC } c := &httpBackendClient{ - registry: registry, + registry: registry, + secretsProvider: secrets.NewEnvironmentProvider(), } c.clientFactory = c.defaultClientFactory return c, nil @@ -296,6 +303,14 @@ func (h *httpBackendClient) defaultClientFactory(ctx context.Context, target *vm target: target, } + // Inject per-backend HTTP headers from MCPServerEntry.spec.headerForward. + // Resolves plaintext + secret-backed headers once here; auth (inner) always + // wins over user-supplied headers because it runs after this tripper. + baseTransport, err = buildHeaderForwardTripper(baseTransport, target.HeaderForward, h.secretsProvider, target.WorkloadID) + if err != nil { + return nil, fmt.Errorf("failed to build header-forward transport: %w", err) + } + // Extract identity and health-check marker from context and propagate them to backend // requests. The health-check marker must be carried through to the DELETE request that // mcp-go emits when closing a streamable-HTTP session: mcp-go creates that request with diff --git a/pkg/vmcp/client/header_forward.go b/pkg/vmcp/client/header_forward.go new file mode 100644 index 0000000000..45cedcbd6f --- /dev/null +++ b/pkg/vmcp/client/header_forward.go @@ -0,0 +1,157 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package client + +import ( + "errors" + "fmt" + "log/slog" + "net/http" + + "github.com/stacklok/toolhive/pkg/secrets" + "github.com/stacklok/toolhive/pkg/transport/middleware" + "github.com/stacklok/toolhive/pkg/vmcp" +) + +// headerForwardRoundTripper injects a pre-resolved set of HTTP headers onto every +// outbound request to a specific backend. Headers are resolved once at client +// creation time (plaintext verbatim + secret identifiers looked up via +// secrets.EnvironmentProvider) and stored in an immutable map. The request is +// cloned before mutation so callers retain an untouched request. +// +// This tripper applies to health checks, listTools/listResources/listPrompts, and +// tool/resource/prompt invocations — every HTTP call made by the vMCP backend +// client passes through the same transport chain. +type headerForwardRoundTripper struct { + base http.RoundTripper + headers http.Header // canonicalized header name → single value +} + +// RoundTrip injects the pre-resolved headers onto a clone of the request and +// delegates to the wrapped transport. Headers already present on the request +// are left untouched so inner transports (auth, identity, trace) can always +// override user-supplied values for the same name. +func (h *headerForwardRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + if len(h.headers) == 0 { + return h.base.RoundTrip(req) + } + reqCopy := req.Clone(req.Context()) + if reqCopy.Header == nil { + reqCopy.Header = make(http.Header) + } + for name, values := range h.headers { + if len(values) == 0 { + continue + } + // Do not clobber headers already set by earlier (outer) tripper stages. + // The vMCP runtime is authoritative for identity/trace/auth headers. + if reqCopy.Header.Get(name) != "" { + continue + } + reqCopy.Header.Set(name, values[0]) + } + return h.base.RoundTrip(reqCopy) +} + +// buildHeaderForwardTripper constructs a headerForwardRoundTripper for the +// backend's pre-resolved HeaderForwardConfig. Returns base unchanged when no +// header injection is configured or the effective header set is empty. +// +// Fails loudly (constructor validation, per go-style.md) when a secret identifier +// cannot be resolved through the provider, so a misconfigured backend surfaces +// at pod startup — not as a silent missing-header on every request. +// +// Restricted header names (matching pkg/transport/middleware.RestrictedHeaders) +// are rejected to prevent Host, Content-Length, Authorization, hop-by-hop, and +// X-Forwarded-* spoofing via user-supplied config. +func buildHeaderForwardTripper( + base http.RoundTripper, + cfg *vmcp.HeaderForwardConfig, + provider secrets.Provider, + backendName string, +) (http.RoundTripper, error) { + if cfg == nil { + return base, nil + } + + headers, err := resolveHeaderForward(cfg, provider, backendName) + if err != nil { + return nil, err + } + if len(headers) == 0 { + return base, nil + } + return &headerForwardRoundTripper{base: base, headers: headers}, nil +} + +// resolveHeaderForward merges plaintext headers with secret-resolved headers into +// a single canonicalized http.Header. Plaintext entries are copied verbatim; +// secret identifiers are looked up via the provider (TOOLHIVE_SECRET_). +// +// Returns an error if any header name is in middleware.RestrictedHeaders or if +// any referenced secret identifier is not present in the environment. +func resolveHeaderForward( + cfg *vmcp.HeaderForwardConfig, + provider secrets.Provider, + backendName string, +) (http.Header, error) { + if cfg == nil { + return nil, nil + } + + size := len(cfg.AddPlaintextHeaders) + len(cfg.AddHeadersFromSecret) + if size == 0 { + return nil, nil + } + out := make(http.Header, size) + + for name, value := range cfg.AddPlaintextHeaders { + canonical := http.CanonicalHeaderKey(name) + if middleware.RestrictedHeaders[canonical] { + return nil, fmt.Errorf( + "backend %q: header %q is restricted and cannot be forwarded", + backendName, canonical, + ) + } + out.Set(canonical, value) + } + + if provider == nil && len(cfg.AddHeadersFromSecret) > 0 { + return nil, fmt.Errorf( + "backend %q: secret provider required to resolve %d header secret(s)", + backendName, len(cfg.AddHeadersFromSecret), + ) + } + + for name, identifier := range cfg.AddHeadersFromSecret { + canonical := http.CanonicalHeaderKey(name) + if middleware.RestrictedHeaders[canonical] { + return nil, fmt.Errorf( + "backend %q: header %q is restricted and cannot be forwarded", + backendName, canonical, + ) + } + value, err := provider.GetSecret(nil, identifier) //nolint:staticcheck // provider ignores ctx + if err != nil { + // Do not include the identifier or value in any user-facing error; + // the log is the only place we record the identifier, not the value. + slog.Error("failed to resolve header-forward secret", + "backend", backendName, "header", canonical, "identifier", identifier, + "error", err) + if errors.Is(err, secrets.ErrSecretNotFound) { + return nil, fmt.Errorf( + "backend %q: header %q secret not found in pod environment", + backendName, canonical, + ) + } + return nil, fmt.Errorf( + "backend %q: failed to resolve header %q from secret provider: %w", + backendName, canonical, err, + ) + } + out.Set(canonical, value) + } + + return out, nil +} diff --git a/pkg/vmcp/client/header_forward_test.go b/pkg/vmcp/client/header_forward_test.go new file mode 100644 index 0000000000..ad1079c96f --- /dev/null +++ b/pkg/vmcp/client/header_forward_test.go @@ -0,0 +1,251 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package client + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/stacklok/toolhive/pkg/secrets" + "github.com/stacklok/toolhive/pkg/vmcp" +) + +// mockProvider is an in-memory stand-in for secrets.Provider so tests don't rely +// on process env. It only implements GetSecret; other methods return errors. +type mockProvider struct { + values map[string]string +} + +func (m *mockProvider) GetSecret(_ context.Context, name string) (string, error) { + if v, ok := m.values[name]; ok { + return v, nil + } + return "", secrets.ErrSecretNotFound +} + +// mockProvider needs to satisfy secrets.Provider; the other methods are unused. +func (*mockProvider) SetSecret(_ context.Context, _, _ string) error { return nil } +func (*mockProvider) DeleteSecret(_ context.Context, _ string) error { return nil } +func (*mockProvider) ListSecrets(_ context.Context) ([]secrets.SecretDescription, error) { + return nil, nil +} +func (*mockProvider) DeleteSecrets(_ context.Context, _ []string) error { return nil } +func (*mockProvider) Cleanup() error { return nil } +func (*mockProvider) Capabilities() secrets.ProviderCapabilities { + return secrets.ProviderCapabilities{CanRead: true} +} + +// captureTripper records the headers seen on the last request that reached it. +type captureTripper struct { + lastHeaders http.Header +} + +func (c *captureTripper) RoundTrip(req *http.Request) (*http.Response, error) { + c.lastHeaders = req.Header.Clone() + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader("")), + Header: make(http.Header), + }, nil +} + +func TestHeaderForwardRoundTripper_InjectsPlaintext(t *testing.T) { + t.Parallel() + + base := &captureTripper{} + rt := &headerForwardRoundTripper{ + base: base, + headers: http.Header{ + "X-Mcp-Toolsets": []string{"projects,issues"}, + "X-Tenant": []string{"acme"}, + }, + } + + req, err := http.NewRequest(http.MethodGet, "https://example.com", http.NoBody) + require.NoError(t, err) + + _, err = rt.RoundTrip(req) + require.NoError(t, err) + + assert.Equal(t, "projects,issues", base.lastHeaders.Get("X-Mcp-Toolsets")) + assert.Equal(t, "acme", base.lastHeaders.Get("X-Tenant")) + // Original request must remain unmutated. + assert.Empty(t, req.Header.Get("X-Mcp-Toolsets")) +} + +func TestHeaderForwardRoundTripper_NoHeadersIsNoOp(t *testing.T) { + t.Parallel() + + base := &captureTripper{} + rt := &headerForwardRoundTripper{base: base, headers: nil} + + req, err := http.NewRequest(http.MethodGet, "https://example.com", http.NoBody) + require.NoError(t, err) + + _, err = rt.RoundTrip(req) + require.NoError(t, err) + assert.Empty(t, base.lastHeaders.Get("X-Mcp-Toolsets")) +} + +func TestHeaderForwardRoundTripper_DoesNotClobberExistingHeader(t *testing.T) { + t.Parallel() + + base := &captureTripper{} + rt := &headerForwardRoundTripper{ + base: base, + headers: http.Header{ + "Authorization": []string{"Bearer user-provided"}, + }, + } + + req, err := http.NewRequest(http.MethodGet, "https://example.com", http.NoBody) + require.NoError(t, err) + // Pre-set the header as an outer (identity/trace/auth) tripper would. + req.Header.Set("Authorization", "Bearer real-auth") + + _, err = rt.RoundTrip(req) + require.NoError(t, err) + + assert.Equal(t, "Bearer real-auth", base.lastHeaders.Get("Authorization"), + "user-supplied header-forward value must not clobber an already-set header") +} + +func TestResolveHeaderForward_PlaintextAndSecretMerged(t *testing.T) { + t.Parallel() + + cfg := &vmcp.HeaderForwardConfig{ + AddPlaintextHeaders: map[string]string{ + "X-Tenant": "acme", + }, + AddHeadersFromSecret: map[string]string{ + "X-API-Key": "HEADER_FORWARD_X_API_KEY_BACKEND_A", + }, + } + provider := &mockProvider{values: map[string]string{ + "HEADER_FORWARD_X_API_KEY_BACKEND_A": "resolved-from-env", + }} + + hdr, err := resolveHeaderForward(cfg, provider, "backend-a") + require.NoError(t, err) + assert.Equal(t, "acme", hdr.Get("X-Tenant")) + assert.Equal(t, "resolved-from-env", hdr.Get("X-Api-Key")) +} + +func TestResolveHeaderForward_SecretNotFoundReturnsError(t *testing.T) { + t.Parallel() + + cfg := &vmcp.HeaderForwardConfig{ + AddHeadersFromSecret: map[string]string{ + "X-API-Key": "HEADER_FORWARD_X_API_KEY_BACKEND_A", + }, + } + provider := &mockProvider{values: map[string]string{}} + + hdr, err := resolveHeaderForward(cfg, provider, "backend-a") + require.Error(t, err) + assert.Nil(t, hdr) + // Error must not leak the identifier or the backend's private values. + assert.NotContains(t, err.Error(), "HEADER_FORWARD_X_API_KEY_BACKEND_A") +} + +func TestResolveHeaderForward_RestrictedHeaderRejected(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + cfg *vmcp.HeaderForwardConfig + }{ + { + name: "plaintext Host rejected", + cfg: &vmcp.HeaderForwardConfig{ + AddPlaintextHeaders: map[string]string{"Host": "attacker.example"}, + }, + }, + { + name: "plaintext Authorization rejected via secret not tested here", + cfg: &vmcp.HeaderForwardConfig{ + AddPlaintextHeaders: map[string]string{"Transfer-Encoding": "chunked"}, + }, + }, + { + name: "secret-backed X-Forwarded-For rejected", + cfg: &vmcp.HeaderForwardConfig{ + AddHeadersFromSecret: map[string]string{ + "X-Forwarded-For": "HEADER_FORWARD_X_FORWARDED_FOR_X", + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + _, err := resolveHeaderForward(tt.cfg, &mockProvider{}, "backend-x") + require.Error(t, err) + assert.Contains(t, err.Error(), "restricted") + }) + } +} + +func TestResolveHeaderForward_NilCfgReturnsNil(t *testing.T) { + t.Parallel() + hdr, err := resolveHeaderForward(nil, nil, "x") + require.NoError(t, err) + assert.Nil(t, hdr) +} + +func TestBuildHeaderForwardTripper_NilCfgReturnsBase(t *testing.T) { + t.Parallel() + base := &captureTripper{} + got, err := buildHeaderForwardTripper(base, nil, nil, "x") + require.NoError(t, err) + assert.Same(t, base, got, "nil cfg must pass base through untouched") +} + +// TestHeaderForwardRoundTripper_EndToEndHTTPTestServer exercises the tripper +// against a real httptest.Server to verify the header reaches a live receiver, +// catching any Clone/Set regressions. +func TestHeaderForwardRoundTripper_EndToEndHTTPTestServer(t *testing.T) { + t.Parallel() + + var receivedToolsets string + var receivedAPIKey string + server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + receivedToolsets = r.Header.Get("X-MCP-Toolsets") + receivedAPIKey = r.Header.Get("X-API-Key") + })) + t.Cleanup(server.Close) + + cfg := &vmcp.HeaderForwardConfig{ + AddPlaintextHeaders: map[string]string{ + "X-MCP-Toolsets": "projects,issues,pull_requests", + }, + AddHeadersFromSecret: map[string]string{ + "X-API-Key": "HEADER_FORWARD_X_API_KEY_BACKEND_E", + }, + } + provider := &mockProvider{values: map[string]string{ + "HEADER_FORWARD_X_API_KEY_BACKEND_E": "secret-token-value", + }} + + rt, err := buildHeaderForwardTripper(http.DefaultTransport, cfg, provider, "backend-e") + require.NoError(t, err) + client := &http.Client{Transport: rt} + + req, err := http.NewRequest(http.MethodGet, server.URL, http.NoBody) + require.NoError(t, err) + resp, err := client.Do(req) + require.NoError(t, err) + t.Cleanup(func() { _ = resp.Body.Close() }) + + assert.Equal(t, "projects,issues,pull_requests", receivedToolsets) + assert.Equal(t, "secret-token-value", receivedAPIKey) +} diff --git a/pkg/vmcp/config/config.go b/pkg/vmcp/config/config.go index 7a2c699290..dfc15b8f68 100644 --- a/pkg/vmcp/config/config.go +++ b/pkg/vmcp/config/config.go @@ -304,6 +304,13 @@ type StaticBackendConfig struct { // +optional CABundlePath string `json:"caBundlePath,omitempty" yaml:"caBundlePath,omitempty"` + // HeaderForward configures per-backend HTTP headers to inject on outbound requests + // to this backend. Only valid when Type is "entry". Populated by the operator from + // MCPServerEntry.spec.headerForward; secret values are never stored here, only + // identifiers that resolve to TOOLHIVE_SECRET_ env vars in the vMCP pod. + // +optional + HeaderForward *vmcp.HeaderForwardConfig `json:"headerForward,omitempty" yaml:"headerForward,omitempty"` + // Metadata is a custom key-value map for storing additional backend information // such as labels, tags, or other arbitrary data (e.g., "env": "prod", "region": "us-east-1"). // This is NOT Kubernetes ObjectMeta - it's a simple string map for user-defined metadata. diff --git a/pkg/vmcp/config/zz_generated.deepcopy.go b/pkg/vmcp/config/zz_generated.deepcopy.go index 80861bff11..1f96e571e4 100644 --- a/pkg/vmcp/config/zz_generated.deepcopy.go +++ b/pkg/vmcp/config/zz_generated.deepcopy.go @@ -23,6 +23,7 @@ package config import ( "github.com/stacklok/toolhive/pkg/audit" "github.com/stacklok/toolhive/pkg/telemetry" + "github.com/stacklok/toolhive/pkg/vmcp" "github.com/stacklok/toolhive/pkg/vmcp/auth/types" ) @@ -460,6 +461,11 @@ func (in *SessionStorageConfig) DeepCopy() *SessionStorageConfig { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *StaticBackendConfig) DeepCopyInto(out *StaticBackendConfig) { *out = *in + if in.HeaderForward != nil { + in, out := &in.HeaderForward, &out.HeaderForward + *out = new(vmcp.HeaderForwardConfig) + (*in).DeepCopyInto(*out) + } if in.Metadata != nil { in, out := &in.Metadata, &out.Metadata *out = make(map[string]string, len(*in)) diff --git a/pkg/vmcp/doc.go b/pkg/vmcp/doc.go index 449f5a9213..69b13a67a8 100644 --- a/pkg/vmcp/doc.go +++ b/pkg/vmcp/doc.go @@ -155,4 +155,6 @@ // - MCP Specification: https://modelcontextprotocol.io/specification // // See individual subpackage documentation for detailed usage and examples. +// +// +groupName=toolhive.stacklok.dev package vmcp diff --git a/pkg/vmcp/health/monitor.go b/pkg/vmcp/health/monitor.go index 75bfd085c5..9608867614 100644 --- a/pkg/vmcp/health/monitor.go +++ b/pkg/vmcp/health/monitor.go @@ -427,13 +427,19 @@ func (m *Monitor) performHealthCheck(ctx context.Context, backend *vmcp.Backend) return } - // Create BackendTarget from Backend + // Create BackendTarget from Backend. Carry CA bundle and header-forward config + // so health checks hit the backend with the same TLS trust and header injection + // as list/call requests — otherwise a healthy-to-the-monitor backend could fail + // for real traffic (or vice versa). target := &vmcp.BackendTarget{ WorkloadID: backend.ID, WorkloadName: backend.Name, BaseURL: backend.BaseURL, TransportType: backend.TransportType, + CABundlePath: backend.CABundlePath, + CABundleData: backend.CABundleData, AuthConfig: backend.AuthConfig, + HeaderForward: backend.HeaderForward, HealthStatus: vmcp.BackendUnknown, // Status is determined by the health check Metadata: backend.Metadata, } diff --git a/pkg/vmcp/registry.go b/pkg/vmcp/registry.go index ba9f2c6b81..ccce6e321c 100644 --- a/pkg/vmcp/registry.go +++ b/pkg/vmcp/registry.go @@ -373,6 +373,7 @@ func BackendToTarget(backend *Backend) *BackendTarget { AuthConfig: backend.AuthConfig, SessionAffinity: false, // TODO: Add session affinity support in future phases HealthStatus: backend.HealthStatus, + HeaderForward: backend.HeaderForward, Metadata: backend.Metadata, } } diff --git a/pkg/vmcp/types.go b/pkg/vmcp/types.go index 0418ce5e15..9c8c257ffa 100644 --- a/pkg/vmcp/types.go +++ b/pkg/vmcp/types.go @@ -15,6 +15,30 @@ import ( // This file contains shared domain types used across multiple vmcp subpackages. // Following DDD principles, these are core domain concepts that cross bounded contexts. +// HeaderForwardConfig configures HTTP headers injected into outbound requests +// from the vMCP runtime to a static backend. AddPlaintextHeaders carries literal +// values; AddHeadersFromSecret maps header names to secret identifiers resolved +// via secrets.EnvironmentProvider at request time (env var TOOLHIVE_SECRET_). +// +// Secret values MUST NOT appear in this struct — only identifiers. The operator +// injects actual Secret values into the vMCP pod as env vars via +// valueFrom.secretKeyRef at Deployment rendering time. +// +gendoc +// +kubebuilder:object:generate=true +type HeaderForwardConfig struct { + // AddPlaintextHeaders is a map of canonical HTTP header name to literal value. + // WARNING: values are stored in plaintext in the vMCP ConfigMap. + // Use AddHeadersFromSecret for sensitive data like API keys or tokens. + // +optional + AddPlaintextHeaders map[string]string `json:"addPlaintextHeaders,omitempty" yaml:"addPlaintextHeaders,omitempty"` + + // AddHeadersFromSecret maps canonical HTTP header name to secret identifier. + // The vMCP runtime resolves each identifier via secrets.EnvironmentProvider, + // which reads TOOLHIVE_SECRET_ from the pod environment. + // +optional + AddHeadersFromSecret map[string]string `json:"addHeadersFromSecret,omitempty" yaml:"addHeadersFromSecret,omitempty"` +} + // BackendTarget identifies a specific backend workload and provides // the information needed to forward requests to it. type BackendTarget struct { @@ -78,6 +102,11 @@ type BackendTarget struct { // HealthStatus indicates the current health of the backend. HealthStatus BackendHealthStatus + // HeaderForward carries per-backend HTTP header injection configuration + // applied by the vMCP client to every outbound request targeting this backend + // (list, call, health-check). Nil when no headers are configured. + HeaderForward *HeaderForwardConfig + // Metadata stores additional backend-specific information. Metadata map[string]string } @@ -319,6 +348,11 @@ type Backend struct { // +optional AuthConfigRef string + // HeaderForward carries per-backend HTTP header injection configuration. + // Only populated for entry-type backends whose MCPServerEntry declares + // spec.headerForward. Nil when the entry has no header forwarding configured. + HeaderForward *HeaderForwardConfig + // Metadata stores additional backend information. Metadata map[string]string } diff --git a/pkg/vmcp/workloads/k8s.go b/pkg/vmcp/workloads/k8s.go index 7a8ffa557e..8202a08c04 100644 --- a/pkg/vmcp/workloads/k8s.go +++ b/pkg/vmcp/workloads/k8s.go @@ -18,6 +18,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" + ctrlutil "github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil" "github.com/stacklok/toolhive/pkg/k8s" transporttypes "github.com/stacklok/toolhive/pkg/transport/types" "github.com/stacklok/toolhive/pkg/vmcp" @@ -582,9 +583,51 @@ func (d *k8sDiscoverer) mcpServerEntryToBackend(ctx context.Context, entry *mcpv return nil } + // Extract header forward configuration. Plaintext headers are copied verbatim. + // Secret-backed headers are mapped to secret identifiers resolved at runtime via + // secrets.EnvironmentProvider (env var TOOLHIVE_SECRET_). The operator + // injects the actual Secret value into the vMCP pod via valueFrom.secretKeyRef. + backend.HeaderForward = buildHeaderForwardFromEntry(entry) + return backend } +// buildHeaderForwardFromEntry converts MCPServerEntry.spec.headerForward into the +// runtime vmcp.HeaderForwardConfig. Returns nil if the entry declares no header +// forwarding. Secret values are never read or stored here — only identifiers. +func buildHeaderForwardFromEntry(entry *mcpv1beta1.MCPServerEntry) *vmcp.HeaderForwardConfig { + if entry.Spec.HeaderForward == nil { + return nil + } + src := entry.Spec.HeaderForward + + var plaintext map[string]string + if len(src.AddPlaintextHeaders) > 0 { + plaintext = make(map[string]string, len(src.AddPlaintextHeaders)) + maps.Copy(plaintext, src.AddPlaintextHeaders) + } + + var secretIdents map[string]string + if len(src.AddHeadersFromSecret) > 0 { + secretIdents = make(map[string]string, len(src.AddHeadersFromSecret)) + for _, ref := range src.AddHeadersFromSecret { + if ref.ValueSecretRef == nil { + continue + } + _, identifier := ctrlutil.GenerateHeaderForwardSecretEnvVarName(entry.Name, ref.HeaderName) + secretIdents[ref.HeaderName] = identifier + } + } + + if plaintext == nil && secretIdents == nil { + return nil + } + return &vmcp.HeaderForwardConfig{ + AddPlaintextHeaders: plaintext, + AddHeadersFromSecret: secretIdents, + } +} + // mapMCPServerEntryPhaseToHealth converts a MCPServerEntryPhase to a backend health status. func mapMCPServerEntryPhaseToHealth(phase mcpv1beta1.MCPServerEntryPhase) vmcp.BackendHealthStatus { switch phase { diff --git a/pkg/vmcp/workloads/k8s_test.go b/pkg/vmcp/workloads/k8s_test.go index edae76e868..8ca1f3b0c5 100644 --- a/pkg/vmcp/workloads/k8s_test.go +++ b/pkg/vmcp/workloads/k8s_test.go @@ -5,6 +5,7 @@ package workloads import ( "context" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -1829,3 +1830,165 @@ func TestFetchCABundleData(t *testing.T) { }) } } + +func TestBuildHeaderForwardFromEntry(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + entry *mcpv1beta1.MCPServerEntry + want *vmcp.HeaderForwardConfig + }{ + { + name: "nil when headerForward absent", + entry: &mcpv1beta1.MCPServerEntry{ + ObjectMeta: metav1.ObjectMeta{Name: "entry-x"}, + Spec: mcpv1beta1.MCPServerEntrySpec{}, + }, + want: nil, + }, + { + name: "plaintext only", + entry: &mcpv1beta1.MCPServerEntry{ + ObjectMeta: metav1.ObjectMeta{Name: "github-copilot"}, + Spec: mcpv1beta1.MCPServerEntrySpec{ + HeaderForward: &mcpv1beta1.HeaderForwardConfig{ + AddPlaintextHeaders: map[string]string{ + "X-MCP-Toolsets": "projects,issues", + "X-Tenant": "acme", + }, + }, + }, + }, + want: &vmcp.HeaderForwardConfig{ + AddPlaintextHeaders: map[string]string{ + "X-MCP-Toolsets": "projects,issues", + "X-Tenant": "acme", + }, + }, + }, + { + name: "secret refs mapped to identifiers scoped by entry name", + entry: &mcpv1beta1.MCPServerEntry{ + ObjectMeta: metav1.ObjectMeta{Name: "gh-copilot"}, + Spec: mcpv1beta1.MCPServerEntrySpec{ + HeaderForward: &mcpv1beta1.HeaderForwardConfig{ + AddHeadersFromSecret: []mcpv1beta1.HeaderFromSecret{ + { + HeaderName: "X-API-Key", + ValueSecretRef: &mcpv1beta1.SecretKeyRef{Name: "api-key-secret", Key: "token"}, + }, + { + HeaderName: "Authorization", + ValueSecretRef: &mcpv1beta1.SecretKeyRef{Name: "auth-secret", Key: "value"}, + }, + }, + }, + }, + }, + want: &vmcp.HeaderForwardConfig{ + AddHeadersFromSecret: map[string]string{ + "X-API-Key": "HEADER_FORWARD_X_API_KEY_GH_COPILOT", + "Authorization": "HEADER_FORWARD_AUTHORIZATION_GH_COPILOT", + }, + }, + }, + { + name: "mixed plaintext and secret", + entry: &mcpv1beta1.MCPServerEntry{ + ObjectMeta: metav1.ObjectMeta{Name: "entry-mix"}, + Spec: mcpv1beta1.MCPServerEntrySpec{ + HeaderForward: &mcpv1beta1.HeaderForwardConfig{ + AddPlaintextHeaders: map[string]string{"X-Tenant": "acme"}, + AddHeadersFromSecret: []mcpv1beta1.HeaderFromSecret{ + { + HeaderName: "X-API-Key", + ValueSecretRef: &mcpv1beta1.SecretKeyRef{Name: "s", Key: "k"}, + }, + }, + }, + }, + }, + want: &vmcp.HeaderForwardConfig{ + AddPlaintextHeaders: map[string]string{"X-Tenant": "acme"}, + AddHeadersFromSecret: map[string]string{ + "X-API-Key": "HEADER_FORWARD_X_API_KEY_ENTRY_MIX", + }, + }, + }, + { + name: "secret ref without ValueSecretRef is skipped", + entry: &mcpv1beta1.MCPServerEntry{ + ObjectMeta: metav1.ObjectMeta{Name: "entry-partial"}, + Spec: mcpv1beta1.MCPServerEntrySpec{ + HeaderForward: &mcpv1beta1.HeaderForwardConfig{ + AddHeadersFromSecret: []mcpv1beta1.HeaderFromSecret{ + {HeaderName: "X-Bad", ValueSecretRef: nil}, + { + HeaderName: "X-Good", + ValueSecretRef: &mcpv1beta1.SecretKeyRef{Name: "s", Key: "k"}, + }, + }, + }, + }, + }, + want: &vmcp.HeaderForwardConfig{ + AddHeadersFromSecret: map[string]string{ + "X-Good": "HEADER_FORWARD_X_GOOD_ENTRY_PARTIAL", + }, + }, + }, + { + name: "empty headerForward maps returns nil", + entry: &mcpv1beta1.MCPServerEntry{ + ObjectMeta: metav1.ObjectMeta{Name: "entry-empty"}, + Spec: mcpv1beta1.MCPServerEntrySpec{ + HeaderForward: &mcpv1beta1.HeaderForwardConfig{}, + }, + }, + want: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := buildHeaderForwardFromEntry(tt.entry) + assert.Equal(t, tt.want, got) + }) + } +} + +// TestBuildHeaderForwardFromEntry_NoSecretValuesLeaked asserts that no value in +// AddHeadersFromSecret contains anything that looks like a Secret key or name — +// only the HEADER_FORWARD_ identifier prefix. Secret values must be resolved at +// runtime via the env, not carried through the static config. +func TestBuildHeaderForwardFromEntry_NoSecretValuesLeaked(t *testing.T) { + t.Parallel() + + entry := &mcpv1beta1.MCPServerEntry{ + ObjectMeta: metav1.ObjectMeta{Name: "leak-check"}, + Spec: mcpv1beta1.MCPServerEntrySpec{ + HeaderForward: &mcpv1beta1.HeaderForwardConfig{ + AddHeadersFromSecret: []mcpv1beta1.HeaderFromSecret{ + { + HeaderName: "X-API-Key", + ValueSecretRef: &mcpv1beta1.SecretKeyRef{ + Name: "super-secret-kubernetes-secret", + Key: "do-not-leak-this-key", + }, + }, + }, + }, + }, + } + + got := buildHeaderForwardFromEntry(entry) + require.NotNil(t, got) + for header, ident := range got.AddHeadersFromSecret { + assert.NotContains(t, ident, "super-secret", "header %q leaked Secret name", header) + assert.NotContains(t, ident, "do-not-leak", "header %q leaked Secret key", header) + assert.True(t, strings.HasPrefix(ident, "HEADER_FORWARD_"), + "header %q identifier %q must carry HEADER_FORWARD_ prefix", header, ident) + } +} diff --git a/pkg/vmcp/zz_generated.deepcopy.go b/pkg/vmcp/zz_generated.deepcopy.go new file mode 100644 index 0000000000..e7e9c43af0 --- /dev/null +++ b/pkg/vmcp/zz_generated.deepcopy.go @@ -0,0 +1,52 @@ +//go:build !ignore_autogenerated + +/* +Copyright 2025 Stacklok + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by controller-gen. DO NOT EDIT. + +package vmcp + +import () + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *HeaderForwardConfig) DeepCopyInto(out *HeaderForwardConfig) { + *out = *in + if in.AddPlaintextHeaders != nil { + in, out := &in.AddPlaintextHeaders, &out.AddPlaintextHeaders + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + if in.AddHeadersFromSecret != nil { + in, out := &in.AddHeadersFromSecret, &out.AddHeadersFromSecret + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HeaderForwardConfig. +func (in *HeaderForwardConfig) DeepCopy() *HeaderForwardConfig { + if in == nil { + return nil + } + out := new(HeaderForwardConfig) + in.DeepCopyInto(out) + return out +}