From acccf3892f28c63af902a981d2ce03b9b456ae63 Mon Sep 17 00:00:00 2001 From: gnacho Date: Tue, 6 Oct 2026 10:05:39 +0200 Subject: [PATCH] fix(clients): deleted clients stay deleted; fleet MACs purged; sane IPs #1278, three related fixes. (1) Deleting a client now writes a tombstone (new deleted_clients table): any source that still reports the MAC online (a cached AP station entry, a collector) no longer re-admits it, and it disappears from the ghost list too. Re-adding the MAC through the allowlist clears the tombstone. (2) The base MAC of every fleet unit (routers.mac plus each agent's bridge MAC) is purged from the registry on every cycle - the units' own management addresses were learned as clients through the switches' FDB and lingered offline forever. (3) Device IPs that are not parseable IPs (some sources report a dash-formatted MAC hostname in the ip field) are dropped at ingestion and on display. --- server-go/internal/adapters/device_seen.go | 66 ++++++++++++++++++- .../internal/adapters/device_seen_test.go | 54 +++++++++++++++ server-go/internal/db/db.go | 39 +++++++++++ .../internal/httpapi/device_overrides.go | 7 ++ server-go/internal/httpapi/settings.go | 1 + 5 files changed, 165 insertions(+), 2 deletions(-) diff --git a/server-go/internal/adapters/device_seen.go b/server-go/internal/adapters/device_seen.go index 632da332..5de130a5 100644 --- a/server-go/internal/adapters/device_seen.go +++ b/server-go/internal/adapters/device_seen.go @@ -1,6 +1,7 @@ package adapters import ( + "net" "strings" ) @@ -22,10 +23,61 @@ func normSeenMAC(mac string) string { // #1145: guarda también el último nombre conocido (hostname de lease o nombre // visible) para que un cliente que deje de verse conserve su nombre en la // lista (el nombre nuevo pisa al viejo solo si no es vacío). +// saneIP: nil si el valor no es una IP (#1278). Algunas fuentes reportan +// el hostname con formato MAC-con-guiones en el campo ip y acaba pintándose +// en la columna IP de la tabla. +func saneIP(ip string) string { + if ip == "" { + return "" + } + if net.ParseIP(ip) == nil { + return "" + } + return ip +} + +// purgeFleetSeen (#1278): borra del registro la MAC base de las unidades de +// flota. Las bocas/gestiones de los propios equipos se aprendían como +// "clientes" vía FDB de los switches y quedaban como fantasmas offline para +// siempre. Purga de MACs BASE (routers.mac): las por-interfaz las cubre la +// lápida del borrado manual. +func (l *Live) purgeFleetSeen() { + macs := map[string]bool{} + // MAC base de cada unidad (tabla routers). + if rows, err := l.db.Query("SELECT mac FROM routers WHERE mac IS NOT NULL AND mac != ''"); err == nil { + for rows.Next() { + var m string + if err := rows.Scan(&m); err == nil { + if n := normSeenMAC(m); n != "" { + macs[n] = true + } + } + } + rows.Close() + } + // MAC de bridge del último push de cada agente (la "mgmt" que los + // switches aprenden por FDB y reaparece como cliente fantasma). + l.mu.Lock() + for _, p := range l.lastPolled { + if m := normSeenMAC(p.brMac); m != "" { + macs[m] = true + } + } + l.mu.Unlock() + for m := range macs { + _, _ = l.db.Exec("DELETE FROM device_seen WHERE mac = ?", m) + } +} + func (l *Live) noteDevicesSeen(devices []Device, nowMs int64) { if l.db == nil { return } + l.purgeFleetSeen() + tomb, err := l.db.DeletedClients() + if err != nil { + tomb = map[string]bool{} + } tx, err := l.db.DB.Begin() if err != nil { return @@ -47,11 +99,14 @@ func (l *Live) noteDevicesSeen(devices []Device, nowMs int64) { if mac == "" { continue } + if tomb[mac] { + continue // #1278: borrado a mano, no re-alta + } name := d.Hostname if name == "" && d.Name != "" && !strings.EqualFold(d.Name, d.MAC) { name = d.Name } - if _, err := stmt.Exec(mac, nowMs, nowMs, name, d.IP); err != nil { + if _, err := stmt.Exec(mac, nowMs, nowMs, name, saneIP(d.IP)); err != nil { return } } @@ -130,12 +185,19 @@ func (l *Live) ghostDevices(devices []Device) []Device { revAlias[canonical] = append(revAlias[canonical], alias) } } + tomb, err := l.db.DeletedClients() + if err != nil { + tomb = map[string]bool{} + } var out []Device for rawMac, s := range byMac { mac := normSeenMAC(rawMac) if live[mac] { continue } + if tomb[mac] { + continue // #1278: borrado a mano, fuera de la lista de fantasmas + } if canonical, linked := links[mac]; linked { if _, canonRegistered := byMac[canonical]; canonRegistered { continue @@ -145,7 +207,7 @@ func (l *Live) ghostDevices(devices []Device) []Device { ID: strings.ToLower(strings.ReplaceAll(mac, ":", "-")), MAC: mac, Type: "desconocido", - IP: s.ip, + IP: saneIP(s.ip), Online: false, Band: "—", FirstSeenMs: s.first, diff --git a/server-go/internal/adapters/device_seen_test.go b/server-go/internal/adapters/device_seen_test.go index 099e7747..9e60bee6 100644 --- a/server-go/internal/adapters/device_seen_test.go +++ b/server-go/internal/adapters/device_seen_test.go @@ -107,3 +107,57 @@ func TestNoteDevicesSeenKeepsLastName(t *testing.T) { t.Fatalf("ghosts = %+v, want 1 con hostname iphone", ghosts) } } + +// #1278: un cliente borrado a mano (lápida) no vuelve a alta aunque una +// fuente lo siga reportando online, y no sale en la lista de fantasmas. +// La re-alta manual (allowlist) quita la lápida. +func TestDeletedClientTombstone(t *testing.T) { + d := openLiveTestDB(t) + l := &Live{db: d} + mac := "12:77:BC:5A:5A:FA" + if err := d.AddDeletedClient(mac); err != nil { + t.Fatal(err) + } + devs := []Device{{MAC: mac, Online: true, Name: "iphone-viejo"}} + l.noteDevicesSeen(devs, 1000) + + count := func() int { + t.Helper() + rows, err := d.Query("SELECT COUNT(*) FROM device_seen WHERE mac = ?", mac) + if err != nil { + t.Fatal(err) + } + defer rows.Close() + if !rows.Next() { + t.Fatal("sin fila de conteo") + } + var n int + if err := rows.Scan(&n); err != nil { + t.Fatal(err) + } + return n + } + + if n := count(); n != 0 { + t.Errorf("la MAC enterrada se dio de alta igual (%d filas)", n) + } + + // Fantasmas: la MAC no sale aunque tenga registro previo. + if _, err := d.Exec("INSERT INTO device_seen (mac, first_seen, last_seen) VALUES (?, 1, 2)", mac); err != nil { + t.Fatal(err) + } + for _, g := range l.ghostDevices(nil) { + if g.MAC == mac { + t.Errorf("la MAC enterrada sale como fantasma: %+v", g) + } + } + + // Re-alta manual: la lápida se quita y vuelve a alta normal. + if err := d.DeleteDeletedClient(mac); err != nil { + t.Fatal(err) + } + l.noteDevicesSeen(devs, 3000) + if n := count(); n != 1 { + t.Errorf("tras quitar la lápida la MAC se da de alta (%d filas)", n) + } +} diff --git a/server-go/internal/db/db.go b/server-go/internal/db/db.go index efe203eb..b3b43c06 100644 --- a/server-go/internal/db/db.go +++ b/server-go/internal/db/db.go @@ -133,6 +133,13 @@ CREATE TABLE IF NOT EXISTS device_seen ( first_seen INTEGER NOT NULL, last_seen INTEGER NOT NULL ); +-- #1278: lápidas de clientes borrados a mano. Sin esto, cualquier fuente +-- que vuelva a reportar la MAC online (una entrada cacheada del AP, un +-- colector) la re-alta y el borrado "no funciona". +CREATE TABLE IF NOT EXISTS deleted_clients ( + mac TEXT PRIMARY KEY, + deleted_at INTEGER NOT NULL +); -- #1151: cliente con varias MACs (un hostname, nunca simultáneas): cada fila -- dice "esta MAC es el mismo cliente que canonical". Clave = alias; el -- canónico no tiene fila propia (ausencia de fila = MAC canónica). @@ -947,6 +954,38 @@ func (d *DB) UpsertKnownMac(k KnownMac) error { return err } +// AddDeletedClient registra una lápida: el usuario borró este cliente a mano +// y NO quiere que vuelva a alta solo (#1278). +func (d *DB) AddDeletedClient(mac string) error { + _, err := d.Exec("INSERT INTO deleted_clients (mac, deleted_at) VALUES (?, ?) ON CONFLICT(mac) DO UPDATE SET deleted_at=excluded.deleted_at", mac, NowMS()) + return err +} + +// DeletedClients devuelve el set de MACs enterradas. +func (d *DB) DeletedClients() (map[string]bool, error) { + rows, err := d.Query("SELECT mac FROM deleted_clients") + if err != nil { + return nil, err + } + defer rows.Close() + out := map[string]bool{} + for rows.Next() { + var mac string + if err := rows.Scan(&mac); err != nil { + continue + } + out[mac] = true + } + return out, rows.Err() +} + +// DeleteDeletedClient quita la lápida: el usuario vuelve a dar de alta la +// MAC (allowlist/alta guiada), así que puede reaparecer con normalidad. +func (d *DB) DeleteDeletedClient(mac string) error { + _, err := d.Exec("DELETE FROM deleted_clients WHERE mac = ?", mac) + return err +} + // DeleteKnownMac quita una MAC de la allowlist. func (d *DB) DeleteKnownMac(mac string) error { _, err := d.Exec("DELETE FROM known_macs WHERE mac = ?", mac) diff --git a/server-go/internal/httpapi/device_overrides.go b/server-go/internal/httpapi/device_overrides.go index 3b73d400..533cd7c3 100644 --- a/server-go/internal/httpapi/device_overrides.go +++ b/server-go/internal/httpapi/device_overrides.go @@ -199,6 +199,13 @@ func (s *server) handleDeviceDelete(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusInternalServerError, "storage_error", "") return } + // #1278: lápida. Sin ella, la siguiente fuente que reporte la MAC como + // online (una entrada cacheada del AP, un colector) la re-daba de alta y + // el borrado "no funcionaba". + if err := s.db.AddDeletedClient(mac); err != nil { + writeError(w, http.StatusInternalServerError, "storage_error", "") + return + } if n, _ := res.RowsAffected(); n == 0 { writeError(w, http.StatusNotFound, "not_found", "") return diff --git a/server-go/internal/httpapi/settings.go b/server-go/internal/httpapi/settings.go index 1e87e802..32d445a4 100644 --- a/server-go/internal/httpapi/settings.go +++ b/server-go/internal/httpapi/settings.go @@ -380,6 +380,7 @@ func (s *server) registerKnownMacsRoutes(mux *http.ServeMux) { return } if err := s.db.UpsertKnownMac(db.KnownMac{MAC: mac, Name: body.Name, Note: body.Note}); err != nil { + _ = s.db.DeleteDeletedClient(mac) // #1278: re-alta manual quita la lápida writeError(w, http.StatusInternalServerError, "db_error") return }