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"])