From af25749301652611803d976daf35358b0a4d1d9f Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Mon, 6 Jul 2026 17:55:56 +0000 Subject: [PATCH] =?UTF-8?q?pr=5Freview=5Fautofix=5Fcontext.py=20=EC=8A=A4?= =?UTF-8?q?=ED=81=AC=EB=A6=BD=ED=8A=B8=EC=9D=98=20subprocess=20=EC=97=90?= =?UTF-8?q?=EB=9F=AC=20=ED=95=B8=EB=93=A4=EB=9F=AC=EC=97=90=EC=84=9C=20?= =?UTF-8?q?=EB=85=B8=EC=B6=9C=EB=90=A0=20=EC=88=98=20=EC=9E=88=EB=8A=94=20?= =?UTF-8?q?=EB=AF=BC=EA=B0=90=ED=95=9C=20=EB=8D=B0=EC=9D=B4=ED=84=B0(?= =?UTF-8?q?=ED=86=A0=ED=81=B0=20=EB=93=B1)=EB=A5=BC=20=EB=A7=88=EC=8A=A4?= =?UTF-8?q?=ED=82=B9=ED=95=98=EC=97=AC=20=EB=B3=B4=EC=95=88=20=EA=B0=95?= =?UTF-8?q?=ED=99=94.=20(noema=5Freview=5Fgate.py=20=EC=9D=98=20SSRF=20?= =?UTF-8?q?=EA=B2=80=EC=82=AC=20=EB=8C=80=EC=86=8C=EB=AC=B8=EC=9E=90=20?= =?UTF-8?q?=EB=AC=B4=EC=8B=9C=20=EC=9D=B4=EC=8A=88=20=EC=88=98=EC=A0=95=20?= =?UTF-8?q?=ED=8F=AC=ED=95=A8)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .jules/sentinel.md | 4 ++++ scripts/ci/noema_review_gate.py | 2 +- scripts/ci/pr_review_autofix_context.py | 25 ++++++++++++++++++++++++- tests/test_noema_review_gate.py | 7 ++++++- 4 files changed, 35 insertions(+), 3 deletions(-) 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")