Skip to content

Commit f2aa438

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, and wires it into both the factory-based chain (thv run / thv-proxyrunner / vMCP) and the inline chain (thv proxy). 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 is byte-exact except that scheme and host are lowercased per RFC 6454 §4. 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. Closes audit row 5 (Origin validation absent). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Juan Antonio Osorio <ozz@stacklok.com>
1 parent d96922d commit f2aa438

13 files changed

Lines changed: 751 additions & 7 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/networking"
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.CreateOriginMiddleware(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
@@ -137,6 +137,12 @@ type RunFlags struct {
137137
RemoteForwardHeaders []string
138138
RemoteForwardHeadersSecret []string
139139

140+
// AllowedOrigins is the HTTP Origin-header allowlist for DNS-rebinding protection
141+
// (MCP 2025-11-25 §"Security Warning"). Empty with a loopback host auto-derives
142+
// loopback-only defaults; empty with a non-loopback host disables the check
143+
// (operator must supply explicit origins for public bind).
144+
AllowedOrigins []string
145+
140146
// Runtime configuration
141147
RuntimeImage string
142148
RuntimeAddPackages []string
@@ -156,6 +162,10 @@ func AddRunFlags(cmd *cobra.Command, config *RunFlags) {
156162
cmd.Flags().StringVar(&config.Name, "name", "", "Name of the MCP server (default to auto-generated from image)")
157163
cmd.Flags().StringVar(&config.Group, "group", "default", "Name of the group this workload should belong to")
158164
cmd.Flags().StringVar(&config.Host, "host", transport.LocalhostIPv4, "Host for the HTTP proxy to listen on (IP or hostname)")
165+
cmd.Flags().StringArrayVar(&config.AllowedOrigins, "allowed-origins", nil,
166+
"Exact-match allowlist for the HTTP Origin header (repeatable). Recommended when binding publicly; "+
167+
"loopback binds derive a default allowlist automatically, non-loopback binds log a warning when "+
168+
"no value is supplied. Example: https://my-mcp.example.com")
159169
cmd.Flags().IntVar(&config.ProxyPort, "proxy-port", 0, "Port for the HTTP proxy to listen on (host port)")
160170
cmd.Flags().IntVar(&config.TargetPort, "target-port", 0,
161171
"Port for the container to expose (only applicable to SSE or Streamable HTTP transport)")
@@ -678,6 +688,7 @@ func buildRunnerConfig(
678688
PrintOverlays: runFlags.PrintOverlays,
679689
}),
680690
runner.WithPublish(runFlags.Publish),
691+
runner.WithAllowedOrigins(runFlags.AllowedOrigins),
681692
}
682693
opts = append(opts, extraOpts...)
683694

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
@@ -97,6 +97,14 @@ type RunConfig struct {
9797
// TargetHost is the host to forward traffic to (only applicable to SSE transport)
9898
TargetHost string `json:"target_host,omitempty" yaml:"target_host,omitempty"`
9999

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

pkg/runner/config_builder.go

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

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

pkg/runner/middleware.go

Lines changed: 43 additions & 3 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"
@@ -20,6 +21,7 @@ import (
2021
"github.com/stacklok/toolhive/pkg/recovery"
2122
"github.com/stacklok/toolhive/pkg/telemetry"
2223
headerfwd "github.com/stacklok/toolhive/pkg/transport/middleware"
24+
"github.com/stacklok/toolhive/pkg/transport/middleware/origin"
2325
"github.com/stacklok/toolhive/pkg/transport/types"
2426
"github.com/stacklok/toolhive/pkg/usagemetrics"
2527
"github.com/stacklok/toolhive/pkg/webhook/mutating"
@@ -43,6 +45,7 @@ func GetSupportedMiddlewareFactories() map[string]types.MiddlewareFactory {
4345
audit.MiddlewareType: audit.CreateMiddleware,
4446
recovery.MiddlewareType: recovery.CreateMiddleware,
4547
headerfwd.HeaderForwardMiddlewareName: headerfwd.CreateMiddleware,
48+
origin.MiddlewareType: origin.CreateMiddleware,
4649
validating.MiddlewareType: validating.CreateMiddleware,
4750
mutating.MiddlewareType: mutating.CreateMiddleware,
4851
}
@@ -56,13 +59,21 @@ func PopulateMiddlewareConfigs(config *RunConfig) error {
5659
var middlewareConfigs []types.MiddlewareConfig
5760
// TODO: Consider extracting other middleware setup into helper functions like addUsageMetricsMiddleware
5861

62+
// Origin-validation middleware (DNS-rebinding protection per MCP 2025-11-25).
63+
// Positioned first in the slice so it runs earliest in the chain — disallowed
64+
// Origin values are rejected before any authentication or business logic.
65+
middlewareConfigs, err := addOriginMiddleware(middlewareConfigs, config)
66+
if err != nil {
67+
return err
68+
}
69+
5970
// Authentication middleware (always present)
6071
authParams := auth.MiddlewareParams{
6172
OIDCConfig: config.OIDCConfig,
6273
}
63-
authConfig, err := types.NewMiddlewareConfig(auth.MiddlewareType, authParams)
64-
if err != nil {
65-
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)
6677
}
6778
middlewareConfigs = append(middlewareConfigs, *authConfig)
6879

@@ -419,6 +430,35 @@ func addAWSStsMiddleware(middlewares []types.MiddlewareConfig, config *RunConfig
419430
return append(middlewares, *awsStsMwConfig), nil
420431
}
421432

433+
// addOriginMiddleware adds Origin-header validation middleware for DNS-rebind
434+
// protection per MCP 2025-11-25 §"Security Warning". Default-derivation logic
435+
// lives in origin.ResolveAllowedOrigins so the standalone `thv proxy` command
436+
// and the runner path agree on behavior.
437+
//
438+
// When the effective allowlist is empty — which happens when the operator
439+
// binds to a non-loopback host without supplying --allowed-origins — the
440+
// middleware is skipped entirely and a WARN is logged so the security-disabled
441+
// state is visible in operator logs. A follow-up PR hardens the non-loopback
442+
// path by requiring an explicit opt-in flag (see audit row 22).
443+
func addOriginMiddleware(middlewares []types.MiddlewareConfig, config *RunConfig) ([]types.MiddlewareConfig, error) {
444+
allowed := origin.ResolveAllowedOrigins(config.Host, config.Port, config.AllowedOrigins)
445+
if len(allowed) == 0 {
446+
slog.Warn("Origin validation disabled — no allowlist configured for non-loopback bind",
447+
"host", config.Host,
448+
"port", config.Port,
449+
"hint", "pass --allowed-origins=https://your-client.example to enable DNS-rebind protection",
450+
)
451+
return middlewares, nil
452+
}
453+
454+
params := origin.MiddlewareParams{AllowedOrigins: allowed}
455+
mwCfg, err := types.NewMiddlewareConfig(origin.MiddlewareType, params)
456+
if err != nil {
457+
return nil, fmt.Errorf("failed to create origin middleware config: %w", err)
458+
}
459+
return append(middlewares, *mwCfg), nil
460+
}
461+
422462
// addRateLimitMiddleware adds rate limit middleware if configured.
423463
func addRateLimitMiddleware(middlewares []types.MiddlewareConfig, config *RunConfig) ([]types.MiddlewareConfig, error) {
424464
if config.RateLimitConfig == nil {

0 commit comments

Comments
 (0)