From 7c004a95f8012228ff6ce6cfa12a67c2d3fd4507 Mon Sep 17 00:00:00 2001 From: Lucas Koontz Date: Sat, 3 Oct 2026 22:15:51 -0700 Subject: [PATCH] feat(ci): build the scratchpad image on a GitHub-hosted runner with OIDC The scratchpad image build runs on ubuntu-latest and pushes with a short-lived AWS role it assumes through GitHub OIDC. GitHub recommends GitHub-hosted runners for public repositories. - Pull requests name no environment, assume gha-anton-ecr-dev and push to minds-anton-scratchpad-dev. - Staging builds name the staging environment, assume gha-anton-ecr-dev and push to minds-anton-scratchpad-dev. - Main builds name the prod environment, assume gha-anton-ecr-prod and push to minds-anton-scratchpad. The gate job still skips fork pull requests. Its new target step picks the environment, role and repository for each way in, and fails on any other caller input. build-push-ecr runs with builder: local, so the image builds on the runner itself. release.yml and publish-staging.yml grant id-token: write on the calling job only. Third-party actions in the build workflow are pinned by commit. The gate tests run the target step for each way in. They also pin the build job's wiring: the hosted runner, its own id-token grant, the role step before the push, and each caller's grant. A new sweep requires the gate on every job that can mint an OIDC token in a workflow an outside account can start, and the runner sweep now covers the same workflows. The actionlint config that declared the mdb-dev and mdb-prod labels is gone, so actionlint flags any new use of them. The README and the release docs page say where each build lands. Work by Lucas Koontz for "Take the public repos off the credentialed runner group and require devops to release a privileged run". Refs: ENG-2000 --- .github/actionlint.yaml | 8 - .github/workflows/publish-staging.yml | 3 +- .github/workflows/release.yml | 1 + .github/workflows/scratchpad-dev-build.yml | 120 ++++-- README.md | 15 + docs/docs/developer/release-and-versioning.md | 4 +- tests/test_fork_build_gate.py | 358 +++++++++++++++--- 7 files changed, 422 insertions(+), 87 deletions(-) delete mode 100644 .github/actionlint.yaml diff --git a/.github/actionlint.yaml b/.github/actionlint.yaml deleted file mode 100644 index a2d0b7139..000000000 --- a/.github/actionlint.yaml +++ /dev/null @@ -1,8 +0,0 @@ -# actionlint needs to be told about self-hosted runner labels, or every -# `runs-on:` referring to one is an error and the real findings drown in noise. -# -# mdb-dev, mdb-prod the org's self-hosted runner pools -self-hosted-runner: - labels: - - mdb-dev - - mdb-prod diff --git a/.github/workflows/publish-staging.yml b/.github/workflows/publish-staging.yml index 425f9eba7..ad2e8fea8 100644 --- a/.github/workflows/publish-staging.yml +++ b/.github/workflows/publish-staging.yml @@ -39,11 +39,12 @@ jobs: runs-on: ubuntu-latest # The worker image staging scratchpad-controller and the PR environments pull - # as minds-anton-scratchpad:staging (ENG-2129). + # as minds-anton-scratchpad-dev:staging. scratchpad-image: needs: release permissions: contents: read + id-token: write # the build job assumes its ECR writer role through GitHub OIDC uses: ./.github/workflows/scratchpad-dev-build.yml with: environment: staging diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 830f5e1b6..c24850317 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -60,6 +60,7 @@ jobs: needs: [auto-release, verify-wheel] permissions: contents: read + id-token: write # the build job assumes its ECR writer role through GitHub OIDC uses: ./.github/workflows/scratchpad-dev-build.yml with: environment: production diff --git a/.github/workflows/scratchpad-dev-build.yml b/.github/workflows/scratchpad-dev-build.yml index 39a82b1a5..3c57dc5b0 100644 --- a/.github/workflows/scratchpad-dev-build.yml +++ b/.github/workflows/scratchpad-dev-build.yml @@ -1,6 +1,6 @@ name: Scratchpad image build -# Builds and pushes the minds-anton-scratchpad image (the sandbox pod image) to ECR. +# Builds and pushes the anton scratchpad image (the sandbox pod image) to ECR. # Two ways in: # # - Every pull request whose branch lives in this repository builds for the @@ -9,13 +9,19 @@ name: Scratchpad image build # - release.yml and publish-staging.yml call this workflow after they mint a # tag, building for `production` and `staging` respectively (ENG-2129). # -# The shared build-push-ecr action tags every build three ways: -# `-`, `-head-` (pull requests only), and the -# moving `` tag. scratchpad-controller points each environment's -# SCRATCHPAD_CONTROLLER__SCRATCHPAD_IMAGE at the moving tag and pulls with -# imagePullPolicy Always, so a release reaches worker pods without anyone -# editing a sha into a values file. There is no Helm chart for this image, so -# this workflow only builds (no deploy job). +# The build runs on a GitHub-hosted runner and pushes with a short-lived AWS +# role it assumes through GitHub OIDC. The gate job's `target` step picks the +# GitHub environment, the role and the ECR repository for each way in: pull +# requests and staging push to minds-anton-scratchpad-dev, and only a +# production build from main pushes to minds-anton-scratchpad. +# +# The shared build-push-ecr action tags every build `-`, +# `-head-` (pull requests only), the moving `` tag, and +# `latest`, all in that build's repository. scratchpad-controller points each +# environment's SCRATCHPAD_CONTROLLER__SCRATCHPAD_IMAGE at the moving tag and +# pulls with imagePullPolicy Always, so a release reaches worker pods without +# anyone editing a sha into a values file. There is no Helm chart for this +# image, so this workflow only builds (no deploy job). # # The image is smoke-tested inside the build, not here: the Dockerfile's last layer runs # docker/image_smoke.py as UID 1000, which executes both entrypoints the controller execs. @@ -45,15 +51,18 @@ defaults: jobs: - # Who may reach the self-hosted build runner: pull requests from branches in - # this repository, and calls from the release pipelines. Anything else is - # refused with a run-summary line saying why. + # Who may build and push the image: pull requests from branches in this + # repository, and calls from the release pipelines. Anything else is refused + # with a run-summary line saying why. gate: runs-on: ubuntu-latest outputs: run: ${{ steps.decide.outputs.run }} + environment: ${{ steps.target.outputs.environment }} + role: ${{ steps.target.outputs.role }} + repository: ${{ steps.target.outputs.repository }} steps: - - name: Decide whether this run may reach the runner + - name: Decide whether this run may build id: decide env: IS_PR: ${{ github.event_name == 'pull_request' }} @@ -79,22 +88,59 @@ jobs: { echo "### Build skipped: fork pull request" echo - echo "The scratchpad image build runs on a self-hosted runner inside our" - echo "cluster, so it does not execute code from a fork. Nothing is wrong" - echo "with this pull request." + echo "The scratchpad image build pushes to our container registry with" + echo "an AWS role, so it does not build code from a fork. Nothing is" + echo "wrong with this pull request." } >> "$GITHUB_STEP_SUMMARY" else echo "run=true" >> "$GITHUB_OUTPUT" fi + # One row per way in. Which role a job can assume follows from the + # environment it names, because the trust policies on the + # gha-anton-ecr-dev and gha-anton-ecr-prod IAM roles (mindsdb/terraform) + # match the OIDC subject and ref. A pull request names no environment, + # so its token carries the pull_request subject, which only the dev role + # trusts. The prod role trusts only the prod environment on main, and + # the prod environment admits only main. + - name: Pick the environment, role and repository for this build + id: target + env: + # Empty on every event except a workflow_call from a release pipeline. + CALLED_FOR: ${{ inputs.environment }} + run: | + set -euo pipefail + case "${CALLED_FOR:-}" in + "") + environment="" + role="arn:aws:iam::168681354662:role/gha-anton-ecr-dev" + repository="minds-anton-scratchpad-dev" + ;; + staging) + environment="staging" + role="arn:aws:iam::168681354662:role/gha-anton-ecr-dev" + repository="minds-anton-scratchpad-dev" + ;; + production) + environment="prod" + role="arn:aws:iam::168681354662:role/gha-anton-ecr-prod" + repository="minds-anton-scratchpad" + ;; + *) + echo "::error::no build target for environment '${CALLED_FOR}'" + exit 1 + ;; + esac + { + echo "environment=$environment" + echo "role=$role" + echo "repository=$repository" + } >> "$GITHUB_OUTPUT" + # The version the image will report, resolved from tags (ENG-1796). # - # Its own job, on ubuntu-latest, for two reasons. The build runs on the - # self-hosted mdb-dev runner and nothing in this repo has ever run setup-uv - # there, so resolving the version in the build job would put an unproven - # dependency in front of the image. And keeping it separate leaves the build's - # checkout shallow: .git never enters the docker context (see .dockerignore), - # so only this job needs the history. + # Its own job so the build's checkout stays shallow: .git never enters the + # docker context (see .dockerignore), so only this job needs the history. # # A release pipeline passes the version it just minted instead, for the same # reason publish-staging.yml pins SETUPTOOLS_SCM_PRETEND_VERSION: a re-run on @@ -105,13 +151,14 @@ jobs: outputs: version: ${{ steps.resolve.outputs.version }} steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 if: inputs.version == '' with: # hatch-vcs derives the version by describing against tags, and the # default fetch-depth of 1 fetches none of them. fetch-depth: 0 - - uses: astral-sh/setup-uv@v5 + persist-credentials: false + - uses: astral-sh/setup-uv@d4b2f3b6ecc6e67c4457f6d3e41ec42d3d0fcb86 # v5.4.2 if: inputs.version == '' with: python-version: "3.12" @@ -164,7 +211,16 @@ jobs: build: needs: [version, gate] if: needs.gate.outputs.run == 'true' - runs-on: mdb-dev + runs-on: ubuntu-latest + # Empty on a pull request, so the job names no environment. The gate's + # `target` step says why that decides which role the job can assume. + environment: ${{ needs.gate.outputs.environment }} + # Both callers grant id-token on the job that calls this workflow, because + # GitHub fails a caller's whole run before any job starts when a called + # job names a scope the caller did not grant. + permissions: + contents: read + id-token: write # mints the OIDC token the ECR writer role is assumed with # Job-level rather than workflow-level so it holds when this workflow is # called: a pull request cancels its own superseded build; a release build # queues behind the previous one for the same environment and is never @@ -179,13 +235,23 @@ jobs: AWS_REGION: us-east-1 steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + with: + persist-credentials: false - uses: mindsdb/github-actions/setup-env@main + - name: Assume the ECR writer role + uses: aws-actions/configure-aws-credentials@e1253824e5c10ff9df46874f81ed3ec929e19cfd # v6.3.0 + with: + role-to-assume: ${{ needs.gate.outputs.role }} + aws-region: ${{ env.AWS_REGION }} - id: build uses: mindsdb/github-actions/build-push-ecr@main with: - # Override the repo-slug default so the image lands at .../minds-anton-scratchpad. - module-name: minds-anton-scratchpad + # Build with buildx on this runner, using the credentials the step + # above configured, instead of the in-cluster remote builder. + builder: local + # The ECR repository name, which overrides the repo-slug default. + module-name: ${{ needs.gate.outputs.repository }} build-for-environment: ${{ inputs.environment || 'development' }} # Expanded unquoted by the shared action, so the value must not contain # whitespace — a PEP 440 version never does, and the shape check in the diff --git a/README.md b/README.md index 9f064eb51..bf14a0505 100644 --- a/README.md +++ b/README.md @@ -316,6 +316,7 @@ Anton uses an automated release flow with two publish streams — **stable** fro - Builds the wheel (hatch-vcs derives the version from that tag) and publishes it to PyPI. - Publishes a GitHub release with auto-generated notes. - Triggers [`tests_e2e_release.yml`](.github/workflows/tests_e2e_release.yml) to run live e2e tests against the released version. + - Builds the scratchpad pod image after those tests pass and pushes it to `minds-anton-scratchpad` as `production` (see [Scratchpad image](#scratchpad-image)). The version computation, tag push, and release creation are shared with the other service repos through the `calver-release.yml` reusable workflow in @@ -326,6 +327,20 @@ workflow supplies the major component and consumes the resulting `tag`/`version` Every push to `staging` publishes a **release candidate** through [`.github/workflows/publish-staging.yml`](.github/workflows/publish-staging.yml): it cuts a PEP 440 pre-release tag (`v2.YY.M.DD.SEQrcN`), publishes a GitHub pre-release, and uploads the wheel to PyPI — so cowork-server staging installs an immutable, versioned Anton instead of a mutable branch or hand-pinned commit (ENG-1159). These never reach production: resolvers ignore pre-releases unless a specifier names one, and PyPI's `info.version` (read by the prod desktop updater) excludes them. +### Scratchpad image + +[`.github/workflows/scratchpad-dev-build.yml`](.github/workflows/scratchpad-dev-build.yml) builds the scratchpad pod image and pushes it to ECR. It runs on a GitHub-hosted runner and gets short-lived AWS credentials through GitHub OIDC. + +| Run | GitHub environment | AWS role | ECR repository | +|---|---|---|---| +| Pull request from a branch in this repo | none | `gha-anton-ecr-dev` | `minds-anton-scratchpad-dev` | +| Push to `staging`, through `publish-staging.yml` | `staging` | `gha-anton-ecr-dev` | `minds-anton-scratchpad-dev` | +| Push to `main`, through `release.yml` | `prod` | `gha-anton-ecr-prod` | `minds-anton-scratchpad` | + +Each build tags the image `-` and moves the `` and `latest` tags in its repository. A pull request build also adds `development-head-`. Pull requests from forks skip the build, and the run summary says why. + +The `staging` environment accepts runs from the `staging` branch only. The `prod` environment accepts runs from `main` only, and `gha-anton-ecr-prod` trusts only `prod` runs on `main`. So in this repo only a `release.yml` run on `main` can push the production image, whether a push or a manual dispatch started it. + ### What you should NOT do - **Don't create GitHub releases manually.** The `v*` tag namespace is locked via a repo ruleset — only the release workflow can create them. Manual attempts will be rejected by GitHub. diff --git a/docs/docs/developer/release-and-versioning.md b/docs/docs/developer/release-and-versioning.md index 65d622df1..e65d89742 100644 --- a/docs/docs/developer/release-and-versioning.md +++ b/docs/docs/developer/release-and-versioning.md @@ -104,7 +104,9 @@ How to ship a stable version: publishes it to PyPI, - publishes a GitHub release with auto-generated notes, - triggers `tests_e2e_release.yml` to run live e2e tests against the - released version. + released version, + - builds the scratchpad pod image after those tests pass and pushes it to + `minds-anton-scratchpad` as `production`. ### Staging release candidates diff --git a/tests/test_fork_build_gate.py b/tests/test_fork_build_gate.py index 9649ba753..0fa3754cc 100644 --- a/tests/test_fork_build_gate.py +++ b/tests/test_fork_build_gate.py @@ -1,25 +1,24 @@ -"""The scratchpad image build must never run a fork's code on the cluster runner. - -``mdb-dev`` is a pod inside the newdev cluster, not a disposable VM. Whatever -runs there holds the runner's Kubernetes service-account token, its IRSA role -into the build account, and a filesystem other repositories' credentialed jobs -write into. A ``pull_request`` run builds the merge ref, which is the fork's -tree, and GitHub reads the workflow from that same tree -- so the fork chooses -the code and we choose only whether to start it. - -Nothing fails at runtime when the guard goes: the build succeeds, for someone it -should not have succeeded for. So these are build assertions rather than -behaviour tests, and none of them may be a substring check on the thing they -guard. A substring passes on an inverted comparison, which is the likeliest way -this decays. The shell is executed instead of read, and the job condition that -starts the runner is compared whole rather than searched. +"""The scratchpad image build must never push a fork's code to our registry. + +The build runs on a GitHub-hosted runner and pushes with an AWS role it assumes +through GitHub OIDC. A ``pull_request`` run builds the merge ref, which is the +fork's tree, and GitHub reads the workflow from that same tree, so the fork +chooses the code and we choose only whether to start it. GitHub withholds +``id-token: write`` from a fork's ``pull_request`` run, so a fork cannot assume +the role even without the gate. The gate turns that into a skip that says why, +and it still holds for any trigger added later that does hand a fork a token. + +These are build assertions rather than behaviour tests, and none of them may be +a substring check on the thing they guard. A substring passes on an inverted +comparison, which is the likeliest way this decays. The shell is executed +instead of read, and the job condition that starts the build is compared whole +rather than searched. One boundary is worth stating because no assertion here covers it. A job that delegates to a reusable workflow in another repository decides its runner label there, and nothing in this tree can see it. ``_reachable_labels`` returns ``None`` for those and the sweep skips them; today they are the shared -``mindsdb/github-actions`` callers. Pinning that surface is the runner-group -change, not this file. +``mindsdb/github-actions`` callers. """ from __future__ import annotations @@ -43,15 +42,19 @@ # has heard of must read as self-hosted, not as safe. _HOSTED_PREFIXES = ("ubuntu-", "windows-", "macos-") -# The exact condition a job on one of our runners has to carry. Compared whole, -# never searched for: `== 'false'` and `always() || ...` both contain this -# expression, and both hand a fork the runner. +# The exact condition a job on one of our runners, or a job that can mint an +# OIDC token, has to carry. Compared whole, never searched for: `== 'false'` and +# `always() || ...` both contain this expression, and both hand a fork the job. _GATE_CONDITION = "needs.gate.outputs.run == 'true'" -# The events that let an account outside this organisation start a run. A job on -# our runners in a workflow triggered only by `push` is a different question and -# belongs to the runner-group change. -_FORK_REACHABLE_TRIGGERS = ("pull_request", "pull_request_target") +# The events only an account with write access to this repository can cause. +# `workflow_call` counts too: a called workflow runs inside its caller's run, and +# the sweeps check the calling job. Both sweeps skip a workflow that only these +# events start. +_WRITE_ACCESS_TRIGGERS = frozenset({"push", "schedule", "workflow_dispatch", "workflow_call"}) + +_DEV_ROLE = "arn:aws:iam::168681354662:role/gha-anton-ecr-dev" +_PROD_ROLE = "arn:aws:iam::168681354662:role/gha-anton-ecr-prod" class _GateRun(NamedTuple): @@ -61,6 +64,36 @@ class _GateRun(NamedTuple): summary: str +class _Target(NamedTuple): + """Where one way into the workflow builds: the gate's ``target`` outputs.""" + + environment: str + role: str + repository: str + + +class _TargetRun(NamedTuple): + """What one execution of the gate's ``target`` shell did.""" + + returncode: int + outputs: dict[str, str] + + +# What each way in must resolve to. The empty key is a pull request, which +# carries no workflow_call input. The roles' trust policies expect exactly these +# pairs: the pull_request subject and the staging environment on the dev role, +# and only the prod environment on the prod role. +_TARGETS = { + "": _Target(environment="", role=_DEV_ROLE, repository="minds-anton-scratchpad-dev"), + "staging": _Target( + environment="staging", role=_DEV_ROLE, repository="minds-anton-scratchpad-dev" + ), + "production": _Target( + environment="prod", role=_PROD_ROLE, repository="minds-anton-scratchpad" + ), +} + + @pytest.fixture(scope="module") def workflow() -> dict: return yaml.safe_load(_WORKFLOW.read_text()) @@ -101,14 +134,36 @@ def _needs(job: dict) -> list[str]: return [str(name) for name in (needs or [])] -def _condition(job: dict) -> str: - """The job's ``if``, with ``${{ }}`` stripped and whitespace collapsed. +def _expression(value: object) -> str: + """``value`` with ``${{ }}`` stripped and whitespace collapsed. Both spellings mean the same thing to GitHub, so both have to compare equal here or the assertion turns into a formatting rule. """ - raw = str(job.get("if", "")) - return " ".join(raw.replace("${{", " ").replace("}}", " ").split()) + return " ".join(str(value).replace("${{", " ").replace("}}", " ").split()) + + +def _condition(job: dict) -> str: + """The job's ``if``, normalised by :func:`_expression`.""" + return _expression(job.get("if", "")) + + +def _grants_id_token(permissions: object) -> bool: + """True when a declared ``permissions`` value includes ``id-token: write``.""" + if permissions == "write-all": + return True + return isinstance(permissions, dict) and permissions.get("id-token") == "write" + + +def _step_using(job: dict, action: str) -> dict: + """The one step in ``job`` whose ``uses`` names ``action``, at any ref.""" + matches = [ + step + for step in job.get("steps") or [] + if str(step.get("uses", "")).split("@", 1)[0] == action + ] + assert len(matches) == 1, f"expected one `{action}` step, found {len(matches)}" + return matches[0] def _is_hosted(labels: list[str]) -> bool: @@ -130,6 +185,17 @@ def _triggers(workflow: dict) -> list[str]: return [str(on)] if on else [] +def _outsider_can_start(workflow: dict) -> bool: + """True when an account without write access here can start a run of ``workflow``. + + Most events allow that: a fork's pull request, a comment, an issue, or a + ``workflow_run`` chained from one of those. So the answer comes by + exclusion, and an event nobody here has heard of reads as reachable, the + same way an unknown runner label reads as ours. + """ + return any(event not in _WRITE_ACCESS_TRIGGERS for event in _triggers(workflow)) + + def _reachable_labels(job: dict, workflows: dict[str, dict], depth: int = 0) -> list[str] | None: """Every runner label this job can land work on, following local ``uses``. @@ -154,11 +220,17 @@ def _reachable_labels(job: dict, workflows: dict[str, dict], depth: int = 0) -> return labels -def _decide_step(workflow: dict) -> dict: +def _gate_step(workflow: dict, step_id: str) -> dict: for step in workflow["jobs"]["gate"]["steps"]: - if step.get("id") == "decide": + if step.get("id") == step_id: return step - raise AssertionError("no `decide` step in the gate job") + raise AssertionError(f"no `{step_id}` step in the gate job") + + +def _outputs_written(output: Path) -> dict[str, str]: + return dict( + line.split("=", 1) for line in output.read_text().splitlines() if "=" in line + ) def _run_the_gate( @@ -170,7 +242,7 @@ def _run_the_gate( ) -> _GateRun: """Execute the gate's real shell and hand back what it wrote.""" script = tmp_path / "decide.sh" - script.write_text(_decide_step(workflow)["run"]) + script.write_text(_gate_step(workflow, "decide")["run"]) output = tmp_path / "github_output" summary = tmp_path / "github_step_summary" output.touch() @@ -190,10 +262,28 @@ def _run_the_gate( }, ) - written = dict( - line.split("=", 1) for line in output.read_text().splitlines() if "=" in line + return _GateRun(outputs=_outputs_written(output), summary=summary.read_text()) + + +def _run_the_target(workflow: dict, tmp_path: Path, called_for: str) -> _TargetRun: + """Execute the gate's real ``target`` shell and hand back what it did.""" + script = tmp_path / "target.sh" + script.write_text(_gate_step(workflow, "target")["run"]) + output = tmp_path / "github_output" + output.touch() + + finished = subprocess.run( + ["bash", str(script)], + check=False, + capture_output=True, + env={ + "PATH": "/usr/bin:/bin:/usr/local/bin", + "CALLED_FOR": called_for, + "GITHUB_OUTPUT": str(output), + }, ) - return _GateRun(outputs=written, summary=summary.read_text()) + + return _TargetRun(returncode=finished.returncode, outputs=_outputs_written(output)) def test_a_fork_pull_request_is_refused(workflow: dict, tmp_path: Path) -> None: @@ -207,7 +297,7 @@ def test_a_fork_pull_request_is_refused(workflow: dict, tmp_path: Path) -> None: run = _run_the_gate(workflow, tmp_path, head_repo="stranger/anton") assert run.outputs.get("run") == "false", ( "the gate let a fork through: a pull request from any GitHub account " - "would build its own tree on a pod inside the cluster" + "would reach the job that assumes our ECR writer role" ) @@ -250,8 +340,8 @@ def test_an_event_with_no_pull_request_is_refused_for_the_right_reason( """ run = _run_the_gate(workflow, tmp_path, head_repo="", is_pr="false") assert run.outputs.get("run") == "false", ( - "an event carrying no pull request reached the cluster build; the gate " - "cannot tell a first-party branch from a fork without that payload" + "an event carrying no pull request reached the build; the gate cannot " + "tell a first-party branch from a fork without that payload" ) assert "fork" not in run.summary.lower(), ( "the run summary blames a fork on an event that has no pull request at " @@ -281,19 +371,24 @@ def test_a_release_pipeline_call_is_allowed(workflow: dict, tmp_path: Path) -> N _RELEASE_CALLERS = {"release.yml": "production", "publish-staging.yml": "staging"} +def _image_build_caller(workflows: dict[str, dict], filename: str) -> dict: + """The one job in ``filename`` that calls the image build workflow.""" + callers = [ + job + for job in (workflows[filename].get("jobs") or {}).values() + if str(job.get("uses", "")).endswith(_WORKFLOW.name) + ] + assert len(callers) == 1, f"{filename} must call {_WORKFLOW.name} exactly once" + return callers[0] + + def test_each_release_pipeline_builds_its_environment_image( workflows: dict[str, dict], ) -> None: """Dropping the job, or its input, leaves that environment's tag frozen at the last image anyone built, and nothing downstream notices.""" for filename, environment in _RELEASE_CALLERS.items(): - callers = [ - job - for job in (workflows[filename].get("jobs") or {}).values() - if str(job.get("uses", "")).endswith(_WORKFLOW.name) - ] - assert len(callers) == 1, f"{filename} must call {_WORKFLOW.name} exactly once" - passed = callers[0].get("with") or {} + passed = _image_build_caller(workflows, filename).get("with") or {} built_for = passed.get("environment") assert built_for == environment, ( f"{filename} builds the scratchpad image for {built_for!r}, not {environment!r}" @@ -304,6 +399,139 @@ def test_each_release_pipeline_builds_its_environment_image( ) +def test_each_release_pipeline_grants_the_token_to_the_image_build_only( + workflows: dict[str, dict], +) -> None: + """The build job declares ``id-token: write``, so each caller has to grant it. + + GitHub caps a called workflow at the calling job's grant, and it checks that + when it loads the file. A caller without the scope fails its whole run before + any job starts, so the pipeline's own notify job never reports it. The grant + belongs on the calling job: a workflow-level grant would hand the token to + every job in the release pipeline. + """ + for filename in _RELEASE_CALLERS: + assert not _grants_id_token(workflows[filename].get("permissions")), ( + f"{filename} grants id-token: write to every job; grant it only on " + f"the job that calls {_WORKFLOW.name}" + ) + caller = _image_build_caller(workflows, filename) + assert _grants_id_token(caller.get("permissions")), ( + f"{filename} calls {_WORKFLOW.name} without granting id-token: write, " + "so every run of it fails before any job starts" + ) + + +@pytest.mark.parametrize("called_for", sorted(_TARGETS), ids=lambda key: key or "pull_request") +def test_each_way_in_builds_into_its_own_tier( + workflow: dict, tmp_path: Path, called_for: str +) -> None: + """The pairs the roles' trust policies expect, executed rather than read. + + A pull request that named an environment would get that environment's OIDC + subject instead of ``pull_request``, and no role would accept it. A + production build that pushed to the dev repository would leave + ``:production`` frozen. Both fail only at run time, and the production row + runs only after a merge to main. + """ + run = _run_the_target(workflow, tmp_path, called_for) + way_in = called_for or "a pull request" + assert run.returncode == 0, f"the target step fails for {way_in}" + assert run.outputs == _TARGETS[called_for]._asdict(), ( + f"{way_in} builds into the wrong place" + ) + + +@pytest.mark.parametrize("called_for", ["development", "prod"]) +def test_an_unknown_environment_stops_the_build( + workflow: dict, tmp_path: Path, called_for: str +) -> None: + """A caller passing a name nobody mapped must fail, not fall through. + + ``development`` is the pull request tag name and ``prod`` the GitHub + environment name, so both are easy to pass by mistake. A fall-through to the + pull request row would build a release into the dev repository, and the + release's moving tag would quietly stop moving. + """ + run = _run_the_target(workflow, tmp_path, called_for) + assert run.returncode != 0, f"the target step accepted {called_for!r}" + assert not run.outputs, ( + f"the target step wrote outputs for {called_for!r}: {run.outputs}" + ) + + +def test_the_build_uses_the_target_the_gate_picked(workflow: dict) -> None: + """The right input has to pick the row, and each value has to reach its step. + + A row picked from anything but the release pipeline's input, or a hardcoded + environment, role or repository in the build job, ignores the mapping. The + mistake shows only when that path next runs. + """ + target_env = _gate_step(workflow, "target").get("env") or {} + assert _expression(target_env.get("CALLED_FOR")) == "inputs.environment", ( + "the target step picks its row from something other than the release " + "pipeline's input, so a pull request can land on a release row" + ) + gate_outputs = workflow["jobs"]["gate"].get("outputs") or {} + for key in _Target._fields: + assert _expression(gate_outputs.get(key)) == f"steps.target.outputs.{key}", ( + f"the gate's `{key}` output does not come from its target step" + ) + + build = workflow["jobs"]["build"] + assert _expression(build.get("environment")) == "needs.gate.outputs.environment", ( + "the build no longer names the environment the gate picked, so its OIDC " + "subject no longer matches the role it assumes" + ) + credentials = _step_using(build, "aws-actions/configure-aws-credentials").get("with") or {} + assert _expression(credentials.get("role-to-assume")) == "needs.gate.outputs.role", ( + "the build assumes a role the gate did not pick" + ) + push = _step_using(build, "mindsdb/github-actions/build-push-ecr").get("with") or {} + assert _expression(push.get("module-name")) == "needs.gate.outputs.repository", ( + "the build pushes to a repository the gate did not pick" + ) + assert push.get("builder") == "local", ( + "the build no longer uses this runner's own builder; the shared action's " + "default reaches for the in-cluster one" + ) + + +def test_the_build_assumes_the_role_before_it_pushes(workflow: dict) -> None: + """build-push-ecr in local mode pushes with whatever AWS credentials the job holds. + + A hosted runner holds none, so the build job has to assume the role before + the push. It also has to grant itself ``id-token: write``. This workflow's + own ``permissions`` grant only ``contents: read``, and a job that declares + none gets that, whatever the caller granted. Either mistake breaks all three + ways in, and only at run time. + """ + build = workflow["jobs"]["build"] + assert _grants_id_token(build.get("permissions")), ( + "the build job does not grant itself id-token: write, so it cannot mint " + "the token it assumes the role with" + ) + steps = build.get("steps") or [] + assume = _step_using(build, "aws-actions/configure-aws-credentials") + push = _step_using(build, "mindsdb/github-actions/build-push-ecr") + assert steps.index(assume) < steps.index(push), ( + "the build pushes before it assumes the role, so the push finds no AWS " + "credentials" + ) + + +def test_the_build_runs_on_a_hosted_runner(workflow: dict) -> None: + """The build brings its own short-lived role, so it needs nothing of ours. + + GitHub recommends GitHub-hosted runners for public repositories like this + one. A build moved back to a self-hosted runner would still pass every gate + assertion in this file, because the gate admits exactly the runs it admitted + before. + """ + labels = _labels(workflow["jobs"]["build"]) + assert _is_hosted(labels), f"the build runs on {labels}, which is a pod inside the cluster" + + def test_the_gate_reads_the_head_repository_from_the_event(workflow: dict) -> None: """Which value gets compared is the part that is quietly wrong-able. @@ -311,7 +539,7 @@ def test_the_gate_reads_the_head_repository_from_the_event(workflow: dict) -> No that would pass a fork calling its branch ``staging``. The repository's full name on the pull request's head is the only field the fork does not control. """ - env = _decide_step(workflow).get("env") or {} + env = _gate_step(workflow, "decide").get("env") or {} assert "github.event.pull_request.head.repo.full_name" in str(env.get("HEAD_REPO")), ( "the gate no longer compares the head repository; a branch name is " "chosen by whoever opened the pull request" @@ -328,12 +556,12 @@ def test_every_job_that_can_reach_our_runners_is_gated(workflows: dict[str, dict Guarding ``build`` alone protects today's file, and reading only that file protects nothing against the next workflow. So the sweep walks every - workflow that a fork can trigger, resolves each job's runner labels through - local ``uses`` calls, and treats any label outside the hosted families as - ours. + workflow an outside account can start, resolves each job's runner labels + through local ``uses`` calls, and treats any label outside the hosted + families as ours. """ for filename, workflow in workflows.items(): - if not any(event in _FORK_REACHABLE_TRIGGERS for event in _triggers(workflow)): + if not _outsider_can_start(workflow): continue for name, job in (workflow.get("jobs") or {}).items(): if name == "gate" and _is_hosted(_labels(job)): @@ -360,6 +588,36 @@ def test_every_job_that_can_reach_our_runners_is_gated(workflows: dict[str, dict ) +def test_every_job_that_can_mint_an_oidc_token_is_gated(workflows: dict[str, dict]) -> None: + """``id-token: write`` is a credential, wherever the job runs. + + A job holding it can trade its token for any role that trusts the token's + subject. In a workflow an outside account can start, that job has to carry + the gate, exactly like a job on our runners. A comment or a ``workflow_run`` + runs on ``main``, the default branch, so a job there that names the ``prod`` + environment gets a token with exactly the subject and ref the prod role + trusts. A job with no ``permissions`` anywhere takes the repository default, + which is a setting this tree cannot see, so it counts as able to mint. + """ + for filename, workflow in workflows.items(): + if not _outsider_can_start(workflow): + continue + for name, job in (workflow.get("jobs") or {}).items(): + permissions = job.get("permissions", workflow.get("permissions")) + if permissions is not None and not _grants_id_token(permissions): + continue + + assert _condition(job) == _GATE_CONDITION, ( + f"job `{name}` in {filename} can mint an OIDC token and its " + f"condition is {_condition(job)!r}, not {_GATE_CONDITION!r}" + ) + assert "gate" in _needs(job), ( + f"job `{name}` in {filename} reads the gate's output without " + "needing it, so the condition sees an empty string and the job " + "is skipped for everyone" + ) + + def test_the_gate_stays_on_a_hosted_runner(workflow: dict) -> None: """A guard that runs on the thing it is guarding has already lost.""" labels = _labels(workflow["jobs"]["gate"])