From e8f492b01c749ef3c647d4fad2e769db82cfe10b Mon Sep 17 00:00:00 2001 From: Jeffrey Louden Date: Fri, 14 Aug 2026 21:12:48 -0400 Subject: [PATCH 1/2] ci: harden assign-ids app-token flow Follow-ups to #622 that were left uncommitted when it merged: - Gate the token mint on the private-key secret too: with only the ASSIGN_IDS_APP_ID var set, create-github-app-token hard-fails, which defeated the intended graceful fallback to GITHUB_TOKEN. Secrets are not readable in `if:`, so a step exposes a boolean instead. - Scope the App token to permission-contents: write. - persist-credentials: false on checkout so PR-controlled `pnpm install` lifecycle scripts can't read the token; the push step re-authenticates via `gh auth setup-git`. - Expand the README verification docs: fork-PR gate caveat and a step-by-step diagnostic order for reading run results. Co-Authored-By: Claude Fable 5 --- .github/README.md | 30 +++++++++++++++++++++++++----- .github/workflows/assign-ids.yml | 27 ++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/.github/README.md b/.github/README.md index 3af7a38cd..10d00cb19 100644 --- a/.github/README.md +++ b/.github/README.md @@ -46,7 +46,12 @@ exactly as before rather than failing outright. ### Verifying it worked -Open a catalog PR that adds an entry with no `id:` and let `assign-ids` commit. +Open a PR **from a branch in `existential-engineering/catalog` itself** that +adds an entry with no `id:`, and let `assign-ids` commit. The job is gated on +`head.repo.fork == false`, so a PR from a fork never runs it — there is no bot +commit and nothing to verify. (Fork PRs still need their IDs assigned some +other way; that is a separate gap, not a broken App.) + Then confirm the required checks ran on the **bot's** commit, not just yours: ```bash @@ -54,11 +59,26 @@ gh pr checks --repo existential-engineering/catalog --json name,bucket \ --jq '[.[]|"\(.name)=\(.bucket)"]|join(" ")' ``` -`validate=pass` and `audit=pass` must both be present. If they are missing, the -App is not wired up and the run is waiting for approval: +`validate=pass` and `audit=pass` must both be present. Their absence is a +symptom, not a diagnosis — they are also missing while a run is still going, +and on fork PRs. Look at the runs themselves before concluding anything: ```bash gh run list --repo existential-engineering/catalog --branch \ - --json databaseId,conclusion,headSha \ - --jq '.[]|select(.conclusion=="action_required")|.databaseId' + --json databaseId,workflowName,event,status,conclusion,headSha \ + --jq '.[]|"\(.workflowName)\t\(.event)\t\(.status)/\(.conclusion)\t\(.headSha[0:7])\t\(.databaseId)"' ``` + +Read it in this order: + +1. **No `Assign IDs` run at all** — the job never fired. Fork PR, or the diff + touched no `data/**/*.yaml`. Nothing to say about the App yet. +2. **`Assign IDs` ran but pushed nothing** — every entry already had an `id:`. + Re-test with an entry that has none. +3. **`Assign IDs` pushed, but `validate`/`audit` have no run on that new + `headSha`** — this is the GITHUB_TOKEN recursion guard: the App is not + wired up. +4. **`validate`/`audit` runs exist on that `headSha` with + `status=action_required`** — they are waiting for manual approval, also a + sign the push was not made under the App identity. +5. **`status=in_progress`/`queued`** — just still running. Wait and re-check. diff --git a/.github/workflows/assign-ids.yml b/.github/workflows/assign-ids.yml index f3aa7f53c..bf15902c6 100644 --- a/.github/workflows/assign-ids.yml +++ b/.github/workflows/assign-ids.yml @@ -32,19 +32,41 @@ jobs: # degrades to the old behaviour rather than failing outright. Set the # ASSIGN_IDS_APP_ID variable and ASSIGN_IDS_APP_PRIVATE_KEY secret to # activate it; see .github/README.md. + # + # Both halves have to be present or create-github-app-token hard-fails, + # which would defeat that fallback. Secrets are not readable in `if:`, so + # the key is checked here instead — this step exposes only a boolean and + # keeps the PEM in step-scoped env. + - name: Check app credentials + id: app-creds + if: vars.ASSIGN_IDS_APP_ID != '' + env: + APP_PRIVATE_KEY: ${{ secrets.ASSIGN_IDS_APP_PRIVATE_KEY }} + run: | + if [ -n "$APP_PRIVATE_KEY" ]; then + echo "configured=true" >> "$GITHUB_OUTPUT" + else + echo "configured=false" >> "$GITHUB_OUTPUT" + fi + - name: Mint app token id: app-token - if: vars.ASSIGN_IDS_APP_ID != '' + if: steps.app-creds.outputs.configured == 'true' uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 with: app-id: ${{ vars.ASSIGN_IDS_APP_ID }} private-key: ${{ secrets.ASSIGN_IDS_APP_PRIVATE_KEY }} + permission-contents: write + # persist-credentials: false — nothing in .git/config for `pnpm install` + # (which runs PR-controlled lifecycle scripts) to read. The push step + # re-authenticates for itself. - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 with: ref: ${{ github.head_ref }} fetch-depth: 0 token: ${{ steps.app-token.outputs.token || secrets.GITHUB_TOKEN }} + persist-credentials: false - uses: pnpm/action-setup@0977fd99725f1db4007ccb2928dbb4e90d06cc86 # v6 @@ -63,9 +85,12 @@ jobs: run: pnpm format - name: Commit changes + env: + GH_TOKEN: ${{ steps.app-token.outputs.token || secrets.GITHUB_TOKEN }} run: | git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" git add -A git diff --staged --quiet || git commit -m "chore: assign IDs to new entries" + gh auth setup-git git push From c812ff2b8d30b91690fa9b9fb2fbc81f1863c23f Mon Sep 17 00:00:00 2001 From: Jeffrey Louden Date: Sat, 15 Aug 2026 05:39:23 -0400 Subject: [PATCH 2/2] ci: address review feedback on assign-ids hardening MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Split the bot commit from the push: git commit now runs with no token in its environment, and both commit and push disable repo-controlled hooks (husky's hooksPath is installed by `pnpm install`'s prepare script) via core.hooksPath=/dev/null. Only the push step sees the token. - README diagnostics: distinguish a path-filter miss (no run created) from a fork PR (run exists, job skipped); note a no-op run also covers already-assigned hardware io keys, not just ids; approval-gated runs show conclusion=action_required (status stays completed — verified against live run data), and list waiting/pending among incomplete statuses. Co-Authored-By: Claude Fable 5 --- .github/README.md | 20 ++++++++++++-------- .github/workflows/assign-ids.yml | 15 +++++++++++---- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/.github/README.md b/.github/README.md index 10d00cb19..b20a40c18 100644 --- a/.github/README.md +++ b/.github/README.md @@ -18,7 +18,7 @@ Either way the outcome is the same and easy to miss: `validate` and `audit` are **required** status checks on `main` (ruleset `11289515`), and neither reports on the commit `assign-ids` just pushed. A run stuck in `action_required` does not show up in `gh pr checks` output at all, so the PR does not go red. It -quietly shows *fewer* checks than it should, every check it does show is green, +quietly shows _fewer_ checks than it should, every check it does show is green, and the PR is unmergeable with nothing obviously wrong. Pushing under an App identity makes the resulting `synchronize` event look like @@ -71,14 +71,18 @@ gh run list --repo existential-engineering/catalog --branch \ Read it in this order: -1. **No `Assign IDs` run at all** — the job never fired. Fork PR, or the diff - touched no `data/**/*.yaml`. Nothing to say about the App yet. -2. **`Assign IDs` ran but pushed nothing** — every entry already had an `id:`. - Re-test with an entry that has none. +1. **No `Assign IDs` run at all** — the path filter never fired: the diff + touched no `data/**/*.yaml`, so no run was created. (A fork PR looks + different — the run exists, but its `assign-ids` job shows as skipped by + the fork gate.) Nothing to say about the App yet. +2. **`Assign IDs` ran but pushed nothing** — nothing needed assigning: every + entry already had an `id:` and every hardware `io` entry already had its + key. Re-test with an entry missing one of those. 3. **`Assign IDs` pushed, but `validate`/`audit` have no run on that new `headSha`** — this is the GITHUB_TOKEN recursion guard: the App is not wired up. 4. **`validate`/`audit` runs exist on that `headSha` with - `status=action_required`** — they are waiting for manual approval, also a - sign the push was not made under the App identity. -5. **`status=in_progress`/`queued`** — just still running. Wait and re-check. + `conclusion=action_required`** — they are waiting for manual approval, + also a sign the push was not made under the App identity. +5. **`status=queued`/`in_progress`/`waiting`/`pending`** — just still + running. Wait and re-check. diff --git a/.github/workflows/assign-ids.yml b/.github/workflows/assign-ids.yml index bf15902c6..8718e1383 100644 --- a/.github/workflows/assign-ids.yml +++ b/.github/workflows/assign-ids.yml @@ -84,13 +84,20 @@ jobs: - name: Format YAML files run: pnpm format + # Git hooks are PR-controlled (`pnpm install` ran the repo's `prepare` + # script, which installs husky's hooksPath), so disable them for the + # bot's git operations and keep the token out of the commit step's + # environment entirely — only the push step ever sees it. - name: Commit changes - env: - GH_TOKEN: ${{ steps.app-token.outputs.token || secrets.GITHUB_TOKEN }} run: | git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" git add -A - git diff --staged --quiet || git commit -m "chore: assign IDs to new entries" + git diff --staged --quiet || git -c core.hooksPath=/dev/null commit -m "chore: assign IDs to new entries" + + - name: Push changes + env: + GH_TOKEN: ${{ steps.app-token.outputs.token || secrets.GITHUB_TOKEN }} + run: | gh auth setup-git - git push + git -c core.hooksPath=/dev/null push