fix: handle CancelledError explicitly in execute_task to fix dead DB update

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)
 

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)