diff --git a/.jules/sentinel.md b/.jules/sentinel.md index be2dfa4bb..5ac120d15 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-08-25 - Prevent SSRF via Hostname Validation in Network Requests +**Vulnerability:** Server-Side Request Forgery (SSRF) +**Learning:** Validating URL schemes is insufficient to prevent SSRF if the URL can target arbitrary external or internal hosts. A malicious URL could target internal network services. +**Prevention:** Always use urllib.parse.urlparse to validate that the parsed URL hostname is restricted to safe loopback addresses (e.g., localhost or 127.0.0.1) before opening the URL when making network requests in CI scripts. diff --git a/scripts/ci/sandboxed_web_e2e.py b/scripts/ci/sandboxed_web_e2e.py index ae0c3105a..756df5d67 100644 --- a/scripts/ci/sandboxed_web_e2e.py +++ b/scripts/ci/sandboxed_web_e2e.py @@ -13,6 +13,7 @@ import tempfile import time import urllib.error +import urllib.parse import urllib.request from collections.abc import Sequence from dataclasses import dataclass @@ -121,6 +122,11 @@ def wait_for_url(url: str, timeout: int, service: Service) -> bool: return True if not (url.startswith("http://") or url.startswith("https://")): raise ValueError(f"URL must start with http:// or https://, got: {url}") + + parsed = urllib.parse.urlparse(url) + if parsed.hostname not in {"localhost", "127.0.0.1"}: + raise ValueError(f"URL must target localhost or 127.0.0.1 to prevent SSRF, got: {parsed.hostname}") + deadline = time.monotonic() + timeout opener = urllib.request.build_opener(NoRedirectHandler()) while time.monotonic() < deadline: diff --git a/tests/test_sandboxed_web_e2e.py b/tests/test_sandboxed_web_e2e.py index 6e092c293..89d933626 100644 --- a/tests/test_sandboxed_web_e2e.py +++ b/tests/test_sandboxed_web_e2e.py @@ -114,6 +114,8 @@ def test_wait_helpers_and_service_cleanup_edges(monkeypatch, tmp_path): assert sandboxed_web_e2e.wait_for_url("http://127.0.0.1:1/", 1, exited_service) is False with pytest.raises(ValueError, match="URL must start with http:// or https://"): sandboxed_web_e2e.wait_for_url("file:///etc/passwd", 1, exited_service) + with pytest.raises(ValueError, match="URL must target localhost or 127.0.0.1 to prevent SSRF"): + sandboxed_web_e2e.wait_for_url("http://example.com/", 1, exited_service) sandboxed_web_e2e.stop_service(exited_service) assert sandboxed_web_e2e.tail_text(tmp_path / "missing.log") == ""