From 00eace8068f27ebe8d9510a0cb25ca7c407f1393 Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Sun, 10 May 2026 20:12:19 +0000 Subject: [PATCH] fix(api/cors): narrow Bundle-2 routes from wildcard to NewCORS(corsCfg) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes CRIT-3 of the 2026-05-10 audit. Bundle 2's OIDC handshake + back-channel-logout + logout + bootstrap + breakglass-login routes were wrapped by middleware.CORS — a hard-coded Access-Control-Allow-Origin: * middleware that ignored the operator's CERTCTL_CORS_ORIGINS knob (CWE-942). The properly-configured middleware.NewCORS(corsCfg) exists right next to it but wasn't used here. The deprecation comment on middleware.CORS said "Kept for health endpoints" but Bundle 2 added four additional call sites without converting them. This commit: - Renames middleware.CORS -> middleware.CORSWildcard with a stronger doc block making the security tradeoff explicit at every remaining call site. The doc references the CI guard + the 2026-05-10 audit closure. - Adds a CorsCfg middleware.CORSConfig field to router.HandlerRegistry and threads it from cmd/server/main.go using the existing cfg.CORS.AllowedOrigins value. The same config that drives the global corsMiddleware now also drives the per-route NewCORS wraps for the auth-exempt direct r.mux.Handle blocks. - Swaps middleware.CORS -> middleware.NewCORS(reg.CorsCfg) for the 7 credentialed auth-exempt routes: - GET /auth/oidc/login - GET /auth/oidc/callback - POST /auth/oidc/back-channel-logout - POST /auth/logout - POST /auth/breakglass/login - GET /api/v1/auth/bootstrap - POST /api/v1/auth/bootstrap - Keeps middleware.CORSWildcard for the 4 credential-free probe routes: - GET /health - GET /ready - GET /api/v1/version - GET /api/v1/auth/info - Adds scripts/ci-guards/cors-wildcard-allowlist.sh — pins the 4-route allowlist; fails CI when a new middleware.CORSWildcard wrap appears outside the allowlist. Adding a new wildcard call site requires updating the allowlist AND documenting why in the commit body. Operators who configured CERTCTL_CORS_ORIGINS=https://admin.example.com expecting the OIDC + BCL + breakglass-login routes to honor it now do. Previously those routes ignored the knob and emitted ACAO: * regardless. Verification gate green: - gofmt -l . clean - go vet ./... clean - go test -short -count=1 ./internal/api/... ./internal/auth/... ./internal/domain/auth/ ./internal/service/auth/ ./cmd/server/ pass - go build ./... clean - scripts/ci-guards/cors-wildcard-allowlist.sh passes (4 allowlisted routes; zero violations) CRIT-1 + CRIT-2 from the same audit are already closed on this branch (commits 68ca42f, ca1e135); CRIT-4 / CRIT-5 remain open and continue to block the v2.1.0 tag. Spec: cowork/auth-bundles-fixes-2026-05-10/03-crit-3-cors-narrow.md. Refs: cowork/auth-bundles-audit-2026-05-10.md CRIT-3 --- cmd/server/main.go | 5 ++ internal/api/middleware/middleware.go | 22 ++++- internal/api/router/router.go | 39 ++++++--- scripts/ci-guards/cors-wildcard-allowlist.sh | 85 ++++++++++++++++++++ 4 files changed, 137 insertions(+), 14 deletions(-) create mode 100755 scripts/ci-guards/cors-wildcard-allowlist.sh diff --git a/cmd/server/main.go b/cmd/server/main.go index e6e0bd3..6033aa2 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -1335,6 +1335,11 @@ func main() { // admin_scep_intune, admin_est, intermediate_ca). Wraps live in // router.go via rbacGate(reg.Checker, perm, handler). Checker: authCheckerAdapter, + // Audit 2026-05-10 CRIT-3 closure — operator-configured CORS + // applied to the credentialed auth-exempt routes (OIDC handshake, + // BCL, logout, bootstrap, breakglass-login). Health probes + // continue to use middleware.CORSWildcard. + CorsCfg: middleware.CORSConfig{AllowedOrigins: cfg.CORS.AllowedOrigins}, }) // Register EST (RFC 7030) handlers if enabled. // diff --git a/internal/api/middleware/middleware.go b/internal/api/middleware/middleware.go index 10e27bd..6492b2d 100644 --- a/internal/api/middleware/middleware.go +++ b/internal/api/middleware/middleware.go @@ -371,9 +371,25 @@ func ContentType(next http.Handler) http.Handler { }) } -// CORS middleware adds CORS headers to allow cross-origin requests. -// Deprecated: Use NewCORS for configurable origins. Kept for health endpoints. -func CORS(next http.Handler) http.Handler { +// CORSWildcard emits Access-Control-Allow-Origin: * unconditionally. ONLY use +// for endpoints that (a) carry no credentials and (b) must be reachable from +// any origin (e.g. K8s/Docker health probes, Prometheus scrapers, the GUI's +// pre-login auth-info probe). Every call site MUST appear in +// scripts/ci-guards/cors-wildcard-allowlist.sh — adding a new call site +// without listing it in the allowlist fails CI. +// +// For credentialed endpoints (sessions, OIDC handshake, BCL, bootstrap, +// breakglass-login, every /api/v1/* mutation route) use +// middleware.NewCORS(corsCfg) which honors CERTCTL_CORS_ORIGINS and emits +// per-origin headers (with Vary: Origin for cache correctness). +// +// History: this function was named `CORS` pre-2026-05-10 and was applied as +// the default CORS middleware on the OIDC handshake, BCL, logout, bootstrap, +// and breakglass-login routes — CRIT-3 of the 2026-05-10 audit +// (cowork/auth-bundles-audit-2026-05-10.md). The fix narrowed those call +// sites to NewCORS(corsCfg) and renamed the wildcard form to make the +// security tradeoff explicit at every remaining call site. +func CORSWildcard(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, PATCH, OPTIONS") diff --git a/internal/api/router/router.go b/internal/api/router/router.go index 5b834f4..2eb4467 100644 --- a/internal/api/router/router.go +++ b/internal/api/router/router.go @@ -195,6 +195,23 @@ type HandlerRegistry struct { // (only valid in tests / demo deployments — production MUST // configure a Checker). Checker auth.PermissionChecker + + // CorsCfg is the operator-configured CORS middleware applied to the + // credentialed auth-exempt routes (OIDC handshake, BCL, logout, + // bootstrap, breakglass-login). Honors CERTCTL_CORS_ORIGINS — deny- + // by-default when AllowedOrigins is empty. Audit 2026-05-10 CRIT-3 + // closure: previously these routes used middleware.CORSWildcard + // (formerly middleware.CORS) which emitted Access-Control-Allow- + // Origin: * regardless of operator config, ignoring the + // CERTCTL_CORS_ORIGINS knob (CWE-942). + // + // Health probes (/health, /ready, /api/v1/version, /api/v1/auth/info) + // continue to use middleware.CORSWildcard because they must be + // reachable from any origin without credentials. Each wildcard call + // site is listed in scripts/ci-guards/cors-wildcard-allowlist.sh — + // the CI guard fails when a new wildcard wrap appears outside the + // allowlist. + CorsCfg middleware.CORSConfig // L-1 master closure (cat-l-fa0c1ac07ab5 + cat-l-8a1fb258a38a): // server-side bulk endpoints replace pre-L-1 client-side N×HTTP // loops in CertificatesPage.tsx. See handler/bulk_renewal.go and @@ -306,18 +323,18 @@ func (r *Router) RegisterHandlers(reg HandlerRegistry) { // Health endpoints (no auth middleware — must always be accessible) r.mux.Handle("GET /health", middleware.Chain( http.HandlerFunc(reg.Health.Health), - middleware.CORS, + middleware.CORSWildcard, middleware.ContentType, )) r.mux.Handle("GET /ready", middleware.Chain( http.HandlerFunc(reg.Health.Ready), - middleware.CORS, + middleware.CORSWildcard, middleware.ContentType, )) // Auth info endpoint (no auth middleware — GUI needs this before login) r.mux.Handle("GET /api/v1/auth/info", middleware.Chain( http.HandlerFunc(reg.Health.AuthInfo), - middleware.CORS, + middleware.CORSWildcard, middleware.ContentType, )) // Version endpoint (no auth middleware — used by rollout probes that @@ -328,7 +345,7 @@ func (r *Router) RegisterHandlers(reg HandlerRegistry) { // is preferred when present. r.mux.Handle("GET /api/v1/version", middleware.Chain( reg.Version, - middleware.CORS, + middleware.CORSWildcard, middleware.ContentType, )) // Auth check endpoint (uses full middleware chain via r.Register) @@ -340,12 +357,12 @@ func (r *Router) RegisterHandlers(reg HandlerRegistry) { // AuthExemptRouterRoutes allowlist above. r.mux.Handle("GET /api/v1/auth/bootstrap", middleware.Chain( http.HandlerFunc(reg.Bootstrap.Available), - middleware.CORS, + middleware.NewCORS(reg.CorsCfg), middleware.ContentType, )) r.mux.Handle("POST /api/v1/auth/bootstrap", middleware.Chain( http.HandlerFunc(reg.Bootstrap.Mint), - middleware.CORS, + middleware.NewCORS(reg.CorsCfg), middleware.ContentType, )) @@ -407,19 +424,19 @@ func (r *Router) RegisterHandlers(reg HandlerRegistry) { // /auth/logout -> caller's own session cookie r.mux.Handle("GET /auth/oidc/login", middleware.Chain( http.HandlerFunc(reg.AuthSessionOIDC.LoginInitiate), - middleware.CORS, middleware.ContentType, + middleware.NewCORS(reg.CorsCfg), middleware.ContentType, )) r.mux.Handle("GET /auth/oidc/callback", middleware.Chain( http.HandlerFunc(reg.AuthSessionOIDC.LoginCallback), - middleware.CORS, middleware.ContentType, + middleware.NewCORS(reg.CorsCfg), middleware.ContentType, )) r.mux.Handle("POST /auth/oidc/back-channel-logout", middleware.Chain( http.HandlerFunc(reg.AuthSessionOIDC.BackChannelLogout), - middleware.CORS, middleware.ContentType, + middleware.NewCORS(reg.CorsCfg), middleware.ContentType, )) r.mux.Handle("POST /auth/logout", middleware.Chain( http.HandlerFunc(reg.AuthSessionOIDC.Logout), - middleware.CORS, middleware.ContentType, + middleware.NewCORS(reg.CorsCfg), middleware.ContentType, )) // Session management. auth.session.list gates the all-actors @@ -457,7 +474,7 @@ func (r *Router) RegisterHandlers(reg HandlerRegistry) { if reg.AuthBreakglass != nil { r.mux.Handle("POST /auth/breakglass/login", middleware.Chain( http.HandlerFunc(reg.AuthBreakglass.Login), - middleware.CORS, middleware.ContentType, + middleware.NewCORS(reg.CorsCfg), middleware.ContentType, )) r.Register("POST /api/v1/auth/breakglass/credentials", rbacGate(reg.Checker, "auth.breakglass.admin", reg.AuthBreakglass.SetPassword)) r.Register("POST /api/v1/auth/breakglass/credentials/{actor_id}/unlock", rbacGate(reg.Checker, "auth.breakglass.admin", reg.AuthBreakglass.Unlock)) diff --git a/scripts/ci-guards/cors-wildcard-allowlist.sh b/scripts/ci-guards/cors-wildcard-allowlist.sh new file mode 100755 index 0000000..a58ff70 --- /dev/null +++ b/scripts/ci-guards/cors-wildcard-allowlist.sh @@ -0,0 +1,85 @@ +#!/usr/bin/env bash +# cors-wildcard-allowlist.sh — Audit 2026-05-10 CRIT-3 ratchet. +# +# middleware.CORSWildcard (formerly middleware.CORS) emits +# Access-Control-Allow-Origin: * unconditionally, ignoring the operator's +# CERTCTL_CORS_ORIGINS knob (CWE-942). It is ONLY safe to use on endpoints +# that (a) carry no credentials and (b) must be reachable from any origin +# (health probes, version probes, the GUI's pre-login auth-info probe). +# +# This guard greps for every middleware.CORSWildcard call site, extracts +# the nearest preceding r.mux.Handle("…") route string, and asserts that +# the route appears in the documented ALLOWLIST below. Adding a new +# wildcard-CORS wrap therefore requires either: +# +# 1. Adding the route to ALLOWLIST below AND documenting why in the +# commit body, or +# 2. Switching the call site to middleware.NewCORS(reg.CorsCfg). +# +# Closes CRIT-3 of cowork/auth-bundles-audit-2026-05-10.md. See also +# internal/api/middleware/middleware.go::CORSWildcard for the doc block. + +set -euo pipefail + +ROUTER=internal/api/router/router.go + +# Routes allowed to use middleware.CORSWildcard. Every entry must be a +# credential-free endpoint that operators expect to be reachable from any +# origin (Kubernetes probes, Prometheus, the pre-login GUI). +ALLOWLIST=( + "GET /health" # K8s/Docker liveness probe + "GET /ready" # K8s/Docker readiness probe + "GET /api/v1/version" # rollout probes; pre-auth + "GET /api/v1/auth/info" # GUI reads before login +) + +if [[ ! -f "$ROUTER" ]]; then + echo "FAIL: $ROUTER not found (run from certctl/ root)" + exit 1 +fi + +# Extract every (route, wrap) pair from the router by finding each +# r.mux.Handle("ROUTE", ...) block and checking whether its wrapping list +# contains middleware.CORSWildcard. +python3 - <