From 88bc5d9d3b516b1dddecb3d8b86dedbf7c4fda6a Mon Sep 17 00:00:00 2001 From: Kpa-clawbot Date: Mon, 25 May 2026 22:16:14 -0700 Subject: [PATCH] fix(#1373): drop ghost "unknown" channel bucket from /api/channels for encrypted-no-key packets (#1377) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What Drops the ghost `unknown` channel bucket from `/api/channels` for encrypted GRP_TXT packets whose decoded JSON sets `channel=""` (server has no PSK to decrypt). Fix A from issue #1373 — cosmetic / immediate. Fix B (server-side decryption / key sharing) is intentionally out of scope and remains for a follow-up issue. ## Why When an operator adds a PSK channel key client-side (via the channel customizer), the channel list shows the newly-decrypted channel correctly — but it ALSO shows a stale `unknown` bucket holding the SAME packets the new channel just decrypted. The bucket is a server-side debug catch-all (`if channelName == "" { channelName = "unknown" }`) that leaks into the user-facing channel list. It's not a real channel; dropping it from `/api/channels` is the right fix until/unless server-side decryption lands. Choice made: keep the `channelName = "unknown"` fallback path removed by adding an early `continue` BEFORE the bucket is created. This keeps the diff minimal, preserves the `hasGarbageChars` filter ordering, and makes the intent obvious ("encrypted-no-key packets are not channels"). The DB path (`cmd/server/db.go`) already filters NULL `channel_hash` at the SQL level and `continue`s on empty; the test pins that contract. ## TDD - Red commit: `35b8ba51c74dcc6200d5cf4a87dc7a0b63b2b2c2` — seeds 5 encrypted GRP_TXT (Channel="") + 3 decrypted (#real) into both PacketStore and DB paths; asserts `GetChannels` returns exactly 1 channel (#real). Fails on assertions, not compile. - Green commit: see follow-up commit on this branch — drops the `"unknown"` fallback in `cmd/server/store.go` `GetChannels`; DB path unchanged (already correct, test pins it). ## Manual verification (staging) After deploy, on a staging instance with encrypted GRP_TXT traffic and no PSKs configured: 1. `curl -s https://staging/api/channels | jq '[.[] | select(.name == "unknown")] | length'` → `0` 2. Real channels with known hashes still appear with correct messageCount. ## Files changed - `cmd/server/store.go` — drop the `if channelName == "" { channelName = "unknown" }` fallback; skip the packet instead. - `cmd/server/channels_no_unknown_bucket_1373_test.go` — new test covering both code paths. Fixes #1373 --------- Co-authored-by: openclaw-bot --- .../channels_no_unknown_bucket_1373_test.go | 121 ++++++++++++++++++ cmd/server/store.go | 8 +- 2 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 cmd/server/channels_no_unknown_bucket_1373_test.go diff --git a/cmd/server/channels_no_unknown_bucket_1373_test.go b/cmd/server/channels_no_unknown_bucket_1373_test.go new file mode 100644 index 00000000..3f709f39 --- /dev/null +++ b/cmd/server/channels_no_unknown_bucket_1373_test.go @@ -0,0 +1,121 @@ +package main + +import ( + "database/sql" + "fmt" + "testing" +) + +// Issue #1373: /api/channels emits a ghost "unknown" bucket for encrypted GRP_TXT +// packets whose decoded JSON sets channel="" (server has no PSK to decrypt). +// Fix A (cosmetic): drop the "unknown" bucket from the response so users only +// see real channels. Encrypted-no-key packets are still observable via the +// encrypted-channels analytics, just not as a fake "unknown" channel. +// +// This test seeds 5 GRP_TXT with Channel="" (encrypted-no-key) + 3 with +// Channel="#real" and asserts GetChannels returns exactly one entry, #real — +// no "unknown" bucket. + +func TestGetChannels_NoUnknownBucket_1373(t *testing.T) { + packets := []*StoreTx{ + makeGrpTx(129, "", "", ""), + makeGrpTx(129, "", "", ""), + makeGrpTx(129, "", "", ""), + makeGrpTx(129, "", "", ""), + makeGrpTx(129, "", "", ""), + makeGrpTx(72, "#real", "hello", "alice"), + makeGrpTx(72, "#real", "world", "bob"), + makeGrpTx(72, "#real", "third", "carol"), + } + store := newChannelTestStore(packets) + + channels := store.GetChannels("") + + var gotNames []string + for _, ch := range channels { + name, _ := ch["name"].(string) + gotNames = append(gotNames, name) + if name == "unknown" { + t.Errorf("GetChannels emitted ghost 'unknown' bucket (issue #1373): %+v", ch) + } + } + if len(channels) != 1 { + t.Fatalf("expected exactly 1 channel (#real), got %d: %v", len(channels), gotNames) + } + if name, _ := channels[0]["name"].(string); name != "#real" { + t.Errorf("expected channel name '#real', got %q", name) + } + if mc, _ := channels[0]["messageCount"].(int); mc != 3 { + t.Errorf("expected messageCount=3 for #real, got %v", channels[0]["messageCount"]) + } +} + +// TestGetChannels_DB_NoUnknownBucket_1373 mirrors the in-memory test against +// the DB-backed GetChannels path in cmd/server/db.go. It seeds GRP_TXT rows +// with channel_hash NULL (encrypted, no PSK known to ingestor) + rows with +// channel_hash="#real" and asserts the response contains only #real. +// +// Note: the DB path already filters NULL channel_hash via the SELECT (`channel_hash IS NOT NULL`), +// AND nullStr("")==empty triggers `continue` in the loop. This test pins that +// contract so a future refactor can't reintroduce an "unknown" bucket on the +// DB side either. +func TestGetChannels_DB_NoUnknownBucket_1373(t *testing.T) { + db := setupTestDB(t) + defer db.Close() + + // Seed 5 encrypted GRP_TXT rows with channel_hash NULL (server had no PSK). + for i := 0; i < 5; i++ { + _, err := db.conn.Exec(`INSERT INTO transmissions + (raw_hex, hash, first_seen, route_type, payload_type, decoded_json, channel_hash) + VALUES (?, ?, '2026-05-25T12:00:00Z', 1, 5, + '{"type":"CHAN","channel":"","text":"","sender":""}', NULL)`, + "AA", sqlHashFor(i)) + if err != nil { + t.Fatalf("seed encrypted row %d: %v", i, err) + } + } + + // Seed 3 decrypted GRP_TXT rows with channel_hash="#real". + for i := 0; i < 3; i++ { + _, err := db.conn.Exec(`INSERT INTO transmissions + (raw_hex, hash, first_seen, route_type, payload_type, decoded_json, channel_hash) + VALUES (?, ?, '2026-05-25T12:00:00Z', 1, 5, + '{"type":"CHAN","channel":"#real","text":"Alice: hi","sender":"Alice"}', '#real')`, + "BB", sqlHashFor(100+i)) + if err != nil { + t.Fatalf("seed real row %d: %v", i, err) + } + } + + channels, err := db.GetChannels() + if err != nil { + t.Fatalf("GetChannels: %v", err) + } + + var gotNames []string + for _, ch := range channels { + name, _ := ch["name"].(string) + gotNames = append(gotNames, name) + if name == "unknown" { + t.Errorf("DB GetChannels emitted ghost 'unknown' bucket (issue #1373): %+v", ch) + } + if name == "" { + t.Errorf("DB GetChannels emitted empty-name channel bucket (issue #1373): %+v", ch) + } + } + if len(channels) != 1 { + t.Fatalf("expected exactly 1 channel (#real), got %d: %v", len(channels), gotNames) + } + if name, _ := channels[0]["name"].(string); name != "#real" { + t.Errorf("expected channel name '#real', got %q", name) + } +} + +// sqlHashFor returns a unique 16-char hex string per index for the +// `hash` UNIQUE column in transmissions. +func sqlHashFor(i int) string { + return fmt.Sprintf("%016x", uint64(0x1373_0000_0000_0000)+uint64(i)) +} + +// silence unused-import warning when the file is reduced. +var _ = sql.ErrNoRows diff --git a/cmd/server/store.go b/cmd/server/store.go index bab6c599..19cd57ff 100644 --- a/cmd/server/store.go +++ b/cmd/server/store.go @@ -4508,7 +4508,13 @@ func (s *PacketStore) GetChannels(region string) []map[string]interface{} { } channelName := decoded.Channel if channelName == "" { - channelName = "unknown" + // Issue #1373: encrypted-no-key packets decode with channel="". + // Previously we bucketed them under a literal "unknown" channel + // which then leaked into /api/channels as a ghost entry next to + // real channels (especially visible after the operator added a + // PSK client-side). Skip them — they belong in encrypted-channels + // analytics, not the user-facing channel list. + continue } ch := channelMap[channelName] if ch == nil {