fix(19.3-06): cors:<profile> headers on every response of the route (CR-01)

- Router.wrap applies the route's profile outside recovery, body limit and
  locale, whatever its position in the list or the group nesting
- bufferedResponse.commit merges Vary instead of replacing it
- TestCORSProfileHeadersOnRefusals, TestCORSProfileVaryKeptWithHandlerVary
  required by name in the gate

docs/architecture/request-lifecycle.md

@@ -37,12 +37,13 @@ surf wraps every non-raw route in the same layers. From the outside in:
 
 1. Optional locale-prefix rewrite, when a `surf.LocaleResolver` is published: an enabled locale as the first path segment is stripped so later layers and routing see the unprefixed path.
 2. CORS, only for the paths configured under `http.cors.paths`, including preflight requests.
-3. Panic recovery. A panic becomes an opaque JSON 500 from [wire](../../modules/wire/README.md), and because the response is buffered until the handler returns, the client never receives half a body.
-4. The request locale, taken from the published `surf.LocaleResolver` when one exists, or from the `Accept-Language` header otherwise, and stored in the context.
-5. The body limit: `http.body_limits.default_bytes`, or the route's own `body.limit:<bytes>`.
-6. The route's middleware, in the order you listed them: group middleware first, then the route's own.
-7. The path constraints. A request whose parameter fails `pact.Router.Where` or `pact.Router.WhereIn` gets a 404 before your handler runs.
-8. Your handler.
+3. The route's CORS profile, when it or its group names `cors:<profile>`. It sits here whatever its position in the middleware list, so refusals and recovered panics carry its headers too.
+4. Panic recovery. A panic becomes an opaque JSON 500 from [wire](../../modules/wire/README.md), and because the response is buffered until the handler returns, the client never receives half a body.
+5. The request locale, taken from the published `surf.LocaleResolver` when one exists, or from the `Accept-Language` header otherwise, and stored in the context.
+6. The body limit: `http.body_limits.default_bytes`, or the route's own `body.limit:<bytes>`.
+7. The route's middleware, in the order you listed them: group middleware first, then the route's own (`cors:<profile>` excepted, see step 3).
+8. The path constraints. A request whose parameter fails `pact.Router.Where` or `pact.Router.WhereIn` gets a 404 before your handler runs.
+9. Your handler.
 
 Raw groups, declared with `pact.Router.GroupRaw`, are for webhooks and file streams. They skip the default body limit, refuse house middleware, and a panic in them returns a bare 500.
 

docs/services/routing.md

@@ -98,7 +98,7 @@ The framework registers these names:
 |------|------|
 | `throttle:<bucket>` or `throttle:<max>,<minutes>` | Rate limiting; see [Rate limiting](rate-limiting.md). |
 | `body.limit:<bytes>` | Replaces the default request body limit for the route. |
-| `cors:<profile>` | CORS headers from `http.cors_profiles.<profile>`; see [CORS profiles](#cors-profiles). |
+| `cors:<profile>` | CORS headers from `http.cors_profiles.<profile>` on every response of the route, refusals and recovered panics included, whatever its position in the list; see [CORS profiles](#cors-profiles). |
 | `turnstile:open`, `turnstile:closed` | Cloudflare Turnstile verification; see [Turnstile](authentication.md#turnstile). |
 | `locale.from-principal` | Switches the request locale to the signed-in user's preferred locale. |
 | `backend` | The admin guard, when the admin is enabled. |
@@ -262,7 +262,7 @@ cors_profiles:
     supports_credentials: true
 ```
 
-This fragment belongs in `config/http.yaml`. A route group then lists `cors:spa` among its middleware names, next to its guard, exactly as it lists `throttle:60,1`.
+This fragment belongs in `config/http.yaml`. The route, or its group, then lists `cors:spa` among its middleware names, as it lists `throttle:60,1`. surf applies the profile outside every other layer of the route, whatever its position in the list or the group nesting. Every response of the route therefore carries the profile's headers, including refusals from guards (401, 403), throttle (429) and Turnstile (422, 503), the body limit (413) and recovered panics (500); otherwise the browser would hide them from the page, and a single-page app could not see the 401 that starts its cookie refresh.
 
 A profile is checked like `http.cors`: credentials with a `*` origin, a pattern that does not compile and a PHP-delimited pattern fail the start-up, with errors naming `http.cors_profiles.<name>.<key>`. Profile names use lower-case letters, digits, underscore and dash, and a profile that sets `paths` fails too.
 

modules/surf/README.md

@@ -28,7 +28,7 @@ Use cases:
 - Named middleware from plugins (`pact.HasMiddleware`), parameterized middleware used as `name:param` (`pact.HasMiddlewareFactories`) and house middleware for the JSON envelope and error handling (`pact.HasHouseMiddleware`). Duplicate or unknown names fail boot.
 - Raw groups (`surf.Router.GroupRaw`) for routes that must not be wrapped in house middleware, such as webhooks or file streams: house middleware is refused there, the default body limit is skipped and a panic returns a bare 500.
 - Built-in middleware names: `throttle:<bucket>` or `throttle:<max>,<minutes>`, `body.limit:<bytes>`, `cors:<profile>`, `turnstile:open` and `turnstile:closed` (Cloudflare Turnstile through `bouncer.Turnstile`, built from `turnstile.secret`, with the client IP from `surf.ClientIP` and refusals written by a published `bouncer.TurnstileWriter` when there is one), `locale.from-principal`, plus `backend` (the admin guard) when the admin is enabled.
-- Named CORS profiles: `cors:<profile>` applies `http.cors_profiles.<profile>` (read by `surf.LoadCORSProfiles`, validated like `http.cors`) to a route group, and surf registers an `OPTIONS` route for each such path that answers the preflight from the profile, since ServeMux would answer it with 405 before the middleware runs. An unknown profile, two profiles on one path and a profiled path that `http.cors.paths` also matches fail boot.
+- Named CORS profiles: `cors:<profile>` applies `http.cors_profiles.<profile>` (read by `surf.LoadCORSProfiles`, validated like `http.cors`) to a route or route group. The profile wraps every other layer of the route, whatever its position in the list or the group nesting, so every response carries its headers: guard refusals (401, 403), throttle (429), Turnstile (422, 503), the body limit (413) and recovered panics (500) included. A handler's own `Vary` is merged with the profile's `Vary: Origin`, not replacing it. surf registers an `OPTIONS` route for each such path that answers the preflight from the profile, since ServeMux would answer it with 405 before the middleware runs. An unknown profile, two profiles on one path and a profiled path that `http.cors.paths` also matches fail boot.
 - Fixed-window rate limiting (`surf.FixedWindowLimiter`): named buckets from plugins that implement `surf.BucketProvider`, or inline limits keyed by the signed-in user, or by client IP for guests. Rejected requests get a 429 with `Retry-After` and `X-RateLimit-*` headers. The in-process `surf.MemoryStore` sits behind the `surf.Store` interface; `surf.BuildRouter` uses a `surf.Store` published on the app instead when there is one, which is how `serve` shares counters across instances through [cooler](../cooler/README.md)'s Redis store (`http.rate_limit.store: redis`).
 - Client IP resolution for limiter keys (`surf.ClientIP`) that only trusts `X-Forwarded-For` hops when the direct peer is inside a configured trusted proxy range (`surf.TrustedProxies`).
 - Every non-raw route runs inside JSON panic recovery (an opaque 500 via [wire](../wire/README.md)), gets the request locale from a published `surf.LocaleResolver` when one exists or from the `Accept-Language` header otherwise (see [towel](../towel/README.md)), and a request body cap. Responses are buffered until the handler returns, so a panic never leaves a half-written body.

modules/surf/cors.go

@@ -251,7 +251,9 @@ func LoadCORSProfiles(cfg *compass.Config) (map[string]CORSConfig, error) {
 }
 
 // profileMiddleware sets the profile's CORS headers on a non-OPTIONS request;
-// preflights reach the OPTIONS route compile registers instead.
+// preflights reach the OPTIONS route compile registers instead. wrap runs it
+// outermost, outside the body limit, locale and recovery, so every answer of
+// the route carries the headers, refusals and recovered panics included.
 func (p *corsPolicy) profileMiddleware(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if r.Method != http.MethodOptions {

modules/surf/cors_profile_test.go

@@ -1,11 +1,14 @@
 package surf
 
 import (
+	"errors"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"strings"
 	"testing"
 
+	"git.golem15.com/golem15/summercms/modules/bouncer"
 	"git.golem15.com/golem15/summercms/modules/pact"
 	"git.golem15.com/golem15/summercms/modules/party"
 )
@@ -457,3 +460,183 @@ func TestTurnstileFactoryParams(t *testing.T) {
 		t.Fatalf("bare turnstile: error = %v", err)
 	}
 }
+
+// guardRoutesPlugin is a routesPluginFunc that also registers named
+// middleware.
+type guardRoutesPlugin struct {
+	routesPluginFunc
+	mw map[string]pact.Middleware
+}
+
+func (p guardRoutesPlugin) Middlewares() map[string]pact.Middleware { return p.mw }
+
+// refusalMiddlewares answers 401 (deny) and 403 (forbid) without calling
+// next, and 413 from readall when the body exceeds the route's limit.
+func refusalMiddlewares() map[string]pact.Middleware {
+	refuse := func(status int) pact.Middleware {
+		return func(http.Handler) http.Handler {
+			return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+				w.Header().Set("Content-Type", "application/json")
+				w.WriteHeader(status)
+				_, _ = w.Write([]byte(`{"error":"refused"}`))
+			})
+		}
+	}
+	return map[string]pact.Middleware{
+		"deny":   refuse(http.StatusUnauthorized),
+		"forbid": refuse(http.StatusForbidden),
+		"readall": func(next http.Handler) http.Handler {
+			return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				if _, err := io.ReadAll(r.Body); err != nil {
+					var tooLarge *http.MaxBytesError
+					if errors.As(err, &tooLarge) {
+						w.WriteHeader(http.StatusRequestEntityTooLarge)
+						return
+					}
+					w.WriteHeader(http.StatusBadRequest)
+					return
+				}
+				next.ServeHTTP(w, r)
+			})
+		},
+	}
+}
+
+// assertSPAProfileHeaders requires the spa profile's answer for an allowed
+// origin.
+func assertSPAProfileHeaders(t *testing.T, label string, rec *httptest.ResponseRecorder, status int) {
+	t.Helper()
+	const www = "https://www.example.com"
+	if rec.Code != status {
+		t.Fatalf("%s: status %d, want %d (body %q)", label, rec.Code, status, rec.Body.String())
+	}
+	hd := rec.Header()
+	if got := hd.Get("Access-Control-Allow-Origin"); got != www {
+		t.Errorf("%s: Allow-Origin = %q, want %q", label, got, www)
+	}
+	if got := hd.Get("Access-Control-Allow-Credentials"); got != "true" {
+		t.Errorf("%s: Allow-Credentials = %q, want true", label, got)
+	}
+	if !varyHas(hd, "Origin") {
+		t.Errorf("%s: Vary = %q, want Origin", label, hd.Values("Vary"))
+	}
+}
+
+func TestCORSProfileHeadersOnRefusals(t *testing.T) {
+	const www = "https://www.example.com"
+	panicky := func(http.ResponseWriter, *http.Request) { panic("boom") }
+	plugin := guardRoutesPlugin{
+		routesPluginFunc: routesPluginFunc{id: "acme.spa", routes: func(r pact.Router) {
+			r.Get("/deny", okHandler, "deny", "cors:spa")
+			r.Get("/forbid", okHandler, "forbid", "cors:spa")
+			r.Group("/guarded", Use("deny"), func(g pact.Router) {
+				g.Get("/me", okHandler, "cors:spa")
+			})
+			r.Get("/throttled", okHandler, "throttle:1,1", "cors:spa")
+			r.Post("/upload", okHandler, "body.limit:16", "readall", "cors:spa")
+			r.Get("/panic", panicky, "cors:spa")
+			r.GroupRaw("/raw", Use("cors:spa"), func(g pact.Router) {
+				g.Get("/panic", panicky)
+			})
+		}},
+		mw: refusalMiddlewares(),
+	}
+	h, err := Assemble(transportApp(t, "development", map[string]string{"http.yaml": transportBodyLimits + spaProfileYAML}), []party.Plugin{plugin})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	t.Run("guard_401", func(t *testing.T) {
+		assertSPAProfileHeaders(t, "deny", serveCORS(h, corsRequest(http.MethodGet, "/deny", www, false)), http.StatusUnauthorized)
+	})
+	t.Run("guard_403", func(t *testing.T) {
+		assertSPAProfileHeaders(t, "forbid", serveCORS(h, corsRequest(http.MethodGet, "/forbid", www, false)), http.StatusForbidden)
+	})
+	t.Run("group_guard_401", func(t *testing.T) {
+		assertSPAProfileHeaders(t, "group deny", serveCORS(h, corsRequest(http.MethodGet, "/guarded/me", www, false)), http.StatusUnauthorized)
+	})
+	t.Run("throttle_429", func(t *testing.T) {
+		assertSPAProfileHeaders(t, "first", serveCORS(h, corsRequest(http.MethodGet, "/throttled", www, false)), http.StatusOK)
+		assertSPAProfileHeaders(t, "second", serveCORS(h, corsRequest(http.MethodGet, "/throttled", www, false)), http.StatusTooManyRequests)
+	})
+	t.Run("body_limit_413", func(t *testing.T) {
+		req := httptest.NewRequest(http.MethodPost, "/upload", strings.NewReader(strings.Repeat("x", 64)))
+		req.Header.Set("Origin", www)
+		assertSPAProfileHeaders(t, "upload", serveCORS(h, req), http.StatusRequestEntityTooLarge)
+	})
+	t.Run("panic_500_json", func(t *testing.T) {
+		rec := serveCORS(h, corsRequest(http.MethodGet, "/panic", www, false))
+		assertSPAProfileHeaders(t, "panic", rec, http.StatusInternalServerError)
+		if !strings.Contains(rec.Header().Get("Content-Type"), "json") {
+			t.Errorf("panic Content-Type = %q, want JSON", rec.Header().Get("Content-Type"))
+		}
+	})
+	t.Run("panic_500_raw", func(t *testing.T) {
+		assertSPAProfileHeaders(t, "raw panic", serveCORS(h, corsRequest(http.MethodGet, "/raw/panic", www, false)), http.StatusInternalServerError)
+	})
+	t.Run("disallowed_origin", func(t *testing.T) {
+		rec := serveCORS(h, corsRequest(http.MethodGet, "/deny", "https://evil.example.org", false))
+		if rec.Code != http.StatusUnauthorized {
+			t.Fatalf("status %d, want 401", rec.Code)
+		}
+		if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" {
+			t.Errorf("Allow-Origin = %q, want none", got)
+		}
+		if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "" {
+			t.Errorf("Allow-Credentials = %q, want none", got)
+		}
+		if !varyHas(rec.Header(), "Origin") {
+			t.Errorf("Vary = %q, want Origin", rec.Header().Values("Vary"))
+		}
+	})
+
+	turnstilePlugin := routesPluginFunc{id: "acme.guest", routes: func(r pact.Router) {
+		r.Post("/guest", okHandler, "turnstile:closed", "cors:spa")
+	}}
+	post := func(h http.Handler) *httptest.ResponseRecorder {
+		req := httptest.NewRequest(http.MethodPost, "/guest", strings.NewReader(`{"name":"x"}`))
+		req.Header.Set("Content-Type", "application/json")
+		req.Header.Set("Origin", www)
+		return serveCORS(h, req)
+	}
+	t.Run("turnstile_422", func(t *testing.T) {
+		// BuildRouter builds one verifier per router, so each Turnstile
+		// case assembles its own app. An empty token is refused before any
+		// siteverify call.
+		app := transportApp(t, "development", map[string]string{"http.yaml": transportBodyLimits + spaProfileYAML})
+		if err := app.Publish[*bouncer.Turnstile](bouncer.NewTurnstile("test-secret", false)); err != nil {
+			t.Fatal(err)
+		}
+		h, err := Assemble(app, []party.Plugin{turnstilePlugin})
+		if err != nil {
+			t.Fatal(err)
+		}
+		assertSPAProfileHeaders(t, "turnstile without a token", post(h), http.StatusUnprocessableEntity)
+	})
+	t.Run("turnstile_503", func(t *testing.T) {
+		h, err := Assemble(transportApp(t, "production", map[string]string{"http.yaml": transportBodyLimits + spaProfileYAML}), []party.Plugin{turnstilePlugin})
+		if err != nil {
+			t.Fatal(err)
+		}
+		assertSPAProfileHeaders(t, "turnstile without a secret in production", post(h), http.StatusServiceUnavailable)
+	})
+}
+
+func TestCORSProfileVaryKeptWithHandlerVary(t *testing.T) {
+	const www = "https://www.example.com"
+	plugin := routesPluginFunc{id: "acme.spa", routes: func(r pact.Router) {
+		r.Get("/x/me", func(w http.ResponseWriter, _ *http.Request) {
+			w.Header().Set("Vary", "Accept-Language")
+			w.WriteHeader(http.StatusOK)
+		}, "cors:spa")
+	}}
+	h, err := Assemble(transportApp(t, "development", map[string]string{"http.yaml": transportBodyLimits + spaProfileYAML}), []party.Plugin{plugin})
+	if err != nil {
+		t.Fatal(err)
+	}
+	rec := serveCORS(h, corsRequest(http.MethodGet, "/x/me", www, false))
+	assertSPAProfileHeaders(t, "handler Vary", rec, http.StatusOK)
+	if !varyHas(rec.Header(), "Accept-Language") {
+		t.Errorf("Vary = %q, want Accept-Language too", rec.Header().Values("Vary"))
+	}
+}

modules/surf/router.go

@@ -413,6 +413,13 @@ func (r *Router) wrap(rt route) (http.Handler, error) {
 		return nil, fmt.Errorf("surf: plugin %q: %w", rt.pluginID, err)
 	}
 	h = orgSlot(h)
+	// The route's cors:<profile> (on the route or an enclosing group) is
+	// applied below, outside recovery, the body limit and locale, so every
+	// answer of the route carries it, a guard's refusal included.
+	profile, err := routeCORSProfile(rt)
+	if err != nil {
+		return nil, err
+	}
 	for i := len(rt.middleware) - 1; i >= 0; i-- {
 		name := rt.middleware[i]
 		if named, ok := r.named[name]; ok {
@@ -437,6 +444,8 @@ func (r *Router) wrap(rt route) (http.Handler, error) {
 					if _, ok := r.corsProfiles[param]; !ok {
 						return nil, fmt.Errorf("surf: plugin %q route %s %s references unknown CORS profile %q (define http.cors_profiles.%s)", rt.pluginID, rt.method, rt.path, param, param)
 					}
+					// Never composed at its list position: see below.
+					continue
 				}
 				if base == "turnstile" && param != "open" && param != "closed" {
 					return nil, fmt.Errorf("surf: plugin %q route %s %s: turnstile:%s must be turnstile:open or turnstile:closed", rt.pluginID, rt.method, rt.path, param)
@@ -467,6 +476,12 @@ func (r *Router) wrap(rt route) (http.Handler, error) {
 	} else {
 		h = recoverJSON(h)
 	}
+	// The CORS profile wraps recovery, so a refusal from a guard listed
+	// before it, a 413 from the body limit and a recovered 500 all carry
+	// the profile's headers; otherwise the browser hides them from the page.
+	if profile != "" {
+		h = r.corsProfiles[profile].profileMiddleware(h)
+	}
 	// Outermost: name the matched route for the access log (accesslog.go).
 	h = recordPattern(rt.method+" "+rt.path, h)
 	return h, nil
@@ -520,7 +535,8 @@ func BuildRouter(app *backpack.App, plugins []party.Plugin) (*Router, error) {
 	}
 	r.corsProfiles = map[string]*corsPolicy{}
 	if err := r.RegisterMiddlewareFactory("surf", "cors", func(param string) pact.Middleware {
-		// wrap refuses an unknown profile before the factory runs.
+		// The name stays reserved for surf, but wrap applies the profile
+		// itself, outermost, and never calls this factory for it.
 		return r.corsProfiles[param].profileMiddleware
 	}); err != nil {
 		return nil, err
@@ -793,6 +809,18 @@ func (*bufferedResponse) Flush() {}
 
 func (w *bufferedResponse) commit(dst http.ResponseWriter) {
 	for key, values := range w.header {
+		if key == "Vary" {
+			// Merged, not replaced: a Vary: Origin a CORS wrapper set on
+			// dst survives a handler that sets its own Vary.
+			for _, line := range values {
+				for _, v := range strings.Split(line, ",") {
+					if v = strings.TrimSpace(v); v != "" {
+						addVary(dst.Header(), v)
+					}
+				}
+			}
+			continue
+		}
 		dst.Header()[key] = append([]string(nil), values...)
 	}
 	status := w.status

scripts/check-phase19.3.sh

@@ -71,6 +71,7 @@ SURF_TESTS=(
 	TestCORSOptionsMatchesLaravel
 	TestCORSProfilePreflightRoute TestCORSProfileUnknown TestCORSProfileWithPathsRefused
 	TestCORSProfileOverlapGlobal TestCORSProfileConflictingProfilesSamePath
+	TestCORSProfileHeadersOnRefusals TestCORSProfileVaryKeptWithHandlerVary
 	TestTurnstileFactoryParams
 	TestAPIDocsOffByDefault TestAPIDocsServesPinnedPage TestAPIDocsSpecJSONOrder
 	TestAPIDocsJSONSpecInput TestAPIDocsProviderCount TestAPIDocsPathValidation