fix(api): drop CORS credentials + skip same-origin decoration

AGENTS.md

@@ -43,6 +43,7 @@ Twelve internal packages under `internal/`:
 13. **Named query pipes**: Pre-defined SQL templates (inspired by Tinybird) with parameter binding, role restrictions, and caching. Stored in NATS KV with `.sql` file directory bootstrap.
 14. **TypeScript SDK**: `@wavehouse/sdk` — zero-dependency client with typed query builder, real-time SSE, live queries with smart aggregation classification (incrementable/decomposable/poll), and codegen CLI.
 15. **Observability invariants**: Stdout is *always* 100% — the slog logger fans out to stdout AND OTLP, and stdout sampling would silently hide records that scraping pipelines (Promtail/Alloy/Vector → Loki) are paying to store. Sampling knobs apply only to OTLP push. WARN+ERROR records always export at 100% regardless of `logs.sample_rate` — silently dropping errors during incidents would be a worse failure mode than the cost of forwarding them all (this is a *non-configurable* floor; do not expose it). gRPC OTel exporters dial lazily, so an unreachable collector never blocks startup; transient failures surface via the OTel SDK's error handler. The OTel Prometheus exporter (when enabled) uses a *private* `prometheus.Registry` to avoid leaking process/Go collectors that `prometheus.DefaultRegisterer` auto-registers into our `/metrics` output. When changing the logger, the sampler, or the provider wiring, preserve these invariants.
+16. **Bearer-token-only CORS posture**: WaveHouse is a Bearer-token API — `Authorization: Bearer <jwt>` on every authenticated request, no cookies, no session middleware. The CORS middleware (`internal/api/router.go` `corsMiddleware`) deliberately **never** emits `Access-Control-Allow-Credentials`, because (a) we don't need it (Bearer tokens are explicit request headers, not browser-managed credentials) and (b) the historical pairing of `Allow-Credentials: true` with `Allow-Origin: *` is a CORS spec violation that browsers reject. The `cors_allowed_origins` allowlist controls *which origins can read responses*, not cookie scope. CSRF protection is structural: cross-site requests can't smuggle a Bearer token because the browser won't auto-attach `Authorization` headers cross-origin. Do not reintroduce cookie-based auth or `Access-Control-Allow-Credentials` without a separate design discussion — the current posture is the answer to GitHub issues #29 and #30.
 
 ## Code Conventions
 

CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **`STATUS_*` and old `ADMINS` consumers in `board-config.env`** — STATUS option IDs had only orchestrator-side consumers and are now unreferenced. `ADMINS` was restored to `board-config.env` after the initial orchestrator-removal commit dropped it (Gemini and Claude both flagged the resulting drift across three inlined copies); both `housekeeping.yml` and `dependabot-automerge.yml` now load `ADMINS` from `board-config.env`. `admin-approval.yml` keeps its own inline copy with the documented latency-avoidance reasoning.
 
 ### Fixed
+- **CORS middleware: spec-compliant wildcard, no credentials, no header decoration on same-origin** (`internal/api/router.go`, `internal/api/router_test.go`, `AGENTS.md`, `config.yaml`, `docs/src/content/docs/configuration.md`, `docs/src/content/docs/deployment.md`): closes [#29](https://github.com/Wave-RF/WaveHouse/issues/29) and bookends [#30](https://github.com/Wave-RF/WaveHouse/issues/30). Three behavior changes, one rationale. (1) Dropped `Access-Control-Allow-Credentials: true` entirely — WaveHouse is a Bearer-token API (`Authorization: Bearer <jwt>`), cookies are never used (verified: no `http.Cookie` / `SetCookie` anywhere in the Go tree, TS SDK sends no `credentials: 'include'`), so credentials mode is unnecessary AND the previous combination of `Allow-Credentials: true` with `Allow-Origin: *` violated the CORS spec — browsers reject that pairing, which silently broke any client that ever set `credentials: 'include'`. (2) Requests with no `Origin` header (same-origin browser navigation, server-to-server, curl) now skip the CORS decoration entirely instead of unconditionally stamping `Allow-Methods`/`Allow-Headers`/`Allow-Credentials` on every response. (3) A preflight from a disallowed origin still returns 204 but with no CORS headers, which the browser treats as preflight failure — same outcome as before but without leaking the methods/headers list to origins that aren't allowed. Allowlist mode sets `Vary: Origin` on both hits *and* rejects so shared caches can't memoize a headerless reject under the URL alone and replay it to a later allowed-origin request. Test coverage in `router_test.go` pins each branch: wildcard echoes `*`, allowlist hit echoes origin + `Vary: Origin`, allowlist miss gets no `Allow-Origin` but still gets `Vary: Origin` (both for regular and OPTIONS), no-Origin requests pass through clean, and a table-driven test asserts `Allow-Credentials` is never emitted across wildcard / empty-allowlist / allowlist-hit. Posture is documented as `AGENTS.md` §"Key Design Decisions" item 16 so future contributors don't reintroduce credentials or cookie auth without a design discussion. Config sample updated with the dev recipe (point at `http://localhost:3000` etc. instead of `*` once a frontend is built).
 - **OTel shutdown no longer hangs process exit when the collector is unreachable** (`cmd/wavehouse/main.go`): the `defer otelShutdown(context.Background())` was unbounded, and the OTel SDK's batch processors don't fully honor the shutdown context against an unreachable gRPC endpoint (the dial retries with backoff continue past the deadline). Bounded the shutdown to 5s. Discovered while writing `tests/integration/otel_test.go` `TestOTel_UnreachableEndpoint_DoesNotBlockStartupOrEmits`.
 - **Coverage badge step no longer fails on every main push** (`.testcoverage.yml`, `AGENTS.md`): the `profile:` field pointed at `tmp/coverage/unit/coverage.txt` (~73% — `./internal/...` only) but `threshold.total` was 80, so `vladopajic/go-test-coverage`'s own gate failed unconditionally even after `make ci` had passed its per-suite + merged gates. Switched the action's profile to `tmp/coverage/total/coverage.txt` — the merged unit+integration+e2e profile `scripts/cov merge` produces at the end of `make ci` (~81% across the same files) — so the 80% gate applies to the project-wide number the badge advertises, not to a single suite. Per-suite gates in the YAML's `suites:` block continue to run inside `make ci` itself against the per-suite profiles before the badge step ever fires. AGENTS.md updated to state 80% project-wide minimum + per-suite minima.
 - **Dependabot auto-merge no longer slipped past CI on consolidated-pipeline PRs** (`main branch protection` ruleset; `.github/workflows/project-orchestrator.yml`; `.github/workflows/claude-review.yml`; `AGENTS.md`): on commit `93b3206` the previously six-job CI was collapsed into a single job named `CI`, but three places still referenced the old multi-job names — the ruleset's `required_status_checks` (`Validate` + `Admin approval` only — `CI` not listed), the `bot-clean` check in `project-orchestrator.yml` (looking for `Build`/`Validate`/`Lint`/`Test`/`Integration Tests`/`SDK Tests`), and `REQUIRED_CHECKS` in `claude-review.yml` (same six). Net effect: a Dependabot PR opened at T+0 would have its `Validate` and `Admin approval` checks satisfied within ~30s (PR-title workflow + the auto-approval workflow), `gh pr merge --auto` would fire, GitHub would see all *required* checks green, and the PR would squash-merge before `ci.yml` even started running. Pre-consolidation runs (PR #90 etc.) didn't expose this because GitHub's auto-merge waits for in-flight checks as a courtesy, and the multi-job CI was producing checks before auto-merge fired — but with one collapsed `CI` check that didn't start until later, the courtesy wait disappeared. Fix: added `CI` to the ruleset's required-checks (now `Admin approval` / `CI` / `Validate`) via the `gh api PUT /repos/Wave-RF/WaveHouse/rulesets/15353356` round-trip, and updated both bot-clean lists to look for `CI` + `Validate`. AGENTS.md note about the required-check set updated to match. Diagnosis steps recorded inline as code comments in both workflow files so future-me knows where to look if the check name changes again.

config.yaml

@@ -6,6 +6,14 @@ data_dir: ./data
 server:
   port: 8080
   shutdown_timeout: 10
+  # CORS allowlist. "*" allows any browser origin; this is fine in dev because
+  # WaveHouse is a Bearer-token API (no cookies, no credentials mode — see
+  # internal/api/router.go corsMiddleware), so cross-origin XHR can't smuggle
+  # auth. In production with a known frontend domain, replace "*" with the
+  # explicit list, e.g.:
+  #   cors_allowed_origins:
+  #     - "https://dashboard.wavehouse.example"
+  #     - "http://localhost:3000"   # local frontend dev server
   cors_allowed_origins:
     - "*"
 

docs/src/content/docs/configuration.md

@@ -38,7 +38,7 @@ export WH_CONFIG=/etc/wavehouse/config.yaml
 | -------- | ------- | ------- | ----------- |
 | `server.port` | `WH_SERVER_PORT` | `8080` | HTTP server listen port. |
 | `server.shutdown_timeout` | `WH_SERVER_SHUTDOWN_TIMEOUT` | `10` | Graceful shutdown timeout in seconds. |
-| `server.cors_allowed_origins` | `WH_SERVER_CORS_ALLOWED_ORIGINS` | `*` | Comma-separated list of allowed CORS origins. `*` allows all origins. |
+| `server.cors_allowed_origins` | `WH_SERVER_CORS_ALLOWED_ORIGINS` | `*` | Comma-separated list of allowed CORS origins. `*` allows any browser origin. WaveHouse is a Bearer-token API — `Access-Control-Allow-Credentials` is intentionally never sent, so this allowlist controls *which origins can read responses*, not cookie scope. Tighten to your frontend's exact origin(s) in production (e.g. `https://dashboard.example.com,http://localhost:3000`). |
 
 ### ClickHouse
 

docs/src/content/docs/deployment.md

@@ -116,7 +116,10 @@ WH_CH_HTTP_SCHEME=http              # Scheme for Bento HTTP inserts (http/https)
 # Schema discovery
 WH_SCHEMA_REFRESH_INTERVAL=60      # Seconds between schema refreshes
 
-# CORS
+# CORS — comma-separated allowlist (or "*" for any origin).
+# WaveHouse is a Bearer-token API; no cookies are used and the middleware
+# deliberately omits Access-Control-Allow-Credentials, so this allowlist only
+# controls *which origins can read responses*, not cookie scope.
 WH_SERVER_CORS_ALLOWED_ORIGINS=https://app.example.com,https://admin.example.com
 
 # Optional auth

internal/api/router.go

@@ -233,14 +233,31 @@ func RequireRole(authEnabled bool, roles ...string) func(http.Handler) http.Hand
 }
 
 // corsMiddleware handles CORS preflight and response headers.
-// When allowedOrigins contains "*", any origin is accepted.
-// Otherwise only listed origins receive CORS headers.
+//
+// Origin policy:
+//   - allowedOrigins == nil/empty OR contains "*": any origin is accepted.
+//     The response echoes "*" (no Vary), which is correct for our Bearer-auth
+//     API because no credentials are required to read the response.
+//   - Otherwise: only origins in the allowlist receive Access-Control-Allow-Origin
+//     (echoed back, with Vary: Origin so caches key on it).
+//
+// Credentials: we deliberately do NOT emit Access-Control-Allow-Credentials.
+// WaveHouse is a Bearer-token API (see internal/api/middleware.go) — clients
+// send Authorization: Bearer <jwt> as an explicit request header, not via
+// browser-managed cookies, so credentials mode is unnecessary. Sending
+// Allow-Credentials: true together with Allow-Origin: "*" is also a CORS
+// spec violation that browsers reject. Keeping credentials off both fixes
+// the spec violation and shrinks the CSRF surface (see issue #30).
+//
+// Non-CORS requests (no Origin header) are passed through unchanged — we
+// don't decorate same-origin responses with CORS noise.
 func corsMiddleware(allowedOrigins []string) func(http.Handler) http.Handler {
 	allowAll := len(allowedOrigins) == 0
 	allowedSet := make(map[string]struct{}, len(allowedOrigins))
 	for _, o := range allowedOrigins {
 		if o == "*" {
 			allowAll = true
+			continue
 		}
 		allowedSet[o] = struct{}{}
 	}
@@ -249,20 +266,42 @@ func corsMiddleware(allowedOrigins []string) func(http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			origin := r.Header.Get("Origin")
 
-			if allowAll {
+			// Same-origin / non-browser request: skip CORS decoration entirely.
+			if origin == "" {
+				next.ServeHTTP(w, r)
+				return
+			}
+
+			allowed := false
+			switch {
+			case allowAll:
 				w.Header().Set("Access-Control-Allow-Origin", "*")
-			} else if _, ok := allowedSet[origin]; ok && origin != "" {
-				w.Header().Set("Access-Control-Allow-Origin", origin)
+				allowed = true
+			default:
+				// In allowlist mode the response is per-origin, so always set
+				// Vary: Origin — even when the origin is rejected. Without
+				// this, a shared cache could memoize the headerless reject
+				// response under the URL alone and replay it to a later
+				// allowed-origin request, stripping the CORS headers and
+				// breaking the legitimate client.
 				w.Header().Set("Vary", "Origin")
+				if _, ok := allowedSet[origin]; ok {
+					w.Header().Set("Access-Control-Allow-Origin", origin)
+					allowed = true
+				}
 			}
 
-			w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS")
-			w.Header().Set("Access-Control-Allow-Headers", "Accept, Authorization, Content-Type, X-Request-ID")
-			w.Header().Set("Access-Control-Expose-Headers", "X-Cache, X-Request-ID")
-			w.Header().Set("Access-Control-Allow-Credentials", "true")
-			w.Header().Set("Access-Control-Max-Age", "3600")
+			if allowed {
+				w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS")
+				w.Header().Set("Access-Control-Allow-Headers", "Accept, Authorization, Content-Type, X-Request-ID")
+				w.Header().Set("Access-Control-Expose-Headers", "X-Cache, X-Request-ID")
+				w.Header().Set("Access-Control-Max-Age", "3600")
+			}
 
 			if r.Method == http.MethodOptions {
+				// Always short-circuit OPTIONS — disallowed origins simply get a
+				// 204 with no CORS headers, which the browser will treat as a
+				// preflight failure.
 				w.WriteHeader(http.StatusNoContent)
 				return
 			}

internal/api/router_test.go

@@ -124,6 +124,91 @@ func TestCORSMiddleware_BlockedOrigin(t *testing.T) {
 
 	assert.True(t, called, "handler still invoked for non-allowed origin")
 	assert.Empty(t, w.Header().Get("Access-Control-Allow-Origin"))
+	assert.Empty(t, w.Header().Get("Access-Control-Allow-Methods"),
+		"non-allowed origins must not receive any CORS headers")
+	// Vary: Origin is still set even though we reject the origin, so that a
+	// shared cache can't memoize this headerless reject response and replay
+	// it to a later allowed-origin request.
+	assert.Equal(t, "Origin", w.Header().Get("Vary"),
+		"allowlist mode must always emit Vary: Origin to keep caches per-origin")
+}
+
+// TestCORSMiddleware_NoCredentialsHeader pins the deliberate omission of
+// Access-Control-Allow-Credentials. WaveHouse is a Bearer-token API; cookies
+// are never used (see issue #30). Emitting credentials=true with a wildcard
+// origin also breaks browsers per the CORS spec.
+func TestCORSMiddleware_NoCredentialsHeader(t *testing.T) {
+	t.Parallel()
+	cases := []struct {
+		name    string
+		allowed []string
+		origin  string
+	}{
+		{"wildcard", []string{"*"}, "https://anything.example.com"},
+		{"empty-allowlist-is-wildcard", nil, "https://anything.example.com"},
+		{"allowlist-hit", []string{"https://app.example.com"}, "https://app.example.com"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			handler := corsMiddleware(tc.allowed)(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+				w.WriteHeader(http.StatusOK)
+			}))
+			req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/", nil)
+			req.Header.Set("Origin", tc.origin)
+			w := httptest.NewRecorder()
+			handler.ServeHTTP(w, req)
+			assert.Empty(t, w.Header().Get("Access-Control-Allow-Credentials"),
+				"Bearer-token API must not emit Access-Control-Allow-Credentials")
+		})
+	}
+}
+
+// TestCORSMiddleware_NoOriginIsPassthrough ensures same-origin / non-browser
+// callers don't get CORS response headers stamped onto every response.
+func TestCORSMiddleware_NoOriginIsPassthrough(t *testing.T) {
+	t.Parallel()
+	handler := corsMiddleware([]string{"*"})(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+	req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/", nil)
+	w := httptest.NewRecorder()
+	handler.ServeHTTP(w, req)
+
+	assert.Equal(t, http.StatusOK, w.Code)
+	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"))
+}
+
+// TestCORSMiddleware_BlockedOriginPreflight pins that a preflight from a
+// disallowed origin returns 204 with no CORS headers — the browser treats
+// that as a preflight failure, so the actual request never fires.
+func TestCORSMiddleware_BlockedOriginPreflight(t *testing.T) {
+	t.Parallel()
+	handler := corsMiddleware([]string{"https://allowed.com"})(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
+		t.Fatal("should not reach handler on OPTIONS")
+	}))
+
+	req := httptest.NewRequestWithContext(context.Background(), http.MethodOptions, "/", nil)
+	req.Header.Set("Origin", "https://evil.com")
+	w := httptest.NewRecorder()
+	handler.ServeHTTP(w, req)
+
+	assert.Equal(t, http.StatusNoContent, w.Code)
+	// Pin the full "no CORS headers" contract — Allow-Origin alone isn't
+	// enough; a regression that leaked Allow-Methods/Allow-Headers/etc. to a
+	// disallowed origin would still slip through that single assertion.
+	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.Empty(t, w.Header().Get("Access-Control-Expose-Headers"))
+	assert.Empty(t, w.Header().Get("Access-Control-Allow-Credentials"))
+	assert.Empty(t, w.Header().Get("Access-Control-Max-Age"))
+	// Vary: Origin is still set in allowlist mode even on a reject, so a
+	// shared cache can't memoize this headerless 204 under the URL alone
+	// and replay it to a later allowed-origin preflight.
+	assert.Equal(t, "Origin", w.Header().Get("Vary"))
 }
 
 func TestNewRouter_RoutesRegistered(t *testing.T) {