From dc3552d8e7005c6bc22522242aa4e773e3c4df1a Mon Sep 17 00:00:00 2001 From: Kpa-clawbot Date: Thu, 11 Jun 2026 16:08:32 +0000 Subject: [PATCH] =?UTF-8?q?test(#1181):=20RED=20=E2=80=94=20endpoint=20cov?= =?UTF-8?q?erage=20+=20generation=20counter=20+=20concurrent=20access?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds failing tests for the three gaps surfaced by Round 1 review: 1. Endpoint coverage [adv #1, djb #1]: TestHiddenNamePrefix_1181_NodeHealth, _BulkHealth, _Paths, _Analytics — all expect IsNameHidden to be honoured at the same sites that already gate on IsBlacklisted. All fail (200 vs 404 / leaked entry). 2. Generation counter [djb #2]: TestHiddenNamePrefixesGeneration_Increments asserts SetHiddenNamePrefixes bumps the counter on every call (mirrors BlacklistGenerationIncrements pattern). Stub returns 0 -> assertion fails on first delta. 3. Slice race [djb #3]: TestHiddenNamePrefixes_ConcurrentAccess hammers Set + IsNameHidden across goroutines. Today this panics on a nil stringslite deref via the plain slice access; the fix wires the slice through atomic.Pointer matching nodeBlacklist. Stubs SetHiddenNamePrefixes / HiddenNamePrefixesGeneration added so the RED commit compiles + fails on assertions, not on missing symbols. --- cmd/server/config.go | 22 ++ .../hidden_name_prefix_1181_endpoints_test.go | 193 ++++++++++++++++++ 2 files changed, 215 insertions(+) create mode 100644 cmd/server/hidden_name_prefix_1181_endpoints_test.go diff --git a/cmd/server/config.go b/cmd/server/config.go index 4f695204..31648c56 100644 --- a/cmd/server/config.go +++ b/cmd/server/config.go @@ -756,6 +756,28 @@ func (c *Config) IsNameHidden(name string) bool { return false } +// SetHiddenNamePrefixes is a stub for the upcoming atomic.Pointer wrapper. +// Currently just assigns the slice (NOT yet race-safe). Bumps the +// generation counter. Wired up properly in the GREEN commit. +func (c *Config) SetHiddenNamePrefixes(prefixes []string) { + if c == nil { + return + } + cp := make([]string, len(prefixes)) + copy(cp, prefixes) + c.HiddenNamePrefixes = cp + // TODO(GREEN): bump c.hiddenNamePrefixesGen + atomic.Store +} + +// HiddenNamePrefixesGeneration is a stub returning 0. GREEN commit returns +// the live atomic counter. +func (c *Config) HiddenNamePrefixesGeneration() uint64 { + if c == nil { + return 0 + } + return 0 +} + // SaveGeoFilter writes the geo_filter section back to config.json on disk. // Pass gf=nil to remove the filter. The rest of config.json is preserved as-is. func SaveGeoFilter(configDir string, gf *GeoFilterConfig) error { diff --git a/cmd/server/hidden_name_prefix_1181_endpoints_test.go b/cmd/server/hidden_name_prefix_1181_endpoints_test.go new file mode 100644 index 00000000..f0cd012b --- /dev/null +++ b/cmd/server/hidden_name_prefix_1181_endpoints_test.go @@ -0,0 +1,193 @@ +package main + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "sync" + "sync/atomic" + "testing" + "time" +) + +// TestHiddenNamePrefix_1181_NodeHealth asserts that /api/nodes/{pk}/health +// returns 404 for a node whose name starts with a hidden prefix — mirroring +// the existing blacklist guard at the top of handleNodeHealth. +// +// Anti-tautology: this test FAILS if the IsNameHidden guard is removed from +// handleNodeHealth (the handler would 200 with health data instead of 404). +func TestHiddenNamePrefix_1181_NodeHealth(t *testing.T) { + srv, router := setupTestServer(t) + + pk := "deadbeef00001184" + if _, err := srv.db.conn.Exec(`INSERT INTO nodes + (public_key, name, role, lat, lon, last_seen, first_seen, advert_count) + VALUES (?, ?, ?, 0, 0, '2026-06-01T00:00:00Z', '2026-06-01T00:00:00Z', 1)`, + pk, "🚫 health me", "companion"); err != nil { + t.Fatalf("insert: %v", err) + } + + get := func() *httptest.ResponseRecorder { + req := httptest.NewRequest("GET", "/api/nodes/"+pk+"/health", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + return w + } + + srv.cfg.HiddenNamePrefixes = []string{"🚫"} + w := get() + if w.Code != http.StatusNotFound { + t.Fatalf("hidden: expected 404 from /api/nodes/%s/health, got %d body=%s", pk, w.Code, w.Body.String()) + } + if strings.Contains(w.Body.String(), "health me") { + t.Fatalf("hidden: name leaked in /health 404 body: %s", w.Body.String()) + } +} + +// TestHiddenNamePrefix_1181_BulkHealth asserts /api/nodes/bulk-health filters +// out nodes whose name starts with a hidden prefix — same shape as the +// existing blacklist filter inside handleBulkHealth. +// +// Anti-tautology: remove the IsNameHidden branch from handleBulkHealth and +// the hidden node leaks back into the response array; this assertion fails. +func TestHiddenNamePrefix_1181_BulkHealth(t *testing.T) { + srv, router := setupTestServer(t) + + pk := "deadbeef00001185" + if _, err := srv.db.conn.Exec(`INSERT INTO nodes + (public_key, name, role, lat, lon, last_seen, first_seen, advert_count) + VALUES (?, ?, ?, 0, 0, '2026-06-01T00:00:00Z', '2026-06-01T00:00:00Z', 1)`, + pk, "🚫 bulk me", "companion"); err != nil { + t.Fatalf("insert: %v", err) + } + + srv.cfg.HiddenNamePrefixes = []string{"🚫"} + srv.cfg.NodeBlacklist = []string{"force-filter-branch"} // force the existing blacklist branch on so results-array path is taken + srv.cfg.SetNodeBlacklist(srv.cfg.NodeBlacklist) + + req := httptest.NewRequest("GET", "/api/nodes/bulk-health?limit=2000", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("status=%d body=%s", w.Code, w.Body.String()) + } + var arr []map[string]interface{} + if err := json.Unmarshal(w.Body.Bytes(), &arr); err != nil { + t.Fatalf("unmarshal: %v body=%s", err, w.Body.String()) + } + for _, e := range arr { + if got, _ := e["public_key"].(string); strings.EqualFold(got, pk) { + t.Fatalf("hidden node %s leaked through /api/nodes/bulk-health", pk) + } + } +} + +// TestHiddenNamePrefix_1181_Paths asserts /api/nodes/{pk}/paths returns 404 +// for a hidden-prefix node, mirroring blacklist behaviour. +func TestHiddenNamePrefix_1181_Paths(t *testing.T) { + srv, router := setupTestServer(t) + + pk := "deadbeef00001186" + if _, err := srv.db.conn.Exec(`INSERT INTO nodes + (public_key, name, role, lat, lon, last_seen, first_seen, advert_count) + VALUES (?, ?, ?, 0, 0, '2026-06-01T00:00:00Z', '2026-06-01T00:00:00Z', 1)`, + pk, "🚫 paths me", "companion"); err != nil { + t.Fatalf("insert: %v", err) + } + + srv.cfg.HiddenNamePrefixes = []string{"🚫"} + req := httptest.NewRequest("GET", "/api/nodes/"+pk+"/paths", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != http.StatusNotFound { + t.Fatalf("hidden: expected 404 from /api/nodes/%s/paths, got %d body=%s", pk, w.Code, w.Body.String()) + } +} + +// TestHiddenNamePrefix_1181_Analytics asserts /api/nodes/{pk}/analytics 404s +// for hidden-prefix nodes. +func TestHiddenNamePrefix_1181_Analytics(t *testing.T) { + srv, router := setupTestServer(t) + + pk := "deadbeef00001187" + if _, err := srv.db.conn.Exec(`INSERT INTO nodes + (public_key, name, role, lat, lon, last_seen, first_seen, advert_count) + VALUES (?, ?, ?, 0, 0, '2026-06-01T00:00:00Z', '2026-06-01T00:00:00Z', 1)`, + pk, "🚫 analytics me", "companion"); err != nil { + t.Fatalf("insert: %v", err) + } + + srv.cfg.HiddenNamePrefixes = []string{"🚫"} + req := httptest.NewRequest("GET", "/api/nodes/"+pk+"/analytics", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != http.StatusNotFound { + t.Fatalf("hidden: expected 404 from /api/nodes/%s/analytics, got %d body=%s", pk, w.Code, w.Body.String()) + } +} + +// TestHiddenNamePrefixesGeneration_Increments asserts the per-source +// generation counter bumps on every Set call — mirrors +// TestConfig_BlacklistGenerationIncrements behaviour. Cache wiring lives in +// a follow-up; the counter is the prerequisite primitive. +func TestHiddenNamePrefixesGeneration_Increments(t *testing.T) { + cfg := &Config{} + g0 := cfg.HiddenNamePrefixesGeneration() + cfg.SetHiddenNamePrefixes([]string{"🚫"}) + g1 := cfg.HiddenNamePrefixesGeneration() + if g1 != g0+1 { + t.Fatalf("first SetHiddenNamePrefixes: gen %d -> %d (want +1)", g0, g1) + } + cfg.SetHiddenNamePrefixes([]string{"🚫"}) + g2 := cfg.HiddenNamePrefixesGeneration() + if g2 != g1+1 { + t.Fatalf("second SetHiddenNamePrefixes: gen %d -> %d (want +1)", g1, g2) + } + cfg.SetHiddenNamePrefixes(nil) + g3 := cfg.HiddenNamePrefixesGeneration() + if g3 != g2+1 { + t.Fatalf("nil SetHiddenNamePrefixes: gen %d -> %d (want +1)", g2, g3) + } +} + +// TestHiddenNamePrefixes_ConcurrentAccess hammers Set + IsNameHidden from +// multiple goroutines. Doesn't assert anything beyond "doesn't panic" — +// atomic.Pointer correctness is what we're verifying, race detector is not +// in scope for this PR's CI (see PR scope). +func TestHiddenNamePrefixes_ConcurrentAccess(t *testing.T) { + cfg := &Config{} + cfg.SetHiddenNamePrefixes([]string{"🚫"}) + + var stop atomic.Bool + var wg sync.WaitGroup + + // Writer + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; !stop.Load(); i++ { + if i%2 == 0 { + cfg.SetHiddenNamePrefixes([]string{"🚫", "test"}) + } else { + cfg.SetHiddenNamePrefixes([]string{"🚫"}) + } + } + }() + + // Readers + for r := 0; r < 4; r++ { + wg.Add(1) + go func() { + defer wg.Done() + for !stop.Load() { + _ = cfg.IsNameHidden("🚫 something") + _ = cfg.IsNameHidden("normal name") + } + }() + } + + time.Sleep(250 * time.Millisecond) + stop.Store(true) + wg.Wait() +}