diff --git a/.github/actions/dependency-audit/action.yml b/.github/actions/dependency-audit/action.yml new file mode 100644 index 00000000..28b18940 --- /dev/null +++ b/.github/actions/dependency-audit/action.yml @@ -0,0 +1,143 @@ +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 + +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 + 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 "::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 + + # 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..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`. # @@ -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/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 diff --git a/.github/scripts/test_ci_checks.py b/.github/scripts/test_ci_checks.py new file mode 100644 index 00000000..a13d5bba --- /dev/null +++ b/.github/scripts/test_ci_checks.py @@ -0,0 +1,476 @@ +"""Tests for the CI job and two Workflow Hardening checks in .github/workflows/test.yml. + +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 +""" + +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") +SHELLCHECK_STEP = ("workflow-hardening", "Shellcheck the local actions") +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 + + +# --- 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/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/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 diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 174ed1a7..714555fb 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,335 +26,26 @@ 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: - gate_failed: ${{ steps.gate.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 + # .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 + uses: ./.github/actions/dependency-audit # zizmor: ignore[self-repository] 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 - 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 - with: - name: dependency-audit - path: /tmp/audit/ - retention-days: 30 - - # 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. @@ -416,7 +89,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..ff6becbb 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1,24 +1,32 @@ 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 - # 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 @@ -49,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 @@ -256,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 (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. @@ -504,7 +507,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 @@ -556,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 @@ -708,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 @@ -766,3 +768,456 @@ 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 + + # 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 + 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 + 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. 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 + # 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 + if: env.SAME_REPO_PR == 'true' + 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: 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 + # 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'); + } + + // 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, + 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. 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 + 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_ci_checks.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 + + # 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 + # 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 + # 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. 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() + 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 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d1119ea8..83cc9287 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, 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 \ .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,47 @@ 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 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. + +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 + 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`). + +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 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.