diff --git a/cmd/thv-operator/api/v1beta1/virtualmcpserver_types.go b/cmd/thv-operator/api/v1beta1/virtualmcpserver_types.go index 58ef7ffffa..41666e024f 100644 --- a/cmd/thv-operator/api/v1beta1/virtualmcpserver_types.go +++ b/cmd/thv-operator/api/v1beta1/virtualmcpserver_types.go @@ -37,6 +37,15 @@ type VirtualMCPServerSpec struct { // +optional OutgoingAuth *OutgoingAuthConfig `json:"outgoingAuth,omitempty"` + // PassthroughHeaders is an allowlist of incoming client request header names + // forwarded verbatim to all backends (e.g. an API key the backend resolves to + // a user). Takes precedence over config.PassthroughHeaders. Names must not be + // restricted headers (Host, hop-by-hop, X-Forwarded-*). Forwarded headers are + // attacker-influenceable unless a trusted upstream sets them. + // +optional + // +listType=atomic + PassthroughHeaders []string `json:"passthroughHeaders,omitempty"` + // ServiceType specifies the Kubernetes service type for the Virtual MCP server // +kubebuilder:validation:Enum=ClusterIP;NodePort;LoadBalancer // +kubebuilder:default=ClusterIP diff --git a/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go b/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go index 2cb12d6c3c..14a4e22dcd 100644 --- a/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go +++ b/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go @@ -3188,6 +3188,11 @@ func (in *VirtualMCPServerSpec) DeepCopyInto(out *VirtualMCPServerSpec) { *out = new(OutgoingAuthConfig) (*in).DeepCopyInto(*out) } + if in.PassthroughHeaders != nil { + in, out := &in.PassthroughHeaders, &out.PassthroughHeaders + *out = make([]string, len(*in)) + copy(*out, *in) + } if in.ServiceAccount != nil { in, out := &in.ServiceAccount, &out.ServiceAccount *out = new(string) diff --git a/cmd/thv-operator/pkg/vmcpconfig/converter.go b/cmd/thv-operator/pkg/vmcpconfig/converter.go index 0f96878b67..03ad85f9fb 100644 --- a/cmd/thv-operator/pkg/vmcpconfig/converter.go +++ b/cmd/thv-operator/pkg/vmcpconfig/converter.go @@ -85,6 +85,11 @@ func (c *Converter) Convert( // without requiring explicit mapping in this converter. config := vmcp.Spec.Config.DeepCopy() + // Promoted top-level field takes precedence over spec.config.passthroughHeaders. + if len(vmcp.Spec.PassthroughHeaders) > 0 { + config.PassthroughHeaders = vmcp.Spec.PassthroughHeaders + } + // Override name with the CR name (authoritative source) config.Name = vmcp.Name diff --git a/cmd/thv-operator/pkg/vmcpconfig/converter_test.go b/cmd/thv-operator/pkg/vmcpconfig/converter_test.go index 52efe5c42d..042af24a60 100644 --- a/cmd/thv-operator/pkg/vmcpconfig/converter_test.go +++ b/cmd/thv-operator/pkg/vmcpconfig/converter_test.go @@ -2196,3 +2196,47 @@ func TestConvertIncomingAuth_PrimaryUpstreamProvider(t *testing.T) { }) } } + +// TestConverter_PassthroughHeaders verifies that spec.passthroughHeaders is promoted +// correctly, takes precedence over spec.config.passthroughHeaders, and that the +// auto-passthrough path (only spec.config.passthroughHeaders set) is preserved. +func TestConverter_PassthroughHeaders(t *testing.T) { + t.Parallel() + + // newVMCP builds a minimal VirtualMCPServer with the given passthrough header slices. + newVMCP := func(topLevel, configLevel []string) *mcpv1beta1.VirtualMCPServer { + return &mcpv1beta1.VirtualMCPServer{ + ObjectMeta: metav1.ObjectMeta{Name: "test-vmcp", Namespace: "default"}, + Spec: mcpv1beta1.VirtualMCPServerSpec{ + GroupRef: &mcpv1beta1.MCPGroupRef{Name: "test-group"}, + IncomingAuth: &mcpv1beta1.IncomingAuthConfig{Type: "anonymous"}, + PassthroughHeaders: topLevel, + Config: vmcpconfig.Config{PassthroughHeaders: configLevel}, + }, + } + } + + tests := []struct { + name string + topLevel []string // spec.passthroughHeaders + config []string // spec.config.passthroughHeaders + want []string + }{ + {name: "top-level only sets headers", topLevel: []string{"x-env"}, want: []string{"x-env"}}, + {name: "top-level wins over config when both set", topLevel: []string{"x-api-key"}, config: []string{"x-tenant"}, want: []string{"x-api-key"}}, + {name: "auto-passthrough: only config-level preserves value", config: []string{"x-tenant"}, want: []string{"x-tenant"}}, + {name: "neither set produces nil"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + converter := newTestConverter(t, newNoOpMockResolver(t)) + ctx := log.IntoContext(context.Background(), logr.Discard()) + config, _, err := converter.Convert(ctx, newVMCP(tt.topLevel, tt.config), nil) + require.NoError(t, err) + require.NotNil(t, config) + assert.Equal(t, tt.want, config.PassthroughHeaders) + }) + } +} 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 178d3b60c6..d038549c06 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 @@ -2130,6 +2130,17 @@ spec: required: - source type: object + passthroughHeaders: + description: |- + PassthroughHeaders is an allowlist of incoming client request header names + forwarded verbatim to all backends. Captured at the vMCP incoming edge by + headerforward.CaptureMiddleware and consumed once at session creation + when the per-session backend client's HeaderForwardConfig is built. Names + must not be in the restricted set (Host, hop-by-hop, X-Forwarded-*, etc.). + items: + type: string + type: array + x-kubernetes-list-type: atomic rateLimiting: description: |- RateLimiting defines rate limiting configuration for the Virtual MCP server. @@ -2723,6 +2734,17 @@ spec: - inline type: string type: object + passthroughHeaders: + description: |- + PassthroughHeaders is an allowlist of incoming client request header names + forwarded verbatim to all backends (e.g. an API key the backend resolves to + a user). Takes precedence over config.PassthroughHeaders. Names must not be + restricted headers (Host, hop-by-hop, X-Forwarded-*). Forwarded headers are + attacker-influenceable unless a trusted upstream sets them. + items: + type: string + type: array + x-kubernetes-list-type: atomic podTemplateSpec: description: |- PodTemplateSpec defines the pod template to use for the Virtual MCP server @@ -5125,6 +5147,17 @@ spec: required: - source type: object + passthroughHeaders: + description: |- + PassthroughHeaders is an allowlist of incoming client request header names + forwarded verbatim to all backends. Captured at the vMCP incoming edge by + headerforward.CaptureMiddleware and consumed once at session creation + when the per-session backend client's HeaderForwardConfig is built. Names + must not be in the restricted set (Host, hop-by-hop, X-Forwarded-*, etc.). + items: + type: string + type: array + x-kubernetes-list-type: atomic rateLimiting: description: |- RateLimiting defines rate limiting configuration for the Virtual MCP server. @@ -5718,6 +5751,17 @@ spec: - inline type: string type: object + passthroughHeaders: + description: |- + PassthroughHeaders is an allowlist of incoming client request header names + forwarded verbatim to all backends (e.g. an API key the backend resolves to + a user). Takes precedence over config.PassthroughHeaders. Names must not be + restricted headers (Host, hop-by-hop, X-Forwarded-*). Forwarded headers are + attacker-influenceable unless a trusted upstream sets them. + items: + type: string + type: array + x-kubernetes-list-type: atomic podTemplateSpec: description: |- PodTemplateSpec defines the pod template to use for the Virtual MCP server 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 59ab53de26..1905b38ebb 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -2133,6 +2133,17 @@ spec: required: - source type: object + passthroughHeaders: + description: |- + PassthroughHeaders is an allowlist of incoming client request header names + forwarded verbatim to all backends. Captured at the vMCP incoming edge by + headerforward.CaptureMiddleware and consumed once at session creation + when the per-session backend client's HeaderForwardConfig is built. Names + must not be in the restricted set (Host, hop-by-hop, X-Forwarded-*, etc.). + items: + type: string + type: array + x-kubernetes-list-type: atomic rateLimiting: description: |- RateLimiting defines rate limiting configuration for the Virtual MCP server. @@ -2726,6 +2737,17 @@ spec: - inline type: string type: object + passthroughHeaders: + description: |- + PassthroughHeaders is an allowlist of incoming client request header names + forwarded verbatim to all backends (e.g. an API key the backend resolves to + a user). Takes precedence over config.PassthroughHeaders. Names must not be + restricted headers (Host, hop-by-hop, X-Forwarded-*). Forwarded headers are + attacker-influenceable unless a trusted upstream sets them. + items: + type: string + type: array + x-kubernetes-list-type: atomic podTemplateSpec: description: |- PodTemplateSpec defines the pod template to use for the Virtual MCP server @@ -5128,6 +5150,17 @@ spec: required: - source type: object + passthroughHeaders: + description: |- + PassthroughHeaders is an allowlist of incoming client request header names + forwarded verbatim to all backends. Captured at the vMCP incoming edge by + headerforward.CaptureMiddleware and consumed once at session creation + when the per-session backend client's HeaderForwardConfig is built. Names + must not be in the restricted set (Host, hop-by-hop, X-Forwarded-*, etc.). + items: + type: string + type: array + x-kubernetes-list-type: atomic rateLimiting: description: |- RateLimiting defines rate limiting configuration for the Virtual MCP server. @@ -5721,6 +5754,17 @@ spec: - inline type: string type: object + passthroughHeaders: + description: |- + PassthroughHeaders is an allowlist of incoming client request header names + forwarded verbatim to all backends (e.g. an API key the backend resolves to + a user). Takes precedence over config.PassthroughHeaders. Names must not be + restricted headers (Host, hop-by-hop, X-Forwarded-*). Forwarded headers are + attacker-influenceable unless a trusted upstream sets them. + items: + type: string + type: array + x-kubernetes-list-type: atomic podTemplateSpec: description: |- PodTemplateSpec defines the pod template to use for the Virtual MCP server diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md index 32c720ce85..f55c8d4384 100644 --- a/docs/operator/crd-api.md +++ b/docs/operator/crd-api.md @@ -329,6 +329,7 @@ _Appears in:_ | `optimizer` _[vmcp.config.OptimizerConfig](#vmcpconfigoptimizerconfig)_ | Optimizer configures the MCP optimizer for context optimization on large toolsets.
When enabled, vMCP exposes only find_tool and call_tool operations to clients
instead of all backend tools directly. This reduces token usage by allowing
LLMs to discover relevant tools on demand rather than receiving all tool definitions. | | Optional: \{\}
| | `sessionStorage` _[vmcp.config.SessionStorageConfig](#vmcpconfigsessionstorageconfig)_ | SessionStorage configures session storage for stateful horizontal scaling.
When provider is "redis", the operator injects Redis connection parameters
(address, db, keyPrefix) here. The Redis password is provided separately via
the THV_SESSION_REDIS_PASSWORD environment variable. | | Optional: \{\}
| | `rateLimiting` _[ratelimit.types.RateLimitConfig](#ratelimittypesratelimitconfig)_ | RateLimiting defines rate limiting configuration for the Virtual MCP server.
Requires Redis session storage to be configured for distributed rate limiting. | | Optional: \{\}
| +| `passthroughHeaders` _string array_ | PassthroughHeaders is an allowlist of incoming client request header names
forwarded verbatim to all backends. Captured at the vMCP incoming edge by
headerforward.CaptureMiddleware and consumed once at session creation
when the per-session backend client's HeaderForwardConfig is built. Names
must not be in the restricted set (Host, hop-by-hop, X-Forwarded-*, etc.). | | Optional: \{\}
| #### vmcp.config.ConflictResolutionConfig @@ -3810,6 +3811,7 @@ _Appears in:_ | --- | --- | --- | --- | | `incomingAuth` _[api.v1beta1.IncomingAuthConfig](#apiv1beta1incomingauthconfig)_ | IncomingAuth configures authentication for clients connecting to the Virtual MCP server.
Must be explicitly set - use "anonymous" type when no authentication is required.
This field takes precedence over config.IncomingAuth and should be preferred because it
supports Kubernetes-native secret references (SecretKeyRef, ConfigMapRef) for secure
dynamic discovery of credentials, rather than requiring secrets to be embedded in config. | | Required: \{\}
| | `outgoingAuth` _[api.v1beta1.OutgoingAuthConfig](#apiv1beta1outgoingauthconfig)_ | OutgoingAuth configures authentication from Virtual MCP to backend MCPServers.
This field takes precedence over config.OutgoingAuth and should be preferred because it
supports Kubernetes-native secret references (SecretKeyRef, ConfigMapRef) for secure
dynamic discovery of credentials, rather than requiring secrets to be embedded in config. | | Optional: \{\}
| +| `passthroughHeaders` _string array_ | PassthroughHeaders is an allowlist of incoming client request header names
forwarded verbatim to all backends (e.g. an API key the backend resolves to
a user). Takes precedence over config.PassthroughHeaders. Names must not be
restricted headers (Host, hop-by-hop, X-Forwarded-*). Forwarded headers are
attacker-influenceable unless a trusted upstream sets them. | | Optional: \{\}
| | `serviceType` _string_ | ServiceType specifies the Kubernetes service type for the Virtual MCP server | ClusterIP | Enum: [ClusterIP NodePort LoadBalancer]
Optional: \{\}
| | `sessionAffinity` _string_ | SessionAffinity controls whether the Service routes repeated client connections to the same pod.
MCP protocols (SSE, streamable-http) are stateful, so ClientIP is the default.
Set to "None" for stateless servers or when using an external load balancer with its own affinity. | ClientIP | Enum: [ClientIP None]
Optional: \{\}
| | `serviceAccount` _string_ | ServiceAccount is the name of an already existing service account to use by the Virtual MCP server.
If not specified, a ServiceAccount will be created automatically and used by the Virtual MCP server. | | Optional: \{\}
| diff --git a/docs/operator/virtualmcpserver-api.md b/docs/operator/virtualmcpserver-api.md index 9e72694faa..88dd752016 100644 --- a/docs/operator/virtualmcpserver-api.md +++ b/docs/operator/virtualmcpserver-api.md @@ -193,6 +193,30 @@ spec: - `externalAuthConfigRef`: Reference an MCPExternalAuthConfig resource - `externalAuthConfigRef` (ExternalAuthConfigRef, optional): Auth config reference (when type=externalAuthConfigRef) +### `.spec.passthroughHeaders` (optional) + +Allowlist of incoming client request headers forwarded verbatim to every +backend. The value is captured at the auth boundary and injected into the +session's outgoing backend requests. **Type**: `[]string`. Names are matched +case-insensitively; restricted headers (`Host`, `Authorization`, +`X-Forwarded-*`, hop-by-hop) are rejected at startup. Takes precedence over +`spec.config.passthroughHeaders`. + +```yaml +spec: + passthroughHeaders: + - x-litellm-api-key # forwarded to all backends; absent on a request → not forwarded +``` + +> **Security:** forwarded headers are caller-controlled. Use only behind a +> trusted upstream (gateway/mesh) that sets or strips them; pair with +> `incomingAuth: anonymous` only in that case. +> +> **Horizontal scaling:** forwarded headers are not persisted across replicas +> (only the `(issuer, subject)` identity tuple is). A session restored on +> another replica re-captures them from the next request — keep clients pinned +> with `sessionAffinity: ClientIP`. + ### `.spec.config.aggregation` (optional) Defines tool aggregation and conflict resolution strategies. diff --git a/pkg/vmcp/cli/serve.go b/pkg/vmcp/cli/serve.go index c26061c974..78f61e7f5e 100644 --- a/pkg/vmcp/cli/serve.go +++ b/pkg/vmcp/cli/serve.go @@ -408,6 +408,7 @@ func Serve(ctx context.Context, cfg ServeConfig) error { AuthMiddleware: authMiddleware, AuthzMiddleware: authzMiddleware, AuthInfoHandler: authInfoHandler, + PassthroughHeaders: vmcpCfg.PassthroughHeaders, RateLimitMiddleware: rateLimitMiddleware, AuthServer: embeddedAuthServer, TelemetryProvider: telemetryProvider, diff --git a/pkg/vmcp/config/config.go b/pkg/vmcp/config/config.go index df29ece070..bdd60e610c 100644 --- a/pkg/vmcp/config/config.go +++ b/pkg/vmcp/config/config.go @@ -179,6 +179,15 @@ type Config struct { // Requires Redis session storage to be configured for distributed rate limiting. // +optional RateLimiting *ratelimittypes.RateLimitConfig `json:"rateLimiting,omitempty" yaml:"rateLimiting,omitempty"` + + // PassthroughHeaders is an allowlist of incoming client request header names + // forwarded verbatim to all backends. Captured at the vMCP incoming edge by + // headerforward.CaptureMiddleware and consumed once at session creation + // when the per-session backend client's HeaderForwardConfig is built. Names + // must not be in the restricted set (Host, hop-by-hop, X-Forwarded-*, etc.). + // +optional + // +listType=atomic + PassthroughHeaders []string `json:"passthroughHeaders,omitempty" yaml:"passthroughHeaders,omitempty"` } // IncomingAuthConfig configures client authentication to the virtual MCP server. diff --git a/pkg/vmcp/config/validator.go b/pkg/vmcp/config/validator.go index e0c30c0cbe..813e382421 100644 --- a/pkg/vmcp/config/validator.go +++ b/pkg/vmcp/config/validator.go @@ -5,12 +5,15 @@ package config import ( "fmt" + "net/http" "path/filepath" "slices" "strings" "time" + httpval "github.com/stacklok/toolhive-core/validation/http" "github.com/stacklok/toolhive/pkg/authserver" + "github.com/stacklok/toolhive/pkg/transport/middleware" "github.com/stacklok/toolhive/pkg/vmcp" authtypes "github.com/stacklok/toolhive/pkg/vmcp/auth/types" ) @@ -84,6 +87,11 @@ func (v *DefaultValidator) Validate(cfg *Config) error { errors = append(errors, err.Error()) } + // Validate passthrough headers allowlist + if err := v.validatePassthroughHeaders(cfg); err != nil { + errors = append(errors, err.Error()) + } + // Note: Optimizer validation is handled by optimizer.GetAndValidateConfig // in pkg/vmcp/optimizer/optimizer.go when the optimizer is constructed. @@ -487,6 +495,25 @@ func (*DefaultValidator) validateCompositeToolRefs(refs []CompositeToolRef) erro return nil } +func (*DefaultValidator) validatePassthroughHeaders(cfg *Config) error { + for i, name := range cfg.PassthroughHeaders { + if name == "" { + return fmt.Errorf("passthroughHeaders[%d]: header name must not be empty", i) + } + + canonical := http.CanonicalHeaderKey(name) + + if middleware.RestrictedHeaders[canonical] { + return fmt.Errorf("passthroughHeaders[%d]: %q is a restricted header and cannot be forwarded", i, canonical) + } + + if err := httpval.ValidateHeaderName(name); err != nil { + return fmt.Errorf("passthroughHeaders[%d]: invalid header name %q: %w", i, name, err) + } + } + return nil +} + // Note: Workflow step validation is now handled by the shared ValidateWorkflowSteps function // in composite_validation.go, which is called by ValidateCompositeToolConfig. diff --git a/pkg/vmcp/config/validator_test.go b/pkg/vmcp/config/validator_test.go index 1fabf51be2..ce513ca793 100644 --- a/pkg/vmcp/config/validator_test.go +++ b/pkg/vmcp/config/validator_test.go @@ -1169,6 +1169,87 @@ func TestValidateAuthServerIntegration(t *testing.T) { } } +func TestValidator_ValidatePassthroughHeaders(t *testing.T) { + t.Parallel() + + // validBaseConfig returns a minimally-valid Config so that only + // passthroughHeaders validation is under test. + validBaseConfig := func(headers []string) *Config { + return &Config{ + Name: "test-vmcp", + Group: "test-group", + IncomingAuth: &IncomingAuthConfig{ + Type: "anonymous", + }, + OutgoingAuth: &OutgoingAuthConfig{ + Source: "inline", + }, + Aggregation: &AggregationConfig{ + ConflictResolution: vmcp.ConflictStrategyPrefix, + ConflictResolutionConfig: &ConflictResolutionConfig{ + PrefixFormat: "{workload}_", + }, + }, + PassthroughHeaders: headers, + } + } + + tests := []struct { + name string + headers []string + wantErr bool + errMsg string + }{ + { + // nil and []string{} both produce zero iterations in range — same code path. + name: "nil slice is valid", + headers: nil, + wantErr: false, + }, + { + name: "valid header names", + headers: []string{"x-env", "x-litellm-api-key"}, + wantErr: false, + }, + { + // Restricted-header check uses middleware.RestrictedHeaders[canonical]; one + // case covers the branch. Canonicalization (e.g. "x-forwarded-for" → canonical) + // is verified by TestCaptureForwardedHeadersMiddleware. + name: "X-Forwarded-For is restricted", + headers: []string{"X-Forwarded-For"}, + wantErr: true, + errMsg: "X-Forwarded-For", + }, + { + name: "empty string header name is rejected", + headers: []string{""}, + wantErr: true, + errMsg: "passthroughHeaders", + }, + { + name: "header name with CRLF injection is rejected", + headers: []string{"X-My-Header\r\nX-Injected: evil"}, + wantErr: true, + errMsg: "passthroughHeaders", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + v := NewValidator() + err := v.Validate(validBaseConfig(tt.headers)) + + if tt.wantErr { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.errMsg) + } else { + require.NoError(t, err) + } + }) + } +} + func TestValidator_ValidateStaticBackends(t *testing.T) { t.Parallel() v := NewValidator() diff --git a/pkg/vmcp/config/zz_generated.deepcopy.go b/pkg/vmcp/config/zz_generated.deepcopy.go index 62bbff9cd6..0559f5a790 100644 --- a/pkg/vmcp/config/zz_generated.deepcopy.go +++ b/pkg/vmcp/config/zz_generated.deepcopy.go @@ -210,6 +210,11 @@ func (in *Config) DeepCopyInto(out *Config) { *out = new(types.RateLimitConfig) (*in).DeepCopyInto(*out) } + if in.PassthroughHeaders != nil { + in, out := &in.PassthroughHeaders, &out.PassthroughHeaders + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Config. diff --git a/pkg/vmcp/headerforward/capture.go b/pkg/vmcp/headerforward/capture.go new file mode 100644 index 0000000000..cb305f9030 --- /dev/null +++ b/pkg/vmcp/headerforward/capture.go @@ -0,0 +1,74 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package headerforward + +import ( + "context" + "net/http" +) + +// forwardedHeadersKey is the unexported context key under which the allowlisted +// incoming request headers are carried from the capture middleware to the +// per-session backend client. Keeping it unexported makes the context value an +// implementation detail of this package: callers use WithForwardedHeaders / +// ForwardedHeadersFromContext rather than the raw key. +type forwardedHeadersKey struct{} + +// WithForwardedHeaders returns a child context carrying the allowlisted forwarded +// headers (canonical header name → value) for the current request. This is +// request-scoped plumbing — the headers are read back by the per-session backend +// client when it builds the outbound transport. +func WithForwardedHeaders(ctx context.Context, headers map[string]string) context.Context { + return context.WithValue(ctx, forwardedHeadersKey{}, headers) +} + +// ForwardedHeadersFromContext returns the forwarded headers captured for the +// current request, or nil if none were captured. +func ForwardedHeadersFromContext(ctx context.Context) map[string]string { + h, _ := ctx.Value(forwardedHeadersKey{}).(map[string]string) + return h +} + +// CaptureMiddleware returns HTTP middleware that copies the allowlisted incoming +// request headers into the request context (via WithForwardedHeaders) so the +// per-session backend client can forward them to backends. Header names are +// matched case-insensitively and stored in canonical form. +// +// The capture is pure plumbing: it does not depend on the request identity and +// carries the values as an explicit, request-scoped context value rather than on +// any business-logic type. If allow is empty the function returns a no-op wrapper +// to avoid allocations on the hot path. +func CaptureMiddleware(allow []string) func(http.Handler) http.Handler { + if len(allow) == 0 { + return func(next http.Handler) http.Handler { return next } + } + + // Precompute the canonical header names once at construction time so the + // per-request handler does only cheap lookups. + canonical := make([]string, len(allow)) + for i, name := range allow { + canonical[i] = http.CanonicalHeaderKey(name) + } + + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Collect the values of allowlisted headers that are actually present. + fwd := make(map[string]string, len(canonical)) + for _, name := range canonical { + if v := r.Header.Get(name); v != "" { + fwd[name] = v + } + } + + if len(fwd) == 0 { + // Nothing to attach — skip the context allocation on the fast path. + next.ServeHTTP(w, r) + return + } + + ctx := WithForwardedHeaders(r.Context(), fwd) + next.ServeHTTP(w, r.WithContext(ctx)) + }) + } +} diff --git a/pkg/vmcp/headerforward/capture_test.go b/pkg/vmcp/headerforward/capture_test.go new file mode 100644 index 0000000000..0e7703914f --- /dev/null +++ b/pkg/vmcp/headerforward/capture_test.go @@ -0,0 +1,94 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package headerforward + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestCaptureMiddleware verifies the header capture middleware that reads +// allowlisted incoming request headers and carries them on a request-scoped +// context value for the per-session backend client to forward. +func TestCaptureMiddleware(t *testing.T) { + t.Parallel() + + // runCapture builds the middleware chain, fires a GET /, and returns the + // forwarded-headers map the terminal handler observed in the request context, + // plus whether any context value was set at all. + runCapture := func(t *testing.T, allowlist []string, reqHeaders map[string]string) (map[string]string, bool) { + t.Helper() + + var got map[string]string + var valueSet bool + terminal := http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + got = ForwardedHeadersFromContext(r.Context()) + _, valueSet = r.Context().Value(forwardedHeadersKey{}).(map[string]string) + }) + + req := httptest.NewRequest(http.MethodGet, "/", nil) + for k, v := range reqHeaders { + req.Header.Set(k, v) + } + CaptureMiddleware(allowlist)(terminal).ServeHTTP(httptest.NewRecorder(), req) + return got, valueSet + } + + t.Run("allowlisted_header_captured_into_context", func(t *testing.T) { + t.Parallel() + got, _ := runCapture(t, []string{"X-Api-Key"}, map[string]string{"X-Api-Key": "secret-value"}) + require.NotNil(t, got) + assert.Equal(t, "secret-value", got["X-Api-Key"]) + }) + + t.Run("non_allowlisted_header_absent", func(t *testing.T) { + t.Parallel() + got, _ := runCapture(t, []string{"X-Api-Key"}, + map[string]string{"X-Api-Key": "val", "X-Secret-Admin": "should-not-appear"}) + require.NotNil(t, got) + _, exists := got["X-Secret-Admin"] + assert.False(t, exists, "non-allowlisted header must not be captured") + }) + + t.Run("empty_allowlist_is_noop", func(t *testing.T) { + t.Parallel() + got, valueSet := runCapture(t, nil, map[string]string{"X-Api-Key": "value"}) + assert.Nil(t, got) + assert.False(t, valueSet, "no context value should be set for an empty allowlist") + }) + + t.Run("no_matching_header_sets_no_context_value", func(t *testing.T) { + t.Parallel() + // Allowlist present but the request carries none of the listed headers. + got, valueSet := runCapture(t, []string{"X-Api-Key"}, map[string]string{"X-Other": "v"}) + assert.Nil(t, got) + assert.False(t, valueSet, "context value must be skipped when nothing is captured") + }) + + t.Run("header_matched_case_insensitively_canonical_form", func(t *testing.T) { + t.Parallel() + // Allowlist uses non-canonical casing; http.Header.Set canonicalises the request side. + got, _ := runCapture(t, []string{"x-api-key"}, map[string]string{"X-Api-Key": "token123"}) + require.NotNil(t, got) + assert.Equal(t, "token123", got["X-Api-Key"], "value must be accessible under the canonical key") + }) + + t.Run("no_panic_without_headers", func(t *testing.T) { + t.Parallel() + assert.NotPanics(t, func() { + runCapture(t, []string{"X-Api-Key"}, nil) + }) + }) +} + +// TestForwardedHeadersFromContext_Empty verifies the accessor returns nil when no +// capture middleware ran (e.g. the empty-allowlist no-op path). +func TestForwardedHeadersFromContext_Empty(t *testing.T) { + t.Parallel() + assert.Nil(t, ForwardedHeadersFromContext(t.Context())) +} diff --git a/pkg/vmcp/server/serve.go b/pkg/vmcp/server/serve.go index 6187cc88a9..92cce14f25 100644 --- a/pkg/vmcp/server/serve.go +++ b/pkg/vmcp/server/serve.go @@ -77,6 +77,11 @@ type ServerConfig struct { // /.well-known/oauth-protected-resource endpoint. AuthInfoHandler http.Handler + // PassthroughHeaders is an allowlist of incoming client header names the vMCP + // forwards, unchanged, to every backend it calls (see headerforward.CaptureMiddleware). + // Empty disables capture. + PassthroughHeaders []string + // AuthServer is the optional embedded authorization server. When non-nil, its // routes are registered on the mux alongside the protected resource metadata. AuthServer *asrunner.EmbeddedAuthServer @@ -330,6 +335,7 @@ func buildServeConfig(cfg *ServerConfig) *Config { AuthMiddleware: cfg.AuthMiddleware, RateLimitMiddleware: cfg.RateLimitMiddleware, AuthInfoHandler: cfg.AuthInfoHandler, + PassthroughHeaders: cfg.PassthroughHeaders, AuthServer: cfg.AuthServer, TelemetryProvider: cfg.TelemetryProvider, AuditConfig: cfg.AuditConfig, diff --git a/pkg/vmcp/server/serve_test.go b/pkg/vmcp/server/serve_test.go index 6e9d552044..65c2c01faa 100644 --- a/pkg/vmcp/server/serve_test.go +++ b/pkg/vmcp/server/serve_test.go @@ -351,6 +351,7 @@ func TestBuildServeConfigMapsSharedFields(t *testing.T) { AuthMiddleware: func(h http.Handler) http.Handler { return h }, RateLimitMiddleware: func(h http.Handler) http.Handler { return h }, AuthInfoHandler: http.NewServeMux(), + PassthroughHeaders: []string{"x-test"}, AuthServer: &asrunner.EmbeddedAuthServer{}, HealthMonitor: &health.Monitor{}, StatusReportingInterval: time.Second, diff --git a/pkg/vmcp/server/server.go b/pkg/vmcp/server/server.go index 2355bd103c..984b3beca0 100644 --- a/pkg/vmcp/server/server.go +++ b/pkg/vmcp/server/server.go @@ -39,6 +39,7 @@ import ( vmcpconfig "github.com/stacklok/toolhive/pkg/vmcp/config" "github.com/stacklok/toolhive/pkg/vmcp/core" "github.com/stacklok/toolhive/pkg/vmcp/discovery" + "github.com/stacklok/toolhive/pkg/vmcp/headerforward" "github.com/stacklok/toolhive/pkg/vmcp/health" "github.com/stacklok/toolhive/pkg/vmcp/internal/backendtelemetry" "github.com/stacklok/toolhive/pkg/vmcp/optimizer" @@ -138,6 +139,13 @@ type Config struct { // Exposes OIDC discovery information about the protected resource. AuthInfoHandler http.Handler + // PassthroughHeaders is an allowlist of incoming client header names that the + // vMCP forwards, unchanged, to every backend it calls. They are captured + // per-request at the incoming edge (headerforward.CaptureMiddleware) and merged + // into the per-session backend client's header-forward config. Empty disables + // capture (no middleware is installed). + PassthroughHeaders []string + // RateLimitMiddleware is the optional rate-limit middleware to apply after // authentication and MCP request parsing. RateLimitMiddleware func(http.Handler) http.Handler @@ -701,6 +709,8 @@ func (s *Server) Handler(_ context.Context) (http.Handler, error) { slog.Info("authentication middleware enabled for MCP endpoints") } + mcpHandler = s.applyForwardedHeaderCapture(mcpHandler) + // Apply Accept header validation (rejects GET requests without Accept: text/event-stream) mcpHandler = headerValidatingMiddleware(mcpHandler) @@ -734,6 +744,21 @@ func (s *Server) applyRateLimiting(next http.Handler) http.Handler { return s.config.RateLimitMiddleware(next) } +// applyForwardedHeaderCapture wraps next with the forwarded-header capture +// middleware when passthrough headers are configured. It copies the allowlisted +// incoming headers into the request context so the per-session backend client +// forwards them to backends (see pkg/vmcp/headerforward). Identity-independent +// plumbing: its position relative to auth is immaterial; it must only run before +// session creation, which every inner handler satisfies. No-op when the allowlist +// is empty. +func (s *Server) applyForwardedHeaderCapture(next http.Handler) http.Handler { + if len(s.config.PassthroughHeaders) == 0 { + return next + } + slog.Info("forwarded-header capture enabled for MCP endpoints", "headers", s.config.PassthroughHeaders) + return headerforward.CaptureMiddleware(s.config.PassthroughHeaders)(next) +} + // Start starts the Virtual MCP Server and begins serving requests. // //nolint:gocyclo // Complexity from health monitoring and startup orchestration is acceptable diff --git a/pkg/vmcp/session/internal/backend/mcp_session.go b/pkg/vmcp/session/internal/backend/mcp_session.go index 45e1f534cf..c70ba8a3c5 100644 --- a/pkg/vmcp/session/internal/backend/mcp_session.go +++ b/pkg/vmcp/session/internal/backend/mcp_session.go @@ -9,6 +9,7 @@ import ( "fmt" "io" "log/slog" + "maps" "net/http" "time" @@ -18,6 +19,7 @@ import ( "github.com/stacklok/toolhive/pkg/auth" "github.com/stacklok/toolhive/pkg/secrets" + "github.com/stacklok/toolhive/pkg/transport/middleware" "github.com/stacklok/toolhive/pkg/versions" "github.com/stacklok/toolhive/pkg/vmcp" vmcpauth "github.com/stacklok/toolhive/pkg/vmcp/auth" @@ -318,7 +320,15 @@ func createMCPClient( // refreshed identity placed on the request context by // auth.TokenValidator.Middleware (see issue #5323). base = &identityRoundTripper{base: base, fallbackIdentity: identity} - base, err = headerforward.BuildHeaderForwardTripper(ctx, base, target.HeaderForward, provider, target.WorkloadID) + // Forwarded headers ride the request context (set by headerforward.CaptureMiddleware + // at the vMCP server's incoming edge) and are merged into the per-session backend + // header-forward config here. The session is created once per request, so the + // captured headers are stable for the session's lifetime. + mergedHeaderForward, err := mergeForwardedHeaders(target.HeaderForward, headerforward.ForwardedHeadersFromContext(ctx)) + if err != nil { + return nil, fmt.Errorf("backend %s: %w", target.WorkloadID, err) + } + base, err = headerforward.BuildHeaderForwardTripper(ctx, base, mergedHeaderForward, provider, target.WorkloadID) if err != nil { return nil, fmt.Errorf("failed to build header-forward transport for backend %s: %w", target.WorkloadID, err) } @@ -511,3 +521,68 @@ func initAndQueryCapabilities( return caps, nil } + +// mergeForwardedHeaders returns a HeaderForwardConfig that combines the static +// backend configuration (base) with any per-request forwarded headers captured +// from the caller's request (see headerforward.CaptureMiddleware). +// +// Rules (applied in order): +// 1. If forwarded is empty, base is returned unchanged (no allocation, same +// pointer). +// 2. A new HeaderForwardConfig is built from a shallow copy of base so the +// shared target.HeaderForward is never mutated. +// 3. Forwarded header names are canonicalized via http.CanonicalHeaderKey and +// checked against middleware.RestrictedHeaders; restricted names are silently +// dropped (defense-in-depth — they were already filtered upstream, but we +// guard here too). +// 4. A forwarded header name that also appears as a static header in base +// (AddPlaintextHeaders or AddHeadersFromSecret) is a misconfiguration: the +// function returns an error rather than silently picking a winner. +func mergeForwardedHeaders(base *vmcp.HeaderForwardConfig, forwarded map[string]string) (*vmcp.HeaderForwardConfig, error) { + if len(forwarded) == 0 { + return base, nil + } + + // Build the merged AddPlaintextHeaders map starting from the static config. + var staticPlaintext map[string]string + if base != nil { + staticPlaintext = base.AddPlaintextHeaders + } + + // Canonical set of header names already owned by the backend's static + // header-forward config (plaintext + secret), used for collision detection. + staticNames := make(map[string]struct{}) + if base != nil { + for k := range base.AddPlaintextHeaders { + staticNames[http.CanonicalHeaderKey(k)] = struct{}{} + } + for k := range base.AddHeadersFromSecret { + staticNames[http.CanonicalHeaderKey(k)] = struct{}{} + } + } + + merged := make(map[string]string, len(staticPlaintext)+len(forwarded)) + maps.Copy(merged, staticPlaintext) + + for name, value := range forwarded { + canonical := http.CanonicalHeaderKey(name) + if middleware.RestrictedHeaders[canonical] { + slog.Debug("Dropping restricted forwarded header", "header", canonical) + continue + } + // Fail loud on collision with a static header-forward value (rule 4). + if _, exists := staticNames[canonical]; exists { + return nil, fmt.Errorf( + "forwarded header %q collides with the backend's static header-forward config", canonical) + } + merged[canonical] = value + } + + out := &vmcp.HeaderForwardConfig{ + AddPlaintextHeaders: merged, + } + if base != nil { + out.AddHeadersFromSecret = base.AddHeadersFromSecret + } + return out, nil +} diff --git a/pkg/vmcp/session/internal/backend/mcp_session_test.go b/pkg/vmcp/session/internal/backend/mcp_session_test.go index 842d0368fd..26c5ee18c2 100644 --- a/pkg/vmcp/session/internal/backend/mcp_session_test.go +++ b/pkg/vmcp/session/internal/backend/mcp_session_test.go @@ -5,6 +5,7 @@ package backend import ( "context" + "maps" "testing" "github.com/stretchr/testify/assert" @@ -27,6 +28,147 @@ func newTestRegistry(t *testing.T) vmcpauth.OutgoingAuthRegistry { return reg } +func TestMergeForwardedHeaders(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + base *vmcp.HeaderForwardConfig + // forwarded is the per-request captured header map, as returned by + // headerforward.ForwardedHeadersFromContext. + forwarded map[string]string + // wantSameAsBase means we expect the returned pointer to equal the base + // pointer (i.e., no merge was needed — original passed through unchanged). + wantSameAsBase bool + wantHeaders map[string]string + // wantErr means mergeForwardedHeaders should return an error — a forwarded + // header name collides with a static header-forward value (plaintext or + // secret) on the backend. + wantErr bool + }{ + { + name: "nil forwarded returns base unchanged", + base: &vmcp.HeaderForwardConfig{AddPlaintextHeaders: map[string]string{"X-Static": "static"}}, + forwarded: nil, + wantSameAsBase: true, + }, + { + name: "nil forwarded nil base returns nil", + base: nil, + forwarded: nil, + wantSameAsBase: true, + }, + { + // Both nil and empty forwarded maps satisfy len==0 and return base unchanged. + name: "empty forwarded returns base unchanged", + base: &vmcp.HeaderForwardConfig{AddPlaintextHeaders: map[string]string{"X-Static": "static"}}, + forwarded: map[string]string{}, + wantSameAsBase: true, + }, + { + name: "forwarded header added to nil base", + base: nil, + forwarded: map[string]string{"X-Litellm-Api-Key": "sk-1"}, + wantHeaders: map[string]string{"X-Litellm-Api-Key": "sk-1"}, + }, + { + name: "forwarded header added to base with other header", + base: &vmcp.HeaderForwardConfig{ + AddPlaintextHeaders: map[string]string{"X-Static": "static-value"}, + }, + forwarded: map[string]string{"X-Litellm-Api-Key": "sk-1"}, + wantHeaders: map[string]string{ + "X-Static": "static-value", + "X-Litellm-Api-Key": "sk-1", + }, + }, + { + // Covers both a canonical restricted header (Host) and a lowercase one + // (x-forwarded-for) to verify case-insensitive matching in a single case. + name: "restricted headers dropped (canonical and lowercase)", + base: nil, + forwarded: map[string]string{ + "Host": "evil.example.com", + "x-forwarded-for": "1.2.3.4", + "X-Litellm-Api-Key": "sk-2", + }, + wantHeaders: map[string]string{"X-Litellm-Api-Key": "sk-2"}, + }, + { + name: "forwarded header colliding with static plaintext returns error", + base: &vmcp.HeaderForwardConfig{ + AddPlaintextHeaders: map[string]string{"X-Litellm-Api-Key": "static-value"}, + }, + forwarded: map[string]string{"X-Litellm-Api-Key": "forwarded-value"}, + wantErr: true, + }, + { + name: "forwarded header colliding with static secret returns error", + base: &vmcp.HeaderForwardConfig{ + AddHeadersFromSecret: map[string]string{"X-Litellm-Api-Key": "my-secret-id"}, + }, + forwarded: map[string]string{"X-Litellm-Api-Key": "sk-5"}, + wantErr: true, + }, + { + name: "base AddHeadersFromSecret preserved unchanged", + base: &vmcp.HeaderForwardConfig{ + AddHeadersFromSecret: map[string]string{"X-Secret-Header": "my-secret-id"}, + }, + forwarded: map[string]string{"X-Litellm-Api-Key": "sk-4"}, + wantHeaders: map[string]string{"X-Litellm-Api-Key": "sk-4"}, + }, + { + name: "base not mutated when forwarded headers added", + base: &vmcp.HeaderForwardConfig{ + AddPlaintextHeaders: map[string]string{"X-Static": "original"}, + }, + forwarded: map[string]string{"X-New": "new-value"}, + // base should not gain X-New + wantHeaders: map[string]string{ + "X-Static": "original", + "X-New": "new-value", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + // Snapshot the original base plaintext headers to verify they are not mutated. + var origBasePlaintext map[string]string + if tc.base != nil { + origBasePlaintext = maps.Clone(tc.base.AddPlaintextHeaders) + } + + got, err := mergeForwardedHeaders(tc.base, tc.forwarded) + + if tc.wantErr { + require.Error(t, err) + assert.Nil(t, got) + return + } + require.NoError(t, err) + + if tc.wantSameAsBase { + assert.Same(t, tc.base, got, "expected the original base pointer to be returned unchanged") + return + } + + require.NotNil(t, got) + assert.Equal(t, tc.wantHeaders, got.AddPlaintextHeaders, + "merged AddPlaintextHeaders mismatch (checks both presence and absence of keys)") + + // Verify base was not mutated. + if tc.base != nil { + assert.Equal(t, origBasePlaintext, tc.base.AddPlaintextHeaders, + "base.AddPlaintextHeaders must not be mutated") + } + }) + } +} + func TestCreateMCPClient_UnsupportedTransport(t *testing.T) { t.Parallel() diff --git a/test/integration/vmcp/helpers/mcp_client.go b/test/integration/vmcp/helpers/mcp_client.go index f280851c55..75167cbdf6 100644 --- a/test/integration/vmcp/helpers/mcp_client.go +++ b/test/integration/vmcp/helpers/mcp_client.go @@ -5,10 +5,13 @@ package helpers import ( "context" + "maps" "strings" + "sync" "testing" "github.com/mark3labs/mcp-go/client" + mcptransport "github.com/mark3labs/mcp-go/client/transport" "github.com/mark3labs/mcp-go/mcp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -30,6 +33,12 @@ import ( type MCPClient struct { client *client.Client tb testing.TB + + // reqHeaders holds the caller-side HTTP headers sent on every outbound + // request to the vMCP server. It is served per-request through a header func + // and guarded by reqHeadersMu so SetHeader can change a header mid-session. + reqHeaders map[string]string + reqHeadersMu *sync.Mutex } // MCPClientOption is a functional option for configuring an MCPClient. @@ -39,6 +48,23 @@ type MCPClientOption func(*mcpClientConfig) type mcpClientConfig struct { clientName string clientVersion string + extraHeaders map[string]string +} + +// WithClientHeader sets the initial value of an HTTP request header that the +// client sends on every outbound request to the vMCP server. Call multiple +// times to set multiple headers; use MCPClient.SetHeader to change a value +// after the client is created. +// +// This is used by header-passthrough tests to inject caller-side headers so +// the capture middleware can forward them to the backend. +func WithClientHeader(name, value string) MCPClientOption { + return func(c *mcpClientConfig) { + if c.extraHeaders == nil { + c.extraHeaders = make(map[string]string) + } + c.extraHeaders[name] = value + } } // NewMCPClient creates and initializes a new MCP client for testing. @@ -73,8 +99,24 @@ func NewMCPClient(ctx context.Context, tb testing.TB, serverURL string, opts ... opt(config) } + // Caller headers are served through a per-request header func backed by a + // mutex-guarded map. This lets header-passthrough tests change a header + // mid-session via SetHeader and assert the backend still observes the value + // captured at backend-session creation. + reqHeaders := map[string]string{} + maps.Copy(reqHeaders, config.extraHeaders) + reqHeadersMu := &sync.Mutex{} + + transportOpts := []mcptransport.StreamableHTTPCOption{ + mcptransport.WithHTTPHeaderFunc(func(context.Context) map[string]string { + reqHeadersMu.Lock() + defer reqHeadersMu.Unlock() + return maps.Clone(reqHeaders) + }), + } + // Create streamable-http client (vMCP only supports streamable-http) - mcpClient, err := client.NewStreamableHttpClient(serverURL) + mcpClient, err := client.NewStreamableHttpClient(serverURL, transportOpts...) require.NoError(tb, err, "failed to create MCP client with streamable-http transport") // Start the transport @@ -97,11 +139,22 @@ func NewMCPClient(ctx context.Context, tb testing.TB, serverURL string, opts ... config.clientName, config.clientVersion, serverURL) return &MCPClient{ - client: mcpClient, - tb: tb, + client: mcpClient, + tb: tb, + reqHeaders: reqHeaders, + reqHeadersMu: reqHeadersMu, } } +// SetHeader updates a caller-side HTTP header sent on subsequent requests to the +// vMCP server. Used by header-passthrough tests to change a header mid-session +// and assert the backend still observes the value captured at session creation. +func (c *MCPClient) SetHeader(name, value string) { + c.reqHeadersMu.Lock() + defer c.reqHeadersMu.Unlock() + c.reqHeaders[name] = value +} + // Close closes the MCP client connection. // This should typically be deferred immediately after client creation. func (c *MCPClient) Close() error { diff --git a/test/integration/vmcp/helpers/vmcp_server.go b/test/integration/vmcp/helpers/vmcp_server.go index fa1262d812..4679b3a9aa 100644 --- a/test/integration/vmcp/helpers/vmcp_server.go +++ b/test/integration/vmcp/helpers/vmcp_server.go @@ -69,10 +69,11 @@ type VMCPServerOption func(*vmcpServerConfig) // vmcpServerConfig holds configuration for creating a test vMCP server. type vmcpServerConfig struct { - conflictStrategy string - prefixFormat string - workflowDefs map[string]*composer.WorkflowDefinition - telemetryProvider *telemetry.Provider + conflictStrategy string + prefixFormat string + workflowDefs map[string]*composer.WorkflowDefinition + telemetryProvider *telemetry.Provider + passthroughHeaders []string } // WithPrefixConflictResolution configures prefix-based conflict resolution. @@ -97,6 +98,19 @@ func WithTelemetryProvider(provider *telemetry.Provider) VMCPServerOption { } } +// WithPassthroughHeaders sets the vMCP server's PassthroughHeaders allowlist, +// matching the production wiring in pkg/vmcp/cli/serve.go. +// +// When this option is used, NewVMCPServer passes the header names to +// vmcpserver.Config.PassthroughHeaders, which installs +// headerforward.CaptureMiddleware so allowlisted headers are captured into the +// request context and forwarded to backends. +func WithPassthroughHeaders(headers ...string) VMCPServerOption { + return func(c *vmcpServerConfig) { + c.passthroughHeaders = headers + } +} + // getFreePort returns an available TCP port on localhost. // This is used for parallel test execution to avoid port conflicts. func getFreePort(tb testing.TB) int { @@ -179,15 +193,20 @@ func NewVMCPServer( // strategy (e.g. prefix format applied by the aggregator). sessionFactory := vmcpsession.NewSessionFactory(outgoingRegistry, vmcpsession.WithAggregator(agg)) - // Create vMCP server with test-specific defaults + // Create vMCP server with test-specific defaults. + // + // PassthroughHeaders wires headerforward.CaptureMiddleware inside the server + // (matching pkg/vmcp/cli/serve.go), so allowlisted headers are captured into + // the request context and forwarded to backends. Empty disables capture. vmcpServer, err := vmcpserver.New(ctx, &vmcpserver.Config{ - Name: "test-vmcp", - Version: "1.0.0", - Host: "127.0.0.1", - Port: getFreePort(tb), // Get a random available port for parallel test execution - AuthMiddleware: auth.AnonymousMiddleware, - TelemetryProvider: config.telemetryProvider, - SessionFactory: sessionFactory, + Name: "test-vmcp", + Version: "1.0.0", + Host: "127.0.0.1", + Port: getFreePort(tb), // Get a random available port for parallel test execution + AuthMiddleware: auth.AnonymousMiddleware, + PassthroughHeaders: config.passthroughHeaders, + TelemetryProvider: config.telemetryProvider, + SessionFactory: sessionFactory, }, rtr, backendClient, discoveryMgr, backendRegistry, config.workflowDefs) require.NoError(tb, err, "failed to create vMCP server") diff --git a/test/integration/vmcp/passthrough_headers_test.go b/test/integration/vmcp/passthrough_headers_test.go new file mode 100644 index 0000000000..d3ed6e1a76 --- /dev/null +++ b/test/integration/vmcp/passthrough_headers_test.go @@ -0,0 +1,158 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package vmcp_test + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/stacklok/toolhive/pkg/vmcp" + "github.com/stacklok/toolhive/test/integration/vmcp/helpers" +) + +// TestVMCPServer_PassthroughHeaders verifies the end-to-end header-passthrough +// chain: +// +// MCP client → headerforward.CaptureMiddleware → request-context value → +// per-session backend client (mergeForwardedHeaders) → backend HTTP request. +// +// The test exercises the real server wiring (via helpers.WithPassthroughHeaders, +// which sets vmcpserver.Config.PassthroughHeaders) so headerforward.CaptureMiddleware +// runs; it does NOT use a stub or reimplementation of the capture logic. +// +// Two assertions are made: +// 1. Allowlisted header present: X-Test-Api-Key sent by the client arrives at +// the backend unchanged. +// 2. Non-allowlisted header absent: X-Secret sent by the client is dropped and +// does not arrive at the backend. +func TestVMCPServer_PassthroughHeaders(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + + // ── Backend ────────────────────────────────────────────────────────────── + // The backend captures incoming HTTP headers into the request context so + // tool handlers can inspect them. The echo_header tool reads both the + // allowlisted and the non-allowlisted header and returns their values (or + // the sentinel "" when a header is missing) so the test can assert + // forwarding behaviour without needing to inspect raw HTTP traffic. + const ( + allowlistedHeader = "X-Test-Api-Key" + nonAllowlistedHeader = "X-Secret" + allowlistedValue = "caller-key-123" + nonAllowlistedValue = "should-not-forward" + absentSentinel = "" + ) + + apiBackend := helpers.CreateBackendServer(t, []helpers.BackendTool{ + helpers.NewBackendTool( + "echo_header", + "Echo the values of two request headers back to the caller", + func(ctx context.Context, _ map[string]any) string { + headers := helpers.GetHTTPHeadersFromContext(ctx) + + apiKey := absentSentinel + secret := absentSentinel + + if headers != nil { + if v := headers.Get(allowlistedHeader); v != "" { + apiKey = v + } + if v := headers.Get(nonAllowlistedHeader); v != "" { + secret = v + } + } + + return fmt.Sprintf( + `{"api_key": %q, "secret": %q}`, + apiKey, + secret, + ) + }, + ), + }, + helpers.WithBackendName("api-backend"), + helpers.WithCaptureHeaders(), // store incoming HTTP headers in context + ) + t.Cleanup(apiBackend.Close) + + // ── vMCP server ─────────────────────────────────────────────────────────── + // WithPassthroughHeaders sets vmcpserver.Config.PassthroughHeaders, which + // installs headerforward.CaptureMiddleware so allowlisted headers are captured + // into the request context and forwarded to backends for each request. + backends := []vmcp.Backend{ + helpers.NewBackend("api", + helpers.WithURL(apiBackend.URL+"/mcp"), + ), + } + + vmcpServer := helpers.NewVMCPServer(ctx, t, backends, + helpers.WithPrefixConflictResolution("{workload}_"), + helpers.WithPassthroughHeaders(allowlistedHeader), + ) + + // ── MCP client ──────────────────────────────────────────────────────────── + // WithClientHeader wires each header into the mcp-go streamable-HTTP + // transport via WithHTTPHeaders so every request to the vMCP server carries + // these headers. + vmcpURL := "http://" + vmcpServer.Address() + "/mcp" + mcpClient := helpers.NewMCPClient(ctx, t, vmcpURL, + helpers.WithClientHeader(allowlistedHeader, allowlistedValue), + helpers.WithClientHeader(nonAllowlistedHeader, nonAllowlistedValue), + ) + t.Cleanup(func() { _ = mcpClient.Close() }) + + // ── Sanity: tool is visible ─────────────────────────────────────────────── + toolsResp := mcpClient.ListTools(ctx) + toolNames := helpers.GetToolNames(toolsResp) + require.Contains(t, toolNames, "api_echo_header", + "echo_header tool should be listed by vMCP") + + // ── Call the tool ───────────────────────────────────────────────────────── + result := mcpClient.CallTool(ctx, "api_echo_header", map[string]any{}) + text := helpers.AssertToolCallSuccess(t, result) + + t.Logf("backend response: %s", text) + + // Assertion 1: allowlisted header was forwarded to the backend. + assert.Contains(t, text, allowlistedValue, + "allowlisted header %q should have been forwarded to the backend", + allowlistedHeader) + + // Assertion 2: non-allowlisted header was dropped by the capture middleware. + assert.NotContains(t, text, nonAllowlistedValue, + "non-allowlisted header %q must not reach the backend", + nonAllowlistedHeader) + + // Confirm the non-allowlisted slot shows the absent sentinel to rule out + // the handler silently returning wrong data. + assert.Contains(t, text, absentSentinel, + "non-allowlisted header slot should contain the absent sentinel") + + // ── Second call: header changes mid-session, backend must not see the change ─ + // Forwarded headers are captured once at backend-session creation and stay + // stable for the session's lifetime. Change the caller's X-Test-Api-Key and + // call again: the backend must still observe the ORIGINAL value, proving the + // session does not re-read forwarded headers per request (nor drop them after + // the first call). + const changedValue = "caller-key-CHANGED" + mcpClient.SetHeader(allowlistedHeader, changedValue) + + result2 := mcpClient.CallTool(ctx, "api_echo_header", map[string]any{}) + text2 := helpers.AssertToolCallSuccess(t, result2) + + t.Logf("backend response (second call): %s", text2) + + assert.Contains(t, text2, allowlistedValue, + "second call must still see the header value captured at session creation (%q)", + allowlistedValue) + assert.NotContains(t, text2, changedValue, + "changed header value %q must not reach the backend — forwarded headers are session-stable", + changedValue) +}