From ce4af14ee6affe5f36211a05669151822207a4d3 Mon Sep 17 00:00:00 2001 From: Pouzor Date: Mon, 30 Mar 2026 22:01:47 +0200 Subject: [PATCH] feat: deduplicate pending devices and skip canvas/hidden nodes on scan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - At scan start, purge any pending entries whose IPs already exist in canvas - Skip canvas nodes (approved) during scan — don't re-add to pending - Skip hidden devices during scan — respect user's hide decision - Add 4 tests covering all new behaviors --- backend/app/services/scanner.py | 42 +++++++++++- backend/tests/test_scan.py | 115 +++++++++++++++++++++++++++++++- 2 files changed, 153 insertions(+), 4 deletions(-) diff --git a/backend/app/services/scanner.py b/backend/app/services/scanner.py index 85a27ea..70e236f 100644 --- a/backend/app/services/scanner.py +++ b/backend/app/services/scanner.py @@ -7,7 +7,7 @@ from typing import Any from sqlalchemy.ext.asyncio import AsyncSession -from app.db.models import PendingDevice, ScanRun +from app.db.models import Node, PendingDevice, ScanRun from app.services.fingerprint import fingerprint_ports, suggest_node_type logger = logging.getLogger(__name__) @@ -107,18 +107,54 @@ async def run_scan(ranges: list[str], db: AsyncSession, run_id: str) -> None: devices_found = 0 try: + # Clean up stale pending devices whose IPs are already in the canvas + # (covers devices approved between scans, or pre-existing canvas nodes) + canvas_ips_result = await db.execute(select(Node.ip).where(Node.ip.isnot(None))) + canvas_ips = {row[0] for row in canvas_ips_result.fetchall()} + if canvas_ips: + stale_result = await db.execute( + select(PendingDevice).where( + PendingDevice.status == "pending", + PendingDevice.ip.in_(canvas_ips), + ) + ) + for stale in stale_result.scalars().all(): + await db.delete(stale) + await db.commit() + for cidr in ranges: # Run nmap in a thread pool — does not block the event loop hosts = await asyncio.to_thread(_nmap_scan, cidr) for host in hosts: + ip = host["ip"] + + # Skip if device is already in the canvas (approved node) + canvas_result = await db.execute( + select(Node).where(Node.ip == ip) + ) + if canvas_result.scalar_one_or_none() is not None: + logger.debug("Skipping %s — already in canvas", ip) + continue + + # Skip if device was explicitly hidden by the user + hidden_result = await db.execute( + select(PendingDevice).where( + PendingDevice.ip == ip, + PendingDevice.status == "hidden", + ) + ) + if hidden_result.scalar_one_or_none() is not None: + logger.debug("Skipping %s — hidden by user", ip) + continue + services = fingerprint_ports(host["open_ports"]) suggested_type = suggest_node_type(host["open_ports"], host.get("mac")) # Update existing pending device or create a new one existing_result = await db.execute( select(PendingDevice).where( - PendingDevice.ip == host["ip"], + PendingDevice.ip == ip, PendingDevice.status == "pending", ) ) @@ -131,7 +167,7 @@ async def run_scan(ranges: list[str], db: AsyncSession, run_id: str) -> None: existing.suggested_type = suggested_type else: device = PendingDevice( - ip=host["ip"], + ip=ip, mac=host.get("mac"), hostname=host.get("hostname"), os=host.get("os"), diff --git a/backend/tests/test_scan.py b/backend/tests/test_scan.py index f355f2a..c03b8b4 100644 --- a/backend/tests/test_scan.py +++ b/backend/tests/test_scan.py @@ -7,7 +7,7 @@ from httpx import AsyncClient from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession -from app.db.models import PendingDevice, ScanRun +from app.db.models import Node, PendingDevice, ScanRun from app.services.scanner import run_scan @@ -199,6 +199,119 @@ async def test_run_scan_creates_new_pending_device(db_session: AsyncSession): assert device.suggested_type == "server" +@pytest.mark.asyncio +async def test_run_scan_purges_stale_pending_for_canvas_nodes(db_session: AsyncSession): + """Pending devices that were already in canvas before scan starts must be removed.""" + node = Node( + id=str(uuid.uuid4()), + label="Existing Server", + type="server", + ip="192.168.1.50", + status="online", + services=[], + pos_x=0.0, + pos_y=0.0, + ) + stale = PendingDevice( + id=str(uuid.uuid4()), + ip="192.168.1.50", + mac=None, + hostname=None, + os=None, + services=[], + suggested_type="generic", + status="pending", + ) + db_session.add(node) + db_session.add(stale) + await db_session.commit() + + run_id = str(uuid.uuid4()) + run = ScanRun(id=run_id, status="running", ranges=["192.168.1.0/24"]) + db_session.add(run) + 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) + + result = await db_session.execute( + select(PendingDevice).where(PendingDevice.ip == "192.168.1.50") + ) + assert result.scalar_one_or_none() is None + + +@pytest.mark.asyncio +async def test_run_scan_skips_ip_already_in_canvas(db_session: AsyncSession): + """Devices whose IP already exists as a canvas Node must not appear in pending.""" + node = Node( + id=str(uuid.uuid4()), + label="Existing Server", + type="server", + ip="192.168.1.50", + status="online", + services=[], + pos_x=0.0, + pos_y=0.0, + ) + db_session.add(node) + await db_session.commit() + + run_id = str(uuid.uuid4()) + run = ScanRun(id=run_id, status="running", ranges=["192.168.1.0/24"]) + db_session.add(run) + 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) + + result = await db_session.execute( + select(PendingDevice).where(PendingDevice.ip == "192.168.1.50") + ) + assert result.scalar_one_or_none() is None + + +@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.""" + hidden = PendingDevice( + id=str(uuid.uuid4()), + ip="192.168.1.50", + mac=None, + hostname=None, + os=None, + services=[], + suggested_type="generic", + status="hidden", + ) + db_session.add(hidden) + await db_session.commit() + + run_id = str(uuid.uuid4()) + run = ScanRun(id=run_id, status="running", ranges=["192.168.1.0/24"]) + db_session.add(run) + 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) + + result = await db_session.execute( + select(PendingDevice).where( + PendingDevice.ip == "192.168.1.50", + PendingDevice.status == "pending", + ) + ) + assert result.scalar_one_or_none() is None + + @pytest.mark.asyncio async def test_run_scan_updates_existing_pending_device(db_session: AsyncSession): """Re-scanning the same IP updates services instead of creating a duplicate."""