From 38982460b9f579516927b42904ecc09dcba72e35 Mon Sep 17 00:00:00 2001 From: luisleo526 Date: Tue, 20 Oct 2026 12:22:22 +0800 Subject: [PATCH 01/13] ci: run all CI on the pf-ci pool and add an authz identity guard Every job that had its own runs-on in ci.yml, docs.yml and native-feed.yml now uses the plain pool labels ["self-hosted","pf-ci","pf-ci-x64"]: no expression, no hosted fallback, pull requests included (they were forced to ubuntu-24.04 before), dependabot included. The native-feed release matrix runs its Linux x64 leg on the x64 pool labels and its Linux arm64 leg on ["self-hosted","pf-ci","pf-ci-arm64"]; only macos-14 stays on GitHub. docs.yml deploy (Pages environment) is unchanged. Each of the three workflows gets a first job `authz` that fails red (exit 1, never a skip) unless the triggering actor is dependabot[bot] or a repository collaborator with admin, maintain or write permission, checked with one call to GET /repos/{repo}/collaborators/{actor}/permission using the default GITHUB_TOKEN (no new secret). Fork pull requests fail red without a lookup; an API error or an empty answer fails red. Every other job needs authz, and ci.yml `ready` fails unless authz succeeded. Co-Authored-By: Claude Sonnet 5.5 --- .github/workflows/ci.yml | 106 ++++++++++++++++-------------- .github/workflows/docs.yml | 51 +++++++++++--- .github/workflows/native-feed.yml | 71 +++++++++++++------- 3 files changed, 146 insertions(+), 82 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3cea7ac..b05d873 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,8 +11,8 @@ permissions: # Only a superseded pull_request run is cancelled: every other event (push to main, # whose run authorizes publication) gets its own run_id group and is never cancelled. -# Pull requests (same-repo, fork, dependabot) run on GitHub-hosted ubuntu-24.04 only; -# push keeps the pool. Docs-only PRs skip the four runtime jobs (docker, database-e2e, +# Every job runs on the pf-ci pool (plain labels, no hosted fallback); dependabot counts as ours. +# Fork PRs fail authz. Docs-only PRs skip the four runtime jobs (docker, database-e2e, # contract, distribution); the Python matrix always runs because tests/test_documentation.py # checks the links in README.md, CONTRIBUTING.md and docs/. release/ready then accepts # those four as skipped and writes no release evidence; release PRs change code, so the @@ -22,9 +22,48 @@ concurrency: cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: + authz: + name: authz + runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] + timeout-minutes: 2 + permissions: + contents: read + steps: + - shell: bash + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + ACTOR: ${{ github.triggering_actor }} + REPO: ${{ github.repository }} + FORK_PR: >- + ${{ (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && + (github.event.pull_request.head.repo.full_name != github.repository || github.event.pull_request.head.repo.fork != false) }} + run: | + set -euo pipefail + if [ "$FORK_PR" = "true" ]; then + echo "::error::Actor '$ACTOR' permission '': fork pull requests are not authorized." + exit 1 + fi + if [ "$ACTOR" = 'dependabot[bot]' ]; then + echo "Authorized dependabot[bot]." + exit 0 + fi + if ! perm=$(gh api "repos/${REPO}/collaborators/${ACTOR}/permission" --jq '.permission'); then + echo "::error::Actor '$ACTOR' permission '': collaborator permission API request failed." + exit 1 + fi + if [ -z "$perm" ]; then + echo "::error::Actor '$ACTOR' permission '': collaborator permission API returned no permission." + exit 1 + fi + case "$perm" in + admin|maintain|write) exit 0 ;; + *) echo "::error::Actor '$ACTOR' permission '$perm': write access is required."; exit 1 ;; + esac + changes: + needs: authz if: github.event_name == 'pull_request' - runs-on: ubuntu-24.04 + runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] timeout-minutes: 5 outputs: code: ${{ steps.paths.outputs.code }} @@ -50,13 +89,8 @@ jobs: test: name: Python ${{ matrix.python-version }} - runs-on: >- - ${{ github.event_name != 'pull_request' && github.event_name != 'pull_request_target' && - github.actor != 'dependabot[bot]' && github.triggering_actor != 'dependabot[bot]' && - contains(fromJSON('["push","pull_request","workflow_dispatch","schedule","release"]'), github.event_name) && - (github.event_name != 'pull_request' || - (github.event.pull_request.head.repo.full_name == github.repository && github.event.pull_request.head.repo.fork == false)) && - fromJSON('["self-hosted","pf-ci","pf-ci-x64"]') || 'ubuntu-24.04' }} + needs: authz + runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] env: PF_CI_LABELS: '["self-hosted","pf-ci","pf-ci-x64"]' strategy: @@ -88,18 +122,12 @@ jobs: docker: name: Docker runtime - needs: changes + needs: [authz, changes] if: >- !cancelled() && (github.event_name != 'pull_request' || (needs.changes.result == 'success' && needs.changes.outputs.code == 'true')) - runs-on: >- - ${{ github.event_name != 'pull_request' && github.event_name != 'pull_request_target' && - github.actor != 'dependabot[bot]' && github.triggering_actor != 'dependabot[bot]' && - contains(fromJSON('["push","pull_request","workflow_dispatch","schedule","release"]'), github.event_name) && - (github.event_name != 'pull_request' || - (github.event.pull_request.head.repo.full_name == github.repository && github.event.pull_request.head.repo.fork == false)) && - fromJSON('["self-hosted","pf-ci","pf-ci-x64"]') || 'ubuntu-24.04' }} + runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] env: PF_CI_LABELS: '["self-hosted","pf-ci","pf-ci-x64"]' steps: @@ -117,18 +145,12 @@ jobs: database-e2e: name: Database E2E - needs: changes + needs: [authz, changes] if: >- !cancelled() && (github.event_name != 'pull_request' || (needs.changes.result == 'success' && needs.changes.outputs.code == 'true')) - runs-on: >- - ${{ github.event_name != 'pull_request' && github.event_name != 'pull_request_target' && - github.actor != 'dependabot[bot]' && github.triggering_actor != 'dependabot[bot]' && - contains(fromJSON('["push","pull_request","workflow_dispatch","schedule","release"]'), github.event_name) && - (github.event_name != 'pull_request' || - (github.event.pull_request.head.repo.full_name == github.repository && github.event.pull_request.head.repo.fork == false)) && - fromJSON('["self-hosted","pf-ci","pf-ci-x64"]') || 'ubuntu-24.04' }} + runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] services: mysql: image: mysql:8.4 @@ -181,18 +203,12 @@ jobs: contract: name: Release contract - needs: changes + needs: [authz, changes] if: >- !cancelled() && (github.event_name != 'pull_request' || (needs.changes.result == 'success' && needs.changes.outputs.code == 'true')) - runs-on: >- - ${{ github.event_name != 'pull_request' && github.event_name != 'pull_request_target' && - github.actor != 'dependabot[bot]' && github.triggering_actor != 'dependabot[bot]' && - contains(fromJSON('["push","pull_request","workflow_dispatch","schedule","release"]'), github.event_name) && - (github.event_name != 'pull_request' || - (github.event.pull_request.head.repo.full_name == github.repository && github.event.pull_request.head.repo.fork == false)) && - fromJSON('["self-hosted","pf-ci","pf-ci-x64"]') || 'ubuntu-24.04' }} + runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] env: PF_CI_LABELS: '["self-hosted","pf-ci","pf-ci-x64"]' steps: @@ -209,18 +225,12 @@ jobs: distribution: name: Distribution - needs: changes + needs: [authz, changes] if: >- !cancelled() && (github.event_name != 'pull_request' || (needs.changes.result == 'success' && needs.changes.outputs.code == 'true')) - runs-on: >- - ${{ github.event_name != 'pull_request' && github.event_name != 'pull_request_target' && - github.actor != 'dependabot[bot]' && github.triggering_actor != 'dependabot[bot]' && - contains(fromJSON('["push","pull_request","workflow_dispatch","schedule","release"]'), github.event_name) && - (github.event_name != 'pull_request' || - (github.event.pull_request.head.repo.full_name == github.repository && github.event.pull_request.head.repo.fork == false)) && - fromJSON('["self-hosted","pf-ci","pf-ci-x64"]') || 'ubuntu-24.04' }} + runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] env: PF_CI_LABELS: '["self-hosted","pf-ci","pf-ci-x64"]' steps: @@ -277,20 +287,15 @@ jobs: ready: name: release/ready if: always() - needs: [changes, test, contract, docker, database-e2e, distribution] - runs-on: >- - ${{ github.event_name != 'pull_request' && github.event_name != 'pull_request_target' && - github.actor != 'dependabot[bot]' && github.triggering_actor != 'dependabot[bot]' && - contains(fromJSON('["push","pull_request","workflow_dispatch","schedule","release"]'), github.event_name) && - (github.event_name != 'pull_request' || - (github.event.pull_request.head.repo.full_name == github.repository && github.event.pull_request.head.repo.fork == false)) && - fromJSON('["self-hosted","pf-ci","pf-ci-x64"]') || 'ubuntu-24.04' }} + needs: [authz, changes, test, contract, docker, database-e2e, distribution] + runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] env: PF_CI_LABELS: '["self-hosted","pf-ci","pf-ci-x64"]' steps: - name: Require the path decision and every expected job result id: gate env: + AUTHZ_RESULT: ${{ needs.authz.result }} EVENT_NAME: ${{ github.event_name }} CHANGES_RESULT: ${{ needs.changes.result }} CODE_CHANGED: ${{ needs.changes.outputs.code }} @@ -299,6 +304,7 @@ jobs: ${{ needs.contract.result }} ${{ needs.docker.result }} ${{ needs.database-e2e.result }} ${{ needs.distribution.result }} run: | + if [ "$AUTHZ_RESULT" != "success" ]; then echo "authz: $AUTHZ_RESULT"; exit 1; fi evidence=true if [ "$EVENT_NAME" = pull_request ]; then if [ "$CHANGES_RESULT" != success ]; then diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index ddd0a51..57967dd 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -10,20 +10,53 @@ permissions: contents: read # Only a superseded pull_request run is cancelled; a push to main (Pages deploy) and a -# dispatch are never cancelled. Pull requests run on a GitHub-hosted runner. +# dispatch are never cancelled. authz and build run on the pf-ci pool (plain labels, no hosted fallback); fork PRs fail authz. concurrency: group: pages-${{ github.event_name == 'pull_request' && github.event.pull_request.number || github.run_id }} cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: + authz: + name: authz + runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] + timeout-minutes: 2 + permissions: + contents: read + steps: + - shell: bash + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + ACTOR: ${{ github.triggering_actor }} + REPO: ${{ github.repository }} + FORK_PR: >- + ${{ (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && + (github.event.pull_request.head.repo.full_name != github.repository || github.event.pull_request.head.repo.fork != false) }} + run: | + set -euo pipefail + if [ "$FORK_PR" = "true" ]; then + echo "::error::Actor '$ACTOR' permission '': fork pull requests are not authorized." + exit 1 + fi + if [ "$ACTOR" = 'dependabot[bot]' ]; then + echo "Authorized dependabot[bot]." + exit 0 + fi + if ! perm=$(gh api "repos/${REPO}/collaborators/${ACTOR}/permission" --jq '.permission'); then + echo "::error::Actor '$ACTOR' permission '': collaborator permission API request failed." + exit 1 + fi + if [ -z "$perm" ]; then + echo "::error::Actor '$ACTOR' permission '': collaborator permission API returned no permission." + exit 1 + fi + case "$perm" in + admin|maintain|write) exit 0 ;; + *) echo "::error::Actor '$ACTOR' permission '$perm': write access is required."; exit 1 ;; + esac + build: - runs-on: >- - ${{ github.event_name != 'pull_request' && github.event_name != 'pull_request_target' && - github.actor != 'dependabot[bot]' && github.triggering_actor != 'dependabot[bot]' && - contains(fromJSON('["push","pull_request","workflow_dispatch","schedule","release"]'), github.event_name) && - (github.event_name != 'pull_request' || - (github.event.pull_request.head.repo.full_name == github.repository && github.event.pull_request.head.repo.fork == false)) && - fromJSON('["self-hosted","pf-ci","pf-ci-x64"]') || 'ubuntu-24.04' }} + needs: authz + runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] env: PF_CI_LABELS: '["self-hosted","pf-ci","pf-ci-x64"]' steps: @@ -44,7 +77,7 @@ jobs: deploy: if: github.event_name != 'pull_request' - needs: build + needs: [authz, build] runs-on: ubuntu-latest environment: name: github-pages diff --git a/.github/workflows/native-feed.yml b/.github/workflows/native-feed.yml index 01bbe6a..72151ee 100644 --- a/.github/workflows/native-feed.yml +++ b/.github/workflows/native-feed.yml @@ -12,28 +12,63 @@ permissions: contents: read # Only a superseded pull_request run is cancelled; push and dispatch are never cancelled. -# Pull requests run on GitHub-hosted runners only (ubuntu-24.04, ubuntu-24.04-arm, macos-14); -# push and dispatch keep the pool for the Linux x64 jobs. +# authz, asan-ubsan, tsan and the Linux release legs run on the pf-ci pool (x64 and arm64 labels, no hosted fallback). +# Fork PRs fail authz; only macos-14 stays on GitHub. concurrency: group: native-feed-${{ github.event_name == 'pull_request' && github.event.pull_request.number || github.run_id }} cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: + authz: + name: authz + runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] + timeout-minutes: 2 + permissions: + contents: read + steps: + - shell: bash + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + ACTOR: ${{ github.triggering_actor }} + REPO: ${{ github.repository }} + FORK_PR: >- + ${{ (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && + (github.event.pull_request.head.repo.full_name != github.repository || github.event.pull_request.head.repo.fork != false) }} + run: | + set -euo pipefail + if [ "$FORK_PR" = "true" ]; then + echo "::error::Actor '$ACTOR' permission '': fork pull requests are not authorized." + exit 1 + fi + if [ "$ACTOR" = 'dependabot[bot]' ]; then + echo "Authorized dependabot[bot]." + exit 0 + fi + if ! perm=$(gh api "repos/${REPO}/collaborators/${ACTOR}/permission" --jq '.permission'); then + echo "::error::Actor '$ACTOR' permission '': collaborator permission API request failed." + exit 1 + fi + if [ -z "$perm" ]; then + echo "::error::Actor '$ACTOR' permission '': collaborator permission API returned no permission." + exit 1 + fi + case "$perm" in + admin|maintain|write) exit 0 ;; + *) echo "::error::Actor '$ACTOR' permission '$perm': write access is required."; exit 1 ;; + esac + release: name: Release (${{ matrix.os }}) + needs: authz strategy: fail-fast: false matrix: os: [ubuntu-24.04, ubuntu-24.04-arm, macos-14] include: - os: ubuntu-24.04 - runner: >- - ${{ github.event_name != 'pull_request' && github.event_name != 'pull_request_target' && - github.actor != 'dependabot[bot]' && github.triggering_actor != 'dependabot[bot]' && - contains(fromJSON('["push","pull_request","workflow_dispatch","schedule","release"]'), github.event_name) && - (github.event_name != 'pull_request' || - (github.event.pull_request.head.repo.full_name == github.repository && github.event.pull_request.head.repo.fork == false)) && - fromJSON('["self-hosted","pf-ci","pf-ci-x64"]') || 'ubuntu-24.04' }} + runner: ${{ fromJSON('["self-hosted","pf-ci","pf-ci-x64"]') }} + - os: ubuntu-24.04-arm + runner: ${{ fromJSON('["self-hosted","pf-ci","pf-ci-arm64"]') }} runs-on: ${{ matrix.runner || matrix.os }} env: PF_CI_LABELS: '["self-hosted","pf-ci","pf-ci-x64"]' @@ -61,13 +96,8 @@ jobs: asan-ubsan: name: ASan + UBSan - runs-on: >- - ${{ github.event_name != 'pull_request' && github.event_name != 'pull_request_target' && - github.actor != 'dependabot[bot]' && github.triggering_actor != 'dependabot[bot]' && - contains(fromJSON('["push","pull_request","workflow_dispatch","schedule","release"]'), github.event_name) && - (github.event_name != 'pull_request' || - (github.event.pull_request.head.repo.full_name == github.repository && github.event.pull_request.head.repo.fork == false)) && - fromJSON('["self-hosted","pf-ci","pf-ci-x64"]') || 'ubuntu-24.04' }} + needs: authz + runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] env: PF_CI_LABELS: '["self-hosted","pf-ci","pf-ci-x64"]' timeout-minutes: 30 @@ -89,13 +119,8 @@ jobs: tsan: name: TSan - runs-on: >- - ${{ github.event_name != 'pull_request' && github.event_name != 'pull_request_target' && - github.actor != 'dependabot[bot]' && github.triggering_actor != 'dependabot[bot]' && - contains(fromJSON('["push","pull_request","workflow_dispatch","schedule","release"]'), github.event_name) && - (github.event_name != 'pull_request' || - (github.event.pull_request.head.repo.full_name == github.repository && github.event.pull_request.head.repo.fork == false)) && - fromJSON('["self-hosted","pf-ci","pf-ci-x64"]') || 'ubuntu-24.04' }} + needs: authz + runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] env: PF_CI_LABELS: '["self-hosted","pf-ci","pf-ci-x64"]' timeout-minutes: 30 From 28d568d08a8d17bac09b76ec5d3cc29f9bcabfa7 Mon Sep 17 00:00:00 2001 From: luisleo526 Date: Tue, 20 Oct 2026 12:30:20 +0800 Subject: [PATCH 02/13] ci: the runtime jobs also require authz to have succeeded docker, database-e2e, contract and distribution use `!cancelled() && ...` in their `if:`, which replaces the implicit success() over `needs`. On a push where authz failed they would still have run on the pool. Add `needs.authz.result == 'success' &&` to those four conditions so that no job runs unless authz passed (owner rulings B / #800 / #801). Co-Authored-By: Claude Sonnet 5.5 --- .github/workflows/ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b05d873..c114589 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -125,6 +125,7 @@ jobs: needs: [authz, changes] if: >- !cancelled() && + needs.authz.result == 'success' && (github.event_name != 'pull_request' || (needs.changes.result == 'success' && needs.changes.outputs.code == 'true')) runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] @@ -148,6 +149,7 @@ jobs: needs: [authz, changes] if: >- !cancelled() && + needs.authz.result == 'success' && (github.event_name != 'pull_request' || (needs.changes.result == 'success' && needs.changes.outputs.code == 'true')) runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] @@ -206,6 +208,7 @@ jobs: needs: [authz, changes] if: >- !cancelled() && + needs.authz.result == 'success' && (github.event_name != 'pull_request' || (needs.changes.result == 'success' && needs.changes.outputs.code == 'true')) runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] @@ -228,6 +231,7 @@ jobs: needs: [authz, changes] if: >- !cancelled() && + needs.authz.result == 'success' && (github.event_name != 'pull_request' || (needs.changes.result == 'success' && needs.changes.outputs.code == 'true')) runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] From a28b58e2f1030f3f6ac8bf1a0c3448fec4841dee Mon Sep 17 00:00:00 2001 From: luisleo526 Date: Tue, 20 Oct 2026 13:27:54 +0800 Subject: [PATCH 03/13] test: pin the new CI policy (pool labels, authz, bot logins) in the workflow tests The workflow tests pinned the old policy (pull requests forced onto hosted runners, no secrets, no needs). Update them to the owner rulings: every job uses the plain pool labels without a hosted fallback, authz is needed by every job and gates release/ready and the runtime jobs, authz may use only secrets.GITHUB_TOKEN with contents: read, and its bot allowlist is the exact dependabot[bot] and pineforge-app[bot]. Each changed pin quotes its old assertion and gains a new rejected case; no test is deleted or loosened. Co-Authored-By: Claude Sonnet 5.5 --- tests/test_pr_tier_workflows.py | 625 +++++++++++++++++++++++++++++--- tests/test_release_adapter.py | 21 +- 2 files changed, 601 insertions(+), 45 deletions(-) diff --git a/tests/test_pr_tier_workflows.py b/tests/test_pr_tier_workflows.py index e5f80fb..d80fe40 100644 --- a/tests/test_pr_tier_workflows.py +++ b/tests/test_pr_tier_workflows.py @@ -1,7 +1,8 @@ -"""Pull-request tier of the CI workflows: hosted runners, cancellation and the docs-only path.""" +"""Pull-request tier of CI: pool runners, authorization, cancellation and the docs-only path.""" from __future__ import annotations +import copy import json import os import re @@ -24,6 +25,12 @@ ) HOSTED_FALLBACK = "|| 'ubuntu-24.04' }}" POOL = "fromJSON('[\"self-hosted\",\"pf-ci\",\"pf-ci-x64\"]')" +POOL_LABELS = ["self-hosted", "pf-ci", "pf-ci-x64"] +MATRIX_RUNNERS = { + "ubuntu-24.04": "${{ fromJSON('[\"self-hosted\",\"pf-ci\",\"pf-ci-x64\"]') }}", + "ubuntu-24.04-arm": "${{ fromJSON('[\"self-hosted\",\"pf-ci\",\"pf-ci-arm64\"]') }}", +} +# These expressions are rejection fixtures for the superseded hosted-fallback policy. OLD_ROUTE = ( "${{ github.event_name != 'pull_request_target' && github.actor != 'dependabot[bot]' && " "(github.event_name != 'pull_request' || " @@ -39,10 +46,35 @@ "github.event.pull_request.number || github.run_id }}" ) -RUNTIME_IF = ( +OLD_RUNTIME_IF = ( "!cancelled() && (github.event_name != 'pull_request' || " "(needs.changes.result == 'success' && needs.changes.outputs.code == 'true'))" ) +# old: RUNTIME_IF = ( +# "!cancelled() && (github.event_name != 'pull_request' || " +# "(needs.changes.result == 'success' && needs.changes.outputs.code == 'true'))" +# ) +RUNTIME_IF = ( + "!cancelled() && needs.authz.result == 'success' && " + "(github.event_name != 'pull_request' || " + "(needs.changes.result == 'success' && needs.changes.outputs.code == 'true'))" +) +FORK_TEST = 'if [ "$FORK_PR" = "true" ]; then' +BOT_TEST = ( + 'if [ "$ACTOR" = \'dependabot[bot]\' ] || [ "$ACTOR" = \'pineforge-app[bot]\' ]; then' +) +PERMISSION_API = ( + 'if ! perm=$(gh api "repos/${REPO}/collaborators/${ACTOR}/permission" ' + "--jq '.permission'); then" +) +FORK_PR_ENV = ( + "${{ (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && " + "(github.event.pull_request.head.repo.full_name != github.repository || " + "github.event.pull_request.head.repo.fork != false) }}" +) +READY_AUTHZ_CHECK = ( + 'if [ "$AUTHZ_RESULT" != "success" ]; then echo "authz: $AUTHZ_RESULT"; exit 1; fi' +) def load(name: str) -> dict[str, Any]: @@ -56,25 +88,139 @@ def step_script(workflow: str, job: str, step_id: str) -> str: return str(next(step["run"] for step in steps if step.get("id") == step_id)) -def pool_routes(workflow: dict[str, Any]) -> dict[str, str]: - """Whitespace-normalized runs-on and matrix runner expressions that can pick the pool.""" - routes: dict[str, str] = {} +def assert_native_release_matrix(matrix: dict[str, Any]) -> None: + assert matrix["os"] == ["ubuntu-24.04", "ubuntu-24.04-arm", "macos-14"] + includes = matrix["include"] + by_os = {entry["os"]: entry for entry in includes} + assert len(by_os) == len(includes), "matrix includes must not override the same OS twice" + assert set(by_os) <= set(matrix["os"]) + for os_name in matrix["os"]: + entry = by_os.get(os_name, {}) + if os_name == "macos-14": + assert "runner" not in entry + else: + assert entry.get("runner") == MATRIX_RUNNERS[os_name] + + +def assert_runner_policy(workflow: dict[str, Any], name: str = "ci.yml") -> None: for job_id, job in workflow["jobs"].items(): - matrix = job.get("strategy", {}).get("matrix", {}) - includes = matrix.get("include", []) if isinstance(matrix, dict) else [] - candidates = [job.get("runs-on", ""), *(entry.get("runner", "") for entry in includes)] - for index, text in enumerate(candidates): - if isinstance(text, str) and "pf-ci-x64" in text: - routes[f"{job_id}#{index}"] = " ".join(text.split()) - return routes + if name == "docs.yml" and job_id == "deploy": + assert job["runs-on"] == "ubuntu-latest" + elif name == "native-feed.yml" and job_id == "release": + assert job["runs-on"] == "${{ matrix.runner || matrix.os }}" + assert_native_release_matrix(job["strategy"]["matrix"]) + else: + assert job["runs-on"] == POOL_LABELS + +def strings(value: Any) -> list[str]: + if isinstance(value, str): + return [value] + if isinstance(value, dict): + return [text for pair in value.items() for item in pair for text in strings(item)] + if isinstance(value, list): + return [text for item in value for text in strings(item)] + return [] -def unguarded(workflow: dict[str, Any]) -> list[str]: - return [ - route - for route, text in pool_routes(workflow).items() - if not (text.startswith(HOSTED_GUARD) and text.endswith(HOSTED_FALLBACK)) - ] + +def assert_secret_references(value: Any, allowed: set[str]) -> None: + for text in strings(value): + assert re.search(r"\bsecrets\s*\[", text) is None + references = re.findall(r"\bsecrets\s*\.\s*[A-Za-z0-9_-]*", text) + assert set(references) <= allowed + + +def assert_permission_policy(workflow: dict[str, Any], name: str) -> None: + assert set(workflow["permissions"].values()) == {"read"} + # old: assert "secrets." not in (WORKFLOWS / name).read_text() + for key, value in workflow.items(): + if key != "jobs": + assert_secret_references(value, set()) + for job_id, job in workflow["jobs"].items(): + allowed = {"secrets.GITHUB_TOKEN"} if job_id == "authz" else set() + assert_secret_references(job, allowed) + # old: assert job_id == "deploy" + # The old restriction applied whenever a job had its own permissions. + if job_id == "authz": + assert job.get("permissions") == {"contents": "read"} + elif name == "docs.yml" and job_id == "deploy": + assert job["permissions"] == {"pages": "write", "id-token": "write"} + assert job["if"] == "github.event_name != 'pull_request'" + assert job["environment"]["name"] == "github-pages" + else: + assert "permissions" not in job + + +def needs(job: dict[str, Any]) -> set[str]: + value = job.get("needs", []) + assert isinstance(value, (str, list)) + return {value} if isinstance(value, str) else set(value) + + +def assert_authz_dependencies(workflow: dict[str, Any]) -> None: + jobs = workflow["jobs"] + assert next(iter(jobs)) == "authz" + assert "needs" not in jobs["authz"] + for job_id, job in jobs.items(): + if job_id != "authz": + assert "authz" in needs(job) + + +def assert_authz_branch(lines: list[str], condition: str, result: str) -> tuple[int, int]: + assert condition in lines + start = lines.index(condition) + assert "fi" in lines[start + 1 :] + end = lines.index("fi", start + 1) + body = lines[start + 1 : end] + assert len(body) == 2, "each identity branch reports its decision, then exits" + assert re.fullmatch(r'echo "[^"]*"', body[0]) is not None + assert body[1] == f"exit {result}" + return start, end + + +def assert_authz_identity(job: dict[str, Any]) -> None: + assert "if" not in job + assert "continue-on-error" not in job + steps = job["steps"] + for step in steps: + assert "${{" not in step.get("run", "") + assert len(steps) == 1, "authz is one bash identity check" + step = steps[0] + assert step["shell"] == "bash" + assert "if" not in step + assert "continue-on-error" not in step + env = step["env"] + assert env["GH_TOKEN"] == "${{ secrets.GITHUB_TOKEN }}" + assert env["ACTOR"] == "${{ github.triggering_actor }}" + assert env["REPO"] == "${{ github.repository }}" + assert "FORK_PR" in env + assert " ".join(env["FORK_PR"].split()) == FORK_PR_ENV + script = step["run"] + assert "=~" not in script + lines = [line.strip() for line in script.splitlines() if line.strip()] + assert lines[0] == "set -euo pipefail" + fork_start, fork_end = assert_authz_branch(lines, FORK_TEST, "1") + bot_start, bot_end = assert_authz_branch(lines, BOT_TEST, "0") + assert "*" not in lines[bot_start] + assert "case" not in lines[bot_start] + api_start, api_end = assert_authz_branch(lines, PERMISSION_API, "1") + empty_start, empty_end = assert_authz_branch(lines, 'if [ -z "$perm" ]; then', "1") + assert fork_start == 1, "fork rejection precedes every identity shortcut" + assert bot_start == fork_end + 1 + assert api_start == bot_end + 1 + assert empty_start == api_end + 1 + permission_case = lines[empty_end + 1 :] + assert len(permission_case) == 4 + assert permission_case[0] == 'case "$perm" in' + assert permission_case[1] == "admin|maintain|write) exit 0 ;;" + assert re.fullmatch(r'\*\) echo "[^"]*"; exit 1 ;;', permission_case[2]) is not None + assert permission_case[3] == "esac" + + +def assert_ready_authz(job: dict[str, Any]) -> None: + gate = next(step for step in job["steps"] if step.get("id") == "gate") + assert gate["env"].get("AUTHZ_RESULT") == "${{ needs.authz.result }}" + assert gate["run"].splitlines()[0] == READY_AUTHZ_CHECK def cancels_only_pull_requests(concurrency: dict[str, str]) -> bool: @@ -84,20 +230,117 @@ def cancels_only_pull_requests(concurrency: dict[str, str]) -> bool: ) +# old: test_pull_requests_never_select_the_pool @pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) -def test_pull_requests_never_select_the_pool(name: str) -> None: +def test_pool_jobs_use_plain_labels_without_hosted_fallback(name: str) -> None: workflow = load(name) - assert pool_routes(workflow), "push and dispatch keep the pool" - assert unguarded(workflow) == [] + # old: assert pool_routes(workflow), "push and dispatch keep the pool" + assert workflow["jobs"]["authz"]["runs-on"] == POOL_LABELS + # old: assert unguarded(workflow) == [] + assert_runner_policy(workflow, name) def test_the_old_same_repo_route_to_the_pool_is_rejected() -> None: - assert unguarded({"jobs": {"build": {"runs-on": OLD_ROUTE}}}) == ["build#0"] - matrix = {"jobs": {"release": {"strategy": {"matrix": {"include": [{"runner": OLD_ROUTE}]}}}}} - assert unguarded(matrix) == ["release#1"] - mixed = {"jobs": {"a": {"runs-on": GUARDED_ROUTE}, "b": {"runs-on": OLD_ROUTE}}} - assert unguarded(mixed) == ["b#0"] - assert unguarded({"jobs": {"a": {"runs-on": GUARDED_ROUTE}}}) == [] + # old: assert unguarded({"jobs": {"build": {"runs-on": OLD_ROUTE}}}) == ["build#0"] + for route in (OLD_ROUTE, GUARDED_ROUTE, "ubuntu-24.04", "macos-14", "windows-latest"): + with pytest.raises(AssertionError): + assert_runner_policy({"jobs": {"build": {"runs-on": route}}}) + matrix = copy.deepcopy(load("native-feed.yml")) + assert_runner_policy(matrix, "native-feed.yml") + matrix["jobs"]["release"]["strategy"]["matrix"]["include"][0]["runner"] = OLD_ROUTE + # old: assert unguarded(matrix) == ["release#1"] + with pytest.raises(AssertionError): + assert_runner_policy(matrix, "native-feed.yml") + # old: assert unguarded(mixed) == ["b#0"] + for job_id in ("a", "b"): + mixed = {"jobs": {"a": {"runs-on": POOL_LABELS}, "b": {"runs-on": POOL_LABELS}}} + mixed["jobs"][job_id]["runs-on"] = OLD_ROUTE + with pytest.raises(AssertionError): + assert_runner_policy(mixed) + # old: assert unguarded({"jobs": {"a": {"runs-on": GUARDED_ROUTE}}}) == [] + with pytest.raises(AssertionError): + assert_runner_policy({"jobs": {"a": {"runs-on": GUARDED_ROUTE}}}) + assert_runner_policy({"jobs": {"a": {"runs-on": POOL_LABELS}}}) + + +@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) +@pytest.mark.parametrize("route", [GUARDED_ROUTE, "ubuntu-24.04", POOL]) +def test_each_pool_job_rejects_expression_or_hosted_routes(name: str, route: str) -> None: + workflow = load(name) + assert_runner_policy(workflow, name) + for job_id in workflow["jobs"]: + if (name, job_id) in {("docs.yml", "deploy"), ("native-feed.yml", "release")}: + continue + changed = copy.deepcopy(workflow) + changed["jobs"][job_id]["runs-on"] = route + with pytest.raises(AssertionError): + assert_runner_policy(changed, name) + + +@pytest.mark.parametrize( + "name,job_id,route", + [ + ("docs.yml", "deploy", POOL_LABELS), + ("docs.yml", "deploy", "ubuntu-24.04"), + ("native-feed.yml", "release", POOL_LABELS), + ("native-feed.yml", "release", "ubuntu-24.04"), + ], +) +def test_runner_exceptions_reject_other_routes(name: str, job_id: str, route: Any) -> None: + workflow = copy.deepcopy(load(name)) + assert_runner_policy(workflow, name) + workflow["jobs"][job_id]["runs-on"] = route + with pytest.raises(AssertionError): + assert_runner_policy(workflow, name) + + +def test_native_release_matrix_uses_only_the_two_linux_pools() -> None: + assert_native_release_matrix(load("native-feed.yml")["jobs"]["release"]["strategy"]["matrix"]) + + +@pytest.mark.parametrize( + "os_name,runner", + [ + ("ubuntu-24.04", GUARDED_ROUTE), + ("ubuntu-24.04", "ubuntu-24.04"), + ("ubuntu-24.04-arm", "ubuntu-24.04-arm"), + ("ubuntu-24.04-arm", MATRIX_RUNNERS["ubuntu-24.04"]), + ("macos-14", MATRIX_RUNNERS["ubuntu-24.04"]), + ], +) +def test_native_release_matrix_rejects_fallbacks_and_wrong_pools( + os_name: str, runner: str +) -> None: + matrix = copy.deepcopy(load("native-feed.yml")["jobs"]["release"]["strategy"]["matrix"]) + assert_native_release_matrix(matrix) + if os_name == "macos-14": + matrix["include"].append({"os": os_name, "runner": runner}) + else: + entry = next(entry for entry in matrix["include"] if entry["os"] == os_name) + entry["runner"] = runner + with pytest.raises(AssertionError): + assert_native_release_matrix(matrix) + + +@pytest.mark.parametrize( + "fault", + ["missing-os", "extra-os", "missing-linux-runner", "duplicate-include", "unknown-include"], +) +def test_native_release_matrix_rejects_missing_or_extra_legs(fault: str) -> None: + matrix = copy.deepcopy(load("native-feed.yml")["jobs"]["release"]["strategy"]["matrix"]) + assert_native_release_matrix(matrix) + if fault == "missing-os": + matrix["os"].remove("macos-14") + elif fault == "extra-os": + matrix["os"].append("windows-latest") + elif fault == "missing-linux-runner": + del matrix["include"][1]["runner"] + elif fault == "duplicate-include": + matrix["include"].append(dict(matrix["include"][0])) + else: + matrix["include"].append({"os": "windows-latest", "runner": "windows-latest"}) + with pytest.raises(AssertionError): + assert_native_release_matrix(matrix) @pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) @@ -122,31 +365,277 @@ def test_unconditional_cancellation_is_rejected() -> None: @pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) def test_pull_request_jobs_hold_no_secret_and_no_write_scope(name: str) -> None: - assert "secrets." not in (WORKFLOWS / name).read_text() workflow = load(name) - assert set(workflow["permissions"].values()) == {"read"} - for job_id, job in workflow["jobs"].items(): - if "permissions" in job: - # Only the Pages deployment writes, and it never runs for a pull request. - assert job_id == "deploy" - assert job["if"] == "github.event_name != 'pull_request'" + # old: assert "secrets." not in (WORKFLOWS / name).read_text() + # old: assert set(workflow["permissions"].values()) == {"read"} + # old: assert job_id == "deploy" + # old: assert job["if"] == "github.event_name != 'pull_request'" + assert_permission_policy(workflow, name) -def test_docs_only_prs_skip_only_the_runtime_jobs() -> None: - jobs = load("ci.yml")["jobs"] +@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) +@pytest.mark.parametrize( + "job_id,key,value", + [ + ("authz", "permissions", {"contents": "write"}), + ("authz", "permissions", {"contents": "read", "actions": "read"}), + ("authz", "env", {"TOKEN": "${{ secrets.OTHER_TOKEN }}"}), + ("authz", "env", {"TOKEN": "${{ secrets.GITHUB_TOKEN_EXTRA }}"}), + ("authz", "env", {"TOKEN": "${{ secrets['GITHUB_TOKEN'] }}"}), + ("extra", "env", {"TOKEN": "${{ secrets.GITHUB_TOKEN }}"}), + ("extra", "steps", [{"run": "echo '${{ secrets.OTHER_TOKEN }}'"}]), + ("extra", "permissions", {"contents": "write"}), + ("deploy", "permissions", {"contents": "write"}), + ], +) +def test_secret_and_permission_policy_rejects_unauthorized_scope( + name: str, job_id: str, key: str, value: Any +) -> None: + workflow = copy.deepcopy(load(name)) + assert_permission_policy(workflow, name) + workflow["jobs"].setdefault(job_id, {})[key] = value + with pytest.raises(AssertionError): + assert_permission_policy(workflow, name) + + +@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) +@pytest.mark.parametrize("key", ["env", "permissions"]) +def test_workflow_level_secrets_and_write_scope_are_rejected(name: str, key: str) -> None: + workflow = copy.deepcopy(load(name)) + assert_permission_policy(workflow, name) + workflow[key] = ( + {"TOKEN": "${{ secrets.GITHUB_TOKEN }}"} if key == "env" else {"contents": "write"} + ) + with pytest.raises(AssertionError): + assert_permission_policy(workflow, name) + + +@pytest.mark.parametrize( + "key,value", + [ + ("permissions", {"pages": "write"}), + ("permissions", {"pages": "write", "id-token": "write", "contents": "write"}), + ("if", "always()"), + ("environment", {"name": "other"}), + ], +) +def test_pages_write_scope_is_limited_to_non_pr_deployment(key: str, value: Any) -> None: + workflow = copy.deepcopy(load("docs.yml")) + assert_permission_policy(workflow, "docs.yml") + workflow["jobs"]["deploy"][key] = value + with pytest.raises(AssertionError): + assert_permission_policy(workflow, "docs.yml") + + +@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) +def test_every_job_waits_for_the_first_authz_job(name: str) -> None: + assert_authz_dependencies(load(name)) + + +@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) +def test_missing_authz_dependencies_and_authz_prerequisites_are_rejected(name: str) -> None: + workflow = load(name) + assert_authz_dependencies(workflow) + for job_id in workflow["jobs"]: + changed = copy.deepcopy(workflow) + if job_id == "authz": + changed["jobs"][job_id]["needs"] = "build" + else: + changed["jobs"][job_id]["needs"] = sorted(needs(changed["jobs"][job_id]) - {"authz"}) + with pytest.raises(AssertionError): + assert_authz_dependencies(changed) + changed = copy.deepcopy(workflow) + changed["jobs"]["authz"] = changed["jobs"].pop("authz") + with pytest.raises(AssertionError): + assert_authz_dependencies(changed) + + +@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) +def test_authz_checks_exact_bots_then_collaborator_permissions(name: str) -> None: + assert_authz_identity(load(name)["jobs"]["authz"]) + + +@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) +@pytest.mark.parametrize( + "login", + [ + "github-actions[bot]", + "pineforge-app", + "pineforge-app[bot]x", + "xpineforge-app[bot]", + "dependabot", + "dependabot[bot]x", + "xdependabot[bot]", + ], +) +def test_authz_rejects_additional_or_lookalike_bot_shortcuts(name: str, login: str) -> None: + job = copy.deepcopy(load(name)["jobs"]["authz"]) + assert_authz_identity(job) + script = job["steps"][0]["run"] + assert BOT_TEST in script + broader_test = BOT_TEST.removesuffix("; then") + f' || [ "$ACTOR" = \'{login}\' ]; then' + job["steps"][0]["run"] = script.replace(BOT_TEST, broader_test) + with pytest.raises(AssertionError): + assert_authz_identity(job) + + +@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) +@pytest.mark.parametrize( + "shortcut", + [ + 'if [[ "$ACTOR" == dependabot* || "$ACTOR" == pineforge-app* ]]; then\n' + 'echo "Authorized bot ${ACTOR}."\nexit 0\nfi', + 'if [[ "$ACTOR" =~ dependabot || "$ACTOR" =~ pineforge-app ]]; then\n' + 'echo "Authorized bot ${ACTOR}."\nexit 0\nfi', + 'case "$ACTOR" in\ndependabot*|pineforge-app*) exit 0 ;;\nesac', + ], + ids=["glob", "regex", "case-glob"], +) +def test_authz_rejects_pattern_bot_shortcuts(name: str, shortcut: str) -> None: + job = copy.deepcopy(load(name)["jobs"]["authz"]) + assert_authz_identity(job) + lines = [line.strip() for line in job["steps"][0]["run"].splitlines()] + start = lines.index(BOT_TEST) + end = lines.index("fi", start) + job["steps"][0]["run"] = "\n".join([*lines[:start], shortcut, *lines[end + 1 :]]) + with pytest.raises(AssertionError): + assert_authz_identity(job) + + +@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) +@pytest.mark.parametrize( + "old,new", + [ + ("set -euo pipefail", "set -eu"), + (PERMISSION_API, PERMISSION_API.replace("if ! ", "if ")), + ("collaborators/${ACTOR}/permission", "collaborators/permission"), + ( + 'exit 1\nfi\nif [ -z "$perm" ]; then', + 'exit 0\nfi\nif [ -z "$perm" ]; then', + ), + ('if [ -z "$perm" ]; then', 'if [ -n "$perm" ]; then'), + ('exit 1\nfi\ncase "$perm" in', 'exit 0\nfi\ncase "$perm" in'), + ("admin|maintain|write) exit 0 ;;", "admin|maintain|write|read) exit 0 ;;"), + ("; exit 1 ;;", "; exit 0 ;;"), + ("Authorized bot ${ACTOR}.", "Authorized bot ${{ github.actor }}."), + ], +) +def test_authz_rejects_identity_check_regressions(name: str, old: str, new: str) -> None: + job = copy.deepcopy(load(name)["jobs"]["authz"]) + assert_authz_identity(job) + script = "\n".join(line.strip() for line in job["steps"][0]["run"].splitlines()) + assert old in script + changed = script.replace(old, new) + assert changed != script + job["steps"][0]["run"] = changed + with pytest.raises(AssertionError): + assert_authz_identity(job) + + +@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) +@pytest.mark.parametrize( + "fault", ["missing-env", "wrong-env", "wrong-test", "exit-zero", "bot-before-fork"] +) +def test_authz_rejects_fork_bypasses(name: str, fault: str) -> None: + job = copy.deepcopy(load(name)["jobs"]["authz"]) + assert_authz_identity(job) + step = job["steps"][0] + lines = [line.strip() for line in step["run"].splitlines()] + fork_start = lines.index(FORK_TEST) + fork_end = lines.index("fi", fork_start) + bot_end = lines.index("fi", lines.index(BOT_TEST)) + if fault == "missing-env": + del step["env"]["FORK_PR"] + elif fault == "wrong-env": + step["env"]["FORK_PR"] = "${{ false }}" + elif fault == "wrong-test": + lines[fork_start] = 'if [ "$FORK_PR" = "false" ]; then' + elif fault == "exit-zero": + lines[fork_end - 1] = "exit 0" + else: + lines = [ + *lines[:fork_start], + *lines[fork_end + 1 : bot_end + 1], + *lines[fork_start : fork_end + 1], + *lines[bot_end + 1 :], + ] + step["run"] = "\n".join(lines) + with pytest.raises(AssertionError): + assert_authz_identity(job) + + +@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) +@pytest.mark.parametrize( + "target,key,value", + [ + ("job", "if", "false"), + ("job", "continue-on-error", "true"), + ("step", "shell", "sh"), + ("step", "if", "false"), + ("step", "continue-on-error", "true"), + ("env", "GH_TOKEN", "${{ secrets.OTHER_TOKEN }}"), + ("env", "ACTOR", "${{ github.actor }}"), + ("env", "REPO", "${{ github.event.pull_request.head.repo.full_name }}"), + ], +) +def test_authz_rejects_untrusted_env_and_masked_failures( + name: str, target: str, key: str, value: str +) -> None: + job = copy.deepcopy(load(name)["jobs"]["authz"]) + assert_authz_identity(job) + targets = {"job": job, "step": job["steps"][0], "env": job["steps"][0]["env"]} + targets[target][key] = value + with pytest.raises(AssertionError): + assert_authz_identity(job) + + +def assert_docs_only_policy(jobs: dict[str, Any]) -> None: assert jobs["changes"]["if"] == "github.event_name == 'pull_request'" + assert jobs["changes"]["needs"] == "authz" # The unit matrix always runs: tests/test_documentation.py checks README.md, CONTRIBUTING.md # and docs/ links, and its check names stay reported. - assert "needs" not in jobs["test"] + # old: assert "needs" not in jobs["test"] + assert jobs["test"]["needs"] == "authz" assert "if" not in jobs["test"] for name in RUNTIME_JOBS: - assert jobs[name]["needs"] == "changes" + # old: assert jobs[name]["needs"] == "changes" + assert needs(jobs[name]) == {"authz", "changes"} + # old: assert " ".join(jobs[name]["if"].split()) == RUNTIME_IF + # old: RUNTIME_IF had "!cancelled() &&" immediately followed by the event condition. assert " ".join(jobs[name]["if"].split()) == RUNTIME_IF - assert set(jobs["ready"]["needs"]) == {"changes", "test", *RUNTIME_JOBS} + # old: assert set(jobs["ready"]["needs"]) == {"changes", "test", *RUNTIME_JOBS} + assert set(jobs["ready"]["needs"]) == {"authz", "changes", "test", *RUNTIME_JOBS} assert jobs["ready"]["if"] == "always()" assert jobs["ready"]["name"] == "release/ready" +def test_docs_only_prs_skip_only_the_runtime_jobs() -> None: + assert_docs_only_policy(load("ci.yml")["jobs"]) + + +@pytest.mark.parametrize( + "job_id,key,value", + [ + ("changes", "needs", []), + ("test", "needs", []), + ("test", "if", "needs.changes.outputs.code == 'true'"), + ("ready", "needs", ["changes", "test", *RUNTIME_JOBS]), + ("ready", "needs", ["authz", "changes", "test", *RUNTIME_JOBS, "extra"]), + *[(job_id, "needs", ["changes"]) for job_id in RUNTIME_JOBS], + *[(job_id, "needs", ["authz", "changes", "extra"]) for job_id in RUNTIME_JOBS], + *[(job_id, "if", OLD_RUNTIME_IF) for job_id in RUNTIME_JOBS], + ], +) +def test_docs_only_policy_rejects_missing_authz_and_extra_dependencies( + job_id: str, key: str, value: Any +) -> None: + jobs = copy.deepcopy(load("ci.yml")["jobs"]) + assert_docs_only_policy(jobs) + jobs[job_id][key] = value + with pytest.raises(AssertionError): + assert_docs_only_policy(jobs) + + def test_evidence_inventory_stays_the_five_gated_jobs() -> None: steps = load("ci.yml")["jobs"]["ready"]["steps"] aggregate = next( @@ -264,6 +753,8 @@ def test_every_other_path_is_classified_as_code( def gate_inputs(event: str, changes: str, code: str, unit: str, runtime: str) -> dict[str, str]: return { + # The previously accepted outcomes had no AUTHZ_RESULT input. + "AUTHZ_RESULT": "success", "EVENT_NAME": event, "CHANGES_RESULT": changes, "CODE_CHANGED": code, @@ -272,16 +763,17 @@ def gate_inputs(event: str, changes: str, code: str, unit: str, runtime: str) -> } -def run_gate(tmp_path: Path, inputs: dict[str, str]) -> tuple[int, str]: +def run_gate(tmp_path: Path, inputs: dict[str, str]) -> tuple[int, str, str]: output = tmp_path / "github-output" output.write_text("") completed = subprocess.run( ["bash", "-e", "-c", step_script("ci.yml", "ready", "gate")], - env={**os.environ, **inputs, "GITHUB_OUTPUT": str(output)}, + # old: env={**os.environ, **inputs, "GITHUB_OUTPUT": str(output)} + env={**inputs, "GITHUB_OUTPUT": str(output)}, capture_output=True, text=True, ) - return completed.returncode, output.read_text().strip() + return completed.returncode, output.read_text().strip(), completed.stdout ALL_SUCCESS = "success success success success" @@ -297,7 +789,13 @@ def run_gate(tmp_path: Path, inputs: dict[str, str]) -> tuple[int, str]: "evidence=false", ), } +AUTHZ_REJECTED = { + f"{case}: authz {result or 'empty'}": {**inputs, "AUTHZ_RESULT": result} + for case, (inputs, _expected) in ACCEPTED.items() + for result in ("failure", "cancelled", "skipped", "") +} REJECTED = { + **AUTHZ_REJECTED, "push skips a runtime job": gate_inputs( "push", "skipped", "", "success", "success skipped success success" ), @@ -344,11 +842,50 @@ def run_gate(tmp_path: Path, inputs: dict[str, str]) -> tuple[int, str]: @pytest.mark.parametrize("case", list(ACCEPTED)) def test_release_ready_accepts_only_the_complete_outcomes(tmp_path: Path, case: str) -> None: inputs, expected = ACCEPTED[case] - assert run_gate(tmp_path, inputs) == (0, expected) + assert inputs["AUTHZ_RESULT"] == "success" + returncode, output, _message = run_gate(tmp_path, inputs) + # old: assert run_gate(tmp_path, inputs) == (0, expected) + assert (returncode, output) == (0, expected) @pytest.mark.parametrize("case", list(REJECTED)) def test_release_ready_rejects_every_unexpected_outcome(tmp_path: Path, case: str) -> None: - returncode, output = run_gate(tmp_path, REJECTED[case]) + # old: returncode, output = run_gate(tmp_path, REJECTED[case]) + returncode, output, _message = run_gate(tmp_path, REJECTED[case]) + assert returncode != 0 + assert output == "" + + +def test_release_ready_checks_authz_first() -> None: + assert_ready_authz(load("ci.yml")["jobs"]["ready"]) + + +@pytest.mark.parametrize("fault", ["missing-result", "wrong-result", "late-check"]) +def test_release_ready_rejects_missing_or_late_authz_checks(fault: str) -> None: + job = copy.deepcopy(load("ci.yml")["jobs"]["ready"]) + assert_ready_authz(job) + gate = next(step for step in job["steps"] if step.get("id") == "gate") + if fault == "missing-result": + del gate["env"]["AUTHZ_RESULT"] + elif fault == "wrong-result": + gate["env"]["AUTHZ_RESULT"] = "${{ needs.test.result }}" + else: + script = gate["run"] + assert script.startswith(READY_AUTHZ_CHECK + "\n") + gate["run"] = script.removeprefix(READY_AUTHZ_CHECK + "\n") + READY_AUTHZ_CHECK + "\n" + with pytest.raises(AssertionError): + assert_ready_authz(job) + + +@pytest.mark.parametrize("case", list(AUTHZ_REJECTED)) +@pytest.mark.parametrize("other_failure", [False, True]) +def test_release_ready_rejects_failed_authz_before_other_outcomes( + tmp_path: Path, case: str, other_failure: bool +) -> None: + inputs = dict(AUTHZ_REJECTED[case]) + if other_failure: + inputs.update(CHANGES_RESULT="failure", UNIT_RESULT="failure") + returncode, output, message = run_gate(tmp_path, inputs) assert returncode != 0 assert output == "" + assert message == f"authz: {inputs['AUTHZ_RESULT']}\n" diff --git a/tests/test_release_adapter.py b/tests/test_release_adapter.py index a39e283..ce48c29 100644 --- a/tests/test_release_adapter.py +++ b/tests/test_release_adapter.py @@ -959,11 +959,17 @@ def test_completed_receipt_replay_does_not_dispatch_or_publish( assert report["deployed"] == saved["deployed"] +def assert_ready_dependencies(job: dict[str, Any]) -> None: + # old: assert set(ci["jobs"]["ready"]["needs"]) == evidence.NEEDS | {"changes"} + assert set(job["needs"]) == evidence.NEEDS | {"changes", "authz"} + + def test_workflows_keep_every_gate_ahead_of_oidc_publication() -> None: workflows = ROOT / ".github/workflows" ci = yaml.load((workflows / "ci.yml").read_text(), Loader=yaml.BaseLoader) assert ci["jobs"]["test"]["strategy"]["matrix"]["python-version"] == ["3.11", "3.12", "3.13"] - assert set(ci["jobs"]["ready"]["needs"]) == evidence.NEEDS | {"changes"} + # old: assert set(ci["jobs"]["ready"]["needs"]) == evidence.NEEDS | {"changes"} + assert_ready_dependencies(ci["jobs"]["ready"]) assert ci["jobs"]["ready"]["if"] == "always()" assert ci["jobs"]["ready"]["name"] == "release/ready" for job, suite, enabled in ( @@ -994,6 +1000,19 @@ def test_workflows_keep_every_gate_ahead_of_oidc_publication() -> None: assert receiver["on"]["repository_dispatch"]["types"] == ["pineforge-release"] +@pytest.mark.parametrize("dependency", ["authz", "changes", *sorted(evidence.NEEDS), "extra"]) +def test_ready_rejects_missing_or_extra_publication_dependencies(dependency: str) -> None: + ci = yaml.load((ROOT / ".github/workflows/ci.yml").read_text(), Loader=yaml.BaseLoader) + job = copy.deepcopy(ci["jobs"]["ready"]) + assert_ready_dependencies(job) + if dependency == "extra": + job["needs"].append(dependency) + else: + job["needs"].remove(dependency) + with pytest.raises(AssertionError): + assert_ready_dependencies(job) + + def test_rollback_is_a_new_patch_and_rejected_events_cannot_return(ready, baseline) -> None: good, good_raw = ready apply(baseline, contract.update_files(baseline, good, good_raw)) From a3bc6f94fb198884e4ba689c2d2bb8c7630254d9 Mon Sep 17 00:00:00 2001 From: luisleo526 Date: Tue, 20 Oct 2026 13:29:55 +0800 Subject: [PATCH 04/13] ci: authz is the same-repository check from the org CI-label canon Replace the API based authz of the three workflows with the canonical job (ci-labels.md, #808): on the pf-ci-x64 pool, pull_request_target fails red, a pull request whose head repository is not this repository fails red, every other event passes. Actors (members, collaborators, dependabot[bot], our release App) are admitted or refused by the dispatcher, not in YAML, and the only secret-free job text is the canonical one. Every other job still needs authz; ci.yml has no paths filter on pull_request and holds the required check, docs.yml and native-feed.yml keep their own authz as a guard. Co-Authored-By: Claude Sonnet 5.5 --- .github/workflows/ci.yml | 39 +++++++------------------------ .github/workflows/docs.yml | 39 +++++++------------------------ .github/workflows/native-feed.yml | 39 +++++++------------------------ 3 files changed, 24 insertions(+), 93 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c114589..4a6dde6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,42 +23,19 @@ concurrency: jobs: authz: - name: authz - runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] - timeout-minutes: 2 - permissions: - contents: read + runs-on: [self-hosted, pf-ci, pf-ci-x64] steps: - - shell: bash + - name: Same-repository pull requests only env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - ACTOR: ${{ github.triggering_actor }} + EVENT: ${{ github.event_name }} + HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} REPO: ${{ github.repository }} - FORK_PR: >- - ${{ (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && - (github.event.pull_request.head.repo.full_name != github.repository || github.event.pull_request.head.repo.fork != false) }} run: | - set -euo pipefail - if [ "$FORK_PR" = "true" ]; then - echo "::error::Actor '$ACTOR' permission '': fork pull requests are not authorized." - exit 1 - fi - if [ "$ACTOR" = 'dependabot[bot]' ]; then - echo "Authorized dependabot[bot]." - exit 0 - fi - if ! perm=$(gh api "repos/${REPO}/collaborators/${ACTOR}/permission" --jq '.permission'); then - echo "::error::Actor '$ACTOR' permission '': collaborator permission API request failed." - exit 1 - fi - if [ -z "$perm" ]; then - echo "::error::Actor '$ACTOR' permission '': collaborator permission API returned no permission." - exit 1 - fi - case "$perm" in - admin|maintain|write) exit 0 ;; - *) echo "::error::Actor '$ACTOR' permission '$perm': write access is required."; exit 1 ;; + case "$EVENT" in + pull_request_target) echo "::error::pull_request_target is not allowed for CI"; exit 1 ;; + pull_request) [ "$HEAD_REPO" = "$REPO" ] || { echo "::error::fork pull requests do not run CI"; exit 1; } ;; esac + echo "authz ok: $EVENT" changes: needs: authz diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 57967dd..64158ba 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -17,42 +17,19 @@ concurrency: jobs: authz: - name: authz - runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] - timeout-minutes: 2 - permissions: - contents: read + runs-on: [self-hosted, pf-ci, pf-ci-x64] steps: - - shell: bash + - name: Same-repository pull requests only env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - ACTOR: ${{ github.triggering_actor }} + EVENT: ${{ github.event_name }} + HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} REPO: ${{ github.repository }} - FORK_PR: >- - ${{ (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && - (github.event.pull_request.head.repo.full_name != github.repository || github.event.pull_request.head.repo.fork != false) }} run: | - set -euo pipefail - if [ "$FORK_PR" = "true" ]; then - echo "::error::Actor '$ACTOR' permission '': fork pull requests are not authorized." - exit 1 - fi - if [ "$ACTOR" = 'dependabot[bot]' ]; then - echo "Authorized dependabot[bot]." - exit 0 - fi - if ! perm=$(gh api "repos/${REPO}/collaborators/${ACTOR}/permission" --jq '.permission'); then - echo "::error::Actor '$ACTOR' permission '': collaborator permission API request failed." - exit 1 - fi - if [ -z "$perm" ]; then - echo "::error::Actor '$ACTOR' permission '': collaborator permission API returned no permission." - exit 1 - fi - case "$perm" in - admin|maintain|write) exit 0 ;; - *) echo "::error::Actor '$ACTOR' permission '$perm': write access is required."; exit 1 ;; + case "$EVENT" in + pull_request_target) echo "::error::pull_request_target is not allowed for CI"; exit 1 ;; + pull_request) [ "$HEAD_REPO" = "$REPO" ] || { echo "::error::fork pull requests do not run CI"; exit 1; } ;; esac + echo "authz ok: $EVENT" build: needs: authz diff --git a/.github/workflows/native-feed.yml b/.github/workflows/native-feed.yml index 72151ee..ddfc7c3 100644 --- a/.github/workflows/native-feed.yml +++ b/.github/workflows/native-feed.yml @@ -20,42 +20,19 @@ concurrency: jobs: authz: - name: authz - runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] - timeout-minutes: 2 - permissions: - contents: read + runs-on: [self-hosted, pf-ci, pf-ci-x64] steps: - - shell: bash + - name: Same-repository pull requests only env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - ACTOR: ${{ github.triggering_actor }} + EVENT: ${{ github.event_name }} + HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} REPO: ${{ github.repository }} - FORK_PR: >- - ${{ (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && - (github.event.pull_request.head.repo.full_name != github.repository || github.event.pull_request.head.repo.fork != false) }} run: | - set -euo pipefail - if [ "$FORK_PR" = "true" ]; then - echo "::error::Actor '$ACTOR' permission '': fork pull requests are not authorized." - exit 1 - fi - if [ "$ACTOR" = 'dependabot[bot]' ]; then - echo "Authorized dependabot[bot]." - exit 0 - fi - if ! perm=$(gh api "repos/${REPO}/collaborators/${ACTOR}/permission" --jq '.permission'); then - echo "::error::Actor '$ACTOR' permission '': collaborator permission API request failed." - exit 1 - fi - if [ -z "$perm" ]; then - echo "::error::Actor '$ACTOR' permission '': collaborator permission API returned no permission." - exit 1 - fi - case "$perm" in - admin|maintain|write) exit 0 ;; - *) echo "::error::Actor '$ACTOR' permission '$perm': write access is required."; exit 1 ;; + case "$EVENT" in + pull_request_target) echo "::error::pull_request_target is not allowed for CI"; exit 1 ;; + pull_request) [ "$HEAD_REPO" = "$REPO" ] || { echo "::error::fork pull requests do not run CI"; exit 1; } ;; esac + echo "authz ok: $EVENT" release: name: Release (${{ matrix.os }}) From 0a08cbda678ccf7b5ced840477eb1bbb38a4a1f1 Mon Sep 17 00:00:00 2001 From: luisleo526 Date: Tue, 20 Oct 2026 13:43:50 +0800 Subject: [PATCH 05/13] test: pin the canonical authz job; release/ready refuses first on a failed authz The workflow tests pinned the earlier API based authz. They now pin the canonical same-repository job from ci-labels.md: exact keys, pool labels, single named step, exact env and script, no secrets or permissions anywhere except the Pages deployment, and ci.yml/docs.yml without a pull_request filter (the required authz must run on every PR; native-feed keeps its paths as a guard). Each rewritten pin quotes the removed one and gets new rejected cases. release/ready is the only always() job: following ci-labels.md rule 3a its first step now refuses (exit 1) unless authz succeeded, before any checkout, and its checkout and repo-code steps also require a successful authz. The four runtime jobs carry needs.authz.result == 'success' in their own if. Co-Authored-By: Claude Sonnet 5.5 --- .github/workflows/ci.yml | 9 +- tests/test_pr_tier_workflows.py | 348 +++++++++++++++++--------------- tests/test_release_adapter.py | 73 +++++++ 3 files changed, 267 insertions(+), 163 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4a6dde6..db23b18 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -273,6 +273,9 @@ jobs: env: PF_CI_LABELS: '["self-hosted","pf-ci","pf-ci-x64"]' steps: + - name: authz must have passed + if: needs.authz.result != 'success' + run: exit 1 - name: Require the path decision and every expected job result id: gate env: @@ -322,11 +325,11 @@ jobs: fi echo "evidence=$evidence" >> "$GITHUB_OUTPUT" - uses: actions/checkout@v7 - if: steps.gate.outputs.evidence == 'true' + if: needs.authz.result == 'success' && steps.gate.outputs.evidence == 'true' with: persist-credentials: false - uses: actions/setup-python@v6 - if: steps.gate.outputs.evidence == 'true' + if: needs.authz.result == 'success' && steps.gate.outputs.evidence == 'true' with: python-version: '3.13' - uses: actions/download-artifact@v6 @@ -335,7 +338,7 @@ jobs: name: python-package-distributions-${{ github.run_attempt }} path: dist/ - name: Require the complete job inventory and bind evidence to this head/base/tree - if: steps.gate.outputs.evidence == 'true' + if: needs.authz.result == 'success' && steps.gate.outputs.evidence == 'true' env: PYTHONPATH: scripts NEEDS_JSON: >- diff --git a/tests/test_pr_tier_workflows.py b/tests/test_pr_tier_workflows.py index d80fe40..12ed9ac 100644 --- a/tests/test_pr_tier_workflows.py +++ b/tests/test_pr_tier_workflows.py @@ -59,18 +59,14 @@ "(github.event_name != 'pull_request' || " "(needs.changes.result == 'success' && needs.changes.outputs.code == 'true'))" ) -FORK_TEST = 'if [ "$FORK_PR" = "true" ]; then' -BOT_TEST = ( - 'if [ "$ACTOR" = \'dependabot[bot]\' ] || [ "$ACTOR" = \'pineforge-app[bot]\' ]; then' -) -PERMISSION_API = ( - 'if ! perm=$(gh api "repos/${REPO}/collaborators/${ACTOR}/permission" ' - "--jq '.permission'); then" -) -FORK_PR_ENV = ( - "${{ (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && " - "(github.event.pull_request.head.repo.full_name != github.repository || " - "github.event.pull_request.head.repo.fork != false) }}" +CANONICAL_AUTHZ_SCRIPT = ( + 'case "$EVENT" in\n' + ' pull_request_target) echo "::error::pull_request_target is not allowed for CI"; ' + "exit 1 ;;\n" + ' pull_request) [ "$HEAD_REPO" = "$REPO" ] || { ' + 'echo "::error::fork pull requests do not run CI"; exit 1; } ;;\n' + "esac\n" + 'echo "authz ok: $EVENT"\n' ) READY_AUTHZ_CHECK = ( 'if [ "$AUTHZ_RESULT" != "success" ]; then echo "authz: $AUTHZ_RESULT"; exit 1; fi' @@ -132,22 +128,15 @@ def assert_secret_references(value: Any, allowed: set[str]) -> None: def assert_permission_policy(workflow: dict[str, Any], name: str) -> None: assert set(workflow["permissions"].values()) == {"read"} - # old: assert "secrets." not in (WORKFLOWS / name).read_text() - for key, value in workflow.items(): - if key != "jobs": - assert_secret_references(value, set()) + # old: allowed = {"secrets.GITHUB_TOKEN"} if job_id == "authz" else set() + assert_secret_references(workflow, set()) for job_id, job in workflow["jobs"].items(): - allowed = {"secrets.GITHUB_TOKEN"} if job_id == "authz" else set() - assert_secret_references(job, allowed) - # old: assert job_id == "deploy" - # The old restriction applied whenever a job had its own permissions. - if job_id == "authz": - assert job.get("permissions") == {"contents": "read"} - elif name == "docs.yml" and job_id == "deploy": + if name == "docs.yml" and job_id == "deploy": assert job["permissions"] == {"pages": "write", "id-token": "write"} assert job["if"] == "github.event_name != 'pull_request'" assert job["environment"]["name"] == "github-pages" else: + # old (for authz): assert job.get("permissions") == {"contents": "read"} assert "permissions" not in job @@ -166,55 +155,24 @@ def assert_authz_dependencies(workflow: dict[str, Any]) -> None: assert "authz" in needs(job) -def assert_authz_branch(lines: list[str], condition: str, result: str) -> tuple[int, int]: - assert condition in lines - start = lines.index(condition) - assert "fi" in lines[start + 1 :] - end = lines.index("fi", start + 1) - body = lines[start + 1 : end] - assert len(body) == 2, "each identity branch reports its decision, then exits" - assert re.fullmatch(r'echo "[^"]*"', body[0]) is not None - assert body[1] == f"exit {result}" - return start, end - - -def assert_authz_identity(job: dict[str, Any]) -> None: - assert "if" not in job - assert "continue-on-error" not in job +def assert_canonical_authz(job: dict[str, Any]) -> None: + assert set(job) == {"runs-on", "steps"} + assert job["runs-on"] == ["self-hosted", "pf-ci", "pf-ci-x64"] + assert_secret_references(job, set()) steps = job["steps"] - for step in steps: - assert "${{" not in step.get("run", "") - assert len(steps) == 1, "authz is one bash identity check" + assert isinstance(steps, list) + assert len(steps) == 1, "authz is exactly the canonical same-repository check" step = steps[0] - assert step["shell"] == "bash" - assert "if" not in step - assert "continue-on-error" not in step - env = step["env"] - assert env["GH_TOKEN"] == "${{ secrets.GITHUB_TOKEN }}" - assert env["ACTOR"] == "${{ github.triggering_actor }}" - assert env["REPO"] == "${{ github.repository }}" - assert "FORK_PR" in env - assert " ".join(env["FORK_PR"].split()) == FORK_PR_ENV + assert set(step) == {"name", "env", "run"} + assert step["name"] == "Same-repository pull requests only" + assert step["env"] == { + "EVENT": "${{ github.event_name }}", + "HEAD_REPO": "${{ github.event.pull_request.head.repo.full_name }}", + "REPO": "${{ github.repository }}", + } script = step["run"] - assert "=~" not in script - lines = [line.strip() for line in script.splitlines() if line.strip()] - assert lines[0] == "set -euo pipefail" - fork_start, fork_end = assert_authz_branch(lines, FORK_TEST, "1") - bot_start, bot_end = assert_authz_branch(lines, BOT_TEST, "0") - assert "*" not in lines[bot_start] - assert "case" not in lines[bot_start] - api_start, api_end = assert_authz_branch(lines, PERMISSION_API, "1") - empty_start, empty_end = assert_authz_branch(lines, 'if [ -z "$perm" ]; then', "1") - assert fork_start == 1, "fork rejection precedes every identity shortcut" - assert bot_start == fork_end + 1 - assert api_start == bot_end + 1 - assert empty_start == api_end + 1 - permission_case = lines[empty_end + 1 :] - assert len(permission_case) == 4 - assert permission_case[0] == 'case "$perm" in' - assert permission_case[1] == "admin|maintain|write) exit 0 ;;" - assert re.fullmatch(r'\*\) echo "[^"]*"; exit 1 ;;', permission_case[2]) is not None - assert permission_case[3] == "esac" + assert isinstance(script, str) + assert " ".join(script.split()) == " ".join(CANONICAL_AUTHZ_SCRIPT.split()) def assert_ready_authz(job: dict[str, Any]) -> None: @@ -367,6 +325,7 @@ def test_unconditional_cancellation_is_rejected() -> None: def test_pull_request_jobs_hold_no_secret_and_no_write_scope(name: str) -> None: workflow = load(name) # old: assert "secrets." not in (WORKFLOWS / name).read_text() + assert "secrets." not in (WORKFLOWS / name).read_text() # old: assert set(workflow["permissions"].values()) == {"read"} # old: assert job_id == "deploy" # old: assert job["if"] == "github.event_name != 'pull_request'" @@ -378,7 +337,9 @@ def test_pull_request_jobs_hold_no_secret_and_no_write_scope(name: str) -> None: "job_id,key,value", [ ("authz", "permissions", {"contents": "write"}), + ("authz", "permissions", {"contents": "read"}), ("authz", "permissions", {"contents": "read", "actions": "read"}), + ("authz", "env", {"TOKEN": "${{ secrets.GITHUB_TOKEN }}"}), ("authz", "env", {"TOKEN": "${{ secrets.OTHER_TOKEN }}"}), ("authz", "env", {"TOKEN": "${{ secrets.GITHUB_TOKEN_EXTRA }}"}), ("authz", "env", {"TOKEN": "${{ secrets['GITHUB_TOKEN'] }}"}), @@ -391,9 +352,11 @@ def test_pull_request_jobs_hold_no_secret_and_no_write_scope(name: str) -> None: def test_secret_and_permission_policy_rejects_unauthorized_scope( name: str, job_id: str, key: str, value: Any ) -> None: - workflow = copy.deepcopy(load(name)) - assert_permission_policy(workflow, name) + original = load(name) + assert_permission_policy(original, name) + workflow = copy.deepcopy(original) workflow["jobs"].setdefault(job_id, {})[key] = value + assert workflow != original with pytest.raises(AssertionError): assert_permission_policy(workflow, name) @@ -451,142 +414,156 @@ def test_missing_authz_dependencies_and_authz_prerequisites_are_rejected(name: s @pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) -def test_authz_checks_exact_bots_then_collaborator_permissions(name: str) -> None: - assert_authz_identity(load(name)["jobs"]["authz"]) +def test_authz_matches_the_canonical_same_repository_job(name: str) -> None: + assert_canonical_authz(load(name)["jobs"]["authz"]) @pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) @pytest.mark.parametrize( - "login", + "comparison", [ - "github-actions[bot]", - "pineforge-app", - "pineforge-app[bot]x", - "xpineforge-app[bot]", - "dependabot", - "dependabot[bot]x", - "xdependabot[bot]", + '[ "$HEAD_REPO" != "$REPO" ]', + '[[ "$HEAD_REPO" =~ "$REPO" ]]', + '[[ "$HEAD_REPO" == "$REPO"* ]]', + '[[ "$HEAD_REPO" == * ]]', ], + ids=["not-equal", "regex", "prefix", "glob"], ) -def test_authz_rejects_additional_or_lookalike_bot_shortcuts(name: str, login: str) -> None: - job = copy.deepcopy(load(name)["jobs"]["authz"]) - assert_authz_identity(job) +def test_authz_rejects_weakened_repository_comparisons(name: str, comparison: str) -> None: + original = load(name)["jobs"]["authz"] + assert_canonical_authz(original) + job = copy.deepcopy(original) script = job["steps"][0]["run"] - assert BOT_TEST in script - broader_test = BOT_TEST.removesuffix("; then") + f' || [ "$ACTOR" = \'{login}\' ]; then' - job["steps"][0]["run"] = script.replace(BOT_TEST, broader_test) + exact_comparison = '[ "$HEAD_REPO" = "$REPO" ]' + assert exact_comparison in script + job["steps"][0]["run"] = script.replace(exact_comparison, comparison) + assert job != original with pytest.raises(AssertionError): - assert_authz_identity(job) - - -@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) -@pytest.mark.parametrize( - "shortcut", - [ - 'if [[ "$ACTOR" == dependabot* || "$ACTOR" == pineforge-app* ]]; then\n' - 'echo "Authorized bot ${ACTOR}."\nexit 0\nfi', - 'if [[ "$ACTOR" =~ dependabot || "$ACTOR" =~ pineforge-app ]]; then\n' - 'echo "Authorized bot ${ACTOR}."\nexit 0\nfi', - 'case "$ACTOR" in\ndependabot*|pineforge-app*) exit 0 ;;\nesac', - ], - ids=["glob", "regex", "case-glob"], -) -def test_authz_rejects_pattern_bot_shortcuts(name: str, shortcut: str) -> None: - job = copy.deepcopy(load(name)["jobs"]["authz"]) - assert_authz_identity(job) - lines = [line.strip() for line in job["steps"][0]["run"].splitlines()] - start = lines.index(BOT_TEST) - end = lines.index("fi", start) - job["steps"][0]["run"] = "\n".join([*lines[:start], shortcut, *lines[end + 1 :]]) - with pytest.raises(AssertionError): - assert_authz_identity(job) + assert_canonical_authz(job) @pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) @pytest.mark.parametrize( "old,new", [ - ("set -euo pipefail", "set -eu"), - (PERMISSION_API, PERMISSION_API.replace("if ! ", "if ")), - ("collaborators/${ACTOR}/permission", "collaborators/permission"), ( - 'exit 1\nfi\nif [ -z "$perm" ]; then', - 'exit 0\nfi\nif [ -z "$perm" ]; then', + 'pull_request_target) echo "::error::pull_request_target is not allowed for CI"; ' + "exit 1 ;;", + "", + ), + ( + 'pull_request_target) echo "::error::pull_request_target is not allowed for CI"; ' + "exit 1 ;;", + "pull_request_target) exit 0 ;;", ), - ('if [ -z "$perm" ]; then', 'if [ -n "$perm" ]; then'), - ('exit 1\nfi\ncase "$perm" in', 'exit 0\nfi\ncase "$perm" in'), - ("admin|maintain|write) exit 0 ;;", "admin|maintain|write|read) exit 0 ;;"), - ("; exit 1 ;;", "; exit 0 ;;"), - ("Authorized bot ${ACTOR}.", "Authorized bot ${{ github.actor }}."), + ("; exit 1 ;;", " ;;"), + ("] || {", "] && {"), + ("exit 1; } ;;", "} ;;"), + ("exit 1; } ;;", "exit 0; } ;;"), + ("] || {", "] || true || {"), + ], + ids=[ + "missing-target-rejection", + "target-success", + "target-exit-dropped", + "and-instead-of-or", + "fork-exit-dropped", + "fork-exit-zero", + "or-true", ], ) -def test_authz_rejects_identity_check_regressions(name: str, old: str, new: str) -> None: - job = copy.deepcopy(load(name)["jobs"]["authz"]) - assert_authz_identity(job) +def test_authz_rejects_canonical_script_regressions(name: str, old: str, new: str) -> None: + original = load(name)["jobs"]["authz"] + assert_canonical_authz(original) + job = copy.deepcopy(original) script = "\n".join(line.strip() for line in job["steps"][0]["run"].splitlines()) assert old in script changed = script.replace(old, new) assert changed != script job["steps"][0]["run"] = changed + assert job != original with pytest.raises(AssertionError): - assert_authz_identity(job) + assert_canonical_authz(job) @pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) -@pytest.mark.parametrize( - "fault", ["missing-env", "wrong-env", "wrong-test", "exit-zero", "bot-before-fork"] -) -def test_authz_rejects_fork_bypasses(name: str, fault: str) -> None: - job = copy.deepcopy(load(name)["jobs"]["authz"]) - assert_authz_identity(job) - step = job["steps"][0] - lines = [line.strip() for line in step["run"].splitlines()] - fork_start = lines.index(FORK_TEST) - fork_end = lines.index("fi", fork_start) - bot_end = lines.index("fi", lines.index(BOT_TEST)) - if fault == "missing-env": - del step["env"]["FORK_PR"] - elif fault == "wrong-env": - step["env"]["FORK_PR"] = "${{ false }}" - elif fault == "wrong-test": - lines[fork_start] = 'if [ "$FORK_PR" = "false" ]; then' - elif fault == "exit-zero": - lines[fork_end - 1] = "exit 0" - else: - lines = [ - *lines[:fork_start], - *lines[fork_end + 1 : bot_end + 1], - *lines[fork_start : fork_end + 1], - *lines[bot_end + 1 :], - ] - step["run"] = "\n".join(lines) +@pytest.mark.parametrize("key", ["EVENT", "HEAD_REPO", "REPO"]) +def test_authz_rejects_missing_canonical_env(name: str, key: str) -> None: + original = load(name)["jobs"]["authz"] + assert_canonical_authz(original) + job = copy.deepcopy(original) + del job["steps"][0]["env"][key] + assert job != original with pytest.raises(AssertionError): - assert_authz_identity(job) + assert_canonical_authz(job) @pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) @pytest.mark.parametrize( "target,key,value", [ + ("job", "name", "Authorize CI"), ("job", "if", "false"), + ("job", "needs", "build"), + ("job", "permissions", {"contents": "read"}), + ("job", "permissions", {"contents": "write"}), + ("job", "timeout-minutes", "5"), ("job", "continue-on-error", "true"), + ("step", "name", "Authorize CI"), + ("step", "shell", "bash"), ("step", "shell", "sh"), ("step", "if", "false"), ("step", "continue-on-error", "true"), - ("env", "GH_TOKEN", "${{ secrets.OTHER_TOKEN }}"), - ("env", "ACTOR", "${{ github.actor }}"), + ("env", "EVENT", "${{ github.event.action }}"), + ("env", "HEAD_REPO", "${{ github.head_ref }}"), + ("env", "HEAD_REPO", "${{ github.event.pull_request.head.repo.fork }}"), + ("env", "HEAD_REPO", "${{ github.repository }}"), ("env", "REPO", "${{ github.event.pull_request.head.repo.full_name }}"), + ("env", "GH_TOKEN", "${{ secrets.GITHUB_TOKEN }}"), + ("env", "GH_TOKEN", "unused"), + ("env", "EXTRA", "unused"), ], ) def test_authz_rejects_untrusted_env_and_masked_failures( - name: str, target: str, key: str, value: str + name: str, target: str, key: str, value: Any ) -> None: - job = copy.deepcopy(load(name)["jobs"]["authz"]) - assert_authz_identity(job) + original = load(name)["jobs"]["authz"] + assert_canonical_authz(original) + job = copy.deepcopy(original) targets = {"job": job, "step": job["steps"][0], "env": job["steps"][0]["env"]} targets[target][key] = value + assert job != original + with pytest.raises(AssertionError): + assert_canonical_authz(job) + + +@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) +@pytest.mark.parametrize("fault", ["missing-step", "second-step"]) +def test_authz_rejects_noncanonical_step_counts(name: str, fault: str) -> None: + original = load(name)["jobs"]["authz"] + assert_canonical_authz(original) + job = copy.deepcopy(original) + if fault == "missing-step": + job["steps"].clear() + else: + job["steps"].append({"name": "Extra step", "run": "true"}) + assert job != original with pytest.raises(AssertionError): - assert_authz_identity(job) + assert_canonical_authz(job) + + +@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) +@pytest.mark.parametrize( + "route", ["ubuntu-24.04", ["ubuntu-24.04"], MATRIX_RUNNERS["ubuntu-24.04"]] +) +def test_authz_rejects_hosted_or_expression_runners(name: str, route: Any) -> None: + original = load(name)["jobs"]["authz"] + assert_canonical_authz(original) + job = copy.deepcopy(original) + job["runs-on"] = route + assert job != original + with pytest.raises(AssertionError): + assert_canonical_authz(job) def assert_docs_only_policy(jobs: dict[str, Any]) -> None: @@ -650,7 +627,58 @@ def test_evidence_inventory_stays_the_five_gated_jobs() -> None: assert set(needs) == evidence.NEEDS # create_evidence rejects anything else, so the path decision job must not leak in. assert "changes" not in needs - assert aggregate["if"] == "steps.gate.outputs.evidence == 'true'" + # old: assert aggregate["if"] == "steps.gate.outputs.evidence == 'true'" + # ci-labels.md rule 3a: a step that runs repo code also requires a successful authz. + assert aggregate["if"] == ( + "needs.authz.result == 'success' && steps.gate.outputs.evidence == 'true'" + ) + + +REFUSE_STEP = { + "name": "authz must have passed", + "if": "needs.authz.result != 'success'", + "run": "exit 1", +} + + +def assert_always_job_refuses_first(job: dict[str, Any]) -> None: + """ci-labels.md rule 3a for a job with `if: always()` that needs authz.""" + assert job["if"] == "always()" + assert "authz" in needs(job) + steps = job["steps"] + assert steps[0] == REFUSE_STEP, "the refusal is the FIRST step, before any checkout" + for step in steps[1:]: + uses_repo = "actions/checkout" in step.get("uses", "") or "run" in step + if uses_repo and step.get("id") != "gate": + assert "needs.authz.result == 'success'" in step.get("if", ""), step + + +def test_always_jobs_refuse_a_failed_authz_before_any_repo_code() -> None: + ci = load("ci.yml")["jobs"] + assert_always_job_refuses_first(ci["ready"]) + # The four runtime jobs use !cancelled(); their own if requires a successful authz. + for name in RUNTIME_JOBS: + assert "needs.authz.result == 'success'" in ci[name]["if"] + for workflow in ("ci.yml", "docs.yml", "native-feed.yml"): + for job_id, job in load(workflow)["jobs"].items(): + condition = job.get("if", "") + if job_id != "ready" and ("always()" in condition or "!cancelled()" in condition): + assert "needs.authz.result == 'success'" in condition, (workflow, job_id) + + +@pytest.mark.parametrize("fault", ["no-refusal", "refusal-not-first", "unguarded-checkout"]) +def test_always_job_rejects_missing_first_refusal_or_unguarded_repo_code(fault: str) -> None: + job = copy.deepcopy(load("ci.yml")["jobs"]["ready"]) + assert_always_job_refuses_first(job) + if fault == "no-refusal": + del job["steps"][0] + elif fault == "refusal-not-first": + job["steps"].insert(1, job["steps"].pop(0)) + else: + checkout = next(s for s in job["steps"] if "actions/checkout" in s.get("uses", "")) + checkout["if"] = "steps.gate.outputs.evidence == 'true'" + with pytest.raises(AssertionError): + assert_always_job_refuses_first(job) def git(repo: Path, *args: str) -> str: diff --git a/tests/test_release_adapter.py b/tests/test_release_adapter.py index ce48c29..41a8710 100644 --- a/tests/test_release_adapter.py +++ b/tests/test_release_adapter.py @@ -959,6 +959,79 @@ def test_completed_receipt_replay_does_not_dispatch_or_publish( assert report["deployed"] == saved["deployed"] +def assert_pull_request_filter_policy(workflow: dict[str, Any], name: str) -> None: + assert "pull_request" in workflow["on"] + pull_request = workflow["on"]["pull_request"] + if name == "native-feed.yml": + assert isinstance(pull_request, dict) + assert pull_request.get("paths") == [ + "native/feed/**", + ".github/workflows/native-feed.yml", + ] + else: + assert name in {"ci.yml", "docs.yml"} + assert pull_request in (None, "") or isinstance(pull_request, dict) + if isinstance(pull_request, dict): + filters = {"paths", "paths-ignore", "branches", "branches-ignore"} + assert not filters & pull_request.keys() + + +@pytest.mark.parametrize("name", ["ci.yml", "docs.yml", "native-feed.yml"]) +def test_pull_request_trigger_filters_match_authz_roles(name: str) -> None: + workflow = yaml.load((ROOT / ".github/workflows" / name).read_text(), Loader=yaml.BaseLoader) + assert_pull_request_filter_policy(workflow, name) + + +@pytest.mark.parametrize("name", ["ci.yml", "docs.yml"]) +@pytest.mark.parametrize( + "key,value", + [ + ("paths", ["src/**"]), + ("paths-ignore", ["docs/**"]), + ("branches", ["main"]), + ("branches-ignore", ["release/**"]), + ], +) +def test_required_authz_rejects_pull_request_filters( + name: str, key: str, value: list[str] +) -> None: + original = yaml.load((ROOT / ".github/workflows" / name).read_text(), Loader=yaml.BaseLoader) + assert_pull_request_filter_policy(original, name) + workflow = copy.deepcopy(original) + workflow["on"]["pull_request"] = {key: value} + assert workflow != original + with pytest.raises(AssertionError): + assert_pull_request_filter_policy(workflow, name) + + +@pytest.mark.parametrize("name", ["ci.yml", "docs.yml"]) +def test_required_authz_rejects_missing_pull_request_trigger(name: str) -> None: + original = yaml.load((ROOT / ".github/workflows" / name).read_text(), Loader=yaml.BaseLoader) + assert_pull_request_filter_policy(original, name) + workflow = copy.deepcopy(original) + del workflow["on"]["pull_request"] + assert workflow != original + with pytest.raises(AssertionError): + assert_pull_request_filter_policy(workflow, name) + + +@pytest.mark.parametrize("paths", [None, [], ["src/**"]]) +def test_native_authz_guard_rejects_missing_or_changed_pull_request_paths( + paths: list[str] | None, +) -> None: + name = "native-feed.yml" + original = yaml.load((ROOT / ".github/workflows" / name).read_text(), Loader=yaml.BaseLoader) + assert_pull_request_filter_policy(original, name) + workflow = copy.deepcopy(original) + if paths is None: + del workflow["on"]["pull_request"]["paths"] + else: + workflow["on"]["pull_request"]["paths"] = paths + assert workflow != original + with pytest.raises(AssertionError): + assert_pull_request_filter_policy(workflow, name) + + def assert_ready_dependencies(job: dict[str, Any]) -> None: # old: assert set(ci["jobs"]["ready"]["needs"]) == evidence.NEEDS | {"changes"} assert set(job["needs"]) == evidence.NEEDS | {"changes", "authz"} From 531f687fc979fa628221aafd44cf14c3165eff3b Mon Sep 17 00:00:00 2001 From: luisleo526 Date: Tue, 20 Oct 2026 14:50:01 +0800 Subject: [PATCH 06/13] ci: release/ready uses !cancelled() instead of always() (ci-labels.md #830) A superseded run that concurrency cancelled must not leave the aggregate job queued for a runner and block the new head's run. `if: ${{ !cancelled() }}` still runs release/ready (red) when authz failed; its first step still refuses unless authz succeeded, before any checkout. Pins that asserted always() now assert the new condition (old line quoted), and a new pin forbids always() in the three PR-tier workflows with a rejected case. The canon authz block was re-compared with ci-labels.md ff778494 and is unchanged in all three files. Co-Authored-By: Claude Sonnet 5.5 --- .github/workflows/ci.yml | 2 +- tests/test_pr_tier_workflows.py | 25 ++++++++++++++++++++----- tests/test_release_adapter.py | 3 ++- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index db23b18..b1cda2b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -267,7 +267,7 @@ jobs: ready: name: release/ready - if: always() + if: ${{ !cancelled() }} needs: [authz, changes, test, contract, docker, database-e2e, distribution] runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] env: diff --git a/tests/test_pr_tier_workflows.py b/tests/test_pr_tier_workflows.py index 12ed9ac..2d546f2 100644 --- a/tests/test_pr_tier_workflows.py +++ b/tests/test_pr_tier_workflows.py @@ -18,6 +18,8 @@ WORKFLOWS = ROOT / ".github/workflows" PR_TIER_WORKFLOWS = ("ci.yml", "docs.yml", "native-feed.yml") RUNTIME_JOBS = ("docker", "database-e2e", "contract", "distribution") +# ci-labels.md #830: the aggregate job runs unless the run was cancelled (never `always()`). +AGGREGATE_IF = "${{ !cancelled() }}" HOSTED_GUARD = ( "${{ github.event_name != 'pull_request' && " @@ -582,7 +584,9 @@ def assert_docs_only_policy(jobs: dict[str, Any]) -> None: assert " ".join(jobs[name]["if"].split()) == RUNTIME_IF # old: assert set(jobs["ready"]["needs"]) == {"changes", "test", *RUNTIME_JOBS} assert set(jobs["ready"]["needs"]) == {"authz", "changes", "test", *RUNTIME_JOBS} - assert jobs["ready"]["if"] == "always()" + # old: assert jobs["ready"]["if"] == "always()" + # ci-labels.md #830: the aggregate uses !cancelled() so a superseded run never blocks the head. + assert jobs["ready"]["if"] == AGGREGATE_IF assert jobs["ready"]["name"] == "release/ready" @@ -642,8 +646,9 @@ def test_evidence_inventory_stays_the_five_gated_jobs() -> None: def assert_always_job_refuses_first(job: dict[str, Any]) -> None: - """ci-labels.md rule 3a for a job with `if: always()` that needs authz.""" - assert job["if"] == "always()" + """ci-labels.md rule 3a for an aggregate job (status function) that needs authz.""" + # old: assert job["if"] == "always()" + assert job["if"] == AGGREGATE_IF assert "authz" in needs(job) steps = job["steps"] assert steps[0] == REFUSE_STEP, "the refusal is the FIRST step, before any checkout" @@ -666,11 +671,21 @@ def test_always_jobs_refuse_a_failed_authz_before_any_repo_code() -> None: assert "needs.authz.result == 'success'" in condition, (workflow, job_id) -@pytest.mark.parametrize("fault", ["no-refusal", "refusal-not-first", "unguarded-checkout"]) +@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) +def test_no_job_uses_always_so_a_superseded_run_never_blocks_the_head(name: str) -> None: + for job_id, job in load(name)["jobs"].items(): + assert "always()" not in job.get("if", ""), (name, job_id) + + +@pytest.mark.parametrize( + "fault", ["always", "no-refusal", "refusal-not-first", "unguarded-checkout"] +) def test_always_job_rejects_missing_first_refusal_or_unguarded_repo_code(fault: str) -> None: job = copy.deepcopy(load("ci.yml")["jobs"]["ready"]) assert_always_job_refuses_first(job) - if fault == "no-refusal": + if fault == "always": + job["if"] = "always()" + elif fault == "no-refusal": del job["steps"][0] elif fault == "refusal-not-first": job["steps"].insert(1, job["steps"].pop(0)) diff --git a/tests/test_release_adapter.py b/tests/test_release_adapter.py index 41a8710..1e0e9ec 100644 --- a/tests/test_release_adapter.py +++ b/tests/test_release_adapter.py @@ -1043,7 +1043,8 @@ def test_workflows_keep_every_gate_ahead_of_oidc_publication() -> None: assert ci["jobs"]["test"]["strategy"]["matrix"]["python-version"] == ["3.11", "3.12", "3.13"] # old: assert set(ci["jobs"]["ready"]["needs"]) == evidence.NEEDS | {"changes"} assert_ready_dependencies(ci["jobs"]["ready"]) - assert ci["jobs"]["ready"]["if"] == "always()" + # old: assert ci["jobs"]["ready"]["if"] == "always()" + assert ci["jobs"]["ready"]["if"] == "${{ !cancelled() }}" assert ci["jobs"]["ready"]["name"] == "release/ready" for job, suite, enabled in ( ("docker", "docker", "PINEFORGE_DOCKER_TEST=1"), From f396ff255a044a1caeb4cd979c9d825238f0901b Mon Sep 17 00:00:00 2001 From: luisleo526 Date: Tue, 20 Oct 2026 14:52:55 +0800 Subject: [PATCH 07/13] test: show the no-always() pin rejecting an always() job in each PR-tier workflow The previous commit message claimed a rejected case for the new pin; only ready was covered, by another assertion. Add the case for all three workflows. Co-Authored-By: Claude Sonnet 5.5 --- tests/test_pr_tier_workflows.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/tests/test_pr_tier_workflows.py b/tests/test_pr_tier_workflows.py index 2d546f2..734dbe0 100644 --- a/tests/test_pr_tier_workflows.py +++ b/tests/test_pr_tier_workflows.py @@ -671,10 +671,24 @@ def test_always_jobs_refuse_a_failed_authz_before_any_repo_code() -> None: assert "needs.authz.result == 'success'" in condition, (workflow, job_id) +def assert_no_always(name: str, jobs: dict[str, Any]) -> None: + for job_id, job in jobs.items(): + assert "always()" not in job.get("if", ""), (name, job_id) + + @pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) def test_no_job_uses_always_so_a_superseded_run_never_blocks_the_head(name: str) -> None: - for job_id, job in load(name)["jobs"].items(): - assert "always()" not in job.get("if", ""), (name, job_id) + assert_no_always(name, load(name)["jobs"]) + + +@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) +def test_an_always_job_condition_is_rejected_in_every_pr_tier_workflow(name: str) -> None: + jobs = copy.deepcopy(load(name)["jobs"]) + assert_no_always(name, jobs) + job_id = next(iter(jobs)) + jobs[job_id]["if"] = "always() && needs.authz.result == 'success'" + with pytest.raises(AssertionError): + assert_no_always(name, jobs) @pytest.mark.parametrize( From f6deb7fc114cf9c9d4009354e36365bf5d9e4c0f Mon Sep 17 00:00:00 2001 From: luisleo526 Date: Tue, 20 Oct 2026 15:29:05 +0800 Subject: [PATCH 08/13] ci: runtime jobs and release/ready refuse first and guard every run step on authz Rule 3a / ci-policy-check R5: docker, database-e2e, contract and distribution start with the canonical refusal step and every checkout, local-action and run step requires needs.authz.result == 'success'; release/ready's gate step too. Pins extended to the five jobs with rejected cases. Co-Authored-By: Claude Sonnet 5.5 --- .github/workflows/ci.yml | 31 ++++++++++++++++++++++++ tests/test_pr_tier_workflows.py | 43 +++++++++++++++++++++++++++------ tests/test_release_adapter.py | 8 ++++++ 3 files changed, 75 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b1cda2b..2cf6e99 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -109,14 +109,21 @@ jobs: env: PF_CI_LABELS: '["self-hosted","pf-ci","pf-ci-x64"]' steps: + - name: authz must have passed + if: needs.authz.result != 'success' + run: exit 1 - uses: actions/checkout@v7 + if: needs.authz.result == 'success' - uses: actions/setup-python@v6 with: python-version: "3.13" cache: pip - run: python -m pip install -e '.[dev]' + if: needs.authz.result == 'success' - run: PINEFORGE_DOCKER_TEST=1 pytest tests/test_docker_integration.py --junitxml=docker.xml + if: needs.authz.result == 'success' - name: Require every real runtime test with zero skips + if: needs.authz.result == 'success' env: PYTHONPATH: scripts run: python -m release_chain.evidence junit --suite docker --path docker.xml @@ -167,15 +174,23 @@ jobs: PINEFORGE_MYSQL_URL: mysql+pymysql://pineforge:pineforge@127.0.0.1:3306/pineforge PINEFORGE_POSTGRES_URL: postgresql+psycopg://pineforge:pineforge@127.0.0.1:5432/pineforge steps: + - name: authz must have passed + if: needs.authz.result != 'success' + run: exit 1 - uses: actions/checkout@v7 + if: needs.authz.result == 'success' - uses: actions/setup-python@v6 with: python-version: "3.13" cache: pip - run: python -m pip install --upgrade pip + if: needs.authz.result == 'success' - run: python -m pip install -e '.[dev,database-e2e]' + if: needs.authz.result == 'success' - run: pytest tests/test_database_e2e.py --junitxml=database.xml + if: needs.authz.result == 'success' - name: Require the disposable database E2E inventory with zero skips + if: needs.authz.result == 'success' env: PYTHONPATH: scripts run: python -m release_chain.evidence junit --suite database --path database.xml @@ -192,13 +207,18 @@ jobs: env: PF_CI_LABELS: '["self-hosted","pf-ci","pf-ci-x64"]' steps: + - name: authz must have passed + if: needs.authz.result != 'success' + run: exit 1 - uses: actions/checkout@v7 + if: needs.authz.result == 'success' with: persist-credentials: false - uses: actions/setup-python@v6 with: python-version: '3.13' - name: Verify synchronized defaults, immutable manifest, source, facts and both OCI children + if: needs.authz.result == 'success' env: GH_TOKEN: ${{ github.token }} run: python scripts/release_adapter.py local @@ -215,7 +235,11 @@ jobs: env: PF_CI_LABELS: '["self-hosted","pf-ci","pf-ci-x64"]' steps: + - name: authz must have passed + if: needs.authz.result != 'success' + run: exit 1 - uses: actions/checkout@v7 + if: needs.authz.result == 'success' with: persist-credentials: false - uses: actions/setup-python@v6 @@ -223,17 +247,23 @@ jobs: python-version: '3.13' cache: pip - run: python -m pip install -e '.[release]' + if: needs.authz.result == 'success' - name: Set reproducible archive timestamps + if: needs.authz.result == 'success' run: echo "SOURCE_DATE_EPOCH=$(git show -s --format=%ct HEAD)" >> "$GITHUB_ENV" - run: python -m build + if: needs.authz.result == 'success' - run: python -m twine check --strict dist/* + if: needs.authz.result == 'success' - name: The canonical sdist does not ship the native feed + if: needs.authz.result == 'success' run: | if tar -tzf dist/*.tar.gz | grep -E '^[^/]+/native/'; then echo "native/ must stay out of the Python sdist" >&2 exit 1 fi - name: Install and backtest both built distributions without importing checkout source + if: needs.authz.result == 'success' env: PYTHONPATH: scripts run: | @@ -278,6 +308,7 @@ jobs: run: exit 1 - name: Require the path decision and every expected job result id: gate + if: needs.authz.result == 'success' env: AUTHZ_RESULT: ${{ needs.authz.result }} EVENT_NAME: ${{ github.event_name }} diff --git a/tests/test_pr_tier_workflows.py b/tests/test_pr_tier_workflows.py index 734dbe0..2269786 100644 --- a/tests/test_pr_tier_workflows.py +++ b/tests/test_pr_tier_workflows.py @@ -645,25 +645,37 @@ def test_evidence_inventory_stays_the_five_gated_jobs() -> None: } -def assert_always_job_refuses_first(job: dict[str, Any]) -> None: - """ci-labels.md rule 3a for an aggregate job (status function) that needs authz.""" - # old: assert job["if"] == "always()" - assert job["if"] == AGGREGATE_IF +def assert_always_job_refuses_first(job: dict[str, Any], *, runtime: bool = False) -> None: + """ci-labels.md rule 3a for a job with a status function that needs authz.""" + # old: assert job["if"] == AGGREGATE_IF + if runtime: + assert " ".join(job["if"].split()) == RUNTIME_IF + else: + # old: assert job["if"] == "always()" + assert job["if"] == AGGREGATE_IF assert "authz" in needs(job) steps = job["steps"] assert steps[0] == REFUSE_STEP, "the refusal is the FIRST step, before any checkout" for step in steps[1:]: - uses_repo = "actions/checkout" in step.get("uses", "") or "run" in step - if uses_repo and step.get("id") != "gate": + # old: uses_repo = "actions/checkout" in step.get("uses", "") or "run" in step + uses_repo = ( + "actions/checkout" in step.get("uses", "") + or step.get("uses", "").startswith("./") + or "run" in step + ) + # old: if uses_repo and step.get("id") != "gate": + # old: assert "needs.authz.result == 'success'" in step.get("if", ""), step + if uses_repo: assert "needs.authz.result == 'success'" in step.get("if", ""), step def test_always_jobs_refuse_a_failed_authz_before_any_repo_code() -> None: ci = load("ci.yml")["jobs"] assert_always_job_refuses_first(ci["ready"]) - # The four runtime jobs use !cancelled(); their own if requires a successful authz. + # Status functions require the refusal and step guards even with a job-level authz check. for name in RUNTIME_JOBS: assert "needs.authz.result == 'success'" in ci[name]["if"] + assert_always_job_refuses_first(ci[name], runtime=True) for workflow in ("ci.yml", "docs.yml", "native-feed.yml"): for job_id, job in load(workflow)["jobs"].items(): condition = job.get("if", "") @@ -710,6 +722,23 @@ def test_always_job_rejects_missing_first_refusal_or_unguarded_repo_code(fault: assert_always_job_refuses_first(job) +@pytest.mark.parametrize("job_id", [*RUNTIME_JOBS, "ready"]) +@pytest.mark.parametrize("fault", ["no-refusal", "unguarded-run"]) +def test_status_jobs_reject_missing_refusal_or_unguarded_run(job_id: str, fault: str) -> None: + original = load("ci.yml")["jobs"][job_id] + runtime = job_id in RUNTIME_JOBS + assert_always_job_refuses_first(original, runtime=runtime) + job = copy.deepcopy(original) + if fault == "no-refusal": + del job["steps"][0] + else: + run = next(step for step in job["steps"][1:] if "run" in step) + del run["if"] + assert job != original + with pytest.raises(AssertionError): + assert_always_job_refuses_first(job, runtime=runtime) + + def git(repo: Path, *args: str) -> str: return subprocess.run( [ diff --git a/tests/test_release_adapter.py b/tests/test_release_adapter.py index 1e0e9ec..66d90d4 100644 --- a/tests/test_release_adapter.py +++ b/tests/test_release_adapter.py @@ -1050,6 +1050,14 @@ def test_workflows_keep_every_gate_ahead_of_oidc_publication() -> None: ("docker", "docker", "PINEFORGE_DOCKER_TEST=1"), ("database-e2e", "database", None), ): + steps = ci["jobs"][job]["steps"] + assert steps[0] == { + "name": "authz must have passed", + "if": "needs.authz.result != 'success'", + "run": "exit 1", + } + assert steps[1]["uses"] == "actions/checkout@v7" + assert steps[1]["if"] == "needs.authz.result == 'success'" commands = "\n".join(step.get("run", "") for step in ci["jobs"][job]["steps"]) assert f"--suite {suite}" in commands if enabled: From c6f110387a8725e5f35acb7700e64d326a728303 Mon Sep 17 00:00:00 2001 From: luisleo526 Date: Tue, 20 Oct 2026 15:31:33 +0800 Subject: [PATCH 09/13] ci: frozen canon (99de80f9): authz copy, ready always(), native-feed runner per leg authz is the canonical job copied as is (ALLOWED_BOTS dependabot[bot] and pineforge-app[bot]) in ci.yml, docs.yml and native-feed.yml. release/ready is back to if: always() (a cancelled run must not skip the required aggregate). native-feed's release matrix gives every leg its own runner (x64 list, arm64 list, macos-14) and runs-on: ${{ matrix.runner }}. Pins for always() flipped; the authz and matrix pins follow in the next commit. Co-Authored-By: Claude Sonnet 5.5 --- .github/workflows/ci.yml | 57 +++++++++++++++++++++++---- .github/workflows/docs.yml | 55 +++++++++++++++++++++++--- .github/workflows/native-feed.yml | 64 ++++++++++++++++++++++++++----- tests/test_pr_tier_workflows.py | 38 ++++++++++-------- tests/test_release_adapter.py | 4 +- 5 files changed, 178 insertions(+), 40 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2cf6e99..a813fc9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,19 +23,62 @@ concurrency: jobs: authz: - runs-on: [self-hosted, pf-ci, pf-ci-x64] + name: authz + # Pool label (needs no toolchain or Docker). Switch to pf-ci-crj once laneboss's + # bulletin says the Cloud Run class is live. + runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] + permissions: {} + # No timeout-minutes: the owner's no-time-limits rule (check-no-time-limits refuses new ones). steps: - - name: Same-repository pull requests only + - name: Only our own pull requests and our own events run CI env: EVENT: ${{ github.event_name }} - HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} REPO: ${{ github.repository }} + HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} + HEAD_IS_FORK: ${{ github.event.pull_request.head.repo.fork }} + ASSOCIATION: ${{ github.event.pull_request.author_association }} + PR_AUTHOR: ${{ github.event.pull_request.user.login }} + WR_HEAD_REPO: ${{ github.event.workflow_run.head_repository.full_name }} + WR_ACTOR: ${{ github.event.workflow_run.triggering_actor.login }} + GH_TOKEN: ${{ github.token }} # read-only use: one permission lookup for workflow_run + # Bot logins whose same-repository pull requests count as ours: dependabot[bot] (CI-DEPBOT A) + # and the org's own GitHub App bot logins by name (space-separated), nothing else. + ALLOWED_BOTS: "dependabot[bot] pineforge-app[bot]" run: | + set -eu + fail() { echo "::error title=authz::$1"; echo "authz: REFUSED: $1" >> "$GITHUB_STEP_SUMMARY"; exit 1; } case "$EVENT" in - pull_request_target) echo "::error::pull_request_target is not allowed for CI"; exit 1 ;; - pull_request) [ "$HEAD_REPO" = "$REPO" ] || { echo "::error::fork pull requests do not run CI"; exit 1; } ;; + push|workflow_dispatch|schedule|merge_group|release|workflow_call|repository_dispatch) + # Only someone with write access to this repository (or our App's token) can cause these + # events; none of them runs pull-request code. + ;; + workflow_run) + # The pwn-request path: it runs in THIS repository with secrets even when the triggering run + # came from a fork. Refuse ALWAYS unless the triggering run's head is this repository, and + # apply the same identity rule to its triggering actor as to a pull-request author. + [ "$WR_HEAD_REPO" = "$REPO" ] || fail "workflow_run triggered from another repository (${WR_HEAD_REPO:-unknown})" + case " $ALLOWED_BOTS " in + *" $WR_ACTOR "*) ;; + *) + # No author_association exists on workflow_run: ask GitHub for the actor's permission on + # this repository; anything but admin/maintain/write (or a failed call) refuses. + perm=$(gh api "repos/$REPO/collaborators/$WR_ACTOR/permission" --jq .permission 2>/dev/null || echo none) + case "$perm" in admin|maintain|write) ;; *) fail "workflow_run triggering actor ${WR_ACTOR:-unknown} has permission '$perm', not write" ;; esac ;; + esac ;; + pull_request) + [ "$HEAD_REPO" = "$REPO" ] || fail "pull request from another repository (${HEAD_REPO:-unknown})" + [ "$HEAD_IS_FORK" = "false" ] || fail "pull request from a fork" + case "$ASSOCIATION" in + OWNER|MEMBER|COLLABORATOR) ;; + *) + case " $ALLOWED_BOTS " in + *" $PR_AUTHOR "*) ;; + *) fail "author ${PR_AUTHOR:-unknown} (${ASSOCIATION:-NONE}) is not an owner, member, collaborator or allowlisted bot" ;; + esac ;; + esac ;; + *) fail "event $EVENT does not run CI (pull_request_target and other events are not allowed)" ;; esac - echo "authz ok: $EVENT" + echo "authz: allowed ($EVENT${PR_AUTHOR:+ by $PR_AUTHOR})" >> "$GITHUB_STEP_SUMMARY" changes: needs: authz @@ -297,7 +340,7 @@ jobs: ready: name: release/ready - if: ${{ !cancelled() }} + if: always() needs: [authz, changes, test, contract, docker, database-e2e, distribution] runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] env: diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 64158ba..ccb959c 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -17,19 +17,62 @@ concurrency: jobs: authz: - runs-on: [self-hosted, pf-ci, pf-ci-x64] + name: authz + # Pool label (needs no toolchain or Docker). Switch to pf-ci-crj once laneboss's + # bulletin says the Cloud Run class is live. + runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] + permissions: {} + # No timeout-minutes: the owner's no-time-limits rule (check-no-time-limits refuses new ones). steps: - - name: Same-repository pull requests only + - name: Only our own pull requests and our own events run CI env: EVENT: ${{ github.event_name }} - HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} REPO: ${{ github.repository }} + HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} + HEAD_IS_FORK: ${{ github.event.pull_request.head.repo.fork }} + ASSOCIATION: ${{ github.event.pull_request.author_association }} + PR_AUTHOR: ${{ github.event.pull_request.user.login }} + WR_HEAD_REPO: ${{ github.event.workflow_run.head_repository.full_name }} + WR_ACTOR: ${{ github.event.workflow_run.triggering_actor.login }} + GH_TOKEN: ${{ github.token }} # read-only use: one permission lookup for workflow_run + # Bot logins whose same-repository pull requests count as ours: dependabot[bot] (CI-DEPBOT A) + # and the org's own GitHub App bot logins by name (space-separated), nothing else. + ALLOWED_BOTS: "dependabot[bot] pineforge-app[bot]" run: | + set -eu + fail() { echo "::error title=authz::$1"; echo "authz: REFUSED: $1" >> "$GITHUB_STEP_SUMMARY"; exit 1; } case "$EVENT" in - pull_request_target) echo "::error::pull_request_target is not allowed for CI"; exit 1 ;; - pull_request) [ "$HEAD_REPO" = "$REPO" ] || { echo "::error::fork pull requests do not run CI"; exit 1; } ;; + push|workflow_dispatch|schedule|merge_group|release|workflow_call|repository_dispatch) + # Only someone with write access to this repository (or our App's token) can cause these + # events; none of them runs pull-request code. + ;; + workflow_run) + # The pwn-request path: it runs in THIS repository with secrets even when the triggering run + # came from a fork. Refuse ALWAYS unless the triggering run's head is this repository, and + # apply the same identity rule to its triggering actor as to a pull-request author. + [ "$WR_HEAD_REPO" = "$REPO" ] || fail "workflow_run triggered from another repository (${WR_HEAD_REPO:-unknown})" + case " $ALLOWED_BOTS " in + *" $WR_ACTOR "*) ;; + *) + # No author_association exists on workflow_run: ask GitHub for the actor's permission on + # this repository; anything but admin/maintain/write (or a failed call) refuses. + perm=$(gh api "repos/$REPO/collaborators/$WR_ACTOR/permission" --jq .permission 2>/dev/null || echo none) + case "$perm" in admin|maintain|write) ;; *) fail "workflow_run triggering actor ${WR_ACTOR:-unknown} has permission '$perm', not write" ;; esac ;; + esac ;; + pull_request) + [ "$HEAD_REPO" = "$REPO" ] || fail "pull request from another repository (${HEAD_REPO:-unknown})" + [ "$HEAD_IS_FORK" = "false" ] || fail "pull request from a fork" + case "$ASSOCIATION" in + OWNER|MEMBER|COLLABORATOR) ;; + *) + case " $ALLOWED_BOTS " in + *" $PR_AUTHOR "*) ;; + *) fail "author ${PR_AUTHOR:-unknown} (${ASSOCIATION:-NONE}) is not an owner, member, collaborator or allowlisted bot" ;; + esac ;; + esac ;; + *) fail "event $EVENT does not run CI (pull_request_target and other events are not allowed)" ;; esac - echo "authz ok: $EVENT" + echo "authz: allowed ($EVENT${PR_AUTHOR:+ by $PR_AUTHOR})" >> "$GITHUB_STEP_SUMMARY" build: needs: authz diff --git a/.github/workflows/native-feed.yml b/.github/workflows/native-feed.yml index ddfc7c3..61787d6 100644 --- a/.github/workflows/native-feed.yml +++ b/.github/workflows/native-feed.yml @@ -20,19 +20,62 @@ concurrency: jobs: authz: - runs-on: [self-hosted, pf-ci, pf-ci-x64] + name: authz + # Pool label (needs no toolchain or Docker). Switch to pf-ci-crj once laneboss's + # bulletin says the Cloud Run class is live. + runs-on: ["self-hosted", "pf-ci", "pf-ci-x64"] + permissions: {} + # No timeout-minutes: the owner's no-time-limits rule (check-no-time-limits refuses new ones). steps: - - name: Same-repository pull requests only + - name: Only our own pull requests and our own events run CI env: EVENT: ${{ github.event_name }} - HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} REPO: ${{ github.repository }} + HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} + HEAD_IS_FORK: ${{ github.event.pull_request.head.repo.fork }} + ASSOCIATION: ${{ github.event.pull_request.author_association }} + PR_AUTHOR: ${{ github.event.pull_request.user.login }} + WR_HEAD_REPO: ${{ github.event.workflow_run.head_repository.full_name }} + WR_ACTOR: ${{ github.event.workflow_run.triggering_actor.login }} + GH_TOKEN: ${{ github.token }} # read-only use: one permission lookup for workflow_run + # Bot logins whose same-repository pull requests count as ours: dependabot[bot] (CI-DEPBOT A) + # and the org's own GitHub App bot logins by name (space-separated), nothing else. + ALLOWED_BOTS: "dependabot[bot] pineforge-app[bot]" run: | + set -eu + fail() { echo "::error title=authz::$1"; echo "authz: REFUSED: $1" >> "$GITHUB_STEP_SUMMARY"; exit 1; } case "$EVENT" in - pull_request_target) echo "::error::pull_request_target is not allowed for CI"; exit 1 ;; - pull_request) [ "$HEAD_REPO" = "$REPO" ] || { echo "::error::fork pull requests do not run CI"; exit 1; } ;; + push|workflow_dispatch|schedule|merge_group|release|workflow_call|repository_dispatch) + # Only someone with write access to this repository (or our App's token) can cause these + # events; none of them runs pull-request code. + ;; + workflow_run) + # The pwn-request path: it runs in THIS repository with secrets even when the triggering run + # came from a fork. Refuse ALWAYS unless the triggering run's head is this repository, and + # apply the same identity rule to its triggering actor as to a pull-request author. + [ "$WR_HEAD_REPO" = "$REPO" ] || fail "workflow_run triggered from another repository (${WR_HEAD_REPO:-unknown})" + case " $ALLOWED_BOTS " in + *" $WR_ACTOR "*) ;; + *) + # No author_association exists on workflow_run: ask GitHub for the actor's permission on + # this repository; anything but admin/maintain/write (or a failed call) refuses. + perm=$(gh api "repos/$REPO/collaborators/$WR_ACTOR/permission" --jq .permission 2>/dev/null || echo none) + case "$perm" in admin|maintain|write) ;; *) fail "workflow_run triggering actor ${WR_ACTOR:-unknown} has permission '$perm', not write" ;; esac ;; + esac ;; + pull_request) + [ "$HEAD_REPO" = "$REPO" ] || fail "pull request from another repository (${HEAD_REPO:-unknown})" + [ "$HEAD_IS_FORK" = "false" ] || fail "pull request from a fork" + case "$ASSOCIATION" in + OWNER|MEMBER|COLLABORATOR) ;; + *) + case " $ALLOWED_BOTS " in + *" $PR_AUTHOR "*) ;; + *) fail "author ${PR_AUTHOR:-unknown} (${ASSOCIATION:-NONE}) is not an owner, member, collaborator or allowlisted bot" ;; + esac ;; + esac ;; + *) fail "event $EVENT does not run CI (pull_request_target and other events are not allowed)" ;; esac - echo "authz ok: $EVENT" + echo "authz: allowed ($EVENT${PR_AUTHOR:+ by $PR_AUTHOR})" >> "$GITHUB_STEP_SUMMARY" release: name: Release (${{ matrix.os }}) @@ -43,10 +86,13 @@ jobs: os: [ubuntu-24.04, ubuntu-24.04-arm, macos-14] include: - os: ubuntu-24.04 - runner: ${{ fromJSON('["self-hosted","pf-ci","pf-ci-x64"]') }} + runner: ["self-hosted", "pf-ci", "pf-ci-x64"] - os: ubuntu-24.04-arm - runner: ${{ fromJSON('["self-hosted","pf-ci","pf-ci-arm64"]') }} - runs-on: ${{ matrix.runner || matrix.os }} + runner: ["self-hosted", "pf-ci", "pf-ci-arm64"] + # The one named exception: macOS stays on GitHub-hosted until there is Mac hardware. + - os: macos-14 + runner: macos-14 + runs-on: ${{ matrix.runner }} env: PF_CI_LABELS: '["self-hosted","pf-ci","pf-ci-x64"]' timeout-minutes: 30 diff --git a/tests/test_pr_tier_workflows.py b/tests/test_pr_tier_workflows.py index 2269786..0558d47 100644 --- a/tests/test_pr_tier_workflows.py +++ b/tests/test_pr_tier_workflows.py @@ -18,8 +18,10 @@ WORKFLOWS = ROOT / ".github/workflows" PR_TIER_WORKFLOWS = ("ci.yml", "docs.yml", "native-feed.yml") RUNTIME_JOBS = ("docker", "database-e2e", "contract", "distribution") -# ci-labels.md #830: the aggregate job runs unless the run was cancelled (never `always()`). -AGGREGATE_IF = "${{ !cancelled() }}" +# ci-labels.md 99de80f9 (frozen 10-20): the required aggregate runs `if: always()`; never +# `!cancelled()` (a cancelled run would SKIP it and GitHub counts a skipped required check as +# success). old: AGGREGATE_IF = "${{ !cancelled() }}" (#830, reverted for security). +AGGREGATE_IF = "always()" HOSTED_GUARD = ( "${{ github.event_name != 'pull_request' && " @@ -584,8 +586,8 @@ def assert_docs_only_policy(jobs: dict[str, Any]) -> None: assert " ".join(jobs[name]["if"].split()) == RUNTIME_IF # old: assert set(jobs["ready"]["needs"]) == {"changes", "test", *RUNTIME_JOBS} assert set(jobs["ready"]["needs"]) == {"authz", "changes", "test", *RUNTIME_JOBS} - # old: assert jobs["ready"]["if"] == "always()" - # ci-labels.md #830: the aggregate uses !cancelled() so a superseded run never blocks the head. + # old: assert jobs["ready"]["if"] == "always()" (then #830: "${{ !cancelled() }}", reverted) + # ci-labels.md 99de80f9: the required aggregate is always(), never skippable by a cancel. assert jobs["ready"]["if"] == AGGREGATE_IF assert jobs["ready"]["name"] == "release/ready" @@ -683,24 +685,28 @@ def test_always_jobs_refuse_a_failed_authz_before_any_repo_code() -> None: assert "needs.authz.result == 'success'" in condition, (workflow, job_id) -def assert_no_always(name: str, jobs: dict[str, Any]) -> None: +def assert_aggregate_is_always(jobs: dict[str, Any]) -> None: + # old: assert "always()" not in job.get("if", "") for every job (the #830 pin, reverted). + assert jobs["ready"]["if"] == "always()" for job_id, job in jobs.items(): - assert "always()" not in job.get("if", ""), (name, job_id) + condition = job.get("if", "") + if job_id == "ready": + continue + # No other job may be a bare `!cancelled()` aggregate that a cancelled run skips. + assert condition.replace(" ", "") not in ("!cancelled()", "${{!cancelled()}}"), job_id -@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) -def test_no_job_uses_always_so_a_superseded_run_never_blocks_the_head(name: str) -> None: - assert_no_always(name, load(name)["jobs"]) +def test_the_required_aggregate_runs_always_never_cancelled_only() -> None: + assert_aggregate_is_always(load("ci.yml")["jobs"]) -@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) -def test_an_always_job_condition_is_rejected_in_every_pr_tier_workflow(name: str) -> None: - jobs = copy.deepcopy(load(name)["jobs"]) - assert_no_always(name, jobs) - job_id = next(iter(jobs)) - jobs[job_id]["if"] = "always() && needs.authz.result == 'success'" +@pytest.mark.parametrize("fault", ["${{ !cancelled() }}", "!cancelled()", "success()", ""]) +def test_an_aggregate_that_a_cancelled_run_can_skip_is_rejected(fault: str) -> None: + jobs = copy.deepcopy(load("ci.yml")["jobs"]) + assert_aggregate_is_always(jobs) + jobs["ready"]["if"] = fault with pytest.raises(AssertionError): - assert_no_always(name, jobs) + assert_aggregate_is_always(jobs) @pytest.mark.parametrize( diff --git a/tests/test_release_adapter.py b/tests/test_release_adapter.py index 66d90d4..94d0bf0 100644 --- a/tests/test_release_adapter.py +++ b/tests/test_release_adapter.py @@ -1043,8 +1043,8 @@ def test_workflows_keep_every_gate_ahead_of_oidc_publication() -> None: assert ci["jobs"]["test"]["strategy"]["matrix"]["python-version"] == ["3.11", "3.12", "3.13"] # old: assert set(ci["jobs"]["ready"]["needs"]) == evidence.NEEDS | {"changes"} assert_ready_dependencies(ci["jobs"]["ready"]) - # old: assert ci["jobs"]["ready"]["if"] == "always()" - assert ci["jobs"]["ready"]["if"] == "${{ !cancelled() }}" + # old: assert ci["jobs"]["ready"]["if"] == "always()" (#830 made it "${{ !cancelled() }}", reverted) + assert ci["jobs"]["ready"]["if"] == "always()" assert ci["jobs"]["ready"]["name"] == "release/ready" for job, suite, enabled in ( ("docker", "docker", "PINEFORGE_DOCKER_TEST=1"), From 6ba6a5dd7320d0a8b3f2fd07dca436d4db5b2427 Mon Sep 17 00:00:00 2001 From: luisleo526 Date: Tue, 20 Oct 2026 16:06:39 +0800 Subject: [PATCH 10/13] test: pin the frozen canon authz, decision cases with mutation, matrix runner pins (vp110a, before canon delta) --- tests/test_pr_tier_workflows.py | 564 +++++++++++++++++++++++++++++--- tests/test_release_adapter.py | 26 +- 2 files changed, 534 insertions(+), 56 deletions(-) diff --git a/tests/test_pr_tier_workflows.py b/tests/test_pr_tier_workflows.py index 0558d47..f4d19db 100644 --- a/tests/test_pr_tier_workflows.py +++ b/tests/test_pr_tier_workflows.py @@ -31,8 +31,11 @@ POOL = "fromJSON('[\"self-hosted\",\"pf-ci\",\"pf-ci-x64\"]')" POOL_LABELS = ["self-hosted", "pf-ci", "pf-ci-x64"] MATRIX_RUNNERS = { - "ubuntu-24.04": "${{ fromJSON('[\"self-hosted\",\"pf-ci\",\"pf-ci-x64\"]') }}", - "ubuntu-24.04-arm": "${{ fromJSON('[\"self-hosted\",\"pf-ci\",\"pf-ci-arm64\"]') }}", + # old: "ubuntu-24.04": "${{ fromJSON('[\"self-hosted\",\"pf-ci\",\"pf-ci-x64\"]') }}", + "ubuntu-24.04": ["self-hosted", "pf-ci", "pf-ci-x64"], + # old: "ubuntu-24.04-arm": "${{ fromJSON('[\"self-hosted\",\"pf-ci\",\"pf-ci-arm64\"]') }}", + "ubuntu-24.04-arm": ["self-hosted", "pf-ci", "pf-ci-arm64"], + "macos-14": "macos-14", } # These expressions are rejection fixtures for the superseded hosted-fallback policy. OLD_ROUTE = ( @@ -63,14 +66,66 @@ "(github.event_name != 'pull_request' || " "(needs.changes.result == 'success' && needs.changes.outputs.code == 'true'))" ) +CANONICAL_AUTHZ_ENV = { + "EVENT": "${{ github.event_name }}", + "REPO": "${{ github.repository }}", + "HEAD_REPO": "${{ github.event.pull_request.head.repo.full_name }}", + "HEAD_IS_FORK": "${{ github.event.pull_request.head.repo.fork }}", + "ASSOCIATION": "${{ github.event.pull_request.author_association }}", + "PR_AUTHOR": "${{ github.event.pull_request.user.login }}", + "WR_HEAD_REPO": "${{ github.event.workflow_run.head_repository.full_name }}", + "WR_ACTOR": "${{ github.event.workflow_run.triggering_actor.login }}", + "GH_TOKEN": "${{ github.token }}", + "ALLOWED_BOTS": "dependabot[bot] pineforge-app[bot]", +} +# Literal transcription of ci-labels.md 99de80f9, including its shell comments. +# old: CANONICAL_AUTHZ_SCRIPT pinned only pull_request_target and HEAD_REPO. CANONICAL_AUTHZ_SCRIPT = ( + "set -eu\n" + 'fail() { echo "::error title=authz::$1"; echo "authz: REFUSED: $1" ' + '>> "$GITHUB_STEP_SUMMARY"; exit 1; }\n' 'case "$EVENT" in\n' - ' pull_request_target) echo "::error::pull_request_target is not allowed for CI"; ' - "exit 1 ;;\n" - ' pull_request) [ "$HEAD_REPO" = "$REPO" ] || { ' - 'echo "::error::fork pull requests do not run CI"; exit 1; } ;;\n' + " push|workflow_dispatch|schedule|merge_group|release|workflow_call|repository_dispatch)\n" + " # Only someone with write access to this repository (or our App's token) can cause these\n" + " # events; none of them runs pull-request code.\n" + " ;;\n" + " workflow_run)\n" + " # The pwn-request path: it runs in THIS repository with secrets even when " + "the triggering run\n" + " # came from a fork. Refuse ALWAYS unless the triggering run's head is " + "this repository, and\n" + " # apply the same identity rule to its triggering actor as to a pull-request author.\n" + ' [ "$WR_HEAD_REPO" = "$REPO" ] || fail ' + '"workflow_run triggered from another repository (${WR_HEAD_REPO:-unknown})"\n' + ' case " $ALLOWED_BOTS " in\n' + ' *" $WR_ACTOR "*) ;;\n' + " *)\n" + " # No author_association exists on workflow_run: ask GitHub for the actor's " + "permission on\n" + " # this repository; anything but admin/maintain/write (or a failed call) refuses.\n" + ' perm=$(gh api "repos/$REPO/collaborators/$WR_ACTOR/permission" ' + "--jq .permission 2>/dev/null || echo none)\n" + ' case "$perm" in admin|maintain|write) ;; *) fail ' + '"workflow_run triggering actor ${WR_ACTOR:-unknown} has permission ' + "'$perm', not write\" ;; esac ;;\n" + " esac ;;\n" + " pull_request)\n" + ' [ "$HEAD_REPO" = "$REPO" ] || fail ' + '"pull request from another repository (${HEAD_REPO:-unknown})"\n' + ' [ "$HEAD_IS_FORK" = "false" ] || fail "pull request from a fork"\n' + ' case "$ASSOCIATION" in\n' + " OWNER|MEMBER|COLLABORATOR) ;;\n" + " *)\n" + ' case " $ALLOWED_BOTS " in\n' + ' *" $PR_AUTHOR "*) ;;\n' + ' *) fail "author ${PR_AUTHOR:-unknown} (${ASSOCIATION:-NONE}) ' + 'is not an owner, member, collaborator or allowlisted bot" ;;\n' + " esac ;;\n" + " esac ;;\n" + ' *) fail "event $EVENT does not run CI ' + '(pull_request_target and other events are not allowed)" ;;\n' "esac\n" - 'echo "authz ok: $EVENT"\n' + 'echo "authz: allowed ($EVENT${PR_AUTHOR:+ by $PR_AUTHOR})" >> "$GITHUB_STEP_SUMMARY"\n' ) READY_AUTHZ_CHECK = ( 'if [ "$AUTHZ_RESULT" != "success" ]; then echo "authz: $AUTHZ_RESULT"; exit 1; fi' @@ -93,13 +148,13 @@ def assert_native_release_matrix(matrix: dict[str, Any]) -> None: includes = matrix["include"] by_os = {entry["os"]: entry for entry in includes} assert len(by_os) == len(includes), "matrix includes must not override the same OS twice" - assert set(by_os) <= set(matrix["os"]) + # old: assert set(by_os) <= set(matrix["os"]) + assert set(by_os) == set(matrix["os"]) for os_name in matrix["os"]: - entry = by_os.get(os_name, {}) - if os_name == "macos-14": - assert "runner" not in entry - else: - assert entry.get("runner") == MATRIX_RUNNERS[os_name] + # old: entry = by_os.get(os_name, {}) + # old: if os_name == "macos-14": assert "runner" not in entry + # old: else: assert entry.get("runner") == MATRIX_RUNNERS[os_name] + assert by_os[os_name].get("runner") == MATRIX_RUNNERS[os_name] def assert_runner_policy(workflow: dict[str, Any], name: str = "ci.yml") -> None: @@ -107,7 +162,8 @@ def assert_runner_policy(workflow: dict[str, Any], name: str = "ci.yml") -> None if name == "docs.yml" and job_id == "deploy": assert job["runs-on"] == "ubuntu-latest" elif name == "native-feed.yml" and job_id == "release": - assert job["runs-on"] == "${{ matrix.runner || matrix.os }}" + # old: assert job["runs-on"] == "${{ matrix.runner || matrix.os }}" + assert job["runs-on"] == "${{ matrix.runner }}" assert_native_release_matrix(job["strategy"]["matrix"]) else: assert job["runs-on"] == POOL_LABELS @@ -139,6 +195,10 @@ def assert_permission_policy(workflow: dict[str, Any], name: str) -> None: assert job["permissions"] == {"pages": "write", "id-token": "write"} assert job["if"] == "github.event_name != 'pull_request'" assert job["environment"]["name"] == "github-pages" + elif job_id == "authz": + # BaseLoader preserves an empty YAML mapping as a Python dict. + # old: assert "permissions" not in job + assert job["permissions"] == {} else: # old (for authz): assert job.get("permissions") == {"contents": "read"} assert "permissions" not in job @@ -159,24 +219,30 @@ def assert_authz_dependencies(workflow: dict[str, Any]) -> None: assert "authz" in needs(job) +def normalized_authz_script(script: str) -> list[str]: + return [line.strip() for line in script.splitlines() if not line.lstrip().startswith("#")] + + def assert_canonical_authz(job: dict[str, Any]) -> None: - assert set(job) == {"runs-on", "steps"} + # old: assert set(job) == {"runs-on", "steps"} + assert set(job) == {"name", "runs-on", "permissions", "steps"} + assert job["name"] == "authz" assert job["runs-on"] == ["self-hosted", "pf-ci", "pf-ci-x64"] + assert job["permissions"] == {} assert_secret_references(job, set()) steps = job["steps"] assert isinstance(steps, list) - assert len(steps) == 1, "authz is exactly the canonical same-repository check" + assert len(steps) == 1, "authz is exactly the frozen canonical decision" step = steps[0] assert set(step) == {"name", "env", "run"} - assert step["name"] == "Same-repository pull requests only" - assert step["env"] == { - "EVENT": "${{ github.event_name }}", - "HEAD_REPO": "${{ github.event.pull_request.head.repo.full_name }}", - "REPO": "${{ github.repository }}", - } + # old: assert step["name"] == "Same-repository pull requests only" + assert step["name"] == "Only our own pull requests and our own events run CI" + # old: env pinned EVENT, HEAD_REPO and REPO only. + assert step["env"] == CANONICAL_AUTHZ_ENV script = step["run"] assert isinstance(script, str) - assert " ".join(script.split()) == " ".join(CANONICAL_AUTHZ_SCRIPT.split()) + # old: assert " ".join(script.split()) == " ".join(CANONICAL_AUTHZ_SCRIPT.split()) + assert normalized_authz_script(script) == normalized_authz_script(CANONICAL_AUTHZ_SCRIPT) def assert_ready_authz(job: dict[str, Any]) -> None: @@ -246,6 +312,8 @@ def test_each_pool_job_rejects_expression_or_hosted_routes(name: str, route: str ("docs.yml", "deploy", "ubuntu-24.04"), ("native-feed.yml", "release", POOL_LABELS), ("native-feed.yml", "release", "ubuntu-24.04"), + ("native-feed.yml", "release", "${{ matrix.runner || matrix.os }}"), + ("native-feed.yml", "release", "${{ matrix.runner || 'ubuntu-24.04' }}"), ], ) def test_runner_exceptions_reject_other_routes(name: str, job_id: str, route: Any) -> None: @@ -256,7 +324,8 @@ def test_runner_exceptions_reject_other_routes(name: str, job_id: str, route: An assert_runner_policy(workflow, name) -def test_native_release_matrix_uses_only_the_two_linux_pools() -> None: +# old: def test_native_release_matrix_uses_only_the_two_linux_pools() -> None: +def test_native_release_matrix_pins_every_runner_and_the_macos_exception() -> None: assert_native_release_matrix(load("native-feed.yml")["jobs"]["release"]["strategy"]["matrix"]) @@ -265,21 +334,31 @@ def test_native_release_matrix_uses_only_the_two_linux_pools() -> None: [ ("ubuntu-24.04", GUARDED_ROUTE), ("ubuntu-24.04", "ubuntu-24.04"), + ("ubuntu-24.04", "macos-14"), + ("ubuntu-24.04", MATRIX_RUNNERS["ubuntu-24.04-arm"]), + ("ubuntu-24.04", "${{ " + POOL + " }}"), ("ubuntu-24.04-arm", "ubuntu-24.04-arm"), + ("ubuntu-24.04-arm", "macos-14"), ("ubuntu-24.04-arm", MATRIX_RUNNERS["ubuntu-24.04"]), + ( + "ubuntu-24.04-arm", + "${{ fromJSON('[\"self-hosted\",\"pf-ci\",\"pf-ci-arm64\"]') }}", + ), ("macos-14", MATRIX_RUNNERS["ubuntu-24.04"]), + ("macos-14", MATRIX_RUNNERS["ubuntu-24.04-arm"]), + ("macos-14", "ubuntu-24.04"), ], ) def test_native_release_matrix_rejects_fallbacks_and_wrong_pools( - os_name: str, runner: str + os_name: str, runner: Any ) -> None: - matrix = copy.deepcopy(load("native-feed.yml")["jobs"]["release"]["strategy"]["matrix"]) - assert_native_release_matrix(matrix) - if os_name == "macos-14": - matrix["include"].append({"os": os_name, "runner": runner}) - else: - entry = next(entry for entry in matrix["include"] if entry["os"] == os_name) - entry["runner"] = runner + original = load("native-feed.yml")["jobs"]["release"]["strategy"]["matrix"] + assert_native_release_matrix(original) + matrix = copy.deepcopy(original) + # old: matrix["include"].append({"os": os_name, "runner": runner}) for macos-14. + entry = next(entry for entry in matrix["include"] if entry["os"] == os_name) + entry["runner"] = runner + assert matrix != original with pytest.raises(AssertionError): assert_native_release_matrix(matrix) @@ -305,6 +384,26 @@ def test_native_release_matrix_rejects_missing_or_extra_legs(fault: str) -> None assert_native_release_matrix(matrix) +@pytest.mark.parametrize("os_name", list(MATRIX_RUNNERS)) +@pytest.mark.parametrize("fault", ["missing-include", "missing-runner", "duplicate-include"]) +def test_native_release_matrix_rejects_incomplete_or_duplicate_includes( + os_name: str, fault: str +) -> None: + original = load("native-feed.yml")["jobs"]["release"]["strategy"]["matrix"] + assert_native_release_matrix(original) + matrix = copy.deepcopy(original) + entry = next(entry for entry in matrix["include"] if entry["os"] == os_name) + if fault == "missing-include": + matrix["include"].remove(entry) + elif fault == "missing-runner": + del entry["runner"] + else: + matrix["include"].append(dict(entry)) + assert matrix != original + with pytest.raises(AssertionError): + assert_native_release_matrix(matrix) + + @pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) def test_only_superseded_pull_request_runs_are_cancelled(name: str) -> None: assert cancels_only_pull_requests(load(name)["concurrency"]) @@ -450,30 +549,50 @@ def test_authz_rejects_weakened_repository_comparisons(name: str, comparison: st @pytest.mark.parametrize( "old,new", [ + # old: 'pull_request_target) echo "..."; exit 1 ;;' was a separate branch. ( - 'pull_request_target) echo "::error::pull_request_target is not allowed for CI"; ' - "exit 1 ;;", - "", + '*) fail "event $EVENT does not run CI ' + '(pull_request_target and other events are not allowed)" ;;', + "*) ;;", ), ( - 'pull_request_target) echo "::error::pull_request_target is not allowed for CI"; ' - "exit 1 ;;", + '*) fail "event $EVENT does not run CI ' + '(pull_request_target and other events are not allowed)" ;;', "pull_request_target) exit 0 ;;", ), - ("; exit 1 ;;", " ;;"), - ("] || {", "] && {"), - ("exit 1; } ;;", "} ;;"), - ("exit 1; } ;;", "exit 0; } ;;"), - ("] || {", "] || true || {"), + # old: ('; exit 1 ;;', ' ;;') and ('exit 1; } ;;', '} ;;'). + ("; exit 1; }", "; }"), + # old: ("] || {", "] && {"). + ("] || fail", "] && fail"), + ("exit 1; }", "return 0; }"), + ("exit 1; }", "exit 0; }"), + # old: ("] || {", "] || true || {"). + ("] || fail", "] || true || fail"), + ("set -eu", "set -u"), + ('[ "$HEAD_IS_FORK" = "false" ]', '[ "$HEAD_IS_FORK" != "true" ]'), + ('[ "$WR_HEAD_REPO" = "$REPO" ]', '[ "$WR_HEAD_REPO" != "$REPO" ]'), + ("OWNER|MEMBER|COLLABORATOR)", "OWNER|MEMBER|COLLABORATOR|CONTRIBUTOR)"), + ("admin|maintain|write)", "admin|maintain|write|read)"), + ('*" $PR_AUTHOR "*)', '*"$PR_AUTHOR"*)'), + ('*" $WR_ACTOR "*)', '*"$WR_ACTOR"*)'), + ("2>/dev/null || echo none)", "2>/dev/null || echo write)"), ], ids=[ "missing-target-rejection", "target-success", - "target-exit-dropped", + "fail-exit-dropped", "and-instead-of-or", - "fork-exit-dropped", - "fork-exit-zero", + "fail-returns-success", + "fail-exits-zero", "or-true", + "no-errexit", + "fork-not-exactly-false", + "workflow-run-head-reversed", + "contributor-allowed", + "read-permission-allowed", + "pr-bot-substring", + "workflow-run-bot-substring", + "failed-lookup-allowed", ], ) def test_authz_rejects_canonical_script_regressions(name: str, old: str, new: str) -> None: @@ -491,7 +610,7 @@ def test_authz_rejects_canonical_script_regressions(name: str, old: str, new: st @pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) -@pytest.mark.parametrize("key", ["EVENT", "HEAD_REPO", "REPO"]) +@pytest.mark.parametrize("key", list(CANONICAL_AUTHZ_ENV)) def test_authz_rejects_missing_canonical_env(name: str, key: str) -> None: original = load(name)["jobs"]["authz"] assert_canonical_authz(original) @@ -502,6 +621,18 @@ def test_authz_rejects_missing_canonical_env(name: str, key: str) -> None: assert_canonical_authz(job) +@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) +@pytest.mark.parametrize("key", ["name", "runs-on", "permissions", "steps"]) +def test_authz_rejects_missing_canonical_job_keys(name: str, key: str) -> None: + original = load(name)["jobs"]["authz"] + assert_canonical_authz(original) + job = copy.deepcopy(original) + del job[key] + assert job != original + with pytest.raises(AssertionError): + assert_canonical_authz(job) + + @pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) @pytest.mark.parametrize( "target,key,value", @@ -511,6 +642,10 @@ def test_authz_rejects_missing_canonical_env(name: str, key: str) -> None: ("job", "needs", "build"), ("job", "permissions", {"contents": "read"}), ("job", "permissions", {"contents": "write"}), + ("job", "permissions", {"contents": "none"}), + ("job", "permissions", "{}"), + ("job", "permissions", []), + ("job", "permissions", None), ("job", "timeout-minutes", "5"), ("job", "continue-on-error", "true"), ("step", "name", "Authorize CI"), @@ -523,8 +658,16 @@ def test_authz_rejects_missing_canonical_env(name: str, key: str) -> None: ("env", "HEAD_REPO", "${{ github.event.pull_request.head.repo.fork }}"), ("env", "HEAD_REPO", "${{ github.repository }}"), ("env", "REPO", "${{ github.event.pull_request.head.repo.full_name }}"), + ("env", "HEAD_IS_FORK", "${{ github.event.repository.fork }}"), + ("env", "ASSOCIATION", "${{ github.actor }}"), + ("env", "PR_AUTHOR", "${{ github.actor }}"), + ("env", "WR_HEAD_REPO", "${{ github.repository }}"), + ("env", "WR_ACTOR", "${{ github.actor }}"), ("env", "GH_TOKEN", "${{ secrets.GITHUB_TOKEN }}"), ("env", "GH_TOKEN", "unused"), + ("env", "ALLOWED_BOTS", "dependabot[bot] pineforge-app[bot] github-actions[bot]"), + ("env", "ALLOWED_BOTS", "dependabot* pineforge-app*"), + ("env", "ALLOWED_BOTS", "dependabot[bot]"), ("env", "EXTRA", "unused"), ], ) @@ -558,7 +701,8 @@ def test_authz_rejects_noncanonical_step_counts(name: str, fault: str) -> None: @pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) @pytest.mark.parametrize( - "route", ["ubuntu-24.04", ["ubuntu-24.04"], MATRIX_RUNNERS["ubuntu-24.04"]] + # old: the expression fixture was MATRIX_RUNNERS["ubuntu-24.04"]. + "route", ["ubuntu-24.04", ["ubuntu-24.04"], "${{ " + POOL + " }}"] ) def test_authz_rejects_hosted_or_expression_runners(name: str, route: Any) -> None: original = load(name)["jobs"]["authz"] @@ -570,6 +714,318 @@ def test_authz_rejects_hosted_or_expression_runners(name: str, route: Any) -> No assert_canonical_authz(job) +AUTHZ_REPO = "pineforge/pineforge-data" + + +def authz_inputs(event: str = "pull_request", **overrides: str) -> dict[str, str]: + inputs = { + "EVENT": event, + "REPO": AUTHZ_REPO, + "HEAD_REPO": "", + "HEAD_IS_FORK": "", + "ASSOCIATION": "", + "PR_AUTHOR": "", + "WR_HEAD_REPO": "", + "WR_ACTOR": "", + "GH_TOKEN": "unit-test-token", + "ALLOWED_BOTS": "dependabot[bot] pineforge-app[bot]", + "FAKE_GH_PERMISSION": "none", + "FAKE_GH_FAIL": "false", + } + if event == "pull_request": + inputs.update( + HEAD_REPO=AUTHZ_REPO, + HEAD_IS_FORK="false", + ASSOCIATION="MEMBER", + PR_AUTHOR="alice", + ) + elif event == "workflow_run": + inputs.update(WR_HEAD_REPO=AUTHZ_REPO, WR_ACTOR="alice") + return {**inputs, **overrides} + + +# Each row is (environment, exit code, refusal reason, permission lookup expected). +# Refusal reasons are literal expectations, independent of the workflow being executed. +AUTHZ_DECISIONS: dict[str, tuple[dict[str, str], int, str, bool]] = { + **{ + f"pr-{association.lower()}": ( + authz_inputs(ASSOCIATION=association), + 0, + "", + False, + ) + for association in ("MEMBER", "OWNER", "COLLABORATOR") + }, + **{ + f"pr-{author}": ( + authz_inputs(ASSOCIATION="NONE", PR_AUTHOR=author), + 0, + "", + False, + ) + for author in ("dependabot[bot]", "pineforge-app[bot]") + }, + **{ + event: (authz_inputs(event), 0, "", False) + for event in ( + "push", + "workflow_dispatch", + "schedule", + "merge_group", + "release", + "workflow_call", + "repository_dispatch", + ) + }, + **{ + f"workflow-run-{actor}": ( + authz_inputs("workflow_run", WR_ACTOR=actor), + 0, + "", + False, + ) + for actor in ("dependabot[bot]", "pineforge-app[bot]") + }, + **{ + f"workflow-run-{permission}": ( + authz_inputs("workflow_run", FAKE_GH_PERMISSION=permission), + 0, + "", + True, + ) + for permission in ("write", "admin", "maintain") + }, + "pr-other-repository": ( + authz_inputs(HEAD_REPO="outsider/pineforge-data", HEAD_IS_FORK="true"), + 1, + "pull request from another repository (outsider/pineforge-data)", + False, + ), + "pr-fork-flag": ( + authz_inputs(HEAD_IS_FORK="true"), + 1, + "pull request from a fork", + False, + ), + "pr-empty-head": ( + authz_inputs(HEAD_REPO=""), + 1, + "pull request from another repository (unknown)", + False, + ), + **{ + f"pr-outsider-{association}": ( + authz_inputs(ASSOCIATION=association, PR_AUTHOR="outsider"), + 1, + f"author outsider ({association}) is not an owner, member, " + "collaborator or allowlisted bot", + False, + ) + for association in ("NONE", "CONTRIBUTOR") + }, + **{ + f"pr-unlisted-{author}": ( + authz_inputs(ASSOCIATION="NONE", PR_AUTHOR=author), + 1, + f"author {author} (NONE) is not an owner, member, collaborator or allowlisted bot", + False, + ) + for author in ( + "github-actions[bot]", + "renovate[bot]", + "pineforge-app", + "pineforge-app[bot]x", + "xpineforge-app[bot]", + "dependabot", + "dependabot[bot]x", + ) + }, + **{ + event: ( + authz_inputs(event), + 1, + f"event {event} does not run CI " + "(pull_request_target and other events are not allowed)", + False, + ) + for event in ("pull_request_target", "unknown_event") + }, + "workflow-run-other-repository": ( + authz_inputs( + "workflow_run", + WR_HEAD_REPO="outsider/pineforge-data", + FAKE_GH_PERMISSION="write", + ), + 1, + "workflow_run triggered from another repository (outsider/pineforge-data)", + False, + ), + "workflow-run-fork-bot": ( + authz_inputs( + "workflow_run", + WR_HEAD_REPO="outsider/pineforge-data", + WR_ACTOR="dependabot[bot]", + ), + 1, + "workflow_run triggered from another repository (outsider/pineforge-data)", + False, + ), + "workflow-run-empty-head": ( + authz_inputs("workflow_run", WR_HEAD_REPO="", FAKE_GH_PERMISSION="write"), + 1, + "workflow_run triggered from another repository (unknown)", + False, + ), + **{ + f"workflow-run-{permission}": ( + authz_inputs("workflow_run", FAKE_GH_PERMISSION=permission), + 1, + f"workflow_run triggering actor alice has permission '{permission}', not write", + True, + ) + for permission in ("read", "triage", "none") + }, + "workflow-run-lookup-fails": ( + authz_inputs("workflow_run", FAKE_GH_PERMISSION="write", FAKE_GH_FAIL="true"), + 1, + "workflow_run triggering actor alice has permission 'none', not write", + True, + ), +} + +FAKE_AUTHZ_GH = r"""#!/bin/bash +set -eu +printf '%s\n' "$@" >> "$FAKE_GH_CALLS" +[ "$#" -eq 4 ] || exit 97 +[ "$1" = api ] || exit 97 +[ "$2" = "repos/$REPO/collaborators/$WR_ACTOR/permission" ] || exit 97 +[ "$3" = --jq ] || exit 97 +[ "$4" = .permission ] || exit 97 +[ "$GH_TOKEN" = unit-test-token ] || exit 97 +printf '%s\n' validated >> "$FAKE_GH_CALLS" +[ "$FAKE_GH_FAIL" = false ] || exit 1 +printf '%s\n' "$FAKE_GH_PERMISSION" +""" + + +def run_authz( + tmp_path: Path, + name: str, + inputs: dict[str, str], + *, + lookup: bool, + script: str | None = None, +) -> tuple[int, str, str]: + """Execute the workflow's decision with no inherited environment or real gh.""" + tmp_path.mkdir(parents=True, exist_ok=True) + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + gh = bin_dir / "gh" + gh.write_text(FAKE_AUTHZ_GH) + gh.chmod(0o700) + summary = tmp_path / "summary" + summary.write_text("") + calls = tmp_path / "gh-calls" + calls.write_text("") + if script is None: + script = load(name)["jobs"]["authz"]["steps"][0]["run"] + completed = subprocess.run( + ["/bin/bash", "-c", script], + cwd=tmp_path, + env={ + **inputs, + # The stub is the only PATH entry; both scripts use shell builtins. + "PATH": str(bin_dir), + "GITHUB_STEP_SUMMARY": str(summary), + "FAKE_GH_CALLS": str(calls), + }, + capture_output=True, + text=True, + check=False, + ) + expected_calls = ( + [ + "api", + f"repos/{inputs['REPO']}/collaborators/{inputs['WR_ACTOR']}/permission", + "--jq", + ".permission", + "validated", + ] + if lookup + else [] + ) + assert calls.read_text().splitlines() == expected_calls + assert completed.stderr == "" + return completed.returncode, summary.read_text(), completed.stdout + + +def allowed_authz_summary(inputs: dict[str, str]) -> str: + author = f" by {inputs['PR_AUTHOR']}" if inputs["PR_AUTHOR"] else "" + return f"authz: allowed ({inputs['EVENT']}{author})\n" + + +@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) +@pytest.mark.parametrize("case", list(AUTHZ_DECISIONS)) +def test_authz_executes_the_frozen_decision(tmp_path: Path, name: str, case: str) -> None: + inputs, expected_code, reason, lookup = AUTHZ_DECISIONS[case] + returncode, summary, output = run_authz(tmp_path, name, inputs, lookup=lookup) + assert returncode == expected_code + if expected_code == 1: + assert summary == f"authz: REFUSED: {reason}\n" + assert output == f"::error title=authz::{reason}\n" + else: + assert reason == "" + assert summary == allowed_authz_summary(inputs) + assert output == "" + + +@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) +@pytest.mark.parametrize( + "case", + [ + "pr-other-repository", + "pr-fork-flag", + "pr-empty-head", + "pr-outsider-NONE", + "pr-unlisted-github-actions[bot]", + "pull_request_target", + "unknown_event", + "workflow-run-fork-bot", + "workflow-run-read", + "workflow-run-lookup-fails", + ], +) +def test_authz_decisions_depend_on_fail(tmp_path: Path, name: str, case: str) -> None: + inputs, expected_code, reason, lookup = AUTHZ_DECISIONS[case] + assert expected_code == 1 + script = load(name)["jobs"]["authz"]["steps"][0]["run"] + fail_definition = CANONICAL_AUTHZ_SCRIPT.splitlines()[1] + assert script.count(fail_definition) == 1 + mutated = script.replace(fail_definition, "fail() { :; }", 1) + assert mutated != script + assert fail_definition not in mutated + assert "fail() { :; }" in mutated + original = run_authz(tmp_path / "original", name, inputs, lookup=lookup, script=script) + assert original == (1, f"authz: REFUSED: {reason}\n", f"::error title=authz::{reason}\n") + changed = run_authz(tmp_path / "mutated", name, inputs, lookup=lookup, script=mutated) + assert changed == (0, allowed_authz_summary(inputs), "") + + +@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) +def test_authz_decision_depends_on_the_exact_bot_allowlist(tmp_path: Path, name: str) -> None: + inputs, expected_code, reason, lookup = AUTHZ_DECISIONS["pr-unlisted-github-actions[bot]"] + assert expected_code == 1 + script = load(name)["jobs"]["authz"]["steps"][0]["run"] + mutated = ( + 'ALLOWED_BOTS="dependabot[bot] pineforge-app[bot] github-actions[bot]"\n' + script + ) + assert mutated != script + original = run_authz(tmp_path / "original", name, inputs, lookup=lookup, script=script) + assert original == (1, f"authz: REFUSED: {reason}\n", f"::error title=authz::{reason}\n") + changed = run_authz(tmp_path / "mutated", name, inputs, lookup=lookup, script=mutated) + assert changed == (0, allowed_authz_summary(inputs), "") + + def assert_docs_only_policy(jobs: dict[str, Any]) -> None: assert jobs["changes"]["if"] == "github.event_name == 'pull_request'" assert jobs["changes"]["needs"] == "authz" @@ -710,13 +1166,16 @@ def test_an_aggregate_that_a_cancelled_run_can_skip_is_rejected(fault: str) -> N @pytest.mark.parametrize( - "fault", ["always", "no-refusal", "refusal-not-first", "unguarded-checkout"] + # old: "always" wrote "always()" back unchanged after the canon was frozen. + "fault", ["cancelled-only", "no-refusal", "refusal-not-first", "unguarded-checkout"] ) def test_always_job_rejects_missing_first_refusal_or_unguarded_repo_code(fault: str) -> None: - job = copy.deepcopy(load("ci.yml")["jobs"]["ready"]) - assert_always_job_refuses_first(job) - if fault == "always": - job["if"] = "always()" + original = load("ci.yml")["jobs"]["ready"] + assert_always_job_refuses_first(original) + job = copy.deepcopy(original) + if fault == "cancelled-only": + # old: job["if"] = "always()" + job["if"] = "${{ !cancelled() }}" elif fault == "no-refusal": del job["steps"][0] elif fault == "refusal-not-first": @@ -724,6 +1183,7 @@ def test_always_job_rejects_missing_first_refusal_or_unguarded_repo_code(fault: else: checkout = next(s for s in job["steps"] if "actions/checkout" in s.get("uses", "")) checkout["if"] = "steps.gate.outputs.evidence == 'true'" + assert job != original with pytest.raises(AssertionError): assert_always_job_refuses_first(job) diff --git a/tests/test_release_adapter.py b/tests/test_release_adapter.py index 94d0bf0..23377fa 100644 --- a/tests/test_release_adapter.py +++ b/tests/test_release_adapter.py @@ -961,6 +961,7 @@ def test_completed_receipt_replay_does_not_dispatch_or_publish( def assert_pull_request_filter_policy(workflow: dict[str, Any], name: str) -> None: assert "pull_request" in workflow["on"] + assert "pull_request_target" not in workflow["on"] pull_request = workflow["on"]["pull_request"] if name == "native-feed.yml": assert isinstance(pull_request, dict) @@ -982,6 +983,17 @@ def test_pull_request_trigger_filters_match_authz_roles(name: str) -> None: assert_pull_request_filter_policy(workflow, name) +@pytest.mark.parametrize("name", ["ci.yml", "docs.yml", "native-feed.yml"]) +def test_pr_tier_triggers_reject_pull_request_target(name: str) -> None: + original = yaml.load((ROOT / ".github/workflows" / name).read_text(), Loader=yaml.BaseLoader) + assert_pull_request_filter_policy(original, name) + workflow = copy.deepcopy(original) + workflow["on"]["pull_request_target"] = "" + assert workflow != original + with pytest.raises(AssertionError): + assert_pull_request_filter_policy(workflow, name) + + @pytest.mark.parametrize("name", ["ci.yml", "docs.yml"]) @pytest.mark.parametrize( "key,value", @@ -1041,11 +1053,17 @@ def test_workflows_keep_every_gate_ahead_of_oidc_publication() -> None: workflows = ROOT / ".github/workflows" ci = yaml.load((workflows / "ci.yml").read_text(), Loader=yaml.BaseLoader) assert ci["jobs"]["test"]["strategy"]["matrix"]["python-version"] == ["3.11", "3.12", "3.13"] + ready_job = ci["jobs"]["ready"] # old: assert set(ci["jobs"]["ready"]["needs"]) == evidence.NEEDS | {"changes"} - assert_ready_dependencies(ci["jobs"]["ready"]) - # old: assert ci["jobs"]["ready"]["if"] == "always()" (#830 made it "${{ !cancelled() }}", reverted) - assert ci["jobs"]["ready"]["if"] == "always()" - assert ci["jobs"]["ready"]["name"] == "release/ready" + assert_ready_dependencies(ready_job) + # old: #830 temporarily required "${{ !cancelled() }}"; the frozen canon reverts it. + assert ready_job["if"] == "always()" + assert ready_job["name"] == "release/ready" + assert ready_job["steps"][0] == { + "name": "authz must have passed", + "if": "needs.authz.result != 'success'", + "run": "exit 1", + } for job, suite, enabled in ( ("docker", "docker", "PINEFORGE_DOCKER_TEST=1"), ("database-e2e", "database", None), From 303a6f2e2edd397d83f91d9eee803f8559288f8a Mon Sep 17 00:00:00 2001 From: luisleo526 Date: Tue, 20 Oct 2026 16:06:12 +0800 Subject: [PATCH 11/13] ci: authz follows canon d7ba0680 (CONTRIBUTOR permission lookup in the pull_request arm) Only the pull_request arm changes (ci.yml, docs.yml, native-feed.yml); ALLOWED_BOTS stays dependabot[bot] pineforge-app[bot]. Co-Authored-By: Claude Sonnet 5.5 --- .github/workflows/ci.yml | 7 ++++++- .github/workflows/docs.yml | 7 ++++++- .github/workflows/native-feed.yml | 7 ++++++- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a813fc9..8d25174 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -73,7 +73,12 @@ jobs: *) case " $ALLOWED_BOTS " in *" $PR_AUTHOR "*) ;; - *) fail "author ${PR_AUTHOR:-unknown} (${ASSOCIATION:-NONE}) is not an owner, member, collaborator or allowlisted bot" ;; + *) + # A member whose org membership is private shows as CONTRIBUTOR: ask GitHub for the + # author's permission on this repository; anything but admin/maintain/write (or a + # failed call) refuses. + perm=$(gh api "repos/$REPO/collaborators/$PR_AUTHOR/permission" --jq .permission 2>/dev/null || echo none) + case "$perm" in admin|maintain|write) ;; *) fail "author ${PR_AUTHOR:-unknown} (${ASSOCIATION:-NONE}) has permission '$perm', not write, and is not an allowlisted bot" ;; esac ;; esac ;; esac ;; *) fail "event $EVENT does not run CI (pull_request_target and other events are not allowed)" ;; diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index ccb959c..466d5be 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -67,7 +67,12 @@ jobs: *) case " $ALLOWED_BOTS " in *" $PR_AUTHOR "*) ;; - *) fail "author ${PR_AUTHOR:-unknown} (${ASSOCIATION:-NONE}) is not an owner, member, collaborator or allowlisted bot" ;; + *) + # A member whose org membership is private shows as CONTRIBUTOR: ask GitHub for the + # author's permission on this repository; anything but admin/maintain/write (or a + # failed call) refuses. + perm=$(gh api "repos/$REPO/collaborators/$PR_AUTHOR/permission" --jq .permission 2>/dev/null || echo none) + case "$perm" in admin|maintain|write) ;; *) fail "author ${PR_AUTHOR:-unknown} (${ASSOCIATION:-NONE}) has permission '$perm', not write, and is not an allowlisted bot" ;; esac ;; esac ;; esac ;; *) fail "event $EVENT does not run CI (pull_request_target and other events are not allowed)" ;; diff --git a/.github/workflows/native-feed.yml b/.github/workflows/native-feed.yml index 61787d6..8d15373 100644 --- a/.github/workflows/native-feed.yml +++ b/.github/workflows/native-feed.yml @@ -70,7 +70,12 @@ jobs: *) case " $ALLOWED_BOTS " in *" $PR_AUTHOR "*) ;; - *) fail "author ${PR_AUTHOR:-unknown} (${ASSOCIATION:-NONE}) is not an owner, member, collaborator or allowlisted bot" ;; + *) + # A member whose org membership is private shows as CONTRIBUTOR: ask GitHub for the + # author's permission on this repository; anything but admin/maintain/write (or a + # failed call) refuses. + perm=$(gh api "repos/$REPO/collaborators/$PR_AUTHOR/permission" --jq .permission 2>/dev/null || echo none) + case "$perm" in admin|maintain|write) ;; *) fail "author ${PR_AUTHOR:-unknown} (${ASSOCIATION:-NONE}) has permission '$perm', not write, and is not an allowlisted bot" ;; esac ;; esac ;; esac ;; *) fail "event $EVENT does not run CI (pull_request_target and other events are not allowed)" ;; From c6a3f651c75e052b600471e445259b751099a88f Mon Sep 17 00:00:00 2001 From: luisleo526 Date: Tue, 20 Oct 2026 16:34:06 +0800 Subject: [PATCH 12/13] test: pineforge-data CI pins follow canon d7ba0680 (CONTRIBUTOR permission lookup) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_pr_tier_workflows.py | 140 +++++++++++++++++++++++++++++--- 1 file changed, 129 insertions(+), 11 deletions(-) diff --git a/tests/test_pr_tier_workflows.py b/tests/test_pr_tier_workflows.py index f4d19db..54cc64a 100644 --- a/tests/test_pr_tier_workflows.py +++ b/tests/test_pr_tier_workflows.py @@ -78,7 +78,8 @@ "GH_TOKEN": "${{ github.token }}", "ALLOWED_BOTS": "dependabot[bot] pineforge-app[bot]", } -# Literal transcription of ci-labels.md 99de80f9, including its shell comments. +# old: Literal transcription of ci-labels.md 99de80f9, including its shell comments. +# Literal transcription of ci-labels.md d7ba0680, including its shell comments. # old: CANONICAL_AUTHZ_SCRIPT pinned only pull_request_target and HEAD_REPO. CANONICAL_AUTHZ_SCRIPT = ( "set -eu\n" @@ -118,8 +119,19 @@ " *)\n" ' case " $ALLOWED_BOTS " in\n' ' *" $PR_AUTHOR "*) ;;\n' - ' *) fail "author ${PR_AUTHOR:-unknown} (${ASSOCIATION:-NONE}) ' - 'is not an owner, member, collaborator or allowlisted bot" ;;\n' + # old: ' *) fail "author ${PR_AUTHOR:-unknown} (${ASSOCIATION:-NONE}) ' + # old: 'is not an owner, member, collaborator or allowlisted bot" ;;\n' + " *)\n" + " # A member whose org membership is private shows as CONTRIBUTOR: ask GitHub " + "for the\n" + " # author's permission on this repository; anything but admin/maintain/write " + "(or a\n" + " # failed call) refuses.\n" + ' perm=$(gh api "repos/$REPO/collaborators/$PR_AUTHOR/permission" ' + "--jq .permission 2>/dev/null || echo none)\n" + ' case "$perm" in admin|maintain|write) ;; *) fail ' + '"author ${PR_AUTHOR:-unknown} (${ASSOCIATION:-NONE}) has permission ' + "'$perm', not write, and is not an allowlisted bot\" ;; esac ;;\n" " esac ;;\n" " esac ;;\n" ' *) fail "event $EVENT does not run CI ' @@ -765,6 +777,21 @@ def authz_inputs(event: str = "pull_request", **overrides: str) -> dict[str, str ) for author in ("dependabot[bot]", "pineforge-app[bot]") }, + **{ + f"pr-contributor-{permission}": ( + authz_inputs(ASSOCIATION="CONTRIBUTOR", FAKE_GH_PERMISSION=permission), + 0, + "", + True, + ) + for permission in ("write", "admin", "maintain") + }, + "pr-none-write": ( + authz_inputs(ASSOCIATION="NONE", FAKE_GH_PERMISSION="write"), + 0, + "", + True, + ), **{ event: (authz_inputs(event), 0, "", False) for event in ( @@ -807,6 +834,27 @@ def authz_inputs(event: str = "pull_request", **overrides: str) -> dict[str, str "pull request from a fork", False, ), + "pr-contributor-other-repository": ( + authz_inputs( + HEAD_REPO="outsider/pineforge-data", + HEAD_IS_FORK="true", + ASSOCIATION="CONTRIBUTOR", + FAKE_GH_PERMISSION="write", + ), + 1, + "pull request from another repository (outsider/pineforge-data)", + False, + ), + "pr-contributor-fork": ( + authz_inputs( + HEAD_IS_FORK="true", + ASSOCIATION="CONTRIBUTOR", + FAKE_GH_PERMISSION="write", + ), + 1, + "pull request from a fork", + False, + ), "pr-empty-head": ( authz_inputs(HEAD_REPO=""), 1, @@ -817,18 +865,52 @@ def authz_inputs(event: str = "pull_request", **overrides: str) -> dict[str, str f"pr-outsider-{association}": ( authz_inputs(ASSOCIATION=association, PR_AUTHOR="outsider"), 1, - f"author outsider ({association}) is not an owner, member, " - "collaborator or allowlisted bot", - False, + # old: f"author outsider ({association}) is not an owner, member, " + # old: "collaborator or allowlisted bot", + f"author outsider ({association}) has permission 'none', not write, " + "and is not an allowlisted bot", + # old: False, + True, ) for association in ("NONE", "CONTRIBUTOR") }, + **{ + f"pr-{association.lower()}-{permission}": ( + authz_inputs(ASSOCIATION=association, FAKE_GH_PERMISSION=permission), + 1, + f"author alice ({association}) has permission '{permission}', not write, " + "and is not an allowlisted bot", + True, + ) + for association in ("NONE", "CONTRIBUTOR") + for permission in ("read", "triage") + }, + "pr-lookup-fails": ( + authz_inputs( + ASSOCIATION="CONTRIBUTOR", FAKE_GH_PERMISSION="write", FAKE_GH_FAIL="true" + ), + 1, + "author alice (CONTRIBUTOR) has permission 'none', not write, " + "and is not an allowlisted bot", + True, + ), + "pr-lookup-empty": ( + authz_inputs(ASSOCIATION="CONTRIBUTOR", FAKE_GH_PERMISSION=""), + 1, + "author alice (CONTRIBUTOR) has permission '', not write, " + "and is not an allowlisted bot", + True, + ), **{ f"pr-unlisted-{author}": ( authz_inputs(ASSOCIATION="NONE", PR_AUTHOR=author), 1, - f"author {author} (NONE) is not an owner, member, collaborator or allowlisted bot", - False, + # old: f"author {author} (NONE) is not an owner, member, " + # old: "collaborator or allowlisted bot", + f"author {author} (NONE) has permission 'none', not write, " + "and is not an allowlisted bot", + # old: False, + True, ) for author in ( "github-actions[bot]", @@ -898,12 +980,19 @@ def authz_inputs(event: str = "pull_request", **overrides: str) -> dict[str, str printf '%s\n' "$@" >> "$FAKE_GH_CALLS" [ "$#" -eq 4 ] || exit 97 [ "$1" = api ] || exit 97 -[ "$2" = "repos/$REPO/collaborators/$WR_ACTOR/permission" ] || exit 97 +# old: [ "$2" = "repos/$REPO/collaborators/$WR_ACTOR/permission" ] || exit 97 +case "$EVENT" in + pull_request) actor=$PR_AUTHOR ;; + workflow_run) actor=$WR_ACTOR ;; + *) exit 97 ;; +esac +[ "$2" = "repos/$REPO/collaborators/$actor/permission" ] || exit 97 [ "$3" = --jq ] || exit 97 [ "$4" = .permission ] || exit 97 [ "$GH_TOKEN" = unit-test-token ] || exit 97 printf '%s\n' validated >> "$FAKE_GH_CALLS" [ "$FAKE_GH_FAIL" = false ] || exit 1 +[ -n "$FAKE_GH_PERMISSION" ] || exit 0 printf '%s\n' "$FAKE_GH_PERMISSION" """ @@ -943,10 +1032,12 @@ def run_authz( text=True, check=False, ) + actor = inputs["PR_AUTHOR"] if inputs["EVENT"] == "pull_request" else inputs["WR_ACTOR"] expected_calls = ( [ "api", - f"repos/{inputs['REPO']}/collaborators/{inputs['WR_ACTOR']}/permission", + # old: f"repos/{inputs['REPO']}/collaborators/{inputs['WR_ACTOR']}/permission", + f"repos/{inputs['REPO']}/collaborators/{actor}/permission", "--jq", ".permission", "validated", @@ -987,6 +1078,10 @@ def test_authz_executes_the_frozen_decision(tmp_path: Path, name: str, case: str "pr-fork-flag", "pr-empty-head", "pr-outsider-NONE", + "pr-outsider-CONTRIBUTOR", + "pr-contributor-read", + "pr-lookup-fails", + "pr-lookup-empty", "pr-unlisted-github-actions[bot]", "pull_request_target", "unknown_event", @@ -1011,6 +1106,27 @@ def test_authz_decisions_depend_on_fail(tmp_path: Path, name: str, case: str) -> assert changed == (0, allowed_authz_summary(inputs), "") +@pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) +@pytest.mark.parametrize("case", ["pr-none-read", "pr-contributor-read"]) +def test_authz_pr_decision_depends_on_write_permission( + tmp_path: Path, name: str, case: str +) -> None: + inputs, expected_code, reason, lookup = AUTHZ_DECISIONS[case] + assert expected_code == 1 + assert lookup + script = load(name)["jobs"]["authz"]["steps"][0]["run"] + prefix, separator, pr_arm = script.partition(" pull_request)\n") + assert separator + permissions = "admin|maintain|write)" + assert pr_arm.count(permissions) == 1 + mutated = prefix + separator + pr_arm.replace(permissions, "admin|maintain|write|read)", 1) + assert mutated != script + original = run_authz(tmp_path / "original", name, inputs, lookup=lookup, script=script) + assert original == (1, f"authz: REFUSED: {reason}\n", f"::error title=authz::{reason}\n") + changed = run_authz(tmp_path / "mutated", name, inputs, lookup=lookup, script=mutated) + assert changed == (0, allowed_authz_summary(inputs), "") + + @pytest.mark.parametrize("name", PR_TIER_WORKFLOWS) def test_authz_decision_depends_on_the_exact_bot_allowlist(tmp_path: Path, name: str) -> None: inputs, expected_code, reason, lookup = AUTHZ_DECISIONS["pr-unlisted-github-actions[bot]"] @@ -1022,7 +1138,9 @@ def test_authz_decision_depends_on_the_exact_bot_allowlist(tmp_path: Path, name: assert mutated != script original = run_authz(tmp_path / "original", name, inputs, lookup=lookup, script=script) assert original == (1, f"authz: REFUSED: {reason}\n", f"::error title=authz::{reason}\n") - changed = run_authz(tmp_path / "mutated", name, inputs, lookup=lookup, script=mutated) + # old: changed = run_authz(tmp_path / "mutated", name, inputs, + # old: lookup=lookup, script=mutated) + changed = run_authz(tmp_path / "mutated", name, inputs, lookup=False, script=mutated) assert changed == (0, allowed_authz_summary(inputs), "") From b21be27f6d424643c81354e5bc91387bd5dea1f0 Mon Sep 17 00:00:00 2001 From: luisleo526 Date: Tue, 20 Oct 2026 16:39:58 +0800 Subject: [PATCH 13/13] test: authz canon pin compares the whole script line by line, comments included Claude read of the delta: the helper that dropped comment lines weakened the copy-as-is pin. Back to full-text equality, now line by line so newlines (shell syntax) are not collapsed. Co-Authored-By: Claude Sonnet 5.5 --- tests/test_pr_tier_workflows.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/test_pr_tier_workflows.py b/tests/test_pr_tier_workflows.py index 54cc64a..f36ae93 100644 --- a/tests/test_pr_tier_workflows.py +++ b/tests/test_pr_tier_workflows.py @@ -231,10 +231,6 @@ def assert_authz_dependencies(workflow: dict[str, Any]) -> None: assert "authz" in needs(job) -def normalized_authz_script(script: str) -> list[str]: - return [line.strip() for line in script.splitlines() if not line.lstrip().startswith("#")] - - def assert_canonical_authz(job: dict[str, Any]) -> None: # old: assert set(job) == {"runs-on", "steps"} assert set(job) == {"name", "runs-on", "permissions", "steps"} @@ -253,8 +249,10 @@ def assert_canonical_authz(job: dict[str, Any]) -> None: assert step["env"] == CANONICAL_AUTHZ_ENV script = step["run"] assert isinstance(script, str) - # old: assert " ".join(script.split()) == " ".join(CANONICAL_AUTHZ_SCRIPT.split()) - assert normalized_authz_script(script) == normalized_authz_script(CANONICAL_AUTHZ_SCRIPT) + # Line by line (newlines are shell syntax), comments included: the canon says "copy as is". + assert [line.strip() for line in script.strip().splitlines()] == [ + line.strip() for line in CANONICAL_AUTHZ_SCRIPT.strip().splitlines() + ] def assert_ready_authz(job: dict[str, Any]) -> None: