From 7608d0725586d5e401459953cb6f4569d2c8f962 Mon Sep 17 00:00:00 2001
From: Pouzor <remy.jardinet@gmail.com>
Date: Mon, 20 Apr 2026 00:31:30 +0200
Subject: [PATCH] fix: flush before reading node IDs in bulk/single approve;
 404/409 guards; catch scan errors
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- db.flush() ensures node.id is populated before reading — fixes bulk approve
  where node_ids were null, causing frontend to skip addNode for every device
- approve_device raises 404 on missing device, 409 on already-processed device
- _background_scan rollbacks dirty session then marks run as "failed"
- Explicit Node() field mapping instead of **model_dump() to prevent injection
- update_scan_config rolls back in-memory change if save_overrides() fails
- clear_pending uses bulk DELETE instead of N individual row deletes
---
 backend/app/api/routes/scan.py | 55 +++++++++++++++++++++++-----------
 backend/tests/test_scan.py     |  4 +--
 2 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/backend/app/api/routes/scan.py b/backend/app/api/routes/scan.py
index 1e4c86c..5c77bf1 100644
--- a/backend/app/api/routes/scan.py
+++ b/backend/app/api/routes/scan.py
@@ -41,7 +41,15 @@ router = APIRouter()
 
 async def _background_scan(run_id: str, ranges: list[str]) -> None:
     async with AsyncSessionLocal() as db:
-        await run_scan(ranges, db, run_id)
+        try:
+            await run_scan(ranges, db, run_id)
+        except Exception:
+            logger.exception("Scan run %s failed unexpectedly", run_id)
+            await db.rollback()
+            run = await db.get(ScanRun, run_id)
+            if run and run.status == "running":
+                run.status = "failed"
+                await db.commit()
 
 
 @router.post("/trigger", response_model=ScanRunResponse)
@@ -89,12 +97,10 @@ async def clear_pending(
     db: AsyncSession = Depends(get_db),
     _: str = Depends(get_current_user),
 ) -> dict[str, int]:
-    result = await db.execute(select(PendingDevice).where(PendingDevice.status == "pending"))
-    devices = result.scalars().all()
-    for device in devices:
-        await db.delete(device)
+    from sqlalchemy import delete as sa_delete
+    result = await db.execute(sa_delete(PendingDevice).where(PendingDevice.status == "pending"))
     await db.commit()
-    return {"deleted": len(devices)}
+    return {"deleted": result.rowcount}
 
 
 @router.get("/hidden", response_model=list[PendingDeviceResponse])
@@ -116,7 +122,7 @@ async def bulk_approve_devices(
         )
     )
     devices = result.scalars().all()
-    node_ids: list[str] = []
+    created_nodes: list[Node] = []
     for device in devices:
         device.status = "approved"
         node = Node(
@@ -128,9 +134,11 @@ async def bulk_approve_devices(
             services=device.services or [],
         )
         db.add(node)
-        node_ids.append(node.id)
-    await db.commit()
+        created_nodes.append(node)
+    await db.flush()  # populates node.id from Python-side default before reading
+    node_ids = [n.id for n in created_nodes]
     approved_device_ids = [d.id for d in devices]
+    await db.commit()
     return {
         "approved": len(node_ids),
         "node_ids": node_ids,
@@ -166,13 +174,24 @@ async def approve_device(
     _: str = Depends(get_current_user),
 ) -> dict[str, Any]:
     device = await db.get(PendingDevice, device_id)
-    if device:
-        device.status = "approved"
-        node = Node(**node_data.model_dump())
-        db.add(node)
-        await db.commit()
-        return {"approved": True, "node_id": node.id}
-    return {"approved": False}
+    if not device:
+        raise HTTPException(status_code=404, detail="Device not found")
+    if device.status != "pending":
+        raise HTTPException(status_code=409, detail="Device already processed")
+    device.status = "approved"
+    node = Node(
+        label=node_data.label,
+        type=node_data.type,
+        ip=node_data.ip,
+        hostname=node_data.hostname,
+        status=node_data.status,
+        services=node_data.services or [],
+    )
+    db.add(node)
+    await db.flush()
+    node_id = node.id
+    await db.commit()
+    return {"approved": True, "node_id": node_id}
 
 
 @router.post("/pending/{device_id}/hide")
@@ -212,10 +231,12 @@ async def get_scan_config(_: str = Depends(get_current_user)) -> ScanConfig:
 
 @router.post("/config", response_model=ScanConfig)
 async def update_scan_config(payload: ScanConfig, _: str = Depends(get_current_user)) -> ScanConfig:
+    previous = settings.scanner_ranges
+    settings.scanner_ranges = payload.ranges
     try:
-        settings.scanner_ranges = payload.ranges
         settings.save_overrides()
         return payload
     except Exception as exc:
+        settings.scanner_ranges = previous
         logger.error("Failed to save scan config: %s", exc)
         raise HTTPException(status_code=500, detail="Failed to save scan config") from exc
diff --git a/backend/tests/test_scan.py b/backend/tests/test_scan.py
index 5e76d70..86ce554 100644
--- a/backend/tests/test_scan.py
+++ b/backend/tests/test_scan.py
@@ -120,8 +120,7 @@ async def test_approve_nonexistent_device(client: AsyncClient, headers):
         json=node_payload,
         headers=headers,
     )
-    assert res.status_code == 200
-    assert res.json()["approved"] is False
+    assert res.status_code == 404
 
 
 # --- Hide device ---
@@ -478,6 +477,7 @@ async def test_bulk_approve_approves_devices(client: AsyncClient, headers, two_p
     data = res.json()
     assert data["approved"] == 2
     assert len(data["node_ids"]) == 2
+    assert all(nid is not None for nid in data["node_ids"]), "node_ids must be non-null UUIDs"
     assert len(data["device_ids"]) == 2
     assert data["skipped"] == 0
     # Pending list should now be empty