From 5c8fc18d41b756f74add06dd6613f233845f228d Mon Sep 17 00:00:00 2001 From: Roman Lutz Date: Sat, 10 Oct 2026 23:44:45 -0700 Subject: [PATCH 1/2] CI Retire obsolete CodeQL merge-group uploads Separate analysis from publishing, preserve failed-scan diagnostics, and cancel only positively verified obsolete merge candidates. Keep native CodeQL publishing for non-merge events and cover the upload race with offline regression checks. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/workflows/codeql.yml | 36 +- build_scripts/codeql_merge_group.py | 309 +++++++++ doc/contributing/4_running_tests.md | 23 + .../build_scripts/test_codeql_merge_group.py | 654 ++++++++++++++++++ 4 files changed, 1021 insertions(+), 1 deletion(-) create mode 100644 build_scripts/codeql_merge_group.py create mode 100644 tests/unit/build_scripts/test_codeql_merge_group.py diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 83d70e9ed7..b4d8479b94 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -21,7 +21,8 @@ jobs: name: CodeQL (${{ matrix.language }}) runs-on: ubuntu-latest permissions: - actions: read + # Merge-group uploads can cancel this run after verifying that its candidate expired. + actions: write contents: read security-events: write strategy: @@ -32,6 +33,7 @@ jobs: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: + ref: ${{ github.sha }} persist-credentials: false - name: Initialize CodeQL @@ -41,6 +43,38 @@ jobs: build-mode: none - name: Perform CodeQL Analysis + id: analysis uses: github/codeql-action/analyze@2892aa5e19bbd11bc0cff5427e3b750a04d9e3c2 # v4.38.2 with: category: "/language:${{ matrix.language }}" + # A literal value is required by the action's failed-analysis diagnostic recovery. + upload: failure-only + post-processed-sarif-path: ${{ runner.temp }}/codeql-upload + + - name: Publish CodeQL results + if: github.event_name != 'merge_group' + uses: github/codeql-action/upload-sarif@2892aa5e19bbd11bc0cff5427e3b750a04d9e3c2 # v4.38.2 + with: + sarif_file: ${{ runner.temp }}/codeql-upload/upload.sarif + category: "/language:${{ matrix.language }}" + + - name: Set up merge-group upload helper + if: github.event_name == 'merge_group' + uses: astral-sh/setup-uv@c18668ad3cf93ea998bef934396af7bb5c839dc7 # v10.2.0 + with: + version: "0.9.17" + enable-cache: false + + - name: Publish merge-group CodeQL results + if: github.event_name == 'merge_group' + timeout-minutes: 8 + env: + GITHUB_TOKEN: ${{ github.token }} + ANALYSIS_OUTCOME: ${{ steps.analysis.outcome }} + SARIF_FILE: ${{ runner.temp }}/codeql-upload/upload.sarif + SARIF_CATEGORY: "/language:${{ matrix.language }}" + run: | + uv run --no-project python -m build_scripts.codeql_merge_group \ + --analysis-outcome "$ANALYSIS_OUTCOME" \ + --sarif-file "$SARIF_FILE" \ + --category "$SARIF_CATEGORY" diff --git a/build_scripts/codeql_merge_group.py b/build_scripts/codeql_merge_group.py new file mode 100644 index 0000000000..503b299a77 --- /dev/null +++ b/build_scripts/codeql_merge_group.py @@ -0,0 +1,309 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT license. +"""Publish prepared CodeQL SARIF or cancel a positively verified obsolete merge group.""" + +from __future__ import annotations + +import argparse +import base64 +import gzip +import http.client +import json +import os +import re +import sys +import time +from dataclasses import dataclass +from pathlib import Path +from typing import TYPE_CHECKING, Any, NoReturn +from urllib.parse import quote, urlsplit + +if TYPE_CHECKING: + from collections.abc import Mapping + + +class GitHubAPIError(RuntimeError): + def __init__(self, *, status: int, message: str, errors: Any = None) -> None: + super().__init__(f"GitHub API returned HTTP {status}: {message}") + self.status = status + self.message = message + self.errors = errors + + +class WorkflowCancelledError(RuntimeError): + pass + + +class GitHubAPI: + TIMEOUT_SECONDS = 30 + + def __init__(self, *, api_url: str, token: str) -> None: + parsed = urlsplit(api_url) + if ( + parsed.scheme != "https" + or not parsed.hostname + or parsed.username + or parsed.password + or parsed.query + or parsed.fragment + or not token + ): + raise ValueError("A HTTPS GitHub API URL and a nonempty Actions token are required") + self._host = parsed.netloc + self._prefix = parsed.path.rstrip("/") + self._token = token + + def request(self, *, method: str, path: str, expected_status: int, payload: dict[str, Any] | None = None) -> Any: + connection = http.client.HTTPSConnection(self._host, timeout=self.TIMEOUT_SECONDS) + headers = { + "Accept": "application/vnd.github+json", + "Authorization": f"Bearer {self._token}", + "Content-Type": "application/json", + "User-Agent": "PyRIT-CodeQL-merge-group", + "X-GitHub-Api-Version": "2022-11-28", + } + try: + connection.request( + method, self._prefix + path, body=json.dumps(payload) if payload else None, headers=headers + ) + response = connection.getresponse() + status, raw = response.status, response.read() + finally: + connection.close() + try: + body = json.loads(raw) if raw else {} + except json.JSONDecodeError as error: + raise GitHubAPIError(status=status, message="Invalid JSON response") from error + if status != expected_status: + message = body.get("message") if isinstance(body, dict) else None + raise GitHubAPIError( + status=status, + message=message if isinstance(message, str) else "Unexpected response", + errors=body.get("errors") if isinstance(body, dict) else None, + ) + return body + + +@dataclass(frozen=True, kw_only=True) +class MergeGroup: + repository: str + ref: str + sha: str + run_id: int + run_attempt: int + + @property + def api_path(self) -> str: + return f"/repos/{self.repository}" + + +def load_merge_group(environment: Mapping[str, str]) -> MergeGroup: + if environment["GITHUB_EVENT_NAME"] != "merge_group": + raise ValueError("This publisher only supports merge_group events") + event = json.loads(Path(environment["GITHUB_EVENT_PATH"]).read_text(encoding="utf-8")) + if not isinstance(event, dict) or not isinstance(event.get("merge_group"), dict): + raise ValueError("Expected a merge-group event object") + group = event["merge_group"] + ref, sha, repository = environment["GITHUB_REF"], environment["GITHUB_SHA"], environment["GITHUB_REPOSITORY"] + base_ref = group["base_ref"] + if not isinstance(base_ref, str) or not base_ref.startswith("refs/heads/"): + raise ValueError("The merge group's base must be a branch ref") + prefix = f"refs/heads/gh-readonly-queue/{base_ref.removeprefix('refs/heads/')}/" + if event["action"] != "checks_requested" or not ref.startswith(prefix) or len(ref) <= len(prefix): + raise ValueError("Unsupported merge-group action or queue ref") + if group["head_ref"] != ref or group["head_sha"] != sha or event["repository"]["full_name"] != repository: + raise ValueError("The merge-group event does not match the tested repository, ref and SHA") + if not re.fullmatch(r"[0-9a-f]{40}", sha) or not re.fullmatch(r"[\w.-]+/[\w.-]+", repository, flags=re.ASCII): + raise ValueError("A full tested SHA and owner/repository name are required") + run_id, run_attempt = environment["GITHUB_RUN_ID"], environment["GITHUB_RUN_ATTEMPT"] + if not re.fullmatch(r"[1-9][0-9]*", run_id) or not re.fullmatch(r"[1-9][0-9]*", run_attempt): + raise ValueError("The current workflow run ID and attempt must be positive integers") + return MergeGroup(repository=repository, ref=ref, sha=sha, run_id=int(run_id), run_attempt=int(run_attempt)) + + +def _current_sha(*, api: GitHubAPI, group: MergeGroup) -> str | None: + ref = quote(group.ref.removeprefix("refs/"), safe="/") + refs = api.request(method="GET", path=f"{group.api_path}/git/matching-refs/{ref}", expected_status=200) + if not isinstance(refs, list) or any( + not isinstance(item, dict) or not isinstance(item.get("ref"), str) for item in refs + ): + raise ValueError("Invalid matching-refs response") + matches = [item for item in refs if item["ref"] == group.ref] + if not matches: + return None + if len(matches) != 1: + raise ValueError("Matching-refs returned more than one exact queue ref") + target = matches[0].get("object") + if not isinstance(target, dict): + raise ValueError("The queue ref has no valid commit object") + sha = target.get("sha") + if target.get("type") != "commit" or not isinstance(sha, str) or not re.fullmatch(r"[0-9a-f]{40}", sha): + raise ValueError("The queue ref does not point to a valid commit") + return sha + + +def _verify_run(*, api: GitHubAPI, group: MergeGroup) -> None: + run = api.request(method="GET", path=f"{group.api_path}/actions/runs/{group.run_id}", expected_status=200) + expected = { + "id": group.run_id, + "run_attempt": group.run_attempt, + "event": "merge_group", + "head_sha": group.sha, + "head_branch": group.ref.removeprefix("refs/heads/"), + "path": ".github/workflows/codeql.yml", + } + if not isinstance(run, dict) or any(run.get(key) != value for key, value in expected.items()): + raise ValueError("Refusing to cancel a different workflow run or attempt") + if run.get("status") == "completed" and run.get("conclusion") == "cancelled": + raise WorkflowCancelledError("The verified CodeQL workflow run is already cancelled") + if run.get("status") != "in_progress" or run.get("conclusion") is not None: + raise ValueError("Refusing to cancel a workflow run that is not in progress") + + +def _verify_no_failed_jobs(*, api: GitHubAPI, group: MergeGroup) -> None: + path = f"{group.api_path}/actions/runs/{group.run_id}/attempts/{group.run_attempt}/jobs?per_page=100" + result = api.request(method="GET", path=path, expected_status=200) + if not isinstance(result, dict) or not isinstance(result.get("jobs"), list): + raise ValueError("Invalid workflow jobs response") + jobs = result["jobs"] + if not jobs or result.get("total_count") != len(jobs): + raise ValueError("Cannot verify all CodeQL matrix jobs before cancellation") + for job in jobs: + if ( + not isinstance(job, dict) + or not isinstance(job.get("steps"), list) + or any(not isinstance(step, dict) for step in job["steps"]) + ): + raise ValueError("Invalid workflow job steps") + conclusions = [job.get("conclusion"), *(step.get("conclusion") for step in job["steps"])] + if any(value not in (None, "success", "skipped", "cancelled") for value in conclusions): + raise RuntimeError("A CodeQL matrix job or step failed; refusing to replace failure with cancellation") + + +def _retire(*, api: GitHubAPI, group: MergeGroup, current_sha: str | None) -> NoReturn: + _verify_run(api=api, group=group) + _verify_no_failed_jobs(api=api, group=group) + reason = "is absent" if current_sha is None else f"now points to {current_sha}" + print( + f"::notice::Verified obsolete merge candidate: {group.ref} {reason}; cancelling run {group.run_id}", flush=True + ) + try: + api.request(method="POST", path=f"{group.api_path}/actions/runs/{group.run_id}/cancel", expected_status=202) + except GitHubAPIError as error: + if error.status == 409: + _verify_run(api=api, group=group) + raise + # Stay in a cancellable step until the runner receives cancellation. Never return a green check. + time.sleep(60) + raise TimeoutError("Cancellation was accepted, but the runner did not stop within 60 seconds") + + +def _ensure_live(*, api: GitHubAPI, group: MergeGroup) -> None: + current_sha = _current_sha(api=api, group=group) + if current_sha != group.sha: + _retire(api=api, group=group, current_sha=current_sha) + + +def _missing_ref_message(group: MergeGroup) -> str: + return f"ref '{group.ref}' not found in this repository" + + +def _wait_for_processing(*, api: GitHubAPI, group: MergeGroup, sarif_id: str) -> None: + for delay in (5, 10, 20, 40, 80): + time.sleep(delay) + path = f"{group.api_path}/code-scanning/sarifs/{quote(sarif_id, safe='')}" + result = api.request(method="GET", path=path, expected_status=200) + if not isinstance(result, dict): + raise ValueError("Invalid SARIF processing response") + status = result.get("processing_status") + if status == "complete": + return + if status == "failed": + errors = result.get("errors") + if errors == [_missing_ref_message(group)]: + _ensure_live(api=api, group=group) + raise RuntimeError(f"SARIF processing failed: {errors}") + if status != "pending": + raise ValueError(f"Unsupported SARIF processing status: {status}") + _ensure_live(api=api, group=group) + raise TimeoutError("SARIF processing did not complete within five status checks") + + +def prepared_sarif(*, path: Path, category: str) -> bytes: + raw = path.read_bytes() + document = json.loads(raw) + if not isinstance(document, dict): + raise ValueError("Expected a prepared SARIF object") + runs = document.get("runs") + if document.get("version") != "2.1.0" or not isinstance(runs, list) or not runs: + raise ValueError("Expected a prepared SARIF 2.1.0 file with CodeQL runs") + for run in runs: + if ( + not isinstance(run, dict) + or not isinstance(run.get("tool"), dict) + or not isinstance(run["tool"].get("driver"), dict) + or not isinstance(run.get("automationDetails"), dict) + or run["tool"]["driver"].get("name") != "CodeQL" + or run["automationDetails"].get("id") != f"{category}/" + ): + raise ValueError("Prepared CodeQL SARIF does not match the matrix language category") + return raw + + +def publish_merge_group(*, api: GitHubAPI, group: MergeGroup, sarif: bytes, checkout: Path) -> str: + _ensure_live(api=api, group=group) + payload = { + "commit_sha": group.sha, + "ref": group.ref, + "sarif": base64.b64encode(gzip.compress(sarif, mtime=0)).decode("ascii"), + "checkout_uri": checkout.resolve().as_uri(), + "tool_name": "CodeQL", + } + try: + result = api.request( + method="POST", path=f"{group.api_path}/code-scanning/sarifs", expected_status=202, payload=payload + ) + except GitHubAPIError as error: + if error.status in (400, 404) and error.message == _missing_ref_message(group) and error.errors in (None, []): + _ensure_live(api=api, group=group) + raise + if not isinstance(result, dict): + raise ValueError("SARIF upload was accepted without a valid upload ID") + sarif_id = result.get("id") + if not isinstance(sarif_id, str) or not sarif_id: + raise ValueError("SARIF upload was accepted without a valid upload ID") + _wait_for_processing(api=api, group=group, sarif_id=sarif_id) + _ensure_live(api=api, group=group) + print(f"CodeQL SARIF {sarif_id} processed for {group.ref} at {group.sha}") + return sarif_id + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--sarif-file", required=True, type=Path) + parser.add_argument( + "--category", + required=True, + choices=("/language:actions", "/language:javascript-typescript", "/language:python"), + ) + parser.add_argument("--analysis-outcome", required=True) + args = parser.parse_args(argv) + try: + if args.analysis_outcome != "success": + raise ValueError("Publishing requires successful CodeQL analysis") + group = load_merge_group(os.environ) + sarif = prepared_sarif(path=args.sarif_file, category=args.category) + api = GitHubAPI(api_url=os.environ["GITHUB_API_URL"], token=os.environ["GITHUB_TOKEN"]) + publish_merge_group(api=api, group=group, sarif=sarif, checkout=Path(os.environ["GITHUB_WORKSPACE"])) + except (WorkflowCancelledError, KeyboardInterrupt) as error: + print(f"CodeQL merge-group publication cancelled: {error}", file=sys.stderr) + return 130 + except (ValueError, KeyError, RuntimeError, OSError, http.client.HTTPException) as error: + message = str(error).replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A") + print(f"::error::{message}", file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/doc/contributing/4_running_tests.md b/doc/contributing/4_running_tests.md index 20f862aa9c..0e6161ce2f 100644 --- a/doc/contributing/4_running_tests.md +++ b/doc/contributing/4_running_tests.md @@ -38,6 +38,29 @@ server, including when the `litellm` or `all` extra is installed. Real adapter and outgoing-header checks run separately as [loopback integration tests](./6_integration_tests.md#litellm-loopback-integration-tests). +### CodeQL merge-group uploads + +CodeQL still analyzes all three languages on PRs, merge groups, main/release pushes, +manual runs and its weekly schedule. Merge groups publish the analyzer's prepared +SARIF through the documented code-scanning API so an expired queue ref can be +distinguished from a genuine upload failure. Categories and fingerprints are +preserved; failed analysis retains CodeQL's native diagnostic upload. + +Only a successful matching-refs response proving that the exact queue ref is absent +or points to another SHA permits retirement. A missing-ref upload rejection is +rechecked to cover deletion between verification and upload. Authentication, +service, processing and analysis failures stay failures. Retirement cancels the +verified current CodeQL run, never reports a skipped scan as success, and refuses +cancellation when a matrix job has already failed. The CodeQL job needs +`actions: write` for self-cancellation; workflow-wide permissions remain empty. +Fork PRs use native CodeQL publishing and GitHub's read-only fork token policy. + +Run the deterministic, offline regression coverage with: + +```bash +uv run --frozen pytest tests/unit/build_scripts/test_codeql_merge_group.py -q +``` + ## Running a subset while iterating For a narrower run, invoke `pytest` directly. You can invoke pytest if it's in your path or via python; either `pytest` or `python -m pytest`. For the following examples, we will use `pytest`. diff --git a/tests/unit/build_scripts/test_codeql_merge_group.py b/tests/unit/build_scripts/test_codeql_merge_group.py new file mode 100644 index 0000000000..12e3c1c9e2 --- /dev/null +++ b/tests/unit/build_scripts/test_codeql_merge_group.py @@ -0,0 +1,654 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT license. +"""Offline contracts for publishing live CodeQL candidates and explicitly retiring obsolete ones.""" + +from __future__ import annotations + +import base64 +import gzip +import http.client +import json +import os +import re +from pathlib import Path +from typing import Any +from unittest.mock import MagicMock, patch + +import pytest +import yaml + +from build_scripts import codeql_merge_group as publisher + +REPO_ROOT = Path(__file__).resolve().parents[3] +SHA = "a" * 40 +REF = f"refs/heads/gh-readonly-queue/main/pr-42-{SHA}" +REPOSITORY = "owner/repository" +API_PATH = f"/repos/{REPOSITORY}" +REF_PATH = f"{API_PATH}/git/matching-refs/{REF.removeprefix('refs/')}" +RUN_PATH = f"{API_PATH}/actions/runs/123" +JOBS_PATH = f"{RUN_PATH}/attempts/1/jobs?per_page=100" +UPLOAD_PATH = f"{API_PATH}/code-scanning/sarifs" +UPLOAD_ID = "sarif-upload-id" +CATEGORY = "/language:python" + + +@pytest.fixture +def group() -> publisher.MergeGroup: + return publisher.MergeGroup(repository=REPOSITORY, ref=REF, sha=SHA, run_id=123, run_attempt=1) + + +@pytest.fixture +def api() -> MagicMock: + return MagicMock(spec=publisher.GitHubAPI) + + +@pytest.fixture +def sarif_file(tmp_path: Path) -> Path: + path = tmp_path / "upload.sarif" + path.write_text( + json.dumps( + { + "version": "2.1.0", + "runs": [ + { + "tool": {"driver": {"name": "CodeQL"}}, + "automationDetails": {"id": CATEGORY + "/"}, + "results": [{"partialFingerprints": {"primaryLocationLineHash": "existing-fingerprint"}}], + } + ], + } + ), + encoding="utf-8", + ) + return path + + +@pytest.fixture +def environment(tmp_path: Path) -> dict[str, str]: + event = tmp_path / "event.json" + event.write_text( + json.dumps( + { + "action": "checks_requested", + "merge_group": {"base_ref": "refs/heads/main", "head_ref": REF, "head_sha": SHA}, + "repository": {"full_name": REPOSITORY}, + } + ), + encoding="utf-8", + ) + return { + "GITHUB_EVENT_NAME": "merge_group", + "GITHUB_EVENT_PATH": str(event), + "GITHUB_REPOSITORY": REPOSITORY, + "GITHUB_REF": REF, + "GITHUB_SHA": SHA, + "GITHUB_RUN_ID": "123", + "GITHUB_RUN_ATTEMPT": "1", + "GITHUB_API_URL": "https://api.github.com", + "GITHUB_TOKEN": "offline-test-token", + "GITHUB_WORKSPACE": str(tmp_path), + } + + +def _refs(sha: str = SHA) -> list[dict[str, Any]]: + return [{"ref": REF, "object": {"type": "commit", "sha": sha}}] + + +def _run(**changes: Any) -> dict[str, Any]: + return { + "id": 123, + "run_attempt": 1, + "event": "merge_group", + "head_sha": SHA, + "head_branch": REF.removeprefix("refs/heads/"), + "path": ".github/workflows/codeql.yml", + "status": "in_progress", + "conclusion": None, + **changes, + } + + +def _jobs() -> dict[str, Any]: + return {"total_count": 3, "jobs": [{"conclusion": None, "steps": [{"conclusion": "success"}]} for _ in range(3)]} + + +def _request_paths(api: MagicMock) -> list[tuple[str, str]]: + return [(call.kwargs["method"], call.kwargs["path"]) for call in api.request.call_args_list] + + +def _publish(*, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path) -> str: + return publisher.publish_merge_group( + api=api, + group=group, + sarif=publisher.prepared_sarif(path=sarif_file, category=CATEGORY), + checkout=sarif_file.parent, + ) + + +def _main(*, environment: dict[str, str], sarif_file: Path, outcome: str = "success") -> int: + with patch.dict(os.environ, environment, clear=True): + return publisher.main(["--sarif-file", str(sarif_file), "--category", CATEGORY, "--analysis-outcome", outcome]) + + +def test_live_matching_ref_uploads_exact_prepared_sarif( + *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path +) -> None: + api.request.side_effect = [_refs(), {"id": UPLOAD_ID}, {"processing_status": "complete"}, _refs()] + with patch.object(publisher.time, "sleep") as sleep: + assert _publish(api=api, group=group, sarif_file=sarif_file) == UPLOAD_ID + assert _request_paths(api) == [ + ("GET", REF_PATH), + ("POST", UPLOAD_PATH), + ("GET", f"{UPLOAD_PATH}/{UPLOAD_ID}"), + ("GET", REF_PATH), + ] + upload = api.request.call_args_list[1].kwargs + assert upload["expected_status"] == 202 + payload = upload["payload"] + assert payload["commit_sha"] == SHA + assert payload["ref"] == REF + assert payload["tool_name"] == "CodeQL" + assert payload["checkout_uri"] == sarif_file.parent.resolve().as_uri() + assert gzip.decompress(base64.b64decode(payload["sarif"])) == sarif_file.read_bytes() + sleep.assert_called_once_with(5) + + +@pytest.mark.parametrize("refs", [[], _refs("b" * 40), [{"ref": REF + "-suffix", "object": {"sha": SHA}}]]) +def test_verified_absent_or_changed_ref_requests_cancellation_without_upload( + *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path, refs: list[dict[str, Any]] +) -> None: + api.request.side_effect = [refs, _run(), _jobs(), {}] + with patch.object(publisher.time, "sleep", side_effect=KeyboardInterrupt): + with pytest.raises(KeyboardInterrupt): + _publish(api=api, group=group, sarif_file=sarif_file) + assert _request_paths(api) == [ + ("GET", REF_PATH), + ("GET", RUN_PATH), + ("GET", JOBS_PATH), + ("POST", RUN_PATH + "/cancel"), + ] + assert api.request.call_args.kwargs["expected_status"] == 202 + + +@pytest.mark.parametrize("status", [400, 404]) +@pytest.mark.parametrize("current_refs", [[], _refs("b" * 40)]) +def test_disappearance_between_precheck_and_upload_is_rechecked_then_cancelled( + *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path, status: int, current_refs: list[dict[str, Any]] +) -> None: + rejection = publisher.GitHubAPIError(status=status, message=f"ref '{REF}' not found in this repository") + api.request.side_effect = [_refs(), rejection, current_refs, _run(), _jobs(), {}] + with patch.object(publisher.time, "sleep", side_effect=KeyboardInterrupt): + with pytest.raises(KeyboardInterrupt): + _publish(api=api, group=group, sarif_file=sarif_file) + assert _request_paths(api) == [ + ("GET", REF_PATH), + ("POST", UPLOAD_PATH), + ("GET", REF_PATH), + ("GET", RUN_PATH), + ("GET", JOBS_PATH), + ("POST", RUN_PATH + "/cancel"), + ] + + +def test_missing_ref_rejection_with_still_live_candidate_remains_failure( + *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path +) -> None: + rejection = publisher.GitHubAPIError(status=404, message=f"ref '{REF}' not found in this repository") + api.request.side_effect = [_refs(), rejection, _refs()] + with pytest.raises(publisher.GitHubAPIError) as raised: + _publish(api=api, group=group, sarif_file=sarif_file) + assert raised.value is rejection + assert _request_paths(api) == [("GET", REF_PATH), ("POST", UPLOAD_PATH), ("GET", REF_PATH)] + + +@pytest.mark.parametrize("stage", ["precheck", "upload", "processing", "recheck", "cancel"]) +@pytest.mark.parametrize("status", [401, 403, 404, 429, 500, 502, 503]) +def test_api_errors_never_become_obsolete_candidate_success( + *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path, stage: str, status: int +) -> None: + error = publisher.GitHubAPIError(status=status, message="authentication, rate limit or service failure") + prefixes: dict[str, list[Any]] = { + "precheck": [], + "upload": [_refs()], + "processing": [_refs(), {"id": UPLOAD_ID}], + "recheck": [ + _refs(), + publisher.GitHubAPIError(status=404, message=f"ref '{REF}' not found in this repository"), + ], + "cancel": [[], _run(), _jobs()], + } + api.request.side_effect = [*prefixes[stage], error] + with patch.object(publisher.time, "sleep"): + with pytest.raises(publisher.GitHubAPIError) as raised: + _publish(api=api, group=group, sarif_file=sarif_file) + assert raised.value is error + cancellations = [path for method, path in _request_paths(api) if method == "POST" and path.endswith("/cancel")] + assert cancellations == ([RUN_PATH + "/cancel"] if stage == "cancel" else []) + + +@pytest.mark.parametrize("stage", ["precheck", "upload", "processing", "cancel"]) +@pytest.mark.parametrize("error", [OSError("network unavailable"), http.client.HTTPException("connection failed")]) +def test_network_errors_remain_failures( + *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path, stage: str, error: Exception +) -> None: + prefixes: dict[str, list[Any]] = { + "precheck": [], + "upload": [_refs()], + "processing": [_refs(), {"id": UPLOAD_ID}], + "cancel": [[], _run(), _jobs()], + } + api.request.side_effect = [*prefixes[stage], error] + with patch.object(publisher.time, "sleep"): + with pytest.raises(type(error)) as raised: + _publish(api=api, group=group, sarif_file=sarif_file) + assert raised.value is error + + +@pytest.mark.parametrize( + "error", + [ + publisher.GitHubAPIError(status=400, message="Invalid SARIF"), + publisher.GitHubAPIError(status=404, message="Not Found"), + publisher.GitHubAPIError(status=422, message=f"ref '{REF}' not found in this repository"), + publisher.GitHubAPIError( + status=400, message=f"ref '{REF}' not found in this repository", errors=["Invalid SARIF"] + ), + ], +) +def test_genuine_or_unsupported_upload_rejection_does_not_consult_expiry( + *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path, error: publisher.GitHubAPIError +) -> None: + api.request.side_effect = [_refs(), error] + with pytest.raises(publisher.GitHubAPIError) as raised: + _publish(api=api, group=group, sarif_file=sarif_file) + assert raised.value is error + assert _request_paths(api) == [("GET", REF_PATH), ("POST", UPLOAD_PATH)] + + +@pytest.mark.parametrize("status", ["pending", "complete", "failed"]) +def test_expiry_after_accepted_upload_is_not_reported_as_success( + *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path, status: str +) -> None: + response: dict[str, Any] = {"processing_status": status} + if status == "failed": + response["errors"] = [f"ref '{REF}' not found in this repository"] + api.request.side_effect = [_refs(), {"id": UPLOAD_ID}, response, [], _run(), _jobs(), {}] + with patch.object(publisher.time, "sleep", side_effect=[None, KeyboardInterrupt]): + with pytest.raises(KeyboardInterrupt): + _publish(api=api, group=group, sarif_file=sarif_file) + assert _request_paths(api)[-4:] == [ + ("GET", REF_PATH), + ("GET", RUN_PATH), + ("GET", JOBS_PATH), + ("POST", RUN_PATH + "/cancel"), + ] + + +@pytest.mark.parametrize("errors", [["Invalid SARIF"], [f"ref '{REF}' not found in this repository", "Invalid SARIF"]]) +def test_processing_rejection_remains_failure_without_expiry_fallback( + *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path, errors: list[str] +) -> None: + api.request.side_effect = [_refs(), {"id": UPLOAD_ID}, {"processing_status": "failed", "errors": errors}] + with patch.object(publisher.time, "sleep"): + with pytest.raises(RuntimeError, match="SARIF processing failed"): + _publish(api=api, group=group, sarif_file=sarif_file) + assert _request_paths(api) == [("GET", REF_PATH), ("POST", UPLOAD_PATH), ("GET", f"{UPLOAD_PATH}/{UPLOAD_ID}")] + + +def test_processing_timeout_is_bounded_and_fails( + *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path +) -> None: + api.request.side_effect = [ + _refs(), + {"id": UPLOAD_ID}, + *[item for _ in range(5) for item in ({"processing_status": "pending"}, _refs())], + ] + with patch.object(publisher.time, "sleep") as sleep: + with pytest.raises(TimeoutError, match="five status checks"): + _publish(api=api, group=group, sarif_file=sarif_file) + assert [call.args[0] for call in sleep.call_args_list] == [5, 10, 20, 40, 80] + assert len(_request_paths(api)) == 12 + assert not any(path.endswith("/cancel") for _, path in _request_paths(api)) + + +def test_accepted_cancellation_never_returns_a_successful_check( + *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path +) -> None: + api.request.side_effect = [[], _run(), _jobs(), {}] + with patch.object(publisher.time, "sleep") as sleep: + with pytest.raises(TimeoutError, match="runner did not stop within 60 seconds"): + _publish(api=api, group=group, sarif_file=sarif_file) + sleep.assert_called_once_with(60) + assert _request_paths(api)[-1] == ("POST", RUN_PATH + "/cancel") + + +@pytest.mark.parametrize( + "changes", + [ + {"id": 999}, + {"run_attempt": 2}, + {"event": "push"}, + {"head_sha": "b" * 40}, + {"head_branch": "main"}, + {"path": ".github/workflows/other.yml"}, + {"status": "completed", "conclusion": "failure"}, + {"status": "completed", "conclusion": "success"}, + ], +) +def test_retirement_cannot_target_another_run_or_override_completed_outcomes( + *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path, changes: dict[str, Any] +) -> None: + api.request.side_effect = [[], _run(**changes)] + with pytest.raises(ValueError, match="Refusing to cancel"): + _publish(api=api, group=group, sarif_file=sarif_file) + assert _request_paths(api) == [("GET", REF_PATH), ("GET", RUN_PATH)] + + +@pytest.mark.parametrize("conclusion", ["failure", "timed_out", "startup_failure", "action_required"]) +@pytest.mark.parametrize("location", ["job", "step"]) +def test_sibling_analysis_failures_block_retirement( + *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path, conclusion: str, location: str +) -> None: + jobs = _jobs() + if location == "job": + jobs["jobs"][0]["conclusion"] = conclusion + else: + jobs["jobs"][0]["steps"][0]["conclusion"] = conclusion + api.request.side_effect = [[], _run(), jobs] + with pytest.raises(RuntimeError, match="refusing to replace failure with cancellation"): + _publish(api=api, group=group, sarif_file=sarif_file) + assert _request_paths(api) == [("GET", REF_PATH), ("GET", RUN_PATH), ("GET", JOBS_PATH)] + + +def test_incomplete_job_listing_cannot_authorize_cancellation( + *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path +) -> None: + jobs = _jobs() + jobs["total_count"] = 101 + api.request.side_effect = [[], _run(), jobs] + with pytest.raises(ValueError, match="Cannot verify all"): + _publish(api=api, group=group, sarif_file=sarif_file) + assert _request_paths(api)[-1] == ("GET", JOBS_PATH) + + +@pytest.mark.parametrize("conflict", [False, True]) +def test_already_cancelled_workflow_has_nonzero_cli_outcome( + *, api: MagicMock, environment: dict[str, str], sarif_file: Path, conflict: bool +) -> None: + cancelled = _run(status="completed", conclusion="cancelled") + responses: list[Any] = [[], cancelled] + if conflict: + responses = [[], _run(), _jobs(), publisher.GitHubAPIError(status=409, message="Conflict"), cancelled] + api.request.side_effect = responses + with patch.object(publisher, "GitHubAPI", return_value=api): + assert _main(environment=environment, sarif_file=sarif_file) == 130 + assert _request_paths(api)[-1] == ("GET", RUN_PATH) + + +@pytest.mark.parametrize("outcome", ["failure", "cancelled", "skipped", ""]) +def test_analyzer_failure_or_nonexecution_cannot_publish_or_cancel( + *, environment: dict[str, str], sarif_file: Path, outcome: str +) -> None: + with patch.object(publisher, "GitHubAPI") as constructor: + assert _main(environment=environment, sarif_file=sarif_file, outcome=outcome) == 1 + constructor.assert_not_called() + + +def test_runner_cancellation_is_nonzero_and_does_not_continue_publishing( + *, api: MagicMock, environment: dict[str, str], sarif_file: Path +) -> None: + api.request.side_effect = [[], _run(), _jobs(), {}] + with ( + patch.object(publisher, "GitHubAPI", return_value=api), + patch.object(publisher.time, "sleep", side_effect=KeyboardInterrupt), + ): + assert _main(environment=environment, sarif_file=sarif_file) == 130 + assert _request_paths(api)[-1] == ("POST", RUN_PATH + "/cancel") + + +@pytest.mark.parametrize( + ("key", "value"), + [ + ("GITHUB_EVENT_NAME", "push"), + ("GITHUB_REF", "refs/heads/main"), + ("GITHUB_SHA", "b" * 40), + ("GITHUB_REPOSITORY", "other/repository"), + ("GITHUB_RUN_ID", "0"), + ("GITHUB_RUN_ATTEMPT", "-1"), + ], +) +def test_invalid_event_identity_fails_before_api_access( + *, environment: dict[str, str], sarif_file: Path, key: str, value: str +) -> None: + environment[key] = value + with patch.object(publisher, "GitHubAPI") as constructor: + assert _main(environment=environment, sarif_file=sarif_file) == 1 + constructor.assert_not_called() + + +def test_event_identifies_exact_tested_merge_group(environment: dict[str, str]) -> None: + assert publisher.load_merge_group(environment) == publisher.MergeGroup( + repository=REPOSITORY, ref=REF, sha=SHA, run_id=123, run_attempt=1 + ) + + +def test_queue_branch_suffix_is_not_used_as_the_tested_head_sha(environment: dict[str, str]) -> None: + environment["GITHUB_SHA"] = "b" * 40 + event_path = Path(environment["GITHUB_EVENT_PATH"]) + event = json.loads(event_path.read_text(encoding="utf-8")) + event["merge_group"]["head_sha"] = environment["GITHUB_SHA"] + event_path.write_text(json.dumps(event), encoding="utf-8") + group = publisher.load_merge_group(environment) + assert group.ref == REF + assert group.sha == "b" * 40 + + +def test_wrong_sarif_category_cannot_publish_or_retire(*, environment: dict[str, str], sarif_file: Path) -> None: + sarif_file.write_text( + sarif_file.read_text(encoding="utf-8").replace(CATEGORY, "/language:actions"), encoding="utf-8" + ) + with patch.object(publisher, "GitHubAPI") as constructor: + assert _main(environment=environment, sarif_file=sarif_file) == 1 + constructor.assert_not_called() + + +@pytest.mark.parametrize("status", [301, 302, 400, 401, 403, 404, 429, 500, 503]) +def test_http_transport_retains_status_and_never_follows_redirects(status: int) -> None: + connection = MagicMock(spec=http.client.HTTPSConnection) + response = MagicMock(spec=http.client.HTTPResponse) + response.status = status + response.read.return_value = json.dumps({"message": "Request rejected"}).encode() + connection.getresponse.return_value = response + with patch.object(publisher.http.client, "HTTPSConnection", return_value=connection) as constructor: + client = publisher.GitHubAPI(api_url="https://github.example/api/v3", token="offline-test-token") + with pytest.raises(publisher.GitHubAPIError) as raised: + client.request(method="GET", path=REF_PATH, expected_status=200) + assert raised.value.status == status + assert raised.value.message == "Request rejected" + constructor.assert_called_once_with("github.example", timeout=30) + assert connection.request.call_args.args[:2] == ("GET", "/api/v3" + REF_PATH) + connection.request.assert_called_once() + connection.close.assert_called_once() + + +@pytest.mark.parametrize( + "api_url", ["http://github.example", "https://token@github.example", "https://github.example?x=1"] +) +def test_http_transport_rejects_insecure_or_credential_bearing_urls(api_url: str) -> None: + with pytest.raises(ValueError, match="HTTPS"): + publisher.GitHubAPI(api_url=api_url, token="offline-test-token") + + +def test_workflow_preserves_required_names_languages_events_and_native_diagnostics() -> None: + workflow = yaml.load( + (REPO_ROOT / ".github" / "workflows" / "codeql.yml").read_text(encoding="utf-8"), Loader=yaml.BaseLoader + ) + assert set(workflow["on"]) == {"push", "pull_request", "merge_group", "workflow_dispatch", "schedule"} + assert workflow["on"]["push"]["branches"] == ["main", "releases/v*"] + assert workflow["on"]["pull_request"]["branches"] == ["main", "releases/**"] + assert workflow["on"]["schedule"] == [{"cron": "23 2 * * 1"}] + assert workflow["permissions"] == {} + assert set(workflow["jobs"]) == {"analyze"} + job = workflow["jobs"]["analyze"] + assert job["name"] == "CodeQL (${{ matrix.language }})" + assert job["strategy"] == { + "fail-fast": "false", + "matrix": {"language": ["actions", "javascript-typescript", "python"]}, + } + assert job["permissions"] == {"actions": "write", "contents": "read", "security-events": "write"} + assert "if" not in job + assert "continue-on-error" not in job + assert all("continue-on-error" not in step for step in job["steps"]) + steps = {step["name"]: step for step in job["steps"]} + assert steps["Checkout"]["with"] == {"ref": "${{ github.sha }}", "persist-credentials": "false"} + analyze = steps["Perform CodeQL Analysis"] + assert "if" not in analyze + assert analyze["uses"] == "github/codeql-action/analyze@2892aa5e19bbd11bc0cff5427e3b750a04d9e3c2" + assert analyze["with"]["category"] == "/language:${{ matrix.language }}" + assert analyze["with"]["upload"] == "failure-only" + assert analyze["with"]["post-processed-sarif-path"] == "${{ runner.temp }}/codeql-upload" + native = steps["Publish CodeQL results"] + assert native["uses"] == "github/codeql-action/upload-sarif@2892aa5e19bbd11bc0cff5427e3b750a04d9e3c2" + assert native["with"] == { + "sarif_file": "${{ runner.temp }}/codeql-upload/upload.sarif", + "category": analyze["with"]["category"], + } + assert native["if"] == "github.event_name != 'merge_group'" + publish = steps["Publish merge-group CodeQL results"] + assert publish["if"] == "github.event_name == 'merge_group'" + assert steps["Set up merge-group upload helper"]["if"] == publish["if"] + assert publish["env"]["ANALYSIS_OUTCOME"] == "${{ steps.analysis.outcome }}" + assert publish["env"]["SARIF_FILE"] == "${{ runner.temp }}/codeql-upload/upload.sarif" + assert publish["env"]["SARIF_CATEGORY"] == analyze["with"]["category"] + assert publish["timeout-minutes"] == "8" + assert "uv run --no-project python -m build_scripts.codeql_merge_group" in publish["run"] + + +@pytest.mark.parametrize( + ("event_name", "fork"), + [ + ("push", False), + ("pull_request", False), + ("pull_request", True), + ("schedule", False), + ("workflow_dispatch", False), + ("merge_group", False), + ], +) +def test_normal_and_fork_events_select_native_upload_and_never_retirement(*, event_name: str, fork: bool) -> None: + workflow = yaml.load( + (REPO_ROOT / ".github" / "workflows" / "codeql.yml").read_text(encoding="utf-8"), Loader=yaml.BaseLoader + ) + steps = {step["name"]: step for step in workflow["jobs"]["analyze"]["steps"]} + selected = [] + for name in ("Publish CodeQL results", "Set up merge-group upload helper", "Publish merge-group CodeQL results"): + condition = steps[name]["if"] + comparison = re.fullmatch(r"github\.event_name (==|!=) '([^']+)'", condition) + assert comparison is not None + matches = event_name == comparison[2] + if matches == (comparison[1] == "=="): + selected.append(name) + if event_name == "merge_group": + assert selected == ["Set up merge-group upload helper", "Publish merge-group CodeQL results"] + else: + assert selected == ["Publish CodeQL results"] + if fork: + assert "pull_request_target" not in workflow["on"] + assert not {"ref", "sha", "token"} & steps["Publish CodeQL results"]["with"].keys() + + +@pytest.mark.parametrize( + "error", + [ + publisher.GitHubAPIError(status=403, message="Forbidden"), + publisher.GitHubAPIError(status=429, message="Rate limited"), + publisher.GitHubAPIError(status=503, message="Unavailable"), + OSError("Network failure"), + ], +) +def test_cli_reports_api_and_network_failures_as_nonzero( + *, + api: MagicMock, + environment: dict[str, str], + sarif_file: Path, + error: Exception, + capsys: pytest.CaptureFixture[str], +) -> None: + api.request.side_effect = error + with patch.object(publisher, "GitHubAPI", return_value=api): + assert _main(environment=environment, sarif_file=sarif_file) == 1 + assert "::error::" in capsys.readouterr().err + assert _request_paths(api) == [("GET", REF_PATH)] + + +def test_cli_only_succeeds_after_analysis_and_live_upload_processing( + *, api: MagicMock, environment: dict[str, str], sarif_file: Path +) -> None: + api.request.side_effect = [_refs(), {"id": UPLOAD_ID}, {"processing_status": "complete"}, _refs()] + with patch.object(publisher, "GitHubAPI", return_value=api), patch.object(publisher.time, "sleep"): + assert _main(environment=environment, sarif_file=sarif_file) == 0 + assert _request_paths(api)[-1] == ("GET", REF_PATH) + + +@pytest.mark.parametrize( + "response", + [None, {}, [{"ref": REF, "object": None}], _refs() * 2, [{"ref": REF, "object": {"type": "tag", "sha": SHA}}]], +) +def test_invalid_ref_response_cannot_authorize_upload_or_retirement( + *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path, response: Any +) -> None: + api.request.return_value = response + with pytest.raises(ValueError): + _publish(api=api, group=group, sarif_file=sarif_file) + assert _request_paths(api) == [("GET", REF_PATH)] + + +@pytest.mark.parametrize( + "document", [[], {"version": "2.1.0", "runs": []}, {"version": "2.1.0", "runs": [{"tool": None}]}] +) +def test_invalid_prepared_sarif_cannot_authorize_retirement( + *, environment: dict[str, str], sarif_file: Path, document: Any +) -> None: + sarif_file.write_text(json.dumps(document), encoding="utf-8") + with patch.object(publisher, "GitHubAPI") as constructor: + assert _main(environment=environment, sarif_file=sarif_file) == 1 + constructor.assert_not_called() + + +@pytest.mark.parametrize("state", ["live", "obsolete", "upload-race"]) +def test_cli_exercises_real_http_adapter_with_only_mocked_connections( + *, environment: dict[str, str], sarif_file: Path, state: str +) -> None: + responses: list[tuple[int, Any]] = [ + (200, _refs()), + (202, {"id": UPLOAD_ID}), + (200, {"processing_status": "complete"}), + (200, _refs()), + ] + if state != "live": + responses = [(200, []), (200, _run()), (200, _jobs()), (202, None)] + if state == "upload-race": + responses = [(200, _refs()), (404, {"message": f"ref '{REF}' not found in this repository"}), *responses] + connections = [] + for status, body in responses: + connection = MagicMock(spec=http.client.HTTPSConnection) + response = MagicMock(spec=http.client.HTTPResponse) + response.status = status + response.read.return_value = json.dumps(body).encode() if body is not None else b"" + connection.getresponse.return_value = response + connections.append(connection) + sleeps = None if state == "live" else KeyboardInterrupt + with ( + patch.object(publisher.http.client, "HTTPSConnection", side_effect=connections), + patch.object(publisher.time, "sleep", side_effect=sleeps), + ): + assert _main(environment=environment, sarif_file=sarif_file) == (0 if state == "live" else 130) + for connection in connections: + connection.request.assert_called_once() + connection.close.assert_called_once() + last_request = connections[-1].request.call_args + assert last_request.args[:2] == (("GET", REF_PATH) if state == "live" else ("POST", RUN_PATH + "/cancel")) + if state == "live": + payload = json.loads(connections[1].request.call_args.kwargs["body"]) + assert set(payload) == {"commit_sha", "ref", "sarif", "checkout_uri", "tool_name"} + assert gzip.decompress(base64.b64decode(payload["sarif"])) == sarif_file.read_bytes() From 7be2374e56712ecc4dc0336622dc45373fa7656c Mon Sep 17 00:00:00 2001 From: Roman Lutz Date: Sun, 11 Oct 2026 00:25:47 -0700 Subject: [PATCH 2/2] MAINT Simplify CodeQL obsolete-run handling Use the pinned GitHub Script client instead of custom Python HTTP and CLI plumbing. Consolidate failure and cancellation regressions into shared offline scenarios while preserving native non-merge uploads and fail-closed merge-group behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/workflows/codeql.yml | 26 +- build_scripts/codeql_merge_group.cjs | 178 +++++ build_scripts/codeql_merge_group.py | 309 -------- doc/contributing/4_running_tests.md | 8 +- .../build_scripts/test_codeql_merge_group.cjs | 255 +++++++ .../build_scripts/test_codeql_merge_group.py | 667 ++---------------- 6 files changed, 511 insertions(+), 932 deletions(-) create mode 100644 build_scripts/codeql_merge_group.cjs delete mode 100644 build_scripts/codeql_merge_group.py create mode 100644 tests/unit/build_scripts/test_codeql_merge_group.cjs diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index b4d8479b94..8c7a056d5a 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -58,23 +58,23 @@ jobs: sarif_file: ${{ runner.temp }}/codeql-upload/upload.sarif category: "/language:${{ matrix.language }}" - - name: Set up merge-group upload helper - if: github.event_name == 'merge_group' - uses: astral-sh/setup-uv@c18668ad3cf93ea998bef934396af7bb5c839dc7 # v10.2.0 - with: - version: "0.9.17" - enable-cache: false - - name: Publish merge-group CodeQL results if: github.event_name == 'merge_group' + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 timeout-minutes: 8 env: - GITHUB_TOKEN: ${{ github.token }} ANALYSIS_OUTCOME: ${{ steps.analysis.outcome }} SARIF_FILE: ${{ runner.temp }}/codeql-upload/upload.sarif SARIF_CATEGORY: "/language:${{ matrix.language }}" - run: | - uv run --no-project python -m build_scripts.codeql_merge_group \ - --analysis-outcome "$ANALYSIS_OUTCOME" \ - --sarif-file "$SARIF_FILE" \ - --category "$SARIF_CATEGORY" + with: + retries: 0 + script: | + const publish = require(require('node:path').resolve('build_scripts', 'codeql_merge_group.cjs')); + await publish({ + github, context, core, + sarifFile: process.env.SARIF_FILE, + category: process.env.SARIF_CATEGORY, + analysisOutcome: process.env.ANALYSIS_OUTCOME, + runAttempt: Number(process.env.GITHUB_RUN_ATTEMPT), + checkout: process.env.GITHUB_WORKSPACE, + }); diff --git a/build_scripts/codeql_merge_group.cjs b/build_scripts/codeql_merge_group.cjs new file mode 100644 index 0000000000..30765f6acb --- /dev/null +++ b/build_scripts/codeql_merge_group.cjs @@ -0,0 +1,178 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// @ts-check + +const { readFile } = require("node:fs/promises"); +const { setTimeout: delay } = require("node:timers/promises"); +const { gzipSync } = require("node:zlib"); +const { pathToFileURL } = require("node:url"); + +/** @param {unknown} value @returns {value is Record} */ +function isObject(value) { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +/** @param {unknown} value @returns {Record} */ +function object(value) { + if (!isObject(value)) throw new Error("Invalid GitHub API or SARIF object"); + return value; +} + +/** + * @typedef {{repo: {owner: string, repo: string}, eventName: string, ref: string, sha: string, + * runId: number, payload: {action?: string, merge_group?: {base_ref?: string, head_ref?: string, head_sha?: string}}}} Context + * @typedef {{request: (route: string, options: Record) => + * Promise<{status: number, data: unknown}>}} GitHub + * @param {{github: GitHub, context: Context, core: {notice: (message: string) => void, info: (message: string) => void}, + * sarifFile: string, category: string, analysisOutcome: string, runAttempt: number, + * checkout: string, sleep?: (milliseconds: number) => Promise}} options + */ +module.exports = async function publish({ + github, context, core, sarifFile, category, analysisOutcome, runAttempt, checkout, sleep = delay +}) { + const group = context.payload.merge_group; + const queuePrefix = `refs/heads/gh-readonly-queue/${group?.base_ref?.slice(11)}/`; + if (analysisOutcome !== "success") throw new Error("Publishing requires successful CodeQL analysis"); + if ( + context.eventName !== "merge_group" || context.payload.action !== "checks_requested" || + !group?.base_ref?.startsWith("refs/heads/") || !context.ref.startsWith(queuePrefix) || context.ref === queuePrefix || + group.head_ref !== context.ref || group.head_sha !== context.sha || !/^[0-9a-f]{40}$/.test(context.sha) || + !Number.isSafeInteger(context.runId) || context.runId <= 0 || !Number.isSafeInteger(runAttempt) || runAttempt <= 0 + ) { + throw new Error("The merge-group context does not match the tested queue ref, SHA and run"); + } + const sarif = await readFile(sarifFile); + const document = object(JSON.parse(sarif.toString("utf8"))); + if ( + document.version !== "2.1.0" || !Array.isArray(document.runs) || !document.runs.length || + document.runs.some((value) => { + const run = object(value); + return object(object(run.tool).driver).name !== "CodeQL" || + object(run.automationDetails).id !== `${category}/`; + }) + ) { + throw new Error("Expected prepared CodeQL SARIF matching the matrix language category"); + } + const missingRef = `ref '${context.ref}' not found in this repository`; + const runOptions = { run_id: context.runId }; + + /** @param {string} route @param {Record} [options] @param {number} [status] */ + async function request(route, options = {}, status = 200) { + const response = await github.request(route, { + ...context.repo, ...options, + headers: { "X-GitHub-Api-Version": "2022-11-28" }, + request: { timeout: 30000 } + }); + if (response.status !== status) throw new Error(`Unexpected HTTP ${response.status} for ${route}`); + return response.data; + } + + async function currentSha() { + const refs = await request("GET /repos/{owner}/{repo}/git/matching-refs/{ref}", { ref: context.ref.slice(5) }); + if (!Array.isArray(refs) || refs.some((value) => typeof object(value).ref !== "string")) { + throw new Error("Invalid matching-refs response"); + } + const matches = refs.map(object).filter((value) => value.ref === context.ref); + if (!matches.length) return null; + if (matches.length !== 1) throw new Error("Matching-refs returned multiple exact queue refs"); + const target = object(matches[0].object); + if (target.type !== "commit" || typeof target.sha !== "string" || !/^[0-9a-f]{40}$/.test(target.sha)) { + throw new Error("The queue ref does not point to a valid commit"); + } + return target.sha; + } + + async function verifyRun() { + const run = object(await request("GET /repos/{owner}/{repo}/actions/runs/{run_id}", runOptions)); + const expected = { + id: context.runId, run_attempt: runAttempt, event: "merge_group", head_sha: context.sha, + head_branch: context.ref.slice(11), path: ".github/workflows/codeql.yml" + }; + if (Object.entries(expected).some(([key, value]) => run[key] !== value)) { + throw new Error("Refusing to cancel a different workflow run or attempt"); + } + if (run.status === "completed" && run.conclusion === "cancelled") throw new Error("CodeQL run is already cancelled"); + if (run.status !== "in_progress" || run.conclusion !== null) { + throw new Error("Refusing to cancel a workflow run that is not in progress"); + } + } + + async function ensureLive() { + const sha = await currentSha(); + if (sha === context.sha) return; + await verifyRun(); + const result = object(await request( + "GET /repos/{owner}/{repo}/actions/runs/{run_id}/attempts/{attempt_number}/jobs", + { ...runOptions, attempt_number: runAttempt, per_page: 100 } + )); + if (!Array.isArray(result.jobs) || !result.jobs.length || result.total_count !== result.jobs.length) { + throw new Error("Cannot verify all CodeQL matrix jobs before cancellation"); + } + for (const value of result.jobs) { + const job = object(value); + if (!Array.isArray(job.steps)) throw new Error("Invalid workflow job steps"); + const conclusions = [job.conclusion, ...job.steps.map((step) => object(step).conclusion)]; + if (conclusions.some((value) => + value != null && value !== "success" && value !== "skipped" && value !== "cancelled" + )) { + throw new Error("A CodeQL matrix job or step failed; refusing to replace failure with cancellation"); + } + } + core.notice(`Verified obsolete merge candidate ${context.ref}: ${sha === null ? "absent" : `now at ${sha}`}`); + try { + await request("POST /repos/{owner}/{repo}/actions/runs/{run_id}/cancel", runOptions, 202); + } catch (error) { + if (isObject(error) && error.status === 409) await verifyRun(); + throw error; + } + // Never finish green while waiting for the runner's cancellation signal. + await sleep(60000); + throw new Error("Cancellation accepted, but the runner did not stop within 60 seconds"); + } + + await ensureLive(); + let accepted; + try { + accepted = object(await request("POST /repos/{owner}/{repo}/code-scanning/sarifs", { + commit_sha: context.sha, + ref: context.ref, + sarif: gzipSync(sarif).toString("base64"), + checkout_uri: pathToFileURL(checkout).href, + tool_name: "CodeQL" + }, 202)); + } catch (error) { + if ( + isObject(error) && (error.status === 400 || error.status === 404) && isObject(error.response) && + isObject(error.response.data) && error.response.data.message === missingRef && + (error.response.data.errors == null || + (Array.isArray(error.response.data.errors) && !error.response.data.errors.length)) + ) { + await ensureLive(); + } + throw error; + } + const sarifId = accepted.id; + if (typeof sarifId !== "string" || !sarifId) throw new Error("SARIF upload accepted without a valid ID"); + for (const milliseconds of [5000, 10000, 20000, 40000, 80000]) { + await sleep(milliseconds); + const result = object(await request( + "GET /repos/{owner}/{repo}/code-scanning/sarifs/{sarif_id}", { sarif_id: sarifId } + )); + if (result.processing_status === "complete") { + await ensureLive(); + core.info(`CodeQL SARIF ${sarifId} processed for ${context.ref} at ${context.sha}`); + return sarifId; + } + if (result.processing_status === "failed") { + if (Array.isArray(result.errors) && result.errors.length === 1 && result.errors[0] === missingRef) { + await ensureLive(); + } + throw new Error(`SARIF processing failed: ${JSON.stringify(result.errors)}`); + } + if (result.processing_status !== "pending") { + throw new Error(`Invalid SARIF processing status: ${result.processing_status}`); + } + await ensureLive(); + } + throw new Error("SARIF processing did not complete within five status checks"); +}; diff --git a/build_scripts/codeql_merge_group.py b/build_scripts/codeql_merge_group.py deleted file mode 100644 index 503b299a77..0000000000 --- a/build_scripts/codeql_merge_group.py +++ /dev/null @@ -1,309 +0,0 @@ -# Copyright (c) Microsoft Corporation. -# Licensed under the MIT license. -"""Publish prepared CodeQL SARIF or cancel a positively verified obsolete merge group.""" - -from __future__ import annotations - -import argparse -import base64 -import gzip -import http.client -import json -import os -import re -import sys -import time -from dataclasses import dataclass -from pathlib import Path -from typing import TYPE_CHECKING, Any, NoReturn -from urllib.parse import quote, urlsplit - -if TYPE_CHECKING: - from collections.abc import Mapping - - -class GitHubAPIError(RuntimeError): - def __init__(self, *, status: int, message: str, errors: Any = None) -> None: - super().__init__(f"GitHub API returned HTTP {status}: {message}") - self.status = status - self.message = message - self.errors = errors - - -class WorkflowCancelledError(RuntimeError): - pass - - -class GitHubAPI: - TIMEOUT_SECONDS = 30 - - def __init__(self, *, api_url: str, token: str) -> None: - parsed = urlsplit(api_url) - if ( - parsed.scheme != "https" - or not parsed.hostname - or parsed.username - or parsed.password - or parsed.query - or parsed.fragment - or not token - ): - raise ValueError("A HTTPS GitHub API URL and a nonempty Actions token are required") - self._host = parsed.netloc - self._prefix = parsed.path.rstrip("/") - self._token = token - - def request(self, *, method: str, path: str, expected_status: int, payload: dict[str, Any] | None = None) -> Any: - connection = http.client.HTTPSConnection(self._host, timeout=self.TIMEOUT_SECONDS) - headers = { - "Accept": "application/vnd.github+json", - "Authorization": f"Bearer {self._token}", - "Content-Type": "application/json", - "User-Agent": "PyRIT-CodeQL-merge-group", - "X-GitHub-Api-Version": "2022-11-28", - } - try: - connection.request( - method, self._prefix + path, body=json.dumps(payload) if payload else None, headers=headers - ) - response = connection.getresponse() - status, raw = response.status, response.read() - finally: - connection.close() - try: - body = json.loads(raw) if raw else {} - except json.JSONDecodeError as error: - raise GitHubAPIError(status=status, message="Invalid JSON response") from error - if status != expected_status: - message = body.get("message") if isinstance(body, dict) else None - raise GitHubAPIError( - status=status, - message=message if isinstance(message, str) else "Unexpected response", - errors=body.get("errors") if isinstance(body, dict) else None, - ) - return body - - -@dataclass(frozen=True, kw_only=True) -class MergeGroup: - repository: str - ref: str - sha: str - run_id: int - run_attempt: int - - @property - def api_path(self) -> str: - return f"/repos/{self.repository}" - - -def load_merge_group(environment: Mapping[str, str]) -> MergeGroup: - if environment["GITHUB_EVENT_NAME"] != "merge_group": - raise ValueError("This publisher only supports merge_group events") - event = json.loads(Path(environment["GITHUB_EVENT_PATH"]).read_text(encoding="utf-8")) - if not isinstance(event, dict) or not isinstance(event.get("merge_group"), dict): - raise ValueError("Expected a merge-group event object") - group = event["merge_group"] - ref, sha, repository = environment["GITHUB_REF"], environment["GITHUB_SHA"], environment["GITHUB_REPOSITORY"] - base_ref = group["base_ref"] - if not isinstance(base_ref, str) or not base_ref.startswith("refs/heads/"): - raise ValueError("The merge group's base must be a branch ref") - prefix = f"refs/heads/gh-readonly-queue/{base_ref.removeprefix('refs/heads/')}/" - if event["action"] != "checks_requested" or not ref.startswith(prefix) or len(ref) <= len(prefix): - raise ValueError("Unsupported merge-group action or queue ref") - if group["head_ref"] != ref or group["head_sha"] != sha or event["repository"]["full_name"] != repository: - raise ValueError("The merge-group event does not match the tested repository, ref and SHA") - if not re.fullmatch(r"[0-9a-f]{40}", sha) or not re.fullmatch(r"[\w.-]+/[\w.-]+", repository, flags=re.ASCII): - raise ValueError("A full tested SHA and owner/repository name are required") - run_id, run_attempt = environment["GITHUB_RUN_ID"], environment["GITHUB_RUN_ATTEMPT"] - if not re.fullmatch(r"[1-9][0-9]*", run_id) or not re.fullmatch(r"[1-9][0-9]*", run_attempt): - raise ValueError("The current workflow run ID and attempt must be positive integers") - return MergeGroup(repository=repository, ref=ref, sha=sha, run_id=int(run_id), run_attempt=int(run_attempt)) - - -def _current_sha(*, api: GitHubAPI, group: MergeGroup) -> str | None: - ref = quote(group.ref.removeprefix("refs/"), safe="/") - refs = api.request(method="GET", path=f"{group.api_path}/git/matching-refs/{ref}", expected_status=200) - if not isinstance(refs, list) or any( - not isinstance(item, dict) or not isinstance(item.get("ref"), str) for item in refs - ): - raise ValueError("Invalid matching-refs response") - matches = [item for item in refs if item["ref"] == group.ref] - if not matches: - return None - if len(matches) != 1: - raise ValueError("Matching-refs returned more than one exact queue ref") - target = matches[0].get("object") - if not isinstance(target, dict): - raise ValueError("The queue ref has no valid commit object") - sha = target.get("sha") - if target.get("type") != "commit" or not isinstance(sha, str) or not re.fullmatch(r"[0-9a-f]{40}", sha): - raise ValueError("The queue ref does not point to a valid commit") - return sha - - -def _verify_run(*, api: GitHubAPI, group: MergeGroup) -> None: - run = api.request(method="GET", path=f"{group.api_path}/actions/runs/{group.run_id}", expected_status=200) - expected = { - "id": group.run_id, - "run_attempt": group.run_attempt, - "event": "merge_group", - "head_sha": group.sha, - "head_branch": group.ref.removeprefix("refs/heads/"), - "path": ".github/workflows/codeql.yml", - } - if not isinstance(run, dict) or any(run.get(key) != value for key, value in expected.items()): - raise ValueError("Refusing to cancel a different workflow run or attempt") - if run.get("status") == "completed" and run.get("conclusion") == "cancelled": - raise WorkflowCancelledError("The verified CodeQL workflow run is already cancelled") - if run.get("status") != "in_progress" or run.get("conclusion") is not None: - raise ValueError("Refusing to cancel a workflow run that is not in progress") - - -def _verify_no_failed_jobs(*, api: GitHubAPI, group: MergeGroup) -> None: - path = f"{group.api_path}/actions/runs/{group.run_id}/attempts/{group.run_attempt}/jobs?per_page=100" - result = api.request(method="GET", path=path, expected_status=200) - if not isinstance(result, dict) or not isinstance(result.get("jobs"), list): - raise ValueError("Invalid workflow jobs response") - jobs = result["jobs"] - if not jobs or result.get("total_count") != len(jobs): - raise ValueError("Cannot verify all CodeQL matrix jobs before cancellation") - for job in jobs: - if ( - not isinstance(job, dict) - or not isinstance(job.get("steps"), list) - or any(not isinstance(step, dict) for step in job["steps"]) - ): - raise ValueError("Invalid workflow job steps") - conclusions = [job.get("conclusion"), *(step.get("conclusion") for step in job["steps"])] - if any(value not in (None, "success", "skipped", "cancelled") for value in conclusions): - raise RuntimeError("A CodeQL matrix job or step failed; refusing to replace failure with cancellation") - - -def _retire(*, api: GitHubAPI, group: MergeGroup, current_sha: str | None) -> NoReturn: - _verify_run(api=api, group=group) - _verify_no_failed_jobs(api=api, group=group) - reason = "is absent" if current_sha is None else f"now points to {current_sha}" - print( - f"::notice::Verified obsolete merge candidate: {group.ref} {reason}; cancelling run {group.run_id}", flush=True - ) - try: - api.request(method="POST", path=f"{group.api_path}/actions/runs/{group.run_id}/cancel", expected_status=202) - except GitHubAPIError as error: - if error.status == 409: - _verify_run(api=api, group=group) - raise - # Stay in a cancellable step until the runner receives cancellation. Never return a green check. - time.sleep(60) - raise TimeoutError("Cancellation was accepted, but the runner did not stop within 60 seconds") - - -def _ensure_live(*, api: GitHubAPI, group: MergeGroup) -> None: - current_sha = _current_sha(api=api, group=group) - if current_sha != group.sha: - _retire(api=api, group=group, current_sha=current_sha) - - -def _missing_ref_message(group: MergeGroup) -> str: - return f"ref '{group.ref}' not found in this repository" - - -def _wait_for_processing(*, api: GitHubAPI, group: MergeGroup, sarif_id: str) -> None: - for delay in (5, 10, 20, 40, 80): - time.sleep(delay) - path = f"{group.api_path}/code-scanning/sarifs/{quote(sarif_id, safe='')}" - result = api.request(method="GET", path=path, expected_status=200) - if not isinstance(result, dict): - raise ValueError("Invalid SARIF processing response") - status = result.get("processing_status") - if status == "complete": - return - if status == "failed": - errors = result.get("errors") - if errors == [_missing_ref_message(group)]: - _ensure_live(api=api, group=group) - raise RuntimeError(f"SARIF processing failed: {errors}") - if status != "pending": - raise ValueError(f"Unsupported SARIF processing status: {status}") - _ensure_live(api=api, group=group) - raise TimeoutError("SARIF processing did not complete within five status checks") - - -def prepared_sarif(*, path: Path, category: str) -> bytes: - raw = path.read_bytes() - document = json.loads(raw) - if not isinstance(document, dict): - raise ValueError("Expected a prepared SARIF object") - runs = document.get("runs") - if document.get("version") != "2.1.0" or not isinstance(runs, list) or not runs: - raise ValueError("Expected a prepared SARIF 2.1.0 file with CodeQL runs") - for run in runs: - if ( - not isinstance(run, dict) - or not isinstance(run.get("tool"), dict) - or not isinstance(run["tool"].get("driver"), dict) - or not isinstance(run.get("automationDetails"), dict) - or run["tool"]["driver"].get("name") != "CodeQL" - or run["automationDetails"].get("id") != f"{category}/" - ): - raise ValueError("Prepared CodeQL SARIF does not match the matrix language category") - return raw - - -def publish_merge_group(*, api: GitHubAPI, group: MergeGroup, sarif: bytes, checkout: Path) -> str: - _ensure_live(api=api, group=group) - payload = { - "commit_sha": group.sha, - "ref": group.ref, - "sarif": base64.b64encode(gzip.compress(sarif, mtime=0)).decode("ascii"), - "checkout_uri": checkout.resolve().as_uri(), - "tool_name": "CodeQL", - } - try: - result = api.request( - method="POST", path=f"{group.api_path}/code-scanning/sarifs", expected_status=202, payload=payload - ) - except GitHubAPIError as error: - if error.status in (400, 404) and error.message == _missing_ref_message(group) and error.errors in (None, []): - _ensure_live(api=api, group=group) - raise - if not isinstance(result, dict): - raise ValueError("SARIF upload was accepted without a valid upload ID") - sarif_id = result.get("id") - if not isinstance(sarif_id, str) or not sarif_id: - raise ValueError("SARIF upload was accepted without a valid upload ID") - _wait_for_processing(api=api, group=group, sarif_id=sarif_id) - _ensure_live(api=api, group=group) - print(f"CodeQL SARIF {sarif_id} processed for {group.ref} at {group.sha}") - return sarif_id - - -def main(argv: list[str] | None = None) -> int: - parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument("--sarif-file", required=True, type=Path) - parser.add_argument( - "--category", - required=True, - choices=("/language:actions", "/language:javascript-typescript", "/language:python"), - ) - parser.add_argument("--analysis-outcome", required=True) - args = parser.parse_args(argv) - try: - if args.analysis_outcome != "success": - raise ValueError("Publishing requires successful CodeQL analysis") - group = load_merge_group(os.environ) - sarif = prepared_sarif(path=args.sarif_file, category=args.category) - api = GitHubAPI(api_url=os.environ["GITHUB_API_URL"], token=os.environ["GITHUB_TOKEN"]) - publish_merge_group(api=api, group=group, sarif=sarif, checkout=Path(os.environ["GITHUB_WORKSPACE"])) - except (WorkflowCancelledError, KeyboardInterrupt) as error: - print(f"CodeQL merge-group publication cancelled: {error}", file=sys.stderr) - return 130 - except (ValueError, KeyError, RuntimeError, OSError, http.client.HTTPException) as error: - message = str(error).replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A") - print(f"::error::{message}", file=sys.stderr) - return 1 - return 0 - - -if __name__ == "__main__": - raise SystemExit(main()) diff --git a/doc/contributing/4_running_tests.md b/doc/contributing/4_running_tests.md index 0e6161ce2f..a4d3bd2685 100644 --- a/doc/contributing/4_running_tests.md +++ b/doc/contributing/4_running_tests.md @@ -45,6 +45,8 @@ manual runs and its weekly schedule. Merge groups publish the analyzer's prepare SARIF through the documented code-scanning API so an expired queue ref can be distinguished from a genuine upload failure. Categories and fingerprints are preserved; failed analysis retains CodeQL's native diagnostic upload. +The merge-group helper uses the authenticated client and context supplied by the +pinned `actions/github-script` action, with no separate HTTP client or Python setup. Only a successful matching-refs response proving that the exact queue ref is absent or points to another SHA permits retirement. A missing-ref upload rejection is @@ -55,12 +57,16 @@ cancellation when a matrix job has already failed. The CodeQL job needs `actions: write` for self-cancellation; workflow-wide permissions remain empty. Fork PRs use native CodeQL publishing and GitHub's read-only fork token policy. -Run the deterministic, offline regression coverage with: +Run the deterministic, offline regression coverage with Node.js installed. +The pytest entry point runs the Node scenario tests and checks workflow wiring: ```bash uv run --frozen pytest tests/unit/build_scripts/test_codeql_merge_group.py -q ``` +The helper's scenarios can also run directly with +`node --test tests/unit/build_scripts/test_codeql_merge_group.cjs`. + ## Running a subset while iterating For a narrower run, invoke `pytest` directly. You can invoke pytest if it's in your path or via python; either `pytest` or `python -m pytest`. For the following examples, we will use `pytest`. diff --git a/tests/unit/build_scripts/test_codeql_merge_group.cjs b/tests/unit/build_scripts/test_codeql_merge_group.cjs new file mode 100644 index 0000000000..72c2315e09 --- /dev/null +++ b/tests/unit/build_scripts/test_codeql_merge_group.cjs @@ -0,0 +1,255 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +const assert = require("node:assert/strict"); +const { test } = require("node:test"); +const { mkdtemp, writeFile, rm } = require("node:fs/promises"); +const { tmpdir } = require("node:os"); +const path = require("node:path"); +const { gunzipSync } = require("node:zlib"); +const { pathToFileURL } = require("node:url"); +const publish = require(path.resolve(__dirname, "..", "..", "..", "build_scripts", "codeql_merge_group.cjs")); +const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor; +const script = process.env.CODEQL_TEST_SCRIPT; + +const SHA = "a".repeat(40); +const REF = `refs/heads/gh-readonly-queue/main/pr-42-${SHA}`; +const ID = "sarif-upload-id"; +const CATEGORY = "/language:python"; +const context = { + repo: { owner: "owner", repo: "repository" }, eventName: "merge_group", ref: REF, sha: SHA, runId: 123, + payload: { action: "checks_requested", merge_group: { base_ref: "refs/heads/main", head_ref: REF, head_sha: SHA } } +}; +const routes = { + ref: "GET /repos/{owner}/{repo}/git/matching-refs/{ref}", + run: "GET /repos/{owner}/{repo}/actions/runs/{run_id}", + jobs: "GET /repos/{owner}/{repo}/actions/runs/{run_id}/attempts/{attempt_number}/jobs", + cancel: "POST /repos/{owner}/{repo}/actions/runs/{run_id}/cancel", + upload: "POST /repos/{owner}/{repo}/code-scanning/sarifs", + processing: "GET /repos/{owner}/{repo}/code-scanning/sarifs/{sarif_id}" +}; +const sarif = { + version: "2.1.0", runs: [{ tool: { driver: { name: "CodeQL" } }, automationDetails: { id: `${CATEGORY}/` }, + results: [{ partialFingerprints: { primaryLocationLineHash: "existing-fingerprint" } }] }] +}; +const refs = (sha = SHA) => [{ ref: REF, object: { type: "commit", sha } }]; +const run = (changes = {}) => ({ + id: 123, run_attempt: 1, event: "merge_group", head_sha: SHA, head_branch: REF.slice(11), + path: ".github/workflows/codeql.yml", status: "in_progress", conclusion: null, ...changes +}); +const jobs = () => ({ + total_count: 3, jobs: Array.from({ length: 3 }, () => ({ conclusion: null, steps: [{ conclusion: "success" }] })) +}); +const apiError = (status, message = "Request failed", errors) => + Object.assign(new Error(`${message} - https://docs.github.com/rest`), { status, response: { data: { message, errors } } }); +const missing = () => apiError(404, `ref '${REF}' not found in this repository`); +const cancelled = new Error("runner cancelled"); +const live = [[routes.ref, refs()], [routes.upload, { id: ID }, 202], [routes.processing, { processing_status: "complete" }], + [routes.ref, refs()]]; +const obsolete = () => [[routes.ref, []], [routes.run, run()], [routes.jobs, jobs()]]; +const scenarios = []; +function scenario(name, replies, error, changes = {}) { + scenarios.push({ name, replies, error, ...changes }); +} + +scenario("live matching candidate preserves prepared SARIF", live); +for (const category of ["/language:actions", "/language:javascript-typescript"]) { + scenario(`live ${category} preserves its category`, live, undefined, { + document: { ...sarif, runs: [{ ...sarif.runs[0], automationDetails: { id: `${category}/` } }] }, + options: { category } + }); +} +const otherSha = "b".repeat(40); +scenario("queue name suffix is not the tested head SHA", [ + [routes.ref, refs(otherSha)], ...live.slice(1, 3), [routes.ref, refs(otherSha)] +], undefined, { + context: { sha: otherSha, payload: { ...context.payload, + merge_group: { ...context.payload.merge_group, head_sha: otherSha } } } +}); +for (const current of [[], refs("b".repeat(40)), [{ ref: REF + "-suffix" }]]) { + scenario(`obsolete ref ${JSON.stringify(current)}`, [[routes.ref, current], ...obsolete().slice(1), + [routes.cancel, {}, 202]], cancelled); +} +for (const status of [400, 404]) { + for (const current of [[], refs("b".repeat(40))]) { + scenario(`upload race ${status} ${JSON.stringify(current)}`, [[routes.ref, refs()], + [routes.upload, apiError(status, `ref '${REF}' not found in this repository`)], [routes.ref, current], + ...obsolete().slice(1), [routes.cancel, {}, 202]], cancelled); + } +} +scenario("missing-ref rejection while candidate remains live fails", [[routes.ref, refs()], [routes.upload, missing()], + [routes.ref, refs()]], /not found in this repository/); + +const stages = { + precheck: [[], routes.ref], + upload: [[[routes.ref, refs()]], routes.upload], + processing: [live.slice(0, 2), routes.processing], + recheck: [[[routes.ref, refs()], [routes.upload, missing()]], routes.ref], + run: [obsolete().slice(0, 1), routes.run], + jobs: [obsolete().slice(0, 2), routes.jobs], + cancel: [obsolete(), routes.cancel] +}; +for (const [stage, [prefix, route]] of Object.entries(stages)) { + for (const status of [401, 403, 404, 429, 500, 502, 503]) { + scenario(`${stage} HTTP ${status} remains a failure`, [...prefix, [route, apiError(status)]], /Request failed/); + } + scenario(`${stage} network failure remains a failure`, [...prefix, [route, new Error("network failed")]], /network failed/); +} +for (const error of [apiError(400, "Invalid SARIF"), apiError(404, "Not Found"), + apiError(422, `ref '${REF}' not found in this repository`), + apiError("404", `ref '${REF}' not found in this repository`), + apiError(400, `ref '${REF}' not found in this repository`, ["Invalid SARIF"])]) { + scenario(`genuine upload rejection ${error.message}`, [[routes.ref, refs()], [routes.upload, error]], error); +} +for (const status of ["pending", "complete", "failed"]) { + scenario(`expiry after accepted upload ${status}`, [...live.slice(0, 2), + [routes.processing, { processing_status: status, errors: [`ref '${REF}' not found in this repository`] }], + ...obsolete(), [routes.cancel, {}, 202]], cancelled); +} +for (const errors of [["Invalid SARIF"], [`ref '${REF}' not found in this repository`, "Invalid SARIF"]]) { + scenario(`processing rejection ${errors}`, [...live.slice(0, 2), + [routes.processing, { processing_status: "failed", errors }]], /SARIF processing failed/); +} +scenario("missing-ref processing rejection with live ref fails", [...live.slice(0, 2), + [routes.processing, { processing_status: "failed", errors: [`ref '${REF}' not found in this repository`] }], + [routes.ref, refs()]], /SARIF processing failed/); +scenario("bounded processing timeout", [...live.slice(0, 2), + ...Array.from({ length: 5 }, () => [[routes.processing, { processing_status: "pending" }], [routes.ref, refs()]]).flat()], + /five status checks/, { delays: [5000, 10000, 20000, 40000, 80000] }); +scenario("accepted cancellation cannot return success", [...obsolete(), [routes.cancel, {}, 202]], + /runner did not stop within 60 seconds/, { signal: false, delays: [60000] }); +scenario("already cancelled workflow", [[routes.ref, []], [routes.run, run({ status: "completed", conclusion: "cancelled" })]], + /already cancelled/); +scenario("cancellation conflict verifies existing cancellation", [...obsolete(), [routes.cancel, apiError(409)], + [routes.run, run({ status: "completed", conclusion: "cancelled" })]], /already cancelled/); +scenario("unexplained cancellation conflict stays failure", [...obsolete(), [routes.cancel, apiError(409)], + [routes.run, run()]], /Request failed/); +for (const change of [{ id: 999 }, { run_attempt: 2 }, { event: "push" }, { head_sha: "b".repeat(40) }, + { head_branch: "main" }, { path: ".github/workflows/other.yml" }, + { status: "completed", conclusion: "failure" }, { status: "completed", conclusion: "success" }]) { + scenario(`cannot cancel unrelated or completed run ${JSON.stringify(change)}`, [[routes.ref, []], [routes.run, run(change)]], + /Refusing to cancel/); +} +for (const conclusion of ["failure", "timed_out", "startup_failure", "action_required"]) { + for (const location of ["job", "step"]) { + const result = jobs(); + if (location === "job") result.jobs[0].conclusion = conclusion; + else result.jobs[0].steps[0].conclusion = conclusion; + scenario(`${location} ${conclusion} cannot be hidden by retirement`, + [[routes.ref, []], [routes.run, run()], [routes.jobs, result]], /refusing to replace failure/); + } +} +for (const result of [{ total_count: 101, jobs: jobs().jobs }, { total_count: 0, jobs: [] }, + { total_count: 1, jobs: [{ steps: {} }] }, { total_count: 1, jobs: [{ steps: [null] }] }]) { + scenario(`invalid job listing ${JSON.stringify(result)}`, [[routes.ref, []], [routes.run, run()], [routes.jobs, result]], + /Cannot verify|Invalid/); +} +for (const outcome of ["failure", "cancelled", "skipped", ""]) { + scenario(`analysis outcome ${outcome} cannot publish or cancel`, [], /successful CodeQL analysis/, + { options: { analysisOutcome: outcome } }); +} +for (const change of [{ eventName: "push" }, { ref: "refs/heads/main" }, { sha: "b".repeat(40) }, { runId: 0 }, + { eventName: "pull_request" }, { eventName: "schedule" }, { eventName: "workflow_dispatch" }, + { payload: { action: "destroyed" } }, + { ref: "refs/heads/gh-readonly-queue/main/", payload: { ...context.payload, + merge_group: { ...context.payload.merge_group, head_ref: "refs/heads/gh-readonly-queue/main/" } } }]) { + scenario(`invalid context ${JSON.stringify(change)}`, [], /merge-group context/, { context: change }); +} +scenario("invalid attempt cannot cancel another run", [], /merge-group context/, { options: { runAttempt: 0 } }); +for (const result of [null, {}, [{ ref: REF, object: null }], [...refs(), ...refs()], + [{ ref: REF, object: { type: "tag", sha: SHA } }], [{ ref: REF, object: { type: "commit", sha: "short" } }]]) { + scenario(`invalid ref response ${JSON.stringify(result)}`, [[routes.ref, result]], /Invalid|multiple|valid commit/); +} +for (const document of [[], { version: "2.1.0", runs: [] }, { version: "2.1.0", runs: [{ tool: null }] }, + { ...sarif, runs: [{ ...sarif.runs[0], automationDetails: { id: "/language:actions/" } }] }]) { + scenario(`invalid prepared SARIF ${JSON.stringify(document)}`, [], /Invalid|Expected prepared/, { document }); +} +scenario("malformed JSON fails before API calls", [], /JSON/, { raw: "not JSON" }); +scenario("unknown processing state fails", [...live.slice(0, 2), [routes.processing, { processing_status: "other" }]], + /Invalid SARIF processing status/); +for (const result of [{}, { id: "" }, { id: 123 }]) { + scenario(`upload ID must be valid ${JSON.stringify(result)}`, [[routes.ref, refs()], [routes.upload, result, 202]], + /without a valid ID/); +} +scenario("unexpected successful HTTP status fails", [[routes.ref, refs(), 201]], /Unexpected HTTP/); +scenario("runner cancellation does not continue upload", [[routes.ref, refs()], [routes.upload, cancelled]], cancelled); + +for (const item of scenarios) { + test(item.name, async (t) => { + const directory = await mkdtemp(path.join(tmpdir(), "pyrit-codeql-")); + t.after(() => rm(directory, { recursive: true, force: true })); + const file = path.join(directory, "upload.sarif"); + const prepared = item.raw ?? JSON.stringify(item.document ?? sarif); + await writeFile(file, prepared); + const replies = [...item.replies]; + const calls = []; + const delays = []; + const notices = []; + const github = { request: async (route, options) => { + calls.push({ route, options }); + assert.ok(replies.length, `Unexpected API call: ${route}`); + const [expected, data, status = 200] = replies.shift(); + assert.equal(route, expected); + assert.equal(options.owner, context.repo.owner); + assert.equal(options.repo, context.repo.repo); + assert.equal(options.request.timeout, 30000); + if (route === routes.ref) assert.equal(options.ref, REF.slice(5)); + if ([routes.run, routes.jobs, routes.cancel].includes(route)) assert.equal(options.run_id, 123); + if (route === routes.jobs) assert.deepEqual([options.attempt_number, options.per_page], [1, 100]); + if (data instanceof Error) throw data; + return { status, data }; + } }; + const actualContext = { ...context, ...item.context }; + const options = { + github, context: actualContext, core: { notice: (message) => notices.push(message), info: () => {} }, + sarifFile: file, category: CATEGORY, analysisOutcome: "success", runAttempt: 1, checkout: directory, + sleep: async (milliseconds) => { + delays.push(milliseconds); + if (milliseconds === 60000 && item.signal !== false) throw cancelled; + }, ...item.options + }; + let resultId; + const execute = async (args) => { + resultId = await publish({ ...args, sleep: options.sleep }); + return resultId; + }; + const invoke = async () => { + if (!script) return execute(options); + Object.assign(process.env, { + SARIF_FILE: file, SARIF_CATEGORY: options.category, ANALYSIS_OUTCOME: options.analysisOutcome, + GITHUB_RUN_ATTEMPT: String(options.runAttempt), GITHUB_WORKSPACE: directory + }); + const requireForScript = (name) => { + if (name === "node:path") return path; + assert.equal(name, path.resolve("build_scripts", "codeql_merge_group.cjs")); + return execute; + }; + await new AsyncFunction("require", "github", "context", "core", script)( + requireForScript, github, actualContext, options.core + ); + }; + if (item.error) { + await assert.rejects(invoke, item.error instanceof Error ? (error) => error === item.error : item.error); + assert.equal(resultId, undefined); + } else { + await invoke(); + assert.equal(resultId, ID); + assert.deepEqual(delays, [5000]); + assert.equal(notices.length, 0); + } + assert.equal(replies.length, 0, "All expected API calls must be consumed"); + if (item.delays) assert.deepEqual(delays, item.delays); + for (const { route, options } of calls.filter((call) => call.route === routes.upload)) { + assert.equal(route, routes.upload); + assert.equal(options.commit_sha, actualContext.sha); + assert.equal(options.ref, REF); + assert.equal(options.tool_name, "CodeQL"); + assert.equal(options.checkout_uri, pathToFileURL(directory).href); + assert.equal(gunzipSync(Buffer.from(options.sarif, "base64")).toString("utf8"), prepared); + } + if (item.error === cancelled && calls.at(-1)?.route === routes.cancel) { + assert.equal(notices.length, 1); + assert.deepEqual(delays.at(-1), 60000); + } + }); +} diff --git a/tests/unit/build_scripts/test_codeql_merge_group.py b/tests/unit/build_scripts/test_codeql_merge_group.py index 12e3c1c9e2..08be155345 100644 --- a/tests/unit/build_scripts/test_codeql_merge_group.py +++ b/tests/unit/build_scripts/test_codeql_merge_group.py @@ -1,485 +1,23 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT license. -"""Offline contracts for publishing live CodeQL candidates and explicitly retiring obsolete ones.""" +"""Run the real merge-group helper offline and preserve CodeQL workflow contracts.""" from __future__ import annotations -import base64 -import gzip -import http.client -import json -import os -import re +import subprocess from pathlib import Path from typing import Any -from unittest.mock import MagicMock, patch +from unittest.mock import patch import pytest import yaml -from build_scripts import codeql_merge_group as publisher - REPO_ROOT = Path(__file__).resolve().parents[3] -SHA = "a" * 40 -REF = f"refs/heads/gh-readonly-queue/main/pr-42-{SHA}" -REPOSITORY = "owner/repository" -API_PATH = f"/repos/{REPOSITORY}" -REF_PATH = f"{API_PATH}/git/matching-refs/{REF.removeprefix('refs/')}" -RUN_PATH = f"{API_PATH}/actions/runs/123" -JOBS_PATH = f"{RUN_PATH}/attempts/1/jobs?per_page=100" -UPLOAD_PATH = f"{API_PATH}/code-scanning/sarifs" -UPLOAD_ID = "sarif-upload-id" -CATEGORY = "/language:python" - - -@pytest.fixture -def group() -> publisher.MergeGroup: - return publisher.MergeGroup(repository=REPOSITORY, ref=REF, sha=SHA, run_id=123, run_attempt=1) - - -@pytest.fixture -def api() -> MagicMock: - return MagicMock(spec=publisher.GitHubAPI) - - -@pytest.fixture -def sarif_file(tmp_path: Path) -> Path: - path = tmp_path / "upload.sarif" - path.write_text( - json.dumps( - { - "version": "2.1.0", - "runs": [ - { - "tool": {"driver": {"name": "CodeQL"}}, - "automationDetails": {"id": CATEGORY + "/"}, - "results": [{"partialFingerprints": {"primaryLocationLineHash": "existing-fingerprint"}}], - } - ], - } - ), - encoding="utf-8", - ) - return path - - -@pytest.fixture -def environment(tmp_path: Path) -> dict[str, str]: - event = tmp_path / "event.json" - event.write_text( - json.dumps( - { - "action": "checks_requested", - "merge_group": {"base_ref": "refs/heads/main", "head_ref": REF, "head_sha": SHA}, - "repository": {"full_name": REPOSITORY}, - } - ), - encoding="utf-8", - ) - return { - "GITHUB_EVENT_NAME": "merge_group", - "GITHUB_EVENT_PATH": str(event), - "GITHUB_REPOSITORY": REPOSITORY, - "GITHUB_REF": REF, - "GITHUB_SHA": SHA, - "GITHUB_RUN_ID": "123", - "GITHUB_RUN_ATTEMPT": "1", - "GITHUB_API_URL": "https://api.github.com", - "GITHUB_TOKEN": "offline-test-token", - "GITHUB_WORKSPACE": str(tmp_path), - } - - -def _refs(sha: str = SHA) -> list[dict[str, Any]]: - return [{"ref": REF, "object": {"type": "commit", "sha": sha}}] - - -def _run(**changes: Any) -> dict[str, Any]: - return { - "id": 123, - "run_attempt": 1, - "event": "merge_group", - "head_sha": SHA, - "head_branch": REF.removeprefix("refs/heads/"), - "path": ".github/workflows/codeql.yml", - "status": "in_progress", - "conclusion": None, - **changes, - } - - -def _jobs() -> dict[str, Any]: - return {"total_count": 3, "jobs": [{"conclusion": None, "steps": [{"conclusion": "success"}]} for _ in range(3)]} - - -def _request_paths(api: MagicMock) -> list[tuple[str, str]]: - return [(call.kwargs["method"], call.kwargs["path"]) for call in api.request.call_args_list] - - -def _publish(*, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path) -> str: - return publisher.publish_merge_group( - api=api, - group=group, - sarif=publisher.prepared_sarif(path=sarif_file, category=CATEGORY), - checkout=sarif_file.parent, - ) - - -def _main(*, environment: dict[str, str], sarif_file: Path, outcome: str = "success") -> int: - with patch.dict(os.environ, environment, clear=True): - return publisher.main(["--sarif-file", str(sarif_file), "--category", CATEGORY, "--analysis-outcome", outcome]) - - -def test_live_matching_ref_uploads_exact_prepared_sarif( - *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path -) -> None: - api.request.side_effect = [_refs(), {"id": UPLOAD_ID}, {"processing_status": "complete"}, _refs()] - with patch.object(publisher.time, "sleep") as sleep: - assert _publish(api=api, group=group, sarif_file=sarif_file) == UPLOAD_ID - assert _request_paths(api) == [ - ("GET", REF_PATH), - ("POST", UPLOAD_PATH), - ("GET", f"{UPLOAD_PATH}/{UPLOAD_ID}"), - ("GET", REF_PATH), - ] - upload = api.request.call_args_list[1].kwargs - assert upload["expected_status"] == 202 - payload = upload["payload"] - assert payload["commit_sha"] == SHA - assert payload["ref"] == REF - assert payload["tool_name"] == "CodeQL" - assert payload["checkout_uri"] == sarif_file.parent.resolve().as_uri() - assert gzip.decompress(base64.b64decode(payload["sarif"])) == sarif_file.read_bytes() - sleep.assert_called_once_with(5) - - -@pytest.mark.parametrize("refs", [[], _refs("b" * 40), [{"ref": REF + "-suffix", "object": {"sha": SHA}}]]) -def test_verified_absent_or_changed_ref_requests_cancellation_without_upload( - *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path, refs: list[dict[str, Any]] -) -> None: - api.request.side_effect = [refs, _run(), _jobs(), {}] - with patch.object(publisher.time, "sleep", side_effect=KeyboardInterrupt): - with pytest.raises(KeyboardInterrupt): - _publish(api=api, group=group, sarif_file=sarif_file) - assert _request_paths(api) == [ - ("GET", REF_PATH), - ("GET", RUN_PATH), - ("GET", JOBS_PATH), - ("POST", RUN_PATH + "/cancel"), - ] - assert api.request.call_args.kwargs["expected_status"] == 202 - - -@pytest.mark.parametrize("status", [400, 404]) -@pytest.mark.parametrize("current_refs", [[], _refs("b" * 40)]) -def test_disappearance_between_precheck_and_upload_is_rechecked_then_cancelled( - *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path, status: int, current_refs: list[dict[str, Any]] -) -> None: - rejection = publisher.GitHubAPIError(status=status, message=f"ref '{REF}' not found in this repository") - api.request.side_effect = [_refs(), rejection, current_refs, _run(), _jobs(), {}] - with patch.object(publisher.time, "sleep", side_effect=KeyboardInterrupt): - with pytest.raises(KeyboardInterrupt): - _publish(api=api, group=group, sarif_file=sarif_file) - assert _request_paths(api) == [ - ("GET", REF_PATH), - ("POST", UPLOAD_PATH), - ("GET", REF_PATH), - ("GET", RUN_PATH), - ("GET", JOBS_PATH), - ("POST", RUN_PATH + "/cancel"), - ] - - -def test_missing_ref_rejection_with_still_live_candidate_remains_failure( - *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path -) -> None: - rejection = publisher.GitHubAPIError(status=404, message=f"ref '{REF}' not found in this repository") - api.request.side_effect = [_refs(), rejection, _refs()] - with pytest.raises(publisher.GitHubAPIError) as raised: - _publish(api=api, group=group, sarif_file=sarif_file) - assert raised.value is rejection - assert _request_paths(api) == [("GET", REF_PATH), ("POST", UPLOAD_PATH), ("GET", REF_PATH)] - - -@pytest.mark.parametrize("stage", ["precheck", "upload", "processing", "recheck", "cancel"]) -@pytest.mark.parametrize("status", [401, 403, 404, 429, 500, 502, 503]) -def test_api_errors_never_become_obsolete_candidate_success( - *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path, stage: str, status: int -) -> None: - error = publisher.GitHubAPIError(status=status, message="authentication, rate limit or service failure") - prefixes: dict[str, list[Any]] = { - "precheck": [], - "upload": [_refs()], - "processing": [_refs(), {"id": UPLOAD_ID}], - "recheck": [ - _refs(), - publisher.GitHubAPIError(status=404, message=f"ref '{REF}' not found in this repository"), - ], - "cancel": [[], _run(), _jobs()], - } - api.request.side_effect = [*prefixes[stage], error] - with patch.object(publisher.time, "sleep"): - with pytest.raises(publisher.GitHubAPIError) as raised: - _publish(api=api, group=group, sarif_file=sarif_file) - assert raised.value is error - cancellations = [path for method, path in _request_paths(api) if method == "POST" and path.endswith("/cancel")] - assert cancellations == ([RUN_PATH + "/cancel"] if stage == "cancel" else []) - - -@pytest.mark.parametrize("stage", ["precheck", "upload", "processing", "cancel"]) -@pytest.mark.parametrize("error", [OSError("network unavailable"), http.client.HTTPException("connection failed")]) -def test_network_errors_remain_failures( - *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path, stage: str, error: Exception -) -> None: - prefixes: dict[str, list[Any]] = { - "precheck": [], - "upload": [_refs()], - "processing": [_refs(), {"id": UPLOAD_ID}], - "cancel": [[], _run(), _jobs()], - } - api.request.side_effect = [*prefixes[stage], error] - with patch.object(publisher.time, "sleep"): - with pytest.raises(type(error)) as raised: - _publish(api=api, group=group, sarif_file=sarif_file) - assert raised.value is error - - -@pytest.mark.parametrize( - "error", - [ - publisher.GitHubAPIError(status=400, message="Invalid SARIF"), - publisher.GitHubAPIError(status=404, message="Not Found"), - publisher.GitHubAPIError(status=422, message=f"ref '{REF}' not found in this repository"), - publisher.GitHubAPIError( - status=400, message=f"ref '{REF}' not found in this repository", errors=["Invalid SARIF"] - ), - ], -) -def test_genuine_or_unsupported_upload_rejection_does_not_consult_expiry( - *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path, error: publisher.GitHubAPIError -) -> None: - api.request.side_effect = [_refs(), error] - with pytest.raises(publisher.GitHubAPIError) as raised: - _publish(api=api, group=group, sarif_file=sarif_file) - assert raised.value is error - assert _request_paths(api) == [("GET", REF_PATH), ("POST", UPLOAD_PATH)] - - -@pytest.mark.parametrize("status", ["pending", "complete", "failed"]) -def test_expiry_after_accepted_upload_is_not_reported_as_success( - *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path, status: str -) -> None: - response: dict[str, Any] = {"processing_status": status} - if status == "failed": - response["errors"] = [f"ref '{REF}' not found in this repository"] - api.request.side_effect = [_refs(), {"id": UPLOAD_ID}, response, [], _run(), _jobs(), {}] - with patch.object(publisher.time, "sleep", side_effect=[None, KeyboardInterrupt]): - with pytest.raises(KeyboardInterrupt): - _publish(api=api, group=group, sarif_file=sarif_file) - assert _request_paths(api)[-4:] == [ - ("GET", REF_PATH), - ("GET", RUN_PATH), - ("GET", JOBS_PATH), - ("POST", RUN_PATH + "/cancel"), - ] - - -@pytest.mark.parametrize("errors", [["Invalid SARIF"], [f"ref '{REF}' not found in this repository", "Invalid SARIF"]]) -def test_processing_rejection_remains_failure_without_expiry_fallback( - *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path, errors: list[str] -) -> None: - api.request.side_effect = [_refs(), {"id": UPLOAD_ID}, {"processing_status": "failed", "errors": errors}] - with patch.object(publisher.time, "sleep"): - with pytest.raises(RuntimeError, match="SARIF processing failed"): - _publish(api=api, group=group, sarif_file=sarif_file) - assert _request_paths(api) == [("GET", REF_PATH), ("POST", UPLOAD_PATH), ("GET", f"{UPLOAD_PATH}/{UPLOAD_ID}")] - - -def test_processing_timeout_is_bounded_and_fails( - *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path -) -> None: - api.request.side_effect = [ - _refs(), - {"id": UPLOAD_ID}, - *[item for _ in range(5) for item in ({"processing_status": "pending"}, _refs())], - ] - with patch.object(publisher.time, "sleep") as sleep: - with pytest.raises(TimeoutError, match="five status checks"): - _publish(api=api, group=group, sarif_file=sarif_file) - assert [call.args[0] for call in sleep.call_args_list] == [5, 10, 20, 40, 80] - assert len(_request_paths(api)) == 12 - assert not any(path.endswith("/cancel") for _, path in _request_paths(api)) +CODEQL_PIN = "2892aa5e19bbd11bc0cff5427e3b750a04d9e3c2" -def test_accepted_cancellation_never_returns_a_successful_check( - *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path -) -> None: - api.request.side_effect = [[], _run(), _jobs(), {}] - with patch.object(publisher.time, "sleep") as sleep: - with pytest.raises(TimeoutError, match="runner did not stop within 60 seconds"): - _publish(api=api, group=group, sarif_file=sarif_file) - sleep.assert_called_once_with(60) - assert _request_paths(api)[-1] == ("POST", RUN_PATH + "/cancel") - - -@pytest.mark.parametrize( - "changes", - [ - {"id": 999}, - {"run_attempt": 2}, - {"event": "push"}, - {"head_sha": "b" * 40}, - {"head_branch": "main"}, - {"path": ".github/workflows/other.yml"}, - {"status": "completed", "conclusion": "failure"}, - {"status": "completed", "conclusion": "success"}, - ], -) -def test_retirement_cannot_target_another_run_or_override_completed_outcomes( - *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path, changes: dict[str, Any] -) -> None: - api.request.side_effect = [[], _run(**changes)] - with pytest.raises(ValueError, match="Refusing to cancel"): - _publish(api=api, group=group, sarif_file=sarif_file) - assert _request_paths(api) == [("GET", REF_PATH), ("GET", RUN_PATH)] - - -@pytest.mark.parametrize("conclusion", ["failure", "timed_out", "startup_failure", "action_required"]) -@pytest.mark.parametrize("location", ["job", "step"]) -def test_sibling_analysis_failures_block_retirement( - *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path, conclusion: str, location: str -) -> None: - jobs = _jobs() - if location == "job": - jobs["jobs"][0]["conclusion"] = conclusion - else: - jobs["jobs"][0]["steps"][0]["conclusion"] = conclusion - api.request.side_effect = [[], _run(), jobs] - with pytest.raises(RuntimeError, match="refusing to replace failure with cancellation"): - _publish(api=api, group=group, sarif_file=sarif_file) - assert _request_paths(api) == [("GET", REF_PATH), ("GET", RUN_PATH), ("GET", JOBS_PATH)] - - -def test_incomplete_job_listing_cannot_authorize_cancellation( - *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path -) -> None: - jobs = _jobs() - jobs["total_count"] = 101 - api.request.side_effect = [[], _run(), jobs] - with pytest.raises(ValueError, match="Cannot verify all"): - _publish(api=api, group=group, sarif_file=sarif_file) - assert _request_paths(api)[-1] == ("GET", JOBS_PATH) - - -@pytest.mark.parametrize("conflict", [False, True]) -def test_already_cancelled_workflow_has_nonzero_cli_outcome( - *, api: MagicMock, environment: dict[str, str], sarif_file: Path, conflict: bool -) -> None: - cancelled = _run(status="completed", conclusion="cancelled") - responses: list[Any] = [[], cancelled] - if conflict: - responses = [[], _run(), _jobs(), publisher.GitHubAPIError(status=409, message="Conflict"), cancelled] - api.request.side_effect = responses - with patch.object(publisher, "GitHubAPI", return_value=api): - assert _main(environment=environment, sarif_file=sarif_file) == 130 - assert _request_paths(api)[-1] == ("GET", RUN_PATH) - - -@pytest.mark.parametrize("outcome", ["failure", "cancelled", "skipped", ""]) -def test_analyzer_failure_or_nonexecution_cannot_publish_or_cancel( - *, environment: dict[str, str], sarif_file: Path, outcome: str -) -> None: - with patch.object(publisher, "GitHubAPI") as constructor: - assert _main(environment=environment, sarif_file=sarif_file, outcome=outcome) == 1 - constructor.assert_not_called() - - -def test_runner_cancellation_is_nonzero_and_does_not_continue_publishing( - *, api: MagicMock, environment: dict[str, str], sarif_file: Path -) -> None: - api.request.side_effect = [[], _run(), _jobs(), {}] - with ( - patch.object(publisher, "GitHubAPI", return_value=api), - patch.object(publisher.time, "sleep", side_effect=KeyboardInterrupt), - ): - assert _main(environment=environment, sarif_file=sarif_file) == 130 - assert _request_paths(api)[-1] == ("POST", RUN_PATH + "/cancel") - - -@pytest.mark.parametrize( - ("key", "value"), - [ - ("GITHUB_EVENT_NAME", "push"), - ("GITHUB_REF", "refs/heads/main"), - ("GITHUB_SHA", "b" * 40), - ("GITHUB_REPOSITORY", "other/repository"), - ("GITHUB_RUN_ID", "0"), - ("GITHUB_RUN_ATTEMPT", "-1"), - ], -) -def test_invalid_event_identity_fails_before_api_access( - *, environment: dict[str, str], sarif_file: Path, key: str, value: str -) -> None: - environment[key] = value - with patch.object(publisher, "GitHubAPI") as constructor: - assert _main(environment=environment, sarif_file=sarif_file) == 1 - constructor.assert_not_called() - - -def test_event_identifies_exact_tested_merge_group(environment: dict[str, str]) -> None: - assert publisher.load_merge_group(environment) == publisher.MergeGroup( - repository=REPOSITORY, ref=REF, sha=SHA, run_id=123, run_attempt=1 - ) - - -def test_queue_branch_suffix_is_not_used_as_the_tested_head_sha(environment: dict[str, str]) -> None: - environment["GITHUB_SHA"] = "b" * 40 - event_path = Path(environment["GITHUB_EVENT_PATH"]) - event = json.loads(event_path.read_text(encoding="utf-8")) - event["merge_group"]["head_sha"] = environment["GITHUB_SHA"] - event_path.write_text(json.dumps(event), encoding="utf-8") - group = publisher.load_merge_group(environment) - assert group.ref == REF - assert group.sha == "b" * 40 - - -def test_wrong_sarif_category_cannot_publish_or_retire(*, environment: dict[str, str], sarif_file: Path) -> None: - sarif_file.write_text( - sarif_file.read_text(encoding="utf-8").replace(CATEGORY, "/language:actions"), encoding="utf-8" - ) - with patch.object(publisher, "GitHubAPI") as constructor: - assert _main(environment=environment, sarif_file=sarif_file) == 1 - constructor.assert_not_called() - - -@pytest.mark.parametrize("status", [301, 302, 400, 401, 403, 404, 429, 500, 503]) -def test_http_transport_retains_status_and_never_follows_redirects(status: int) -> None: - connection = MagicMock(spec=http.client.HTTPSConnection) - response = MagicMock(spec=http.client.HTTPResponse) - response.status = status - response.read.return_value = json.dumps({"message": "Request rejected"}).encode() - connection.getresponse.return_value = response - with patch.object(publisher.http.client, "HTTPSConnection", return_value=connection) as constructor: - client = publisher.GitHubAPI(api_url="https://github.example/api/v3", token="offline-test-token") - with pytest.raises(publisher.GitHubAPIError) as raised: - client.request(method="GET", path=REF_PATH, expected_status=200) - assert raised.value.status == status - assert raised.value.message == "Request rejected" - constructor.assert_called_once_with("github.example", timeout=30) - assert connection.request.call_args.args[:2] == ("GET", "/api/v3" + REF_PATH) - connection.request.assert_called_once() - connection.close.assert_called_once() - - -@pytest.mark.parametrize( - "api_url", ["http://github.example", "https://token@github.example", "https://github.example?x=1"] -) -def test_http_transport_rejects_insecure_or_credential_bearing_urls(api_url: str) -> None: - with pytest.raises(ValueError, match="HTTPS"): - publisher.GitHubAPI(api_url=api_url, token="offline-test-token") - - -def test_workflow_preserves_required_names_languages_events_and_native_diagnostics() -> None: +@pytest.fixture(scope="module") +def steps() -> dict[str, Any]: workflow = yaml.load( (REPO_ROOT / ".github" / "workflows" / "codeql.yml").read_text(encoding="utf-8"), Loader=yaml.BaseLoader ) @@ -496,159 +34,70 @@ def test_workflow_preserves_required_names_languages_events_and_native_diagnosti "matrix": {"language": ["actions", "javascript-typescript", "python"]}, } assert job["permissions"] == {"actions": "write", "contents": "read", "security-events": "write"} - assert "if" not in job - assert "continue-on-error" not in job + assert "if" not in job and "continue-on-error" not in job assert all("continue-on-error" not in step for step in job["steps"]) - steps = {step["name"]: step for step in job["steps"]} + return {step["name"]: step for step in job["steps"]} + + +def test_codeql_merge_group_scenarios(steps: dict[str, Any]) -> None: + script = steps["Publish merge-group CodeQL results"]["with"]["script"] + with patch.dict("os.environ", {"CODEQL_TEST_SCRIPT": script}): + result = subprocess.run( + ["node", "--test", "--test-reporter=tap", str(Path(__file__).with_suffix(".cjs"))], + cwd=REPO_ROOT, + capture_output=True, + text=True, + check=False, + timeout=60, + ) + assert result.returncode == 0, result.stdout + result.stderr + assert "# fail 0" in result.stdout and "# skipped 0" in result.stdout + + +def test_codeql_workflow_analysis_and_upload_wiring(steps: dict[str, Any]) -> None: assert steps["Checkout"]["with"] == {"ref": "${{ github.sha }}", "persist-credentials": "false"} analyze = steps["Perform CodeQL Analysis"] assert "if" not in analyze - assert analyze["uses"] == "github/codeql-action/analyze@2892aa5e19bbd11bc0cff5427e3b750a04d9e3c2" - assert analyze["with"]["category"] == "/language:${{ matrix.language }}" - assert analyze["with"]["upload"] == "failure-only" - assert analyze["with"]["post-processed-sarif-path"] == "${{ runner.temp }}/codeql-upload" + assert analyze["id"] == "analysis" + assert analyze["uses"] == f"github/codeql-action/analyze@{CODEQL_PIN}" + assert steps["Initialize CodeQL"]["uses"] == f"github/codeql-action/init@{CODEQL_PIN}" + assert steps["Initialize CodeQL"]["with"] == {"languages": "${{ matrix.language }}", "build-mode": "none"} + assert analyze["with"] == { + "category": "/language:${{ matrix.language }}", + "upload": "failure-only", + "post-processed-sarif-path": "${{ runner.temp }}/codeql-upload", + } native = steps["Publish CodeQL results"] - assert native["uses"] == "github/codeql-action/upload-sarif@2892aa5e19bbd11bc0cff5427e3b750a04d9e3c2" + assert native["uses"] == f"github/codeql-action/upload-sarif@{CODEQL_PIN}" assert native["with"] == { "sarif_file": "${{ runner.temp }}/codeql-upload/upload.sarif", "category": analyze["with"]["category"], } - assert native["if"] == "github.event_name != 'merge_group'" publish = steps["Publish merge-group CodeQL results"] - assert publish["if"] == "github.event_name == 'merge_group'" - assert steps["Set up merge-group upload helper"]["if"] == publish["if"] - assert publish["env"]["ANALYSIS_OUTCOME"] == "${{ steps.analysis.outcome }}" - assert publish["env"]["SARIF_FILE"] == "${{ runner.temp }}/codeql-upload/upload.sarif" - assert publish["env"]["SARIF_CATEGORY"] == analyze["with"]["category"] + assert publish["uses"] == "actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3" + assert publish["with"]["retries"] == "0" assert publish["timeout-minutes"] == "8" - assert "uv run --no-project python -m build_scripts.codeql_merge_group" in publish["run"] + assert publish["env"] == { + "ANALYSIS_OUTCOME": "${{ steps.analysis.outcome }}", + "SARIF_FILE": "${{ runner.temp }}/codeql-upload/upload.sarif", + "SARIF_CATEGORY": analyze["with"]["category"], + } + script = publish["with"]["script"] + for value in ("codeql_merge_group.cjs", "github, context, core", "await publish(", "process.env.ANALYSIS_OUTCOME"): + assert value in script + assert "process.env.GITHUB_RUN_ATTEMPT" in script + assert "process.env.GITHUB_WORKSPACE" in script + assert "github-token" not in publish["with"] + assert not any("setup-uv@" in step.get("uses", "") for step in steps.values()) -@pytest.mark.parametrize( - ("event_name", "fork"), - [ - ("push", False), - ("pull_request", False), - ("pull_request", True), - ("schedule", False), - ("workflow_dispatch", False), - ("merge_group", False), - ], -) -def test_normal_and_fork_events_select_native_upload_and_never_retirement(*, event_name: str, fork: bool) -> None: - workflow = yaml.load( - (REPO_ROOT / ".github" / "workflows" / "codeql.yml").read_text(encoding="utf-8"), Loader=yaml.BaseLoader - ) - steps = {step["name"]: step for step in workflow["jobs"]["analyze"]["steps"]} +@pytest.mark.parametrize("event", ["push", "pull_request", "schedule", "workflow_dispatch", "merge_group"]) +def test_native_and_merge_group_publishers_are_mutually_exclusive(*, steps: dict[str, Any], event: str) -> None: selected = [] - for name in ("Publish CodeQL results", "Set up merge-group upload helper", "Publish merge-group CodeQL results"): - condition = steps[name]["if"] - comparison = re.fullmatch(r"github\.event_name (==|!=) '([^']+)'", condition) - assert comparison is not None - matches = event_name == comparison[2] - if matches == (comparison[1] == "=="): + for name, operator in (("Publish CodeQL results", "!="), ("Publish merge-group CodeQL results", "==")): + assert steps[name]["if"] == f"github.event_name {operator} 'merge_group'" + if (event == "merge_group") == (operator == "=="): selected.append(name) - if event_name == "merge_group": - assert selected == ["Set up merge-group upload helper", "Publish merge-group CodeQL results"] - else: - assert selected == ["Publish CodeQL results"] - if fork: - assert "pull_request_target" not in workflow["on"] - assert not {"ref", "sha", "token"} & steps["Publish CodeQL results"]["with"].keys() - - -@pytest.mark.parametrize( - "error", - [ - publisher.GitHubAPIError(status=403, message="Forbidden"), - publisher.GitHubAPIError(status=429, message="Rate limited"), - publisher.GitHubAPIError(status=503, message="Unavailable"), - OSError("Network failure"), - ], -) -def test_cli_reports_api_and_network_failures_as_nonzero( - *, - api: MagicMock, - environment: dict[str, str], - sarif_file: Path, - error: Exception, - capsys: pytest.CaptureFixture[str], -) -> None: - api.request.side_effect = error - with patch.object(publisher, "GitHubAPI", return_value=api): - assert _main(environment=environment, sarif_file=sarif_file) == 1 - assert "::error::" in capsys.readouterr().err - assert _request_paths(api) == [("GET", REF_PATH)] - - -def test_cli_only_succeeds_after_analysis_and_live_upload_processing( - *, api: MagicMock, environment: dict[str, str], sarif_file: Path -) -> None: - api.request.side_effect = [_refs(), {"id": UPLOAD_ID}, {"processing_status": "complete"}, _refs()] - with patch.object(publisher, "GitHubAPI", return_value=api), patch.object(publisher.time, "sleep"): - assert _main(environment=environment, sarif_file=sarif_file) == 0 - assert _request_paths(api)[-1] == ("GET", REF_PATH) - - -@pytest.mark.parametrize( - "response", - [None, {}, [{"ref": REF, "object": None}], _refs() * 2, [{"ref": REF, "object": {"type": "tag", "sha": SHA}}]], -) -def test_invalid_ref_response_cannot_authorize_upload_or_retirement( - *, api: MagicMock, group: publisher.MergeGroup, sarif_file: Path, response: Any -) -> None: - api.request.return_value = response - with pytest.raises(ValueError): - _publish(api=api, group=group, sarif_file=sarif_file) - assert _request_paths(api) == [("GET", REF_PATH)] - - -@pytest.mark.parametrize( - "document", [[], {"version": "2.1.0", "runs": []}, {"version": "2.1.0", "runs": [{"tool": None}]}] -) -def test_invalid_prepared_sarif_cannot_authorize_retirement( - *, environment: dict[str, str], sarif_file: Path, document: Any -) -> None: - sarif_file.write_text(json.dumps(document), encoding="utf-8") - with patch.object(publisher, "GitHubAPI") as constructor: - assert _main(environment=environment, sarif_file=sarif_file) == 1 - constructor.assert_not_called() - - -@pytest.mark.parametrize("state", ["live", "obsolete", "upload-race"]) -def test_cli_exercises_real_http_adapter_with_only_mocked_connections( - *, environment: dict[str, str], sarif_file: Path, state: str -) -> None: - responses: list[tuple[int, Any]] = [ - (200, _refs()), - (202, {"id": UPLOAD_ID}), - (200, {"processing_status": "complete"}), - (200, _refs()), - ] - if state != "live": - responses = [(200, []), (200, _run()), (200, _jobs()), (202, None)] - if state == "upload-race": - responses = [(200, _refs()), (404, {"message": f"ref '{REF}' not found in this repository"}), *responses] - connections = [] - for status, body in responses: - connection = MagicMock(spec=http.client.HTTPSConnection) - response = MagicMock(spec=http.client.HTTPResponse) - response.status = status - response.read.return_value = json.dumps(body).encode() if body is not None else b"" - connection.getresponse.return_value = response - connections.append(connection) - sleeps = None if state == "live" else KeyboardInterrupt - with ( - patch.object(publisher.http.client, "HTTPSConnection", side_effect=connections), - patch.object(publisher.time, "sleep", side_effect=sleeps), - ): - assert _main(environment=environment, sarif_file=sarif_file) == (0 if state == "live" else 130) - for connection in connections: - connection.request.assert_called_once() - connection.close.assert_called_once() - last_request = connections[-1].request.call_args - assert last_request.args[:2] == (("GET", REF_PATH) if state == "live" else ("POST", RUN_PATH + "/cancel")) - if state == "live": - payload = json.loads(connections[1].request.call_args.kwargs["body"]) - assert set(payload) == {"commit_sha", "ref", "sarif", "checkout_uri", "tool_name"} - assert gzip.decompress(base64.b64decode(payload["sarif"])) == sarif_file.read_bytes() + assert selected == ["Publish merge-group CodeQL results" if event == "merge_group" else "Publish CodeQL results"] + # Fork PRs retain the action's native token/ref handling, without a privileged PR trigger. + assert not {"ref", "sha", "token"} & steps["Publish CodeQL results"]["with"].keys()