From 0e3b6232a79823c0fb2085f9820505df41da1e7c Mon Sep 17 00:00:00 2001 From: wind Date: Mon, 2 Mar 2026 15:21:09 +0100 Subject: [PATCH 1/2] feat: MAC-stable device identity + label as primary display name Backend: - upsert_device() now matches by MAC first; if MAC seen at a new IP, updates ip_address while preserving label/trusted/device_type - Adds ix_devices_mac_address index (model + idempotent migration) - RiskOut gains label: str | None populated from r.device.label - device_appeared event detail now includes 'label' (parity with device_disappeared which already had it) - 3 new tests: create new, MAC-first IP update, no-MAC IP fallback Frontend: - Risk interface gains label: string | null - RisksPage: risk cards + device filter use label ?? hostname ?? ip - DeviceDetailPage: page title uses label ?? hostname ?? ip_address; subtitle shows the next level down (hostname or IP as context) Closes #111 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- backend/app/api/__init__.py | 2 + backend/app/db.py | 66 +++++++++++++----- backend/app/models/device.py | 3 + backend/app/scan_runner.py | 1 + backend/tests/test_api.py | 91 +++++++++++++++++++++++++ frontend/src/pages/DeviceDetailPage.tsx | 10 ++- frontend/src/pages/RisksPage.tsx | 9 +-- frontend/src/types/api.ts | 1 + 8 files changed, 159 insertions(+), 24 deletions(-) diff --git a/backend/app/api/__init__.py b/backend/app/api/__init__.py index 3a3a058..5efdf86 100644 --- a/backend/app/api/__init__.py +++ b/backend/app/api/__init__.py @@ -240,6 +240,7 @@ class RiskOut(BaseModel): device_id: int ip_address: str hostname: str | None + label: str | None severity: str display_severity: str # may differ from severity based on active network profile check_id: str @@ -406,6 +407,7 @@ def _risk_to_out(r, profile: str = "standard_home") -> RiskOut: # noqa: ANN001 device_id=r.device_id, ip_address=r.device.ip_address, hostname=r.device.hostname, + label=r.device.label, severity=r.severity, display_severity=display_severity_for_check(r.check_id, r.severity, profile), check_id=r.check_id, diff --git a/backend/app/db.py b/backend/app/db.py index f8c2da6..f877461 100644 --- a/backend/app/db.py +++ b/backend/app/db.py @@ -46,6 +46,13 @@ def _migrate_schema(engine) -> None: if column not in existing_cols: conn.execute(text(f"ALTER TABLE {table} ADD COLUMN {column} {col_def}")) # noqa: S608 — table/column/col_def are internal constants, not user input conn.commit() + # Ensure the MAC address index exists (CREATE INDEX IF NOT EXISTS is idempotent). + conn.execute( + text( + "CREATE INDEX IF NOT EXISTS ix_devices_mac_address ON devices (mac_address)" # noqa: S608 — DDL constant + ) + ) + conn.commit() def init_db() -> None: @@ -93,21 +100,42 @@ def upsert_device( hostname: str | None = None, os_guess: str | None = None, ) -> Device: - """Insert or update a Device row keyed on ip_address. - - If a Device with the given ip_address already exists, only non-None - fields are written so that richer data from a previous scan is never - overwritten with None. The caller is responsible for committing. - - Returns the Device instance (either existing or newly created). + """Insert or update a Device row, using MAC address as the primary identity key. + + Lookup order: + 1. If mac_address is provided, find an existing device with that MAC. + - Found at the same IP → normal update (non-None fields only). + - Found at a different IP → update ip_address to the new one, preserving + user-set fields (label, trusted, device_type). + 2. Fall back to ip_address lookup (covers devices that don't broadcast MAC, + e.g. traffic routed through a switch without ARP visibility). + 3. If no existing device is found, create a new one. + + The caller is responsible for committing. Returns the Device instance. """ from sqlalchemy import select from app.models.device import Device - stmt = select(Device).where(Device.ip_address == ip_address) - device: Device | None = session.execute(stmt).scalar_one_or_none() + device: Device | None = None + + # --- MAC-first lookup --- + if mac_address is not None: + device = session.execute( + select(Device).where(Device.mac_address == mac_address) + ).scalar_one_or_none() + if device is not None and device.ip_address != ip_address: + # Device moved to a new IP — update the address in place so + # user-assigned label/trusted/device_type are preserved. + device.ip_address = ip_address + # --- IP fallback --- + if device is None: + device = session.execute( + select(Device).where(Device.ip_address == ip_address) + ).scalar_one_or_none() + + # --- Create --- if device is None: device = Device( ip_address=ip_address, @@ -117,15 +145,17 @@ def upsert_device( os_guess=os_guess, ) session.add(device) - else: - if mac_address is not None: - device.mac_address = mac_address - if vendor is not None: - device.vendor = vendor - if hostname is not None: - device.hostname = hostname - if os_guess is not None: - device.os_guess = os_guess + return device + + # --- Update non-None scan fields (never overwrite user-set fields) --- + if mac_address is not None: + device.mac_address = mac_address + if vendor is not None: + device.vendor = vendor + if hostname is not None: + device.hostname = hostname + if os_guess is not None: + device.os_guess = os_guess return device diff --git a/backend/app/models/device.py b/backend/app/models/device.py index 0f94624..eca8499 100644 --- a/backend/app/models/device.py +++ b/backend/app/models/device.py @@ -5,6 +5,7 @@ Column, DateTime, ForeignKey, + Index, Integer, String, UniqueConstraint, @@ -22,6 +23,8 @@ class Device(Base): __table_args__ = ( # One row per IP address — upserts update in place rather than inserting duplicates. UniqueConstraint("ip_address", name="uq_devices_ip_address"), + # Index for MAC-first lookup in upsert_device(). + Index("ix_devices_mac_address", "mac_address"), ) id = Column(Integer, primary_key=True, index=True) diff --git a/backend/app/scan_runner.py b/backend/app/scan_runner.py index 01d8e63..7c845cc 100644 --- a/backend/app/scan_runner.py +++ b/backend/app/scan_runner.py @@ -323,6 +323,7 @@ def _record_scan_events( { "ip": d.ip_address, "hostname": d.hostname, + "label": d.label, "mac": d.mac_address, "vendor": d.vendor, } diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index 5c81023..7cfac6a 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -965,3 +965,94 @@ def test_wan_returns_stored_ip(client, db_engine): db.query(AppSetting).filter(AppSetting.key.in_(["wan_ip", "wan_ip_detected_at"])).delete() db.commit() db.close() + + +# ── upsert_device MAC-first identity ───────────────────────────────────────── + + +def test_upsert_device_creates_new(db_engine): + """upsert_device creates a new Device when MAC and IP are both unseen.""" + from sqlalchemy.orm import sessionmaker + + from app.db import upsert_device + from app.models.device import Device + + Session = sessionmaker(bind=db_engine) + db = Session() + try: + device = upsert_device( + db, + ip_address="192.168.99.10", + mac_address="aa:bb:cc:dd:ee:01", + hostname="newhost", + ) + db.commit() + assert device.ip_address == "192.168.99.10" + assert device.hostname == "newhost" + finally: + db.query(Device).filter(Device.mac_address == "aa:bb:cc:dd:ee:01").delete() + db.commit() + db.close() + + +def test_upsert_device_mac_first_updates_ip(db_engine): + """upsert_device uses MAC as primary key: if MAC seen at a new IP, updates IP.""" + from sqlalchemy.orm import sessionmaker + + from app.db import upsert_device + from app.models.device import Device + + Session = sessionmaker(bind=db_engine) + db = Session() + try: + # Create device at original IP + d = upsert_device( + db, + ip_address="192.168.99.20", + mac_address="aa:bb:cc:dd:ee:02", + ) + d.label = "my-server" + d.trusted = True + db.commit() + original_id = d.id + + # Simulate DHCP lease change: same MAC, new IP + d2 = upsert_device( + db, + ip_address="192.168.99.21", + mac_address="aa:bb:cc:dd:ee:02", + ) + db.commit() + + assert d2.id == original_id, "Should be the same DB row" + assert d2.ip_address == "192.168.99.21", "IP should be updated" + assert d2.label == "my-server", "User label must be preserved" + assert d2.trusted is True, "Trusted flag must be preserved" + finally: + db.query(Device).filter(Device.mac_address == "aa:bb:cc:dd:ee:02").delete() + db.commit() + db.close() + + +def test_upsert_device_no_mac_falls_back_to_ip(db_engine): + """upsert_device falls back to IP lookup when MAC is not provided.""" + from sqlalchemy.orm import sessionmaker + + from app.db import upsert_device + from app.models.device import Device + + Session = sessionmaker(bind=db_engine) + db = Session() + try: + d1 = upsert_device(db, ip_address="192.168.99.30", hostname="alpha") + db.commit() + + d2 = upsert_device(db, ip_address="192.168.99.30", hostname="alpha-updated") + db.commit() + + assert d1.id == d2.id + assert d2.hostname == "alpha-updated" + finally: + db.query(Device).filter(Device.ip_address == "192.168.99.30").delete() + db.commit() + db.close() diff --git a/frontend/src/pages/DeviceDetailPage.tsx b/frontend/src/pages/DeviceDetailPage.tsx index f49f2f6..de287e5 100644 --- a/frontend/src/pages/DeviceDetailPage.tsx +++ b/frontend/src/pages/DeviceDetailPage.tsx @@ -190,8 +190,14 @@ export function DeviceDetailPage() { {device.trusted && ( diff --git a/frontend/src/pages/RisksPage.tsx b/frontend/src/pages/RisksPage.tsx index d882fb8..ef1961c 100644 --- a/frontend/src/pages/RisksPage.tsx +++ b/frontend/src/pages/RisksPage.tsx @@ -203,7 +203,7 @@ function RiskModal({ className="text-[var(--color-accent-positive)] hover:underline" onClick={onClose} > - {risk.hostname || risk.ip_address} + {risk.label ?? risk.hostname ?? risk.ip_address} @@ -381,8 +381,8 @@ export function RisksPage() { {devices.map((d) => ( ))} @@ -428,7 +428,8 @@ export function RisksPage() {
{risks.map((risk) => { - const deviceLabel = risk.hostname || risk.ip_address; + const deviceLabel = + risk.label ?? risk.hostname ?? risk.ip_address; return (