diff --git a/.github/workflows/pr-run.yml b/.github/workflows/pr-run.yml index 933a84b..07f0355 100644 --- a/.github/workflows/pr-run.yml +++ b/.github/workflows/pr-run.yml @@ -71,7 +71,7 @@ jobs: timeout-minutes: 5 permissions: contents: read - pull-requests: read + pull-requests: write issues: write outputs: go: ${{ steps.check.outputs.match == 'true' && steps.check.outputs.error == '' && steps.comment.outputs.id != '' }} @@ -273,7 +273,7 @@ jobs: permissions: contents: read id-token: write - pull-requests: read + pull-requests: write issues: write outputs: outcome: ${{ steps.wait.outputs.outcome }} @@ -318,7 +318,7 @@ jobs: timeout-minutes: 5 permissions: contents: read - pull-requests: read + pull-requests: write issues: write steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 diff --git a/docs/operations.md b/docs/operations.md index a3ca382..351ec82 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -415,13 +415,13 @@ jobs: startsWith(github.event.comment.body, fromJSON('"/forge-perf\n"'))) && contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association) uses: fil-forge/forge-perf/.github/workflows/pr-run.yml@ # main - permissions: {contents: read, packages: write, pull-requests: read, issues: write, id-token: write} + permissions: {contents: read, packages: write, pull-requests: write, issues: write, id-token: write} with: {service: , dockerfile: Dockerfile, target: prod} ``` The inputs are `service` (the repository's name, which the workflow checks), `dockerfile` and `context` (paths from the repository root, `Dockerfile` and `.` by default), `target` (empty for the last stage) and `build-args`, one `NAME=value` per line. In `build-args`, `{commit}`, `{sha7}`, `{pr}` and `{tag}` become the head commit, its first seven characters, the pull request number and the image tag, since the caller's own `github.sha` is main's on a comment. The build must produce the flavour `config/images.tracked` follows: the prod target for ingot, sprue, hilt and swarf, `Dockerfile.dev` for guppy, the plain Dockerfile for the rest. The `if` starts a run only for a comment that begins with `/forge-perf`, alone or followed by a space, tab or line break, so a comment such as `/forge-performance` starts nothing. The role `forge-perf-ci-request` trusts the main branch of ingot, piri, sprue and hilt, where comment-triggered workflows run. Another repository needs its entry in `request_repositories` in `terraform/envs/bootstrap/account/main.tf`, applied, before its comments can file a request. -The workflow reads the pull request and writes its comment and reaction through the issues API, so it needs `pull-requests: read` and `issues: write`. The build job runs the branch's Dockerfile with `packages: write` and no AWS access. The request and wait jobs hold the role, and run only forge-perf's `scripts/ci/pr_run.py`. Every job checks it out at `job.workflow_sha`, the commit the caller pins. That role can write `requests/*` and read `status/*`, and nothing else. +The check, wait and report jobs comment on the pull request and react to the triggering comment through the issues API. GitHub checks `pull-requests: write` for a comment whose issue is a pull request, whichever API it arrives through, so those jobs need `pull-requests: write` as well as `issues: write`, and the caller has to grant both. The build job runs the branch's Dockerfile with `packages: write` and no AWS access. The request and wait jobs hold the role, and run only forge-perf's `scripts/ci/pr_run.py`. Every job checks it out at `job.workflow_sha`, the commit the caller pins. That role can write `requests/*` and read `status/*`, and nothing else. **Moving the pin.** A change to `pr-run.yml` or `scripts/ci/pr_run.py` reaches a repository when its `forge-perf.yml` names a newer forge-perf commit, through a pull request in that repository. Until then a forge-perf change cannot run with that repository's `packages: write`, which covers every tag of its image. forge-perf has no release tags, so Dependabot leaves the pin alone. diff --git a/scripts/ci/tests/workflows_test.sh b/scripts/ci/tests/workflows_test.sh index dd0ce52..254fb6d 100755 --- a/scripts/ci/tests/workflows_test.sh +++ b/scripts/ci/tests/workflows_test.sh @@ -63,10 +63,14 @@ fi [ "$(echo "$refs" | awk '{ print $1 }')" -ge 5 ] || { echo "FAIL: fewer than five forge-perf checkouts: $refs" >&2; exit 1; } echo "ok: pr-run.yml checks forge-perf out at its own commit" -# Comments and reactions go through the issues API, which issues: write -# covers; the one pull request call reads it. No job may ask for more. -if grep -nE '^\s*pull-requests:\s*write' "$pr_run"; then - echo "FAIL: pr-run.yml asks for pull-requests: write" >&2 +# The check, wait and report jobs comment on the pull request and react to its +# comment through the issues API. GitHub checks pull-requests: write for a +# comment whose issue is a pull request, so every job that writes issues must +# write pull requests too. +issues_write="$(grep -cE '^\s*issues:\s*write' "$pr_run")" +prs_write="$(grep -cE '^\s*pull-requests:\s*write' "$pr_run")" +if [ "$issues_write" -lt 1 ] || [ "$issues_write" != "$prs_write" ]; then + echo "FAIL: pr-run.yml has $issues_write issues: write but $prs_write pull-requests: write" >&2 exit 1 fi -echo "ok: pr-run.yml only reads pull requests" +echo "ok: pr-run.yml jobs that write issues also write pull requests"