diff --git a/cmd/server/nodes_geofilter_optin_test.go b/cmd/server/nodes_geofilter_optin_test.go new file mode 100644 index 00000000..c96f29a2 --- /dev/null +++ b/cmd/server/nodes_geofilter_optin_test.go @@ -0,0 +1,79 @@ +package main + +import ( + "encoding/json" + "net/http/httptest" + "testing" +) + +// Regression test: configuring geo_filter alone must not change what +// GET /api/nodes returns. Before this fix, setting geo_filter silently +// hid every node outside the polygon that hadn't yet been re-tagged +// foreign_advert=1 by the ingestor (which only happens on that node's +// next ADVERT) — including the live map, which lists straight off this +// endpoint. The filter is now opt-in via ?geoFilter=1. +func TestHandleNodes_GeoFilterExcludedByDefault(t *testing.T) { + apiKey := "a-strong-api-key-for-testing" + srv, router, _ := setupGeoFilterServer(t, apiKey) + srv.setGeoFilter(&GeoFilterConfig{ + LatMin: floatPtr(53.0), LatMax: floatPtr(59.0), + LonMin: floatPtr(6.0), LonMax: floatPtr(15.0), + }) + + mustExecDB(t, srv.db, `INSERT INTO nodes (public_key, name, lat, lon, foreign_advert) VALUES ('pk-inside', 'InsideNode', 55.7, 10.5, 0)`) + mustExecDB(t, srv.db, `INSERT INTO nodes (public_key, name, lat, lon, foreign_advert) VALUES ('pk-outside-untagged', 'OutsideUntagged', 44.4, 26.1, 0)`) + mustExecDB(t, srv.db, `INSERT INTO nodes (public_key, name, lat, lon, foreign_advert) VALUES ('pk-outside-tagged', 'OutsideTagged', 52.4, 10.8, 1)`) + + names := func(w *httptest.ResponseRecorder) map[string]bool { + var body map[string]interface{} + if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil { + t.Fatalf("unmarshal: %v", err) + } + nodes, ok := body["nodes"].([]interface{}) + if !ok { + t.Fatal("expected nodes array") + } + out := make(map[string]bool, len(nodes)) + for _, n := range nodes { + m := n.(map[string]interface{}) + out[m["name"].(string)] = true + } + return out + } + + t.Run("default request returns every node regardless of geo_filter", func(t *testing.T) { + req := httptest.NewRequest("GET", "/api/nodes?limit=50", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != 200 { + t.Fatalf("expected 200, got %d", w.Code) + } + got := names(w) + for _, want := range []string{"InsideNode", "OutsideUntagged", "OutsideTagged"} { + if !got[want] { + t.Errorf("expected %s in default (unfiltered) response, got %v", want, got) + } + } + }) + + t.Run("geoFilter=1 excludes untagged out-of-polygon nodes but keeps foreign-tagged ones", func(t *testing.T) { + req := httptest.NewRequest("GET", "/api/nodes?limit=50&geoFilter=1", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != 200 { + t.Fatalf("expected 200, got %d", w.Code) + } + got := names(w) + if !got["InsideNode"] { + t.Error("expected InsideNode (within polygon) to be present") + } + if !got["OutsideTagged"] { + t.Error("expected OutsideTagged (foreign_advert=1) to be present even though it's outside the polygon — #730") + } + if got["OutsideUntagged"] { + t.Error("expected OutsideUntagged (outside polygon, not yet foreign-tagged) to be excluded when geoFilter=1") + } + }) +} + +func floatPtr(f float64) *float64 { return &f } diff --git a/cmd/server/openapi.go b/cmd/server/openapi.go index afd233db..46464ecb 100644 --- a/cmd/server/openapi.go +++ b/cmd/server/openapi.go @@ -80,6 +80,7 @@ func routeDescriptions() map[string]routeMeta { QueryParams: []paramMeta{ {Name: "role", Description: "Filter by node role", Type: "string"}, {Name: "status", Description: "Filter by status (active/stale/offline)", Type: "string"}, + {Name: "geoFilter", Description: "Set to \"1\" to exclude nodes outside the configured geo_filter (unless foreign_advert-tagged). Opt-in — omitting it returns every node regardless of geo_filter.", Type: "string"}, }}, "GET /api/nodes/search": {Summary: "Search nodes", Description: "Search nodes by name or public key prefix.", Tag: "nodes", QueryParams: []paramMeta{{Name: "q", Description: "Search query", Type: "string", Required: true}}}, "GET /api/nodes/bulk-health": {Summary: "Bulk node health", Description: "Returns health status for all nodes in one call.", Tag: "nodes"}, diff --git a/cmd/server/routes.go b/cmd/server/routes.go index 9f47fe9f..d56f0207 100644 --- a/cmd/server/routes.go +++ b/cmd/server/routes.go @@ -1426,7 +1426,16 @@ func (s *Server) handleNodes(w http.ResponseWriter, r *http.Request) { } } } - if s.cfg.GeoFilter != nil { + // Opt-in only (?geoFilter=1): configuring geo_filter alone must not + // change what /api/nodes (and therefore the live map, which lists + // straight off this endpoint) returns by default. It used to — any node + // outside the polygon and not YET foreign_advert-tagged (which only + // happens on that node's next ADVERT after geo_filter was configured) + // would silently vanish from every view the moment geo_filter was set, + // with no per-request way to see them anyway. geo_filter's own + // ingestor-side tagging (and the explicit prune-geo-filter admin flow) + // are unaffected by this — this only gates the passive declutter view. + if s.cfg.GeoFilter != nil && q.Get("geoFilter") == "1" { filtered := nodes[:0] for _, node := range nodes { // Foreign-flagged nodes (#730) are kept even when their GPS lies