feat: deduplicate pending devices and skip canvas/hidden nodes on scan
- 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
This commit is contained in:
@@ -7,7 +7,7 @@ from typing import Any
|
|||||||
|
|
||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
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
|
from app.services.fingerprint import fingerprint_ports, suggest_node_type
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
@@ -107,18 +107,54 @@ async def run_scan(ranges: list[str], db: AsyncSession, run_id: str) -> None:
|
|||||||
|
|
||||||
devices_found = 0
|
devices_found = 0
|
||||||
try:
|
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:
|
for cidr in ranges:
|
||||||
# Run nmap in a thread pool — does not block the event loop
|
# Run nmap in a thread pool — does not block the event loop
|
||||||
hosts = await asyncio.to_thread(_nmap_scan, cidr)
|
hosts = await asyncio.to_thread(_nmap_scan, cidr)
|
||||||
|
|
||||||
for host in hosts:
|
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"])
|
services = fingerprint_ports(host["open_ports"])
|
||||||
suggested_type = suggest_node_type(host["open_ports"], host.get("mac"))
|
suggested_type = suggest_node_type(host["open_ports"], host.get("mac"))
|
||||||
|
|
||||||
# Update existing pending device or create a new one
|
# Update existing pending device or create a new one
|
||||||
existing_result = await db.execute(
|
existing_result = await db.execute(
|
||||||
select(PendingDevice).where(
|
select(PendingDevice).where(
|
||||||
PendingDevice.ip == host["ip"],
|
PendingDevice.ip == ip,
|
||||||
PendingDevice.status == "pending",
|
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
|
existing.suggested_type = suggested_type
|
||||||
else:
|
else:
|
||||||
device = PendingDevice(
|
device = PendingDevice(
|
||||||
ip=host["ip"],
|
ip=ip,
|
||||||
mac=host.get("mac"),
|
mac=host.get("mac"),
|
||||||
hostname=host.get("hostname"),
|
hostname=host.get("hostname"),
|
||||||
os=host.get("os"),
|
os=host.get("os"),
|
||||||
|
|||||||
+114
-1
@@ -7,7 +7,7 @@ from httpx import AsyncClient
|
|||||||
from sqlalchemy import select
|
from sqlalchemy import select
|
||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
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
|
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"
|
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
|
@pytest.mark.asyncio
|
||||||
async def test_run_scan_updates_existing_pending_device(db_session: AsyncSession):
|
async def test_run_scan_updates_existing_pending_device(db_session: AsyncSession):
|
||||||
"""Re-scanning the same IP updates services instead of creating a duplicate."""
|
"""Re-scanning the same IP updates services instead of creating a duplicate."""
|
||||||
|
|||||||
Reference in New Issue
Block a user