From be05d38ed6adf428539a563fd719ff45523d0782 Mon Sep 17 00:00:00 2001 From: Dennis Jakobsen Date: Fri, 18 Sep 2026 16:41:39 +0200 Subject: [PATCH 1/8] fix(reach): hide blacklisted and hidden identities on the Reach page /api/nodes/{pubkey}/reach exposed identities that other views hide: - The target check only covered the node blacklist and a hidden node name. Observer-blacklisted pubkeys, nodes hidden by their observer name, and observer-only pubkeys with a hidden name got a full 200 report (name, links). - A visible node's report listed hidden and blacklisted neighbours and direct observers by pubkey and name. One shared rule now decides (identityHidden): node blacklist, observer blacklist, or any node/observer name with a hidden-name prefix. - Target: blacklists are checked before the cache; names are read live (one bulk json_each query) on every serve, so a hidden target 404s with the unknown-node body even when its report is cached. - Lists: hidden identities are removed from links and direct_observers on every serve, cached or not, with bidirectional_links and direct_observers recounted. A rename, a prefix change or a blacklist change applies on the next request; the cached body is reused while nothing changes. - A failed name lookup fails closed (500), never serving unfiltered data. - neighbor_degree / degree_rank / nodes_with_edges are unchanged here. Tests cover each case, after cache warm-up and in both directions; they fail against the previous handler. A before/after benchmark measures the added per-request lookup. Co-Authored-By: Claude Opus 5 --- cmd/server/identity_visibility.go | 105 ++++++ cmd/server/node_reach.go | 211 +++++++++-- cmd/server/node_reach_test.go | 3 +- .../node_reach_visibility_bench_test.go | 101 +++++ cmd/server/node_reach_visibility_test.go | 345 ++++++++++++++++++ docs/api-spec.md | 28 +- 6 files changed, 757 insertions(+), 36 deletions(-) create mode 100644 cmd/server/identity_visibility.go create mode 100644 cmd/server/node_reach_visibility_bench_test.go create mode 100644 cmd/server/node_reach_visibility_test.go diff --git a/cmd/server/identity_visibility.go b/cmd/server/identity_visibility.go new file mode 100644 index 000000000..765a481b4 --- /dev/null +++ b/cmd/server/identity_visibility.go @@ -0,0 +1,105 @@ +package main + +import ( + "encoding/json" + "fmt" + "strings" +) + +// identityHidden is the shared visibility rule for a pubkey's identity on +// per-node views such as Reach: an identity is hidden when it is +// node-blacklisted, observer-blacklisted, or when any of its known names — +// node name or observer name — starts with a configured hidden-name prefix +// (#1181). pubkey may be any case. +func identityHidden(cfg *Config, pubkey string, names ...string) bool { + if cfg == nil { + return false + } + if cfg.IsBlacklisted(pubkey) || cfg.IsObserverBlacklisted(pubkey) { + return true + } + for _, name := range names { + if cfg.IsNameHidden(name) { + return true + } + } + return false +} + +// identityNamesSQL looks up the node and observer names of a JSON array of +// lower-case pubkeys in one statement with fixed text (cheap to prepare, +// unlike a variable-length IN list). Observer ids are stored upper-case, +// node keys lower-case; both sides use their primary-key index. +const identityNamesSQL = ` + SELECT n.public_key, COALESCE(n.name, '') FROM json_each(?1) j JOIN nodes n ON n.public_key = j.value + UNION ALL + SELECT o.id, COALESCE(o.name, '') FROM json_each(?1) j JOIN observers o ON o.id = upper(j.value) + UNION ALL + SELECT o.id, COALESCE(o.name, '') FROM json_each(?1) j JOIN observers o ON o.id = j.value AND o.id <> upper(j.value)` + +// identityNames returns the current node and observer names of the given +// pubkeys, read live from the DB in one bulk query (no per-pubkey lookups), +// keyed by lowercase pubkey. A rename into a hidden prefix therefore takes +// effect on the next request, cached response or not. Returns (nil, nil) when +// no hidden-name prefix is configured: names cannot hide anything then. +func (s *Server) identityNames(pubkeys []string) (map[string][]string, error) { + if s.cfg == nil || len(s.cfg.ActiveHiddenNamePrefixes()) == 0 || len(pubkeys) == 0 { + return nil, nil + } + if s.db == nil || s.db.conn == nil { + return nil, fmt.Errorf("identity names: no database") + } + seen := make(map[string]bool, len(pubkeys)) + keys := make([]string, 0, len(pubkeys)) + for _, pk := range pubkeys { + pk = strings.ToLower(pk) + if pk != "" && !seen[pk] { + seen[pk] = true + keys = append(keys, pk) + } + } + arg, err := json.Marshal(keys) + if err != nil { + return nil, err + } + out := make(map[string][]string, len(keys)) + add := func(pk, name string) { + if name != "" { + pk = strings.ToLower(pk) + out[pk] = append(out[pk], name) + } + } + if err := s.scanIdentityNames(identityNamesSQL, []interface{}{string(arg)}, add); err != nil { + return nil, fmt.Errorf("identity names: %w", err) + } + return out, nil +} + +func (s *Server) scanIdentityNames(q string, args []interface{}, add func(pk, name string)) error { + rows, err := s.db.conn.Query(q, args...) + if err != nil { + return err + } + defer rows.Close() + for rows.Next() { + var pk, name string + if err := rows.Scan(&pk, &name); err != nil { + return err + } + add(pk, name) + } + return rows.Err() +} + +// isIdentityHidden applies identityHidden to one pubkey with its live names. +// A failed name lookup is returned as an error so callers fail closed. +func (s *Server) isIdentityHidden(pubkey string) (bool, error) { + if identityHidden(s.cfg, pubkey) { + return true, nil + } + names, err := s.identityNames([]string{pubkey}) + if err != nil { + return false, err + } + return identityHidden(s.cfg, pubkey, names[strings.ToLower(pubkey)]...), nil +} diff --git a/cmd/server/node_reach.go b/cmd/server/node_reach.go index 8cb1d9283..6e0f99338 100644 --- a/cmd/server/node_reach.go +++ b/cmd/server/node_reach.go @@ -261,17 +261,26 @@ func clampDays(d int) int { // --- bounded TTL cache. perf is gated by the time window; this just avoids // recompute under dashboard polling. Keyed "pubkey|days". --- // -// reachCacheMax bounds entry count; at ~2KB of marshalled JSON per entry the -// worst case is well under 1MB, so an entry cap (rather than a byte budget) -// keeps the bookkeeping trivial while staying memory-safe. +// reachCacheMax bounds entry count; an entry holds the report plus its +// marshalled JSON (~2KB each for a typical node, so a few KB per entry), so +// the worst case stays in the low MB and an entry cap (rather than a byte +// budget) keeps the bookkeeping trivial while staying memory-safe. const ( reachCacheTTL = 5 * time.Minute reachCacheMax = 256 ) +// reachCacheEntry keeps the computed report (resp, unfiltered) and the body +// last served for it. Visibility is applied on every serve (visibleReach), not +// at compute time, so a cached report never outlives a blacklist, hidden-prefix +// or rename change; hiddenKey records which identities the cached body omits so +// an unchanged result reuses it without re-marshalling. type reachCacheEntry struct { - at time.Time - raw []byte + at time.Time + resp NodeReachResponse + raw []byte + hiddenKey string + found bool // false only for the "node not found" result, which is never cached } // reachState bundles per-server reach caches. Was a set of package-level @@ -295,17 +304,17 @@ type reachState struct { degreeSnap *degreeSnapshot } -// reachCacheGet returns the cached marshalled JSON for key. The returned slice -// is shared (not copied): it is treated as immutable — only ever handed to -// w.Write — so callers MUST NOT mutate it. -func (s *Server) reachCacheGet(key string) ([]byte, bool) { +// reachCacheGet returns the cached entry for key. Its raw slice and resp +// slices are shared (not copied) and treated as immutable — raw is only ever +// handed to w.Write — so callers MUST NOT mutate them. +func (s *Server) reachCacheGet(key string) (reachCacheEntry, bool) { s.reach.cacheMu.RLock() defer s.reach.cacheMu.RUnlock() e, ok := s.reach.cache[key] if !ok || time.Since(e.at) > reachCacheTTL { - return nil, false + return reachCacheEntry{}, false } - return e.raw, true + return e, true } // reachCacheLen returns the current entry count in the reach response cache. @@ -350,7 +359,7 @@ func isHexPubkey(s string) bool { return true } -func (s *Server) reachCachePut(key string, raw []byte) { +func (s *Server) reachCachePut(key string, e reachCacheEntry) { s.reach.cacheMu.Lock() defer s.reach.cacheMu.Unlock() if s.reach.cache == nil { @@ -359,7 +368,110 @@ func (s *Server) reachCachePut(key string, raw []byte) { if _, exists := s.reach.cache[key]; !exists && len(s.reach.cache) >= reachCacheMax { s.evictReachLocked() } - s.reach.cache[key] = reachCacheEntry{at: time.Now(), raw: raw} + s.reach.cache[key] = e +} + +// reachCacheSetBody stores a re-marshalled body for the entry computed at at, +// keeping its original TTL; a no-op if the entry was replaced or evicted. +func (s *Server) reachCacheSetBody(key string, at time.Time, raw []byte, hiddenKey string) { + s.reach.cacheMu.Lock() + defer s.reach.cacheMu.Unlock() + if e, ok := s.reach.cache[key]; ok && e.at.Equal(at) { + e.raw, e.hiddenKey = raw, hiddenKey + s.reach.cache[key] = e + } +} + +// visibleReach applies visibility to a report on every serve, cached or not, +// so no visibility change waits for the cache TTL. One live-name lookup covers +// the target and every listed identity; each is checked with identityHidden +// against its live names and the name recorded in the report. targetHidden +// reports that the target itself is hidden now (the caller must 404). +// Otherwise every hidden identity is removed from links and direct_observers +// and the counts derived from those lists are recomputed; hiddenKey lists the +// removed pubkeys (sorted, comma-joined; "" when none, in which case resp is +// returned as is, without copying). +func (s *Server) visibleReach(resp NodeReachResponse) (out NodeReachResponse, hiddenKey string, targetHidden bool, err error) { + pks := make([]string, 0, 1+len(resp.Links)+len(resp.DirectObservers)) + pks = append(pks, resp.Node.Pubkey) + for _, l := range resp.Links { + pks = append(pks, l.Pubkey) + } + for _, o := range resp.DirectObservers { + pks = append(pks, o.Pubkey) + } + live, err := s.identityNames(pks) + if err != nil { + return NodeReachResponse{}, "", false, err + } + hidden := func(pk, recorded string) bool { + return identityHidden(s.cfg, pk, recorded) || identityHidden(s.cfg, pk, live[strings.ToLower(pk)]...) + } + if hidden(resp.Node.Pubkey, resp.Node.Name) { + return NodeReachResponse{}, "", true, nil + } + var removed []string + for _, l := range resp.Links { + if hidden(l.Pubkey, l.Name) { + removed = append(removed, l.Pubkey) + } + } + for _, o := range resp.DirectObservers { + if hidden(o.Pubkey, o.Name) { + removed = append(removed, o.Pubkey) + } + } + if len(removed) == 0 { + return resp, "", false, nil + } + drop := make(map[string]bool, len(removed)) + for _, pk := range removed { + drop[pk] = true + } + links := make([]NodeReachLink, 0, len(resp.Links)) + bidir := 0 + for _, l := range resp.Links { + if !drop[l.Pubkey] { + if l.Bidir { + bidir++ + } + links = append(links, l) + } + } + obs := make([]NodeReachObserver, 0, len(resp.DirectObservers)) + for _, o := range resp.DirectObservers { + if !drop[o.Pubkey] { + obs = append(obs, o) + } + } + keys := make([]string, 0, len(drop)) + for pk := range drop { + keys = append(keys, pk) + } + sort.Strings(keys) + out = resp // Node, Window, ReliableTokens are the target's own + out.Links, out.DirectObservers = links, obs + out.Importance.BidirectionalLinks, out.Importance.DirectObservers = bidir, len(obs) + return out, strings.Join(keys, ","), false, nil +} + +// reachBody returns e's body with visibility applied now, re-marshalling (and +// refreshing the cached body) only when the set of hidden identities changed. +// targetHidden means the caller must answer 404 instead. +func (s *Server) reachBody(key string, e reachCacheEntry) (raw []byte, targetHidden bool, err error) { + vis, hiddenKey, targetHidden, err := s.visibleReach(e.resp) + if err != nil || targetHidden { + return nil, targetHidden, err + } + if e.raw != nil && hiddenKey == e.hiddenKey { + return e.raw, false, nil + } + raw, err = json.Marshal(vis) + if err != nil { + return nil, false, err + } + s.reachCacheSetBody(key, e.at, raw, hiddenKey) + return raw, false, nil } // evictReachLocked drops expired entries first; if still at the cap it evicts @@ -396,11 +508,10 @@ func (s *Server) handleNodeReach(w http.ResponseWriter, r *http.Request) { writeError(w, 400, "invalid pubkey: expected 64 hex chars") return } - if s.cfg != nil && s.cfg.IsBlacklisted(pubkey) { - writeError(w, 404, "Not found") - return - } - if s.isPubkeyHidden(pubkey) { + // Hidden identities 404 so callers learn nothing about them (#1181): + // blacklisted / observer-blacklisted pubkeys here, hidden node or + // observer names (live) on every serve — see visibleReach. + if identityHidden(s.cfg, pubkey) { writeError(w, 404, "Not found") return } @@ -427,9 +538,8 @@ func (s *Server) handleNodeReach(w http.ResponseWriter, r *http.Request) { // the gen actually moved (#1629 round-2, adversarial #5). s.reachPurgeIfBlacklistGenChanged(gen) cacheKey := pubkey + "|" + strconv.Itoa(days) + "|g" + strconv.FormatUint(gen, 10) - if raw, ok := s.reachCacheGet(cacheKey); ok { - w.Header().Set("Content-Type", "application/json") - w.Write(raw) + if e, ok := s.reachCacheGet(cacheKey); ok { + s.writeReachEntry(w, cacheKey, e) return } @@ -437,9 +547,19 @@ func (s *Server) handleNodeReach(w http.ResponseWriter, r *http.Request) { // shared computation uses the triggering request's context; a disconnect // there can cancel the in-flight scan for all waiters (acceptable — the // next request recomputes). + // Cache miss: check the target's live names before the expensive scan. + // A failed name lookup fails closed. + if hidden, err := s.isIdentityHidden(pubkey); err != nil { + log.Printf("[reach] visibility check failed for %s: %v", pubkey, err) + writeError(w, 500, "reach computation failed") + return + } else if hidden { + writeError(w, 404, "Not found") + return + } v, err, _ := s.reach.sf.Do(cacheKey, func() (interface{}, error) { - if raw, ok := s.reachCacheGet(cacheKey); ok { - return raw, nil + if e, ok := s.reachCacheGet(cacheKey); ok { + return e, nil } resp, ok, cErr := s.computeNodeReach(r.Context(), pubkey, days) if cErr != nil { @@ -449,22 +569,49 @@ func (s *Server) handleNodeReach(w http.ResponseWriter, r *http.Request) { return nil, cErr } if !ok { - return []byte(nil), nil + return reachCacheEntry{}, nil + } + // Marshal the visible body once here so the waiters sharing this + // result reuse it (each still re-checks visibility in reachBody). + e := reachCacheEntry{at: time.Now(), resp: resp, found: true} + vis, hiddenKey, targetHidden, vErr := s.visibleReach(resp) + if vErr != nil { + return nil, vErr } - raw, mErr := json.Marshal(resp) - if mErr != nil { - log.Printf("[reach] marshal failed for %s: %v", cacheKey, mErr) - return nil, mErr + if !targetHidden { // else leave raw nil; writeReachEntry re-checks and 404s + raw, mErr := json.Marshal(vis) + if mErr != nil { + log.Printf("[reach] marshal failed for %s: %v", cacheKey, mErr) + return nil, mErr + } + e.raw, e.hiddenKey = raw, hiddenKey } - s.reachCachePut(cacheKey, raw) - return raw, nil + s.reachCachePut(cacheKey, e) + return e, nil }) if err != nil { writeError(w, 500, "reach computation failed") return } - raw, _ := v.([]byte) - if len(raw) == 0 { + e, _ := v.(reachCacheEntry) + if !e.found { + writeError(w, 404, "Not found") + return + } + s.writeReachEntry(w, cacheKey, e) +} + +// writeReachEntry writes a found entry's body with visibility applied now: +// 404 when the target itself became hidden. A failed live-name lookup fails +// closed (500), never serving unfiltered data. +func (s *Server) writeReachEntry(w http.ResponseWriter, key string, e reachCacheEntry) { + raw, targetHidden, err := s.reachBody(key, e) + if err != nil { + log.Printf("[reach] serving %s failed: %v", key, err) + writeError(w, 500, "reach computation failed") + return + } + if targetHidden { writeError(w, 404, "Not found") return } diff --git a/cmd/server/node_reach_test.go b/cmd/server/node_reach_test.go index 606f8e2c7..5352e44ac 100644 --- a/cmd/server/node_reach_test.go +++ b/cmd/server/node_reach_test.go @@ -5,6 +5,7 @@ import ( "database/sql" "strconv" "testing" + "time" _ "modernc.org/sqlite" ) @@ -189,7 +190,7 @@ func TestReachCacheEviction_BoundedNotWiped(t *testing.T) { srv := &Server{} resetReachState(t, srv) for i := 0; i < reachCacheMax+50; i++ { - srv.reachCachePut("k"+strconv.Itoa(i), []byte("x")) + srv.reachCachePut("k"+strconv.Itoa(i), reachCacheEntry{at: time.Now(), raw: []byte("x"), found: true}) } srv.reach.cacheMu.RLock() n := len(srv.reach.cache) diff --git a/cmd/server/node_reach_visibility_bench_test.go b/cmd/server/node_reach_visibility_bench_test.go new file mode 100644 index 000000000..1a07b3a48 --- /dev/null +++ b/cmd/server/node_reach_visibility_bench_test.go @@ -0,0 +1,101 @@ +package main + +import ( + "database/sql" + "fmt" + "net/http" + "path/filepath" + "strings" + "testing" + "time" + + _ "modernc.org/sqlite" +) + +// benchReachVisibilityDB builds a file-backed DB where a target repeater has +// `neighbours` links (half heard both ways) plus 5 direct observers — the Reach +// report a cache hit re-checks for visibility. Uses only APIs that also exist +// on master so the same benchmark runs before/after. +func benchReachVisibilityDB(b *testing.B, neighbours int) (*DB, string) { + b.Helper() + conn, err := sql.Open("sqlite", filepath.Join(b.TempDir(), "vis.db")) + if err != nil { + b.Fatal(err) + } + b.Cleanup(func() { conn.Close() }) + for _, s := range []string{ + `CREATE TABLE nodes (public_key TEXT PRIMARY KEY, name TEXT, role TEXT, lat REAL, lon REAL, last_seen TEXT, first_seen TEXT, advert_count INTEGER DEFAULT 0, battery_mv INTEGER, temperature_c REAL, foreign_advert INTEGER DEFAULT 0)`, + `CREATE TABLE observers (id TEXT PRIMARY KEY, name TEXT)`, + `CREATE TABLE transmissions (id INTEGER PRIMARY KEY, from_pubkey TEXT, payload_type INTEGER)`, + `CREATE TABLE observations (id INTEGER PRIMARY KEY, transmission_id INTEGER, observer_idx INTEGER, snr REAL, path_json TEXT, timestamp INTEGER)`, + `CREATE TABLE neighbor_edges (node_a TEXT NOT NULL, node_b TEXT NOT NULL, count INTEGER DEFAULT 1, last_seen TEXT, PRIMARY KEY (node_a, node_b))`, + `CREATE INDEX idx_obs_ts ON observations(timestamp)`, + } { + if _, err := conn.Exec(s); err != nil { + b.Fatal(err) + } + } + target := pk64("01fa") + tx, err := conn.Begin() + if err != nil { + b.Fatal(err) + } + ins := func(q string, args ...interface{}) { + if _, err := tx.Exec(q, args...); err != nil { + b.Fatal(err) + } + } + ins(`INSERT INTO nodes (public_key, name, role, lat, lon, last_seen, first_seen) VALUES (?, 'Target', 'repeater', 56.1, 10.2, '2026-09-01T00:00:00Z', '2026-06-01T00:00:00Z')`, target) + for i := 0; i < 5; i++ { + ins(`INSERT INTO observers (id, name) VALUES (?, ?)`, strings.ToUpper(pk64(fmt.Sprintf("0b%02x", i))), fmt.Sprintf("Observer %d", i)) + } + now := time.Now().Unix() + id := 0 + for i := 0; i < neighbours; i++ { + tok := fmt.Sprintf("%04x", 0x2000+i) + ins(`INSERT INTO nodes (public_key, name, role, lat, lon, last_seen, first_seen) VALUES (?, ?, 'repeater', 56.2, 10.3, '2026-09-01T00:00:00Z', '2026-06-01T00:00:00Z')`, + pk64(tok), fmt.Sprintf("Neighbour %d", i)) + paths := []string{`["` + strings.ToUpper(tok) + `","01FA"]`} + if i%2 == 0 { + paths = append(paths, `["01FA","`+strings.ToUpper(tok)+`"]`) + } + for _, p := range paths { + id++ + ins(`INSERT INTO transmissions (id, from_pubkey, payload_type) VALUES (?, '', 5)`, id) + ins(`INSERT INTO observations (id, transmission_id, observer_idx, snr, path_json, timestamp) VALUES (?, ?, ?, -7.0, ?, ?)`, id, id, 1+id%5, p, now) + } + } + if err := tx.Commit(); err != nil { + b.Fatal(err) + } + db := &DB{conn: conn} + db.isV3Flag.forceTrue() + return db, target +} + +// BenchmarkNodeReachCacheHitVisibility: /api/nodes/{pk}/reach served from the +// response cache for a node with 50 links, with and without a configured +// hidden-name prefix (only then do names need a live DB lookup). +func BenchmarkNodeReachCacheHitVisibility(b *testing.B) { + for _, c := range []struct { + name string + prefixes []string + }{{"prefixes_configured", []string{"🚫"}}, {"no_prefixes", nil}} { + b.Run(c.name, func(b *testing.B) { + db, target := benchReachVisibilityDB(b, 50) + cfg := &Config{HiddenNamePrefixes: c.prefixes} + srv := &Server{store: &PacketStore{db: db}, db: db, cfg: cfg, perfStats: NewPerfStats()} + path := "/api/nodes/" + target + "/reach?days=7" + if rr := serveReach(srv, path); rr.Code != http.StatusOK || !strings.Contains(rr.Body.String(), "Neighbour 49") { + b.Fatalf("warm-up: %d %.200s", rr.Code, rr.Body.String()) + } + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + if rr := serveReach(srv, path); rr.Code != http.StatusOK { + b.Fatalf("status %d", rr.Code) + } + } + }) + } +} diff --git a/cmd/server/node_reach_visibility_test.go b/cmd/server/node_reach_visibility_test.go new file mode 100644 index 000000000..9940a0155 --- /dev/null +++ b/cmd/server/node_reach_visibility_test.go @@ -0,0 +1,345 @@ +package main + +import ( + "context" + "database/sql" + "encoding/json" + "fmt" + "net/http" + "sort" + "strings" + "testing" + "time" + + _ "modernc.org/sqlite" +) + +// Reach visibility: the target and every listed neighbour / direct observer +// follow identityHidden — node blacklist, observer blacklist, hidden node or +// observer name — checked live on every request, cached report or not. + +type visFixture struct { + db *DB + cfg *Config + srv *Server + // target and neighbours (pubkeys, lower-case) + n, hidName, blNode, blObs, hidObsName, visible string + // observer-only pubkeys (lower-case; stored upper-case in observers.id) + o1, o2Hidden, o3Blacklisted string +} + +// newReachVisibilityDB builds a Reach DB where target N has, in 30 days: +// +// A "🚫 private A" — hidden by node name (we hear A) +// B "Bravo" — node-blacklisted (bidirectional) +// C "Charlie" — observer-blacklisted (we hear C) +// D "Delta" — visible node name, observer row "🚫 obs D" (D hears us) +// E "Echo" — visible (bidirectional) +// O1 "Obs one" — observer-only, visible (direct observer) +// O2 "🚫 hidden obs" — observer-only, hidden name (direct observer) +// O3 "Obs three" — observer-only, observer-blacklisted (direct observer) +func newReachVisibilityDB(t *testing.T) *visFixture { + t.Helper() + f := &visFixture{ + n: pk64("01fa"), hidName: pk64("aabb"), blNode: pk64("ccdd"), blObs: pk64("ee11"), + hidObsName: pk64("dd22"), visible: pk64("e5e5"), + o1: pk64("0b01"), o2Hidden: pk64("0b02"), o3Blacklisted: pk64("0b03"), + } + conn, err := sql.Open("sqlite", ":memory:") + if err != nil { + t.Fatal(err) + } + conn.SetMaxOpenConns(1) // one shared :memory: database + t.Cleanup(func() { conn.Close() }) + up := strings.ToUpper + now := time.Now().Unix() + stmts := []string{ + `CREATE TABLE nodes (public_key TEXT PRIMARY KEY, name TEXT, role TEXT, lat REAL, lon REAL, last_seen TEXT, first_seen TEXT, advert_count INTEGER DEFAULT 0, battery_mv INTEGER, temperature_c REAL, foreign_advert INTEGER DEFAULT 0)`, + `CREATE TABLE observers (id TEXT PRIMARY KEY, name TEXT)`, + `CREATE TABLE transmissions (id INTEGER PRIMARY KEY, from_pubkey TEXT, payload_type INTEGER)`, + `CREATE TABLE observations (id INTEGER PRIMARY KEY, transmission_id INTEGER, observer_idx INTEGER, snr REAL, path_json TEXT, timestamp INTEGER)`, + `CREATE TABLE neighbor_edges (node_a TEXT NOT NULL, node_b TEXT NOT NULL, count INTEGER DEFAULT 1, last_seen TEXT, PRIMARY KEY (node_a, node_b))`, + } + for _, s := range stmts { + if _, err := conn.Exec(s); err != nil { + t.Fatal(err) + } + } + for _, n := range []struct{ pk, name string }{ + {f.n, "Target"}, {f.hidName, "🚫 private A"}, {f.blNode, "Bravo"}, {f.blObs, "Charlie"}, + {f.hidObsName, "Delta"}, {f.visible, "Echo"}, + } { + if _, err := conn.Exec(`INSERT INTO nodes (public_key, name, role, lat, lon, last_seen, first_seen, advert_count) VALUES (?, ?, 'repeater', 56.1, 10.2, '2026-09-01T00:00:00Z', '2026-06-01T00:00:00Z', 1)`, n.pk, n.name); err != nil { + t.Fatal(err) + } + } + // observers rowid 1..5 (observations reference them by rowid) + for _, o := range []struct{ id, name string }{ + {up(f.o1), "Obs one"}, {up(f.o2Hidden), "🚫 hidden obs"}, {up(f.o3Blacklisted), "Obs three"}, + {up(f.hidObsName), "🚫 obs D"}, {up(pk64("0b05")), "Relay watcher"}, + } { + if _, err := conn.Exec(`INSERT INTO observers (id, name) VALUES (?, ?)`, o.id, o.name); err != nil { + t.Fatal(err) + } + } + paths := []struct { + path string + obs int + }{ + {`["AABB","01FA","CCDD"]`, 5}, // we hear A, B hears us + {`["CCDD","01FA"]`, 5}, // we hear B → B bidirectional; watcher hears us + {`["EE11","01FA","DD22"]`, 5}, // we hear C, D hears us + {`["E5E5","01FA","E5E5"]`, 5}, // E both ways + {`["01FA"]`, 1}, // O1 heard us directly + {`["01FA"]`, 2}, // O2 heard us directly + {`["01FA"]`, 3}, // O3 heard us directly + } + for i, p := range paths { + if _, err := conn.Exec(`INSERT INTO transmissions (id, from_pubkey, payload_type) VALUES (?, '', 5)`, i+1); err != nil { + t.Fatal(err) + } + if _, err := conn.Exec(`INSERT INTO observations (id, transmission_id, observer_idx, snr, path_json, timestamp) VALUES (?, ?, ?, -7.0, ?, ?)`, i+1, i+1, p.obs, p.path, now); err != nil { + t.Fatal(err) + } + } + f.db = &DB{conn: conn} + f.db.isV3Flag.forceTrue() + f.cfg = &Config{ + NodeBlacklist: []string{f.blNode}, + ObserverBlacklist: []string{up(f.blObs), f.o3Blacklisted}, + HiddenNamePrefixes: []string{"🚫"}, + } + f.srv = &Server{store: newTestStoreWithDB(t, f.db, f.cfg), db: f.db, cfg: f.cfg, perfStats: NewPerfStats()} + resetReachState(t, f.srv) + return f +} + +func (f *visFixture) get(t *testing.T, pk string) (*http.Response, NodeReachResponse, string) { + t.Helper() + rr := serveReach(f.srv, "/api/nodes/"+pk+"/reach?days=30") + var resp NodeReachResponse + if rr.Code == http.StatusOK { + if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil { + t.Fatalf("bad json: %v", err) + } + } + return rr.Result(), resp, rr.Body.String() +} + +func linkSet(resp NodeReachResponse) string { + var out []string + for _, l := range resp.Links { + out = append(out, l.Pubkey[:4]) + } + sort.Strings(out) + return strings.Join(out, ",") +} + +func obsSet(resp NodeReachResponse) string { + var out []string + for _, o := range resp.DirectObservers { + out = append(out, o.Pubkey[:4]) + } + sort.Strings(out) + return strings.Join(out, ",") +} + +// The reviewer's target cases: every hidden or blacklisted identity 404s with +// the same body as an unknown node, including observer-only pubkeys and +// hiding by observer name. +func TestNodeReach_HiddenTargets404(t *testing.T) { + f := newReachVisibilityDB(t) + unknown := serveReach(f.srv, "/api/nodes/"+pk64("beef")+"/reach?days=30") + if unknown.Code != http.StatusNotFound { + t.Fatalf("unknown node: %d", unknown.Code) + } + for name, pk := range map[string]string{ + "hidden by node name": f.hidName, + "node-blacklisted": f.blNode, + "observer-blacklisted (has a node row)": f.blObs, + "hidden by observer name only": f.hidObsName, + "observer-only, hidden name": f.o2Hidden, + "observer-only, observer-blacklisted": f.o3Blacklisted, + "upper-case form of a hidden observer id": strings.ToUpper(f.o2Hidden), + } { + res, _, body := f.get(t, pk) + if res.StatusCode != http.StatusNotFound || body != unknown.Body.String() { + t.Errorf("%s: status=%d body=%q, want the unknown-node 404", name, res.StatusCode, body) + } + } + for name, pk := range map[string]string{"visible node": f.n, "visible observer-only": f.o1} { + if res, _, body := f.get(t, pk); res.StatusCode != http.StatusOK { + t.Errorf("%s: status=%d body=%s, want 200", name, res.StatusCode, body) + } + } +} + +// A visible node's Reach page never lists a hidden or blacklisted identity — +// not in links, not in direct_observers, not by name or pubkey anywhere in the +// body — and the counts derived from those lists match what is shown. +func TestNodeReach_HiddenNeighboursFiltered(t *testing.T) { + f := newReachVisibilityDB(t) + res, resp, body := f.get(t, f.n) + if res.StatusCode != http.StatusOK { + t.Fatalf("status=%d body=%s", res.StatusCode, body) + } + watcher := pk64("0b05") + if got, want := linkSet(resp), strings.Join(sortedPrefixes(f.visible, f.o1, watcher), ","); got != want { + t.Fatalf("links=%s want %s", got, want) + } + if got, want := obsSet(resp), strings.Join(sortedPrefixes(f.o1, watcher), ","); got != want { + t.Fatalf("direct_observers=%s want %s", got, want) + } + if resp.Importance.BidirectionalLinks != 1 || resp.Importance.DirectObservers != 2 { + t.Fatalf("counts=%+v want 1 two-way (Echo; Bravo removed) and 2 direct observers", resp.Importance) + } + for _, leak := range []string{f.hidName, f.blNode, f.blObs, f.hidObsName, f.o2Hidden, f.o3Blacklisted, + "private A", "Bravo", "Charlie", "Delta", "hidden obs", "Obs three", "obs D"} { + if strings.Contains(strings.ToLower(body), strings.ToLower(leak)) { + t.Errorf("body leaks %q", leak) + } + } +} + +func sortedPrefixes(pks ...string) []string { + out := make([]string, len(pks)) + for i, pk := range pks { + out[i] = pk[:4] + } + sort.Strings(out) + return out +} + +// A warm cache never outlives a visibility change: renames (node or observer +// row), hidden-prefix config and blacklist changes apply on the very next +// request, in both directions, for neighbours and for the target itself. +func TestNodeReach_WarmCacheHonoursVisibilityChanges(t *testing.T) { + f := newReachVisibilityDB(t) + exec := func(q string, args ...interface{}) { + t.Helper() + if _, err := f.db.conn.Exec(q, args...); err != nil { + t.Fatal(err) + } + } + has := func(resp NodeReachResponse, pk string) bool { + return strings.Contains(","+linkSet(resp)+",", ","+pk[:4]+",") + } + _, warm, _ := f.get(t, f.n) + if !has(warm, f.visible) || f.srv.reachCacheLen() == 0 { + t.Fatalf("warm-up: links=%s cache=%d", linkSet(warm), f.srv.reachCacheLen()) + } + + // Neighbour renamed into a hidden prefix → gone on the next (cached) request. + exec(`UPDATE nodes SET name = '🚫 Echo' WHERE public_key = ?`, f.visible) + _, resp, body := f.get(t, f.n) + if has(resp, f.visible) || strings.Contains(body, f.visible) || resp.Importance.BidirectionalLinks != 0 { + t.Fatalf("renamed-hidden neighbour still served: links=%s two-way=%d", linkSet(resp), resp.Importance.BidirectionalLinks) + } + if res, _, _ := f.get(t, f.visible); res.StatusCode != http.StatusNotFound { + t.Fatalf("renamed-hidden node's own reach: %d", res.StatusCode) + } + // ...and back. + exec(`UPDATE nodes SET name = 'Echo' WHERE public_key = ?`, f.visible) + if _, resp, _ := f.get(t, f.n); !has(resp, f.visible) { + t.Fatalf("un-hidden neighbour not restored: %s", linkSet(resp)) + } + + // Observer row renamed into a hidden prefix. + exec(`UPDATE observers SET name = '🚫 now hidden' WHERE id = ?`, strings.ToUpper(f.o1)) + if _, resp, _ := f.get(t, f.n); has(resp, f.o1) || strings.Contains(obsSet(resp), f.o1[:4]) { + t.Fatalf("renamed-hidden observer still served: links=%s obs=%s", linkSet(resp), obsSet(resp)) + } + exec(`UPDATE observers SET name = 'Obs one' WHERE id = ?`, strings.ToUpper(f.o1)) + + // Hidden-prefix config change (does not purge the cache). + f.cfg.SetHiddenNamePrefixes([]string{"🚫", "Obs one"}) + if _, resp, _ := f.get(t, f.n); has(resp, f.o1) { + t.Fatalf("prefix change ignored by the cache: %s", linkSet(resp)) + } + f.cfg.SetHiddenNamePrefixes([]string{"🚫"}) + if _, resp, _ := f.get(t, f.n); !has(resp, f.o1) { + t.Fatalf("prefix removal ignored: %s", linkSet(resp)) + } + + // Blacklist change. + f.cfg.SetNodeBlacklist([]string{f.blNode, f.visible}) + if _, resp, _ := f.get(t, f.n); has(resp, f.visible) { + t.Fatalf("blacklist change ignored: %s", linkSet(resp)) + } + + // The target itself hidden after its report was cached. + _, _, _ = f.get(t, f.n) + exec(`UPDATE nodes SET name = '🚫 Target' WHERE public_key = ?`, f.n) + if res, _, _ := f.get(t, f.n); res.StatusCode != http.StatusNotFound { + t.Fatalf("target renamed hidden after warm-up: %d, want 404", res.StatusCode) + } +} + +// A failed live-name lookup never serves unfiltered data from the cache. +func TestNodeReach_NameLookupFailureFailsClosed(t *testing.T) { + f := newReachVisibilityDB(t) + if res, _, _ := f.get(t, f.n); res.StatusCode != http.StatusOK { + t.Fatalf("warm-up: %d", res.StatusCode) + } + if _, err := f.db.conn.Exec(`ALTER TABLE observers RENAME TO observers_gone`); err != nil { + t.Fatal(err) + } + res, _, body := f.get(t, f.n) + if res.StatusCode != http.StatusInternalServerError || strings.Contains(body, "links") { + t.Fatalf("status=%d body=%s, want 500 without report data", res.StatusCode, body) + } +} + +// With nothing hidden the body is exactly the computed report, and without +// configured prefixes no name lookup touches the DB. +func TestNodeReach_NothingHiddenBodyUnchanged(t *testing.T) { + f := newReachVisibilityDB(t) + f.cfg.SetNodeBlacklist(nil) + f.cfg.ObserverBlacklist = nil + f.cfg.SetHiddenNamePrefixes(nil) + resetReachState(t, f.srv) + + res, _, body := f.get(t, f.n) + if res.StatusCode != http.StatusOK { + t.Fatalf("status=%d", res.StatusCode) + } + resp, ok, err := f.srv.computeNodeReach(context.Background(), f.n, 30) + if err != nil || !ok { + t.Fatalf("compute: ok=%v err=%v", ok, err) + } + var got NodeReachResponse + if err := json.Unmarshal([]byte(body), &got); err != nil { + t.Fatal(err) + } + if len(got.Links) != len(resp.Links) || len(got.DirectObservers) != len(resp.DirectObservers) || + got.Importance != resp.Importance { + t.Fatalf("unfiltered report changed: got %d links/%d obs %+v, want %d/%d %+v", + len(got.Links), len(got.DirectObservers), got.Importance, len(resp.Links), len(resp.DirectObservers), resp.Importance) + } + if names, err := (&Server{cfg: &Config{}}).identityNames([]string{f.n}); names != nil || err != nil { + t.Fatalf("identityNames without prefixes = %v, %v; want nil, nil and no DB access", names, err) + } +} + +// identityNames handles a large batch (a hub's full link list) in one query +// and maps upper-case observer ids back to lower-case pubkeys. +func TestIdentityNames_LargeBatch(t *testing.T) { + f := newReachVisibilityDB(t) + pks := make([]string, 0, 1010) + for i := 0; i < 1000; i++ { + pks = append(pks, pk64(fmt.Sprintf("f%05x", i))) + } + pks = append(pks, f.o2Hidden, strings.ToUpper(f.hidName), f.hidObsName) + names, err := f.srv.identityNames(pks) + if err != nil { + t.Fatal(err) + } + if fmt.Sprint(names[f.o2Hidden]) != "[🚫 hidden obs]" || fmt.Sprint(names[f.hidName]) != "[🚫 private A]" { + t.Fatalf("names=%v", names) + } + d := names[f.hidObsName] + sort.Strings(d) + if fmt.Sprint(d) != "[Delta 🚫 obs D]" { + t.Fatalf("node + observer names for D = %v", d) + } +} diff --git a/docs/api-spec.md b/docs/api-spec.md index e964efe18..84d79553f 100644 --- a/docs/api-spec.md +++ b/docs/api-spec.md @@ -722,11 +722,24 @@ to a recent window. Identifies nodes only by **unique 2–3 byte** path prefixes `reliable_tokens: []` means the node has no unique 1–3 byte prefix and cannot be reliably identified in paths; `links`/`direct_observers` will be empty. +### Visibility + +An identity is **hidden** when its pubkey is in `nodeBlacklist` or +`observerBlacklist`, or when its node name **or** observer name starts with a +`hiddenNamePrefixes` entry. A hidden target returns `404` (same body as an +unknown node). Hidden identities are omitted from `links` and +`direct_observers`, and `bidirectional_links` / `direct_observers` count only +what is listed. Names are read live on every request — including cached +reports — so a blacklist, prefix or rename change applies on the next request. +`neighbor_degree`, `degree_rank` and `nodes_with_edges` are counts over the +whole neighbour graph and are not changed by this filtering. + ### Caching & limits - **Response cache:** computed responses are cached for **5 minutes** per - `pubkey|days`. Polling faster than that returns an identical body — clients - should not expect sub-5-minute freshness. + `pubkey|days`. Polling faster than that returns the same report — clients + should not expect sub-5-minute freshness. Visibility (above) is applied on + every request, cached or not. - **Scan cap:** the windowed path scan is hard-capped at **200,000** rows. A node with more matching observations in the window is truncated (counts become a representative sample rather than exhaustive). @@ -741,12 +754,21 @@ Returned when `:pubkey` is not a 64-char hex string. ### Response `404` -Returned when the node is unknown or blacklisted. +Returned when the node is unknown or hidden (see Visibility). ```json { "error": "Not found" } ``` +### Response `500` + +Returned when the scan fails, or when the live name lookup for visibility +fails — the endpoint fails closed rather than serving unfiltered data. + +```json +{ "error": "reach computation failed" } +``` + --- ## GET /api/packets From 713e1cfd0ca5b81b20bb15fa802cdef91e92ad07 Mon Sep 17 00:00:00 2001 From: Dennis Jakobsen Date: Fri, 18 Sep 2026 16:56:57 +0200 Subject: [PATCH 2/8] fix(reach): cover inactive nodes and mixed-case observer ids in visibility Independent review follow-ups: - A node hidden by name that aged out of `nodes` into inactive_nodes still appeared in links by pubkey (its adverts stay in a 14/30-day window). The live name lookup now includes inactive_nodes (probed, so minimal DBs without the table keep working). - Observer ids arrive raw from the MQTT topic; match them case-insensitively (one pass over the observers table) instead of only exact upper/lower case. - Gate the lookup on HasHiddenNamePrefixes, which uses exactly IsNameHidden's predicate (a whitespace-only prefix counts), instead of ActiveHiddenNamePrefixes. - Docs: hiding applies on the next request; un-hiding by rename can take up to the cache TTL (the recorded name still counts). - Tests for each case plus the recorded-name guard; the inactive, mixed-case and recorded-name tests fail with the respective check removed. Co-Authored-By: Claude Opus 5 --- cmd/server/config.go | 29 +++++++++ cmd/server/identity_visibility.go | 60 +++++++++++++---- cmd/server/node_reach.go | 4 ++ cmd/server/node_reach_visibility_test.go | 83 ++++++++++++++++++++++-- docs/api-spec.md | 15 +++-- 5 files changed, 167 insertions(+), 24 deletions(-) diff --git a/cmd/server/config.go b/cmd/server/config.go index 40b463623..ca91385ec 100644 --- a/cmd/server/config.go +++ b/cmd/server/config.go @@ -957,6 +957,35 @@ func (c *Config) IsNameHidden(name string) bool { return false } +// HasHiddenNamePrefixes reports whether IsNameHidden can return true for some +// name — i.e. at least one non-empty prefix is active. It uses exactly +// IsNameHidden's predicate (a whitespace-only prefix counts), so callers can +// skip name lookups safely when it is false. +func (c *Config) HasHiddenNamePrefixes() bool { + if c == nil { + return false + } + pp := c.hiddenPrefixesPtr.Load() + if pp == nil { + built := make([]string, len(c.HiddenNamePrefixes)) + copy(built, c.HiddenNamePrefixes) + if c.hiddenPrefixesPtr.CompareAndSwap(nil, &built) { + pp = &built + } else { + pp = c.hiddenPrefixesPtr.Load() + } + } + if pp == nil { + return false + } + for _, p := range *pp { + if p != "" { + return true + } + } + return false +} + // ActiveHiddenNamePrefixes returns a copy of the hide prefixes IsNameHidden // is actually enforcing right now, with empty/whitespace entries dropped — // the same entries IsNameHidden skips. Reads through the same atomic pointer diff --git a/cmd/server/identity_visibility.go b/cmd/server/identity_visibility.go index 765a481b4..89d4aacaf 100644 --- a/cmd/server/identity_visibility.go +++ b/cmd/server/identity_visibility.go @@ -26,24 +26,31 @@ func identityHidden(cfg *Config, pubkey string, names ...string) bool { return false } -// identityNamesSQL looks up the node and observer names of a JSON array of -// lower-case pubkeys in one statement with fixed text (cheap to prepare, -// unlike a variable-length IN list). Observer ids are stored upper-case, -// node keys lower-case; both sides use their primary-key index. +// identityNamesSQL looks up every name a JSON array of lower-case pubkeys is +// known by, in one statement with fixed text (cheap to prepare, unlike a +// variable-length IN list): +// - nodes and inactive_nodes: keys are lower-case (the ingestor normalises +// them), so the primary-key index is used per pubkey. inactive_nodes holds +// nodes aged out of `nodes`, whose adverts can still sit in a Reach window. +// - observers: ids arrive raw from the MQTT topic in any case, so they are +// matched case-insensitively in one pass over the (small) table. const identityNamesSQL = ` SELECT n.public_key, COALESCE(n.name, '') FROM json_each(?1) j JOIN nodes n ON n.public_key = j.value UNION ALL - SELECT o.id, COALESCE(o.name, '') FROM json_each(?1) j JOIN observers o ON o.id = upper(j.value) + SELECT o.id, COALESCE(o.name, '') FROM observers o WHERE lower(o.id) IN (SELECT value FROM json_each(?1))` + +const identityNamesInactiveSQL = identityNamesSQL + ` UNION ALL - SELECT o.id, COALESCE(o.name, '') FROM json_each(?1) j JOIN observers o ON o.id = j.value AND o.id <> upper(j.value)` + SELECT i.public_key, COALESCE(i.name, '') FROM json_each(?1) j JOIN inactive_nodes i ON i.public_key = j.value` -// identityNames returns the current node and observer names of the given -// pubkeys, read live from the DB in one bulk query (no per-pubkey lookups), -// keyed by lowercase pubkey. A rename into a hidden prefix therefore takes -// effect on the next request, cached response or not. Returns (nil, nil) when -// no hidden-name prefix is configured: names cannot hide anything then. +// identityNames returns the current node, inactive-node and observer names of +// the given pubkeys, read live from the DB in one bulk query (no per-pubkey +// lookups), keyed by lowercase pubkey. A rename into a hidden prefix +// therefore takes effect on the next request, cached response or not. +// Returns (nil, nil) when no hidden-name prefix is configured: names cannot +// hide anything then. func (s *Server) identityNames(pubkeys []string) (map[string][]string, error) { - if s.cfg == nil || len(s.cfg.ActiveHiddenNamePrefixes()) == 0 || len(pubkeys) == 0 { + if s.cfg == nil || !s.cfg.HasHiddenNamePrefixes() || len(pubkeys) == 0 { return nil, nil } if s.db == nil || s.db.conn == nil { @@ -69,12 +76,39 @@ func (s *Server) identityNames(pubkeys []string) (map[string][]string, error) { out[pk] = append(out[pk], name) } } - if err := s.scanIdentityNames(identityNamesSQL, []interface{}{string(arg)}, add); err != nil { + q := identityNamesSQL + hasInactive, err := s.hasInactiveNodesTable() + if err != nil { + return nil, fmt.Errorf("identity names: %w", err) + } + if hasInactive { + q = identityNamesInactiveSQL + } + if err := s.scanIdentityNames(q, []interface{}{string(arg)}, add); err != nil { return nil, fmt.Errorf("identity names: %w", err) } return out, nil } +// hasInactiveNodesTable reports whether inactive_nodes exists. Production +// schemas always have it (dbschema.AssertReady); minimal DBs may not. A +// positive answer is cached — tables are never dropped at runtime — while a +// negative one is re-probed so a table created later is picked up. +func (s *Server) hasInactiveNodesTable() (bool, error) { + if s.reach.inactiveNodesTable.Load() { + return true, nil + } + var n int + err := s.db.conn.QueryRow(`SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'inactive_nodes'`).Scan(&n) + if err != nil { + return false, err + } + if n > 0 { + s.reach.inactiveNodesTable.Store(true) + } + return n > 0, nil +} + func (s *Server) scanIdentityNames(q string, args []interface{}, add func(pk, name string)) error { rows, err := s.db.conn.Query(q, args...) if err != nil { diff --git a/cmd/server/node_reach.go b/cmd/server/node_reach.go index 6e0f99338..51f9d1e59 100644 --- a/cmd/server/node_reach.go +++ b/cmd/server/node_reach.go @@ -302,6 +302,10 @@ type reachState struct { degreeMu sync.Mutex degreeSnap *degreeSnapshot + + // inactiveNodesTable caches a positive inactive_nodes probe for the + // visibility name lookup (identity_visibility.go). + inactiveNodesTable atomic.Bool } // reachCacheGet returns the cached entry for key. Its raw slice and resp diff --git a/cmd/server/node_reach_visibility_test.go b/cmd/server/node_reach_visibility_test.go index 9940a0155..e621937cb 100644 --- a/cmd/server/node_reach_visibility_test.go +++ b/cmd/server/node_reach_visibility_test.go @@ -26,6 +26,8 @@ type visFixture struct { n, hidName, blNode, blObs, hidObsName, visible string // observer-only pubkeys (lower-case; stored upper-case in observers.id) o1, o2Hidden, o3Blacklisted string + // hidden identities outside the plain nodes/observers tables + inactiveHidden, mixedCaseObs string } // newReachVisibilityDB builds a Reach DB where target N has, in 30 days: @@ -38,12 +40,15 @@ type visFixture struct { // O1 "Obs one" — observer-only, visible (direct observer) // O2 "🚫 hidden obs" — observer-only, hidden name (direct observer) // O3 "Obs three" — observer-only, observer-blacklisted (direct observer) +// G "🚫 gone quiet" — only in inactive_nodes, hidden name (advert heard by N) +// M "Mixed case" — observer-only, visible, id stored in mixed case (direct observer) func newReachVisibilityDB(t *testing.T) *visFixture { t.Helper() f := &visFixture{ n: pk64("01fa"), hidName: pk64("aabb"), blNode: pk64("ccdd"), blObs: pk64("ee11"), hidObsName: pk64("dd22"), visible: pk64("e5e5"), o1: pk64("0b01"), o2Hidden: pk64("0b02"), o3Blacklisted: pk64("0b03"), + inactiveHidden: pk64("7777"), mixedCaseObs: pk64("0b0abc"), } conn, err := sql.Open("sqlite", ":memory:") if err != nil { @@ -59,6 +64,7 @@ func newReachVisibilityDB(t *testing.T) *visFixture { `CREATE TABLE transmissions (id INTEGER PRIMARY KEY, from_pubkey TEXT, payload_type INTEGER)`, `CREATE TABLE observations (id INTEGER PRIMARY KEY, transmission_id INTEGER, observer_idx INTEGER, snr REAL, path_json TEXT, timestamp INTEGER)`, `CREATE TABLE neighbor_edges (node_a TEXT NOT NULL, node_b TEXT NOT NULL, count INTEGER DEFAULT 1, last_seen TEXT, PRIMARY KEY (node_a, node_b))`, + `CREATE TABLE inactive_nodes (public_key TEXT PRIMARY KEY, name TEXT, role TEXT, lat REAL, lon REAL, last_seen TEXT, first_seen TEXT, advert_count INTEGER DEFAULT 0)`, } for _, s := range stmts { if _, err := conn.Exec(s); err != nil { @@ -77,6 +83,7 @@ func newReachVisibilityDB(t *testing.T) *visFixture { for _, o := range []struct{ id, name string }{ {up(f.o1), "Obs one"}, {up(f.o2Hidden), "🚫 hidden obs"}, {up(f.o3Blacklisted), "Obs three"}, {up(f.hidObsName), "🚫 obs D"}, {up(pk64("0b05")), "Relay watcher"}, + {"0B0aBC" + f.mixedCaseObs[6:], "Mixed case"}, // rowid 6, raw mixed-case id } { if _, err := conn.Exec(`INSERT INTO observers (id, name) VALUES (?, ?)`, o.id, o.name); err != nil { t.Fatal(err) @@ -93,6 +100,7 @@ func newReachVisibilityDB(t *testing.T) *visFixture { {`["01FA"]`, 1}, // O1 heard us directly {`["01FA"]`, 2}, // O2 heard us directly {`["01FA"]`, 3}, // O3 heard us directly + {`["01FA"]`, 6}, // M (mixed-case id) heard us directly } for i, p := range paths { if _, err := conn.Exec(`INSERT INTO transmissions (id, from_pubkey, payload_type) VALUES (?, '', 5)`, i+1); err != nil { @@ -102,6 +110,17 @@ func newReachVisibilityDB(t *testing.T) *visFixture { t.Fatal(err) } } + // G aged out of `nodes` into inactive_nodes; its advert, relayed first by + // N, is still inside the 30-day window ("we hear G"). + if _, err := conn.Exec(`INSERT INTO inactive_nodes (public_key, name, role) VALUES (?, '🚫 gone quiet', 'repeater')`, f.inactiveHidden); err != nil { + t.Fatal(err) + } + if _, err := conn.Exec(`INSERT INTO transmissions (id, from_pubkey, payload_type) VALUES (100, ?, ?)`, f.inactiveHidden, PayloadADVERT); err != nil { + t.Fatal(err) + } + if _, err := conn.Exec(`INSERT INTO observations (id, transmission_id, observer_idx, snr, path_json, timestamp) VALUES (100, 100, 5, -7.0, '["01FA"]', ?)`, now-10*86400); err != nil { + t.Fatal(err) + } f.db = &DB{conn: conn} f.db.isV3Flag.forceTrue() f.cfg = &Config{ @@ -184,17 +203,18 @@ func TestNodeReach_HiddenNeighboursFiltered(t *testing.T) { t.Fatalf("status=%d body=%s", res.StatusCode, body) } watcher := pk64("0b05") - if got, want := linkSet(resp), strings.Join(sortedPrefixes(f.visible, f.o1, watcher), ","); got != want { + if got, want := linkSet(resp), strings.Join(sortedPrefixes(f.visible, f.o1, watcher, f.mixedCaseObs), ","); got != want { t.Fatalf("links=%s want %s", got, want) } - if got, want := obsSet(resp), strings.Join(sortedPrefixes(f.o1, watcher), ","); got != want { + if got, want := obsSet(resp), strings.Join(sortedPrefixes(f.o1, watcher, f.mixedCaseObs), ","); got != want { t.Fatalf("direct_observers=%s want %s", got, want) } - if resp.Importance.BidirectionalLinks != 1 || resp.Importance.DirectObservers != 2 { - t.Fatalf("counts=%+v want 1 two-way (Echo; Bravo removed) and 2 direct observers", resp.Importance) + if resp.Importance.BidirectionalLinks != 1 || resp.Importance.DirectObservers != 3 { + t.Fatalf("counts=%+v want 1 two-way (Echo; Bravo removed) and 3 direct observers", resp.Importance) } for _, leak := range []string{f.hidName, f.blNode, f.blObs, f.hidObsName, f.o2Hidden, f.o3Blacklisted, - "private A", "Bravo", "Charlie", "Delta", "hidden obs", "Obs three", "obs D"} { + f.inactiveHidden, "private A", "Bravo", "Charlie", "Delta", "hidden obs", "Obs three", + "obs D", "gone quiet"} { if strings.Contains(strings.ToLower(body), strings.ToLower(leak)) { t.Errorf("body leaks %q", leak) } @@ -251,6 +271,15 @@ func TestNodeReach_WarmCacheHonoursVisibilityChanges(t *testing.T) { } exec(`UPDATE observers SET name = 'Obs one' WHERE id = ?`, strings.ToUpper(f.o1)) + // Observer whose id is stored in mixed case, renamed into a hidden prefix. + exec(`UPDATE observers SET name = '🚫 mixed now hidden' WHERE lower(id) = ?`, f.mixedCaseObs) + if _, resp, _ := f.get(t, f.n); has(resp, f.mixedCaseObs) { + t.Fatalf("renamed-hidden mixed-case observer still served: %s", linkSet(resp)) + } + if res, _, _ := f.get(t, f.mixedCaseObs); res.StatusCode != http.StatusNotFound { + t.Fatalf("renamed-hidden mixed-case observer's own reach: %d", res.StatusCode) + } + // Hidden-prefix config change (does not purge the cache). f.cfg.SetHiddenNamePrefixes([]string{"🚫", "Obs one"}) if _, resp, _ := f.get(t, f.n); has(resp, f.o1) { @@ -343,3 +372,47 @@ func TestIdentityNames_LargeBatch(t *testing.T) { t.Fatalf("node + observer names for D = %v", d) } } + +// A neighbour hidden when the report was computed stays hidden after its row +// disappears from the DB: the name recorded in the cached report still counts. +func TestNodeReach_RecordedNameStillHidesAfterRowDeleted(t *testing.T) { + f := newReachVisibilityDB(t) + if _, resp, _ := f.get(t, f.n); strings.Contains(linkSet(resp), f.hidName[:4]) { + t.Fatalf("warm-up already leaks %s", f.hidName[:4]) + } + if _, err := f.db.conn.Exec(`DELETE FROM nodes WHERE public_key = ?`, f.hidName); err != nil { + t.Fatal(err) + } + if _, resp, body := f.get(t, f.n); strings.Contains(linkSet(resp), f.hidName[:4]) || strings.Contains(body, "private A") { + t.Fatalf("deleted hidden neighbour served from the cache: %s", linkSet(resp)) + } +} + +// A whitespace-only prefix is honoured by IsNameHidden, so it must also turn +// on the live name lookup. +func TestNodeReach_WhitespacePrefixStillLooksUpNames(t *testing.T) { + f := newReachVisibilityDB(t) + f.get(t, f.n) // warm + f.cfg.SetHiddenNamePrefixes([]string{" "}) + if _, err := f.db.conn.Exec(`UPDATE nodes SET name = ' sneaky' WHERE public_key = ?`, f.visible); err != nil { + t.Fatal(err) + } + if _, resp, _ := f.get(t, f.n); strings.Contains(linkSet(resp), f.visible[:4]) { + t.Fatalf("whitespace-prefixed rename not hidden: %s", linkSet(resp)) + } + if !f.cfg.HasHiddenNamePrefixes() || (&Config{HiddenNamePrefixes: []string{""}}).HasHiddenNamePrefixes() { + t.Fatalf("HasHiddenNamePrefixes must mirror IsNameHidden: only empty prefixes are inert") + } +} + +// Minimal DBs without inactive_nodes still work (the table is probed). +func TestNodeReach_NoInactiveNodesTable(t *testing.T) { + f := newReachVisibilityDB(t) + if _, err := f.db.conn.Exec(`DROP TABLE inactive_nodes`); err != nil { + t.Fatal(err) + } + res, resp, body := f.get(t, f.n) + if res.StatusCode != http.StatusOK || strings.Contains(body, "private A") || !strings.Contains(linkSet(resp), f.visible[:4]) { + t.Fatalf("status=%d links=%s", res.StatusCode, linkSet(resp)) + } +} diff --git a/docs/api-spec.md b/docs/api-spec.md index 84d79553f..0e68a3772 100644 --- a/docs/api-spec.md +++ b/docs/api-spec.md @@ -725,12 +725,15 @@ reliably identified in paths; `links`/`direct_observers` will be empty. ### Visibility An identity is **hidden** when its pubkey is in `nodeBlacklist` or -`observerBlacklist`, or when its node name **or** observer name starts with a -`hiddenNamePrefixes` entry. A hidden target returns `404` (same body as an -unknown node). Hidden identities are omitted from `links` and -`direct_observers`, and `bidirectional_links` / `direct_observers` count only -what is listed. Names are read live on every request — including cached -reports — so a blacklist, prefix or rename change applies on the next request. +`observerBlacklist`, or when any of its names — node, inactive node or +observer — starts with a `hiddenNamePrefixes` entry. A hidden target returns +`404` (same body as an unknown node). Hidden identities are omitted from +`links` and `direct_observers`, and `bidirectional_links` / `direct_observers` +count only what is listed. Names are read live on every request — including +cached reports — so **hiding** (a blacklist or prefix change, or a rename into +a hidden prefix) applies on the next request. Un-hiding a neighbour by +renaming it can take up to the 5-minute cache TTL, because the name recorded +when the report was computed still counts. `neighbor_degree`, `degree_rank` and `nodes_with_edges` are counts over the whole neighbour graph and are not changed by this filtering. From 783ca7593db8a51e3fe4ab2f7d35578e6cf70c88 Mon Sep 17 00:00:00 2001 From: Dennis Jakobsen Date: Fri, 18 Sep 2026 17:02:49 +0200 Subject: [PATCH 3/8] fix(reach): inactive_nodes names count only while the node is inactive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit inactive_nodes keeps a node's old row when it returns to `nodes` (INSERT OR REPLACE on move, never deleted), so a node that went quiet as "🚫 …" and came back under a visible name stayed hidden. Consult an inactive name only while the pubkey has no nodes row. Test covers the returned node; the inactive-only case stays covered by fixture node G. Co-Authored-By: Claude Opus 5 --- cmd/server/identity_visibility.go | 5 ++++- cmd/server/node_reach_visibility_test.go | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/cmd/server/identity_visibility.go b/cmd/server/identity_visibility.go index 89d4aacaf..746ca2349 100644 --- a/cmd/server/identity_visibility.go +++ b/cmd/server/identity_visibility.go @@ -32,6 +32,8 @@ func identityHidden(cfg *Config, pubkey string, names ...string) bool { // - nodes and inactive_nodes: keys are lower-case (the ingestor normalises // them), so the primary-key index is used per pubkey. inactive_nodes holds // nodes aged out of `nodes`, whose adverts can still sit in a Reach window. +// Its row is never removed when a node returns, so an inactive name only +// counts while the pubkey has no nodes row (otherwise it is stale). // - observers: ids arrive raw from the MQTT topic in any case, so they are // matched case-insensitively in one pass over the (small) table. const identityNamesSQL = ` @@ -41,7 +43,8 @@ const identityNamesSQL = ` const identityNamesInactiveSQL = identityNamesSQL + ` UNION ALL - SELECT i.public_key, COALESCE(i.name, '') FROM json_each(?1) j JOIN inactive_nodes i ON i.public_key = j.value` + SELECT i.public_key, COALESCE(i.name, '') FROM json_each(?1) j JOIN inactive_nodes i ON i.public_key = j.value + WHERE NOT EXISTS (SELECT 1 FROM nodes n WHERE n.public_key = j.value)` // identityNames returns the current node, inactive-node and observer names of // the given pubkeys, read live from the DB in one bulk query (no per-pubkey diff --git a/cmd/server/node_reach_visibility_test.go b/cmd/server/node_reach_visibility_test.go index e621937cb..f97e52327 100644 --- a/cmd/server/node_reach_visibility_test.go +++ b/cmd/server/node_reach_visibility_test.go @@ -416,3 +416,21 @@ func TestNodeReach_NoInactiveNodesTable(t *testing.T) { t.Fatalf("status=%d links=%s", res.StatusCode, linkSet(resp)) } } + +// inactive_nodes keeps a returning node's old row. A node that went quiet as +// "🚫 …" and came back under a visible name is visible again: an inactive +// name only counts while the pubkey has no nodes row. +func TestNodeReach_StaleInactiveNameDoesNotHideReturnedNode(t *testing.T) { + f := newReachVisibilityDB(t) + if _, err := f.db.conn.Exec(`INSERT INTO inactive_nodes (public_key, name, role) VALUES (?, '🚫 old name', 'repeater')`, f.visible); err != nil { + t.Fatal(err) + } + if res, _, _ := f.get(t, f.visible); res.StatusCode != http.StatusOK { + t.Fatalf("returned node's own reach: %d, want 200", res.StatusCode) + } + if _, resp, _ := f.get(t, f.n); !strings.Contains(linkSet(resp), f.visible[:4]) { + t.Fatalf("returned node hidden from links by a stale inactive name: %s", linkSet(resp)) + } + // (The inactive-only case — no nodes row, hidden inactive name — is + // fixture node G, checked in TestNodeReach_HiddenNeighboursFiltered.) +} From 2c15adb21ee74253cb149a65e57aa2a57677a61d Mon Sep 17 00:00:00 2001 From: Dennis Jakobsen Date: Fri, 18 Sep 2026 17:06:35 +0200 Subject: [PATCH 4/8] fix(reach): a nameless return does not un-hide an inactive hidden name A hidden node that came back with a nameless advert got a nodes row with name '', which switched its hidden inactive_nodes name off and listed its pubkey again. An inactive name now counts unless the pubkey has a nodes row with a non-empty name. Docs state the rule. Co-Authored-By: Claude Opus 5 --- cmd/server/identity_visibility.go | 5 +++-- cmd/server/node_reach_visibility_test.go | 8 ++++++++ docs/api-spec.md | 5 +++-- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/cmd/server/identity_visibility.go b/cmd/server/identity_visibility.go index 746ca2349..f9e33f6c4 100644 --- a/cmd/server/identity_visibility.go +++ b/cmd/server/identity_visibility.go @@ -33,7 +33,8 @@ func identityHidden(cfg *Config, pubkey string, names ...string) bool { // them), so the primary-key index is used per pubkey. inactive_nodes holds // nodes aged out of `nodes`, whose adverts can still sit in a Reach window. // Its row is never removed when a node returns, so an inactive name only -// counts while the pubkey has no nodes row (otherwise it is stale). +// counts while the pubkey has no nodes row with a name — a current name +// supersedes it; a nameless return (stored as an empty name) does not. // - observers: ids arrive raw from the MQTT topic in any case, so they are // matched case-insensitively in one pass over the (small) table. const identityNamesSQL = ` @@ -44,7 +45,7 @@ const identityNamesSQL = ` const identityNamesInactiveSQL = identityNamesSQL + ` UNION ALL SELECT i.public_key, COALESCE(i.name, '') FROM json_each(?1) j JOIN inactive_nodes i ON i.public_key = j.value - WHERE NOT EXISTS (SELECT 1 FROM nodes n WHERE n.public_key = j.value)` + WHERE NOT EXISTS (SELECT 1 FROM nodes n WHERE n.public_key = j.value AND COALESCE(n.name, '') <> '')` // identityNames returns the current node, inactive-node and observer names of // the given pubkeys, read live from the DB in one bulk query (no per-pubkey diff --git a/cmd/server/node_reach_visibility_test.go b/cmd/server/node_reach_visibility_test.go index f97e52327..7dcf10ff0 100644 --- a/cmd/server/node_reach_visibility_test.go +++ b/cmd/server/node_reach_visibility_test.go @@ -431,6 +431,14 @@ func TestNodeReach_StaleInactiveNameDoesNotHideReturnedNode(t *testing.T) { if _, resp, _ := f.get(t, f.n); !strings.Contains(linkSet(resp), f.visible[:4]) { t.Fatalf("returned node hidden from links by a stale inactive name: %s", linkSet(resp)) } + // A nameless return (the ingestor stores '') does not supersede the old + // hidden name: the node stays hidden. + if _, err := f.db.conn.Exec(`UPDATE nodes SET name = '' WHERE public_key = ?`, f.visible); err != nil { + t.Fatal(err) + } + if _, resp, _ := f.get(t, f.n); strings.Contains(linkSet(resp), f.visible[:4]) { + t.Fatalf("nameless return un-hid a hidden inactive name: %s", linkSet(resp)) + } // (The inactive-only case — no nodes row, hidden inactive name — is // fixture node G, checked in TestNodeReach_HiddenNeighboursFiltered.) } diff --git a/docs/api-spec.md b/docs/api-spec.md index 0e68a3772..cbab2dae6 100644 --- a/docs/api-spec.md +++ b/docs/api-spec.md @@ -725,8 +725,9 @@ reliably identified in paths; `links`/`direct_observers` will be empty. ### Visibility An identity is **hidden** when its pubkey is in `nodeBlacklist` or -`observerBlacklist`, or when any of its names — node, inactive node or -observer — starts with a `hiddenNamePrefixes` entry. A hidden target returns +`observerBlacklist`, or when any of its names — node, observer, or its +`inactive_nodes` name while it has no named `nodes` row — starts with a +`hiddenNamePrefixes` entry. A hidden target returns `404` (same body as an unknown node). Hidden identities are omitted from `links` and `direct_observers`, and `bidirectional_links` / `direct_observers` count only what is listed. Names are read live on every request — including From 25986c9c2fc999e13bcd727fc7c156aa65691c48 Mon Sep 17 00:00:00 2001 From: Dennis Jakobsen Date: Sat, 19 Sep 2026 06:26:59 +0200 Subject: [PATCH 5/8] test(reach): over-filtering controls, explicit cache hit/miss/invalidation - Visible controls stay listed with their own Reach page: a name that merely contains the hidden prefix, and a node visible under both its node and observer name. - Cache: a hit reuses the computed report; a hidden-prefix change is applied to that same cached report at serve time (no recompute, no bypass); a blacklist change purges and recomputes. - neighbor_degree / degree_rank / nodes_with_edges are identical with and without hidden identities configured (rank contract untouched). - Move the singleflight comment back above the singleflight call. Co-Authored-By: Claude Opus 5 --- cmd/server/node_reach.go | 9 +- cmd/server/node_reach_visibility_test.go | 104 +++++++++++++++++++++++ 2 files changed, 109 insertions(+), 4 deletions(-) diff --git a/cmd/server/node_reach.go b/cmd/server/node_reach.go index 51f9d1e59..76897815b 100644 --- a/cmd/server/node_reach.go +++ b/cmd/server/node_reach.go @@ -547,10 +547,6 @@ func (s *Server) handleNodeReach(w http.ResponseWriter, r *http.Request) { return } - // singleflight: collapse a thundering herd on a cold key to one scan. The - // shared computation uses the triggering request's context; a disconnect - // there can cancel the in-flight scan for all waiters (acceptable — the - // next request recomputes). // Cache miss: check the target's live names before the expensive scan. // A failed name lookup fails closed. if hidden, err := s.isIdentityHidden(pubkey); err != nil { @@ -561,6 +557,11 @@ func (s *Server) handleNodeReach(w http.ResponseWriter, r *http.Request) { writeError(w, 404, "Not found") return } + + // singleflight: collapse a thundering herd on a cold key to one scan. The + // shared computation uses the triggering request's context; a disconnect + // there can cancel the in-flight scan for all waiters (acceptable — the + // next request recomputes). v, err, _ := s.reach.sf.Do(cacheKey, func() (interface{}, error) { if e, ok := s.reachCacheGet(cacheKey); ok { return e, nil diff --git a/cmd/server/node_reach_visibility_test.go b/cmd/server/node_reach_visibility_test.go index 7dcf10ff0..de4b410d6 100644 --- a/cmd/server/node_reach_visibility_test.go +++ b/cmd/server/node_reach_visibility_test.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" "sort" + "strconv" "strings" "testing" "time" @@ -442,3 +443,106 @@ func TestNodeReach_StaleInactiveNameDoesNotHideReturnedNode(t *testing.T) { // (The inactive-only case — no nodes row, hidden inactive name — is // fixture node G, checked in TestNodeReach_HiddenNeighboursFiltered.) } + +// Over-filtering controls: names that merely contain a hidden prefix, and a +// node visible under both its node and observer name, stay listed and keep +// their own Reach page. +func TestNodeReach_VisibleControlsNotOverfiltered(t *testing.T) { + f := newReachVisibilityDB(t) + midName, dual := pk64("5151"), pk64("6262") + now := time.Now().Unix() + for _, q := range []struct { + sql string + args []interface{} + }{ + {`INSERT INTO nodes (public_key, name, role, lat, lon, last_seen, first_seen, advert_count) VALUES (?, 'Relay 🚫 mid-name', 'repeater', 56.1, 10.2, '2026-09-01T00:00:00Z', '2026-06-01T00:00:00Z', 1)`, []interface{}{midName}}, + {`INSERT INTO nodes (public_key, name, role, lat, lon, last_seen, first_seen, advert_count) VALUES (?, 'Dual', 'repeater', 56.1, 10.2, '2026-09-01T00:00:00Z', '2026-06-01T00:00:00Z', 1)`, []interface{}{dual}}, + {`INSERT INTO observers (id, name) VALUES (?, 'Dual obs')`, []interface{}{strings.ToUpper(dual)}}, + {`INSERT INTO transmissions (id, from_pubkey, payload_type) VALUES (200, '', 5), (201, '', 5)`, nil}, + {`INSERT INTO observations (id, transmission_id, observer_idx, snr, path_json, timestamp) VALUES (200, 200, 5, -7.0, '["5151","01FA"]', ?), (201, 201, 5, -7.0, '["01FA","6262"]', ?)`, []interface{}{now, now}}, + } { + if _, err := f.db.conn.Exec(q.sql, q.args...); err != nil { + t.Fatal(err) + } + } + _, resp, _ := f.get(t, f.n) + for name, pk := range map[string]string{"mid-name 🚫": midName, "dual visible": dual, "visible node": f.visible, "visible observer": f.o1} { + if !strings.Contains(linkSet(resp), pk[:4]) { + t.Errorf("%s (%s) over-filtered: links=%s", name, pk[:4], linkSet(resp)) + } + } + for _, pk := range []string{midName, dual} { + if res, _, body := f.get(t, pk); res.StatusCode != http.StatusOK { + t.Errorf("%s own reach: %d %s", pk[:4], res.StatusCode, body) + } + } +} + +// Cache: a hit reuses the computed report; a hidden-prefix change is applied +// to that same cached report at serve time (no recompute, no bypass); a +// blacklist change purges and recomputes. +func TestNodeReach_CacheHitMissAndInvalidation(t *testing.T) { + f := newReachVisibilityDB(t) + key := func() string { + return f.n + "|30|g" + strconv.FormatUint(f.cfg.BlacklistGeneration(), 10) + } + _, first, body1 := f.get(t, f.n) // miss → computed and cached + e1, ok := f.srv.reachCacheGet(key()) + if !ok || !strings.Contains(linkSet(first), f.visible[:4]) { + t.Fatalf("miss did not cache (ok=%v) or links wrong: %s", ok, linkSet(first)) + } + _, _, body2 := f.get(t, f.n) // hit + if e2, _ := f.srv.reachCacheGet(key()); !e2.at.Equal(e1.at) || body2 != body1 { + t.Fatalf("second request was not a cache hit with the same body") + } + + f.cfg.SetHiddenNamePrefixes([]string{"🚫", "Echo"}) + _, resp, _ := f.get(t, f.n) + e3, ok := f.srv.reachCacheGet(key()) + if !ok || !e3.at.Equal(e1.at) { + t.Fatalf("prefix change must filter the cached report at serve time, not recompute (ok=%v)", ok) + } + if strings.Contains(linkSet(resp), f.visible[:4]) || !strings.Contains(e3.hiddenKey, f.visible) { + t.Fatalf("cached report served unfiltered after prefix change: links=%s hiddenKey=%q", linkSet(resp), e3.hiddenKey) + } + f.cfg.SetHiddenNamePrefixes([]string{"🚫"}) + + genBefore := f.cfg.BlacklistGeneration() + f.cfg.SetNodeBlacklist([]string{f.blNode, f.o1}) + _, resp, _ = f.get(t, f.n) + e4, ok := f.srv.reachCacheGet(key()) + if !ok || f.cfg.BlacklistGeneration() == genBefore || e4.at.Equal(e1.at) || f.srv.reachCacheLen() != 1 { + t.Fatalf("blacklist change must purge and recompute (ok=%v, len=%d)", ok, f.srv.reachCacheLen()) + } + if strings.Contains(linkSet(resp), f.o1[:4]) || strings.Contains(obsSet(resp), f.o1[:4]) { + t.Fatalf("newly blacklisted observer still served: links=%s obs=%s", linkSet(resp), obsSet(resp)) + } +} + +// The filtering does not touch the rank/total contract: neighbour degree, rank +// and total are identical with and without hidden identities configured. +func TestNodeReach_FilteringLeavesRankFieldsUnchanged(t *testing.T) { + f := newReachVisibilityDB(t) + for _, pair := range [][2]string{{f.n, f.hidName}, {f.n, f.visible}, {f.n, f.blNode}, {f.visible, f.hidName}} { + a, b := pair[0], pair[1] + if a > b { + a, b = b, a + } + if _, err := f.db.conn.Exec(`INSERT INTO neighbor_edges (node_a, node_b) VALUES (?, ?)`, a, b); err != nil { + t.Fatal(err) + } + } + _, hiddenCfg, _ := f.get(t, f.n) + plain := &Config{} + srv := &Server{store: newTestStoreWithDB(t, f.db, plain), db: f.db, cfg: plain, perfStats: NewPerfStats()} + resetReachState(t, srv) + rr := serveReach(srv, "/api/nodes/"+f.n+"/reach?days=30") + var noHiding NodeReachResponse + if err := json.Unmarshal(rr.Body.Bytes(), &noHiding); err != nil { + t.Fatal(err) + } + h, p := hiddenCfg.Importance, noHiding.Importance + if h.NeighborDegree != 3 || h.NeighborDegree != p.NeighborDegree || h.DegreeRank != p.DegreeRank || h.NodesWithEdges != p.NodesWithEdges { + t.Fatalf("rank fields changed by filtering: with hiding %+v, without %+v", h, p) + } +} From 9c8352e82acf6a5ee67b7d8337cccf3f75a12260 Mon Sep 17 00:00:00 2001 From: Dennis Jakobsen Date: Sat, 19 Sep 2026 07:04:20 +0200 Subject: [PATCH 6/8] perf(reach): return only hiding names from the visibility lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Final independent review: with a hidden-name prefix configured, a Reach cache hit scanned one row per listed identity plus the whole observers table, 1.7 ms (hub: 150 links, 100 direct observers) and 5.0 ms (2000 observers) vs ~0.1 ms on master. - The lookup now filters by prefix in SQL (byte-exact substr/length on BLOB = strings.HasPrefix, inlined per enforced prefix), so it normally returns no rows: 0.33 ms (hub) and 0.66 ms (2000 observers). Go still confirms every returned name with IsNameHidden. - Without prefixes the lookup is skipped entirely and the hot path does no per-identity lower-casing: ~24 µs, at or below master. - The name lookup uses the request context (a gone client frees its pool connection). - Config: one lazy hide-prefix helper shared by IsNameHidden, ActiveHiddenNamePrefixes and the new EnforcedHiddenNamePrefixes (which replaces HasHiddenNamePrefixes and also feeds the SQL). - Tests: realistic-scale benchmark; SQL prefix match is byte-exact (case, multi-byte, exact-length, shorter); only hiding names are returned; reachCacheSetBody never attaches a body to a newer entry; the nothing-hidden report is compared in full, not by counts. - Docs: un-hiding the target by rename can also lag up to the cache TTL; honest worst-case cache size. Co-Authored-By: Claude Opus 5 --- cmd/server/config.go | 68 ++++------- cmd/server/identity_visibility.go | 110 +++++++++-------- cmd/server/node_reach.go | 56 +++++---- .../node_reach_visibility_bench_test.go | 73 +++++++++++ cmd/server/node_reach_visibility_test.go | 113 +++++++++++++++--- docs/api-spec.md | 7 +- 6 files changed, 285 insertions(+), 142 deletions(-) diff --git a/cmd/server/config.go b/cmd/server/config.go index ca91385ec..c4308dae9 100644 --- a/cmd/server/config.go +++ b/cmd/server/config.go @@ -928,25 +928,7 @@ func (c *Config) IsBlacklisted(pubkey string) bool { // IsBlacklisted's CAS-style lazy first-read materialisation for the // JSON-load path where SetHiddenNamePrefixes was never called. func (c *Config) IsNameHidden(name string) bool { - if c == nil { - return false - } - pp := c.hiddenPrefixesPtr.Load() - if pp == nil { - // Lazy first-read materialisation from the JSON-loaded slice. - // CAS-style: if another goroutine wins the race, drop ours. - built := make([]string, len(c.HiddenNamePrefixes)) - copy(built, c.HiddenNamePrefixes) - if c.hiddenPrefixesPtr.CompareAndSwap(nil, &built) { - pp = &built - } else { - pp = c.hiddenPrefixesPtr.Load() - } - } - if pp == nil || len(*pp) == 0 { - return false - } - for _, p := range *pp { + for _, p := range c.hiddenPrefixes() { if p == "" { continue } @@ -957,13 +939,12 @@ func (c *Config) IsNameHidden(name string) bool { return false } -// HasHiddenNamePrefixes reports whether IsNameHidden can return true for some -// name — i.e. at least one non-empty prefix is active. It uses exactly -// IsNameHidden's predicate (a whitespace-only prefix counts), so callers can -// skip name lookups safely when it is false. -func (c *Config) HasHiddenNamePrefixes() bool { +// hiddenPrefixes returns the active hide-prefix slice (shared, read-only), +// materialising it lazily from the JSON-loaded HiddenNamePrefixes on first +// read. CAS-style: if another goroutine wins the race, ours is dropped. +func (c *Config) hiddenPrefixes() []string { if c == nil { - return false + return nil } pp := c.hiddenPrefixesPtr.Load() if pp == nil { @@ -976,14 +957,23 @@ func (c *Config) HasHiddenNamePrefixes() bool { } } if pp == nil { - return false + return nil } - for _, p := range *pp { + return *pp +} + +// EnforcedHiddenNamePrefixes returns (a copy of) exactly the prefixes +// IsNameHidden enforces — every non-empty entry, a whitespace-only one +// included — or nil when none is active, so callers can skip name lookups +// safely and match the same prefixes elsewhere (e.g. in SQL). +func (c *Config) EnforcedHiddenNamePrefixes() []string { + var out []string + for _, p := range c.hiddenPrefixes() { if p != "" { - return true + out = append(out, p) } } - return false + return out } // ActiveHiddenNamePrefixes returns a copy of the hide prefixes IsNameHidden @@ -996,24 +986,12 @@ func (c *Config) HasHiddenNamePrefixes() bool { // instead of hardcoding one. An empty result means the deployment offers no // prefix-based hiding, and the page must not claim it does. func (c *Config) ActiveHiddenNamePrefixes() []string { - if c == nil { - return nil - } - pp := c.hiddenPrefixesPtr.Load() - if pp == nil { - built := make([]string, len(c.HiddenNamePrefixes)) - copy(built, c.HiddenNamePrefixes) - if c.hiddenPrefixesPtr.CompareAndSwap(nil, &built) { - pp = &built - } else { - pp = c.hiddenPrefixesPtr.Load() - } - } - if pp == nil { + prefixes := c.hiddenPrefixes() + if prefixes == nil { return nil } - out := make([]string, 0, len(*pp)) - for _, p := range *pp { + out := make([]string, 0, len(prefixes)) + for _, p := range prefixes { if strings.TrimSpace(p) == "" { continue } diff --git a/cmd/server/identity_visibility.go b/cmd/server/identity_visibility.go index f9e33f6c4..745931659 100644 --- a/cmd/server/identity_visibility.go +++ b/cmd/server/identity_visibility.go @@ -1,6 +1,7 @@ package main import ( + "context" "encoding/json" "fmt" "strings" @@ -26,9 +27,12 @@ func identityHidden(cfg *Config, pubkey string, names ...string) bool { return false } -// identityNamesSQL looks up every name a JSON array of lower-case pubkeys is -// known by, in one statement with fixed text (cheap to prepare, unlike a -// variable-length IN list): +// hiddenNamesSQL builds the lookup for a JSON array (?1) of lower-case +// pubkeys that returns only names starting with one of the n hidden-name +// prefixes bound as ?2..?n+1 — normally no rows at all, so a Reach serve does +// not scan a row per listed identity. The prefix test is inlined per prefix +// (usually one) and compares bytes (substr/length on BLOB), i.e. exactly +// strings.HasPrefix as IsNameHidden applies it; NULL names never match. // - nodes and inactive_nodes: keys are lower-case (the ingestor normalises // them), so the primary-key index is used per pubkey. inactive_nodes holds // nodes aged out of `nodes`, whose adverts can still sit in a Reach window. @@ -36,25 +40,37 @@ func identityHidden(cfg *Config, pubkey string, names ...string) bool { // counts while the pubkey has no nodes row with a name — a current name // supersedes it; a nameless return (stored as an empty name) does not. // - observers: ids arrive raw from the MQTT topic in any case, so they are -// matched case-insensitively in one pass over the (small) table. -const identityNamesSQL = ` - SELECT n.public_key, COALESCE(n.name, '') FROM json_each(?1) j JOIN nodes n ON n.public_key = j.value +// matched case-insensitively; the cheap prefix test runs first. +func hiddenNamesSQL(n int, withInactive bool) string { + match := func(col string) string { + terms := make([]string, n) + for i := range terms { + terms[i] = fmt.Sprintf("substr(CAST(%s AS BLOB), 1, length(CAST(?%d AS BLOB))) = CAST(?%d AS BLOB)", col, i+2, i+2) + } + return "(" + strings.Join(terms, " OR ") + ")" + } + q := `SELECT n.public_key, n.name FROM json_each(?1) j JOIN nodes n ON n.public_key = j.value WHERE ` + match("n.name") + ` UNION ALL - SELECT o.id, COALESCE(o.name, '') FROM observers o WHERE lower(o.id) IN (SELECT value FROM json_each(?1))` - -const identityNamesInactiveSQL = identityNamesSQL + ` + SELECT o.id, o.name FROM observers o WHERE ` + match("o.name") + ` AND lower(o.id) IN (SELECT value FROM json_each(?1))` + if withInactive { + q += ` UNION ALL - SELECT i.public_key, COALESCE(i.name, '') FROM json_each(?1) j JOIN inactive_nodes i ON i.public_key = j.value - WHERE NOT EXISTS (SELECT 1 FROM nodes n WHERE n.public_key = j.value AND COALESCE(n.name, '') <> '')` + SELECT i.public_key, i.name FROM json_each(?1) j JOIN inactive_nodes i ON i.public_key = j.value + WHERE ` + match("i.name") + ` + AND NOT EXISTS (SELECT 1 FROM nodes n WHERE n.public_key = j.value AND COALESCE(n.name, '') <> '')` + } + return q +} -// identityNames returns the current node, inactive-node and observer names of -// the given pubkeys, read live from the DB in one bulk query (no per-pubkey -// lookups), keyed by lowercase pubkey. A rename into a hidden prefix -// therefore takes effect on the next request, cached response or not. -// Returns (nil, nil) when no hidden-name prefix is configured: names cannot -// hide anything then. -func (s *Server) identityNames(pubkeys []string) (map[string][]string, error) { - if s.cfg == nil || !s.cfg.HasHiddenNamePrefixes() || len(pubkeys) == 0 { +// hiddenIdentityNames returns, keyed by lowercase pubkey, the current names of +// the given pubkeys that start with a hidden-name prefix: node, inactive-node +// (see above) and observer names, read live from the DB in one bulk query (no +// per-pubkey lookups). A rename into a hidden prefix therefore takes effect +// on the next request, cached response or not. Returns (nil, nil) when no +// hidden-name prefix is configured: names cannot hide anything then. +func (s *Server) hiddenIdentityNames(ctx context.Context, pubkeys []string) (map[string][]string, error) { + prefixes := s.cfg.EnforcedHiddenNamePrefixes() + if len(prefixes) == 0 || len(pubkeys) == 0 { return nil, nil } if s.db == nil || s.db.conn == nil { @@ -69,26 +85,34 @@ func (s *Server) identityNames(pubkeys []string) (map[string][]string, error) { keys = append(keys, pk) } } - arg, err := json.Marshal(keys) + keysArg, err := json.Marshal(keys) if err != nil { return nil, err } - out := make(map[string][]string, len(keys)) - add := func(pk, name string) { - if name != "" { - pk = strings.ToLower(pk) - out[pk] = append(out[pk], name) - } + hasInactive, err := s.hasInactiveNodesTable(ctx) + if err != nil { + return nil, fmt.Errorf("identity names: %w", err) + } + args := make([]interface{}, 0, 1+len(prefixes)) + args = append(args, string(keysArg)) + for _, p := range prefixes { + args = append(args, p) } - q := identityNamesSQL - hasInactive, err := s.hasInactiveNodesTable() + rows, err := s.db.conn.QueryContext(ctx, hiddenNamesSQL(len(prefixes), hasInactive), args...) if err != nil { return nil, fmt.Errorf("identity names: %w", err) } - if hasInactive { - q = identityNamesInactiveSQL + defer rows.Close() + out := make(map[string][]string) + for rows.Next() { + var pk, name string + if err := rows.Scan(&pk, &name); err != nil { + return nil, fmt.Errorf("identity names: %w", err) + } + pk = strings.ToLower(pk) + out[pk] = append(out[pk], name) } - if err := s.scanIdentityNames(q, []interface{}{string(arg)}, add); err != nil { + if err := rows.Err(); err != nil { return nil, fmt.Errorf("identity names: %w", err) } return out, nil @@ -98,12 +122,12 @@ func (s *Server) identityNames(pubkeys []string) (map[string][]string, error) { // schemas always have it (dbschema.AssertReady); minimal DBs may not. A // positive answer is cached — tables are never dropped at runtime — while a // negative one is re-probed so a table created later is picked up. -func (s *Server) hasInactiveNodesTable() (bool, error) { +func (s *Server) hasInactiveNodesTable(ctx context.Context) (bool, error) { if s.reach.inactiveNodesTable.Load() { return true, nil } var n int - err := s.db.conn.QueryRow(`SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'inactive_nodes'`).Scan(&n) + err := s.db.conn.QueryRowContext(ctx, `SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'inactive_nodes'`).Scan(&n) if err != nil { return false, err } @@ -113,29 +137,13 @@ func (s *Server) hasInactiveNodesTable() (bool, error) { return n > 0, nil } -func (s *Server) scanIdentityNames(q string, args []interface{}, add func(pk, name string)) error { - rows, err := s.db.conn.Query(q, args...) - if err != nil { - return err - } - defer rows.Close() - for rows.Next() { - var pk, name string - if err := rows.Scan(&pk, &name); err != nil { - return err - } - add(pk, name) - } - return rows.Err() -} - // isIdentityHidden applies identityHidden to one pubkey with its live names. // A failed name lookup is returned as an error so callers fail closed. -func (s *Server) isIdentityHidden(pubkey string) (bool, error) { +func (s *Server) isIdentityHidden(ctx context.Context, pubkey string) (bool, error) { if identityHidden(s.cfg, pubkey) { return true, nil } - names, err := s.identityNames([]string{pubkey}) + names, err := s.hiddenIdentityNames(ctx, []string{pubkey}) if err != nil { return false, err } diff --git a/cmd/server/node_reach.go b/cmd/server/node_reach.go index 76897815b..f9c9192f6 100644 --- a/cmd/server/node_reach.go +++ b/cmd/server/node_reach.go @@ -262,9 +262,10 @@ func clampDays(d int) int { // recompute under dashboard polling. Keyed "pubkey|days". --- // // reachCacheMax bounds entry count; an entry holds the report plus its -// marshalled JSON (~2KB each for a typical node, so a few KB per entry), so -// the worst case stays in the low MB and an entry cap (rather than a byte -// budget) keeps the bookkeeping trivial while staying memory-safe. +// marshalled JSON. A typical node needs a few KB; a hub with hundreds of links +// and direct observers can reach a few hundred KB (≈195 KB of JSON for 300 +// links + 100 observers), so the cap bounds the worst case at tens of MB while +// keeping the bookkeeping trivial (an entry cap rather than a byte budget). const ( reachCacheTTL = 5 * time.Minute reachCacheMax = 256 @@ -395,21 +396,28 @@ func (s *Server) reachCacheSetBody(key string, at time.Time, raw []byte, hiddenK // and the counts derived from those lists are recomputed; hiddenKey lists the // removed pubkeys (sorted, comma-joined; "" when none, in which case resp is // returned as is, without copying). -func (s *Server) visibleReach(resp NodeReachResponse) (out NodeReachResponse, hiddenKey string, targetHidden bool, err error) { - pks := make([]string, 0, 1+len(resp.Links)+len(resp.DirectObservers)) - pks = append(pks, resp.Node.Pubkey) - for _, l := range resp.Links { - pks = append(pks, l.Pubkey) - } - for _, o := range resp.DirectObservers { - pks = append(pks, o.Pubkey) - } - live, err := s.identityNames(pks) - if err != nil { - return NodeReachResponse{}, "", false, err +func (s *Server) visibleReach(ctx context.Context, resp NodeReachResponse) (out NodeReachResponse, hiddenKey string, targetHidden bool, err error) { + var live map[string][]string // hiding names by pubkey; nil without prefixes + if len(s.cfg.EnforcedHiddenNamePrefixes()) > 0 { + pks := make([]string, 0, 1+len(resp.Links)+len(resp.DirectObservers)) + pks = append(pks, resp.Node.Pubkey) + for _, l := range resp.Links { + pks = append(pks, l.Pubkey) + } + for _, o := range resp.DirectObservers { + pks = append(pks, o.Pubkey) + } + if live, err = s.hiddenIdentityNames(ctx, pks); err != nil { + return NodeReachResponse{}, "", false, err + } } + // Report pubkeys are lower-case (the handler, resolver and scan all + // normalise them), matching the keys of live. hidden := func(pk, recorded string) bool { - return identityHidden(s.cfg, pk, recorded) || identityHidden(s.cfg, pk, live[strings.ToLower(pk)]...) + if identityHidden(s.cfg, pk, recorded) { + return true + } + return live != nil && identityHidden(s.cfg, pk, live[pk]...) } if hidden(resp.Node.Pubkey, resp.Node.Name) { return NodeReachResponse{}, "", true, nil @@ -462,8 +470,8 @@ func (s *Server) visibleReach(resp NodeReachResponse) (out NodeReachResponse, hi // reachBody returns e's body with visibility applied now, re-marshalling (and // refreshing the cached body) only when the set of hidden identities changed. // targetHidden means the caller must answer 404 instead. -func (s *Server) reachBody(key string, e reachCacheEntry) (raw []byte, targetHidden bool, err error) { - vis, hiddenKey, targetHidden, err := s.visibleReach(e.resp) +func (s *Server) reachBody(ctx context.Context, key string, e reachCacheEntry) (raw []byte, targetHidden bool, err error) { + vis, hiddenKey, targetHidden, err := s.visibleReach(ctx, e.resp) if err != nil || targetHidden { return nil, targetHidden, err } @@ -543,13 +551,13 @@ func (s *Server) handleNodeReach(w http.ResponseWriter, r *http.Request) { s.reachPurgeIfBlacklistGenChanged(gen) cacheKey := pubkey + "|" + strconv.Itoa(days) + "|g" + strconv.FormatUint(gen, 10) if e, ok := s.reachCacheGet(cacheKey); ok { - s.writeReachEntry(w, cacheKey, e) + s.writeReachEntry(w, r, cacheKey, e) return } // Cache miss: check the target's live names before the expensive scan. // A failed name lookup fails closed. - if hidden, err := s.isIdentityHidden(pubkey); err != nil { + if hidden, err := s.isIdentityHidden(r.Context(), pubkey); err != nil { log.Printf("[reach] visibility check failed for %s: %v", pubkey, err) writeError(w, 500, "reach computation failed") return @@ -579,7 +587,7 @@ func (s *Server) handleNodeReach(w http.ResponseWriter, r *http.Request) { // Marshal the visible body once here so the waiters sharing this // result reuse it (each still re-checks visibility in reachBody). e := reachCacheEntry{at: time.Now(), resp: resp, found: true} - vis, hiddenKey, targetHidden, vErr := s.visibleReach(resp) + vis, hiddenKey, targetHidden, vErr := s.visibleReach(r.Context(), resp) if vErr != nil { return nil, vErr } @@ -603,14 +611,14 @@ func (s *Server) handleNodeReach(w http.ResponseWriter, r *http.Request) { writeError(w, 404, "Not found") return } - s.writeReachEntry(w, cacheKey, e) + s.writeReachEntry(w, r, cacheKey, e) } // writeReachEntry writes a found entry's body with visibility applied now: // 404 when the target itself became hidden. A failed live-name lookup fails // closed (500), never serving unfiltered data. -func (s *Server) writeReachEntry(w http.ResponseWriter, key string, e reachCacheEntry) { - raw, targetHidden, err := s.reachBody(key, e) +func (s *Server) writeReachEntry(w http.ResponseWriter, r *http.Request, key string, e reachCacheEntry) { + raw, targetHidden, err := s.reachBody(r.Context(), key, e) if err != nil { log.Printf("[reach] serving %s failed: %v", key, err) writeError(w, 500, "reach computation failed") diff --git a/cmd/server/node_reach_visibility_bench_test.go b/cmd/server/node_reach_visibility_bench_test.go index 1a07b3a48..304a61eab 100644 --- a/cmd/server/node_reach_visibility_bench_test.go +++ b/cmd/server/node_reach_visibility_bench_test.go @@ -2,6 +2,7 @@ package main import ( "database/sql" + "encoding/json" "fmt" "net/http" "path/filepath" @@ -99,3 +100,75 @@ func BenchmarkNodeReachCacheHitVisibility(b *testing.B) { }) } } + +// benchReachScaleDB is a realistic-to-large Reach report: `links` neighbours +// (half both ways), `direct` observers that heard the target at 0 hops, +// `observers` rows in total and `nodes` rows in total — what the per-serve +// visibility lookup has to cover. +func benchReachScaleDB(b *testing.B, links, direct, observers, nodes int) (*DB, string) { + b.Helper() + db, target := benchReachVisibilityDB(b, links) + tx, err := db.conn.Begin() + if err != nil { + b.Fatal(err) + } + ins := func(q string, args ...interface{}) { + if _, err := tx.Exec(q, args...); err != nil { + b.Fatal(err) + } + } + for i := 5; i < observers; i++ { + ins(`INSERT INTO observers (id, name) VALUES (?, ?)`, strings.ToUpper(pk64(fmt.Sprintf("0c%04x", i))), fmt.Sprintf("Observer %d", i)) + } + for i := 1 + links; i < nodes; i++ { + ins(`INSERT INTO nodes (public_key, name, role, lat, lon, last_seen, first_seen) VALUES (?, ?, 'companion', 56.2, 10.3, '2026-09-01T00:00:00Z', '2026-06-01T00:00:00Z')`, + pk64(fmt.Sprintf("d%05x", i)), fmt.Sprintf("Companion %d", i)) + } + now := time.Now().Unix() + for i := 0; i < direct; i++ { // observer rowid 6.. heard the target at 0 hops + id := 100000 + i + ins(`INSERT INTO transmissions (id, from_pubkey, payload_type) VALUES (?, '', 5)`, id) + ins(`INSERT INTO observations (id, transmission_id, observer_idx, snr, path_json, timestamp) VALUES (?, ?, ?, -7.0, '["01FA"]', ?)`, id, id, 6+i, now) + } + if err := tx.Commit(); err != nil { + b.Fatal(err) + } + return db, target +} + +// BenchmarkNodeReachCacheHitVisibilityScale: the same cache hit at realistic +// and large report sizes (links / direct observers / observers / nodes). +func BenchmarkNodeReachCacheHitVisibilityScale(b *testing.B) { + for _, sz := range []struct { + name string + links, direct, observers, nodes int + }{ + {"hub_150l_100o_200obs_3000n", 150, 100, 200, 3000}, + {"large_300l_100o_2000obs_5000n", 300, 100, 2000, 5000}, + } { + for _, c := range []struct { + name string + prefixes []string + }{{"prefixes", []string{"🚫"}}, {"no_prefixes", nil}} { + b.Run(sz.name+"/"+c.name, func(b *testing.B) { + db, target := benchReachScaleDB(b, sz.links, sz.direct, sz.observers, sz.nodes) + cfg := &Config{HiddenNamePrefixes: c.prefixes} + srv := &Server{store: &PacketStore{db: db}, db: db, cfg: cfg, perfStats: NewPerfStats()} + path := "/api/nodes/" + target + "/reach?days=7" + rr := serveReach(srv, path) + var resp NodeReachResponse + if rr.Code != http.StatusOK || json.Unmarshal(rr.Body.Bytes(), &resp) != nil || + len(resp.Links) < sz.links || len(resp.DirectObservers) < sz.direct { + b.Fatalf("warm-up: %d links=%d obs=%d", rr.Code, len(resp.Links), len(resp.DirectObservers)) + } + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + if rr := serveReach(srv, path); rr.Code != http.StatusOK { + b.Fatalf("status %d", rr.Code) + } + } + }) + } + } +} diff --git a/cmd/server/node_reach_visibility_test.go b/cmd/server/node_reach_visibility_test.go index de4b410d6..d89170f7b 100644 --- a/cmd/server/node_reach_visibility_test.go +++ b/cmd/server/node_reach_visibility_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "fmt" "net/http" + "reflect" "sort" "strconv" "strings" @@ -341,36 +342,110 @@ func TestNodeReach_NothingHiddenBodyUnchanged(t *testing.T) { if err := json.Unmarshal([]byte(body), &got); err != nil { t.Fatal(err) } - if len(got.Links) != len(resp.Links) || len(got.DirectObservers) != len(resp.DirectObservers) || - got.Importance != resp.Importance { - t.Fatalf("unfiltered report changed: got %d links/%d obs %+v, want %d/%d %+v", - len(got.Links), len(got.DirectObservers), got.Importance, len(resp.Links), len(resp.DirectObservers), resp.Importance) + // Full comparison (not just counts). Link order among equal-scored links + // is not specified, so compare as sorted lists; round-trip resp through + // JSON so pointer fields compare by value. + var want NodeReachResponse + raw, _ := json.Marshal(resp) + if err := json.Unmarshal(raw, &want); err != nil { + t.Fatal(err) + } + sortReport := func(r *NodeReachResponse) { + sort.Slice(r.Links, func(i, j int) bool { return r.Links[i].Pubkey < r.Links[j].Pubkey }) + sort.Slice(r.DirectObservers, func(i, j int) bool { return r.DirectObservers[i].Pubkey < r.DirectObservers[j].Pubkey }) + } + sortReport(&got) + sortReport(&want) + want.Window.Since = got.Window.Since // computed a moment apart + if !reflect.DeepEqual(got, want) { + t.Fatalf("unfiltered report changed:\n got %+v\nwant %+v", got, want) } - if names, err := (&Server{cfg: &Config{}}).identityNames([]string{f.n}); names != nil || err != nil { - t.Fatalf("identityNames without prefixes = %v, %v; want nil, nil and no DB access", names, err) + if names, err := (&Server{cfg: &Config{}}).hiddenIdentityNames(context.Background(), []string{f.n}); names != nil || err != nil { + t.Fatalf("hiddenIdentityNames without prefixes = %v, %v; want nil, nil and no DB access", names, err) } } -// identityNames handles a large batch (a hub's full link list) in one query -// and maps upper-case observer ids back to lower-case pubkeys. -func TestIdentityNames_LargeBatch(t *testing.T) { +// hiddenIdentityNames handles a large batch (a hub's full link list) in one +// query, returns only names that start with a hidden prefix, and maps +// upper-case observer ids back to lower-case pubkeys. +func TestHiddenIdentityNames_LargeBatchOnlyHidingNames(t *testing.T) { f := newReachVisibilityDB(t) pks := make([]string, 0, 1010) for i := 0; i < 1000; i++ { pks = append(pks, pk64(fmt.Sprintf("f%05x", i))) } - pks = append(pks, f.o2Hidden, strings.ToUpper(f.hidName), f.hidObsName) - names, err := f.srv.identityNames(pks) + pks = append(pks, f.o2Hidden, strings.ToUpper(f.hidName), f.hidObsName, f.visible, f.o1, f.inactiveHidden) + names, err := f.srv.hiddenIdentityNames(context.Background(), pks) if err != nil { t.Fatal(err) } - if fmt.Sprint(names[f.o2Hidden]) != "[🚫 hidden obs]" || fmt.Sprint(names[f.hidName]) != "[🚫 private A]" { - t.Fatalf("names=%v", names) + want := map[string]string{ + f.o2Hidden: "[🚫 hidden obs]", + f.hidName: "[🚫 private A]", + f.hidObsName: "[🚫 obs D]", // its visible node name "Delta" is not returned + f.inactiveHidden: "[🚫 gone quiet]", + } + if len(names) != len(want) { + t.Fatalf("names=%v, want only the hiding names %v", names, want) + } + for pk, w := range want { + if fmt.Sprint(names[pk]) != w { + t.Fatalf("names[%s]=%v want %s (all=%v)", pk[:4], names[pk], w, names) + } } - d := names[f.hidObsName] - sort.Strings(d) - if fmt.Sprint(d) != "[Delta 🚫 obs D]" { - t.Fatalf("node + observer names for D = %v", d) +} + +// The SQL prefix test is byte-exact strings.HasPrefix, as IsNameHidden. +func TestHiddenIdentityNames_PrefixMatchIsExact(t *testing.T) { + f := newReachVisibilityDB(t) + cases := []struct { + name string + hide bool + }{ + {"Ab visible", false}, // case differs from prefix "AB" + {"AB", true}, // exactly the prefix + {"A", false}, // shorter than the prefix + {"AB side", true}, + {"x AB", false}, // prefix not at the start + {"🚫", true}, // multi-byte prefix + {"🚫x", true}, + {"\U0001F6ABx", true}, // same emoji, escaped + } + f.cfg.SetHiddenNamePrefixes([]string{"AB", "🚫"}) + pks := make([]string, len(cases)) + for i, c := range cases { + pks[i] = pk64(fmt.Sprintf("e0%02x", i)) + if _, err := f.db.conn.Exec(`INSERT INTO nodes (public_key, name, role) VALUES (?, ?, 'repeater')`, pks[i], c.name); err != nil { + t.Fatal(err) + } + } + names, err := f.srv.hiddenIdentityNames(context.Background(), pks) + if err != nil { + t.Fatal(err) + } + for i, c := range cases { + if got := len(names[pks[i]]) > 0; got != c.hide || got != f.cfg.IsNameHidden(c.name) { + t.Errorf("%q: SQL hiding=%v, IsNameHidden=%v, want %v", c.name, got, f.cfg.IsNameHidden(c.name), c.hide) + } + } +} + +// reachCacheSetBody never attaches a body to an entry computed at another +// time: a body built from an older report must not be reused for a newer one. +func TestReachCacheSetBody_StaleEntryUntouched(t *testing.T) { + srv := &Server{} + resetReachState(t, srv) + old := time.Now().Add(-time.Minute) + cur := reachCacheEntry{at: time.Now(), raw: []byte(`{"current":true}`), hiddenKey: "", found: true} + srv.reachCachePut("k", cur) + srv.reachCacheSetBody("k", old, []byte(`{"stale":true}`), "x") + got, ok := srv.reachCacheGet("k") + if !ok || string(got.raw) != `{"current":true}` || got.hiddenKey != "" { + t.Fatalf("stale body attached to a newer entry: %s %q", got.raw, got.hiddenKey) + } + srv.reachCacheSetBody("k", cur.at, []byte(`{"refreshed":true}`), "y") + if got, _ := srv.reachCacheGet("k"); string(got.raw) != `{"refreshed":true}` || got.hiddenKey != "y" { + t.Fatalf("matching entry not refreshed: %s %q", got.raw, got.hiddenKey) } } @@ -401,8 +476,8 @@ func TestNodeReach_WhitespacePrefixStillLooksUpNames(t *testing.T) { if _, resp, _ := f.get(t, f.n); strings.Contains(linkSet(resp), f.visible[:4]) { t.Fatalf("whitespace-prefixed rename not hidden: %s", linkSet(resp)) } - if !f.cfg.HasHiddenNamePrefixes() || (&Config{HiddenNamePrefixes: []string{""}}).HasHiddenNamePrefixes() { - t.Fatalf("HasHiddenNamePrefixes must mirror IsNameHidden: only empty prefixes are inert") + if len(f.cfg.EnforcedHiddenNamePrefixes()) != 1 || (&Config{HiddenNamePrefixes: []string{""}}).EnforcedHiddenNamePrefixes() != nil { + t.Fatalf("EnforcedHiddenNamePrefixes must mirror IsNameHidden: only empty prefixes are inert") } } diff --git a/docs/api-spec.md b/docs/api-spec.md index cbab2dae6..d98e41c2b 100644 --- a/docs/api-spec.md +++ b/docs/api-spec.md @@ -732,9 +732,10 @@ An identity is **hidden** when its pubkey is in `nodeBlacklist` or `links` and `direct_observers`, and `bidirectional_links` / `direct_observers` count only what is listed. Names are read live on every request — including cached reports — so **hiding** (a blacklist or prefix change, or a rename into -a hidden prefix) applies on the next request. Un-hiding a neighbour by -renaming it can take up to the 5-minute cache TTL, because the name recorded -when the report was computed still counts. +a hidden prefix) applies on the next request. Un-hiding by renaming — of a +neighbour or of the target itself — can take up to the 5-minute cache TTL +(plus the server's 30 s node cache for the target), because the name recorded +when the report was computed still counts. This errs on the side of hiding. `neighbor_degree`, `degree_rank` and `nodes_with_edges` are counts over the whole neighbour graph and are not changed by this filtering. From b4de6b2718a76df5eaf1a27000778f687f34364e Mon Sep 17 00:00:00 2001 From: Dennis Jakobsen Date: Sat, 19 Sep 2026 07:19:18 +0200 Subject: [PATCH 7/8] perf(reach): scale benchmark includes inactive_nodes; bind the inactive NOT EXISTS to its row Final-review follow-up: production schemas always have inactive_nodes, so the scale benchmark now creates it (with aged-out rows, some for listed pubkeys) and measures the lookup's third branch. Binding the NOT EXISTS to i.public_key instead of j.value is ~7 % faster in an interleaved A/B (1.09 -> 1.01 ms at 300 links / 2000 observers). Co-Authored-By: Claude Opus 5 --- cmd/server/identity_visibility.go | 2 +- cmd/server/node_reach_visibility_bench_test.go | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/cmd/server/identity_visibility.go b/cmd/server/identity_visibility.go index 745931659..468372625 100644 --- a/cmd/server/identity_visibility.go +++ b/cmd/server/identity_visibility.go @@ -57,7 +57,7 @@ func hiddenNamesSQL(n int, withInactive bool) string { UNION ALL SELECT i.public_key, i.name FROM json_each(?1) j JOIN inactive_nodes i ON i.public_key = j.value WHERE ` + match("i.name") + ` - AND NOT EXISTS (SELECT 1 FROM nodes n WHERE n.public_key = j.value AND COALESCE(n.name, '') <> '')` + AND NOT EXISTS (SELECT 1 FROM nodes n WHERE n.public_key = i.public_key AND COALESCE(n.name, '') <> '')` } return q } diff --git a/cmd/server/node_reach_visibility_bench_test.go b/cmd/server/node_reach_visibility_bench_test.go index 304a61eab..fe449088e 100644 --- a/cmd/server/node_reach_visibility_bench_test.go +++ b/cmd/server/node_reach_visibility_bench_test.go @@ -31,6 +31,8 @@ func benchReachVisibilityDB(b *testing.B, neighbours int) (*DB, string) { `CREATE TABLE observations (id INTEGER PRIMARY KEY, transmission_id INTEGER, observer_idx INTEGER, snr REAL, path_json TEXT, timestamp INTEGER)`, `CREATE TABLE neighbor_edges (node_a TEXT NOT NULL, node_b TEXT NOT NULL, count INTEGER DEFAULT 1, last_seen TEXT, PRIMARY KEY (node_a, node_b))`, `CREATE INDEX idx_obs_ts ON observations(timestamp)`, + // Production schemas always have inactive_nodes (dbschema.AssertReady). + `CREATE TABLE inactive_nodes (public_key TEXT PRIMARY KEY, name TEXT, role TEXT, lat REAL, lon REAL, last_seen TEXT, first_seen TEXT, advert_count INTEGER DEFAULT 0)`, } { if _, err := conn.Exec(s); err != nil { b.Fatal(err) @@ -124,6 +126,15 @@ func benchReachScaleDB(b *testing.B, links, direct, observers, nodes int) (*DB, ins(`INSERT INTO nodes (public_key, name, role, lat, lon, last_seen, first_seen) VALUES (?, ?, 'companion', 56.2, 10.3, '2026-09-01T00:00:00Z', '2026-06-01T00:00:00Z')`, pk64(fmt.Sprintf("d%05x", i)), fmt.Sprintf("Companion %d", i)) } + // Aged-out nodes: as many inactive rows as nodes, a tenth of them for + // pubkeys that are listed in the report (their old rows are kept). + for i := 0; i < nodes; i++ { + pk := pk64(fmt.Sprintf("a%05x", i)) + if i%10 == 0 && i/10 < links { + pk = pk64(fmt.Sprintf("%04x", 0x2000+i/10)) + } + ins(`INSERT OR IGNORE INTO inactive_nodes (public_key, name, role) VALUES (?, ?, 'companion')`, pk, fmt.Sprintf("Old %d", i)) + } now := time.Now().Unix() for i := 0; i < direct; i++ { // observer rowid 6.. heard the target at 0 hops id := 100000 + i From eb4afecd7d6054577ce41360a88128f239686242 Mon Sep 17 00:00:00 2001 From: Dennis Jakobsen Date: Sat, 19 Sep 2026 07:23:34 +0200 Subject: [PATCH 8/8] perf(reach): look up node and inactive names in one pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Join nodes and inactive_nodes by primary key in a single pass over the listed pubkeys instead of a separate inactive_nodes branch with a correlated NOT EXISTS; "no named nodes row" becomes COALESCE(n.name, '') = '' on the joined row. Same results (tests and mutation checks unchanged), ~11 % faster in an interleaved A/B: hub 518 -> 460 µs, 300 links / 2000 observers 985 -> 883 µs. Co-Authored-By: Claude Opus 5 --- cmd/server/identity_visibility.go | 34 ++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/cmd/server/identity_visibility.go b/cmd/server/identity_visibility.go index 468372625..2695ee407 100644 --- a/cmd/server/identity_visibility.go +++ b/cmd/server/identity_visibility.go @@ -49,17 +49,23 @@ func hiddenNamesSQL(n int, withInactive bool) string { } return "(" + strings.Join(terms, " OR ") + ")" } - q := `SELECT n.public_key, n.name FROM json_each(?1) j JOIN nodes n ON n.public_key = j.value WHERE ` + match("n.name") + ` - UNION ALL - SELECT o.id, o.name FROM observers o WHERE ` + match("o.name") + ` AND lower(o.id) IN (SELECT value FROM json_each(?1))` + // One pass over the keys: the nodes row and, when present, the + // inactive_nodes row are joined by primary key; the inactive name is only + // returned while the node has no named row. + nodes := `SELECT j.value, COALESCE(n.name, ''), '' FROM json_each(?1) j + LEFT JOIN nodes n ON n.public_key = j.value + WHERE ` + match("n.name") if withInactive { - q += ` + nodes = `SELECT j.value, COALESCE(n.name, ''), + CASE WHEN COALESCE(n.name, '') = '' THEN COALESCE(i.name, '') ELSE '' END + FROM json_each(?1) j + LEFT JOIN nodes n ON n.public_key = j.value + LEFT JOIN inactive_nodes i ON i.public_key = j.value + WHERE ` + match("n.name") + ` OR (COALESCE(n.name, '') = '' AND ` + match("i.name") + `)` + } + return nodes + ` UNION ALL - SELECT i.public_key, i.name FROM json_each(?1) j JOIN inactive_nodes i ON i.public_key = j.value - WHERE ` + match("i.name") + ` - AND NOT EXISTS (SELECT 1 FROM nodes n WHERE n.public_key = i.public_key AND COALESCE(n.name, '') <> '')` - } - return q + SELECT o.id, o.name, '' FROM observers o WHERE ` + match("o.name") + ` AND lower(o.id) IN (SELECT value FROM json_each(?1))` } // hiddenIdentityNames returns, keyed by lowercase pubkey, the current names of @@ -105,12 +111,16 @@ func (s *Server) hiddenIdentityNames(ctx context.Context, pubkeys []string) (map defer rows.Close() out := make(map[string][]string) for rows.Next() { - var pk, name string - if err := rows.Scan(&pk, &name); err != nil { + var pk, name, inactiveName string + if err := rows.Scan(&pk, &name, &inactiveName); err != nil { return nil, fmt.Errorf("identity names: %w", err) } pk = strings.ToLower(pk) - out[pk] = append(out[pk], name) + for _, nm := range []string{name, inactiveName} { + if nm != "" { + out[pk] = append(out[pk], nm) // callers confirm with IsNameHidden + } + } } if err := rows.Err(); err != nil { return nil, fmt.Errorf("identity names: %w", err)