diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 7039d665..b7fab68a 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 - Prevent Information Disclosure in URL Validation Errors +**Vulnerability:** Information Disclosure / Secret Leakage +**Learning:** When validating URLs (e.g., preventing SSRF by checking for `http://` or `https://`), including the original input URL in the raised exception message (`got: {api_url}`) can inadvertently leak sensitive information, such as API keys or internal domain structures, into CI logs if the environment variable was misconfigured or manipulated. +**Prevention:** Never reflect sensitive input values (like URLs or tokens) in security validation error messages. Provide a clear, static message indicating why the validation failed without echoing the untrusted input. diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index 621e4506..47f35db8 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -277,9 +277,11 @@ def call_llm(repo: str, number: int, pr: dict[str, Any], diff: str, truncated: b if not api_url or not api_key: print("Noema LLM review unavailable: NOEMA_LLM_API_URL or NOEMA_LLM_API_KEY is not configured.") return None + api_url_lower = api_url.lower() + if not (api_url_lower.startswith("http://") or api_url_lower.startswith("https://")): + raise ValueError("NOEMA_LLM_API_URL must start with http:// or https:// to prevent SSRF vulnerabilities") + parsed = urllib.parse.urlparse(api_url) - if parsed.scheme.lower() not in {"http", "https"}: - raise ValueError("URL scheme must be http or https") hostname = (parsed.hostname or "").lower() if not hostname: raise ValueError("URL must have a valid hostname") @@ -299,9 +301,6 @@ 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://")): - raise ValueError(f"NOEMA_LLM_API_URL must start with http:// or https:// to prevent SSRF vulnerabilities, got: {api_url}") - prompt = { "role": "user", "content": "\n".join( diff --git a/scripts/ci/run_opencode_review_model_pool.sh b/scripts/ci/run_opencode_review_model_pool.sh index 6cdf1b85..09e314d7 100644 --- a/scripts/ci/run_opencode_review_model_pool.sh +++ b/scripts/ci/run_opencode_review_model_pool.sh @@ -101,6 +101,11 @@ assert_reasoning_effort_for_candidate() { is_context_overflow_failure() { local opencode_json_file="$1" + local exit_code="${2:-}" + + if [ "$exit_code" = "124" ]; then + return 1 + fi [ -s "$opencode_json_file" ] || return 1 grep -Eiq 'ContextOverflowError|tokens_limit_reached|Request body too large|context window' "$opencode_json_file" @@ -132,7 +137,7 @@ run_one_model_attempt() { set -e if [ "$opencode_status" -ne 0 ]; then printf 'OpenCode %s attempt %s/%s failed with exit %s.\n' "$model_candidate" "$attempt" "$attempts" "$opencode_status" - if is_context_overflow_failure "$opencode_json_file"; then + if is_context_overflow_failure "$opencode_json_file" "$opencode_status"; then printf 'OpenCode %s attempt %s/%s exceeded the provider context window; skipping remaining attempts for this model.\n' "$model_candidate" "$attempt" "$attempts" return 2 fi @@ -143,7 +148,7 @@ run_one_model_attempt() { if [ -z "$session_id" ] || [ "$session_id" = "null" ]; then printf 'OpenCode %s attempt %s/%s JSON output did not include a session id.\n' "$model_candidate" "$attempt" "$attempts" cat "$opencode_json_file" - if is_context_overflow_failure "$opencode_json_file"; then + if is_context_overflow_failure "$opencode_json_file" "$opencode_status"; then printf 'OpenCode %s attempt %s/%s exceeded the provider context window; skipping remaining attempts for this model.\n' "$model_candidate" "$attempt" "$attempts" return 2 fi diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index cc68ff28..f7e54288 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -206,7 +206,7 @@ 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="^NOEMA_LLM_API_URL must start with http:// or https:// to prevent SSRF vulnerabilities$"): noema.call_llm("owner/repo", 1, pr, "diff", False) monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example.test/chat") @@ -240,7 +240,7 @@ def fake_urlopen(request, timeout): # Test invalid scheme (and no original URL in error) monkeypatch.setenv("NOEMA_LLM_API_URL", "file:///etc/passwd") - with pytest.raises(ValueError, match="URL scheme must be http or https"): + with pytest.raises(ValueError, match="^NOEMA_LLM_API_URL must start with http:// or https:// to prevent SSRF vulnerabilities$"): noema.call_llm("owner/repo", 1, pr, "diff", False) # Test localhost rejection