fix: Use secure defaults for Pico channel setup and stop leaking the token in the URL

pkg/channels/pico/pico.go

@@ -251,7 +251,14 @@ func (c *PicoChannel) handleWebSocket(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	conn, err := c.upgrader.Upgrade(w, r, nil)
+	// If the client authenticated via a subprotocol (e.g. "token.xxx"), echo
+	// it back in the upgrade response so the browser accepts the connection.
+	var responseHeader http.Header
+	if proto := c.matchedSubprotocol(r); proto != "" {
+		responseHeader = http.Header{"Sec-WebSocket-Protocol": {proto}}
+	}
+
+	conn, err := c.upgrader.Upgrade(w, r, responseHeader)
 	if err != nil {
 		logger.ErrorCF("pico", "WebSocket upgrade failed", map[string]any{
 			"error": err.Error(),
@@ -282,8 +289,11 @@ func (c *PicoChannel) handleWebSocket(w http.ResponseWriter, r *http.Request) {
 	go c.readLoop(pc)
 }
 
-// authenticate checks the Bearer token from the Authorization header.
-// Query parameter authentication is only allowed when AllowTokenQuery is explicitly enabled.
+// authenticate checks for a valid token in the following order:
+//  1. Authorization: Bearer <token> 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
 func (c *PicoChannel) authenticate(r *http.Request) bool {
 	token := c.config.Token
 	if token == "" {
@@ -298,6 +308,11 @@ func (c *PicoChannel) authenticate(r *http.Request) bool {
 		}
 	}
 
+	// Check Sec-WebSocket-Protocol subprotocol ("token.<value>")
+	if c.matchedSubprotocol(r) != "" {
+		return true
+	}
+
 	// Check query parameter only when explicitly allowed
 	if c.config.AllowTokenQuery {
 		if r.URL.Query().Get("token") == token {
@@ -308,6 +323,18 @@ 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.<value>"), or "" if none match.
+func (c *PicoChannel) matchedSubprotocol(r *http.Request) string {
+	token := c.config.Token
+	for _, proto := range websocket.Subprotocols(r) {
+		if after, ok := strings.CutPrefix(proto, "token."); ok && after == token {
+			return proto
+		}
+	}
+	return ""
+}
+
 // readLoop reads messages from a WebSocket connection.
 func (c *PicoChannel) readLoop(pc *picoConn) {
 	defer func() {

web/backend/api/pico.go

@@ -65,8 +65,23 @@ 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 sensible defaults if not. Returns true if config was changed.
+// enables it with minimal secure defaults if not. Returns true if config was changed.
+//
+// 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) {
 	cfg, err := config.LoadConfig(h.configPath)
 	if err != nil {
@@ -85,14 +100,10 @@ func (h *Handler) ensurePicoChannel() (bool, error) {
 		changed = true
 	}
 
-	if !cfg.Channels.Pico.AllowTokenQuery {
-		cfg.Channels.Pico.AllowTokenQuery = true
-		changed = true
-	}
-
-	// Make sure origins are allowed (frontend might be running on a different port like 5173 during dev)
+	// 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 = []string{"*"}
+		cfg.Channels.Pico.AllowOrigins = defaultPicoOrigins
 		changed = true
 	}
 

web/backend/api/pico_test.go

@@ -0,0 +1,176 @@
+package api
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"path/filepath"
+	"testing"
+
+	"github.com/sipeed/picoclaw/pkg/config"
+)
+
+func TestEnsurePicoChannel_FreshConfig(t *testing.T) {
+	configPath := filepath.Join(t.TempDir(), "config.json")
+	h := NewHandler(configPath)
+
+	changed, err := h.ensurePicoChannel()
+	if err != nil {
+		t.Fatalf("ensurePicoChannel() error = %v", err)
+	}
+	if !changed {
+		t.Fatal("ensurePicoChannel() should report changed on a fresh config")
+	}
+
+	cfg, err := config.LoadConfig(configPath)
+	if err != nil {
+		t.Fatalf("LoadConfig() error = %v", err)
+	}
+
+	if !cfg.Channels.Pico.Enabled {
+		t.Error("expected Pico to be enabled after setup")
+	}
+	if cfg.Channels.Pico.Token == "" {
+		t.Error("expected a non-empty token after setup")
+	}
+}
+
+func TestEnsurePicoChannel_DoesNotEnableTokenQuery(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)
+	}
+
+	if cfg.Channels.Pico.AllowTokenQuery {
+		t.Error("setup must not enable allow_token_query by default")
+	}
+}
+
+func TestEnsurePicoChannel_DoesNotSetWildcardOrigins(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)
+	}
+
+	for _, origin := range cfg.Channels.Pico.AllowOrigins {
+		if origin == "*" {
+			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_PreservesUserSettings(t *testing.T) {
+	configPath := filepath.Join(t.TempDir(), "config.json")
+
+	// Pre-configure with custom user settings
+	cfg := config.DefaultConfig()
+	cfg.Channels.Pico.Enabled = true
+	cfg.Channels.Pico.Token = "user-custom-token"
+	cfg.Channels.Pico.AllowTokenQuery = true
+	cfg.Channels.Pico.AllowOrigins = []string{"https://myapp.example.com"}
+	if err := config.SaveConfig(configPath, cfg); err != nil {
+		t.Fatalf("SaveConfig() error = %v", err)
+	}
+
+	h := NewHandler(configPath)
+
+	changed, err := h.ensurePicoChannel()
+	if err != nil {
+		t.Fatalf("ensurePicoChannel() error = %v", err)
+	}
+	if changed {
+		t.Error("ensurePicoChannel() should not change a fully configured config")
+	}
+
+	cfg, err = config.LoadConfig(configPath)
+	if err != nil {
+		t.Fatalf("LoadConfig() error = %v", err)
+	}
+
+	if cfg.Channels.Pico.Token != "user-custom-token" {
+		t.Errorf("token = %q, want %q", cfg.Channels.Pico.Token, "user-custom-token")
+	}
+	if !cfg.Channels.Pico.AllowTokenQuery {
+		t.Error("user's allow_token_query=true must be preserved")
+	}
+	if len(cfg.Channels.Pico.AllowOrigins) != 1 || cfg.Channels.Pico.AllowOrigins[0] != "https://myapp.example.com" {
+		t.Errorf("allow_origins = %v, want [https://myapp.example.com]", cfg.Channels.Pico.AllowOrigins)
+	}
+}
+
+func TestEnsurePicoChannel_Idempotent(t *testing.T) {
+	configPath := filepath.Join(t.TempDir(), "config.json")
+	h := NewHandler(configPath)
+
+	// First call sets things up
+	if _, err := h.ensurePicoChannel(); err != nil {
+		t.Fatalf("first ensurePicoChannel() error = %v", err)
+	}
+
+	cfg1, _ := config.LoadConfig(configPath)
+	token1 := cfg1.Channels.Pico.Token
+
+	// Second call should be a no-op
+	changed, err := h.ensurePicoChannel()
+	if err != nil {
+		t.Fatalf("second ensurePicoChannel() error = %v", err)
+	}
+	if changed {
+		t.Error("second ensurePicoChannel() should not report changed")
+	}
+
+	cfg2, _ := config.LoadConfig(configPath)
+	if cfg2.Channels.Pico.Token != token1 {
+		t.Error("token should not change on subsequent calls")
+	}
+}
+
+func TestHandlePicoSetup_Response(t *testing.T) {
+	configPath := filepath.Join(t.TempDir(), "config.json")
+	h := NewHandler(configPath)
+
+	req := httptest.NewRequest("POST", "/api/pico/setup", nil)
+	rec := httptest.NewRecorder()
+
+	h.handlePicoSetup(rec, req)
+
+	if rec.Code != http.StatusOK {
+		t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK)
+	}
+
+	var resp map[string]any
+	if err := json.NewDecoder(rec.Body).Decode(&resp); err != nil {
+		t.Fatalf("failed to decode response: %v", err)
+	}
+
+	if resp["token"] == nil || resp["token"] == "" {
+		t.Error("response should contain a non-empty token")
+	}
+	if resp["ws_url"] == nil || resp["ws_url"] == "" {
+		t.Error("response should contain ws_url")
+	}
+	if resp["enabled"] != true {
+		t.Error("response should have enabled=true")
+	}
+	if resp["changed"] != true {
+		t.Error("response should have changed=true on first setup")
+	}
+}

web/frontend/src/lib/pico-chat-controller.ts

@@ -165,8 +165,10 @@ export async function connectChat() {
       console.warn("Could not parse ws_url:", error)
     }
 
-    const url = `${finalWsUrl}?token=${encodeURIComponent(token)}&session_id=${encodeURIComponent(activeSessionIdRef)}`
-    const socket = new WebSocket(url)
+    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.
+    const socket = new WebSocket(url, [`token.${token}`])
 
     if (generation !== connectionGeneration) {
       socket.close()