Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 1 addition & 1 deletion .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
**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`.
## 2026-07-13 - Complete the Fix for Command Injection Security Theater
**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`.
**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.
**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-28 - Prevent SSRF via URL parsing and Command Injection via explicit shell=False
**Vulnerability:** Server-Side Request Forgery (SSRF) and Implicit Shell Usage in Subprocess
Expand Down
6 changes: 3 additions & 3 deletions scripts/ci/pingora_edge_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,9 @@ class ContentSizeExceededError(PolicyError):
class NoRedirectHandler(HTTPRedirectHandler):
"""Reject redirects so validated GitHub API requests keep one origin."""

def redirect_request(self, *_args: object, **_kwargs: object) -> None:
"""Return no follow-up request for any HTTP redirect response."""
return None
def redirect_request(self, req: Request, fp: object, code: int, msg: str, headers: object, newurl: str) -> None:
"""Raise an HTTPError instead of following the redirect."""
raise HTTPError(req.full_url, code, msg, headers, fp)
Comment thread
seonghobae marked this conversation as resolved.


github_opener = build_opener(NoRedirectHandler())
Expand Down
6 changes: 3 additions & 3 deletions scripts/ci/reconcile_repository_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ class _NoPagesRedirects(HTTPRedirectHandler):
"""Refuse redirects so Pages verification cannot be redirected off GitHub Pages."""

def redirect_request(self, req, fp, code, msg, headers, newurl):
"""Return no follow-up request for any redirect."""

return None
"""Raise an HTTPError instead of following the redirect."""
from urllib.error import HTTPError
raise HTTPError(req.full_url, code, msg, headers, fp)


def _require_exact_dict(value: Any, *, field: str) -> dict[str, Any]:
Expand Down
3 changes: 2 additions & 1 deletion tests/test_pingora_edge_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,8 @@ def test_github_open_json_rejects_nonapproved_origins(url: str) -> None:
def test_github_opener_never_constructs_redirect_requests() -> None:
"""The policy opener refuses redirects rather than changing API origins."""

assert policy.NoRedirectHandler().redirect_request(None, None, 302, "Found", {}, "https://evil.example") is None
with pytest.raises(HTTPError):
policy.NoRedirectHandler().redirect_request(policy.Request("https://example.com"), None, 302, "Found", {}, "https://evil.example")


def test_annotation_escapes_workflow_command_fields() -> None:
Expand Down
7 changes: 3 additions & 4 deletions tests/test_repository_metadata_live_verification.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,11 @@ def build_ok(handler):
]
assert len(handlers) == 1
assert isinstance(handlers[0], RECONCILER._NoPagesRedirects)
assert (
from urllib.error import HTTPError
with pytest.raises(HTTPError):
handlers[0].redirect_request(
None, None, 302, "redirect", {}, "http://127.0.0.1/"
RECONCILER.Request("https://example.com"), None, 302, "redirect", {}, "http://127.0.0.1/"
)
is None
)

with pytest.raises(RuntimeError, match="not built"):
RECONCILER._pages_publication_ready("Repo", {**ready, "status": "building"})
Expand Down
Loading