diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 2da382f934..bc454b7b5f 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -51,3 +51,7 @@ **Vulnerability:** Denial of Service / Availability **Learning:** Strix security scanners crashed when the backend LLM returned an 'HTTP Error 502: Bad Gateway' response. This was because 'bad gateway' string match and generic 'APIError' were missing from the `is_llm_api_connection_error` function in the Strix retry gate. **Prevention:** Always include `bad gateway` and `APIError` in string match conditions when handling HTTP API Connection exceptions for LLM backends to ensure proper fail-closed and retry handling. +## 2026-10-08 - Make urllib Redirect Rejection Explicit +**Vulnerability:** Authenticated redirect rejection must remain fail-closed if the configured opener chain changes. +**Learning:** In the current `build_opener(_RejectRedirects())` chain, returning `None` did not forward the redirect: `HTTPDefaultErrorHandler` already raised `HTTPError`. Raising the same typed error directly in `_RejectRedirects` is defense in depth that removes dependence on the downstream handler order; it does not prove a prior SSRF or credential leak. +**Prevention:** Exercise redirects through the production opener and assert that transport receives only the original request. Keep the explicit `HTTPError` raise so later opener composition cannot weaken the fail-closed boundary. diff --git a/scripts/ci/codeql_ghas_configuration_identity.py b/scripts/ci/codeql_ghas_configuration_identity.py index 53e00c41c6..9f2e7bbf56 100644 --- a/scripts/ci/codeql_ghas_configuration_identity.py +++ b/scripts/ci/codeql_ghas_configuration_identity.py @@ -48,7 +48,7 @@ def redirect_request( _new_url: str, ) -> None: """Refuse every redirect so bearer headers never cross the reviewed authority.""" - return None + raise urllib.error.HTTPError(_request.full_url, _code, _message, _headers, _file_pointer) _GITHUB_API_OPENER = urllib.request.build_opener(_RejectRedirects()) diff --git a/scripts/ci/strix_evidence_binding.py b/scripts/ci/strix_evidence_binding.py index 7319040df2..0f2af27583 100644 --- a/scripts/ci/strix_evidence_binding.py +++ b/scripts/ci/strix_evidence_binding.py @@ -87,7 +87,7 @@ def redirect_request( _new_url: str, ) -> None: """Refuse every redirect so bearer headers never cross the reviewed authority.""" - return None + raise HTTPError(_request.full_url, _code, _message, _headers, _file_pointer) _GITHUB_API_OPENER = build_opener(_RejectRedirects()) diff --git a/tests/test_github_api_url_boundary.py b/tests/test_github_api_url_boundary.py index a9050584fd..79b31de87d 100644 --- a/tests/test_github_api_url_boundary.py +++ b/tests/test_github_api_url_boundary.py @@ -8,6 +8,7 @@ import re import subprocess from typing import Any +import urllib.error from urllib.request import Request from urllib.response import addinfourl @@ -178,9 +179,9 @@ def test_codeql_identity_client_never_constructs_redirect_request_with_bearer_to ) handler = identity._RejectRedirects() - redirected = handler.redirect_request(request, None, 302, "Found", {}, target) + with pytest.raises(urllib.error.HTTPError): + handler.redirect_request(request, None, 302, "Found", {}, target) - assert redirected is None assert request.get_header("Authorization") == "Bearer test-token" @@ -195,9 +196,9 @@ def test_strix_evidence_client_never_constructs_redirect_request_with_bearer_tok ) handler = binding._RejectRedirects() - redirected = handler.redirect_request(request, None, 302, "Found", {}, target) + with pytest.raises(urllib.error.HTTPError): + handler.redirect_request(request, None, 302, "Found", {}, target) - assert redirected is None assert request.get_header("Authorization") == "Bearer test-token" @@ -246,10 +247,23 @@ def test_documented_opener_lineage_references_published_commits() -> None: assert "9c19c6e00eafc028068719ab482282c1256f8893" in baseline assert "b35410673ce60f9a693532daf74862c08971e9e3" not in evidence assert "72e17608cac2d673b50b8380301649fb86d18096" not in evidence + _assert_g17_evidence_is_published(baseline) -def test_published_lineage_guard_rejects_unreachable_g17_evidence() -> None: +def test_sentinel_describes_redirect_change_as_hardening_not_prior_bypass() -> None: + """Security guidance must distinguish existing rejection from hardening.""" + sentinel = Path(".jules/sentinel.md").read_text(encoding="utf-8") + section = sentinel.split( + "## 2026-10-08 - Make urllib Redirect Rejection Explicit", 1 + )[1] + + assert "HTTPDefaultErrorHandler` already raised `HTTPError`" in section + assert "defense in depth" in section + assert "could still be processed" not in section + + +def test_published_lineage_guard_rejects_unreachable_g17_evidence(monkeypatch: pytest.MonkeyPatch) -> None: """A commit-shaped but unpublished G-17 evidence identifier must fail closed.""" baseline = Path("docs/product-technical-gap-baseline.md").read_text( encoding="utf-8" @@ -260,6 +274,12 @@ def test_published_lineage_guard_rejects_unreachable_g17_evidence() -> None: 1, ) + monkeypatch.setattr( + subprocess, + "run", + lambda *a, **kw: type("MockProc", (object,), {"returncode": 1})(), + ) + with pytest.raises(AssertionError, match="not published"): _assert_g17_evidence_is_published(mutated)