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 }