fix: make scan stop interrupt in-flight nmap range

Stop button had no effect: run_scan only checked the cancel flag between
CIDR ranges and between hosts, never inside the per-range nmap call. For a
single /24 the whole scan is one blocking call, so cancel was ignored for
minutes and the run status stayed 'running'.

- thread run_id into _nmap_scan/_ping_sweep/_nmap_port_scan; check cancel
  before each phase and skip queued hosts once cancelled
- flip ScanRun status to 'cancelled' eagerly in the stop endpoint so the UI
  reacts immediately instead of waiting for a checkpoint

Fixes #218

ha-relevant: yes
This commit is contained in:
Pouzor
2026-06-28 11:13:14 +02:00
parent 3cedb40d17
commit da2c1c356a
4 changed files with 107 additions and 9 deletions
+7 -1
View File
@@ -1,7 +1,7 @@
import ipaddress
import logging
import uuid
from datetime import datetime
from datetime import datetime, timezone
from typing import Any
from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException
@@ -191,6 +191,12 @@ async def stop_scan(
if run.status != "running":
raise HTTPException(status_code=409, detail="Scan is not running")
request_cancel(run_id)
# Flip status eagerly so the UI reflects the stop immediately, instead of
# waiting for run_scan to reach its next cancellation checkpoint (which may
# be blocked inside a long nmap call). run_scan converges to the same state.
run.status = "cancelled"
run.finished_at = datetime.now(timezone.utc)
await db.commit()
return {"stopping": True}
+26 -6
View File
@@ -175,7 +175,7 @@ def _arp_table_hosts(network: str) -> dict[str, dict[str, Any]]:
return {}
async def _ping_sweep(target: str) -> dict[str, dict[str, Any]]:
async def _ping_sweep(target: str, run_id: str | None = None) -> dict[str, dict[str, Any]]:
"""
Phase 1: Concurrent ICMP ping sweep + ARP cache.
Pings all IPs in the CIDR in parallel (up to 50 at once, 1s timeout each).
@@ -205,6 +205,12 @@ async def _ping_sweep(target: str) -> dict[str, dict[str, Any]]:
alive_ips: set[str] = {ip for ip in ping_results if ip is not None}
logger.info("[Phase 1] %d/%d hosts responded to ping", len(alive_ips), len(all_ips))
# Cancelled during the sweep — bail before the (potentially long) Phase 2
# port scan. Returning empty makes _nmap_scan skip nmap entirely.
if run_id is not None and _is_cancelled(run_id):
logger.info("[Phase 1] %s — scan cancelled, skipping hostname/ARP enrichment", target)
return {}
# ARP cache: catch devices that block ICMP but were recently active,
# and enrich ping-alive hosts with their MAC addresses.
arp_cache = await asyncio.to_thread(_arp_table_hosts, target)
@@ -286,7 +292,8 @@ def _nmap_scan_single(host_dict: dict[str, Any], port_spec: str = _EXTRA_PORTS)
async def _nmap_port_scan(
alive: dict[str, dict[str, Any]], port_spec: str = _EXTRA_PORTS
alive: dict[str, dict[str, Any]], port_spec: str = _EXTRA_PORTS,
run_id: str | None = None,
) -> list[dict[str, Any]]:
"""
Phase 2: Per-IP service detection with bounded concurrency.
@@ -301,6 +308,11 @@ async def _nmap_port_scan(
async def _scan_with_sem(host_dict: dict[str, Any]) -> dict[str, Any]:
async with semaphore:
# Once cancelled, skip the expensive nmap call for every host still
# queued behind the semaphore — return it unscanned so the gather
# unwinds fast instead of blocking the stop for minutes.
if run_id is not None and _is_cancelled(run_id):
return host_dict
return await asyncio.to_thread(_nmap_scan_single, host_dict, port_spec)
raw = await asyncio.gather(*[_scan_with_sem(h) for h in alive.values()], return_exceptions=True)
@@ -314,24 +326,32 @@ async def _nmap_port_scan(
return results
async def _nmap_scan(target: str, port_spec: str = _EXTRA_PORTS) -> list[dict[str, Any]]:
async def _nmap_scan(
target: str, port_spec: str = _EXTRA_PORTS, run_id: str | None = None
) -> list[dict[str, Any]]:
"""
Two-phase scan for a CIDR range.
Phase 1: Concurrent ping sweep to find alive hosts (fast, no false positives).
Phase 2: Per-IP nmap port scan with service detection (bounded concurrency, 10 at a time).
``run_id`` lets each phase poll for cancellation so a stop request takes
effect mid-range instead of only at CIDR/host boundaries in run_scan.
"""
logger.info("[Scan] Starting scan for %s — nmap available: %s", target, _NMAP_AVAILABLE)
if run_id is not None and _is_cancelled(run_id):
logger.info("[Scan] %s — cancelled before start, skipping", target)
return []
if not _NMAP_AVAILABLE:
logger.warning("[Scan] nmap not available — returning mock data")
return _mock_scan(target)
try:
alive = await _ping_sweep(target)
alive = await _ping_sweep(target, run_id=run_id)
logger.info("[Phase 1] Found %d alive host(s) in %s: %s",
len(alive), target, ", ".join(sorted(alive.keys())))
except Exception as exc:
logger.error("Phase 1 ping sweep failed: %s", exc)
raise RuntimeError(str(exc)) from exc
return await _nmap_port_scan(alive, port_spec)
return await _nmap_port_scan(alive, port_spec, run_id=run_id)
async def _mdns_discover(timeout: float = 4.0) -> list[dict[str, Any]]:
@@ -563,7 +583,7 @@ async def run_scan(
for cidr in ranges:
if _is_cancelled(run_id):
break
hosts = await _nmap_scan(cidr, port_spec)
hosts = await _nmap_scan(cidr, port_spec, run_id=run_id)
for host in hosts:
if _is_cancelled(run_id):
break
+5 -1
View File
@@ -652,6 +652,10 @@ async def test_stop_scan_success(client: AsyncClient, headers, db_session: Async
assert res.json() == {"stopping": True}
# run_id added to cancel set
assert run.id in _cancelled_runs
# status flipped eagerly so the UI reacts without waiting for a checkpoint
await db_session.refresh(run)
assert run.status == "cancelled"
assert run.finished_at is not None
# cleanup for other tests
_cancelled_runs.discard(run.id)
@@ -691,7 +695,7 @@ async def test_run_scan_cancelled_mid_scan_skips_remaining_cidrs(db_session: Asy
call_count = 0
def nmap_side_effect(target: str, port_spec: str | None = None):
def nmap_side_effect(target: str, port_spec: str | None = None, run_id: str | None = None):
nonlocal call_count
call_count += 1
# Signal cancellation after the first CIDR scan completes
+69 -1
View File
@@ -277,6 +277,74 @@ async def test_nmap_scan_raises_on_sweep_error():
await _nmap_scan("192.168.1.0/24")
# ---------------------------------------------------------------------------
# Cancellation responsiveness (issue #218)
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_nmap_scan_cancelled_before_start_skips_phases():
"""A run already cancelled returns immediately without touching the network."""
from app.services.scanner import _cancelled_runs, _nmap_scan, request_cancel
run_id = "cancel-before-start"
request_cancel(run_id)
try:
with patch("app.services.scanner._ping_sweep", new_callable=AsyncMock) as mock_sweep, \
patch("app.services.scanner._nmap_port_scan", new_callable=AsyncMock) as mock_port:
result = await _nmap_scan("192.168.1.0/24", run_id=run_id)
assert result == []
mock_sweep.assert_not_called()
mock_port.assert_not_called()
finally:
_cancelled_runs.discard(run_id)
@pytest.mark.asyncio
async def test_ping_sweep_cancelled_mid_sweep_returns_empty():
"""Cancelling during Phase 1 bails before Phase 2 — no alive hosts returned."""
from app.services.scanner import _cancelled_runs, _ping_sweep, request_cancel
run_id = "cancel-during-sweep"
async def _fake_subprocess(*args, **kwargs):
proc = AsyncMock()
proc.wait = AsyncMock(return_value=1)
proc.returncode = 1
return proc
request_cancel(run_id)
try:
with patch("app.services.scanner.asyncio.create_subprocess_exec", new=_fake_subprocess), \
patch("app.services.scanner._arp_table_hosts", return_value={}):
result = await _ping_sweep("192.168.1.0/30", run_id=run_id)
assert result == {}
finally:
_cancelled_runs.discard(run_id)
@pytest.mark.asyncio
async def test_nmap_port_scan_skips_queued_hosts_when_cancelled():
"""Once cancelled, queued hosts return unscanned instead of invoking nmap."""
from app.services.scanner import _cancelled_runs, _nmap_port_scan, request_cancel
run_id = "cancel-port-scan"
alive = {
"192.168.1.10": {
"ip": "192.168.1.10", "mac": None, "hostname": None,
"os": None, "open_ports": [],
},
}
request_cancel(run_id)
try:
with patch("app.services.scanner._nmap_scan_single") as mock_single:
result = await _nmap_port_scan(alive, run_id=run_id)
mock_single.assert_not_called()
assert result[0]["ip"] == "192.168.1.10"
assert result[0]["open_ports"] == []
finally:
_cancelled_runs.discard(run_id)
# ---------------------------------------------------------------------------
# _mdns_discover
# ---------------------------------------------------------------------------
@@ -665,7 +733,7 @@ async def test_run_scan_deep_scan_passes_port_spec_to_nmap(mem_db):
captured = {}
async def fake_nmap(target, port_spec):
async def fake_nmap(target, port_spec, run_id=None):
captured["port_spec"] = port_spec
return []