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

AGENTS.md

@@ -41,6 +41,7 @@ Ten internal packages under `internal/`:
 12. **Structured queries**: Type-safe query AST endpoint (`POST /v1/tables/{table}/query`) validated against schema, with permission enforcement, timestamp bucketing for cache optimization, and `DefaultMaxRows` (10,000) limit cap.
 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. **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/configuration.md`, `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 15 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).
 - **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/configuration.md

@@ -33,7 +33,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/deployment.md

@@ -109,7 +109,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

@@ -209,14 +209,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{}{}
 	}
@@ -225,20 +242,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,79 @@ 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)
+	assert.Empty(t, w.Header().Get("Access-Control-Allow-Origin"))
 }
 
 func TestNewRouter_RoutesRegistered(t *testing.T) {