dandri/meshcore-analyzer
1
0
Fork 0
mirror of https://github.com/Kpa-clawbot/meshcore-analyzer.git synced 2026-09-01 23:28:19 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(geo-prune): address PR #738 review feedback

Browse Source 
- Fix TOCTOU race: confirm now requires pubkeys from preview in request
  body; server intersects with still-outside nodes so exactly the
  previewed set is deleted (no more, no less)
- Add cascade comment to DeleteNodesByPubkeys documenting that only the
  nodes table is affected (no FK constraints today)
- Log each deleted node by name + pubkey for operator visibility
- Return deleted node list in confirm response so UI shows what happened
- Check lat/lon nil directly instead of passing 0.0 to NodePassesGeoFilter
- Update confirm test to send pubkeys body; add test for missing body → 400

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
efiten
2026-05-01 14:12:35 +02:00
co-authored by Claude Sonnet 4.6
parent 79162d8bdb
commit 0f565e4aae
4 changed files with 63 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/server/db.go
+2
View File
@@ -2384,6 +2384,8 @@ func (db *DB) GetNodesForGeoPrune() ([]NodeForGeoPrune, error) {
}
// DeleteNodesByPubkeys deletes nodes by their public keys and returns the count deleted.
// Only the nodes table is affected — references in transmissions or other tables are
// not cascaded (no FK constraints exist today; revisit if schema adds them).
func (db *DB) DeleteNodesByPubkeys(pubkeys []string) (int64, error) {
if len(pubkeys) == 0 {
return 0, nil
cmd/server/routes.go
+32 -10
View File
@@ -2654,6 +2654,8 @@ func (s *Server) handleAdminPrune(w http.ResponseWriter, r *http.Request) {
// handlePruneGeoFilter identifies (dry_run=true, default) or deletes (confirm=true)
// nodes whose GPS coordinates fall outside the currently configured geo_filter.
// Nodes with no GPS fix are always kept. Requires geo_filter to be configured.
// Confirm requires the pubkeys from the preview in the request body to prevent
// TOCTOU races: only nodes in the passed list AND still outside the filter are deleted.
func (s *Server) handlePruneGeoFilter(w http.ResponseWriter, r *http.Request) {
if s.cfg.GeoFilter == nil || len(s.cfg.GeoFilter.Polygon) < 3 {
writeError(w, http.StatusBadRequest, "no geo_filter configured")
@@ -2675,14 +2677,10 @@ func (s *Server) handlePruneGeoFilter(w http.ResponseWriter, r *http.Request) {
var outside []nodeResult
for _, n := range nodes {
var lat, lon float64
if n.Lat != nil {
lat = *n.Lat
if n.Lat == nil || n.Lon == nil {
continue // no GPS — always keep
}
if n.Lon != nil {
lon = *n.Lon
}
if !NodePassesGeoFilter(lat, lon, s.cfg.GeoFilter) {
if !NodePassesGeoFilter(*n.Lat, *n.Lon, s.cfg.GeoFilter) {
outside = append(outside, nodeResult{PubKey: n.PubKey, Name: n.Name, Lat: n.Lat, Lon: n.Lon})
}
}
@@ -2697,9 +2695,29 @@ func (s *Server) handlePruneGeoFilter(w http.ResponseWriter, r *http.Request) {
return
}
// Confirmed — delete the nodes
pubkeys := make([]string, len(outside))
for i, n := range outside {
// Confirmed delete — require pubkeys from the preview to prevent TOCTOU:
// only nodes that were shown in preview AND are still outside the filter are deleted.
var body struct {
Pubkeys []string `json:"pubkeys"`
}
if err := json.NewDecoder(r.Body).Decode(&body); err != nil || len(body.Pubkeys) == 0 {
writeError(w, http.StatusBadRequest, "confirm requires pubkeys from preview in request body")
return
}
allowed := make(map[string]bool, len(body.Pubkeys))
for _, pk := range body.Pubkeys {
allowed[pk] = true
}
var toDelete []nodeResult
for _, n := range outside {
if allowed[n.PubKey] {
toDelete = append(toDelete, n)
}
}
pubkeys := make([]string, len(toDelete))
for i, n := range toDelete {
pubkeys[i] = n.PubKey
}
deleted, err := s.db.DeleteNodesByPubkeys(pubkeys)
@@ -2707,10 +2725,14 @@ func (s *Server) handlePruneGeoFilter(w http.ResponseWriter, r *http.Request) {
writeError(w, http.StatusInternalServerError, "delete failed")
return
}
for _, n := range toDelete {
log.Printf("[geo-prune] deleted node %q (%s)", n.Name, n.PubKey)
}
log.Printf("[geo-prune] deleted %d nodes outside geo filter", deleted)
writeJSON(w, map[string]interface{}{
"dryRun": false,
"deleted": deleted,
"nodes": toDelete,
})
}
cmd/server/routes_test.go
+24 -5
View File
@@ -4263,23 +4263,29 @@ func TestPruneGeoFilterEndpoint(t *testing.T) {
t.Run("confirm=true deletes outside nodes", func(t *testing.T) {
srv, router := setupPruneGeoFilterServer(t, apiKey, gf)
req := httptest.NewRequest("POST", "/api/admin/prune-geo-filter?confirm=true", nil)
body := strings.NewReader(`{"pubkeys":["aaaa111122223333"]}`)
req := httptest.NewRequest("POST", "/api/admin/prune-geo-filter?confirm=true", body)
req.Header.Set("X-API-Key", apiKey)
req.Header.Set("Content-Type", "application/json")
w := httptest.NewRecorder()
router.ServeHTTP(w, req)
if w.Code != 200 {
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
}
var body map[string]interface{}
json.Unmarshal(w.Body.Bytes(), &body)
if body["dryRun"] != false {
var resp map[string]interface{}
json.Unmarshal(w.Body.Bytes(), &resp)
if resp["dryRun"] != false {
t.Error("expected dryRun=false")
}
deleted, _ := body["deleted"].(float64)
deleted, _ := resp["deleted"].(float64)
if deleted != 1 {
t.Errorf("expected 1 deleted, got %v", deleted)
}
nodes, _ := resp["nodes"].([]interface{})
if len(nodes) != 1 {
t.Errorf("expected 1 node in response, got %d", len(nodes))
}
// Verify node is actually gone from DB
var count int
@@ -4294,6 +4300,19 @@ func TestPruneGeoFilterEndpoint(t *testing.T) {
}
})
t.Run("confirm=true without pubkeys body returns 400", func(t *testing.T) {
_, router := setupPruneGeoFilterServer(t, apiKey, gf)
req := httptest.NewRequest("POST", "/api/admin/prune-geo-filter?confirm=true", nil)
req.Header.Set("X-API-Key", apiKey)
w := httptest.NewRecorder()
router.ServeHTTP(w, req)
if w.Code != http.StatusBadRequest {
t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String())
}
})
t.Run("returns 400 when no geo filter configured", func(t *testing.T) {
_, router := setupPruneGeoFilterServer(t, apiKey, nil)
public/customize-v2.js
+5 -2
View File
@@ -1464,9 +1464,11 @@
if (!apiKey) { _gfPruneMsg(container, 'API key required.', false); return; }
var count = _gfPruneNodes.length;
if (!confirm('Delete ' + count + ' node' + (count !== 1 ? 's' : '') + ' from the database? This cannot be undone.')) return;
var pubkeys = _gfPruneNodes.map(function (n) { return n.pubkey; });
fetch('/api/admin/prune-geo-filter?confirm=true', {
method: 'POST',
headers: { 'X-API-Key': apiKey }
headers: { 'X-API-Key': apiKey, 'Content-Type': 'application/json' },
body: JSON.stringify({ pubkeys: pubkeys })
}).then(function (r) {
if (!r.ok) return r.json().then(function (e) { throw new Error(e.error || ('HTTP ' + r.status)); });
return r.json();
@@ -1474,7 +1476,8 @@
_gfPruneNodes = [];
var resultEl = container.querySelector('#cv2-gf-prune-result');
if (resultEl) resultEl.style.display = 'none';
_gfPruneMsg(container, 'Deleted ' + data.deleted + ' node' + (data.deleted !== 1 ? 's' : '') + '.', true);
var n = data.deleted;
_gfPruneMsg(container, 'Deleted ' + n + ' node' + (n !== 1 ? 's' : '') + '.', true);
}).catch(function (e) { _gfPruneMsg(container, 'Error: ' + e.message, false); });
}