fix(dashboard): drop wildcard CORS on authenticated SSE endpoint#4
fix(dashboard): drop wildcard CORS on authenticated SSE endpoint#4jackthepunished wants to merge 1 commit intomainfrom
Conversation
The /api/dashboard/stream SSE handler hardcoded
Access-Control-Allow-Origin: *, allowing any third-party origin in a
logged-in user's browser to subscribe to that user's real-time dashboard
events.
Replace with an allowlist sourced from WS_ALLOWED_ORIGINS (same
dashboard frontend already protects the WebSocket upgrade). The handler
now takes authoritative control of CORS for this route, clearing the
permissive defaults set by httputil.CORS() upstream:
- empty Origin (same-origin / non-browser): no CORS headers emitted
- Origin in allowlist: echoed back with Vary: Origin and credentials
- "*" entry: echoes the request Origin (never literal "*", which
EventSource rejects with credentials)
- otherwise: 403
Fixes poyrazK#347.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 58 minutes and 44 seconds.Comment |
There was a problem hiding this comment.
Pull request overview
Hardens the authenticated dashboard SSE stream (/api/dashboard/stream) against cross-origin subscription by removing the wildcard CORS behavior and enforcing an explicit Origin allowlist (aligned with the existing WebSocket allowlist configuration).
Changes:
- Adds per-endpoint CORS enforcement for the dashboard SSE stream, removing
Access-Control-Allow-Origin: *and validatingOriginagainst an allowlist. - Extends the dashboard handler constructor to accept and parse allowed origins (including comma-separated input).
- Adds/updates tests to assert no wildcard CORS leakage and to validate allow/deny CORS behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| internal/handlers/dashboard_handler.go | Implements allowlist-based CORS handling for SSE and removes wildcard header emission. |
| internal/handlers/dashboard_handler_test.go | Tightens existing SSE test and adds a new CORS-focused test suite for the stream endpoint. |
| internal/api/setup/router.go | Wires cfg.WSAllowedOrigins into the dashboard handler so SSE and WS share the same allowlist source. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| go r.ServeHTTP(w, req) | ||
|
|
||
| time.Sleep(100 * time.Millisecond) | ||
| cancel() | ||
|
|
||
| assert.Contains(t, w.Header().Get("Content-Type"), "text/event-stream") | ||
| assert.Contains(t, w.Body.String(), "event:summary") | ||
| // Same-origin (no Origin header) → must not emit a wildcard CORS header. See #347. | ||
| assert.Empty(t, w.Header().Get("Access-Control-Allow-Origin")) |
| runStream := func(t *testing.T, h *DashboardHandler, origin string) *httptest.ResponseRecorder { | ||
| t.Helper() | ||
| gin.SetMode(gin.TestMode) | ||
| r := gin.New() |
| headers.Set("Access-Control-Allow-Origin", origin) | ||
| headers.Set("Access-Control-Allow-Credentials", "true") | ||
| headers.Set("Vary", "Origin") | ||
| return true |
| func (h *DashboardHandler) applyCORS(c *gin.Context) bool { | ||
| headers := c.Writer.Header() | ||
| headers.Del("Access-Control-Allow-Origin") | ||
| headers.Del("Access-Control-Allow-Credentials") |
| // literal "*" entry opts into permissive mode for non-browser clients; even | ||
| // then the response echoes the request Origin rather than "*", because the | ||
| // SSE EventSource API ignores wildcards when credentials are involved. See #347. |
| origin := c.GetHeader("Origin") | ||
| if origin == "" { | ||
| return true | ||
| } | ||
|
|
||
| allowed := false | ||
| for _, o := range h.allowedOrigins { | ||
| if o == "*" || o == origin { | ||
| allowed = true | ||
| break | ||
| } | ||
| } | ||
| if !allowed { | ||
| c.AbortWithStatus(http.StatusForbidden) | ||
| return false | ||
| } |
Summary
/api/dashboard/stream(an authenticated SSE endpoint) was sendingAccess-Control-Allow-Origin: *, letting any third-party origin in a logged-in user's browser subscribe to that user's real-time dashboard events.Originagainst an allowlist (sourced from the existingWS_ALLOWED_ORIGINSenv var that already gates the dashboard's WebSocket upgrade).Origin(same-origin / non-browser): noAccess-Control-*headers emittedOriginin allowlist: echoed back withVary: OriginandAllow-Credentials: true*allowlist entry: echoes the requestOrigin(never literal*, whichEventSourcerejects with credentials)403Test plan
TestDashboardHandlerStreamEventsCORScovers: allowed origin echoed, disallowed origin → 403 (service not called), empty allowlist rejects cross-origin,*echoes the origin, same-origin emits no CORS header.TestDashboardHandlerStreamEventstightened to assert no wildcard leaks on same-origin requests.go test ./internal/handlers/...— please run; Go isn't available in this dev container so I couldn't execute it locally.WS_ALLOWED_ORIGINSto the dashboard origin(s); without it the SSE stream becomes same-origin only.