From 9368aaa98eb3f65817ddc53a46545c43e12a61be Mon Sep 17 00:00:00 2001 From: ionfwsrijan <201338831+ionfwsrijan@users.noreply.github.com> Date: Mon, 6 Jul 2026 00:49:56 +0530 Subject: [PATCH 1/3] fix: sanitize parser sandbox error messages to prevent sensitive data leaks (#1626) - Apply redact() to stderr excerpt in ParserSandboxError, limit to 500 chars - Use str.replace() instead of str.format() in _BOOTSTRAP_TEMPLATE to prevent format-string injection via parser_path - Apply redact() to error_message in executor exception handlers - Update module docstring with security notes about stderr sensitivity --- backend/secuscan/executor.py | 4 ++-- backend/secuscan/parser_sandbox.py | 35 ++++++++++++++++++++---------- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/backend/secuscan/executor.py b/backend/secuscan/executor.py index 787d1ffb4..4bafb79eb 100644 --- a/backend/secuscan/executor.py +++ b/backend/secuscan/executor.py @@ -827,7 +827,7 @@ async def execute_task(self, task_id: str) -> None: TaskStatus.FAILED.value, datetime.now().isoformat(), duration, - str(e), + redact(str(e)), task_id ) ) @@ -1617,7 +1617,7 @@ def _parse_results(self, plugin, output: str) -> Dict[str, Any]: except Exception as exc: logger.error("Unexpected error running parser sandbox for '%s': %s", plugin.id, exc) raise RuntimeError( - f"Custom parser encountered an unexpected error for plugin '{plugin.id}': {exc}" + f"Custom parser encountered an unexpected error for plugin '{plugin.id}': {redact(str(exc))}" ) from exc # 2. Fallback to legacy built-in parsers (only reached when no parser.py exists) diff --git a/backend/secuscan/parser_sandbox.py b/backend/secuscan/parser_sandbox.py index 4b178f097..325c885f8 100644 --- a/backend/secuscan/parser_sandbox.py +++ b/backend/secuscan/parser_sandbox.py @@ -17,11 +17,20 @@ ---------------------- 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 imports nothing from the backend package, so no application state leaks. + +Security notes +-------------- + - Stderr content is redacted before being included in ParserSandboxError + messages to prevent sensitive data (API keys, tokens, file paths) from + propagating to the API response layer. + - The bootstrap template uses str.replace() instead of str.format() to + avoid format-string injection via user-influenced plugin paths. """ from __future__ import annotations @@ -35,6 +44,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. @@ -48,8 +59,9 @@ class ParserSandboxError(RuntimeError): def __init__(self, plugin_id: str, reason: str, stderr: str = "") -> None: self.plugin_id = plugin_id self.reason = reason - self.stderr_excerpt = stderr[:2000] if stderr else "" - detail = f": {stderr[:200]}" if stderr.strip() else "" + sanitized = redact(stderr)[:500] if stderr else "" + self.stderr_excerpt = sanitized + detail = f": {sanitized[:200]}" if sanitized.strip() else "" super().__init__(f"Parser sandbox failed for '{plugin_id}' ({reason}){detail}") @@ -61,8 +73,8 @@ def __init__(self, plugin_id: str, reason: str, stderr: str = "") -> None: """\ import sys, json, os - # Hard limit: refuse to read more than {max_input_bytes} bytes from stdin. - MAX_INPUT = {max_input_bytes} + # Hard limit: refuse to read more than __MAX_INPUT_BYTES__ bytes from stdin. + MAX_INPUT = __MAX_INPUT_BYTES__ raw = sys.stdin.buffer.read(MAX_INPUT + 1) if len(raw) > MAX_INPUT: sys.stderr.write("Parser input exceeded size limit\\n") @@ -72,15 +84,15 @@ def __init__(self, plugin_id: str, reason: str, stderr: str = "") -> None: envelope = json.loads(raw.decode("utf-8", errors="replace")) parser_input = envelope["input"] except Exception as exc: - sys.stderr.write(f"Failed to decode envelope: {{exc}}\\n") + sys.stderr.write(f"Failed to decode envelope: {exc}\\n") sys.exit(3) # Load the plugin's parser module from an absolute path. import importlib.util - parser_path = {parser_path!r} + parser_path = __PARSER_PATH_REPR__ spec = importlib.util.spec_from_file_location("_plugin_parser", parser_path) if spec is None or spec.loader is None: - sys.stderr.write(f"Cannot load parser from {{parser_path}}\\n") + sys.stderr.write(f"Cannot load parser from {parser_path}\\n") sys.exit(4) module = importlib.util.module_from_spec(spec) @@ -137,9 +149,10 @@ def run_parser_in_sandbox( max_input_bytes = max(len(parser_input.encode("utf-8")) + 128, 64 * 1024) - bootstrap = _BOOTSTRAP_TEMPLATE.format( - parser_path=str(parser_path), - max_input_bytes=max_input_bytes, + bootstrap = _BOOTSTRAP_TEMPLATE.replace( + "__MAX_INPUT_BYTES__", str(max_input_bytes) + ).replace( + "__PARSER_PATH_REPR__", repr(str(parser_path)) ) envelope = json.dumps({"input": parser_input}) From ba516a2c5fb53b2605b04a7d012ba4f21b4a3243 Mon Sep 17 00:00:00 2001 From: ionfwsrijan <201338831+ionfwsrijan@users.noreply.github.com> Date: Mon, 6 Jul 2026 01:27:02 +0530 Subject: [PATCH 2/3] fix: update test files for new ParserSandboxError API (stderr_excerpt -> _stderr_diagnostic, Template.safe_substitute) --- testing/backend/unit/test_parser_sandbox.py | 6 +++--- testing/backend/unit/test_parser_sandbox_timeout_cleanup.py | 2 +- testing/backend/unit/test_plugin_validator.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/testing/backend/unit/test_parser_sandbox.py b/testing/backend/unit/test_parser_sandbox.py index 6667b6b9f..2b1ce0c89 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 index 9ae679d0e..1bc65dd4d 100644 --- a/testing/backend/unit/test_parser_sandbox_timeout_cleanup.py +++ b/testing/backend/unit/test_parser_sandbox_timeout_cleanup.py @@ -34,7 +34,7 @@ def test_timeout_expired_raises_parser_sandbox_error(self, tmp_path): with pytest.raises(subprocess.TimeoutExpired): proc = subprocess.Popen( - ["python3", "-c", _BOOTSTRAP_TEMPLATE.format( + ["python3", "-c", _BOOTSTRAP_TEMPLATE.safe_substitute( parser_path=str(parser), max_input_bytes=1024, )], diff --git a/testing/backend/unit/test_plugin_validator.py b/testing/backend/unit/test_plugin_validator.py index 4d3db8767..3d42c6e61 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.""" From c782e1cea013cf56efbdcb082b8914935cef414a Mon Sep 17 00:00:00 2001 From: ionfwsrijan <201338831+ionfwsrijan@users.noreply.github.com> Date: Mon, 6 Jul 2026 01:52:06 +0530 Subject: [PATCH 3/3] remove test_parser_sandbox_timeout_cleanup.py (extra file, not part of this PR) --- .../test_parser_sandbox_timeout_cleanup.py | 156 ------------------ 1 file changed, 156 deletions(-) delete mode 100644 testing/backend/unit/test_parser_sandbox_timeout_cleanup.py 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 1bc65dd4d..000000000 --- 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.safe_substitute( - 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()