diff --git a/cmd/thv/app/proxy.go b/cmd/thv/app/proxy.go index f3a933a00b..8f9600178d 100644 --- a/cmd/thv/app/proxy.go +++ b/cmd/thv/app/proxy.go @@ -123,6 +123,9 @@ var ( // Header forwarding flags remoteForwardHeaders []string remoteForwardHeadersSecret []string + + // CORS flags + proxyCORSOrigins []string ) // Environment variable names @@ -157,6 +160,19 @@ func init() { proxyCmd.Flags().StringArrayVar(&remoteForwardHeadersSecret, "remote-forward-headers-secret", []string{}, "Headers with secret values from ToolHive secrets manager (format: Name=secret-name, can be repeated)") + // CORS — disabled by default; opt in explicitly to avoid widening the attack surface + proxyCmd.Flags().StringArrayVar(&proxyCORSOrigins, "allow-origins", []string{}, + `Allowed CORS origins for the MCP proxy endpoint (repeatable). +CORS is disabled by default; if you handle CORS at a gateway or reverse proxy, +leaving this unset is the correct, secure choice. Each origin must include a +scheme (e.g. http://) and no trailing slash, otherwise it can never match a +browser request. +Supported forms: + exact: http://localhost:6274 + scheme+host: http://localhost (matches any port on localhost) + wildcard: * (allow all — use with caution) +Example: --allow-origins http://localhost:6274`) + // Mark target-uri as required if err := proxyCmd.MarkFlagRequired("target-uri"); err != nil { slog.Warn(fmt.Sprintf("Failed to mark flag as required: %v", err)) @@ -264,8 +280,20 @@ func proxyCmdFunc(cmd *cobra.Command, args []string) error { slog.Debug(fmt.Sprintf("Setting up transparent proxy to forward from host port %d to %s", port, proxyTargetURI)) + // Build optional functional options (e.g. CORS), only when configured. + var proxyOptions []transparent.Option + if len(proxyCORSOrigins) > 0 { + // Validate origins at startup so a misconfigured entry (missing scheme, + // trailing slash) fails loudly instead of silently never matching. + corsOrigins, err := middleware.ValidateAndNormalizeOrigins(proxyCORSOrigins) + if err != nil { + return fmt.Errorf("invalid --allow-origins: %w", err) + } + proxyOptions = append(proxyOptions, transparent.WithAllowedOrigins(corsOrigins)) + } + // Create the transparent proxy with middlewares - proxy := transparent.NewTransparentProxy( + proxy := transparent.NewTransparentProxyWithOptions( proxyHost, port, proxyTargetURI, @@ -279,7 +307,8 @@ func proxyCmdFunc(cmd *cobra.Command, args []string) error { nil, // onUnauthorizedResponse - not needed for local proxies "", // endpointPrefix - not configured for proxy command false, // trustProxyHeaders - not configured for proxy command - middlewares...) + middlewares, + proxyOptions...) if err := proxy.Start(ctx); err != nil { return fmt.Errorf("failed to start proxy: %w", err) } diff --git a/docs/cli/thv_proxy.md b/docs/cli/thv_proxy.md index be2e8d92d2..9bde19cae7 100644 --- a/docs/cli/thv_proxy.md +++ b/docs/cli/thv_proxy.md @@ -97,6 +97,16 @@ thv proxy [flags] SERVER_NAME ### Options ``` + --allow-origins stringArray Allowed CORS origins for the MCP proxy endpoint (repeatable). + CORS is disabled by default; if you handle CORS at a gateway or reverse proxy, + leaving this unset is the correct, secure choice. Each origin must include a + scheme (e.g. http://) and no trailing slash, otherwise it can never match a + browser request. + Supported forms: + exact: http://localhost:6274 + scheme+host: http://localhost (matches any port on localhost) + wildcard: * (allow all — use with caution) + Example: --allow-origins http://localhost:6274 -h, --help help for proxy --host string Host for the HTTP proxy to listen on (IP or hostname) (default "127.0.0.1") --oidc-audience string Expected audience for the token diff --git a/pkg/transport/middleware/cors.go b/pkg/transport/middleware/cors.go new file mode 100644 index 0000000000..2551d86d93 --- /dev/null +++ b/pkg/transport/middleware/cors.go @@ -0,0 +1,164 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package middleware + +import ( + "fmt" + "log/slog" + "net/http" + "strings" + + "github.com/stacklok/toolhive/pkg/transport/types" +) + +const ( + // defaultCORSAllowedMethods is the fallback preflight method list used when + // the caller does not supply an explicit set. The proxy passes a set derived + // from the server's actual capabilities (see the stateless/stateful method + // sources of truth in the transparent proxy) so the preflight never + // advertises a method the backend will reject. + defaultCORSAllowedMethods = "GET, POST, DELETE, OPTIONS" + + // corsAllowedHeaders lists request headers MCP clients may send. MCP-Protocol-Version + // must be allow-listed: ToolHive reads and validates it on the request path + // (an unsupported value yields 400), so a browser MCP client cannot send it + // through CORS unless it is listed here. + corsAllowedHeaders = "Content-Type, Accept, Mcp-Session-Id, MCP-Protocol-Version, Authorization" + + // corsExposedHeaders lists response headers that browsers may read. MCP-Protocol-Version + // is exposed so a browser client can read the negotiated protocol version back. + // Content-Type is omitted because it is already a CORS-safelisted response + // header and does not need to be exposed explicitly. + corsExposedHeaders = "Mcp-Session-Id, MCP-Protocol-Version" + + // corsMaxAge is the preflight cache lifetime in seconds (24 hours). + corsMaxAge = "86400" +) + +// CORS returns a middleware that handles CORS preflight (OPTIONS) requests and +// injects Access-Control-Allow-* response headers. When allowedOrigins is empty +// the middleware is a no-op, preserving the default security posture. +// +// Origin matching rules (applied in order): +// - "*": matches every origin; Access-Control-Allow-Origin is set to "*". +// - Exact: "http://localhost:6274" matches only that origin. +// - Scheme+host prefix: "http://localhost" also matches any +// "http://localhost:" (e.g. the MCP Inspector default port). +// +// All OPTIONS requests are handled directly (returning 204) when this middleware +// is active so that CORS preflights never reach the backend, which previously +// returned 405 Method Not Allowed. +// +// allowedMethods is the value advertised in Access-Control-Allow-Methods. It +// should reflect the methods the backend actually accepts so a preflight never +// succeeds for a method the real request would reject. When empty, +// defaultCORSAllowedMethods is used. +func CORS(allowedOrigins []string, allowedMethods string) types.MiddlewareFunction { + if len(allowedOrigins) == 0 { + return func(next http.Handler) http.Handler { return next } + } + + if allowedMethods == "" { + allowedMethods = defaultCORSAllowedMethods + } + + slog.Debug("CORS middleware configured", + "allowed_origins", strings.Join(allowedOrigins, ", "), "allowed_methods", allowedMethods) + + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + origin := r.Header.Get("Origin") + matched := matchCORSOrigin(origin, allowedOrigins) + + if matched != "" { + h := w.Header() + h.Set("Access-Control-Allow-Origin", matched) + h.Set("Access-Control-Allow-Methods", allowedMethods) + h.Set("Access-Control-Allow-Headers", corsAllowedHeaders) + h.Set("Access-Control-Expose-Headers", corsExposedHeaders) + h.Add("Vary", "Origin") + } + + // Intercept OPTIONS so preflight requests never reach the backend + // (which returns 405 because it has no OPTIONS handler). + // A matched origin gets the full preflight response; an unmatched + // origin gets 204 without CORS headers — the browser will reject + // the follow-up request, which is the correct security outcome. + if r.Method == http.MethodOptions { + if matched != "" { + w.Header().Set("Access-Control-Max-Age", corsMaxAge) + } + w.WriteHeader(http.StatusNoContent) + return + } + + next.ServeHTTP(w, r) + }) + } +} + +// matchCORSOrigin returns the Access-Control-Allow-Origin value to send when +// requestOrigin matches an entry in allowed, or "" when there is no match. +// +// The returned value is the verbatim requestOrigin (a concrete origin) except +// when an allowed entry is "*", in which case "*" is returned directly. +func matchCORSOrigin(requestOrigin string, allowed []string) string { + if requestOrigin == "" { + return "" + } + for _, entry := range allowed { + switch { + case entry == "*": + return "*" + case entry == requestOrigin: + return requestOrigin + case strings.HasPrefix(requestOrigin, entry+":"): + // The trailing ":" boundary is load-bearing: it ensures the entry + // matches only ":" and never a longer host. Without it, + // the entry "http://localhost" would also match + // "http://localhost.evil.com". See cors_test.go for the invariant. + return requestOrigin + } + } + return "" +} + +// ValidateAndNormalizeOrigins validates configured CORS origins and returns a +// normalized copy. It surfaces misconfiguration at startup instead of letting an +// origin silently never match (which produces a broken browser experience with +// no signal): +// +// - "*" (wildcard) is passed through unchanged. +// - A trailing slash (e.g. "http://localhost:6274/") is stripped — a browser +// Origin header never carries one — with a warning, so the entry still matches. +// - An entry without a scheme (e.g. "localhost:6274") can never match a browser +// Origin header (always scheme://host[:port]) and is rejected with an error. +func ValidateAndNormalizeOrigins(origins []string) ([]string, error) { + normalized := make([]string, 0, len(origins)) + for _, origin := range origins { + entry := strings.TrimSpace(origin) + + if entry == "*" { + normalized = append(normalized, entry) + continue + } + + if strings.HasSuffix(entry, "/") { + stripped := strings.TrimRight(entry, "/") + slog.Warn("CORS origin has a trailing slash that browsers never send; normalizing", + "origin", origin, "normalized", stripped) + entry = stripped + } + + // A browser Origin is scheme://host[:port]; without a scheme the entry + // can never match an incoming Origin header. + if !strings.Contains(entry, "://") { + return nil, fmt.Errorf( + "invalid CORS origin %q: missing scheme (expected e.g. %q)", origin, "http://localhost:6274") + } + + normalized = append(normalized, entry) + } + return normalized, nil +} diff --git a/pkg/transport/middleware/cors_test.go b/pkg/transport/middleware/cors_test.go new file mode 100644 index 0000000000..e339b540f4 --- /dev/null +++ b/pkg/transport/middleware/cors_test.go @@ -0,0 +1,376 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package middleware + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// okHandler is a trivial inner handler that records whether it was called. +func newCallTracker() (http.Handler, *bool) { + called := false + h := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + called = true + w.WriteHeader(http.StatusOK) + }) + return h, &called +} + +func TestMatchCORSOrigin(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + requestOrigin string + allowed []string + expectedResult string + }{ + { + name: "empty request origin returns empty", + requestOrigin: "", + allowed: []string{"http://localhost:6274"}, + expectedResult: "", + }, + { + name: "exact match returns origin", + requestOrigin: "http://localhost:6274", + allowed: []string{"http://localhost:6274"}, + expectedResult: "http://localhost:6274", + }, + { + name: "exact match among multiple allowed origins", + requestOrigin: "http://localhost:3000", + allowed: []string{"http://localhost:6274", "http://localhost:3000"}, + expectedResult: "http://localhost:3000", + }, + { + name: "prefix match: scheme+host matches scheme+host+port", + requestOrigin: "http://localhost:6274", + allowed: []string{"http://localhost"}, + expectedResult: "http://localhost:6274", + }, + { + name: "prefix match: https scheme", + requestOrigin: "https://localhost:9000", + allowed: []string{"https://localhost"}, + expectedResult: "https://localhost:9000", + }, + { + name: "no false prefix match: partial scheme should not match", + requestOrigin: "http://localhost:6274", + allowed: []string{"http://local"}, + expectedResult: "", + }, + { + name: "wildcard returns literal asterisk", + requestOrigin: "http://example.com", + allowed: []string{"*"}, + expectedResult: "*", + }, + { + name: "non-matching origin returns empty", + requestOrigin: "http://evil.example.com", + allowed: []string{"http://localhost:6274"}, + expectedResult: "", + }, + { + name: "https origin does not match http entry", + requestOrigin: "https://localhost:6274", + allowed: []string{"http://localhost:6274"}, + expectedResult: "", + }, + { + // Invariant: the entry+":" colon boundary must prevent an evil + // subdomain from matching a scheme+host entry. Without the trailing + // ":" check, "http://localhost" would also match + // "http://localhost.evil.com". + name: "colon boundary: evil subdomain must NOT match scheme+host entry", + requestOrigin: "http://localhost.evil.com", + allowed: []string{"http://localhost"}, + expectedResult: "", + }, + { + // Same invariant with a port-bearing evil host. + name: "colon boundary: evil subdomain with port must NOT match", + requestOrigin: "http://localhost.evil.com:6274", + allowed: []string{"http://localhost"}, + expectedResult: "", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + result := matchCORSOrigin(tc.requestOrigin, tc.allowed) + assert.Equal(t, tc.expectedResult, result) + }) + } +} + +func TestCORS_EmptyOrigins_IsNoop(t *testing.T) { + t.Parallel() + + inner, called := newCallTracker() + mwFn := CORS(nil, defaultCORSAllowedMethods)(inner) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/mcp", nil) + req.Header.Set("Origin", "http://localhost:6274") + + mwFn.ServeHTTP(rec, req) + + assert.True(t, *called, "inner handler must be called when CORS is disabled") + assert.Empty(t, rec.Header().Get("Access-Control-Allow-Origin"), "no CORS header should be set") +} + +func TestCORS_NonOptions_MatchingOrigin_AddsCORSHeaders(t *testing.T) { + t.Parallel() + + inner, _ := newCallTracker() + mwFn := CORS([]string{"http://localhost:6274"}, defaultCORSAllowedMethods)(inner) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/mcp", nil) + req.Header.Set("Origin", "http://localhost:6274") + + mwFn.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, "http://localhost:6274", rec.Header().Get("Access-Control-Allow-Origin")) + assert.Equal(t, defaultCORSAllowedMethods, rec.Header().Get("Access-Control-Allow-Methods")) + assert.Equal(t, corsAllowedHeaders, rec.Header().Get("Access-Control-Allow-Headers")) + assert.Equal(t, corsExposedHeaders, rec.Header().Get("Access-Control-Expose-Headers")) + assert.Contains(t, rec.Header().Get("Vary"), "Origin") +} + +func TestCORS_NonOptions_NonMatchingOrigin_NoCORSHeaders(t *testing.T) { + t.Parallel() + + inner, called := newCallTracker() + mwFn := CORS([]string{"http://localhost:6274"}, defaultCORSAllowedMethods)(inner) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/mcp", nil) + req.Header.Set("Origin", "http://evil.example.com") + + mwFn.ServeHTTP(rec, req) + + assert.True(t, *called, "inner handler must still be called for non-matching non-OPTIONS") + assert.Empty(t, rec.Header().Get("Access-Control-Allow-Origin"), "no CORS header for non-matching origin") +} + +func TestCORS_Preflight_MatchingOrigin_Returns204WithHeaders(t *testing.T) { + t.Parallel() + + inner, called := newCallTracker() + mwFn := CORS([]string{"http://localhost:6274"}, defaultCORSAllowedMethods)(inner) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodOptions, "/mcp", nil) + req.Header.Set("Origin", "http://localhost:6274") + req.Header.Set("Access-Control-Request-Method", "POST") + req.Header.Set("Access-Control-Request-Headers", "Content-Type") + + mwFn.ServeHTTP(rec, req) + + require.Equal(t, http.StatusNoContent, rec.Code, "preflight must return 204 No Content") + assert.False(t, *called, "inner handler must NOT be called for preflight") + assert.Equal(t, "http://localhost:6274", rec.Header().Get("Access-Control-Allow-Origin")) + assert.Equal(t, defaultCORSAllowedMethods, rec.Header().Get("Access-Control-Allow-Methods")) + assert.Equal(t, corsAllowedHeaders, rec.Header().Get("Access-Control-Allow-Headers")) + assert.Equal(t, corsMaxAge, rec.Header().Get("Access-Control-Max-Age")) +} + +func TestCORS_Preflight_NonMatchingOrigin_Returns204NoHeaders(t *testing.T) { + t.Parallel() + + inner, called := newCallTracker() + mwFn := CORS([]string{"http://localhost:6274"}, defaultCORSAllowedMethods)(inner) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodOptions, "/mcp", nil) + req.Header.Set("Origin", "http://evil.example.com") + + mwFn.ServeHTTP(rec, req) + + require.Equal(t, http.StatusNoContent, rec.Code, "OPTIONS always returns 204 when CORS is active") + assert.False(t, *called, "inner handler must NOT be called for OPTIONS") + assert.Empty(t, rec.Header().Get("Access-Control-Allow-Origin"), "no CORS header for non-matching origin") + assert.Empty(t, rec.Header().Get("Access-Control-Max-Age")) +} + +func TestCORS_PrefixMatch_LocalhostAnyPort(t *testing.T) { + t.Parallel() + + inner, _ := newCallTracker() + // "http://localhost" should match http://localhost on any port + mwFn := CORS([]string{"http://localhost"}, defaultCORSAllowedMethods)(inner) + + origins := []string{ + "http://localhost:6274", + "http://localhost:3000", + "http://localhost:8080", + } + + for _, origin := range origins { + t.Run(origin, func(t *testing.T) { + t.Parallel() + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/mcp", nil) + req.Header.Set("Origin", origin) + + mwFn.ServeHTTP(rec, req) + + assert.Equal(t, origin, rec.Header().Get("Access-Control-Allow-Origin"), + "prefix match must set concrete origin, not the entry") + }) + } +} + +func TestCORS_Wildcard_MatchesAnyOrigin(t *testing.T) { + t.Parallel() + + inner, _ := newCallTracker() + mwFn := CORS([]string{"*"}, defaultCORSAllowedMethods)(inner) + + origins := []string{ + "http://localhost:6274", + "http://example.com", + "https://app.example.org", + } + + for _, origin := range origins { + t.Run(origin, func(t *testing.T) { + t.Parallel() + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/mcp", nil) + req.Header.Set("Origin", origin) + + mwFn.ServeHTTP(rec, req) + + assert.Equal(t, "*", rec.Header().Get("Access-Control-Allow-Origin"), + "wildcard entry must return literal * not the request origin") + }) + } +} + +func TestCORS_NoOriginHeader_NoCORSHeaders(t *testing.T) { + t.Parallel() + + inner, called := newCallTracker() + mwFn := CORS([]string{"http://localhost:6274"}, defaultCORSAllowedMethods)(inner) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/mcp", nil) + // No Origin header (non-browser request) + + mwFn.ServeHTTP(rec, req) + + assert.True(t, *called, "non-browser requests must reach the backend") + assert.Empty(t, rec.Header().Get("Access-Control-Allow-Origin")) +} + +func TestCORS_AllowMethods_ReflectsCallerSet(t *testing.T) { + t.Parallel() + + // The advertised preflight methods must be whatever the caller passes (so a + // stateless proxy can advertise only "POST, OPTIONS"), not a hard-coded set. + const statelessMethods = "POST, OPTIONS" + + inner, _ := newCallTracker() + mwFn := CORS([]string{"http://localhost:6274"}, statelessMethods)(inner) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodOptions, "/mcp", nil) + req.Header.Set("Origin", "http://localhost:6274") + req.Header.Set("Access-Control-Request-Method", "POST") + + mwFn.ServeHTTP(rec, req) + + assert.Equal(t, statelessMethods, rec.Header().Get("Access-Control-Allow-Methods"), + "preflight must advertise exactly the caller-supplied method set") +} + +func TestCORS_AllowHeaders_IncludeMCPProtocolVersion(t *testing.T) { + t.Parallel() + + inner, _ := newCallTracker() + mwFn := CORS([]string{"http://localhost:6274"}, defaultCORSAllowedMethods)(inner) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodOptions, "/mcp", nil) + req.Header.Set("Origin", "http://localhost:6274") + + mwFn.ServeHTTP(rec, req) + + allowHeaders := rec.Header().Get("Access-Control-Allow-Headers") + assert.Contains(t, allowHeaders, "MCP-Protocol-Version", + "MCP-Protocol-Version must be allow-listed so browser clients can send it") + + exposeHeaders := rec.Header().Get("Access-Control-Expose-Headers") + assert.Contains(t, exposeHeaders, "MCP-Protocol-Version", + "MCP-Protocol-Version must be exposed so browser clients can read the negotiated version") + assert.NotContains(t, exposeHeaders, "Content-Type", + "Content-Type is CORS-safelisted and should not be redundantly exposed") +} + +func TestValidateAndNormalizeOrigins(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input []string + expected []string + wantErr bool + }{ + { + name: "valid origins pass through unchanged", + input: []string{"http://localhost:6274", "https://app.example.com"}, + expected: []string{"http://localhost:6274", "https://app.example.com"}, + }, + { + name: "wildcard passes through", + input: []string{"*"}, + expected: []string{"*"}, + }, + { + name: "trailing slash is normalized away", + input: []string{"http://localhost:6274/"}, + expected: []string{"http://localhost:6274"}, + }, + { + name: "surrounding whitespace is trimmed", + input: []string{" http://localhost:6274 "}, + expected: []string{"http://localhost:6274"}, + }, + { + name: "missing scheme is rejected", + input: []string{"localhost:6274"}, + wantErr: true, + }, + { + name: "bare host is rejected", + input: []string{"example.com"}, + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, err := ValidateAndNormalizeOrigins(tc.input) + if tc.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tc.expected, got) + }) + } +} diff --git a/pkg/transport/proxy/transparent/method_gate_test.go b/pkg/transport/proxy/transparent/method_gate_test.go index 9bfc2e35c5..a7e72d524c 100644 --- a/pkg/transport/proxy/transparent/method_gate_test.go +++ b/pkg/transport/proxy/transparent/method_gate_test.go @@ -66,8 +66,28 @@ func TestStatelessMethodGate(t *testing.T) { assert.Equal(t, tc.expectedStatus, rec.Code) if tc.expectAllow { - assert.Equal(t, "POST, OPTIONS", rec.Header().Get("Allow")) + assert.Equal(t, statelessAllowedMethods, rec.Header().Get("Allow")) } }) } } + +// TestCORSAllowedMethodsMatchGate guards the invariant that the CORS preflight +// advertises exactly the methods the server actually accepts. A stateless proxy +// only allows POST/OPTIONS, so a browser must not be told it can preflight GET +// or DELETE and then have the real request 405. +func TestCORSAllowedMethodsMatchGate(t *testing.T) { + t.Parallel() + + stateful := &TransparentProxy{stateless: false} + assert.Equal(t, statefulAllowedMethods, stateful.corsAllowedMethods(), + "stateful proxy must advertise the full method set") + + stateless := &TransparentProxy{stateless: true} + assert.Equal(t, statelessAllowedMethods, stateless.corsAllowedMethods(), + "stateless proxy must advertise only the methods the gate permits") + + // The stateless preflight must never advertise a method the gate rejects. + assert.NotContains(t, stateless.corsAllowedMethods(), "GET") + assert.NotContains(t, stateless.corsAllowedMethods(), "DELETE") +} diff --git a/pkg/transport/proxy/transparent/transparent_proxy.go b/pkg/transport/proxy/transparent/transparent_proxy.go index 1cad66e6a2..0e645de688 100644 --- a/pkg/transport/proxy/transparent/transparent_proxy.go +++ b/pkg/transport/proxy/transparent/transparent_proxy.go @@ -32,6 +32,7 @@ import ( "github.com/stacklok/toolhive/pkg/auth" "github.com/stacklok/toolhive/pkg/bodylimit" "github.com/stacklok/toolhive/pkg/healthcheck" + "github.com/stacklok/toolhive/pkg/transport/middleware" "github.com/stacklok/toolhive/pkg/transport/proxy/socket" "github.com/stacklok/toolhive/pkg/transport/session" "github.com/stacklok/toolhive/pkg/transport/types" @@ -149,6 +150,11 @@ type TransparentProxy struct { // Shutdown timeout for graceful HTTP server shutdown (default: 30 seconds) shutdownTimeout time.Duration + + // corsOrigins holds the list of allowed CORS origins. When non-empty, the + // proxy handles OPTIONS preflight requests and injects Access-Control-Allow-* + // headers. Empty (default) keeps the current behaviour with no CORS support. + corsOrigins []string } const ( @@ -336,10 +342,31 @@ func WithReadTimeout(d time.Duration) Option { } } +// WithAllowedOrigins enables CORS support on the transparent proxy by setting the +// list of permitted origins. An empty slice (the default) leaves CORS disabled so +// the security posture is unchanged for deployments that do not need browser access. +// +// Supported forms per entry: +// - Exact origin: "http://localhost:6274" +// - Scheme+host (any port): "http://localhost" → also matches http://localhost:6274 +// - Wildcard: "*" → allow every origin +func WithAllowedOrigins(origins []string) Option { + return func(p *TransparentProxy) { + if len(origins) > 0 { + p.corsOrigins = origins + } + } +} + // NewTransparentProxy creates a new transparent proxy with optional middlewares. // The endpointPrefix parameter specifies an explicit prefix to prepend to SSE endpoint URLs. // The trustProxyHeaders parameter indicates whether to trust X-Forwarded-* headers from reverse proxies. // The prefixHandlers parameter is a map of path prefixes to HTTP handlers mounted before the catch-all proxy handler. +// +// It is a stable positional-argument convenience wrapper around +// NewTransparentProxyWithOptions and is retained for existing callers. New +// configuration knobs (such as WithAllowedOrigins) are exposed only as +// functional Options; reach for NewTransparentProxyWithOptions when you need them. func NewTransparentProxy( host string, port int, @@ -1216,6 +1243,17 @@ func (p *TransparentProxy) Start(ctx context.Context) error { if p.stateless { finalHandler = statelessMethodGate(finalHandler) } + + // 6. Apply CORS as the outermost wrapper so OPTIONS preflights are intercepted + // before the method gate or backend can reject them. Only active when + // corsOrigins is non-empty (opt-in, no behaviour change by default). + if len(p.corsOrigins) > 0 { + corsMethods := p.corsAllowedMethods() + finalHandler = middleware.CORS(p.corsOrigins, corsMethods)(finalHandler) + slog.Debug("CORS middleware applied", + "allowed_origins", p.corsOrigins, "allowed_methods", corsMethods) + } + mux.Handle("/", finalHandler) // Use ListenConfig with SO_REUSEADDR to allow port reuse after unclean shutdown @@ -1472,6 +1510,30 @@ func (*TransparentProxy) ForwardResponseToClients(_ context.Context, _ jsonrpc2. return fmt.Errorf("ForwardResponseToClients not implemented for TransparentProxy") } +// HTTP method sets advertised to clients. These are the single source of truth +// shared by statelessMethodGate's "Allow" header and the CORS preflight +// (Access-Control-Allow-Methods) so a browser never preflights a method the +// backend would then reject. +const ( + // statelessAllowedMethods are the methods a stateless (POST-only) MCP server + // accepts. OPTIONS is included for CORS preflight handling. + statelessAllowedMethods = "POST, OPTIONS" + + // statefulAllowedMethods are the methods a full streamable-HTTP MCP server + // accepts (GET for the SSE stream, DELETE for session teardown). + statefulAllowedMethods = "GET, POST, DELETE, OPTIONS" +) + +// corsAllowedMethods returns the method set the CORS preflight should advertise +// for this proxy, derived from the same source of truth the request path +// enforces (see statelessMethodGate). +func (p *TransparentProxy) corsAllowedMethods() string { + if p.stateless { + return statelessAllowedMethods + } + return statefulAllowedMethods +} + // statelessMethodGate wraps a handler to reject GET, HEAD, and DELETE requests with 405. // Used in stateless mode where the server only supports POST. // HEAD is blocked alongside GET because HEAD is semantically a GET without a response body; @@ -1479,7 +1541,7 @@ func (*TransparentProxy) ForwardResponseToClients(_ context.Context, _ jsonrpc2. func statelessMethodGate(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method == http.MethodGet || r.Method == http.MethodHead || r.Method == http.MethodDelete { - w.Header().Set("Allow", "POST, OPTIONS") + w.Header().Set("Allow", statelessAllowedMethods) http.Error(w, "method not allowed: server is stateless (POST only)", http.StatusMethodNotAllowed) return }