From b66d62c5e392983209779e114be9df8e700febc2 Mon Sep 17 00:00:00 2001 From: Bill Wallace Date: Thu, 27 Aug 2026 09:49:02 -0400 Subject: [PATCH 1/7] ci: measure the CodSpeed simulation benches one package at a time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The simulation gate ran all eight packages' bench processes concurrently on the shared nashua box, on the premise -- stated in codspeed-walltime's own comment -- that instruction counting is immune to contention. #76 showed it is not. There, `decode CT-512x512-near-lossless.JLS (.81 near-lossless) — warm` was reported as a 19.8ms -> 37.9ms regression (-47.76%) on a commit whose entire diff was one vitest file. charls' source and its built wasm were byte-identical to main's, its real wall-clock bench duration was unchanged (26.7s vs 27.5s), and the identical -47.76% reappeared on the following commit, so it was reproducible rather than flake. Per-package completion times from that run put charls at 27s in, sharing the box with six or seven siblings, while dicom-codec then ran alone for ~5m54s: the packages are measured under wildly different neighbours, and #76 changed what those neighbours do (its openjph benches got 3-7.4x faster). Whatever the mechanism inside Cachegrind, a gate that measures eight packages simultaneously cannot attribute a per-package delta -- and it spent #76 blaming a package the PR never touched. --workspace-concurrency=1 rather than just dropping --parallel: pnpm's default workspace concurrency is 4, so removing the flag alone would still have run four bench processes against each other. Verified the distinction directly -- with the flag, three filtered packages run strictly back to back; with --parallel all three start within 25ms and overlap. This also makes the two CodSpeed jobs use one idiom, since codspeed-walltime already serialises this way. Cost is about 3 minutes: dicom-codec alone is ~6m of the current 6m22s bench step, and the job timeout is 100 minutes. Landing this resets the comparison basis for every bench previously measured under contention, so the first main run after merge is the new baseline -- expect one round of large apparent deltas there and nothing to act on. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/bench.yml | 31 ++++++++++++++++++++++++++++++- .github/workflows/pr-checks.yml | 6 ++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 091659f..e22971a 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -395,4 +395,33 @@ jobs: SCOPE_FLAGS: ${{ steps.scope.outputs.flags }} with: mode: simulation - run: bash tools/ci/with-nashua-lock.sh pnpm --parallel $SCOPE_FLAGS run bench + # --workspace-concurrency=1, matching codspeed-walltime in + # pr-checks.yml. NOT merely dropping --parallel: pnpm's default + # workspace concurrency is 4, so removing the flag alone would still + # run four packages' bench processes against each other. + # + # This job used to run all eight in parallel, on the premise -- stated + # in codspeed-walltime's own comment -- that instruction counting is + # immune to contention. That premise does not survive #76. The charls + # bench `decode CT-512x512-near-lossless.JLS (.81 near-lossless) — + # warm` was reported as a 19.8ms -> 37.9ms regression on a commit + # whose entire diff was one vitest file, with charls' source and its + # built wasm byte-identical to main's and its real wall-clock bench + # duration unchanged (26.7s vs 27.5s). The same -47.76% appeared again + # on the next commit, so it was reproducible rather than flake. Per + # package completion times from that run put charls at 27s in, sharing + # the box with six or seven siblings, while dicom-codec then ran alone + # for ~5m54s -- i.e. the packages are measured under wildly different + # neighbours, and #76 changed what those neighbours do (its openjph + # benches got 3-7.4x faster). + # + # Whatever the mechanism inside Cachegrind, a gate that measures eight + # packages simultaneously cannot attribute a per-package delta, and it + # spent #76 blaming an untouched package. Serial costs ~3 minutes: + # dicom-codec alone is ~6m of the 6m22s bench step, and the job + # timeout is 100 minutes. + # + # Landing this resets the comparison basis for every bench measured + # under contention, so the first main run after merge is the new + # baseline -- expect one round of large apparent deltas there. + run: bash tools/ci/with-nashua-lock.sh pnpm --workspace-concurrency=1 $SCOPE_FLAGS run bench diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 6bb018b..ae18d07 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -519,8 +519,10 @@ jobs: - name: Run CodSpeed benchmarks (walltime) # Walltime measures actual elapsed time, so parallel benchmark # processes would contend for cores and add noise — run packages - # sequentially (--workspace-concurrency=1), unlike the simulation job - # where instruction counting is immune to contention. + # sequentially (--workspace-concurrency=1). The simulation job in + # bench.yml now does the same: it was left parallel on the premise that + # instruction counting is immune to contention, and #76 showed it is + # not. See the comment on that job's run step. uses: CodSpeedHQ/action@4e969336ab9acd4f6f8d025fdd793292b0835df0 # v4.18.2 env: # Keep this in env, NOT `${{ }}` in the run: below — an env value is From 19fd94451fcfd589070ff7df2e58423d06f7145f Mon Sep 17 00:00:00 2001 From: Bill Wallace Date: Thu, 27 Aug 2026 09:58:52 -0400 Subject: [PATCH 2/7] ci: push release refs atomically so rejected branch updates leave no tags `git push --follow-tags HEAD:main` updates each ref independently. The 2026-08-24 release run (32733067241) showed the failure mode: main's protected branch hook declined `HEAD -> main`, but all eight version tags pushed successfully anyway, leaving them on a `chore(release): publish` commit that is not an ancestor of main. That turned a clean, retryable failure into a wedged repo. Every later release run died at `git tag -a` with "tag '@cornerstonejs/dicom-codec@1.0.12' already exists" -- earlier than the real problem and with a misleading message -- and recovery required a human deleting eight remote tags. Nothing had been published; npm latest still matched main's manifests throughout. --atomic makes all refs land or none. A rejected branch update can no longer publish tags for a release that did not happen. This does not fix the underlying rejection: the token needs a genuine bypass on main's ruleset, which is a repo-admin change and is being handled separately. It makes the next failure recoverable by re-running rather than by hand. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/release.yml | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c2a7c55..e61903a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -263,7 +263,18 @@ jobs: # NOT trigger further workflow runs (GitHub suppresses them for # GITHUB_TOKEN pushes), which is why the GitHub Releases are created # by a later job rather than by a separate tag-triggered workflow. - git push --follow-tags \ + # + # --atomic: all refs land or none do. Without it git updates each ref + # independently, and the 2026-08-24 run (32733067241) showed what that + # costs -- `! [remote rejected] HEAD -> main (protected branch hook + # declined)` while all eight version tags pushed successfully anyway. + # That left them pointing at a `chore(release): publish` commit which + # is not an ancestor of main, so every later run died at `git tag -a` + # with "tag already exists" before it even reached this push, and the + # recovery needed a human deleting eight remote tags. A rejected + # branch update must not be able to publish tags for a release that + # did not happen. + git push --atomic --follow-tags \ "https://x-access-token:${GH_TOKEN}@github.com/${GITHUB_REPOSITORY}.git" HEAD:main echo "sha=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT" From 69c77cfc7959d4c952c3fcba42167932b7a27208 Mon Sep 17 00:00:00 2001 From: Bill Wallace Date: Thu, 27 Aug 2026 10:34:02 -0400 Subject: [PATCH 3/7] ci: push the release commit with an org-owned GitHub App, not GITHUB_TOKEN The release has been broken since #87. `git push HEAD:main` is declined -- `protected branch hook declined` -- because main requires a pull request and the built-in GITHUB_TOKEN has no exemption from it. tools/release/setup-branch-ruleset.sh was written to fix that by migrating to a ruleset listing the "GitHub Actions" app as a bypass actor. That cannot work. App 15368 is owned by `github`, not by this org, and a repository ruleset only accepts bypass actors belonging to the repo or its owning organization, so the API refuses it: 422 Actor GitHub Actions integration must be part of the ruleset source or owner organization There is no repository setting that grants GITHUB_TOKEN a push to a PR-protected branch. An org-owned GitHub App is the supported route, and unlike a PAT it belongs to the org rather than to a person, so releases do not break when someone's token expires or they leave -- which was the whole point of moving off the maintainer SSH key CircleCI used. release.yml now mints a token per run from that App via actions/create-github-app-token, gated on `vars.RELEASE_APP_ID` so it no-ops on forks and before the one-time setup. When the App is not configured the job still runs and still fails at the push, but logs a warning naming the setup doc instead of leaving the next person to re-derive all of the above from "protected branch hook declined". setup-branch-ruleset.sh takes the App slug, resolves its id, and refuses to continue if the App is not owned by the org -- failing locally with an explanation rather than letting GitHub return the 422 this script exists to avoid. It also warns when the App is not installed on the org, since that yields a ruleset that looks correct and still cannot push. Its header carries the full org-owner UI walkthrough. Uses gh's built-in --jq throughout: this runs on a maintainer's laptop, where standalone jq is not a given. Does not change what humans need to merge: the ruleset reproduces main's current rules exactly (1 approving review, code-owner review, dismiss stale on push, require last-push approval, no force push, no deletion). Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/release.yml | 46 +++++++-- orphan-tags.txt | 8 ++ tools/release/README.md | 72 +++++++++++--- tools/release/setup-branch-ruleset.sh | 133 ++++++++++++++++++++++---- 4 files changed, 221 insertions(+), 38 deletions(-) create mode 100644 orphan-tags.txt diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e61903a..53aa9e0 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -221,13 +221,43 @@ jobs: count=$(jq 'length' release-plan.json) echo "count=$count" >> "$GITHUB_OUTPUT" jq -r '.[] | " \(.name): \(.previousVersion) -> \(.version) [\(.releaseType)]"' release-plan.json + - name: Mint a push token from the release GitHub App + id: app-token + # The built-in GITHUB_TOKEN CANNOT push to main, and no repository + # setting can grant it that: main's protection requires a pull request, + # and the "GitHub Actions" app it authenticates as (id 15368) is owned by + # `github`, not by this org. Adding it as a ruleset bypass actor is + # rejected outright -- "Actor GitHub Actions integration must be part of + # the ruleset source or owner organization" (HTTP 422). An org-owned app + # is the supported way to let CI push to a protected branch without a + # human's personal credential in the pipeline. + # + # Configured with an org/repo VARIABLE plus a SECRET, both optional, so + # this step no-ops on a fork or before the one-time setup has been done. + # See tools/release/setup-branch-ruleset.sh for that setup. + if: steps.version.outputs.count != '0' && vars.RELEASE_APP_ID != '' + uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + with: + app-id: ${{ vars.RELEASE_APP_ID }} + private-key: ${{ secrets.RELEASE_APP_PRIVATE_KEY }} - name: Commit, tag and push id: push if: steps.version.outputs.count != '0' env: - GH_TOKEN: ${{ github.token }} + # The app token when configured; otherwise GITHUB_TOKEN, which gets as + # far as the push and is then declined. Falling back rather than + # failing early keeps the version/changelog/tag work visible in the log + # (and keeps forks working), but the warning below says what is wrong so + # nobody has to re-derive it from "protected branch hook declined". + GH_TOKEN: ${{ steps.app-token.outputs.token || github.token }} + APP_TOKEN_CONFIGURED: ${{ steps.app-token.outputs.token != '' }} run: | set -euo pipefail + + if [ "$APP_TOKEN_CONFIGURED" != "true" ]; then + echo "::warning::RELEASE_APP_ID is not set, so this push uses GITHUB_TOKEN, which main's branch protection will decline. An org owner must complete the one-time setup in tools/release/setup-branch-ruleset.sh." + fi + git config user.name "github-actions[bot]" git config user.email "41898282+github-actions[bot]@users.noreply.github.com" @@ -257,12 +287,14 @@ jobs: git tag -a "$tag" -m "$tag" done - # Pushed with GITHUB_TOKEN, which main's ruleset lets bypass the pull - # request requirement. The token goes in the remote URL because - # checkout was told not to persist it (see above). Note this push does - # NOT trigger further workflow runs (GitHub suppresses them for - # GITHUB_TOKEN pushes), which is why the GitHub Releases are created - # by a later job rather than by a separate tag-triggered workflow. + # Pushed with the release App's token, which main's ruleset lists as a + # bypass actor for the pull request requirement. (It previously said + # GITHUB_TOKEN could do this; it cannot -- see the app-token step.) The + # token goes in the remote URL because checkout was told not to persist + # it (see above). Note this push does NOT trigger further workflow runs + # -- GitHub suppresses them for GITHUB_TOKEN pushes, and an App token + # is likewise not a user credential -- which is why the GitHub Releases + # are created by a later job rather than by a tag-triggered workflow. # # --atomic: all refs land or none do. Without it git updates each ref # independently, and the 2026-08-24 run (32733067241) showed what that diff --git a/orphan-tags.txt b/orphan-tags.txt new file mode 100644 index 0000000..6dfd41a --- /dev/null +++ b/orphan-tags.txt @@ -0,0 +1,8 @@ +@cornerstonejs/codec-big-endian@0.1.2 +@cornerstonejs/codec-charls@1.2.6 +@cornerstonejs/codec-libjpeg-turbo-12bit@0.4.4 +@cornerstonejs/codec-libjpeg-turbo-8bit@1.2.5 +@cornerstonejs/codec-little-endian@0.0.8 +@cornerstonejs/codec-openjpeg@1.3.3 +@cornerstonejs/codec-openjph@2.4.10 +@cornerstonejs/dicom-codec@1.0.12 diff --git a/tools/release/README.md b/tools/release/README.md index 27ee02d..8d0b947 100644 --- a/tools/release/README.md +++ b/tools/release/README.md @@ -2,8 +2,11 @@ Releases are fully automated: merge to `main`, and [.github/workflows/release.yml](../../.github/workflows/release.yml) versions, tags, publishes and -writes the GitHub Releases. There are no release secrets to rotate — npm auth is OIDC trusted -publishing and git auth is the built-in `GITHUB_TOKEN`. +writes the GitHub Releases. npm auth is OIDC trusted publishing, so there is no npm token to +rotate. Git auth for the one push to `main` is an org-owned GitHub App (`RELEASE_APP_ID` + +`RELEASE_APP_PRIVATE_KEY`) — the built-in `GITHUB_TOKEN` cannot push to a PR-protected branch and +cannot be granted a ruleset bypass, since the app it authenticates as is owned by `github` rather +than by this org. See [§2 below](#2-main-branch-ruleset). ## How a release is decided @@ -52,9 +55,14 @@ token runs nothing but `npm`, the pinned actions and `publish-order.mjs` (node b `pnpm-lock.yaml` (pnpm records each importer's *specifier*, so rewriting dicom-codec's ranges strands the lockfile and the next `--frozen-lockfile` install fails), commits `chore(release): publish [skip ci]` and one annotated tag per released package, and pushes to - `main` with `GITHUB_TOKEN`. The token is passed to `git push` in the remote URL rather than - persisted into `.git/config` by `actions/checkout`, so it is not sitting on disk while `pnpm - install` runs. The job outputs the pushed commit SHA. + `main` with a token minted per-run from the release App. The token is passed to `git push` in the + remote URL rather than persisted into `.git/config` by `actions/checkout`, so it is not sitting on + disk while `pnpm install` runs. The job outputs the pushed commit SHA. + + The push is `--atomic`: without it a declined branch update still publishes the tags, which is how + run [32733067241](https://github.com/cornerstonejs/codecs/actions/runs/32733067241) left eight + version tags on a commit that never reached `main` and wedged every later release at + `git tag -a` with "tag already exists". 3. **`publish`** — checks out that SHA, replays the dists, and publishes each package with `npm publish --ignore-scripts` in the dependency order `publish-order.mjs` computes — dicom-codec goes out after the six siblings whose ranges it carries. `--ignore-scripts` is deliberate: @@ -107,14 +115,52 @@ After the first green release, harden on npmjs.com: set each package's *Publishi "Require two-factor authentication and disallow tokens", and delete the old `NPM_TOKEN` from the CircleCI project (CircleCI no longer runs anything for this repo — the project should be disabled). -### 2. `main` branch ruleset +### 2. Release GitHub App + `main` branch ruleset + +**Must be done by an organization owner** — repo admin cannot create or install the App. + +The release job needs to push the version commit to `main`, which requires a pull request. The +built-in `GITHUB_TOKEN` cannot be exempted from that: the "GitHub Actions" app it authenticates as +(id 15368) is owned by `github`, and a ruleset only accepts bypass actors belonging to the repo or +its owning org, so GitHub rejects it with -```bash -gh auth login # as a repo admin -bash tools/release/setup-branch-ruleset.sh ``` +422 Actor GitHub Actions integration must be part of the ruleset source or owner organization +``` + +An org-owned App is the supported route. Unlike a PAT it belongs to the org rather than a person, so +it does not break when someone's token expires or they leave. + +1. **Create and install the App** — the full UI walkthrough is STEP 1 in + [setup-branch-ruleset.sh](setup-branch-ruleset.sh)'s header. Summary: create + `cornerstonejs-release` under the org with **Contents: read and write** and nothing else, no + webhook, generate a private key, install it on `codecs` only, then + + ```bash + gh variable set RELEASE_APP_ID --repo cornerstonejs/codecs --body '' + gh secret set RELEASE_APP_PRIVATE_KEY --repo cornerstonejs/codecs < /path/to/key.pem + rm /path/to/key.pem + ``` + +2. **Migrate the branch protection:** + + ```bash + gh auth login # as the org owner + gh auth refresh -s admin:org # ruleset writes need this scope + RELEASE_APP_SLUG=cornerstonejs-release bash tools/release/setup-branch-ruleset.sh + ``` + + Replaces main's classic branch protection with an equivalent ruleset listing that App as a bypass + actor. Review requirements for humans are unchanged: 1 approving review, code-owner review, stale + reviews dismissed on push, last-push approval, no force pushes, no branch deletion. See the + script's header for why the classic rule has to go rather than sit alongside the ruleset. + +3. **Verify**, then re-run the failed Release workflow: + + ```bash + gh api repos/cornerstonejs/codecs/rulesets + gh api repos/cornerstonejs/codecs/branches/main/protection # expect 404 + ``` -Replaces main's classic branch protection with an equivalent ruleset that lets the GitHub Actions app -bypass the pull-request requirement, so the release job can push the version commit. Review -requirements for humans are unchanged. See the script's header for why the classic rule has to go -rather than sit alongside the ruleset. +If `RELEASE_APP_ID` is unset the release still runs and fails at the push, but logs a warning naming +this section rather than only `protected branch hook declined`. diff --git a/tools/release/setup-branch-ruleset.sh b/tools/release/setup-branch-ruleset.sh index 84ac0c1..752c170 100644 --- a/tools/release/setup-branch-ruleset.sh +++ b/tools/release/setup-branch-ruleset.sh @@ -1,42 +1,139 @@ #!/usr/bin/env bash # # One-time setup: move main's protection from classic branch protection to a -# repository ruleset that lists the GitHub Actions app as a bypass actor. +# repository ruleset that lists an ORG-OWNED GitHub App as a bypass actor, so +# the release workflow can push the version commit and tags. +# +# MUST BE RUN BY AN ORGANIZATION OWNER. Repo admin is not enough: creating the +# App and installing it on the repo are org-level actions. See STEP 1 below. # # WHY # The release workflow pushes the `chore(release): publish` commit and the -# per-package tags with the built-in GITHUB_TOKEN. Classic branch protection -# has no bypass list — only repo admins skip the pull-request requirement — -# which is why the CircleCI release had to push with a maintainer's personal -# SSH key. Rulesets do support bypass actors, so migrating lets the bot push -# with no personal credential anywhere in the pipeline. +# per-package tags. Classic branch protection has no bypass list — only repo +# admins skip the pull-request requirement — which is why the CircleCI release +# had to push with a maintainer's personal SSH key. Rulesets do support bypass +# actors, so migrating lets CI push with no personal credential anywhere. # # A ruleset cannot relax classic protection: when both exist GitHub applies # the most restrictive of the two. The classic rule must therefore be deleted, # which is why this script does both halves. # +# WHY NOT THE BUILT-IN GITHUB_TOKEN +# An earlier version of this script used the "GitHub Actions" app (id 15368) +# as the bypass actor. That cannot work, and GitHub rejects it outright: +# +# HTTP 422: Actor GitHub Actions integration must be part of the ruleset +# source or owner organization +# +# App 15368 is owned by `github`, not by this organization, and a ruleset only +# accepts bypass actors belonging to the repo or its owning org. There is no +# repository setting that grants the built-in GITHUB_TOKEN a push to a +# PR-protected branch. An org-owned App is the supported route, and unlike a +# PAT it is not tied to any individual's account or expiry. +# # WHAT CHANGES FOR HUMANS # Nothing. The ruleset below reproduces main's current rules exactly: # 1 approving review, code-owner review required, stale reviews dismissed on # push, last-push approval required, no force pushes, no branch deletion. # -# PREREQUISITES -# gh auth login, as a repo admin. +# --------------------------------------------------------------------------- +# STEP 1 — create the App and install it (org owner, GitHub UI, ~5 minutes) +# +# 1. https://github.com/organizations/cornerstonejs/settings/apps/new +# GitHub App name: cornerstonejs-release +# Homepage URL: https://github.com/cornerstonejs/codecs +# Webhook: UNCHECK "Active" — this App never receives events +# Repository permissions: +# Contents ......... Read and write (push the commit + tags) +# Metadata ......... Read-only (added automatically) +# Nothing else. Do NOT grant Actions, Packages, or Administration. +# "Where can this GitHub App be installed?" -> Only on this account +# Create, then note the App ID shown on the settings page. # -# Verify afterwards with: -# gh api repos/cornerstonejs/codecs/rulesets -# gh api repos/cornerstonejs/codecs/branches/main/protection # expect 404 +# 2. Still on the App's page: "Private keys" -> "Generate a private key". +# A .pem downloads. It is shown once. +# +# 3. "Install App" (left sidebar) -> Install on cornerstonejs -> +# "Only select repositories" -> codecs -> Install. +# +# 4. Store the credentials on the repo (or the org, if you prefer to share +# the App with other repos later): +# gh variable set RELEASE_APP_ID --repo cornerstonejs/codecs --body '' +# gh secret set RELEASE_APP_PRIVATE_KEY --repo cornerstonejs/codecs < /path/to/key.pem +# Then delete the local .pem. release.yml reads exactly these two names. +# +# STEP 2 — run this script, which does the ruleset half: +# gh auth login # as the org owner +# gh auth refresh -s admin:org # ruleset writes need this scope +# RELEASE_APP_SLUG=cornerstonejs-release bash tools/release/setup-branch-ruleset.sh +# +# STEP 3 — verify: +# gh api repos/cornerstonejs/codecs/rulesets +# gh api repos/cornerstonejs/codecs/branches/main/protection # expect 404 +# Then re-run the failed Release workflow. The push step logs a warning if +# RELEASE_APP_ID is missing, so a misconfigured STEP 1 says so plainly +# instead of failing with "protected branch hook declined". +# --------------------------------------------------------------------------- set -euo pipefail REPO="${REPO:-cornerstonejs/codecs}" +ORG="${REPO%%/*}" + +# The App whose installation is allowed to bypass the pull-request rule. Must be +# owned by $ORG — see "WHY NOT THE BUILT-IN GITHUB_TOKEN" above. Pass the slug +# from the App's URL (github.com/organizations//settings/apps/), +# which is the name lowercased with spaces as hyphens. +RELEASE_APP_SLUG="${RELEASE_APP_SLUG:-cornerstonejs-release}" + +# gh's built-in --jq, not standalone jq: this script is run from a maintainer's +# own machine, where jq is not a given (release.yml can assume it, a laptop +# cannot). One call, both fields, split below. +if ! APP_INFO=$(gh api "apps/$RELEASE_APP_SLUG" --jq '"\(.id) \(.owner.login)"' 2>/dev/null); then + cat >&2 < bash tools/release/setup-branch-ruleset.sh + +The slug is the last path segment of the App's settings URL. +MSG + exit 1 +fi + +read -r RELEASE_APP_ID RELEASE_APP_OWNER <<<"$APP_INFO" + +# Fail here rather than let the API return the 422 this script exists to avoid. +if [ "$RELEASE_APP_OWNER" != "$ORG" ]; then + cat >&2 </dev/null); then + if ! printf '%s\n' "$INSTALLS" | grep -qx "$RELEASE_APP_SLUG"; then + echo "WARNING: '$RELEASE_APP_SLUG' is not installed on $ORG. Do STEP 1.3." >&2 + echo >&2 + fi +else + echo "NOTE: could not list org installations (needs admin:org); skipping the" >&2 + echo " install check. Confirm STEP 1.3 was done." >&2 + echo >&2 +fi echo "Current protection on $REPO main:" gh api "repos/$REPO/branches/main/protection" || true @@ -59,7 +156,7 @@ gh api -X POST "repos/$REPO/rulesets" --input - < Date: Thu, 27 Aug 2026 10:34:22 -0400 Subject: [PATCH 4/7] chore: drop stray orphan-tags.txt Scratch output from the one-off tag cleanup (the list of eight version tags orphaned by run 32733067241), swept in by `git add -A`. Not repo content. Co-Authored-By: Claude Opus 5 (1M context) --- orphan-tags.txt | 8 -------- 1 file changed, 8 deletions(-) delete mode 100644 orphan-tags.txt diff --git a/orphan-tags.txt b/orphan-tags.txt deleted file mode 100644 index 6dfd41a..0000000 --- a/orphan-tags.txt +++ /dev/null @@ -1,8 +0,0 @@ -@cornerstonejs/codec-big-endian@0.1.2 -@cornerstonejs/codec-charls@1.2.6 -@cornerstonejs/codec-libjpeg-turbo-12bit@0.4.4 -@cornerstonejs/codec-libjpeg-turbo-8bit@1.2.5 -@cornerstonejs/codec-little-endian@0.0.8 -@cornerstonejs/codec-openjpeg@1.3.3 -@cornerstonejs/codec-openjph@2.4.10 -@cornerstonejs/dicom-codec@1.0.12 From b863c56dc6054bd7ce6a3d40e1128672444fbcfd Mon Sep 17 00:00:00 2001 From: Bill Wallace Date: Thu, 27 Aug 2026 10:50:37 -0400 Subject: [PATCH 5/7] ci: allow a repo-scoped deploy key as the release push credential The App route added in the previous commit needs an organization owner to create and install the App, which is not available here -- Alireza granted repo admin on codecs, and /organizations/cornerstonejs/settings/apps/new 404s for a member. A deploy key is the repo-admin-sized answer. `DeployKey` is a valid ruleset bypass actor per GitHub's rules API, and a deploy key belongs to the repository by definition, so it satisfies "part of the ruleset source" with none of the ownership problem that makes app 15368 a 422. Like the App and unlike a PAT it is not tied to any individual, so releases survive people leaving. release.yml now resolves a credential in preference order -- App token, then deploy key over SSH, then GITHUB_TOKEN -- logs which one it picked, and warns when it lands on the last. The key is written to a mktemp file rather than a command line and removed on a trap. The deploy-key push is an ordinary push, unlike the App and GITHUB_TOKEN cases, so it WOULD retrigger this workflow on main. The `[skip ci]` already in the release commit message is what stops that being a loop; noted at the push and in the release doc so it does not get removed as dead weight. setup-branch-ruleset.sh takes BYPASS=deploy-key (default) or BYPASS=app and keeps every existing guard for the app path. One thing worth reviewing rather than just applying: the DeployKey actor takes `actor_id: null`, so it is a category, not a specific key. EVERY write-enabled deploy key on the repo -- present and future -- gains a bypass on main. The script therefore lists them and asks the operator to look, instead of counting them. This repo currently has a read-write key `Codecs CircleCI` (id 108740348, 2024-09-18); CircleCI runs nothing here any more, so it should be deleted rather than silently promoted into a credential that bypasses branch protection. That asymmetry is the deploy key's real cost against the App, and it is now in the comparison table in tools/release/README.md rather than left for someone to discover. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/release.yml | 75 +++++++--- tools/release/README.md | 90 +++++++++--- tools/release/setup-branch-ruleset.sh | 188 ++++++++++++++++++-------- 3 files changed, 261 insertions(+), 92 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 53aa9e0..925ac97 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -233,8 +233,9 @@ jobs: # human's personal credential in the pipeline. # # Configured with an org/repo VARIABLE plus a SECRET, both optional, so - # this step no-ops on a fork or before the one-time setup has been done. - # See tools/release/setup-branch-ruleset.sh for that setup. + # this step no-ops on a fork, before the one-time setup has been done, or + # when the deploy-key route below is used instead. See + # tools/release/setup-branch-ruleset.sh for both setups. if: steps.version.outputs.count != '0' && vars.RELEASE_APP_ID != '' uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 with: @@ -244,19 +245,37 @@ jobs: id: push if: steps.version.outputs.count != '0' env: - # The app token when configured; otherwise GITHUB_TOKEN, which gets as - # far as the push and is then declined. Falling back rather than - # failing early keeps the version/changelog/tag work visible in the log - # (and keeps forks working), but the warning below says what is wrong so - # nobody has to re-derive it from "protected branch hook declined". + # Three credentials, in order of preference. Both of the first two work + # because main's ruleset lists them as bypass actors; GITHUB_TOKEN + # cannot be listed at all (see the app-token step) and is only here so + # that forks and a not-yet-configured repo still reach the push and + # report something useful instead of failing earlier and vaguer. + # + # 1. App token — org-owned App. Best: scoped to Contents: write, + # expires in an hour, and belongs to the org. + # Requires an ORG OWNER to create and install it. + # 2. Deploy key — repo-scoped SSH key with write access. Slightly + # blunter (write to the whole repo, no expiry) but a + # REPO ADMIN can set it up alone, and like the App it + # is not tied to any individual's account. + # 3. GITHUB_TOKEN — declined by main's protection. Warns below. GH_TOKEN: ${{ steps.app-token.outputs.token || github.token }} APP_TOKEN_CONFIGURED: ${{ steps.app-token.outputs.token != '' }} + RELEASE_DEPLOY_KEY: ${{ secrets.RELEASE_DEPLOY_KEY }} run: | set -euo pipefail - if [ "$APP_TOKEN_CONFIGURED" != "true" ]; then - echo "::warning::RELEASE_APP_ID is not set, so this push uses GITHUB_TOKEN, which main's branch protection will decline. An org owner must complete the one-time setup in tools/release/setup-branch-ruleset.sh." + # Resolved before any git work so the log says which credential is in + # play before it matters. + if [ "$APP_TOKEN_CONFIGURED" = "true" ]; then + PUSH_VIA=app + elif [ -n "${RELEASE_DEPLOY_KEY:-}" ]; then + PUSH_VIA=deploy-key + else + PUSH_VIA=github-token + echo "::warning::Neither RELEASE_APP_ID nor RELEASE_DEPLOY_KEY is configured, so this push uses GITHUB_TOKEN, which main's branch protection will decline. See tools/release/setup-branch-ruleset.sh (the deploy-key route needs only repo admin)." fi + echo "Pushing via: $PUSH_VIA" git config user.name "github-actions[bot]" git config user.email "41898282+github-actions[bot]@users.noreply.github.com" @@ -287,14 +306,17 @@ jobs: git tag -a "$tag" -m "$tag" done - # Pushed with the release App's token, which main's ruleset lists as a - # bypass actor for the pull request requirement. (It previously said - # GITHUB_TOKEN could do this; it cannot -- see the app-token step.) The - # token goes in the remote URL because checkout was told not to persist - # it (see above). Note this push does NOT trigger further workflow runs - # -- GitHub suppresses them for GITHUB_TOKEN pushes, and an App token - # is likewise not a user credential -- which is why the GitHub Releases - # are created by a later job rather than by a tag-triggered workflow. + # Pushed with whichever credential main's ruleset lists as a bypass + # actor. (An earlier comment here said GITHUB_TOKEN could do this; it + # cannot -- see the app-token step.) Credentials are passed per-push + # rather than persisted, because checkout was told not to keep them in + # .git/config while `pnpm install` runs. + # + # The commit message carries [skip ci] and that matters for the deploy + # key: GitHub suppresses workflow runs for GITHUB_TOKEN pushes and App + # tokens are likewise not user credentials, but a deploy-key push is an + # ordinary push and WOULD retrigger this workflow on main. [skip ci] is + # what stops that being a release loop, so do not remove it. # # --atomic: all refs land or none do. Without it git updates each ref # independently, and the 2026-08-24 run (32733067241) showed what that @@ -306,8 +328,23 @@ jobs: # recovery needed a human deleting eight remote tags. A rejected # branch update must not be able to publish tags for a release that # did not happen. - git push --atomic --follow-tags \ - "https://x-access-token:${GH_TOKEN}@github.com/${GITHUB_REPOSITORY}.git" HEAD:main + if [ "$PUSH_VIA" = "deploy-key" ]; then + # Key material into a file the runner discards with the job, never + # onto a command line. IdentitiesOnly stops any agent key being tried + # first; accept-new is safe on an ephemeral runner with no prior + # known_hosts to be spoofed against. + key_file=$(mktemp) + printf '%s\n' "$RELEASE_DEPLOY_KEY" > "$key_file" + chmod 600 "$key_file" + export GIT_SSH_COMMAND="ssh -i $key_file -o IdentitiesOnly=yes -o StrictHostKeyChecking=accept-new" + trap 'rm -f "$key_file"' EXIT + + git push --atomic --follow-tags \ + "git@github.com:${GITHUB_REPOSITORY}.git" HEAD:main + else + git push --atomic --follow-tags \ + "https://x-access-token:${GH_TOKEN}@github.com/${GITHUB_REPOSITORY}.git" HEAD:main + fi echo "sha=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT" - name: Nothing to release diff --git a/tools/release/README.md b/tools/release/README.md index 8d0b947..6d1523c 100644 --- a/tools/release/README.md +++ b/tools/release/README.md @@ -3,10 +3,11 @@ Releases are fully automated: merge to `main`, and [.github/workflows/release.yml](../../.github/workflows/release.yml) versions, tags, publishes and writes the GitHub Releases. npm auth is OIDC trusted publishing, so there is no npm token to -rotate. Git auth for the one push to `main` is an org-owned GitHub App (`RELEASE_APP_ID` + -`RELEASE_APP_PRIVATE_KEY`) — the built-in `GITHUB_TOKEN` cannot push to a PR-protected branch and -cannot be granted a ruleset bypass, since the app it authenticates as is owned by `github` rather -than by this org. See [§2 below](#2-main-branch-ruleset). +rotate. Git auth for the one push to `main` is either a repo deploy key (`RELEASE_DEPLOY_KEY`) or an +org-owned GitHub App (`RELEASE_APP_ID` + `RELEASE_APP_PRIVATE_KEY`) — the built-in `GITHUB_TOKEN` +cannot push to a PR-protected branch and cannot be granted a ruleset bypass, since the app it +authenticates as is owned by `github` rather than by this org. See +[§2 below](#2-a-push-credential-for-main--the-branch-ruleset). ## How a release is decided @@ -115,9 +116,21 @@ After the first green release, harden on npmjs.com: set each package's *Publishi "Require two-factor authentication and disallow tokens", and delete the old `NPM_TOKEN` from the CircleCI project (CircleCI no longer runs anything for this repo — the project should be disabled). -### 2. Release GitHub App + `main` branch ruleset +### 2. A push credential for `main` + the branch ruleset -**Must be done by an organization owner** — repo admin cannot create or install the App. +Two routes. **The deploy-key route needs only repo admin**; the App route is better hygiene but +requires an organization owner. `release.yml` accepts either and prefers the App when both exist. + +| | Deploy key | GitHub App | +|---|---|---| +| Who can set it up | repo admin | **org owner** | +| Scope | write to the whole repo | `Contents: write` | +| Lifetime | no expiry | token expires hourly | +| Bypass granularity | **every** write-enabled deploy key on the repo | that one App | +| Secrets | `RELEASE_DEPLOY_KEY` | `RELEASE_APP_ID` + `RELEASE_APP_PRIVATE_KEY` | + +Neither is a personal credential, which is the thing to preserve — the point of moving off +CircleCI's arrangement was that releases must not depend on one person's key. The release job needs to push the version commit to `main`, which requires a pull request. The built-in `GITHUB_TOKEN` cannot be exempted from that: the "GitHub Actions" app it authenticates as @@ -128,12 +141,49 @@ its owning org, so GitHub rejects it with 422 Actor GitHub Actions integration must be part of the ruleset source or owner organization ``` -An org-owned App is the supported route. Unlike a PAT it belongs to the org rather than a person, so -it does not break when someone's token expires or they leave. +A **deploy key** or an **org-owned App** can both be listed as bypass actors. A deploy key belongs to +the repository, so it satisfies "part of the ruleset source" with no ownership question — which is +why it works without org access. + +#### Route A — deploy key (repo admin) + +1. **Create the key and store both halves.** Full walkthrough is `STEP 1-DEPLOY-KEY` in + [setup-branch-ruleset.sh](setup-branch-ruleset.sh)'s header. Summary: + + ```bash + ssh-keygen -t ed25519 -N '' -C 'codecs release' -f ./codecs-release-key + gh repo deploy-key add ./codecs-release-key.pub \ + --repo cornerstonejs/codecs --title 'codecs release' --allow-write + gh secret set RELEASE_DEPLOY_KEY --repo cornerstonejs/codecs < ./codecs-release-key + rm ./codecs-release-key ./codecs-release-key.pub + ``` + +2. **Migrate the branch protection:** -1. **Create and install the App** — the full UI walkthrough is STEP 1 in - [setup-branch-ruleset.sh](setup-branch-ruleset.sh)'s header. Summary: create - `cornerstonejs-release` under the org with **Contents: read and write** and nothing else, no + ```bash + gh auth login # as a repo admin + bash tools/release/setup-branch-ruleset.sh + ``` + +> [!WARNING] +> The `DeployKey` bypass actor takes `actor_id: null` — it is a **category, not a specific key**. +> Every write-enabled deploy key on the repo, present and future, can then push to `main` without +> review. The script lists them and asks you to look. Audit before enabling, and delete any left over +> from retired CI: +> +> ```bash +> gh repo deploy-key list --repo cornerstonejs/codecs +> gh repo deploy-key delete --repo cornerstonejs/codecs +> ``` +> +> As of this writing there is a read-write key `Codecs CircleCI` (id 108740348, added 2024-09-18). +> CircleCI no longer runs anything for this repo, so it should be deleted rather than promoted into a +> credential that bypasses branch protection. + +#### Route B — GitHub App (org owner) + +1. **Create and install the App** — walkthrough is `STEP 1-APP` in the script's header. Summary: + create `cornerstonejs-release` under the org with **Contents: read and write** and nothing else, no webhook, generate a private key, install it on `codecs` only, then ```bash @@ -146,14 +196,20 @@ it does not break when someone's token expires or they leave. ```bash gh auth login # as the org owner - gh auth refresh -s admin:org # ruleset writes need this scope - RELEASE_APP_SLUG=cornerstonejs-release bash tools/release/setup-branch-ruleset.sh + BYPASS=app RELEASE_APP_SLUG=cornerstonejs-release bash tools/release/setup-branch-ruleset.sh ``` - Replaces main's classic branch protection with an equivalent ruleset listing that App as a bypass - actor. Review requirements for humans are unchanged: 1 approving review, code-owner review, stale - reviews dismissed on push, last-push approval, no force pushes, no branch deletion. See the - script's header for why the classic rule has to go rather than sit alongside the ruleset. +#### Either route + +The script replaces main's classic branch protection with an equivalent ruleset listing the chosen +bypass actor. Review requirements for humans are unchanged: 1 approving review, code-owner review, +stale reviews dismissed on push, last-push approval, no force pushes, no branch deletion. See the +script's header for why the classic rule has to go rather than sit alongside the ruleset. + +One behavioural note for the deploy-key route: GitHub suppresses workflow runs for `GITHUB_TOKEN` +pushes, and App tokens are likewise not user credentials, but a deploy-key push is an ordinary push +and *would* retrigger the release workflow on `main`. The `[skip ci]` in the release commit message +is what prevents a loop — do not remove it. 3. **Verify**, then re-run the failed Release workflow: diff --git a/tools/release/setup-branch-ruleset.sh b/tools/release/setup-branch-ruleset.sh index 752c170..75b3f79 100644 --- a/tools/release/setup-branch-ruleset.sh +++ b/tools/release/setup-branch-ruleset.sh @@ -1,11 +1,23 @@ #!/usr/bin/env bash # # One-time setup: move main's protection from classic branch protection to a -# repository ruleset that lists an ORG-OWNED GitHub App as a bypass actor, so -# the release workflow can push the version commit and tags. +# repository ruleset with a bypass actor the release workflow can push as. # -# MUST BE RUN BY AN ORGANIZATION OWNER. Repo admin is not enough: creating the -# App and installing it on the repo are org-level actions. See STEP 1 below. +# TWO ROUTES. Pick by what access you have: +# +# BYPASS=deploy-key (default) REPO ADMIN is enough. +# A repo-scoped SSH deploy key with write access. Blunter than the App -- +# write to the whole repo, no expiry -- but it belongs to the repository +# rather than to a person, and needs nobody above repo admin. +# +# BYPASS=app Requires an ORGANIZATION OWNER. +# An org-owned GitHub App. Better hygiene: scoped to Contents: write, the +# token expires in an hour, and it is auditable as an app. Preferred if +# you can get an owner to do STEP 1-APP. +# +# Both are recognised by release.yml, which prefers the App when both exist. +# Neither puts a personal credential in the pipeline, which is the thing the +# CircleCI setup (a maintainer's own SSH key) got wrong. # # WHY # The release workflow pushes the `chore(release): publish` commit and the @@ -37,7 +49,28 @@ # push, last-push approval required, no force pushes, no branch deletion. # # --------------------------------------------------------------------------- -# STEP 1 — create the App and install it (org owner, GitHub UI, ~5 minutes) +# STEP 1-DEPLOY-KEY — the repo-admin route (~3 minutes, no org access) +# +# 1. Generate a keypair. Nothing but this repo will ever use it, so it does +# not belong in ~/.ssh: +# ssh-keygen -t ed25519 -N '' -C 'codecs release' -f ./codecs-release-key +# +# 2. Add the PUBLIC half as a deploy key WITH WRITE ACCESS: +# gh repo deploy-key add ./codecs-release-key.pub \ +# --repo cornerstonejs/codecs --title 'codecs release' --allow-write +# (UI equivalent: Settings -> Deploy keys -> Add deploy key, tick +# "Allow write access".) +# +# 3. Add the PRIVATE half as the secret release.yml reads, then delete both +# local halves -- the repo and the secret are the only copies you need: +# gh secret set RELEASE_DEPLOY_KEY --repo cornerstonejs/codecs < ./codecs-release-key +# rm ./codecs-release-key ./codecs-release-key.pub +# +# 4. Run this script (default BYPASS=deploy-key), then STEP 3. +# gh auth login # as a repo admin +# bash tools/release/setup-branch-ruleset.sh +# +# STEP 1-APP — the org-owner route (GitHub UI, ~5 minutes) # # 1. https://github.com/organizations/cornerstonejs/settings/apps/new # GitHub App name: cornerstonejs-release @@ -62,78 +95,125 @@ # gh secret set RELEASE_APP_PRIVATE_KEY --repo cornerstonejs/codecs < /path/to/key.pem # Then delete the local .pem. release.yml reads exactly these two names. # -# STEP 2 — run this script, which does the ruleset half: -# gh auth login # as the org owner -# gh auth refresh -s admin:org # ruleset writes need this scope -# RELEASE_APP_SLUG=cornerstonejs-release bash tools/release/setup-branch-ruleset.sh +# 2. Run this script in app mode, then STEP 3: +# gh auth login # as the org owner +# BYPASS=app RELEASE_APP_SLUG=cornerstonejs-release \ +# bash tools/release/setup-branch-ruleset.sh # -# STEP 3 — verify: +# STEP 3 — verify, either route: # gh api repos/cornerstonejs/codecs/rulesets # gh api repos/cornerstonejs/codecs/branches/main/protection # expect 404 -# Then re-run the failed Release workflow. The push step logs a warning if -# RELEASE_APP_ID is missing, so a misconfigured STEP 1 says so plainly -# instead of failing with "protected branch hook declined". +# Then re-run the failed Release workflow. Its push step logs which +# credential it used, and warns if neither is configured, so a half-done +# STEP 1 says so plainly instead of failing with "protected branch hook +# declined". # --------------------------------------------------------------------------- set -euo pipefail REPO="${REPO:-cornerstonejs/codecs}" ORG="${REPO%%/*}" +BYPASS="${BYPASS:-deploy-key}" -# The App whose installation is allowed to bypass the pull-request rule. Must be -# owned by $ORG — see "WHY NOT THE BUILT-IN GITHUB_TOKEN" above. Pass the slug -# from the App's URL (github.com/organizations//settings/apps/), -# which is the name lowercased with spaces as hyphens. -RELEASE_APP_SLUG="${RELEASE_APP_SLUG:-cornerstonejs-release}" - -# gh's built-in --jq, not standalone jq: this script is run from a maintainer's -# own machine, where jq is not a given (release.yml can assume it, a laptop -# cannot). One call, both fields, split below. -if ! APP_INFO=$(gh api "apps/$RELEASE_APP_SLUG" --jq '"\(.id) \(.owner.login)"' 2>/dev/null); then - cat >&2 </dev/null | grep -F 'read-write' || true) + if [ -z "$WRITE_KEYS" ]; then + echo "WARNING: $REPO has no write-enabled deploy key, so the release still" >&2 + echo " cannot push. Do STEP 1-DEPLOY-KEY 1-3." >&2 + else + echo "Write-enabled deploy keys that this ruleset will let bypass review:" + printf '%s\n' "$WRITE_KEYS" | sed 's/^/ /' + echo + echo "Delete any that are not the release key:" + echo " gh repo deploy-key delete --repo $REPO" + fi + echo + ;; + + app) + # The App whose installation is allowed to bypass the pull-request rule. Must + # be owned by $ORG — see "WHY NOT THE BUILT-IN GITHUB_TOKEN" above. Pass the + # slug from the App's URL + # (github.com/organizations//settings/apps/), which is the name + # lowercased with spaces as hyphens. + RELEASE_APP_SLUG="${RELEASE_APP_SLUG:-cornerstonejs-release}" + + # gh's built-in --jq, not standalone jq: this script is run from a + # maintainer's own machine, where jq is not a given (release.yml can assume + # it, a laptop cannot). One call, both fields, split below. + if ! APP_INFO=$(gh api "apps/$RELEASE_APP_SLUG" --jq '"\(.id) \(.owner.login)"' 2>/dev/null); then + cat >&2 < bash tools/release/setup-branch-ruleset.sh +Complete STEP 1-APP in this script's header first, then re-run with the slug: + BYPASS=app RELEASE_APP_SLUG= bash tools/release/setup-branch-ruleset.sh The slug is the last path segment of the App's settings URL. MSG - exit 1 -fi + exit 1 + fi -read -r RELEASE_APP_ID RELEASE_APP_OWNER <<<"$APP_INFO" + read -r RELEASE_APP_ID RELEASE_APP_OWNER <<<"$APP_INFO" -# Fail here rather than let the API return the 422 this script exists to avoid. -if [ "$RELEASE_APP_OWNER" != "$ORG" ]; then - cat >&2 <&2 </dev/null); then - if ! printf '%s\n' "$INSTALLS" | grep -qx "$RELEASE_APP_SLUG"; then - echo "WARNING: '$RELEASE_APP_SLUG' is not installed on $ORG. Do STEP 1.3." >&2 - echo >&2 - fi -else - echo "NOTE: could not list org installations (needs admin:org); skipping the" >&2 - echo " install check. Confirm STEP 1.3 was done." >&2 - echo >&2 -fi + # An App that is not installed on the repo yields a ruleset that looks + # correct and still cannot push. Warn rather than fail: listing installations + # needs admin:org, which the operator may deliberately not have granted. + if INSTALLS=$(gh api "orgs/$ORG/installations" --jq '.installations[].app_slug' 2>/dev/null); then + if ! printf '%s\n' "$INSTALLS" | grep -qx "$RELEASE_APP_SLUG"; then + echo "WARNING: '$RELEASE_APP_SLUG' is not installed on $ORG. Do STEP 1-APP.3." >&2 + fi + else + echo "NOTE: could not list org installations (needs admin:org); skipping" >&2 + echo " the install check. Confirm STEP 1-APP.3 was done." >&2 + fi + echo + ;; + + *) + echo "BYPASS must be 'deploy-key' (repo admin) or 'app' (org owner); got '$BYPASS'." >&2 + exit 1 + ;; +esac echo "Current protection on $REPO main:" gh api "repos/$REPO/branches/main/protection" || true @@ -155,11 +235,7 @@ gh api -X POST "repos/$REPO/rulesets" --input - < Date: Thu, 27 Aug 2026 11:09:34 -0400 Subject: [PATCH 6/7] ci: scope the release App token, and gate the deploy-key bypass on an audit Two review findings on #89. Accepted: the DeployKey bypass actor is category-wide, and the script only warned about it. `actor_id: null` means every write-enabled deploy key on the repo can push to main without review, so creating this ruleset can silently promote an unrelated credential into a branch-protection bypass. This repo had exactly that case -- a read-write `Codecs CircleCI` key, years after CircleCI stopped running here. A warning above a y/N prompt is too easy to scroll past for an escalation that quiet, so the operator now has to type `audited` after reading the list of keys (or set DEPLOY_KEYS_AUDITED=1 for non-interactive use). Also accepted, from zizmor: an App token inherits the installation's blanket permissions. Added `permission-contents: write` so the release token is narrowed to what the push needs, and a later widening of the App does not silently widen the release token. Rejected: adding continue-on-error to the app-token step so a failed mint falls through to the deploy key. That trades a loud failure for a silent credential downgrade -- from a token scoped to Contents: write and expiring in an hour, to one with write access to the whole repo and no expiry -- in a pipeline that publishes to npm, detectable only by reading the log of a release that appeared to succeed. If someone configured the App, the App is what should be used or the run should stop. The action validates the private key up front and retries transient 5xx itself, so what reaches that failure is a real misconfiguration, and --atomic means the failed run leaves nothing to clean up. Recorded as a comment at the step so it does not get "fixed" later. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/release.yml | 15 ++++++++++++++ tools/release/setup-branch-ruleset.sh | 29 +++++++++++++++++++++------ 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 925ac97..17c1a71 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -236,11 +236,26 @@ jobs: # this step no-ops on a fork, before the one-time setup has been done, or # when the deploy-key route below is used instead. See # tools/release/setup-branch-ruleset.sh for both setups. + # + # Deliberately NOT continue-on-error. If RELEASE_APP_ID is set and + # minting fails, this job stops rather than quietly pushing with the + # deploy key instead. Falling through would swap a token scoped to + # Contents: write and expiring in an hour for one with write access to + # the whole repo and no expiry -- a downgrade nobody asked for, in a + # pipeline that publishes to npm, discoverable only by reading the log of + # a release that appeared to succeed. A broken App config should be + # fixed; the action validates the private key up front and retries + # transient 5xx itself, so what reaches here is a real misconfiguration, + # and --atomic means the failed run leaves no debris to clean up. if: steps.version.outputs.count != '0' && vars.RELEASE_APP_ID != '' uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 with: app-id: ${{ vars.RELEASE_APP_ID }} private-key: ${{ secrets.RELEASE_APP_PRIVATE_KEY }} + # Narrow the token to what the push needs. Without this it inherits + # every permission the installation holds, so a later widening of the + # App silently widens the release token too. + permission-contents: write - name: Commit, tag and push id: push if: steps.version.outputs.count != '0' diff --git a/tools/release/setup-branch-ruleset.sh b/tools/release/setup-branch-ruleset.sh index 75b3f79..482bb3c 100644 --- a/tools/release/setup-branch-ruleset.sh +++ b/tools/release/setup-branch-ruleset.sh @@ -132,21 +132,38 @@ case "$BYPASS" in # to main without review. # # So the write-enabled keys are listed here rather than merely counted, and - # the operator is asked to look. A leftover key from a retired CI system is - # the case that matters: it stops being an unused credential and becomes one - # that bypasses branch protection. + # the operator has to acknowledge the list by name before the ruleset is + # created. A leftover key from a retired CI system is the case that matters: + # it stops being an unused credential and becomes one that bypasses branch + # protection. This repo had exactly that -- a read-write `Codecs CircleCI` + # key, years after CircleCI stopped running here. A warning printed above a + # y/N prompt is too easy to scroll past for a privilege escalation that + # silent, hence the typed acknowledgement. WRITE_KEYS=$(gh repo deploy-key list --repo "$REPO" 2>/dev/null | grep -F 'read-write' || true) if [ -z "$WRITE_KEYS" ]; then echo "WARNING: $REPO has no write-enabled deploy key, so the release still" >&2 echo " cannot push. Do STEP 1-DEPLOY-KEY 1-3." >&2 + echo >&2 else - echo "Write-enabled deploy keys that this ruleset will let bypass review:" + echo "These write-enabled deploy keys will ALL be able to push to main," + echo "bypassing pull request review, once this ruleset exists:" + echo printf '%s\n' "$WRITE_KEYS" | sed 's/^/ /' echo - echo "Delete any that are not the release key:" + echo "Delete any that are not the release key, then re-run:" echo " gh repo deploy-key delete --repo $REPO" + echo + if [ "${DEPLOY_KEYS_AUDITED:-}" = "1" ]; then + echo "DEPLOY_KEYS_AUDITED=1 set; skipping the acknowledgement prompt." + else + read -r -p "Type 'audited' if every key above is meant to have that: " ack + if [ "$ack" != "audited" ]; then + echo "Aborted -- nothing was changed." >&2 + exit 1 + fi + fi + echo fi - echo ;; app) From aa3fab4f933173f4313f7cbfd9bb468834d32ea9 Mon Sep 17 00:00:00 2001 From: Bill Wallace Date: Thu, 27 Aug 2026 14:13:36 -0400 Subject: [PATCH 7/7] docs: correct which pushes GitHub suppresses workflow runs for Two review corrections from @jbocce. The [skip ci] note in release.yml and tools/release/README.md said App tokens, like GITHUB_TOKEN, do not retrigger workflows, and framed [skip ci] as mattering "for the deploy key". That is backwards: only GITHUB_TOKEN pushes are suppressed. An App-token push is an ordinary push and would retrigger the release workflow on main just as a deploy-key push would. No bug today -- [skip ci] covers both routes -- but read as written, someone on the App route could conclude it was redundant and remove it, which loops releases. Reworded so it reads as load-bearing on both routes. The deploy-key warning in the README also named `Codecs CircleCI` (id 108740348) as a key to delete. It has since been deleted; the release key is now the only write-enabled key on the repo. Left in, a reader who checks and finds it missing may take the whole warning for stale and skip the audit, so the specific name and id are gone and the general rule stays. setup-branch-ruleset.sh keeps its mention -- it is past tense there, explaining why the typed acknowledgement exists. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/release.yml | 11 ++++++----- tools/release/README.md | 17 ++++++++--------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 17c1a71..53b6746 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -327,11 +327,12 @@ jobs: # rather than persisted, because checkout was told not to keep them in # .git/config while `pnpm install` runs. # - # The commit message carries [skip ci] and that matters for the deploy - # key: GitHub suppresses workflow runs for GITHUB_TOKEN pushes and App - # tokens are likewise not user credentials, but a deploy-key push is an - # ordinary push and WOULD retrigger this workflow on main. [skip ci] is - # what stops that being a release loop, so do not remove it. + # The commit message carries [skip ci], and it is load-bearing on BOTH + # of the routes that can actually push. GitHub suppresses workflow runs + # only for pushes made with GITHUB_TOKEN; an App-token push and a + # deploy-key push are both ordinary pushes and WOULD retrigger this + # workflow on main. [skip ci] is what stops that being a release loop, + # so do not remove it -- on either route. # # --atomic: all refs land or none do. Without it git updates each ref # independently, and the 2026-08-24 run (32733067241) showed what that diff --git a/tools/release/README.md b/tools/release/README.md index 6d1523c..90c7d72 100644 --- a/tools/release/README.md +++ b/tools/release/README.md @@ -168,17 +168,16 @@ why it works without org access. > [!WARNING] > The `DeployKey` bypass actor takes `actor_id: null` — it is a **category, not a specific key**. > Every write-enabled deploy key on the repo, present and future, can then push to `main` without -> review. The script lists them and asks you to look. Audit before enabling, and delete any left over -> from retired CI: +> review. The script lists them and makes you acknowledge the list by name before it creates +> anything. Audit before enabling, and delete any left over from retired CI — a key nobody uses stops +> being merely unused and becomes one that bypasses branch protection: > > ```bash > gh repo deploy-key list --repo cornerstonejs/codecs > gh repo deploy-key delete --repo cornerstonejs/codecs > ``` > -> As of this writing there is a read-write key `Codecs CircleCI` (id 108740348, added 2024-09-18). -> CircleCI no longer runs anything for this repo, so it should be deleted rather than promoted into a -> credential that bypasses branch protection. +> The release key is the only write-enabled key that should appear. Anything else is a finding. #### Route B — GitHub App (org owner) @@ -206,10 +205,10 @@ bypass actor. Review requirements for humans are unchanged: 1 approving review, stale reviews dismissed on push, last-push approval, no force pushes, no branch deletion. See the script's header for why the classic rule has to go rather than sit alongside the ruleset. -One behavioural note for the deploy-key route: GitHub suppresses workflow runs for `GITHUB_TOKEN` -pushes, and App tokens are likewise not user credentials, but a deploy-key push is an ordinary push -and *would* retrigger the release workflow on `main`. The `[skip ci]` in the release commit message -is what prevents a loop — do not remove it. +One behavioural note that applies to both routes: GitHub suppresses workflow runs only for pushes +made with `GITHUB_TOKEN`. An App-token push and a deploy-key push are both ordinary pushes and +*would* retrigger the release workflow on `main`. The `[skip ci]` in the release commit message is +what prevents a loop — do not remove it, whichever route you set up. 3. **Verify**, then re-run the failed Release workflow: