Skip to content

Commit 322b09f

Browse files
JAORMXclaude
andcommitted
Add Origin header validation middleware
ToolHive's proxy layer had no Origin-header validation, and the legacy HTTP+SSE transport sent `Access-Control-Allow-Origin: *`, leaving both modes open to DNS-rebinding attacks from browser clients. MCP 2025-11-25 §"Security Warning" requires servers to validate Origin on all connections and respond 403 when the value is invalid. This change introduces a dedicated middleware at pkg/transport/middleware/origin/ that rejects requests whose Origin header is present and not in an operator-configured allowlist. It is wired centrally in runner.Run for both the factory-based chain (thv run / thv-proxyrunner) and inline in the `thv proxy` chain. Wiring is done in runner.Run rather than in the builder because that is the only point where the effective Host/Port/AllowedOrigins are fully resolved: the CLI builder (WithMiddlewareFromFlags) defers port resolution to validateConfig, so an earlier hook would see port 0 and silently disable loopback-default protection for `thv run`. The middleware is prepended so Origin validation runs first in the chain. Behavior: - New --allowed-origins flag on `thv run` and `thv proxy` accepts a repeatable exact-match list. When empty and the bind host is loopback, a default loopback-only allowlist is derived automatically (http://localhost:PORT + 127.0.0.1 + [::1]). When empty and the bind is non-loopback, the middleware is skipped and a warning is logged — the bind-opt-in hardening lands in a follow-up. - Matching parses each value with net/url and compares scheme://host [:port] with scheme and host lowercased per RFC 6454 §4. Values carrying userinfo (RFC 6454 §6) or that fail to parse never match, closing an Origin-smuggling vector. Requests with multiple Origin headers are rejected outright. - 403 responses carry a JSON-RPC error body (id: null, code -32600, message "Origin not allowed"). - `Access-Control-Allow-Origin: *` removed from the httpsse SSE handler; the wildcard would have neutered any Origin enforcement via preflight response inheritance. Operator CRDs (MCPServer/MCPRemoteProxy/VirtualMCPServer) do not yet expose an allowedOrigins field, and vMCP composes its own middleware chain that does not reference this package. Both are deferred to follow-ups; until then operator/vMCP non-loopback deployments log the WARN above rather than enforcing. Closes audit row 5 (Origin validation absent). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Juan Antonio Osorio <ozz@stacklok.com>
1 parent 6f63ac0 commit 322b09f

15 files changed

Lines changed: 872 additions & 8 deletions

File tree

cmd/thv/app/proxy.go

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"github.com/stacklok/toolhive/pkg/oauthproto/tokenexchange"
2525
"github.com/stacklok/toolhive/pkg/transport"
2626
"github.com/stacklok/toolhive/pkg/transport/middleware"
27+
"github.com/stacklok/toolhive/pkg/transport/middleware/origin"
2728
"github.com/stacklok/toolhive/pkg/transport/proxy/transparent"
2829
"github.com/stacklok/toolhive/pkg/transport/types"
2930
)
@@ -110,9 +111,10 @@ Dynamic client registration (automatic OAuth client setup):
110111
}
111112

112113
var (
113-
proxyHost string
114-
proxyPort int
115-
proxyTargetURI string
114+
proxyHost string
115+
proxyPort int
116+
proxyTargetURI string
117+
proxyAllowedOrigins []string
116118

117119
resourceURL string // Explicit resource URL for OAuth discovery endpoint (RFC 9728)
118120

@@ -133,6 +135,10 @@ const (
133135
func init() {
134136
proxyCmd.Flags().StringVar(&proxyHost, "host", transport.LocalhostIPv4, "Host for the HTTP proxy to listen on (IP or hostname)")
135137
proxyCmd.Flags().IntVar(&proxyPort, "port", 0, "Port for the HTTP proxy to listen on (host port)")
138+
proxyCmd.Flags().StringArrayVar(&proxyAllowedOrigins, "allowed-origins", nil,
139+
"Exact-match allowlist for the HTTP Origin header (repeatable). Recommended when binding publicly; "+
140+
"loopback binds derive a default allowlist automatically, non-loopback binds log a warning when "+
141+
"no value is supplied. Example: https://my-mcp.example.com")
136142
proxyCmd.Flags().StringVar(
137143
&proxyTargetURI,
138144
"target-uri",
@@ -226,6 +232,22 @@ func proxyCmdFunc(cmd *cobra.Command, args []string) error {
226232
// Create middlewares slice for incoming request authentication
227233
var middlewares []types.NamedMiddleware
228234

235+
// Origin-header validation (DNS-rebinding protection per MCP 2025-11-25
236+
// §"Security Warning"). Added first so disallowed Origins are rejected
237+
// before authentication or any outbound token acquisition runs.
238+
if allowed := origin.ResolveAllowedOrigins(proxyHost, port, proxyAllowedOrigins); len(allowed) > 0 {
239+
middlewares = append(middlewares, types.NamedMiddleware{
240+
Name: origin.MiddlewareType,
241+
Function: origin.NewHandler(allowed),
242+
})
243+
} else {
244+
slog.Warn("Origin validation disabled — no allowlist configured for non-loopback bind",
245+
"host", proxyHost,
246+
"port", port,
247+
"hint", "pass --allowed-origins=https://your-client.example to enable DNS-rebind protection",
248+
)
249+
}
250+
229251
// Get OIDC configuration if enabled (for protecting the proxy endpoint)
230252
oidcConfig := getProxyOIDCConfig(cmd)
231253

cmd/thv/app/run_flags.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,12 @@ type RunFlags struct {
141141
RemoteForwardHeaders []string
142142
RemoteForwardHeadersSecret []string
143143

144+
// AllowedOrigins is the HTTP Origin-header allowlist for DNS-rebinding protection
145+
// (MCP 2025-11-25 §"Security Warning"). Empty with a loopback host auto-derives
146+
// loopback-only defaults; empty with a non-loopback host disables the check
147+
// (operator must supply explicit origins for public bind).
148+
AllowedOrigins []string
149+
144150
// Runtime configuration
145151
RuntimeImage string
146152
RuntimeAddPackages []string
@@ -160,6 +166,10 @@ func AddRunFlags(cmd *cobra.Command, config *RunFlags) {
160166
cmd.Flags().StringVar(&config.Name, "name", "", "Name of the MCP server (default to auto-generated from image)")
161167
cmd.Flags().StringVar(&config.Group, "group", "default", "Name of the group this workload should belong to")
162168
cmd.Flags().StringVar(&config.Host, "host", transport.LocalhostIPv4, "Host for the HTTP proxy to listen on (IP or hostname)")
169+
cmd.Flags().StringArrayVar(&config.AllowedOrigins, "allowed-origins", nil,
170+
"Exact-match allowlist for the HTTP Origin header (repeatable). Recommended when binding publicly; "+
171+
"loopback binds derive a default allowlist automatically, non-loopback binds log a warning when "+
172+
"no value is supplied. Example: https://my-mcp.example.com")
163173
cmd.Flags().IntVar(&config.ProxyPort, "proxy-port", 0, "Port for the HTTP proxy to listen on (host port)")
164174
cmd.Flags().IntVar(&config.TargetPort, "target-port", 0,
165175
"Port for the container to expose (only applicable to SSE or Streamable HTTP transport)")
@@ -685,6 +695,7 @@ func buildRunnerConfig(
685695
PrintOverlays: runFlags.PrintOverlays,
686696
}),
687697
runner.WithPublish(runFlags.Publish),
698+
runner.WithAllowedOrigins(runFlags.AllowedOrigins),
688699
}
689700
opts = append(opts, extraOpts...)
690701

docs/cli/thv_proxy.md

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/cli/thv_run.md

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/server/docs.go

Lines changed: 8 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/server/swagger.json

Lines changed: 8 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/server/swagger.yaml

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/runner/config.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,14 @@ type RunConfig struct {
103103
// TargetHost is the host to forward traffic to (only applicable to SSE transport)
104104
TargetHost string `json:"target_host,omitempty" yaml:"target_host,omitempty"`
105105

106+
// AllowedOrigins is the allowlist of values accepted on the HTTP Origin header,
107+
// used for DNS-rebinding protection per MCP 2025-11-25 §"Security Warning".
108+
// When empty and Host is loopback (127.0.0.1 / localhost / [::1]), a default
109+
// loopback-only allowlist is derived at middleware-wiring time.
110+
// When empty and Host is non-loopback, the middleware is disabled — operators
111+
// exposing the proxy publicly must configure an explicit allowlist.
112+
AllowedOrigins []string `json:"allowed_origins,omitempty" yaml:"allowed_origins,omitempty"`
113+
106114
// Publish lists ports to publish to the host in format "hostPort:containerPort"
107115
Publish []string `json:"publish,omitempty" yaml:"publish,omitempty"`
108116

pkg/runner/config_builder.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,18 @@ func WithAllowDockerGateway(allow bool) RunConfigBuilderOption {
331331
}
332332
}
333333

334+
// WithAllowedOrigins sets the HTTP Origin-header allowlist used for
335+
// DNS-rebinding protection (MCP 2025-11-25 §"Security Warning").
336+
// An empty slice defers the choice to middleware wiring, which derives a
337+
// loopback-only default when the bind host is loopback and otherwise leaves
338+
// the middleware disabled.
339+
func WithAllowedOrigins(origins []string) RunConfigBuilderOption {
340+
return func(b *runConfigBuilder) error {
341+
b.config.AllowedOrigins = origins
342+
return nil
343+
}
344+
}
345+
334346
// WithTrustProxyHeaders sets whether to trust X-Forwarded-* headers from reverse proxies
335347
func WithTrustProxyHeaders(trust bool) RunConfigBuilderOption {
336348
return func(b *runConfigBuilder) error {

pkg/runner/middleware.go

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package runner
55

66
import (
77
"fmt"
8+
"log/slog"
89

910
"github.com/stacklok/toolhive/pkg/audit"
1011
"github.com/stacklok/toolhive/pkg/auth"
@@ -21,6 +22,7 @@ import (
2122
"github.com/stacklok/toolhive/pkg/recovery"
2223
"github.com/stacklok/toolhive/pkg/telemetry"
2324
headerfwd "github.com/stacklok/toolhive/pkg/transport/middleware"
25+
"github.com/stacklok/toolhive/pkg/transport/middleware/origin"
2426
"github.com/stacklok/toolhive/pkg/transport/types"
2527
"github.com/stacklok/toolhive/pkg/usagemetrics"
2628
"github.com/stacklok/toolhive/pkg/webhook/mutating"
@@ -45,6 +47,7 @@ func GetSupportedMiddlewareFactories() map[string]types.MiddlewareFactory {
4547
audit.MiddlewareType: audit.CreateMiddleware,
4648
recovery.MiddlewareType: recovery.CreateMiddleware,
4749
headerfwd.HeaderForwardMiddlewareName: headerfwd.CreateMiddleware,
50+
origin.MiddlewareType: origin.CreateMiddleware,
4851
validating.MiddlewareType: validating.CreateMiddleware,
4952
mutating.MiddlewareType: mutating.CreateMiddleware,
5053
}
@@ -57,22 +60,28 @@ func GetSupportedMiddlewareFactories() map[string]types.MiddlewareFactory {
5760
func PopulateMiddlewareConfigs(config *RunConfig) error {
5861
var middlewareConfigs []types.MiddlewareConfig
5962
// TODO: Consider extracting other middleware setup into helper functions like addUsageMetricsMiddleware
63+
//
64+
// NOTE: Origin-validation middleware is intentionally NOT added here. It is
65+
// wired centrally in runner.Run (via prependOriginMiddleware) for both the
66+
// operator/proxyrunner path (this function) and the CLI path
67+
// (WithMiddlewareFromFlags), because that is the only place where the
68+
// effective Host/Port/AllowedOrigins are fully resolved.
6069

6170
// Authentication middleware (always present)
6271
authParams := auth.MiddlewareParams{
6372
OIDCConfig: config.OIDCConfig,
6473
}
65-
authConfig, err := types.NewMiddlewareConfig(auth.MiddlewareType, authParams)
66-
if err != nil {
67-
return fmt.Errorf("failed to create auth middleware config: %w", err)
74+
authConfig, authErr := types.NewMiddlewareConfig(auth.MiddlewareType, authParams)
75+
if authErr != nil {
76+
return fmt.Errorf("failed to create auth middleware config: %w", authErr)
6877
}
6978
middlewareConfigs = append(middlewareConfigs, *authConfig)
7079

7180
// Upstream swap middleware (if embedded auth server is configured)
7281
// This exchanges ToolHive JWTs for upstream IdP tokens when embedded auth server is used.
7382
// IMPORTANT: Must run BEFORE token exchange middleware so it can read the `tsid` claim
7483
// from the original ToolHive JWT before any token modification occurs.
75-
middlewareConfigs, err = addUpstreamSwapMiddleware(middlewareConfigs, config)
84+
middlewareConfigs, err := addUpstreamSwapMiddleware(middlewareConfigs, config)
7685
if err != nil {
7786
return err
7887
}
@@ -421,6 +430,45 @@ func addAWSStsMiddleware(middlewares []types.MiddlewareConfig, config *RunConfig
421430
return append(middlewares, *awsStsMwConfig), nil
422431
}
423432

433+
// prependOriginMiddleware prepends Origin-header validation middleware for
434+
// DNS-rebind protection per MCP 2025-11-25 §"Security Warning". It is placed at
435+
// the front of the chain so disallowed Origin values are rejected before
436+
// authentication or any business logic runs. Default-derivation logic lives in
437+
// origin.ResolveAllowedOrigins so the standalone `thv proxy` command and the
438+
// runner path agree on behavior.
439+
//
440+
// This is called from runner.Run after both middleware-population paths
441+
// (PopulateMiddlewareConfigs and WithMiddlewareFromFlags) have run, because
442+
// that is the only point where the effective Host/Port/AllowedOrigins are
443+
// fully resolved — the CLI builder defers port resolution to validateConfig.
444+
//
445+
// When the effective allowlist is empty — which happens when the operator
446+
// binds to a non-loopback host without supplying --allowed-origins — the
447+
// middleware is skipped entirely and a WARN is logged so the security-disabled
448+
// state is visible in operator logs. A follow-up PR hardens the non-loopback
449+
// path by requiring an explicit opt-in flag (see audit row 22).
450+
func prependOriginMiddleware(middlewares []types.MiddlewareConfig, config *RunConfig) ([]types.MiddlewareConfig, error) {
451+
allowed := origin.ResolveAllowedOrigins(config.Host, config.Port, config.AllowedOrigins)
452+
if len(allowed) == 0 {
453+
slog.Warn("Origin validation disabled — no allowlist configured for non-loopback bind",
454+
"host", config.Host,
455+
"port", config.Port,
456+
"hint", "pass --allowed-origins=https://your-client.example to enable DNS-rebind protection",
457+
)
458+
return middlewares, nil
459+
}
460+
461+
params := origin.MiddlewareParams{AllowedOrigins: allowed}
462+
mwCfg, err := types.NewMiddlewareConfig(origin.MiddlewareType, params)
463+
if err != nil {
464+
return nil, fmt.Errorf("failed to create origin middleware config: %w", err)
465+
}
466+
// Prepend so Origin validation is the outermost wrapper (runs first at
467+
// request time). Build a new slice to avoid mutating the caller's backing
468+
// array.
469+
return append([]types.MiddlewareConfig{*mwCfg}, middlewares...), nil
470+
}
471+
424472
// addRateLimitMiddleware adds rate limit middleware if configured.
425473
func addRateLimitMiddleware(middlewares []types.MiddlewareConfig, config *RunConfig) ([]types.MiddlewareConfig, error) {
426474
if config.RateLimitConfig == nil {

0 commit comments

Comments
 (0)