diff --git a/cmd/thv-operator/api/v1beta1/mcpserver_types.go b/cmd/thv-operator/api/v1beta1/mcpserver_types.go index 95a0372b14..1b32df29f5 100644 --- a/cmd/thv-operator/api/v1beta1/mcpserver_types.go +++ b/cmd/thv-operator/api/v1beta1/mcpserver_types.go @@ -369,6 +369,18 @@ type MCPServerSpec struct { // +optional TrustProxyHeaders bool `json:"trustProxyHeaders,omitempty"` + // RedactToolResultSecrets enables best-effort credential-shape scanning + // on tools/call responses (AWS/GitHub/Slack/Google/Stripe keys, JWTs, PEM + // private keys): matches are redacted before the response reaches the + // client. Off by default; enable when the backend MCP server is not + // fully trusted. This setting is ONLY applicable when Transport is + // "stdio" (the streamable-HTTP proxy path) -- it has no effect when + // Transport is "streamable-http" or "sse" (those reverse-proxy to an + // already-HTTP backend without inspecting message content). + // +kubebuilder:default=false + // +optional + RedactToolResultSecrets bool `json:"redactToolResultSecrets,omitempty"` + // EndpointPrefix is the path prefix to prepend to SSE endpoint URLs. // This is used to handle path-based ingress routing scenarios where the ingress // strips a path prefix before forwarding to the backend. diff --git a/cmd/thv-operator/controllers/mcpserver_runconfig.go b/cmd/thv-operator/controllers/mcpserver_runconfig.go index 373fd0e0db..32d32e7786 100644 --- a/cmd/thv-operator/controllers/mcpserver_runconfig.go +++ b/cmd/thv-operator/controllers/mcpserver_runconfig.go @@ -150,6 +150,7 @@ func (r *MCPServerReconciler) createRunConfigFromMCPServer(m *mcpv1beta1.MCPServ runner.WithProxyMode(transporttypes.ProxyMode(effectiveProxyMode)), runner.WithHost(proxyHost), runner.WithTrustProxyHeaders(m.Spec.TrustProxyHeaders), + runner.WithRedactToolResultSecrets(m.Spec.RedactToolResultSecrets), runner.WithEndpointPrefix(m.Spec.EndpointPrefix), runner.WithToolsFilter(toolsFilter), runner.WithEnvVars(envVars), diff --git a/cmd/thv-operator/controllers/mcpserver_runconfig_test.go b/cmd/thv-operator/controllers/mcpserver_runconfig_test.go index d3b1f9b942..b2ed43e488 100644 --- a/cmd/thv-operator/controllers/mcpserver_runconfig_test.go +++ b/cmd/thv-operator/controllers/mcpserver_runconfig_test.go @@ -1681,3 +1681,42 @@ func TestCreateRunConfigFromMCPServer_SetsMCPServerGeneration(t *testing.T) { assert.Equal(t, int64(7), rc.MCPServerGeneration, "MCPServerGeneration should match MCPServer .metadata.generation") } + +// TestCreateRunConfigFromMCPServer_RedactToolResultSecrets verifies +// MCPServerSpec.RedactToolResultSecrets flows into RunConfig, mirroring +// TrustProxyHeaders's plumbing. +func TestCreateRunConfigFromMCPServer_RedactToolResultSecrets(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + enabled bool + }{ + {name: "enabled", enabled: true}, + {name: "disabled (default)", enabled: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + m := v1beta1test.NewMCPServer("redact-secrets-server", "default", + v1beta1test.WithImage("ghcr.io/example/mcp:v1"), + v1beta1test.Mutate(func(m *mcpv1beta1.MCPServer) { + m.Spec.RedactToolResultSecrets = tt.enabled + })) + + r := newTestMCPServerReconciler( + fake.NewClientBuilder().WithScheme(testutil.NewScheme(t)).WithObjects(m).Build(), + testutil.NewScheme(t), + kubernetes.PlatformKubernetes, + ) + + rc, err := r.createRunConfigFromMCPServer(m) + + require.NoError(t, err) + require.NotNil(t, rc) + assert.Equal(t, tt.enabled, rc.RedactToolResultSecrets) + }) + } +} diff --git a/cmd/thv/app/run_flags.go b/cmd/thv/app/run_flags.go index 695909bdc3..e39532d0df 100644 --- a/cmd/thv/app/run_flags.go +++ b/cmd/thv/app/run_flags.go @@ -106,6 +106,10 @@ type RunFlags struct { // on the streamable HTTP proxy. StrictProtocolValidation bool + // RedactToolResultSecrets enables best-effort credential-shape scanning + // on tools/call responses relayed by the streamable HTTP proxy. + RedactToolResultSecrets bool + // Endpoint prefix for SSE endpoint URLs EndpointPrefix string @@ -290,6 +294,10 @@ func AddRunFlags(cmd *cobra.Command, config *RunFlags) { cmd.Flags().BoolVar(&config.StrictProtocolValidation, "strict-protocol-validation", false, "Reject client requests whose MCP-Protocol-Version header is an unknown/unsupported MCP revision with HTTP 400 "+ "(streamable-HTTP proxy only; an absent header is accepted). Off by default: any version is accepted.") + cmd.Flags().BoolVar(&config.RedactToolResultSecrets, "redact-tool-result-secrets", false, + "Scan tools/call responses for credential-shaped content (AWS/GitHub/Slack/Google/Stripe keys, JWTs, PEM "+ + "private keys, generic Bearer tokens) and redact matches before relaying them to the client. "+ + "Off by default; enable when the backend MCP server is not fully trusted.") cmd.Flags().BoolVar(&config.Stateless, "stateless", false, "Declare the server as stateless (POST-only, no SSE). "+ "Use for MCP servers implementing streamable-HTTP stateless mode.") @@ -705,6 +713,7 @@ func buildRunnerConfig( runner.WithAllowDockerGateway(runFlags.AllowDockerGateway), runner.WithTrustProxyHeaders(runFlags.TrustProxyHeaders), runner.WithStrictProtocolValidation(runFlags.StrictProtocolValidation), + runner.WithRedactToolResultSecrets(runFlags.RedactToolResultSecrets), runner.WithStateless(runFlags.Stateless), runner.WithSessionTTL(runFlags.SessionTTL), runner.WithEndpointPrefix(runFlags.EndpointPrefix), diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpservers.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpservers.yaml index b2597e7f10..5694ef8cc6 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpservers.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpservers.yaml @@ -515,6 +515,18 @@ spec: - message: at least one of shared, perUser, or tools must be configured rule: has(self.shared) || has(self.perUser) || (has(self.tools) && size(self.tools) > 0) + redactToolResultSecrets: + default: false + description: |- + RedactToolResultSecrets enables best-effort credential-shape scanning + on tools/call responses (AWS/GitHub/Slack/Google/Stripe keys, JWTs, PEM + private keys): matches are redacted before the response reaches the + client. Off by default; enable when the backend MCP server is not + fully trusted. This setting is ONLY applicable when Transport is + "stdio" (the streamable-HTTP proxy path) -- it has no effect when + Transport is "streamable-http" or "sse" (those reverse-proxy to an + already-HTTP backend without inspecting message content). + type: boolean replicas: description: |- Replicas is the desired number of proxy runner (thv run) pod replicas. @@ -1459,6 +1471,18 @@ spec: - message: at least one of shared, perUser, or tools must be configured rule: has(self.shared) || has(self.perUser) || (has(self.tools) && size(self.tools) > 0) + redactToolResultSecrets: + default: false + description: |- + RedactToolResultSecrets enables best-effort credential-shape scanning + on tools/call responses (AWS/GitHub/Slack/Google/Stripe keys, JWTs, PEM + private keys): matches are redacted before the response reaches the + client. Off by default; enable when the backend MCP server is not + fully trusted. This setting is ONLY applicable when Transport is + "stdio" (the streamable-HTTP proxy path) -- it has no effect when + Transport is "streamable-http" or "sse" (those reverse-proxy to an + already-HTTP backend without inspecting message content). + type: boolean replicas: description: |- Replicas is the desired number of proxy runner (thv run) pod replicas. 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 c9d92c0a94..cecd7a8b4f 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 @@ -3195,6 +3195,14 @@ spec: - message: at least one of shared, perUser, or tools must be configured rule: has(self.shared) || has(self.perUser) || (has(self.tools) && size(self.tools) > 0) + redactToolResultSecrets: + default: false + description: |- + RedactToolResultSecrets enables best-effort credential-shape scanning + (see pkg/mcp/secretscan) on tools/call results before they are relayed + to the client: matches are redacted in place. Opt-in (default false); + enable when a backend this vMCP aggregates is not fully trusted. + type: boolean sessionStorage: description: |- SessionStorage configures session storage for stateful horizontal scaling. @@ -7186,6 +7194,14 @@ spec: - message: at least one of shared, perUser, or tools must be configured rule: has(self.shared) || has(self.perUser) || (has(self.tools) && size(self.tools) > 0) + redactToolResultSecrets: + default: false + description: |- + RedactToolResultSecrets enables best-effort credential-shape scanning + (see pkg/mcp/secretscan) on tools/call results before they are relayed + to the client: matches are redacted in place. Opt-in (default false); + enable when a backend this vMCP aggregates is not fully trusted. + type: boolean sessionStorage: description: |- SessionStorage configures session storage for stateful horizontal scaling. diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpservers.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpservers.yaml index cc26407b74..08959a6ebe 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpservers.yaml @@ -518,6 +518,18 @@ spec: - message: at least one of shared, perUser, or tools must be configured rule: has(self.shared) || has(self.perUser) || (has(self.tools) && size(self.tools) > 0) + redactToolResultSecrets: + default: false + description: |- + RedactToolResultSecrets enables best-effort credential-shape scanning + on tools/call responses (AWS/GitHub/Slack/Google/Stripe keys, JWTs, PEM + private keys): matches are redacted before the response reaches the + client. Off by default; enable when the backend MCP server is not + fully trusted. This setting is ONLY applicable when Transport is + "stdio" (the streamable-HTTP proxy path) -- it has no effect when + Transport is "streamable-http" or "sse" (those reverse-proxy to an + already-HTTP backend without inspecting message content). + type: boolean replicas: description: |- Replicas is the desired number of proxy runner (thv run) pod replicas. @@ -1462,6 +1474,18 @@ spec: - message: at least one of shared, perUser, or tools must be configured rule: has(self.shared) || has(self.perUser) || (has(self.tools) && size(self.tools) > 0) + redactToolResultSecrets: + default: false + description: |- + RedactToolResultSecrets enables best-effort credential-shape scanning + on tools/call responses (AWS/GitHub/Slack/Google/Stripe keys, JWTs, PEM + private keys): matches are redacted before the response reaches the + client. Off by default; enable when the backend MCP server is not + fully trusted. This setting is ONLY applicable when Transport is + "stdio" (the streamable-HTTP proxy path) -- it has no effect when + Transport is "streamable-http" or "sse" (those reverse-proxy to an + already-HTTP backend without inspecting message content). + type: boolean replicas: description: |- Replicas is the desired number of proxy runner (thv run) pod replicas. 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 4738f3df92..89e7cec9e4 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -3198,6 +3198,14 @@ spec: - message: at least one of shared, perUser, or tools must be configured rule: has(self.shared) || has(self.perUser) || (has(self.tools) && size(self.tools) > 0) + redactToolResultSecrets: + default: false + description: |- + RedactToolResultSecrets enables best-effort credential-shape scanning + (see pkg/mcp/secretscan) on tools/call results before they are relayed + to the client: matches are redacted in place. Opt-in (default false); + enable when a backend this vMCP aggregates is not fully trusted. + type: boolean sessionStorage: description: |- SessionStorage configures session storage for stateful horizontal scaling. @@ -7189,6 +7197,14 @@ spec: - message: at least one of shared, perUser, or tools must be configured rule: has(self.shared) || has(self.perUser) || (has(self.tools) && size(self.tools) > 0) + redactToolResultSecrets: + default: false + description: |- + RedactToolResultSecrets enables best-effort credential-shape scanning + (see pkg/mcp/secretscan) on tools/call results before they are relayed + to the client: matches are redacted in place. Opt-in (default false); + enable when a backend this vMCP aggregates is not fully trusted. + type: boolean sessionStorage: description: |- SessionStorage configures session storage for stateful horizontal scaling. diff --git a/docs/cli/thv_run.md b/docs/cli/thv_run.md index 1ff9313604..8d97e46e2a 100644 --- a/docs/cli/thv_run.md +++ b/docs/cli/thv_run.md @@ -159,6 +159,7 @@ thv run [flags] SERVER_OR_IMAGE_OR_PROTOCOL [-- ARGS...] --proxy-mode string Proxy mode for stdio (streamable-http or sse (deprecated, will be removed)) (default "streamable-http") --proxy-port int Port for the HTTP proxy to listen on (host port) -p, --publish stringArray Publish a container's port(s) to the host (format: hostPort:containerPort) + --redact-tool-result-secrets Scan tools/call responses for credential-shaped content (AWS/GitHub/Slack/Google/Stripe keys, JWTs, PEM private keys, generic Bearer tokens) and redact matches before relaying them to the client. Off by default; enable when the backend MCP server is not fully trusted. --remote-auth Enable OAuth/OIDC authentication to remote MCP server (default false) --remote-auth-authorize-url string OAuth authorization endpoint URL (alternative to --remote-auth-issuer for non-OIDC OAuth) --remote-auth-bearer-token string Bearer token for remote server authentication (alternative to OAuth) diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md index 4ca0746e9a..1a1d80b260 100644 --- a/docs/operator/crd-api.md +++ b/docs/operator/crd-api.md @@ -421,6 +421,7 @@ _Appears in:_ | `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: \{\}
| +| `redactToolResultSecrets` _boolean_ | RedactToolResultSecrets enables best-effort credential-shape scanning
(see pkg/mcp/secretscan) on tools/call results before they are relayed
to the client: matches are redacted in place. Opt-in (default false);
enable when a backend this vMCP aggregates is not fully trusted. | false | Optional: \{\}
| #### vmcp.config.ConflictResolutionConfig @@ -3303,6 +3304,7 @@ _Appears in:_ | `authServerRef` _[api.v1beta1.AuthServerRef](#apiv1beta1authserverref)_ | AuthServerRef optionally references a resource that configures an embedded
OAuth 2.0/OIDC authorization server to authenticate MCP clients.
Currently the only supported kind is MCPExternalAuthConfig (type: embeddedAuthServer). | | Optional: \{\}
| | `telemetryConfigRef` _[api.v1beta1.MCPTelemetryConfigReference](#apiv1beta1mcptelemetryconfigreference)_ | TelemetryConfigRef references an MCPTelemetryConfig resource for shared telemetry configuration.
The referenced MCPTelemetryConfig must exist in the same namespace as this MCPServer.
Cross-namespace references are not supported for security and isolation reasons. | | Optional: \{\}
| | `trustProxyHeaders` _boolean_ | TrustProxyHeaders indicates whether to trust X-Forwarded-* headers from reverse proxies
When enabled, the proxy will use X-Forwarded-Proto, X-Forwarded-Host, X-Forwarded-Port,
and X-Forwarded-Prefix headers to construct endpoint URLs | false | Optional: \{\}
| +| `redactToolResultSecrets` _boolean_ | RedactToolResultSecrets enables best-effort credential-shape scanning
on tools/call responses (AWS/GitHub/Slack/Google/Stripe keys, JWTs, PEM
private keys): matches are redacted before the response reaches the
client. Off by default; enable when the backend MCP server is not
fully trusted. This setting is ONLY applicable when Transport is
"stdio" (the streamable-HTTP proxy path) -- it has no effect when
Transport is "streamable-http" or "sse" (those reverse-proxy to an
already-HTTP backend without inspecting message content). | false | Optional: \{\}
| | `endpointPrefix` _string_ | EndpointPrefix is the path prefix to prepend to SSE endpoint URLs.
This is used to handle path-based ingress routing scenarios where the ingress
strips a path prefix before forwarding to the backend. | | Optional: \{\}
| | `groupRef` _[api.v1beta1.MCPGroupRef](#apiv1beta1mcpgroupref)_ | GroupRef references the MCPGroup this server belongs to.
The referenced MCPGroup must be in the same namespace. | | 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: \{\}
| diff --git a/docs/server/docs.go b/docs/server/docs.go index 54cee99290..cd2115f178 100644 --- a/docs/server/docs.go +++ b/docs/server/docs.go @@ -1646,6 +1646,10 @@ const docTemplate = `{ "description": "RateLimitNamespace is the Kubernetes namespace for Redis key derivation.", "type": "string" }, + "redact_tool_result_secrets": { + "description": "RedactToolResultSecrets enables best-effort credential-shape scanning\non tools/call responses relayed by the proxy (streamable-HTTP or the\ntransparent reverse-proxy, whichever the transport resolves to):\nmatches are redacted before the response reaches the client. Opt-in\n(default false); enable it when the backend MCP server is not fully\ntrusted.", + "type": "boolean" + }, "registry_api_url": { "description": "RegistryAPIURL is the registry API URL that served this server's metadata.\nEmpty when the server was not discovered via registry lookup.", "type": "string" diff --git a/docs/server/swagger.json b/docs/server/swagger.json index e44f4aeb4b..27283315ce 100644 --- a/docs/server/swagger.json +++ b/docs/server/swagger.json @@ -1639,6 +1639,10 @@ "description": "RateLimitNamespace is the Kubernetes namespace for Redis key derivation.", "type": "string" }, + "redact_tool_result_secrets": { + "description": "RedactToolResultSecrets enables best-effort credential-shape scanning\non tools/call responses relayed by the proxy (streamable-HTTP or the\ntransparent reverse-proxy, whichever the transport resolves to):\nmatches are redacted before the response reaches the client. Opt-in\n(default false); enable it when the backend MCP server is not fully\ntrusted.", + "type": "boolean" + }, "registry_api_url": { "description": "RegistryAPIURL is the registry API URL that served this server's metadata.\nEmpty when the server was not discovered via registry lookup.", "type": "string" diff --git a/docs/server/swagger.yaml b/docs/server/swagger.yaml index fa19d25384..381cd4e7bd 100644 --- a/docs/server/swagger.yaml +++ b/docs/server/swagger.yaml @@ -1785,6 +1785,15 @@ components: description: RateLimitNamespace is the Kubernetes namespace for Redis key derivation. type: string + redact_tool_result_secrets: + description: |- + RedactToolResultSecrets enables best-effort credential-shape scanning + on tools/call responses relayed by the proxy (streamable-HTTP or the + transparent reverse-proxy, whichever the transport resolves to): + matches are redacted before the response reaches the client. Opt-in + (default false); enable it when the backend MCP server is not fully + trusted. + type: boolean registry_api_url: description: |- RegistryAPIURL is the registry API URL that served this server's metadata. diff --git a/pkg/mcp/secretscan/secretscan.go b/pkg/mcp/secretscan/secretscan.go new file mode 100644 index 0000000000..88efe71c8c --- /dev/null +++ b/pkg/mcp/secretscan/secretscan.go @@ -0,0 +1,147 @@ +// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +// Package secretscan provides a best-effort content inspector for MCP +// tool-call results relayed through a ToolHive proxy. +// +// Background: the proxy sits between the calling client (e.g. an LLM agent) +// and an MCP backend server. The backend is not fully trusted -- it may be +// misconfigured, compromised, or malicious -- yet its tool-call output is +// otherwise forwarded to the client byte-for-byte on both the streamable-HTTP +// and legacy SSE transports (see pkg/transport/proxy/streamable and +// pkg/transport/proxy/transparent). A backend that returns credential-shaped +// text in a tool result would have that text delivered straight through, +// with no containment boundary. This package closes that specific gap: it +// recognizes common credential shapes in `tools/call` response text content +// and redacts them in place before the response reaches the client. +// +// This is defense-in-depth, not a content firewall: it only pattern-matches +// well-known credential formats in TextContent. It does not decode or scan +// binary/base64 payloads (ImageContent, AudioContent, embedded resource +// blobs), and it cannot catch secrets that don't match a known shape. Callers +// should treat a scan failure (malformed result JSON) as non-fatal and +// forward the original content unchanged -- this package must never be the +// reason a legitimate tool call breaks. +package secretscan + +import ( + "encoding/json" + "fmt" + "regexp" + + sdkmcp "github.com/stacklok/toolhive-core/mcpcompat/mcp" +) + +// redactionPlaceholder replaces a matched secret. It intentionally carries no +// information about the matched value (not even its length or pattern name) +// so the redaction itself cannot leak anything about the secret it replaced. +const redactionPlaceholder = "[REDACTED-BY-TOOLHIVE]" + +// patterns lists the credential shapes scanned for. Each is a high-confidence, +// low-false-positive match on a known secret format; deliberately narrow +// rather than a generic "assignment to a sensitive-looking key name" heuristic, +// which would false-positive constantly on ordinary tool output. +var patterns = []*regexp.Regexp{ + // AWS access key ID. + regexp.MustCompile(`\bAKIA[0-9A-Z]{16}\b`), + // GitHub personal access / app / OAuth / refresh tokens. + regexp.MustCompile(`\bgh[pousr]_[0-9A-Za-z]{36,}\b`), + regexp.MustCompile(`\bgithub_pat_[0-9A-Za-z_]{22,}\b`), + // Slack tokens (bot/user/app/legacy). + regexp.MustCompile(`\bxox[baprs]-[0-9A-Za-z-]{10,}\b`), + // Google API key. + regexp.MustCompile(`\bAIza[0-9A-Za-z_-]{35}\b`), + // Stripe live/test secret keys. + regexp.MustCompile(`\bsk_(?:live|test)_[0-9A-Za-z]{16,}\b`), + // Generic JWT (three dot-separated base64url segments). + regexp.MustCompile(`\bey[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\b`), + // PEM-encoded private key blocks (RSA/EC/PKCS8/OpenSSH/generic). + regexp.MustCompile(`(?s)-----BEGIN [A-Z ]*PRIVATE KEY-----.*?-----END [A-Z ]*PRIVATE KEY-----`), + regexp.MustCompile(`(?s)-----BEGIN OPENSSH PRIVATE KEY-----.*?-----END OPENSSH PRIVATE KEY-----`), + // Generic "Authorization: Bearer " shape. Unlike the patterns + // above, this doesn't identify a specific issuer -- it catches any + // opaque bearer credential by the way it is carried, which is the most + // common shape for exfiltrated API/session tokens that don't match a + // named provider's format. + regexp.MustCompile(`(?i)\bBearer\s+[A-Za-z0-9\-._~+/=]{8,}\b`), +} + +// Result reports what ScanAndRedactToolCallResult did. +type Result struct { + // Redacted is the (possibly modified) result payload, always safe to + // forward to the client. + Redacted json.RawMessage + // Matched is true if one or more patterns matched and were redacted. + Matched bool +} + +// ScanAndRedactToolCallResult inspects the `result` payload of a +// `tools/call` JSON-RPC response and redacts any TextContent entries that +// match a known credential shape. +// +// It is best-effort and fails open: if raw cannot be decoded as an MCP +// CallToolResult, it is returned unchanged with Matched=false and a non-nil +// error describing why decoding failed, so the caller can log it without +// treating it as a reason to block the response. +func ScanAndRedactToolCallResult(raw json.RawMessage) (Result, error) { + if len(raw) == 0 { + return Result{Redacted: raw}, nil + } + + var result sdkmcp.CallToolResult + if err := json.Unmarshal(raw, &result); err != nil { + return Result{Redacted: raw}, fmt.Errorf("decoding tool call result: %w", err) + } + + if !RedactContentInPlace(result.Content) { + return Result{Redacted: raw}, nil + } + + encoded, err := json.Marshal(result) + if err != nil { + // Should not happen -- result round-tripped through the same type's + // (Un)MarshalJSON -- but fail open rather than block the response. + return Result{Redacted: raw}, fmt.Errorf("re-encoding redacted tool call result: %w", err) + } + return Result{Redacted: encoded, Matched: true}, nil +} + +// RedactContentInPlace scans content for credential-shaped text (see the +// package doc) and redacts matches in place, entry by entry. Returns true if +// anything matched. Non-text content (images, audio, embedded resources) is +// left untouched -- this package does not decode binary/base64 payloads. +// +// Exported so callers that already hold a decoded []sdkmcp.Content -- e.g. +// pkg/vmcp/server, which builds a CallToolResult's Content from the core's +// domain result without ever serializing to JSON -- can redact without a +// round trip through ScanAndRedactToolCallResult's JSON (de)serialization. +func RedactContentInPlace(content []sdkmcp.Content) bool { + matched := false + for i, c := range content { + text, ok := c.(sdkmcp.TextContent) + if !ok { + continue + } + redactedText, hit := redactText(text.Text) + if !hit { + continue + } + matched = true + text.Text = redactedText + content[i] = text + } + return matched +} + +// redactText replaces every pattern match in s with redactionPlaceholder. +// Returns the (possibly unmodified) string and whether anything matched. +func redactText(s string) (string, bool) { + matched := false + for _, p := range patterns { + if p.MatchString(s) { + matched = true + s = p.ReplaceAllString(s, redactionPlaceholder) + } + } + return s, matched +} diff --git a/pkg/mcp/secretscan/secretscan_test.go b/pkg/mcp/secretscan/secretscan_test.go new file mode 100644 index 0000000000..e6529b7715 --- /dev/null +++ b/pkg/mcp/secretscan/secretscan_test.go @@ -0,0 +1,85 @@ +// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package secretscan + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestScanAndRedactToolCallResult_RedactsKnownCredentialShapes(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + text string + }{ + {"aws access key", "your key is " + "AKIA" + strings.Repeat("Q", 16) + ", keep it safe"}, + {"github pat", "token: ghp_" + strings.Repeat("a", 36)}, + {"slack token", "xoxb-" + strings.Repeat("1", 10) + "-" + strings.Repeat("a", 16)}, + {"google api key", "AIza" + strings.Repeat("A", 35)}, + {"stripe secret key", "sk_live_" + strings.Repeat("a", 24)}, + {"jwt", "ey" + strings.Repeat("A", 12) + "." + strings.Repeat("B", 12) + "." + strings.Repeat("C", 12)}, + {"pem private key", "-----BEGIN " + "RSA PRIVATE KEY-----" + "\nMIIBogIBAAJ...\n" + "-----END " + "RSA PRIVATE KEY-----"}, + {"generic bearer token", "Authorization: Bearer " + strings.Repeat("x", 24)}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + raw := callToolResultJSON(t, tc.text) + + result, err := ScanAndRedactToolCallResult(raw) + require.NoError(t, err) + require.True(t, result.Matched) + require.NotContains(t, string(result.Redacted), tc.text) + require.Contains(t, string(result.Redacted), redactionPlaceholder) + }) + } +} + +func TestScanAndRedactToolCallResult_LeavesOrdinaryTextUnchanged(t *testing.T) { + t.Parallel() + + raw := callToolResultJSON(t, "the weather in NYC is 72F and sunny") + + result, err := ScanAndRedactToolCallResult(raw) + require.NoError(t, err) + require.False(t, result.Matched) + require.JSONEq(t, string(raw), string(result.Redacted)) +} + +func TestScanAndRedactToolCallResult_FailsOpenOnMalformedInput(t *testing.T) { + t.Parallel() + + raw := json.RawMessage(`{not valid json`) + + result, err := ScanAndRedactToolCallResult(raw) + require.Error(t, err) + require.False(t, result.Matched) + require.Equal(t, raw, result.Redacted) +} + +func TestScanAndRedactToolCallResult_EmptyInput(t *testing.T) { + t.Parallel() + + result, err := ScanAndRedactToolCallResult(nil) + require.NoError(t, err) + require.False(t, result.Matched) +} + +func callToolResultJSON(t *testing.T, text string) json.RawMessage { + t.Helper() + payload := map[string]any{ + "content": []map[string]any{ + {"type": "text", "text": text}, + }, + } + b, err := json.Marshal(payload) + require.NoError(t, err) + return b +} diff --git a/pkg/runner/config.go b/pkg/runner/config.go index 268c6692a8..620cfbbd73 100644 --- a/pkg/runner/config.go +++ b/pkg/runner/config.go @@ -213,6 +213,14 @@ type RunConfig struct { // version string (an absent header is always accepted in either mode). StrictProtocolValidation bool `json:"strict_protocol_validation,omitempty" yaml:"strict_protocol_validation,omitempty"` + // RedactToolResultSecrets enables best-effort credential-shape scanning + // on tools/call responses relayed by the proxy (streamable-HTTP or the + // transparent reverse-proxy, whichever the transport resolves to): + // matches are redacted before the response reaches the client. Opt-in + // (default false); enable it when the backend MCP server is not fully + // trusted. + RedactToolResultSecrets bool `json:"redact_tool_result_secrets,omitempty" yaml:"redact_tool_result_secrets,omitempty"` + // Stateless indicates the server only supports POST (no SSE/GET). // When true, the proxy returns 405 for incoming GET requests and uses a // POST-based health check instead of the default GET probe. diff --git a/pkg/runner/config_builder.go b/pkg/runner/config_builder.go index 417c76a23f..36d61e8c1f 100644 --- a/pkg/runner/config_builder.go +++ b/pkg/runner/config_builder.go @@ -386,6 +386,17 @@ func WithStrictProtocolValidation(strict bool) RunConfigBuilderOption { } } +// WithRedactToolResultSecrets sets whether the streamable HTTP proxy scans +// tools/call responses for credential-shaped content and redacts matches +// before relaying them to the client. Opt-in (default false); enable it +// when the backend MCP server is not fully trusted. +func WithRedactToolResultSecrets(enabled bool) RunConfigBuilderOption { + return func(b *runConfigBuilder) error { + b.config.RedactToolResultSecrets = enabled + return nil + } +} + // WithStateless declares the server is stateless (POST-only, no SSE). func WithStateless(stateless bool) RunConfigBuilderOption { return func(b *runConfigBuilder) error { diff --git a/pkg/runner/config_builder_test.go b/pkg/runner/config_builder_test.go index 0087d97ffd..461d1095ef 100644 --- a/pkg/runner/config_builder_test.go +++ b/pkg/runner/config_builder_test.go @@ -1569,6 +1569,33 @@ func TestWithStrictProtocolValidation(t *testing.T) { } } +// TestWithRedactToolResultSecrets verifies the builder option sets +// RunConfig.RedactToolResultSecrets, mirroring WithStrictProtocolValidation's +// plumbing (see cmd/thv/app/run_flags.go's --redact-tool-result-secrets flag). +func TestWithRedactToolResultSecrets(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + enabled bool + }{ + {name: "enabled", enabled: true}, + {name: "disabled (default)", enabled: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + builder := &runConfigBuilder{config: NewRunConfig()} + err := WithRedactToolResultSecrets(tt.enabled)(builder) + + require.NoError(t, err) + assert.Equal(t, tt.enabled, builder.config.RedactToolResultSecrets) + }) + } +} + func TestResolveRegistryServerName(t *testing.T) { t.Parallel() diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index 41e978ddf6..e7d03d296b 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -226,6 +226,7 @@ func (r *Runner) Run(ctx context.Context) error { Debug: r.Config.Debug, TrustProxyHeaders: r.Config.TrustProxyHeaders, StrictProtocolValidation: r.Config.StrictProtocolValidation, + RedactToolResultSecrets: r.Config.RedactToolResultSecrets, EndpointPrefix: r.Config.EndpointPrefix, SessionTTL: effectiveSessionTTL, } diff --git a/pkg/transport/factory.go b/pkg/transport/factory.go index f54fb13a1f..28be88923c 100644 --- a/pkg/transport/factory.go +++ b/pkg/transport/factory.go @@ -53,6 +53,7 @@ func (*Factory) Create(config types.Config, opts ...Option) (types.Transport, er ) stdio.SetProxyMode(config.ProxyMode) stdio.SetStrictProtocolValidation(config.StrictProtocolValidation) + stdio.SetSecretRedaction(config.RedactToolResultSecrets) if config.SessionStorage != nil { stdio.SetSessionStorage(config.SessionStorage) } @@ -82,6 +83,7 @@ func (*Factory) Create(config types.Config, opts ...Option) (types.Transport, er ) httpTransport.sessionStorage = config.SessionStorage httpTransport.sessionTTL = config.SessionTTL + httpTransport.redactToolResultSecrets = config.RedactToolResultSecrets tr = httpTransport case types.TransportTypeStreamableHTTP: httpTransport := NewHTTPTransport( @@ -101,6 +103,7 @@ func (*Factory) Create(config types.Config, opts ...Option) (types.Transport, er ) httpTransport.sessionStorage = config.SessionStorage httpTransport.sessionTTL = config.SessionTTL + httpTransport.redactToolResultSecrets = config.RedactToolResultSecrets tr = httpTransport case types.TransportTypeInspector: // HTTP transport is not implemented yet diff --git a/pkg/transport/http.go b/pkg/transport/http.go index 7f9bb174b9..2abd89d486 100644 --- a/pkg/transport/http.go +++ b/pkg/transport/http.go @@ -62,6 +62,12 @@ type HTTPTransport struct { // stateless indicates the server is POST-only (no SSE/GET support) stateless bool + // redactToolResultSecrets controls whether the underlying transparent + // proxy scans tools/call responses for credential-shaped content and + // redacts matches before relaying them to the client. Default false. + // See transparent.WithSecretRedaction. + redactToolResultSecrets bool + // tokenSource is the OAuth token source for remote authentication tokenSource oauth2.TokenSource @@ -443,6 +449,7 @@ func (t *HTTPTransport) buildProxyOptions(remoteBasePath, remoteRawQuery string) if t.sessionStorage != nil { opts = append(opts, transparent.WithSessionStorage(t.sessionStorage)) } + opts = append(opts, transparent.WithSecretRedaction(t.redactToolResultSecrets)) return opts } diff --git a/pkg/transport/proxy/streamable/secretscan_test.go b/pkg/transport/proxy/streamable/secretscan_test.go new file mode 100644 index 0000000000..590d9b7b37 --- /dev/null +++ b/pkg/transport/proxy/streamable/secretscan_test.go @@ -0,0 +1,92 @@ +// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package streamable + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" + "golang.org/x/exp/jsonrpc2" + + sdkmcp "github.com/stacklok/toolhive-core/mcpcompat/mcp" +) + +func toolCallResultResponse(t *testing.T, text string) *jsonrpc2.Response { + t.Helper() + resp, err := jsonrpc2.NewResponse( + jsonrpc2.Int64ID(1), + map[string]any{ + "content": []map[string]any{ + {"type": "text", "text": text}, + }, + }, + nil, + ) + require.NoError(t, err) + return resp +} + +func TestInspectToolCallResponse_RedactsCredentialShapedToolResult(t *testing.T) { + t.Parallel() + + ghToken := "ghp_" + strings.Repeat("a", 36) + resp := toolCallResultResponse(t, "here is the token: "+ghToken) + p := &HTTPProxy{redactToolResultSecrets: true} + + out := p.inspectToolCallResponse(string(sdkmcp.MethodToolsCall), resp) + + outResp, ok := out.(*jsonrpc2.Response) + require.True(t, ok) + require.NotContains(t, string(outResp.Result), ghToken) + require.Contains(t, string(outResp.Result), "REDACTED-BY-TOOLHIVE") +} + +func TestInspectToolCallResponse_DisabledByDefault(t *testing.T) { + t.Parallel() + + ghToken := "ghp_" + strings.Repeat("a", 36) + resp := toolCallResultResponse(t, ghToken) + p := &HTTPProxy{} // redactToolResultSecrets left at its zero value (false) + + out := p.inspectToolCallResponse(string(sdkmcp.MethodToolsCall), resp) + + require.Same(t, resp, out) +} + +func TestInspectToolCallResponse_IgnoresNonToolCallMethods(t *testing.T) { + t.Parallel() + + ghToken := "ghp_" + strings.Repeat("a", 36) + resp := toolCallResultResponse(t, ghToken) + p := &HTTPProxy{redactToolResultSecrets: true} + + out := p.inspectToolCallResponse("resources/read", resp) + + require.Same(t, resp, out) +} + +func TestInspectToolCallResponse_IgnoresErrorResponses(t *testing.T) { + t.Parallel() + + resp, err := jsonrpc2.NewResponse(jsonrpc2.Int64ID(1), nil, jsonrpc2.NewError(-32000, "boom")) + require.NoError(t, err) + p := &HTTPProxy{redactToolResultSecrets: true} + + out := p.inspectToolCallResponse(string(sdkmcp.MethodToolsCall), resp) + + require.Same(t, resp, out) +} + +func TestInspectToolCallResponse_IgnoresNonResponseMessages(t *testing.T) { + t.Parallel() + + req, err := jsonrpc2.NewNotification("notifications/message", nil) + require.NoError(t, err) + p := &HTTPProxy{redactToolResultSecrets: true} + + out := p.inspectToolCallResponse(string(sdkmcp.MethodToolsCall), req) + + require.Same(t, req, out) +} diff --git a/pkg/transport/proxy/streamable/streamable_proxy.go b/pkg/transport/proxy/streamable/streamable_proxy.go index 6d339d1b7e..7622adcab1 100644 --- a/pkg/transport/proxy/streamable/streamable_proxy.go +++ b/pkg/transport/proxy/streamable/streamable_proxy.go @@ -27,6 +27,7 @@ import ( "github.com/stacklok/toolhive/pkg/diagnostics" "github.com/stacklok/toolhive/pkg/healthcheck" "github.com/stacklok/toolhive/pkg/mcp" + "github.com/stacklok/toolhive/pkg/mcp/secretscan" "github.com/stacklok/toolhive/pkg/transport/session" "github.com/stacklok/toolhive/pkg/transport/types" ) @@ -135,6 +136,12 @@ type HTTPProxy struct { // 2025-03-26 when the header is missing. Set via WithStrictProtocolValidation. strictProtocolValidation bool + // redactToolResultSecrets enables scanning tools/call responses for + // credential-shaped content before relaying them to the client (see + // pkg/mcp/secretscan). Default false: the MCP backend is often + // operator-trusted. Set via WithSecretRedaction. + redactToolResultSecrets bool + // Health checker healthChecker *healthcheck.HealthChecker @@ -215,6 +222,15 @@ func WithStrictProtocolValidation(enabled bool) Option { return func(p *HTTPProxy) { p.strictProtocolValidation = enabled } } +// WithSecretRedaction enables best-effort credential-shape scanning (see +// pkg/mcp/secretscan) on tools/call responses before they are relayed to the +// client. Opt-in (default false): the MCP backend behind this proxy is often +// operator-trusted, and scanning adds per-response overhead, so this is only +// worth enabling when the backend is not fully trusted. +func WithSecretRedaction(enabled bool) Option { + return func(p *HTTPProxy) { p.redactToolResultSecrets = enabled } +} + // NewHTTPProxy creates a new HTTPProxy for streamable HTTP transport. func NewHTTPProxy( host string, @@ -663,6 +679,8 @@ func (p *HTTPProxy) handleSingleRequest( return } + msg = p.inspectToolCallResponse(req.Method, msg) + if setSessionHeader { w.Header().Set("Mcp-Session-Id", sessID) } @@ -735,7 +753,7 @@ func (p *HTTPProxy) handleSingleRequestSSE( // Progress does not end the request; keep waiting for more // progress or the final response. case msg := <-waitCh: - p.writeSingleRequestSSEFinalResponse(w, flusher, msg, ck) + p.writeSingleRequestSSEFinalResponse(w, flusher, req.Method, msg, ck) return case <-ctx.Done(): writeSSEErrorEvent(w, flusher, req.ID, ctx.Err()) @@ -756,7 +774,7 @@ func (p *HTTPProxy) handleSingleRequestSSE( // logged; the client simply does not get a final frame in that case, matching // the prior (pre-progress) behavior's error handling. func (p *HTTPProxy) writeSingleRequestSSEFinalResponse( - w http.ResponseWriter, flusher http.Flusher, msg jsonrpc2.Message, ck string, + w http.ResponseWriter, flusher http.Flusher, method string, msg jsonrpc2.Message, ck string, ) { finalMsg := msg if r, ok := msg.(*jsonrpc2.Response); ok && r.ID.IsValid() { @@ -767,6 +785,7 @@ func (p *HTTPProxy) writeSingleRequestSSEFinalResponse( } finalMsg = restored } + finalMsg = p.inspectToolCallResponse(method, finalMsg) data, err := jsonrpc2.EncodeMessage(finalMsg) if err != nil { @@ -778,6 +797,35 @@ func (p *HTTPProxy) writeSingleRequestSSEFinalResponse( } } +// inspectToolCallResponse applies best-effort secret redaction (see +// pkg/mcp/secretscan) to the result of a tools/call response before it is +// forwarded to the client, when redactToolResultSecrets is enabled (see +// WithSecretRedaction). The MCP backend behind this proxy is not fully +// trusted (it may be misconfigured, compromised, or malicious), so its tool +// output is scanned for credential-shaped text before the proxy relays it. +// Any other method, a non-Response message, an error response, or a result +// that fails to decode is returned unchanged -- this check must never be the +// reason a legitimate tool call breaks. +func (p *HTTPProxy) inspectToolCallResponse(method string, msg jsonrpc2.Message) jsonrpc2.Message { + if !p.redactToolResultSecrets || method != string(sdkmcp.MethodToolsCall) { + return msg + } + resp, ok := msg.(*jsonrpc2.Response) + if !ok || resp.Error != nil || len(resp.Result) == 0 { + return msg + } + scan, err := secretscan.ScanAndRedactToolCallResult(resp.Result) + if err != nil { + slog.Debug("tool call result secret scan skipped", "error", err) + return msg + } + if !scan.Matched { + return msg + } + slog.Warn("redacted credential-shaped content in tool call result") + return &jsonrpc2.Response{Result: scan.Redacted, ID: resp.ID} +} + // writeSSEErrorEvent writes a best-effort JSON-RPC error as a single SSE // message event (via writeSSEData), for a request whose response headers // (200 + text/event-stream) have already been sent -- an HTTP error status can diff --git a/pkg/transport/proxy/transparent/response_processor.go b/pkg/transport/proxy/transparent/response_processor.go index a6e0ca765f..cbf0afa2f5 100644 --- a/pkg/transport/proxy/transparent/response_processor.go +++ b/pkg/transport/proxy/transparent/response_processor.go @@ -37,17 +37,23 @@ func (*NoOpResponseProcessor) ShouldProcess(_ *http.Response) bool { } // createResponseProcessor is a factory function that creates the appropriate -// response processor based on transport type. +// response processor based on transport type. redactSecrets enables the +// pkg/mcp/secretscan-backed scan of tools/call results (see +// WithSecretRedaction); false preserves prior behavior exactly. func createResponseProcessor( transportType string, proxy *TransparentProxy, endpointPrefix string, trustProxyHeaders bool, + redactSecrets bool, ) ResponseProcessor { switch transportType { case types.TransportTypeSSE.String(): - return NewSSEResponseProcessor(proxy, endpointPrefix, trustProxyHeaders) + return NewSSEResponseProcessor(proxy, endpointPrefix, trustProxyHeaders, redactSecrets) case types.TransportTypeStreamableHTTP.String(): + if redactSecrets { + return NewSecretRedactionResponseProcessor() + } return &NoOpResponseProcessor{} default: // Default to no-op for unknown transport types diff --git a/pkg/transport/proxy/transparent/secret_redaction_response_processor.go b/pkg/transport/proxy/transparent/secret_redaction_response_processor.go new file mode 100644 index 0000000000..51ff2d7a79 --- /dev/null +++ b/pkg/transport/proxy/transparent/secret_redaction_response_processor.go @@ -0,0 +1,226 @@ +// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package transparent + +import ( + "bufio" + "bytes" + "encoding/json" + "fmt" + "io" + "log/slog" + "mime" + "net/http" + "strconv" + "strings" + + "github.com/stacklok/toolhive/pkg/mcp/secretscan" +) + +// maxSecretScanBodyBytes bounds how much of a non-streaming response body +// this processor buffers to scan. A response larger than this is forwarded +// unscanned rather than risking unbounded memory growth on a hostile or +// oversized upstream body. +const maxSecretScanBodyBytes = 8 << 20 // 8 MB, matches bodylimit.DefaultMaxRequestBodySize + +// SecretRedactionResponseProcessor scans MCP responses emitted by the +// (untrusted) backend this proxy fronts for credential-shaped content in a +// tools/call result (see pkg/mcp/secretscan) and redacts matches before the +// response reaches the client. Opt-in; see WithSecretRedaction. Constructed +// only when enabled, so unlike other processors it carries no enabled flag +// of its own. +// +// Handles the two response shapes a streamable-HTTP MCP server may emit for +// a single request: a plain application/json body, or a text/event-stream +// response carrying one or more "data:" JSON-RPC frames (MCP allows a server +// to reply to a POST with either shape). +type SecretRedactionResponseProcessor struct{} + +// NewSecretRedactionResponseProcessor creates a new secret-redaction response +// processor. +func NewSecretRedactionResponseProcessor() *SecretRedactionResponseProcessor { + return &SecretRedactionResponseProcessor{} +} + +// ShouldProcess returns true for JSON and SSE response bodies -- the two +// shapes a streamable-HTTP MCP response can take. +func (*SecretRedactionResponseProcessor) ShouldProcess(resp *http.Response) bool { + mediaType, _, _ := mime.ParseMediaType(resp.Header.Get("Content-Type")) + return mediaType == "application/json" || mediaType == "text/event-stream" +} + +// ProcessResponse redacts credential-shaped content from the response body. +func (p *SecretRedactionResponseProcessor) ProcessResponse(resp *http.Response) error { + if !p.ShouldProcess(resp) { + return nil + } + mediaType, _, _ := mime.ParseMediaType(resp.Header.Get("Content-Type")) + if mediaType == "text/event-stream" { + p.processSSE(resp) + return nil + } + return p.processJSON(resp) +} + +// chainedBody re-splices a size-limited read prefix back onto the reader it +// came from, so a body too large to safely buffer is still forwarded intact +// (unscanned) rather than truncated. Close defers to the original body so +// the underlying connection is still released once the client finishes +// reading. +type chainedBody struct { + io.Reader + closer io.Closer +} + +func (c chainedBody) Close() error { return c.closer.Close() } + +// processJSON scans a complete (non-streaming) JSON response body. +func (*SecretRedactionResponseProcessor) processJSON(resp *http.Response) error { + original := resp.Body + limited := io.LimitReader(original, maxSecretScanBodyBytes+1) + data, err := io.ReadAll(limited) + if err != nil { + return fmt.Errorf("reading response body for secret scan: %w", err) + } + if int64(len(data)) > maxSecretScanBodyBytes { + slog.Debug("response body exceeds secret-scan size limit; forwarding unscanned", + "limit_bytes", maxSecretScanBodyBytes) + resp.Body = chainedBody{Reader: io.MultiReader(bytes.NewReader(data), original), closer: original} + return nil + } + if err := original.Close(); err != nil { + slog.Debug("failed to close upstream response body", "error", err) + } + + redacted, changed, scanErr := redactJSONRPCBody(data) + if scanErr != nil || !changed { + // Not a recognizable JSON-RPC envelope, or nothing matched -- forward + // the original bytes unchanged (fail open; a scan miss is not a + // proxy error). + resp.Body = io.NopCloser(bytes.NewReader(data)) + return nil + } + resp.Body = io.NopCloser(bytes.NewReader(redacted)) + resp.ContentLength = int64(len(redacted)) + resp.Header.Set("Content-Length", strconv.Itoa(len(redacted))) + return nil +} + +// processSSE streams an SSE response line by line, redacting each "data:" +// frame's JSON-RPC payload as it is forwarded, so a long-lived stream is +// never fully buffered. +func (*SecretRedactionResponseProcessor) processSSE(resp *http.Response) { + original := resp.Body + pr, pw := io.Pipe() + resp.Body = pr + + go func() { + defer func() { + if err := pw.Close(); err != nil { + slog.Debug("failed to close pipe writer", "error", err) + } + }() + defer func() { + if err := original.Close(); err != nil { + slog.Debug("failed to close upstream response body", "error", err) + } + }() + + scanner := bufio.NewScanner(original) + // Matches sse_response_processor.go's buffer size rationale: the + // default 64KB token limit is too small for a data line carrying a + // sizeable tool result. + scanner.Buffer(make([]byte, 0, 1024), 1024*1024) + + // dataBuf accumulates consecutive raw "data:" lines belonging to the + // SAME SSE event. Per the SSE spec, a compliant client concatenates + // them (joined by "\n") into one logical value before consuming it -- + // scanning each "data:" line in isolation would let a hostile backend + // split a single JSON-RPC message across lines specifically to evade + // this scanner while the real client reassembles it and still sees + // the whole payload. flush reconstructs that same logical value. + var dataBuf []string + flush := func() bool { + if len(dataBuf) == 0 { + return true + } + joined := joinSSEDataLines(dataBuf) + lines := dataBuf + if redacted, changed, err := redactJSONRPCBody([]byte(joined)); err == nil && changed { + lines = []string{"data: " + string(redacted)} + } + dataBuf = nil + for _, l := range lines { + if _, err := pw.Write([]byte(l + "\n")); err != nil { + return false + } + } + return true + } + + for scanner.Scan() { + line := scanner.Text() + if strings.HasPrefix(line, "data:") { + dataBuf = append(dataBuf, line) + continue + } + if !flush() { + return + } + if _, err := pw.Write([]byte(line + "\n")); err != nil { + return + } + } + // A stream that ends without a trailing blank line still has a + // pending event to reassemble and forward. + if !flush() { + return + } + if err := scanner.Err(); err != nil { + slog.Error("failed to scan SSE response body for secret scan", "error", err) + } + }() +} + +// joinSSEDataLines reconstructs the logical value of one SSE event's data +// field from its raw "data:" lines, per the spec: each line's content (after +// stripping the "data:" prefix and at most one leading space) is joined with +// "\n". +func joinSSEDataLines(rawLines []string) string { + contents := make([]string, len(rawLines)) + for i, l := range rawLines { + contents[i] = strings.TrimSpace(strings.TrimPrefix(l, "data:")) + } + return strings.Join(contents, "\n") +} + +// redactJSONRPCBody decodes data as a JSON object, redacts its "result" +// field via secretscan (if present), and returns the re-encoded body. changed +// is false whenever nothing needed redacting -- including when data doesn't +// decode as a JSON-RPC response object at all (e.g. a request, a batch, or a +// malformed frame), which is reported via a non-nil err so callers can fail +// open without treating it as a real error. +func redactJSONRPCBody(data []byte) (redacted []byte, changed bool, err error) { + var envelope map[string]json.RawMessage + if err := json.Unmarshal(data, &envelope); err != nil { + return nil, false, fmt.Errorf("decoding JSON-RPC envelope: %w", err) + } + result, ok := envelope["result"] + if !ok || len(result) == 0 { + return nil, false, nil + } + scan, err := secretscan.ScanAndRedactToolCallResult(result) + if err != nil { + return nil, false, err + } + if !scan.Matched { + return nil, false, nil + } + envelope["result"] = scan.Redacted + out, err := json.Marshal(envelope) + if err != nil { + return nil, false, fmt.Errorf("re-encoding JSON-RPC envelope: %w", err) + } + return out, true, nil +} diff --git a/pkg/transport/proxy/transparent/secret_redaction_response_processor_test.go b/pkg/transport/proxy/transparent/secret_redaction_response_processor_test.go new file mode 100644 index 0000000000..cd0e458e9e --- /dev/null +++ b/pkg/transport/proxy/transparent/secret_redaction_response_processor_test.go @@ -0,0 +1,225 @@ +// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package transparent + +import ( + "bufio" + "net/http" + "net/http/httptest" + "net/http/httputil" + "net/url" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const sentinelToken = "DAST-SENTINEL-TOKEN-DO-NOT-FORWARD" //nolint:gosec // test fixture, not a real credential + +func toolCallResultJSON() string { + return `{"jsonrpc":"2.0","id":1,"result":{"content":[{"type":"text","text":"Authorization: Bearer ` + sentinelToken + `"}]}}` +} + +// toolCallResultJSONSplit returns the same JSON-RPC message as +// toolCallResultJSON, but split into two fragments at a whitespace- +// insignificant JSON boundary (right after a top-level comma) -- valid to +// reassemble with a "\n" join, exactly as an SSE client concatenating two +// consecutive "data:" lines would. Used to simulate a hostile backend +// splitting a message across "data:" lines specifically to evade a +// per-line-only scanner. +func toolCallResultJSONSplit() (first, second string) { + return `{"jsonrpc":"2.0",`, + `"id":1,"result":{"content":[{"type":"text","text":"Authorization: Bearer ` + sentinelToken + `"}]}}` +} + +// buildRedactingProxy wires up a *httputil.ReverseProxy fronting target using +// a TransparentProxy configured with WithSecretRedaction(true) for the given +// transport type -- mirroring the createBasicProxy/modifyResponse harness +// used by the existing tests in this package (see TestStreamingSessionIDDetection). +func buildRedactingProxy(t *testing.T, transportType string, targetURL *url.URL) *httputil.ReverseProxy { + t.Helper() + p := NewTransparentProxyWithOptions( + "127.0.0.1", 0, targetURL.String(), nil, nil, nil, + false, false, transportType, nil, nil, "", false, nil, + WithSecretRedaction(true), + ) + return &httputil.ReverseProxy{ + Rewrite: func(pr *httputil.ProxyRequest) { + pr.SetURL(targetURL) + pr.SetXForwarded() + }, + FlushInterval: -1, + Transport: newTracingTransport(http.DefaultTransport, p), + ModifyResponse: p.modifyResponse, + } +} + +func TestSecretRedaction_StreamableHTTP_JSONResponse(t *testing.T) { + t.Parallel() + + target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(toolCallResultJSON())) + })) + defer target.Close() + targetURL, err := url.Parse(target.URL) + require.NoError(t, err) + + proxy := buildRedactingProxy(t, "streamable-http", targetURL) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, target.URL, nil) + proxy.ServeHTTP(rec, req) + + assert.NotContains(t, rec.Body.String(), sentinelToken, + "sentinel token must not reach the client") + assert.Contains(t, rec.Body.String(), "REDACTED-BY-TOOLHIVE") +} + +func TestSecretRedaction_StreamableHTTP_SSEResponse(t *testing.T) { + t.Parallel() + + target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/event-stream") + w.WriteHeader(http.StatusOK) + w.Write([]byte("data: " + toolCallResultJSON() + "\n\n")) + w.(http.Flusher).Flush() + })) + defer target.Close() + targetURL, err := url.Parse(target.URL) + require.NoError(t, err) + + proxy := buildRedactingProxy(t, "streamable-http", targetURL) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, target.URL, nil) + proxy.ServeHTTP(rec, req) + + assert.NotContains(t, rec.Body.String(), sentinelToken, + "sentinel token must not reach the client") + assert.Contains(t, rec.Body.String(), "REDACTED-BY-TOOLHIVE") +} + +func TestSecretRedaction_LegacySSETransport_DataLine(t *testing.T) { + t.Parallel() + + target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/event-stream") + w.WriteHeader(http.StatusOK) + w.Write([]byte("data: " + toolCallResultJSON() + "\n\n")) + w.(http.Flusher).Flush() + })) + defer target.Close() + targetURL, err := url.Parse(target.URL) + require.NoError(t, err) + + proxy := buildRedactingProxy(t, "sse", targetURL) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, target.URL, nil) + proxy.ServeHTTP(rec, req) + + sc := bufio.NewScanner(rec.Body) + var found bool + for sc.Scan() { + line := sc.Text() + assert.NotContains(t, line, sentinelToken, "sentinel token must not reach the client") + if strings.Contains(line, "REDACTED-BY-TOOLHIVE") { + found = true + } + } + assert.True(t, found, "expected a redacted data line") +} + +func TestSecretRedaction_Disabled_ByDefault(t *testing.T) { + t.Parallel() + + target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(toolCallResultJSON())) + })) + defer target.Close() + targetURL, err := url.Parse(target.URL) + require.NoError(t, err) + + // No WithSecretRedaction option -- default false preserves prior behavior. + p := NewTransparentProxy("127.0.0.1", 0, targetURL.String(), nil, nil, nil, + false, false, "streamable-http", nil, nil, "", false) + proxy := &httputil.ReverseProxy{ + Rewrite: func(pr *httputil.ProxyRequest) { + pr.SetURL(targetURL) + pr.SetXForwarded() + }, + FlushInterval: -1, + Transport: newTracingTransport(http.DefaultTransport, p), + ModifyResponse: p.modifyResponse, + } + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, target.URL, nil) + proxy.ServeHTTP(rec, req) + + assert.Contains(t, rec.Body.String(), sentinelToken, + "disabled by default: response must pass through unmodified") +} + +// TestSecretRedaction_StreamableHTTP_SplitAcrossDataLines is a regression +// test for a bypass: a hostile backend that splits a single JSON-RPC message +// across two "data:" lines (valid per the SSE spec -- a compliant client +// reassembles them) must not evade the scanner, which used to inspect each +// "data:" line in isolation. +func TestSecretRedaction_StreamableHTTP_SplitAcrossDataLines(t *testing.T) { + t.Parallel() + + first, second := toolCallResultJSONSplit() + target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/event-stream") + w.WriteHeader(http.StatusOK) + w.Write([]byte("data: " + first + "\ndata: " + second + "\n\n")) + w.(http.Flusher).Flush() + })) + defer target.Close() + targetURL, err := url.Parse(target.URL) + require.NoError(t, err) + + proxy := buildRedactingProxy(t, "streamable-http", targetURL) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, target.URL, nil) + proxy.ServeHTTP(rec, req) + + assert.NotContains(t, rec.Body.String(), sentinelToken, + "sentinel token split across data: lines must not reach the client") + assert.Contains(t, rec.Body.String(), "REDACTED-BY-TOOLHIVE") +} + +// TestSecretRedaction_LegacySSETransport_SplitAcrossDataLines is the same +// regression as above, for the legacy sse transport type's response +// processor. +func TestSecretRedaction_LegacySSETransport_SplitAcrossDataLines(t *testing.T) { + t.Parallel() + + first, second := toolCallResultJSONSplit() + target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/event-stream") + w.WriteHeader(http.StatusOK) + w.Write([]byte("data: " + first + "\ndata: " + second + "\n\n")) + w.(http.Flusher).Flush() + })) + defer target.Close() + targetURL, err := url.Parse(target.URL) + require.NoError(t, err) + + proxy := buildRedactingProxy(t, "sse", targetURL) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, target.URL, nil) + proxy.ServeHTTP(rec, req) + + body := rec.Body.String() + assert.NotContains(t, body, sentinelToken, + "sentinel token split across data: lines must not reach the client") + assert.Contains(t, body, "REDACTED-BY-TOOLHIVE") +} diff --git a/pkg/transport/proxy/transparent/sse_response_processor.go b/pkg/transport/proxy/transparent/sse_response_processor.go index 52d1ae6364..e6964998f3 100644 --- a/pkg/transport/proxy/transparent/sse_response_processor.go +++ b/pkg/transport/proxy/transparent/sse_response_processor.go @@ -57,12 +57,15 @@ func (c sseRewriteConfig) hasRewriteConfig() bool { var sessionRe = regexp.MustCompile(`sessionId=([0-9A-Fa-f-]+)|"sessionId"\s*:\s*"([^"]+)"`) // SSEResponseProcessor handles SSE-specific response processing including: -// - Session ID extraction from SSE streams -// - Endpoint URL rewriting for path-based routing +// - Session ID extraction from SSE streams +// - Endpoint URL rewriting for path-based routing +// - Optional credential-shape redaction of tools/call results (see +// WithSecretRedaction) type SSEResponseProcessor struct { proxy *TransparentProxy endpointPrefix string trustProxyHeaders bool + redactSecrets bool } // NewSSEResponseProcessor creates a new SSE response processor. @@ -70,11 +73,13 @@ func NewSSEResponseProcessor( proxy *TransparentProxy, endpointPrefix string, trustProxyHeaders bool, + redactSecrets bool, ) *SSEResponseProcessor { return &SSEResponseProcessor{ proxy: proxy, endpointPrefix: endpointPrefix, trustProxyHeaders: trustProxyHeaders, + redactSecrets: redactSecrets, } } @@ -212,43 +217,88 @@ type sseLineProcessor struct { rewriteConfig sseRewriteConfig currentEventType string sessionFound bool + redactSecrets bool + + // dataBuf accumulates consecutive raw "data:" lines belonging to the SAME + // SSE event. Per the SSE spec, a compliant client concatenates them + // (joined by "\n") into one logical value before consuming it -- URL + // rewriting and redaction must operate on that same reassembled value, not + // each "data:" line in isolation, or a hostile backend could split a + // single JSON-RPC message across lines specifically to evade the + // redaction scan while the real client still reassembles and sees it. + dataBuf []string } -// processLine processes a single SSE line and returns the potentially modified line. -func (s *sseLineProcessor) processLine(line string) string { +// processLine processes a single SSE line and returns the lines to emit in +// its place: zero while a "data:" run is being buffered, or more than one +// when a buffered run is flushed ahead of a non-data line. +func (s *sseLineProcessor) processLine(line string) []string { // Parse SSE event type if strings.HasPrefix(line, "event:") { + out := s.flushDataBuf() s.currentEventType = strings.TrimSpace(strings.TrimPrefix(line, "event:")) - return line + return append(out, line) } // Empty line marks the end of an SSE event, reset event type if line == "" { + out := s.flushDataBuf() s.currentEventType = "" - return line + return append(out, line) } - // Process data lines + // Accumulate data lines; extraction/rewrite/redaction happens once the + // run is flushed (see flushDataBuf), on the reassembled value. if strings.HasPrefix(line, "data:") { - return s.processDataLine(line) + s.extractSessionID(line) + s.dataBuf = append(s.dataBuf, line) + return nil } - return line + out := s.flushDataBuf() + return append(out, line) } -// processDataLine handles SSE data lines for session extraction and URL rewriting. -func (s *sseLineProcessor) processDataLine(line string) string { - dataContent := strings.TrimSpace(strings.TrimPrefix(line, "data:")) +// flushDataBuf reassembles any buffered "data:" lines into one logical value +// (per the SSE spec), applies endpoint-URL rewriting and/or secret redaction +// to that value, and returns the line(s) to emit. When neither transform +// changes anything, the original raw lines are returned unchanged so a +// disabled/no-op pass is byte-identical to the pre-buffering behavior. +func (s *sseLineProcessor) flushDataBuf() []string { + if len(s.dataBuf) == 0 { + return nil + } + rawLines := s.dataBuf + s.dataBuf = nil - // Extract session ID for tracking (from any data line) - s.extractSessionID(line) + value := joinSSEDataLines(rawLines) + changed := false - // Rewrite endpoint URLs only for "endpoint" events if s.currentEventType == "endpoint" && s.rewriteConfig.hasRewriteConfig() { - return s.rewriteDataLine(line, dataContent) + rewritten, err := rewriteEndpointURL(value, s.rewriteConfig) + switch { + case err != nil: + //nolint:gosec // G706: logging endpoint URL from SSE stream + slog.Warn("failed to rewrite endpoint URL", "url", value, "error", err) + case rewritten != value: + //nolint:gosec // G706: logging endpoint URLs from SSE stream + slog.Debug("rewrote SSE endpoint URL", "from", value, "to", rewritten) + value = rewritten + changed = true + } } - return line + if s.redactSecrets { + if redacted, ok, err := redactJSONRPCBody([]byte(value)); err == nil && ok { + value = string(redacted) + changed = true + } + } + + if !changed { + return rawLines + } + return []string{"data: " + value} } // extractSessionID extracts and stores the session ID from a data line. @@ -269,24 +319,6 @@ func (s *sseLineProcessor) extractSessionID(line string) { } } -// rewriteDataLine rewrites the URL in an endpoint event's data line. -func (s *sseLineProcessor) rewriteDataLine(line, dataContent string) string { - rewrittenURL, err := rewriteEndpointURL(dataContent, s.rewriteConfig) - if err != nil { - //nolint:gosec // G706: logging endpoint URL from SSE stream - slog.Warn("failed to rewrite endpoint URL", - "url", dataContent, "error", err) - return line - } - if rewrittenURL != dataContent { - //nolint:gosec // G706: logging endpoint URLs from SSE stream - slog.Debug("rewrote SSE endpoint URL", - "from", dataContent, "to", rewrittenURL) - return "data: " + rewrittenURL - } - return line -} - // processSSEStream processes an SSE stream, extracting session IDs and rewriting URLs. func (s *SSEResponseProcessor) processSSEStream(originalBody io.Reader, pw *io.PipeWriter, rewriteConfig sseRewriteConfig) { scanner := bufio.NewScanner(originalBody) @@ -299,10 +331,19 @@ func (s *SSEResponseProcessor) processSSEStream(originalBody io.Reader, pw *io.P processor := &sseLineProcessor{ proxy: s.proxy, rewriteConfig: rewriteConfig, + redactSecrets: s.redactSecrets, } for scanner.Scan() { - line := processor.processLine(scanner.Text()) + for _, line := range processor.processLine(scanner.Text()) { + if _, err := pw.Write([]byte(line + "\n")); err != nil { + return + } + } + } + // A stream that ends without a trailing blank line still has a pending + // event to reassemble and forward. + for _, line := range processor.flushDataBuf() { if _, err := pw.Write([]byte(line + "\n")); err != nil { return } diff --git a/pkg/transport/proxy/transparent/transparent_proxy.go b/pkg/transport/proxy/transparent/transparent_proxy.go index cb8d6d12b2..ee2cb91009 100644 --- a/pkg/transport/proxy/transparent/transparent_proxy.go +++ b/pkg/transport/proxy/transparent/transparent_proxy.go @@ -108,6 +108,12 @@ type TransparentProxy struct { // stateless indicates the server is POST-only (no SSE/GET support) stateless bool + // redactToolResultSecrets enables scanning tools/call responses for + // credential-shaped content and redacting matches before relaying them + // to the client (see pkg/mcp/secretscan). Default false. Set via + // WithSecretRedaction. + redactToolResultSecrets bool + // Callback when health check fails (for remote servers) onHealthCheckFailed types.HealthCheckFailedCallback @@ -269,6 +275,17 @@ func WithStateless() Option { } } +// WithSecretRedaction enables best-effort credential-shape scanning (see +// pkg/mcp/secretscan) on tools/call responses before they are relayed to the +// client. Opt-in (default false): the MCP backend behind this proxy is often +// operator-trusted, and scanning adds per-response overhead, so this is only +// worth enabling when the backend is not fully trusted. +func WithSecretRedaction(enabled bool) Option { + return func(p *TransparentProxy) { + p.redactToolResultSecrets = enabled + } +} + // withHealthCheckPingTimeout sets the health check ping timeout. // This is primarily useful for testing with shorter timeouts. // Ignores non-positive timeouts; default will be used. @@ -497,6 +514,7 @@ func NewTransparentProxyWithOptions( proxy, endpointPrefix, trustProxyHeaders, + proxy.redactToolResultSecrets, ) // Create health checker always for Kubernetes probes diff --git a/pkg/transport/stdio.go b/pkg/transport/stdio.go index 4a8c9b262c..68bcd1dad8 100644 --- a/pkg/transport/stdio.go +++ b/pkg/transport/stdio.go @@ -73,6 +73,12 @@ type StdioTransport struct { // See streamable.WithStrictProtocolValidation. strictProtocolValidation bool + // redactToolResultSecrets controls whether the streamable HTTP proxy + // scans tools/call responses for credential-shaped content and redacts + // matches before relaying them to the client. Default false. See + // streamable.WithSecretRedaction. + redactToolResultSecrets bool + // Mutex for protecting shared state mutex sync.Mutex @@ -152,6 +158,14 @@ func (t *StdioTransport) SetStrictProtocolValidation(strict bool) { t.strictProtocolValidation = strict } +// SetSecretRedaction configures whether the streamable HTTP proxy scans +// tools/call responses for credential-shaped content and redacts matches +// before relaying them to the client. Default false: enable when the +// backend MCP server is not fully trusted. +func (t *StdioTransport) SetSecretRedaction(enabled bool) { + t.redactToolResultSecrets = enabled +} + // SetSessionStorage configures a custom session storage backend. // When set, the underlying proxy will use this storage instead of the default // in-memory store, enabling session sharing across replicas (e.g. Redis-backed). @@ -278,6 +292,7 @@ func (t *StdioTransport) streamableProxyOptions() []streamable.Option { streamable.WithAuthInfoHandler(t.authInfoHandler), streamable.WithPrefixHandlers(t.prefixHandlers), streamable.WithStrictProtocolValidation(t.strictProtocolValidation), + streamable.WithSecretRedaction(t.redactToolResultSecrets), ) } diff --git a/pkg/transport/types/transport.go b/pkg/transport/types/transport.go index f94254384a..104e756d91 100644 --- a/pkg/transport/types/transport.go +++ b/pkg/transport/types/transport.go @@ -255,6 +255,15 @@ type Config struct { // consulted for the stdio/streamable-HTTP proxy path (see factory.go). StrictProtocolValidation bool + // RedactToolResultSecrets enables best-effort credential-shape scanning + // (see pkg/mcp/secretscan) on tools/call responses relayed by the + // streamable HTTP proxy: matches are redacted before the response + // reaches the client. Opt-in (default false) because the backend MCP + // server is often operator-trusted and the scan adds per-response + // overhead; enable it when the backend is not fully trusted. Only + // consulted for the stdio/streamable-HTTP proxy path (see factory.go). + RedactToolResultSecrets bool + // ProxyMode is the proxy mode for stdio transport ("sse" or "streamable-http") ProxyMode ProxyMode diff --git a/pkg/vmcp/cli/serve.go b/pkg/vmcp/cli/serve.go index 0f7efa9bcb..216eae21fe 100644 --- a/pkg/vmcp/cli/serve.go +++ b/pkg/vmcp/cli/serve.go @@ -440,6 +440,7 @@ func Serve(ctx context.Context, cfg ServeConfig) error { CodeModeConfig: codemode.FromConfig(vmcpCfg.CodeMode), SessionFactory: sessionFactory, SessionStorage: vmcpCfg.SessionStorage, + RedactToolResultSecrets: vmcpCfg.RedactToolResultSecrets, // Core collaborators: server.New routes through core.New + Serve, so the core // is the single aggregator and authorizer. The aggregator is the same instance // that backs discovery; Authz feeds the core admission seam (nil = allow-all). diff --git a/pkg/vmcp/config/config.go b/pkg/vmcp/config/config.go index 6712a2c87f..7c0a2c3f08 100644 --- a/pkg/vmcp/config/config.go +++ b/pkg/vmcp/config/config.go @@ -201,6 +201,14 @@ type Config struct { // +optional // +listType=atomic PassthroughHeaders []string `json:"passthroughHeaders,omitempty" yaml:"passthroughHeaders,omitempty"` + + // RedactToolResultSecrets enables best-effort credential-shape scanning + // (see pkg/mcp/secretscan) on tools/call results before they are relayed + // to the client: matches are redacted in place. Opt-in (default false); + // enable when a backend this vMCP aggregates is not fully trusted. + // +optional + // +kubebuilder:default=false + RedactToolResultSecrets bool `json:"redactToolResultSecrets,omitempty" yaml:"redactToolResultSecrets,omitempty"` } // IncomingAuthConfig configures client authentication to the virtual MCP server. diff --git a/pkg/vmcp/server/derive.go b/pkg/vmcp/server/derive.go index a1799babc7..c177a9041b 100644 --- a/pkg/vmcp/server/derive.go +++ b/pkg/vmcp/server/derive.go @@ -88,8 +88,9 @@ func deriveServerConfig( SessionStorage: cfg.SessionStorage, SessionManagerConfig: sessionManagerConfig, // Cross-cutting (also on core.Config) — R3, not a clean partition: - TelemetryProvider: cfg.TelemetryProvider, - AuditConfig: cfg.AuditConfig, + TelemetryProvider: cfg.TelemetryProvider, + AuditConfig: cfg.AuditConfig, + RedactToolResultSecrets: cfg.RedactToolResultSecrets, } } diff --git a/pkg/vmcp/server/derive_test.go b/pkg/vmcp/server/derive_test.go index 645d8cddbc..c654866668 100644 --- a/pkg/vmcp/server/derive_test.go +++ b/pkg/vmcp/server/derive_test.go @@ -51,6 +51,7 @@ func populatedLegacyConfig() *Config { Watcher: stubWatcher{}, StatusReporter: stubServeReporter{}, SessionStorage: &vmcpconfig.SessionStorageConfig{}, + RedactToolResultSecrets: true, } } diff --git a/pkg/vmcp/server/modern_dispatch.go b/pkg/vmcp/server/modern_dispatch.go index e7d79d4dc1..79b6e1bb7a 100644 --- a/pkg/vmcp/server/modern_dispatch.go +++ b/pkg/vmcp/server/modern_dispatch.go @@ -339,7 +339,8 @@ func (s *Server) dispatchModernToolCall( bi.BackendName = s.backendDisplayName(ctx, result.BackendID) } } - writeModernResult(w, parsed.ID, newModernCallToolResult(result, s.config.Name, s.config.Version)) + writeModernResult(w, parsed.ID, + newModernCallToolResult(result, s.config.Name, s.config.Version, s.config.RedactToolResultSecrets)) } // dispatchModernResourceRead re-homes authzCallGate's resources/read branch diff --git a/pkg/vmcp/server/modern_envelope.go b/pkg/vmcp/server/modern_envelope.go index 0da4e60260..a63aab3be5 100644 --- a/pkg/vmcp/server/modern_envelope.go +++ b/pkg/vmcp/server/modern_envelope.go @@ -11,6 +11,7 @@ import ( "github.com/stacklok/toolhive-core/mcpcompat/mcp" mcpparser "github.com/stacklok/toolhive/pkg/mcp" + "github.com/stacklok/toolhive/pkg/mcp/secretscan" transportsession "github.com/stacklok/toolhive/pkg/transport/session" "github.com/stacklok/toolhive/pkg/vmcp" "github.com/stacklok/toolhive/pkg/vmcp/conversion" @@ -411,19 +412,26 @@ func newModernPromptsList( // newModernCallToolResult builds the tools/call wire result from the core's // ToolCallResult. StructuredContent is omitted entirely (not merely // omitempty-false) when the core did not set it, matching the SDK's -// omitempty behavior. +// omitempty behavior. When redactSecrets is set, Content is scanned for +// credential-shaped text (see pkg/mcp/secretscan) before being returned. // // result is non-nil on every call: dispatchModernToolCall (modern_dispatch.go) // dereferences result.BackendID before calling this builder, so a nil result // would already have panicked upstream. -func newModernCallToolResult(result *vmcp.ToolCallResult, serverName, serverVersion string) modernCallToolResult { +func newModernCallToolResult( + result *vmcp.ToolCallResult, serverName, serverVersion string, redactSecrets bool, +) modernCallToolResult { var structuredContent any if len(result.StructuredContent) > 0 { structuredContent = result.StructuredContent } + content := conversion.ToMCPContents(result.Content) + if redactSecrets { + secretscan.RedactContentInPlace(content) + } return modernCallToolResult{ ResultType: modernResultTypeComplete, - Content: conversion.ToMCPContents(result.Content), + Content: content, StructuredContent: structuredContent, IsError: result.IsError, Meta: newModernResultMeta(result.Meta, serverName, serverVersion), diff --git a/pkg/vmcp/server/modern_envelope_test.go b/pkg/vmcp/server/modern_envelope_test.go index c63c3abc6f..8687507271 100644 --- a/pkg/vmcp/server/modern_envelope_test.go +++ b/pkg/vmcp/server/modern_envelope_test.go @@ -81,7 +81,7 @@ func TestModernEnvelopeCommonFields(t *testing.T) { build: func(*testing.T) any { return newModernCallToolResult(&vmcp.ToolCallResult{ Content: []vmcp.Content{{Type: vmcp.ContentTypeText, Text: "hello"}}, - }, testServerName, testServerVersion) + }, testServerName, testServerVersion, false) }, wantCacheable: false, }, @@ -156,7 +156,7 @@ func TestModernResultMetaPreservesBackendMeta(t *testing.T) { { name: "tools/call", build: func() any { - return newModernCallToolResult(&vmcp.ToolCallResult{Meta: backendMeta}, testServerName, testServerVersion) + return newModernCallToolResult(&vmcp.ToolCallResult{Meta: backendMeta}, testServerName, testServerVersion, false) }, }, { @@ -212,7 +212,7 @@ func TestModernResultMetaOverwritesSpoofedServerInfo(t *testing.T) { modernServerInfoKey: map[string]any{"name": "attacker-server", "version": "666"}, } - raw, err := json.Marshal(newModernCallToolResult(&vmcp.ToolCallResult{Meta: spoofed}, testServerName, testServerVersion)) + raw, err := json.Marshal(newModernCallToolResult(&vmcp.ToolCallResult{Meta: spoofed}, testServerName, testServerVersion, false)) require.NoError(t, err) var decoded map[string]any @@ -354,7 +354,7 @@ func TestModernEnvelopeEmptyCollections(t *testing.T) { name: "tools/call content", field: "content", build: func(*testing.T) any { - return newModernCallToolResult(&vmcp.ToolCallResult{}, testServerName, testServerVersion) + return newModernCallToolResult(&vmcp.ToolCallResult{}, testServerName, testServerVersion, false) }, }, { @@ -422,7 +422,7 @@ func TestModernCallToolResult(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - raw, err := json.Marshal(newModernCallToolResult(tt.domainResult, testServerName, testServerVersion)) + raw, err := json.Marshal(newModernCallToolResult(tt.domainResult, testServerName, testServerVersion, false)) require.NoError(t, err) var decoded map[string]any diff --git a/pkg/vmcp/server/secretscan_test.go b/pkg/vmcp/server/secretscan_test.go new file mode 100644 index 0000000000..a4555e5b67 --- /dev/null +++ b/pkg/vmcp/server/secretscan_test.go @@ -0,0 +1,73 @@ +// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package server + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/stacklok/toolhive-core/mcpcompat/mcp" + "github.com/stacklok/toolhive/pkg/vmcp" +) + +// TestCoreToolHandler_RedactsCredentialShapedToolResult verifies the Legacy +// (SDK) tools/call path scans and redacts credential-shaped content in the +// core's result when Config.RedactToolResultSecrets is enabled -- the same +// gap connector-gateway (which serves through this Server, not ToolHive's +// own proxies) would otherwise have. +func TestCoreToolHandler_RedactsCredentialShapedToolResult(t *testing.T) { + t.Parallel() + + const toolName = "t" + ghToken := "ghp_" + strings.Repeat("a", 36) + fc := &fakeCore{ + tools: []vmcp.Tool{{Name: toolName}}, + callResult: &vmcp.ToolCallResult{ + Content: []vmcp.Content{{Type: vmcp.ContentTypeText, Text: "here is the token: " + ghToken}}, + }, + } + srv, sessionID, _ := registerServeSession(t, fc) + srv.config.RedactToolResultSecrets = true + + req := mcp.CallToolRequest{Params: mcp.CallToolParams{Name: toolName, Arguments: map[string]any{}}} + res, err := srv.coreToolHandler(sessionID, toolName, "")(t.Context(), req) + + require.NoError(t, err) + require.NotNil(t, res) + require.Len(t, res.Content, 1) + text, ok := res.Content[0].(mcp.TextContent) + require.True(t, ok) + assert.NotContains(t, text.Text, ghToken) + assert.Contains(t, text.Text, "REDACTED-BY-TOOLHIVE") +} + +// TestCoreToolHandler_DisabledByDefault verifies redaction is opt-in: with +// Config.RedactToolResultSecrets left at its zero value, credential-shaped +// content passes through unchanged. +func TestCoreToolHandler_DisabledByDefault(t *testing.T) { + t.Parallel() + + const toolName = "t" + ghToken := "ghp_" + strings.Repeat("a", 36) + fc := &fakeCore{ + tools: []vmcp.Tool{{Name: toolName}}, + callResult: &vmcp.ToolCallResult{ + Content: []vmcp.Content{{Type: vmcp.ContentTypeText, Text: ghToken}}, + }, + } + srv, sessionID, _ := registerServeSession(t, fc) + + req := mcp.CallToolRequest{Params: mcp.CallToolParams{Name: toolName, Arguments: map[string]any{}}} + res, err := srv.coreToolHandler(sessionID, toolName, "")(t.Context(), req) + + require.NoError(t, err) + require.NotNil(t, res) + require.Len(t, res.Content, 1) + text, ok := res.Content[0].(mcp.TextContent) + require.True(t, ok) + assert.Equal(t, ghToken, text.Text, "disabled by default: content must pass through unmodified") +} diff --git a/pkg/vmcp/server/serve.go b/pkg/vmcp/server/serve.go index 7fd80a36f2..0940fd743c 100644 --- a/pkg/vmcp/server/serve.go +++ b/pkg/vmcp/server/serve.go @@ -140,6 +140,11 @@ type ServerConfig struct { // AuditConfig is the cross-cutting audit configuration (also consumed by // core.New). If nil, no audit logging is performed. AuditConfig *audit.Config + + // RedactToolResultSecrets enables best-effort credential-shape scanning + // (see pkg/mcp/secretscan) on tools/call results before they are relayed + // to the client. Opt-in (default false); see Config.RedactToolResultSecrets. + RedactToolResultSecrets bool } // Serve is the transport-side entry point of the New/Serve split: it wraps an @@ -438,5 +443,6 @@ func buildServeConfig(cfg *ServerConfig) *Config { StatusReportingInterval: cfg.StatusReportingInterval, Watcher: cfg.Watcher, SessionStorage: cfg.SessionStorage, + RedactToolResultSecrets: cfg.RedactToolResultSecrets, } } diff --git a/pkg/vmcp/server/serve_handlers.go b/pkg/vmcp/server/serve_handlers.go index e8f039b144..b7b27624c4 100644 --- a/pkg/vmcp/server/serve_handlers.go +++ b/pkg/vmcp/server/serve_handlers.go @@ -15,6 +15,7 @@ import ( "github.com/stacklok/toolhive/pkg/audit" "github.com/stacklok/toolhive/pkg/auth" mcpparser "github.com/stacklok/toolhive/pkg/mcp" + "github.com/stacklok/toolhive/pkg/mcp/secretscan" "github.com/stacklok/toolhive/pkg/vmcp" "github.com/stacklok/toolhive/pkg/vmcp/conversion" vmcpsession "github.com/stacklok/toolhive/pkg/vmcp/session" @@ -306,9 +307,14 @@ func (s *Server) coreToolHandler(sessionID, toolName, backendName string) server return conversion.ErrorToToolResult(err), nil } + content := conversion.ToMCPContents(result.Content) + if s.config.RedactToolResultSecrets { + secretscan.RedactContentInPlace(content) + } + return &mcp.CallToolResult{ Result: mcp.Result{Meta: conversion.ToMCPMeta(result.Meta)}, - Content: conversion.ToMCPContents(result.Content), + Content: content, StructuredContent: result.StructuredContent, IsError: result.IsError, }, nil diff --git a/pkg/vmcp/server/serve_session_test.go b/pkg/vmcp/server/serve_session_test.go index 39f0fca730..d5d07665af 100644 --- a/pkg/vmcp/server/serve_session_test.go +++ b/pkg/vmcp/server/serve_session_test.go @@ -153,12 +153,13 @@ type fakeCore struct { lastCompleteRef atomic.Value // vmcp.CompletionRef completeValues []string // returned by Complete when completeErr is nil - callErr error // when set, CallTool returns it (e.g. vmcp.ErrAuthorizationFailed) - readErr error // when set, ReadResource returns it - readMeta map[string]any // when set, ReadResource sets it as result.Meta on success - promptErr error // when set, GetPrompt returns it (e.g. vmcp.ErrAuthorizationFailed) - completeErr error // when set, Complete returns it (e.g. vmcp.ErrAuthorizationFailed) - lookupResourceErr error // when set, LookupResource returns it for an ADVERTISED URI (admission denial) + callErr error // when set, CallTool returns it (e.g. vmcp.ErrAuthorizationFailed) + callResult *vmcp.ToolCallResult // when set, CallTool returns it instead of the default {Text:"ok"} + readErr error // when set, ReadResource returns it + readMeta map[string]any // when set, ReadResource sets it as result.Meta on success + promptErr error // when set, GetPrompt returns it (e.g. vmcp.ErrAuthorizationFailed) + completeErr error // when set, Complete returns it (e.g. vmcp.ErrAuthorizationFailed) + lookupResourceErr error // when set, LookupResource returns it for an ADVERTISED URI (admission denial) // invalidateCacheCalls counts InvalidateCapabilityCache invocations, so tests // covering the list_changed sink can assert the cache was re-swept (#5748). @@ -214,6 +215,9 @@ func (f *fakeCore) CallTool( if f.callErr != nil { return nil, f.callErr } + if f.callResult != nil { + return f.callResult, nil + } return &vmcp.ToolCallResult{Content: []vmcp.Content{{Type: vmcp.ContentTypeText, Text: "ok"}}}, nil } diff --git a/pkg/vmcp/server/serve_test.go b/pkg/vmcp/server/serve_test.go index cdfc7ebd22..47669cbf82 100644 --- a/pkg/vmcp/server/serve_test.go +++ b/pkg/vmcp/server/serve_test.go @@ -375,6 +375,7 @@ func TestBuildServeConfigMapsSharedFields(t *testing.T) { SessionManagerConfig: testMinimalSessionManagerConfig(), TelemetryProvider: &telemetry.Provider{}, AuditConfig: &audit.Config{}, + RedactToolResultSecrets: true, } got := reflect.ValueOf(*buildServeConfig(src)) diff --git a/pkg/vmcp/server/server.go b/pkg/vmcp/server/server.go index f237be9108..b777ae2f69 100644 --- a/pkg/vmcp/server/server.go +++ b/pkg/vmcp/server/server.go @@ -262,6 +262,16 @@ type Config struct { // from the HTTP middleware. When non-nil, Name must be non-empty (the Cedar resource // entity name). Authz *authz.Config + + // RedactToolResultSecrets enables best-effort credential-shape scanning + // (see pkg/mcp/secretscan) on tools/call results before they are relayed + // to the client: matches are redacted in place. Opt-in (default false), + // matching the same knob on ToolHive's own proxies (streamable. + // WithSecretRedaction / transparent.WithSecretRedaction) -- the backend + // MCP servers this vMCP aggregates are often operator-trusted, and the + // scan adds per-call overhead, so this is only worth enabling when a + // backend is not fully trusted. + RedactToolResultSecrets bool } // Server is the Virtual MCP Server that aggregates multiple backends. diff --git a/pkg/workloads/upgrade/applier.go b/pkg/workloads/upgrade/applier.go index a60b9d10d7..d1346ec435 100644 --- a/pkg/workloads/upgrade/applier.go +++ b/pkg/workloads/upgrade/applier.go @@ -308,6 +308,7 @@ func (a *Applier) buildUpgradedConfig( runner.WithAllowDockerGateway(old.AllowDockerGateway), runner.WithTrustProxyHeaders(old.TrustProxyHeaders), runner.WithStrictProtocolValidation(old.StrictProtocolValidation), + runner.WithRedactToolResultSecrets(old.RedactToolResultSecrets), runner.WithProxyMode(old.ProxyMode), runner.WithCmdArgs(slices.Clone(old.CmdArgs)), runner.WithStateless(old.Stateless),