diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 7039d665..e985e0f2 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -26,3 +26,7 @@ **Vulnerability:** Server-Side Request Forgery (SSRF) / Local File Inclusion **Learning:** External URL fetching with `urllib.request.urlopen` (like API endpoints passed via environment variables) can accept schemes like `file://` implicitly, which could allow arbitrary file reading or internal network scanning if the environment is misconfigured or manipulated. **Prevention:** Always validate that URLs explicitly start with `http://` or `https://` before using them in standard library requests. Append to suppress linter warnings only after verifying the input is validated. +## 2026-07-06 - Scrub Errors from Secondary Subprocess Wrapper +**Vulnerability:** Information Disclosure / Secret Leakage +**Learning:** We previously patched the main PR execution script to scrub sensitive tokens from `gh` command failure logs. However, the secondary `pr_review_autofix_context.py` script was missed. When its `run_json` method failed to execute a `gh` command, it threw a `RuntimeError` with the raw, unscrubbed `completed.stderr`, risking leakage of repository tokens or secrets on GitHub Actions runners if a sub-command fails. +**Prevention:** Always ensure standard error handling functions (like `run_json`) in every script apply uniform secret scrubbing to command output strings before they are raised into exception traceback formats. diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index 621e4506..51c96c2a 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -299,7 +299,7 @@ def call_llm(repo: str, number: int, pr: dict[str, Any], diff: str, truncated: b if ip.is_private or ip.is_loopback or ip.is_link_local or ip.is_multicast or ip.is_unspecified: raise ValueError("URL cannot target internal IP addresses") - if not (api_url.startswith("http://") or api_url.startswith("https://")): + if not (api_url.lower().startswith("http://") or api_url.lower().startswith("https://")): raise ValueError(f"NOEMA_LLM_API_URL must start with http:// or https:// to prevent SSRF vulnerabilities, got: {api_url}") prompt = { diff --git a/scripts/ci/pr_review_autofix_context.py b/scripts/ci/pr_review_autofix_context.py index 442cfd15..e976e7ff 100755 --- a/scripts/ci/pr_review_autofix_context.py +++ b/scripts/ci/pr_review_autofix_context.py @@ -17,6 +17,28 @@ SHA_RE = re.compile(r"^[0-9a-fA-F]{40}$") +# ⚡ Bolt: Pre-compiled regex patterns to avoid recompilation on every scrub_sensitive_data call. +# Impact: Improves string processing performance in error reporting. +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***'), +) + +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 run_json(args: list[str]) -> Any: """Run gh and decode JSON.""" completed = subprocess.run( @@ -28,7 +50,8 @@ def run_json(args: list[str]) -> Any: shell=False, ) if completed.returncode != 0: - raise RuntimeError(completed.stderr.strip()) + scrubbed_stderr = scrub_sensitive_data(completed.stderr.strip()) + raise RuntimeError(f"Command failed ({completed.returncode}): gh\n{scrubbed_stderr}") return json.loads(completed.stdout or "null") diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index cc68ff28..fa7a49bd 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -206,7 +206,12 @@ def test_call_llm_handles_configuration_and_verdicts(monkeypatch): monkeypatch.setenv("NOEMA_LLM_API_URL", "file:///etc/passwd") monkeypatch.setenv("NOEMA_LLM_API_KEY", "secret") - with pytest.raises(ValueError, match="must start with http:// or https://"): + with pytest.raises(ValueError, match="URL scheme must be http or https"): + noema.call_llm("owner/repo", 1, pr, "diff", False) + + monkeypatch.setenv("NOEMA_LLM_API_URL", "ftp://llm.example.test/chat") + monkeypatch.setenv("NOEMA_LLM_API_KEY", "secret") + with pytest.raises(ValueError, match="URL scheme must be http or https"): noema.call_llm("owner/repo", 1, pr, "diff", False) monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example.test/chat")