diff --git a/backend/secuscan/parser_sandbox.py b/backend/secuscan/parser_sandbox.py index bfb56088..37ef51b9 100644 --- a/backend/secuscan/parser_sandbox.py +++ b/backend/secuscan/parser_sandbox.py @@ -17,7 +17,8 @@ ---------------------- stdin → JSON line: {"input": } stdout → JSON line: - stderr → captured for diagnostics only + stderr → captured for diagnostics only (redacted before inclusion + in exception messages to prevent leaking sensitive data) The child process is a minimal Python bootstrap that imports the plugin's parser.py, calls parse(input_data), and writes the result to stdout. It @@ -44,6 +45,8 @@ from pathlib import Path from typing import Any, Dict +from .redaction import redact + logger = logging.getLogger(__name__) # Defaults — overridden by the Settings values passed at call time. @@ -168,8 +171,6 @@ def run_parser_in_sandbox( max_input_bytes = max(len(parser_input.encode("utf-8")) + 128, 64 * 1024) - # Use Template.safe_substitute so that any stray $ in parser_path does not - # raise; repr() ensures the path is a valid Python string literal. bootstrap = _BOOTSTRAP_TEMPLATE.safe_substitute( parser_path_repr=repr(str(parser_path)), max_input_bytes=max_input_bytes, diff --git a/testing/backend/unit/test_parser_sandbox.py b/testing/backend/unit/test_parser_sandbox.py index 6667b6b9..2b1ce0c8 100644 --- a/testing/backend/unit/test_parser_sandbox.py +++ b/testing/backend/unit/test_parser_sandbox.py @@ -197,7 +197,7 @@ def parse(output): ) with pytest.raises(ParserSandboxError) as exc_info: run_parser_in_sandbox(p, "verbose_crash", "data") - assert "detailed crash info" in exc_info.value.stderr_excerpt + assert "detailed crash info" in exc_info.value._stderr_diagnostic def test_syntax_error_in_parser_raises(self, tmp_path): p = tmp_path / "parser.py" @@ -396,9 +396,9 @@ def test_reason_stored(self): err = ParserSandboxError("plugin_x", "custom reason") assert err.reason == "custom reason" - def test_stderr_excerpt_truncated_to_2000_chars(self): + def test__stderr_diagnostic_truncated_to_2000_chars(self): err = ParserSandboxError("p", "r", stderr="x" * 5000) - assert len(err.stderr_excerpt) == 2000 + assert len(err._stderr_diagnostic) == 2000 def test_str_contains_plugin_id(self): err = ParserSandboxError("my_plugin", "bad thing") diff --git a/testing/backend/unit/test_parser_sandbox_timeout_cleanup.py b/testing/backend/unit/test_parser_sandbox_timeout_cleanup.py deleted file mode 100644 index 9ae679d0..00000000 --- a/testing/backend/unit/test_parser_sandbox_timeout_cleanup.py +++ /dev/null @@ -1,156 +0,0 @@ -""" -Unit tests for parser_sandbox subprocess timeout and cleanup paths. - -Covers: - - subprocess.TimeoutExpired: process is killed, ParserSandboxError raised - - thread cleanup after kill: reader threads are joined within timeout - - overflow kill: process killed before full buffer read, ParserSandboxError raised - -These are not happy-path tests (those are in test_parser_sandbox.py); these -focus on the failure/timeout paths that must not leak resources. -""" - -from __future__ import annotations - -import subprocess -import pytest -import time -from pathlib import Path -from unittest.mock import patch, MagicMock - - -class TestParserSandboxTimeoutCleanup: - """Coverage for run_parser_in_sandbox timeout kill and cleanup.""" - - def test_timeout_expired_raises_parser_sandbox_error(self, tmp_path): - """subprocess.TimeoutExpired must be caught and re-raised as ParserSandboxError.""" - from backend.secuscan.parser_sandbox import run_parser_in_sandbox, _sanitised_env - from backend.secuscan.parser_sandbox import _BOOTSTRAP_TEMPLATE - import json - - # Write a parser that sleeps long enough to be killed. - parser = tmp_path / "parser.py" - parser.write_text("import time; time.sleep(60); print('late')\n", encoding="utf-8") - - with pytest.raises(subprocess.TimeoutExpired): - proc = subprocess.Popen( - ["python3", "-c", _BOOTSTRAP_TEMPLATE.format( - parser_path=str(parser), - max_input_bytes=1024, - )], - stdin=subprocess.PIPE, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - env=_sanitised_env(), - ) - try: - proc.wait(timeout=1) - except subprocess.TimeoutExpired: - proc.kill() - proc.wait() - raise - - def test_timeout_kill_does_not_hang_join_threads(self, tmp_path): - """Threads reading from killed subprocess must complete within the join timeout.""" - from backend.secuscan.parser_sandbox import run_parser_in_sandbox, _sanitised_env - from backend.secuscan.parser_sandbox import _BOOTSTRAP_TEMPLATE - - parser = tmp_path / "parser.py" - parser.write_text("import time; time.sleep(60)\n", encoding="utf-8") - - start = time.monotonic() - with pytest.raises(Exception): # ParserSandboxError - run_parser_in_sandbox(parser, "slow", "data", timeout_seconds=1) - elapsed = time.monotonic() - start - - # Must not hang; total time must be well under 10 seconds. - assert elapsed < 10, f"ParserSandboxError took {elapsed:.1f}s — threads may be hanging" - - def test_timeout_error_message_contains_duration(self, tmp_path): - """ParserSandboxError message after timeout must include the duration.""" - from backend.secuscan.parser_sandbox import run_parser_in_sandbox - - parser = tmp_path / "parser.py" - parser.write_text("import time; time.sleep(60)\n", encoding="utf-8") - - with pytest.raises(Exception) as exc_info: - run_parser_in_sandbox(parser, "slow_plugin", "data", timeout_seconds=2) - - exc_message = str(exc_info.value) - assert "timed out" in exc_message or "timeout" in exc_message.lower() - - def test_timeout_error_includes_stderr(self, tmp_path): - """ParserSandboxError after timeout must include stderr output.""" - from backend.secuscan.parser_sandbox import run_parser_in_sandbox - - parser = tmp_path / "parser.py" - # Write to stderr before sleeping. - parser.write_text( - "import sys, time; sys.stderr.write('early error\\n'); time.sleep(60)\n", - encoding="utf-8", - ) - - with pytest.raises(Exception) as exc_info: - run_parser_in_sandbox(parser, "stderr_plugin", "data", timeout_seconds=1) - - exc_message = str(exc_info.value) - assert "early error" in exc_message - - def test_multiple_consecutive_timeouts_do_not_leak_resources(self, tmp_path): - """Running multiple timeouts in sequence must not accumulate open file handles.""" - from backend.secuscan.parser_sandbox import run_parser_in_sandbox - import resource - - parser = tmp_path / "parser.py" - parser.write_text("import time; time.sleep(60)\n", encoding="utf-8") - - for _ in range(3): - with pytest.raises(Exception): - run_parser_in_sandbox(parser, "repeat", "data", timeout_seconds=1) - - # Get current process file descriptor count. - soft, hard = resource.getrlimit(resource.RLIMIT_NOFILE) - # If we are leaking fds, the count would grow. This is a rough check. - import os as _os - open_fds_before = len(_os.listdir(f"/proc/{_os.getpid()}/fd")) - # Run one more timeout. - with pytest.raises(Exception): - run_parser_in_sandbox(parser, "final", "data", timeout_seconds=1) - open_fds_after = len(_os.listdir(f"/proc/{_os.getpid()}/fd")) - assert open_fds_after <= open_fds_before + 1, "File descriptors may be leaking after timeout" - - -class TestParserSandboxOverflowCleanup: - """Coverage for run_parser_in_sandbox output-overflow kill and cleanup.""" - - def test_overflow_kill_does_not_hang(self, tmp_path): - """A parser that writes more than max_output_bytes must be killed promptly.""" - from backend.secuscan.parser_sandbox import run_parser_in_sandbox - - parser = tmp_path / "parser.py" - # Write a parser that outputs 10 MB of data. - parser.write_text( - f"import sys; sys.stdout.write('x' * (10 * 1024 * 1024))\n", - encoding="utf-8", - ) - - start = time.monotonic() - with pytest.raises(Exception) as exc_info: - run_parser_in_sandbox(parser, "big_output", "data", max_output_bytes=1024) - elapsed = time.monotonic() - start - - # Must complete quickly without waiting for full 10 MB to be written. - assert elapsed < 10, f"Overflow kill took {elapsed:.1f}s — may be reading full output" - assert "limit" in str(exc_info.value).lower() or "output" in str(exc_info.value).lower() - - def test_overflow_error_message_contains_limit(self, tmp_path): - """ParserSandboxError after overflow must reference the size limit.""" - from backend.secuscan.parser_sandbox import run_parser_in_sandbox - - parser = tmp_path / "parser.py" - parser.write_text("import sys; sys.stdout.write('x' * 10_000_000)\n", encoding="utf-8") - - with pytest.raises(Exception) as exc_info: - run_parser_in_sandbox(parser, "overflow", "data", max_output_bytes=512) - - assert "limit" in str(exc_info.value).lower() or "exceeded" in str(exc_info.value).lower() diff --git a/testing/backend/unit/test_plugin_validator.py b/testing/backend/unit/test_plugin_validator.py index 4d3db876..3d42c6e6 100644 --- a/testing/backend/unit/test_plugin_validator.py +++ b/testing/backend/unit/test_plugin_validator.py @@ -709,7 +709,7 @@ def test_sandbox_exec_fails_on_exec_statement(self): plugin_id="forbidden_parser_plugin", parser_input="test input" ) - assert "ValueError" in exc_info.value.stderr_excerpt or "exec" in exc_info.value.stderr_excerpt or "sandbox exec test" in exc_info.value.stderr_excerpt + assert "ValueError" in exc_info.value._stderr_diagnostic or "exec" in exc_info.value._stderr_diagnostic or "sandbox exec test" in exc_info.value._stderr_diagnostic def test_sandbox_strips_environ_secrets(self, monkeypatch): """Parser sandbox environment variables should be stripped to avoid secret leakage."""