diff --git a/pkg/channels/pico/pico.go b/pkg/channels/pico/pico.go index 7247f9db2..206e71f92 100644 --- a/pkg/channels/pico/pico.go +++ b/pkg/channels/pico/pico.go @@ -251,8 +251,7 @@ func (c *PicoChannel) handleWebSocket(w http.ResponseWriter, r *http.Request) { return } - // If the client authenticated via a subprotocol (e.g. "token.xxx"), echo - // it back in the upgrade response so the browser accepts the connection. + // Echo the matched subprotocol back so the browser accepts the upgrade. var responseHeader http.Header if proto := c.matchedSubprotocol(r); proto != "" { responseHeader = http.Header{"Sec-WebSocket-Protocol": {proto}} @@ -289,11 +288,10 @@ func (c *PicoChannel) handleWebSocket(w http.ResponseWriter, r *http.Request) { go c.readLoop(pc) } -// authenticate checks for a valid token in the following order: -// 1. Authorization: Bearer header (preferred) -// 2. Sec-WebSocket-Protocol subprotocol with prefix "token." — this lets -// browser-based clients pass the token without putting it in the URL -// 3. Query parameter "token" — only when AllowTokenQuery is explicitly on +// authenticate checks the request for a valid token: +// 1. Authorization: Bearer header +// 2. Sec-WebSocket-Protocol "token." (for browsers that can't set headers) +// 3. Query parameter "token" (only when AllowTokenQuery is on) func (c *PicoChannel) authenticate(r *http.Request) bool { token := c.config.Token if token == "" { @@ -323,8 +321,8 @@ func (c *PicoChannel) authenticate(r *http.Request) bool { return false } -// matchedSubprotocol returns the first Sec-WebSocket-Protocol value that -// carries a valid token (format: "token."), or "" if none match. +// matchedSubprotocol returns the "token." subprotocol that matches +// the configured token, or "" if none do. func (c *PicoChannel) matchedSubprotocol(r *http.Request) string { token := c.config.Token for _, proto := range websocket.Subprotocols(r) { diff --git a/web/backend/api/gateway.go b/web/backend/api/gateway.go index 1813cac92..f50f7609a 100644 --- a/web/backend/api/gateway.go +++ b/web/backend/api/gateway.go @@ -281,7 +281,7 @@ func (h *Handler) startGatewayLocked(initialStatus string) (int, error) { gateway.logs.Reset() // Ensure Pico Channel is configured before starting gateway - if _, err := h.ensurePicoChannel(); err != nil { + if _, err := h.ensurePicoChannel(""); err != nil { log.Printf("Warning: failed to ensure pico channel: %v", err) // Non-fatal: gateway can still start without pico channel } diff --git a/web/backend/api/pico.go b/web/backend/api/pico.go index b6a1c3cd9..2d2201e16 100644 --- a/web/backend/api/pico.go +++ b/web/backend/api/pico.go @@ -65,24 +65,14 @@ func (h *Handler) handleRegenPicoToken(w http.ResponseWriter, r *http.Request) { }) } -// defaultPicoOrigins is a restricted set of localhost origins used when the -// user hasn't configured any explicit allow_origins. This covers the common -// local-dev scenarios (Vite on 5173, launcher on 18800) without opening the -// WebSocket to arbitrary cross-origin pages. -var defaultPicoOrigins = []string{ - "http://localhost:5173", - "http://127.0.0.1:5173", - "http://localhost:18800", - "http://127.0.0.1:18800", -} - -// ensurePicoChannel checks if the Pico Channel is properly configured and -// enables it with minimal secure defaults if not. Returns true if config was changed. +// ensurePicoChannel enables the Pico channel with sane defaults if it isn't +// already configured. Returns true when the config was modified. // -// Setup only enables the channel and creates a token. It deliberately does not -// turn on allow_token_query (prefer header-based auth) or set wildcard origins -// (prefer a restricted localhost allowlist). -func (h *Handler) ensurePicoChannel() (bool, error) { +// callerOrigin is the Origin header from the setup request. If non-empty and +// no origins are configured yet, it's written as the allowed origin so the +// WebSocket handshake works for whatever host the caller is on (LAN, custom +// port, etc.). Pass "" when there's no request context. +func (h *Handler) ensurePicoChannel(callerOrigin string) (bool, error) { cfg, err := config.LoadConfig(h.configPath) if err != nil { return false, fmt.Errorf("failed to load config: %w", err) @@ -100,10 +90,9 @@ func (h *Handler) ensurePicoChannel() (bool, error) { changed = true } - // Only populate origins when the user hasn't configured any. Use a - // restricted localhost allowlist instead of "*" to limit the attack surface. - if len(cfg.Channels.Pico.AllowOrigins) == 0 { - cfg.Channels.Pico.AllowOrigins = defaultPicoOrigins + // Seed origins from the request instead of hardcoding ports. + if len(cfg.Channels.Pico.AllowOrigins) == 0 && callerOrigin != "" { + cfg.Channels.Pico.AllowOrigins = []string{callerOrigin} changed = true } @@ -120,7 +109,7 @@ func (h *Handler) ensurePicoChannel() (bool, error) { // // POST /api/pico/setup func (h *Handler) handlePicoSetup(w http.ResponseWriter, r *http.Request) { - changed, err := h.ensurePicoChannel() + changed, err := h.ensurePicoChannel(r.Header.Get("Origin")) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return diff --git a/web/backend/api/pico_test.go b/web/backend/api/pico_test.go index d0deebe89..46149fa09 100644 --- a/web/backend/api/pico_test.go +++ b/web/backend/api/pico_test.go @@ -14,7 +14,7 @@ func TestEnsurePicoChannel_FreshConfig(t *testing.T) { configPath := filepath.Join(t.TempDir(), "config.json") h := NewHandler(configPath) - changed, err := h.ensurePicoChannel() + changed, err := h.ensurePicoChannel("") if err != nil { t.Fatalf("ensurePicoChannel() error = %v", err) } @@ -39,7 +39,7 @@ func TestEnsurePicoChannel_DoesNotEnableTokenQuery(t *testing.T) { configPath := filepath.Join(t.TempDir(), "config.json") h := NewHandler(configPath) - if _, err := h.ensurePicoChannel(); err != nil { + if _, err := h.ensurePicoChannel(""); err != nil { t.Fatalf("ensurePicoChannel() error = %v", err) } @@ -57,7 +57,7 @@ func TestEnsurePicoChannel_DoesNotSetWildcardOrigins(t *testing.T) { configPath := filepath.Join(t.TempDir(), "config.json") h := NewHandler(configPath) - if _, err := h.ensurePicoChannel(); err != nil { + if _, err := h.ensurePicoChannel("http://localhost:18800"); err != nil { t.Fatalf("ensurePicoChannel() error = %v", err) } @@ -71,9 +71,44 @@ func TestEnsurePicoChannel_DoesNotSetWildcardOrigins(t *testing.T) { t.Error("setup must not set wildcard origin '*'") } } +} - if len(cfg.Channels.Pico.AllowOrigins) == 0 { - t.Error("expected default localhost origins to be populated") +func TestEnsurePicoChannel_NoOriginWithoutCaller(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.json") + h := NewHandler(configPath) + + if _, err := h.ensurePicoChannel(""); err != nil { + t.Fatalf("ensurePicoChannel() error = %v", err) + } + + cfg, err := config.LoadConfig(configPath) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + + // Without a caller origin, allow_origins stays empty (CheckOrigin + // allows all when the list is empty, so the channel still works). + if len(cfg.Channels.Pico.AllowOrigins) != 0 { + t.Errorf("allow_origins = %v, want empty when no caller origin", cfg.Channels.Pico.AllowOrigins) + } +} + +func TestEnsurePicoChannel_SetsCallerOrigin(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.json") + h := NewHandler(configPath) + + lanOrigin := "http://192.168.1.9:18800" + if _, err := h.ensurePicoChannel(lanOrigin); err != nil { + t.Fatalf("ensurePicoChannel() error = %v", err) + } + + cfg, err := config.LoadConfig(configPath) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + + if len(cfg.Channels.Pico.AllowOrigins) != 1 || cfg.Channels.Pico.AllowOrigins[0] != lanOrigin { + t.Errorf("allow_origins = %v, want [%s]", cfg.Channels.Pico.AllowOrigins, lanOrigin) } } @@ -92,7 +127,7 @@ func TestEnsurePicoChannel_PreservesUserSettings(t *testing.T) { h := NewHandler(configPath) - changed, err := h.ensurePicoChannel() + changed, err := h.ensurePicoChannel("") if err != nil { t.Fatalf("ensurePicoChannel() error = %v", err) } @@ -120,8 +155,10 @@ func TestEnsurePicoChannel_Idempotent(t *testing.T) { configPath := filepath.Join(t.TempDir(), "config.json") h := NewHandler(configPath) + origin := "http://localhost:18800" + // First call sets things up - if _, err := h.ensurePicoChannel(); err != nil { + if _, err := h.ensurePicoChannel(origin); err != nil { t.Fatalf("first ensurePicoChannel() error = %v", err) } @@ -129,7 +166,7 @@ func TestEnsurePicoChannel_Idempotent(t *testing.T) { token1 := cfg1.Channels.Pico.Token // Second call should be a no-op - changed, err := h.ensurePicoChannel() + changed, err := h.ensurePicoChannel(origin) if err != nil { t.Fatalf("second ensurePicoChannel() error = %v", err) } @@ -143,6 +180,30 @@ func TestEnsurePicoChannel_Idempotent(t *testing.T) { } } +func TestHandlePicoSetup_IncludesRequestOrigin(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.json") + h := NewHandler(configPath) + + req := httptest.NewRequest("POST", "/api/pico/setup", nil) + req.Header.Set("Origin", "http://10.0.0.5:3000") + rec := httptest.NewRecorder() + + h.handlePicoSetup(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK) + } + + cfg, err := config.LoadConfig(configPath) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + + if len(cfg.Channels.Pico.AllowOrigins) != 1 || cfg.Channels.Pico.AllowOrigins[0] != "http://10.0.0.5:3000" { + t.Errorf("allow_origins = %v, want [http://10.0.0.5:3000]", cfg.Channels.Pico.AllowOrigins) + } +} + func TestHandlePicoSetup_Response(t *testing.T) { configPath := filepath.Join(t.TempDir(), "config.json") h := NewHandler(configPath) diff --git a/web/frontend/src/lib/pico-chat-controller.ts b/web/frontend/src/lib/pico-chat-controller.ts index 27efdcc59..0e77d1ad0 100644 --- a/web/frontend/src/lib/pico-chat-controller.ts +++ b/web/frontend/src/lib/pico-chat-controller.ts @@ -166,8 +166,7 @@ export async function connectChat() { } const url = `${finalWsUrl}?session_id=${encodeURIComponent(activeSessionIdRef)}` - // Pass the token via the Sec-WebSocket-Protocol header instead of a query - // parameter to avoid leaking it in logs, browser history, and proxies. + // Send token as a subprotocol so it doesn't end up in the URL. const socket = new WebSocket(url, [`token.${token}`]) if (generation !== connectionGeneration) {