From a83d6d325c28620bb393d7b79a3cc74fe8130c38 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 18 Sep 2026 18:39:45 +0900 Subject: [PATCH 01/20] fix(security): allowlist https://api.github.com before urllib urlopen Semgrep OSS and Bandit B310 Medium alerts on main flagged dynamic urllib use in CodeQL identity and Strix evidence helpers. Fail closed unless the URL is https://api.github.com so file:// and arbitrary hosts cannot reach urlopen. Co-authored-by: Cursor --- scripts/ci/codeql_ghas_configuration_identity.py | 13 ++++++++++++- scripts/ci/strix_evidence_binding.py | 14 +++++++++++++- tests/test_codeql_ghas_configuration_identity.py | 12 ++++++++++++ tests/test_strix_evidence_binding.py | 10 ++++++++++ 4 files changed, 47 insertions(+), 2 deletions(-) diff --git a/scripts/ci/codeql_ghas_configuration_identity.py b/scripts/ci/codeql_ghas_configuration_identity.py index 86e2997c8a..1594c2fe3a 100644 --- a/scripts/ci/codeql_ghas_configuration_identity.py +++ b/scripts/ci/codeql_ghas_configuration_identity.py @@ -142,8 +142,19 @@ def format_identity(identity: tuple[str, str]) -> str: return f"{analysis_key} {category}" + +def _assert_github_https_api_url(url: str) -> None: + """Reject non-HTTPS / non-api.github.com URLs before urllib (Semgrep/Bandit B310).""" + parsed = urllib.parse.urlparse(url) + if parsed.scheme != "https" or (parsed.hostname or "").lower() != "api.github.com": + raise ConfigurationIdentityError( + "refusing urllib GET: only https://api.github.com URLs are allowed" + ) + + def _request_json(url: str, *, token: str, timeout_seconds: int) -> Any: """GET one GitHub REST URL and decode JSON, or raise ConfigurationIdentityError.""" + _assert_github_https_api_url(url) request = urllib.request.Request( url, headers={ @@ -155,7 +166,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 urllib.request.urlopen(request, timeout=timeout_seconds) as response: # noqa: S310 - https api.github.com only 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..2d5001c64a 100644 --- a/scripts/ci/strix_evidence_binding.py +++ b/scripts/ci/strix_evidence_binding.py @@ -27,6 +27,7 @@ from pathlib import Path from typing import Any from urllib.error import HTTPError, URLError +from urllib.parse import urlparse from urllib.request import Request, urlopen @@ -245,11 +246,22 @@ def load_changed_paths_from_github( ) + +def _assert_github_https_api_url(url: str) -> None: + """Reject non-HTTPS / non-api.github.com URLs before urlopen (Semgrep/Bandit B310).""" + parsed = urlparse(url) + if parsed.scheme != "https" or (parsed.hostname or "").lower() != "api.github.com": + raise EvidenceBindingError( + "refusing urllib GET: only https://api.github.com URLs are allowed" + ) + + def default_github_opener(url: str, token: str) -> Any: """Fetch one GitHub API JSON document with a bounded Authorization header.""" if not token: raise EvidenceBindingError("GitHub token is required for changed-file evidence") + _assert_github_https_api_url(url) request = Request( url, headers={ @@ -261,7 +273,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 urlopen(request, timeout=30) as response: # noqa: S310 - https api.github.com only 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..202e6a3f88 100644 --- a/tests/test_codeql_ghas_configuration_identity.py +++ b/tests/test_codeql_ghas_configuration_identity.py @@ -495,3 +495,15 @@ def test_list_codeql_analyses_rejects_non_list_payload(monkeypatch): monkeypatch.setattr(identity, "_request_json", lambda url, token, timeout_seconds: {"ok": True}) with pytest.raises(identity.ConfigurationIdentityError): identity.list_codeql_analyses("ContextualWisdomLab/wardnet", token="opaque") + + +def test_request_json_rejects_non_github_https_urls(monkeypatch): + """urllib allowlist must fail closed before urlopen (Semgrep/Bandit Medium).""" + import scripts.ci.codeql_ghas_configuration_identity as mod + calls = [] + monkeypatch.setattr(mod.urllib.request, "urlopen", lambda *a, **k: calls.append((a, k))) + with pytest.raises(mod.ConfigurationIdentityError, match="api.github.com"): + mod._request_json("http://evil.example/x", token="t", timeout_seconds=1) + with pytest.raises(mod.ConfigurationIdentityError, match="api.github.com"): + mod._request_json("https://evil.example/x", token="t", timeout_seconds=1) + assert calls == [] diff --git a/tests/test_strix_evidence_binding.py b/tests/test_strix_evidence_binding.py index 60d3ceb517..093b3f8ce6 100644 --- a/tests/test_strix_evidence_binding.py +++ b/tests/test_strix_evidence_binding.py @@ -969,3 +969,13 @@ def test_workspace_missing_root_returns_false(tmp_path: Path) -> None: missing = tmp_path / "missing-root" assert binding.workspace_contains_expected_diff(missing, "a.py", "body") is False + + +def test_assert_github_https_api_url_allowlist(): + """Only https://api.github.com may reach urlopen in evidence binding.""" + import scripts.ci.strix_evidence_binding as mod + mod._assert_github_https_api_url("https://api.github.com/repos/o/r") + with pytest.raises(mod.EvidenceBindingError, match="api.github.com"): + mod._assert_github_https_api_url("file:///etc/passwd") + with pytest.raises(mod.EvidenceBindingError, match="api.github.com"): + mod._assert_github_https_api_url("https://example.com/x") From 225260a8f949da525da5ff2190e3b413f41f88c0 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 19 Sep 2026 05:55:20 +0900 Subject: [PATCH 02/20] test(security): pin GitHub API redirect credential boundary --- ...ql_ghas_configuration_redirect_contract.py | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 tests/test_codeql_ghas_configuration_redirect_contract.py diff --git a/tests/test_codeql_ghas_configuration_redirect_contract.py b/tests/test_codeql_ghas_configuration_redirect_contract.py new file mode 100644 index 0000000000..462da45619 --- /dev/null +++ b/tests/test_codeql_ghas_configuration_redirect_contract.py @@ -0,0 +1,60 @@ +"""Credential-egress contract for GHAS configuration-identity HTTP redirects.""" + +from __future__ import annotations + +from email.message import Message +import urllib.request + +import pytest + +from scripts.ci import codeql_ghas_configuration_identity as identity + + +def _redirect_headers(location: str) -> Message: + """Build the header shape urllib passes to ``redirect_request``.""" + headers = Message() + headers["Location"] = location + return headers + + +def test_github_api_redirect_handler_rejects_external_origin_before_forwarding_bearer(): + """An admitted GitHub API request must not redirect its bearer token off-origin.""" + request = urllib.request.Request( + "https://api.github.com/repos/ContextualWisdomLab/.github/code-scanning/analyses", + headers={"Authorization": "Bearer sentinel-secret"}, + method="GET", + ) + handler = identity._GitHubApiRedirectHandler() + + with pytest.raises(identity.ConfigurationIdentityError, match="api.github.com"): + handler.redirect_request( + request, + None, + 302, + "Found", + _redirect_headers("https://evil.example/capture"), + "https://evil.example/capture", + ) + + +def test_github_api_redirect_handler_preserves_same_origin_redirects(): + """Legitimate GitHub API redirects remain usable without weakening the origin boundary.""" + request = urllib.request.Request( + "https://api.github.com/repos/ContextualWisdomLab/.github/code-scanning/analyses", + headers={"Authorization": "Bearer sentinel-secret"}, + method="GET", + ) + handler = identity._GitHubApiRedirectHandler() + + redirected = handler.redirect_request( + request, + None, + 302, + "Found", + _redirect_headers("https://api.github.com/repositories/123/code-scanning/analyses"), + "https://api.github.com/repositories/123/code-scanning/analyses", + ) + + assert redirected is not None + assert redirected.full_url == "https://api.github.com/repositories/123/code-scanning/analyses" + assert redirected.get_header("Authorization") == "Bearer sentinel-secret" From 0ae2204ebcff0441ec5e7ca41ffdd01bdc135a26 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 19 Sep 2026 05:57:57 +0900 Subject: [PATCH 03/20] fix(security): contain GitHub API redirects to admitted origin --- .../ci/codeql_ghas_configuration_identity.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/scripts/ci/codeql_ghas_configuration_identity.py b/scripts/ci/codeql_ghas_configuration_identity.py index 1594c2fe3a..e78e9c1865 100644 --- a/scripts/ci/codeql_ghas_configuration_identity.py +++ b/scripts/ci/codeql_ghas_configuration_identity.py @@ -127,8 +127,6 @@ def pairing_ready( category = language_category(language) base_for_language = {item for item in base_ids if item[1] == category} if not base_for_language: - # No base configuration for this language means GHAS will not demand one - # on the head for introduced-alert computation of that language. return True, [] missing = missing_base_identities(base_for_language, head_ids, language=language) return not missing, missing @@ -142,7 +140,6 @@ def format_identity(identity: tuple[str, str]) -> str: return f"{analysis_key} {category}" - def _assert_github_https_api_url(url: str) -> None: """Reject non-HTTPS / non-api.github.com URLs before urllib (Semgrep/Bandit B310).""" parsed = urllib.parse.urlparse(url) @@ -152,6 +149,18 @@ def _assert_github_https_api_url(url: str) -> None: ) +class _GitHubApiRedirectHandler(urllib.request.HTTPRedirectHandler): + """Allow redirects only while the request remains on the GitHub REST origin.""" + + def redirect_request(self, req, fp, code, msg, headers, newurl): + target = urllib.parse.urljoin(req.full_url, newurl) + _assert_github_https_api_url(target) + return super().redirect_request(req, fp, code, msg, headers, target) + + +_GITHUB_API_OPENER = urllib.request.build_opener(_GitHubApiRedirectHandler()) + + def _request_json(url: str, *, token: str, timeout_seconds: int) -> Any: """GET one GitHub REST URL and decode JSON, or raise ConfigurationIdentityError.""" _assert_github_https_api_url(url) @@ -166,7 +175,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: # noqa: S310 - https api.github.com only + 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:] @@ -316,4 +325,4 @@ def main(argv: Sequence[str] | None = None) -> int: if __name__ == "__main__": # pragma: no cover - exercised through ``main`` tests - raise SystemExit(main()) + raise SystemExit(main()) \ No newline at end of file From 062663af1f566b4118406e63074750adf950871b Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 19 Sep 2026 07:01:11 +0900 Subject: [PATCH 04/20] test(security): pin Strix redirect credential boundary --- ...trix_evidence_binding_redirect_contract.py | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 tests/test_strix_evidence_binding_redirect_contract.py diff --git a/tests/test_strix_evidence_binding_redirect_contract.py b/tests/test_strix_evidence_binding_redirect_contract.py new file mode 100644 index 0000000000..21a4ddb6df --- /dev/null +++ b/tests/test_strix_evidence_binding_redirect_contract.py @@ -0,0 +1,52 @@ +"""Fail-closed redirect contract for authenticated Strix GitHub API reads.""" + +from __future__ import annotations + +from urllib.request import Request + +import pytest + +from scripts.ci import strix_evidence_binding as binding + + +def _authenticated_request() -> Request: + """Build one admitted GitHub REST request carrying a bearer credential.""" + + return Request( + "https://api.github.com/repos/ContextualWisdomLab/example/pulls/1/files", + headers={"Authorization": "Bearer secret"}, + method="GET", + ) + + +def test_authenticated_redirect_rejects_cross_origin_before_bearer_forwarding() -> None: + """A 30x target outside api.github.com must fail before Request creation.""" + + handler = binding._GitHubApiRedirectHandler() + with pytest.raises(binding.EvidenceBindingError, match="only https://api.github.com"): + handler.redirect_request( + _authenticated_request(), + None, + 302, + "Found", + {}, + "https://evil.example/collect", + ) + + +def test_authenticated_redirect_preserves_same_origin_request() -> None: + """An admitted same-origin redirect keeps the authenticated GitHub request.""" + + handler = binding._GitHubApiRedirectHandler() + redirected = handler.redirect_request( + _authenticated_request(), + None, + 302, + "Found", + {}, + "/repositories/1/pulls/1/files?page=2", + ) + + assert redirected is not None + assert redirected.full_url == "https://api.github.com/repositories/1/pulls/1/files?page=2" + assert redirected.get_header("Authorization") == "Bearer secret" From 2708a6beb69a6cfdb4bdb2ec83eb5383d62d9be4 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 19 Sep 2026 07:02:05 +0900 Subject: [PATCH 05/20] fix(security): contain Strix GitHub API redirects --- scripts/ci/strix_evidence_binding.py | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/scripts/ci/strix_evidence_binding.py b/scripts/ci/strix_evidence_binding.py index 2d5001c64a..30d71ba8a3 100644 --- a/scripts/ci/strix_evidence_binding.py +++ b/scripts/ci/strix_evidence_binding.py @@ -27,8 +27,8 @@ from pathlib import Path from typing import Any from urllib.error import HTTPError, URLError -from urllib.parse import urlparse -from urllib.request import Request, urlopen +from urllib.parse import urljoin, urlparse +from urllib.request import HTTPRedirectHandler, Request, build_opener FULL_SHA_RE = re.compile(r"^[0-9a-f]{40}$") @@ -246,7 +246,6 @@ def load_changed_paths_from_github( ) - def _assert_github_https_api_url(url: str) -> None: """Reject non-HTTPS / non-api.github.com URLs before urlopen (Semgrep/Bandit B310).""" parsed = urlparse(url) @@ -256,6 +255,20 @@ def _assert_github_https_api_url(url: str) -> None: ) +class _GitHubApiRedirectHandler(HTTPRedirectHandler): + """Allow redirects only while an authenticated request remains on GitHub REST.""" + + def redirect_request(self, req, fp, code, msg, headers, newurl): + """Revalidate the target before urllib can copy the Authorization header.""" + + target = urljoin(req.full_url, newurl) + _assert_github_https_api_url(target) + return super().redirect_request(req, fp, code, msg, headers, target) + + +_GITHUB_API_OPENER = build_opener(_GitHubApiRedirectHandler()) + + def default_github_opener(url: str, token: str) -> Any: """Fetch one GitHub API JSON document with a bounded Authorization header.""" @@ -273,7 +286,9 @@ def default_github_opener(url: str, token: str) -> Any: method="GET", ) try: - with urlopen(request, timeout=30) as response: # noqa: S310 - https api.github.com only + with _GITHUB_API_OPENER.open( + request, timeout=30 + ) as response: # noqa: S310 - HTTPS api.github.com only, redirects revalidated payload = response.read() except HTTPError as exc: raise EvidenceBindingError( From 3758b890e012548420da6c3978d3e116ce8b814a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 18 Sep 2026 22:32:08 +0000 Subject: [PATCH 06/20] chore(deps): bump anyio from 4.14.0 to 4.14.2 Bumps [anyio](https://github.com/agronholm/anyio) from 4.14.0 to 4.14.2. - [Release notes](https://github.com/agronholm/anyio/releases) - [Commits](https://github.com/agronholm/anyio/compare/4.14.0...4.14.2) --- updated-dependencies: - dependency-name: anyio dependency-version: 4.14.2 dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- requirements-strix-ci-hashes.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/requirements-strix-ci-hashes.txt b/requirements-strix-ci-hashes.txt index 9e705850b5..eb83beda17 100644 --- a/requirements-strix-ci-hashes.txt +++ b/requirements-strix-ci-hashes.txt @@ -140,9 +140,9 @@ annotated-types==0.7.0 \ --hash=sha256:1f02e8b43a8fbbc3f3e0d4f0f4bfc8131bcb4eebe8849b8e5c773f3a1c582a53 \ --hash=sha256:aff07c09a53a08bc8cfccb9c85b05f1aa9a2a6f23728d790723543408344ce89 # via pydantic -anyio==4.14.0 \ - --hash=sha256:b47c1f9ccf73e67021df785332508f99379c68fa7d0684e8e3492cb1d4b23f89 \ - --hash=sha256:dd9b7a2a9799ed6552fde617b2c5df02b7fdd7d88392fc48101e51bae46164d9 +anyio==4.14.2 \ + --hash=sha256:9f505dda5ac9f0c8309b5e8bd445a8c2bf7246f3ce950121e45ea15bc41d1494 \ + --hash=sha256:cfa139f3ed1a23ee8f88a145ddb5ac7605b8bbfd8592baacd7ce3d8bb4313c7f # via # google-genai # gql From 4dcd25c9f2789e4b8acbeef603e118dd80bfa014 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 19 Sep 2026 10:59:36 +0900 Subject: [PATCH 07/20] test(security): align Strix transport seam with dedicated opener --- tests/conftest.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index 6f0c91d00f..c87bf8ba46 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,6 +7,7 @@ import pytest from scripts.ci import materialize_base_python_requirements as materializer +from scripts.ci import strix_evidence_binding as strix_binding @pytest.fixture(autouse=True) @@ -21,6 +22,26 @@ def clear_trusted_uv_process_caches() -> Iterator[None]: opener_cache_clear() +@pytest.fixture(autouse=True) +def preserve_strix_transport_test_seam( + request: pytest.FixtureRequest, + monkeypatch: pytest.MonkeyPatch, +) -> Iterator[None]: + """Route legacy Strix transport fakes through the production dedicated opener seam.""" + if request.node.path.name != "test_strix_evidence_binding.py": + yield + return + + original_open = strix_binding._GITHUB_API_OPENER.open + monkeypatch.setattr(strix_binding, "urlopen", original_open, raising=False) + monkeypatch.setattr( + strix_binding._GITHUB_API_OPENER, + "open", + lambda *args, **kwargs: strix_binding.urlopen(*args, **kwargs), + ) + yield + + class FakeHttpResponse: """Expose bounded context-managed reads from one deterministic final URL.""" From 834d285f90241b4741247408001fd7534ce5a3b0 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 19 Sep 2026 19:13:25 +0900 Subject: [PATCH 08/20] test(security): bind authenticated openers at owned seams Replace retired urllib urlopen monkeypatches with direct CodeQL and Strix dedicated-opener patches. Remove the PR-specific global conftest bridge so both security helpers exercise the same explicit transport boundary without live network access. --- tests/conftest.py | 21 ------------------- ...test_codeql_ghas_configuration_identity.py | 16 +++++++------- tests/test_strix_evidence_binding.py | 8 +++---- 3 files changed, 12 insertions(+), 33 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index c87bf8ba46..6f0c91d00f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,7 +7,6 @@ import pytest from scripts.ci import materialize_base_python_requirements as materializer -from scripts.ci import strix_evidence_binding as strix_binding @pytest.fixture(autouse=True) @@ -22,26 +21,6 @@ def clear_trusted_uv_process_caches() -> Iterator[None]: opener_cache_clear() -@pytest.fixture(autouse=True) -def preserve_strix_transport_test_seam( - request: pytest.FixtureRequest, - monkeypatch: pytest.MonkeyPatch, -) -> Iterator[None]: - """Route legacy Strix transport fakes through the production dedicated opener seam.""" - if request.node.path.name != "test_strix_evidence_binding.py": - yield - return - - original_open = strix_binding._GITHUB_API_OPENER.open - monkeypatch.setattr(strix_binding, "urlopen", original_open, raising=False) - monkeypatch.setattr( - strix_binding._GITHUB_API_OPENER, - "open", - lambda *args, **kwargs: strix_binding.urlopen(*args, **kwargs), - ) - yield - - class FakeHttpResponse: """Expose bounded context-managed reads from one deterministic final URL.""" diff --git a/tests/test_codeql_ghas_configuration_identity.py b/tests/test_codeql_ghas_configuration_identity.py index 202e6a3f88..414f654f79 100644 --- a/tests/test_codeql_ghas_configuration_identity.py +++ b/tests/test_codeql_ghas_configuration_identity.py @@ -412,7 +412,7 @@ def fake_urlopen(request, timeout=30): 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_urlopen) 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): @@ -501,7 +501,7 @@ def test_request_json_rejects_non_github_https_urls(monkeypatch): """urllib allowlist must fail closed before urlopen (Semgrep/Bandit Medium).""" import scripts.ci.codeql_ghas_configuration_identity as mod calls = [] - monkeypatch.setattr(mod.urllib.request, "urlopen", lambda *a, **k: calls.append((a, k))) + monkeypatch.setattr(mod._GITHUB_API_OPENER, "open", lambda *a, **k: calls.append((a, k))) with pytest.raises(mod.ConfigurationIdentityError, match="api.github.com"): mod._request_json("http://evil.example/x", token="t", timeout_seconds=1) with pytest.raises(mod.ConfigurationIdentityError, match="api.github.com"): diff --git a/tests/test_strix_evidence_binding.py b/tests/test_strix_evidence_binding.py index 093b3f8ce6..7eb29c9375 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", From 545648ee56dec2397b88b87a04622d2dc3eae96f Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 19 Sep 2026 20:42:54 +0900 Subject: [PATCH 09/20] chore(deps): isolate AnyIO security owner delta Restore the unrelated #2269 URL-opener paths to protected main while retaining the AnyIO 4.14.2 pin and hashes. The URL/redirect responsibility remains in canonical #2279; this PR owns only the dependency security update. Validated with 56 focused tests, 3,335 full tests plus 28 skipped/40 subtests, warnings-as-errors, diff check, and pip-audit reporting no known vulnerabilities. --- .../ci/codeql_ghas_configuration_identity.py | 28 ++------- scripts/ci/strix_evidence_binding.py | 31 +--------- ...test_codeql_ghas_configuration_identity.py | 26 +++----- ...ql_ghas_configuration_redirect_contract.py | 60 ------------------- tests/test_strix_evidence_binding.py | 18 ++---- ...trix_evidence_binding_redirect_contract.py | 52 ---------------- 6 files changed, 17 insertions(+), 198 deletions(-) delete mode 100644 tests/test_codeql_ghas_configuration_redirect_contract.py delete mode 100644 tests/test_strix_evidence_binding_redirect_contract.py diff --git a/scripts/ci/codeql_ghas_configuration_identity.py b/scripts/ci/codeql_ghas_configuration_identity.py index e78e9c1865..86e2997c8a 100644 --- a/scripts/ci/codeql_ghas_configuration_identity.py +++ b/scripts/ci/codeql_ghas_configuration_identity.py @@ -127,6 +127,8 @@ def pairing_ready( category = language_category(language) base_for_language = {item for item in base_ids if item[1] == category} if not base_for_language: + # No base configuration for this language means GHAS will not demand one + # on the head for introduced-alert computation of that language. return True, [] missing = missing_base_identities(base_for_language, head_ids, language=language) return not missing, missing @@ -140,30 +142,8 @@ def format_identity(identity: tuple[str, str]) -> str: return f"{analysis_key} {category}" -def _assert_github_https_api_url(url: str) -> None: - """Reject non-HTTPS / non-api.github.com URLs before urllib (Semgrep/Bandit B310).""" - parsed = urllib.parse.urlparse(url) - if parsed.scheme != "https" or (parsed.hostname or "").lower() != "api.github.com": - raise ConfigurationIdentityError( - "refusing urllib GET: only https://api.github.com URLs are allowed" - ) - - -class _GitHubApiRedirectHandler(urllib.request.HTTPRedirectHandler): - """Allow redirects only while the request remains on the GitHub REST origin.""" - - def redirect_request(self, req, fp, code, msg, headers, newurl): - target = urllib.parse.urljoin(req.full_url, newurl) - _assert_github_https_api_url(target) - return super().redirect_request(req, fp, code, msg, headers, target) - - -_GITHUB_API_OPENER = urllib.request.build_opener(_GitHubApiRedirectHandler()) - - def _request_json(url: str, *, token: str, timeout_seconds: int) -> Any: """GET one GitHub REST URL and decode JSON, or raise ConfigurationIdentityError.""" - _assert_github_https_api_url(url) request = urllib.request.Request( url, headers={ @@ -175,7 +155,7 @@ def _request_json(url: str, *, token: str, timeout_seconds: int) -> Any: method="GET", ) try: - with _GITHUB_API_OPENER.open(request, timeout=timeout_seconds) as response: + with urllib.request.urlopen(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:] @@ -325,4 +305,4 @@ def main(argv: Sequence[str] | None = None) -> int: if __name__ == "__main__": # pragma: no cover - exercised through ``main`` tests - raise SystemExit(main()) \ No newline at end of file + raise SystemExit(main()) diff --git a/scripts/ci/strix_evidence_binding.py b/scripts/ci/strix_evidence_binding.py index 30d71ba8a3..eafe777476 100644 --- a/scripts/ci/strix_evidence_binding.py +++ b/scripts/ci/strix_evidence_binding.py @@ -27,8 +27,7 @@ from pathlib import Path from typing import Any from urllib.error import HTTPError, URLError -from urllib.parse import urljoin, urlparse -from urllib.request import HTTPRedirectHandler, Request, build_opener +from urllib.request import Request, urlopen FULL_SHA_RE = re.compile(r"^[0-9a-f]{40}$") @@ -246,35 +245,11 @@ def load_changed_paths_from_github( ) -def _assert_github_https_api_url(url: str) -> None: - """Reject non-HTTPS / non-api.github.com URLs before urlopen (Semgrep/Bandit B310).""" - parsed = urlparse(url) - if parsed.scheme != "https" or (parsed.hostname or "").lower() != "api.github.com": - raise EvidenceBindingError( - "refusing urllib GET: only https://api.github.com URLs are allowed" - ) - - -class _GitHubApiRedirectHandler(HTTPRedirectHandler): - """Allow redirects only while an authenticated request remains on GitHub REST.""" - - def redirect_request(self, req, fp, code, msg, headers, newurl): - """Revalidate the target before urllib can copy the Authorization header.""" - - target = urljoin(req.full_url, newurl) - _assert_github_https_api_url(target) - return super().redirect_request(req, fp, code, msg, headers, target) - - -_GITHUB_API_OPENER = build_opener(_GitHubApiRedirectHandler()) - - def default_github_opener(url: str, token: str) -> Any: """Fetch one GitHub API JSON document with a bounded Authorization header.""" if not token: raise EvidenceBindingError("GitHub token is required for changed-file evidence") - _assert_github_https_api_url(url) request = Request( url, headers={ @@ -286,9 +261,7 @@ def default_github_opener(url: str, token: str) -> Any: method="GET", ) try: - with _GITHUB_API_OPENER.open( - request, timeout=30 - ) as response: # noqa: S310 - HTTPS api.github.com only, redirects revalidated + with urlopen(request, timeout=30) as response: # noqa: S310 - GitHub HTTPS only 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 414f654f79..23ca662ea7 100644 --- a/tests/test_codeql_ghas_configuration_identity.py +++ b/tests/test_codeql_ghas_configuration_identity.py @@ -412,7 +412,7 @@ def fake_urlopen(request, timeout=30): assert "ref=refs%2Fheads%2Fmain" in request.full_url return _Response() - monkeypatch.setattr(identity._GITHUB_API_OPENER, "open", fake_urlopen) + monkeypatch.setattr(identity.urllib.request, "urlopen", fake_urlopen) 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._GITHUB_API_OPENER, "open", raise_http) + monkeypatch.setattr(identity.urllib.request, "urlopen", 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._GITHUB_API_OPENER, "open", raise_url) + monkeypatch.setattr(identity.urllib.request, "urlopen", 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._GITHUB_API_OPENER, - "open", + identity.urllib.request, + "urlopen", 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._GITHUB_API_OPENER, - "open", + identity.urllib.request, + "urlopen", lambda request, timeout=30: _Bad(), ) with pytest.raises(identity.ConfigurationIdentityError): @@ -495,15 +495,3 @@ def test_list_codeql_analyses_rejects_non_list_payload(monkeypatch): monkeypatch.setattr(identity, "_request_json", lambda url, token, timeout_seconds: {"ok": True}) with pytest.raises(identity.ConfigurationIdentityError): identity.list_codeql_analyses("ContextualWisdomLab/wardnet", token="opaque") - - -def test_request_json_rejects_non_github_https_urls(monkeypatch): - """urllib allowlist must fail closed before urlopen (Semgrep/Bandit Medium).""" - import scripts.ci.codeql_ghas_configuration_identity as mod - calls = [] - monkeypatch.setattr(mod._GITHUB_API_OPENER, "open", lambda *a, **k: calls.append((a, k))) - with pytest.raises(mod.ConfigurationIdentityError, match="api.github.com"): - mod._request_json("http://evil.example/x", token="t", timeout_seconds=1) - with pytest.raises(mod.ConfigurationIdentityError, match="api.github.com"): - mod._request_json("https://evil.example/x", token="t", timeout_seconds=1) - assert calls == [] diff --git a/tests/test_codeql_ghas_configuration_redirect_contract.py b/tests/test_codeql_ghas_configuration_redirect_contract.py deleted file mode 100644 index 462da45619..0000000000 --- a/tests/test_codeql_ghas_configuration_redirect_contract.py +++ /dev/null @@ -1,60 +0,0 @@ -"""Credential-egress contract for GHAS configuration-identity HTTP redirects.""" - -from __future__ import annotations - -from email.message import Message -import urllib.request - -import pytest - -from scripts.ci import codeql_ghas_configuration_identity as identity - - -def _redirect_headers(location: str) -> Message: - """Build the header shape urllib passes to ``redirect_request``.""" - headers = Message() - headers["Location"] = location - return headers - - -def test_github_api_redirect_handler_rejects_external_origin_before_forwarding_bearer(): - """An admitted GitHub API request must not redirect its bearer token off-origin.""" - request = urllib.request.Request( - "https://api.github.com/repos/ContextualWisdomLab/.github/code-scanning/analyses", - headers={"Authorization": "Bearer sentinel-secret"}, - method="GET", - ) - handler = identity._GitHubApiRedirectHandler() - - with pytest.raises(identity.ConfigurationIdentityError, match="api.github.com"): - handler.redirect_request( - request, - None, - 302, - "Found", - _redirect_headers("https://evil.example/capture"), - "https://evil.example/capture", - ) - - -def test_github_api_redirect_handler_preserves_same_origin_redirects(): - """Legitimate GitHub API redirects remain usable without weakening the origin boundary.""" - request = urllib.request.Request( - "https://api.github.com/repos/ContextualWisdomLab/.github/code-scanning/analyses", - headers={"Authorization": "Bearer sentinel-secret"}, - method="GET", - ) - handler = identity._GitHubApiRedirectHandler() - - redirected = handler.redirect_request( - request, - None, - 302, - "Found", - _redirect_headers("https://api.github.com/repositories/123/code-scanning/analyses"), - "https://api.github.com/repositories/123/code-scanning/analyses", - ) - - assert redirected is not None - assert redirected.full_url == "https://api.github.com/repositories/123/code-scanning/analyses" - assert redirected.get_header("Authorization") == "Bearer sentinel-secret" diff --git a/tests/test_strix_evidence_binding.py b/tests/test_strix_evidence_binding.py index 7eb29c9375..60d3ceb517 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._GITHUB_API_OPENER, "open", raise_http) + monkeypatch.setattr(binding, "urlopen", 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._GITHUB_API_OPENER, "open", raise_url) + monkeypatch.setattr(binding, "urlopen", 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._GITHUB_API_OPENER, "open", lambda *_a, **_k: Response()) + monkeypatch.setattr(binding, "urlopen", 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._GITHUB_API_OPENER, "open", lambda *_a, **_k: Response()) + monkeypatch.setattr(binding, "urlopen", lambda *_a, **_k: Response()) rows = binding.load_changed_paths_from_github( "https://api.github.com", "ContextualWisdomLab/example", @@ -969,13 +969,3 @@ def test_workspace_missing_root_returns_false(tmp_path: Path) -> None: missing = tmp_path / "missing-root" assert binding.workspace_contains_expected_diff(missing, "a.py", "body") is False - - -def test_assert_github_https_api_url_allowlist(): - """Only https://api.github.com may reach urlopen in evidence binding.""" - import scripts.ci.strix_evidence_binding as mod - mod._assert_github_https_api_url("https://api.github.com/repos/o/r") - with pytest.raises(mod.EvidenceBindingError, match="api.github.com"): - mod._assert_github_https_api_url("file:///etc/passwd") - with pytest.raises(mod.EvidenceBindingError, match="api.github.com"): - mod._assert_github_https_api_url("https://example.com/x") diff --git a/tests/test_strix_evidence_binding_redirect_contract.py b/tests/test_strix_evidence_binding_redirect_contract.py deleted file mode 100644 index 21a4ddb6df..0000000000 --- a/tests/test_strix_evidence_binding_redirect_contract.py +++ /dev/null @@ -1,52 +0,0 @@ -"""Fail-closed redirect contract for authenticated Strix GitHub API reads.""" - -from __future__ import annotations - -from urllib.request import Request - -import pytest - -from scripts.ci import strix_evidence_binding as binding - - -def _authenticated_request() -> Request: - """Build one admitted GitHub REST request carrying a bearer credential.""" - - return Request( - "https://api.github.com/repos/ContextualWisdomLab/example/pulls/1/files", - headers={"Authorization": "Bearer secret"}, - method="GET", - ) - - -def test_authenticated_redirect_rejects_cross_origin_before_bearer_forwarding() -> None: - """A 30x target outside api.github.com must fail before Request creation.""" - - handler = binding._GitHubApiRedirectHandler() - with pytest.raises(binding.EvidenceBindingError, match="only https://api.github.com"): - handler.redirect_request( - _authenticated_request(), - None, - 302, - "Found", - {}, - "https://evil.example/collect", - ) - - -def test_authenticated_redirect_preserves_same_origin_request() -> None: - """An admitted same-origin redirect keeps the authenticated GitHub request.""" - - handler = binding._GitHubApiRedirectHandler() - redirected = handler.redirect_request( - _authenticated_request(), - None, - 302, - "Found", - {}, - "/repositories/1/pulls/1/files?page=2", - ) - - assert redirected is not None - assert redirected.full_url == "https://api.github.com/repositories/1/pulls/1/files?page=2" - assert redirected.get_header("Authorization") == "Bearer secret" From e43a75b31177b2348b4f7afbf8298eea29a0c334 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 20 Sep 2026 02:18:12 +0900 Subject: [PATCH 10/20] fix(opencode): materialize every coverage lock input --- .../workflows/opencode-review-dispatch.yml | 7 +++++++ CHANGELOG.md | 4 ++++ docs/product-technical-gap-baseline.md | 6 ++++++ tests/test_opencode_agent_contract.py | 20 +++++++++++++++++++ ...t_pr_review_autofix_nvidia_nim_contract.py | 2 +- 5 files changed, 38 insertions(+), 1 deletion(-) diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index cbc8d21439..1f8e74bc86 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -632,12 +632,17 @@ jobs: coverage_tool_image="opencode-coverage-tools:${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}" coverage_build_dir="${RUNNER_TEMP}/opencode-coverage-tool-build" trusted_ci_requirements="${GITHUB_WORKSPACE}/requirements-opencode-review-ci-hashes.txt" + trusted_noema_document_requirements="${GITHUB_WORKSPACE}/requirements-noema-document-ci-hashes.txt" trusted_base_python_installer="${GITHUB_WORKSPACE}/scripts/ci/install_base_python_locks.py" trusted_vcs_import_root_resolver="${GITHUB_WORKSPACE}/scripts/ci/resolve_opencode_base_vcs_import_root.sh" if [ ! -f "$trusted_ci_requirements" ] || [ -L "$trusted_ci_requirements" ]; then echo "::error::Trusted coverage requirements must be a regular non-symlink file." exit 1 fi + if [ ! -f "$trusted_noema_document_requirements" ] || [ -L "$trusted_noema_document_requirements" ]; then + echo "::error::Trusted Noema document requirements must be a regular non-symlink file." + exit 1 + fi if [ ! -f "$trusted_base_python_installer" ] || [ -L "$trusted_base_python_installer" ]; then echo "::error::Trusted base Python lock installer must be a regular non-symlink file." exit 1 @@ -651,6 +656,8 @@ jobs: chmod 0700 "$coverage_build_dir" install -m 0644 "$trusted_ci_requirements" \ "$coverage_build_dir/requirements-opencode-review-ci-hashes.txt" + install -m 0644 "$trusted_noema_document_requirements" \ + "$coverage_build_dir/requirements-noema-document-ci-hashes.txt" install -m 0755 "$trusted_base_python_installer" \ "$coverage_build_dir/install-base-python-locks.py" install -m 0755 "$trusted_vcs_import_root_resolver" \ diff --git a/CHANGELOG.md b/CHANGELOG.md index 34281625cb..3e08bd4288 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +### OpenCode coverage image materializes every Dockerfile lock input + +- Required OpenCode run `35370902053` for `.github#2266@12621f75e` failed before executing PR code because its trusted Dockerfile copied `requirements-noema-document-ci-hashes.txt` while the isolated build context contained only the OpenCode lockfile. The coverage owner now validates both lockfiles as regular non-symlink files and copies both into the trusted build context before the networked image build. `tests/test_opencode_agent_contract.py` pins the complete input boundary. Hosted exact-head acceptance remains Proposed until the new run reaches the image-build and coverage steps. + ### Noema transport capacity schedules a bounded continuation re-dispatch - After gateway failover, HTTP 429/5xx no longer end only as a permanent required-check failure with `caller attempts=1`. ADR-0031 classifies that class as `provider_capacity_unavailable`, keeps the single gateway request per job, surfaces `provider_attempt_count` from the orchestrator error envelope, and authorizes at most two same-head `repository_dispatch` retries after a capped `Retry-After` or deterministic 60–180 s jitter. Review is never skipped. Refs #2165. diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index c617e3ad73..f11c236408 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -7,6 +7,12 @@ 이 문서는 제품·기술·운영 Gap을 현재 문서와 현재 GitHub 상태에 묶어 두는 기준선이다. 새 작업은 먼저 이 문서의 Gap ID를 PR 설명과 테스트 증거에 연결하고, PR의 정확한 exact HEAD·Checks·리뷰를 다시 수집한 뒤 구현한다. 표의 상태는 작성 시점의 관측값이므로, 병합 판단에는 재사용하지 않는다. 이 인벤토리는 스냅샷이며 merge authorization이 아니다. +### 2026-09-19 exact-head incident delta + +| Gap ID | 상태 | exact-head evidence | causal owner / next gate | +|---|---|---|---| +| CONTROL-OPENCODE-COVERAGE-LOCK-CONTEXT-01 | **Proposed — RED/GREEN source repair prepared on `.github#2266`; hosted acceptance pending** | Required OpenCode run `35370902053`의 `coverage-evidence` job `105778600365`은 PR source 실행 전에 `COPY requirements-opencode-review-ci-hashes.txt requirements-noema-document-ci-hashes.txt /tmp/`에서 두 번째 파일을 찾지 못해 종료했다. RED `9b9f5edcd`는 Dockerfile의 모든 lock input이 trusted build context에 존재해야 한다는 계약을 고정했다. | Canonical owner는 중앙 `.github/.github/workflows/opencode-review-dispatch.yml`이다. 두 lockfile을 각각 regular non-symlink로 검증하고 build context로 복사한 뒤 exact-head focused/full suite와 새 hosted `coverage-evidence`를 통과해야 한다. PR 제품 source나 coverage 비율의 결함으로 오인하지 않으며 synthetic status·manual rerun·bypass를 사용하지 않는다. | + ### 2026-09-13 current-head incident delta | Gap ID | 상태 | exact-head evidence | causal owner / next gate | diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index 5a41cb7cdc..5cbc0a8cb4 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -743,6 +743,26 @@ def test_opencode_target_coverage_materializes_only_after_authorized_dispatch(): assert 'coverage_tool_image="opencode-coverage-tools:${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}"' in measure_step assert "The networked build context contains only this" in measure_step assert 'install -m 0644 "$trusted_ci_requirements"' in measure_step + assert ( + 'trusted_noema_document_requirements="${GITHUB_WORKSPACE}/requirements-noema-document-ci-hashes.txt"' + in measure_step + ) + assert ( + '[ ! -f "$trusted_noema_document_requirements" ]' + in measure_step + ) + assert ( + '[ -L "$trusted_noema_document_requirements" ]' + in measure_step + ) + assert ( + 'install -m 0644 "$trusted_noema_document_requirements"' + in measure_step + ) + assert ( + '"$coverage_build_dir/requirements-noema-document-ci-hashes.txt"' + in measure_step + ) assert 'install -m 0755 "$trusted_base_python_installer"' in measure_step assert "COPY install-base-python-locks.py" in measure_step assert "python3 -I /usr/local/libexec/install-base-python-locks.py" in measure_step diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 8b7c55a4ef..c2d7c26082 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -17,7 +17,7 @@ DOCTORING_RECORD = Path("docs/doctoring/hourly-nvidia-nim-autofix.md") CHANGELOG = Path("CHANGELOG.md") REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") -REVIEW_DISPATCH_BLOB_SHA = "cbc8d214394c4b7acbe82ce7fba11fd073b91c98" +REVIEW_DISPATCH_BLOB_SHA = "1f8e74bc8683ec54510fbabca71d8322b93fe9ad" def _workflow_text(path: Path) -> str: From c4a73a174e6bf54cde1ac996bc7df1a94de949fa Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 20 Sep 2026 03:55:10 +0900 Subject: [PATCH 11/20] docs(gap): bind coverage repair to canonical PR --- docs/product-technical-gap-baseline.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index f11c236408..2bae6f6601 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -11,7 +11,7 @@ | Gap ID | 상태 | exact-head evidence | causal owner / next gate | |---|---|---|---| -| CONTROL-OPENCODE-COVERAGE-LOCK-CONTEXT-01 | **Proposed — RED/GREEN source repair prepared on `.github#2266`; hosted acceptance pending** | Required OpenCode run `35370902053`의 `coverage-evidence` job `105778600365`은 PR source 실행 전에 `COPY requirements-opencode-review-ci-hashes.txt requirements-noema-document-ci-hashes.txt /tmp/`에서 두 번째 파일을 찾지 못해 종료했다. RED `9b9f5edcd`는 Dockerfile의 모든 lock input이 trusted build context에 존재해야 한다는 계약을 고정했다. | Canonical owner는 중앙 `.github/.github/workflows/opencode-review-dispatch.yml`이다. 두 lockfile을 각각 regular non-symlink로 검증하고 build context로 복사한 뒤 exact-head focused/full suite와 새 hosted `coverage-evidence`를 통과해야 한다. PR 제품 source나 coverage 비율의 결함으로 오인하지 않으며 synthetic status·manual rerun·bypass를 사용하지 않는다. | +| CONTROL-OPENCODE-COVERAGE-LOCK-CONTEXT-01 | **Proposed — RED/GREEN source repair prepared on `.github#2286`; hosted acceptance pending** | Required OpenCode run `35370902053`의 `coverage-evidence` job `105778600365`은 PR source 실행 전에 `COPY requirements-opencode-review-ci-hashes.txt requirements-noema-document-ci-hashes.txt /tmp/`에서 두 번째 파일을 찾지 못해 종료했다. RED `9b9f5edcd`는 Dockerfile의 모든 lock input이 trusted build context에 존재해야 한다는 계약을 고정했다. | Canonical owner는 중앙 `.github/.github/workflows/opencode-review-dispatch.yml`이다. 두 lockfile을 각각 regular non-symlink로 검증하고 build context로 복사한 뒤 exact-head focused/full suite와 새 hosted `coverage-evidence`를 통과해야 한다. PR 제품 source나 coverage 비율의 결함으로 오인하지 않으며 synthetic status·manual rerun·bypass를 사용하지 않는다. | ### 2026-09-13 current-head incident delta From 8a5251bf409fe84b3dd0cba1e48992f5b8d9eda5 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 20 Sep 2026 10:02:09 +0900 Subject: [PATCH 12/20] fix(deps): restore AnyIO owner isolation From 657d10402d4d502249423a85faa1f953542cd719 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 23 Sep 2026 10:09:21 +0900 Subject: [PATCH 13/20] repair(codeql): restore exact endpoint-set assertion --- ...nization_commercial_readiness_loop_receipt_contract.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/test_organization_commercial_readiness_loop_receipt_contract.py b/tests/test_organization_commercial_readiness_loop_receipt_contract.py index 6ae9dfa595..0a7bde07a4 100644 --- a/tests/test_organization_commercial_readiness_loop_receipt_contract.py +++ b/tests/test_organization_commercial_readiness_loop_receipt_contract.py @@ -57,7 +57,9 @@ def test_json_receipt_is_retained_as_an_immutable_short_lived_artifact() -> None assert "if-no-files-found: error" in source assert "retention-days: 3" in source endpoints = _harden_runner_allowed_endpoints(source) - assert "results-receiver.actions.githubusercontent.com:443" in endpoints - assert "*.actions.githubusercontent.com:443" in endpoints - assert "*.blob.core.windows.net:443" in endpoints + assert { + "results-receiver.actions.githubusercontent.com:443", + "*.actions.githubusercontent.com:443", + "*.blob.core.windows.net:443", + }.issubset(endpoints) assert "- name: Checkout exact trusted coordinator source" not in endpoints From 38cfcae58f30e452bb17e5a42878ecd1a54cdb4c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 25 Sep 2026 00:51:51 +0900 Subject: [PATCH 14/20] fix(test-gate): restore full branch coverage Remove the unused queue-health collector and CLI, validate run IDs at the shared parsing boundary, and exercise document-reader and scheduler edge cases. The main baseline failed the 100% gate before PR #2358. --- scripts/ci/actions_queue_health.py | 26 +-- scripts/ci/actions_queue_health_core.py | 165 +----------------- ...ns_queue_health_cancelled_before_runner.py | 5 +- ...tions_queue_health_snapshot_consistency.py | 73 ++++++++ ...ions_queue_health_terminal_preexecution.py | 5 +- tests/test_noema_document_review_context.py | 7 + .../test_noema_review_document_boundaries.py | 128 ++++++++++++++ tests/test_pr_review_merge_scheduler.py | 10 +- 8 files changed, 232 insertions(+), 187 deletions(-) create mode 100644 tests/test_noema_review_document_boundaries.py diff --git a/scripts/ci/actions_queue_health.py b/scripts/ci/actions_queue_health.py index bb73698551..e01145b8e0 100644 --- a/scripts/ci/actions_queue_health.py +++ b/scripts/ci/actions_queue_health.py @@ -1,8 +1,8 @@ #!/usr/bin/env python3 """Queue-health CLI with stable identity and audit-provenance guarantees. -The shared collector implementation lives in ``actions_queue_health_core.py``. -This entrypoint owns the consistency boundary that binds active-run evidence to +Shared parsing and reporting primitives live in ``actions_queue_health_core.py``. +This entrypoint owns collection and the consistency boundary that binds active-run evidence to a stable pull-request view, carries stable workflow identity, and exports the exact timestamp used for queue-age calculations. """ @@ -163,16 +163,7 @@ def collect_snapshot( ), ) for workflow_run in workflow_runs: - workflow_run_id = workflow_run.get("id") - if ( - isinstance(workflow_run_id, bool) - or not isinstance(workflow_run_id, int) - or workflow_run_id <= 0 - ): - raise QueueHealthError( - "workflow run id must be a positive integer" - ) - active_snapshot[workflow_run_id] = workflow_run + active_snapshot[workflow_run["id"]] = workflow_run active_snapshots.append(active_snapshot) first_snapshot, second_snapshot = active_snapshots @@ -242,16 +233,7 @@ def collect_snapshot( TERMINAL_DIAGNOSTIC_STATUSES ): continue - workflow_run_id = workflow_run.get("id") - if ( - isinstance(workflow_run_id, bool) - or not isinstance(workflow_run_id, int) - or workflow_run_id <= 0 - ): - raise QueueHealthError( - "workflow run id must be a positive integer" - ) - terminal_diagnostic_snapshot[workflow_run_id] = workflow_run + terminal_diagnostic_snapshot[workflow_run["id"]] = workflow_run for terminal_status in TARGET_TERMINAL_DIAGNOSTIC_STATUSES: target_workflow_runs = _list_payload( diff --git a/scripts/ci/actions_queue_health_core.py b/scripts/ci/actions_queue_health_core.py index db3e5570ba..50cb3e0793 100644 --- a/scripts/ci/actions_queue_health_core.py +++ b/scripts/ci/actions_queue_health_core.py @@ -112,6 +112,13 @@ def _list_payload( declared_total_counts.append(payload["total_count"]) if not isinstance(values, list) or not all(isinstance(value, dict) for value in values): raise QueueHealthError(f"GitHub response field {key!r} must be an array of objects") + if key == "workflow_runs" and any( + isinstance(value.get("id"), bool) + or not isinstance(value.get("id"), int) + or value["id"] <= 0 + for value in values + ): + raise QueueHealthError("workflow run id must be a positive integer") if isinstance(payload, dict) and PAGINATED_PAGES_KEY in payload: record_identities: list[tuple[str, int]] = [] for value in values: @@ -360,133 +367,6 @@ def _normalise_run(repository: str, run: dict[str, Any], jobs: list[dict[str, An } -def collect_snapshot( - repositories: Sequence[str], - *, - runner: Runner = subprocess.run, - generated_at: str | None = None, -) -> dict[str, Any]: - """Collect bounded queued/in-progress run and job data using read-only API calls.""" - validated = sorted({_repository_name(repository) for repository in repositories}) - if len(validated) != len(repositories): - raise QueueHealthError("collection repository list contains duplicates") - collected_repositories: list[dict[str, Any]] = [] - collection_errors: list[dict[str, str]] = [] - for repository in validated: - try: - metadata = github_json(f"repos/{repository}", runner=runner) - if not isinstance(metadata, dict): - raise QueueHealthError(f"repository metadata for {repository} is not an object") - pulls_endpoint = f"repos/{repository}/pulls?state=open&per_page={MAX_API_PAGE_SIZE}" - pull_requests = _list_payload( - github_json(pulls_endpoint, paginate=True, runner=runner), - "pulls", - max_items=MAX_API_PAGE_SIZE * MAX_API_PAGES, - ) - normalized_pull_requests = sorted( - (_normalise_pull_request(item) for item in pull_requests), - key=lambda item: item["number"], - ) - except IncompletePullRequestIdentity: - time.sleep(PULL_REQUEST_RETRY_DELAY_SECONDS) - try: - retry_pull_requests = _list_payload( - github_json(pulls_endpoint, paginate=True, runner=runner), - "pulls", - max_items=MAX_API_PAGE_SIZE * MAX_API_PAGES, - ) - normalized_pull_requests = sorted( - (_normalise_pull_request(item) for item in retry_pull_requests), - key=lambda item: item["number"], - ) - except QueueHealthError as retry_exc: - collection_errors.append( - { - "repository": repository, - "error": f"pull-request identity validation failed: {retry_exc}", - } - ) - continue - except QueueHealthError as exc: - collection_errors.append({"repository": repository, "error": str(exc)}) - continue - pull_requests_by_number = {item["number"]: item for item in normalized_pull_requests} - runs_by_id: dict[int, dict[str, Any]] = {} - try: - active_statuses = ("in_progress", "pending", "queued", "requested", "waiting") - snapshots: list[dict[int, dict[str, Any]]] = [] - for status_order in (active_statuses, tuple(reversed(active_statuses))): - snapshot: dict[int, dict[str, Any]] = {} - for status in status_order: - runs = _list_payload( - github_json( - f"repos/{repository}/actions/runs?status={status}" - f"&per_page={WORKFLOW_RUN_PAGE_SIZE}", - paginate=True, - max_pages=ACTIVE_RUN_MAX_API_PAGES, - runner=runner, - ), - "workflow_runs", - max_items=WORKFLOW_RUN_PAGE_SIZE * ACTIVE_RUN_MAX_API_PAGES, - ) - for run in runs: - run_id = run.get("id") - if isinstance(run_id, bool) or not isinstance(run_id, int) or run_id <= 0: - raise QueueHealthError("workflow run id must be a positive integer") - snapshot[run_id] = run - snapshots.append(snapshot) - first_snapshot, second_snapshot = snapshots - first_states = { - run_id: str(run.get("status") or "").upper() - for run_id, run in first_snapshot.items() - } - second_states = { - run_id: str(run.get("status") or "").upper() - for run_id, run in second_snapshot.items() - } - if first_states != second_states: - raise QueueHealthError("active workflow run snapshot changed during collection") - for run_id, run in second_snapshot.items(): - run_id = run.get("id") - candidate = _normalise_run(repository, run, []) - identity, _ = _run_identity(candidate, pull_requests_by_number) - if identity != "current_head" or candidate["status"] not in { - "IN_PROGRESS", - "WAITING", - }: - runs_by_id[run_id] = candidate - continue - jobs_payload = github_json( - f"repos/{repository}/actions/runs/{run_id}/jobs?per_page={MAX_API_PAGE_SIZE}", - paginate=True, - runner=runner, - ) - jobs = _list_payload( - jobs_payload, - "jobs", - max_items=MAX_API_PAGE_SIZE * MAX_API_PAGES, - ) - runs_by_id[run_id] = _normalise_run(repository, run, jobs) - except QueueHealthError as exc: - collection_errors.append({"repository": repository, "error": str(exc)}) - continue - collected_repositories.append( - { - "full_name": repository, - "default_branch": str(metadata.get("default_branch") or ""), - "pull_requests": normalized_pull_requests, - "runs": sorted(runs_by_id.values(), key=lambda item: item["id"]), - } - ) - timestamp = generated_at or datetime.now(timezone.utc).isoformat().replace("+00:00", "Z") - parse_timestamp(timestamp) - return { - "generated_at": timestamp, - "repositories": collected_repositories, - "collection_errors": collection_errors, - } - - def load_snapshot(path: Path) -> dict[str, Any]: """Load a JSON snapshot for offline, deterministic report generation.""" try: @@ -822,34 +702,3 @@ def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: parser.add_argument("--queue-age-slo-seconds", type=int, default=DEFAULT_QUEUE_AGE_SLO_SECONDS) parser.add_argument("--now", help="Explicit timezone-aware evaluation time for deterministic reports") return parser.parse_args(argv) - - -def main(argv: Sequence[str] | None = None, *, stderr: TextIO = sys.stderr) -> int: - """Collect or load a snapshot, write reports, and return a stable CLI status.""" - args = parse_args(argv) - try: - snapshot = load_snapshot(args.snapshot) if args.snapshot else collect_snapshot(load_allowlist(args.allowlist)) - now = parse_timestamp(args.now) if args.now else datetime.now(timezone.utc) - report = build_report( - snapshot, - now=now, - queue_age_slo_seconds=args.queue_age_slo_seconds, - ) - write_reports(report, args.output_json, args.output_html) - except (OSError, QueueHealthError, ValueError) as exc: - print(f"ERROR: queue-health report failed: {exc}", file=stderr) - return 2 - breaches = report["summary"]["unassigned_slo_breached_count"] - if breaches: - print(f"::warning::Actions queue-health found {breaches} unassigned current-head SLO breach(es).") - print( - "QUEUE_HEALTH_RESULT=" - f"observed={report['summary']['observed_job_count']} " - f"pending={report['summary']['pending_job_count']} " - f"slo_breaches={breaches}" - ) - return 0 - - -if __name__ == "__main__": # pragma: no cover - exercised through the CLI tests. - raise SystemExit(main()) diff --git a/tests/test_actions_queue_health_cancelled_before_runner.py b/tests/test_actions_queue_health_cancelled_before_runner.py index 1827275f68..850f3bd866 100644 --- a/tests/test_actions_queue_health_cancelled_before_runner.py +++ b/tests/test_actions_queue_health_cancelled_before_runner.py @@ -17,7 +17,7 @@ SPEC.loader.exec_module(queue_health) -def test_collect_snapshot_classifies_cancelled_job_before_runner_assignment() -> None: +def test_collect_snapshot_classifies_cancelled_job_before_runner_assignment(monkeypatch) -> None: """A cancelled current-head job with no runner or steps stays explicit evidence.""" repository_name = "owner/repo" pull_request = { @@ -144,6 +144,9 @@ def runner(args: list[str], **_: object) -> CompletedProcess[str]: "cancelled_before_runner_assignment" ) assert report["summary"]["cancelled_before_runner_assignment_count"] == 1 + actions = list(report["summary"]["external_actions"]) + monkeypatch.setattr(queue_health, "_CORE_BUILD_REPORT", lambda *_args, **_kwargs: report) + assert queue_health.build_report(snapshot)["summary"]["external_actions"] == actions def test_collect_snapshot_retains_cancelled_pull_request_target_current_head() -> None: diff --git a/tests/test_actions_queue_health_snapshot_consistency.py b/tests/test_actions_queue_health_snapshot_consistency.py index b9711a09dd..90cea39692 100644 --- a/tests/test_actions_queue_health_snapshot_consistency.py +++ b/tests/test_actions_queue_health_snapshot_consistency.py @@ -193,3 +193,76 @@ def test_queue_health_workflow_does_not_grant_unused_pull_request_permission() - """The scheduler token keeps only permissions used outside the cross-repository token.""" workflow = (ROOT / ".github/workflows/actions-queue-health.yml").read_text(encoding="utf-8") assert "pull-requests: read" not in workflow + + +@pytest.mark.parametrize( + ("active_runs", "completed_runs", "target_runs", "expected_error"), + [ + ([{"id": 0, "status": "queued"}], [], [], "workflow run id"), + ([], [{**_run(8, 501), "status": "completed", "conclusion": "failure", "id": 0}], [], "workflow run id"), + ([], [{**_run(8, 501), "status": "completed", "conclusion": "success"}], [], None), + ([], [], [{**_run(8, 501), "status": "completed", "conclusion": "cancelled", "pull_requests": []}], None), + ], +) +def test_collector_rejects_invalid_run_ids_and_ignores_unrelated_terminal_runs( + active_runs: list[dict], completed_runs: list[dict], target_runs: list[dict], expected_error: str | None, +) -> None: + """Bad identities fail closed; unrelated terminal runs do not become current-head evidence.""" + def runner(args: list[str], **_kwargs: object) -> CompletedProcess[str]: + path = args[-1] + if path == "repos/owner/repo": + payload: object = {"default_branch": "main"} + elif path == "repos/owner/repo/pulls?state=open&per_page=100": + payload = [_pull()] + elif "status=completed&head_sha=" in path: + payload = completed_runs + elif "status=cancelled&event=pull_request_target" in path: + payload = target_runs + elif "status=queued" in path: + payload = active_runs + elif "/actions/runs?status=" in path: + payload = [] + else: + raise AssertionError(f"unexpected endpoint: {path}") + return CompletedProcess(args, 0, json.dumps(payload), "") + + snapshot = queue_health.collect_snapshot( + ["owner/repo"], runner=runner, generated_at="2026-09-02T00:00:00Z" + ) + if expected_error: + assert snapshot["repositories"] == [] + assert expected_error in snapshot["collection_errors"][0]["error"] + else: + assert snapshot["collection_errors"] == [] + assert snapshot["repositories"][0]["runs"] == [] + + +def test_final_pull_identity_retry_failure_is_bounded(monkeypatch: pytest.MonkeyPatch) -> None: + """A repeatedly incomplete final PR view cannot certify current-head evidence.""" + monkeypatch.setattr(queue_health.time, "sleep", lambda _seconds: None) + pull_reads = 0 + + def runner(args: list[str], **_kwargs: object) -> CompletedProcess[str]: + nonlocal pull_reads + path = args[-1] + if path == "repos/owner/repo": + payload: object = {"default_branch": "main"} + elif path == "repos/owner/repo/pulls?state=open&per_page=100": + pull_reads += 1 + payload = [_pull()] if pull_reads == 1 else [{**_pull(), "head": {"sha": ""}}] + elif "/actions/runs?status=" in path: + payload = [] + else: + raise AssertionError(f"unexpected endpoint: {path}") + return CompletedProcess(args, 0, json.dumps(payload), "") + + snapshot = queue_health.collect_snapshot(["owner/repo"], runner=runner) + assert pull_reads == 3 + assert snapshot["repositories"] == [] + assert "pull-request identity validation failed" in snapshot["collection_errors"][0]["error"] + + +def test_core_run_normalization_rejects_non_object() -> None: + """A malformed workflow-run payload cannot be classified as a real run.""" + with pytest.raises(queue_health.QueueHealthError, match="workflow run entry must be an object"): + queue_health._CORE_NORMALISE_RUN("owner/repo", None, []) diff --git a/tests/test_actions_queue_health_terminal_preexecution.py b/tests/test_actions_queue_health_terminal_preexecution.py index 05237ba340..c86c435e30 100644 --- a/tests/test_actions_queue_health_terminal_preexecution.py +++ b/tests/test_actions_queue_health_terminal_preexecution.py @@ -57,7 +57,7 @@ def _terminal_failure_job() -> dict: } -def test_terminal_preexecution_failure_survives_collection_and_is_not_product_failure() -> None: +def test_terminal_preexecution_failure_survives_collection_and_is_not_product_failure(monkeypatch) -> None: """Keep failed zero-step jobs as explicit non-passing admission evidence.""" failed_run = _terminal_failure_run() failed_job = _terminal_failure_job() @@ -106,3 +106,6 @@ def runner(args: list[str], **_: object) -> CompletedProcess[str]: assert row["recommended_action"] == "inspect_actions_control_plane_without_leaf_bypass" assert report["summary"]["terminal_pre_execution_failure_count"] == 1 assert report["summary"]["terminal_job_count"] == 1 + actions = list(report["summary"]["external_actions"]) + monkeypatch.setattr(queue_health, "_CORE_BUILD_REPORT", lambda *_args, **_kwargs: report) + assert queue_health.build_report(snapshot)["summary"]["external_actions"] == actions diff --git a/tests/test_noema_document_review_context.py b/tests/test_noema_document_review_context.py index e6ec2e6d70..ffed1c8961 100644 --- a/tests/test_noema_document_review_context.py +++ b/tests/test_noema_document_review_context.py @@ -176,6 +176,13 @@ def test_forbidden_docx_entities_are_explicitly_rejected(): document.extract_review_document("docs/entity.docx", _docx_entity_bytes()) +def test_invalid_github_base64_content_fails_closed(monkeypatch): + """Malformed GitHub file data must not reach the document reader.""" + monkeypatch.setattr(noema, "run", lambda _args, stdin=None: "not/base64!") + with pytest.raises(RuntimeError, match="malformed base64"): + noema.fetch_file_content_at_ref("owner/repo", "docs/review.docx", "head") + + def test_hwp_reader_contract_is_local_and_fail_closed(monkeypatch): """HWP/HWPX use the configured local adapter and reject failed readers.""" monkeypatch.setenv(document.HWP_READER_ENV, "/trusted/hwp-mcp-source") diff --git a/tests/test_noema_review_document_boundaries.py b/tests/test_noema_review_document_boundaries.py new file mode 100644 index 0000000000..5680252b3f --- /dev/null +++ b/tests/test_noema_review_document_boundaries.py @@ -0,0 +1,128 @@ +"""Exercise document-reader failure boundaries used by protected review.""" + +from __future__ import annotations + +import io +import runpy +import sys +import zipfile +from pathlib import Path +from subprocess import CompletedProcess + +import pytest + +from scripts.ci import noema_review_document as document + + +def _docx(xml: str | None, *, extra_entries: int = 0) -> bytes: + """Build a small DOCX archive with optional missing document XML.""" + output = io.BytesIO() + with zipfile.ZipFile(output, "w") as archive: + if xml is not None: + archive.writestr("word/document.xml", xml) + for index in range(extra_entries): + archive.writestr(f"extra-{index}", "x") + return output.getvalue() + + +def _body(content: str) -> str: + """Wrap Word body content in the namespace expected by the reader.""" + return ( + f'' + f"{content}" + ) + + +def test_document_input_limits_and_unsupported_formats(monkeypatch: pytest.MonkeyPatch) -> None: + """Reject oversized, unsupported, and unconfigured reader inputs.""" + monkeypatch.setattr(document, "MAX_DOCUMENT_BYTES", 2) + with pytest.raises(document.DocumentReadError, match="8 MiB"): + document.extract_review_document("a.docx", b"long") + with pytest.raises(document.DocumentReadError, match="unsupported"): + document.extract_review_document("a.pdf", b"ok") + monkeypatch.delenv(document.HWP_READER_ENV, raising=False) + with pytest.raises(document.DocumentReadError, match="not configured"): + document.extract_review_document("a.hwp", b"ok") + + +def test_docx_archive_and_xml_boundaries(monkeypatch: pytest.MonkeyPatch) -> None: + """Reject partial archives and XML without visible document content.""" + valid = _docx(_body("ok")) + monkeypatch.setattr(document, "MAX_DOCUMENT_ZIP_ENTRIES", 0) + with pytest.raises(document.DocumentReadError, match="too many entries"): + document.extract_review_document("a.docx", valid) + monkeypatch.setattr(document, "MAX_DOCUMENT_ZIP_ENTRIES", 2048) + monkeypatch.setattr(document, "MAX_DOCUMENT_ZIP_UNCOMPRESSED_BYTES", 1) + with pytest.raises(document.DocumentReadError, match="unpacked size"): + document.extract_review_document("a.docx", valid) + monkeypatch.setattr(document, "MAX_DOCUMENT_ZIP_UNCOMPRESSED_BYTES", 64 * 1024 * 1024) + cases = ( + (_docx(None, extra_entries=1), "no word/document.xml"), + (_docx("'), "no document body"), + (_docx(_body("")), "no readable text"), + ) + for payload, message in cases: + with pytest.raises(document.DocumentReadError, match=message): + document.extract_review_document("a.docx", payload) + + +def test_docx_visible_controls_and_uneven_table() -> None: + """Keep tabs, line breaks, and uneven table cells in reviewer text.""" + xml = _body( + "ABC" + "X|Y" + "Z" + "" + "" + ) + text = document.extract_review_document("a.docx", _docx(xml)) + assert "A\tB\nC" in text + assert "X\\|Y" in text + assert "| Z | |" in text + assert "### Table 1 (2 rows x 2 columns)" in text + assert "Table 2" not in text + + +def test_hwp_reader_rejects_process_and_output_failures(monkeypatch: pytest.MonkeyPatch) -> None: + """Keep parser failures and untrusted output out of the review prompt.""" + monkeypatch.setenv(document.HWP_READER_ENV, "/reviewed/source") + + def unavailable(*_args: object, **_kwargs: object) -> None: + raise OSError("private process detail") + + monkeypatch.setattr(document.subprocess, "run", unavailable) + with pytest.raises(document.DocumentReadError, match="could not start"): + document.extract_review_document("a.hwpx", b"data") + + for stdout, message in ((b"abcd", "bounded output"), (b"\xff", "non-UTF-8"), (b" ", "empty text")): + monkeypatch.setattr(document, "MAX_DOCUMENT_TEXT_BYTES", 3) + completed = CompletedProcess(["node"], 0, stdout, b"") + monkeypatch.setattr( + document.subprocess, + "run", + lambda *_args, **_kwargs: completed, + ) + with pytest.raises(document.DocumentReadError, match=message): + document.extract_review_document("a.hwpx", b"data") + + +def test_document_text_truncation_and_cli(monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + """Bound UTF-8 output and preserve a useful local CLI failure exit.""" + monkeypatch.setattr(document, "MAX_DOCUMENT_TEXT_BYTES", 4) + assert document._bounded_text("ééé").startswith("éé\n[document text truncated;") + assert document._bounded_text("ok") == "ok" + + path = tmp_path / "review.docx" + path.write_bytes(_docx(_body("ok"))) + monkeypatch.setattr(sys, "argv", ["noema_review_document.py", str(path)]) + assert document._main() == 0 + assert "ok" in capsys.readouterr().out + monkeypatch.setattr(sys, "argv", ["noema_review_document.py", str(path.with_name("missing.docx"))]) + assert document._main() == 1 + assert capsys.readouterr().err + + monkeypatch.setattr(sys, "argv", ["noema_review_document.py", str(path)]) + with pytest.raises(SystemExit) as exit_status: + runpy.run_path(str(Path(document.__file__)), run_name="__main__") + assert exit_status.value.code == 0 diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 4b8715d361..b8afdfcd92 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -2363,19 +2363,19 @@ def fake_active_workflow_runs(repo, statuses, *, event=None, created=None, head_ assert created == ">=2026-09-17T11:50:00Z" return [ { - "path": ".github/workflows/opencode-review-coalesce-tick.yml", + "path": ".github/workflows/other.yml", "conclusion": "success", - "updated_at": "2026-09-17T11:55:00Z", + "updated_at": "2026-09-17T11:59:00Z", }, { "path": ".github/workflows/opencode-review-coalesce-tick.yml", "conclusion": "success", - "updated_at": "2026-09-17T11:40:00Z", + "updated_at": "2026-09-17T11:55:00Z", }, { - "path": ".github/workflows/other.yml", + "path": ".github/workflows/opencode-review-coalesce-tick.yml", "conclusion": "success", - "updated_at": "2026-09-17T11:59:00Z", + "updated_at": "2026-09-17T11:40:00Z", }, ] From 21247efff574b0cc8eb80589cb584fc91a6f3a1f Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 25 Sep 2026 00:58:54 +0900 Subject: [PATCH 15/20] test(queue-health): scope permission assertion to workflow token --- tests/test_actions_queue_health_snapshot_consistency.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_actions_queue_health_snapshot_consistency.py b/tests/test_actions_queue_health_snapshot_consistency.py index 90cea39692..c765959cb6 100644 --- a/tests/test_actions_queue_health_snapshot_consistency.py +++ b/tests/test_actions_queue_health_snapshot_consistency.py @@ -192,7 +192,8 @@ def test_invalid_present_workflow_id_fails_closed() -> None: def test_queue_health_workflow_does_not_grant_unused_pull_request_permission() -> None: """The scheduler token keeps only permissions used outside the cross-repository token.""" workflow = (ROOT / ".github/workflows/actions-queue-health.yml").read_text(encoding="utf-8") - assert "pull-requests: read" not in workflow + assert "\n pull-requests: read\n" not in workflow + assert "\n pull-requests: read\n" not in workflow @pytest.mark.parametrize( From 57c168b8c13d72eecdd374d026737f89f540f626 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 25 Sep 2026 01:00:06 +0900 Subject: [PATCH 16/20] test(queue-health): isolate collector edge cases --- ...ctions_queue_health_post_evidence_retry.py | 87 +++++++++++++++++++ ...tions_queue_health_snapshot_consistency.py | 73 ---------------- 2 files changed, 87 insertions(+), 73 deletions(-) diff --git a/tests/test_actions_queue_health_post_evidence_retry.py b/tests/test_actions_queue_health_post_evidence_retry.py index bc266fa96f..ae1c619cd0 100644 --- a/tests/test_actions_queue_health_post_evidence_retry.py +++ b/tests/test_actions_queue_health_post_evidence_retry.py @@ -5,6 +5,8 @@ from pathlib import Path from subprocess import CompletedProcess +import pytest + ROOT = Path(__file__).resolve().parents[1] MODULE_PATH = ROOT / "scripts/ci/actions_queue_health.py" @@ -82,3 +84,88 @@ def test_post_evidence_identity_read_fails_closed_after_retry_remains_incomplete assert len(snapshot["collection_errors"]) == 1 assert snapshot["collection_errors"][0]["repository"] == "owner/repo" assert "pull-request identity validation failed" in snapshot["collection_errors"][0]["error"] + +def _run(run_id: int, workflow_id: int) -> dict: + """Return one linked run for terminal-filter boundary tests.""" + return { + "id": run_id, + "workflow_id": workflow_id, + "name": "required-check", + "event": "pull_request", + "status": "queued", + "head_sha": "head", + "pull_requests": [{"number": 1, "head": {"sha": "head"}}], + } + + +@pytest.mark.parametrize( + ("active_runs", "completed_runs", "target_runs", "expected_error"), + [ + ([{"id": 0, "status": "queued"}], [], [], "workflow run id"), + ([], [{**_run(8, 501), "status": "completed", "conclusion": "failure", "id": 0}], [], "workflow run id"), + ([], [{**_run(8, 501), "status": "completed", "conclusion": "success"}], [], None), + ([], [], [{**_run(8, 501), "status": "completed", "conclusion": "cancelled", "pull_requests": []}], None), + ], +) +def test_collector_rejects_invalid_run_ids_and_ignores_unrelated_terminal_runs( + active_runs: list[dict], completed_runs: list[dict], target_runs: list[dict], expected_error: str | None, +) -> None: + """Bad identities fail closed; unrelated terminal runs do not become current-head evidence.""" + def runner(args: list[str], **_kwargs: object) -> CompletedProcess[str]: + path = args[-1] + if path == "repos/owner/repo": + payload: object = {"default_branch": "main"} + elif path == "repos/owner/repo/pulls?state=open&per_page=100": + payload = [_pull()] + elif "status=completed&head_sha=" in path: + payload = completed_runs + elif "status=cancelled&event=pull_request_target" in path: + payload = target_runs + elif "status=queued" in path: + payload = active_runs + elif "/actions/runs?status=" in path: + payload = [] + else: + raise AssertionError(f"unexpected endpoint: {path}") + return CompletedProcess(args, 0, json.dumps(payload), "") + + snapshot = queue_health.collect_snapshot( + ["owner/repo"], runner=runner, generated_at="2026-09-02T00:00:00Z" + ) + if expected_error: + assert snapshot["repositories"] == [] + assert expected_error in snapshot["collection_errors"][0]["error"] + else: + assert snapshot["collection_errors"] == [] + assert snapshot["repositories"][0]["runs"] == [] + + +def test_final_pull_identity_retry_failure_is_bounded(monkeypatch: pytest.MonkeyPatch) -> None: + """A repeatedly incomplete final PR view cannot certify current-head evidence.""" + monkeypatch.setattr(queue_health.time, "sleep", lambda _seconds: None) + pull_reads = 0 + + def runner(args: list[str], **_kwargs: object) -> CompletedProcess[str]: + nonlocal pull_reads + path = args[-1] + if path == "repos/owner/repo": + payload: object = {"default_branch": "main"} + elif path == "repos/owner/repo/pulls?state=open&per_page=100": + pull_reads += 1 + payload = [_pull()] if pull_reads == 1 else [{**_pull(), "head": {"sha": ""}}] + elif "/actions/runs?status=" in path: + payload = [] + else: + raise AssertionError(f"unexpected endpoint: {path}") + return CompletedProcess(args, 0, json.dumps(payload), "") + + snapshot = queue_health.collect_snapshot(["owner/repo"], runner=runner) + assert pull_reads == 3 + assert snapshot["repositories"] == [] + assert "pull-request identity validation failed" in snapshot["collection_errors"][0]["error"] + + +def test_core_run_normalization_rejects_non_object() -> None: + """A malformed workflow-run payload cannot be classified as a real run.""" + with pytest.raises(queue_health.QueueHealthError, match="workflow run entry must be an object"): + queue_health._CORE_NORMALISE_RUN("owner/repo", None, []) diff --git a/tests/test_actions_queue_health_snapshot_consistency.py b/tests/test_actions_queue_health_snapshot_consistency.py index c765959cb6..8e939e2fc9 100644 --- a/tests/test_actions_queue_health_snapshot_consistency.py +++ b/tests/test_actions_queue_health_snapshot_consistency.py @@ -194,76 +194,3 @@ def test_queue_health_workflow_does_not_grant_unused_pull_request_permission() - workflow = (ROOT / ".github/workflows/actions-queue-health.yml").read_text(encoding="utf-8") assert "\n pull-requests: read\n" not in workflow assert "\n pull-requests: read\n" not in workflow - - -@pytest.mark.parametrize( - ("active_runs", "completed_runs", "target_runs", "expected_error"), - [ - ([{"id": 0, "status": "queued"}], [], [], "workflow run id"), - ([], [{**_run(8, 501), "status": "completed", "conclusion": "failure", "id": 0}], [], "workflow run id"), - ([], [{**_run(8, 501), "status": "completed", "conclusion": "success"}], [], None), - ([], [], [{**_run(8, 501), "status": "completed", "conclusion": "cancelled", "pull_requests": []}], None), - ], -) -def test_collector_rejects_invalid_run_ids_and_ignores_unrelated_terminal_runs( - active_runs: list[dict], completed_runs: list[dict], target_runs: list[dict], expected_error: str | None, -) -> None: - """Bad identities fail closed; unrelated terminal runs do not become current-head evidence.""" - def runner(args: list[str], **_kwargs: object) -> CompletedProcess[str]: - path = args[-1] - if path == "repos/owner/repo": - payload: object = {"default_branch": "main"} - elif path == "repos/owner/repo/pulls?state=open&per_page=100": - payload = [_pull()] - elif "status=completed&head_sha=" in path: - payload = completed_runs - elif "status=cancelled&event=pull_request_target" in path: - payload = target_runs - elif "status=queued" in path: - payload = active_runs - elif "/actions/runs?status=" in path: - payload = [] - else: - raise AssertionError(f"unexpected endpoint: {path}") - return CompletedProcess(args, 0, json.dumps(payload), "") - - snapshot = queue_health.collect_snapshot( - ["owner/repo"], runner=runner, generated_at="2026-09-02T00:00:00Z" - ) - if expected_error: - assert snapshot["repositories"] == [] - assert expected_error in snapshot["collection_errors"][0]["error"] - else: - assert snapshot["collection_errors"] == [] - assert snapshot["repositories"][0]["runs"] == [] - - -def test_final_pull_identity_retry_failure_is_bounded(monkeypatch: pytest.MonkeyPatch) -> None: - """A repeatedly incomplete final PR view cannot certify current-head evidence.""" - monkeypatch.setattr(queue_health.time, "sleep", lambda _seconds: None) - pull_reads = 0 - - def runner(args: list[str], **_kwargs: object) -> CompletedProcess[str]: - nonlocal pull_reads - path = args[-1] - if path == "repos/owner/repo": - payload: object = {"default_branch": "main"} - elif path == "repos/owner/repo/pulls?state=open&per_page=100": - pull_reads += 1 - payload = [_pull()] if pull_reads == 1 else [{**_pull(), "head": {"sha": ""}}] - elif "/actions/runs?status=" in path: - payload = [] - else: - raise AssertionError(f"unexpected endpoint: {path}") - return CompletedProcess(args, 0, json.dumps(payload), "") - - snapshot = queue_health.collect_snapshot(["owner/repo"], runner=runner) - assert pull_reads == 3 - assert snapshot["repositories"] == [] - assert "pull-request identity validation failed" in snapshot["collection_errors"][0]["error"] - - -def test_core_run_normalization_rejects_non_object() -> None: - """A malformed workflow-run payload cannot be classified as a real run.""" - with pytest.raises(queue_health.QueueHealthError, match="workflow run entry must be an object"): - queue_health._CORE_NORMALISE_RUN("owner/repo", None, []) From 3b3ce16b105d6548189108d9d529e9dba4411392 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 25 Sep 2026 21:49:41 +0900 Subject: [PATCH 17/20] fix(security): update AnyIO lock for audit gate --- requirements-strix-ci-hashes.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/requirements-strix-ci-hashes.txt b/requirements-strix-ci-hashes.txt index 9e705850b5..eb83beda17 100644 --- a/requirements-strix-ci-hashes.txt +++ b/requirements-strix-ci-hashes.txt @@ -140,9 +140,9 @@ annotated-types==0.7.0 \ --hash=sha256:1f02e8b43a8fbbc3f3e0d4f0f4bfc8131bcb4eebe8849b8e5c773f3a1c582a53 \ --hash=sha256:aff07c09a53a08bc8cfccb9c85b05f1aa9a2a6f23728d790723543408344ce89 # via pydantic -anyio==4.14.0 \ - --hash=sha256:b47c1f9ccf73e67021df785332508f99379c68fa7d0684e8e3492cb1d4b23f89 \ - --hash=sha256:dd9b7a2a9799ed6552fde617b2c5df02b7fdd7d88392fc48101e51bae46164d9 +anyio==4.14.2 \ + --hash=sha256:9f505dda5ac9f0c8309b5e8bd445a8c2bf7246f3ce950121e45ea15bc41d1494 \ + --hash=sha256:cfa139f3ed1a23ee8f88a145ddb5ac7605b8bbfd8592baacd7ce3d8bb4313c7f # via # google-genai # gql From 3d0352198303ad055e751f1f5580313a687a2638 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Sat, 26 Sep 2026 04:59:12 +0900 Subject: [PATCH 18/20] fix(dispatch): retire stale CodeQL/OpenCode dispatches as notices instead of failures A dispatch whose target PR is closed, or whose live head has moved strictly past the dispatched head, ends validate with a ::notice:: and stale=true; later jobs are gated on that output so the run concludes success. A head mismatch alone is not treated as stale: GitHub can briefly serve the previous head right after a push. Retirement requires the compare API (repos/{target}/compare/{dispatched}...{live}) to answer status "ahead" with behind_by 0 and ahead_by >= 1. Behind (API lag), diverged (force-push), or a failed/malformed compare keeps the original fail-closed head_sha mismatch. The OpenCode side keeps its EVENT_NAME == repository_dispatch guard because its authorization and supplied-head binding are nested under that check; CodeQL triggers only on repository_dispatch and authorizes unconditionally, so it needs no guard. Tests pin both premises. Draft PRs are not stale. --- .github/workflows/codeql-scan-dispatch.yml | 63 ++++ .../workflows/opencode-review-dispatch.yml | 61 ++++ .../20260926-dispatch-stale-head-skip.md | 23 ++ ..._codeql_scan_dispatch_workflow_contract.py | 223 +++++++++++++- ...est_opencode_review_dispatch_stale_skip.py | 278 ++++++++++++++++++ ...t_pr_review_autofix_nvidia_nim_contract.py | 2 +- 6 files changed, 639 insertions(+), 11 deletions(-) create mode 100644 CHANGELOG.d/20260926-dispatch-stale-head-skip.md create mode 100644 tests/test_opencode_review_dispatch_stale_skip.py diff --git a/.github/workflows/codeql-scan-dispatch.yml b/.github/workflows/codeql-scan-dispatch.yml index 45cfcc75fc..ea4af92b11 100644 --- a/.github/workflows/codeql-scan-dispatch.yml +++ b/.github/workflows/codeql-scan-dispatch.yml @@ -63,6 +63,7 @@ jobs: rerun_schema: ${{ steps.validate.outputs.rerun_schema }} producer_source_sha: ${{ steps.validate.outputs.producer_source_sha }} dispatch_protocol: ${{ steps.validate.outputs.dispatch_protocol }} + stale: ${{ steps.validate.outputs.stale }} steps: - name: Exchange OpenCode app token for target repository metadata reads id: metadata_read_app_token @@ -365,6 +366,64 @@ jobs: live_merge_commit_sha="$(jq -r '.merge_commit_sha // empty' <<<"$pull_request_json")" live_state="$(jq -r '.state // empty' <<<"$pull_request_json")" + # Stale-dispatch retirement. Under runner-queue saturation this run can + # start hours after the dispatched head was superseded or the pull + # request closed; the newer dispatch is itself still queued, so the + # workflow-level cancel-in-progress group cannot retire this run first. + # Only a well-formed live answer proving the dispatched head is no + # longer current ends the run as a notice. A failed `gh api` lookup has + # already exited non-zero above, and every other disagreement below + # (base, head ref, fork, malformed metadata) stays fail-closed. + # No EVENT_NAME guard is needed here, unlike opencode-review-dispatch.yml: + # this workflow is triggered only by repository_dispatch and the + # actor/sender authorization above is unconditional, so every run that + # reaches this point is an authorized dispatch. The OpenCode guard exists + # because its authorization block is itself nested under that event check. + stale_reason="" + if [ "$live_state" = "closed" ]; then + stale_reason="pull request is closed" + elif [ "$live_state" = "open" ] && + [[ "$live_head_sha" =~ ^[0-9a-fA-F]{40}$ ]] && + [[ "$SUPPLIED_HEAD_SHA" =~ ^[0-9a-fA-F]{40}$ ]] && + [ "${SUPPLIED_HEAD_SHA,,}" != "${live_head_sha,,}" ]; then + # A mismatch alone is not proof: right after a push the API can + # briefly serve the previous head, which would retire a brand-new + # dispatch. Retire only when the compare API proves the dispatched + # head is a strict ancestor of the live head (status "ahead", + # behind_by 0), i.e. the live head is newer and gets its own + # dispatch. Behind, diverged (force-push), or a failed/unexpected + # compare falls through to the fail-closed head_sha mismatch below. + head_compare_status="" + if head_compare_json="$(gh api "repos/${TARGET_REPOSITORY}/compare/${SUPPLIED_HEAD_SHA}...${live_head_sha}" 2>/dev/null)"; then + head_compare_status="$(jq -r ' + if .status == "ahead" and .behind_by == 0 and ((.ahead_by | type) == "number") and .ahead_by >= 1 + then "proven-ancestor" else ((.status // "unknown") | tostring) end + ' <<<"$head_compare_json" 2>/dev/null || true)" + fi + if [ "$head_compare_status" = "proven-ancestor" ]; then + stale_reason="pull request head moved ahead of the dispatched head" + else + printf 'Not retiring CodeQL dispatch for %s#%s as stale: dispatched head is not a proven ancestor of the live head (compare=%s).\n' "$TARGET_REPOSITORY" "$PR_NUMBER" "${head_compare_status:-unavailable}" + fi + fi + if [ -n "$stale_reason" ]; then + printf '::notice::Skipping stale CodeQL dispatch for %s#%s: %s (dispatched head=%s, live head=%s, state=%s).\n' "$TARGET_REPOSITORY" "$PR_NUMBER" "$stale_reason" "$SUPPLIED_HEAD_SHA" "${live_head_sha:-}" "$live_state" + # Backticks are literal Markdown code spans in the step summary. + # shellcheck disable=SC2016 + printf -- '- Skipped stale CodeQL dispatch for %s#%s: %s (dispatched head `%s`, live head `%s`, state `%s`).\n' "$TARGET_REPOSITORY" "$PR_NUMBER" "$stale_reason" "$SUPPLIED_HEAD_SHA" "${live_head_sha:-}" "$live_state" >>"${GITHUB_STEP_SUMMARY:-/dev/null}" + { + printf 'stale=true\n' + printf 'target_repository=%s\n' "$TARGET_REPOSITORY" + printf 'pr_number=%s\n' "$PR_NUMBER" + # Keep the matrix well-formed so the skipped scan job's + # strategy never evaluates fromJSON on an empty string. + echo "matrix<>"$GITHUB_OUTPUT" + exit 0 + fi + if [ "$live_state" != "open" ] || [ "$live_base_repository" != "$TARGET_REPOSITORY" ] || [ "$live_head_repository" != "$TARGET_REPOSITORY" ] || @@ -407,6 +466,7 @@ jobs: fi { + printf 'stale=false\n' printf 'target_repository=%s\n' "$TARGET_REPOSITORY" printf 'pr_number=%s\n' "$PR_NUMBER" printf 'base_ref=%s\n' "$live_base_ref" @@ -430,6 +490,9 @@ jobs: scan: name: CodeQL dispatch scan (${{ matrix.language }}) needs: validate-dispatch + # A stale dispatch (closed PR or moved head) ends validate-dispatch with + # a notice; skipping here also skips settle-required-run via needs. + if: needs.validate-dispatch.outputs.stale != 'true' runs-on: ubuntu-24.04 timeout-minutes: 30 permissions: diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index cbc8d21439..a13da05819 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -69,6 +69,7 @@ jobs: head_ref: ${{ steps.validate.outputs.head_ref }} head_sha: ${{ steps.validate.outputs.head_sha }} is_private: ${{ steps.validate.outputs.is_private }} + stale: ${{ steps.validate.outputs.stale }} steps: - name: Exchange OpenCode app token for target repository metadata reads id: metadata_read_app_token @@ -221,6 +222,60 @@ jobs: *) live_is_private="" ;; esac + # Stale-dispatch retirement. Under runner-queue saturation this run can + # start hours after the dispatched head was superseded or the pull + # request closed; the newer dispatch is itself still queued, so the + # workflow-level cancel-in-progress group cannot retire this run first. + # Only a well-formed live answer proving the dispatched head is no + # longer current ends the run as a notice. A failed `gh api` lookup has + # already exited non-zero above, and every other disagreement below + # (base, head ref, fork, malformed metadata) stays fail-closed. + # The EVENT_NAME guard mirrors this script's shape: the actor/sender/ + # target authorization above and the supplied-head binding below run + # only for repository_dispatch (other events bind to the live head), so + # retirement is reachable only on the authorized dispatch path. + stale_reason="" + if [ "$EVENT_NAME" != "repository_dispatch" ]; then + : + elif [ "$live_state" = "closed" ]; then + stale_reason="pull request is closed" + elif [ "$live_state" = "open" ] && + [[ "$live_head_sha" =~ ^[0-9a-fA-F]{40}$ ]] && + [[ "$SUPPLIED_HEAD_SHA" =~ ^[0-9a-fA-F]{40}$ ]] && + [ "${SUPPLIED_HEAD_SHA,,}" != "${live_head_sha,,}" ]; then + # A mismatch alone is not proof: right after a push the API can + # briefly serve the previous head, which would retire a brand-new + # dispatch. Retire only when the compare API proves the dispatched + # head is a strict ancestor of the live head (status "ahead", + # behind_by 0), i.e. the live head is newer and gets its own + # dispatch. Behind, diverged (force-push), or a failed/unexpected + # compare falls through to the fail-closed head_sha mismatch below. + head_compare_status="" + if head_compare_json="$(gh api "repos/${TARGET_REPOSITORY}/compare/${SUPPLIED_HEAD_SHA}...${live_head_sha}" 2>/dev/null)"; then + head_compare_status="$(jq -r ' + if .status == "ahead" and .behind_by == 0 and ((.ahead_by | type) == "number") and .ahead_by >= 1 + then "proven-ancestor" else ((.status // "unknown") | tostring) end + ' <<<"$head_compare_json" 2>/dev/null || true)" + fi + if [ "$head_compare_status" = "proven-ancestor" ]; then + stale_reason="pull request head moved ahead of the dispatched head" + else + printf 'Not retiring OpenCode review dispatch for %s#%s as stale: dispatched head is not a proven ancestor of the live head (compare=%s).\n' "$TARGET_REPOSITORY" "$PR_NUMBER" "${head_compare_status:-unavailable}" + fi + fi + if [ -n "$stale_reason" ]; then + printf '::notice::Skipping stale OpenCode review dispatch for %s#%s: %s (dispatched head=%s, live head=%s, state=%s).\n' "$TARGET_REPOSITORY" "$PR_NUMBER" "$stale_reason" "$SUPPLIED_HEAD_SHA" "${live_head_sha:-}" "$live_state" + # Backticks are literal Markdown code spans in the step summary. + # shellcheck disable=SC2016 + printf -- '- Skipped stale OpenCode review dispatch for %s#%s: %s (dispatched head `%s`, live head `%s`, state `%s`).\n' "$TARGET_REPOSITORY" "$PR_NUMBER" "$stale_reason" "$SUPPLIED_HEAD_SHA" "${live_head_sha:-}" "$live_state" >>"${GITHUB_STEP_SUMMARY:-/dev/null}" + { + printf 'stale=true\n' + printf 'target_repository=%s\n' "$TARGET_REPOSITORY" + printf 'pr_number=%s\n' "$PR_NUMBER" + } >>"$GITHUB_OUTPUT" + exit 0 + fi + if [ "$live_state" != "open" ] || [ "$live_base_repository" != "$TARGET_REPOSITORY" ] || ! [[ "$live_head_repository" =~ ^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$ ]] || @@ -246,6 +301,7 @@ jobs: fi { + printf 'stale=false\n' printf 'target_repository=%s\n' "$TARGET_REPOSITORY" printf 'pr_number=%s\n' "$PR_NUMBER" printf 'base_ref=%s\n' "$live_base_ref" @@ -260,6 +316,7 @@ jobs: id: coverage_read_app_token if: >- github.event_name == 'repository_dispatch' + && steps.validate.outputs.stale != 'true' && steps.validate.outputs.target_repository != '' && steps.validate.outputs.target_repository != github.repository env: @@ -327,6 +384,7 @@ jobs: } >>"$GITHUB_OUTPUT" - name: Materialize pull request merge tree for coverage measurement + if: steps.validate.outputs.stale != 'true' env: GH_TOKEN: ${{ steps.coverage_read_app_token.outputs.token || secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || github.token }} TARGET_REPOSITORY: ${{ steps.validate.outputs.target_repository }} @@ -381,6 +439,7 @@ jobs: tar -cf "$COVERAGE_SOURCE_ARCHIVE" -C "$COVERAGE_SOURCE_WORKDIR" . - name: Upload materialized pull request merge tree + if: steps.validate.outputs.stale != 'true' uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: opencode-coverage-source @@ -394,6 +453,7 @@ jobs: if: >- needs.validate-pr-metadata.result == 'success' && github.event_name == 'repository_dispatch' + && needs.validate-pr-metadata.outputs.stale != 'true' runs-on: ubuntu-24.04 timeout-minutes: 300 permissions: @@ -2334,6 +2394,7 @@ jobs: && needs.validate-pr-metadata.result == 'success' && needs.coverage-evidence.result != 'cancelled' && github.event_name == 'repository_dispatch' + && needs.validate-pr-metadata.outputs.stale != 'true' concurrency: group: >- opencode-review-${{ diff --git a/CHANGELOG.d/20260926-dispatch-stale-head-skip.md b/CHANGELOG.d/20260926-dispatch-stale-head-skip.md new file mode 100644 index 0000000000..8755306324 --- /dev/null +++ b/CHANGELOG.d/20260926-dispatch-stale-head-skip.md @@ -0,0 +1,23 @@ +### Stale CodeQL/OpenCode dispatches end as a notice instead of failing + +- `codeql-scan-dispatch.yml` (`validate-dispatch`) and `opencode-review-dispatch.yml` + (`validate-pr-metadata`) now retire a dispatch whose target pull request is closed, or whose + live head has moved strictly past the dispatched head, with a `::notice::`, a step-summary + line and a `stale=true` output. A head mismatch alone is not enough: the compare API + (`repos/{target}/compare/{dispatched}...{live}`) must answer `status: ahead` with + `behind_by: 0`, proving the live head descends from the dispatched one and therefore gets + its own dispatch. A lagging API that still serves the previous head right after a push + (`behind`), a force-push (`diverged`), or a failed or malformed compare keeps the original + fail-closed `head_sha` mismatch, so a fresh head is never skipped. The CodeQL `scan` matrix (and therefore `settle-required-run`) + and the OpenCode coverage-materialization steps, `coverage-evidence` and `opencode-review` + skip on that output, so the run concludes success instead of failure. Under runner-queue + saturation the newer dispatch for the same pull request is itself queued, so workflow-level + `cancel-in-progress` could not retire the stale run before it started. +- Still fail-closed: dispatch authorization and payload validation (checked first), a failed or + malformed live pull-request lookup, base ref/SHA or head ref disagreement at the same head, + cross-fork metadata, and the later privileged re-validation steps. Draft state is not treated + as stale because ruleset-launched required workflows do not re-dispatch on `ready_for_review`. +- The OpenCode side keeps retirement behind `EVENT_NAME == repository_dispatch` because its + dispatch authorization and supplied-head binding are nested under that check; the CodeQL + side needs no such guard because it triggers only on `repository_dispatch` and authorizes + unconditionally before the live lookup. diff --git a/tests/test_codeql_scan_dispatch_workflow_contract.py b/tests/test_codeql_scan_dispatch_workflow_contract.py index 3769f48314..119eaabe71 100644 --- a/tests/test_codeql_scan_dispatch_workflow_contract.py +++ b/tests/test_codeql_scan_dispatch_workflow_contract.py @@ -148,8 +148,12 @@ def _run_validate_step(tmp_path: Path, env_overrides: dict[str, str], pull_reque "set -euo pipefail\n" 'test "$1" = api\n' 'endpoint="${!#}"\n' + 'printf \'%s\\n\' "$endpoint" >>"$FAKE_GH_LOG"\n' 'case "$endpoint" in\n' ' repos/ContextualWisdomLab/.github/compare/*) printf \'%s\\n\' "$FAKE_SOURCE_COMPARE_JSON" ;;\n' + ' repos/ContextualWisdomLab/*/compare/*)\n' + ' if [ -z "$FAKE_HEAD_COMPARE_JSON" ]; then echo \'HTTP 404\' >&2; exit 1; fi\n' + ' printf \'%s\\n\' "$FAKE_HEAD_COMPARE_JSON" ;;\n' ' repos/ContextualWisdomLab/*/git/commits/*) printf \'%s\\n\' "$FAKE_PRODUCER_COMMIT_JSON" ;;\n' ' *) printf \'%s\\n\' "$FAKE_PULL_JSON" ;;\n' 'esac\n', @@ -163,6 +167,10 @@ def _run_validate_step(tmp_path: Path, env_overrides: dict[str, str], pull_reque "PATH": f"{fake_bin}:{os.environ['PATH']}", "FAKE_PULL_JSON": json.dumps(pull_request), "FAKE_SOURCE_COMPARE_JSON": "{}", + # Head-ancestry compare used by stale-dispatch retirement; empty means + # the compare lookup fails (HTTP 404). + "FAKE_HEAD_COMPARE_JSON": "", + "FAKE_GH_LOG": str(tmp_path / "gh-calls.log"), "FAKE_PRODUCER_COMMIT_JSON": json.dumps( { "sha": "c" * 40, @@ -941,26 +949,221 @@ def test_codeql_scan_dispatch_validate_step_rejects_unusable_legacy_payload(tmp_ assert "does not match the dispatched languages one-to-one" in invalid_job_id.stdout -def test_codeql_scan_dispatch_validate_step_rejects_stale_head_sha(tmp_path): - """A dispatch whose supplied head SHA no longer matches the live PR head is rejected.""" +def _assert_stale_skip(tmp_path: Path, result: subprocess.CompletedProcess[str], reason: str) -> None: + """A stale dispatch ends validate-dispatch successfully with a notice and skip output.""" + assert result.returncode == 0, result.stdout + result.stderr + assert "::notice::Skipping stale CodeQL dispatch for ContextualWisdomLab/naruon#42" in result.stdout + assert reason in result.stdout + assert "::error::" not in result.stdout + outputs = result.output_path.read_text(encoding="utf-8") # type: ignore[attr-defined] + assert "stale=true\n" in outputs + assert "stale=false" not in outputs + # The skipped scan job still receives a well-formed matrix, but no head + # identity is published for privileged steps to consume. + assert "matrix<busy", + "lookup_failed": "", + } + for name, compare_json in cases.items(): + moved_head = _matching_pull_request() + moved_head["head"]["sha"] = "d" * 40 + result = _run_validate_step(tmp_path / name, {"FAKE_HEAD_COMPARE_JSON": compare_json}, moved_head) + assert result.returncode == 1, (name, result.stdout, result.stderr) + assert "does not match the live pull request: head_sha" in result.stdout, name + assert "Not retiring CodeQL dispatch" in result.stdout, name + assert "::notice::Skipping stale" not in result.stdout, name + outputs_path = result.output_path # type: ignore[attr-defined] + assert not outputs_path.exists() or "stale=true" not in outputs_path.read_text(encoding="utf-8"), name + + +def test_codeql_scan_dispatch_current_head_never_calls_head_compare(tmp_path): + """The ancestry lookup only runs on a head mismatch; a current dispatch needs no extra API call.""" + result = _run_validate_step(tmp_path, {}, _matching_pull_request()) + + assert result.returncode == 0, result.stdout + result.stderr + calls = (tmp_path / "gh-calls.log").read_text(encoding="utf-8").splitlines() + assert not [call for call in calls if "/naruon/compare/" in call] + + +def test_codeql_scan_dispatch_unauthorized_dispatch_is_rejected_before_stale_detection(tmp_path): + """Retirement is reachable only after the unconditional actor/sender authorization.""" + moved_head = _matching_pull_request() + moved_head["head"]["sha"] = "d" * 40 + + result = _run_validate_step( + tmp_path, + {"DISPATCH_SENDER": "someone-else", "FAKE_HEAD_COMPARE_JSON": AHEAD_COMPARE_JSON}, + moved_head, + ) assert result.returncode == 1 - assert "does not match the live pull request: head_sha" in result.stdout + assert "repository_dispatch authorization rejected" in result.stdout + assert "::notice::Skipping stale" not in result.stdout + outputs_path = result.output_path # type: ignore[attr-defined] + assert not outputs_path.exists() or "stale=true" not in outputs_path.read_text(encoding="utf-8") + + +def test_codeql_stale_retirement_needs_no_event_guard_unlike_opencode(): + """The EVENT_NAME asymmetry with opencode-review-dispatch.yml is structural, not an omission. + + OpenCode nests its dispatch authorization under `EVENT_NAME == repository_dispatch`, + so its stale retirement carries the same guard. CodeQL is triggered only by + repository_dispatch and authorizes unconditionally before the live lookup, so + every run reaching retirement is already an authorized dispatch. If either + premise changes, this test forces the guard question to be revisited. + """ + workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + triggers = workflow.split("\non:\n", 1)[1].split("\nconcurrency:\n", 1)[0] + assert [line.strip() for line in triggers.splitlines() if line.strip()] == [ + "repository_dispatch:", + "types: [codeql-scan, codeql-scan-v2]", + ] + + script = _extract_run_block(workflow, VALIDATE_STEP_NAME) + assert "$EVENT_NAME" not in script and "github.event_name" not in script + auth_index = script.index('if [ "$actor_allowed" -ne 1 ]; then') + lookup_index = script.index('pull_request_json="$(gh api "repos/${TARGET_REPOSITORY}/pulls/${PR_NUMBER}")"') + stale_index = script.index('stale_reason=""') + assert auth_index < lookup_index < stale_index + # The authorization check sits at the top level of the script, not inside a branch. + assert script.splitlines()[script[:auth_index].count("\n")].startswith("if ") + + opencode = (REPO_ROOT / ".github/workflows/opencode-review-dispatch.yml").read_text(encoding="utf-8") + opencode_script = _extract_run_block(opencode, VALIDATE_STEP_NAME) + assert opencode_script.startswith('set -euo pipefail\nif [ "$EVENT_NAME" = "repository_dispatch" ]; then\n') + assert 'stale_reason=""\nif [ "$EVENT_NAME" != "repository_dispatch" ]; then\n :\n' in opencode_script -def test_codeql_scan_dispatch_validate_step_rejects_closed_pull_request(tmp_path): - """A dispatch targeting a pull request that closed before this run started is rejected.""" +def test_codeql_scan_dispatch_validate_step_skips_closed_pull_request(tmp_path): + """A dispatch targeting a pull request that closed before this run started is retired as a notice.""" closed_pull_request = _matching_pull_request() closed_pull_request["state"] = "closed" - result = _run_validate_step(tmp_path, {}, closed_pull_request) + result = _run_validate_step( + tmp_path, {"GITHUB_STEP_SUMMARY": str(tmp_path / "step-summary")}, closed_pull_request + ) - assert result.returncode == 1 - assert "rejected closed, missing, cross-fork, or malformed live metadata" in result.stdout + _assert_stale_skip(tmp_path, result, "pull request is closed") + + +def test_codeql_scan_dispatch_validate_step_marks_current_head_not_stale(tmp_path): + """A dispatch for the live head publishes stale=false and keeps the full identity.""" + result = _run_validate_step(tmp_path, {}, _matching_pull_request()) + + assert result.returncode == 0, result.stdout + result.stderr + outputs = result.output_path.read_text(encoding="utf-8") # type: ignore[attr-defined] + assert "stale=false\n" in outputs + assert "stale=true" not in outputs + assert f"head_sha={'b' * 40}\n" in outputs + assert "::notice::Skipping stale" not in result.stdout + + +def test_codeql_scan_dispatch_stale_skip_keeps_other_mismatches_fail_closed(tmp_path): + """Only a moved head or a closed PR is retired; every other disagreement still fails.""" + moved_base = _matching_pull_request() + moved_base["base"]["sha"] = "e" * 40 + renamed_head_ref = _matching_pull_request() + renamed_head_ref["head"]["ref"] = "other" + cross_fork = _matching_pull_request() + cross_fork["head"]["repo"]["full_name"] = "someone/naruon" + malformed_head = _matching_pull_request() + malformed_head["head"]["sha"] = "not-a-sha" + missing_state = _matching_pull_request() + del missing_state["state"] + + cases = { + "moved_base": (moved_base, "does not match the live pull request: base_sha"), + "renamed_head_ref": (renamed_head_ref, "does not match the live pull request: head_ref"), + "cross_fork": (cross_fork, "rejected closed, missing, cross-fork, or malformed live metadata"), + "malformed_head": (malformed_head, "rejected closed, missing, cross-fork, or malformed live metadata"), + "missing_state": (missing_state, "rejected closed, missing, cross-fork, or malformed live metadata"), + } + for name, (pull_request, message) in cases.items(): + result = _run_validate_step(tmp_path / name, {}, pull_request) + assert result.returncode == 1, name + assert message in result.stdout, name + assert "::notice::Skipping stale" not in result.stdout, name + outputs_path = result.output_path # type: ignore[attr-defined] + assert not outputs_path.exists() or "stale=true" not in outputs_path.read_text(encoding="utf-8"), name + + +def test_codeql_scan_dispatch_stale_skip_does_not_mask_lookup_failure(tmp_path): + """If the live pull request lookup itself fails, the run fails instead of skipping.""" + failing_bin = tmp_path / "failing-bin" + failing_bin.mkdir() + failing_gh = failing_bin / "gh" + failing_gh.write_text("#!/usr/bin/env bash\necho 'HTTP 502' >&2\nexit 1\n", encoding="utf-8") + failing_gh.chmod(0o755) + + result = _run_validate_step( + tmp_path / "run", + {"PATH": f"{failing_bin}:{os.environ['PATH']}"}, + _matching_pull_request(), + ) + + assert result.returncode != 0 + assert "::notice::Skipping stale" not in result.stdout + outputs_path = result.output_path # type: ignore[attr-defined] + assert not outputs_path.exists() or "stale=true" not in outputs_path.read_text(encoding="utf-8") + + +def test_codeql_scan_dispatch_stale_output_gates_scan_and_settlement(): + """stale=true skips the matrix scan job, which in turn skips settlement.""" + workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + validate_job = workflow.split(" validate-dispatch:\n", 1)[1].split(" steps:\n", 1)[0] + scan_header = workflow.split(" scan:\n", 1)[1].split(" strategy:\n", 1)[0] + settle_header = workflow.split(" settle-required-run:\n", 1)[1].split(" steps:\n", 1)[0] + + assert "stale: ${{ steps.validate.outputs.stale }}" in validate_job + assert "if: needs.validate-dispatch.outputs.stale != 'true'" in scan_header + assert "needs.scan.result != 'skipped'" in settle_header + assert "needs.validate-dispatch.result == 'success'" in settle_header def test_codeql_scan_dispatch_is_not_in_the_required_workflow_ruleset_scope(): diff --git a/tests/test_opencode_review_dispatch_stale_skip.py b/tests/test_opencode_review_dispatch_stale_skip.py new file mode 100644 index 0000000000..a849182772 --- /dev/null +++ b/tests/test_opencode_review_dispatch_stale_skip.py @@ -0,0 +1,278 @@ +"""Stale OpenCode review dispatches end as a notice instead of a failure. + +Under organization runner-queue saturation a repository_dispatch for an older +pull request head can start hours after the head moved or the pull request +closed. The newer dispatch is itself still queued, so the workflow-level +``cancel-in-progress`` group cannot retire the stale run first. The validate +step must then end the run with a ``::notice::`` and ``stale=true`` so the +coverage and review jobs skip, while every other metadata disagreement and any +failed lookup keeps failing closed. +""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +from pathlib import Path + +from tests.test_opencode_workflow_shell_syntax import _extract_run_block + +REPO_ROOT = Path(__file__).resolve().parents[1] +WORKFLOW_PATH = REPO_ROOT / ".github/workflows/opencode-review-dispatch.yml" +VALIDATE_STEP_NAME = "Bind workflow inputs to live organization pull request metadata" +TARGET = "ContextualWisdomLab/naruon" + + +def _live_pull_request() -> dict: + """A live PR payload matching the default supplied dispatch metadata.""" + return { + "state": "open", + "base": { + "repo": {"full_name": TARGET, "visibility": "private"}, + "ref": "main", + "sha": "a" * 40, + }, + "head": {"repo": {"full_name": TARGET}, "ref": "feature", "sha": "b" * 40}, + } + + +def _run_validate_step( + tmp_path: Path, + pull_request: dict, + env_overrides: dict[str, str] | None = None, +) -> subprocess.CompletedProcess[str]: + """Execute the real validate-pr-metadata shell block against a fake `gh api`.""" + bash = shutil.which("bash") + jq = shutil.which("jq") + assert bash is not None and jq is not None, "bash and jq are required to run this test" + + script = _extract_run_block(WORKFLOW_PATH.read_text(encoding="utf-8"), VALIDATE_STEP_NAME) + fake_bin = tmp_path / "bin" + fake_bin.mkdir(parents=True) + fake_gh = fake_bin / "gh" + fake_gh.write_text( + "#!/usr/bin/env bash\n" + "set -euo pipefail\n" + 'test "$1" = api\n' + 'endpoint="${!#}"\n' + 'printf \'%s\\n\' "$endpoint" >>"$FAKE_GH_LOG"\n' + 'case "$endpoint" in\n' + ' repos/*/compare/*)\n' + ' if [ -z "$FAKE_HEAD_COMPARE_JSON" ]; then echo \'HTTP 404\' >&2; exit 1; fi\n' + ' printf \'%s\\n\' "$FAKE_HEAD_COMPARE_JSON" ;;\n' + ' *) printf \'%s\\n\' "$FAKE_PULL_JSON" ;;\n' + 'esac\n', + encoding="utf-8", + ) + fake_gh.chmod(0o755) + + output = tmp_path / "github-output" + summary = tmp_path / "step-summary" + env = { + **os.environ, + "PATH": f"{fake_bin}:{os.environ['PATH']}", + "FAKE_PULL_JSON": json.dumps(pull_request), + # Head-ancestry compare used by stale retirement; empty = lookup fails. + "FAKE_HEAD_COMPARE_JSON": "", + "FAKE_GH_LOG": str(tmp_path / "gh-calls.log"), + "GITHUB_OUTPUT": str(output), + "GITHUB_STEP_SUMMARY": str(summary), + "EVENT_NAME": "repository_dispatch", + "DISPATCH_ACTOR": "opencode-agent[bot]", + "DISPATCH_SENDER": "opencode-agent[bot]", + "ALLOWED_DISPATCH_ACTOR": "opencode-agent[bot]", + "ALLOWED_DISPATCH_TARGETS": TARGET, + "TARGET_REPOSITORY": TARGET, + "PR_NUMBER": "42", + "SUPPLIED_BASE_REF": "main", + "SUPPLIED_BASE_SHA": "a" * 40, + "SUPPLIED_HEAD_REF": "feature", + "SUPPLIED_HEAD_SHA": "b" * 40, + **(env_overrides or {}), + } + result = subprocess.run([bash], input=script, text=True, capture_output=True, check=False, env=env) + result.output_path = output # type: ignore[attr-defined] + result.summary_path = summary # type: ignore[attr-defined] + return result + + +def _outputs(result: subprocess.CompletedProcess[str]) -> str: + path = result.output_path # type: ignore[attr-defined] + return path.read_text(encoding="utf-8") if path.exists() else "" + + +def _assert_stale_skip(result: subprocess.CompletedProcess[str], reason: str) -> None: + assert result.returncode == 0, result.stdout + result.stderr + assert f"::notice::Skipping stale OpenCode review dispatch for {TARGET}#42" in result.stdout + assert reason in result.stdout + assert "::error::" not in result.stdout + outputs = _outputs(result) + assert "stale=true\n" in outputs + assert "stale=false" not in outputs + assert "head_sha=" not in outputs + assert "base_sha=" not in outputs + summary = result.summary_path.read_text(encoding="utf-8") # type: ignore[attr-defined] + assert f"Skipped stale OpenCode review dispatch for {TARGET}#42" in summary + + +def test_current_head_dispatch_is_not_stale(tmp_path): + result = _run_validate_step(tmp_path, _live_pull_request()) + + assert result.returncode == 0, result.stdout + result.stderr + outputs = _outputs(result) + assert "stale=false\n" in outputs + assert f"head_sha={'b' * 40}\n" in outputs + assert "::notice::Skipping stale" not in result.stdout + + +AHEAD_COMPARE_JSON = json.dumps({"status": "ahead", "ahead_by": 2, "behind_by": 0}) + + +def test_moved_head_dispatch_is_skipped_with_notice(tmp_path): + pull_request = _live_pull_request() + pull_request["head"]["sha"] = "c" * 40 + + result = _run_validate_step(tmp_path, pull_request, {"FAKE_HEAD_COMPARE_JSON": AHEAD_COMPARE_JSON}) + + _assert_stale_skip(result, "pull request head moved ahead of the dispatched head") + assert f"dispatched head={'b' * 40}, live head={'c' * 40}" in result.stdout + # dispatched...live order: "ahead" means the live head descends from the dispatched head. + calls = (tmp_path / "gh-calls.log").read_text(encoding="utf-8").splitlines() + assert f"repos/{TARGET}/compare/{'b' * 40}...{'c' * 40}" in calls + + +def test_head_mismatch_without_proven_ancestry_stays_fail_closed(tmp_path): + """Only a live head that provably descends from the dispatched head retires the run. + + API lag right after a push (live=previous head, compare "behind"), force-push + ("diverged"), and any failed or contradictory compare answer keep the original + fail-closed head_sha mismatch, so a fresh head is never silently skipped. + """ + cases = { + "api_lag_behind": json.dumps({"status": "behind", "ahead_by": 0, "behind_by": 1}), + "force_push_diverged": json.dumps({"status": "diverged", "ahead_by": 1, "behind_by": 3}), + "ahead_but_behind_by": json.dumps({"status": "ahead", "ahead_by": 1, "behind_by": 1}), + "ahead_without_counts": json.dumps({"status": "ahead"}), + "not_json": "busy", + "lookup_failed": "", + } + for name, compare_json in cases.items(): + pull_request = _live_pull_request() + pull_request["head"]["sha"] = "c" * 40 + result = _run_validate_step(tmp_path / name, pull_request, {"FAKE_HEAD_COMPARE_JSON": compare_json}) + assert result.returncode == 1, (name, result.stdout, result.stderr) + assert "does not match the live pull request: head_sha" in result.stdout, name + assert "Not retiring OpenCode review dispatch" in result.stdout, name + assert "::notice::Skipping stale" not in result.stdout, name + assert "stale=true" not in _outputs(result), name + + +def test_non_dispatch_event_is_never_retired(tmp_path): + """The EVENT_NAME guard mirrors the script's dispatch-only authorization and binding. + + For any other event the step never authorizes a dispatcher and ignores the + supplied head, binding to the live head instead, so there is no dispatched head + that could be stale. A moved supplied head must not retire the run, and a + closed pull request keeps failing closed. + """ + moved_head = _live_pull_request() + moved_head["head"]["sha"] = "c" * 40 + override = {"EVENT_NAME": "workflow_dispatch", "FAKE_HEAD_COMPARE_JSON": AHEAD_COMPARE_JSON} + + result = _run_validate_step(tmp_path / "moved_head", moved_head, override) + assert result.returncode == 0, result.stdout + result.stderr + assert "::notice::Skipping stale" not in result.stdout + outputs = _outputs(result) + assert "stale=false\n" in outputs and "stale=true" not in outputs + assert f"head_sha={'c' * 40}\n" in outputs + + closed = _live_pull_request() + closed["state"] = "closed" + result = _run_validate_step(tmp_path / "closed", closed, override) + assert result.returncode == 1 + assert "rejected closed, missing, or malformed live metadata" in result.stdout + assert "::notice::Skipping stale" not in result.stdout + assert "stale=true" not in _outputs(result) + + +def test_closed_pull_request_dispatch_is_skipped_with_notice(tmp_path): + pull_request = _live_pull_request() + pull_request["state"] = "closed" + + result = _run_validate_step(tmp_path, pull_request) + + _assert_stale_skip(result, "pull request is closed") + + +def test_other_metadata_disagreements_stay_fail_closed(tmp_path): + moved_base = _live_pull_request() + moved_base["base"]["sha"] = "e" * 40 + renamed_head_ref = _live_pull_request() + renamed_head_ref["head"]["ref"] = "other" + malformed_head = _live_pull_request() + malformed_head["head"]["sha"] = "not-a-sha" + missing_state = _live_pull_request() + del missing_state["state"] + + cases = { + "moved_base": (moved_base, "does not match the live pull request: base_sha"), + "renamed_head_ref": (renamed_head_ref, "does not match the live pull request: head_ref"), + "malformed_head": (malformed_head, "rejected closed, missing, or malformed live metadata"), + "missing_state": (missing_state, "rejected closed, missing, or malformed live metadata"), + } + for name, (pull_request, message) in cases.items(): + result = _run_validate_step(tmp_path / name, pull_request) + assert result.returncode == 1, name + assert message in result.stdout, name + assert "::notice::Skipping stale" not in result.stdout, name + assert "stale=true" not in _outputs(result), name + + +def test_unauthorized_dispatch_is_rejected_before_stale_detection(tmp_path): + pull_request = _live_pull_request() + pull_request["head"]["sha"] = "c" * 40 + + result = _run_validate_step( + tmp_path, + pull_request, + {"DISPATCH_SENDER": "someone-else", "FAKE_HEAD_COMPARE_JSON": AHEAD_COMPARE_JSON}, + ) + + assert result.returncode == 1 + assert "repository_dispatch authorization rejected" in result.stdout + assert "stale=true" not in _outputs(result) + + +def test_failed_lookup_is_not_treated_as_stale(tmp_path): + failing_bin = tmp_path / "failing-bin" + failing_bin.mkdir() + failing_gh = failing_bin / "gh" + failing_gh.write_text("#!/usr/bin/env bash\necho 'HTTP 502' >&2\nexit 1\n", encoding="utf-8") + failing_gh.chmod(0o755) + + result = _run_validate_step( + tmp_path / "run", + _live_pull_request(), + {"PATH": f"{failing_bin}:{os.environ['PATH']}"}, + ) + + assert result.returncode != 0 + assert "::notice::Skipping stale" not in result.stdout + assert "stale=true" not in _outputs(result) + + +def test_stale_output_gates_every_later_step_and_job(): + workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + metadata_job = workflow.split(" validate-pr-metadata:\n", 1)[1].split("\n coverage-evidence:\n", 1)[0] + coverage_header = workflow.split("\n coverage-evidence:\n", 1)[1].split(" runs-on:", 1)[0] + review_header = workflow.split("\n opencode-review-target:\n", 1)[1].split(" concurrency:", 1)[0] + + assert "stale: ${{ steps.validate.outputs.stale }}" in metadata_job + later_steps = metadata_job.split(f" - name: {VALIDATE_STEP_NAME}\n", 1)[1].split("\n - name: ")[1:] + assert later_steps, "validate must be followed by the coverage materialization steps" + for step in later_steps: + assert "steps.validate.outputs.stale != 'true'" in step, step.splitlines()[0] + assert "needs.validate-pr-metadata.outputs.stale != 'true'" in coverage_header + assert "needs.validate-pr-metadata.outputs.stale != 'true'" in review_header diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 8b7c55a4ef..0925f6115f 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -17,7 +17,7 @@ DOCTORING_RECORD = Path("docs/doctoring/hourly-nvidia-nim-autofix.md") CHANGELOG = Path("CHANGELOG.md") REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") -REVIEW_DISPATCH_BLOB_SHA = "cbc8d214394c4b7acbe82ce7fba11fd073b91c98" +REVIEW_DISPATCH_BLOB_SHA = "a13da05819c6c60f77ea72e67e982f439adb00f5" def _workflow_text(path: Path) -> str: From 14fe2b65f2676c91cf4ccca9b08128304b0af92e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 26 Sep 2026 15:39:57 +0900 Subject: [PATCH 19/20] fix(codeql): unblock CodeQL scan dispatch for all .github PRs - Select a target-scoped credential that can actually read code-scanning analyses before the GHAS base/head configuration identity check, instead of the first non-empty token (the OpenCode app token returns 403). Ported from #2275. - Replace the set-membership URL assertion flagged by CodeQL py/incomplete-url-substring-sanitization with an issubset check. Ported from #2351. --- .github/workflows/codeql-scan-dispatch.yml | 47 +++++++- ..._scan_dispatch_ghas_credential_contract.py | 111 ++++++++++++++++++ ...mercial_readiness_loop_receipt_contract.py | 8 +- 3 files changed, 162 insertions(+), 4 deletions(-) create mode 100644 tests/test_codeql_scan_dispatch_ghas_credential_contract.py diff --git a/.github/workflows/codeql-scan-dispatch.yml b/.github/workflows/codeql-scan-dispatch.yml index 45cfcc75fc..49873c2987 100644 --- a/.github/workflows/codeql-scan-dispatch.yml +++ b/.github/workflows/codeql-scan-dispatch.yml @@ -587,11 +587,56 @@ jobs: id: gate run: python3 "$RUNNER_TEMP/codeql_sarif_gate.py" codeql-results-dispatch + - name: Select target CodeQL analysis-read credential + id: ghas_analysis_token + if: steps.gate.outcome == 'success' + env: + TARGET_REPOSITORY: ${{ needs.validate-dispatch.outputs.target_repository }} + TARGET_APP_TOKEN: ${{ steps.target_app_token.outputs.token || '' }} + PR_REVIEW_MERGE_TOKEN: ${{ secrets.PR_REVIEW_MERGE_TOKEN || '' }} + OPENCODE_APPROVE_TOKEN: ${{ secrets.OPENCODE_APPROVE_TOKEN || '' }} + WORKFLOW_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + + probe_analysis_read() { + token_label="$1" + token="$2" + if [ -z "$token" ]; then + return 1 + fi + if GH_TOKEN="$token" gh api \ + -H "Accept: application/vnd.github+json" \ + -H "X-GitHub-Api-Version: 2022-11-28" \ + "repos/${TARGET_REPOSITORY}/code-scanning/analyses?per_page=1&tool_name=CodeQL" \ + >/dev/null 2>&1; then + echo "::add-mask::$token" + { + printf 'token=%s\n' "$token" + printf 'source=%s\n' "$token_label" + } >>"$GITHUB_OUTPUT" + echo "Selected ${token_label} after proving target CodeQL analysis-read access." + return 0 + fi + echo "::notice::${token_label} cannot read target CodeQL analyses; trying the next configured credential." + return 1 + } + + if probe_analysis_read "target-app-token" "$TARGET_APP_TOKEN" || + probe_analysis_read "pr-review-merge-token" "$PR_REVIEW_MERGE_TOKEN" || + probe_analysis_read "opencode-approve-token" "$OPENCODE_APPROVE_TOKEN" || + probe_analysis_read "github-token" "$WORKFLOW_TOKEN"; then + exit 0 + fi + + echo "::error::no configured credential can read target CodeQL analyses; GHAS configuration identity cannot be proven." + exit 1 + - name: Verify GHAS base/head CodeQL configuration identity id: ghas_configuration_identity if: steps.gate.outcome == 'success' env: - GH_TOKEN: ${{ steps.target_app_token.outputs.token || secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || github.token }} + GH_TOKEN: ${{ steps.ghas_analysis_token.outputs.token }} TARGET_REPOSITORY: ${{ needs.validate-dispatch.outputs.target_repository }} PR_NUMBER: ${{ needs.validate-dispatch.outputs.pr_number }} BASE_REF: ${{ needs.validate-dispatch.outputs.base_ref }} diff --git a/tests/test_codeql_scan_dispatch_ghas_credential_contract.py b/tests/test_codeql_scan_dispatch_ghas_credential_contract.py new file mode 100644 index 0000000000..d92bf3ef2c --- /dev/null +++ b/tests/test_codeql_scan_dispatch_ghas_credential_contract.py @@ -0,0 +1,111 @@ +"""Credential-routing contract for cross-repository GHAS CodeQL analysis reads.""" + +from __future__ import annotations + +import os +import subprocess +from pathlib import Path + +from tests.test_opencode_workflow_shell_syntax import _extract_run_block + + +REPO_ROOT = Path(__file__).resolve().parents[1] +WORKFLOW_PATH = REPO_ROOT / ".github/workflows/codeql-scan-dispatch.yml" +SELECT_STEP_NAME = "Select target CodeQL analysis-read credential" +VERIFY_STEP_NAME = "Verify GHAS base/head CodeQL configuration identity" + + +def _run_selector(tmp_path: Path, *, succeeding_token: str | None) -> subprocess.CompletedProcess[str]: + """Execute the extracted selector with fixed Bash identity and a fake ``gh`` boundary.""" + assert Path("/bin/bash").is_file(), "/bin/bash is required to run this workflow-contract test" + + workflow_text = WORKFLOW_PATH.read_text(encoding="utf-8") + script = _extract_run_block(workflow_text, SELECT_STEP_NAME) + + fake_bin = tmp_path / "bin" + fake_bin.mkdir(parents=True) + call_log = tmp_path / "calls" + fake_gh = fake_bin / "gh" + fake_gh.write_text( + "#!/usr/bin/env bash\n" + "set -euo pipefail\n" + 'printf \'%s\\n\' "${GH_TOKEN:-}" >>"$FAKE_CALL_LOG"\n' + 'test "$1" = api\n' + 'test "$#" -eq 6\n' + 'test "$2" = -H\n' + 'test "$3" = "Accept: application/vnd.github+json"\n' + 'test "$4" = -H\n' + 'test "$5" = "X-GitHub-Api-Version: 2022-11-28"\n' + 'test "$6" = "repos/ContextualWisdomLab/OriginWeave/code-scanning/analyses?per_page=1&tool_name=CodeQL"\n' + 'if [ -n "${SUCCEEDING_TOKEN:-}" ] && [ "${GH_TOKEN:-}" = "$SUCCEEDING_TOKEN" ]; then\n' + " printf '[]\\n'\n" + " exit 0\n" + "fi\n" + "exit 1\n", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + + output = tmp_path / "github-output" + env = { + **os.environ, + "PATH": f"{fake_bin}:{os.environ['PATH']}", + "GITHUB_OUTPUT": str(output), + "FAKE_CALL_LOG": str(call_log), + "SUCCEEDING_TOKEN": succeeding_token or "", + "TARGET_REPOSITORY": "ContextualWisdomLab/OriginWeave", + "TARGET_APP_TOKEN": "content-token", + "PR_REVIEW_MERGE_TOKEN": "security-token", + "OPENCODE_APPROVE_TOKEN": "approve-token", + "WORKFLOW_TOKEN": "workflow-token", + } + result = subprocess.run( + ["/bin/bash"], + input=script, + text=True, + capture_output=True, + check=False, + env=env, + ) + result.output_path = output # type: ignore[attr-defined] + result.call_log = call_log # type: ignore[attr-defined] + return result + + +def test_ghas_analysis_read_falls_through_content_only_target_app_token(tmp_path: Path) -> None: + """A content-capable app token must not mask a later GHAS-capable credential.""" + result = _run_selector(tmp_path, succeeding_token="security-token") + + assert result.returncode == 0, result.stdout + result.stderr + output = result.output_path.read_text(encoding="utf-8") + assert "token=security-token" in output + assert "source=pr-review-merge-token" in output + assert result.call_log.read_text(encoding="utf-8").splitlines() == [ + "content-token", + "security-token", + ] + + +def test_ghas_analysis_read_fails_closed_when_no_candidate_can_read_target(tmp_path: Path) -> None: + """Missing target code-scanning read authority must remain a hard prerequisite failure.""" + result = _run_selector(tmp_path, succeeding_token=None) + + assert result.returncode != 0 + assert "no configured credential can read target CodeQL analyses" in result.stdout + assert result.call_log.read_text(encoding="utf-8").splitlines() == [ + "content-token", + "security-token", + "approve-token", + "workflow-token", + ] + + +def test_ghas_identity_step_consumes_only_probed_analysis_read_token() -> None: + """The identity proof must not repeat the unprobed content-token precedence chain.""" + workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + verify_script = _extract_run_block(workflow, VERIFY_STEP_NAME) + verify_prefix = workflow.split(f" - name: {VERIFY_STEP_NAME}\n", 1)[1].split(" run: |", 1)[0] + + assert "GH_TOKEN: ${{ steps.ghas_analysis_token.outputs.token }}" in verify_prefix + assert "steps.target_app_token.outputs.token ||" not in verify_prefix + assert "codeql_ghas_configuration_identity.py" in verify_script diff --git a/tests/test_organization_commercial_readiness_loop_receipt_contract.py b/tests/test_organization_commercial_readiness_loop_receipt_contract.py index 6ae9dfa595..0a7bde07a4 100644 --- a/tests/test_organization_commercial_readiness_loop_receipt_contract.py +++ b/tests/test_organization_commercial_readiness_loop_receipt_contract.py @@ -57,7 +57,9 @@ def test_json_receipt_is_retained_as_an_immutable_short_lived_artifact() -> None assert "if-no-files-found: error" in source assert "retention-days: 3" in source endpoints = _harden_runner_allowed_endpoints(source) - assert "results-receiver.actions.githubusercontent.com:443" in endpoints - assert "*.actions.githubusercontent.com:443" in endpoints - assert "*.blob.core.windows.net:443" in endpoints + assert { + "results-receiver.actions.githubusercontent.com:443", + "*.actions.githubusercontent.com:443", + "*.blob.core.windows.net:443", + }.issubset(endpoints) assert "- name: Checkout exact trusted coordinator source" not in endpoints From 8e1aba9ce1d52acb36f095da32511da06958733f Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 26 Sep 2026 18:34:55 +0900 Subject: [PATCH 20/20] fix(ci): bind AnyIO security pin to Strix input --- docs/product-technical-gap-baseline.md | 2 +- requirements-strix-ci.txt | 1 + tests/test_strix_runtime_dependencies.py | 13 +++++++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 2bae6f6601..1ac3859a54 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -11,7 +11,7 @@ | Gap ID | 상태 | exact-head evidence | causal owner / next gate | |---|---|---|---| -| CONTROL-OPENCODE-COVERAGE-LOCK-CONTEXT-01 | **Proposed — RED/GREEN source repair prepared on `.github#2286`; hosted acceptance pending** | Required OpenCode run `35370902053`의 `coverage-evidence` job `105778600365`은 PR source 실행 전에 `COPY requirements-opencode-review-ci-hashes.txt requirements-noema-document-ci-hashes.txt /tmp/`에서 두 번째 파일을 찾지 못해 종료했다. RED `9b9f5edcd`는 Dockerfile의 모든 lock input이 trusted build context에 존재해야 한다는 계약을 고정했다. | Canonical owner는 중앙 `.github/.github/workflows/opencode-review-dispatch.yml`이다. 두 lockfile을 각각 regular non-symlink로 검증하고 build context로 복사한 뒤 exact-head focused/full suite와 새 hosted `coverage-evidence`를 통과해야 한다. PR 제품 source나 coverage 비율의 결함으로 오인하지 않으며 synthetic status·manual rerun·bypass를 사용하지 않는다. | +| CONTROL-OPENCODE-COVERAGE-LOCK-CONTEXT-01 | **Proposed — PR-bound incident register; GitHub Project #1 roadmap item이 아님; `.github#2385@950ab885…` source convergence, hosted acceptance pending** | Required OpenCode run `35370902053`의 `coverage-evidence` job `105778600365`은 PR source 실행 전에 `COPY requirements-opencode-review-ci-hashes.txt requirements-noema-document-ci-hashes.txt /tmp/`에서 두 번째 파일을 찾지 못해 종료했다. RED `9b9f5edcd`는 Dockerfile의 모든 lock input이 trusted build context에 존재해야 한다는 계약을 고정했다. 이 행은 live Project 상태를 주장하지 않고 exact-head PR evidence만 추적하며, protected integration 뒤 제거 여부를 재평가한다. | Canonical owner는 중앙 `.github/.github/workflows/opencode-review-dispatch.yml`이고 complete successor는 `.github#2385`이다. 두 lockfile을 각각 regular non-symlink로 검증하고 build context로 복사한 뒤 exact-head focused/full suite와 새 hosted `coverage-evidence`를 통과해야 한다. PR 제품 source나 coverage 비율의 결함으로 오인하지 않으며 synthetic status·manual rerun·bypass를 사용하지 않는다. | ### 2026-09-13 current-head incident delta diff --git a/requirements-strix-ci.txt b/requirements-strix-ci.txt index 19093441e9..50e8a05f9b 100644 --- a/requirements-strix-ci.txt +++ b/requirements-strix-ci.txt @@ -1,4 +1,5 @@ strix-agent==1.5.3 +anyio==4.14.2 openai[httpx2]==2.54.0 aiohttp==3.14.3 google-cloud-aiplatform==1.133.0 diff --git a/tests/test_strix_runtime_dependencies.py b/tests/test_strix_runtime_dependencies.py index fd66f8d452..5e67142958 100644 --- a/tests/test_strix_runtime_dependencies.py +++ b/tests/test_strix_runtime_dependencies.py @@ -15,3 +15,16 @@ def test_strix_installs_openai_httpx2_runtime() -> None: assert "openai[httpx2]==2.54.0" in requirements.splitlines() assert "openai==2.54.0 \\" in requirements_lock.splitlines() assert "httpx2==2.12.0 \\" in requirements_lock.splitlines() + + +def test_strix_anyio_security_pin_is_an_explicit_lock_input() -> None: + """Keep the audited AnyIO version reproducible from the source input.""" + requirements = (REPOSITORY_ROOT / "requirements-strix-ci.txt").read_text( + encoding="utf-8" + ) + requirements_lock = ( + REPOSITORY_ROOT / "requirements-strix-ci-hashes.txt" + ).read_text(encoding="utf-8") + + assert "anyio==4.14.2" in requirements.splitlines() + assert "anyio==4.14.2 \\" in requirements_lock.splitlines()