diff --git a/cmd/ingestor/channel_proposals_test.go b/cmd/ingestor/channel_proposals_test.go index c58bb8335..7e3f5e775 100644 --- a/cmd/ingestor/channel_proposals_test.go +++ b/cmd/ingestor/channel_proposals_test.go @@ -102,6 +102,34 @@ func TestChannelProposalSubmitDuplicatesAndCase(t *testing.T) { } } +// #251: letter case is part of a hashtag channel's identity. The key is the +// first 16 bytes of sha256 of the exact name (firmware docs/companion_protocol.md, +// "Hashtag Channels": sha256("#test")), so a different case derives a different +// key. That is why #HelloWorld and #helloworld stay separate proposals instead +// of being normalised into one. +func TestHashtagKeyDerivationIsCaseSensitive(t *testing.T) { + if got := deriveHashtagChannelKey("#test"); got != "9cd8fcf22a47333b591d96a2b848b73f" { + t.Fatalf("#test key = %s, want the firmware-documented 9cd8fcf22a47333b591d96a2b848b73f", got) + } + if deriveHashtagChannelKey("#HelloWorld") == deriveHashtagChannelKey("#helloworld") { + t.Fatal("different case must derive a different key") + } + + f := newProposalFixture(t, nil) + a := f.run(f.enqueue(channelregistry.OpSubmit, "#HelloWorld")) + b := f.run(f.enqueue(channelregistry.OpSubmit, "#helloworld")) + if a.Proposal == nil || b.Proposal == nil || a.Proposal.ID == b.Proposal.ID || + a.Proposal.Name != "#HelloWorld" || b.Proposal.Name != "#helloworld" { + t.Fatalf("case variants must be two proposals with their names unchanged: %+v %+v", a, b) + } + f.run(f.enqueue(channelregistry.OpApprove, a.Proposal.ID)) + f.run(f.enqueue(channelregistry.OpApprove, b.Proposal.ID)) + keys := f.keys.Channels() + if keys["#HelloWorld"] == "" || keys["#helloworld"] == "" || keys["#HelloWorld"] == keys["#helloworld"] { + t.Fatalf("both case variants need their own key: %q %q", keys["#HelloWorld"], keys["#helloworld"]) + } +} + func TestChannelProposalLimits(t *testing.T) { f := newProposalFixture(t, &channelregistry.Config{MaxPending: 2, MaxApproved: 1}) a := f.run(f.enqueue(channelregistry.OpSubmit, "#a")).Proposal.ID diff --git a/cmd/server/channel_proposals.go b/cmd/server/channel_proposals.go index bd4bf7524..07d90c918 100644 --- a/cmd/server/channel_proposals.go +++ b/cmd/server/channel_proposals.go @@ -24,6 +24,8 @@ import ( "math" "net/http" "os" + "sort" + "strings" "sync" "time" @@ -62,6 +64,12 @@ type AdminChannelProposal struct { // channelKeys), so approving it adds nothing and revoking it does not // stop decryption. BuiltIn bool `json:"builtIn,omitempty"` + // NearDuplicateOf lists the other proposal names and built-in names that + // differ from this one only by letter case (#251). Hashtag keys are + // derived from the exact bytes of the name, so "#HelloWorld" and + // "#helloworld" are different channels (different keys) and stay separate + // proposals; this is a hint for the administrator, not a merge. + NearDuplicateOf []string `json:"nearDuplicateOf,omitempty"` } // ChannelProposalRequestResponse is GET /api/channel-proposals/requests/{id}. @@ -80,8 +88,13 @@ type ChannelProposalListResponse struct { type approvedSnapshot struct { channels []ApprovedChannel - builtAt time.Time // when the list was last read successfully - expires time.Time + // hidden holds the names GET /api/channels leaves out (#251): channels + // with stored messages whose proposal is not approved (revoked, rejected + // or suggested again) and that the ingestor does not decrypt through its + // config-derived keys. Built once per refresh and never mutated after. + hidden map[string]bool + builtAt time.Time // when the list was last read successfully + expires time.Time } // builtinNamesCache is the last parsed builtin names file, keyed by its @@ -90,6 +103,7 @@ type builtinNamesCache struct { mod time.Time size int64 names map[string]bool + ok bool // the file parsed; false means the set is unknown } type channelProposalService struct { @@ -179,10 +193,11 @@ func (p *channelProposalService) refundSubmission() { } } -// approvedChannels returns the approved channels for GET /api/channels from a -// short-lived cache. It returns a fresh slice the caller may keep; the cached -// one is never handed out or mutated. -func (p *channelProposalService) approvedChannels(ctx context.Context) []ApprovedChannel { +// snapshot returns the current approved/hidden snapshot, refreshing it when +// the TTL ran out. The snapshot is immutable once built: callers may read it +// after the lock is released but must never write to it. nil means no +// database or no snapshot could be read yet. +func (p *channelProposalService) snapshot(ctx context.Context) *approvedSnapshot { if p == nil || p.db == nil { return nil } @@ -190,14 +205,19 @@ func (p *channelProposalService) approvedChannels(ctx context.Context) []Approve defer p.approvedMu.Unlock() now := p.now() if p.approved == nil || now.After(p.approved.expires) { - names, err := channelregistry.ListApprovedNames(ctx, p.db, channelregistry.MaxListed) + names, notApproved, err := p.readProposalNames(ctx) switch { case err == nil: chans := make([]ApprovedChannel, len(names)) for i, n := range names { chans[i] = ApprovedChannel{Name: n, Hash: n} } - p.approved = &approvedSnapshot{channels: chans, builtAt: now, expires: now.Add(approvedCacheTTL)} + p.approved = &approvedSnapshot{ + channels: chans, + hidden: p.hiddenNames(notApproved), + builtAt: now, + expires: now.Add(approvedCacheTTL), + } case p.approved == nil: log.Printf("[channel-proposals] listing approved channels failed: %v", err) return nil @@ -207,11 +227,94 @@ func (p *channelProposalService) approvedChannels(ctx context.Context) []Approve p.approved.expires = now.Add(approvedCacheTTL) } } - if len(p.approved.channels) == 0 { + return p.approved +} + +// readProposalNames reads the approved names and the not-approved channels +// that have stored traffic (two read-only queries; one refresh per +// approvedCacheTTL at most). +func (p *channelProposalService) readProposalNames(ctx context.Context) (approved, notApproved []string, err error) { + if approved, err = channelregistry.ListApprovedNames(ctx, p.db, channelregistry.MaxListed); err != nil { + return nil, nil, err + } + if notApproved, err = channelregistry.ListNotApprovedChannelsWithTraffic(ctx, p.db); err != nil { + return nil, nil, err + } + return approved, notApproved, nil +} + +// hiddenNames returns the channels GET /api/channels must leave out, from the +// names of not-approved proposals that have stored traffic. Only a channel +// that was decrypted at some point has stored traffic, so such a name was +// approved or config-decrypted before; revoking, rejecting a re-suggestion or +// a re-suggestion that is still pending all keep it hidden, and only an +// approval lists it again. Exceptions: +// - a name the ingestor decrypts through its config-derived keys (built-in, +// rainbow table, hashChannels, channelKeys) is never hidden, whatever its +// proposal says, because that traffic keeps being decrypted; +// - when the ingestor's names file is missing or unreadable nothing is +// hidden: a channel is only hidden once it is known not to be one of +// those. +func (p *channelProposalService) hiddenNames(notApproved []string) map[string]bool { + if len(notApproved) == 0 { return nil } - out := make([]ApprovedChannel, len(p.approved.channels)) - copy(out, p.approved.channels) + builtin, ok := p.loadBuiltinNames() + if !ok { + return nil + } + hidden := make(map[string]bool, len(notApproved)) + for _, n := range notApproved { + if !builtin[n] { + hidden[n] = true + } + } + if len(hidden) == 0 { + return nil + } + return hidden +} + +// hideRevoked drops the channels whose proposal is not approved from resp +// (#251) and names them in resp.HiddenChannels so the page can keep live +// updates from re-creating their rows. +// The messages stay in the database and readable per channel; only the list +// leaves them out, and the list returns when the proposal is approved again. +// With nothing revoked resp is left as is; otherwise resp.Channels becomes a +// filtered copy, because the slice it came in with is a cache's and must not +// be modified. O(len). +func (p *channelProposalService) hideRevoked(ctx context.Context, resp *ChannelListResponse) { + snap := p.snapshot(ctx) + if snap == nil || len(snap.hidden) == 0 { + return + } + out := append(resp.Channels[:0:0], resp.Channels...) // copy, then compact in place + kept := 0 + for _, ch := range out { + if name, _ := ch["name"].(string); snap.hidden[name] { + continue + } + out[kept] = ch + kept++ + } + resp.Channels = out[:kept] + resp.HiddenChannels = make([]string, 0, len(snap.hidden)) + for name := range snap.hidden { + resp.HiddenChannels = append(resp.HiddenChannels, name) + } + sort.Strings(resp.HiddenChannels) +} + +// approvedChannels returns the approved channels for GET /api/channels from a +// short-lived cache. It returns a fresh slice the caller may keep; the cached +// one is never handed out or mutated. +func (p *channelProposalService) approvedChannels(ctx context.Context) []ApprovedChannel { + snap := p.snapshot(ctx) + if snap == nil || len(snap.channels) == 0 { + return nil + } + out := make([]ApprovedChannel, len(snap.channels)) + copy(out, snap.channels) return out } @@ -239,28 +342,35 @@ func (p *channelProposalService) noteDecision(st channelregistry.RequestStatus) // changed, and a bad version is logged once, not on every poll. A missing or // unreadable file means none are known. func (p *channelProposalService) builtinNames() map[string]bool { + names, _ := p.loadBuiltinNames() + return names +} + +// loadBuiltinNames is builtinNames plus whether the set is known: ok is +// false for a missing or unreadable file, where "no names" would be a guess. +func (p *channelProposalService) loadBuiltinNames() (names map[string]bool, ok bool) { if p == nil || p.queue == nil { - return nil + return nil, false } info, err := os.Stat(p.queue.BuiltinNamesPath()) if err != nil { if !os.IsNotExist(err) { log.Printf("[channel-proposals] reading built-in channel names failed: %v", err) } - return nil + return nil, false } p.builtinMu.Lock() defer p.builtinMu.Unlock() if c := p.builtin; c != nil && c.mod.Equal(info.ModTime()) && c.size == info.Size() { - return c.names + return c.names, c.ok } - names, err := p.queue.ReadBuiltinNames() + names, err = p.queue.ReadBuiltinNames() if err != nil { log.Printf("[channel-proposals] reading built-in channel names failed: %v", err) names = nil } - p.builtin = &builtinNamesCache{mod: info.ModTime(), size: info.Size(), names: names} - return names + p.builtin = &builtinNamesCache{mod: info.ModTime(), size: info.Size(), names: names, ok: err == nil} + return names, err == nil } func noStore(w http.ResponseWriter) { @@ -397,14 +507,67 @@ func (s *Server) handleAdminChannelProposals(w http.ResponseWriter, r *http.Requ writeError(w, http.StatusInternalServerError, "could not list suggestions") return } + // The near-duplicate hint must see a proposal in another status than + // the one being viewed, so it reads every name separately; the rows + // above keep the status filter in SQL (before the limit). + names, err := channelregistry.ListAllNames(r.Context(), p.db, channelregistry.MaxListed) + if err != nil { + log.Printf("[channel-proposals] admin name list failed: %v", err) + writeError(w, http.StatusInternalServerError, "could not list suggestions") + return + } builtin := p.builtinNames() + byFold := caseFoldIndex(names, builtin) for _, pr := range list { - resp.Proposals = append(resp.Proposals, AdminChannelProposal{Proposal: pr, BuiltIn: builtin[pr.Name]}) + resp.Proposals = append(resp.Proposals, AdminChannelProposal{ + Proposal: pr, + BuiltIn: builtin[pr.Name], + NearDuplicateOf: nearDuplicates(byFold, pr.Name), + }) } } writeJSON(w, resp) } +// caseFoldIndex groups every proposal name and built-in name by its lower-case +// form, each distinct name once. +func caseFoldIndex(names []string, builtin map[string]bool) map[string][]string { + seen := make(map[string]bool, len(names)+len(builtin)) + idx := make(map[string][]string, len(names)+len(builtin)) + add := func(name string) { + if seen[name] { + return + } + seen[name] = true + k := strings.ToLower(name) + idx[k] = append(idx[k], name) + } + for _, n := range names { + add(n) + } + for n := range builtin { + add(n) + } + return idx +} + +// nearDuplicates returns the other names in idx that differ from name only by +// letter case, sorted; nil when there are none. +func nearDuplicates(idx map[string][]string, name string) []string { + group := idx[strings.ToLower(name)] + if len(group) < 2 { + return nil + } + out := make([]string, 0, len(group)-1) + for _, n := range group { + if n != name { + out = append(out, n) + } + } + sort.Strings(out) + return out +} + // loadAdminProposal validates id and loads the proposal, writing the // 400/404/500 response itself and returning ok=false when the caller must // not proceed. Shared by the decision handler below and the revoke handler. diff --git a/cmd/server/channel_revoked_hidden_test.go b/cmd/server/channel_revoked_hidden_test.go new file mode 100644 index 000000000..36407622f --- /dev/null +++ b/cmd/server/channel_revoked_hidden_test.go @@ -0,0 +1,528 @@ +package main + +import ( + "context" + "database/sql" + "fmt" + "os" + "path/filepath" + "reflect" + "sort" + "strings" + "testing" + "time" + + "github.com/meshcore-analyzer/channelregistry" +) + +// #251: an administrator who revokes a shared channel expects it to leave the +// channel list. The decoded messages stay in the database, so the list query +// still returns the channel; GET /api/channels leaves out names whose proposal +// is revoked, unless the ingestor decrypts that name through its config-derived +// keys (built-in, rainbow table, hashChannels, channelKeys). + +// addChannelTraffic stores one decoded message on a channel, the way the +// ingestor does for a decrypted GRP_TXT. +func (ps *proposalServer) addChannelTraffic(name string) { + ps.t.Helper() + hash := fmt.Sprintf("%x", name) // unique per name + if _, err := ps.srv.db.conn.Exec(`INSERT INTO transmissions (raw_hex, hash, first_seen, route_type, payload_type, decoded_json, channel_hash) + VALUES ('EEFF', ?, '2024-01-01T00:00:00Z', 1, 5, ?, ?)`, + hash, `{"type":"CHAN","channel":"`+name+`","text":"alice: hello","sender":"alice"}`, name); err != nil { + ps.t.Fatal(err) + } + ps.srv.db.channelsCache.reset() +} + +// expireApproved moves the clock past the approved-snapshot TTL, as the +// passing of time does between two refreshes of the channel list. +func (ps *proposalServer) expireApproved() { ps.clock = ps.clock.Add(approvedCacheTTL + time.Second) } + +func (ps *proposalServer) setStatus(id, status string) { + ps.t.Helper() + if _, err := ps.srv.db.conn.Exec(`UPDATE channel_proposals SET status = ?, reviewed_at = ? WHERE id = ?`, + status, ps.clock.UnixMilli(), id); err != nil { + ps.t.Fatal(err) + } +} + +func (ps *proposalServer) listedNames(query string) []string { + ps.t.Helper() + w := ps.do("GET", "/api/channels"+query, "", "") + if w.Code != 200 { + ps.t.Fatalf("GET /api/channels%s = %d: %s", query, w.Code, w.Body) + } + var names []string + for _, ch := range decode[ChannelListResponse](ps.t, w).Channels { + names = append(names, fmt.Sprint(ch["name"])) + } + sort.Strings(names) + return names +} + +func containsName(list []string, s string) bool { + for _, v := range list { + if v == s { + return true + } + } + return false +} + +func newRevokeFixture(t *testing.T) *proposalServer { + t.Helper() + ps := newProposalServer(t, strongTestKey, enabledConfig(nil), true) + // The ingestor has published its config-derived names (it does so before + // it applies any command). + if err := ps.queue.WriteBuiltinNames([]string{"#chat"}); err != nil { + t.Fatal(err) + } + return ps +} + +// Acceptance 1: after a revoke the channel is gone from the list although +// historical messages exist; the messages themselves stay readable. +func TestRevokedChannelIsHiddenFromList(t *testing.T) { + ps := newRevokeFixture(t) + ps.addChannelTraffic("#helloworld") + ps.insert("aaaaaaaaaaaaaaaa", "#helloworld", channelregistry.StatusApproved) + ps.expireApproved() + if got := ps.listedNames(""); !containsName(got, "#helloworld") { + t.Fatalf("precondition: an approved channel with traffic is listed: %v", got) + } + + ps.setStatus("aaaaaaaaaaaaaaaa", channelregistry.StatusRevoked) + ps.expireApproved() + got := ps.listedNames("") + if containsName(got, "#helloworld") { + t.Fatalf("revoked channel still listed: %v", got) + } + if !containsName(got, "#test") { + t.Fatalf("unrelated channels must stay listed: %v", got) + } + // History is kept: the conversation can still be read directly. + w := ps.do("GET", "/api/channels/%23helloworld/messages", "", "") + if w.Code != 200 || decode[ChannelMessagesResponse](t, w).Total != 1 { + t.Fatalf("history of a revoked channel must stay readable: %d %s", w.Code, w.Body) + } + // A revoked channel is not in approvedChannels either. + if resp := decode[ChannelListResponse](t, ps.do("GET", "/api/channels", "", "")); len(resp.ApprovedChannels) != 0 { + t.Fatalf("approvedChannels = %+v", resp.ApprovedChannels) + } +} + +// The revoke result the admin's poller reads invalidates the snapshot early, +// so the list changes within one refresh without waiting for the TTL. +func TestRevokeHidesChannelWithoutWaitingForTTL(t *testing.T) { + ps := newRevokeFixture(t) + ps.addChannelTraffic("#helloworld") + ps.insert("aaaaaaaaaaaaaaaa", "#helloworld", channelregistry.StatusApproved) + ps.expireApproved() + if got := ps.listedNames(""); !containsName(got, "#helloworld") { + t.Fatalf("precondition: %v", got) + } + + ps.setStatus("aaaaaaaaaaaaaaaa", channelregistry.StatusRevoked) + reqID := channelregistry.NewID() + p := &channelregistry.Proposal{ID: "aaaaaaaaaaaaaaaa", Name: "#helloworld", Status: channelregistry.StatusRevoked} + if err := ps.queue.Complete(channelregistry.Result{RequestID: reqID, Status: channelregistry.RequestRevoked, Proposal: p, CompletedAt: ps.clock.Add(time.Millisecond).UnixMilli()}); err != nil { + t.Fatal(err) + } + if w := ps.do("GET", "/api/channel-proposals/requests/"+reqID, "", ""); w.Code != 200 { + t.Fatalf("request status = %d", w.Code) + } + if got := ps.listedNames(""); containsName(got, "#helloworld") { + t.Fatalf("revoked channel still listed inside the TTL: %v", got) + } +} + +// Acceptance 2a: re-approval shows the channel again, with its history. +func TestReapprovedChannelIsListedAgain(t *testing.T) { + ps := newRevokeFixture(t) + ps.addChannelTraffic("#helloworld") + ps.insert("aaaaaaaaaaaaaaaa", "#helloworld", channelregistry.StatusRevoked) + ps.expireApproved() + if got := ps.listedNames(""); containsName(got, "#helloworld") { + t.Fatalf("precondition: revoked channel hidden: %v", got) + } + + // The ingestor revives the same row: revoked -> pending (suggested + // again) -> approved. + ps.setStatus("aaaaaaaaaaaaaaaa", channelregistry.StatusPending) + ps.expireApproved() + ps.setStatus("aaaaaaaaaaaaaaaa", channelregistry.StatusApproved) + ps.expireApproved() + var found map[string]interface{} + for _, ch := range decode[ChannelListResponse](t, ps.do("GET", "/api/channels", "", "")).Channels { + if ch["name"] == "#helloworld" { + found = ch + } + } + if found == nil { + t.Fatal("re-approved channel is not listed") + } + if found["messageCount"] != float64(1) { + t.Fatalf("history must come back with the channel: %v", found) + } +} + +// Acceptance 2b: a channel the ingestor decrypts through its config-derived +// keys is never hidden by a revoked proposal for the same name. +func TestBuiltinChannelIsNeverHiddenByRevokedProposal(t *testing.T) { + ps := newRevokeFixture(t) + if err := ps.queue.WriteBuiltinNames([]string{"#test", "#chat"}); err != nil { + t.Fatal(err) + } + // "#test" has seeded traffic and is decrypted by the built-in layer. + ps.insert("aaaaaaaaaaaaaaaa", "#test", channelregistry.StatusRevoked) + ps.expireApproved() + if got := ps.listedNames(""); !containsName(got, "#test") { + t.Fatalf("built-in channel hidden by a revoked proposal: %v", got) + } +} + +// Hiding needs a positive "not config-decrypted": without the ingestor's +// names file (missing or unreadable) nothing is hidden. +func TestRevokedChannelIsNotHiddenWithoutBuiltinNames(t *testing.T) { + ps := newProposalServer(t, strongTestKey, enabledConfig(nil), true) // no names file written + ps.insert("aaaaaaaaaaaaaaaa", "#test", channelregistry.StatusRevoked) + ps.expireApproved() + if got := ps.listedNames(""); !containsName(got, "#test") { + t.Fatalf("fail-open expected without the built-in names file: %v", got) + } + + if err := os.MkdirAll(filepath.Dir(ps.queue.BuiltinNamesPath()), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(ps.queue.BuiltinNamesPath(), []byte("{not json"), 0o644); err != nil { + t.Fatal(err) + } + ps.expireApproved() + if got := ps.listedNames(""); !containsName(got, "#test") { + t.Fatalf("fail-open expected with an unreadable built-in names file: %v", got) + } +} + +// Only the exact name is hidden: a different-case channel is a different +// channel (different key) and stays listed. +func TestRevokedChannelHidesOnlyTheExactName(t *testing.T) { + ps := newRevokeFixture(t) + ps.addChannelTraffic("#HelloWorld") + ps.addChannelTraffic("#helloworld") + ps.insert("aaaaaaaaaaaaaaaa", "#HelloWorld", channelregistry.StatusRevoked) + ps.insert("bbbbbbbbbbbbbbbb", "#helloworld", channelregistry.StatusApproved) + ps.expireApproved() + got := ps.listedNames("") + if containsName(got, "#HelloWorld") || !containsName(got, "#helloworld") { + t.Fatalf("list = %v: want #helloworld listed and #HelloWorld hidden", got) + } +} + +// The filter works on a copy: the DB's cached slice keeps the hidden channel +// (it is only hidden from this response), and includeEncrypted still appends. +func TestRevokedFilterDoesNotMutateCacheAndKeepsEncrypted(t *testing.T) { + ps := newRevokeFixture(t) + ps.addChannelTraffic("#helloworld") + if _, err := ps.srv.db.conn.Exec(`INSERT INTO transmissions (raw_hex, hash, first_seen, route_type, payload_type, decoded_json, channel_hash) + VALUES ('EEFF', 'enc0000000000001', '2024-01-01T00:00:00Z', 1, 5, '{}', 'enc_AB')`); err != nil { + t.Fatal(err) + } + ps.insert("aaaaaaaaaaaaaaaa", "#helloworld", channelregistry.StatusRevoked) + ps.expireApproved() + for i := 0; i < 3; i++ { + got := ps.listedNames("?includeEncrypted=true") + if containsName(got, "#helloworld") || !containsName(got, "Encrypted (0xAB)") { + t.Fatalf("list = %v", got) + } + } + cached, err := ps.srv.db.GetChannels("") + if err != nil { + t.Fatal(err) + } + var seen bool + for _, ch := range cached { + if ch["name"] == "#helloworld" { + seen = true + } + } + if !seen { + t.Fatal("the cached channel list was altered by the filter") + } +} + +// Performance: the revoked set rides on the approved-names snapshot, so a +// list request does not query the proposals table again inside the TTL. +func TestRevokedSetIsServedFromTheSnapshot(t *testing.T) { + ps := newRevokeFixture(t) + ps.addChannelTraffic("#helloworld") + ps.insert("aaaaaaaaaaaaaaaa", "#helloworld", channelregistry.StatusRevoked) + ps.expireApproved() + if got := ps.listedNames(""); containsName(got, "#helloworld") { + t.Fatalf("precondition: %v", got) + } + // Dropping the table would fail any further query; the snapshot answers. + if _, err := ps.srv.db.conn.Exec(`DROP TABLE channel_proposals`); err != nil { + t.Fatal(err) + } + for i := 0; i < 5; i++ { + if got := ps.listedNames(""); containsName(got, "#helloworld") { + t.Fatalf("request %d queried the proposals table instead of the snapshot: %v", i, got) + } + } +} + +// With nothing to hide the filter must be a no-op that returns the cached +// slice untouched (no copy per request), and report no hidden names. +func TestHideRevokedIsNoOpWithoutHiddenNames(t *testing.T) { + ps := newRevokeFixture(t) + in := []map[string]interface{}{{"name": "#a"}, {"name": "#b"}} + resp := ChannelListResponse{Channels: in} + ps.svc.hideRevoked(context.Background(), &resp) + if !reflect.DeepEqual(in, resp.Channels) || &in[0] != &resp.Channels[0] { + t.Fatalf("expected the same slice back, got %v", resp.Channels) + } + if resp.HiddenChannels != nil { + t.Fatalf("hiddenChannels must be omitted when nothing is hidden: %v", resp.HiddenChannels) + } +} + +// A rolling upgrade (table not created yet) must not hide or break anything. +func TestRevokedFilterMissingTable(t *testing.T) { + ps := newProposalServer(t, strongTestKey, enabledConfig(nil), false) + if got := ps.listedNames(""); !containsName(got, "#test") { + t.Fatalf("list = %v", got) + } +} + +// The hidden decision is read per channel that has stored traffic, not from a +// capped list of proposal rows: an old revoked proposal must stay hidden no +// matter how many newer ones exist (review of #257, finding 3). +func TestRevokedChannelStaysHiddenBehindManyNewerRevokedRows(t *testing.T) { + ps := newRevokeFixture(t) + ps.addChannelTraffic("#oldrevoked") + if _, err := ps.srv.db.conn.Exec(`INSERT INTO channel_proposals (id, name, status, created_at, reviewed_at) VALUES ('0000000000000001', '#oldrevoked', 'revoked', 1, 1)`); err != nil { + t.Fatal(err) + } + // More newer revoked rows than any read cap (MaxListed = 1024). + tx, err := ps.srv.db.conn.Begin() + if err != nil { + t.Fatal(err) + } + for i := 0; i < channelregistry.MaxListed+1; i++ { + name := fmt.Sprintf("#newer%d", i) + if _, err := tx.Exec(`INSERT INTO channel_proposals (id, name, status, created_at, reviewed_at) VALUES (?, ?, 'revoked', ?, ?)`, + fmt.Sprintf("%016x", i+2), name, 1000+i, 1000+i); err != nil { + t.Fatal(err) + } + // Each newer revoked channel has stored traffic too, as a real + // revoked channel does. + if _, err := tx.Exec(`INSERT INTO transmissions (raw_hex, hash, first_seen, route_type, payload_type, decoded_json, channel_hash) + VALUES ('EEFF', ?, '2024-01-01T00:00:00Z', 1, 5, ?, ?)`, + fmt.Sprintf("newer%016x", i), `{"type":"CHAN","channel":"`+name+`","text":"a: b","sender":"a"}`, name); err != nil { + t.Fatal(err) + } + } + if err := tx.Commit(); err != nil { + t.Fatal(err) + } + ps.expireApproved() + if got := ps.listedNames(""); containsName(got, "#oldrevoked") { + t.Fatalf("an old revoked channel reappeared behind %d newer revoked rows: %v", channelregistry.MaxListed+1, got) + } +} + +// Review of #257, finding 2: a revoked name that is suggested again (pending) +// or rejected must stay hidden. Only an approval, or the ingestor decrypting +// the name through its built-in/config list, lists the channel. +func TestPendingAndRejectedProposalsKeepAChannelHidden(t *testing.T) { + ps := newRevokeFixture(t) + ps.addChannelTraffic("#helloworld") + ps.insert("aaaaaaaaaaaaaaaa", "#helloworld", channelregistry.StatusRevoked) + steps := []struct { + status string + listed bool + comment string + }{ + {channelregistry.StatusRevoked, false, "revoked"}, + {channelregistry.StatusPending, false, "suggested again by a visitor"}, + {channelregistry.StatusRejected, false, "re-suggestion rejected by the administrator"}, + {channelregistry.StatusApproved, true, "approved again"}, + {channelregistry.StatusRevoked, false, "revoked again"}, + } + for _, st := range steps { + ps.setStatus("aaaaaaaaaaaaaaaa", st.status) + ps.expireApproved() + if got := containsName(ps.listedNames(""), "#helloworld"); got != st.listed { + t.Fatalf("%s: listed = %v, want %v", st.comment, got, st.listed) + } + } + // A pending or rejected name the ingestor decrypts through its config is + // still listed. + if err := ps.queue.WriteBuiltinNames([]string{"#test", "#chat"}); err != nil { + t.Fatal(err) + } + ps.insert("bbbbbbbbbbbbbbbb", "#test", channelregistry.StatusRejected) + ps.expireApproved() + if got := ps.listedNames(""); !containsName(got, "#test") { + t.Fatalf("a built-in channel hidden by a rejected proposal: %v", got) + } +} + +// Review of #257, finding 2 (frontend half): the response names the hidden +// channels, so the page can keep live updates from re-creating their rows. +func TestChannelListNamesTheHiddenChannels(t *testing.T) { + ps := newRevokeFixture(t) + ps.addChannelTraffic("#helloworld") + ps.insert("aaaaaaaaaaaaaaaa", "#helloworld", channelregistry.StatusRevoked) + ps.expireApproved() + resp := decode[ChannelListResponse](t, ps.do("GET", "/api/channels", "", "")) + if !reflect.DeepEqual(resp.HiddenChannels, []string{"#helloworld"}) { + t.Fatalf("hiddenChannels = %v", resp.HiddenChannels) + } + ps.setStatus("aaaaaaaaaaaaaaaa", channelregistry.StatusApproved) + ps.expireApproved() + w := ps.do("GET", "/api/channels", "", "") + if strings.Contains(w.Body.String(), "hiddenChannels") { + t.Fatalf("hiddenChannels must be omitted when nothing is hidden: %s", w.Body) + } +} + +// Review of #257, finding 1: the admin list applies the status filter in SQL +// before the row limit. One old approved proposal must stay reachable (and +// removable) behind more newer rejected ones than the limit. +func TestAdminStatusFilterIsAppliedBeforeTheLimit(t *testing.T) { + ps := newRevokeFixture(t) + limit := ps.svc.limits.MaxPending + ps.svc.limits.MaxApproved + ps.svc.limits.MaxQueuedRequests + if _, err := ps.srv.db.conn.Exec(`INSERT INTO channel_proposals (id, name, status, created_at, reviewed_at) VALUES ('0000000000000001', '#oldapproved', 'approved', 1, 1)`); err != nil { + t.Fatal(err) + } + tx, err := ps.srv.db.conn.Begin() + if err != nil { + t.Fatal(err) + } + for i := 0; i < limit; i++ { + if _, err := tx.Exec(`INSERT INTO channel_proposals (id, name, status, created_at, reviewed_at) VALUES (?, ?, 'rejected', ?, ?)`, + fmt.Sprintf("%016x", i+2), fmt.Sprintf("#spam%d", i), 1000+i, 1000+i); err != nil { + t.Fatal(err) + } + } + if err := tx.Commit(); err != nil { + t.Fatal(err) + } + for status, want := range map[string]int{"approved": 1, "rejected": limit} { + w := ps.do("GET", "/api/admin/channel-proposals?status="+status, "", strongTestKey) + if got := len(decode[ChannelProposalListResponse](t, w).Proposals); got != want { + t.Fatalf("?status=%s returned %d proposals, want %d", status, got, want) + } + } +} + +// The near-duplicate hint is read from a separate name-only list, so it still +// sees a case variant that is older than the row limit. +func TestNearDuplicateHintSeesProposalsBeyondTheRowLimit(t *testing.T) { + ps := newRevokeFixture(t) + limit := ps.svc.limits.MaxPending + ps.svc.limits.MaxApproved + ps.svc.limits.MaxQueuedRequests + if _, err := ps.srv.db.conn.Exec(`INSERT INTO channel_proposals (id, name, status, created_at, reviewed_at) VALUES ('0000000000000001', '#HelloWorld', 'rejected', 1, 1)`); err != nil { + t.Fatal(err) + } + tx, err := ps.srv.db.conn.Begin() + if err != nil { + t.Fatal(err) + } + for i := 0; i < limit; i++ { + if _, err := tx.Exec(`INSERT INTO channel_proposals (id, name, status, created_at, reviewed_at) VALUES (?, ?, 'pending', ?, NULL)`, + fmt.Sprintf("%016x", i+2), fmt.Sprintf("#p%d", i), 1000+i); err != nil { + t.Fatal(err) + } + } + if _, err := tx.Exec(`INSERT INTO channel_proposals (id, name, status, created_at, reviewed_at) VALUES ('ffffffffffffffff', '#helloworld', 'pending', 999999, NULL)`); err != nil { + t.Fatal(err) + } + if err := tx.Commit(); err != nil { + t.Fatal(err) + } + list := decode[ChannelProposalListResponse](t, ps.do("GET", "/api/admin/channel-proposals?status=pending", "", strongTestKey)) + for _, p := range list.Proposals { + if p.Name == "#helloworld" { + if !reflect.DeepEqual(p.NearDuplicateOf, []string{"#HelloWorld"}) { + t.Fatalf("nearDuplicateOf = %v", p.NearDuplicateOf) + } + return + } + } + t.Fatal("#helloworld not in the pending list") +} + +// Case handling of proposal names (#251), decided from the key derivation: +// the hashtag key is the first 16 bytes of sha256 of the exact name (firmware +// docs/companion_protocol.md; companion app derivePskFromHashtag), so +// #HelloWorld and #helloworld are different channels. They stay separate +// proposals; the admin list points out the near-duplicate instead. +func TestAdminListFlagsCaseNearDuplicates(t *testing.T) { + ps := newRevokeFixture(t) + if err := ps.queue.WriteBuiltinNames([]string{"#test", "#chat"}); err != nil { + t.Fatal(err) + } + ps.insert("aaaaaaaaaaaaaaaa", "#HelloWorld", channelregistry.StatusApproved) + ps.insert("bbbbbbbbbbbbbbbb", "#helloworld", channelregistry.StatusApproved) + ps.insert("cccccccccccccccc", "#Test", channelregistry.StatusPending) // near a built-in name + ps.insert("dddddddddddddddd", "#unique", channelregistry.StatusPending) + ps.insert("eeeeeeeeeeeeeeee", "#HELLOWORLD", channelregistry.StatusRejected) + + near := func(status string) map[string][]string { + q := "" + if status != "" { + q = "?status=" + status + } + w := ps.do("GET", "/api/admin/channel-proposals"+q, "", strongTestKey) + out := map[string][]string{} + for _, p := range decode[ChannelProposalListResponse](t, w).Proposals { + out[p.Name] = p.NearDuplicateOf + } + return out + } + all := near("") + want := map[string][]string{ + "#HelloWorld": {"#HELLOWORLD", "#helloworld"}, + "#helloworld": {"#HELLOWORLD", "#HelloWorld"}, + "#HELLOWORLD": {"#HelloWorld", "#helloworld"}, + "#Test": {"#test"}, + "#unique": nil, + } + if !reflect.DeepEqual(all, want) { + t.Fatalf("nearDuplicateOf = %v\nwant %v", all, want) + } + // A status filter must not hide the other-status duplicate. + if got := near("pending")["#Test"]; !reflect.DeepEqual(got, []string{"#test"}) { + t.Fatalf("pending view: %v", got) + } + // The field is omitted when there is nothing to warn about. + w := ps.do("GET", "/api/admin/channel-proposals?status=pending", "", strongTestKey) + if strings.Count(w.Body.String(), `"nearDuplicateOf"`) != 1 { + t.Fatalf("nearDuplicateOf must be omitted when empty: %s", w.Body) + } +} + +// Perf bound for the per-request filter: 5000 channels, a few revoked. +func BenchmarkHideRevoked(b *testing.B) { + p := &channelProposalService{now: time.Now, db: new(sql.DB)} // never queried: the snapshot is fresh + p.approved = &approvedSnapshot{ + hidden: map[string]bool{"#r1": true, "#r2": true, "#r3": true}, + expires: time.Now().Add(time.Hour), + } + chans := make([]map[string]interface{}, 5000) + for i := range chans { + chans[i] = map[string]interface{}{"name": fmt.Sprintf("#chan%d", i)} + } + chans[10]["name"] = "#r1" + ctx := context.Background() + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + resp := ChannelListResponse{Channels: chans} + p.hideRevoked(ctx, &resp) + if len(resp.Channels) != 4999 { + b.Fatalf("len = %d", len(resp.Channels)) + } + } +} diff --git a/cmd/server/openapi.go b/cmd/server/openapi.go index 6cd374fae..17e27bc05 100644 --- a/cmd/server/openapi.go +++ b/cmd/server/openapi.go @@ -150,7 +150,7 @@ func routeDescriptions() map[string]routeMeta { }}, // Channels - "GET /api/channels": {Summary: "List channels", Description: "Returns known mesh channels with message counts. approvedChannels ([{name, hash}]) lists the shared hashtag channels an administrator approved, even before they carry traffic; omitted when empty.", Tag: "channels"}, + "GET /api/channels": {Summary: "List channels", Description: "Returns known mesh channels with message counts. approvedChannels ([{name, hash}]) lists the shared hashtag channels an administrator approved, even before they carry traffic; omitted when empty. A channel with stored messages whose shared-channel proposal is not approved (revoked, suggested again and pending, or that re-suggestion rejected) is left out of the list (#251), unless the ingestor also decrypts that name through its built-in/config list (rainbow table, hashChannels, channelKeys); only approving the proposal lists it again, with its history. hiddenChannels ([string], omitted when empty) names the channels left out, so the page does not re-create their rows from live packets. The messages stay readable per channel (GET /api/channels/{hash}/messages) and nothing is deleted. Decided per channel with stored messages from a short-lived snapshot of the proposals table (10s, dropped early when an approve/revoke result is read), never a per-request query and without a row cap. When the ingestor's built-in names file is missing or unreadable nothing is hidden. GET /api/analytics/channels is not filtered and still counts these channels.", Tag: "channels"}, "GET /api/channels/{hash}/messages": {Summary: "Get channel messages", Description: "Returns messages for a specific channel.", Tag: "channels"}, // Shared channel proposals (internal/channelregistry). The server only @@ -158,10 +158,10 @@ func routeDescriptions() map[string]routeMeta { "GET /api/channel-proposals/config": {Summary: "Channel suggestion availability", Description: "Returns {enabled}: whether public suggestions are open (requires channelProposals.enabled and a strong apiKey).", Tag: "channels"}, "POST /api/channel-proposals": {Summary: "Suggest a public hashtag channel", Description: "Body {name}, exactly one JSON object without other fields. Only public hashtag channel names (at most 31 UTF-8 bytes including #, case preserved, no control, line-separator or invisible formatting characters) are accepted — never keys. Returns 202 {requestId}; 400 invalid body or name, 403 disabled, 429 rate limited, 503 queue full (both with Retry-After).", Tag: "channels"}, "GET /api/channel-proposals/requests/{requestId}": {Summary: "Status of a suggestion or review request", Description: "Returns {status: queued|pending|approved|rejected|revoked|error, proposal: {id, name, status, createdAt, reviewedAt}, error, builtIn}. builtIn is true when the ingestor already decrypts the name through its built-in/config list. 404 when unknown or expired (24h).", Tag: "channels"}, - "GET /api/admin/channel-proposals": {Summary: "List channel suggestions", Description: "Returns {proposals, enabled}, newest first, bounded. A proposal whose name the ingestor already decrypts through its built-in/config list (rainbow table, hashChannels, channelKeys) carries builtIn: true. Optional status filter.", Tag: "admin", Auth: true, QueryParams: []paramMeta{{Name: "status", Description: "pending, approved, rejected or revoked", Type: "string"}}}, + "GET /api/admin/channel-proposals": {Summary: "List channel suggestions", Description: "Returns {proposals, enabled}, newest first, bounded. A proposal whose name the ingestor already decrypts through its built-in/config list (rainbow table, hashChannels, channelKeys) carries builtIn: true. A proposal whose name differs from another proposal's or a built-in name only by letter case carries nearDuplicateOf: [those names] — a hint, not a merge: hashtag keys are derived from the exact bytes of the name (sha256 of \"#name\"), so #HelloWorld and #helloworld are different channels and stay separate proposals. The hint also sees proposals outside the status filter. Optional status filter.", Tag: "admin", Auth: true, QueryParams: []paramMeta{{Name: "status", Description: "pending, approved, rejected or revoked", Type: "string"}}}, "POST /api/admin/channel-proposals/{id}/approve": {Summary: "Approve a channel suggestion", Description: "Queues the approval and returns 202 {requestId}. Approved channels are decrypted by the ingestor and listed for everyone.", Tag: "admin", Auth: true}, "POST /api/admin/channel-proposals/{id}/reject": {Summary: "Reject a channel suggestion", Description: "Queues the rejection and returns 202 {requestId}.", Tag: "admin", Auth: true}, - "POST /api/admin/channel-proposals/{id}/revoke": {Summary: "Revoke an approved channel suggestion", Description: "Undoes a previous approval: the ingestor stops decrypting the channel and it drops out of GET /api/channels' approvedChannels. Historical messages already decoded and stored are NOT deleted or hidden. Synchronous precondition check: 202 {requestId} only when the proposal is currently approved; 409 (no side effect, nothing queued) when it is not.", Tag: "admin", Auth: true}, + "POST /api/admin/channel-proposals/{id}/revoke": {Summary: "Revoke an approved channel suggestion", Description: "Undoes a previous approval: the ingestor stops decrypting the channel and it drops out of GET /api/channels' approvedChannels. The channel also leaves GET /api/channels (#251) unless the ingestor still decrypts that name through its built-in/config list. Historical messages already decoded and stored are NOT deleted: they stay readable per channel (GET /api/channels/{hash}/messages), and the channel returns to the list, with its history, if the suggestion is approved again. Synchronous precondition check: 202 {requestId} only when the proposal is currently approved; 409 (no side effect, nothing queued) when it is not.", Tag: "admin", Auth: true}, // Observers "GET /api/observers": {Summary: "List observers", Description: "Returns all known packet observers/gateways.", Tag: "observers"}, diff --git a/cmd/server/routes.go b/cmd/server/routes.go index 7228a9430..1e2c67703 100644 --- a/cmd/server/routes.go +++ b/cmd/server/routes.go @@ -3316,7 +3316,13 @@ func (s *Server) handleChannels(w http.ResponseWriter, r *http.Request) { channels = append(channels[:len(channels):len(channels)], encrypted...) } } - writeJSON(w, ChannelListResponse{Channels: channels, ApprovedChannels: s.channelProposals().approvedChannels(r.Context())}) + // #251: a revoked shared channel leaves the list; its messages stay + // stored and readable. Without a database there are no proposals, so + // the in-memory branch below has nothing to hide. + props := s.channelProposals() + resp := ChannelListResponse{Channels: channels, ApprovedChannels: props.approvedChannels(r.Context())} + props.hideRevoked(r.Context(), &resp) + writeJSON(w, resp) return } if s.store != nil { diff --git a/cmd/server/types.go b/cmd/server/types.go index 2805a87af..58058e23d 100644 --- a/cmd/server/types.go +++ b/cmd/server/types.go @@ -1464,6 +1464,11 @@ type ChannelListResponse struct { // ApprovedChannels are the shared hashtag channels an administrator // approved, listed even before they carry any traffic. Omitted when empty. ApprovedChannels []ApprovedChannel `json:"approvedChannels,omitempty"` + // HiddenChannels names the channels with stored messages that are left out + // of Channels because their shared-channel proposal is not approved (#251). + // The page uses it so a live message does not bring such a row back. + // Omitted when nothing is hidden. + HiddenChannels []string `json:"hiddenChannels,omitempty"` } // ApprovedChannel is one shared hashtag channel. Hash equals Name: decrypted diff --git a/docs/api-spec.md b/docs/api-spec.md index 739620d36..624e6b430 100644 --- a/docs/api-spec.md +++ b/docs/api-spec.md @@ -1255,10 +1255,42 @@ List decoded channels with message counts. // before they carry traffic. Omitted when there are none. hash == name. "approvedChannels"?: [ { "name": string, "hash": string } - ] + ], + // Channels with stored messages left out of `channels` because their + // proposal is not approved (see below). Omitted when none. + "hiddenChannels"?: string[] } ``` +**Revoked channels are left out of `channels`.** A channel with stored messages +whose shared-channel proposal is **not approved** — revoked (see +[revoke](#post-apiadminchannel-proposalsidrevoke)), suggested again and +pending, or that re-suggestion rejected — does not appear in the list. Only +approving the proposal lists it again, with its history. Such a name has stored +messages only because it was decrypted earlier (approved, or through the +config), so a pending or rejected proposal never hides a channel that was never +decrypted. The exception is a name the ingestor also decrypts through its +built-in/config list (built-in keys, rainbow table, `hashChannels`, +`channelKeys`): that traffic keeps being decrypted, so such a channel is never +hidden. When the ingestor's `builtin-channels.json` is missing or unreadable, +nothing is hidden. Nothing is deleted: `GET /api/channels/:hash/messages` still +returns the history of a hidden channel. + +`hiddenChannels` (`string[]`, omitted when empty) names the channels that were +left out this way. The Channels page uses it so a live WebSocket packet for a +stored message does not create the list row again. + +The decision is read once per 10 s snapshot (the one behind `approvedChannels`, +dropped early when an approve or revoke result is read), per channel that has +stored messages, so there is no per-request query of the proposals table and no +row cap. Proposals that are not approved are removed by retention +(`channelProposals.retentionDays` after the review); a hidden channel whose +messages are still stored then appears again. + +`GET /api/analytics/channels` is not filtered: it still lists revoked channels +with their message and sender counts (no message text). Hiding is a list-level +measure, not a confidentiality control: the history stays readable by name. + --- ## GET /api/channels/:hash/messages @@ -1372,9 +1404,18 @@ A duplicate suggestion reports the existing proposal and its status (for a rejec Requires `X-API-Key`. Optional `?status=pending|approved|rejected|revoked`. Newest first, bounded. ```jsonc -{ "proposals": [Proposal & { "builtIn"?: true }], "enabled": boolean } +{ "proposals": [Proposal & { "builtIn"?: true, "nearDuplicateOf"?: string[] }], "enabled": boolean } ``` +**Letter case in names.** A hashtag channel's key is the first 16 bytes of +`sha256("#name")` of the exact name (MeshCore `docs/companion_protocol.md`, "Hashtag +Channels"; the meshcore-open app's `derivePskFromHashtag` does not change case), +so `#HelloWorld` and `#helloworld` are different channels with different keys. +They are therefore separate proposals and are never merged. `nearDuplicateOf` +lists the other proposal names (any status, also outside the `status` filter) +and built-in names that differ from this one only by letter case, sorted; it is +omitted when there are none. It is a hint for the administrator. + ## POST /api/admin/channel-proposals/:id/approve and /reject Requires `X-API-Key`. `202 { "requestId": string }`, `404` for an unknown proposal. Only pending proposals change; repeating the stored decision is harmless, and a contradicting one (reject after approve) ends with status `error`. @@ -1398,7 +1439,7 @@ Missing or wrong key: `401`. No key configured, or a weak one: `403`. There is a real race between the `409` check and the ingestor actually applying the command: another admin could approve, reject or revoke the same proposal in between. The ingestor re-validates the status from scratch when it applies the command and is the true source of truth; the synchronous `409` here is only a best-effort fast-fail for the common case, not a guarantee. When the race is lost, the request ends with status `error` and an error such as `suggestion is not approved (it is pending)`. -**Historical messages are not affected.** Revoking a channel only removes its decryption key going forward — messages the ingestor already decoded and stored while the channel was approved stay exactly as they are and remain visible on the Channels page. There is no mechanism (and none is planned as part of this) to hide or delete previously-decoded messages when a channel is revoked. +**Historical messages are kept, the channel leaves the list.** Revoking a channel removes its decryption key going forward and takes the channel out of `GET /api/channels` (unless the name is a [built-in name](#built-in-names), which keeps decrypting and is never hidden). Messages the ingestor already decoded and stored while the channel was approved are **not deleted**: `GET /api/channels/:hash/messages` still returns them, and the channel returns to the list with its history if the suggestion is approved again (suggest the name again, then approve). The server stays read-only: it only filters the list. The channel stays hidden while the name is re-suggested and pending, and after the administrator rejects that re-suggestion; to hide it again after an approval, revoke it again. A rejected re-suggestion therefore does not bring the channel back. **Retention.** A revoked proposal's row is not deleted at revoke time — only its status changes, keeping `reviewedAt` as the audit timestamp of when it was revoked. It is removed later by the same retention sweep that prunes rejected proposals, once `reviewedAt` is older than `channelProposals.retentionDays` (see [Configuration](user-guide/configuration.md#shared-channel-suggestions)). Approved rows are still never pruned. diff --git a/docs/user-guide/channels.md b/docs/user-guide/channels.md index a8782c0fa..8e4226b44 100644 --- a/docs/user-guide/channels.md +++ b/docs/user-guide/channels.md @@ -85,10 +85,12 @@ An administrator can undo a previous approval from the same **Pending / Approved Removing a channel: - Stops the ingestor from decrypting new traffic on it (unless it is a [built-in name](#built-in-names) — in the rainbow table or configured via `channelKeys`/`hashChannels` in `config.json` — in which case that key keeps working; see [Configuration](configuration.md#channel-decryption)). -- Takes it out of everyone's **Network** section going forward. -- Does **not** delete or hide any messages that were already decoded and shown while it was approved — those stay on the Channels page exactly as before. Revoking only affects future traffic. +- Takes it out of everyone's **Network** section and out of the channel list, even if messages were already decoded while it was approved. The channel page reloads its list when it opens, when you change the region or the encrypted toggle, and in the tab where the administrator removed the channel; there is no periodic reload. Another tab that has the channel open keeps showing it until its list reloads (for example after a page reload), and a conversation open on it closes then. +- Does **not** delete any message that was already decoded — the history stays stored and comes back with the channel if the name is approved again. A [built-in name](#built-in-names) is never hidden, because its key keeps decrypting. -A removed channel shows up under the **Revoked** tab. Suggesting the same name again later starts a fresh review from **Pending** — it is never auto-approved just because it was approved before. +Letter case matters: `#HelloWorld` and `#helloworld` are different channels with different keys, so they are separate suggestions. When a suggestion differs from another one (or from a built-in name) only by case, the review dialog says "Same name in different case" so the administrator can decide. + +A removed channel shows up under the **Revoked** tab. Suggesting the same name again later starts a fresh review from **Pending** — it is never auto-approved just because it was approved before. The channel stays out of the list while that suggestion is pending and if you reject it; only **Approve** lists it again, with its history. The analytics page still counts messages of removed channels (counts only, no text), and a removed channel's history stays readable by its name. The local **Add Channel** tools (PSK channels, **Monitor Hashtag Channel**) are unchanged and still only affect your browser. diff --git a/docs/user-guide/configuration.md b/docs/user-guide/configuration.md index 65d4e780e..d01c0a09c 100644 --- a/docs/user-guide/configuration.md +++ b/docs/user-guide/configuration.md @@ -151,7 +151,7 @@ Approved channels stay decrypted and listed when `enabled` is later set to `fals Auto-approval uses the same name validation, global submission rate limit, queue and `maxApproved` cap as manual approval. At capacity, a new suggestion fails rather than becoming an unapproved row. Be careful on a public instance: visitors can fill the approved-channel allowance. Rejected and revoked names remain protected from automatic re-approval while their rows exist. Retention eventually removes these rows, so an old name can be proposed as new again; a permanent blocklist is not provided. Turning off `autoApprove` affects new submissions only and does not revoke already approved channels. -An administrator can also revoke a previously approved channel (see [Channels](channels.md#revoking-an-approved-channel)) — this undoes the decryption going forward but never deletes or hides messages already decoded while it was approved. A revoked row is retained and pruned by the same `retentionDays` rule as a rejected one (counted from when it was revoked, not when it was first submitted); an approved row is still never pruned. Revoking introduces no new configuration of its own — it reuses the `maxPending`/`maxApproved`/`retentionDays`/`submissionsPerHour` limits above. +An administrator can also revoke a previously approved channel (see [Channels](channels.md#revoking-an-approved-channel)) — this undoes the decryption going forward and takes the channel out of the channel list, but never deletes messages already decoded while it was approved (they return with the channel if it is approved again; a name the ingestor decrypts through its built-in/config list is never hidden). The channel also stays out of the list while the name is re-suggested (pending) or that re-suggestion is rejected; only an approval lists it again. Once the row is pruned by `retentionDays`, a channel whose messages are still stored shows in the list again. A revoked row is retained and pruned by the same `retentionDays` rule as a rejected one (counted from when it was revoked, not when it was first submitted); an approved row is still never pruned. Revoking introduces no new configuration of its own — it reuses the `maxPending`/`maxApproved`/`retentionDays`/`submissionsPerHour` limits above. ## Map defaults diff --git a/internal/channelregistry/sql.go b/internal/channelregistry/sql.go index dc54a8ee3..10145fed1 100644 --- a/internal/channelregistry/sql.go +++ b/internal/channelregistry/sql.go @@ -53,6 +53,66 @@ func ListApprovedNames(ctx context.Context, db *sql.DB, limit int) ([]string, er return out, rows.Err() } +// ListNotApprovedChannelsWithTraffic returns the names of proposals that are +// not approved (pending, rejected or revoked) and whose channel has decoded +// messages stored; these are the channels GET /api/channels leaves out unless +// the ingestor decrypts the name through its config (#251). +// +// The decision is read per channel that can appear in the list, not from a +// capped list of proposal rows: the result is a subset of the distinct +// channel_hash values GetChannels already returns in full, so it needs no +// limit and an old revoked proposal never drops out behind newer ones. A +// proposal without stored traffic has no list entry to hide and is skipped. +// A missing proposals table yields an empty result. +func ListNotApprovedChannelsWithTraffic(ctx context.Context, db *sql.DB) ([]string, error) { + rows, err := db.QueryContext(ctx, + `SELECT p.name FROM channel_proposals p + WHERE p.status <> 'approved' + AND EXISTS (SELECT 1 FROM transmissions t + WHERE t.payload_type = 5 AND t.channel_hash = p.name)`) + if err != nil { + if IsMissingTable(err) { + return nil, nil + } + return nil, err + } + defer rows.Close() + var out []string + for rows.Next() { + var name string + if err := rows.Scan(&name); err != nil { + return nil, err + } + out = append(out, name) + } + return out, rows.Err() +} + +// ListAllNames returns every proposal name regardless of status, newest +// first, capped at limit (MaxListed at most). The admin list uses it to spot +// names that differ only by letter case without depending on the rows it +// returns. A missing table yields an empty result. +func ListAllNames(ctx context.Context, db *sql.DB, limit int) ([]string, error) { + rows, err := db.QueryContext(ctx, + `SELECT name FROM channel_proposals ORDER BY created_at DESC, id LIMIT ?`, clampLimit(limit)) + if err != nil { + if IsMissingTable(err) { + return nil, nil + } + return nil, err + } + defer rows.Close() + var out []string + for rows.Next() { + var name string + if err := rows.Scan(&name); err != nil { + return nil, err + } + out = append(out, name) + } + return out, rows.Err() +} + // ListProposals returns proposals, newest first, optionally filtered by // status ("" for all). A missing table yields an empty result. func ListProposals(ctx context.Context, db *sql.DB, status string, limit int) ([]Proposal, error) { diff --git a/public/channel-proposals.css b/public/channel-proposals.css index 44535f9a3..071dd5e6c 100644 --- a/public/channel-proposals.css +++ b/public/channel-proposals.css @@ -73,6 +73,7 @@ .ch-proposals-name { font-weight: 600; overflow-wrap: anywhere; } .ch-proposals-meta { font-size: 12px; color: var(--text-muted); } .ch-proposals-builtin { font-size: 12px; color: var(--warning); } +.ch-proposals-neardup { font-size: 12px; color: var(--warning); } .ch-proposals-actions { display: flex; gap: 6px; align-items: center; } .ch-proposals-actions button, .ch-proposals-toolbar button { min-height: 48px; } diff --git a/public/channel-proposals.js b/public/channel-proposals.js index 120cba65a..ced2af769 100644 --- a/public/channel-proposals.js +++ b/public/channel-proposals.js @@ -235,6 +235,9 @@ // ── Suggest form (a section of the Add Channel modal) ───────────────── var BUILTIN_NOTE = 'This site already decrypts it through its built-in channel list, so sharing it changes nothing.'; + // #251: hashtag keys come from the exact name (sha256 of "#name"), so a + // different case is a different channel; the server only points it out. + var NEAR_DUP_NOTE = 'Hashtag keys are derived from the exact name, so these are different channels with different keys.'; // Plain text for the suggest form's status line (shown with textContent). function suggestMessage(st, fallbackName) { @@ -443,10 +446,18 @@ ? 'Built in: already decrypted' : ''; return '