From 75a0ba8bd5273698b78e1dbc30d3b607d3858587 Mon Sep 17 00:00:00 2001 From: jack Date: Sat, 18 Jul 2026 12:30:33 +0800 Subject: [PATCH] fix(web): let the browser extension WS handshake past the Origin gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #141 hardened corsMiddleware to 403 any untrusted Origin before it reaches a handler. But a Chrome extension always sends 'Origin: chrome-extension://' on the WebSocket handshake, which is never same-origin, so /api/browser/ext/ws was being rejected before the bridge's own pairing-token auth could run — the extension could no longer connect. Exempt the extension WS endpoint (GET only, exact path) from the Origin gate, mirroring the existing isAuthExempt carve-out. The bridge still authenticates via its pairing token (verified: an evil-origin socket that completes the upgrade is rejected with 'authentication required'), so the drive-by-POST vector PR #141 closed stays closed. Adds TestCORSMiddlewareExtensionWSHandshake to pin the regression. --- internal/web/cors_test.go | 52 +++++++++++++++++++++++++++++++++++++++ internal/web/server.go | 17 ++++++++++++- 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/internal/web/cors_test.go b/internal/web/cors_test.go index d0cb8acd..954f9850 100644 --- a/internal/web/cors_test.go +++ b/internal/web/cors_test.go @@ -67,6 +67,58 @@ func TestCORSMiddlewareRejectsUntrustedSimpleRequestsBeforeSideEffects(t *testin } } +// TestCORSMiddlewareExtensionWSHandshake pins the PR #141 regression: a Chrome +// extension always sends `Origin: chrome-extension://` on the WS handshake, +// which is never same-origin. The Origin gate must let that handshake through so +// the bridge's own pairing-token auth (bridge.HandleWS) can run — otherwise the +// extension can never connect. The exemption must stay scoped to GET on the exact +// WS path so it cannot be abused to smuggle a cross-origin mutating POST. +func TestCORSMiddlewareExtensionWSHandshake(t *testing.T) { + const wsPath = "/api/browser/ext/ws" + const extOrigin = "chrome-extension://ekcnniaefmnhnemnpphikhgfoofnojnd" + reached := func(rec *httptest.ResponseRecorder) bool { return rec.Code == http.StatusTeapot } + + mkInner := func() http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusTeapot) // 418 = "reached the inner handler" + }) + } + req := func(method, path, origin string) *http.Request { + r := httptest.NewRequest(method, "http://127.0.0.1:8080"+path, nil) + r.Host = "127.0.0.1:8080" + if origin != "" { + r.Header.Set("Origin", origin) + } + return r + } + + cases := []struct { + name string + method string + path string + origin string + wantThrough bool // true = passes the Origin gate (reaches inner handler) + }{ + {"extension origin on WS path is admitted", http.MethodGet, wsPath, extOrigin, true}, + // Untrusted origins are also admitted here; the bridge token check is the + // real boundary on this endpoint. Pinning it documents the trade-off. + {"untrusted origin on WS path reaches token check", http.MethodGet, wsPath, "https://evil.com", true}, + {"extension origin on a mutating path is still blocked", http.MethodPost, "/api/browser/config", extOrigin, false}, + {"extension origin POST to WS path is blocked (GET-only)", http.MethodPost, wsPath, extOrigin, false}, + {"untrusted origin on a normal API is still blocked", http.MethodGet, "/api/config", "https://evil.com", false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + rec := httptest.NewRecorder() + corsMiddleware(mkInner()).ServeHTTP(rec, req(c.method, c.path, c.origin)) + if got := reached(rec); got != c.wantThrough { + t.Errorf("%s %s origin=%q: through=%v (status %d), want through=%v", + c.method, c.path, c.origin, got, rec.Code, c.wantThrough) + } + }) + } +} + func TestCORSMiddlewareAllowsTrustedOriginsAndNonBrowserClients(t *testing.T) { inner := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusTeapot) diff --git a/internal/web/server.go b/internal/web/server.go index fa1d5540..0521b48e 100644 --- a/internal/web/server.go +++ b/internal/web/server.go @@ -654,6 +654,13 @@ func isAllowedWebOrigin(r *http.Request) bool { return false } +// isBrowserExtensionWS reports whether r is the Chrome extension's WS handshake. +// The bridge authenticates via its own pairing token (see isAuthExempt), so the +// Origin gate must not 403 it merely for carrying a chrome-extension:// Origin. +func isBrowserExtensionWS(r *http.Request) bool { + return r.Method == http.MethodGet && r.URL.Path == "/api/browser/ext/ws" +} + func corsMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { origin := r.Header.Get("Origin") @@ -661,7 +668,15 @@ func corsMiddleware(next http.Handler) http.Handler { // page can send a "simple" no-cors POST whose response is unreadable but // whose side effect still happens. Reject an untrusted browser Origin before // any API handler can mutate config, start an agent, or control the Mac. - if origin != "" && !isAllowedWebOrigin(r) { + // + // Exception: the extension WS endpoint. Chrome extensions always send + // `Origin: chrome-extension://` on the WS handshake, which is never + // same-origin, so this gate would otherwise 403 the bridge before its own + // pairing-token auth runs (mirrors the isAuthExempt carve-out). This + // endpoint only ever upgrades a WS; it performs no config/agent mutation on + // a cross-origin simple request, so the exemption does not reopen the + // drive-by-POST vector this middleware exists to close. + if origin != "" && !isAllowedWebOrigin(r) && !isBrowserExtensionWS(r) { http.Error(w, "cross-origin request denied", http.StatusForbidden) return }