🛡️ Sentinel: [HIGH] Fix SSRF vulnerability in Noema LLM API call

.jules/sentinel.md

@@ -26,3 +26,8 @@
 **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-09 - Prevent SSRF via Redirects in urllib
+
+**Vulnerability:** Initial URL validation for SSRF (e.g., checking scheme and IP address) is insufficient if the HTTP client automatically follows redirects. In `urllib.request.urlopen`, redirects are followed by default, allowing an attacker to bypass initial checks by returning a 302 redirect to an internal IP (like `169.254.169.254` or `127.0.0.1`).
+**Learning:** `urllib.request.urlopen` does not inherit the security properties of the initial URL string check. It will follow HTTP redirects unconditionally to any target URL, creating a severe SSRF risk when dealing with external API endpoints that can be manipulated by malicious responses.
+**Prevention:** Explicitly disable redirects by subclassing `urllib.request.HTTPRedirectHandler`, overriding `redirect_request` to raise an `urllib.error.HTTPError`, and using `urllib.request.build_opener(NoRedirectHandler())` instead of the default `urlopen`.

scripts/ci/noema_review_gate.py

@@ -11,6 +11,7 @@
 import socket
 import subprocess
 import sys
+import urllib.error
 import urllib.parse
 import urllib.request
 from collections.abc import Sequence
@@ -257,6 +258,22 @@ def fetch_diff(repo: str, number: int) -> tuple[str, bool]:
     return diff, truncated
 
 
+class NoRedirectHandler(urllib.request.HTTPRedirectHandler):
+    """A URL opener handler that refuses to follow redirects to prevent SSRF."""
+
+    def redirect_request(
+        self,
+        req: urllib.request.Request,
+        fp: Any,
+        code: int,
+        msg: str,
+        headers: Any,
+        newurl: str,
+    ) -> None:
+        """Raise an HTTPError instead of following the redirect."""
+        raise urllib.error.HTTPError(req.full_url, code, msg, headers, fp)
+
+
 def extract_json_object(text: str) -> dict[str, Any]:
     """Extract a JSON object from a strict or lightly wrapped LLM response."""
     stripped = text.strip()
@@ -341,7 +358,8 @@ def call_llm(repo: str, number: int, pr: dict[str, Any], diff: str, truncated: b
         },
         method="POST",
     )
-    with urllib.request.urlopen(request, timeout=120) as response:  # nosec B310
+    opener = urllib.request.build_opener(NoRedirectHandler())
+    with opener.open(request, timeout=120) as response:  # nosec B310
         raw = response.read().decode("utf-8")
     data = json.loads(raw)
     content = (((data.get("choices") or [{}])[0].get("message") or {}).get("content") or "").strip()

tests/test_noema_review_gate.py

@@ -216,23 +216,33 @@ def fake_urlopen(request, timeout):
         seen["body"] = json.loads(request.data.decode("utf-8"))
         return FakeResponse({"choices": [{"message": {"content": '{"decision":"approve","summary":"ok","findings":[]}'}}]})
 
-    monkeypatch.setattr(noema.urllib.request, "urlopen", fake_urlopen)
+    # Since we replaced urlopen with build_opener, we mock build_opener
+    class FakeOpener:
+        def __init__(self, call_func):
+            self.call_func = call_func
+        def open(self, request, timeout=None):
+            return self.call_func(request, timeout)
+
+    monkeypatch.setattr(noema.urllib.request, "build_opener", lambda *args: FakeOpener(fake_urlopen))
     verdict = noema.call_llm("owner/repo", 1, pr, "diff", True)
     assert verdict["decision"] == "approve"
     assert seen["url"] == "https://llm.example.test/chat"
     assert seen["body"]["model"] == "review-model"
 
+    def fake_urlopen_defer(request, timeout=None):
+        return FakeResponse({"choices": [{"message": {"content": '{"decision":"defer"}'}}]})
+
     monkeypatch.setattr(
         noema.urllib.request,
-        "urlopen",
-        lambda *args, **kwargs: FakeResponse({"choices": [{"message": {"content": '{"decision":"defer"}'}}]}),
+        "build_opener",
+        lambda *args: FakeOpener(fake_urlopen_defer)
     )
     with pytest.raises(RuntimeError, match="unsupported decision"):
         noema.call_llm("owner/repo", 1, pr, "diff", False)
 
     # Test case-insensitive valid URL
     monkeypatch.setenv("NOEMA_LLM_API_URL", "HTTPS://llm.example.test/chat")
-    monkeypatch.setattr(noema.urllib.request, "urlopen", fake_urlopen)
+    monkeypatch.setattr(noema.urllib.request, "build_opener", lambda *args: FakeOpener(fake_urlopen))
     assert noema.call_llm("owner/repo", 1, pr, "diff", True)["decision"] == "approve"
 
     # Test invalid scheme (and no original URL in error)
@@ -273,7 +283,7 @@ def fake_getaddrinfo(host, port, *args, **kwargs):
     def fake_getaddrinfo_error(host, port, *args, **kwargs):
         raise socket.gaierror("Name or service not known")
     monkeypatch.setattr(socket, "getaddrinfo", fake_getaddrinfo_error)
-    monkeypatch.setattr(noema.urllib.request, "urlopen", fake_urlopen)
+    monkeypatch.setattr(noema.urllib.request, "build_opener", lambda *args: FakeOpener(fake_urlopen))
     assert noema.call_llm("owner/repo", 1, pr, "diff", True)["decision"] == "approve"
 
     # Test invalid IP string from getaddrinfo (unlikely but theoretically possible)
@@ -286,6 +296,22 @@ def fake_getaddrinfo_invalid_ip(host, port, *args, **kwargs):
     assert noema.call_llm("owner/repo", 1, pr, "diff", True)["decision"] == "approve"
 
 
+def test_noema_redirect_handler_rejects_redirects():
+    """Noema must not follow redirects after validating the initial URL."""
+    handler = noema.NoRedirectHandler()
+    request = noema.urllib.request.Request("https://llm.example.test/chat")
+
+    with pytest.raises(noema.urllib.error.HTTPError):
+        handler.redirect_request(
+            request,
+            fp=None,
+            code=302,
+            msg="Found",
+            headers={},
+            newurl="http://169.254.169.254/latest/meta-data/",
+        )
+
+
 def test_call_llm_rejects_control_character_scheme_evasion(monkeypatch):
     """A URL with an embedded tab is normalized by urlparse to an http scheme
     with a valid hostname, but its raw form does not start with http:// — the