fix: flush before reading node IDs in bulk/single approve; 404/409 guards; catch scan errors

- 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
This commit is contained in:
Pouzor
2026-04-20 00:31:30 +02:00
parent 1bc6798d76
commit 7608d07255
2 changed files with 40 additions and 19 deletions
+38 -17
View File
@@ -41,7 +41,15 @@ router = APIRouter()
async def _background_scan(run_id: str, ranges: list[str]) -> None: async def _background_scan(run_id: str, ranges: list[str]) -> None:
async with AsyncSessionLocal() as db: 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) @router.post("/trigger", response_model=ScanRunResponse)
@@ -89,12 +97,10 @@ async def clear_pending(
db: AsyncSession = Depends(get_db), db: AsyncSession = Depends(get_db),
_: str = Depends(get_current_user), _: str = Depends(get_current_user),
) -> dict[str, int]: ) -> dict[str, int]:
result = await db.execute(select(PendingDevice).where(PendingDevice.status == "pending")) from sqlalchemy import delete as sa_delete
devices = result.scalars().all() result = await db.execute(sa_delete(PendingDevice).where(PendingDevice.status == "pending"))
for device in devices:
await db.delete(device)
await db.commit() await db.commit()
return {"deleted": len(devices)} return {"deleted": result.rowcount}
@router.get("/hidden", response_model=list[PendingDeviceResponse]) @router.get("/hidden", response_model=list[PendingDeviceResponse])
@@ -116,7 +122,7 @@ async def bulk_approve_devices(
) )
) )
devices = result.scalars().all() devices = result.scalars().all()
node_ids: list[str] = [] created_nodes: list[Node] = []
for device in devices: for device in devices:
device.status = "approved" device.status = "approved"
node = Node( node = Node(
@@ -128,9 +134,11 @@ async def bulk_approve_devices(
services=device.services or [], services=device.services or [],
) )
db.add(node) db.add(node)
node_ids.append(node.id) created_nodes.append(node)
await db.commit() 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] approved_device_ids = [d.id for d in devices]
await db.commit()
return { return {
"approved": len(node_ids), "approved": len(node_ids),
"node_ids": node_ids, "node_ids": node_ids,
@@ -166,13 +174,24 @@ async def approve_device(
_: str = Depends(get_current_user), _: str = Depends(get_current_user),
) -> dict[str, Any]: ) -> dict[str, Any]:
device = await db.get(PendingDevice, device_id) device = await db.get(PendingDevice, device_id)
if device: if not device:
device.status = "approved" raise HTTPException(status_code=404, detail="Device not found")
node = Node(**node_data.model_dump()) if device.status != "pending":
db.add(node) raise HTTPException(status_code=409, detail="Device already processed")
await db.commit() device.status = "approved"
return {"approved": True, "node_id": node.id} node = Node(
return {"approved": False} 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") @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) @router.post("/config", response_model=ScanConfig)
async def update_scan_config(payload: ScanConfig, _: str = Depends(get_current_user)) -> ScanConfig: async def update_scan_config(payload: ScanConfig, _: str = Depends(get_current_user)) -> ScanConfig:
previous = settings.scanner_ranges
settings.scanner_ranges = payload.ranges
try: try:
settings.scanner_ranges = payload.ranges
settings.save_overrides() settings.save_overrides()
return payload return payload
except Exception as exc: except Exception as exc:
settings.scanner_ranges = previous
logger.error("Failed to save scan config: %s", exc) logger.error("Failed to save scan config: %s", exc)
raise HTTPException(status_code=500, detail="Failed to save scan config") from exc raise HTTPException(status_code=500, detail="Failed to save scan config") from exc
+2 -2
View File
@@ -120,8 +120,7 @@ async def test_approve_nonexistent_device(client: AsyncClient, headers):
json=node_payload, json=node_payload,
headers=headers, headers=headers,
) )
assert res.status_code == 200 assert res.status_code == 404
assert res.json()["approved"] is False
# --- Hide device --- # --- Hide device ---
@@ -478,6 +477,7 @@ async def test_bulk_approve_approves_devices(client: AsyncClient, headers, two_p
data = res.json() data = res.json()
assert data["approved"] == 2 assert data["approved"] == 2
assert len(data["node_ids"]) == 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 len(data["device_ids"]) == 2
assert data["skipped"] == 0 assert data["skipped"] == 0
# Pending list should now be empty # Pending list should now be empty