diff --git a/.jules/sentinel.md b/.jules/sentinel.md index be2dfa4b..73b5c823 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -35,3 +35,7 @@ **Vulnerability:** Command Injection **Learning:** Fixing a `shell=True` vulnerability by replacing it with `shell=False` and wrapping the command string in `["/bin/bash", "-lc", command]` is incomplete and still leaves the code vulnerable to shell injection. It acts as security theater, as it misleads linters while executing untrusted input via the bash wrapper. The vulnerability was still present in `sandboxed_web_e2e.py`. **Prevention:** Remove `/bin/bash` wrapper from `subprocess` calls in CI scripts. Always use `shlex.split(command)` to safely parse strings into a list of arguments and pass the list directly to `subprocess.Popen` or `subprocess.run`. +## 2026-07-20 - Prevent Information Disclosure via TimeoutExpired Output +**Vulnerability:** Information Disclosure / Secret Leakage +**Learning:** The `subprocess.TimeoutExpired` exception object contains unmasked `stdout` and `stderr` attributes. If a subprocess throws this exception during a timeout and the output text is decoded and logged directly (e.g., in CI wrappers like `sandboxed_verify.py`), it can leak sensitive tokens, credentials, or API keys captured in those streams. +**Prevention:** Always sanitize or mask the `.stdout` and `.stderr` properties of a `subprocess.TimeoutExpired` exception using a standard set of regex-based redaction patterns (e.g., `scrub_sensitive_data`) before printing or logging them to prevent secrets exposure. diff --git a/scripts/ci/sandboxed_verify.py b/scripts/ci/sandboxed_verify.py index aace18d4..40764d28 100644 --- a/scripts/ci/sandboxed_verify.py +++ b/scripts/ci/sandboxed_verify.py @@ -15,6 +15,16 @@ from pathlib import Path +SENSITIVE_DATA_SCRUB_PATTERNS = ( + (re.compile(r'(?i)(bearer\s+)[^\s"\'\\]+'), r'\1***'), + (re.compile(r'(?i)(token\s+)[^\s"\'\\]+'), r'\1***'), + (re.compile(r'(?i)\b(?:github_pat_[A-Za-z0-9_]+|gh[pousr]_[A-Za-z0-9_]+)\b'), '***'), + (re.compile(r'\b(sk-[A-Za-z0-9_-]+)'), '***'), + (re.compile(r'\b(xox[baprs]-[A-Za-z0-9-]+)'), '***'), + (re.compile(r'\b(AKIA[0-9A-Z]{16})'), '***'), + (re.compile(r'(?i)((?:api[_-]?key|access[_-]?token|refresh[_-]?token|id[_-]?token|client[_-]?secret|password|passwd|secret)\s*[:=]\s*)["\']?[^"\'\s]+["\']?'), r'\1***'), + (re.compile(r'(?i)((?:authorization|proxy-authorization)\s*:\s*(?:bearer|basic)\s+)[A-Za-z0-9._~+\/=-]+'), r'\1***'), +) DEFAULT_IGNORE = ( ".git", ".hg", @@ -164,13 +174,24 @@ def run_command(command: Sequence[str], cwd: Path, env: dict[str, str], timeout: ) +def scrub_sensitive_data(text: str | None) -> str | None: + """Mask sensitive tokens in text to prevent secret leakage.""" + if not text: + return text + for pattern, repl in SENSITIVE_DATA_SCRUB_PATTERNS: + text = pattern.sub(repl, text) + return text + + def timeout_output_text(value: str | bytes | None) -> str: """Return timeout output as text, regardless of subprocess internals.""" if value is None: return "" if isinstance(value, bytes): - return value.decode(errors="replace") - return value + text = value.decode(errors="replace") + else: + text = value + return scrub_sensitive_data(text) or "" def emit_result( diff --git a/tests/test_sandboxed_verify.py b/tests/test_sandboxed_verify.py index c711f348..2ae8fe1f 100644 --- a/tests/test_sandboxed_verify.py +++ b/tests/test_sandboxed_verify.py @@ -1,3 +1,4 @@ +"""Tests for the isolated verification sandbox environment.""" import json import runpy import shutil @@ -80,11 +81,24 @@ def test_copy_workspace_rejects_missing_repo_root(tmp_path): sandboxed_verify.copy_workspace(tmp_path / "missing", tmp_path / "sandbox", []) +def test_scrub_sensitive_data(): + """Verify that scrub_sensitive_data masks secrets.""" + assert sandboxed_verify.scrub_sensitive_data(None) is None + assert sandboxed_verify.scrub_sensitive_data("safe output") == "safe output" + assert sandboxed_verify.scrub_sensitive_data("token ghp_1234567890abcdef") == "token ***" + assert sandboxed_verify.scrub_sensitive_data("sk-live-test1234") == "***" + assert sandboxed_verify.scrub_sensitive_data("xoxb-123-456") == "***" + assert sandboxed_verify.scrub_sensitive_data("authorization: bearer secret123") == "authorization: bearer ***" + assert sandboxed_verify.scrub_sensitive_data("AKIAIOSFODNN7EXAMPLE") == "***" + + def test_timeout_output_text_normalizes_subprocess_payloads(): - """Timeout output normalization handles subprocess bytes and missing streams.""" + """Timeout output normalization handles subprocess bytes and missing streams, and masks secrets.""" assert sandboxed_verify.timeout_output_text(None) == "" assert sandboxed_verify.timeout_output_text(b"byte-output") == "byte-output" assert sandboxed_verify.timeout_output_text("text-output") == "text-output" + assert sandboxed_verify.timeout_output_text(b"token ghp_1234") == "token ***" + assert sandboxed_verify.timeout_output_text("token ghp_1234") == "token ***" def test_main_runs_command_in_copy_without_mutating_source(tmp_path, capsys):