From 2ee23dc9c6fe5cedb0e0a422ab1122ec0526cebb Mon Sep 17 00:00:00 2001 From: Anshul Jain Date: Mon, 6 Jul 2026 09:04:30 +0530 Subject: [PATCH] fix(security): log every scan attempt blocked before execution Previously, the audit log only recorded scans that passed every gate and began executing, via executor.create_task's "task_created" event. Any attempt rejected earlier -- missing consent, unknown plugin, invalid preset, unauthorised target/credential/session policy, invalid input, or a target blocked by safe mode -- raised its HTTPException and exited the code path before ever reaching a log write. An administrator had no way to determine, post-incident, whether a user had attempted to scan an unauthorised target: the misuse simply left no trace. Fix: added _log_blocked_scan_attempt, a thin wrapper around the existing db.log_audit with severity="warning", and called it at every rejection point in POST /task/start before its raise: - task_blocked_invalid_payload (payload size/field-length guard) - task_blocked_consent (consent not granted) - task_blocked_plugin_not_found (unknown plugin_id) - task_blocked_invalid_preset (preset not valid for the plugin) - task_blocked_policy (target/credential/session policy missing or insufficiently permissive) - task_blocked_invalid_input (timeout/max_scan_time out of bounds) - task_blocked_safe_mode (target rejected or validation timed out under safe mode -- the scenario the issue's repro steps describe) - task_blocked_rate_limit (per-client-per-plugin quota exceeded) Moved db = await get_db() to the top of start_task so it's available to every rejection branch, including ones that previously ran before the database handle was fetched. Testing: - testing/backend/integration/test_task_start_audit_log.py: 5 new tests covering missing consent, unknown plugin, invalid preset, and safe-mode target rejection all producing an audit_log row with severity "warning", plus a regression guard confirming the existing successful- path 'task_created' entry is unaffected (exactly one entry, not zero or duplicated). - pytest testing/backend/unit -q -m "not benchmark" -- 2210 passed (6 pre-existing failures unrelated to this change, confirmed identical on unmodified main -- sandbox/subprocess tests failing in this sandbox) - pytest testing/backend/integration -q -m "not benchmark" -- 289 passed, 9 skipped (284 baseline + 5 new) - ruff check backend testing/backend -- all checks passed - scripts/check-artifacts.sh origin/main -- clean Fixes #1623 --- backend/secuscan/routes.py | 94 +++++++++++++- .../integration/test_task_start_audit_log.py | 118 ++++++++++++++++++ 2 files changed, 211 insertions(+), 1 deletion(-) create mode 100644 testing/backend/integration/test_task_start_audit_log.py diff --git a/backend/secuscan/routes.py b/backend/secuscan/routes.py index 14252b25..f87b7cac 100644 --- a/backend/secuscan/routes.py +++ b/backend/secuscan/routes.py @@ -307,6 +307,34 @@ async def get_all_presets(): } +async def _log_blocked_scan_attempt( + db, + owner: str, + event_type: str, + message: str, + context: Optional[Dict] = None, + plugin_id: Optional[str] = None, +) -> None: + """ + Record a scan attempt that was rejected before it ever started executing. + + Issue #1623: the audit log previously only recorded scans that passed + every gate and began running (via executor.create_task's "task_created" + event). Attempts blocked by safe mode, rejected for missing consent, or + otherwise stopped during request validation left no trace at all, so an + administrator had no way to tell, post-incident, whether a user had + attempted to scan an unauthorised target. Every rejection path in + start_task now writes an entry here before raising its HTTPException. + """ + await db.log_audit( + event_type, + message, + severity="warning", + context={"owner_id": owner, **(context or {})}, + plugin_id=plugin_id, + ) + + @router.post("/task/start", dependencies=[Depends(task_start_limiter), Depends(check_scan_rate_limit)]) async def start_task( request: TaskCreateRequest, @@ -317,16 +345,27 @@ async def start_task( """ Start a new scan task. """ + db = await get_db() + # ── Payload size / field-length guard ───────────────────────────────── raw_body = await raw_request.body() execution_context = normalize_execution_context(request.execution_context) ok, status_code, error_msg = validate_task_start_payload(raw_body, request.inputs, execution_context) if not ok: + await _log_blocked_scan_attempt( + db, owner, "task_blocked_invalid_payload", f"Task start rejected: {error_msg}", + context={"plugin_id": request.plugin_id}, plugin_id=request.plugin_id, + ) raise HTTPException(status_code=status_code, detail=error_msg) # Validate consent if settings.require_consent and not request.consent_granted: logger.warning(f"Task start failed: Consent not granted. Request: {request}") + await _log_blocked_scan_attempt( + db, owner, "task_blocked_consent", "Task start rejected: consent not granted", + context={"plugin_id": request.plugin_id, "target": (request.inputs or {}).get("target")}, + plugin_id=request.plugin_id, + ) raise HTTPException( status_code=400, detail="Consent required. You must acknowledge the legal notice." @@ -338,6 +377,10 @@ async def start_task( if not plugin: logger.warning(f"Task start failed: Plugin not found: {request.plugin_id}") + await _log_blocked_scan_attempt( + db, owner, "task_blocked_plugin_not_found", f"Task start rejected: plugin not found ({request.plugin_id})", + plugin_id=request.plugin_id, + ) raise HTTPException(status_code=404, detail=f"Plugin not found: {request.plugin_id}") preset_ok, preset_error = validate_preset_name( @@ -347,21 +390,41 @@ async def start_task( ) if not preset_ok: logger.warning("Task start failed: %s", preset_error) + await _log_blocked_scan_attempt( + db, owner, "task_blocked_invalid_preset", f"Task start rejected: {preset_error}", + plugin_id=request.plugin_id, + ) raise HTTPException(status_code=400, detail=preset_error) - db = await get_db() target_policy = await get_target_policy(db, owner, execution_context.get("target_policy_id")) credential_profile = await get_credential_profile(db, owner, execution_context.get("credential_profile_id")) session_profile = await get_session_profile(db, owner, execution_context.get("session_profile_id")) if execution_context.get("target_policy_id") and not target_policy: + await _log_blocked_scan_attempt( + db, owner, "task_blocked_policy", "Task start rejected: target policy not found", + plugin_id=request.plugin_id, + ) raise HTTPException(status_code=400, detail="Target policy not found for this workspace") if execution_context.get("credential_profile_id") and not credential_profile: + await _log_blocked_scan_attempt( + db, owner, "task_blocked_policy", "Task start rejected: credential profile not found", + plugin_id=request.plugin_id, + ) raise HTTPException(status_code=400, detail="Credential profile not found for this workspace") if execution_context.get("session_profile_id") and not session_profile: + await _log_blocked_scan_attempt( + db, owner, "task_blocked_policy", "Task start rejected: session profile not found", + plugin_id=request.plugin_id, + ) raise HTTPException(status_code=400, detail="Session profile not found for this workspace") if (credential_profile or session_profile) and not (target_policy and target_policy.get("allow_authenticated_scan")): + await _log_blocked_scan_attempt( + db, owner, "task_blocked_policy", + "Task start rejected: authenticated scan not permitted by target policy", + plugin_id=request.plugin_id, + ) raise HTTPException( status_code=400, detail="Authenticated scans require a target policy with authenticated scanning enabled.", @@ -373,6 +436,11 @@ async def start_task( ) if requires_exploit_policy and not (target_policy and target_policy.get("allow_exploit_validation")): + await _log_blocked_scan_attempt( + db, owner, "task_blocked_policy", + "Task start rejected: exploit validation not permitted by target policy", + plugin_id=request.plugin_id, + ) raise HTTPException( status_code=400, detail="Offensive validation requires a target policy that explicitly allows exploit validation.", @@ -400,8 +468,16 @@ async def start_task( try: tval = int(effective_inputs[tkey]) except (TypeError, ValueError): + await _log_blocked_scan_attempt( + db, owner, "task_blocked_invalid_input", f"Task start rejected: invalid value for {tkey}", + plugin_id=request.plugin_id, + ) raise HTTPException(status_code=400, detail=f"Invalid value for {tkey}: must be an integer") if tval <= 0 or tval > settings.sandbox_timeout: + await _log_blocked_scan_attempt( + db, owner, "task_blocked_invalid_input", f"Task start rejected: {tkey} out of allowed bounds", + plugin_id=request.plugin_id, + ) raise HTTPException(status_code=400, detail=f"{tkey} must be between 1 and {settings.sandbox_timeout} seconds") if target := effective_inputs.get("target"): @@ -416,6 +492,12 @@ async def start_task( ) except asyncio.TimeoutError: logger.warning("Task start failed: Target validation timed out for '%s'", target_str) + await _log_blocked_scan_attempt( + db, owner, "task_blocked_safe_mode", + "Task start rejected: target validation timed out in safe mode", + context={"target": target_str, "safe_mode": safe_mode}, + plugin_id=request.plugin_id, + ) raise HTTPException( status_code=400, detail="Target validation timed out in safe mode (SecuScan Guardrail)", @@ -423,6 +505,12 @@ async def start_task( if not is_valid: logger.warning(f"Task start failed: Target validation failed for '{target}': {error_msg}") + await _log_blocked_scan_attempt( + db, owner, "task_blocked_safe_mode", + f"Task start rejected: target validation failed ({error_msg})", + context={"target": target_str, "safe_mode": safe_mode}, + plugin_id=request.plugin_id, + ) raise HTTPException(status_code=400, detail=error_msg) # Check rate limits per (client, plugin) so one client cannot exhaust @@ -435,6 +523,10 @@ async def start_task( ) if not can_execute: + await _log_blocked_scan_attempt( + db, owner, "task_blocked_rate_limit", "Task start rejected: rate limit exceeded", + plugin_id=request.plugin_id, + ) raise HTTPException(status_code=429, detail=error_msg) # Create task record first so we have a real task_id for the limiter diff --git a/testing/backend/integration/test_task_start_audit_log.py b/testing/backend/integration/test_task_start_audit_log.py new file mode 100644 index 00000000..aa0677ba --- /dev/null +++ b/testing/backend/integration/test_task_start_audit_log.py @@ -0,0 +1,118 @@ +""" +Integration tests for #1623: scan attempts blocked before execution (missing +consent, unknown plugin, invalid preset, unauthorised policy configuration, +invalid input, safe-mode target rejection, rate limiting) must leave an +audit log entry, not silently disappear. +""" + +import asyncio + +from backend.secuscan.database import get_db + + +async def get_audit_entries(event_type: str): + db = await get_db() + return await db.fetchall( + "SELECT event_type, message, severity, context_json, plugin_id FROM audit_log WHERE event_type = ?", + (event_type,), + ) + + +def test_missing_consent_is_logged(test_client): + payload = { + "plugin_id": "http_inspector", + "inputs": {"url": "http://127.0.0.1:8000"}, + "consent_granted": False, + } + + response = test_client.post("/api/v1/task/start", json=payload) + assert response.status_code == 400 + + entries = asyncio.run(get_audit_entries("task_blocked_consent")) + assert len(entries) >= 1 + latest = entries[-1] + assert latest["severity"] == "warning" + assert latest["plugin_id"] == "http_inspector" + + +def test_unknown_plugin_is_logged(test_client): + payload = { + "plugin_id": "this-plugin-does-not-exist", + "inputs": {"url": "http://127.0.0.1:8000"}, + "consent_granted": True, + } + + response = test_client.post("/api/v1/task/start", json=payload) + assert response.status_code == 404 + + entries = asyncio.run(get_audit_entries("task_blocked_plugin_not_found")) + assert len(entries) >= 1 + assert entries[-1]["plugin_id"] == "this-plugin-does-not-exist" + + +def test_invalid_preset_is_logged(test_client): + payload = { + "plugin_id": "http_inspector", + "preset": "definitely-not-a-real-preset", + "inputs": {"url": "http://127.0.0.1:8000"}, + "consent_granted": True, + } + + response = test_client.post("/api/v1/task/start", json=payload) + assert response.status_code == 400 + + entries = asyncio.run(get_audit_entries("task_blocked_invalid_preset")) + assert len(entries) >= 1 + + +def test_safe_mode_target_rejection_is_logged(test_client): + # A public, non-loopback target is rejected in safe mode (the default), + # since no target policy grants allow_public_targets. start_task's + # safe-mode check currently keys specifically on the "target" input + # field (see the separately-flagged follow-up about plugins whose field + # is named "url"/"host"/"domain" instead), so it's included alongside + # the plugin's own declared "url" field to exercise that path here. + payload = { + "plugin_id": "http_inspector", + "preset": "quick", + "inputs": {"url": "http://8.8.8.8", "target": "8.8.8.8"}, + "consent_granted": True, + } + + response = test_client.post("/api/v1/task/start", json=payload) + assert response.status_code == 400 + + entries = asyncio.run(get_audit_entries("task_blocked_safe_mode")) + assert len(entries) >= 1 + context = entries[-1]["context_json"] + assert "8.8.8.8" in context + + +def test_successful_task_creation_is_still_logged_as_before(test_client): + # Guards against a regression where instrumenting the rejection paths + # accidentally breaks (or duplicates) the existing "task_created" audit + # entry for the success path. + from unittest.mock import patch + + with patch("backend.secuscan.executor.TaskExecutor._execute_command") as mock_exec: + mock_exec.return_value = ("Mocked successful output", 0) + + payload = { + "plugin_id": "http_inspector", + "preset": "quick", + "inputs": {"url": "http://127.0.0.1:8000"}, + "consent_granted": True, + } + + response = test_client.post("/api/v1/task/start", json=payload) + assert response.status_code == 200 + task_id = response.json()["task_id"] + + db = asyncio.run(get_db()) + entries = asyncio.run( + db.fetchall( + "SELECT event_type FROM audit_log WHERE task_id = ? AND event_type = 'task_created'", + (task_id,), + ) + ) + assert len(entries) == 1