From a49222f37bc1b7fe4ed4dbbebbb56986fb80d3b5 Mon Sep 17 00:00:00 2001 From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Date: Mon, 11 May 2026 13:42:42 +0800 Subject: [PATCH] fix(realtime): allow same-origin WebSocket (mobile/CLI) (#2395) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(realtime): allow same-origin WebSocket clients (mobile/CLI) The previous CheckOrigin implementation (PR #2318) bypassed the Origin check whenever the request URL carried `client_platform=mobile` and no browser session cookie. That contract requires every native client to remember to add a query parameter — and in practice mobile clients hit ws://localhost:8080/ws with no extra params, so the Origin filled by the WebSocket library (the server's own host) gets rejected. Replace the platform-specific bypass with same-origin acceptance: if Origin's host equals the request Host, allow the upgrade. This is gorilla/websocket's default CheckOrigin behavior, restored alongside the existing cross-origin allowlist (for browser web/desktop clients). Native clients are now zero-config. CSRF defense is unaffected: SameSite=Strict cookies, the multica_csrf token, workspace membership check, and the allowlist itself remain in place. Browser CSWSH attacks fail both same-origin (browser forces Origin = page origin, not the server's Host) and allowlist checks. Refs: https://pkg.go.dev/github.com/gorilla/websocket https://cheatsheetseries.owasp.org/cheatsheets/WebSocket_Security_Cheat_Sheet.html Co-Authored-By: Claude Opus 4.7 Co-authored-by: multica-agent * fix(realtime): use case-insensitive Host comparison for same-origin HTTP host is case-insensitive (RFC 7230 §2.7.3), and gorilla/websocket's default checkSameOrigin uses equalASCIIFold(u.Host, r.Host). The plain == comparison would reject legitimate same-origin requests with a case-mismatched Host header (e.g. Host: LOCALHOST:8080 vs Origin: http://localhost:8080). Switch to strings.EqualFold and cover the case with a regression test. Co-Authored-By: Claude Opus 4.7 Co-authored-by: multica-agent --------- Co-authored-by: Claude Opus 4.7 Co-authored-by: multica-agent --- server/internal/realtime/hub.go | 17 +++--- server/internal/realtime/hub_test.go | 78 ++++++++++++++-------------- 2 files changed, 48 insertions(+), 47 deletions(-) diff --git a/server/internal/realtime/hub.go b/server/internal/realtime/hub.go index 4bfec8901..ab84c6a26 100644 --- a/server/internal/realtime/hub.go +++ b/server/internal/realtime/hub.go @@ -5,6 +5,7 @@ import ( "encoding/json" "log/slog" "net/http" + "net/url" "os" "strings" "sync" @@ -81,13 +82,15 @@ func checkOrigin(r *http.Request) bool { if origin == "" { return true } - // Native mobile clients authenticate with an explicit first-frame token. - // Origin is a browser CSRF control, so only skip it for mobile requests - // that are not carrying the browser session cookie. - if r.URL.Query().Get("client_platform") == "mobile" { - if _, err := r.Cookie(auth.AuthCookieName); err == http.ErrNoCookie { - return true - } + // Same-origin: native clients (mobile, CLI) have no real page host, so + // their WebSocket library fills Origin with the connection target — + // which equals the server's own Host. They authenticate via bearer + // token, not auto-attached cookies, so CSRF (the attack the explicit + // allowlist below defends against) does not apply. This matches the + // gorilla/websocket default CheckOrigin behavior; the allowlist exists + // in addition to support cross-origin browser clients (web/desktop). + if u, err := url.Parse(origin); err == nil && strings.EqualFold(u.Host, r.Host) { + return true } origins := allowedWSOrigins.Load().([]string) for _, allowed := range origins { diff --git a/server/internal/realtime/hub_test.go b/server/internal/realtime/hub_test.go index 541cb945d..df379ae48 100644 --- a/server/internal/realtime/hub_test.go +++ b/server/internal/realtime/hub_test.go @@ -81,46 +81,6 @@ func connectWS(t *testing.T, server *httptest.Server) *websocket.Conn { return conn } -func TestCheckOrigin_AllowsMobileClientWithoutCookie(t *testing.T) { - prevOrigins := allowedWSOrigins.Load().([]string) - SetAllowedOrigins([]string{"https://app.example.com"}) - t.Cleanup(func() { SetAllowedOrigins(prevOrigins) }) - - req := httptest.NewRequest(http.MethodGet, "/ws?client_platform=mobile", nil) - req.Header.Set("Origin", "https://not-allowed.example.com") - - if !checkOrigin(req) { - t.Fatal("expected mobile request without browser auth cookie to bypass Origin whitelist") - } -} - -func TestCheckOrigin_RejectsDisallowedOriginWithoutMobileClient(t *testing.T) { - prevOrigins := allowedWSOrigins.Load().([]string) - SetAllowedOrigins([]string{"https://app.example.com"}) - t.Cleanup(func() { SetAllowedOrigins(prevOrigins) }) - - req := httptest.NewRequest(http.MethodGet, "/ws", nil) - req.Header.Set("Origin", "https://not-allowed.example.com") - - if checkOrigin(req) { - t.Fatal("expected disallowed Origin without mobile client platform to be rejected") - } -} - -func TestCheckOrigin_RejectsMobileClientWithBrowserCookie(t *testing.T) { - prevOrigins := allowedWSOrigins.Load().([]string) - SetAllowedOrigins([]string{"https://app.example.com"}) - t.Cleanup(func() { SetAllowedOrigins(prevOrigins) }) - - req := httptest.NewRequest(http.MethodGet, "/ws?client_platform=mobile", nil) - req.Header.Set("Origin", "https://not-allowed.example.com") - req.AddCookie(&http.Cookie{Name: auth.AuthCookieName, Value: "browser-session"}) - - if checkOrigin(req) { - t.Fatal("expected disallowed mobile Origin with browser auth cookie to be rejected") - } -} - // totalClients counts all currently registered clients. func totalClients(hub *Hub) int { hub.mu.RLock() @@ -351,3 +311,41 @@ func (l *lockedWriter) Write(p []byte) (int, error) { defer l.mu.Unlock() return l.w.Write(p) } + +func TestCheckOrigin(t *testing.T) { + prev := allowedWSOrigins.Load().([]string) + SetAllowedOrigins([]string{ + "http://localhost:3000", + "https://multica.ai", + }) + t.Cleanup(func() { SetAllowedOrigins(prev) }) + + cases := []struct { + name string + host string + origin string + want bool + }{ + {"empty origin allowed", "api.multica.ai", "", true}, + {"same-origin allowed (native client default)", "localhost:8080", "http://localhost:8080", true}, + {"same-origin allowed (https)", "api.multica.ai", "https://api.multica.ai", true}, + {"same-origin allowed (case-insensitive host, RFC 7230)", "API.Multica.AI", "https://api.multica.ai", true}, + {"whitelisted origin allowed (web cross-origin)", "localhost:8080", "http://localhost:3000", true}, + {"whitelisted origin allowed (prod web)", "api.multica.ai", "https://multica.ai", true}, + {"unknown origin rejected (CSWSH defense)", "api.multica.ai", "https://evil.com", false}, + {"different port rejected", "localhost:8080", "http://localhost:9999", false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + r := httptest.NewRequest(http.MethodGet, "/ws", nil) + r.Host = tc.host + if tc.origin != "" { + r.Header.Set("Origin", tc.origin) + } + if got := checkOrigin(r); got != tc.want { + t.Fatalf("checkOrigin(host=%q, origin=%q) = %v, want %v", tc.host, tc.origin, got, tc.want) + } + }) + } +}