fix(httpapi): stop sending Access-Control-Allow-Origin: * on the REST API and /events

docs/configuration.md

@@ -743,6 +743,11 @@ Add the public domain(s) to `trusted_hosts` to allow them:
 - A request that carries an `Origin` header must likewise have a loopback or trusted
   origin host (MCP spec requirement); requests without `Origin` (non-browser clients,
   reverse proxies) are never rejected by the Origin check.
+- The same allowlist drives CORS on the REST API (`/api/v1/*`) and the `/events` SSE
+  stream: the request `Origin` is echoed back in `Access-Control-Allow-Origin` only when
+  it is loopback or trusted, and no CORS headers are sent otherwise. Earlier versions
+  sent `Access-Control-Allow-Origin: *` there unconditionally, so a separate web app that
+  calls the REST API cross-origin now needs its host in `trusted_hosts`.
 - Loopback hosts (`localhost`, `127.0.0.1`, `[::1]`) are always accepted; requests on
   non-loopback listeners are never subject to Host validation.
 - Environment override: `MCPPROXY_TRUSTED_HOSTS` (comma-separated list).

docs/operations/reverse-proxy.md

@@ -74,6 +74,24 @@ client served **from your public domain** works as soon as that domain is in
 frontend hosted on a **different** origin must have its own host added to
 `trusted_hosts` too, or its requests are rejected.
 
+### CORS on the REST API and `/events`
+
+The same allowlist decides CORS on the REST API (`/api/v1/*`) and the SSE stream
+(`/events`): MCPProxy echoes the request `Origin` in
+`Access-Control-Allow-Origin` only when that origin is loopback or in
+`trusted_hosts`, and sends no CORS headers at all otherwise. Every response
+carries `Vary: Origin`.
+
+:::caution Behaviour change
+Earlier versions sent `Access-Control-Allow-Origin: *` on these endpoints
+unconditionally. If a **separate** web app calls the REST API cross-origin from a
+public domain, add that app's host to `trusted_hosts` — unlike the `Host` check,
+which only ever applied to the MCP endpoints, this one applies to the REST
+surface, so a REST-only proxied deployment may have been running with
+`trusted_hosts` empty. The embedded Web UI is same-origin under `/ui/` and is
+unaffected, as are non-browser clients, which send no `Origin` at all.
+:::
+
 ## `trusted_proxies` — forwarded headers {#trusted_proxies-forwarded-headers}
 
 A reverse proxy rewrites the connection MCPProxy sees: the peer address becomes the

internal/httpapi/cors_test.go

@@ -0,0 +1,172 @@
+package httpapi
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
+
+	"github.com/stretchr/testify/assert"
+	"go.uber.org/zap"
+)
+
+// corsController supplies both config doors the CORS middleware depends on:
+// GetCurrentConfig (read by apiKeyAuthMiddleware) and GetConfig (read by
+// trustedHostsProvider).
+type corsController struct {
+	baseController
+	apiKey       string
+	trustedHosts []string
+}
+
+func (m *corsController) GetCurrentConfig() any {
+	return &config.Config{APIKey: m.apiKey}
+}
+
+func (m *corsController) GetConfig() (*config.Config, error) {
+	return &config.Config{APIKey: m.apiKey, TrustedHosts: m.trustedHosts}, nil
+}
+
+const corsTestAPIKey = "test-api-key"
+
+func newCORSServer(trustedHosts []string) *Server {
+	return NewServer(&corsController{apiKey: corsTestAPIKey, trustedHosts: trustedHosts}, zap.NewNop().Sugar(), nil)
+}
+
+// TestCORSOriginAllowlist pins the exact Access-Control-Allow-Origin behavior
+// on both doors that emit it: the shared /api/v1 surface and the /events SSE
+// stream. The wildcard "*" must never appear, and a disallowed (or absent)
+// Origin must produce no CORS headers at all.
+func TestCORSOriginAllowlist(t *testing.T) {
+	tests := []struct {
+		name         string
+		origin       string
+		trustedHosts []string
+		wantEcho     bool
+	}{
+		{name: "absent origin", origin: "", wantEcho: false},
+		{name: "loopback ipv4 with port", origin: "http://127.0.0.1:5173", wantEcho: true},
+		{name: "localhost with port", origin: "http://localhost:3000", wantEcho: true},
+		{name: "loopback ipv6 bracketed", origin: "http://[::1]:8080", wantEcho: true},
+		{name: "remote https origin", origin: "https://evil.example", wantEcho: false},
+		{name: "null origin", origin: "null", wantEcho: false},
+		{name: "loopback suffix confusion", origin: "http://127.0.0.1.evil.com", wantEcho: false},
+		{name: "origin with path is not an origin", origin: "http://localhost:3000/path", wantEcho: false},
+		{
+			name:         "trusted host echoed",
+			origin:       "https://ui.example.com",
+			trustedHosts: []string{"ui.example.com"},
+			wantEcho:     true,
+		},
+		{
+			name:         "wildcard trusted host echoes concrete origin",
+			origin:       "https://evil.example",
+			trustedHosts: []string{"*"},
+			wantEcho:     true,
+		},
+	}
+
+	doors := []struct {
+		name   string
+		method string
+		path   string
+	}{
+		// HEAD, not GET: handleSSEEvents returns immediately on HEAD while a
+		// GET blocks on the live stream.
+		{name: "api", method: http.MethodGet, path: "/api/v1/status"},
+		{name: "sse", method: http.MethodHead, path: "/events"},
+	}
+
+	for _, tc := range tests {
+		for _, door := range doors {
+			t.Run(tc.name+"/"+door.name, func(t *testing.T) {
+				srv := newCORSServer(tc.trustedHosts)
+
+				req := httptest.NewRequest(door.method, door.path, nil)
+				req.Header.Set("X-API-Key", corsTestAPIKey)
+				if tc.origin != "" {
+					req.Header.Set("Origin", tc.origin)
+				}
+				w := httptest.NewRecorder()
+				srv.ServeHTTP(w, req)
+
+				gotOrigin := w.Header().Get("Access-Control-Allow-Origin")
+				assert.NotEqual(t, "*", gotOrigin, "wildcard CORS origin must never be emitted")
+
+				if tc.wantEcho {
+					assert.Equal(t, tc.origin, gotOrigin, "allowed origin must be echoed verbatim")
+				} else {
+					assert.Empty(t, gotOrigin, "disallowed origin must get no Access-Control-Allow-Origin")
+					assert.Empty(t, w.Header().Get("Access-Control-Allow-Methods"))
+					assert.Empty(t, w.Header().Get("Access-Control-Allow-Headers"))
+				}
+
+				// Vary: Origin is required on every response, allowed or not,
+				// so shared caches never serve one origin's response to another.
+				assert.Contains(t, w.Header().Values("Vary"), "Origin", "Vary must advertise Origin")
+
+				// Credentialed CORS is never enabled.
+				assert.Empty(t, w.Header().Get("Access-Control-Allow-Credentials"))
+			})
+		}
+	}
+}
+
+// TestCORSOnUnauthenticatedResponse checks that the CORS decision is made
+// ahead of authentication, so a 401 still carries Vary: Origin and does not
+// become a cache entry served to a different origin.
+func TestCORSOnUnauthenticatedResponse(t *testing.T) {
+	srv := newCORSServer(nil)
+
+	req := httptest.NewRequest(http.MethodGet, "/api/v1/servers", nil)
+	req.Header.Set("Origin", "http://localhost:5173")
+	w := httptest.NewRecorder()
+	srv.ServeHTTP(w, req)
+
+	assert.Equal(t, http.StatusUnauthorized, w.Code)
+	assert.Contains(t, w.Header().Values("Vary"), "Origin")
+	assert.Equal(t, "http://localhost:5173", w.Header().Get("Access-Control-Allow-Origin"))
+}
+
+// TestCORSPreflight pins the OPTIONS short-circuit, which runs ahead of
+// apiKeyAuthMiddleware: browsers never send credentials on a preflight, so it
+// must stay unauthenticated, but it must still only advertise CORS to an
+// allowed origin.
+func TestCORSPreflight(t *testing.T) {
+	t.Run("allowed origin", func(t *testing.T) {
+		srv := newCORSServer(nil)
+
+		req := httptest.NewRequest(http.MethodOptions, "/api/v1/servers", nil)
+		req.Header.Set("Origin", "http://localhost:5173")
+		req.Header.Set("Access-Control-Request-Method", "POST")
+		w := httptest.NewRecorder()
+		srv.ServeHTTP(w, req)
+
+		assert.Less(t, w.Code, 300, "preflight must not require an API key")
+		assert.Equal(t, "http://localhost:5173", w.Header().Get("Access-Control-Allow-Origin"))
+		// PATCH is advertised: the REST API registers PATCH routes
+		// (/api/v1/servers/{id}, /api/v1/config), and omitting it makes them
+		// unreachable from an allowed cross-origin caller.
+		assert.Equal(t, "GET, POST, PUT, PATCH, DELETE, OPTIONS", w.Header().Get("Access-Control-Allow-Methods"))
+		assert.NotEmpty(t, w.Header().Get("Access-Control-Allow-Headers"))
+		assert.Contains(t, w.Header().Values("Vary"), "Origin")
+		assert.Empty(t, w.Header().Get("Access-Control-Allow-Credentials"))
+	})
+
+	t.Run("disallowed origin", func(t *testing.T) {
+		srv := newCORSServer(nil)
+
+		req := httptest.NewRequest(http.MethodOptions, "/api/v1/servers", nil)
+		req.Header.Set("Origin", "https://evil.example")
+		req.Header.Set("Access-Control-Request-Method", "POST")
+		w := httptest.NewRecorder()
+		srv.ServeHTTP(w, req)
+
+		assert.Less(t, w.Code, 300, "preflight still short-circuits")
+		assert.Empty(t, w.Header().Get("Access-Control-Allow-Origin"))
+		assert.Empty(t, w.Header().Get("Access-Control-Allow-Methods"))
+		assert.Empty(t, w.Header().Get("Access-Control-Allow-Headers"))
+		assert.Contains(t, w.Header().Values("Vary"), "Origin")
+	})
+}

internal/httpapi/server.go

@@ -25,6 +25,7 @@ import (
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/connect"
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/contracts"
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/httpx"
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/launch"
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/logs"
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/management"
@@ -761,6 +762,64 @@ func (s *Server) trustedProxiesProvider() config.TrustedProxiesProvider {
 	}
 }
 
+// trustedHostsProvider yields the LIVE trusted_hosts list through the
+// controller's config, evaluated per request so hot-reload takes effect
+// without a restart. Same shape as trustedProxiesProvider: the nil guards are
+// load-bearing because test controllers return a nil config.
+func (s *Server) trustedHostsProvider() func() []string {
+	return func() []string {
+		if s.controller == nil {
+			return nil
+		}
+		if cfg, err := s.controller.GetConfig(); err == nil && cfg != nil {
+			return cfg.TrustedHosts
+		}
+		return nil
+	}
+}
+
+// corsMiddleware replaces the former unconditional
+// "Access-Control-Allow-Origin: *" (SEC-04). It echoes the request Origin only
+// when that origin passes the same allowlist the MCP surface uses
+// (httpx.OriginAllowed: loopback on any port, or a host in config
+// trusted_hosts) and emits nothing at all otherwise - including when there is
+// no Origin header, which is every non-browser client. The embedded Web UI is
+// same-origin under /ui/, so nothing legitimate needs the wildcard.
+//
+// The OPTIONS short-circuit stays ahead of apiKeyAuthMiddleware: browsers
+// never send credentials on a preflight, so gating it behind the API key would
+// break legitimate cross-origin use.
+func (s *Server) corsMiddleware() func(http.Handler) http.Handler {
+	trustedHosts := s.trustedHostsProvider()
+	return func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			// Add, never Set: another layer may already have written a Vary
+			// and clobbering it would poison shared caches.
+			w.Header().Add("Vary", "Origin")
+
+			origin := r.Header.Get("Origin")
+			if origin != "" && httpx.OriginAllowed(origin, trustedHosts()) {
+				// Echo the concrete origin rather than "*", even when
+				// trusted_hosts is ["*"]: a reflected origin is what lets a
+				// browser cache the response per origin alongside Vary, and
+				// "*" is illegal on a credentialed response. (Reflecting an
+				// arbitrary origin is NOT itself a substitute for the
+				// allowlist — trusted_hosts ["*"] deliberately disables it.)
+				w.Header().Set("Access-Control-Allow-Origin", origin)
+				w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, PATCH, DELETE, OPTIONS")
+				w.Header().Set("Access-Control-Allow-Headers", "Content-Type, Authorization, X-API-Key")
+			}
+
+			if r.Method == http.MethodOptions {
+				w.WriteHeader(http.StatusOK)
+				return
+			}
+
+			next.ServeHTTP(w, r)
+		})
+	}
+}
+
 // setupRoutes configures all API routes
 func (s *Server) setupRoutes() {
 	s.logger.Debug("Setting up HTTP API routes")
@@ -780,21 +839,9 @@ func (s *Server) setupRoutes() {
 	s.router.Use(s.correlationIDMiddleware()) // Correlation ID and request source tracking
 	s.logger.Debug("Core middleware configured (request ID, logging, recovery, correlation ID)")
 
-	// CORS headers for browser access
-	s.router.Use(func(next http.Handler) http.Handler {
-		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			w.Header().Set("Access-Control-Allow-Origin", "*")
-			w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS")
-			w.Header().Set("Access-Control-Allow-Headers", "Content-Type, Authorization, X-API-Key")
-
-			if r.Method == "OPTIONS" {
-				w.WriteHeader(http.StatusOK)
-				return
-			}
-
-			next.ServeHTTP(w, r)
-		})
-	})
+	// CORS headers for browser access (SEC-04). Registered with Use so it
+	// covers every route on this router, /events included.
+	s.router.Use(s.corsMiddleware())
 
 	// Health and readiness endpoints (Kubernetes-compatible with legacy aliases)
 	// See healthzHandler() and readyzHandler() for swagger documentation
@@ -3954,7 +4001,9 @@ func (s *Server) handleSSEEvents(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Content-Type", "text/event-stream")
 	w.Header().Set("Cache-Control", "no-cache")
 	w.Header().Set("Connection", "keep-alive")
-	w.Header().Set("Access-Control-Allow-Origin", "*")
+	// No Access-Control-Allow-Origin here: /events is a chi route on the same
+	// router, so corsMiddleware already decided this response's CORS headers.
+	// A second write would only be a place for the two policies to drift.
 	w.Header().Set("X-Accel-Buffering", "no") // Disable nginx buffering
 
 	// For HEAD requests, just return headers without body