Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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-08 - Prevent SSRF in Web E2E Readiness Polling
**Vulnerability:** Server-Side Request Forgery (SSRF) / Local File Inclusion
**Learning:** Checking that a user-provided `--backend-ready-url` or `--frontend-ready-url` starts with `http://` or `https://` is insufficient if the script blindly requests the URL. This allows attackers to proxy requests through CI to internal endpoints or metadata services (e.g. `169.254.169.254`).
**Prevention:** Strictly parse the URL using `urllib.parse.urlparse` and ensure the hostname belongs to an explicitly allowed loopback interface (e.g., `127.0.0.1`, `localhost`, `::1`).
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ Semantic Versioning where the repository publishes a release.

### Fixed

- Replaced prefix-only HTTP/HTTPS readiness validation with robust SSRF protection in the `sandboxed_web_e2e.py` E2E test harness.
- Added log redaction for standard output and error captured during subprocess execution in `sandboxed_verify.py` and `sandboxed_web_e2e.py` wrappers.
- Bounded the Strix quality self-test's deterministic timeout fixtures to 3-second process and 5-second fake-sleep budgets so exact-head policy evidence completes inside the existing job limit without changing production Strix scanner timeouts, providers, credentials, or review semantics.
- Allowed commas and ASCII parentheses in the bounded Strix changed-file path policy so legal tracked Packrat fixtures can receive exact-head security analysis, while rejecting raw `..` components before normalization and keeping controls, backslashes, whitespace ambiguity, and shell punctuation fail-closed.
- Bound each review-agent invocation key to the wrapper's complete canonical payload, including the base branch and requesting actor; altered fields with a valid-format key now fail before durable-leader election or forwarding, and wrapper write permission is job-scoped.
Expand Down
15 changes: 11 additions & 4 deletions scripts/ci/sandboxed_verify.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@
from collections.abc import Sequence
from pathlib import Path

if __package__ in (None, ""): # pragma: no cover
sys.path.insert(0, str(Path(__file__).resolve().parents[2]))

from scripts.ci.redact_sensitive_log import redact_text


DEFAULT_IGNORE = (
".git",
Expand Down Expand Up @@ -169,8 +174,10 @@ def timeout_output_text(value: str | bytes | None) -> str:
if value is None:
return ""
if isinstance(value, bytes):
return value.decode(errors="replace")
return value
text = value.decode(errors="replace")
else:
text = value
return redact_text(text)


def emit_result(
Expand Down Expand Up @@ -219,9 +226,9 @@ def main(argv: Sequence[str] | None = None) -> int:
try:
completed = run_command(args.command, copied_repo, env, args.timeout)
if completed.stdout:
print(completed.stdout, end="")
print(redact_text(completed.stdout), end="")
if completed.stderr:
print(completed.stderr, end="", file=sys.stderr)
print(redact_text(completed.stderr), end="", file=sys.stderr)
exit_code = completed.returncode
except subprocess.TimeoutExpired as exc:
stdout = timeout_output_text(exc.stdout)
Expand Down
13 changes: 10 additions & 3 deletions scripts/ci/sandboxed_web_e2e.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,17 @@
import tempfile
import time
import urllib.error
import urllib.parse
import urllib.request
from collections.abc import Sequence
from dataclasses import dataclass
from pathlib import Path

if __package__ in (None, ""):
if __package__ in (None, ""): # pragma: no cover
sys.path.insert(0, str(Path(__file__).resolve().parents[2]))

from scripts.ci import sandboxed_verify
from scripts.ci.redact_sensitive_log import redact_text


RESULT_MARKER = "SANDBOXED_WEB_E2E_RESULT"
Expand Down Expand Up @@ -121,6 +123,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 ("127.0.0.1", "localhost", "::1"):
raise ValueError(f"Readiness URL must use localhost or loopback IP, got: {url}")

deadline = time.monotonic() + timeout
opener = urllib.request.build_opener(NoRedirectHandler())
while time.monotonic() < deadline:
Expand Down Expand Up @@ -232,9 +239,9 @@ def main(argv: Sequence[str] | None = None) -> int:
try:
completed = run_shell(args.e2e_cmd, copied_repo, env, args.e2e_timeout)
if completed.stdout:
print(completed.stdout, end="")
print(redact_text(completed.stdout), end="")
if completed.stderr:
print(completed.stderr, end="", file=sys.stderr)
print(redact_text(completed.stderr), end="", file=sys.stderr)
exit_code = completed.returncode
return exit_code
except subprocess.TimeoutExpired as exc:
Expand Down
46 changes: 46 additions & 0 deletions tests/test_sandboxed_verify.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import subprocess
import json
import runpy
import shutil
Expand Down Expand Up @@ -198,3 +199,48 @@ def test_module_main_entrypoint(monkeypatch, tmp_path):
if module is not None:
sys.modules["scripts.ci.sandboxed_verify"] = module
assert exc_info.value.code == 0

def test_sandboxed_verify_redacts_stdout_and_stderr(monkeypatch, tmp_path, capsys):
"""The wrapper redacts sensitive tokens from completed process stdout and stderr."""
class CompletedProcess:
def __init__(self, stdout, stderr, returncode):
self.stdout = stdout
self.stderr = stderr
self.returncode = returncode

def run_command(command, cwd, env, timeout):
return CompletedProcess(
"Here is my api_key: 'ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ12345678'",
"Failed because of password: mysecretpassword123",
0
)

monkeypatch.setattr(sandboxed_verify, "run_command", run_command)

sandboxed_verify.main(["echo", "test"])

out, err = capsys.readouterr()
assert "ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ12345678" not in out
assert "[REDACTED]" in out
assert "mysecretpassword123" not in err
assert "[REDACTED]" in err

def test_timeout_redacts_bytes_output(monkeypatch, tmp_path, capsys):
"""The wrapper redacts sensitive tokens from timeout stdout and stderr bytes."""
def run_command(command, cwd, env, timeout):
raise subprocess.TimeoutExpired(
cmd="mock",
timeout=10,
output=b"Timeout with token: ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ12345678",
stderr=b"Timeout error with password: mysecretpassword123"
)

monkeypatch.setattr(sandboxed_verify, "run_command", run_command)

sandboxed_verify.main(["echo", "test"])

out, err = capsys.readouterr()
assert "ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ12345678" not in out
assert "[REDACTED]" in out
assert "mysecretpassword123" not in err
assert "[REDACTED]" in err
52 changes: 52 additions & 0 deletions tests/test_sandboxed_web_e2e.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import subprocess
import json
import os
import runpy
Expand Down Expand Up @@ -598,3 +599,54 @@ 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

def test_sandboxed_web_e2e_redacts_stdout_and_stderr(monkeypatch, tmp_path, capsys):
"""The E2E wrapper redacts sensitive tokens from completed process stdout and stderr."""
class CompletedProcess:
def __init__(self, stdout, stderr, returncode):
self.stdout = stdout
self.stderr = stderr
self.returncode = returncode

def run_shell(command, cwd, env, timeout):
return CompletedProcess(
"Found token: 'github_pat_11AAAAAAA000000000000000000000000000000000000000000000000000000000000000000000'",
"Error: could not login with session_key: 123456",
1
)

def mock_wait_for_url(*args):
return True

def mock_start_service(*args):
class Service:
label = "mock"
command = "mock"
process = type("Proc", (), {"poll": lambda self: None, "pid": 123, "wait": lambda self, timeout: None})()
log_path = tmp_path / "mock.log"
def __init__(self):
self.log_path.touch()
return Service()

monkeypatch.setattr(sandboxed_web_e2e, "run_shell", run_shell)
monkeypatch.setattr(sandboxed_web_e2e, "wait_for_url", mock_wait_for_url)
monkeypatch.setattr(sandboxed_web_e2e, "start_service", mock_start_service)

sandboxed_web_e2e.main(["--backend-cmd", "mock", "--frontend-cmd", "mock", "--e2e-cmd", "mock"])

out, err = capsys.readouterr()
assert "github_pat_11AAAAAAA000000000000000000000000000000000000000000000000000000000000000000000" not in out
assert "[REDACTED]" in out
assert "session_key: 123456" not in err
assert "[REDACTED]" in err
def test_wait_for_url_rejects_external_hosts(monkeypatch, tmp_path):
class RunningProcess:
def poll(self):
return None
service = sandboxed_web_e2e.Service("web", "serve", RunningProcess(), tmp_path / "mock.log")

with pytest.raises(ValueError, match="Readiness URL must use localhost or loopback IP"):
sandboxed_web_e2e.wait_for_url("http://169.254.169.254/latest/meta-data/", 10, service)

with pytest.raises(ValueError, match="Readiness URL must use localhost or loopback IP"):
sandboxed_web_e2e.wait_for_url("https://example.com", 10, service)
Loading