diff --git a/.ci/behavior-ownership/partition.v1.json b/.ci/behavior-ownership/partition.v1.json index 224da1520..125caf189 100644 --- a/.ci/behavior-ownership/partition.v1.json +++ b/.ci/behavior-ownership/partition.v1.json @@ -72,6 +72,10 @@ "group": "artifacts", "target": "backend/app/adapters/checkers/model_runtime.py" }, + { + "group": "shared", + "target": "backend/app/adapters/github/app_facts.py" + }, { "group": "shared", "target": "backend/app/adapters/guide_document_tools/child.py" @@ -424,6 +428,10 @@ "group": "artifacts", "target": "backend/app/interfaces/external_services.py" }, + { + "group": "shared", + "target": "backend/app/interfaces/github_facts.py" + }, { "group": "shared", "target": "backend/app/interfaces/guide_document_tools.py" @@ -2509,7 +2517,7 @@ "target": "backend/scripts/validate_test_lane_evidence.py" } ], - "authority_digest": "cbc1693904d86116e7bc0038d353d79e933be2778aa44d9e4cf3dbef06d6867c", + "authority_digest": "4a436c91f2b8c2f5ada861654d326ad3fb2d84ddc41fb4c760bf86b0438aca72", "protected_base_commit": "7676ce4347db0c9694962a9b587a20765e16eac6", "schema": "workstream.behavior-ownership-partition.v1" } diff --git a/.commitrail/initiatives/WS-GHPROOF-001/WS-GHPROOF-001-02.md b/.commitrail/initiatives/WS-GHPROOF-001/WS-GHPROOF-001-02.md new file mode 100644 index 000000000..376eae4f6 --- /dev/null +++ b/.commitrail/initiatives/WS-GHPROOF-001/WS-GHPROOF-001-02.md @@ -0,0 +1,225 @@ +# WS-GHPROOF-001-02 — GitHub facts port and GitHub App adapter + +- Initiative: WS-GHPROOF-001 +- Durable disposition: Complete +- Intended merge outcome: WorkStream has a typed, secret-safe GitHub facts capability that reads one pull request's merge state, author, reviews, changed files and base-branch required-check results as a read-only GitHub App, registered per ADR 0014 and not yet called by a production path. + +## Intent + +Chunk 04 must read GitHub facts inside the selected POST evaluation handler. +This chunk provides the bounded, tested capability it will call, so the wiring +chunk adds no transport, credential or retry logic of its own. + +## Current behavior + +- No GitHub client, port or setting exists under `backend/app`. +- `backend/app/interfaces/external_services.py` defines the ADR 0014 adapter + protocol, shared errors and `ExternalServiceAdapterFactory`. +- `SELECTED_POST_HANDLER_TIMEOUT_SECONDS` in + `backend/app/modules/checkers/api/external_policy.py` is 3,520 seconds + (3,400 s execution budget plus 60 s prelude and 60 s finalization margins). + +## Bounded change + +### Allowed + +- `backend/app/interfaces/github_facts.py`: capability and provider keys, the + frozen `workstream.github_facts.v1` model, typed errors and the + `GitHubFactsAdapter` port. +- `backend/app/adapters/github/__init__.py` (composition root) and + `backend/app/adapters/github/app_facts.py` (GitHub App provider). +- `backend/app/core/config.py` GitHub App settings, the private-key secret + custody and their validators; + commented examples in `backend/.env.example`. +- `backend/tests/test_github_facts_adapter.py`, its lane-catalogue entry and + shared-family fingerprint, and the behavior-ownership partition additions. +- `backend/tests/test_artifact_architecture.py`: the GitHub adapter package + is registered as one more approved external-service composition root. + +### Not allowed + +- Any production caller, POST wiring, ART custody, checker change, migration, + API route or hosted secret configuration (chunks 03 to 06). + +## Design and decisions + +- **GitHub App.** The intended App is `flow-workstream-proof-reader` (App id + 5454828), installed on the Flow-Research repositories with read access to + administration, checks, metadata, pull requests and statuses. Its private key + stays in the operator's secret store; no key is committed, and tests generate + a throwaway RSA key. +- **Authentication.** An RS256 App JWT (PyJWT, 60 s backdated, 9 min lifetime) + is used only to look up the repository installation (unless + `WORKSTREAM_GITHUB_APP_INSTALLATION_ID` is set) and to exchange an installation + token scoped to that one repository. Tokens are cached per repository until + five minutes before GitHub's `expires_at`. A 401 on a cached token forces one + fresh exchange; a second 401 is access denied. +- **Requests.** `GET /repos/{r}/pulls/{n}`; + `GET /repos/{r}/branches/{base}/protection/required_status_checks` for the + pull request's actual base branch; then `check-runs` and the combined + `status` for the head SHA, with `per_page=100` and `Link: rel="next"` + pagination up to 10 pages. A next link to another origin is a protocol error, + so a response cannot redirect the credential. Redirects are not followed. +- **Required checks.** The required set is the union of protection + `checks[].context` (with its optional App id; `-1` or null means any source) + and `contexts` (any source). A check restricted to an App matches only that + App's check runs; an unrestricted check matches check runs and commit + statuses by name. +- **Re-runs.** A head commit can carry several check runs with one name. A live + read of Flow-Research/harnessy-v2 PR #169 found 47 runs for 24 names, where a + failed first attempt was re-run successfully; judging any run per name gave a + false failure. For each required check only the most recent check run + (highest GitHub id) and the most recent commit status (highest id) count. + When both exist, both must be `success`; otherwise the first non-success value + is reported. No match is `missing`; an incomplete run is `pending`. `neutral` + and `skipped` are not `success`. +- **Missing protection fails closed.** A 404 from the protection endpoint, or an + empty required-check list, raises `GitHubRequiredChecksNotConfiguredError` + (a non-retryable configuration error). Without required checks "CI passed" + has no meaning, and retrying cannot fix it. +- **Errors.** Retries exhausted or a rate-limit wait beyond the deadline: + `GitHubFactsUnavailableError` (retryable). Pull request 404: + `GitHubPullRequestNotFoundError`. 401/403 without rate-limit headers, or an + App not installed: `GitHubFactsAccessDeniedError`. Malformed bodies, another + repository's pull request or other 4xx: `GitHubFactsProtocolError`. Chunk 04 + decides how each maps to an infrastructure failure or a checker finding. +- **Retry budget.** Transport errors, 408, 5xx and rate limits retry up to + `WORKSTREAM_GITHUB_FACTS_MAXIMUM_ATTEMPTS` (default 3, max 5) per request, + with 0.5 s exponential backoff, honoring `Retry-After` and + `x-ratelimit-reset`. One facts read, all retries included, is bounded by + `WORKSTREAM_GITHUB_FACTS_TOTAL_DEADLINE_SECONDS` (default 45 s, max 120 s), + about 3% of the 3,520 s handler timeout at the maximum. Chunk 04 must place + this budget inside the handler prelude or reserve it from the execution + budget; the 60 s prelude margin alone is already partly used. +- **Credential custody.** The PEM key is the secret setting + `WORKSTREAM_GITHUB_APP_PRIVATE_KEY`. Settings remove it from structured input + before validation and hold it only as a private redacted `SecretStr`, + following the `pagination_cursor_hmac_secret` custody path (direct, JSON, + string and dotenv inputs; cleared on failure). It must be configured together + with the App id. Single-line values with literal `\n` separators are + accepted. The adapter parses it once (bounded to 16 KiB) and holds only an + `RSAPrivateKey`. Tokens live + only in `_InstallationToken`, whose representation is redacted. Every + adapter error is raised `from None` and carries only its code. Retry logs + carry only status and attempt. Token-exchange bodies are never hashed or + returned; facts hold only GitHub data and response digests. +- **Integrity facts (owner decision 20, 2026-10-24).** The reader stays + factual and makes no policy decision. It adds the pull request author + (`login`, numeric `id`, `type` such as `User` or `Bot`); every submitted review + from `GET /pulls/{n}/reviews` (review `id`, reviewer or null for a deleted + account, `state`, `commit_id`, `submitted_at`), unfiltered, because choosing + each reviewer's latest review is the checker's rule; and every changed file + from `GET /pulls/{n}/files` (`filename`, `previous_filename` for renames, + `status`). Both lists paginate with `per_page=100`; each page body is hashed. +- **Incomplete file lists fail closed.** GitHub lists at most 3,000 changed + files. If the pull request's `changed_files` exceeds 3,000, or the listing + length differs from it, the read raises `GitHubFactsIncompleteError` (a + protocol error), so a protected-path rule can never miss a file. +- **One pull-request state (security review, time-of-check binding).** Facts + come from several calls; reviews and files are listed by pull-request number, + not by commit, and protection can change between calls, so one object could + mix two states. After every other read the adapter re-reads the pull request + (`pull_request_final`, hashed like the first read) and requires `head_sha`, + `merged`, `merge_commit_sha`, `base_ref`, `state` and `changed_files` to equal + the first read exactly. If they differ, it discards the whole read and starts + again, up to three whole reads within the same deadline, then raises the + retryable `GitHubFactsInconsistentError` (an availability error, so chunk 04 + maps it to an infrastructure failure). The changed-file listing must match + the count both reads reported. Every check run returned for the head SHA + must report that `head_sha`, and every combined-status page must report it as + its `sha`; otherwise the read is a protocol error. The facts' `head_sha` and + `merge_commit_sha` are the single state all facts were bound to; each + required check records `check_run_head_sha` and `commit_status_sha` so the + checker can verify the binding. Reviews keep their own `commit_id`; the + checker counts only reviews of the head SHA. +- **One canonical pull-request identity (security review, parser + differential).** The facts carry GitHub's own pull-request `html_url`. For the + judged check run they carry `check_run_html_url`, `check_run_details_url` and + `workflow_run_id`. The App has no Actions read permission, so the workflow run + id is taken only from a GitHub Actions App (id 15368) `details_url` of the + form `https://github.com/{repository}/actions/runs/{id}[/job/{job}]` for this + repository; other producers give null. `parse_pull_request_url` in the port + module is the single parser for a claimed pull-request URL: it accepts only + ASCII `https://github.com/{owner}/{name}/pull/{number}`. Chunk 04 must take + the pull-request number from this function, and the checker re-asserts exact + equality of the claim with the facts, so fetcher and checker cannot disagree. +- **Check provenance.** Each required check also records `app_bound` (the + requirement names an App), `check_run_app_id` (the App that produced the + judged latest run, or null) and `commit_status_judged` (a commit status, which + has no App identity, was judged). The schema id stays + `workstream.github_facts.v1` because the shape is unreleased. +- **Facts.** `merge_commit_sha` is null unless merged, because GitHub reports a + test-merge commit for open pull requests. `responses` lists the SHA-256 of + each raw facts response body in request order. +- Rejected: following redirects or arbitrary `Link` origins; caching tokens + across repositories; treating missing protection as "no checks required"; + logging response bodies. + +## Acceptance criteria + +- [x] Only the latest re-run per required check counts, in both directions, + and the required set is the union of `checks` and `contexts`. +- [x] Author, all reviews (paginated, unfiltered) and all changed files + (paginated) are returned and hashed; more files than GitHub can list, or a + short listing, fails as incomplete. +- [x] A pull request that moves during a read is re-read and bound to one + state; one that keeps moving, or whose merged flag flips, fails as + inconsistent; check runs or statuses for another commit fail. +- [x] Facts carry the pull-request `html_url` and each judged run's URLs and + workflow run id; the canonical URL parser rejects every non-canonical form. +- [x] Each required check records its App binding and the producing App or + status source of the judged result. +- [x] Happy path returns the exact facts, response digests, a verifiable JWT + and a repository-scoped token request. +- [x] Pagination is followed within the origin; a foreign link fails. +- [x] Missing protection, unknown pull request, wrong repository and access + denial raise their typed errors. +- [x] 5xx and transport errors retry then succeed; exhausted retries and + deadline-bounded rate limits raise `GitHubFactsUnavailableError`. +- [x] The key, JWT and tokens are absent from error strings, representations, + rendered tracebacks, log records and facts. +- [x] The factory registers the sole `github_app` provider; incomplete or unsafe + settings fail. + +## Risk and review routing + +- Risk class: L1, new external credential custody without a production caller. + The App's administration read permission is needed for classic branch + protection reads. +- Required reviewers: security, senior engineering, QA. +- Human review focus: the GitHub App permission set (classic branch protection + reads need repository administration read), fail-closed missing protection, + strict `success` versus GitHub's own acceptance of `neutral`/`skipped`, and + the retry budget placement for chunk 04. + +## Evidence + +| Claim | Command or proof | Result | Remaining uncertainty | +|---|---|---|---| +| Adapter behavior and secret custody | `pytest tests/test_github_facts_adapter.py` | 122 passed | Uses `httpx.MockTransport`; the re-run rule comes from a separate live read, not from this suite | +| Every fail-closed branch rejects | `coverage run --branch --include='app/adapters/github/*,app/interfaces/github_facts.py' -m pytest tests/test_github_facts_adapter.py`; `coverage report -m` | 100% line and branch on `app_facts.py`, `github/__init__.py` and `github_facts.py` (before: 90% and 98%) | Coverage shows execution; each added test asserts the typed error or outcome | +| Re-run test detects order-based selection | Temporarily selecting the last listed run made both re-run cases fail | Detected | Manual negative control | +| Secret tests detect leaks | Temporarily unredacting the token repr, and logging the Authorization value, each made one test fail | Detected | Manual negative control | +| Lane catalogue, boundaries, ownership | `tests/test_ci_lane_catalogue.py`, `tests/architecture/test_module_boundaries.py`, `tests/test_behavior_ownership.py`, `scripts.behavior_ownership validate`, `module_boundaries`, `authorization_boundary`, `test_structure_boundary` | Pass | None | +| Settings | `tests/test_config.py` plus the new settings cases | Pass | None | +| Lint | `ruff check app tests scripts`; `docstr-coverage` | Pass (80.2%) | — | + +These ran on macOS in a `uv pip` environment resolved from `pyproject.toml`, +because `uv.lock` supports Linux only. Hosted Backend CI owns the locked run. + +## Review findings + +- Coverage review: three unreachable branches were removed rather than kept as + dead code: a deadline check between consistency attempts (the next request + already fails with `GitHubFactsUnavailableError` once the deadline is spent), + a full-URL branch in the API-path helper (only paths reach it), and the + natural exit of the retry `for` loop (every exit is an explicit break). + +## Reconciliation + +- Current-source reconciliation: based on WS-GHPROOF-001-01 over `main` after + PR #674. +- Roadmap: no impact. No production path calls the capability, so no + capability, exposure or dependency on `main` changes. +- Next usable boundary: chunk 04 POST wiring after PR #677 and chunk 03. +- Remaining risks: live GitHub behavior is unproven until chunk 06. diff --git a/backend/.env.example b/backend/.env.example index 97c7d2ab0..caa4988cb 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -118,3 +118,15 @@ WORKSTREAM_GUIDE_DOCUMENT_PARSER_SOCKET=/var/lib/workstream/guide-runtime/guide. # WORKSTREAM_ARTIFACT_ADMISSION_PROJECT_MAXIMUM_BYTES=1073741824 # WORKSTREAM_ARTIFACT_ADMISSION_DEPLOYMENT_MAXIMUM_BYTES=4294967296 # WORKSTREAM_ARTIFACT_SCRATCH_ROOT=/tmp/workstream-local-artifact-scratch + +# Read-only GitHub App for proof facts (WS-GHPROOF-001). Not called by any +# production path yet. The PEM private key comes from the deployment secret +# store as WORKSTREAM_GITHUB_APP_PRIVATE_KEY (literal \n separators are +# accepted); never put it in this file. The installation is looked up per +# repository when no installation id is set. +# WORKSTREAM_GITHUB_APP_ID=5454828 +# WORKSTREAM_GITHUB_APP_INSTALLATION_ID= +# WORKSTREAM_GITHUB_API_BASE_URL=https://api.github.com +# WORKSTREAM_GITHUB_API_REQUEST_TIMEOUT_SECONDS=10.0 +# WORKSTREAM_GITHUB_FACTS_TOTAL_DEADLINE_SECONDS=45.0 +# WORKSTREAM_GITHUB_FACTS_MAXIMUM_ATTEMPTS=3 diff --git a/backend/app/adapters/github/__init__.py b/backend/app/adapters/github/__init__.py new file mode 100644 index 000000000..3cab209dd --- /dev/null +++ b/backend/app/adapters/github/__init__.py @@ -0,0 +1,67 @@ +"""Composition root for the read-only GitHub facts capability (ADR 0014). + +No production path calls this yet. The selected POST evaluation wiring +(WS-GHPROOF-001 chunk 04) is its first intended caller. +""" + +from __future__ import annotations + +import httpx + +from app.adapters.github.app_facts import ( + GitHubAppFactsAdapter, + GitHubAppFactsConfiguration, + load_github_app_private_key, +) +from app.core.config import Settings +from app.interfaces.external_services import ( + ExternalServiceAdapterFactory, + ExternalServiceAdapterIdentity, + ExternalServiceConfigurationError, +) +from app.interfaces.github_facts import ( + GITHUB_APP_PROVIDER_KEY, + GITHUB_FACTS_CAPABILITY_KEY, + GitHubFactsAdapter, +) + + +def _github_app_adapter( + settings: Settings, transport: httpx.AsyncBaseTransport | None +) -> GitHubAppFactsAdapter: + """Construct the provider from complete settings or fail as configuration.""" + app_id = settings.github_app_id + private_key = settings.github_app_private_key + if app_id is None or private_key is None: + raise ExternalServiceConfigurationError( + ExternalServiceAdapterIdentity(GITHUB_FACTS_CAPABILITY_KEY, GITHUB_APP_PROVIDER_KEY) + ) + return GitHubAppFactsAdapter( + GitHubAppFactsConfiguration( + app_id=app_id, + installation_id=settings.github_app_installation_id, + api_base_url=settings.github_api_base_url, + request_timeout_seconds=settings.github_api_request_timeout_seconds, + total_deadline_seconds=settings.github_facts_total_deadline_seconds, + maximum_attempts=settings.github_facts_maximum_attempts, + ), + load_github_app_private_key(private_key), + transport=transport, + ) + + +def github_facts_adapter_factory( + settings: Settings, + *, + transport: httpx.AsyncBaseTransport | None = None, +) -> ExternalServiceAdapterFactory[GitHubFactsAdapter]: + """Register the sole GitHub App provider for the GitHub facts capability.""" + factory = ExternalServiceAdapterFactory[GitHubFactsAdapter](GITHUB_FACTS_CAPABILITY_KEY) + factory.register( + GITHUB_APP_PROVIDER_KEY, + lambda: _github_app_adapter(settings, transport), + ) + return factory + + +__all__ = ("github_facts_adapter_factory",) diff --git a/backend/app/adapters/github/app_facts.py b/backend/app/adapters/github/app_facts.py new file mode 100644 index 000000000..f51f706cd --- /dev/null +++ b/backend/app/adapters/github/app_facts.py @@ -0,0 +1,808 @@ +"""GitHub App adapter that reads pull-request proof facts with bounded retries. + +Credential custody: the RSA private key is held only as a parsed key object, +and installation tokens only inside ``_InstallationToken``. Neither is placed in +an exception, log record, representation or returned fact. Every adapter error +is raised ``from None`` so that no HTTP exception carrying request headers is +chained to it. +""" + +from __future__ import annotations + +import asyncio +from collections.abc import Awaitable, Callable +from dataclasses import dataclass +from datetime import UTC, datetime +import hashlib +import logging +import re +import time +from typing import Any +from urllib.parse import quote, urlsplit + +import httpx +import jwt +from cryptography.hazmat.primitives.asymmetric.rsa import RSAPrivateKey +from cryptography.hazmat.primitives.serialization import load_pem_private_key +from pydantic import SecretStr, ValidationError + +from app.interfaces.external_services import ( + ExternalServiceAdapterIdentity, + ExternalServiceConfigurationError, +) +from app.interfaces.github_facts import ( + GITHUB_APP_PROVIDER_KEY, + GITHUB_FACTS_CAPABILITY_KEY, + GitHubFactsAccessDeniedError, + GitHubFactsProtocolError, + GitHubFactsUnavailableError, + GITHUB_MAXIMUM_LISTED_FILES, + GitHubActorFact, + GitHubChangedFileFact, + GitHubFactsIncompleteError, + GitHubFactsInconsistentError, + GitHubPullRequestFacts, + GitHubPullRequestNotFoundError, + GitHubRequiredCheckFact, + GitHubReviewFact, + GitHubRequiredChecksNotConfiguredError, + GitHubResponseDigest, + is_github_repository, +) + +logger = logging.getLogger(__name__) + +GITHUB_API_VERSION = "2022-11-28" +MAXIMUM_PAGES = 10 +PAGE_SIZE = 100 +MAXIMUM_PRIVATE_KEY_BYTES = 16_384 +# GitHub accepts App JWTs for at most ten minutes; backdate for clock drift. +JWT_BACKDATE_SECONDS = 60 +JWT_LIFETIME_SECONDS = 540 +# Refresh an installation token this long before GitHub's stated expiry. +TOKEN_REFRESH_MARGIN_SECONDS = 300 +INITIAL_BACKOFF_SECONDS = 0.5 +GITHUB_ACTIONS_APP_ID = 15368 +_ACTIONS_DETAILS_URL = re.compile( + r"https://github\.com/([A-Za-z0-9][A-Za-z0-9-]{0,38}/[A-Za-z0-9._-]{1,100})" + r"/actions/runs/([1-9][0-9]{0,18})(?:/job/[1-9][0-9]{0,18})?" +) +# Whole facts reads attempted when the pull request moves during a read. +FACTS_CONSISTENCY_ATTEMPTS = 3 +_BOUND_PULL_FIELDS = ( + "head_sha", + "merged", + "merge_commit_sha", + "base_ref", + "state", + "changed_files_count", +) +_CHECK_RUN_CONCLUSIONS = frozenset( + { + "success", + "failure", + "neutral", + "cancelled", + "skipped", + "timed_out", + "action_required", + "stale", + } +) +_STATUS_STATES = frozenset({"success", "failure", "error", "pending"}) +_IDENTITY = ExternalServiceAdapterIdentity(GITHUB_FACTS_CAPABILITY_KEY, GITHUB_APP_PROVIDER_KEY) + + +class _InstallationToken: + """Installation access token whose value never appears in a representation.""" + + __slots__ = ("_value", "refresh_at") + + def __init__(self, value: str, refresh_at: float) -> None: + self._value = value + self.refresh_at = refresh_at + + def header(self) -> str: + """Return the Authorization header value for one GitHub request.""" + return f"Bearer {self._value}" + + def __repr__(self) -> str: + """Redact the token value.""" + return "_InstallationToken()" + + +@dataclass(frozen=True, slots=True) +class GitHubAppFactsConfiguration: + """Validated non-secret adapter limits and endpoints.""" + + app_id: int + installation_id: int | None + api_base_url: str + request_timeout_seconds: float + total_deadline_seconds: float + maximum_attempts: int + + +def load_github_app_private_key(secret: SecretStr) -> RSAPrivateKey: + """Parse one bounded PEM RSA key without exposing its bytes.""" + key: object = None + try: + data = secret.get_secret_value().encode("ascii") + if len(data) <= MAXIMUM_PRIVATE_KEY_BYTES: + key = load_pem_private_key(data, password=None) + except (UnicodeEncodeError, ValueError, TypeError): + key = None + data = b"" + if not isinstance(key, RSAPrivateKey): + raise ExternalServiceConfigurationError(_IDENTITY) from None + return key + + +class _Deadline: + """Monotonic budget shared by every request and retry of one facts read.""" + + def __init__(self, seconds: float, monotonic: Callable[[], float]) -> None: + self._monotonic = monotonic + self._expires_at = monotonic() + seconds + + def remaining(self) -> float: + """Return the seconds left in this budget.""" + return self._expires_at - self._monotonic() + + +class GitHubAppFactsAdapter: + """Read one pull request's merge and required-check facts as a GitHub App.""" + + __slots__ = ( + "_configuration", + "_private_key", + "_transport", + "_wall_clock", + "_monotonic", + "_sleep", + "_tokens", + "_installations", + "_token_lock", + "_api_origin", + ) + + def __init__( + self, + configuration: GitHubAppFactsConfiguration, + private_key: RSAPrivateKey, + *, + transport: httpx.AsyncBaseTransport | None = None, + wall_clock: Callable[[], float] = time.time, + monotonic: Callable[[], float] = time.monotonic, + sleep: Callable[[float], Awaitable[None]] = asyncio.sleep, + ) -> None: + if not isinstance(private_key, RSAPrivateKey): + raise ExternalServiceConfigurationError(_IDENTITY) + self._configuration = configuration + self._private_key = private_key + self._transport = transport + self._wall_clock = wall_clock + self._monotonic = monotonic + self._sleep = sleep + self._tokens: dict[str, _InstallationToken] = {} + self._installations: dict[str, int] = {} + self._token_lock = asyncio.Lock() + parsed = urlsplit(configuration.api_base_url) + self._api_origin = (parsed.scheme, parsed.netloc) + + def __repr__(self) -> str: + """Show only the non-secret App id.""" + return f"GitHubAppFactsAdapter(app_id={self._configuration.app_id})" + + @property + def identity(self) -> ExternalServiceAdapterIdentity: + """Return the fixed GitHub App facts provider identity.""" + return _IDENTITY + + async def fetch_pull_request_facts( + self, + *, + repository: str, + pull_request_number: int, + ) -> GitHubPullRequestFacts: + """Read the pull request, base-branch required checks and head-SHA results.""" + if ( + not is_github_repository(repository) + or type(pull_request_number) is not int + or pull_request_number < 1 + ): + raise ExternalServiceConfigurationError(_IDENTITY) + deadline = _Deadline(self._configuration.total_deadline_seconds, self._monotonic) + async with httpx.AsyncClient( + transport=self._transport, + follow_redirects=False, + headers={ + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": GITHUB_API_VERSION, + "User-Agent": "workstream-github-facts", + }, + ) as client: + for _attempt in range(FACTS_CONSISTENCY_ATTEMPTS): + digests: list[GitHubResponseDigest] = [] + session = _FactsSession(self, client, repository, deadline, digests) + facts = await self._read_consistent_facts( + session, repository, pull_request_number, digests + ) + if facts is not None: + return facts + # An exhausted deadline already raised GitHubFactsUnavailableError from + # the next request, so reaching here means the state kept moving. + raise GitHubFactsInconsistentError(_IDENTITY) + + async def _read_consistent_facts( + self, + session: _FactsSession, + repository: str, + pull_request_number: int, + digests: list[GitHubResponseDigest], + ) -> GitHubPullRequestFacts | None: + """Read every fact once, bracketed by two pull-request reads. + + Reviews and files are listed by pull request number, not by commit, and + protection can change between calls. The final pull-request read must + repeat the first one exactly (head SHA, merged flag, merge commit SHA, + base branch, state, changed-file count), so every fact belongs to one + pull-request state. Returns None when the state moved. + """ + pull_path = f"/repos/{repository}/pulls/{pull_request_number}" + pull = await session.read_json( + "pull_request", pull_path, not_found=GitHubPullRequestNotFoundError + ) + pull_facts = _pull_request_facts(pull, repository, pull_request_number) + required = _required_checks( + await session.read_json( + "required_status_checks", + f"/repos/{repository}/branches/" + f"{quote(pull_facts['base_ref'], safe='')}" + "/protection/required_status_checks", + not_found=GitHubRequiredChecksNotConfiguredError, + ) + ) + head_sha = pull_facts["head_sha"] + changed_files_count = pull_facts["changed_files_count"] + if changed_files_count > GITHUB_MAXIMUM_LISTED_FILES: + raise GitHubFactsIncompleteError(_IDENTITY) + reviews = await session.read_pages( + "reviews", f"{pull_path}/reviews", item_key=None + ) + files = await session.read_pages( + "files", + f"{pull_path}/files", + item_key=None, + maximum_pages=GITHUB_MAXIMUM_LISTED_FILES // PAGE_SIZE, + ) + check_runs = await session.read_pages( + "check_runs", + f"/repos/{repository}/commits/{head_sha}/check-runs", + item_key="check_runs", + ) + statuses = await session.read_pages( + "commit_status", + f"/repos/{repository}/commits/{head_sha}/status", + item_key="statuses", + require_sha=head_sha, + ) + final = _pull_request_facts( + await session.read_json( + "pull_request_final", pull_path, not_found=GitHubPullRequestNotFoundError + ), + repository, + pull_request_number, + ) + if any(final[key] != pull_facts[key] for key in _BOUND_PULL_FIELDS): + return None + if len(files) != changed_files_count: + raise GitHubFactsIncompleteError(_IDENTITY) + if any(run.get("head_sha") != head_sha for run in check_runs): + raise GitHubFactsProtocolError(_IDENTITY) + try: + return GitHubPullRequestFacts( + repository=pull_facts["repository"], + pull_request_number=pull_request_number, + html_url=pull_facts["html_url"], + state=pull_facts["state"], + merged=pull_facts["merged"], + merge_commit_sha=pull_facts["merge_commit_sha"], + base_ref=pull_facts["base_ref"], + head_sha=head_sha, + author=_actor(pull_facts["author"]), + reviews=tuple(_review(item) for item in reviews), + changed_files_count=changed_files_count, + changed_files=tuple( + GitHubChangedFileFact( + filename=item["filename"], + previous_filename=item.get("previous_filename"), + status=item["status"], + ) + for item in files + ), + required_checks=tuple( + _required_check_fact( + name, app_id, check_runs, statuses, head_sha, pull_facts["repository"] + ) + for name, app_id in required + ), + fetched_at=datetime.fromtimestamp(self._wall_clock(), UTC), + responses=tuple(digests), + ) + except (ValidationError, KeyError, TypeError): + raise GitHubFactsProtocolError(_IDENTITY) from None + + # -- credentials ------------------------------------------------------- + + def _app_jwt(self) -> str: + """Sign a short-lived RS256 App JWT; it authenticates only token exchange.""" + now = int(self._wall_clock()) + return jwt.encode( + { + "iat": now - JWT_BACKDATE_SECONDS, + "exp": now + JWT_LIFETIME_SECONDS, + "iss": str(self._configuration.app_id), + }, + self._private_key, + algorithm="RS256", + ) + + def _cached_token(self, repository: str) -> _InstallationToken | None: + """Return a repository-scoped token that is not yet due for refresh.""" + token = self._tokens.get(repository.casefold()) + if token is None or self._wall_clock() >= token.refresh_at: + return None + return token + + def _forget_token(self, repository: str) -> None: + """Drop a token that GitHub rejected.""" + self._tokens.pop(repository.casefold(), None) + + async def _installation_token(self, session: _FactsSession) -> _InstallationToken: + """Return a cached repository-scoped token or exchange a fresh App JWT.""" + repository = session.repository + async with self._token_lock: + token = self._cached_token(repository) + if token is not None: + return token + installation_id = self._configuration.installation_id + if installation_id is None: + installation_id = self._installations.get(repository.casefold()) + if installation_id is None: + body = await session.app_json( + "GET", f"/repos/{repository}/installation", self._app_jwt() + ) + installation_id = body.get("id") if isinstance(body, dict) else None + if type(installation_id) is not int or installation_id < 1: + raise GitHubFactsProtocolError(_IDENTITY) + self._installations[repository.casefold()] = installation_id + body = await session.app_json( + "POST", + f"/app/installations/{installation_id}/access_tokens", + self._app_jwt(), + json_body={"repositories": [repository.split("/", 1)[1]]}, + ) + value = body.get("token") if isinstance(body, dict) else None + expires_at = body.get("expires_at") if isinstance(body, dict) else None + try: + expiry = datetime.fromisoformat(str(expires_at).replace("Z", "+00:00")) + except ValueError: + expiry = None + if ( + not isinstance(value, str) + or not value + or any(character.isspace() for character in value) + or expiry is None + or expiry.tzinfo is None + ): + raise GitHubFactsProtocolError(_IDENTITY) + token = _InstallationToken( + value, expiry.timestamp() - TOKEN_REFRESH_MARGIN_SECONDS + ) + self._tokens[repository.casefold()] = token + return token + + +class _FactsSession: + """One bounded facts read: shared deadline, retries and response digests.""" + + def __init__( + self, + adapter: GitHubAppFactsAdapter, + client: httpx.AsyncClient, + repository: str, + deadline: _Deadline, + digests: list[GitHubResponseDigest], + ) -> None: + self._adapter = adapter + self._client = client + self.repository = repository + self._deadline = deadline + self._digests = digests + + def _url(self, path: str) -> str: + """Resolve an API path against the configured origin.""" + return self._adapter._configuration.api_base_url + path + + async def app_json( + self, + method: str, + path: str, + app_jwt: str, + *, + json_body: dict[str, Any] | None = None, + ) -> object: + """Send one App-JWT request; its body may hold a token and is never hashed.""" + response = await self._send( + method, + self._url(path), + authorization=lambda: f"Bearer {app_jwt}", + json_body=json_body, + not_found=GitHubFactsAccessDeniedError, + installation=False, + ) + return _json(response) + + async def read_json( + self, + label: str, + path: str, + *, + not_found: type[Exception], + ) -> object: + """Read and hash one installation-token GET response.""" + response = await self._installation_get(self._url(path), not_found=not_found) + self._record(label, response) + return _json(response) + + async def read_pages( + self, + label: str, + path: str, + *, + item_key: str | None, + maximum_pages: int = MAXIMUM_PAGES, + require_sha: str | None = None, + ) -> list[dict]: + """Follow same-origin ``next`` links for a bounded number of pages. + + ``item_key`` names the list inside an object body; None means the body + itself is the list. More pages than ``maximum_pages`` fail closed. + ``require_sha`` requires each page body's ``sha`` to equal that commit. + """ + url = self._url(path) + f"?per_page={PAGE_SIZE}" + items: list[dict] = [] + for page in range(1, maximum_pages + 1): + response = await self._installation_get(url, not_found=GitHubFactsProtocolError) + self._record(f"{label}.page.{page}", response) + body = _json(response) + if require_sha is not None and ( + not isinstance(body, dict) or body.get("sha") != require_sha + ): + raise GitHubFactsProtocolError(_IDENTITY) + if item_key is None: + page_items = body + else: + page_items = body.get(item_key) if isinstance(body, dict) else None + if not isinstance(page_items, list) or not all( + isinstance(item, dict) for item in page_items + ): + raise GitHubFactsProtocolError(_IDENTITY) + items.extend(page_items) + next_url = response.links.get("next", {}).get("url") + if next_url is None: + return items + parsed = urlsplit(next_url) + if (parsed.scheme, parsed.netloc) != self._adapter._api_origin: + raise GitHubFactsProtocolError(_IDENTITY) + url = next_url + raise GitHubFactsProtocolError(_IDENTITY) + + def _record(self, label: str, response: httpx.Response) -> None: + """Keep the SHA-256 of one raw facts response body.""" + digest = hashlib.sha256(response.content).hexdigest() + self._digests.append(GitHubResponseDigest(request=label, sha256=f"sha256:{digest}")) + + async def _installation_get( + self, url: str, *, not_found: type[Exception] + ) -> httpx.Response: + """GET with the installation token, refreshing a rejected token once.""" + token = await self._adapter._installation_token(self) + try: + return await self._send( + "GET", + url, + authorization=token.header, + json_body=None, + not_found=not_found, + installation=True, + ) + except _TokenRejected: + # A revoked or expired cached token gets one fresh exchange. + self._adapter._forget_token(self.repository) + token = await self._adapter._installation_token(self) + try: + return await self._send( + "GET", + url, + authorization=token.header, + json_body=None, + not_found=not_found, + installation=True, + ) + except _TokenRejected: + raise GitHubFactsAccessDeniedError(_IDENTITY) from None + + async def _send( + self, + method: str, + url: str, + *, + authorization: Callable[[], str], + json_body: dict[str, Any] | None, + not_found: type[Exception], + installation: bool, + ) -> httpx.Response: + """Send with bounded retries for transport errors, 5xx and rate limits.""" + configuration = self._adapter._configuration + attempt = 0 + while True: + attempt += 1 + remaining = self._deadline.remaining() + if remaining <= 0: + break + timeout = min(configuration.request_timeout_seconds, remaining) + response: httpx.Response | None = None + try: + response = await self._client.request( + method, + url, + json=json_body, + headers={"Authorization": authorization()}, + timeout=timeout, + ) + except httpx.TransportError: + response = None + delay = INITIAL_BACKOFF_SECONDS * (2 ** (attempt - 1)) + if response is not None: + status = response.status_code + if 200 <= status < 300: + return response + if status == 404: + raise not_found(_IDENTITY) from None + if status == 401: + if installation: + raise _TokenRejected() + raise GitHubFactsAccessDeniedError(_IDENTITY) from None + rate_limit_delay = _rate_limit_delay(response, self._adapter._wall_clock()) + if status in {403, 429} and rate_limit_delay is not None: + delay = max(delay, rate_limit_delay) + elif status == 403: + raise GitHubFactsAccessDeniedError(_IDENTITY) from None + elif status != 408 and status < 500: + raise GitHubFactsProtocolError(_IDENTITY) from None + logger.warning( + "github_facts_request_retryable status=%s attempt=%s", + None if response is None else response.status_code, + attempt, + ) + if ( + attempt >= configuration.maximum_attempts + or delay >= self._deadline.remaining() + ): + break + await self._adapter._sleep(delay) + raise GitHubFactsUnavailableError(_IDENTITY) from None + + +class _TokenRejected(Exception): + """Internal signal that GitHub rejected an installation token.""" + + +def _json(response: httpx.Response) -> object: + """Parse a JSON body or fail as a protocol error.""" + try: + return response.json() + except ValueError: + raise GitHubFactsProtocolError(_IDENTITY) from None + + +def _rate_limit_delay(response: httpx.Response, now: float) -> float | None: + """Return GitHub's requested wait for a primary or secondary rate limit.""" + retry_after = response.headers.get("retry-after") + if retry_after is not None: + try: + return max(0.0, float(int(retry_after))) + except ValueError: + return None + if response.headers.get("x-ratelimit-remaining") == "0": + try: + return max(0.0, float(int(response.headers["x-ratelimit-reset"])) - now) + except (KeyError, ValueError): + return None + return None + + +def _pull_request_facts(body: object, repository: str, number: int) -> dict[str, Any]: + """Extract the pull request fields and confirm it belongs to the repository.""" + try: + if not isinstance(body, dict): + raise TypeError + base = body["base"] + head = body["head"] + full_name = base["repo"]["full_name"] + merged = body["merged"] + values = { + "repository": full_name, + "state": body["state"], + "merged": merged, + "merge_commit_sha": body["merge_commit_sha"] if merged is True else None, + "base_ref": base["ref"], + "head_sha": head["sha"], + "html_url": body["html_url"], + "author": body["user"], + "changed_files_count": body["changed_files"], + } + valid = ( + body["number"] == number + and isinstance(full_name, str) + and full_name.casefold() == repository.casefold() + and isinstance(values["base_ref"], str) + and isinstance(values["head_sha"], str) + and type(values["changed_files_count"]) is int + and values["changed_files_count"] >= 0 + ) + except (KeyError, TypeError): + valid = False + if not valid: + raise GitHubFactsProtocolError(_IDENTITY) from None + return values + + +def _required_checks(body: object) -> tuple[tuple[str, int | None], ...]: + """Return required (context, app id) pairs from ``checks`` and ``contexts``. + + ``checks`` entries carry an optional App restriction; a name only in + ``contexts`` accepts any source. No required check fails closed. + """ + if not isinstance(body, dict): + raise GitHubFactsProtocolError(_IDENTITY) + checks = body.get("checks", []) + contexts = body.get("contexts", []) + if not isinstance(checks, list) or not isinstance(contexts, list): + raise GitHubFactsProtocolError(_IDENTITY) + required: dict[str, int | None] = {} + for item in checks: + context = item.get("context") if isinstance(item, dict) else None + app_id = item.get("app_id") if isinstance(item, dict) else None + if not isinstance(context, str) or not context: + raise GitHubFactsProtocolError(_IDENTITY) + if app_id is not None and (type(app_id) is not int or app_id < 1): + # GitHub reports -1 or null for "any source". + app_id = None + required[context] = app_id + for context in contexts: + if not isinstance(context, str) or not context: + raise GitHubFactsProtocolError(_IDENTITY) + required.setdefault(context, None) + if not required: + raise GitHubRequiredChecksNotConfiguredError(_IDENTITY) + return tuple(sorted(required.items())) + + +def _latest(items: list[dict]) -> dict | None: + """Return the most recent item by GitHub id; re-runs get higher ids.""" + if any(type(item.get("id")) is not int for item in items): + raise GitHubFactsProtocolError(_IDENTITY) + return max(items, key=lambda item: item["id"], default=None) + + +def _workflow_run_id(run: dict, repository: str) -> int | None: + """Return the Actions workflow run id from a GitHub Actions run's details URL. + + The App lacks Actions read permission, so the run id comes from GitHub's own + ``details_url`` (``https://github.com/{repo}/actions/runs/{id}/job/{job}``), + and only for the GitHub Actions App and this repository. + """ + app = run.get("app") + details_url = run.get("details_url") + if not isinstance(app, dict) or app.get("id") != GITHUB_ACTIONS_APP_ID: + return None + if not isinstance(details_url, str): + return None + match = _ACTIONS_DETAILS_URL.fullmatch(details_url) + if match is None or match.group(1).casefold() != repository.casefold(): + return None + return int(match.group(2)) + + +def _actor(body: object) -> GitHubActorFact: + """Translate one GitHub user object.""" + if not isinstance(body, dict): + raise GitHubFactsProtocolError(_IDENTITY) + return GitHubActorFact(login=body["login"], id=body["id"], type=body["type"]) + + +def _review(body: dict) -> GitHubReviewFact: + """Translate one review; GitHub reports a deleted reviewer as null.""" + user = body.get("user") + return GitHubReviewFact( + id=body["id"], + reviewer=None if user is None else _actor(user), + state=body["state"], + commit_id=body.get("commit_id"), + submitted_at=body.get("submitted_at"), + ) + + +def _required_check_fact( + name: str, + app_id: int | None, + check_runs: list[dict], + statuses: list[dict], + head_sha: str, + repository: str, +) -> GitHubRequiredCheckFact: + """Judge only the latest check run and latest status for one required check. + + A re-run supersedes an earlier attempt with the same name. When both a check + run and a commit status exist for the name, both latest results must succeed. + The fact records which App produced the judged run. + """ + observed: list[str] = [] + runs = [] + for run in check_runs: + app = run.get("app") + run_app_id = app.get("id") if isinstance(app, dict) else None + if run.get("name") == name and (app_id is None or run_app_id == app_id): + runs.append(run) + run = _latest(runs) + check_run_app_id = None + check_run_head_sha = None + html_url = details_url = workflow_run_id = None + if run is not None: + check_run_head_sha = run.get("head_sha") + html_url = run.get("html_url") + details_url = run.get("details_url") + workflow_run_id = _workflow_run_id(run, repository) + app = run.get("app") + check_run_app_id = app.get("id") if isinstance(app, dict) else None + if check_run_app_id is not None and type(check_run_app_id) is not int: + raise GitHubFactsProtocolError(_IDENTITY) + if run.get("status") != "completed": + observed.append("pending") + else: + conclusion = run.get("conclusion") + if conclusion not in _CHECK_RUN_CONCLUSIONS: + raise GitHubFactsProtocolError(_IDENTITY) + observed.append(conclusion) + status_judged = False + if app_id is None: + # Commit statuses carry no App identity, so they satisfy only + # requirements that accept any source. + status = _latest([item for item in statuses if item.get("context") == name]) + if status is not None: + state = status.get("state") + if state not in _STATUS_STATES: + raise GitHubFactsProtocolError(_IDENTITY) + observed.append(state) + status_judged = True + conclusion = ( + "missing" + if not observed + else next((value for value in observed if value != "success"), "success") + ) + return GitHubRequiredCheckFact( + name=name, + app_id=app_id, + app_bound=app_id is not None, + conclusion=conclusion, + check_run_app_id=check_run_app_id, + check_run_head_sha=check_run_head_sha, + check_run_html_url=html_url, + check_run_details_url=details_url, + workflow_run_id=workflow_run_id, + commit_status_judged=status_judged, + commit_status_sha=head_sha if status_judged else None, + ) diff --git a/backend/app/core/config.py b/backend/app/core/config.py index 5e832c24c..5234f3008 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -288,6 +288,17 @@ class Settings(BaseSettings): gt=0.0, le=3700.0, ) + # Read-only GitHub App used to read pull-request proof facts. Its PEM + # private key is the secret WORKSTREAM_GITHUB_APP_PRIVATE_KEY, held only as + # a private redacted attribute (see github_app_private_key). + github_app_id: int | None = Field(default=None, ge=1) + github_app_installation_id: int | None = Field(default=None, ge=1) + github_api_base_url: str = "https://api.github.com" + github_api_request_timeout_seconds: float = Field(default=10.0, gt=0.0, le=30.0) + # One facts read, including every retry, stays far inside the selected POST + # handler budget (SELECTED_POST_HANDLER_TIMEOUT_SECONDS). + github_facts_total_deadline_seconds: float = Field(default=45.0, gt=0.0, le=120.0) + github_facts_maximum_attempts: int = Field(default=3, ge=1, le=5) @field_validator("checker_model_proxy_socket", mode="before") @classmethod @@ -312,6 +323,7 @@ def empty_model_proxy_socket(cls, value: object) -> object: _api_rate_limit_key_secret: SecretStr | None = PrivateAttr(default=None) _pagination_cursor_hmac_secret: SecretStr | None = PrivateAttr(default=None) + _github_app_private_key: SecretStr | None = PrivateAttr(default=None) _artifact_s3_access_key_id: SecretStr | None = PrivateAttr(default=None) _artifact_s3_secret_access_key: SecretStr | None = PrivateAttr(default=None) _artifact_s3_session_token: SecretStr | None = PrivateAttr(default=None) @@ -332,24 +344,29 @@ def __init__(self, **values: object) -> None: secret: SecretStr | None = None cursor_secret: SecretStr | None = None s3_secrets = _EMPTY_ARTIFACT_S3_SECRETS + github_key: SecretStr | None = None try: _normalize_observability_endpoint_input(values) _normalize_artifact_s3_endpoint_input(values) secret = _extract_api_rate_limit_key_secret(values) cursor_secret = _extract_pagination_cursor_hmac_secret(values) s3_secrets = _extract_artifact_s3_static_secrets(values) + github_key = _extract_github_app_private_key(values) super().__init__(**values) self._validate_observability_endpoint_security() self._api_rate_limit_key_secret = secret self._pagination_cursor_hmac_secret = cursor_secret self._set_artifact_s3_static_secrets(s3_secrets) + self._github_app_private_key = github_key if not _ALTERNATE_VALIDATION_RESTORES_SECRETS.get(): self._validate_artifact_s3_secret_contract() + self._validate_github_app_secret_contract() except Exception: _clear_settings_private_secrets(self) values.clear() secret = None cursor_secret = None + github_key = None s3_secrets = _EMPTY_ARTIFACT_S3_SECRETS raise @@ -368,6 +385,7 @@ def model_validate(cls, obj: object, **kwargs: object) -> Self: if isinstance(obj, Mapping) and ( "api_rate_limit_key_secret" in obj or "pagination_cursor_hmac_secret" in obj + or _GITHUB_APP_PRIVATE_KEY_FIELD in obj or _ARTIFACT_S3_SENSITIVE_INPUT_FIELDS.intersection(obj) or _OBSERVABILITY_SENSITIVE_INPUT_FIELDS.intersection(obj) ): @@ -403,6 +421,7 @@ def model_validate_json( if isinstance(parsed, Mapping) and ( "api_rate_limit_key_secret" in parsed or "pagination_cursor_hmac_secret" in parsed + or _GITHUB_APP_PRIVATE_KEY_FIELD in parsed or _ARTIFACT_S3_SENSITIVE_INPUT_FIELDS.intersection(parsed) or _OBSERVABILITY_SENSITIVE_INPUT_FIELDS.intersection(parsed) ): @@ -425,6 +444,7 @@ def model_validate_strings(cls, obj: object, **kwargs: object) -> Self: if isinstance(obj, Mapping) and ( "api_rate_limit_key_secret" in obj or "pagination_cursor_hmac_secret" in obj + or _GITHUB_APP_PRIVATE_KEY_FIELD in obj or _ARTIFACT_S3_SENSITIVE_INPUT_FIELDS.intersection(obj) or _OBSERVABILITY_SENSITIVE_INPUT_FIELDS.intersection(obj) ): @@ -449,6 +469,7 @@ def _validate_secret_mapping( secret: SecretStr | None = None cursor_secret: SecretStr | None = None s3_secrets = _EMPTY_ARTIFACT_S3_SECRETS + github_key: SecretStr | None = None settings: Settings | None = None try: _normalize_observability_endpoint_input(sanitized) @@ -464,6 +485,11 @@ def _validate_secret_mapping( else None ) s3_secrets = _extract_artifact_s3_static_secrets(sanitized) + github_key = ( + _extract_github_app_private_key(sanitized) + if _GITHUB_APP_PRIVATE_KEY_FIELD in sanitized + else None + ) if secret is not None: sanitized["api_rate_limit_key_secret"] = None if cursor_secret is not None: @@ -481,6 +507,9 @@ def _validate_secret_mapping( settings._pagination_cursor_hmac_secret = cursor_secret settings._set_artifact_s3_static_secrets(s3_secrets) settings._validate_artifact_s3_secret_contract() + if github_key is not None: + settings._github_app_private_key = github_key + settings._validate_github_app_secret_contract() return settings except Exception: if settings is not None: @@ -489,6 +518,7 @@ def _validate_secret_mapping( secret = None cursor_secret = None s3_secrets = _EMPTY_ARTIFACT_S3_SECRETS + github_key = None settings = None raise @@ -505,6 +535,7 @@ def settings_customise_sources( if isinstance(dotenv_settings, DotEnvSettingsSource): dotenv_settings.env_vars.pop("workstream_api_rate_limit_key_secret", None) dotenv_settings.env_vars.pop("workstream_pagination_cursor_hmac_secret", None) + dotenv_settings.env_vars.pop(f"workstream_{_GITHUB_APP_PRIVATE_KEY_FIELD}", None) for field_name in _ARTIFACT_S3_SECRET_FIELDS: dotenv_settings.env_vars.pop(f"workstream_{field_name}", None) return init_settings, env_settings, dotenv_settings, file_secret_settings @@ -519,6 +550,16 @@ def pagination_cursor_hmac_secret(self) -> SecretStr | None: """Return the validated, redacted pagination-cursor key setting.""" return self._pagination_cursor_hmac_secret + @property + def github_app_private_key(self) -> SecretStr | None: + """Return the redacted GitHub App PEM private key.""" + return self._github_app_private_key + + def _validate_github_app_secret_contract(self) -> None: + """Require the GitHub App id and private key together.""" + if (self.github_app_id is None) != (self._github_app_private_key is None): + raise ValueError("GitHub App id and private key must be configured together") + @property def artifact_s3_access_key_id(self) -> SecretStr | None: """Return the local/CI MinIO access-key identifier.""" @@ -568,6 +609,31 @@ def _validate_artifact_s3_secret_contract(self) -> None: secrets = _EMPTY_ARTIFACT_S3_SECRETS raise ValueError("native AWS S3 rejects static credentials") + @model_validator(mode="after") + def validate_github_app(self) -> Settings: + """Require a complete GitHub App identity and a credential-safe API origin.""" + if self.github_app_installation_id is not None and self.github_app_id is None: + raise ValueError("GitHub App installation id requires a GitHub App id") + parsed = urlsplit(self.github_api_base_url) + loopback_http = ( + parsed.scheme == "http" + and parsed.hostname in {"127.0.0.1", "localhost", "::1"} + and self.environment in {"local", "dev", "development", "test"} + ) + if ( + (parsed.scheme != "https" and not loopback_http) + or not parsed.hostname + or parsed.username is not None + or parsed.password is not None + or parsed.query + or parsed.fragment + or self.github_api_base_url.endswith("/") + ): + raise ValueError("GitHub API base URL must be a credential-free HTTPS origin") + if self.github_api_request_timeout_seconds > self.github_facts_total_deadline_seconds: + raise ValueError("GitHub request timeout cannot exceed the facts deadline") + return self + @model_validator(mode="after") def validate_project_agent_runtime(self) -> Settings: """Reject partial no-inference identities and invalid adapter options.""" @@ -902,6 +968,7 @@ def _clear_settings_private_secrets(settings: object) -> None: return private_values["_api_rate_limit_key_secret"] = None private_values["_pagination_cursor_hmac_secret"] = None + private_values["_github_app_private_key"] = None private_values["_artifact_s3_access_key_id"] = None private_values["_artifact_s3_secret_access_key"] = None private_values["_artifact_s3_session_token"] = None @@ -1085,6 +1152,43 @@ def _extract_pagination_cursor_hmac_secret( return secret +_GITHUB_APP_PRIVATE_KEY_FIELD = "github_app_private_key" +MAXIMUM_GITHUB_APP_PRIVATE_KEY_CHARACTERS = 16_384 + + +def _extract_github_app_private_key(values: dict[str, object]) -> SecretStr | None: + """Remove the GitHub App PEM key from structured input; the adapter parses it. + + A single-line value with literal ``\\n`` separators, as some secret stores + emit, is converted to a multi-line PEM document. + """ + raw_value = _pop_optional_secret_source( + values, + field_name=_GITHUB_APP_PRIVATE_KEY_FIELD, + environment_name="WORKSTREAM_GITHUB_APP_PRIVATE_KEY", + ) + if raw_value is _MISSING_SECRET or raw_value is None or raw_value == "": + return None + if isinstance(raw_value, SecretStr): + text = raw_value.get_secret_value() + elif isinstance(raw_value, str): + text = raw_value + else: + del raw_value + raise ValueError("invalid GitHub App private key") + del raw_value + if "\n" not in text and "\\n" in text: + text = text.replace("\\n", "\n") + if ( + len(text) > MAXIMUM_GITHUB_APP_PRIVATE_KEY_CHARACTERS + or "-----BEGIN" not in text + or "PRIVATE KEY-----" not in text + ): + del text + raise ValueError("invalid GitHub App private key") + return SecretStr(text) + + def _extract_artifact_s3_static_secrets( values: dict[str, object], ) -> tuple[SecretStr | None, SecretStr | None, SecretStr | None]: diff --git a/backend/app/interfaces/github_facts.py b/backend/app/interfaces/github_facts.py new file mode 100644 index 000000000..683ab1f6a --- /dev/null +++ b/backend/app/interfaces/github_facts.py @@ -0,0 +1,261 @@ +"""Typed GitHub pull-request facts capability for proof comparison. + +A trusted WorkStream owner reads these facts outside the checker sandbox. The +networkless checker receives them as request input and compares them with the +contributor's claims. The facts never contain a credential. +""" + +from __future__ import annotations + +from datetime import datetime +import re +from typing import Annotated, Literal, Protocol + +from pydantic import ( + AfterValidator, + BaseModel, + ConfigDict, + Field, + StrictBool, + StrictInt, + StrictStr, +) + +from app.interfaces.external_services import ( + ExternalServiceAdapter, + ExternalServiceAdapterError, + ExternalServiceConfigurationError, + ExternalServiceProtocolError, + ExternalServiceUnavailableError, +) + + +GITHUB_FACTS_CAPABILITY_KEY = "github_facts" +GITHUB_APP_PROVIDER_KEY = "github_app" +GITHUB_FACTS_SCHEMA_VERSION = "workstream.github_facts.v1" + +# GitHub owner logins are at most 39 characters; repository names at most 100. +_GITHUB_REPOSITORY = re.compile( + r"^[A-Za-z0-9](?:[A-Za-z0-9-]{0,38})/(?!\.{1,2}$)[A-Za-z0-9._-]{1,100}$" +) + +GitSha = Annotated[StrictStr, Field(pattern=r"^[0-9a-f]{40}$")] +Sha256 = Annotated[StrictStr, Field(pattern=r"^sha256:[0-9a-f]{64}$")] +# Check-run conclusions and commit-status states, plus two derived values: +# ``pending`` (not completed) and ``missing`` (no matching check or status). +GitHubCheckConclusion = Literal[ + "success", + "failure", + "neutral", + "cancelled", + "skipped", + "timed_out", + "action_required", + "stale", + "error", + "pending", + "missing", +] + + +def is_github_repository(value: object) -> bool: + """Return whether a value is one canonical ``owner/name`` repository.""" + return isinstance(value, str) and _GITHUB_REPOSITORY.fullmatch(value) is not None + + +_CANONICAL_PULL_REQUEST_URL = re.compile( + r"https://github\.com/([A-Za-z0-9][A-Za-z0-9-]{0,38}/[A-Za-z0-9._-]{1,100})" + r"/pull/([1-9][0-9]{0,9})" +) + + +def parse_pull_request_url(value: object) -> tuple[str, int] | None: + """Return (repository, number) from one canonical pull-request URL, or None. + + This is the single parser for a claimed pull-request URL. Only + ``https://github.com/{owner}/{name}/pull/{number}`` in ASCII is accepted: + no port, user information, query, fragment, trailing slash, percent + encoding or backslash. The checker re-asserts exact equality of the claim + with the facts, so the reader and the checker cannot disagree. + """ + if not isinstance(value, str) or not value.isascii(): + return None + match = _CANONICAL_PULL_REQUEST_URL.fullmatch(value) + if match is None or not is_github_repository(match.group(1)): + return None + return match.group(1), int(match.group(2)) + + +def _github_repository(value: str) -> str: + """Validate one canonical repository name.""" + if not is_github_repository(value): + raise ValueError("GitHub repository must be owner/name") + return value + + +GitHubRepository = Annotated[StrictStr, AfterValidator(_github_repository)] + + +class _FactsValue(BaseModel): + """Strict immutable facts value.""" + + model_config = ConfigDict(extra="forbid", frozen=True, revalidate_instances="always") + + +# GitHub changed-file listings return at most this many files per pull request. +GITHUB_MAXIMUM_LISTED_FILES = 3000 + + +class GitHubActorFact(_FactsValue): + """One GitHub account as GitHub reports it (``type`` is e.g. User or Bot).""" + + login: Annotated[StrictStr, Field(min_length=1, max_length=100)] + id: Annotated[StrictInt, Field(ge=1)] + type: Annotated[StrictStr, Field(min_length=1, max_length=50)] + + +class GitHubReviewFact(_FactsValue): + """One submitted pull-request review, unfiltered. + + ``reviewer`` is null when GitHub no longer has the account. Policy, such as + taking each reviewer's latest review, belongs to the consumer. + """ + + id: Annotated[StrictInt, Field(ge=1)] + reviewer: GitHubActorFact | None + state: Annotated[StrictStr, Field(min_length=1, max_length=50)] + commit_id: GitSha | None + submitted_at: datetime | None + + +class GitHubChangedFileFact(_FactsValue): + """One changed file; ``previous_filename`` is set for renames.""" + + filename: Annotated[StrictStr, Field(min_length=1, max_length=4096)] + previous_filename: Annotated[StrictStr, Field(min_length=1, max_length=4096)] | None + status: Annotated[StrictStr, Field(min_length=1, max_length=50)] + + +class GitHubRequiredCheckFact(_FactsValue): + """One check required by base-branch protection and its head-SHA result. + + ``app_id`` is the requirement's App restriction and ``app_bound`` says + whether one exists. ``check_run_app_id`` is the App that produced the judged + latest check run (null when no run was judged) and ``check_run_head_sha`` the + commit GitHub reports for that run. ``commit_status_judged`` is true when a + commit status, which carries no App identity, was judged; + ``commit_status_sha`` is the commit of that combined status. + ``check_run_html_url`` and ``check_run_details_url`` are GitHub's URLs for the + judged run, and ``workflow_run_id`` is the GitHub Actions workflow run that + produced it, taken from a GitHub Actions ``details_url`` of this repository + (null for other producers). + """ + + name: Annotated[StrictStr, Field(min_length=1, max_length=255)] + app_id: StrictInt | None + app_bound: StrictBool + conclusion: GitHubCheckConclusion + check_run_app_id: StrictInt | None + check_run_head_sha: GitSha | None + check_run_html_url: Annotated[StrictStr, Field(max_length=2048)] | None + check_run_details_url: Annotated[StrictStr, Field(max_length=2048)] | None + workflow_run_id: Annotated[StrictInt, Field(ge=1)] | None + commit_status_judged: StrictBool + commit_status_sha: GitSha | None + + +class GitHubResponseDigest(_FactsValue): + """SHA-256 of one raw GitHub response body that produced these facts.""" + + request: Annotated[StrictStr, Field(pattern=r"^[a-z_]+(?:\.page\.[1-9][0-9]*)?$")] + sha256: Sha256 + + +class GitHubPullRequestFacts(_FactsValue): + """Immutable ``workstream.github_facts.v1`` value for one pull request. + + ``merge_commit_sha`` is null unless the pull request is merged, because + GitHub reports a test-merge commit for open pull requests. ``head_sha`` and + ``merge_commit_sha`` are the single pull-request state every other fact was + bound to: the adapter re-reads the pull request after all other reads and + discards the read if that state moved. Serialize with + ``model_dump(mode="json")`` to place the value in checker request input. + """ + + schema_version: Literal["workstream.github_facts.v1"] = GITHUB_FACTS_SCHEMA_VERSION + repository: GitHubRepository + pull_request_number: Annotated[StrictInt, Field(ge=1)] + html_url: Annotated[StrictStr, Field(min_length=1, max_length=2048)] + state: Literal["open", "closed"] + merged: StrictBool + merge_commit_sha: GitSha | None + base_ref: Annotated[StrictStr, Field(min_length=1, max_length=255)] + head_sha: GitSha + author: GitHubActorFact + reviews: tuple[GitHubReviewFact, ...] + changed_files_count: Annotated[StrictInt, Field(ge=0, le=GITHUB_MAXIMUM_LISTED_FILES)] + changed_files: tuple[GitHubChangedFileFact, ...] + required_checks: tuple[GitHubRequiredCheckFact, ...] = Field(min_length=1) + fetched_at: datetime + responses: tuple[GitHubResponseDigest, ...] = Field(min_length=1) + + +class GitHubFactsUnavailableError(ExternalServiceUnavailableError): + """GitHub stayed unavailable or rate-limited within the bounded retry budget.""" + + code = "github_facts_unavailable" + + +class GitHubFactsAccessDeniedError(ExternalServiceConfigurationError): + """The GitHub App credential, installation or permission is not usable.""" + + code = "github_facts_access_denied" + + +class GitHubRequiredChecksNotConfiguredError(ExternalServiceConfigurationError): + """The base branch has no classic branch-protection required checks. + + Without required checks, "CI passed" has no meaning, so facts fail closed. + """ + + code = "github_required_checks_not_configured" + + +class GitHubPullRequestNotFoundError(ExternalServiceAdapterError): + """The repository has no pull request with the requested number.""" + + code = "github_pull_request_not_found" + category = "not_found" + + +class GitHubFactsProtocolError(ExternalServiceProtocolError): + """GitHub returned a response outside the supported facts protocol.""" + + code = "github_facts_protocol_invalid" + + +class GitHubFactsInconsistentError(ExternalServiceUnavailableError): + """The pull request kept changing while facts were read, within the deadline. + + Retryable: a later read may observe one stable state. + """ + + code = "github_facts_inconsistent" + + +class GitHubFactsIncompleteError(GitHubFactsProtocolError): + """GitHub cannot list every changed file, so the facts would be partial.""" + + code = "github_facts_incomplete" + + +class GitHubFactsAdapter(ExternalServiceAdapter, Protocol): + """Read verified facts about one pull request with a read-only credential.""" + + async def fetch_pull_request_facts( + self, + *, + repository: str, + pull_request_number: int, + ) -> GitHubPullRequestFacts: + """Return facts or raise a secret-free ``ExternalServiceAdapterError``.""" diff --git a/backend/scripts/behavior_ownership.py b/backend/scripts/behavior_ownership.py index 8a639acf8..56f25d3ca 100644 --- a/backend/scripts/behavior_ownership.py +++ b/backend/scripts/behavior_ownership.py @@ -909,6 +909,10 @@ "backend/app/modules/authorization/domain/project_catalogue.py", "backend/app/modules/authorization/project_catalogue.py", }) +GITHUB_PROOF_FACTS_TARGETS = frozenset({ + "backend/app/adapters/github/app_facts.py", + "backend/app/interfaces/github_facts.py", +}) class BehaviorOwnershipError(RuntimeError): @@ -1180,6 +1184,7 @@ def _validate_additive_partition_transition( | PILOT_02B_ATOMIC_IMPORT_TARGETS | PILOT_02C_BULK_TRANSITION_TARGETS | PROJECT_CATALOGUE_TARGETS + | GITHUB_PROOF_FACTS_TARGETS ) expected_additions = (approved_additions & additions) - set(trusted_targets) if POL_03A_DECLARATIVE_MODEL_TARGET in additions: diff --git a/backend/scripts/test_lane_catalogue.py b/backend/scripts/test_lane_catalogue.py index 5a35a62b5..cd8724338 100644 --- a/backend/scripts/test_lane_catalogue.py +++ b/backend/scripts/test_lane_catalogue.py @@ -97,6 +97,7 @@ class TestLane: "tests/test_checker_image.py", "tests/test_checker_smol_cache.py", "tests/checkers/external/test_harnessy_proof_checker.py", + "tests/test_github_facts_adapter.py", "tests/test_api_controls.py", "tests/test_identifiers.py", "tests/test_identifier_schema.py", diff --git a/backend/tests/test_artifact_architecture.py b/backend/tests/test_artifact_architecture.py index 8886cb97c..fb898deef 100644 --- a/backend/tests/test_artifact_architecture.py +++ b/backend/tests/test_artifact_architecture.py @@ -33,6 +33,7 @@ AGENT_COMPOSITION_ROOT = APP_ROOT / "adapters/project_agents/__init__.py" GUIDE_PARSER_COMPOSITION_ROOT = APP_ROOT / "adapters/guide_document_tools/__init__.py" OBSERVABILITY_COMPOSITION_ROOT = APP_ROOT / "adapters" / "observability.py" +GITHUB_COMPOSITION_ROOT = APP_ROOT / "adapters" / "github" / "__init__.py" AGENT_ADAPTER_MODULES = { "app.adapters.project_agents.manager_authored", "app.adapters.project_agents.omnicore_agent", @@ -384,6 +385,7 @@ def _assert_concrete_adapter_composition( AGENT_COMPOSITION_ROOT: 2, OBSERVABILITY_COMPOSITION_ROOT: 1, GUIDE_PARSER_COMPOSITION_ROOT: 1, + GITHUB_COMPOSITION_ROOT: 1, } assert adapter_calls == [ COMPOSITION_ROOT, S3_ADAPTER_MODULE, CHECKER_COMPOSITION_ROOT, CHECKER_COMPOSITION_ROOT, diff --git a/backend/tests/test_ci_lane_catalogue.py b/backend/tests/test_ci_lane_catalogue.py index 1491a4b45..e32d9ab12 100644 --- a/backend/tests/test_ci_lane_catalogue.py +++ b/backend/tests/test_ci_lane_catalogue.py @@ -286,8 +286,8 @@ def test_semantic_family_membership_has_an_independent_fingerprint() -> None: """A catalogue-internal move across semantic families must fail this contract.""" expected = { "shared": ( - 260, - "2fe14cb5b5987c540fc30a153a2429adc16243b55aca0e565892338c306282dd", + 261, + "d903ceb93c8f54c401100a58226909458c4d41d10fd618421d8d0d1ec78f0c07", ), "project": ( 231, diff --git a/backend/tests/test_github_facts_adapter.py b/backend/tests/test_github_facts_adapter.py new file mode 100644 index 000000000..3a9b044ee --- /dev/null +++ b/backend/tests/test_github_facts_adapter.py @@ -0,0 +1,1355 @@ +"""GitHub App facts adapter: facts, bounded retries and credential custody.""" + +from __future__ import annotations + +import hashlib +import json +import logging +import traceback + +import httpx +import jwt +import pytest +from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives.asymmetric import ec, rsa +from pydantic import SecretStr, ValidationError + +from app.adapters.github import github_facts_adapter_factory +from app.adapters.github.app_facts import ( + GitHubAppFactsAdapter, + GitHubAppFactsConfiguration, + load_github_app_private_key, + _InstallationToken, +) +from app.core.config import Settings +from app.interfaces.external_services import ( + ExternalServiceAdapterError, + ExternalServiceConfigurationError, +) +from app.interfaces.github_facts import ( + GitHubFactsAccessDeniedError, + GitHubFactsIncompleteError, + GitHubFactsInconsistentError, + GitHubFactsProtocolError, + GitHubFactsUnavailableError, + GitHubPullRequestFacts, + GitHubPullRequestNotFoundError, + GitHubRequiredChecksNotConfiguredError, + parse_pull_request_url, +) + +API = "https://api.github.test" +REPOSITORY = "acme/widgets" +HEAD_SHA = "a" * 40 +MERGE_SHA = "b" * 40 +TOKEN = "ghs_installationTokenSecretValue0001" +SECOND_TOKEN = "ghs_installationTokenSecretValue0002" +NOW = 1_800_000_000.0 +APP_ID = 4242 + +PRIVATE_KEY = rsa.generate_private_key(public_exponent=65537, key_size=2048) +PRIVATE_PEM = PRIVATE_KEY.private_bytes( + serialization.Encoding.PEM, + serialization.PrivateFormat.PKCS8, + serialization.NoEncryption(), +).decode() +PEM_BODY_LINE = PRIVATE_PEM.splitlines()[1] +# Throwaway key generated per test run; no real key exists in the repository. + + +def _pull(**changes: object) -> dict: + body = { + "number": 12, + "state": "closed", + "merged": True, + "merge_commit_sha": MERGE_SHA, + "base": {"ref": "main", "repo": {"full_name": REPOSITORY}}, + "head": {"sha": HEAD_SHA}, + "html_url": f"https://github.com/{REPOSITORY}/pull/12", + "user": {"login": "agent-bot", "id": 501, "type": "Bot"}, + "changed_files": 2, + } + body.update(changes) + return body + + +RUN_DETAILS_URL = f"https://github.com/{REPOSITORY}/actions/runs/345/job/678" +REVIEWS = [ + { + "id": 61, + "user": {"login": "reviewer", "id": 7, "type": "User"}, + "state": "APPROVED", + "commit_id": HEAD_SHA, + "submitted_at": "2027-01-15T07:00:00Z", + }, + { + "id": 62, + "user": None, + "state": "COMMENTED", + "commit_id": MERGE_SHA, + "submitted_at": "2027-01-15T07:30:00Z", + }, +] +FILES = [ + {"filename": "src/app.py", "status": "modified"}, + {"filename": ".github/workflows/ci.yml", "previous_filename": "ci.yml", "status": "renamed"}, +] + + +class FakeGitHub: + """Route-table GitHub fake that records every request it receives.""" + + def __init__(self) -> None: + self.requests: list[httpx.Request] = [] + self.bodies: dict[str, bytes] = {} + self.tokens = [TOKEN, SECOND_TOKEN] + self.routes: dict[tuple[str, str], list] = {} + self.route("GET", f"/repos/{REPOSITORY}/installation", {"id": 77}) + self.route("POST", "/app/installations/77/access_tokens", self._token) + self.route("GET", f"/repos/{REPOSITORY}/pulls/12", _pull()) + self.route("GET", f"/repos/{REPOSITORY}/pulls/12/reviews", REVIEWS) + self.route("GET", f"/repos/{REPOSITORY}/pulls/12/files", FILES) + self.route( + "GET", + f"/repos/{REPOSITORY}/branches/main/protection/required_status_checks", + { + "strict": True, + "contexts": ["backend", "lint"], + "checks": [ + {"context": "backend", "app_id": 15368}, + {"context": "lint", "app_id": None}, + ], + }, + ) + self.route( + "GET", + f"/repos/{REPOSITORY}/commits/{HEAD_SHA}/check-runs", + { + "total_count": 1, + "check_runs": [ + { + "id": 10, + "html_url": f"https://github.com/{REPOSITORY}/runs/10", + "details_url": RUN_DETAILS_URL, + "name": "backend", + "head_sha": HEAD_SHA, "status": "completed", + "conclusion": "success", + "app": {"id": 15368}, + } + ], + }, + ) + self.route( + "GET", + f"/repos/{REPOSITORY}/commits/{HEAD_SHA}/status", + {"sha": HEAD_SHA, "state": "success", "statuses": [{"id": 21, "context": "lint", "state": "success"}]}, + ) + + def route(self, method: str, path: str, *responses: object) -> None: + """Queue responses; the last one repeats once the queue is exhausted.""" + self.routes[(method, path)] = list(responses) + + def _token(self, _request: httpx.Request) -> httpx.Response: + return httpx.Response( + 201, + json={"token": self.tokens.pop(0), "expires_at": "2027-01-15T09:00:00Z"}, + ) + + def handler(self, request: httpx.Request) -> httpx.Response: + self.requests.append(request) + queue = self.routes.get((request.method, request.url.path)) + if queue is None: + return httpx.Response(404, json={"message": "Not Found"}) + item = queue.pop(0) if len(queue) > 1 else queue[0] + if isinstance(item, Exception): + raise item + if callable(item): + response = item(request) + elif isinstance(item, httpx.Response): + response = item + else: + response = httpx.Response(200, json=item) + self.bodies[str(request.url)] = response.content + return response + + def secrets(self) -> list[str]: + """Every credential the adapter sent, plus the issued tokens.""" + sent = [ + request.headers["authorization"].removeprefix("Bearer ") + for request in self.requests + if "authorization" in request.headers + ] + return [*sent, TOKEN, SECOND_TOKEN, PEM_BODY_LINE] + + +class Clock: + def __init__(self) -> None: + self.now = 0.0 + self.sleeps: list[float] = [] + + def monotonic(self) -> float: + return self.now + + async def sleep(self, seconds: float) -> None: + self.sleeps.append(seconds) + self.now += seconds + + +def _adapter( + fake: FakeGitHub, + clock: Clock | None = None, + *, + installation_id: int | None = None, + deadline: float = 45.0, + attempts: int = 3, +) -> GitHubAppFactsAdapter: + clock = clock or Clock() + return GitHubAppFactsAdapter( + GitHubAppFactsConfiguration( + app_id=APP_ID, + installation_id=installation_id, + api_base_url=API, + request_timeout_seconds=10.0, + total_deadline_seconds=deadline, + maximum_attempts=attempts, + ), + PRIVATE_KEY, + transport=httpx.MockTransport(fake.handler), + wall_clock=lambda: NOW, + monotonic=clock.monotonic, + sleep=clock.sleep, + ) + + +async def _fetch(adapter: GitHubAppFactsAdapter) -> GitHubPullRequestFacts: + return await adapter.fetch_pull_request_facts(repository=REPOSITORY, pull_request_number=12) + + +def _sha(content: bytes) -> str: + return "sha256:" + hashlib.sha256(content).hexdigest() + + +async def test_happy_path_returns_facts_with_response_digests() -> None: + fake = FakeGitHub() + facts = await _fetch(_adapter(fake)) + + assert facts.model_dump(mode="json") == { + "schema_version": "workstream.github_facts.v1", + "repository": REPOSITORY, + "pull_request_number": 12, + "html_url": f"https://github.com/{REPOSITORY}/pull/12", + "state": "closed", + "merged": True, + "merge_commit_sha": MERGE_SHA, + "base_ref": "main", + "head_sha": HEAD_SHA, + "author": {"login": "agent-bot", "id": 501, "type": "Bot"}, + "reviews": [ + { + "id": 61, + "reviewer": {"login": "reviewer", "id": 7, "type": "User"}, + "state": "APPROVED", + "commit_id": HEAD_SHA, + "submitted_at": "2027-01-15T07:00:00Z", + }, + { + "id": 62, + "reviewer": None, + "state": "COMMENTED", + "commit_id": MERGE_SHA, + "submitted_at": "2027-01-15T07:30:00Z", + }, + ], + "changed_files_count": 2, + "changed_files": [ + {"filename": "src/app.py", "previous_filename": None, "status": "modified"}, + { + "filename": ".github/workflows/ci.yml", + "previous_filename": "ci.yml", + "status": "renamed", + }, + ], + "required_checks": [ + { + "name": "backend", + "app_id": 15368, + "app_bound": True, + "conclusion": "success", + "check_run_app_id": 15368, + "check_run_head_sha": HEAD_SHA, + "check_run_html_url": f"https://github.com/{REPOSITORY}/runs/10", + "check_run_details_url": RUN_DETAILS_URL, + "workflow_run_id": 345, + "commit_status_judged": False, + "commit_status_sha": None, + }, + { + "name": "lint", + "app_id": None, + "app_bound": False, + "conclusion": "success", + "check_run_app_id": None, + "check_run_head_sha": None, + "check_run_html_url": None, + "check_run_details_url": None, + "workflow_run_id": None, + "commit_status_judged": True, + "commit_status_sha": HEAD_SHA, + }, + ], + "fetched_at": "2027-01-15T08:00:00Z", + "responses": [ + { + "request": "pull_request", + "sha256": _sha(fake.bodies[f"{API}/repos/{REPOSITORY}/pulls/12"]), + }, + { + "request": "required_status_checks", + "sha256": _sha( + fake.bodies[ + f"{API}/repos/{REPOSITORY}/branches/main/protection/" + "required_status_checks" + ] + ), + }, + { + "request": "reviews.page.1", + "sha256": _sha( + fake.bodies[f"{API}/repos/{REPOSITORY}/pulls/12/reviews?per_page=100"] + ), + }, + { + "request": "files.page.1", + "sha256": _sha( + fake.bodies[f"{API}/repos/{REPOSITORY}/pulls/12/files?per_page=100"] + ), + }, + { + "request": "check_runs.page.1", + "sha256": _sha( + fake.bodies[ + f"{API}/repos/{REPOSITORY}/commits/{HEAD_SHA}/check-runs?per_page=100" + ] + ), + }, + { + "request": "commit_status.page.1", + "sha256": _sha( + fake.bodies[f"{API}/repos/{REPOSITORY}/commits/{HEAD_SHA}/status?per_page=100"] + ), + }, + { + "request": "pull_request_final", + "sha256": _sha(fake.bodies[f"{API}/repos/{REPOSITORY}/pulls/12"]), + }, + ], + } + lookup, exchange, *facts_requests = fake.requests + claims = jwt.decode( + lookup.headers["authorization"].removeprefix("Bearer "), + PRIVATE_KEY.public_key(), + algorithms=["RS256"], + options={"verify_exp": False, "verify_iat": False}, + ) + assert claims == {"iss": str(APP_ID), "iat": int(NOW) - 60, "exp": int(NOW) + 540} + assert json.loads(exchange.content) == {"repositories": ["widgets"]} + assert {request.headers["authorization"] for request in facts_requests} == { + f"Bearer {TOKEN}" + } + assert all(request.headers["x-github-api-version"] for request in fake.requests) + + +async def test_installation_token_is_cached_per_repository_until_refresh() -> None: + fake = FakeGitHub() + adapter = _adapter(fake, installation_id=77) + await _fetch(adapter) + await _fetch(adapter) + + paths = [request.url.path for request in fake.requests] + assert paths.count("/app/installations/77/access_tokens") == 1 + assert f"/repos/{REPOSITORY}/installation" not in paths + + +async def test_pagination_follows_same_origin_next_links() -> None: + fake = FakeGitHub() + runs_path = f"/repos/{REPOSITORY}/commits/{HEAD_SHA}/check-runs" + page_two = f"{API}{runs_path}?per_page=100&page=2" + fake.route( + "GET", + runs_path, + httpx.Response( + 200, + json={ + "check_runs": [ + {"id": 4, "name": "other", "head_sha": HEAD_SHA, "status": "completed", "conclusion": "failure"} + ] + }, + headers={"link": f'<{page_two}>; rel="next"'}, + ), + { + "check_runs": [ + { + "id": 11, + "name": "backend", + "head_sha": HEAD_SHA, "status": "completed", + "conclusion": "failure", + "app": {"id": 15368}, + } + ] + }, + ) + facts = await _fetch(_adapter(fake)) + + assert facts.required_checks[0].conclusion == "failure" + assert [item.request for item in facts.responses][4:6] == [ + "check_runs.page.1", + "check_runs.page.2", + ] + + +async def test_pagination_refuses_a_foreign_origin_link() -> None: + fake = FakeGitHub() + fake.route( + "GET", + f"/repos/{REPOSITORY}/commits/{HEAD_SHA}/check-runs", + httpx.Response( + 200, + json={"check_runs": []}, + headers={"link": '; rel="next"'}, + ), + ) + with pytest.raises(GitHubFactsProtocolError): + await _fetch(_adapter(fake)) + assert all(request.url.host == "api.github.test" for request in fake.requests) + + +async def test_required_check_conclusions_fail_closed() -> None: + fake = FakeGitHub() + fake.route( + "GET", + f"/repos/{REPOSITORY}/branches/main/protection/required_status_checks", + {"contexts": ["backend", "lint", "docs", "e2e", "wrong-app"], "checks": []}, + ) + fake.route( + "GET", + f"/repos/{REPOSITORY}/commits/{HEAD_SHA}/check-runs", + { + "check_runs": [ + {"id": 1, "name": "backend", "head_sha": HEAD_SHA, "status": "completed", "conclusion": "success"}, + {"id": 2, "name": "e2e", "head_sha": HEAD_SHA, "status": "in_progress", "conclusion": None}, + {"id": 3, "name": "wrong-app", "head_sha": HEAD_SHA, "status": "completed", "conclusion": "skipped"}, + ] + }, + ) + fake.route( + "GET", + f"/repos/{REPOSITORY}/commits/{HEAD_SHA}/status", + { + "sha": HEAD_SHA, "statuses": [ + {"id": 23, "context": "backend", "state": "failure"}, + {"id": 24, "context": "lint", "state": "error"}, + ] + }, + ) + facts = await _fetch(_adapter(fake)) + + assert {item.name: item.conclusion for item in facts.required_checks} == { + "backend": "failure", + "docs": "missing", + "e2e": "pending", + "lint": "error", + "wrong-app": "skipped", + } + + +@pytest.mark.parametrize( + ("older", "newer", "expected"), + [("failure", "success", "success"), ("success", "failure", "failure")], + ids=("rerun-passed", "rerun-failed"), +) +async def test_only_the_latest_rerun_of_a_required_check_counts( + older: str, newer: str, expected: str +) -> None: + fake = FakeGitHub() + runs_path = f"/repos/{REPOSITORY}/commits/{HEAD_SHA}/check-runs" + run = {"name": "backend", "head_sha": HEAD_SHA, "status": "completed", "app": {"id": 15368}} + # The newer run is on the first page and the older on the second, so + # response order cannot decide the result. + fake.route( + "GET", + runs_path, + httpx.Response( + 200, + json={"check_runs": [{**run, "id": 900, "conclusion": newer}]}, + headers={"link": f'<{API}{runs_path}?per_page=100&page=2>; rel="next"'}, + ), + {"check_runs": [{**run, "id": 899, "conclusion": older}]}, + ) + fake.route( + "GET", + f"/repos/{REPOSITORY}/commits/{HEAD_SHA}/status", + { + "sha": HEAD_SHA, "statuses": [ + {"id": 31, "context": "lint", "state": "failure"}, + {"id": 32, "context": "lint", "state": "success"}, + ] + }, + ) + facts = await _fetch(_adapter(fake)) + assert {item.name: item.conclusion for item in facts.required_checks} == { + "backend": expected, + "lint": "success", + } + + +async def test_required_checks_are_the_union_of_checks_and_contexts() -> None: + fake = FakeGitHub() + fake.route( + "GET", + f"/repos/{REPOSITORY}/branches/main/protection/required_status_checks", + { + "contexts": ["backend", "docs"], + "checks": [{"context": "backend", "app_id": 15368}, {"context": "lint", "app_id": -1}], + }, + ) + facts = await _fetch(_adapter(fake)) + assert [(item.name, item.app_id, item.conclusion) for item in facts.required_checks] == [ + ("backend", 15368, "success"), + ("docs", None, "missing"), + ("lint", None, "success"), + ] + + +async def test_app_restricted_check_ignores_other_apps_and_statuses() -> None: + fake = FakeGitHub() + fake.route( + "GET", + f"/repos/{REPOSITORY}/commits/{HEAD_SHA}/check-runs", + { + "check_runs": [ + { + "id": 12, + "name": "backend", + "head_sha": HEAD_SHA, "status": "completed", + "conclusion": "success", + "app": {"id": 999}, + } + ] + }, + ) + fake.route( + "GET", + f"/repos/{REPOSITORY}/commits/{HEAD_SHA}/status", + {"sha": HEAD_SHA, "statuses": [{"id": 22, "context": "backend", "state": "success"}, {"id": 21, "context": "lint", "state": "success"}]}, + ) + facts = await _fetch(_adapter(fake)) + assert facts.required_checks[0].conclusion == "missing" + + +async def test_unmerged_pull_request_reports_no_merge_commit() -> None: + fake = FakeGitHub() + fake.route("GET", f"/repos/{REPOSITORY}/pulls/12", _pull(state="open", merged=False)) + facts = await _fetch(_adapter(fake)) + assert (facts.state, facts.merged, facts.merge_commit_sha) == ("open", False, None) + + +@pytest.mark.parametrize( + "protection", + [ + httpx.Response(404, json={"message": "Branch not protected"}), + {"contexts": [], "checks": []}, + ], + ids=("not-protected", "no-required-checks"), +) +async def test_missing_required_checks_fail_closed(protection: object) -> None: + fake = FakeGitHub() + fake.route( + "GET", + f"/repos/{REPOSITORY}/branches/main/protection/required_status_checks", + protection, + ) + with pytest.raises(GitHubRequiredChecksNotConfiguredError) as caught: + await _fetch(_adapter(fake)) + assert caught.value.retryable is False + + +async def test_unknown_pull_request_is_not_found() -> None: + fake = FakeGitHub() + fake.route("GET", f"/repos/{REPOSITORY}/pulls/12", httpx.Response(404)) + with pytest.raises(GitHubPullRequestNotFoundError): + await _fetch(_adapter(fake)) + + +async def test_pull_request_from_another_repository_is_rejected() -> None: + fake = FakeGitHub() + fake.route( + "GET", + f"/repos/{REPOSITORY}/pulls/12", + _pull(base={"ref": "main", "repo": {"full_name": "acme/other"}}), + ) + with pytest.raises(GitHubFactsProtocolError): + await _fetch(_adapter(fake)) + + +async def test_server_error_is_retried_then_succeeds() -> None: + fake = FakeGitHub() + clock = Clock() + fake.route( + "GET", + f"/repos/{REPOSITORY}/pulls/12", + httpx.Response(502), + httpx.ConnectError("connection refused"), + _pull(), + ) + facts = await _fetch(_adapter(fake, clock)) + assert facts.merged is True + assert clock.sleeps == [0.5, 1.0] + + +async def test_retries_exhausted_raise_facts_unavailable() -> None: + fake = FakeGitHub() + clock = Clock() + fake.route("GET", f"/repos/{REPOSITORY}/pulls/12", httpx.Response(503)) + with pytest.raises(GitHubFactsUnavailableError) as caught: + await _fetch(_adapter(fake, clock)) + pulls = [r for r in fake.requests if r.url.path.endswith("/pulls/12")] + assert len(pulls) == 3 + assert clock.sleeps == [0.5, 1.0] + assert caught.value.retryable is True + + +async def test_total_deadline_bounds_retries() -> None: + fake = FakeGitHub() + clock = Clock() + fake.route("GET", f"/repos/{REPOSITORY}/pulls/12", httpx.Response(500)) + with pytest.raises(GitHubFactsUnavailableError): + await _fetch(_adapter(fake, clock, deadline=1.0, attempts=5)) + assert sum(clock.sleeps) < 1.0 + + +async def test_rate_limit_waits_for_reset_within_the_deadline() -> None: + fake = FakeGitHub() + clock = Clock() + fake.route( + "GET", + f"/repos/{REPOSITORY}/pulls/12", + httpx.Response( + 403, + headers={"x-ratelimit-remaining": "0", "x-ratelimit-reset": str(int(NOW) + 7)}, + ), + httpx.Response(429, headers={"retry-after": "3"}), + _pull(), + ) + facts = await _fetch(_adapter(fake, clock)) + assert facts.merged is True + assert clock.sleeps == [7.0, 3.0] + + +async def test_rate_limit_beyond_the_deadline_fails_without_waiting() -> None: + fake = FakeGitHub() + clock = Clock() + fake.route( + "GET", + f"/repos/{REPOSITORY}/pulls/12", + httpx.Response(429, headers={"retry-after": "3600"}), + ) + with pytest.raises(GitHubFactsUnavailableError): + await _fetch(_adapter(fake, clock)) + assert clock.sleeps == [] + + +async def test_forbidden_without_rate_limit_is_access_denied() -> None: + fake = FakeGitHub() + fake.route( + "GET", + f"/repos/{REPOSITORY}/branches/main/protection/required_status_checks", + httpx.Response(403, json={"message": "Resource not accessible by integration"}), + ) + with pytest.raises(GitHubFactsAccessDeniedError): + await _fetch(_adapter(fake)) + + +async def test_rejected_cached_token_is_refreshed_once() -> None: + fake = FakeGitHub() + adapter = _adapter(fake, installation_id=77) + await _fetch(adapter) + fake.route("GET", f"/repos/{REPOSITORY}/pulls/12", httpx.Response(401), _pull()) + await _fetch(adapter) + exchanges = [r for r in fake.requests if r.url.path.endswith("/access_tokens")] + assert len(exchanges) == 2 + assert fake.requests[-1].headers["authorization"] == f"Bearer {SECOND_TOKEN}" + + +@pytest.mark.parametrize( + ("repository", "number"), + [("acme", 12), ("acme/../x", 12), ("acme/widgets/extra", 12), (REPOSITORY, 0)], +) +async def test_invalid_request_is_refused_before_any_network_call( + repository: str, number: int +) -> None: + fake = FakeGitHub() + with pytest.raises(ExternalServiceConfigurationError): + await _adapter(fake).fetch_pull_request_facts( + repository=repository, pull_request_number=number + ) + assert fake.requests == [] + + +def _assert_secret_free(text: str, secrets: list[str]) -> None: + for secret in secrets: + assert secret not in text + + +@pytest.mark.parametrize( + "failure", + ["unavailable", "access_denied", "not_found", "protocol", "token_exchange_denied"], +) +async def test_credentials_never_reach_errors_logs_or_reprs( + failure: str, caplog: pytest.LogCaptureFixture +) -> None: + fake = FakeGitHub() + pulls = f"/repos/{REPOSITORY}/pulls/12" + if failure == "unavailable": + fake.route("GET", pulls, httpx.ConnectError(f"refused Bearer {TOKEN}")) + elif failure == "access_denied": + fake.route("GET", pulls, httpx.Response(401)) + elif failure == "not_found": + fake.route("GET", pulls, httpx.Response(404)) + elif failure == "protocol": + fake.route("GET", pulls, httpx.Response(200, content=f"not json {TOKEN}".encode())) + else: + fake.route("POST", "/app/installations/77/access_tokens", httpx.Response(403)) + adapter = _adapter(fake) + caplog.set_level(logging.DEBUG) + + with pytest.raises(ExternalServiceAdapterError) as caught: + await _fetch(adapter) + + error = caught.value + rendered = "".join(traceback.format_exception(error)) + assert error.__cause__ is None and error.__suppress_context__ is True + log_text = "\n".join( + f"{record.getMessage()} {record.args!r} {record.__dict__!r}" for record in caplog.records + ) + secrets = fake.secrets() + assert len(secrets) > 3 + for text in (str(error), repr(error), rendered, log_text, repr(adapter)): + _assert_secret_free(text, secrets) + + +async def test_successful_facts_and_token_holder_are_secret_free( + caplog: pytest.LogCaptureFixture, +) -> None: + fake = FakeGitHub() + caplog.set_level(logging.DEBUG) + adapter = _adapter(fake) + facts = await _fetch(adapter) + secrets = fake.secrets() + texts = [ + facts.model_dump_json(), + repr(facts), + repr(adapter), + repr(_InstallationToken(TOKEN, NOW)), + "\n".join(record.getMessage() for record in caplog.records), + ] + for text in texts: + _assert_secret_free(text, secrets) + + +def _settings(**values: object) -> Settings: + configured: dict[str, object] = { + "environment": "test", + "github_app_id": APP_ID, + "github_app_private_key": PRIVATE_PEM, + "github_api_base_url": API, + } + configured.update(values) + return Settings(_env_file=None, **configured) + + +async def test_factory_registers_the_github_app_provider() -> None: + fake = FakeGitHub() + factory = github_facts_adapter_factory( + _settings(github_app_installation_id=77), + transport=httpx.MockTransport(fake.handler), + ) + adapter = factory.create("github_app") + assert (adapter.identity.capability_key, adapter.identity.provider_key) == ( + "github_facts", + "github_app", + ) + facts = await _fetch(adapter) + assert facts.repository == REPOSITORY + + +def test_factory_refuses_missing_or_unusable_credentials() -> None: + unconfigured = Settings(_env_file=None, environment="test") + with pytest.raises(ExternalServiceConfigurationError): + github_facts_adapter_factory(unconfigured).create("github_app") + unusable = "-----BEGIN PRIVATE KEY-----\nnot-a-key\n-----END PRIVATE KEY-----\n" + with pytest.raises(ExternalServiceConfigurationError) as caught: + github_facts_adapter_factory(_settings(github_app_private_key=unusable)).create( + "github_app" + ) + assert caught.value.__cause__ is None + + +def test_private_key_setting_is_redacted_and_accepts_escaped_newlines( + monkeypatch: pytest.MonkeyPatch, +) -> None: + settings = _settings() + assert settings.github_app_private_key is not None + assert settings.github_app_private_key.get_secret_value() == PRIVATE_PEM + for text in (repr(settings), str(settings.model_dump()), settings.model_dump_json()): + assert PEM_BODY_LINE not in text + monkeypatch.setenv("WORKSTREAM_GITHUB_APP_PRIVATE_KEY", PRIVATE_PEM.replace("\n", "\\n")) + from_environment = Settings(_env_file=None, environment="test", github_app_id=APP_ID) + assert from_environment.github_app_private_key is not None + assert from_environment.github_app_private_key.get_secret_value() == PRIVATE_PEM + + +def test_invalid_private_key_setting_error_omits_the_value() -> None: + bogus = "not-a-pem-secret-value-0123456789" + with pytest.raises(ValueError) as caught: + _settings(github_app_private_key=bogus) + assert bogus not in "".join(traceback.format_exception(caught.value)) + + +@pytest.mark.parametrize( + "values", + [ + {"github_app_private_key": None}, + {"github_app_id": None}, + {"github_api_base_url": "http://api.github.example"}, + {"github_api_base_url": "https://user:pass@api.github.com"}, + {"github_api_base_url": "https://api.github.com/"}, + {"github_api_base_url": "http://127.0.0.1:8080", "environment": "production"}, + {"github_facts_total_deadline_seconds": 5.0}, + {"github_facts_total_deadline_seconds": 121.0}, + ], +) +def test_settings_reject_incomplete_or_unsafe_configuration(values: dict) -> None: + with pytest.raises(ValueError): + _settings(**values) + + +def test_settings_accept_loopback_http_only_for_local_and_test() -> None: + settings = _settings(github_api_base_url="http://127.0.0.1:8080") + assert settings.github_api_base_url == "http://127.0.0.1:8080" + + +async def test_reviews_and_files_paginate_and_reviews_are_returned_unfiltered() -> None: + fake = FakeGitHub() + reviews_path = f"/repos/{REPOSITORY}/pulls/12/reviews" + files_path = f"/repos/{REPOSITORY}/pulls/12/files" + fake.route( + "GET", + reviews_path, + httpx.Response( + 200, + json=[REVIEWS[0]], + headers={"link": f'<{API}{reviews_path}?per_page=100&page=2>; rel="next"'}, + ), + [{**REVIEWS[0], "id": 63, "state": "CHANGES_REQUESTED"}], + ) + page_one = [{"filename": f"f{index}.py", "status": "added"} for index in range(100)] + fake.route("GET", f"/repos/{REPOSITORY}/pulls/12", _pull(changed_files=101)) + fake.route( + "GET", + files_path, + httpx.Response( + 200, + json=page_one, + headers={"link": f'<{API}{files_path}?per_page=100&page=2>; rel="next"'}, + ), + [{"filename": "last.py", "status": "added"}], + ) + facts = await _fetch(_adapter(fake)) + assert [(item.id, item.state) for item in facts.reviews] == [ + (61, "APPROVED"), + (63, "CHANGES_REQUESTED"), + ] + assert len(facts.changed_files) == facts.changed_files_count == 101 + assert [item.request for item in facts.responses][2:6] == [ + "reviews.page.1", + "reviews.page.2", + "files.page.1", + "files.page.2", + ] + + +async def test_more_changed_files_than_github_can_list_fails_closed() -> None: + fake = FakeGitHub() + fake.route("GET", f"/repos/{REPOSITORY}/pulls/12", _pull(changed_files=3001)) + with pytest.raises(GitHubFactsIncompleteError): + await _fetch(_adapter(fake)) + assert not any(request.url.path.endswith("/files") for request in fake.requests) + + +async def test_changed_file_listing_shorter_than_reported_fails_closed() -> None: + fake = FakeGitHub() + fake.route("GET", f"/repos/{REPOSITORY}/pulls/12", _pull(changed_files=3)) + with pytest.raises(GitHubFactsIncompleteError): + await _fetch(_adapter(fake)) + + +async def test_check_run_app_and_status_source_are_recorded() -> None: + fake = FakeGitHub() + fake.route( + "GET", + f"/repos/{REPOSITORY}/branches/main/protection/required_status_checks", + {"contexts": ["lint"], "checks": []}, + ) + fake.route( + "GET", + f"/repos/{REPOSITORY}/commits/{HEAD_SHA}/check-runs", + { + "check_runs": [ + { + "id": 5, + "name": "lint", + "head_sha": HEAD_SHA, "status": "completed", + "conclusion": "success", + "app": {"id": 777}, + } + ] + }, + ) + fake.route("GET", f"/repos/{REPOSITORY}/commits/{HEAD_SHA}/status", {"sha": HEAD_SHA, "statuses": []}) + facts = await _fetch(_adapter(fake)) + check = facts.required_checks[0] + assert (check.app_bound, check.check_run_app_id, check.commit_status_judged) == ( + False, + 777, + False, + ) + + +def _pull_reads(fake: FakeGitHub) -> int: + return sum(1 for r in fake.requests if r.url.path == f"/repos/{REPOSITORY}/pulls/12") + + +async def test_pull_request_moving_once_is_reread_and_bound_to_one_state() -> None: + fake = FakeGitHub() + moved = _pull(head={"sha": "c" * 40}) + # The first whole read ends on a moved head; the second sees one state. + fake.route("GET", f"/repos/{REPOSITORY}/pulls/12", _pull(), moved, _pull()) + facts = await _fetch(_adapter(fake)) + assert facts.head_sha == HEAD_SHA + assert _pull_reads(fake) == 4 + labels = [item.request for item in facts.responses] + assert labels[0] == "pull_request" and labels[-1] == "pull_request_final" + assert labels.count("pull_request") == 1 + + +async def test_head_that_keeps_moving_fails_closed_as_inconsistent() -> None: + fake = FakeGitHub() + shas = [f"{index:x}" * 40 for index in range(1, 8)] + for sha in shas: + fake.route("GET", f"/repos/{REPOSITORY}/commits/{sha}/check-runs", {"check_runs": []}) + fake.route("GET", f"/repos/{REPOSITORY}/commits/{sha}/status", {"sha": sha, "statuses": []}) + fake.route("GET", f"/repos/{REPOSITORY}/pulls/12", *(_pull(head={"sha": sha}) for sha in shas)) + with pytest.raises(GitHubFactsInconsistentError) as caught: + await _fetch(_adapter(fake)) + assert caught.value.retryable is True + assert _pull_reads(fake) == 6 + + +async def test_merged_flag_flipping_during_the_read_is_inconsistent() -> None: + fake = FakeGitHub() + open_pull = _pull(state="open", merged=False) + fake.route( + "GET", + f"/repos/{REPOSITORY}/pulls/12", + open_pull, _pull(), open_pull, _pull(), open_pull, _pull(), + ) + with pytest.raises(GitHubFactsInconsistentError): + await _fetch(_adapter(fake)) + + +async def test_check_run_for_another_commit_is_a_protocol_error() -> None: + fake = FakeGitHub() + fake.route( + "GET", + f"/repos/{REPOSITORY}/commits/{HEAD_SHA}/check-runs", + {"check_runs": [{"id": 9, "name": "backend", "head_sha": "c" * 40, + "status": "completed", "conclusion": "success", + "app": {"id": 15368}}]}, + ) + with pytest.raises(GitHubFactsProtocolError): + await _fetch(_adapter(fake)) + + +async def test_combined_status_for_another_commit_is_a_protocol_error() -> None: + fake = FakeGitHub() + fake.route( + "GET", + f"/repos/{REPOSITORY}/commits/{HEAD_SHA}/status", + {"sha": "c" * 40, "statuses": []}, + ) + with pytest.raises(GitHubFactsProtocolError): + await _fetch(_adapter(fake)) + + +@pytest.mark.parametrize( + ("app_id", "details_url", "expected"), + [ + (15368, RUN_DETAILS_URL, 345), + (15368, f"https://github.com/{REPOSITORY}/actions/runs/345", 345), + (999, RUN_DETAILS_URL, None), + (15368, "https://github.com/acme/other/actions/runs/345/job/1", None), + (15368, f"https://example.test/{REPOSITORY}/actions/runs/345", None), + (15368, None, None), + ], + ids=("actions-job", "actions-run", "other-app", "other-repo", "other-host", "none"), +) +async def test_workflow_run_id_comes_only_from_actions_runs_of_this_repository( + app_id: int, details_url: str | None, expected: int | None +) -> None: + fake = FakeGitHub() + fake.route( + "GET", + f"/repos/{REPOSITORY}/branches/main/protection/required_status_checks", + {"contexts": [], "checks": [{"context": "backend", "app_id": app_id}]}, + ) + fake.route( + "GET", + f"/repos/{REPOSITORY}/commits/{HEAD_SHA}/check-runs", + {"check_runs": [{"id": 9, "name": "backend", "head_sha": HEAD_SHA, + "status": "completed", "conclusion": "success", + "details_url": details_url, "app": {"id": app_id}}]}, + ) + facts = await _fetch(_adapter(fake)) + assert facts.required_checks[0].workflow_run_id == expected + assert facts.required_checks[0].check_run_details_url == details_url + + +@pytest.mark.parametrize( + "url", + [ + "https://github.com/acme/widgets/pull/12/", + "https://github.com/acme/widgets/pull/12?x=1", + "https://github.com/acme/widgets/pull/12#top", + "https://github.com:443/acme/widgets/pull/12", + "https://user@github.com/acme/widgets/pull/12", + "https://github.com/acme/widgets/pull/%31%32", + "https://github.com/acme\\widgets/pull/12", + "https://github.com/acme/widgets/pull/012", + "https://github.com/acme/widgéts/pull/12", + "http://github.com/acme/widgets/pull/12", + "https://github.com/acme/widgets/pulls/12", + "https://github.com/acme/../pull/12", + 12, + ], +) +def test_pull_request_url_parser_rejects_noncanonical_forms(url: object) -> None: + assert parse_pull_request_url(url) is None + + +def test_pull_request_url_parser_returns_repository_and_number() -> None: + assert parse_pull_request_url("https://github.com/Acme/widgets.v2/pull/12") == ( + "Acme/widgets.v2", + 12, + ) + + + +# Fail-closed branches: each malformed or hostile response is rejected --------- + +PROTECTION = f"/repos/{REPOSITORY}/branches/main/protection/required_status_checks" +RUNS = f"/repos/{REPOSITORY}/commits/{HEAD_SHA}/check-runs" +STATUS = f"/repos/{REPOSITORY}/commits/{HEAD_SHA}/status" +EC_KEY = ec.generate_private_key(ec.SECP256R1()) + + +def test_private_key_loader_rejects_oversized_and_non_rsa_keys() -> None: + oversized = PRIVATE_PEM + "\n" + "A" * 20_000 + ec_pem = EC_KEY.private_bytes( + serialization.Encoding.PEM, + serialization.PrivateFormat.PKCS8, + serialization.NoEncryption(), + ).decode() + for value in (oversized, ec_pem, "é" * 10): + with pytest.raises(ExternalServiceConfigurationError) as caught: + load_github_app_private_key(SecretStr(value)) + assert caught.value.__cause__ is None + with pytest.raises(ExternalServiceConfigurationError): + GitHubAppFactsAdapter( + GitHubAppFactsConfiguration( + app_id=APP_ID, + installation_id=None, + api_base_url=API, + request_timeout_seconds=10.0, + total_deadline_seconds=45.0, + maximum_attempts=3, + ), + EC_KEY, # type: ignore[arg-type] + ) + + +@pytest.mark.parametrize( + "lookup", + [{}, {"id": "77"}, {"id": 0}, []], + ids=("no-id", "string-id", "zero-id", "not-object"), +) +async def test_unusable_installation_lookup_is_a_protocol_error(lookup: object) -> None: + fake = FakeGitHub() + fake.route("GET", f"/repos/{REPOSITORY}/installation", lookup) + with pytest.raises(GitHubFactsProtocolError): + await _fetch(_adapter(fake)) + + +@pytest.mark.parametrize( + "token_body", + [ + {"token": TOKEN, "expires_at": "later"}, + {"token": TOKEN, "expires_at": "2027-01-15T09:00:00"}, + {"token": TOKEN}, + {"token": "", "expires_at": "2027-01-15T09:00:00Z"}, + {"token": "ghs_bad token", "expires_at": "2027-01-15T09:00:00Z"}, + {"token": 5, "expires_at": "2027-01-15T09:00:00Z"}, + [], + ], + ids=("bad-expiry", "naive-expiry", "no-expiry", "empty", "whitespace", "non-string", "list"), +) +async def test_unusable_token_exchange_is_a_protocol_error(token_body: object) -> None: + fake = FakeGitHub() + fake.route( + "POST", + "/app/installations/77/access_tokens", + httpx.Response(201, json=token_body), + ) + with pytest.raises(GitHubFactsProtocolError): + await _fetch(_adapter(fake)) + assert not any(r.url.path.endswith("/pulls/12") for r in fake.requests) + + +async def test_app_jwt_rejected_is_access_denied() -> None: + fake = FakeGitHub() + fake.route("GET", f"/repos/{REPOSITORY}/installation", httpx.Response(401)) + with pytest.raises(GitHubFactsAccessDeniedError): + await _fetch(_adapter(fake)) + + +async def test_unexpected_client_error_is_a_protocol_error_without_retry() -> None: + fake = FakeGitHub() + clock = Clock() + fake.route("GET", f"/repos/{REPOSITORY}/pulls/12", httpx.Response(422)) + with pytest.raises(GitHubFactsProtocolError): + await _fetch(_adapter(fake, clock)) + assert clock.sleeps == [] + + +@pytest.mark.parametrize( + "headers", + [ + {"retry-after": "soon"}, + {"x-ratelimit-remaining": "0"}, + {"x-ratelimit-remaining": "0", "x-ratelimit-reset": "tomorrow"}, + ], + ids=("bad-retry-after", "no-reset", "bad-reset"), +) +async def test_forbidden_with_unusable_rate_limit_headers_is_access_denied( + headers: dict, +) -> None: + fake = FakeGitHub() + clock = Clock() + fake.route("GET", f"/repos/{REPOSITORY}/pulls/12", httpx.Response(403, headers=headers)) + with pytest.raises(GitHubFactsAccessDeniedError): + await _fetch(_adapter(fake, clock)) + assert clock.sleeps == [] + + +async def test_deadline_spent_by_one_slow_response_stops_further_requests() -> None: + fake = FakeGitHub() + clock = Clock() + + def slow_pull(_request: httpx.Request) -> httpx.Response: + clock.now += 60.0 + return httpx.Response(200, json=_pull()) + + fake.route("GET", f"/repos/{REPOSITORY}/pulls/12", slow_pull) + with pytest.raises(GitHubFactsUnavailableError): + await _fetch(_adapter(fake, clock, deadline=45.0)) + assert not any(r.url.path == PROTECTION for r in fake.requests) + + +@pytest.mark.parametrize( + "pull", + [ + [], + {k: v for k, v in _pull().items() if k != "base"}, + _pull(head={"sha": None}), + _pull(changed_files="2"), + _pull(changed_files=-1), + _pull(number=13), + _pull(user=None), + _pull(user={"login": "agent-bot", "id": 501}), + _pull(merged=True, merge_commit_sha="not-a-sha"), + _pull(state="draft"), + ], + ids=( + "not-object", + "no-base", + "null-head", + "string-file-count", + "negative-file-count", + "other-number", + "null-author", + "author-without-type", + "bad-merge-sha", + "unknown-state", + ), +) +async def test_malformed_pull_request_is_a_protocol_error(pull: object) -> None: + fake = FakeGitHub() + fake.route("GET", f"/repos/{REPOSITORY}/pulls/12", pull) + with pytest.raises(GitHubFactsProtocolError): + await _fetch(_adapter(fake)) + + +@pytest.mark.parametrize( + "protection", + [ + [], + {"checks": "backend", "contexts": []}, + {"checks": [], "contexts": "backend"}, + {"checks": [{"app_id": 1}], "contexts": []}, + {"checks": [{"context": "", "app_id": 1}], "contexts": []}, + {"checks": ["backend"], "contexts": []}, + {"checks": [], "contexts": [7]}, + ], + ids=( + "not-object", + "checks-not-list", + "contexts-not-list", + "check-without-context", + "empty-context", + "check-not-object", + "context-not-string", + ), +) +async def test_malformed_branch_protection_is_a_protocol_error(protection: object) -> None: + fake = FakeGitHub() + fake.route("GET", PROTECTION, protection) + with pytest.raises(GitHubFactsProtocolError): + await _fetch(_adapter(fake)) + + +@pytest.mark.parametrize( + ("path", "body"), + [ + (RUNS, {"check_runs": "backend"}), + (RUNS, {"check_runs": ["backend"]}), + (RUNS, []), + (f"/repos/{REPOSITORY}/pulls/12/reviews", {"reviews": []}), + (STATUS, {"sha": HEAD_SHA, "statuses": None}), + ], + ids=("runs-not-list", "run-not-object", "runs-body-list", "reviews-object", "statuses-null"), +) +async def test_malformed_listing_page_is_a_protocol_error(path: str, body: object) -> None: + fake = FakeGitHub() + fake.route("GET", path, body) + with pytest.raises(GitHubFactsProtocolError): + await _fetch(_adapter(fake)) + + +async def test_listing_longer_than_the_page_bound_fails_closed() -> None: + fake = FakeGitHub() + reviews = f"/repos/{REPOSITORY}/pulls/12/reviews" + fake.route( + "GET", + reviews, + httpx.Response( + 200, + json=[REVIEWS[0]], + headers={"link": f'<{API}{reviews}?per_page=100&page=99>; rel="next"'}, + ), + ) + with pytest.raises(GitHubFactsProtocolError): + await _fetch(_adapter(fake)) + assert sum(r.url.path == reviews for r in fake.requests) == 10 + + +@pytest.mark.parametrize( + ("runs", "statuses"), + [ + ([{"name": "lint", "head_sha": HEAD_SHA, "status": "completed", + "conclusion": "success"}], []), + ([{"id": "5", "name": "lint", "head_sha": HEAD_SHA, "status": "completed", + "conclusion": "success"}], []), + ([{"id": 5, "name": "lint", "head_sha": HEAD_SHA, "status": "completed", + "conclusion": "success", "app": {"id": "15368"}}], []), + ([{"id": 5, "name": "lint", "head_sha": HEAD_SHA, "status": "completed", + "conclusion": "exploded"}], []), + ([], [{"id": 1, "context": "lint", "state": "green"}]), + ([], [{"context": "lint", "state": "success"}]), + ], + ids=( + "run-without-id", + "run-string-id", + "run-string-app-id", + "unknown-conclusion", + "unknown-status-state", + "status-without-id", + ), +) +async def test_malformed_check_result_is_a_protocol_error(runs: list, statuses: list) -> None: + fake = FakeGitHub() + fake.route("GET", PROTECTION, {"contexts": ["lint"], "checks": []}) + fake.route("GET", RUNS, {"check_runs": runs}) + fake.route("GET", STATUS, {"sha": HEAD_SHA, "statuses": statuses}) + with pytest.raises(GitHubFactsProtocolError): + await _fetch(_adapter(fake)) + + +async def test_malformed_review_is_a_protocol_error() -> None: + fake = FakeGitHub() + fake.route( + "GET", + f"/repos/{REPOSITORY}/pulls/12/reviews", + [{**REVIEWS[0], "commit_id": "not-a-sha"}], + ) + with pytest.raises(GitHubFactsProtocolError): + await _fetch(_adapter(fake)) + + +def test_facts_model_rejects_a_noncanonical_repository() -> None: + facts = GitHubPullRequestFacts.model_validate( + { + "repository": REPOSITORY, + "pull_request_number": 12, + "html_url": f"https://github.com/{REPOSITORY}/pull/12", + "state": "closed", + "merged": True, + "merge_commit_sha": MERGE_SHA, + "base_ref": "main", + "head_sha": HEAD_SHA, + "author": {"login": "a", "id": 1, "type": "User"}, + "reviews": [], + "changed_files_count": 0, + "changed_files": [], + "required_checks": [ + { + "name": "backend", + "app_id": 1, + "app_bound": True, + "conclusion": "success", + "check_run_app_id": 1, + "check_run_head_sha": HEAD_SHA, + "check_run_html_url": None, + "check_run_details_url": None, + "workflow_run_id": None, + "commit_status_judged": False, + "commit_status_sha": None, + } + ], + "fetched_at": "2027-01-15T08:00:00Z", + "responses": [{"request": "pull_request", "sha256": "sha256:" + "0" * 64}], + } + ) + for repository in ("acme", "acme/..", "-acme/widgets"): + with pytest.raises(ValidationError): + GitHubPullRequestFacts.model_validate( + {**facts.model_dump(), "repository": repository} + )