From fb20157bc5b1c066c9e6a399d5edc4ab25b7dea9 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Mon, 26 Oct 2026 16:42:30 -0700 Subject: [PATCH] ci: open the competitor and adoption review PRs with the App token A PR opened or a branch pushed with GITHUB_TOKEN triggers no workflow, so CI never ran on the human review PRs from update-competitive-matrix.yml and update-adoption-wall.yml. Both review paths now push and open their PR with the copilotkit-bot App token that the auto-land path already mints. The review PR is still never merged by the workflow. The mint step's condition is now the union of the two steps it serves. The workflow token loses the scopes it no longer uses: pull-requests write on both jobs, and contents write on the competitive-matrix job. --- .github/workflows/update-adoption-wall.yml | 32 +++++---- .../workflows/update-competitive-matrix.yml | 46 ++++++------- .../competitive-matrix-slack.test.ts | 69 +++++++++++++++++++ 3 files changed, 110 insertions(+), 37 deletions(-) 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'"); + }); + } +});