diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fee33cc73..34281625cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,6 +96,7 @@ - Raised `hourly-review-repair.yml`'s discovery ceiling from 50 to 200 while rotating deterministic 50-PR deep-inspection windows by hourly run number. The scheduler hydrates only the selected window and stops immediately after its single dispatch, preserving access to newer PRs without quadrupling expensive review/check/comment work. See `docs/doctoring/hourly-review-repair-single-file-consolidation.md`'s 2026-09-03 follow-up. ## [Unreleased] +- **Bind GitHub REST redirect evidence to both production opener chains.** `.github#2279` now feeds a synthetic same-authority 302 through the CodeQL identity and Strix evidence clients' real module-level openers, proving the redirect target is never contacted and the bearer header is never forwarded. Removing `_RejectRedirects` from either opener makes the contract fail on the forbidden second request. Four stale Strix HTTP/transport/JSON fixtures now patch that same production seam; direct handler unit cases and standalone CodeQL materialization remain unchanged. - **Define an evidence-backed repository README quality standard.** Added `docs/repository-readme-quality-standard.md` as the shared review contract for product-first structure, code-current onboarding, authority boundaries, durable quality signals, and repository/source/dependency license due diligence. Product repositories continue to own their own README prose; the standard is linked from the root documentation map and does not centralize or generate product claims. - Include merge-scheduler entrypoint, core, and regression-test changes in the existing runtime-quality workflow's trigger and suite selector. Scheduler diff --git a/docs/doctoring/github-api-published-lineage-authority.md b/docs/doctoring/github-api-published-lineage-authority.md new file mode 100644 index 0000000000..5f6a363848 --- /dev/null +++ b/docs/doctoring/github-api-published-lineage-authority.md @@ -0,0 +1,28 @@ +# GitHub API evidence published-lineage authority + +Status: Proposed repair evidence for `.github` PR #2279. Hosted exact-head security and independent review remain mandatory. + +## Finding + +The first published-lineage contract checked that the documentation named intended replacement SHAs and omitted two known unreachable candidates. That established expected spelling but not repository reachability. A 40-hex identifier can satisfy those assertions while referring to no commit published in the repository, so the contract did not make G-17's evidence lineage independently reconstructable. + +Current-head review identified that gap and required the G-17 evidence identifiers themselves to resolve and belong to the current published branch ancestry. + +## RED → repair + +- Structural RED `c37db5405142da1d0fa2ae972cbacab28563c370` factors a G-17 evidence validator and adds a mutation control that substitutes the first evidence commit with the all-zero, commit-shaped identifier. The intentionally shape-only validator accepts that mutation, so the regression fails instead of giving false assurance. +- Minimal repair `b339370ed1e032527e504ca3500a2f0ca825ff77` keeps validation in the existing GitHub API authority contract. For every full SHA named in the single G-17 row it now requires both `git cat-file -e ^{commit}` and `git merge-base --is-ancestor HEAD` to succeed. The negative mutation therefore fails closed, while the documented published evidence must be resolvable in current history. + +The repair does not change either production HTTP client, credential handling, redirect policy, workflow threshold, or the standalone `$RUNNER_TEMP` CodeQL materialization boundary. It strengthens only executable evidence traceability. + +## Invariants + +1. G-17 has exactly one gap-register row. +2. Every full commit SHA named by that row resolves as a commit in the checked-out repository. +3. Every such evidence commit is an ancestor of the exact checked-out head; detached or unreachable object-store artifacts are not accepted as published lineage. +4. A syntactically valid but unreachable 40-hex identifier fails the contract. +5. Exact-head hosted CI/security gates and independent review remain distinct from this focused local invariant. + +## Rejected alternatives + +Checking only SHA syntax was rejected because it proves formatting rather than publication. Checking only that expected strings occur in Markdown was rejected because unreachable objects can still be named. GitHub API lookups were unnecessary for the repository-local invariant and would add network/credential authority to a test whose evidence is already in Git history. diff --git a/docs/doctoring/github-api-url-authority-2248.md b/docs/doctoring/github-api-url-authority-2248.md new file mode 100644 index 0000000000..01db8f1f17 --- /dev/null +++ b/docs/doctoring/github-api-url-authority-2248.md @@ -0,0 +1,73 @@ +# GitHub REST URL authority boundary for central CI clients + +Status: Proposed repair for `.github` issue #2248; exact-head hosted security and independent review remain mandatory. + +## Problem + +Protected `.github/main` at `64aa08d7fa487deacd41c761c36277ca68cab6c9` contains two central CI HTTP clients: + +- `scripts/ci/codeql_ghas_configuration_identity.py` for CodeQL analyses; +- `scripts/ci/strix_evidence_binding.py` for pull-request changed-file evidence. + +The whole-tree Semgrep gate reported `python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected` at both original dynamic `urlopen` sites, and Bandit B310 reported the same class. A comment-only suppression would not prove the security premise that bearer-authenticated requests stay inside GitHub REST authority. + +The first repair made the initial URL predicate executable, but exact-head CodeRabbit review then identified a second authority transition: Python's default `HTTPRedirectHandler` can construct a redirected request from the already-authorized request and preserve request headers, including `Authorization`. Validating only the first `https://api.github.com/...` URL therefore did not prevent a 3xx response from redirecting the bearer token to another authority. + +## Initial URL RED → repair + +Structural RED `4732f3e29ab8cd0b88506beecd4e70bdfaafb8da` requires both clients to reject, before network/file opener execution: + +- `http://api.github.com/...`; +- `https://api.github.com.evil.example/...`; +- `https://api.github.com@evil.example/...`; +- `https://api.github.com:443/...` because the canonical authority is exact; +- an otherwise canonical URL carrying a fragment; +- `file:///etc/passwd`. + +The production predicate requires scheme exactly `https`, network authority exactly `api.github.com`, an absolute path, and no fragment. The positive control proves exact `https://api.github.com/...` reaches the injected opener and decodes JSON normally. + +A temporary shared helper candidate was removed because `codeql-scan-dispatch.yml` materializes `codeql_ghas_configuration_identity.py` into `$RUNNER_TEMP` and executes it as a standalone file. The CodeQL helper therefore keeps its small fail-closed transport boundary self-contained instead of gaining a repository-local import dependency that the workflow does not materialize. + +## Redirect RED → repair + +CodeRabbit's current-head review of `9ba43f284da51bfa6aaa389d3fb67f8b232fbba5` correctly rejected the initial-only guard: default `urllib` redirect handling can create a new request after the first authority check and carry the bearer header to the new target. + +Structural redirect RED `7a00442cbfd01408068a060c2bebba84041a33eb` adds hostile redirect targets for a lookalike HTTPS host, `http://api.github.com/...`, and `file:///...`. The contract requires both clients' redirect handlers to return no redirected request while the original request retains its bearer header; the repair also blocks same-authority redirects so there is no unreviewed second authority transition at all. + +Production repair lineage: + +- `a2e9126416c96bb8c5fa1e00190a8eca45758883` replaces CodeQL's default `urlopen` transport with a local `OpenerDirector` whose `_RejectRedirects` handler refuses every redirect; +- `4c7bcbeb06e421b98b0992b62cac06eaae45a98c` applies the same fail-closed boundary to the Strix evidence client; +- `e06b6dd84b012db9c3fafc09d417a85f4aaeff4c` adds direct-handler hostile cases, canonical opener positive controls, and same-authority redirects to the refusal contract; +- `57477289ebec5631b0c48f0bc419f336dbe19deb` closes the remaining executable-binding gap: both actual module-level production openers receive a synthetic 302 through their real HTTPS open/response chains, and the regression proves transport sees exactly the original canonical request plus bearer and never receives a redirected request. + +The redirect repair removes the two dynamic `urlopen` sinks rather than broadening a Semgrep/Bandit suppression. A 3xx response now terminates as the opener's HTTP error path; no second request object is created and the bearer credential cannot be forwarded by redirect machinery. The executable proof patches only the actual opener's bounded HTTPS transport slot for a synthetic response; it does not replace `open()`, call the redirect handler directly as its oracle, or contact a network endpoint. + +## Production opener-chain RED → evidence repair + +Current-head review found that the direct `_RejectRedirects.redirect_request(...)` unit cases would remain green if either production `_GITHUB_API_OPENER` were accidentally rebuilt with Python's default redirect handler. Commit `57477289ebec5631b0c48f0bc419f336dbe19deb` therefore drives each public client path through its actual module-level opener. A synthetic HTTPS transport returns `302 Location: https://api.github.com/repos/ContextualWisdomLab/redirected`; the contract requires the client-specific HTTP error and exactly one transport call containing the original bearer header. + +Mutation RED temporarily replaced both `build_opener(_RejectRedirects())` constructions with `build_opener()`. Both new tests failed on the forbidden second request and recorded `Authorization='Bearer test-token'` at that redirect target. Restoring the production constructors made the complete authority file GREEN (`31 passed`, including malformed-authority parse failures for both clients and all four redirect target classes). This binds the executable claim to the production handler chain without adding network I/O, sharing runtime helpers, or changing the standalone CodeQL module. + +The broader focused run then exposed four pre-existing Strix fixtures still patching the removed module-level `urlopen` symbol: HTTP error, URL error, malformed JSON, and success. Their RED result was `2 failed, 77 passed` because monkeypatch setup stopped before those cases reached production. They now patch `binding._GITHUB_API_OPENER.open`, matching the real call path; the three-file CodeQL/Strix/authority suite passes in both normal and `GITHUB_ACTIONS=true` modes (`87 passed` each), with 100% statement and branch coverage across the two affected production modules. + +A clean worktree at predecessor `25f83aaee9eb97e423f6ef2467e722035bc2e362` reproduced those two Strix failures in the full suite (`2 failed, 3354 passed, 28 skipped, 40 subtests`) and the repository-wide pre-existing 98% coverage gate (`262` missed statements). The repair removes the two causal suite failures and all misses in the two affected production modules; it does not claim to close unrelated coverage debt in `actions_queue_health*`, Rust materialization, Noema document handling, or scheduler code. + +## Alternatives rejected + +Broad Semgrep/Bandit suppression, path exclusion, or threshold weakening were rejected because they hide unrelated findings. Revalidating only the final response URL was rejected because the unauthorized network contact would already have occurred. Preserving redirects while stripping only `Authorization` was rejected because the client would still contact a target outside the stated GitHub REST authority. A custom redirect-following policy was unnecessary for these CI reads; blocking redirects entirely is the smaller authority surface. + +## Evidence and acceptance + +Primary scanner rule inspected at [semgrep/semgrep-rules revision `40b8c63f75dc7c22c8a77482d73bfb864b146f7e`](https://github.com/semgrep/semgrep-rules/commit/40b8c63f75dc7c22c8a77482d73bfb864b146f7e): `python/lang/security/audit/dynamic-urllib-use-detected.yaml`. Python stdlib `HTTPRedirectHandler` behavior was inspected during review because redirect construction is the second network-authority decision that the original source predicate did not control. + +Acceptance requires all of the following on the exact PR head: + +1. `tests/test_github_api_url_boundary.py` passes initial hostile-authority, direct-handler redirect-refusal, actual-production-opener synthetic-302, and canonical positive-control cases for both clients; +2. existing CodeQL GHAS identity and Strix evidence-binding suites remain green; +3. Semgrep and Python/Bandit no longer report the #2248 baseline findings and introduce no replacement Medium+ finding; +4. no security rule, path, threshold, or required check is weakened; +5. independent current-head review confirms redirects cannot create a second request carrying the bearer token; +6. the standalone `$RUNNER_TEMP` CodeQL materialization contract remains intact. + +Hosted exact-head evidence is mandatory. Source inspection, structural RED/repair lineage, and review comments are not substitutes for repository/security GREEN. diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index d2b52efcaa..c617e3ad73 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -100,6 +100,7 @@ flowchart LR | G-14 | release/changelog/version 증거가 각 PR에 분산되고 현재 central repo 보호 main의 release candidate가 명확하지 않다 | 운영자는 어떤 기능이 supportable release인지 확인할 수 없다 | merge 후 release readiness ledger, CHANGELOG, semantic version/tag, rollback/operability evidence를 함께 갱신한다 | | G-15 | 첨부파일 처리 경계가 제품별로 다르고, 1MB 상한은 업무 데이터와 맞지 않으며 미지원 MIME/컨테이너가 parser registry에서 명시적으로 pending/quarantine 되는지 확인되지 않았다. 현재 20MB 초과 파일 가능성과 PDF/HWP/HWPX·이미지·압축파일의 parse/sidecar 흐름을 하나의 exact contract로 묶지 못했다 | 큰 업무 첨부를 거부하거나 파싱 실패를 조용히 잃으면 고객의 메일·문서 업무가 중단된다 | naruon/newsdom-api 소유 PR에서 streaming upload, configurable bounded limit above 20MB, MIME sniffing, parser capability registry, quarantine/retry, source-position provenance, and ADR를 추가하고 size/unsupported-type/zip-bomb tests를 required evidence로 만든다 | | G-16 | Required Pingora policy treated a changed documentation PNG screenshot as UTF-8 runtime evidence | Valid UI evidence blocked otherwise valid product PRs before policy evaluation | This branch verifies bounded PNG magic before exemption while runtime paths and malformed assets continue to fail closed; protected-main delivery remains the release gate | +| G-17 | `.github#2279` blocked authenticated GitHub REST redirects in source, but redirect tests invoked `_RejectRedirects` directly and four Strix transport fixtures still patched the removed `urlopen` seam | A future opener-composition regression could forward a bearer token on a 3xx while redirect tests stayed green; Strix error mapping could fail before exercising production | Proposed `57477289ebec5631b0c48f0bc419f336dbe19deb` sends all four synthetic redirect classes through both real module-level openers; `663ffac390d27ab21daa58b91b624d3f00dce7de` moves every Strix fixture to the production opener; `9c19c6e00eafc028068719ab482282c1256f8893` adds malformed-authority coverage and records the owner evidence. Mutation RED proves the default opener contacts a second same-authority URL with the bearer header. The focused suite passes twice (`87 passed` normal and `GITHUB_ACTIONS=true`) with 100% statement/branch coverage on both affected modules. Exact-head hosted security and independent review remain required | ## 4. 열린 PR live inventory @@ -3411,3 +3412,15 @@ workflow instead of two, org-wide. `strix.yml` (the other single-consumer gate) alone -- it is a documented multi-PR hot-file collision zone. Contract: `tests/test_docs_only_pr_runner_admission.py::test_sast_semgrep_folds_the_gate_into_its_single_consumer_at_step_level`, `tests/test_required_security_runner_image_contract.py`. + +## 2026-09-19 GitHub API production-opener redirect proof + +**Status:** Proposed on `ContextualWisdomLab/.github#2279`; exact-head hosted checks and qualifying independent review remain mandatory. + +**Context Map / owner.** The central `.github` CI bounded context owns the bearer-authenticated CodeQL-analysis and Strix changed-file GitHub REST clients. GitHub remains the upstream REST authority. Product repositories consume only the released central workflow contract; they do not copy either client. + +**Gap.** Initial URL admission and direct `_RejectRedirects.redirect_request()` unit cases did not prove that each module-level production `OpenerDirector` actually retained the no-redirect handler chain. A future opener reconstruction could silently re-enable authenticated redirects while the prior tests stayed green. + +**Action.** Exact `57477289ebec5631b0c48f0bc419f336dbe19deb` adds a dependency-free synthetic-302 transport to `tests/test_github_api_url_boundary.py`. For both actual production openers, the case drives a canonical bearer request through the real HTTPS open/response chain, requires the typed HTTP-302 failure mapping, and proves transport receives exactly one original request; lookalike HTTPS, HTTP, `file:`, and same-authority redirect targets never receive a second request or bearer. Exact `e0b0b4d4fff5b6ea88236a1e91dcd7dbb3be09b5` repairs the doctoring claim so direct-handler coverage is not mislabeled as production-chain proof. + +**Evidence / remaining condition.** The standalone fixture mechanism was executed locally against Python stdlib and produced one canonical request followed by terminal HTTP 302 for every hostile target. This is mechanism evidence, not repository acceptance. Final authority requires focused/full exact-tree GREEN, fresh exact-head Security/SAST/Python Security/CodeQL/runtime-quality checks, no unresolved actionable review, ordinary protected-main integration, and downstream consumer validation. No scanner suppression, redirect allowlist widening, provider fallback, workflow gate weakening, or credential-boundary change is included. diff --git a/scripts/ci/codeql_ghas_configuration_identity.py b/scripts/ci/codeql_ghas_configuration_identity.py index 86e2997c8a..53e00c41c6 100644 --- a/scripts/ci/codeql_ghas_configuration_identity.py +++ b/scripts/ci/codeql_ghas_configuration_identity.py @@ -28,12 +28,32 @@ DEFAULT_SETUP_ANALYSIS_KEY = "dynamic/github-code-scanning/codeql:analyze" CODEQL_TOOL_NAME = "CodeQL" +GITHUB_API_AUTHORITY = "api.github.com" class ConfigurationIdentityError(RuntimeError): """Report a fail-closed GHAS configuration-identity contract failure.""" +class _RejectRedirects(urllib.request.HTTPRedirectHandler): + """Prevent authenticated GitHub REST requests from creating redirect requests.""" + + def redirect_request( + self, + _request: urllib.request.Request, + _file_pointer: Any, + _code: int, + _message: str, + _headers: Any, + _new_url: str, + ) -> None: + """Refuse every redirect so bearer headers never cross the reviewed authority.""" + return None + + +_GITHUB_API_OPENER = urllib.request.build_opener(_RejectRedirects()) + + def language_category(language: str) -> str: """Return the CodeQL category string GHAS uses for one language.""" normalized = str(language or "").strip().lower() @@ -142,8 +162,29 @@ def format_identity(identity: tuple[str, str]) -> str: return f"{analysis_key} {category}" +def _require_github_api_url(url: str) -> str: + """Reject any REST target outside canonical HTTPS ``api.github.com`` authority.""" + try: + parsed = urllib.parse.urlsplit(url) + except ValueError as exc: + raise ConfigurationIdentityError( + "GitHub API URL must use canonical https://api.github.com authority" + ) from exc + if ( + parsed.scheme != "https" + or parsed.netloc != GITHUB_API_AUTHORITY + or not parsed.path.startswith("/") + or parsed.fragment + ): + raise ConfigurationIdentityError( + "GitHub API URL must use canonical https://api.github.com authority" + ) + return url + + def _request_json(url: str, *, token: str, timeout_seconds: int) -> Any: - """GET one GitHub REST URL and decode JSON, or raise ConfigurationIdentityError.""" + """GET one canonical GitHub REST URL without redirects, or fail closed.""" + url = _require_github_api_url(url) request = urllib.request.Request( url, headers={ @@ -155,7 +196,7 @@ def _request_json(url: str, *, token: str, timeout_seconds: int) -> Any: method="GET", ) try: - with urllib.request.urlopen(request, timeout=timeout_seconds) as response: + with _GITHUB_API_OPENER.open(request, timeout=timeout_seconds) as response: payload = response.read().decode("utf-8") except urllib.error.HTTPError as exc: body = exc.read().decode("utf-8", errors="replace")[-400:] diff --git a/scripts/ci/strix_evidence_binding.py b/scripts/ci/strix_evidence_binding.py index eafe777476..7319040df2 100644 --- a/scripts/ci/strix_evidence_binding.py +++ b/scripts/ci/strix_evidence_binding.py @@ -27,7 +27,8 @@ from pathlib import Path from typing import Any from urllib.error import HTTPError, URLError -from urllib.request import Request, urlopen +from urllib.parse import urlsplit +from urllib.request import HTTPRedirectHandler, Request, build_opener FULL_SHA_RE = re.compile(r"^[0-9a-f]{40}$") @@ -46,6 +47,7 @@ SAFE_PATH_RE = re.compile(r"^(?!/)(?!.*(?:^|/)\.\.(?:/|$))[A-Za-z0-9_./ \[\]@+-]+$") MAX_CHANGED_FILES = 3_000 MAX_PAGES = 31 +GITHUB_API_AUTHORITY = "api.github.com" class EvidenceScope(str, Enum): @@ -72,6 +74,23 @@ class EvidenceBindingError(ValueError): """Raised when authenticated Strix evidence cannot be established.""" +class _RejectRedirects(HTTPRedirectHandler): + """Prevent authenticated GitHub REST requests from creating redirect requests.""" + + def redirect_request( + self, + _request: Request, + _file_pointer: Any, + _code: int, + _message: str, + _headers: Any, + _new_url: str, + ) -> None: + """Refuse every redirect so bearer headers never cross the reviewed authority.""" + return None + + +_GITHUB_API_OPENER = build_opener(_RejectRedirects()) OpenJson = Callable[[str, str], Any] @@ -245,11 +264,33 @@ def load_changed_paths_from_github( ) +def _require_github_api_url(url: str) -> str: + """Reject any REST target outside canonical HTTPS ``api.github.com`` authority.""" + + try: + parsed = urlsplit(url) + except ValueError as exc: + raise EvidenceBindingError( + "GitHub API URL must use canonical https://api.github.com authority" + ) from exc + if ( + parsed.scheme != "https" + or parsed.netloc != GITHUB_API_AUTHORITY + or not parsed.path.startswith("/") + or parsed.fragment + ): + raise EvidenceBindingError( + "GitHub API URL must use canonical https://api.github.com authority" + ) + return url + + def default_github_opener(url: str, token: str) -> Any: - """Fetch one GitHub API JSON document with a bounded Authorization header.""" + """Fetch one canonical GitHub API JSON document without redirects.""" if not token: raise EvidenceBindingError("GitHub token is required for changed-file evidence") + url = _require_github_api_url(url) request = Request( url, headers={ @@ -261,7 +302,7 @@ def default_github_opener(url: str, token: str) -> Any: method="GET", ) try: - with urlopen(request, timeout=30) as response: # noqa: S310 - GitHub HTTPS only + with _GITHUB_API_OPENER.open(request, timeout=30) as response: payload = response.read() except HTTPError as exc: raise EvidenceBindingError( diff --git a/tests/test_codeql_ghas_configuration_identity.py b/tests/test_codeql_ghas_configuration_identity.py index 23ca662ea7..817cd56497 100644 --- a/tests/test_codeql_ghas_configuration_identity.py +++ b/tests/test_codeql_ghas_configuration_identity.py @@ -406,13 +406,13 @@ def __enter__(self): def __exit__(self, exc_type, exc, tb) -> None: del exc_type, exc, tb - def fake_urlopen(request, timeout=30): + def fake_open(request, timeout=30): del timeout assert "tool_name=CodeQL" in request.full_url assert "ref=refs%2Fheads%2Fmain" in request.full_url return _Response() - monkeypatch.setattr(identity.urllib.request, "urlopen", fake_urlopen) + monkeypatch.setattr(identity._GITHUB_API_OPENER, "open", fake_open) rows = identity.list_codeql_analyses( "ContextualWisdomLab/wardnet", token="opaque", @@ -437,7 +437,7 @@ def raise_http(request, timeout=30): del request, timeout raise _HTTPError("https://api.github.com/x", 403, "forbidden", hdrs=None, fp=None) - monkeypatch.setattr(identity.urllib.request, "urlopen", raise_http) + monkeypatch.setattr(identity._GITHUB_API_OPENER, "open", raise_http) with pytest.raises(identity.ConfigurationIdentityError) as excinfo: identity._request_json("https://api.github.com/x", token="t", timeout_seconds=1) assert "HTTP 403" in str(excinfo.value) @@ -446,7 +446,7 @@ def raise_url(request, timeout=30): del request, timeout raise identity.urllib.error.URLError("down") - monkeypatch.setattr(identity.urllib.request, "urlopen", raise_url) + monkeypatch.setattr(identity._GITHUB_API_OPENER, "open", raise_url) with pytest.raises(identity.ConfigurationIdentityError): identity._request_json("https://api.github.com/x", token="t", timeout_seconds=1) @@ -465,8 +465,8 @@ def __exit__(self, exc_type, exc, tb) -> None: del exc_type, exc, tb monkeypatch.setattr( - identity.urllib.request, - "urlopen", + identity._GITHUB_API_OPENER, + "open", lambda request, timeout=30: _Empty(), ) assert identity._request_json("https://api.github.com/x", token="t", timeout_seconds=1) == [] @@ -482,8 +482,8 @@ def __exit__(self, exc_type, exc, tb) -> None: del exc_type, exc, tb monkeypatch.setattr( - identity.urllib.request, - "urlopen", + identity._GITHUB_API_OPENER, + "open", lambda request, timeout=30: _Bad(), ) with pytest.raises(identity.ConfigurationIdentityError): diff --git a/tests/test_github_api_url_boundary.py b/tests/test_github_api_url_boundary.py new file mode 100644 index 0000000000..a9050584fd --- /dev/null +++ b/tests/test_github_api_url_boundary.py @@ -0,0 +1,278 @@ +"""Fail-closed GitHub REST authority contracts for central CI HTTP clients.""" + +from __future__ import annotations + +from email.message import Message +from io import BytesIO +from pathlib import Path +import re +import subprocess +from typing import Any +from urllib.request import Request +from urllib.response import addinfourl + +import pytest + +from scripts.ci import codeql_ghas_configuration_identity as identity +from scripts.ci import strix_evidence_binding as binding + + +UNTRUSTED_GITHUB_API_URLS = ( + "http://api.github.com/repos/ContextualWisdomLab/example", + "https://api.github.com.evil.example/repos/ContextualWisdomLab/example", + "https://api.github.com@evil.example/repos/ContextualWisdomLab/example", + "https://api.github.com:443/repos/ContextualWisdomLab/example", + "https://api.github.com/repos/ContextualWisdomLab/example#fragment", + "https://[api.github.com/repos/ContextualWisdomLab/example", + "file:///etc/passwd", +) +REDIRECT_TARGETS = ( + "https://api.github.com/repos/ContextualWisdomLab/redirected", + "https://api.github.com.evil.example/repos/ContextualWisdomLab/example", + "http://api.github.com/repos/ContextualWisdomLab/example", + "file:///etc/passwd", +) +CANONICAL_GITHUB_API_URL = "https://api.github.com/repos/ContextualWisdomLab/example" +G17_ROW_PREFIX = "| G-17 |" +FULL_COMMIT_SHA = re.compile(r"`([0-9a-f]{40})`") + + +class _SyntheticRedirectTransport: + """Return one synthetic 302 while recording every request reaching transport.""" + + def __init__(self, target: str) -> None: + """Store the redirect target and initialize the observed request ledger.""" + self.target = target + self.calls: list[tuple[str, str | None]] = [] + + def https_open(self, request: Request) -> Any: + """Return a synthetic redirect response without contacting a network target.""" + self.calls.append((request.full_url, request.get_header("Authorization"))) + headers = Message() + headers["Location"] = self.target + response = addinfourl(BytesIO(b""), headers, request.full_url, code=302) + response.msg = "Found" + return response + + +class _JsonResponse: + """Minimal context-managed JSON response for opener-boundary contracts.""" + + def __enter__(self) -> _JsonResponse: + """Enter the fake response context.""" + return self + + def __exit__(self, *_args: Any) -> None: + """Leave the fake response context without suppressing exceptions.""" + return None + + def read(self) -> bytes: + """Return an empty JSON array payload.""" + return b"[]" + + +def _unexpected_open(*_args: Any, **_kwargs: Any) -> Any: + """Fail if a rejected authority reaches the network/file opener boundary.""" + pytest.fail("rejected GitHub API authority reached opener") + + +def _assert_g17_evidence_is_published(baseline: str) -> None: + """Require every full G-17 evidence SHA to resolve in current published ancestry.""" + rows = [line for line in baseline.splitlines() if line.startswith(G17_ROW_PREFIX)] + assert len(rows) == 1, "G-17 must have exactly one gap-register row" + evidence_shas = FULL_COMMIT_SHA.findall(rows[0]) + assert evidence_shas, "G-17 must name full commit evidence" + + repository_root = Path(__file__).resolve().parents[1] + for evidence_sha in evidence_shas: + resolvable = subprocess.run( + ["git", "cat-file", "-e", f"{evidence_sha}^{{commit}}"], + cwd=repository_root, + check=False, + capture_output=True, + text=True, + ) + assert resolvable.returncode == 0, f"G-17 evidence {evidence_sha} is not published" + + ancestor = subprocess.run( + ["git", "merge-base", "--is-ancestor", evidence_sha, "HEAD"], + cwd=repository_root, + check=False, + capture_output=True, + text=True, + ) + assert ancestor.returncode == 0, ( + f"G-17 evidence {evidence_sha} is not published in current HEAD ancestry" + ) + + +@pytest.mark.parametrize("url", UNTRUSTED_GITHUB_API_URLS) +def test_codeql_identity_client_rejects_noncanonical_github_api_authority( + monkeypatch: pytest.MonkeyPatch, url: str +) -> None: + """CodeQL GHAS reads must reject non-HTTPS or non-api.github.com authorities.""" + monkeypatch.setattr(identity._GITHUB_API_OPENER, "open", _unexpected_open) + + with pytest.raises(identity.ConfigurationIdentityError, match="GitHub API URL"): + identity._request_json(url, token="test-token", timeout_seconds=1) + + +@pytest.mark.parametrize("url", UNTRUSTED_GITHUB_API_URLS) +def test_strix_evidence_client_rejects_noncanonical_github_api_authority( + monkeypatch: pytest.MonkeyPatch, url: str +) -> None: + """Strix evidence reads must reject non-HTTPS or non-api.github.com authorities.""" + monkeypatch.setattr(binding._GITHUB_API_OPENER, "open", _unexpected_open) + + with pytest.raises(binding.EvidenceBindingError, match="GitHub API URL"): + binding.default_github_opener(url, "test-token") + + +@pytest.mark.parametrize("target", REDIRECT_TARGETS) +@pytest.mark.parametrize("client", ("codeql", "strix")) +def test_production_openers_reject_redirect_without_forwarding_bearer( + monkeypatch: pytest.MonkeyPatch, + target: str, + client: str, +) -> None: + """Drive a synthetic 302 through each actual opener and forbid a second request.""" + if client == "codeql": + opener = identity._GITHUB_API_OPENER + call = lambda: identity._request_json( + CANONICAL_GITHUB_API_URL, + token="test-token", + timeout_seconds=1, + ) + error_type = identity.ConfigurationIdentityError + else: + opener = binding._GITHUB_API_OPENER + call = lambda: binding.default_github_opener( + CANONICAL_GITHUB_API_URL, + "test-token", + ) + error_type = binding.EvidenceBindingError + + transport = _SyntheticRedirectTransport(target) + monkeypatch.setitem( + opener.handle_open, + "https", + [transport, *opener.handle_open["https"]], + ) + + with pytest.raises(error_type, match="HTTP 302"): + call() + + assert transport.calls == [ + (CANONICAL_GITHUB_API_URL, "Bearer test-token"), + ] + + +@pytest.mark.parametrize("target", REDIRECT_TARGETS) +def test_codeql_identity_client_never_constructs_redirect_request_with_bearer_token( + target: str, +) -> None: + """A GitHub response must not redirect CodeQL credentials to another URL.""" + request = Request( + CANONICAL_GITHUB_API_URL, + headers={"Authorization": "Bearer test-token"}, + ) + handler = identity._RejectRedirects() + + redirected = handler.redirect_request(request, None, 302, "Found", {}, target) + + assert redirected is None + assert request.get_header("Authorization") == "Bearer test-token" + + +@pytest.mark.parametrize("target", REDIRECT_TARGETS) +def test_strix_evidence_client_never_constructs_redirect_request_with_bearer_token( + target: str, +) -> None: + """A GitHub response must not redirect Strix credentials to another URL.""" + request = Request( + CANONICAL_GITHUB_API_URL, + headers={"Authorization": "Bearer test-token"}, + ) + handler = binding._RejectRedirects() + + redirected = handler.redirect_request(request, None, 302, "Found", {}, target) + + assert redirected is None + assert request.get_header("Authorization") == "Bearer test-token" + + +def test_canonical_github_api_authority_reaches_both_openers( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The exact HTTPS GitHub REST authority remains an allowed production control.""" + identity_calls: list[str] = [] + strix_calls: list[str] = [] + + def identity_open(request: Any, **_kwargs: Any) -> _JsonResponse: + """Record the CodeQL client's validated request URL.""" + identity_calls.append(request.full_url) + return _JsonResponse() + + def strix_open(request: Any, **_kwargs: Any) -> _JsonResponse: + """Record the Strix client's validated request URL.""" + strix_calls.append(request.full_url) + return _JsonResponse() + + monkeypatch.setattr(identity._GITHUB_API_OPENER, "open", identity_open) + monkeypatch.setattr(binding._GITHUB_API_OPENER, "open", strix_open) + + assert identity._request_json( + CANONICAL_GITHUB_API_URL, + token="test-token", + timeout_seconds=1, + ) == [] + assert binding.default_github_opener(CANONICAL_GITHUB_API_URL, "test-token") == [] + assert identity_calls == [CANONICAL_GITHUB_API_URL] + assert strix_calls == [CANONICAL_GITHUB_API_URL] + + +def test_documented_opener_lineage_references_published_commits() -> None: + """Owner evidence must name the published commits that carry each repair.""" + doctoring = Path( + "docs/doctoring/github-api-url-authority-2248.md" + ).read_text(encoding="utf-8") + baseline = Path("docs/product-technical-gap-baseline.md").read_text( + encoding="utf-8" + ) + evidence = doctoring + baseline + + assert "57477289ebec5631b0c48f0bc419f336dbe19deb" in doctoring + assert "663ffac390d27ab21daa58b91b624d3f00dce7de" in baseline + 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: + """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" + ) + mutated = baseline.replace( + "57477289ebec5631b0c48f0bc419f336dbe19deb", + "0000000000000000000000000000000000000000", + 1, + ) + + with pytest.raises(AssertionError, match="not published"): + _assert_g17_evidence_is_published(mutated) + + +def test_doctoring_qualifies_foreign_semgrep_revision_owner() -> None: + """Foreign evidence must identify its repository instead of resembling a local SHA.""" + doctoring = Path( + "docs/doctoring/github-api-url-authority-2248.md" + ).read_text(encoding="utf-8") + revision = "40b8c63f75dc7c22c8a77482d73bfb864b146f7e" + expected_link = ( + f"[semgrep/semgrep-rules revision `{revision}`]" + f"(https://github.com/semgrep/semgrep-rules/commit/{revision})" + ) + + assert expected_link in doctoring diff --git a/tests/test_strix_evidence_binding.py b/tests/test_strix_evidence_binding.py index 60d3ceb517..90a5454ecb 100644 --- a/tests/test_strix_evidence_binding.py +++ b/tests/test_strix_evidence_binding.py @@ -658,14 +658,14 @@ def raise_http(*_args: object, **_kwargs: object) -> object: fp=BytesIO(), ) - monkeypatch.setattr(binding, "urlopen", raise_http) + monkeypatch.setattr(binding._GITHUB_API_OPENER, "open", raise_http) with pytest.raises(binding.EvidenceBindingError, match="HTTP 403"): binding.default_github_opener("https://api.github.com/x", "token") def raise_url(*_args: object, **_kwargs: object) -> object: raise binding.URLError("down") - monkeypatch.setattr(binding, "urlopen", raise_url) + monkeypatch.setattr(binding._GITHUB_API_OPENER, "open", raise_url) with pytest.raises(binding.EvidenceBindingError, match="URLError"): binding.default_github_opener("https://api.github.com/x", "token") @@ -687,7 +687,7 @@ def __exit__(self, *_args: object) -> None: return None - monkeypatch.setattr(binding, "urlopen", lambda *_a, **_k: Response()) + monkeypatch.setattr(binding._GITHUB_API_OPENER, "open", lambda *_a, **_k: Response()) with pytest.raises(binding.EvidenceBindingError, match="not JSON"): binding.default_github_opener("https://api.github.com/x", "token") @@ -713,7 +713,7 @@ def __exit__(self, *_args: object) -> None: return None - monkeypatch.setattr(binding, "urlopen", lambda *_a, **_k: Response()) + monkeypatch.setattr(binding._GITHUB_API_OPENER, "open", lambda *_a, **_k: Response()) rows = binding.load_changed_paths_from_github( "https://api.github.com", "ContextualWisdomLab/example",