fix(serve, weixin): an HTTP client names no migration source; a login id names one file

Two weaknesses found while triaging the open CodeQL alerts on main-v2,
none of which they flagged:

- POST /submit {"input":"/migrate --from <dir>"} read and imported
  sessions from any host directory the client named, and told it whether
  the path existed. The HTTP entry now refuses a /migrate or /migration
  that names a source, next to its refusal of `!` shell input. A plain
  /migrate still works there; the terminal UI and desktop keep --from.
- The Weixin bot saved its token under the ilink_bot_id from the upstream
  login response. A malicious or intercepted upstream could place the file
  outside the accounts directory. The id must now be a single local file
  name.

Each has a test that fails without its fix.

The 196 open alerts themselves are false positives: session ids are
validated by SessionRef.validate, legacy transcript paths are confined by
resolveSessionPath, the model-settings probe is pinned to loopback, and
proc.Command runs argv without a shell.

internal/bot/weixin/weixin_login.go

@@ -46,6 +46,9 @@ func savedAccountPath(accountID string) string {
 	if root == "" || accountID == "" {
 		return ""
 	}
+	if !validAccountID(accountID) {
+		return ""
+	}
 	return filepath.Join(weixinAccountDir(root), accountID+".json")
 }
 
@@ -100,7 +103,15 @@ func HasSavedAccount(accountID string) bool {
 	return err == nil && account.Token != ""
 }
 
+// validAccountID keeps an id from the upstream login response to one file name.
+func validAccountID(accountID string) bool {
+	return filepath.IsLocal(accountID) && filepath.Base(accountID) == accountID
+}
+
 func saveAccount(accountID string, account savedAccount) error {
+	if !validAccountID(accountID) {
+		return fmt.Errorf("weixin account id %q is not a valid file name", accountID)
+	}
 	path := savedAccountPath(accountID)
 	if path == "" {
 		return fmt.Errorf("reasonix user config dir is unavailable")

internal/bot/weixin/weixin_test.go

@@ -174,3 +174,18 @@ func TestGetUpdatesAcceptsNumericIlinkMessageID(t *testing.T) {
 		t.Fatal("expected queued inbound message")
 	}
 }
+
+func TestSaveAccountRejectsTraversalID(t *testing.T) {
+	isolateWeixinUserConfig(t)
+	for _, id := range []string{"../../evil", `..\evil`, "a/b", ".."} {
+		if err := saveAccount(id, savedAccount{Token: "t"}); err == nil {
+			t.Fatalf("saveAccount(%q) accepted a non-local id", id)
+		}
+		if path := savedAccountPath(id); path != "" {
+			t.Fatalf("savedAccountPath(%q) = %q, want empty", id, path)
+		}
+	}
+	if err := saveAccount("bot-123", savedAccount{Token: "t"}); err != nil {
+		t.Fatalf("saveAccount(valid id): %v", err)
+	}
+}

internal/serve/submit_request.go

@@ -57,5 +57,16 @@ func decodeSubmitRequest(w http.ResponseWriter, r *http.Request) (submitRequest,
 		http.Error(w, "shell commands are unavailable over HTTP", http.StatusForbidden)
 		return submitRequest{}, "", false
 	}
+	if namesMigrationSource(trimmed) {
+		http.Error(w, "a migration source directory is unavailable over HTTP", http.StatusForbidden)
+		return submitRequest{}, "", false
+	}
 	return body, trimmed, true
 }
+
+// namesMigrationSource reports a /migrate that picks its own source: a host
+// directory to read and import, which only a local frontend may choose.
+func namesMigrationSource(input string) bool {
+	fields := strings.Fields(input)
+	return len(fields) > 1 && (fields[0] == "/migrate" || fields[0] == "/migration")
+}

internal/serve/submit_request_migrate_test.go

@@ -0,0 +1,34 @@
+package serve
+
+import (
+	"bytes"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+// An HTTP client may start a migration but not choose the directory it reads:
+// that is a host path only a local frontend names.
+func TestSubmitRefusesAnExplicitMigrationSource(t *testing.T) {
+	for input, allowed := range map[string]bool{
+		`/migrate --from "C:\Users\someone\Old"`: false,
+		"/migration /etc":                        false,
+		"/migrate":                               true,
+		"please /migrate --from x":               true,
+	} {
+		body, err := json.Marshal(map[string]string{"input": input})
+		if err != nil {
+			t.Fatal(err)
+		}
+		w := httptest.NewRecorder()
+		r := httptest.NewRequest(http.MethodPost, "/submit", bytes.NewReader(body))
+		_, _, ok := decodeSubmitRequest(w, r)
+		if ok != allowed {
+			t.Fatalf("%q: accepted=%v, want %v (status %d)", input, ok, allowed, w.Code)
+		}
+		if !allowed && w.Code != http.StatusForbidden {
+			t.Fatalf("%q: status %d, want 403", input, w.Code)
+		}
+	}
+}