feat(database): add cleanup for dangling parentMAC references and related tests

front/js/ui_components.js

@@ -989,14 +989,14 @@ function renderDeviceLink(data, container, useName = false) {
       'data-alertdown': device.devAlertDown,
       'data-sleeping': device.devIsSleeping || 0,
       'data-archived': device.devIsArchived || 0,
-      'data-isnew':    device.devIsNew       || 0,
+      'data-isnew':    device.devIsNew      || 0,
       'data-icon': device.devIcon
     });
 
   return `
     <a href="${badge.url}" target="_blank">
       <span class="custom-chip">
-        <span class="iconPreview">${atob(device.devIcon)}</span>
+        <span class="iconPreview">${device.devIcon ? atob(device.devIcon) : ''}</span>
         ${useName ? encodeSpecialChars(device.devName) : data.text}
         <span>
           (${badge.iconHtml})

server/database.py

@@ -16,6 +16,8 @@ from db.db_upgrade import (
     ensure_Settings,
     ensure_Indexes,
     ensure_mac_lowercase_triggers,
+    ensure_dangling_parentmac_cleanup_trigger,
+    cleanup_existing_dangling_parentmac,
     migrate_to_camelcase,
     migrate_timestamps_to_utc,
 )
@@ -225,6 +227,10 @@ class DB:
             # Normalization triggers
             ensure_mac_lowercase_triggers(self.sql)
 
+            # Prevent/repair dangling devParentMAC references left by deleted devices
+            ensure_dangling_parentmac_cleanup_trigger(self.sql)
+            cleanup_existing_dangling_parentmac(self.sql)
+
             # Device history table + audit triggers
             ensure_deviceshistory_table(self.sql)
             ensure_deviceshistory_triggers(self.sql)

server/db/db_upgrade.py

@@ -146,6 +146,74 @@ def ensure_mac_lowercase_triggers(sql):
         return False
 
 
+# Sentinel devParentMAC values that are never actual device references
+PARENT_MAC_SENTINELS = ("", "internet", "null")
+
+
+def ensure_dangling_parentmac_cleanup_trigger(sql):
+    """
+    Ensures a trigger exists that clears devParentMAC/devParentMACSource on any
+    device that referenced a device MAC which was just deleted, preventing
+    dangling Parent Node references.
+
+    Note: this intentionally does NOT touch the NEWDEV_devParentMAC setting.
+    Settings are sourced from app.conf and get re-imported verbatim on every
+    restart (see importConfigs()), so a DB-only fix here would be silently
+    reverted. Stale NEWDEV_devParentMAC values are instead guarded against at
+    the point of use in create_new_devices() (server/scan/device_handling.py).
+    """
+    try:
+        sql.execute(
+            "SELECT name FROM sqlite_master WHERE type='trigger' AND name='trg_clear_dangling_parentmac_on_delete'"
+        )
+        if not sql.fetchone():
+            mylog("verbose", ["[db_upgrade] Creating trigger 'trg_clear_dangling_parentmac_on_delete'"])
+            sql.execute("""
+                CREATE TRIGGER trg_clear_dangling_parentmac_on_delete
+                AFTER DELETE ON Devices
+                FOR EACH ROW
+                WHEN OLD.devMac IS NOT NULL AND OLD.devMac != ''
+                BEGIN
+                    UPDATE Devices
+                    SET devParentMAC = '', devParentMACSource = ''
+                    WHERE LOWER(devParentMAC) = LOWER(OLD.devMac);
+                END;
+            """)
+
+        return True
+
+    except Exception as e:
+        mylog("none", [f"[db_upgrade] ERROR while ensuring dangling parentMAC trigger: {e}"])
+        return False
+
+
+def cleanup_existing_dangling_parentmac(sql) -> bool:
+    """
+    One-time/idempotent cleanup for installations that already have devParentMAC
+    values pointing to a MAC no longer present in Devices. The delete trigger
+    only prevents new dangling references going forward, so this repairs data
+    left over from before the trigger existed.
+    """
+    try:
+        sentinel_list = ", ".join(f"'{v}'" for v in PARENT_MAC_SENTINELS)
+
+        sql.execute(f"""
+            UPDATE Devices
+            SET devParentMAC = '', devParentMACSource = ''
+            WHERE devParentMAC IS NOT NULL
+              AND LOWER(devParentMAC) NOT IN ({sentinel_list})
+              AND LOWER(devParentMAC) NOT IN (SELECT LOWER(devMac) FROM Devices)
+        """)
+        if sql.rowcount > 0:
+            mylog("verbose", [f"[db_upgrade] Cleared {sql.rowcount} dangling devParentMAC reference(s)"])
+
+        return True
+
+    except Exception as e:
+        mylog("none", [f"[db_upgrade] ERROR while cleaning up dangling parentMAC references: {e}"])
+        return False
+
+
 def ensure_views(sql) -> bool:
     """
     Ensures required views exist.

server/scan/device_handling.py

@@ -730,6 +730,22 @@ def create_new_devices(db):
     mylog("debug", f"[New Devices] Collecting New Devices Query: {query}")
     current_scan_data = sql.execute(query).fetchall()
 
+    # Resolve the default Parent Node setting once and guard against it pointing
+    # to a MAC that no longer exists (e.g. that device was since deleted) -
+    # falling back to unset rather than seeding new devices with a dangling reference.
+    default_parent_mac_setting = get_setting_value("NEWDEV_devParentMAC")
+    if default_parent_mac_setting:
+        existing_device_macs = {
+            str(row[0]).lower() for row in sql.execute("SELECT devMac FROM Devices").fetchall() if row[0]
+        }
+        if default_parent_mac_setting.lower() not in existing_device_macs:
+            mylog(
+                "verbose",
+                f"[New Devices] NEWDEV_devParentMAC '{default_parent_mac_setting}' no longer "
+                "exists in Devices - treating as unset",
+            )
+            default_parent_mac_setting = ""
+
     for row in current_scan_data:
         (
             scanMac,
@@ -771,7 +787,7 @@ def create_new_devices(db):
             scanParentMAC
             if scanParentMAC and scanMac.lower() != "internet"
             else (
-                get_setting_value("NEWDEV_devParentMAC")
+                default_parent_mac_setting
                 if scanMac.lower() != "internet"
                 else "null"
             )

test/db/test_dangling_parentmac_cleanup.py

@@ -0,0 +1,172 @@
+"""
+Unit tests for dangling devParentMAC cleanup.
+
+Tests verify that:
+- Deleting a device clears devParentMAC/devParentMACSource on devices that
+  referenced it as their Parent Node.
+- Sentinel values ('', 'internet', 'null') are never touched.
+- Valid parent references are left untouched.
+- The one-time migration repairs pre-existing dangling data and is idempotent.
+
+Note: the NEWDEV_devParentMAC *setting* is intentionally NOT handled here.
+Settings are sourced from app.conf and get re-imported verbatim on every
+restart, so a DB-only fix would be silently reverted. That case is instead
+guarded against at the point of use in create_new_devices() — see
+test/scan/test_field_lock_scan_integration.py.
+"""
+
+import sys
+import os
+import pytest
+import sqlite3
+import tempfile
+
+INSTALL_PATH = os.getenv('NETALERTX_APP', '/app')
+sys.path.extend([f"{INSTALL_PATH}/server/plugins", f"{INSTALL_PATH}/server"])
+
+from db.db_upgrade import (  # noqa: E402
+    ensure_dangling_parentmac_cleanup_trigger,
+    cleanup_existing_dangling_parentmac,
+)
+
+
+@pytest.fixture
+def temp_db():
+    """Create a temporary database for testing"""
+    fd, db_path = tempfile.mkstemp(suffix='.db')
+    os.close(fd)
+
+    conn = sqlite3.connect(db_path)
+    cursor = conn.cursor()
+
+    cursor.execute("""
+        CREATE TABLE Devices (
+            devMac TEXT PRIMARY KEY COLLATE NOCASE,
+            devParentMAC TEXT,
+            devParentMACSource TEXT
+        )
+    """)
+
+    conn.commit()
+
+    yield cursor, conn
+
+    conn.close()
+    os.unlink(db_path)
+
+
+class TestDanglingParentMacTrigger:
+    """Test suite for the AFTER DELETE cleanup trigger"""
+
+    def test_trigger_clears_dependent_devices_on_delete(self, temp_db):
+        cursor, conn = temp_db
+        assert ensure_dangling_parentmac_cleanup_trigger(cursor) is True
+
+        cursor.execute(
+            "INSERT INTO Devices (devMac, devParentMAC, devParentMACSource) VALUES (?, ?, ?)",
+            ("aa:bb:cc:dd:ee:01", "", ""),
+        )
+        cursor.execute(
+            "INSERT INTO Devices (devMac, devParentMAC, devParentMACSource) VALUES (?, ?, ?)",
+            ("aa:bb:cc:dd:ee:02", "aa:bb:cc:dd:ee:01", "NEWDEV"),
+        )
+        conn.commit()
+
+        cursor.execute("DELETE FROM Devices WHERE devMac = ?", ("aa:bb:cc:dd:ee:01",))
+        conn.commit()
+
+        cursor.execute(
+            "SELECT devParentMAC, devParentMACSource FROM Devices WHERE devMac = ?",
+            ("aa:bb:cc:dd:ee:02",),
+        )
+        row = cursor.fetchone()
+        assert row == ("", "")
+
+    def test_trigger_ignores_unrelated_deletes(self, temp_db):
+        cursor, conn = temp_db
+        ensure_dangling_parentmac_cleanup_trigger(cursor)
+
+        cursor.execute(
+            "INSERT INTO Devices (devMac, devParentMAC) VALUES (?, ?)",
+            ("aa:bb:cc:dd:ee:01", "internet"),
+        )
+        cursor.execute(
+            "INSERT INTO Devices (devMac, devParentMAC) VALUES (?, ?)",
+            ("aa:bb:cc:dd:ee:02", ""),
+        )
+        conn.commit()
+
+        cursor.execute("DELETE FROM Devices WHERE devMac = ?", ("aa:bb:cc:dd:ee:02",))
+        conn.commit()
+
+        cursor.execute(
+            "SELECT devParentMAC FROM Devices WHERE devMac = ?", ("aa:bb:cc:dd:ee:01",)
+        )
+        assert cursor.fetchone() == ("internet",)
+
+
+class TestCleanupExistingDanglingParentMac:
+    """Test suite for the one-time/idempotent data repair migration"""
+
+    def test_cleanup_clears_dangling_reference(self, temp_db):
+        cursor, conn = temp_db
+
+        cursor.execute(
+            "INSERT INTO Devices (devMac, devParentMAC, devParentMACSource) VALUES (?, ?, ?)",
+            ("aa:bb:cc:dd:ee:02", "aa:bb:cc:dd:ee:99", "NEWDEV"),
+        )
+        conn.commit()
+
+        assert cleanup_existing_dangling_parentmac(cursor) is True
+
+        cursor.execute(
+            "SELECT devParentMAC, devParentMACSource FROM Devices WHERE devMac = ?",
+            ("aa:bb:cc:dd:ee:02",),
+        )
+        assert cursor.fetchone() == ("", "")
+
+    def test_cleanup_preserves_valid_and_sentinel_values(self, temp_db):
+        cursor, conn = temp_db
+
+        cursor.execute(
+            "INSERT INTO Devices (devMac, devParentMAC) VALUES (?, ?)",
+            ("aa:bb:cc:dd:ee:01", ""),
+        )
+        cursor.execute(
+            "INSERT INTO Devices (devMac, devParentMAC) VALUES (?, ?)",
+            ("aa:bb:cc:dd:ee:02", "aa:bb:cc:dd:ee:01"),
+        )
+        cursor.execute(
+            "INSERT INTO Devices (devMac, devParentMAC) VALUES (?, ?)",
+            ("aa:bb:cc:dd:ee:03", "internet"),
+        )
+        conn.commit()
+
+        cleanup_existing_dangling_parentmac(cursor)
+
+        cursor.execute(
+            "SELECT devParentMAC FROM Devices WHERE devMac = ?", ("aa:bb:cc:dd:ee:02",)
+        )
+        assert cursor.fetchone() == ("aa:bb:cc:dd:ee:01",)
+
+        cursor.execute(
+            "SELECT devParentMAC FROM Devices WHERE devMac = ?", ("aa:bb:cc:dd:ee:03",)
+        )
+        assert cursor.fetchone() == ("internet",)
+
+    def test_cleanup_is_idempotent(self, temp_db):
+        cursor, conn = temp_db
+
+        cursor.execute(
+            "INSERT INTO Devices (devMac, devParentMAC) VALUES (?, ?)",
+            ("aa:bb:cc:dd:ee:02", "aa:bb:cc:dd:ee:99"),
+        )
+        conn.commit()
+
+        assert cleanup_existing_dangling_parentmac(cursor) is True
+        assert cleanup_existing_dangling_parentmac(cursor) is True
+
+        cursor.execute(
+            "SELECT devParentMAC FROM Devices WHERE devMac = ?", ("aa:bb:cc:dd:ee:02",)
+        )
+        assert cursor.fetchone() == ("",)

test/scan/test_field_lock_scan_integration.py

@@ -231,6 +231,66 @@ def test_create_new_devices_sets_sources(scan_db_for_new_devices):
     assert row["devVlanSource"] == "NEWDEV"
 
 
+def test_create_new_devices_ignores_dangling_newdev_parentmac(scan_db_for_new_devices):
+    """A stale NEWDEV_devParentMAC pointing to a since-deleted device is treated as unset,
+    instead of seeding the new device with another dangling Parent Node reference."""
+    cur = scan_db_for_new_devices.cursor()
+    cur.execute(
+        """
+        INSERT INTO CurrentScan (
+            scanMac, scanName, scanVendor, scanSourcePlugin, scanLastIP,
+            scanSyncHubNode, scanParentMAC, scanParentPort,
+            scanSite, scanSSID, scanType
+        ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
+        """,
+        (
+            "aa:bb:cc:dd:ee:11",
+            "DeviceTwo",
+            "AcmeVendor",
+            "ARPSCAN",
+            "192.168.1.11",
+            "",
+            "",  # no parent reported by the scan itself
+            "",
+            "",
+            "",
+            "",
+        ),
+    )
+    scan_db_for_new_devices.commit()
+
+    settings = {
+        "NEWDEV_devType": "default-type",
+        # points to a MAC that does not (and never did, in this test) exist in Devices
+        "NEWDEV_devParentMAC": "99:99:99:99:99:99",
+        "NEWDEV_devOwner": "owner",
+        "NEWDEV_devGroup": "group",
+        "NEWDEV_devComments": "",
+        "NEWDEV_devLocation": "",
+        "NEWDEV_devCustomProps": "",
+        "NEWDEV_devParentRelType": "uplink",
+        "SYNC_node_name": "SYNCNODE",
+    }
+
+    db = Mock()
+    db.sql_connection = scan_db_for_new_devices
+    db.sql = cur
+    db.commitDB = scan_db_for_new_devices.commit
+
+    with patch.multiple(
+        device_handling,
+        get_setting_value=Mock(side_effect=lambda key: settings.get(key, "")),
+        safe_int=Mock(return_value=0),
+    ):
+        device_handling.create_new_devices(db)
+
+    row = cur.execute(
+        "SELECT devParentMAC FROM Devices WHERE devMac = ?", ("aa:bb:cc:dd:ee:11",)
+    ).fetchone()
+
+    assert row["devParentMAC"] == ""
+
+
 def test_scan_updates_newdev_device_name(scan_db, mock_device_handlers):
     """Scanner discovers name for device with NEWDEV source."""
     cur = scan_db.cursor()