diff --git a/backend/secuscan/executor.py b/backend/secuscan/executor.py index 3b45fbbe..0482314d 100644 --- a/backend/secuscan/executor.py +++ b/backend/secuscan/executor.py @@ -20,7 +20,6 @@ from .plugins import get_plugin_manager from .models import TaskStatus from .ratelimit import concurrent_limiter -from .ratelimit import concurrent_limiter # Modular Scanners from .scanners.port_scanner import PortScanner @@ -421,8 +420,9 @@ async def execute_task(self, task_id: str): task_id=task_id ) finally: - # Always clean up: remove from the in-memory registry and - # release the concurrency slot regardless of how the task ended. + # Always runs regardless of success, failure, or cancellation. + # Remove from in-memory registry and release the concurrency slot + # so future tasks are not permanently blocked. self.running_tasks.pop(task_id, None) await concurrent_limiter.release(task_id) diff --git a/testing/backend/unit/test_executor.py b/testing/backend/unit/test_executor.py index fd21aa7c..678ae09b 100644 --- a/testing/backend/unit/test_executor.py +++ b/testing/backend/unit/test_executor.py @@ -1,8 +1,14 @@ import asyncio import json +import uuid + +import pytest +from unittest.mock import AsyncMock, MagicMock, patch from backend.secuscan.config import settings +from backend.secuscan.database import get_db, init_db from backend.secuscan.executor import TaskExecutor +from backend.secuscan.models import TaskStatus from backend.secuscan.plugins import get_plugin_manager, init_plugins @@ -150,3 +156,120 @@ def test_classify_command_result_fails_on_undefined_flag_even_with_zero_exit(set assert status == "failed" assert error is not None + + +@pytest.mark.asyncio +async def test_execute_task_sets_cancelled_status_in_db(setup_test_environment): + """ + When execute_task() is cancelled, the DB row must be updated to + CANCELLED status via the explicit except asyncio.CancelledError handler. + This directly exercises the executor path, not an isolated helper. + """ + await init_db(settings.database_path) + db = await get_db() + + task_id = str(uuid.uuid4()) + await db.execute( + """ + INSERT INTO tasks (id, plugin_id, tool_name, target, inputs_json, + status, consent_granted, safe_mode) + VALUES (?, ?, ?, ?, ?, ?, ?, ?) + """, + (task_id, "nmap", "nmap", "127.0.0.1", '{"target":"127.0.0.1"}', + TaskStatus.QUEUED.value, 1, 1) + ) + + executor = TaskExecutor() + + async def raise_cancelled(*args, **kwargs): + raise asyncio.CancelledError() + + with patch.object(executor, "_execute_command", side_effect=raise_cancelled), \ + patch("backend.secuscan.executor.concurrent_limiter") as mock_limiter, \ + patch("backend.secuscan.executor.get_plugin_manager") as mock_pm: + + mock_limiter.release = AsyncMock() + + mock_plugin = MagicMock() + mock_plugin.name = "nmap" + mock_plugin.presets = {} + mock_plugin.docker_image = None + mock_pm.return_value.get_plugin.return_value = mock_plugin + mock_pm.return_value.build_command.return_value = ["nmap", "127.0.0.1"] + + task = asyncio.create_task(executor.execute_task(task_id)) + await asyncio.sleep(0) + task.cancel() + try: + await task + except asyncio.CancelledError: + pass + + row = await db.fetchone( + "SELECT status FROM tasks WHERE id = ?", (task_id,) + ) + assert row["status"] == TaskStatus.CANCELLED.value, ( + f"Expected CANCELLED in DB, got {row['status']}. " + "except asyncio.CancelledError handler is not writing to DB." + ) + mock_limiter.release.assert_called_once_with(task_id) + + +@pytest.mark.asyncio +async def test_execute_task_releases_limiter_on_normal_completion(setup_test_environment): + """ + Concurrency slot must be released in finally even on successful completion. + """ + await init_db(settings.database_path) + db = await get_db() + + task_id = str(uuid.uuid4()) + await db.execute( + """ + INSERT INTO tasks (id, plugin_id, tool_name, target, inputs_json, + status, consent_granted, safe_mode) + VALUES (?, ?, ?, ?, ?, ?, ?, ?) + """, + (task_id, "nmap", "nmap", "127.0.0.1", '{"target":"127.0.0.1"}', + TaskStatus.QUEUED.value, 1, 1) + ) + + executor = TaskExecutor() + + async def fake_command(*args, **kwargs): + return "80/tcp open http", 0 + + with patch.object(executor, "_execute_command", side_effect=fake_command), \ + patch("backend.secuscan.executor.concurrent_limiter") as mock_limiter, \ + patch("backend.secuscan.executor.get_plugin_manager") as mock_pm: + + mock_limiter.release = AsyncMock() + + mock_plugin = MagicMock() + mock_plugin.name = "nmap" + mock_plugin.presets = {} + mock_plugin.docker_image = None + mock_plugin.output = {"parser": "builtin_nmap", "format": "text"} + mock_plugin.category = "Network" + mock_plugin.id = "nmap" + mock_pm.return_value.get_plugin.return_value = mock_plugin + mock_pm.return_value.build_command.return_value = ["nmap", "127.0.0.1"] + mock_pm.return_value.plugins_dir = MagicMock() + mock_pm.return_value.plugins_dir.__truediv__ = MagicMock( + return_value=MagicMock( + __truediv__=MagicMock(return_value=MagicMock(exists=lambda: False)) + ) + ) + + await executor.execute_task(task_id) + + mock_limiter.release.assert_called_once_with(task_id) + + +def test_cancelled_error_is_not_subclass_of_exception(): + """ + Documents the Python 3.8+ behaviour: CancelledError is a BaseException, + not Exception. If this fails, the language changed and the except ordering + in execute_task() needs revisiting. + """ + assert not issubclass(asyncio.CancelledError, Exception) \ No newline at end of file