Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -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-24 - SSRF Vulnerability in sandboxed_web_e2e.py
**Vulnerability:** The `wait_for_url` function in `scripts/ci/sandboxed_web_e2e.py` did not validate the URL hostname before making requests, creating a Server-Side Request Forgery (SSRF) risk.
**Learning:** Arbitrary URLs passed to internal utilities must be rigorously validated, especially in CI environments, to ensure they do not access unintended network locations or internal services.
**Prevention:** Parse the URL, accept only the exact `localhost` name or an address for which `ipaddress.ip_address(...).is_loopback` is true, and keep redirects disabled. Do not treat unspecified, link-local, or hostname-suffix lookalikes as loopback.
17 changes: 17 additions & 0 deletions scripts/ci/sandboxed_web_e2e.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from __future__ import annotations

import argparse
import ipaddress
import json
import os
import signal
Expand All @@ -13,6 +14,7 @@
import tempfile
import time
import urllib.error
import urllib.parse
import urllib.request
from collections.abc import Sequence
from dataclasses import dataclass
Expand Down Expand Up @@ -45,6 +47,18 @@ class Service:
log_path: Path


def is_loopback_hostname(hostname: str | None) -> bool:
"""Return whether a parsed hostname is an exact local loopback target."""
if hostname is None:
return False
if hostname.casefold() == "localhost":
return True
try:
return ipaddress.ip_address(hostname).is_loopback
except ValueError:
return False
Comment thread
seonghobae marked this conversation as resolved.
Outdated


def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace:
"""Parse CLI arguments for sandboxed web E2E execution."""
parser = argparse.ArgumentParser(
Expand Down Expand Up @@ -121,6 +135,9 @@ 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 not is_loopback_hostname(parsed.hostname):
raise ValueError(f"URL hostname must be restricted to safe loopback addresses, got: {parsed.hostname}")
deadline = time.monotonic() + timeout
opener = urllib.request.build_opener(NoRedirectHandler())
while time.monotonic() < deadline:
Expand Down
23 changes: 22 additions & 1 deletion tests/test_sandboxed_web_e2e.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ def test_wait_helpers_and_service_cleanup_edges(monkeypatch, tmp_path):

assert sandboxed_web_e2e.wait_for_url("", 1, exited_service) is True
assert sandboxed_web_e2e.wait_for_url("http://127.0.0.1:1/", 1, exited_service) is False
assert sandboxed_web_e2e.wait_for_url("http://127.0.0.2:1/", 1, exited_service) is False
Comment thread
seonghobae marked this conversation as resolved.
Outdated
assert sandboxed_web_e2e.wait_for_url("http://localhost:1/", 1, exited_service) is False
assert sandboxed_web_e2e.wait_for_url("http://[::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)
sandboxed_web_e2e.stop_service(exited_service)
Expand Down Expand Up @@ -232,7 +235,7 @@ def test_no_redirect_handler_raises_httperror_without_following():
"""Readiness checks must raise HTTPError on redirects to prevent attacker-controlled internal URLs."""
import urllib.error

request = sandboxed_web_e2e.urllib.request.Request("https://example.test/ready")
request = sandboxed_web_e2e.urllib.request.Request("http://127.0.0.1/ready")

with pytest.raises(urllib.error.HTTPError) as exc_info:
sandboxed_web_e2e.NoRedirectHandler().redirect_request(request, None, 302, "Found", {}, "http://127.0.0.1")
Expand Down Expand Up @@ -598,3 +601,21 @@ def test_module_import_and_main_entrypoint(monkeypatch, tmp_path):
if module is not None:
sys.modules["scripts.ci.sandboxed_web_e2e"] = module
assert exc_info.value.code == 0


@pytest.mark.parametrize(
"url",
[
"http://example.com/ready",
"http:///ready",
"http://0.0.0.0/ready",
"http://[::]/ready",
"http://169.254.169.254/latest/meta-data/",
"http://127.0.0.1.evil.test/ready",
"http://localhost@evil.test/ready",
],
)
def test_wait_for_url_rejects_non_loopback_hostnames(url):
"""Readiness polling rejects public, unspecified, link-local, and hostname-confusion targets."""
with pytest.raises(ValueError, match="URL hostname must be restricted to safe loopback addresses"):
sandboxed_web_e2e.wait_for_url(url, 1, None)
Loading