diff --git a/.github/workflows/update-adoption-wall.yml b/.github/workflows/update-adoption-wall.yml index 906e287b..e760360b 100644 --- a/.github/workflows/update-adoption-wall.yml +++ b/.github/workflows/update-adoption-wall.yml @@ -62,8 +62,11 @@ jobs: # the adopter state lives on the orphan `adoption-data` branch of this # repo precisely so it never has to. # - # These permissions still cover every push EXCEPT the auto-land onto the - # default branch. That one cannot use the workflow token at all: ruleset + # The workflow token pushes only the orphan state branch. Both PR paths + # (auto-land and review) push and open their PR with the App token, so + # that CI runs on the PR; the workflow token needs no pull-requests + # scope. The auto-land onto the default branch cannot use the workflow + # token at all: ruleset # `blorpathupap` requires an approved pull request on the default branch, # and the GitHub Actions app (id 15368) CANNOT be added as a bypass actor # -- the API rejects it with "Actor GitHub Actions integration must be @@ -71,7 +74,6 @@ jobs: # org installation. The "Mint app token" step below is the ONLY path; see # the note there for how its exposure is bounded. contents: write - pull-requests: write steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -452,8 +454,11 @@ jobs: # - resolving ~180 repos and probing every avatar can run long, and an # app token only lives an hour -- minting at job start could hand the # push an already-expired token; - # - a run that is not auto-landing (needs-review, unconfined, no change) - # never mints a token at all. + # - a run with nothing to push (no change, no state branch, or a status + # that is neither safe nor needs-review) never mints a token at all. + # Its condition is exactly the union of the two steps it serves: the + # auto-land below and the review PR after it. The review PR needs it too: + # a PR opened, or a branch pushed, with the workflow token triggers no CI. # Only `contents` (push the branch) and `pull-requests` (open and merge # the PR) are requested. The token is scoped to this repo, is never # echoed and is never written to a file -- it reaches git and gh through @@ -463,9 +468,9 @@ jobs: id: app-token if: >- steps.state_branch.outputs.exists == 'true' && - steps.wall.outputs.status == 'safe' && steps.changes.outputs.changed == 'true' && - steps.confine.outputs.confined == 'true' + (steps.wall.outputs.status == 'safe' || + steps.wall.outputs.status == 'needs-review') uses: actions/create-github-app-token@64ab729a6294000f5aa4a4bc0b73d1f32bc45510 # v3.3.0 with: app-id: "1108748" @@ -489,9 +494,9 @@ jobs: export GIT_CONFIG_VALUE_0="AUTHORIZATION: basic ${AUTH}" # Overrides the github-actions[bot] identity set by "Configure git # identity" above, for THIS commit only. That step still serves the - # review-PR path, which still pushes as the workflow token. The author - # of a commit should be whoever actually put it on the branch, and on - # the default branch that is copilotkit-bot. + # review-PR path. The author of a commit should be whoever actually + # put it on the branch, and on the default branch that is + # copilotkit-bot. git config user.name "copilotkit-bot[bot]" git config user.email "194550712+copilotkit-bot[bot]@users.noreply.github.com" # Run id + attempt for the same reason as the review-PR branch below: @@ -522,8 +527,9 @@ jobs: # off, a dead logo — goes through a PR instead. A SAFE run whose diff # reaches outside the generated region comes here too: it is a real # change, it just does not get to land unreviewed. - # Needs the org's "Allow GitHub Actions to create and approve pull - # requests" setting; without it gh pr create 403s on the workflow token. + # Pushed and opened with the App token, like the auto-land PR, so that CI + # runs on it (the workflow token triggers no workflow). It is NEVER merged + # here: it waits for a human. - name: Open review PR id: pr if: >- @@ -532,7 +538,7 @@ jobs: (steps.wall.outputs.status == 'needs-review' || (steps.wall.outputs.status == 'safe' && steps.confine.outputs.confined != 'true')) env: - GH_TOKEN: ${{ github.token }} + GH_TOKEN: ${{ steps.app-token.outputs.token }} STATUS: ${{ steps.wall.outputs.status }} run: | AUTH=$(printf 'x-access-token:%s' "$GH_TOKEN" | base64 | tr -d '\n') diff --git a/.github/workflows/update-competitive-matrix.yml b/.github/workflows/update-competitive-matrix.yml index 44bdf40d..4ff78e53 100644 --- a/.github/workflows/update-competitive-matrix.yml +++ b/.github/workflows/update-competitive-matrix.yml @@ -27,8 +27,9 @@ jobs: update-matrix: runs-on: ubuntu-latest permissions: - contents: write - pull-requests: write + # Read only: the workflow token only reads the GitHub API here. Every + # push and every PR goes through the App token ("Mint app token"). + contents: read steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -105,20 +106,19 @@ jobs: echo "$STATUS" fi - # The workflow token cannot land this: a PR it opens triggers no CI, and - # ruleset `blorpathupap` requires an approved PR on the default branch. - # copilotkit-bot (app 1108748) is a PULL-REQUEST-ONLY bypass actor on that - # ruleset. This is the same app, the same pinned action, the same secret - # and the same scopes that update-adoption-wall.yml and fix-drift.yml use. - # Minted behind the SAME condition as the push it serves, so a run that is - # not auto-landing never mints a token. The token reaches git and gh - # through the environment only. + # The workflow token cannot serve either PR path: a PR it opens or a + # branch it pushes triggers no CI, and ruleset `blorpathupap` requires an + # approved PR on the default branch. copilotkit-bot (app 1108748) is a + # PULL-REQUEST-ONLY bypass actor on that ruleset. This is the same app, + # the same pinned action, the same secret and the same scopes that + # update-adoption-wall.yml and fix-drift.yml use. Minted for any changed + # run, which is exactly the union of the two steps it serves: the + # auto-land below and the review PR after it. A run with no change never + # mints a token. The token reaches git and gh through the environment + # only. - name: Mint app token id: app-token - if: >- - steps.changes.outputs.changed == 'true' && - steps.changes.outputs.docs_changed == 'false' && - steps.confine.outputs.confined == 'true' + if: steps.changes.outputs.changed == 'true' uses: actions/create-github-app-token@64ab729a6294000f5aa4a4bc0b73d1f32bc45510 # v3.3.0 with: app-id: "1108748" @@ -168,25 +168,23 @@ jobs: echo "merged=true" >> "$GITHUB_OUTPUT" # A docs/ change, or a state change that is not confined, goes to a - # review PR on the workflow token, as before. - - name: Configure git credentials - if: >- - steps.changes.outputs.changed == 'true' && - steps.confine.outputs.confined != 'true' - run: | - git config --local --unset-all http.https://github.com/.extraheader || true - git config --local url."https://x-access-token:${{ secrets.GITHUB_TOKEN }}@github.com/".insteadOf "https://github.com/" - + # review PR for a human. It is pushed and opened with the App token so + # that CI runs on it, and it is NEVER merged here: it waits for a human. - name: Create PR id: pr if: >- steps.changes.outputs.changed == 'true' && steps.confine.outputs.confined != 'true' env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_TOKEN: ${{ steps.app-token.outputs.token }} RUN_ID: ${{ github.run_id }} DOCS_CHANGED: ${{ steps.changes.outputs.docs_changed }} run: | + AUTH=$(printf 'x-access-token:%s' "$GH_TOKEN" | base64 | tr -d '\n') + echo "::add-mask::${AUTH}" + export GIT_CONFIG_COUNT=1 + export GIT_CONFIG_KEY_0="http.https://github.com/.extraheader" + export GIT_CONFIG_VALUE_0="AUTHORIZATION: basic ${AUTH}" git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" # The run id keeps two runs on the same day on separate branches. diff --git a/src/__tests__/competitive-matrix-slack.test.ts b/src/__tests__/competitive-matrix-slack.test.ts index 382efabf..4faf175a 100644 --- a/src/__tests__/competitive-matrix-slack.test.ts +++ b/src/__tests__/competitive-matrix-slack.test.ts @@ -251,3 +251,72 @@ describe("Notify Slack digest (workflow jq/awk contract)", () => { expect(text.split("\n")[0]).toBe(REPORT_MSG); }); }); + +/** The whole named step, from its `- name:` line to the next step. */ +function stepBlock(yaml: string, name: string): string { + const lines = yaml.split("\n"); + const start = lines.findIndex((l) => l.trim() === `- name: ${name}`); + if (start === -1) throw new Error(`no step "${name}" in the workflow`); + const stepIndent = lines[start].indexOf("-"); + const body = [lines[start]]; + for (const line of lines.slice(start + 1)) { + const indent = line.length - line.trimStart().length; + if (line.trim() !== "" && indent <= stepIndent) break; + body.push(line); + } + return body.join("\n"); +} + +/** The step's `if:` condition (inline or folded `>-`), as one line. */ +function stepIf(block: string): string { + const lines = block.split("\n"); + const inline = lines.find((l) => /^\s*if: (?!>-\s*$)/.test(l)); + if (inline) return inline.trim().slice("if: ".length); + const at = lines.findIndex((l) => l.trim() === "if: >-"); + if (at === -1) throw new Error("step has no `if:` condition"); + const keyIndent = lines[at].indexOf("if:"); + const cond: string[] = []; + for (const line of lines.slice(at + 1)) { + const indent = line.length - line.trimStart().length; + if (line.trim() === "" || indent <= keyIndent) break; + cond.push(line.trim()); + } + return cond.join(" "); +} + +// A PR opened, or a branch pushed, with the workflow token (GITHUB_TOKEN) +// triggers no workflow, so CI never runs on it. The human review PR has to be +// opened with the copilotkit-bot App token, like the auto-land PR, but it must +// never be merged by the workflow. +describe("review PRs are opened with the App token", () => { + const cases = [ + { file: "update-competitive-matrix.yml", review: "Create PR" }, + { file: "update-adoption-wall.yml", review: "Open review PR" }, + ]; + for (const { file, review } of cases) { + const yaml = readFileSync( + resolve(dirname(fileURLToPath(import.meta.url)), "../../.github/workflows", file), + "utf8", + ); + + it(`${file}: "${review}" pushes and opens the PR on the App token`, () => { + const block = stepBlock(yaml, review); + expect(block).toContain("GH_TOKEN: ${{ steps.app-token.outputs.token }}"); + expect(block).not.toContain("secrets.GITHUB_TOKEN"); + expect(block).not.toContain("github.token"); + expect(block).toContain('GIT_CONFIG_VALUE_0="AUTHORIZATION: basic ${AUTH}"'); + // No git credential for the workflow token is left in .git/config. + expect(yaml).not.toContain("${{ secrets.GITHUB_TOKEN }}@github.com"); + }); + + it(`${file}: "${review}" never merges its PR`, () => { + expect(stepBlock(yaml, review)).not.toMatch(/gh pr merge|gh api .*\/merge/); + }); + + it(`${file}: the App token is minted for the review path too`, () => { + const mint = stepIf(stepBlock(yaml, "Mint app token")); + expect(mint).not.toContain("confined == 'true'"); + expect(mint).toContain("steps.changes.outputs.changed == 'true'"); + }); + } +});