diff --git a/backend/app/services/scanner.py b/backend/app/services/scanner.py index 909f268..c19d1a5 100644 --- a/backend/app/services/scanner.py +++ b/backend/app/services/scanner.py @@ -410,6 +410,38 @@ def _mock_scan(target: str) -> list[dict[str, Any]]: ] +async def _dedupe_pending_by_ip(db: AsyncSession) -> int: + """Collapse duplicate non-hidden inventory rows that share an IP into one. + + Keeps an ``approved`` row when present (it carries canvas-link semantics), + otherwise the oldest row, and deletes the rest. Returns the number deleted. + """ + rows = (await db.execute( + select(PendingDevice) + .where(PendingDevice.status != "hidden", PendingDevice.ip.isnot(None)) + .order_by(PendingDevice.discovered_at) + )).scalars().all() + + by_ip: dict[str, list[PendingDevice]] = {} + for row in rows: + if row.ip is None: # guarded by the query, but keeps the type checker happy + continue + by_ip.setdefault(row.ip, []).append(row) + + deleted = 0 + for group in by_ip.values(): + if len(group) < 2: + continue + keep = next((r for r in group if r.status == "approved"), group[0]) + for dup in group: + if dup is not keep: + await db.delete(dup) + deleted += 1 + if deleted: + await db.commit() + return deleted + + async def run_scan( ranges: list[str], db: AsyncSession, @@ -440,6 +472,11 @@ async def run_scan( ) hidden_ips: set[str] = {row[0] for row in hidden_ips_result.fetchall()} + # Collapse any pre-existing duplicate inventory rows (same IP, non-hidden) + # left over from older scans, so the device shows up exactly once even if + # it isn't re-discovered this run (e.g. now offline). + await _dedupe_pending_by_ip(db) + # Start mDNS discovery in the background while nmap scans run mdns_task = asyncio.create_task(_mdns_discover()) @@ -468,19 +505,30 @@ async def run_scan( services = fingerprint_ports(open_ports) suggested_type = suggest_node_type(open_ports, host.get("mac")) - existing_result = await db.execute( - select(PendingDevice).where( - PendingDevice.ip == ip, - PendingDevice.status == "pending", - ) - ) - existing = existing_result.scalar_one_or_none() - if existing: - existing.mac = host.get("mac") or existing.mac - existing.hostname = host.get("hostname") or existing.hostname - existing.os = host.get("os") or existing.os - existing.services = services - existing.suggested_type = suggested_type + # One inventory row per device (by IP). Match across pending AND + # approved so a re-scan of an already-approved device refreshes its + # row instead of spawning a fresh "pending" duplicate. Hidden rows + # are already skipped above. + existing_rows = (await db.execute( + select(PendingDevice) + .where(PendingDevice.ip == ip, PendingDevice.status != "hidden") + .order_by(PendingDevice.discovered_at) + )).scalars().all() + + if existing_rows: + # Prefer an approved row (it owns the canvas link semantics), + # otherwise the oldest. Collapse any leftover duplicates created + # by earlier scans. + keep = next((r for r in existing_rows if r.status == "approved"), existing_rows[0]) + for dup in existing_rows: + if dup is not keep: + await db.delete(dup) + keep.mac = host.get("mac") or keep.mac + keep.hostname = host.get("hostname") or keep.hostname + keep.os = host.get("os") or keep.os + keep.services = services + keep.suggested_type = suggested_type + # status preserved — an approved device stays approved. else: db.add(PendingDevice( ip=ip, diff --git a/backend/tests/test_scan.py b/backend/tests/test_scan.py index 5879390..dc6b518 100644 --- a/backend/tests/test_scan.py +++ b/backend/tests/test_scan.py @@ -476,6 +476,59 @@ async def test_run_scan_records_ip_already_in_canvas(db_session: AsyncSession): assert device.status == "pending" +@pytest.mark.asyncio +async def test_run_scan_refreshes_approved_device_without_duplicating(db_session: AsyncSession): + """Re-scanning an already-approved device updates its row in place instead of + spawning a fresh pending duplicate, and keeps it approved.""" + approved = PendingDevice( + id=str(uuid.uuid4()), ip="192.168.1.50", mac=None, hostname="old", + os=None, services=[], suggested_type="server", status="approved", + ) + db_session.add(approved) + run_id = str(uuid.uuid4()) + db_session.add(ScanRun(id=run_id, status="running", ranges=["192.168.1.0/24"])) + await db_session.commit() + + with ( + patch("app.services.scanner._nmap_scan", return_value=[MOCK_HOST]), + patch("app.api.routes.status.broadcast_scan_update", new_callable=AsyncMock), + ): + await run_scan(["192.168.1.0/24"], db_session, run_id) + + rows = (await db_session.execute( + select(PendingDevice).where(PendingDevice.ip == "192.168.1.50") + )).scalars().all() + assert len(rows) == 1 + assert rows[0].status == "approved" + assert rows[0].hostname == "myhost.lan" # refreshed from the scan + + +@pytest.mark.asyncio +async def test_run_scan_collapses_existing_duplicate_rows(db_session: AsyncSession): + """Pre-existing duplicate inventory rows for one IP are collapsed to a single + row at scan start, even if the device is not re-discovered.""" + for status in ("approved", "pending", "pending"): + db_session.add(PendingDevice( + id=str(uuid.uuid4()), ip="192.168.1.77", mac=None, hostname=None, + os=None, services=[], suggested_type="server", status=status, + )) + run_id = str(uuid.uuid4()) + db_session.add(ScanRun(id=run_id, status="running", ranges=["192.168.1.0/24"])) + await db_session.commit() + + with ( + patch("app.services.scanner._nmap_scan", return_value=[]), + patch("app.api.routes.status.broadcast_scan_update", new_callable=AsyncMock), + ): + await run_scan(["192.168.1.0/24"], db_session, run_id) + + rows = (await db_session.execute( + select(PendingDevice).where(PendingDevice.ip == "192.168.1.77") + )).scalars().all() + assert len(rows) == 1 + assert rows[0].status == "approved" # approved row is the one kept + + @pytest.mark.asyncio async def test_run_scan_skips_hidden_device(db_session: AsyncSession): """Devices previously hidden by the user must not re-appear in pending on re-scan."""