diff --git a/cmd/server/routes.go b/cmd/server/routes.go index 3465aa18..4c16bad2 100644 --- a/cmd/server/routes.go +++ b/cmd/server/routes.go @@ -7,10 +7,12 @@ import ( "errors" "fmt" "log" + "maps" "math" "net/http" "regexp" "runtime" + "slices" "sort" "strconv" "strings" @@ -2618,8 +2620,8 @@ func (s *Server) handleAnalyticsSubpaths(w http.ResponseWriter, r *http.Request) // Issue #1217: honor the Time window filter on Route Patterns. window := ParseTimeWindow(r) data := s.store.GetAnalyticsSubpathsWithWindow(region, minLen, maxLen, limit, window) - if s.cfg != nil && len(s.cfg.NodeBlacklist) > 0 { - data = s.filterBlacklistedFromSubpaths(data) + if s.subpathFilterActive() { + data, _ = s.dropHiddenPaths(data, "subpaths", "path", "", s.blacklistedNodeNames()) } writeJSON(w, data) return @@ -2684,9 +2686,10 @@ func (s *Server) handleAnalyticsSubpathsBulk(w http.ResponseWriter, r *http.Requ } results := s.store.GetAnalyticsSubpathsBulkWithWindow(region, groups, ParseTimeWindow(r)) - if s.cfg != nil && len(s.cfg.NodeBlacklist) > 0 { + if s.subpathFilterActive() { + names := s.blacklistedNodeNames() for i, r := range results { - results[i] = s.filterBlacklistedFromSubpaths(r) + results[i], _ = s.dropHiddenPaths(r, "subpaths", "path", "", names) } } writeJSON(w, map[string]interface{}{"results": results}) @@ -2722,7 +2725,19 @@ func (s *Server) handleAnalyticsSubpathDetail(w http.ResponseWriter, r *http.Req writeIndexLoading503(w) return } - writeJSON(w, s.store.GetSubpathDetail(rawHops)) + detail := s.store.GetSubpathDetail(rawHops) + // #2184: the raw hops are prefixes, so the check above only catches a + // full key. Check the nodes the store resolved them to instead. + if s.subpathFilterActive() { + // Match each hop by its resolved key, so a blacklisted node without a + // name is caught and a node that only shares its name is not. + if _, dropped := s.dropHiddenPaths(detail, "nodes", "name", "pubkey", nil); dropped > 0 { + writeError(w, 404, "Not found") + return + } + detail, _ = s.dropHiddenPaths(detail, "parentPaths", "path", "", s.blacklistedNodeNames()) + } + writeJSON(w, detail) return } writeJSON(w, SubpathDetailResponse{ @@ -3827,33 +3842,62 @@ func (s *Server) filterBlacklistedFromTopology(data map[string]interface{}) map[ return data } -// filterBlacklistedFromSubpaths removes blacklisted node references from -// the subpaths analytics response. -func (s *Server) filterBlacklistedFromSubpaths(data map[string]interface{}) map[string]interface{} { - if subpaths, ok := data["subpaths"]; ok { - if arr, ok := subpaths.([]interface{}); ok { - var filtered []interface{} - for _, item := range arr { - if m, ok := item.(map[string]interface{}); ok { - if hops, ok := m["hops"].([]interface{}); ok { - skip := false - for _, h := range hops { - if hp, ok := h.(string); ok && (s.cfg.IsBlacklisted(hp) || s.isPubkeyHidden(hp)) { - skip = true - break - } - } - if skip { - continue - } - } - } - filtered = append(filtered, item) - } - data["subpaths"] = filtered +// subpathFilterActive reports whether Route Patterns responses need the +// nodeBlacklist / hiddenNamePrefixes filter. +func (s *Server) subpathFilterActive() bool { + return s.cfg != nil && (len(s.cfg.NodeBlacklist) > 0 || len(s.cfg.HiddenNamePrefixes) > 0) +} + +// blacklistedNodeNames returns the names of the blacklisted nodes the store +// knows, for filtering responses that carry node names but no keys. +func (s *Server) blacklistedNodeNames() map[string]bool { + if s.store == nil || s.cfg == nil || len(s.cfg.NodeBlacklist) == 0 { + return nil + } + nodes, _ := s.store.getCachedNodesAndPM() + names := make(map[string]bool) + for _, n := range nodes { + if n.Name != "" && s.cfg.IsBlacklisted(n.PublicKey) { + names[n.Name] = true } } - return data + return names +} + +// subpathPathHidden reports whether a resolved " → " path shows a blacklisted +// or hidden-prefix node. Issue #2184: a subpath carries the names the store +// resolved its hops to, not keys, and a hop prefix alone cannot be pinned to +// one node, so the filter matches what the reader would see. +func (s *Server) subpathPathHidden(path string, blacklisted map[string]bool) bool { + for _, name := range strings.Split(path, " → ") { + if blacklisted[name] || s.cfg.IsNameHidden(name) { + return true + } + } + return false +} + +// dropHiddenPaths removes the entries of data[listKey] whose field (a node +// name, or a resolved " → " path) shows a blacklisted or hidden-prefix node, +// or, when keyField is set, whose key there is blacklisted, and reports how +// many it removed. It returns a copy: data may be the store's cached +// response, shared with other requests. +func (s *Server) dropHiddenPaths(data map[string]interface{}, listKey, field, keyField string, blacklisted map[string]bool) (map[string]interface{}, int) { + entries, ok := data[listKey].([]map[string]interface{}) + if !ok { + return data, 0 + } + kept := slices.Clone(entries)[:0] + for _, e := range entries { + path, _ := e[field].(string) + key, _ := e[keyField].(string) + if !s.subpathPathHidden(path, blacklisted) && (key == "" || !s.cfg.IsBlacklisted(key)) { + kept = append(kept, e) + } + } + out := maps.Clone(data) + out[listKey] = kept + return out, len(entries) - len(kept) } // handleDroppedPackets returns recently dropped packets for investigation. diff --git a/cmd/server/subpaths_blacklist_2184_test.go b/cmd/server/subpaths_blacklist_2184_test.go new file mode 100644 index 00000000..2ff88304 --- /dev/null +++ b/cmd/server/subpaths_blacklist_2184_test.go @@ -0,0 +1,194 @@ +package main + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/gorilla/mux" +) + +// Issue #2184: nodeBlacklist and hiddenNamePrefixes were not applied to Route +// Patterns. The seeded path ["aa","bb"] resolves its first hop to TestRepeater +// (aabbccdd11223344), so the subpath "TestRepeater → bb" is listed. + +const seededRepeaterKey = "aabbccdd11223344" + +func subpathPaths(t *testing.T, body []byte) []string { + t.Helper() + var resp struct { + Subpaths []struct { + Path string `json:"path"` + } `json:"subpaths"` + } + if err := json.Unmarshal(body, &resp); err != nil { + t.Fatalf("unmarshal: %v body=%s", err, body) + } + out := make([]string, 0, len(resp.Subpaths)) + for _, sp := range resp.Subpaths { + out = append(out, sp.Path) + } + return out +} + +func bulkSubpathPaths(t *testing.T, body []byte) []string { + t.Helper() + var resp struct { + Results []struct { + Subpaths []struct { + Path string `json:"path"` + } `json:"subpaths"` + } `json:"results"` + } + if err := json.Unmarshal(body, &resp); err != nil { + t.Fatalf("unmarshal: %v body=%s", err, body) + } + var out []string + for _, r := range resp.Results { + for _, sp := range r.Subpaths { + out = append(out, sp.Path) + } + } + return out +} + +func anyContains(paths []string, name string) bool { + for _, p := range paths { + if strings.Contains(p, name) { + return true + } + } + return false +} + +func getSubpathBody2184(t *testing.T, srv *Server, url string) (int, []byte) { + t.Helper() + req := httptest.NewRequest("GET", url, nil) + w := httptest.NewRecorder() + srv.router.ServeHTTP(w, req) + return w.Code, w.Body.Bytes() +} + +func TestSubpathsBlacklist2184(t *testing.T) { + cases := []struct { + name string + blacklist []string + hidden []string + wantShown bool + }{ + {"no filter lists the repeater", nil, nil, true}, + {"blacklisted key drops it", []string{seededRepeaterKey}, nil, false}, + {"blacklisted key in capitals drops it", []string{strings.ToUpper(seededRepeaterKey)}, nil, false}, + {"hidden name prefix drops it", nil, []string{"TestRep"}, false}, + // Another key with the same 1-byte prefix: the displayed node is not + // the blacklisted one, so the subpath stays. + {"same-prefix key keeps it", []string{"aa00000000000000"}, nil, true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + srv, _ := setupTestServer(t) + if c.blacklist != nil { + srv.cfg.NodeBlacklist = c.blacklist + srv.cfg.SetNodeBlacklist(c.blacklist) + } + if c.hidden != nil { + srv.cfg.SetHiddenNamePrefixes(c.hidden) + } + + code, body := getSubpathBody2184(t, srv, "/api/analytics/subpaths?minLen=2&maxLen=8") + if code != http.StatusOK { + t.Fatalf("subpaths status=%d body=%s", code, body) + } + if got := anyContains(subpathPaths(t, body), "TestRepeater"); got != c.wantShown { + t.Errorf("subpaths: TestRepeater shown=%v, want %v (%s)", got, c.wantShown, body) + } + + code, body = getSubpathBody2184(t, srv, "/api/analytics/subpaths-bulk?groups=2-2:50,3-8:50") + if code != http.StatusOK { + t.Fatalf("subpaths-bulk status=%d body=%s", code, body) + } + if got := anyContains(bulkSubpathPaths(t, body), "TestRepeater"); got != c.wantShown { + t.Errorf("subpaths-bulk: TestRepeater shown=%v, want %v (%s)", got, c.wantShown, body) + } + + code, body = getSubpathBody2184(t, srv, "/api/analytics/subpath-detail?hops=aa,bb") + wantCode := http.StatusOK + if !c.wantShown { + wantCode = http.StatusNotFound + } + if code != wantCode { + t.Errorf("subpath-detail status=%d, want %d (%s)", code, wantCode, body) + } + }) + } +} + +// The filter must not change the store's cached response: a later request, +// or one after the blacklist is cleared, still sees the full list. +func TestSubpathsBlacklistLeavesCacheIntact2184(t *testing.T) { + srv, _ := setupTestServer(t) + srv.cfg.NodeBlacklist = []string{seededRepeaterKey} + srv.cfg.SetNodeBlacklist(srv.cfg.NodeBlacklist) + if _, body := getSubpathBody2184(t, srv, "/api/analytics/subpaths?minLen=2&maxLen=8"); anyContains(subpathPaths(t, body), "TestRepeater") { + t.Fatalf("blacklisted repeater still listed: %s", body) + } + + srv.cfg.NodeBlacklist = nil + srv.cfg.SetNodeBlacklist(nil) + if _, body := getSubpathBody2184(t, srv, "/api/analytics/subpaths?minLen=2&maxLen=8"); !anyContains(subpathPaths(t, body), "TestRepeater") { + t.Errorf("after clearing the blacklist the repeater is missing: the filter mutated the cached response (%s)", body) + } +} + +// setupSubpathServer2184 is setupTestServer with a SQL statement run on the +// seeded DB before the store loads, so the store sees the changed rows. +func setupSubpathServer2184(t *testing.T, stmt string) *Server { + t.Helper() + db := setupTestDB(t) + seedTestData(t, db) + if _, err := db.conn.Exec(stmt); err != nil { + t.Fatalf("exec %q: %v", stmt, err) + } + srv := NewServer(db, &Config{Port: 3000}, NewHub()) + store := NewPacketStore(db, nil) + if err := store.Load(); err != nil { + t.Fatalf("store.Load failed: %v", err) + } + if !store.WaitIndexesReady(5 * time.Second) { + t.Fatalf("background indexes never became ready") + } + srv.store = store + srv.router = mux.NewRouter() + srv.RegisterRoutes(srv.router) + return srv +} + +// A blacklisted node without a name (the firmware allows one) has no name to +// match, so the detail must check the resolved key. +func TestSubpathDetailBlacklistUnnamedNode2184(t *testing.T) { + srv := setupSubpathServer2184(t, `UPDATE nodes SET name = '' WHERE public_key = '`+seededRepeaterKey+`'`) + srv.cfg.NodeBlacklist = []string{seededRepeaterKey} + srv.cfg.SetNodeBlacklist(srv.cfg.NodeBlacklist) + + code, body := getSubpathBody2184(t, srv, "/api/analytics/subpath-detail?hops=aa,bb") + if code != http.StatusNotFound { + t.Errorf("subpath-detail status=%d, want 404 (%s)", code, body) + } +} + +// Another node that only shares the blacklisted node's name does not hide the +// detail of a path through the seeded repeater. +func TestSubpathDetailBlacklistSameNameOtherNode2184(t *testing.T) { + const other = "ff99887766554433" + srv := setupSubpathServer2184(t, `INSERT INTO nodes (public_key, name, role) VALUES ('`+other+`', 'TestRepeater', 'repeater')`) + srv.cfg.NodeBlacklist = []string{other} + srv.cfg.SetNodeBlacklist(srv.cfg.NodeBlacklist) + + code, body := getSubpathBody2184(t, srv, "/api/analytics/subpath-detail?hops=aa,bb") + if code != http.StatusOK { + t.Errorf("subpath-detail status=%d, want 200 (%s)", code, body) + } +}