From 426e510cc8765bc3f6a168588203b71b6a2314d5 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Sat, 3 Oct 2026 19:17:42 +0300 Subject: [PATCH 01/16] Move the dependency audit into a local composite action security.yml's audit job now checks out the repository and runs .github/actions/dependency-audit, which holds the audit's steps, so the PR workflow can run the same audit without a second copy. The job keeps its read-only permissions and its gate_failed output, and the artifact keeps its dependency-audit name. Dependabot's github-actions entry also scans .github/actions/*, so the pins inside the composite action stay updated. Co-Authored-By: Claude Opus 5.5 --- .github/actions/dependency-audit/action.yml | 153 ++++++++++++++++++++ .github/dependabot.yml | 7 +- .github/workflows/security.yml | 129 +---------------- 3 files changed, 166 insertions(+), 123 deletions(-) create mode 100644 .github/actions/dependency-audit/action.yml diff --git a/.github/actions/dependency-audit/action.yml b/.github/actions/dependency-audit/action.yml new file mode 100644 index 00000000..77c2bf19 --- /dev/null +++ b/.github/actions/dependency-audit/action.yml @@ -0,0 +1,153 @@ +name: Dependency audit +description: >- + Scans the dependency trees pyproject.toml permits with Trivy and pip-audit + (.github/scripts/audit-deps.sh), renders the report into the job summary, + annotates each blocking advisory, and fails on a fixable HIGH or CRITICAL one. + Uploads the reports as the dependency-audit artifact. The calling job checks + out the repository first and keeps its token read-only, since resolving the + trees can run a dependency's setup.py. + +inputs: + python-version: + description: The Python that runs the report formatter. + required: true + +outputs: + failed: + description: >- + 'true' when the gate found a fixable HIGH or CRITICAL advisory, 'false' + when it passed, empty when the gate did not run. + value: ${{ steps.gate.outputs.failed }} + +runs: + using: composite + steps: + - name: Set up Python + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: ${{ inputs.python-version }} + + # Pinned so a new uv release cannot change which trees get scanned: + # setup-uv installs the uv pinned in uv.lock. + - name: Install uv + uses: astral-sh/setup-uv@c18668ad3cf93ea998bef934396af7bb5c839dc7 # v10.2.0 + with: + version-file: "uv.lock" + + - name: Install Trivy + uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0 + with: + scan-type: filesystem + scan-ref: . + # This invocation exists only to install Trivy. The real scan runs + # in audit-deps.sh, because the action cannot compile the dependency + # trees the scan needs. The action always scans scan-ref, and with the + # paths skipped below the repo root has nothing Trivy can scan, so + # hide-progress (TRIVY_QUIET) keeps that empty scan from logging a + # "Supported files not found" warning. Errors still print. + skip-setup-trivy: false + format: table + exit-code: "0" + scanners: vuln + trivy-config: "" + hide-progress: true + # The migration skill's sample apps pin vulnerable versions on + # purpose and are never installed (skills/tests/README.md). + skip-dirs: skills/tests/fixtures + # uv.lock pins this repository's own CI environment, not what a + # consumer installs; audit-deps.sh scans the published ranges. + skip-files: uv.lock + + - name: Run dependency audit + id: audit + shell: bash + run: | + set -uo pipefail + bash .github/scripts/audit-deps.sh /tmp/audit + echo "ran=true" >> "$GITHUB_OUTPUT" + + - name: Render report + id: render + shell: bash + run: | + # `shell: bash` runs this as `bash --noprofile --norc -eo pipefail {0}`; + # `set -o` can only turn options ON, so an explicit `set +e` is required + # for $? to be observable. + set -uo pipefail + set +e + python .github/scripts/format_audit.py \ + "runtime-ceiling=/tmp/audit/trivy-runtime-ceiling.json" \ + "runtime-floor=/tmp/audit/trivy-runtime-floor.json" \ + "runtime-floor-pydantic-v2=/tmp/audit/trivy-runtime-floor-pydantic-v2.json" \ + "dev-ceiling=/tmp/audit/trivy-dev-ceiling.json" \ + --pip-audit "runtime-ceiling=/tmp/audit/pip-audit-runtime-ceiling.json" \ + --pip-audit "runtime-floor=/tmp/audit/pip-audit-runtime-floor.json" \ + --pip-audit "runtime-floor-pydantic-v2=/tmp/audit/pip-audit-runtime-floor-pydantic-v2.json" \ + --pip-audit "dev-ceiling=/tmp/audit/pip-audit-dev-ceiling.json" \ + --context "pyproject.toml dependencies + dev group, resolved at Python 3.10 (the current resolution, and the lowest versions the published specs permit under each pydantic major)" \ + --blocking \ + > /tmp/audit/comment.md 2>/tmp/audit/format.err + render_exit=$? + set -e + echo "exit=${render_exit}" >> "$GITHUB_OUTPUT" + + - name: Publish to job summary + if: always() && steps.render.outputs.exit == '0' + shell: bash + run: cat /tmp/audit/comment.md >> "$GITHUB_STEP_SUMMARY" + + # Emit one annotation per blocking advisory. This is the only channel + # that reaches a fork PR, where no PR comment is posted for want of a + # write token. + - name: Annotate blocking advisories + if: always() && steps.audit.outputs.ran == 'true' + shell: bash + run: | + set -uo pipefail + python .github/scripts/format_audit.py \ + "runtime-ceiling=/tmp/audit/trivy-runtime-ceiling.json" \ + "runtime-floor=/tmp/audit/trivy-runtime-floor.json" \ + "runtime-floor-pydantic-v2=/tmp/audit/trivy-runtime-floor-pydantic-v2.json" \ + "dev-ceiling=/tmp/audit/trivy-dev-ceiling.json" \ + --annotations + + # The single pass/fail decision, made by the same tested code that + # rendered the report -- so the comment and the check can never disagree. + # Blocks on fixable HIGH/CRITICAL only, and fails closed if a gating + # scanner report could not be parsed. + - name: Gate on HIGH/CRITICAL + id: gate + shell: bash + run: | + set -uo pipefail + set +e + python .github/scripts/format_audit.py \ + "runtime-ceiling=/tmp/audit/trivy-runtime-ceiling.json" \ + "runtime-floor=/tmp/audit/trivy-runtime-floor.json" \ + "runtime-floor-pydantic-v2=/tmp/audit/trivy-runtime-floor-pydantic-v2.json" \ + "dev-ceiling=/tmp/audit/trivy-dev-ceiling.json" \ + --pip-audit "runtime-ceiling=/tmp/audit/pip-audit-runtime-ceiling.json" \ + --pip-audit "runtime-floor=/tmp/audit/pip-audit-runtime-floor.json" \ + --pip-audit "runtime-floor-pydantic-v2=/tmp/audit/pip-audit-runtime-floor-pydantic-v2.json" \ + --pip-audit "dev-ceiling=/tmp/audit/pip-audit-dev-ceiling.json" \ + --gate + gate_exit=$? + set -e + if [ "${gate_exit}" -ne 0 ]; then + echo "failed=true" >> "$GITHUB_OUTPUT" + echo "::error title=Dependency audit failed::Fixable HIGH/CRITICAL advisories are present. See the job summary for the full report and the required version bumps." + exit 1 + fi + echo "failed=false" >> "$GITHUB_OUTPUT" + + # if: always() is load-bearing: the Gate step above exits non-zero on a + # failing audit, and that is precisely when the jobs that read this + # artifact (the PR comment, the weekly Slack message) need it to tell + # someone what broke. + - name: Upload audit artifacts + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: dependency-audit + path: /tmp/audit/ + retention-days: 30 diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 557bdd52..4a1aff19 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -99,11 +99,14 @@ updates: labels: - "dependencies" - # GitHub Actions versions. + # GitHub Actions versions: "/" covers .github/workflows, and the local + # composite actions under .github/actions need their own entry. # Note: cooldown.semver-major-days is not supported for github-actions -- # Dependabot only honours it on semver-strict ecosystems like uv and npm. - package-ecosystem: "github-actions" - directory: "/" + directories: + - "/" + - "/.github/actions/*" schedule: interval: "weekly" day: "monday" diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 174ed1a7..429f5947 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -54,135 +54,22 @@ jobs: permissions: contents: read outputs: - gate_failed: ${{ steps.gate.outputs.failed }} + gate_failed: ${{ steps.audit.outputs.failed }} steps: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false - - name: Set up Python - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 - with: - python-version: ${{ env.PYTHON_VERSION }} - - # Pinned so a new uv release cannot change which trees get scanned: - # setup-uv installs the uv pinned in uv.lock. - - name: Install uv - uses: astral-sh/setup-uv@c18668ad3cf93ea998bef934396af7bb5c839dc7 # v10.2.0 - with: - version-file: "uv.lock" - - - name: Install Trivy - uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0 - with: - scan-type: filesystem - scan-ref: . - # This invocation exists only to install Trivy. The real scan runs - # in audit-deps.sh, because the action cannot compile the dependency - # trees the scan needs. The action always scans scan-ref, and with the - # paths skipped below the repo root has nothing Trivy can scan, so - # hide-progress (TRIVY_QUIET) keeps that empty scan from logging a - # "Supported files not found" warning. Errors still print. - skip-setup-trivy: false - format: table - exit-code: "0" - scanners: vuln - trivy-config: "" - hide-progress: true - # The migration skill's sample apps pin vulnerable versions on - # purpose and are never installed (skills/tests/README.md). - skip-dirs: skills/tests/fixtures - # uv.lock pins this repository's own CI environment, not what a - # consumer installs; audit-deps.sh scans the published ranges. - skip-files: uv.lock - - - name: Run dependency audit + # .github/actions/dependency-audit holds the audit's steps. It is a + # local action, so it runs from the checkout above. zizmor asks for the + # self-repository syntax ($/...) instead of ./, which actionlint (v1.7.12) + # rejects. + - name: Dependency audit id: audit - run: | - set -uo pipefail - bash .github/scripts/audit-deps.sh /tmp/audit - echo "ran=true" >> "$GITHUB_OUTPUT" - - - name: Render report - id: render - run: | - # GitHub runs this as `bash -e {0}`; `set -o` can only turn options - # ON, so an explicit `set +e` is required for $? to be observable. - set -uo pipefail - set +e - python .github/scripts/format_audit.py \ - "runtime-ceiling=/tmp/audit/trivy-runtime-ceiling.json" \ - "runtime-floor=/tmp/audit/trivy-runtime-floor.json" \ - "runtime-floor-pydantic-v2=/tmp/audit/trivy-runtime-floor-pydantic-v2.json" \ - "dev-ceiling=/tmp/audit/trivy-dev-ceiling.json" \ - --pip-audit "runtime-ceiling=/tmp/audit/pip-audit-runtime-ceiling.json" \ - --pip-audit "runtime-floor=/tmp/audit/pip-audit-runtime-floor.json" \ - --pip-audit "runtime-floor-pydantic-v2=/tmp/audit/pip-audit-runtime-floor-pydantic-v2.json" \ - --pip-audit "dev-ceiling=/tmp/audit/pip-audit-dev-ceiling.json" \ - --context "pyproject.toml dependencies + dev group, resolved at Python 3.10 (the current resolution, and the lowest versions the published specs permit under each pydantic major)" \ - --blocking \ - > /tmp/audit/comment.md 2>/tmp/audit/format.err - render_exit=$? - set -e - echo "exit=${render_exit}" >> "$GITHUB_OUTPUT" - - - name: Publish to job summary - if: always() && steps.render.outputs.exit == '0' - run: cat /tmp/audit/comment.md >> "$GITHUB_STEP_SUMMARY" - - # Emit one annotation per blocking advisory. This is the only channel - # that reaches a fork PR, where the comment step below is skipped for - # want of a write token. - - name: Annotate blocking advisories - if: always() && steps.audit.outputs.ran == 'true' - run: | - set -uo pipefail - python .github/scripts/format_audit.py \ - "runtime-ceiling=/tmp/audit/trivy-runtime-ceiling.json" \ - "runtime-floor=/tmp/audit/trivy-runtime-floor.json" \ - "runtime-floor-pydantic-v2=/tmp/audit/trivy-runtime-floor-pydantic-v2.json" \ - "dev-ceiling=/tmp/audit/trivy-dev-ceiling.json" \ - --annotations - - # The single pass/fail decision, made by the same tested code that - # rendered the report -- so the comment and the check can never disagree. - # Blocks on fixable HIGH/CRITICAL only, and fails closed if a gating - # scanner report could not be parsed. - - name: Gate on HIGH/CRITICAL - id: gate - run: | - set -uo pipefail - set +e - python .github/scripts/format_audit.py \ - "runtime-ceiling=/tmp/audit/trivy-runtime-ceiling.json" \ - "runtime-floor=/tmp/audit/trivy-runtime-floor.json" \ - "runtime-floor-pydantic-v2=/tmp/audit/trivy-runtime-floor-pydantic-v2.json" \ - "dev-ceiling=/tmp/audit/trivy-dev-ceiling.json" \ - --pip-audit "runtime-ceiling=/tmp/audit/pip-audit-runtime-ceiling.json" \ - --pip-audit "runtime-floor=/tmp/audit/pip-audit-runtime-floor.json" \ - --pip-audit "runtime-floor-pydantic-v2=/tmp/audit/pip-audit-runtime-floor-pydantic-v2.json" \ - --pip-audit "dev-ceiling=/tmp/audit/pip-audit-dev-ceiling.json" \ - --gate - gate_exit=$? - set -e - if [ "${gate_exit}" -ne 0 ]; then - echo "failed=true" >> "$GITHUB_OUTPUT" - echo "::error title=Dependency audit failed::Fixable HIGH/CRITICAL advisories are present. See the job summary for the full report and the required version bumps." - exit 1 - fi - echo "failed=false" >> "$GITHUB_OUTPUT" - - # if: always() is load-bearing: the Gate step above exits non-zero on a - # failing audit, and that is precisely when the comment job needs this - # artifact to tell the author what broke. - - name: Upload audit artifacts - if: always() - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + uses: ./.github/actions/dependency-audit # zizmor: ignore[self-repository] with: - name: dependency-audit - path: /tmp/audit/ - retention-days: 30 + python-version: ${{ env.PYTHON_VERSION }} # Holds the only write token in this workflow, and does nothing but download # an artifact and post it. It never runs dependency resolution, so PR-authored From 142b9ede479b521a99729d10d65428f40adea113 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Sat, 3 Oct 2026 19:18:46 +0300 Subject: [PATCH 02/16] Run every PR check from test.yml The pre-commit job moves from pre-commit.yml, which is deleted, and the Dependency Audit, Post Audit Comment, Dependency Review, Audit Script Tests and Workflow Hardening jobs move from security.yml. Job ids and names are unchanged, so the check names the ruleset requires keep passing. test.yml's audit job runs the shared composite action. security.yml keeps only the weekly and manual audit and its Slack notification. Since test.yml runs on every push to main, the audit now runs on each one instead of only when a dependency file changes. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/pre-commit.yml | 57 ------- .github/workflows/security.yml | 225 +----------------------- .github/workflows/test.yml | 283 ++++++++++++++++++++++++++++++- 3 files changed, 286 insertions(+), 279 deletions(-) delete mode 100644 .github/workflows/pre-commit.yml diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml deleted file mode 100644 index 903ec334..00000000 --- a/.github/workflows/pre-commit.yml +++ /dev/null @@ -1,57 +0,0 @@ -name: pre-commit - -on: - pull_request: - push: - branches: [master, main] - -permissions: - contents: read - -jobs: - # The job id is the required status check "pre-commit" on main; keep it. - # - # These steps do what pre-commit/action v3.0.1 does, written out: its last - # release pins actions/cache@v4, which targets the deprecated Node 20 - # runtime, and it has had no release since. pre-commit itself comes from - # uv.lock rather than a pip install. - pre-commit: - runs-on: ubuntu-24.04 - steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - persist-credentials: false - - - name: Install uv - id: setup-uv - uses: astral-sh/setup-uv@c18668ad3cf93ea998bef934396af7bb5c839dc7 # v10.2.0 - with: - version-file: "uv.lock" - python-version: "3.11" - enable-cache: true - - # Hook environments, keyed on the config that defines them and on the - # Python they were built with, since a hook venv does not survive an - # interpreter change. This is the cache pre-commit/action used to provide. - - name: Cache pre-commit hook environments - uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 - with: - path: ~/.cache/pre-commit - key: >- - pre-commit-${{ runner.os }}-py${{ steps.setup-uv.outputs.python-version }}-${{ - hashFiles('.pre-commit-config.yaml') }} - - # pre-commit itself comes from the locked dev group. The ruff, mypy and - # typos hooks are `repo: local` and run through `uv run --locked`, which - # syncs .venv to the default groups first: the project, its dependencies - # (pydantic 2) and the dev tools, so mypy checks against the SDK's real - # dependencies. - - name: Run pre-commit - run: >- - uv run --locked - pre-commit run --all-files --show-diff-on-failure --color=always - - # The SDK imports pydantic differently per major, so its types are checked - # against pydantic 1 as well (the hook above ran against pydantic 2). - - name: Type-check against pydantic 1 - run: uv run --locked --group pydantic-v1 mypy diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 429f5947..883fe87e 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -1,25 +1,8 @@ name: Security +# The weekly and manual dependency audit. Pull requests and pushes to main run +# the same audit (.github/actions/dependency-audit) in test.yml. on: - # DELIBERATELY NOT path-filtered. "Dependency Audit", "Audit Script Tests" - # and "Workflow Hardening" are required status checks on main. GitHub treats - # a required check that never runs as perpetually pending rather than - # passing, so a path filter here would block every PR that happens not to - # touch a dependency file. The audit takes under two minutes, which is - # cheaper than that failure mode. Not base-filtered either: a stacked PR, - # whose base is another PR's branch, gets the same checks before it merges. - pull_request: - # Run on every merge to main too, so a regression is surfaced immediately - # (failed run on main) rather than waiting for the next PR to trip over it. - # No PR comment is posted on push; the job summary carries the detail. - # Filtered here because nothing gates on a push run. - push: - branches: [main, master] - paths: - - "pyproject.toml" - - ".github/workflows/security.yml" - - ".github/scripts/audit-deps.sh" - - ".github/scripts/format_audit.py" # Weekly sweep. A dependency set that was clean when it merged does not stay # clean -- advisories are published against versions that already shipped, so # without a scheduled re-scan the gate only ever sees a tree at the moment it @@ -28,14 +11,13 @@ on: - cron: "0 9 * * 1" # Mondays 09:00 UTC workflow_dispatch: {} -# Read-only by default. pull-requests: write is granted per-job, only to the -# job that posts the comment. +# Read-only. Neither job writes to the repository; Slack is reached through +# its own webhook secret. permissions: contents: read concurrency: group: security-${{ github.ref }} - cancel-in-progress: ${{ github.event_name == 'pull_request' }} env: PYTHON_VERSION: "3.11" @@ -44,13 +26,9 @@ jobs: audit: name: Dependency Audit runs-on: ubuntu-24.04 - # Read-only ON PURPOSE. `uv pip compile` builds an sdist to read its + # Read-only ON PURPOSE: `uv pip compile` builds an sdist to read its # metadata for any dependency without a wheel, which runs that package's - # setup.py on the runner -- against a dependency list the PR author - # controls. Holding a `pull-requests: write` GITHUB_TOKEN across that step - # would hand arbitrary PR-authored code a writable token. The comment is - # posted by a separate job that has the token but never executes any of - # this PR's dependency code. + # setup.py on the runner (see the audit job in test.yml). permissions: contents: read outputs: @@ -71,195 +49,6 @@ jobs: with: python-version: ${{ env.PYTHON_VERSION }} - # Holds the only write token in this workflow, and does nothing but download - # an artifact and post it. It never runs dependency resolution, so PR-authored - # package code and the writable token never coexist in the same job. - comment: - name: Post Audit Comment - runs-on: ubuntu-24.04 - needs: [audit] - # always(): the comment matters most when the audit FAILED. - # Fork PRs get a read-only token, so the post would fail -- they are served - # by the ::error:: annotations the audit job emits instead. - if: | - always() && - github.event_name == 'pull_request' && - github.event.pull_request.head.repo.full_name == github.repository - permissions: - contents: read - pull-requests: write - steps: - # NODE_OPTIONS: the unzip library download-artifact v8.0.1 bundles still - # calls the deprecated Buffer() constructor, so every download prints - # DEP0005 (actions/download-artifact#484). That is the action's code, not - # this workflow's, and no newer release exists. This hides DEP0005 alone; - # drop it once a release stops printing the warning. - - name: Download audit artifacts - id: download - continue-on-error: true - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 - env: - NODE_OPTIONS: --disable-warning=DEP0005 - with: - name: dependency-audit - path: /tmp/audit - - # The script checks the report itself. hashFiles() in `if:` cannot: - # it ignores every file outside the workspace, so it returns '' for - # anything under /tmp/audit. - - name: Comment on PR - if: steps.download.outcome == 'success' - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 - with: - # listComments is paginated: on a busy PR the marker may not be on - # page 1, and missing it would post a duplicate comment every run. - script: | - const fs = require('fs'); - const REPORT = '/tmp/audit/comment.md'; - const MARKER = ''; - // GitHub rejects a comment body longer than this. - const MAX_COMMENT_CHARS = 65536; - - // format_audit.py starts every body it renders with the marker, so - // a report without it means the render step did not finish. - let body = fs.existsSync(REPORT) ? fs.readFileSync(REPORT, 'utf8') : ''; - if (!body.startsWith(MARKER)) { - core.warning( - `No rendered audit report in the artifact (${REPORT}), so no PR comment ` + - 'was posted. See the Dependency Audit job for what went wrong.' - ); - return; - } - if (body.length > MAX_COMMENT_CHARS) { - const runUrl = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}` + - `/actions/runs/${context.runId}`; - core.warning( - `The audit report is ${body.length} characters, over GitHub's ` + - `${MAX_COMMENT_CHARS}-character comment limit. The PR comment links to ` + - 'the job summary instead.' - ); - body = [ - MARKER, - '', - '## Dependency Security Audit', - '', - `The report is ${body.length} characters, too long for a PR comment ` + - `(GitHub allows ${MAX_COMMENT_CHARS}). Read it in the ` + - `[job summary](${runUrl}).`, - '', - ].join('\n'); - } - - const comments = await github.paginate(github.rest.issues.listComments, { - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - per_page: 100, - }); - // Match on author AND marker so a human quoting the report can - // never have their comment overwritten by CI. - const existing = comments.find(c => - c.user?.login === 'github-actions[bot]' && - c.body?.startsWith(MARKER) - ); - if (existing) { - await github.rest.issues.updateComment({ - owner: context.repo.owner, - repo: context.repo.repo, - comment_id: existing.id, - body, - }); - } else { - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - body, - }); - } - - # Free on public repositories. Flags dependencies a PR *introduces*, which - # the tree scan above cannot distinguish from ones that were already there, - # and additionally checks licences. - dependency-review: - name: Dependency Review - runs-on: ubuntu-24.04 - if: github.event_name == 'pull_request' - permissions: - contents: read - pull-requests: write - steps: - - name: Checkout - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - persist-credentials: false - - - name: Dependency Review - uses: actions/dependency-review-action@a1d282b36b6f3519aa1f3fc636f609c47dddb294 # v5.0.0 - with: - fail-on-severity: high - comment-summary-in-pr: on-failure - - # The audit scripts decide whether a release ships. Their contract is - # load-bearing, so it is tested like any other code. - audit-scripts-test: - name: Audit Script Tests - runs-on: ubuntu-24.04 - steps: - - name: Checkout - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - persist-credentials: false - - - name: Install uv - uses: astral-sh/setup-uv@c18668ad3cf93ea998bef934396af7bb5c839dc7 # v10.2.0 - with: - version-file: "uv.lock" - python-version: ${{ env.PYTHON_VERSION }} - enable-cache: true - - # pytest comes from uv.lock's dev group, so a new pytest release cannot - # fail this job through .github/scripts/pytest.ini, which turns every - # warning into an error; -c reads that file rather than the SDK's - # [tool.pytest] in pyproject.toml. The scripts under test are stdlib only, - # so --only-dev leaves the project uninstalled. The schema drift check's and - # the API coverage report's tests run here too: they live next to the audit - # scripts and need no more. - - name: Run CI script tests - run: >- - uv run --locked --only-dev - pytest -c .github/scripts/pytest.ini -q - .github/scripts/test_format_audit.py .github/scripts/test_check_schema_drift.py - .github/scripts/test_api_coverage.py - - - name: Shellcheck the shell scripts - run: shellcheck .github/scripts/audit-deps.sh scripts/generate_models.sh - - # A CVE gate that runs in a workflow an attacker can rewrite is not a gate. - workflow-hardening: - name: Workflow Hardening - runs-on: ubuntu-24.04 - steps: - - name: Checkout - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - persist-credentials: false - - # No inputs: the repository has no action.yml, so the runner builds its - # Dockerfile and runs actionlint with no arguments. actionlint exits 1 - # on any finding, and a non-zero container exit fails the step. - - name: actionlint - uses: rhysd/actionlint@914e7df21a07ef503a81201c76d2b11c789d3fca # v1.7.12 - - - name: zizmor - uses: zizmorcore/zizmor-action@cc914d7f3750a2d13d75c7f184a1060aa0e9d482 # v0.6.4 - with: - # Findings are uploaded to code scanning by default, which needs - # Advanced Security. Keep it to the job log and the exit code. - advanced-security: false - persona: regular - - # Weekly only. A scheduled run has no PR to comment on, so Slack is the only # channel that reaches a person -- which is why it carries the findings # themselves (packages, counts, upgrade targets) rather than just a verdict. @@ -303,7 +92,7 @@ jobs: # Rebuilding the message from the audit job's own artifact keeps all the # Slack escaping inside the unit-tested renderer, rather than # interpolating scanner output into the workflow's payload block. - # NODE_OPTIONS: see the comment job's download step. + # NODE_OPTIONS: see the download step of test.yml's comment job. - name: Download audit artifacts if: steps.check.outputs.configured == 'true' continue-on-error: true diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 27b90a44..3eb1856e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1,19 +1,27 @@ name: Test on: # Every PR, whatever its base: a stacked PR, whose base is another PR's - # branch, gets the full suite before it merges. + # branch, gets the full suite before it merges. No path filter: GitHub + # leaves a required check that never runs pending, which blocks the PR. pull_request: + # Every merge to main runs everything too, the dependency audit included, + # so a regression shows as a failed run on main rather than on the next PR. + # No PR comment is posted on push; the job summaries carry the detail. push: branches: - main - master -# Least privilege. Nothing in this workflow writes to the repository; the -# Permit API calls authenticate with their own secret, not GITHUB_TOKEN. +# Least privilege: read-only by default. Nothing in this workflow writes to the +# repository; the Permit API calls authenticate with their own secret, not +# GITHUB_TOKEN. pull-requests: write is granted per job, only to the two jobs +# that post on the PR: the audit comment and dependency review. permissions: contents: read env: + # The Python the dependency audit and its scripts' tests run on. + PYTHON_VERSION: "3.11" PROJECT_ID: 7f55831d77c642739bc17733ab0af138 #github actions project id (under 'Permit.io Tests' workspace) ENV_NAME: python-sdk-ci # The PDP the required `pytest` jobs run against, pinned by version and by the @@ -504,7 +512,7 @@ jobs: --api-coverage-record "${RUNNER_TEMP}/api-coverage/offline.jsonl" # A missing artifact is not an error here: the report says "not run" for the - # e2e column. NODE_OPTIONS: see the same step in security.yml. + # e2e column. NODE_OPTIONS: see the comment job's download step. - name: Download the e2e request records if: ${{ !cancelled() }} continue-on-error: true @@ -766,3 +774,270 @@ jobs: sys.exit("the scanner found nothing in the 2.x sample app on Python 3.9") print(f"Python 3.9: {len(report['findings'])} findings") PY + + # The job id is the required status check "pre-commit" on main; keep it. + # + # These steps do what pre-commit/action v3.0.1 does, written out: its last + # release pins actions/cache@v4, which targets the deprecated Node 20 + # runtime, and it has had no release since. pre-commit itself comes from + # uv.lock rather than a pip install. + pre-commit: + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Install uv + id: setup-uv + uses: astral-sh/setup-uv@c18668ad3cf93ea998bef934396af7bb5c839dc7 # v10.2.0 + with: + version-file: "uv.lock" + python-version: "3.11" + enable-cache: true + + # Hook environments, keyed on the config that defines them and on the + # Python they were built with, since a hook venv does not survive an + # interpreter change. This is the cache pre-commit/action used to provide. + - name: Cache pre-commit hook environments + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: ~/.cache/pre-commit + key: >- + pre-commit-${{ runner.os }}-py${{ steps.setup-uv.outputs.python-version }}-${{ + hashFiles('.pre-commit-config.yaml') }} + + # pre-commit itself comes from the locked dev group. The ruff, mypy and + # typos hooks are `repo: local` and run through `uv run --locked`, which + # syncs .venv to the default groups first: the project, its dependencies + # (pydantic 2) and the dev tools, so mypy checks against the SDK's real + # dependencies. + - name: Run pre-commit + run: >- + uv run --locked + pre-commit run --all-files --show-diff-on-failure --color=always + + # The SDK imports pydantic differently per major, so its types are checked + # against pydantic 1 as well (the hook above ran against pydantic 2). + - name: Type-check against pydantic 1 + run: uv run --locked --group pydantic-v1 mypy + + # The dependency audit (.github/actions/dependency-audit), on every PR and + # every push to main. security.yml runs the same audit weekly. + audit: + name: Dependency Audit + runs-on: ubuntu-24.04 + # Read-only ON PURPOSE. `uv pip compile` builds an sdist to read its + # metadata for any dependency without a wheel, which runs that package's + # setup.py on the runner -- against a dependency list the PR author + # controls. Holding a `pull-requests: write` GITHUB_TOKEN across that step + # would hand arbitrary PR-authored code a writable token. The comment is + # posted by a separate job that has the token but never executes any of + # this PR's dependency code. + permissions: + contents: read + outputs: + gate_failed: ${{ steps.audit.outputs.failed }} + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + # .github/actions/dependency-audit holds the audit's steps. It is a + # local action, so it runs from the checkout above. zizmor asks for the + # self-repository syntax ($/...) instead of ./, which actionlint (v1.7.12) + # rejects. + - name: Dependency audit + id: audit + uses: ./.github/actions/dependency-audit # zizmor: ignore[self-repository] + with: + python-version: ${{ env.PYTHON_VERSION }} + + # Holds a write token, and does nothing but download an artifact and post it. + # It never runs dependency resolution, so PR-authored package code and the + # writable token never coexist in the same job. + comment: + name: Post Audit Comment + runs-on: ubuntu-24.04 + needs: [audit] + # always(): the comment matters most when the audit FAILED. + # Fork PRs get a read-only token, so the post would fail -- they are served + # by the ::error:: annotations the audit job emits instead. + if: | + always() && + github.event_name == 'pull_request' && + github.event.pull_request.head.repo.full_name == github.repository + permissions: + contents: read + pull-requests: write + steps: + # NODE_OPTIONS: the unzip library download-artifact v8.0.1 bundles still + # calls the deprecated Buffer() constructor, so every download prints + # DEP0005 (actions/download-artifact#484). That is the action's code, not + # this workflow's, and no newer release exists. This hides DEP0005 alone; + # drop it once a release stops printing the warning. + - name: Download audit artifacts + id: download + continue-on-error: true + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + env: + NODE_OPTIONS: --disable-warning=DEP0005 + with: + name: dependency-audit + path: /tmp/audit + + # The script checks the report itself. hashFiles() in `if:` cannot: + # it ignores every file outside the workspace, so it returns '' for + # anything under /tmp/audit. + - name: Comment on PR + if: steps.download.outcome == 'success' + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + # listComments is paginated: on a busy PR the marker may not be on + # page 1, and missing it would post a duplicate comment every run. + script: | + const fs = require('fs'); + const REPORT = '/tmp/audit/comment.md'; + const MARKER = ''; + // GitHub rejects a comment body longer than this. + const MAX_COMMENT_CHARS = 65536; + + // format_audit.py starts every body it renders with the marker, so + // a report without it means the render step did not finish. + let body = fs.existsSync(REPORT) ? fs.readFileSync(REPORT, 'utf8') : ''; + if (!body.startsWith(MARKER)) { + core.warning( + `No rendered audit report in the artifact (${REPORT}), so no PR comment ` + + 'was posted. See the Dependency Audit job for what went wrong.' + ); + return; + } + if (body.length > MAX_COMMENT_CHARS) { + const runUrl = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}` + + `/actions/runs/${context.runId}`; + core.warning( + `The audit report is ${body.length} characters, over GitHub's ` + + `${MAX_COMMENT_CHARS}-character comment limit. The PR comment links to ` + + 'the job summary instead.' + ); + body = [ + MARKER, + '', + '## Dependency Security Audit', + '', + `The report is ${body.length} characters, too long for a PR comment ` + + `(GitHub allows ${MAX_COMMENT_CHARS}). Read it in the ` + + `[job summary](${runUrl}).`, + '', + ].join('\n'); + } + + const comments = await github.paginate(github.rest.issues.listComments, { + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + per_page: 100, + }); + // Match on author AND marker so a human quoting the report can + // never have their comment overwritten by CI. + const existing = comments.find(c => + c.user?.login === 'github-actions[bot]' && + c.body?.startsWith(MARKER) + ); + if (existing) { + await github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existing.id, + body, + }); + } else { + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + body, + }); + } + + # Free on public repositories. Flags dependencies a PR *introduces*, which + # the tree scan above cannot distinguish from ones that were already there, + # and additionally checks licences. + dependency-review: + name: Dependency Review + runs-on: ubuntu-24.04 + if: github.event_name == 'pull_request' + permissions: + contents: read + pull-requests: write + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Dependency Review + uses: actions/dependency-review-action@a1d282b36b6f3519aa1f3fc636f609c47dddb294 # v5.0.0 + with: + fail-on-severity: high + comment-summary-in-pr: on-failure + + # The audit scripts decide whether a release ships. Their contract is + # load-bearing, so it is tested like any other code. + audit-scripts-test: + name: Audit Script Tests + runs-on: ubuntu-24.04 + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Install uv + uses: astral-sh/setup-uv@c18668ad3cf93ea998bef934396af7bb5c839dc7 # v10.2.0 + with: + version-file: "uv.lock" + python-version: ${{ env.PYTHON_VERSION }} + enable-cache: true + + # pytest comes from uv.lock's dev group, so a new pytest release cannot + # fail this job through .github/scripts/pytest.ini, which turns every + # warning into an error; -c reads that file rather than the SDK's + # [tool.pytest] in pyproject.toml. The scripts under test are stdlib only, + # so --only-dev leaves the project uninstalled. The schema drift check's and + # the API coverage report's tests run here too: they live next to the audit + # scripts and need no more. + - name: Run CI script tests + run: >- + uv run --locked --only-dev + pytest -c .github/scripts/pytest.ini -q + .github/scripts/test_format_audit.py .github/scripts/test_check_schema_drift.py + .github/scripts/test_api_coverage.py + + - name: Shellcheck the shell scripts + run: shellcheck .github/scripts/audit-deps.sh scripts/generate_models.sh + + # A CVE gate that runs in a workflow an attacker can rewrite is not a gate. + workflow-hardening: + name: Workflow Hardening + runs-on: ubuntu-24.04 + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + # No inputs: the repository has no action.yml, so the runner builds its + # Dockerfile and runs actionlint with no arguments. actionlint exits 1 + # on any finding, and a non-zero container exit fails the step. + - name: actionlint + uses: rhysd/actionlint@914e7df21a07ef503a81201c76d2b11c789d3fca # v1.7.12 + + - name: zizmor + uses: zizmorcore/zizmor-action@cc914d7f3750a2d13d75c7f184a1060aa0e9d482 # v0.6.4 + with: + # Findings are uploaded to code scanning by default, which needs + # Advanced Security. Keep it to the job log and the exit code. + advanced-security: false + persona: regular From 09725dbb8f2a09120360f45226e7c813782eae4d Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Sat, 3 Oct 2026 19:19:53 +0300 Subject: [PATCH 03/16] Skip the audit comment's steps, not its job, where it cannot post The Post Audit Comment job used to be skipped on pushes and on PRs from forks. It now runs on every event (still after a failed audit) and its download and comment steps run only on a pull request from a branch of this repository, so the job succeeds with its steps skipped elsewhere. An aggregate check can then require it to succeed. Same-repo PRs get the comment as before. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/test.yml | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3eb1856e..00627fc6 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -861,16 +861,20 @@ jobs: name: Post Audit Comment runs-on: ubuntu-24.04 needs: [audit] - # always(): the comment matters most when the audit FAILED. - # Fork PRs get a read-only token, so the post would fail -- they are served - # by the ::error:: annotations the audit job emits instead. - if: | - always() && - github.event_name == 'pull_request' && - github.event.pull_request.head.repo.full_name == github.repository + # always(): the comment matters most when the audit FAILED. The job runs on + # every event, so it is never skipped, and its steps post only on a pull + # request from a branch of this repository; elsewhere they are skipped and + # the job succeeds. Fork PRs get a read-only token, so the post would fail + # -- they are served by the ::error:: annotations the audit job emits + # instead. A push has no PR to comment on. + if: always() permissions: contents: read pull-requests: write + env: + SAME_REPO_PR: >- + ${{ github.event_name == 'pull_request' && + github.event.pull_request.head.repo.full_name == github.repository }} steps: # NODE_OPTIONS: the unzip library download-artifact v8.0.1 bundles still # calls the deprecated Buffer() constructor, so every download prints @@ -879,6 +883,7 @@ jobs: # drop it once a release stops printing the warning. - name: Download audit artifacts id: download + if: env.SAME_REPO_PR == 'true' continue-on-error: true uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 env: @@ -891,7 +896,7 @@ jobs: # it ignores every file outside the workspace, so it returns '' for # anything under /tmp/audit. - name: Comment on PR - if: steps.download.outcome == 'success' + if: env.SAME_REPO_PR == 'true' && steps.download.outcome == 'success' uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: # listComments is paginated: on a busy PR the marker may not be on From 429f59c377415491f15814eb30c45dac8fad3e2e Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Sat, 3 Oct 2026 19:20:12 +0300 Subject: [PATCH 04/16] Add a CI job that fails unless every needed job succeeded CI, the job named CI in test.yml, runs whatever happened to the jobs it needs and fails when any of them failed, was cancelled or was skipped, since GitHub counts a skipped required check as passing. Dependency review may be skipped on a push, where it does not run. CI exits 2 when the job results cannot be read or their count is not EXPECTED_JOBS. CI needs every job in test.yml but e2e-unpinned-pdp, which tests PDPs this repository does not pin and stays non-blocking. Comments that named the per-job required checks now describe CI. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/test.yml | 77 ++++++++++++++++++++++++++++++-------- 1 file changed, 61 insertions(+), 16 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 00627fc6..1e24c04d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -24,9 +24,9 @@ env: PYTHON_VERSION: "3.11" PROJECT_ID: 7f55831d77c642739bc17733ab0af138 #github actions project id (under 'Permit.io Tests' workspace) ENV_NAME: python-sdk-ci - # The PDP the required `pytest` jobs run against, pinned by version and by the - # digest of that version's multi-arch image index, so a new PDP release cannot - # fail a required check. Docker pulls by the digest; the tag only names it. + # The PDP the `pytest` jobs run against, pinned by version and by the digest + # of that version's multi-arch image index, so a new PDP release cannot fail + # CI. Docker pulls by the digest; the tag only names it. # Dependabot does not update this. The `e2e (latest PDP image)` job runs the # suite against permitio/pdp-v2:latest, so a new release shows up there first. # To move the pin, take the version's `digest` from @@ -57,11 +57,6 @@ jobs: - pydantic-version: 'pydantic>=2.0.0' dependency-group: pydantic-v2 pydantic-major: '2' - # NOTE: this name and the matrix shape are load-bearing. Branch protection - # on main requires the contexts "pytest (Pydantic pydantic<2.0.0)" and - # "pytest (Pydantic pydantic>=2.0.0)" by exact string. Renaming the job or - # changing the matrix silently makes those contexts unsatisfiable, which - # blocks every PR from merging until branch protection is updated to match. name: pytest (Pydantic ${{ matrix.pydantic-version }}) steps: - name: Checkout code @@ -264,9 +259,9 @@ jobs: echo "::warning title=Scratch env leaked::Failed to delete environment ${ENV_ID}. Delete it by hand." fi - # The e2e tests against PDPs this repository does not pin. Neither leg is a - # required check, so a new PDP release or a change to the cloud PDP shows up - # here without blocking a PR. + # The e2e tests against PDPs this repository does not pin. CI does not need + # this job, so a new PDP release or a change to the cloud PDP shows up here + # without blocking a PR. # - latest PDP image: the suite against permitio/pdp-v2:latest, on pydantic 2. # Red here with `pytest` green means the newest PDP release behaves unlike # PINNED_PDP_IMAGE. @@ -564,8 +559,7 @@ jobs: if-no-files-found: warn # Offline suite on every supported Python. It needs no secrets and no PDP, so - # it also runs on fork PRs. Kept apart from `pytest` above, whose name and - # matrix are required status checks. + # it also runs on fork PRs. compatibility: runs-on: ubuntu-24.04 timeout-minutes: 15 @@ -716,7 +710,7 @@ jobs: # The migration skill's tests (skills/tests): MIGRATION.md, the skill and its # scanner, checked against each other and against the SDK. They run apart from - # the SDK's suite, with skills/tests/pytest.ini. Not a required check. + # the SDK's suite, with skills/tests/pytest.ini. migration-skill: name: Migration Skill Tests (pydantic ${{ matrix.pydantic }}) runs-on: ubuntu-24.04 @@ -775,8 +769,6 @@ jobs: print(f"Python 3.9: {len(report['findings'])} findings") PY - # The job id is the required status check "pre-commit" on main; keep it. - # # These steps do what pre-commit/action v3.0.1 does, written out: its last # release pins actions/cache@v4, which targets the deprecated Node 20 # runtime, and it has had no release since. pre-commit itself comes from @@ -1046,3 +1038,56 @@ jobs: # Advanced Security. Keep it to the job log and the exit code. advanced-security: false persona: regular + + # The one check to require. It runs whatever happened to the jobs it needs, since + # GitHub counts a skipped required check as passing, and fails unless each of them + # succeeded: a failed, cancelled or skipped job fails it. The audit comment job + # succeeds with its steps skipped where it cannot post. The one exception is + # dependency review, which runs on pull requests only: on a push its job is + # skipped and that passes; on a pull request it must succeed. CI does not need + # e2e-unpinned-pdp, which reports on PDPs this repository does not pin. + # Exits 1 when a job did not succeed, and 2 when the results of fewer or more than + # EXPECTED_JOBS jobs arrive. Change EXPECTED_JOBS when you change needs. + ci: + name: CI + if: always() + needs: + - api-coverage + - audit + - audit-scripts-test + - comment + - compatibility + - dependency-review + - migration-skill + - pre-commit + - pytest + - workflow-hardening + runs-on: ubuntu-24.04 + timeout-minutes: 5 + permissions: {} + steps: + - name: Check the needed jobs + shell: bash + env: + NEEDS: ${{ toJSON(needs) }} + EVENT: ${{ github.event_name }} + EXPECTED_JOBS: 10 + run: | + if ! results=$(jq -r 'to_entries[] | "\(.key) \(.value.result)"' <<<"$NEEDS"); then + echo "::error title=CI::Could not read the job results." + exit 2 + fi + printf '%s\n' "$results" + count=$(grep -c . <<<"$results" || true) + if [[ $count -ne $EXPECTED_JOBS ]]; then + echo "::error title=CI::${count} job results, expected ${EXPECTED_JOBS}." + exit 2 + fi + failed=$(grep -v ' success$' <<<"$results" || true) + if [[ $EVENT == push ]]; then + failed=$(grep -vx 'dependency-review skipped' <<<"$failed" || true) + fi + if [[ -n $failed ]]; then + echo "::error title=CI::Jobs that did not succeed: ${failed//$'\n'/, }" + exit 1 + fi From c8dff9e5ff7e1704eaf67d036fdd7a4601880b4b Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Sat, 3 Oct 2026 19:21:14 +0300 Subject: [PATCH 05/16] Fail Workflow Hardening when CI does not need every job A new step in the Workflow Hardening job reads test.yml with yq and fails when its jobs, minus CI and the ADVISORY_JOBS list, differ from CI's needs, when an ADVISORY_JOBS entry is not a job, is listed twice or is also in CI's needs, or when CI's EXPECTED_JOBS is not the number of jobs CI needs. It exits 2 when it reads no jobs. ADVISORY_JOBS holds e2e-unpinned-pdp, with the reason it does not block a PR. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/test.yml | 72 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 69 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1e24c04d..3f0f61d4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -260,8 +260,8 @@ jobs: fi # The e2e tests against PDPs this repository does not pin. CI does not need - # this job, so a new PDP release or a change to the cloud PDP shows up here - # without blocking a PR. + # this job (it is in ADVISORY_JOBS in the Workflow Hardening job), so a new PDP + # release or a change to the cloud PDP shows up here without blocking a PR. # - latest PDP image: the suite against permitio/pdp-v2:latest, on pydantic 2. # Red here with `pytest` green means the newest PDP release behaves unlike # PINNED_PDP_IMAGE. @@ -1039,6 +1039,70 @@ jobs: advanced-security: false persona: regular + # CI gates only the jobs in its needs, so every other job in this workflow + # must be listed there or in ADVISORY_JOBS, the jobs that run without + # gating a PR. Exits 1 when the jobs, CI's needs and ADVISORY_JOBS disagree + # or CI's EXPECTED_JOBS is not the number of jobs it needs, and 2 when no + # job is read. + - name: Check that CI needs every job + shell: bash + env: + WORKFLOW: .github/workflows/test.yml + # Job ids, separated by spaces. + # - e2e-unpinned-pdp: the e2e tests against the latest PDP image and the + # cloud PDP, which this repository does not pin, so that a new PDP + # release shows up there without blocking a PR. + ADVISORY_JOBS: e2e-unpinned-pdp + run: | + export LC_ALL=C + if ! jobs=$(yq '.jobs | keys | .[] | select(. != "ci")' "$WORKFLOW" | sort) || + [[ -z $jobs ]]; then + echo "::error title=CI needs::No jobs read from $WORKFLOW." + exit 2 + fi + needs=$(yq '.jobs.ci.needs[]' "$WORKFLOW" | sort) + expected=$(yq '.jobs.ci.steps[] | select(.name == "Check the needed jobs") | + .env.EXPECTED_JOBS' "$WORKFLOW") + advisory=$(tr -s ' ' '\n' <<<"$ADVISORY_JOBS" | grep . | sort || true) + status=0 + twice=$(uniq -d <<<"$advisory") + if [[ -n $twice ]]; then + echo "::error title=CI needs::ADVISORY_JOBS lists ${twice//$'\n'/, } twice." + status=1 + fi + advisory=$(uniq <<<"$advisory") + stale=$(comm -23 <(echo "$advisory") <(echo "$jobs")) + if [[ -n $stale ]]; then + echo "::error title=CI needs::ADVISORY_JOBS lists ${stale//$'\n'/, }," \ + "which is not a job in $WORKFLOW." + status=1 + fi + both=$(comm -12 <(echo "$advisory") <(uniq <<<"$needs")) + if [[ -n $both ]]; then + echo "::error title=CI needs::CI needs ${both//$'\n'/, }, which ADVISORY_JOBS" \ + "lists as not gating a PR." + status=1 + fi + gated=$(comm -23 <(echo "$jobs") <(echo "$advisory")) + if [[ $gated != "$needs" ]]; then + echo "::error title=CI needs::CI's needs must list every job in $WORKFLOW" \ + "but CI and ADVISORY_JOBS, once each. Add a new job to CI's needs and" \ + "change EXPECTED_JOBS, or list it in ADVISORY_JOBS with the reason." + diff <(echo "$gated") <(echo "$needs") || true + status=1 + fi + count=$(grep -c . <<<"$needs" || true) + if [[ $expected != "$count" ]]; then + echo "::error title=CI needs::EXPECTED_JOBS in the CI job is ${expected:-not set}," \ + "but CI needs $count jobs." + status=1 + fi + if [[ $status -eq 0 ]]; then + echo "CI needs every job but the advisory ones (${advisory//$'\n'/, }):" \ + "${needs//$'\n'/, }." + fi + exit "$status" + # The one check to require. It runs whatever happened to the jobs it needs, since # GitHub counts a skipped required check as passing, and fails unless each of them # succeeded: a failed, cancelled or skipped job fails it. The audit comment job @@ -1047,7 +1111,9 @@ jobs: # skipped and that passes; on a pull request it must succeed. CI does not need # e2e-unpinned-pdp, which reports on PDPs this repository does not pin. # Exits 1 when a job did not succeed, and 2 when the results of fewer or more than - # EXPECTED_JOBS jobs arrive. Change EXPECTED_JOBS when you change needs. + # EXPECTED_JOBS jobs arrive. Change EXPECTED_JOBS when you change needs. The + # Workflow Hardening job fails when a job is in neither needs nor its + # ADVISORY_JOBS, or when EXPECTED_JOBS is not the number of jobs in needs. ci: name: CI if: always() From d4674eea60d71c5480c2549b1ff99c23b79fd29a Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Sat, 3 Oct 2026 19:24:28 +0300 Subject: [PATCH 06/16] Test the CI job and the job-list check against planted inputs test_ci_checks.py reads both steps' bash and env from test.yml with yq and runs them as a `shell: bash` step runs, against planted job results (failed, cancelled and skipped jobs, a missing or extra result, unreadable JSON, dependency review skipped on a push and on other events) and planted workflows (a job CI does not need, a need that is not a job, stale, repeated or needed advisory entries, a wrong EXPECTED_JOBS, no jobs). The Audit Script Tests job runs it with the other CI script tests. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/test_ci_checks.py | 388 ++++++++++++++++++++++++++++++ .github/workflows/test.yml | 5 +- 2 files changed, 391 insertions(+), 2 deletions(-) create mode 100644 .github/scripts/test_ci_checks.py diff --git a/.github/scripts/test_ci_checks.py b/.github/scripts/test_ci_checks.py new file mode 100644 index 00000000..8c373184 --- /dev/null +++ b/.github/scripts/test_ci_checks.py @@ -0,0 +1,388 @@ +"""Tests for the CI job and the job-list check in .github/workflows/test.yml. + +Both are bash in a workflow `run:` block. These tests read each block and its +`env:` from test.yml with yq, and run it the way GitHub runs a `shell: bash` +step, against planted job results and planted workflows. They need bash, jq and +yq (mikefarah v4) on PATH, as GitHub's ubuntu-24.04 runners have them. + +Run with: +uv run --only-dev pytest -c .github/scripts/pytest.ini .github/scripts/test_ci_checks.py +""" + +from __future__ import annotations + +import copy +import json +import os +import shutil +import subprocess +from pathlib import Path +from typing import Any + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +WORKFLOW = REPO_ROOT / ".github" / "workflows" / "test.yml" +CI_STEP = ("ci", "Check the needed jobs") +NEEDS_CHECK_STEP = ("workflow-hardening", "Check that CI needs every job") +ADVISORY_JOB = "e2e-unpinned-pdp" + + +def tool(name: str) -> str: + path = shutil.which(name) + if path is None: + pytest.fail(f"{name} is not on PATH; these tests run the workflow's bash, which needs it") + return path + + +def read_workflow(path: Path) -> dict[str, Any]: + completed = subprocess.run( # noqa: S603 - yq reads the workflow file under test + [tool("yq"), "-o=json", ".", str(path)], + capture_output=True, + text=True, + check=True, + ) + workflow: dict[str, Any] = json.loads(completed.stdout) + return workflow + + +def find_step(workflow: dict[str, Any], job_and_step: tuple[str, str]) -> dict[str, Any]: + job, name = job_and_step + steps: list[dict[str, Any]] = [ + step for step in workflow["jobs"][job]["steps"] if step.get("name") == name + ] + assert len(steps) == 1, f"expected one step named {name!r} in job {job!r}, found {len(steps)}" + return steps[0] + + +def run_step( + step: dict[str, Any], tmp_path: Path, env: dict[str, str] +) -> subprocess.CompletedProcess[str]: + """Runs a step's `run:` block as `shell: bash` does, with its env and `env` on top.""" + assert step["shell"] == "bash" + script = tmp_path / "step.sh" + script.write_text(step["run"], encoding="utf-8") + step_env = {key: str(value) for key, value in step.get("env", {}).items()} + return subprocess.run( # noqa: S603 - bash runs the workflow's own step script + [tool("bash"), "--noprofile", "--norc", "-eo", "pipefail", str(script)], + env={"PATH": os.environ["PATH"], **step_env, **env}, + cwd=REPO_ROOT, + capture_output=True, + text=True, + check=False, + ) + + +@pytest.fixture(scope="module") +def workflow() -> dict[str, Any]: + return read_workflow(WORKFLOW) + + +@pytest.fixture(scope="module") +def needed(workflow: dict[str, Any]) -> list[str]: + needs: list[str] = workflow["jobs"]["ci"]["needs"] + return needs + + +# --- the CI job --------------------------------------------------------------- + + +def results(needed: list[str], overrides: dict[str, str] | None = None) -> dict[str, Any]: + """`toJSON(needs)` for the given jobs, each a success unless `overrides` gives its result.""" + overrides = overrides or {} + return {job: {"result": overrides.get(job, "success"), "outputs": {}} for job in needed} + + +def run_ci( + workflow: dict[str, Any], tmp_path: Path, needs: dict[str, Any] | str, event: str +) -> subprocess.CompletedProcess[str]: + raw = needs if isinstance(needs, str) else json.dumps(needs) + return run_step(find_step(workflow, CI_STEP), tmp_path, {"NEEDS": raw, "EVENT": event}) + + +def test_ci_is_named_ci_and_runs_whatever_happened_to_its_needs(workflow: dict[str, Any]) -> None: + ci = workflow["jobs"]["ci"] + assert ci["name"] == "CI" + assert ci["if"] == "always()" + + +def test_ci_does_not_need_the_advisory_job(needed: list[str]) -> None: + assert ADVISORY_JOB not in needed + + +@pytest.mark.parametrize("event", ["pull_request", "push"]) +def test_ci_passes_when_every_needed_job_succeeded( + workflow: dict[str, Any], needed: list[str], tmp_path: Path, event: str +) -> None: + completed = run_ci(workflow, tmp_path, results(needed), event) + assert completed.returncode == 0, completed.stdout + completed.stderr + assert "::error" not in completed.stdout + + +@pytest.mark.parametrize("event", ["pull_request", "push"]) +@pytest.mark.parametrize("result", ["failure", "cancelled", "skipped"]) +def test_ci_fails_when_a_needed_job_did_not_succeed( + workflow: dict[str, Any], needed: list[str], tmp_path: Path, result: str, event: str +) -> None: + completed = run_ci(workflow, tmp_path, results(needed, {"pytest": result}), event) + assert completed.returncode == 1 + assert f"::error title=CI::Jobs that did not succeed: pytest {result}" in completed.stdout + + +def test_ci_names_every_job_that_did_not_succeed( + workflow: dict[str, Any], needed: list[str], tmp_path: Path +) -> None: + needs = results(needed, {"audit": "failure", "comment": "skipped"}) + completed = run_ci(workflow, tmp_path, needs, "pull_request") + assert completed.returncode == 1 + assert "Jobs that did not succeed: audit failure, comment skipped" in completed.stdout + + +def test_ci_lets_dependency_review_be_skipped_on_a_push( + workflow: dict[str, Any], needed: list[str], tmp_path: Path +) -> None: + completed = run_ci( + workflow, tmp_path, results(needed, {"dependency-review": "skipped"}), "push" + ) + assert completed.returncode == 0, completed.stdout + completed.stderr + + +@pytest.mark.parametrize("event", ["pull_request", "workflow_dispatch", "schedule"]) +def test_ci_fails_when_dependency_review_is_skipped_on_any_other_event( + workflow: dict[str, Any], needed: list[str], tmp_path: Path, event: str +) -> None: + completed = run_ci(workflow, tmp_path, results(needed, {"dependency-review": "skipped"}), event) + assert completed.returncode == 1 + assert "Jobs that did not succeed: dependency-review skipped" in completed.stdout + + +@pytest.mark.parametrize("result", ["failure", "cancelled"]) +def test_ci_fails_when_dependency_review_does_not_succeed_on_a_push( + workflow: dict[str, Any], needed: list[str], tmp_path: Path, result: str +) -> None: + completed = run_ci(workflow, tmp_path, results(needed, {"dependency-review": result}), "push") + assert completed.returncode == 1 + assert f"Jobs that did not succeed: dependency-review {result}" in completed.stdout + + +def test_ci_fails_when_another_job_is_skipped_on_a_push( + workflow: dict[str, Any], needed: list[str], tmp_path: Path +) -> None: + needs = results(needed, {"dependency-review": "skipped", "comment": "skipped"}) + completed = run_ci(workflow, tmp_path, needs, "push") + assert completed.returncode == 1 + assert "Jobs that did not succeed: comment skipped" in completed.stdout + + +def test_ci_exits_2_when_a_job_result_is_missing( + workflow: dict[str, Any], needed: list[str], tmp_path: Path +) -> None: + completed = run_ci(workflow, tmp_path, results(needed[1:]), "pull_request") + assert completed.returncode == 2 + assert f"{len(needed) - 1} job results, expected {len(needed)}" in completed.stdout + + +def test_ci_exits_2_on_an_extra_job_result( + workflow: dict[str, Any], needed: list[str], tmp_path: Path +) -> None: + completed = run_ci(workflow, tmp_path, results([*needed, "extra"]), "pull_request") + assert completed.returncode == 2 + assert f"{len(needed) + 1} job results, expected {len(needed)}" in completed.stdout + + +@pytest.mark.parametrize("raw", ["not json", "[1]"]) +def test_ci_exits_2_when_the_results_cannot_be_read( + workflow: dict[str, Any], tmp_path: Path, raw: str +) -> None: + completed = run_ci(workflow, tmp_path, raw, "pull_request") + assert completed.returncode == 2 + assert "Could not read the job results" in completed.stdout + + +@pytest.mark.parametrize("raw", ["", "{}", "[]"]) +def test_ci_exits_2_when_no_job_result_arrives( + workflow: dict[str, Any], needed: list[str], tmp_path: Path, raw: str +) -> None: + completed = run_ci(workflow, tmp_path, raw, "pull_request") + assert completed.returncode == 2 + assert f"0 job results, expected {len(needed)}" in completed.stdout + + +# --- the job-list check in Workflow Hardening --------------------------------- + + +def set_expected_jobs(planted: dict[str, Any], value: object) -> None: + find_step(planted, CI_STEP)["env"]["EXPECTED_JOBS"] = value + + +def run_needs_check( + workflow: dict[str, Any], + tmp_path: Path, + planted: dict[str, Any] | None, + advisory: str | None = None, +) -> subprocess.CompletedProcess[str]: + """Runs the check against `planted` (JSON is YAML), or a missing file when it is None.""" + path = tmp_path / "planted.yml" + if planted is not None: + path.write_text(json.dumps(planted), encoding="utf-8") + env = {"WORKFLOW": str(path)} + if advisory is not None: + env["ADVISORY_JOBS"] = advisory + return run_step(find_step(workflow, NEEDS_CHECK_STEP), tmp_path, env) + + +def test_needs_check_passes_on_the_committed_workflow( + workflow: dict[str, Any], needed: list[str], tmp_path: Path +) -> None: + completed = run_step(find_step(workflow, NEEDS_CHECK_STEP), tmp_path, {}) + assert completed.returncode == 0, completed.stdout + completed.stderr + assert f"({ADVISORY_JOB}): {', '.join(sorted(needed))}." in completed.stdout + + +def test_expected_jobs_is_the_number_of_needed_jobs( + workflow: dict[str, Any], needed: list[str] +) -> None: + assert int(find_step(workflow, CI_STEP)["env"]["EXPECTED_JOBS"]) == len(needed) + + +def test_the_advisory_list_holds_the_e2e_job_only(workflow: dict[str, Any]) -> None: + assert find_step(workflow, NEEDS_CHECK_STEP)["env"]["ADVISORY_JOBS"] == ADVISORY_JOB + + +def test_needs_check_fails_when_ci_does_not_need_a_job( + workflow: dict[str, Any], tmp_path: Path +) -> None: + planted = copy.deepcopy(workflow) + planted["jobs"]["ci"]["needs"].remove("compatibility") + set_expected_jobs(planted, len(planted["jobs"]["ci"]["needs"])) + completed = run_needs_check(workflow, tmp_path, planted) + assert completed.returncode == 1 + assert "CI's needs must list every job" in completed.stdout + assert "< compatibility" in completed.stdout + assert "EXPECTED_JOBS in the CI job" not in completed.stdout + + +def test_needs_check_fails_on_a_new_job_ci_does_not_need( + workflow: dict[str, Any], tmp_path: Path +) -> None: + planted = copy.deepcopy(workflow) + planted["jobs"]["new-job"] = {"runs-on": "ubuntu-24.04", "steps": [{"run": "true"}]} + completed = run_needs_check(workflow, tmp_path, planted) + assert completed.returncode == 1 + assert "< new-job" in completed.stdout + + +def test_needs_check_fails_when_ci_needs_a_job_that_does_not_exist( + workflow: dict[str, Any], tmp_path: Path +) -> None: + planted = copy.deepcopy(workflow) + planted["jobs"]["ci"]["needs"].append("no-such-job") + set_expected_jobs(planted, len(planted["jobs"]["ci"]["needs"])) + completed = run_needs_check(workflow, tmp_path, planted) + assert completed.returncode == 1 + assert "> no-such-job" in completed.stdout + assert "EXPECTED_JOBS in the CI job" not in completed.stdout + + +def test_needs_check_fails_when_ci_needs_a_job_twice( + workflow: dict[str, Any], tmp_path: Path +) -> None: + planted = copy.deepcopy(workflow) + planted["jobs"]["ci"]["needs"].append("pytest") + set_expected_jobs(planted, len(planted["jobs"]["ci"]["needs"])) + completed = run_needs_check(workflow, tmp_path, planted) + assert completed.returncode == 1 + assert "> pytest" in completed.stdout + + +def test_needs_check_fails_when_an_advisory_job_is_not_a_job( + workflow: dict[str, Any], tmp_path: Path +) -> None: + planted = copy.deepcopy(workflow) + del planted["jobs"][ADVISORY_JOB] + completed = run_needs_check(workflow, tmp_path, planted) + assert completed.returncode == 1 + assert f"ADVISORY_JOBS lists {ADVISORY_JOB}, which is not a job" in completed.stdout + assert "CI's needs must list" not in completed.stdout + + +def test_needs_check_fails_on_a_stale_advisory_entry( + workflow: dict[str, Any], tmp_path: Path +) -> None: + advisory = f"{ADVISORY_JOB} gone-job" + completed = run_needs_check(workflow, tmp_path, copy.deepcopy(workflow), advisory) + assert completed.returncode == 1 + assert "ADVISORY_JOBS lists gone-job, which is not a job" in completed.stdout + + +def test_needs_check_fails_on_an_advisory_entry_listed_twice( + workflow: dict[str, Any], tmp_path: Path +) -> None: + advisory = f"{ADVISORY_JOB} {ADVISORY_JOB}" + completed = run_needs_check(workflow, tmp_path, copy.deepcopy(workflow), advisory) + assert completed.returncode == 1 + assert f"ADVISORY_JOBS lists {ADVISORY_JOB} twice" in completed.stdout + assert "CI's needs must list" not in completed.stdout + + +def test_needs_check_fails_when_ci_needs_an_advisory_job( + workflow: dict[str, Any], tmp_path: Path +) -> None: + planted = copy.deepcopy(workflow) + planted["jobs"]["ci"]["needs"].append(ADVISORY_JOB) + set_expected_jobs(planted, len(planted["jobs"]["ci"]["needs"])) + completed = run_needs_check(workflow, tmp_path, planted) + assert completed.returncode == 1 + assert f"CI needs {ADVISORY_JOB}, which ADVISORY_JOBS" in completed.stdout + + +def test_needs_check_fails_when_a_job_is_neither_needed_nor_advisory( + workflow: dict[str, Any], tmp_path: Path +) -> None: + completed = run_needs_check(workflow, tmp_path, copy.deepcopy(workflow), "") + assert completed.returncode == 1 + assert f"< {ADVISORY_JOB}" in completed.stdout + + +@pytest.mark.parametrize("delta", [-1, 1]) +def test_needs_check_fails_on_a_wrong_expected_jobs( + workflow: dict[str, Any], needed: list[str], tmp_path: Path, delta: int +) -> None: + planted = copy.deepcopy(workflow) + set_expected_jobs(planted, len(needed) + delta) + completed = run_needs_check(workflow, tmp_path, planted) + assert completed.returncode == 1 + assert ( + f"EXPECTED_JOBS in the CI job is {len(needed) + delta}, but CI needs {len(needed)} jobs" + ) in completed.stdout + assert "CI's needs must list" not in completed.stdout + + +def test_needs_check_fails_when_ci_has_no_expected_jobs( + workflow: dict[str, Any], needed: list[str], tmp_path: Path +) -> None: + planted = copy.deepcopy(workflow) + find_step(planted, CI_STEP)["name"] = "Renamed" + completed = run_needs_check(workflow, tmp_path, planted) + assert completed.returncode == 1 + assert f"EXPECTED_JOBS in the CI job is not set, but CI needs {len(needed)}" in ( + completed.stdout + ) + + +@pytest.mark.parametrize( + "planted", + [ + None, + {}, + {"on": "push"}, + {"jobs": {"ci": {"runs-on": "ubuntu-24.04", "steps": [{"run": "true"}]}}}, + ], + ids=["missing file", "empty", "no jobs", "only ci"], +) +def test_needs_check_exits_2_when_no_job_is_read( + workflow: dict[str, Any], tmp_path: Path, planted: dict[str, Any] | None +) -> None: + completed = run_needs_check(workflow, tmp_path, planted) + assert completed.returncode == 2 + assert "No jobs read from" in completed.stdout diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3f0f61d4..604757c5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1004,13 +1004,14 @@ jobs: # [tool.pytest] in pyproject.toml. The scripts under test are stdlib only, # so --only-dev leaves the project uninstalled. The schema drift check's and # the API coverage report's tests run here too: they live next to the audit - # scripts and need no more. + # scripts and need no more. So do the tests of the CI job's and the job-list + # check's bash, which also run bash, jq and yq from the runner image. - name: Run CI script tests run: >- uv run --locked --only-dev pytest -c .github/scripts/pytest.ini -q .github/scripts/test_format_audit.py .github/scripts/test_check_schema_drift.py - .github/scripts/test_api_coverage.py + .github/scripts/test_api_coverage.py .github/scripts/test_ci_checks.py - name: Shellcheck the shell scripts run: shellcheck .github/scripts/audit-deps.sh scripts/generate_models.sh From 644f4ea8dd9786a35c598f914b49ac95170f8e57 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Sat, 3 Oct 2026 19:26:16 +0300 Subject: [PATCH 07/16] Document the CI check and where the weekly audit runs CONTRIBUTING.md gains a CI section: CI in test.yml is the one check to require, how to add a job (to CI's needs with EXPECTED_JOBS, or to ADVISORY_JOBS with the reason), what the job-list check enforces, and that the weekly audit runs from security.yml. The e2e and CI script test sections, the skills tests README and the Dependabot comments no longer name security.yml as the PR audit or the per-job required checks. Co-Authored-By: Claude Opus 5.5 --- .github/dependabot.yml | 10 +++---- CONTRIBUTING.md | 68 +++++++++++++++++++++++++++++++----------- skills/tests/README.md | 4 +-- 3 files changed, 58 insertions(+), 24 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 4a1aff19..0f4b1b35 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -6,9 +6,9 @@ updates: # Consumers never see uv.lock -- this package publishes open `>=` ranges -- # so a Dependabot PR here raises the *floor* consumers are allowed to install # on, not just the version CI happens to resolve. That is the whole point: - # the floor is the exposure, and the audit gate in security.yml scans it - # explicitly. pydantic's floors are the exception: they are kept by hand - # (see `ignore` below). + # the floor is the exposure, and the dependency audit (test.yml on every PR, + # security.yml weekly) scans it explicitly. pydantic's floors are the + # exception: they are kept by hand (see `ignore` below). - package-ecosystem: "uv" directory: "/" schedule: @@ -55,8 +55,8 @@ updates: # the old requirements.txt); # - a major bump would drop pydantic 1 support. # An ignore with no update-types also stops Dependabot security updates - # for pydantic. The audit gate in security.yml scans the newest pydantic - # and its pydantic 1 and pydantic 2 floors (resolved for Python 3.10), and + # for pydantic. The dependency audit scans the newest pydantic and its + # pydantic 1 and pydantic 2 floors (resolved for Python 3.10), and # fails on a fixable advisory. The pydantic versions in uv.lock move with # `uv lock --upgrade-package pydantic`. # diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d1119ea8..71c89cac 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -131,14 +131,17 @@ See [skills/tests/README.md](skills/tests/README.md). ### The CI scripts' tests `.github/scripts` holds the dependency audit's report formatter, the schema drift check and -the API coverage report, with their tests. They need only pytest and the standard library, -and run with their own pytest config, which turns every warning into an error. The command -is the one the `Audit Script Tests` job runs: +the API coverage report, with their tests, and the tests of the `CI` job and of the job-list +check (see [CI](#ci)). They need only pytest and the standard library, and run with their own +pytest config, which turns every warning into an error. `test_ci_checks.py` also runs the +bash of those two steps, read from `test.yml`, so it needs bash, jq and +[yq](https://github.com/mikefarah/yq) v4 on `PATH`, as GitHub's runners have them. The +command is the one the `Audit Script Tests` job runs: ```sh uv run --only-dev pytest -c .github/scripts/pytest.ini \ .github/scripts/test_format_audit.py .github/scripts/test_check_schema_drift.py \ - .github/scripts/test_api_coverage.py + .github/scripts/test_api_coverage.py .github/scripts/test_ci_checks.py ``` ### End-to-end tests @@ -146,22 +149,22 @@ uv run --only-dev pytest -c .github/scripts/pytest.ini \ The tests marked `e2e` talk to a real Permit environment through a running PDP. `uv run pytest` with no arguments runs the whole suite (`testpaths` is `tests/`). -CI (`.github/workflows/test.yml`) runs the e2e tests in four jobs. Each job creates its own +`.github/workflows/test.yml` runs the e2e tests in four jobs. Each job creates its own scratch environment in the CI project and deletes it when the job ends, whether the tests passed or not: -- `pytest (Pydantic pydantic<2.0.0)` and `pytest (Pydantic pydantic>=2.0.0)`, the required - checks, run the whole suite against a PDP container. Its image is `PINNED_PDP_IMAGE` at +- `pytest (Pydantic pydantic<2.0.0)` and `pytest (Pydantic pydantic>=2.0.0)`, which the `CI` + job needs, run the whole suite against a PDP container. Its image is `PINNED_PDP_IMAGE` at the top of the workflow: `permitio/pdp-v2` pinned by version and digest, so a new PDP - release cannot fail a required check. -- `e2e (latest PDP image)` is not a required check. Once both `pytest` jobs pass, it runs - the whole suite on pydantic 2 against `permitio/pdp-v2:latest` and logs the digest - `:latest` resolved to. If it fails while `pytest` passes, the newest PDP release behaves - differently from the pinned one. -- `e2e (cloud PDP)` is not a required check. Once both `pytest` jobs pass, it runs - `tests/test_cloud_pdp_e2e.py` against the hosted cloud PDP, - `https://cloudpdp.api.permit.io`, with no container. Its tests create a small RBAC policy - in the scratch environment, wait for the cloud PDP to apply it, and check the exact + release cannot fail `CI`. +- `e2e (latest PDP image)` does not block a pull request: `CI` does not need it. Once both + `pytest` jobs pass, it runs the whole suite on pydantic 2 against `permitio/pdp-v2:latest` + and logs the digest `:latest` resolved to. If it fails while `pytest` passes, the newest + PDP release behaves differently from the pinned one. +- `e2e (cloud PDP)`, the other leg of that job, does not block a pull request either. Once + both `pytest` jobs pass, it runs `tests/test_cloud_pdp_e2e.py` against the hosted cloud + PDP, `https://cloudpdp.api.permit.io`, with no container. Its tests create a small RBAC + policy in the scratch environment, wait for the cloud PDP to apply it, and check the exact answers of `check`, `bulk_check`, `get_user_permissions` and `filter_objects`. Three more check that `get_user_tenants`, `permit.pdp_api` and the facts methods with `proxy_facts_via_pdp` on, whose routes the cloud PDP does not serve, raise the SDK's error @@ -186,7 +189,7 @@ The jobs set: it (see "API coverage report"). Without `API_TIER=prod` (or an explicit `PDP_CONTROL_PLANE`), `tests/conftest.py` sends API -calls to `http://localhost:8000`. To reproduce the required jobs locally with an +calls to `http://localhost:8000`. To reproduce the `pytest` jobs locally with an environment-level API key, on the PDP image they pin: ```sh @@ -225,6 +228,37 @@ Then refresh the PDP spec snapshot the API coverage report reads, from a contain new image (see "API coverage report" below). Until then, the `Audit Script Tests` job fails: a test there checks that `.github/api-specs/pdp.source.json` names the pinned image. +## CI + +`.github/workflows/test.yml` holds every check a pull request must pass, and runs on every +pull request and every push to `main`. Its last job, `CI`, is the one check to require: it +needs every other job in the workflow and fails unless each of them succeeded. A job that +failed, was cancelled or was skipped fails it, because GitHub counts a skipped required check +as passing. The one exception is `Dependency Review`, which runs on pull requests only: on a +push it is skipped, and `CI` passes. `Post Audit Comment` runs on every event, posts only on +a pull request from a branch of this repository, and elsewhere succeeds with its steps +skipped. + +To add a job to `test.yml`, do one of these in the same change: + +- add its id to the `needs` of the `ci` job, and set `EXPECTED_JOBS` in that job's step to + the new number of jobs in `needs`; +- or, if it must not block a pull request, add its id to `ADVISORY_JOBS` in the `Check that + CI needs every job` step of the `Workflow Hardening` job, with a comment saying why. + `e2e-unpinned-pdp` (`e2e (latest PDP image)` and `e2e (cloud PDP)`) is the only one. + +That step fails `Workflow Hardening` when a job is in neither list, when an `ADVISORY_JOBS` +entry is not a job, is listed twice or is also in `needs`, or when `EXPECTED_JOBS` is not +the number of jobs in `needs`. `CI` itself exits 2 when the number of job results it gets is +not `EXPECTED_JOBS`. When you delete a job, remove its id from `needs` and lower +`EXPECTED_JOBS`, or remove it from `ADVISORY_JOBS`. + +`.github/workflows/security.yml` is the weekly dependency audit. Every Monday at 09:00 UTC, +and when started with Run workflow, it runs the same audit as the `Dependency Audit` job +(`.github/actions/dependency-audit`) and posts the result to Slack. It gates no pull +request. Neither do the schema drift check (`schema-drift.yml`) and the weekly API coverage +run (`api-coverage.yml`). + ## Regenerating the sync stubs The blocking client, `permit.sync.Permit`, wraps the async classes at runtime, which type diff --git a/skills/tests/README.md b/skills/tests/README.md index 470f6d4b..f3b07a9e 100644 --- a/skills/tests/README.md +++ b/skills/tests/README.md @@ -38,7 +38,7 @@ repository's dependencies: - GitHub's dependency graph, Dependency Review, Dependabot and Snyk find manifests by file name, so they skip these files. -- The Trivy step in `.github/workflows/security.yml` and `python-sdk-publish.yml` also skips - `skills/tests/fixtures`. +- The Trivy step of the dependency audit (`.github/actions/dependency-audit/action.yml`) and + of `python-sdk-publish.yml` also skips `skills/tests/fixtures`. - `test_no_fixture_file_has_a_name_github_reads_as_a_dependency_manifest` fails if a fixture is stored under a manifest name again. From ca0001ca9a731e482594efb0f08144fda53c30c1 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Sat, 3 Oct 2026 19:54:15 +0300 Subject: [PATCH 08/16] Skip the audit comment when the PR head has moved on test.yml has no concurrency group, so a newer push no longer cancels the run of the older commit, and that run can finish last. Its comment step now reads the PR's current head and posts only when it is the commit the run audited, so the audit comment never goes back to an older commit's report. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/test.yml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 604757c5..31a5ab29 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -930,6 +930,23 @@ jobs: ].join('\n'); } + // Nothing cancels the run of a commit the PR has moved past, so it + // can finish after the run of the new head. Only the run of the PR's + // current head posts, so the comment never shows an older report. + const { data: pr } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.issue.number, + }); + const auditedSha = context.payload.pull_request.head.sha; + if (pr.head.sha !== auditedSha) { + core.notice( + `This run audited ${auditedSha}, but the PR head is now ${pr.head.sha}. ` + + 'The run of the new head posts its report.' + ); + return; + } + const comments = await github.paginate(github.rest.issues.listComments, { owner: context.repo.owner, repo: context.repo.repo, From ac96a17acade340a71fc40ac5fe3e33551fe3a4a Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Sat, 3 Oct 2026 19:54:31 +0300 Subject: [PATCH 09/16] Drop the audit's unread gate_failed output Nothing reads the audit jobs' gate_failed output, in test.yml or in security.yml, nor the composite action's failed output that feeds it: the comment and Slack jobs read the dependency-audit artifact and the audit job's result. The gate step still fails the job on a fixable HIGH or CRITICAL advisory. Co-Authored-By: Claude Opus 5.5 --- .github/actions/dependency-audit/action.yml | 10 ---------- .github/workflows/security.yml | 3 --- .github/workflows/test.yml | 3 --- 3 files changed, 16 deletions(-) diff --git a/.github/actions/dependency-audit/action.yml b/.github/actions/dependency-audit/action.yml index 77c2bf19..28b18940 100644 --- a/.github/actions/dependency-audit/action.yml +++ b/.github/actions/dependency-audit/action.yml @@ -12,13 +12,6 @@ inputs: description: The Python that runs the report formatter. required: true -outputs: - failed: - description: >- - 'true' when the gate found a fixable HIGH or CRITICAL advisory, 'false' - when it passed, empty when the gate did not run. - value: ${{ steps.gate.outputs.failed }} - runs: using: composite steps: @@ -116,7 +109,6 @@ runs: # Blocks on fixable HIGH/CRITICAL only, and fails closed if a gating # scanner report could not be parsed. - name: Gate on HIGH/CRITICAL - id: gate shell: bash run: | set -uo pipefail @@ -134,11 +126,9 @@ runs: gate_exit=$? set -e if [ "${gate_exit}" -ne 0 ]; then - echo "failed=true" >> "$GITHUB_OUTPUT" echo "::error title=Dependency audit failed::Fixable HIGH/CRITICAL advisories are present. See the job summary for the full report and the required version bumps." exit 1 fi - echo "failed=false" >> "$GITHUB_OUTPUT" # if: always() is load-bearing: the Gate step above exits non-zero on a # failing audit, and that is precisely when the jobs that read this diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 883fe87e..714555fb 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -31,8 +31,6 @@ jobs: # setup.py on the runner (see the audit job in test.yml). permissions: contents: read - outputs: - gate_failed: ${{ steps.audit.outputs.failed }} steps: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -44,7 +42,6 @@ jobs: # self-repository syntax ($/...) instead of ./, which actionlint (v1.7.12) # rejects. - name: Dependency audit - id: audit uses: ./.github/actions/dependency-audit # zizmor: ignore[self-repository] with: python-version: ${{ env.PYTHON_VERSION }} diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 31a5ab29..1295ce39 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -828,8 +828,6 @@ jobs: # this PR's dependency code. permissions: contents: read - outputs: - gate_failed: ${{ steps.audit.outputs.failed }} steps: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -841,7 +839,6 @@ jobs: # self-repository syntax ($/...) instead of ./, which actionlint (v1.7.12) # rejects. - name: Dependency audit - id: audit uses: ./.github/actions/dependency-audit # zizmor: ignore[self-repository] with: python-version: ${{ env.PYTHON_VERSION }} From 5b6af2a9eacd30e94343573ec3b8664d6602616b Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Sat, 3 Oct 2026 19:56:53 +0300 Subject: [PATCH 10/16] Shellcheck the composite action's bash in Workflow Hardening actionlint shellchecks the run blocks of workflows only, so moving the audit's bash into .github/actions/dependency-audit took it out of CI's shellcheck. A new Workflow Hardening step shellchecks every bash step of the local actions with actionlint's options: expressions replaced by a placeholder and the checks it turns off left off. It exits 1 on a finding and 2 when it reads no bash step. test_ci_checks.py runs the step against the committed actions and against planted ones: a finding, an expression, a variable set by env:, no action and no bash step. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/test_ci_checks.py | 98 +++++++++++++++++++++++++++++-- .github/workflows/test.yml | 48 ++++++++++++++- CONTRIBUTING.md | 16 +++-- 3 files changed, 149 insertions(+), 13 deletions(-) diff --git a/.github/scripts/test_ci_checks.py b/.github/scripts/test_ci_checks.py index 8c373184..a13d5bba 100644 --- a/.github/scripts/test_ci_checks.py +++ b/.github/scripts/test_ci_checks.py @@ -1,9 +1,10 @@ -"""Tests for the CI job and the job-list check in .github/workflows/test.yml. +"""Tests for the CI job and two Workflow Hardening checks in .github/workflows/test.yml. -Both are bash in a workflow `run:` block. These tests read each block and its -`env:` from test.yml with yq, and run it the way GitHub runs a `shell: bash` -step, against planted job results and planted workflows. They need bash, jq and -yq (mikefarah v4) on PATH, as GitHub's ubuntu-24.04 runners have them. +The CI job, the job-list check and the local actions' shellcheck are bash in a +workflow `run:` block. These tests read each block and its `env:` from test.yml +with yq, and run it the way GitHub runs a `shell: bash` step, against planted +job results, planted workflows and planted actions. They need bash, jq, yq +(mikefarah v4) and shellcheck on PATH, as GitHub's ubuntu-24.04 runners have them. Run with: uv run --only-dev pytest -c .github/scripts/pytest.ini .github/scripts/test_ci_checks.py @@ -25,6 +26,7 @@ WORKFLOW = REPO_ROOT / ".github" / "workflows" / "test.yml" CI_STEP = ("ci", "Check the needed jobs") NEEDS_CHECK_STEP = ("workflow-hardening", "Check that CI needs every job") +SHELLCHECK_STEP = ("workflow-hardening", "Shellcheck the local actions") ADVISORY_JOB = "e2e-unpinned-pdp" @@ -386,3 +388,89 @@ def test_needs_check_exits_2_when_no_job_is_read( completed = run_needs_check(workflow, tmp_path, planted) assert completed.returncode == 2 assert "No jobs read from" in completed.stdout + + +# --- the local actions' shellcheck in Workflow Hardening ---------------------- + + +def bash_step(name: str, script: str) -> dict[str, str]: + return {"name": name, "shell": "bash", "run": script} + + +def run_shellcheck_step( + workflow: dict[str, Any], tmp_path: Path, actions: dict[str, list[dict[str, str]]] +) -> subprocess.CompletedProcess[str]: + """Runs the step against planted actions, each a list of composite steps (JSON is YAML).""" + actions_dir = tmp_path / "actions" + actions_dir.mkdir() + for name, steps in actions.items(): + (actions_dir / name).mkdir() + action = {"name": name, "runs": {"using": "composite", "steps": steps}} + (actions_dir / name / "action.yml").write_text(json.dumps(action), encoding="utf-8") + tool("shellcheck") + return run_step( + find_step(workflow, SHELLCHECK_STEP), tmp_path, {"ACTIONS_DIR": str(actions_dir)} + ) + + +def test_shellcheck_step_passes_on_the_committed_actions( + workflow: dict[str, Any], tmp_path: Path +) -> None: + tool("shellcheck") + completed = run_step(find_step(workflow, SHELLCHECK_STEP), tmp_path, {}) + assert completed.returncode == 0, completed.stdout + completed.stderr + assert "Shellcheck found nothing" in completed.stdout + + +def test_shellcheck_step_fails_on_a_finding_and_names_its_step( + workflow: dict[str, Any], tmp_path: Path +) -> None: + actions = { + "clean": [bash_step("Clean", 'echo "clean"')], + "mixed": [ + {"name": "Checkout", "uses": "actions/checkout@v7"}, + bash_step("Quoted", 'echo "$HOME"'), + bash_step("Unquoted", "echo $HOME"), + ], + } + completed = run_shellcheck_step(workflow, tmp_path, actions) + assert completed.returncode == 1 + assert "SC2086" in completed.stdout + assert 'Step "Unquoted" has the shellcheck findings above' in completed.stdout + assert 'Step "Quoted"' not in completed.stdout + assert 'Step "Clean"' not in completed.stdout + + +def test_shellcheck_step_reads_expressions_as_placeholders( + workflow: dict[str, Any], tmp_path: Path +) -> None: + actions = {"expressions": [bash_step("Expression", 'echo "${{ inputs.python-version }}"')]} + completed = run_shellcheck_step(workflow, tmp_path, actions) + assert completed.returncode == 0, completed.stdout + completed.stderr + + +def test_shellcheck_step_leaves_off_the_checks_actionlint_turns_off( + workflow: dict[str, Any], tmp_path: Path +) -> None: + actions = {"env": [bash_step("Variable from env", 'echo "$set_by_env"')]} + completed = run_shellcheck_step(workflow, tmp_path, actions) + assert completed.returncode == 0, completed.stdout + completed.stderr + + +@pytest.mark.parametrize( + ("actions", "error"), + [ + ({}, "Could not read"), + ({"uses-only": [{"name": "Checkout", "uses": "actions/checkout@v7"}]}, "No bash step read"), + ], + ids=["no action", "no bash step"], +) +def test_shellcheck_step_exits_2_when_no_bash_step_is_read( + workflow: dict[str, Any], + tmp_path: Path, + actions: dict[str, list[dict[str, str]]], + error: str, +) -> None: + completed = run_shellcheck_step(workflow, tmp_path, actions) + assert completed.returncode == 2 + assert f"title=Shellcheck::{error}" in completed.stdout diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1295ce39..1936b930 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1018,8 +1018,9 @@ jobs: # [tool.pytest] in pyproject.toml. The scripts under test are stdlib only, # so --only-dev leaves the project uninstalled. The schema drift check's and # the API coverage report's tests run here too: they live next to the audit - # scripts and need no more. So do the tests of the CI job's and the job-list - # check's bash, which also run bash, jq and yq from the runner image. + # scripts and need no more. So do the tests of the bash of the CI job, the + # job-list check and the local actions' shellcheck, which also run bash, + # jq, yq and shellcheck from the runner image. - name: Run CI script tests run: >- uv run --locked --only-dev @@ -1054,6 +1055,49 @@ jobs: advanced-security: false persona: regular + # actionlint shellchecks the `run:` blocks of workflows, not those of the + # local actions under .github/actions. This step shellchecks each of their + # `shell: bash` steps as actionlint would: every ${{ }} expression replaced + # by a placeholder, and the checks actionlint turns off left off (SC2154, + # for one, since `env:` sets variables shellcheck cannot see). Exits 1 on a + # finding, and 2 when no step is read. + - name: Shellcheck the local actions + shell: bash + env: + ACTIONS_DIR: .github/actions + run: | + status=0 + checked=0 + for action in "$ACTIONS_DIR"/*/action.yml; do + if ! count=$(yq '.runs.steps | length' "$action"); then + echo "::error file=$action,title=Shellcheck::Could not read $action." + exit 2 + fi + for ((i = 0; i < count; i++)); do + if [[ $(yq ".runs.steps[$i].shell" "$action") != bash ]]; then + continue + fi + checked=$((checked + 1)) + name=$(yq ".runs.steps[$i].name // \"step $((i + 1))\"" "$action") + if ! yq ".runs.steps[$i].run" "$action" | + sed -E 's/\$\{\{[^}]*\}\}/_/g' | + shellcheck --norc --shell=bash \ + --exclude=SC1091,SC2043,SC2050,SC2153,SC2154,SC2157,SC2194 -; then + echo "::error file=$action,title=Shellcheck::Step \"$name\" has the" \ + "shellcheck findings above." + status=1 + fi + done + done + if [[ $checked -eq 0 ]]; then + echo "::error title=Shellcheck::No bash step read from $ACTIONS_DIR/*/action.yml." + exit 2 + fi + if [[ $status -eq 0 ]]; then + echo "Shellcheck found nothing in the $checked bash steps of $ACTIONS_DIR." + fi + exit "$status" + # CI gates only the jobs in its needs, so every other job in this workflow # must be listed there or in ADVISORY_JOBS, the jobs that run without # gating a PR. Exits 1 when the jobs, CI's needs and ADVISORY_JOBS disagree diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 71c89cac..c805e178 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -131,12 +131,12 @@ See [skills/tests/README.md](skills/tests/README.md). ### The CI scripts' tests `.github/scripts` holds the dependency audit's report formatter, the schema drift check and -the API coverage report, with their tests, and the tests of the `CI` job and of the job-list -check (see [CI](#ci)). They need only pytest and the standard library, and run with their own -pytest config, which turns every warning into an error. `test_ci_checks.py` also runs the -bash of those two steps, read from `test.yml`, so it needs bash, jq and -[yq](https://github.com/mikefarah/yq) v4 on `PATH`, as GitHub's runners have them. The -command is the one the `Audit Script Tests` job runs: +the API coverage report, with their tests, and the tests of the `CI` job, the job-list check +and the local actions' shellcheck (see [CI](#ci)). They need only pytest and the standard +library, and run with their own pytest config, which turns every warning into an error. +`test_ci_checks.py` also runs the bash of those three steps, read from `test.yml`, so it +needs bash, jq, [yq](https://github.com/mikefarah/yq) v4 and shellcheck on `PATH`, as +GitHub's runners have them. The command is the one the `Audit Script Tests` job runs: ```sh uv run --only-dev pytest -c .github/scripts/pytest.ini \ @@ -259,6 +259,10 @@ and when started with Run workflow, it runs the same audit as the `Dependency Au request. Neither do the schema drift check (`schema-drift.yml`) and the weekly API coverage run (`api-coverage.yml`). +actionlint shellchecks the bash in workflows but not in the local actions under +`.github/actions`, so the `Shellcheck the local actions` step of `Workflow Hardening` does +that, with the options actionlint uses. + ## Regenerating the sync stubs The blocking client, `permit.sync.Permit`, wraps the async classes at runtime, which type From b5c93f30718cc43d98622279103ac2b69effe556 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Sat, 3 Oct 2026 19:57:01 +0300 Subject: [PATCH 11/16] Say that CI does not need the advisory jobs CONTRIBUTING.md's CI section said CI needs every other job in test.yml, which left out e2e-unpinned-pdp, the one job in ADVISORY_JOBS. Co-Authored-By: Claude Opus 5.5 --- CONTRIBUTING.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c805e178..fb2efc15 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -232,12 +232,12 @@ a test there checks that `.github/api-specs/pdp.source.json` names the pinned im `.github/workflows/test.yml` holds every check a pull request must pass, and runs on every pull request and every push to `main`. Its last job, `CI`, is the one check to require: it -needs every other job in the workflow and fails unless each of them succeeded. A job that -failed, was cancelled or was skipped fails it, because GitHub counts a skipped required check -as passing. The one exception is `Dependency Review`, which runs on pull requests only: on a -push it is skipped, and `CI` passes. `Post Audit Comment` runs on every event, posts only on -a pull request from a branch of this repository, and elsewhere succeeds with its steps -skipped. +needs every other job in the workflow but the advisory ones (see below), and fails unless +each of them succeeded. A job that failed, was cancelled or was skipped fails it, because +GitHub counts a skipped required check as passing. The one exception is `Dependency Review`, +which runs on pull requests only: on a push it is skipped, and `CI` passes. +`Post Audit Comment` runs on every event, posts only on a pull request from a branch of this +repository, and elsewhere succeeds with its steps skipped. To add a job to `test.yml`, do one of these in the same change: From c39101bfa453f55007703c41718590cfad7d7edf Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Sat, 3 Oct 2026 19:57:26 +0300 Subject: [PATCH 12/16] Note the check names the ruleset requires until it requires CI Moving the jobs dropped the comments that said the pytest lanes' and pre-commit's names are required checks. Until the ruleset on main requires CI alone, it still requires six check names, so CI's leading comment and CONTRIBUTING.md's CI section list them and say not to rename those jobs until then. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/test.yml | 5 +++++ CONTRIBUTING.md | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1936b930..ff6becbb 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1173,6 +1173,11 @@ jobs: # EXPECTED_JOBS jobs arrive. Change EXPECTED_JOBS when you change needs. The # Workflow Hardening job fails when a job is in neither needs nor its # ADVISORY_JOBS, or when EXPECTED_JOBS is not the number of jobs in needs. + # Until the ruleset on main requires CI alone, it requires the check names of + # six jobs here, so do not rename those jobs until then (a required check that + # never reports leaves every pull request waiting): + # "pytest (Pydantic pydantic<2.0.0)", "pytest (Pydantic pydantic>=2.0.0)", + # "pre-commit", "Dependency Audit", "Audit Script Tests", "Workflow Hardening". ci: name: CI if: always() diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fb2efc15..83cc9287 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -239,6 +239,12 @@ which runs on pull requests only: on a push it is skipped, and `CI` passes. `Post Audit Comment` runs on every event, posts only on a pull request from a branch of this repository, and elsewhere succeeds with its steps skipped. +Until the `main` ruleset requires `CI` alone, it requires the check names of six jobs: +`pytest (Pydantic pydantic<2.0.0)`, `pytest (Pydantic pydantic>=2.0.0)`, `pre-commit`, +`Dependency Audit`, `Audit Script Tests` and `Workflow Hardening`. Do not rename those jobs +until then: GitHub leaves a required check that never reports pending, which blocks every +pull request. + To add a job to `test.yml`, do one of these in the same change: - add its id to the `needs` of the `ci` job, and set `EXPECTED_JOBS` in that job's step to From 4d8837870728247b26bc8d70e8b6d5d6b11491b4 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Sat, 3 Oct 2026 19:57:38 +0300 Subject: [PATCH 13/16] Describe every CI script test in the pytest.ini header The header named the audit formatter's and the schema drift check's tests only, and said they need nothing but pytest and the standard library. It now covers every test file in .github/scripts and says that test_ci_checks.py also runs bash, jq, yq and shellcheck. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/pytest.ini | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/scripts/pytest.ini b/.github/scripts/pytest.ini index 6f9dbdab..13ef6052 100644 --- a/.github/scripts/pytest.ini +++ b/.github/scripts/pytest.ini @@ -1,8 +1,9 @@ -# Configuration for the CI script tests alone (format_audit.py and -# check_schema_drift.py), passed with -c so pytest does not use the SDK's +# Configuration for the CI script tests alone (the test_*.py files in +# .github/scripts), passed with -c so pytest does not use the SDK's # configuration in pyproject.toml ([tool.pytest]), whose testpaths and # asyncio_mode belong to the SDK's suite. These tests need only pytest and the -# standard library, and warn about nothing: any warning is an error. +# standard library, though test_ci_checks.py also runs bash, jq, yq (mikefarah +# v4) and shellcheck. They warn about nothing: any warning is an error. [pytest] # strict_config, strict_markers, strict_xfail and strict_parametrization_ids. strict = true From bdb639ce13712e1bac190fe75f90e41a93bc9db1 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Sat, 3 Oct 2026 20:05:04 +0300 Subject: [PATCH 14/16] Point the publish Trivy comment at the dependency-audit action Co-Authored-By: Claude Opus 5.5 --- .github/workflows/python-sdk-publish.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python-sdk-publish.yml b/.github/workflows/python-sdk-publish.yml index bc85f6ab..08943f97 100644 --- a/.github/workflows/python-sdk-publish.yml +++ b/.github/workflows/python-sdk-publish.yml @@ -175,7 +175,7 @@ jobs: exit-code: "0" scanners: vuln trivy-config: "" - # See the same step in security.yml: this only installs Trivy, and + # See the same step in .github/actions/dependency-audit: this only installs Trivy, and # hide-progress keeps its empty scan from logging a warning. hide-progress: true # The action's cache is on by default and restores the Trivy binary From 6ca2789b1c296b55d9b4e237ec866d08f3aa401f Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Sat, 3 Oct 2026 20:18:48 +0300 Subject: [PATCH 15/16] Skip Migration Skill Tests to check that CI fails (temporary) Reverted in the next commit. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ff6becbb..9d1a7c55 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -713,6 +713,7 @@ jobs: # the SDK's suite, with skills/tests/pytest.ini. migration-skill: name: Migration Skill Tests (pydantic ${{ matrix.pydantic }}) + if: github.event_name == 'never' runs-on: ubuntu-24.04 timeout-minutes: 15 permissions: From 559241ac3c233ae74611ec0eefe149c018ef29f4 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Sat, 3 Oct 2026 20:34:10 +0300 Subject: [PATCH 16/16] Revert the temporary Migration Skill Tests skip CI failed on it as intended: "Jobs that did not succeed: migration-skill skipped", exit 1. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/test.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9d1a7c55..ff6becbb 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -713,7 +713,6 @@ jobs: # the SDK's suite, with skills/tests/pytest.ini. migration-skill: name: Migration Skill Tests (pydantic ${{ matrix.pydantic }}) - if: github.event_name == 'never' runs-on: ubuntu-24.04 timeout-minutes: 15 permissions: