From 12e2c1652e8175404738af8187f080bc40210150 Mon Sep 17 00:00:00 2001 From: Noah Schatz Date: Mon, 19 Oct 2026 23:11:17 +0000 Subject: [PATCH 1/2] Publish through npm trusted publishing, and ask for the release approval only when there is something to release The release workflow no longer reads an npm token. The publish job's `id-token: write` is the npm credential: npm exchanges the runner's OIDC ID token for a short-lived publish token per package, checked against each package's trusted publisher (cosyte/config, release.yml, release). - pnpm 10.34.5 hands the upload to the npm CLI on PATH, and Node 22.14 ships npm 10, so the publish job installs exactly npm 11.21.0 and reds if another npm is first on PATH. npm 12 needs Node 22.22.2. - The publish job's setup-node takes no registry-url, so no npmrc with a token slot is generated. changesets/action v1.9.0 (the pinned sha) writes no .npmrc when it has no token and OIDC is available. - The credential declaration replaces NPM_TOKEN with GITHUB_OIDC_ID_TOKEN, exposed through a new `runner` route that is satisfied by the job's permission grant. credential-surface.mjs refuses `id-token: write` on an ungated job, and the publish preflight refuses before the build when the OIDC variables are missing. The publish job used to start on every push to main without pending changesets, so the release environment asked for an approval on every ordinary push. It now starts only for a version commit, or when the registry does not report every package's current version as present (registry-presence.mjs `pending`, asked in the ungated preflight). The only skipping answer is the state in which `changeset publish` would publish nothing; an unreadable registry keeps the job in. test/release-arm-condition.test.ts evaluates both jobs' committed `if:` lines on every path. RELEASING.md retracts its claim that pnpm carries trusted publishing, and gives the owner's `npm trust github` step per package, which each package needs before its first trusted publish. Harness-Task: 108 Co-Authored-By: Claude Opus 5.5 --- .github/credential-surface.json | 36 ++-- .github/workflows/release.yml | 191 +++++++++++++----- README.md | 4 +- RELEASING.md | 259 ++++++++++++++---------- npm-config-allow.json | 4 +- scripts/credential-surface.mjs | 82 +++++++- scripts/publish-preflight.mjs | 28 ++- scripts/registry-presence.mjs | 174 +++++++++++++++- test/credential-surface.test.ts | 129 ++++++++---- test/npm-config-allow.test.ts | 79 ++++---- test/publish-preflight.test.ts | 72 +++++-- test/registry-presence.test.ts | 244 +++++++++++++++++++++++ test/release-arm-condition.test.ts | 306 +++++++++++++++++++++++++++++ 13 files changed, 1319 insertions(+), 289 deletions(-) create mode 100644 test/release-arm-condition.test.ts diff --git a/.github/credential-surface.json b/.github/credential-surface.json index d0af9bc..cbe4911 100644 --- a/.github/credential-surface.json +++ b/.github/credential-surface.json @@ -17,38 +17,34 @@ }, "credentials": [ { - "name": "NPM_TOKEN", - "tokenClass": "npm Automation token, or a granular access token with publish rights on the @cosyte scope. A classic Publish token demands a 2FA one-time password CI cannot supply and dies with EOTP at the last step, after a green build.", - "storage": "organization", - "storageNote": "An organization secret on the Cosyte org, shared by every @cosyte/* repository. A repository-level NPM_TOKEN silently OVERRIDES the organization one, so it must live in exactly one place; rotating it affects every @cosyte/* repository, not just this one.", + "name": "GITHUB_OIDC_ID_TOKEN", + "tokenClass": "A GitHub Actions OIDC ID token, minted on request for a job that grants `id-token: write`, with the audience `npm:registry.npmjs.org`. npm 11.5.1 or later exchanges it, package by package, for a short-lived publish token, and the registry issues one only when the token's claims match that package's trusted publisher: repository cosyte/config, workflow release.yml, environment release. Nothing long-lived exists to issue, store or leak.", + "storage": "github-provided", + "storageNote": "Never stored. The runner sets ACTIONS_ID_TOKEN_REQUEST_URL and ACTIONS_ID_TOKEN_REQUEST_TOKEN in every step of a job that grants `id-token: write`, and in no other job; the ID token itself is requested by npm inside `npm publish` and is never written to a file, a secret or the job's environment. What can drift is WHICH jobs hold the grant, and the GITHUB_TOKEN permissions below pin that per job. The trusted publisher that makes the exchange succeed lives on npm, per package, and is the owner's to register.", "requiredForPublish": true, "registryAuth": true, "exposures": [ { "job": "publish", "step": "Publish", - "as": "env", - "name": "NPM_TOKEN", - "mode": "value" + "as": "runner", + "name": "ACTIONS_ID_TOKEN_REQUEST_URL", + "mode": "value", + "grantedBy": "id-token: write", + "note": "Set by the runner in every step of the publish job, not by the workflow. The Publish step is the consumer: changesets/action checks for both variables to choose trusted publishing, and npm, run by pnpm under `changeset publish`, requests the ID token with them." }, { "job": "publish", "step": "Publish", - "as": "env", - "name": "NODE_AUTH_TOKEN", - "mode": "value" - }, - { - "job": "publish", - "step": "The publish configuration must be one the allow-set permits", - "as": "env", - "name": "NODE_AUTH_TOKEN", + "as": "runner", + "name": "ACTIONS_ID_TOKEN_REQUEST_TOKEN", "mode": "value", - "note": "S0081. The configuration allow-check runs earlier in this same gated job and must resolve the SAME configuration the publish resolves. The user npmrc actions/setup-node generates carries `//registry.npmjs.org/:_authToken=${NODE_AUTH_TOKEN}`, and npm expands that reference while resolving; without the variable this step would read an unresolvable reference, which that gate is fail-closed about. NPM_TOKEN itself is deliberately NOT exposed here: nothing on the configuration path reads it, so copying it would widen this surface for nothing. The check prints every credential-denoting key with a fixed redaction marker instead of its value, and scrubs credential-shaped text from every byte it emits; test/npm-config-allow.test.ts asserts both, on a pass and on a refusal." + "grantedBy": "id-token: write" } ], - "issuedForms": [{ "id": "npm-automation-token", "pattern": "npm_[A-Za-z0-9]{36}" }], - "retiredWhen": "An npm Trusted Publisher is registered for all eight @cosyte/* packages against org Cosyte, repository config, workflow release.yml, environment release. Removing this token before that publisher exists breaks every publish, so the order is not negotiable. RELEASING.md, 'Still deferred: OIDC trusted publishing', has the sequence." + "issuedForms": [], + "issuedFormsNote": "Empty, and explicit. The ID token is requested by npm and sent only as a request header, which npm's debug log does not record (npm 11.21.0's npm-registry-fetch logs the method, the status and the URL), and the token npm receives in exchange is held in npm's memory for one command and written to no file. The npm token shapes the log collector already redacts stay in release.yml regardless.", + "retiredWhen": "This repository stops publishing to npm from GitHub Actions, or npm retires trusted publishing. Removing `id-token: write` from the publish job retires nothing: it stops every publish, and the publish preflight refuses before the build when the two variables are missing." }, { "name": "RELEASE_PR_TOKEN", @@ -129,7 +125,7 @@ "issuedForms": [ { "id": "actions-installation-token", "pattern": "gh[pousr]_[A-Za-z0-9]{36,}" } ], - "retiredWhen": "Never, while this repository uses GitHub Actions. What retires is a GRANT: `id-token: write` on the publish job becomes the ONLY npm credential once trusted publishing is registered, and `pull-requests: write` on the version job retires if the Version Packages PR ever stops being opened by this workflow." + "retiredWhen": "Never, while this repository uses GitHub Actions. What retires is a GRANT: `pull-requests: write` on the version job retires if the Version Packages PR ever stops being opened by this workflow. `id-token: write` on the publish job does not retire: it is what GITHUB_OIDC_ID_TOKEN, the only npm credential, comes from." } ], "settings": [ diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ae23f4c..e6cf6fd 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -10,7 +10,7 @@ concurrency: permissions: contents: write # create the version PR / tags / releases - id-token: write # npm provenance + id-token: write # npm trusted publishing and provenance, granted to the publish job only pull-requests: write # open the "Version Packages" PR # Changesets flow: a push to main with pending changesets opens a "Version Packages" PR. Merging that @@ -34,7 +34,7 @@ permissions: # gate ran AFTER the approval, so an approver could be asked to approve a run that could not # succeed. They now run before anyone is asked for anything. # * `version` (UNGATED) opens or refreshes the Version Packages PR. It is given NO `publish:` input -# and NO npm credentials, so it cannot reach the registry even if something else here is wrong. +# and NO `id-token: write`, so it cannot reach the registry even if something else here is wrong. # It carries its own copy of the Gate 1 accounting check, because the split would otherwise have # let one push (a version commit that ALSO carries a pending changeset) skip both arms' checks # and go green with packages bumped and unpublished. See that step for the full argument. @@ -44,22 +44,57 @@ permissions: # where the risk is. # # WHY THE ARM CONDITION IS NOT A CLASSIFIER, and this is the part not to "simplify". This repo -# deliberately refuses to gate the publish on `release-notes.mjs`'s `is-release` output, because a -# classifier that wrongly answered "not a release" would WITHHOLD A PUBLISH ON A GREEN RUN, which is -# the exact silent-withholding failure Gate 1 exists to close (run 30640138565, 2026-07-31). That -# refusal is unchanged. The arm condition below is `has-changesets`, which is the SAME question -# `changesets/action` asks itself to choose its own arm ("are there pending changesets"), computed -# from the same directory by the same rule. Moving that question one step earlier cannot change what -# publishes: on the branch where it is wrong, the action would have taken the version arm and -# published nothing anyway. +# deliberately refuses to gate the publish on `release-notes.mjs`'s `is-release` output ALONE, +# because a classifier that wrongly answered "not a release" would WITHHOLD A PUBLISH ON A GREEN RUN, +# which is the exact silent-withholding failure Gate 1 exists to close (run 30640138565, +# 2026-07-31). That refusal is unchanged. The arm condition below is `has-changesets`, which is the +# SAME question `changesets/action` asks itself to choose its own arm ("are there pending +# changesets"), computed from the same directory by the same rule. Moving that question one step +# earlier cannot change what publishes: on the branch where it is wrong, the action would have taken +# the version arm and published nothing anyway. +# +# THE PUBLISH ARM ALSO ASKS WHETHER THERE IS ANYTHING TO PUBLISH (2026-10-19). Before this, every +# push to `main` with no pending changesets started the gated job, so the `release` environment asked +# for an approval on every ordinary push, including the many with nothing to release, and the runs +# piled up waiting. The publish job now starts only when the push is a version commit (`is-release`) +# OR the registry does not report every package's current version as present (`has-unpublished`, +# from scripts/registry-presence.mjs `pending`). The second question is the one `changeset publish` +# itself asks before publishing each package, over the same manifests, so skipping the job on its +# "nothing" answer skips a `changeset publish` that would have published nothing. Every other answer +# keeps the job in: a version the registry lacks, a registry that could not be read (`unknown`), and +# a step that did not report at all. So the classifier can still only ADD a run, never withhold one. +# The paths, each with what it costs: +# +# pending changesets -> `version` runs, `publish` skipped, nothing waits +# no changesets, version commit -> `publish` runs and asks for its approval +# no changesets, a version npm lacks -> `publish` runs and asks for its approval +# no changesets, registry unreadable -> `publish` runs and asks for its approval +# no changesets, every version on npm -> `publish` skipped, no deployment request +# +# test/release-arm-condition.test.ts evaluates the two jobs' `if:` lines, read out of this file, on +# every one of those paths. # --------------------------------------------------------------------------- # -# AUTH TODAY: NPM_TOKEN. This repo is PUBLIC, so npm provenance is already live; `NPM_CONFIG_PROVENANCE` -# auto-enables below on the public visibility check. The runner toolchain floor is bumped for OIDC -# (packageManager `pnpm@10.34.5` >= 10.16, Node `22.14`; publish runs via `pnpm run release`, so pnpm, -# not npm, carries trusted publishing). What remains for tokenless OIDC is a FOUNDER step, not a build: -# register the npm-side Trusted Publisher, then remove NPM_TOKEN / NODE_AUTH_TOKEN here. RELEASING.md -# has the turnkey steps. Until then, NPM_TOKEN must stay an npm *Automation* token. +# AUTH: npm TRUSTED PUBLISHING (OIDC), AND NO npm TOKEN ANYWHERE IN THIS FILE. The `publish` job +# grants `id-token: write`, so the runner hands it the OIDC token request variables, and npm 11.5.1 +# or later exchanges a GitHub-signed ID token for a short-lived publish token per package. npm checks +# that token's claims against the trusted publisher registered on each package: repository +# `cosyte/config`, workflow `release.yml`, environment `release`. Nothing long-lived is stored. +# +# * THE npm VERSION IS LOAD-BEARING. `changeset publish` runs `pnpm publish`, and pnpm 10.34.5 packs +# the tarball and then runs the `npm` CLI on PATH to upload it (`runNpm(opts.npmPath, ["publish", +# "--ignore-scripts", ])` in its dist/pnpm.cjs, read 2026-10-19), so pnpm does NOT carry +# trusted publishing on its own. Node 22.14 ships npm 10, which has no OIDC support, so the publish +# job installs an exact npm 11 first. See that step. +# * changesets/action at the sha pinned below (v1.9.0) writes no `.npmrc` when it is given no +# npm token and the OIDC variables are present, and logs "using npm trusted publishing". +# * npm accepts trusted publishing from GitHub-hosted runners only, which `ubuntu-latest` is. +# * Provenance stays on: this repo is PUBLIC and `NPM_CONFIG_PROVENANCE` is set from that below. +# +# A PACKAGE PUBLISHES THIS WAY ONLY ONCE ITS TRUSTED PUBLISHER IS REGISTERED ON npm, which is the +# owner's step (`npm trust github`), done at the first release that needs it. RELEASING.md, "npm +# trusted publishing", has the commands. Until then a publish fails at the registry with an +# authentication error and publishes nothing. # # --------------------------------------------------------------------------- # WHY THIS IS STILL A STANDALONE WORKFLOW AND NOT A THIN CALLER OF @@ -101,6 +136,7 @@ jobs: contents: read outputs: has-changesets: ${{ steps.pending.outputs.has-changesets }} + has-unpublished: ${{ steps.unpublished.outputs.has-unpublished }} is-release: ${{ steps.notes.outputs.is-release }} packages: ${{ steps.notes.outputs.packages }} steps: @@ -181,6 +217,19 @@ jobs: echo "No pending changesets: this push takes the PUBLISH arm, gated on the release environment." fi + # IS THERE ANYTHING FOR THE PUBLISH ARM TO DO. Asked only on the publish arm, and only so that + # the gated job, and its request for an approval, can be left out when the answer is "nothing". + # The header says why this cannot withhold a publish: only "every version is on the registry" + # skips the job, and that is the state in which `changeset publish` publishes nothing. + # + # Before install, like the gates above: scripts/registry-presence.mjs is zero-dependency and + # reads the PUBLIC registry, so this job needs no credential for it. A step that fails here + # reds `preflight`, and then neither arm runs. + - name: Is any package version missing from the registry + id: unpublished + if: ${{ steps.pending.outputs.has-changesets == 'false' }} + run: node scripts/registry-presence.mjs pending --output "$GITHUB_OUTPUT" + - run: pnpm install --frozen-lockfile - name: Verify run: | @@ -194,7 +243,8 @@ jobs: # --------------------------------------------------------------------------- # VERSION ARM. Ungated. Opens or refreshes the "Version Packages" PR and NOTHING ELSE. No - # `publish:` input, no NPM_TOKEN, no `id-token`, so this job cannot reach the registry. + # `publish:` input and no `id-token`, which is the only npm credential, so this job cannot reach the + # registry. # --------------------------------------------------------------------------- version: needs: preflight @@ -258,8 +308,9 @@ jobs: version: pnpm run version # NO `publish:` INPUT, DELIBERATELY, AND IT IS THIS JOB'S WHOLE SAFETY PROPERTY. The action # only ever calls `runPublish` when a publish script was supplied; with none, the version - # arm is the only arm it can take. Combined with the absent NPM_TOKEN below, an ungated job - # that reached the registry would need two independent mistakes rather than one. + # arm is the only arm it can take. Combined with the absent `id-token: write` on this job, + # an ungated job that reached the registry would need two independent mistakes rather + # than one. createGithubReleases: false env: # The other half of the version-PR fix: this is the token the action opens the PR with. It @@ -276,8 +327,8 @@ jobs: # need `Workflows: write`: `pnpm run version` changes `packages/*/package.json`, # `packages/*/CHANGELOG.md` and `.changeset/`, none of which is under `.github/workflows/`. # - # NOTE THE ABSENCE OF NPM_TOKEN / NODE_AUTH_TOKEN HERE. That absence is a control, not an - # oversight: see the `publish:` comment above. + # NOTE THE ABSENCE OF ANY npm CREDENTIAL HERE, AND OF `id-token: write` ON THIS JOB. That + # absence is a control, not an oversight: see the `publish:` comment above. GITHUB_TOKEN: ${{ secrets.RELEASE_PR_TOKEN || secrets.GITHUB_TOKEN }} # THE GATE 1 SAFETY NET, RESTORED ON THIS ARM. Do not delete it as a duplicate of the step in @@ -324,18 +375,21 @@ jobs: # --------------------------------------------------------------------------- # PUBLISH ARM. GATED on the protected `release` environment, which is the human acknowledgment and - # stays exactly where it was. This is the only job in this file that can reach npm. + # stays exactly where it was. This is the only job in this file that can reach npm. It starts only + # when there is something to release: the header's table of paths is the whole rule. # --------------------------------------------------------------------------- publish: needs: preflight - if: ${{ needs.preflight.outputs.has-changesets != 'true' }} + if: ${{ needs.preflight.outputs.has-changesets != 'true' && (needs.preflight.outputs.is-release == 'true' || needs.preflight.outputs.has-unpublished != 'false') }} + # GitHub-hosted, and it must stay so: npm trusted publishing does not accept self-hosted runners. runs-on: ubuntu-latest permissions: contents: write # create tags / GitHub releases - id-token: write # npm provenance + id-token: write # npm trusted publishing (the only npm credential) and provenance # Release approval gate: the `release` environment carries a required reviewer (`NSchatz`) and a - # `main`-only deployment-branch policy, so the publish waits for an explicit approval. This is - # also the OIDC subject an npm Trusted Publisher would bind to. + # `main`-only deployment-branch policy, so the publish waits for an explicit approval. It is also + # the environment each package's npm trusted publisher is registered with (RELEASING.md), so npm + # mints a publish token for these packages only to a job that went through this gate. environment: release steps: - uses: actions/checkout@v7 @@ -343,23 +397,54 @@ jobs: fetch-depth: 0 persist-credentials: false - uses: pnpm/action-setup@v6 + # NO `registry-url`, DELIBERATELY. With it, `actions/setup-node` generates a user npmrc carrying + # `//registry.npmjs.org/:_authToken=${NODE_AUTH_TOKEN}` and, in v6, exports a placeholder value + # for that variable when none is given. Under trusted publishing there is no token to put + # there, so the slot would hold the placeholder: sent to the registry whenever the OIDC + # exchange fails, and an unresolvable reference (which Gate 3 refuses) on a later setup-node + # that stops exporting it. Without the input npm uses its own default registry, which Gate 3's + # `require` rule still pins, and a failed exchange reads as "not logged in". - uses: actions/setup-node@v6 with: node-version: "22.14" - registry-url: https://registry.npmjs.org cache: pnpm + # TRUSTED PUBLISHING NEEDS npm 11.5.1 OR LATER (https://docs.npmjs.com/trusted-publishers, read + # 2026-10-19), AND NODE 22.14 SHIPS npm 10. pnpm hands the upload to whichever `npm` is first on + # PATH (see the header), so this is the npm that publishes. + # + # PINNED EXACTLY, AND CHECKED. 11.21.0 was published to npm on 2026-09-30. npm 12 is not an + # option on this Node: it requires `^22.22.2 || ^24.15.0 || >=26.0.0`. A different npm on PATH + # after the install reds here, before the configuration check and the build. + # + # BEFORE GATE 3, on purpose: Gate 3 asks `npm config ls` for the effective configuration, and + # the answer has to come from the same npm that publishes, or it is evidence about the wrong + # process. + - name: Install the npm that publishes + shell: bash + run: | + set -euo pipefail + want=11.21.0 + npm install --global --no-audit --no-fund "npm@$want" + found=$(npm --version) + if [ "$found" != "$want" ]; then + echo "::error title=Wrong npm on the publish path::npm $found is first on PATH after" \ + "installing npm $want. Trusted publishing needs npm 11.5.1 or later, and this job" \ + "publishes with whatever npm is first on PATH." + exit 1 + fi + echo "npm $found is first on PATH; it is the npm that uploads every tarball." + # GATE 3: THE EFFECTIVE CONFIGURATION OF THIS PUBLISH MUST BE ONE SOMEBODY APPROVED. # # THE HOLE THIS CLOSES (S0055 finding F9). Everything that decides WHERE these tarballs go, # WHAT goes inside them and WHAT METADATA rides along is npm/pnpm configuration, and that # configuration is assembled at publish time out of sources nothing in this repository used to - # inspect: a global npmrc, the user npmrc `actions/setup-node` GENERATES two steps above and - # points NPM_CONFIG_USERCONFIG at, a repository or per-package `.npmrc`, `pnpm-workspace.yaml`, - # the root manifest's `pnpm` block, each package's `publishConfig`, `.changeset/config.json`, - # and this job's environment. A redirected `registry`, a disabled `provenance`, a widened - # `access` or an injected lifecycle-script setting changes what reaches the public registry - # WITHOUT changing a tracked file in a way review would see. + # inspect: a global npmrc, a user npmrc, a repository or per-package `.npmrc`, + # `pnpm-workspace.yaml`, the root manifest's `pnpm` block, each package's `publishConfig`, + # `.changeset/config.json`, and this job's environment. A redirected `registry`, a disabled + # `provenance`, a widened `access` or an injected lifecycle-script setting changes what reaches + # the public registry WITHOUT changing a tracked file in a way review would see. # # WHY IT RUNS HERE AND NOT IN `preflight`, and this is the part not to "simplify". Effective # configuration is a property of a SPECIFIC PROCESS: its environment, its working directory, @@ -369,36 +454,30 @@ jobs: # than no answer, because the conclusion is the only thing anyone reads. The accepted cost is # that a refusal here happens after an approver has already approved the run. # - # THE `env:` BLOCK BELOW IS THE PUBLISH STEP'S, MINUS EXACTLY ONE VARIABLE, AND BOTH HALVES OF - # THAT ARE DELIBERATE. It is DECLARED in .github/credential-surface.json (S0080), which is what - # made this decision a reviewed one rather than a default. + # THE `env:` BLOCK BELOW IS THE PUBLISH STEP'S npm CONFIGURATION, AND NOTHING ELSE. It is + # DECLARED in .github/credential-surface.json (S0080), which is what made this decision a + # reviewed one rather than a default. # # * NPM_CONFIG_PROVENANCE is npm CONFIGURATION supplied at STEP level, so a check that ran # without it would be reporting on a configuration the publish does not have. That is the # same class of error as running this check in `preflight`. - # * NODE_AUTH_TOKEN is here because the user npmrc `actions/setup-node` generates two steps - # above contains `//registry.npmjs.org/:_authToken=${NODE_AUTH_TOKEN}`, and npm expands - # that reference when it resolves configuration. Without the variable this step would be - # reading a DIFFERENT state of that file than the publish reads: an unresolvable - # reference, which this gate is fail-closed about. It would refuse every release, or worse - # depend on the npm version's tolerance for it. - # * NPM_TOKEN is NOT here, and that omission is the point of naming this block at all. - # Nothing on the configuration path reads it: npm reads only `npm_config_*` variables, and - # the registry credential reaches npm through the generated npmrc above. It is in the - # publish step because `changesets/action` wants it. Copying it here would widen this - # repository's declared credential surface by one step and buy this check nothing, and a - # surface that grows for nothing is exactly what credential-surface.mjs exists to refuse. + # * No credential is here, because there is none to resolve: the publish authenticates + # through the OIDC exchange npm performs inside `npm publish`, which no configuration + # source carries, and setup-node above generates no npmrc. The publish step's + # GITHUB_TOKEN is not npm configuration and is not copied either: a surface that grows + # for nothing is exactly what credential-surface.mjs exists to refuse. # # If the publish step's npm CONFIGURATION env changes, change this with it, and update # .github/credential-surface.json in the same commit. The check never prints a credential # value; see its redaction rule and the suite that proves it (test/npm-config-allow.test.ts). # - # IT RUNS BEFORE `pnpm install`, so nothing has been fetched or built under a configuration - # nobody approved, and it is zero-dependency for that reason. It ADDS a refusal after the - # existing environment gate; it removes, loosens and relocates nothing. + # IT RUNS BEFORE `pnpm install`, so none of this repository's dependencies has been fetched or + # built under a configuration nobody approved, and it is zero-dependency for that reason. The + # one fetch before it is the exact npm pin in the step above, which has to come first for the + # reason given there. It ADDS a refusal after the existing environment gate; it removes, + # loosens and relocates nothing. - name: The publish configuration must be one the allow-set permits env: - NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} NPM_CONFIG_PROVENANCE: ${{ github.event.repository.visibility == 'public' }} run: node scripts/npm-config-allow.mjs @@ -439,8 +518,12 @@ jobs: createGithubReleases: false env: GITHUB_TOKEN: ${{ secrets.RELEASE_PR_TOKEN || secrets.GITHUB_TOKEN }} - NPM_TOKEN: ${{ secrets.NPM_TOKEN }} - NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + # NO npm TOKEN, DELIBERATELY. The job's `id-token: write` is the publish credential: the + # runner gives every step the OIDC token request variables, changesets/action sees no + # token and leaves npm's configuration alone, and npm exchanges the ID token per package. + # The publish preflight (`pnpm run release`) refuses before the build when those variables + # are missing, which is what removing the grant would cause. + # # npm provenance requires a PUBLIC source repo; auto-enables when this repo is public. NPM_CONFIG_PROVENANCE: ${{ github.event.repository.visibility == 'public' }} diff --git a/README.md b/README.md index 7ffbc50..d856d6c 100644 --- a/README.md +++ b/README.md @@ -177,8 +177,8 @@ line does and does not claim are in [ADR 0002](documentation/decisions/0002-the-0-1-0-version-line.md), and the per-package audit behind it is [`documentation/release-0.1.0-audit.md`](documentation/release-0.1.0-audit.md). Releases are managed with Changesets and, once the environment is created (a one-time setup step), gated on a -protected `release` environment; the full pipeline (and the OIDC and npm provenance migration -deferred to launch) is documented in [`RELEASING.md`](RELEASING.md). +protected `release` environment and published through npm trusted publishing with provenance; the +full pipeline is documented in [`RELEASING.md`](RELEASING.md). ## Contributing diff --git a/RELEASING.md b/RELEASING.md index 216a588..e2c13cb 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -37,21 +37,46 @@ waiting on a human is the step that produces a pull request, which anyone can cl Every step names its actor, what triggers it, and how you know it finished. Steps that wait on a human carry a budget. -| # | step | actor | trigger | signal it completed | wait budget | -| --- | ---------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------- | -------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------- | -| 1 | Land a change with a changeset (`pnpm changeset`) | contributor | a pull request into `main` | the PR merges with a `.changeset/*.md` file in it | none (ordinary review) | -| 2 | `preflight` job: both release gates, then the verify ladder | automation, ungated | push to `main` | the `preflight` job is green in the `Release` run | none, nothing waits | -| 3 | `version` job: open or refresh the "Version Packages" PR | automation, ungated | `preflight` green **and** at least one pending changeset | a PR titled `Version Packages` on branch `changeset-release/main` exists or was force-pushed, and its required checks are running | none, nothing waits | -| 4 | Merge the "Version Packages" PR | **release owner** (today: `NSchatz`) | that PR is open and its required checks are green | the PR is merged; a version commit lands on `main` | **24 hours** from the moment the PR's checks go green | -| 5 | `publish` job: `changeset publish`, then tag and release each bumped package | automation, **gated on the protected `release` environment** | push to `main` with **no** pending changesets, and `preflight` green | the run leaves `waiting`, `npm view @` answers for every bumped package, and one GitHub release per `@` tag exists | **24 hours** from the run entering `waiting` | -| 6 | Approve the publish | **release approver** (today: `NSchatz`, the required reviewer on the `release` environment) | the `publish` job enters `waiting` | the deployment is approved and the job starts | counted inside step 5's budget | +| # | step | actor | trigger | signal it completed | wait budget | +| --- | ---------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------- | +| 1 | Land a change with a changeset (`pnpm changeset`) | contributor | a pull request into `main` | the PR merges with a `.changeset/*.md` file in it | none (ordinary review) | +| 2 | `preflight` job: both release gates, then the verify ladder | automation, ungated | push to `main` | the `preflight` job is green in the `Release` run | none, nothing waits | +| 3 | `version` job: open or refresh the "Version Packages" PR | automation, ungated | `preflight` green **and** at least one pending changeset | a PR titled `Version Packages` on branch `changeset-release/main` exists or was force-pushed, and its required checks are running | none, nothing waits | +| 4 | Merge the "Version Packages" PR | **release owner** (today: `NSchatz`) | that PR is open and its required checks are green | the PR is merged; a version commit lands on `main` | **24 hours** from the moment the PR's checks go green | +| 5 | `publish` job: `changeset publish`, then tag and release each bumped package | automation, **gated on the protected `release` environment** | push to `main` with **no** pending changesets, `preflight` green, **and** something to release (below) | the run leaves `waiting`, `npm view @` answers for every bumped package, and one GitHub release per `@` tag exists | **24 hours** from the run entering `waiting` | +| 6 | Approve the publish | **release approver** (today: `NSchatz`, the required reviewer on the `release` environment) | the `publish` job enters `waiting` | the deployment is approved and the job starts | counted inside step 5's budget | The two arms in steps 3 and 5 are exclusive and the workflow chooses between them by asking the same question `changesets/action` asks itself: **are there pending changesets in `.changeset/`?** If yes, -the ungated version arm runs and nothing is published. If no, the gated publish arm runs. Nothing -else decides it, and in particular the `release-notes.mjs` classifier does not: a classifier that -wrongly said "not a release" would withhold a publish on a green run, which is the exact failure the -changeset guard exists to close. +the ungated version arm runs and nothing is published. If no, the gated publish arm is the one that +can run. The `release-notes.mjs` classifier does not choose the arm: a classifier that wrongly said +"not a release" would withhold a publish on a green run, which is the exact failure the changeset +guard exists to close. + +**The publish arm runs only when there is something to release** (2026-10-19). Until then it ran on +every push to `main` without pending changesets, so the `release` environment asked for an approval +on every ordinary push and the runs piled up waiting. Now `preflight` also asks the public registry +whether every package's current version is already there (`scripts/registry-presence.mjs pending`), +and the `publish` job's condition is: + +```yaml +if: ${{ needs.preflight.outputs.has-changesets != 'true' && (needs.preflight.outputs.is-release == 'true' || needs.preflight.outputs.has-unpublished != 'false') }} +``` + +| push | `has-changesets` | `is-release` | `has-unpublished` | what runs | +| ------------------------------------------------------ | ---------------- | ------------ | ----------------- | ---------------------------------------------------------- | +| carries a pending changeset | `true` | any | (not asked) | `version`; `publish` skipped, no deployment request | +| a version commit (a Version Packages PR merged) | `false` | `true` | any | `publish`, which waits for its approval | +| no changesets, a package version npm does not have | `false` | `false` | `true` | `publish`, which waits for its approval | +| no changesets, the registry could not be read | `false` | `false` | `unknown` | `publish`, which waits for its approval | +| no changesets, every version already on npm (ordinary) | `false` | `false` | `false` | `preflight` only; `publish` skipped, no deployment request | + +This cannot withhold a publish, and the reason is the same one that makes `has-changesets` safe to +ask early. The only answer that skips the job is "every version is already on the registry", which is +the question `changeset publish` asks before publishing each package, over the same manifests, so the +skipped job would have published nothing. A version the registry lacks, a registry that did not +answer, and a step that did not report all keep the job in. `test/release-arm-condition.test.ts` +evaluates both jobs' `if:` lines, read from `release.yml`, on every row above. ### Roles, named @@ -220,8 +245,10 @@ is actually there. Both choices exist to close the same hole: the accounting reads `npm view` rather than the run's own output. 2. If the re-run reports the same packages missing, the publish genuinely failed for them. Open the run's `npm-debug-log-config-run-attempt` artifact, which the workflow uploads on failure - with credentials redacted, and read the npm error. The usual causes are an expired or wrong-typed - `NPM_TOKEN` (see the authentication section) and an npm-side 403 on a scoped package. + with credentials redacted, and read the npm error. The usual causes are a package whose npm + trusted publisher is missing or names a different repository, workflow or environment (see + [npm trusted publishing](#npm-trusted-publishing-the-owners-step-per-package)), and an npm-side + 403 on a scoped package. 3. Fix the cause and re-run again. **Do not hand-publish and do not bump the version to get past it.** A version consumed by a failed publish is not burned: the same version can be published again because nothing reached the registry under it. @@ -306,9 +333,10 @@ so a local refusal is not by itself evidence about the release. Read the source **What it protects.** Everything that decides **where** these tarballs go, **what** goes inside them and **what metadata rides along** is npm/pnpm configuration, and that configuration is assembled at -publish time out of sources nothing in this repository used to inspect: a global npmrc, the user -npmrc `actions/setup-node` **generates** on the runner and points `NPM_CONFIG_USERCONFIG` at, a -repository or per-package `.npmrc`, `pnpm-workspace.yaml`, the root manifest's `pnpm` block, each +publish time out of sources nothing in this repository used to inspect: a global npmrc, a user npmrc +(on a runner, `actions/setup-node` **generates** one when given a `registry-url`, which the `publish` +job deliberately is not), a repository or per-package `.npmrc`, `pnpm-workspace.yaml`, the root +manifest's `pnpm` block, each package's `publishConfig`, `.changeset/config.json`, and the job's own environment. A redirected `registry`, a disabled `provenance`, a widened `access` or an injected lifecycle-script setting changes what reaches the public registry **without changing a tracked file in a way review would @@ -418,14 +446,17 @@ human. The `preflight` and `version` jobs do not reference it and never wait. **If the approval budget lapses.** A `publish` run sitting in `waiting` for more than 24 hours is the release owner's to end: approve it, or cancel the run and say why in the version PR's thread. A -cancelled publish is recoverable (the version commit is on `main` and the next push re-runs the -publish arm); a forgotten one is what produces the backlog this pipeline was rebuilt to stop. +cancelled publish is recoverable (the version commit is on `main`, the registry still lacks its +versions, so the next push to `main` runs the publish arm again); a forgotten one is what produces the +backlog this pipeline was rebuilt to stop. **Do not widen the reviewer set to admit an automation identity, and do not remove `environment: release` from the `publish` job.** Either would mean a package could reach npm with no human acknowledgment, a published version is permanent, and there is no operator decision authorizing -it. If the queue is still painful after the organization-wide split lands, the next levers are a -second human reviewer or npm Trusted Publishers with OIDC, and both are the operator's call. +it. npm trusted publishing (below) changed the credential, not the gate: the trusted publisher names +this environment, so it adds a second reason the gate cannot be skipped. If the queue is still painful +after the organization-wide split lands, the next lever is a second human reviewer, and that is the +operator's call. ## Release bodies: why `createGithubReleases` is off @@ -480,21 +511,35 @@ multi-package shape, and now the ungated-version / gated-publish split, which is against `cosyte/.github` in the evidence record. If the shared workflow ever grows a multi-package mode, revisit this. -## Authentication today - -This repository is **public**, so publishing authenticates with `NPM_TOKEN`, an org-level secret -shared across the `@cosyte/*` repositories, and **with provenance**. `NPM_CONFIG_PROVENANCE` is wired -to `github.event.repository.visibility == 'public'`, so provenance is on with no workflow edit. - -`NPM_TOKEN` **must be an npm _Automation_ token** (or a granular token). A classic _Publish_ token -demands a 2FA one-time password that CI cannot supply, and the publish dies with `EOTP This operation -requires a one-time password from your authenticator`: after a green build, at the very last step. -Note that a repository-level `NPM_TOKEN` silently overrides the org-level one, so keep the token in -exactly one place. - -`NPM_TOKEN` and `NODE_AUTH_TOKEN` are supplied to the `publish` job **only**. The ungated `version` -job has neither, and is given no `publish:` input either, so reaching the registry from an ungated job -would take two independent mistakes rather than one. Keep it that way. +## Authentication: npm trusted publishing + +Publishing authenticates through **npm trusted publishing (OIDC)**, with **no npm token anywhere** in +`release.yml` and none read by this repository. The `publish` job grants `permissions: id-token: +write`, so the runner gives its steps the OIDC token request variables; inside `npm publish`, npm +requests a GitHub-signed ID token and exchanges it, package by package, for a short-lived publish +token. The registry issues one only when the ID token's claims match that package's trusted +publisher: repository `cosyte/config`, workflow `release.yml`, environment `release`. This repository +is **public**, so the publish is also **with provenance**: `NPM_CONFIG_PROVENANCE` is wired to +`github.event.repository.visibility == 'public'`. + +What has to be true on the runner, each checked or pinned in `release.yml`: + +- **npm 11.5.1 or later.** [npm's trusted publishing page](https://docs.npmjs.com/trusted-publishers) + (read 2026-10-19) requires it, with Node 22.14.0 or later. `changeset publish` runs `pnpm publish`, + and pnpm 10.34.5 packs the tarball and then runs the `npm` CLI on PATH to upload it + (`runNpm(opts.npmPath, ["publish", "--ignore-scripts", ])` in its `dist/pnpm.cjs`, read + 2026-10-19), so the npm version is what decides. Node 22.14 ships npm 10, so the `publish` job + installs exactly **npm 11.21.0** (published 2026-09-30) and reds if a different npm is first on + PATH. npm 12 needs Node `^22.22.2`, so it is not an option on this Node. +- **A GitHub-hosted runner.** npm does not accept trusted publishing from self-hosted runners. +- **No token in npm's configuration.** The `publish` job's `setup-node` takes no `registry-url`, so + no npmrc with a `${NODE_AUTH_TOKEN}` slot is generated, and `changesets/action` (v1.9.0, the pinned + sha) writes no `.npmrc` when it is given no npm token and the OIDC variables are present. + +`id-token: write` is granted to the `publish` job **only**. The ungated `version` job does not have +it, and is given no `publish:` input either, so reaching the registry from an ungated job would take +two independent mistakes rather than one. Keep it that way: `id-token: write` on any other job is the +npm credential leaving the gated job, and `credential-surface.mjs` refuses it. ### The surface is declared, not described @@ -538,49 +583,50 @@ token published. Every remedy below is a human action taken against npm or GitHu the compensating action per credential is the whole of it. **A published version is permanent.** A version that has reached the registry stays there and is not -undone by revoking the credential that published it. Revoking `NPM_TOKEN` stops the **next** publish; -it does nothing about the one that already happened. npm's unpublish window is narrow and its use is a +undone by revoking the credential that published it. Revoking a package's trusted publisher stops the +**next** publish; it does nothing about the one that already happened. npm's unpublish window is narrow and its use is a founder decision with consumers on the other side of it, so the compensating action for a bad publish is a **new version**, not a withdrawal. Treat "the credential was compromised" and "the wrong bytes shipped" as two separate incidents with two separate remedies, and do not let revoking a token feel like it addressed the second. -### `NPM_TOKEN` - -**Blast radius before you start.** This is an **organization** secret shared by every `@cosyte/*` -repository. Rotating it affects all of them, not just this one, and there is no per-repository -override that is safe to introduce: a repository-level `NPM_TOKEN` silently outranks the -organization-level one, which is exactly how two tokens end up live with nobody sure which one -published. - -- **Issue.** On npmjs.com as the `@cosyte` scope owner: Access Tokens, then Generate New Token, then - **Granular Access Token** (or **Automation**). Grant it read and write on the `@cosyte` scope, - nothing else, and set the shortest expiry the release cadence tolerates. **Do not issue a classic - Publish token**: it demands a 2FA one-time password that CI cannot supply, and the publish dies with - `EOTP This operation requires a one-time password from your authenticator` after a green build, at - the very last step. -- **Install.** Set it in **exactly one place**: the `cosyte` organization secrets - (`gh secret set NPM_TOKEN --org cosyte --visibility all`). Then confirm no repository-level copy - exists anywhere it would shadow the organization one: - `gh secret list --repo cosyte/config` must not list `NPM_TOKEN`. Delete any that does. -- **Verify.** Do not verify by publishing. Run the `Release` workflow on a `main` with no pending - changesets, approve the `publish` deployment, and read the run: `changeset publish` reports "no new - packages" and the accounting step says every bumped package is on the registry. A wrong-typed token - fails there with `EOTP` and an expired one with `E401`, both **before** anything is written, because - the publish preflight (`scripts/publish-preflight.mjs`) refuses an absent or empty token before the - build even runs. -- **Revoke.** On npmjs.com, Access Tokens, revoke the old token **after** the new one is installed and - verified, not before: the two steps overlap deliberately so no release window is left with no - working token. Then re-read `gh secret list --org Cosyte` and confirm one entry, one value. -- **Compensating action.** If the token is believed compromised, invert the order: **revoke first**, - accept that every `@cosyte/*` publish is blocked until a replacement is installed, and say so in the - org channel because thirteen other repositories share it. Then `npm token list` and audit recent - versions of all eight packages (`npm view versions --json`) against this repository's tags. A - version that reached the registry cannot be pulled back by revoking the token that published it: if - the bytes are wrong, publish a corrected **new** version and, only if the content is genuinely - dangerous, take the unpublish question to the founder. Finally, harden: set the packages and the org - to "Require two-factor authentication and disallow tokens", which makes a stolen token useless and - is the same setting the OIDC cutover wants anyway. +### `GITHUB_OIDC_ID_TOKEN` + +**What it is.** The OIDC ID token GitHub mints for a job that grants `id-token: write`, which npm +exchanges for a short-lived publish token per package. Nothing is stored: the runner sets +`ACTIONS_ID_TOKEN_REQUEST_URL` and `ACTIONS_ID_TOKEN_REQUEST_TOKEN` in the `publish` job's steps, npm +requests the ID token itself, and the exchanged token lives in npm's memory for one command. So the +procedure below is about the two things that **can** be issued and revoked: the `id-token: write` +grant in `release.yml`, and each package's **trusted publisher** on npm. + +- **Issue.** The owner registers a trusted publisher on each of the eight packages, once, at the first + release that needs it: see + [npm trusted publishing](#npm-trusted-publishing-the-owners-step-per-package) for the commands. The + workflow side is already issued: `id-token: write` on the `publish` job, declared in + `.github/credential-surface.json` as the `runner` exposure this credential comes from. +- **Install.** Nothing to install in GitHub: no secret, no variable. The grant is the installation, + and it changes only in a commit that changes `release.yml` and the declaration together. The + organization's `NPM_TOKEN` secret, which other `@cosyte/*` repositories may still read, is not + referenced here, and `credential-surface.mjs` refuses a release workflow that references any secret + the declaration does not name. +- **Verify.** `npm trust list ` shows one GitHub configuration naming `cosyte/config`, + `release.yml` and `release` for every package. In a run, the `Publish` step logs + `No NPM_TOKEN found, but OIDC is available - using npm trusted publishing`, and the publish + preflight (`scripts/publish-preflight.mjs`) has already refused, before the build, any run in which + the two variables were missing. A package with no trusted publisher fails the upload with an + authentication error and publishes nothing; the accounting step then names it. +- **Revoke.** `npm trust revoke --id ` (the id from `npm trust list `) on each package, + which stops the next publish of that package from this workflow. To stop every publish from this + repository at once, remove `id-token: write` from the `publish` job and from the declaration in one + commit; the preflight then refuses every release before the build. +- **Compensating action.** If a trusted publisher is believed misused, revoke it first, then audit + recent versions of all eight packages (`npm view versions --json`) against this repository's + tags and the `Release` runs that published them; every trusted publish carries a provenance + statement naming the run. A version that reached the registry cannot be pulled back by revoking the + publisher: if the bytes are wrong, publish a corrected **new** version and, only if the content is + genuinely dangerous, take the unpublish question to the founder. Then harden: set each package to + "Require two-factor authentication and disallow tokens", which leaves trusted publishing working and + makes any stolen npm token useless. ### `RELEASE_PR_TOKEN` @@ -625,7 +671,8 @@ workflow-level grants, `id-token: write` included. - **Issue.** Not applicable, and deliberately so: no human issues this credential. The equivalent act is granting it a permission, which is an edit to a `permissions:` block in `release.yml` **and** to `.github/credential-surface.json` in the same commit. A grant added to only one of the two does not - merge. + merge. `id-token: write` is a grant of this kind with a second meaning: it is the source of + `GITHUB_OIDC_ID_TOKEN`, the npm credential, so it belongs on `publish` and on nothing else. - **Install.** Not applicable. The token is never stored. What is installed is the grant: keep every job's `permissions:` block explicit and minimal, and keep the workflow-level block as the ceiling rather than as a default anyone inherits by accident. @@ -635,15 +682,16 @@ workflow-level grants, `id-token: write` included. nothing else. - **Revoke.** Narrow or remove the grant in `release.yml` and in the declaration together, then merge. The next job mints a token without it; there is no live credential to invalidate. Removing - `id-token: write` from `publish` also turns npm provenance off, so treat that grant as part of the - publish contract rather than as hygiene. + `id-token: write` from `publish` stops every publish (it is where the npm credential comes from) and + turns npm provenance off, so treat that grant as part of the publish contract rather than as + hygiene. - **Compensating action.** A leak of this token is bounded by the job that held it and expires with that job, so the remedy is not revocation but audit: read the run's log, list what the token could reach under that job's declared grants, and check `main`'s history and this repository's releases and pull requests for anything it wrote. If a grant turns out to have been wider than the work needed, narrow it in both files in one commit. Anything the token published to npm is out of its - reach by construction: it has never held registry credentials, and the version arm holds no npm - token at all. + reach by construction: it has never held registry credentials, and the version arm holds no + `id-token: write` at all. ## Proving the pipeline without burning a version @@ -704,27 +752,38 @@ with seven red cases on a tree whose only change was a `CHANGELOG.md`. The fix i than in the caller: **`scripts/attw.mjs` strips those two keys from the environment of the `attw` child**, in both copies of the wrapper, so every scaffolded parser inherits it. -## Still deferred: OIDC trusted publishing - -**Provenance is live** (the repository is public). **OIDC trusted publishing**, publishing with no -token at all, is the remaining step. A turnkey sequence: - -1. ~~**Bump the runner toolchain floor**~~: **DONE.** `packageManager` is now `pnpm@10.34.5` - (>= 10.16) and the `setup-node` pins are `22.14` (>= 22.14) across `ci.yml` (`release-dry-run`) - and `release.yml`; `engines.node` is `>=22.14`. Since publish runs via `pnpm run release`, **pnpm** - carries OIDC trusted publishing, so the npm-CLI floor (npm >= 11.5.1) is not on the publish path - and no `npm i -g npm@...` step is needed. `pnpm/action-setup@v6` reads `packageManager`, so the - dry-run and release jobs install 10.34.5. -2. **Configure the Trusted Publisher on npm**: for each of the eight `@cosyte/*` packages: Settings, - then Trusted Publisher, then GitHub org `cosyte`, repository `config`, workflow filename - `release.yml`, environment name `release`, allowed action `npm publish`. **The environment name is - still `release` after the 2026-08-22 split**, because the publish job is the one that kept it. -3. **Remove `NPM_TOKEN` / `NODE_AUTH_TOKEN`** from the workflow and repository secrets; keep - `permissions: id-token: write` on the `publish` job (already present). -4. **Harden npm**: set the package and org to "Require two-factor authentication and disallow tokens"; - OIDC trusted publishers keep working, stolen tokens become useless. - -Steps 2 to 4 are founder steps, not a build. +## npm trusted publishing: the owner's step, per package + +The workflow side is done: no npm token in `release.yml`, `id-token: write` on the `publish` job, npm +11.21.0 on its PATH. What is left is on npm, and it is the owner's: **each of the eight packages +publishes this way only once it has a trusted publisher**, registered at the first release that needs +it. Until then a publish of that package fails at the registry with an authentication error and +publishes nothing, and the accounting step names it. + +`npm trust` needs npm 11.15.0 or later locally, two-factor authentication on the account, and write +access to the package (npm's `npm-trust` command page, read 2026-10-19). For each package: + +```bash +for pkg in eslint-config prettier-config process script-utils test-utils tsconfig tsup-config vitest-config; do + npm trust github "@cosyte/$pkg" --repo cosyte/config --file release.yml --env release --allow-publish --yes + sleep 2 +done +npm trust list @cosyte/tsconfig # repeat per package to verify +``` + +Or on npmjs.com: the package's Settings, then Trusted Publisher, then GitHub Actions with organization +`cosyte`, repository `config`, workflow filename `release.yml`, environment `release`. The workflow +filename is the file name only, extension included. **The environment name matters**: it is what ties +the token to the `release` approval, so leave it set. + +Then, once every package has published through it at least once, harden each package to "Require +two-factor authentication and disallow tokens": trusted publishing keeps working and a stolen npm +token stops working. + +**Retracted.** This section used to say that pnpm carries trusted publishing and that "the npm-CLI +floor (npm >= 11.5.1) is not on the publish path and no `npm i -g npm@...` step is needed". pnpm +10.34.5 delegates the upload to the npm CLI (see "Authentication: npm trusted publishing" above), so +the npm floor is on the publish path, and the `publish` job now installs npm 11.21.0. ## The evidence behind all of this diff --git a/npm-config-allow.json b/npm-config-allow.json index 7f775a3..0fc6980 100644 --- a/npm-config-allow.json +++ b/npm-config-allow.json @@ -26,7 +26,7 @@ { "key": "registry", "value": "https://registry.npmjs.org/", - "why": "The public npm registry, which is where these eight packages belong. actions/setup-node writes this into the user config it generates for the publish job, so it arrives as a contributed value rather than as npm's default. Only this one URL is permitted, and the `require` rule below re-checks it against the resolvers' full maps so a permissive entry here could not widen it." + "why": "The public npm registry, which is where these eight packages belong. The publish job's actions/setup-node takes no `registry-url` (release.yml says why), so on the runner this is npm's own default; where a source does contribute it, as setup-node's generated user config does when given a `registry-url`, only this one URL is permitted, and the `require` rule below re-checks it against the resolvers' full maps so a permissive entry here could not widen it." }, { "key": "provenance", @@ -36,7 +36,7 @@ { "key": "//registry.npmjs.org/:_authToken", "anyValue": true, - "why": "The registry credential actions/setup-node writes into the generated user config. Permitted by KEY and never by value: the token is a secret, this file is public, and the check refuses an allow-set that pins one. The registry it authenticates to is pinned by the `require` rule below, so this cannot become a credential offered to somewhere else." + "why": "The registry credential slot actions/setup-node writes into the user config it generates when given a `registry-url`. The publish job is given none and authenticates through npm trusted publishing, so on the runner no source contributes this key; it stays permitted for a maintainer's local run. Permitted by KEY and never by value: the token is a secret, this file is public, and the check refuses an allow-set that pins one. The registry it authenticates to is pinned by the `require` rule below, so this cannot become a credential offered to somewhere else." }, { "key": "always-auth", diff --git a/scripts/credential-surface.mjs b/scripts/credential-surface.mjs index b04456c..8516675 100644 --- a/scripts/credential-surface.mjs +++ b/scripts/credential-surface.mjs @@ -38,8 +38,8 @@ // // WHY IT PARSES THE WORKFLOW RATHER THAN GREPPING IT. Half of what this check has to answer is a // question about SCOPE ("is this secret reachable by one step or by every job"), and scope is -// structure. A grep also cannot tell prose from wiring: `release.yml`'s header comments name -// `NPM_TOKEN`, `NODE_AUTH_TOKEN` and `RELEASE_PR_TOKEN` several times in ordinary English, and a +// structure. A grep also cannot tell prose from wiring: `release.yml`'s comments name +// `RELEASE_PR_TOKEN`, `GITHUB_TOKEN` and `id-token: write` several times in ordinary English, and a // text scan would report every one of those as an exposure. So the file is parsed, comments and all, // by the deliberately small YAML subset reader below. // @@ -88,8 +88,20 @@ const MIN_LABEL_TEXT = 20; /** Storage locations a credential may declare. One credential, one home: see AC1. */ const STORAGE_LOCATIONS = ["organization", "repository", "environment", "github-provided"]; -/** How a credential reaches the thing that consumes it. */ -const EXPOSURE_ROUTES = ["env", "with"]; +/** + * How a credential reaches the thing that consumes it. + * + * `env` and `with` are a `secrets.NAME` reference the workflow writes into a step. `runner` is a + * variable the RUNNER sets in every step of a job because that job holds a permission grant, which + * is how GitHub hands out the OIDC token request (`id-token: write` sets + * `ACTIONS_ID_TOKEN_REQUEST_URL` and `ACTIONS_ID_TOKEN_REQUEST_TOKEN`). No `secrets.` reference + * exists for it, so its exposure is the grant: a `runner` exposure names the permission that causes + * it (`grantedBy`), and it is where it says only when that job grants exactly that. + */ +const EXPOSURE_ROUTES = ["env", "with", "runner"]; + +/** The shape of a `runner` exposure's `grantedBy`: one `permissions:` entry, `scope: level`. */ +const GRANT = /^([a-z][a-z-]*): (read|write)$/; /** What the workflow does with the credential at an exposure. */ const EXPOSURE_MODES = ["value", "presence-test"]; @@ -637,6 +649,11 @@ function validateExposures(exposures, label) { `${label} has an exposure whose \`mode\` is not one of ${EXPOSURE_MODES.join(", ")}`, ); } + if (exposure.as === "runner" && !GRANT.test(exposure.grantedBy ?? "")) { + problems.push( + `${label} has a \`runner\` exposure without a \`grantedBy\` of the form \`scope: read|write\``, + ); + } } return problems; } @@ -948,6 +965,7 @@ export function checkCredentialSurface({ repoRoot, declarationPath, workflowPath checkStepNameAmbiguity(declaration, jobs, findings); const matched = checkReferences(declaration, references, findings); + matchRunnerExposures(declaration, jobs, matched); checkDeclaredExposures(declaration, jobs, matched, findings); checkSettings(declaration, jobs, findings); checkPermissions(declaration, workflow, jobs, findings); @@ -1055,6 +1073,38 @@ function exposureKey(name, exposure) { return [name, exposure.job, exposure.step, exposure.as, exposure.name].join(""); } +/** + * @param job A job index entry. + * @param grantedBy A `scope: level` string. + * @returns Whether the job's own `permissions:` block grants exactly that. + */ +function jobGrants(job, grantedBy) { + const [, scope, level] = GRANT.exec(grantedBy) ?? []; + const found = readPermissions(job.node); + return scope !== undefined && found.grants !== null && found.grants[scope] === level; +} + +/** + * A `runner` exposure is where the declaration says when its step exists and its job grants the + * permission that makes the runner set the variable. There is no reference to find, so this marks + * it seen on that evidence, and `checkDeclaredExposures` reports it absent otherwise. + * + * @param declaration The declaration. + * @param jobs The job index. + * @param seen Exposure keys matched so far; extended in place. + */ +function matchRunnerExposures(declaration, jobs, seen) { + for (const credential of declaration.credentials) { + for (const exposure of credential.exposures) { + if (exposure.as !== "runner") continue; + const job = jobs.get(exposure.job); + if (job === undefined) continue; + if (!job.steps.some((step) => step.id === exposure.step)) continue; + if (jobGrants(job, exposure.grantedBy)) seen.add(exposureKey(credential.name, exposure)); + } + } +} + /** * A declaration is a promise in both directions: a credential that no longer appears where it says * it does is a failure, not a success on the grounds that nothing extra turned up. @@ -1078,9 +1128,11 @@ function checkDeclaredExposures(declaration, jobs, seen, findings) { const reason = job === undefined ? `job "${exposure.job}" does not exist in the workflow` - : job.steps.some((step) => step.id === exposure.step) - ? `that step does not set \`${exposure.as}: ${exposure.name}\` from it` - : `job "${exposure.job}" has no step named "${exposure.step}"`; + : !job.steps.some((step) => step.id === exposure.step) + ? `job "${exposure.job}" has no step named "${exposure.step}"` + : exposure.as === "runner" + ? `job "${exposure.job}" does not grant \`${exposure.grantedBy}\`, so the runner never sets \`${exposure.name}\`` + : `that step does not set \`${exposure.as}: ${exposure.name}\` from it`; findings.push({ code: "declared-exposure-absent", message: `\`${credential.name}\` is declared at ${describeDeclaredScope(exposure)} but ${reason}`, @@ -1200,7 +1252,9 @@ function checkPermissions(declaration, workflow, jobs, findings) { /** * The job that can reach the registry must reference the protected deployment environment. Checked * against the declared publish job AND against every job actually holding registry credentials, so - * hoisting `NPM_TOKEN` into an ungated job fails here as well as on scope. + * hoisting a registry secret into an ungated job fails here as well as on scope, and so does + * granting an ungated job the permission a `runner` registry credential comes from (`id-token: + * write` is enough to publish through npm trusted publishing). * * @param declaration The declaration. * @param jobs The job index. @@ -1218,6 +1272,18 @@ function checkEnvironment(declaration, jobs, references, findings) { mustBeGated.add(reference.location.job); } } + const registryGrants = new Set(); + for (const credential of declaration.credentials) { + if (!credential.registryAuth) continue; + for (const exposure of credential.exposures) { + if (exposure.as === "runner") registryGrants.add(exposure.grantedBy); + } + } + for (const [jobId, job] of jobs) { + for (const grant of registryGrants) { + if (jobGrants(job, grant)) mustBeGated.add(jobId); + } + } for (const jobId of mustBeGated) { const job = jobs.get(jobId); if (job === undefined) { diff --git a/scripts/publish-preflight.mjs b/scripts/publish-preflight.mjs index 22b4bc7..2374209 100644 --- a/scripts/publish-preflight.mjs +++ b/scripts/publish-preflight.mjs @@ -18,6 +18,13 @@ // declared to hand them to, and refuses when one of them is absent or empty. Nothing is contacted, // nothing is written, and no credential VALUE is ever read into a message. // +// UNDER npm TRUSTED PUBLISHING THE VARIABLES ARE THE RUNNER'S. The required credential is the OIDC +// token request GitHub sets up for a job that grants `id-token: write` (`ACTIONS_ID_TOKEN_REQUEST_URL` +// and `ACTIONS_ID_TOKEN_REQUEST_TOKEN`), declared as a `runner` exposure. Without the grant, npm skips +// the exchange silently and the registry refuses the upload after the build, which is exactly the +// late, unnamed failure this script exists to move forward. Present variables do not prove that the +// package's trusted publisher is registered on npm; only the registry can answer that. +// // WHY IT LIVES ON THE PUBLISH COMMAND PATH RATHER THAN IN THE WORKFLOW'S STEP LIST. Two reasons. // The workflow's step list is `release.yml`'s control flow, which this change deliberately does not // touch. And a step in the workflow would only guard the workflow: `pnpm run release` run by hand, @@ -55,7 +62,8 @@ import { DEFAULT_DECLARATION, InvocationError, loadDeclaration } from "./credent * by `credential-surface.mjs` at merge time instead. * * @param declaration A validated declaration. - * @returns A list of `{ credential, variable }`, one per variable that must be present. + * @returns A list of `{ credential, variable, grantedBy }`, one per variable that must be present. + * `grantedBy` is set for a variable the runner supplies because of a permission grant. */ export function requiredVariables(declaration) { const job = declaration.publishPath.job; @@ -64,9 +72,13 @@ export function requiredVariables(declaration) { if (!credential.requiredForPublish) continue; for (const exposure of credential.exposures) { if (exposure.job !== job) continue; - if (exposure.as !== "env") continue; + if (exposure.as !== "env" && exposure.as !== "runner") continue; if (exposure.mode === "presence-test") continue; - required.push({ credential: credential.name, variable: exposure.name }); + required.push({ + credential: credential.name, + variable: exposure.name, + grantedBy: exposure.as === "runner" ? exposure.grantedBy : undefined, + }); } } return required; @@ -116,13 +128,19 @@ export function preflight({ repoRoot, declarationPath, env = process.env } = {}) `publish-preflight: REFUSING TO PUBLISH. ${missing.length} required credential(s) are absent`, "or empty in this environment, and the registry has NOT been contacted.", ]; - for (const { credential, variable } of missing) { + for (const { credential, variable, grantedBy } of missing) { const declared = declaration.credentials.find((entry) => entry.name === credential); report.push( ` ${variable} is empty. It carries ${credential}, whose single permitted storage`, ` location is: ${declared.storage}.`, - ` Required token class: ${declared.tokenClass}`, ); + if (grantedBy !== undefined) { + report.push( + ` The runner sets it only in a job that grants \`${grantedBy}\`: check the`, + ` \`${declaration.publishPath.job}\` job's \`permissions:\` block, and that this runs in that job.`, + ); + } + report.push(` Required token class: ${declared.tokenClass}`); } report.push( `The declaration is ${DEFAULT_DECLARATION}; ${declaration.documentation.file}, section`, diff --git a/scripts/registry-presence.mjs b/scripts/registry-presence.mjs index 44f10d0..0200e84 100644 --- a/scripts/registry-presence.mjs +++ b/scripts/registry-presence.mjs @@ -44,19 +44,30 @@ // ZERO DEPENDENCIES, like the other release gates here, so a broken or malicious install cannot // decide whether a release gate runs. // +// THE SECOND QUESTION: IS ANYTHING LEFT TO PUBLISH AT ALL (`pending`). Asked by the ungated +// `preflight` job on a push with no pending changesets, so the gated `publish` job, and with it the +// request for an approval on the `release` environment, starts only when there is something for it +// to do. It is the question `changeset publish` asks itself before it publishes ("is this package's +// version already on the registry"), over the same packages, asked one job earlier. The answer can +// only ever skip a publish job whose `changeset publish` would have published nothing: a version the +// registry does not report as present, for any reason, keeps the job in. +// // Usage: // node scripts/registry-presence.mjs account --arm publish|version [--out ] // [--budget-seconds ] [--probe-interval-seconds ] +// node scripts/registry-presence.mjs pending [--repo ] [--output ] // // Exit codes, which are a contract asserted by test/registry-presence.test.ts: -// 0 every bumped package is on the registry -// 1 at least one is absent after the whole budget, or could not be read at all +// 0 `account`: every bumped package is on the registry. `pending`: an answer was reached and +// written (`true`, `false` or `unknown`). +// 1 `account`: at least one is absent after the whole budget, or could not be read at all. // 2 the accounting could not run: a bad invocation, or a work list that names no packages. // Distinct from 1 because "we could not check" is not "we checked and it was missing", and // non-zero either way, so the release stops regardless. import { spawn } from "node:child_process"; -import { writeFileSync } from "node:fs"; +import { appendFileSync, readdirSync, readFileSync, statSync, writeFileSync } from "node:fs"; +import { join, resolve } from "node:path"; import { isCliEntrypoint } from "../packages/script-utils/index.js"; @@ -410,6 +421,151 @@ export async function account({ return { code: missing.length + unreadable.length > 0 ? 1 : 0, expected, rows }; } +/** `pending`'s answer when at least one version is not on the registry. */ +export const HAS_UNPUBLISHED = "true"; +/** `pending`'s answer when the registry reported every version present. */ +export const NOTHING_UNPUBLISHED = "false"; +/** `pending`'s answer when no version was reported absent but at least one could not be read. */ +export const UNPUBLISHED_UNKNOWN = "unknown"; + +/** + * Every package `changeset publish` would consider, with the version it would publish. + * + * Read from the working tree, which is what `changeset publish` reads: every `packages/*` manifest + * that is not `private`. It refuses rather than returning nothing, because "no packages" answered as + * "nothing to publish" would skip the publish job on a workspace this check failed to read. + * + * @param {string} root Repository root. + * @returns {{ name: string, version: string }[]} One entry per publishable package, sorted by name. + */ +export function workspaceVersions(root) { + const packagesDir = join(root, "packages"); + let entries; + try { + entries = readdirSync(packagesDir); + } catch (error) { + throw new InvocationError(`cannot read ${packagesDir}: ${error.message}`); + } + const found = []; + for (const entry of entries) { + const manifestPath = join(packagesDir, entry, "package.json"); + try { + if (!statSync(manifestPath).isFile()) continue; + } catch { + continue; + } + let manifest; + try { + manifest = JSON.parse(readFileSync(manifestPath, "utf8")); + } catch (error) { + throw new InvocationError(`cannot read ${manifestPath}: ${error.message}`); + } + if (manifest.private === true) continue; + if (typeof manifest.name !== "string" || manifest.name === "") { + throw new InvocationError(`${manifestPath} has no name`); + } + if (typeof manifest.version !== "string" || manifest.version === "") { + throw new InvocationError(`${manifestPath} has no version`); + } + found.push({ name: manifest.name, version: manifest.version }); + } + if (found.length === 0) { + throw new InvocationError(`no publishable packages found under ${packagesDir}`); + } + return found.sort((a, b) => a.name.localeCompare(b.name)); +} + +/** + * Is any package's version missing from the registry. + * + * ONE PROBE PER PACKAGE, NO BUDGET, and that is the opposite of `account` on purpose. `account` + * waits because a just-published version can 404 for minutes and reading that as "never published" + * reds a release that happened. Here the same delay reads as "not published yet", which keeps the + * publish job in and asks for an approval nobody needed: the cheap direction to be wrong in. Waiting + * would only make an ordinary push slower to say the same thing. + * + * The answer is ABSENT-FIRST: one version the registry reports absent is something to publish, + * whatever the other probes said. Only when every probe answered "present" is the answer "nothing". + * A probe that got no answer makes the answer `unknown`, which the workflow treats as "something": + * a check that could not read the registry cannot be the reason a publish job did not run. + * + * @param {object} options + * @param {{name: string, version: string}[]} options.packages The work list. + * @param {number} [options.probeTimeoutSeconds] How long one probe may take. + * @param {object} [options.env] The environment probes run in. + * @param {Function} [options.probe] The registry, which is the boundary a test replaces. + * @param {Function} [options.log] Where lines and annotations go. + * @returns {Promise<{answer: string, rows: object[]}>} The answer and one row per package. + */ +export async function pending({ + packages, + probeTimeoutSeconds = PRESENCE_PROBE_TIMEOUT_SECONDS, + env = process.env, + probe = probeWithNpm, + log = (line) => process.stdout.write(`${line}\n`), +} = {}) { + const rows = []; + for (const { name, version } of packages) { + const answer = await probe({ name, version, env, timeoutSeconds: probeTimeoutSeconds }); + rows.push({ name, version, state: answer.state, detail: answer.detail }); + } + + const named = (state) => + rows + .filter((row) => row.state === state) + .map((row) => `${row.name}@${row.version}`) + .join(", "); + const absent = rows.filter((row) => row.state === ABSENT); + const unreadable = rows.filter((row) => row.state === UNREADABLE); + + for (const row of unreadable) { + log(`${row.name}@${row.version}: the registry did not answer. ${row.detail}`); + } + + if (absent.length > 0) { + log( + `${absent.length} of ${rows.length} package version(s) are not on the registry: ` + + `${named(ABSENT)}. The publish job runs and asks for its approval.`, + ); + return { answer: HAS_UNPUBLISHED, rows }; + } + if (unreadable.length > 0) { + log( + `::warning title=Registry could not be read::${unreadable.length} of ${rows.length} ` + + `package version(s) could not be checked: ${named(UNREADABLE)}. Nothing was reported ` + + `absent, but this check cannot prove there is nothing to publish, so the publish job runs ` + + `and asks for its approval. If the approver finds nothing to publish, the job's own ` + + `\`changeset publish\` will say so.`, + ); + return { answer: UNPUBLISHED_UNKNOWN, rows }; + } + log( + `All ${rows.length} package version(s) are already on the registry, so \`changeset publish\` ` + + `would publish nothing. The publish job does not run and no approval is asked for.`, + ); + return { answer: NOTHING_UNPUBLISHED, rows }; +} + +/** + * @param {string[]} argv Arguments after `pending`. + * @returns {{ repo: string, output?: string }} The parsed options. + */ +function parsePendingArgs(argv) { + const options = { repo: resolve(import.meta.dirname, "..") }; + for (let i = 0; i < argv.length; i += 1) { + const flag = argv[i]; + const value = argv[i + 1]; + if (flag !== "--repo" && flag !== "--output") { + throw new InvocationError(`unknown argument ${JSON.stringify(flag)}`); + } + if (value === undefined) throw new InvocationError(`${flag} needs a value`); + i += 1; + if (flag === "--repo") options.repo = resolve(value); + else options.output = value; + } + return options; +} + /** * @param {string[]} argv Arguments after the script name. * @returns {object} The parsed options. @@ -418,7 +574,7 @@ function parseArgs(argv) { const [subcommand, ...rest] = argv; if (subcommand !== "account") { throw new InvocationError( - `unknown subcommand ${JSON.stringify(subcommand ?? "")}. Expected \`account\`.`, + `unknown subcommand ${JSON.stringify(subcommand ?? "")}. Expected \`account\` or \`pending\`.`, ); } const options = { arm: "publish" }; @@ -459,6 +615,16 @@ function parseArgs(argv) { * @returns {Promise} The process exit code. */ export async function main(argv, { env = process.env, log } = {}) { + if (argv[0] === "pending") { + const options = parsePendingArgs(argv.slice(1)); + const packages = workspaceVersions(options.repo); + const result = await pending({ packages, env, log }); + // Appended, because the file is `$GITHUB_OUTPUT`, which earlier steps of the job also write. + if (options.output !== undefined) { + appendFileSync(options.output, `has-unpublished=${result.answer}\n`, "utf8"); + } + return 0; + } const options = parseArgs(argv); const packages = parseBumpedPackages(env.BUMPED_PACKAGES); const result = await account({ ...options, packages, env, log }); diff --git a/test/credential-surface.test.ts b/test/credential-surface.test.ts index 192a29f..91670c1 100644 --- a/test/credential-surface.test.ts +++ b/test/credential-surface.test.ts @@ -112,6 +112,7 @@ interface Exposure { as: string; name: string; mode?: string; + grantedBy?: string; } interface Credential { @@ -144,6 +145,20 @@ function realDeclaration(): Declaration { return JSON.parse(readFileSync(join(REPO, DECLARATION), "utf8")) as Declaration; } +/** + * A line in the publish step's `env:` block, right after its one `secrets.` reference. An undeclared + * secret inserted before it lands in step "Publish" of job "publish". + */ +const PUBLISH_ENV_ANCHOR = " # NO npm TOKEN, DELIBERATELY."; + +/** The version job's permissions block, which must never grant the npm credential's permission. */ +const VERSION_PERMISSIONS = + ' permissions:\n contents: write # push the version commit to changeset-release/main\n pull-requests: write # open / update the "Version Packages" PR\n'; + +/** The publish job's permissions block, whose `id-token: write` is the npm credential. */ +const PUBLISH_PERMISSIONS = + " permissions:\n contents: write # create tags / GitHub releases\n id-token: write # npm trusted publishing (the only npm credential) and provenance\n"; + describe("the committed surface is the declared surface (positive control)", () => { it("passes against this repository as committed", () => { const result = run(["--repo", REPO]); @@ -192,8 +207,8 @@ describe("a secret the declaration does not name (AC2)", () => { const root = fixture(); edit(root, WORKFLOW, (text) => text.replace( - " NPM_TOKEN: ${{ secrets.NPM_TOKEN }}\n", - " NPM_TOKEN: ${{ secrets.NPM_TOKEN }}\n SMUGGLED: ${{ secrets.LEGACY_DEPLOY_KEY }}\n", + PUBLISH_ENV_ANCHOR, + ` SMUGGLED: \${{ secrets.LEGACY_DEPLOY_KEY }}\n${PUBLISH_ENV_ANCHOR}`, ), ); const result = run(["--repo", root]); @@ -215,10 +230,10 @@ describe("a secret the declaration does not name (AC2)", () => { }); it("does not mistake the workflow's prose for wiring", () => { - // release.yml's header comments name NPM_TOKEN, NODE_AUTH_TOKEN and RELEASE_PR_TOKEN in - // ordinary English several times. A gate that grepped would report every one of those. + // release.yml's comments name RELEASE_PR_TOKEN, GITHUB_TOKEN and `id-token: write` in ordinary + // English several times. A gate that grepped would report every one of those. const workflow = readFileSync(join(REPO, WORKFLOW), "utf8"); - expect(workflow).toContain("# register the npm-side Trusted Publisher, then remove NPM_TOKEN"); + expect(workflow).toContain("# ABSENT, IT FALLS BACK AND WARNS. RELEASE_PR_TOKEN is optional"); const root = fixture(); edit(root, WORKFLOW, (text) => text.replace( @@ -234,13 +249,13 @@ describe("a declared credential that is no longer where it says (AC3)", () => { it("fails and names the credential rather than passing because nothing extra was found", () => { const root = fixture(); edit(root, WORKFLOW, (text) => - text.replace(" NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}\n", ""), + text.replace(" GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}\n", ""), ); const result = run(["--repo", root]); expect(result.status).toBe(1); expect(result.output).toContain("declared-exposure-absent"); - expect(result.output).toContain("NPM_TOKEN"); - expect(result.output).toContain("NODE_AUTH_TOKEN"); + expect(result.output).toContain("`GITHUB_TOKEN`"); + expect(result.output).toContain("GH_TOKEN"); expect(result.output).not.toContain("undeclared-secret"); }); @@ -248,28 +263,46 @@ describe("a declared credential that is no longer where it says (AC3)", () => { const root = fixture(); // `replaceAll`, not `replace`, and the difference is load-bearing rather than stylistic. This // case is about the credential being gone EVERYWHERE, so the mutation has to remove every - // reference; a single-occurrence replace leaves one behind the moment a second step legitimately - // consumes the token, and the checker then correctly reports per-exposure findings instead of - // the whole-credential one. S0081 added exactly such a step (the configuration allow-check, which - // must resolve the same npmrc the publish resolves and therefore needs the same NODE_AUTH_TOKEN), - // and that is what turned this into a fixture bug rather than a checker bug. The assertion below - // is unchanged: the refusal this case exists for is intact. + // reference; RELEASE_PR_TOKEN is consumed by two steps, and a single-occurrence replace would + // leave one behind, after which the checker correctly reports per-exposure findings instead of + // the whole-credential one. edit(root, WORKFLOW, (text) => text - .replaceAll(" NPM_TOKEN: ${{ secrets.NPM_TOKEN }}\n", "") - .replaceAll(" NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}\n", ""), + .replaceAll("secrets.RELEASE_PR_TOKEN || ", "") + .replace("${{ secrets.RELEASE_PR_TOKEN != '' }}", "'false'"), + ); + const result = run(["--repo", root]); + expect(result.status).toBe(1); + expect(result.output).toContain("declared-credential-absent"); + expect(result.output).toContain("RELEASE_PR_TOKEN"); + }); + + it("reports the npm credential gone when the publish job stops granting `id-token: write`", () => { + // GITHUB_OIDC_ID_TOKEN has no `secrets.` reference to go missing: the runner sets its two + // variables because of the grant, so the grant IS its exposure. Removing it is the + // trusted-publishing analogue of deleting the token from the publish step. + const root = fixture(); + edit(root, WORKFLOW, (text) => + text.replace( + PUBLISH_PERMISSIONS, + " permissions:\n contents: write # create tags / GitHub releases\n", + ), ); const result = run(["--repo", root]); expect(result.status).toBe(1); expect(result.output).toContain("declared-credential-absent"); - expect(result.output).toContain("NPM_TOKEN"); + expect(result.output).toContain("GITHUB_OIDC_ID_TOKEN"); + expect(result.output).toContain( + 'job "publish" does not grant `id-token: write`, so the runner never sets `ACTIONS_ID_TOKEN_REQUEST_TOKEN`', + ); + expect(result.output).toContain("permissions-widened"); }); it("reports a declared step that no longer exists in the workflow", () => { const root = fixture(); editDeclaration(root, (declaration) => { - const npm = declaration.credentials.find((entry) => entry.name === "NPM_TOKEN"); - npm!.exposures[0].step = "A step nobody wrote"; + const oidc = declaration.credentials.find((entry) => entry.name === "GITHUB_OIDC_ID_TOKEN"); + oidc!.exposures[0].step = "A step nobody wrote"; }); const result = run(["--repo", root]); expect(result.status).toBe(1); @@ -284,20 +317,21 @@ describe("a credential exposed more broadly than declared (AC4)", () => { edit(root, WORKFLOW, (text) => text.replace( " environment: release\n steps:\n", - " environment: release\n env:\n NPM_TOKEN: ${{ secrets.NPM_TOKEN }}\n steps:\n", + " environment: release\n env:\n GITHUB_TOKEN: ${{ secrets.RELEASE_PR_TOKEN }}\n steps:\n", ), ); const result = run(["--repo", root]); expect(result.status).toBe(1); expect(result.output).toContain("scope-widened"); expect(result.output).toContain('job-level `env` in job "publish"'); - expect(result.output).toContain('the declaration permits only step "Publish"'); + expect(result.output).toContain("the declaration permits only step"); + expect(result.output).toContain('step "Publish" of job "publish" (`env: GITHUB_TOKEN`)'); }); it("fails when a step-scoped token is hoisted to workflow level", () => { const root = fixture(); edit(root, WORKFLOW, (text) => - text.replace("jobs:\n", "env:\n NPM_TOKEN: ${{ secrets.NPM_TOKEN }}\njobs:\n"), + text.replace("jobs:\n", "env:\n GITHUB_TOKEN: ${{ secrets.RELEASE_PR_TOKEN }}\njobs:\n"), ); const result = run(["--repo", root]); expect(result.status).toBe(1); @@ -310,7 +344,7 @@ describe("a credential exposed more broadly than declared (AC4)", () => { edit(root, WORKFLOW, (text) => text.replace( " - name: Changesets must be able to bump something\n", - " - name: Changesets must be able to bump something\n env:\n NPM_TOKEN: ${{ secrets.NPM_TOKEN }}\n", + " - name: Changesets must be able to bump something\n env:\n GITHUB_TOKEN: ${{ secrets.RELEASE_PR_TOKEN }}\n", ), ); const result = run(["--repo", root]); @@ -406,7 +440,21 @@ describe("the registry-reaching job and its protected environment (AC5)", () => expect(result.output).toContain("staging"); }); - it("fails when a registry credential turns up in an ungated job", () => { + it("fails when the npm credential's grant turns up in an ungated job", () => { + // Under trusted publishing `id-token: write` is enough to publish, so an ungated job holding it + // is a registry-reaching job without the approval, whatever its steps say. + const root = fixture(); + edit(root, WORKFLOW, (text) => + text.replace(VERSION_PERMISSIONS, `${VERSION_PERMISSIONS} id-token: write\n`), + ); + const result = run(["--repo", root]); + expect(result.status).toBe(1); + expect(result.output).toContain("environment-missing"); + expect(result.output).toContain('job "version" can reach the registry'); + expect(result.output).toContain("permissions-widened"); + }); + + it("fails when a registry secret turns up anywhere, since none is declared any more", () => { const root = fixture(); edit(root, WORKFLOW, (text) => text.replace( @@ -416,8 +464,20 @@ describe("the registry-reaching job and its protected environment (AC5)", () => ); const result = run(["--repo", root]); expect(result.status).toBe(1); - expect(result.output).toContain("environment-missing"); - expect(result.output).toContain('job "version"'); + expect(result.output).toContain("undeclared-secret"); + expect(result.output).toContain("secrets.NPM_TOKEN"); + }); + + it("refuses a `runner` exposure that does not say which grant produces it", () => { + const root = fixture(); + editDeclaration(root, (declaration) => { + const oidc = declaration.credentials.find((entry) => entry.name === "GITHUB_OIDC_ID_TOKEN"); + delete oidc!.exposures[0].grantedBy; + }); + const result = run(["--repo", root]); + expect(result.status).toBe(1); + expect(result.output).toContain("declaration-invalid"); + expect(result.output).toContain("`grantedBy`"); }); }); @@ -515,18 +575,21 @@ describe("log scrubbing must cover every declared issued form (AC7)", () => { it("fails when a form is redacted but the post-redaction assertion stops looking for it", () => { const root = fixture(); edit(root, WORKFLOW, (text) => - text.replace("if grep -rqiE 'npm_[A-Za-z0-9]{36}|", "if grep -rqiE 'placeholder_no_match|"), + text.replace("|github_pat_[A-Za-z0-9_]{22,}|", "|placeholder_no_match|"), ); const result = run(["--repo", root]); expect(result.status).toBe(1); expect(result.output).toContain("redaction-assertion-incomplete"); - expect(result.output).toContain("npm_[A-Za-z0-9]{36}"); + expect(result.output).toContain("github_pat_[A-Za-z0-9_]{22,}"); }); it("fails when the scrubbing rule itself is dropped", () => { const root = fixture(); edit(root, WORKFLOW, (text) => - text.replace(" -e 's/npm_[A-Za-z0-9]{36}/npm_REDACTED/g' \\\n", ""), + text.replace( + " -e 's/github_pat_[A-Za-z0-9_]{22,}/github_pat_REDACTED/g' \\\n", + "", + ), ); const result = run(["--repo", root]); expect(result.status).toBe(1); @@ -652,7 +715,7 @@ describe("the no-rollback and registry-permanence statements (AC10)", () => { it("fails when a declared credential has no compensating action recorded", () => { const root = fixture(); edit(root, DOCS, (text) => { - const section = text.indexOf("### `NPM_TOKEN`"); + const section = text.indexOf("### `GITHUB_OIDC_ID_TOKEN`"); const at = text.indexOf("- **Compensating action.**", section); expect(at).toBeGreaterThan(-1); return `${text.slice(0, at)}- **Something else.**${text.slice(at + "- **Compensating action.**".length)}`; @@ -671,11 +734,11 @@ describe("one run reports every disagreement (AC11)", () => { text // 1: an undeclared secret. .replace( - " NPM_TOKEN: ${{ secrets.NPM_TOKEN }}\n", - " NPM_TOKEN: ${{ secrets.NPM_TOKEN }}\n SMUGGLED: ${{ secrets.LEGACY_DEPLOY_KEY }}\n", + PUBLISH_ENV_ANCHOR, + ` SMUGGLED: \${{ secrets.LEGACY_DEPLOY_KEY }}\n${PUBLISH_ENV_ANCHOR}`, ) // 2: a declared exposure that is gone. - .replace(" NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}\n", "") + .replace(" GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}\n", "") // 3: the protected environment removed. .replace(" environment: release\n", "") // 4: a permission grant widened. diff --git a/test/npm-config-allow.test.ts b/test/npm-config-allow.test.ts index 6d2aa0b..eef7115 100644 --- a/test/npm-config-allow.test.ts +++ b/test/npm-config-allow.test.ts @@ -654,23 +654,23 @@ describe("npm-config-allow: where the check runs", () => { ); expect(checkStepEnv).not.toBeNull(); const stepText = checkStepEnv?.[1] ?? ""; - expect(stepText).toContain("NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}"); expect(stepText).toContain( "NPM_CONFIG_PROVENANCE: ${{ github.event.repository.visibility == 'public' }}", ); - // AND NOT ONE VARIABLE MORE. `NPM_TOKEN` is in the publish step because `changesets/action` - // wants it; nothing on the CONFIGURATION path reads it, since npm reads only `npm_config_*` and - // the registry credential arrives through the generated npmrc. Copying it here would widen this - // repository's declared credential surface (S0080) by one step for no gain. Matched as an env - // KEY, because the line above legitimately contains the substring `secrets.NPM_TOKEN`. - expect(stepText).not.toMatch(/^\s+NPM_TOKEN:/m); + // AND NOT ONE VARIABLE MORE. Under npm trusted publishing no configuration source carries a + // credential: the publish authenticates through the OIDC exchange inside `npm publish`, and the + // publish job's setup-node generates no npmrc for a token to be expanded into. So no secret has + // any business here, and copying the publish step's GITHUB_TOKEN would widen this repository's + // declared credential surface (S0080) by one step for no gain. + expect(stepText).not.toContain("secrets."); + expect(stepText).not.toMatch(/^\s+(NPM_TOKEN|NODE_AUTH_TOKEN|GITHUB_TOKEN):/m); // C20's workflow half: nothing may let this step be skipped or downgraded to a warning. A gate // that can be skipped is a gate that will be. expect(stepText).not.toContain("continue-on-error"); expect(stepText).not.toMatch(/^\s+if:/m); }); - it("declares its own credential exposure in the S0080 surface declaration (C10, C11)", () => { + it("is named in the S0080 surface declaration, and given no credential there (C10, C11)", () => { // `scripts/credential-surface.mjs` already refuses a workflow that disagrees with the // declaration, and it runs in the required `verify` job. This asserts the other half from this // side: the step this spec added is NAMED in the declaration rather than having been slipped @@ -679,21 +679,18 @@ describe("npm-config-allow: where the check runs", () => { readFileSync(join(REPO_ROOT, ".github", "credential-surface.json"), "utf8"), ); const step = "The publish configuration must be one the allow-set permits"; - const npmToken = declaration.credentials.find( - (entry: { name: string }) => entry.name === "NPM_TOKEN", + // No credential is declared at this step: under trusted publishing it resolves a configuration + // that carries none. + for (const credential of declaration.credentials) { + expect( + credential.exposures.filter((exposure: { step: string }) => exposure.step === step), + `${credential.name} at the allow-check step`, + ).toEqual([]); + } + // And no npm token is declared anywhere: the npm credential is the publish job's OIDC grant. + expect(declaration.credentials.map((entry: { name: string }) => entry.name)).not.toContain( + "NPM_TOKEN", ); - expect( - npmToken.exposures.some( - (exposure: { job: string; step: string; name: string }) => - exposure.job === "publish" && - exposure.step === step && - exposure.name === "NODE_AUTH_TOKEN", - ), - ).toBe(true); - // And NPM_TOKEN itself is declared nowhere but the publish step. - expect( - npmToken.exposures.filter((exposure: { name: string }) => exposure.name === "NPM_TOKEN"), - ).toEqual([{ job: "publish", step: "Publish", as: "env", name: "NPM_TOKEN", mode: "value" }]); const provenance = declaration.settings.find( (entry: { name: string }) => entry.name === "NPM_CONFIG_PROVENANCE", @@ -726,10 +723,15 @@ describe("npm-config-allow: where the check runs", () => { expect(preflightLines.some((line) => /^ {4}environment:/.test(line))).toBe(false); expect(versionLines.some((line) => /^ {4}environment:/.test(line))).toBe(false); - // Registry credentials still reach no job but the gated one. - expect(preflight).not.toContain("secrets.NPM_TOKEN"); - expect(version).not.toContain("secrets.NPM_TOKEN"); - expect(publish).toContain("secrets.NPM_TOKEN"); + // The registry credential still reaches no job but the gated one. Under trusted publishing it + // is the `id-token: write` grant, matched as a job-level permission line (six spaces), and no + // npm token is referenced by any job at all. + const grantsIdToken = (lines: string[]) => + lines.some((line) => /^ {6}id-token: write\b/.test(line)); + expect(grantsIdToken(preflightLines)).toBe(false); + expect(grantsIdToken(versionLines)).toBe(false); + expect(grantsIdToken(blocks.get("publish") ?? [])).toBe(true); + for (const job of [preflight, version, publish]) expect(job).not.toContain("secrets.NPM_TOKEN"); }); it( @@ -1168,7 +1170,7 @@ describe("npm-config-allow: the real repository", () => { /** * Build the context the check will meet, without inheriting the machine's own npm configuration. * - * @param ci Whether to add what `actions/setup-node` puts in place for the gated publish job. + * @param ci Whether to add what the gated publish job's own steps put in place. * @returns The environment. */ function repoContext(ci: boolean): NodeJS.ProcessEnv { @@ -1181,18 +1183,13 @@ describe("npm-config-allow: the real repository", () => { NPM_CONFIG_GLOBALCONFIG: emptyRc, }; if (ci) { - // What `actions/setup-node@v6` with `registry-url` actually does: it GENERATES an npmrc under - // RUNNER_TEMP and exports NPM_CONFIG_USERCONFIG at it, then release.yml's publish step adds - // the provenance flag and the token the generated file expands. - const userRc = join(home, "setup-node.npmrc"); - writeFileSync( - userRc, - "registry=https://registry.npmjs.org/\n//registry.npmjs.org/:_authToken=${NODE_AUTH_TOKEN}\n", - "utf8", - ); - env.NPM_CONFIG_USERCONFIG = userRc; + // What the publish job gives the check under npm trusted publishing: setup-node runs WITHOUT + // `registry-url`, so no user npmrc is generated and NPM_CONFIG_USERCONFIG is not exported, and + // the step adds only the provenance flag. The runner's OIDC token request variables are in + // the job too; they are not npm configuration, and their presence must not change a verdict. env.NPM_CONFIG_PROVENANCE = "true"; - env.NODE_AUTH_TOKEN = FAKE_TOKEN; + env.ACTIONS_ID_TOKEN_REQUEST_URL = "https://token.actions.invalid/idtoken?api-version=2.0"; + env.ACTIONS_ID_TOKEN_REQUEST_TOKEN = FAKE_TOKEN; } return env; } @@ -1219,9 +1216,9 @@ describe("npm-config-allow: the real repository", () => { it( "passes in the context the GATED PUBLISH JOB will actually give it (C3, C10)", () => { - // The one that matters: the generated user config, its token, and the provenance flag the - // publish step supplies. If the committed allow-set does not cover this, the first real - // release refuses, and finding that out here costs nothing. + // The one that matters: npm's own defaults plus the provenance flag the publish step supplies, + // with no credential in any source. If the committed allow-set does not cover this, the first + // real release refuses, and finding that out here costs nothing. const { code, output } = runCheck(REPO_ROOT, repoContext(true)); expect(code).toBe(0); expect(output).toContain("provenance = true"); diff --git a/test/publish-preflight.test.ts b/test/publish-preflight.test.ts index b21bb72..6c718ed 100644 --- a/test/publish-preflight.test.ts +++ b/test/publish-preflight.test.ts @@ -7,11 +7,13 @@ import { afterEach, describe, expect, it } from "vitest"; // THE NEGATIVE CONTROL FOR THE PUBLISH PREFLIGHT. // -// The defect: `changeset publish` discovers a missing npm token by asking the registry and being -// refused, which happens at the LAST step of a job that has already checked out, installed, built -// eight packages and waited for a human to approve a protected deployment. The approval is spent, -// the build is paid for, and the diagnostic is an `E401` from npm rather than a sentence naming the -// credential and where it is supposed to live. +// The defect: `changeset publish` discovers a missing npm credential by asking the registry and +// being refused, which happens at the LAST step of a job that has already checked out, installed, +// built eight packages and waited for a human to approve a protected deployment. The approval is +// spent, the build is paid for, and the diagnostic is an authentication error from npm rather than a +// sentence naming the credential and where it is supposed to come from. Under npm trusted publishing +// the credential is the OIDC token request the runner sets up for a job granted `id-token: write`, +// and npm skips the exchange SILENTLY when it is missing, so the late failure is the only symptom. // // The preflight is worth exactly what its pair of controls proves, so this suite asserts BOTH // directions and one boundary: @@ -31,7 +33,10 @@ const PREFLIGHT = join(REPO, "scripts", "publish-preflight.mjs"); const DECLARATION = join(".github", "credential-surface.json"); /** A value no message may ever echo. If it appears in the output, the preflight leaked a secret. */ -const SENTINEL = "npm_thisvalueMUSTneverBEprinted000000"; +const SENTINEL = "thisvalueMUSTneverBEprinted000000"; + +/** The runner's OIDC token request URL, which carries no secret but is not echoed either. */ +const SENTINEL_URL = "https://token.actions.invalid/idtoken?api-version=2.0"; const temporaryDirs: string[] = []; @@ -80,50 +85,77 @@ function fixture(): string { return root; } -/** Every environment variable the committed declaration requires, all present and plausible. */ -const FULLY_CREDENTIALED = { NPM_TOKEN: SENTINEL, NODE_AUTH_TOKEN: SENTINEL }; +/** + * Every environment variable the committed declaration requires, all present and plausible: what the + * runner sets in a job that grants `id-token: write`. + */ +const FULLY_CREDENTIALED = { + ACTIONS_ID_TOKEN_REQUEST_URL: SENTINEL_URL, + ACTIONS_ID_TOKEN_REQUEST_TOKEN: SENTINEL, +}; describe("a required credential that is absent or empty (AC8)", () => { it("stops before the registry is contacted and names the missing credential", () => { const result = run(fixture(), {}); expect(result.status).toBe(1); expect(result.output).toContain("REFUSING TO PUBLISH"); - expect(result.output).toContain("NPM_TOKEN"); - expect(result.output).toContain("NODE_AUTH_TOKEN"); + expect(result.output).toContain("ACTIONS_ID_TOKEN_REQUEST_URL"); + expect(result.output).toContain("ACTIONS_ID_TOKEN_REQUEST_TOKEN"); + expect(result.output).toContain("GITHUB_OIDC_ID_TOKEN"); expect(result.output).toContain("the registry has NOT been contacted"); }); it("treats an empty value as absent", () => { - const result = run(fixture(), { NPM_TOKEN: "", NODE_AUTH_TOKEN: "" }); + const result = run(fixture(), { + ACTIONS_ID_TOKEN_REQUEST_URL: "", + ACTIONS_ID_TOKEN_REQUEST_TOKEN: "", + }); expect(result.status).toBe(1); - expect(result.output).toContain("NPM_TOKEN"); + expect(result.output).toContain("ACTIONS_ID_TOKEN_REQUEST_TOKEN"); }); it("treats a whitespace-only value as absent", () => { - const result = run(fixture(), { NPM_TOKEN: " ", NODE_AUTH_TOKEN: " \t " }); + const result = run(fixture(), { + ACTIONS_ID_TOKEN_REQUEST_URL: " ", + ACTIONS_ID_TOKEN_REQUEST_TOKEN: " \t ", + }); expect(result.status).toBe(1); - expect(result.output).toContain("NPM_TOKEN"); + expect(result.output).toContain("ACTIONS_ID_TOKEN_REQUEST_TOKEN"); }); it("catches the half-set case, where one of a credential's two variables is missing", () => { - const result = run(fixture(), { NPM_TOKEN: SENTINEL }); + const result = run(fixture(), { ACTIONS_ID_TOKEN_REQUEST_URL: SENTINEL_URL }); expect(result.status).toBe(1); - expect(result.output).toContain("NODE_AUTH_TOKEN"); + expect(result.output).toContain("ACTIONS_ID_TOKEN_REQUEST_TOKEN"); + expect(result.output).not.toContain("ACTIONS_ID_TOKEN_REQUEST_URL is empty"); }); - it("tells the operator the token class and the one place it may live", () => { + it("tells the operator what the credential is and the grant it comes from", () => { const result = run(fixture(), {}); - expect(result.output).toContain("organization"); - expect(result.output).toContain("Automation token"); + expect(result.output).toContain("github-provided"); + expect(result.output).toContain("OIDC ID token"); + // The one fix for a runner-provided credential is the job's grant, so the refusal names it. + expect(result.output).toContain("grants `id-token: write`"); + expect(result.output).toContain("`publish` job's `permissions:` block"); expect(result.output).toContain("Credential rotation, revocation, and compensating actions"); }); it("never echoes a credential value", () => { - const result = run(fixture(), { NPM_TOKEN: SENTINEL }); + const result = run(fixture(), { ACTIONS_ID_TOKEN_REQUEST_TOKEN: SENTINEL }); expect(result.status).toBe(1); expect(result.output).not.toContain(SENTINEL); }); + it("requires the runner's OIDC variables, and no npm token, under trusted publishing", () => { + // The declaration is what the preflight reads, so this pins what the committed one requires: + // a leftover NPM_TOKEN requirement would refuse every trusted publish, and a missing OIDC one + // would let a job without `id-token: write` build eight packages before npm refused it. + const result = run(fixture(), { NPM_TOKEN: SENTINEL, NODE_AUTH_TOKEN: SENTINEL }); + expect(result.status).toBe(1); + expect(result.output).not.toContain("NPM_TOKEN is empty"); + expect(result.output).toContain("ACTIONS_ID_TOKEN_REQUEST_URL is empty"); + }); + it("passes when every required credential is present (positive control)", () => { const result = run(fixture(), FULLY_CREDENTIALED); expect(result.status).toBe(0); diff --git a/test/registry-presence.test.ts b/test/registry-presence.test.ts index 8df95f8..1d380cb 100644 --- a/test/registry-presence.test.ts +++ b/test/registry-presence.test.ts @@ -41,11 +41,16 @@ import { hermeticEnv } from "./support/fixture-registry.mjs"; import { renderNotes, slugFor } from "../scripts/release-notes.mjs"; import { ABSENT, + HAS_UNPUBLISHED, + NOTHING_UNPUBLISHED, PRESENCE_BUDGET_SECONDS, PRESENT, + UNPUBLISHED_UNKNOWN, UNREADABLE, account, parseBumpedPackages, + pending, + workspaceVersions, } from "../scripts/registry-presence.mjs"; const REPO_ROOT = join(import.meta.dirname, ".."); @@ -754,3 +759,242 @@ describe("AC-3 and AC-4 rest on an exit code, so the accounting refuses what it expect(`${run.stdout}${run.stderr}`).toMatch(/could not run/); }); }); + +// --------------------------------------------------------------------------- +// `pending`: is there anything for the gated publish job to do at all +// +// The ungated preflight asks this so that the `release` environment requests an approval only when +// a publish could happen. The property that matters is ONE-SIDED: the answer may skip the publish +// job only when the registry reported every version present, because that is the state in which +// `changeset publish` publishes nothing. Every other registry answer must keep the job in. Graded +// against the same loopback registry and the same real `npm view` as the accounting above. +// --------------------------------------------------------------------------- + +/** Run `pending` against a fixture registry, collecting everything it said. */ +async function runPending(packages: { name: string; version: string }[], url: string) { + const lines: string[] = []; + const result = await pending({ + packages, + probeTimeoutSeconds: 10, + env: registryEnv(url), + log: (line: string) => lines.push(line), + }); + return { ...result, log: lines.join("\n") }; +} + +/** A throwaway workspace whose `packages/*` manifests are exactly `manifests`. */ +function workspace(manifests: Record>): string { + const root = tempDir("registry-presence-workspace-"); + for (const [dir, manifest] of Object.entries(manifests)) { + mkdirSync(join(root, "packages", dir), { recursive: true }); + writeFileSync(join(root, "packages", dir, "package.json"), JSON.stringify(manifest), "utf8"); + } + return root; +} + +/** The shipped CLI, run as the workflow runs it, without blocking the in-process registry. */ +function runPendingCli( + args: string[], + env: NodeJS.ProcessEnv, +): Promise<{ status: number; output: string }> { + const child = spawn( + process.execPath, + [join(REPO_ROOT, "scripts", "registry-presence.mjs"), "pending", ...args], + { env, stdio: ["ignore", "pipe", "pipe"] }, + ); + let output = ""; + child.stdout.setEncoding("utf8"); + child.stderr.setEncoding("utf8"); + child.stdout.on("data", (chunk: string) => (output += chunk)); + child.stderr.on("data", (chunk: string) => (output += chunk)); + return new Promise((resolve) => { + child.on("close", (status) => resolve({ status: status ?? -1, output })); + }); +} + +describe("pending: only a registry that has every version may skip the publish job", () => { + it( + "answers `false` when every version is on the registry, which is the one skipping answer", + { timeout: 60_000 }, + async () => { + const a = pkg("pending-all-present-a"); + const b = pkg("pending-all-present-b"); + const registry = await startRegistry({ + [a.name]: { kind: "present" }, + [b.name]: { kind: "present" }, + }); + + const result = await runPending([a, b], registry.url); + + expect(result.answer).toBe(NOTHING_UNPUBLISHED); + expect(result.rows.map((row) => row.state)).toEqual([PRESENT, PRESENT]); + expect(result.log).toContain("would publish nothing"); + expect(result.log).toContain("no approval is asked for"); + }, + ); + + it("answers `true` when one version is absent, and names it", { timeout: 60_000 }, async () => { + const here = pkg("pending-one-absent-here"); + const gone = pkg("pending-one-absent-gone"); + const registry = await startRegistry({ + [here.name]: { kind: "present" }, + [gone.name]: { kind: "absent" }, + }); + + const result = await runPending([here, gone], registry.url); + + expect(result.answer).toBe(HAS_UNPUBLISHED); + expect(result.log).toContain(`${gone.name}@${gone.version}`); + expect(result.log).toContain("The publish job runs"); + }); + + it( + "answers `true` for a package the registry has never heard of", + { timeout: 60_000 }, + async () => { + // The fixture registry 404s every name it was not told about, which is a first publish. + const brandNew = pkg("pending-never-published"); + const registry = await startRegistry({}); + + const result = await runPending([brandNew], registry.url); + + expect(result.rows[0].state).toBe(ABSENT); + expect(result.answer).toBe(HAS_UNPUBLISHED); + }, + ); + + it( + "answers `unknown`, which keeps the job in, when the registry cannot be read", + { timeout: 120_000 }, + async () => { + const here = pkg("pending-unreadable-here"); + const dark = pkg("pending-unreadable-dark"); + const registry = await startRegistry({ + [here.name]: { kind: "present" }, + [dark.name]: { kind: "status", code: 503 }, + }); + + const result = await runPending([here, dark], registry.url); + + expect(result.rows[1].state).toBe(UNREADABLE); + expect(result.answer).toBe(UNPUBLISHED_UNKNOWN); + expect(result.answer).not.toBe(NOTHING_UNPUBLISHED); + expect(result.log).toContain("::warning title=Registry could not be read::"); + expect(result.log).toContain("cannot prove there is nothing to publish"); + }, + ); + + it( + "lets one absent version decide even when another could not be read", + { timeout: 120_000 }, + async () => { + const gone = pkg("pending-mixed-gone"); + const dark = pkg("pending-mixed-dark"); + const registry = await startRegistry({ + [gone.name]: { kind: "absent" }, + [dark.name]: { kind: "status", code: 503 }, + }); + + expect((await runPending([gone, dark], registry.url)).answer).toBe(HAS_UNPUBLISHED); + }, + ); + + it( + "probes each version once and does not wait out propagation", + { timeout: 60_000 }, + async () => { + // A version 404ing just after a publish reads here as "not published yet", which keeps the + // job in: the cheap direction. Waiting the accounting's budget on every ordinary push would + // make the push slower to say the same thing. + const racer = pkg("pending-racer"); + const registry = await startRegistry({ [racer.name]: { kind: "appears-after", probes: 1 } }); + + const result = await runPending([racer], registry.url); + + expect(result.answer).toBe(HAS_UNPUBLISHED); + expect(registry.hits.get(racer.name)).toBe(1); + }, + ); +}); + +describe("pending: the work list is what `changeset publish` would consider", () => { + it("reads every non-private packages/* manifest, sorted by name", () => { + const root = workspace({ + b: { name: `${SCOPE}/b`, version: "2.0.0" }, + a: { name: `${SCOPE}/a`, version: "1.0.0" }, + root: { name: `${SCOPE}/private`, version: "0.0.0", private: true }, + }); + expect(workspaceVersions(root)).toEqual([ + { name: `${SCOPE}/a`, version: "1.0.0" }, + { name: `${SCOPE}/b`, version: "2.0.0" }, + ]); + }); + + it("reads this repository's eight published packages", () => { + const names = workspaceVersions(REPO_ROOT).map((entry) => entry.name); + expect(names).toHaveLength(8); + expect(names.every((name) => name.startsWith("@cosyte/"))).toBe(true); + }); + + it("refuses a workspace with nothing publishable rather than answering `nothing to publish`", () => { + const root = workspace({ only: { name: `${SCOPE}/private`, version: "0.0.0", private: true } }); + expect(() => workspaceVersions(root)).toThrow(/no publishable packages/); + }); + + it("refuses a manifest with no version rather than probing `name@undefined`", () => { + const root = workspace({ broken: { name: `${SCOPE}/broken` } }); + expect(() => workspaceVersions(root)).toThrow(/has no version/); + }); +}); + +describe("pending: the CLI the preflight step runs", () => { + it( + "appends `has-unpublished=` to the output file and exits 0", + { timeout: 60_000 }, + async () => { + const root = workspace({ only: { name: `${SCOPE}/cli-present`, version: "1.0.0" } }); + const registry = await startRegistry({ [`${SCOPE}/cli-present`]: { kind: "present" } }); + const output = join(tempDir("registry-presence-output-"), "github-output"); + writeFileSync(output, "has-changesets=false\n", "utf8"); + + const result = await runPendingCli( + ["--repo", root, "--output", output], + registryEnv(registry.url), + ); + + expect(result.status, result.output).toBe(0); + // Appended, not overwritten: earlier steps of the job write the same file. + expect(readFileSync(output, "utf8")).toBe("has-changesets=false\nhas-unpublished=false\n"); + }, + ); + + it( + "writes `true` for an absent version, which the publish job's condition reads as `run`", + { timeout: 60_000 }, + async () => { + const root = workspace({ only: { name: `${SCOPE}/cli-absent`, version: "1.0.0" } }); + const registry = await startRegistry({ [`${SCOPE}/cli-absent`]: { kind: "absent" } }); + const output = join(tempDir("registry-presence-output-"), "github-output"); + + const result = await runPendingCli( + ["--repo", root, "--output", output], + registryEnv(registry.url), + ); + + expect(result.status, result.output).toBe(0); + expect(readFileSync(output, "utf8")).toBe("has-unpublished=true\n"); + }, + ); + + it("exits 2 and writes no answer when it could not read the workspace", async () => { + const root = workspace({}); + const output = join(tempDir("registry-presence-output-"), "github-output"); + writeFileSync(output, "", "utf8"); + + const result = await runPendingCli(["--repo", root, "--output", output], hermeticEnv()); + + expect(result.status).toBe(2); + expect(result.output).toMatch(/could not run/); + expect(readFileSync(output, "utf8")).toBe(""); + }); +}); diff --git a/test/release-arm-condition.test.ts b/test/release-arm-condition.test.ts new file mode 100644 index 0000000..c051e5c --- /dev/null +++ b/test/release-arm-condition.test.ts @@ -0,0 +1,306 @@ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +import { describe, expect, it } from "vitest"; + +import { parseWorkflow } from "../scripts/credential-surface.mjs"; + +// WHICH JOB RUNS ON WHICH PUSH, EVALUATED FROM THE CONDITIONS release.yml SHIPS. +// +// The `release` environment asks a human for an approval every time a job that names it starts. Until +// 2026-10-19 the `publish` job started on every push to `main` without pending changesets, so an +// approval was requested on every ordinary push and the runs piled up waiting. The fix is a job +// condition, and a job condition cannot be run locally (`act` is not available here), so this suite +// does the next best thing: it reads the `if:` expressions out of the committed workflow, with the +// same YAML reader the credential-surface gate uses, and evaluates them on every path a push can +// take. +// +// THE EVALUATOR REFUSES WHAT IT DOES NOT KNOW. It implements exactly the subset these conditions use +// (context paths, single-quoted strings, `==`, `!=`, `&&`, `||`, parentheses) with GitHub's +// semantics for it: a missing output is the empty string, and string comparison ignores case. A +// condition that grows a function call or another operator makes this suite throw instead of +// evaluating something it does not understand. +// +// THE PROPERTY THAT MATTERS IS ONE-SIDED. Skipping `publish` is safe only on the path where +// `changeset publish` would publish nothing: no pending changesets, not a version commit, and the +// registry reported every version present. Every other combination must start it. + +const WORKFLOW = readFileSync( + join(import.meta.dirname, "..", ".github", "workflows", "release.yml"), + "utf8", +); + +interface Node { + kind: "map" | "seq" | "scalar"; + entries?: Map; + items?: Node[]; + value?: string | null; +} + +const workflow = parseWorkflow(WORKFLOW) as Node; + +function child(node: Node | undefined, key: string): Node | undefined { + return node?.kind === "map" ? node.entries?.get(key) : undefined; +} + +function scalar(node: Node | undefined): string | undefined { + return node?.kind === "scalar" ? (node.value ?? undefined) : undefined; +} + +const jobs = child(workflow, "jobs"); + +function jobIf(jobId: string): string { + const expression = scalar(child(child(jobs, jobId), "if")); + if (expression === undefined) throw new Error(`job ${jobId} has no \`if:\``); + return expression; +} + +function stepIf(jobId: string, stepName: string): string { + const steps = child(child(jobs, jobId), "steps")?.items ?? []; + const step = steps.find((candidate) => scalar(child(candidate, "name")) === stepName); + if (step === undefined) throw new Error(`job ${jobId} has no step named ${stepName}`); + const expression = scalar(child(step, "if")); + if (expression === undefined) throw new Error(`step ${stepName} has no \`if:\``); + return expression; +} + +type Token = + | { kind: "path"; value: string } + | { kind: "string"; value: string } + | { kind: "op"; value: "==" | "!=" | "&&" | "||" | "(" | ")" }; + +function tokenize(source: string): Token[] { + const tokens: Token[] = []; + const pattern = /\s*(?:([A-Za-z_][A-Za-z0-9_.-]*)|'((?:[^']|'')*)'|(==|!=|&&|\|\||\(|\)))/y; + let at = 0; + while (at < source.length) { + if (/^\s*$/.test(source.slice(at))) break; + pattern.lastIndex = at; + const match = pattern.exec(source); + if (match === null) { + throw new Error(`the evaluator does not know ${JSON.stringify(source.slice(at))}`); + } + if (match[1] !== undefined) tokens.push({ kind: "path", value: match[1] }); + else if (match[2] !== undefined) + tokens.push({ kind: "string", value: match[2].replace(/''/g, "'") }); + else tokens.push({ kind: "op", value: match[3] as "==" }); + at = pattern.lastIndex; + } + return tokens; +} + +/** + * Evaluate one `${{ }}` condition against a context of output values. + * + * @param expression The `if:` text as committed, with or without `${{ }}`. + * @param context Every known path's value. A path not listed reads as `''`, as an unset output does. + * @returns Whether the job or step would run. + */ +function evaluate(expression: string, context: Record): boolean { + const inner = /^\$\{\{([\s\S]*)\}\}$/.exec(expression.trim())?.[1] ?? expression; + const tokens = tokenize(inner); + let i = 0; + const peek = () => tokens[i]; + const take = () => tokens[i++]; + + const operand = (): string | boolean => { + const token = take(); + if (token === undefined) throw new Error("the condition ends early"); + if (token.kind === "op" && token.value === "(") { + const value = or(); + const close = take(); + if (close?.kind !== "op" || close.value !== ")") throw new Error("unbalanced parentheses"); + return value; + } + if (token.kind === "string") return token.value; + if (token.kind === "path") { + if (!/^(needs\.preflight|steps\.[a-z-]+)\.outputs\.[a-z-]+$/.test(token.value)) { + throw new Error(`the evaluator does not know the context ${token.value}`); + } + return context[token.value] ?? ""; + } + throw new Error(`unexpected ${token.value}`); + }; + const comparison = (): string | boolean => { + const left = operand(); + const next = peek(); + if (next?.kind === "op" && (next.value === "==" || next.value === "!=")) { + take(); + const right = operand(); + const equal = String(left).toLowerCase() === String(right).toLowerCase(); + return next.value === "==" ? equal : !equal; + } + return left; + }; + const truthy = (value: string | boolean) => value !== false && value !== ""; + const and = (): string | boolean => { + let value = comparison(); + while (peek()?.kind === "op" && peek()?.value === "&&") { + take(); + const right = comparison(); + value = truthy(value) ? right : value; + } + return value; + }; + function or(): string | boolean { + let value = and(); + while (peek()?.kind === "op" && peek()?.value === "||") { + take(); + const right = and(); + value = truthy(value) ? value : right; + } + return value; + } + + const result = or(); + if (i !== tokens.length) throw new Error("trailing tokens in the condition"); + return truthy(result); +} + +/** What one push looks like to the jobs after `preflight`. */ +interface Push { + hasChangesets: "true" | "false"; + isRelease: "true" | "false"; + /** What the `pending` step wrote, or `undefined` when it did not run or did not report. */ + hasUnpublished?: "true" | "false" | "unknown"; +} + +function decide(push: Push) { + const asked = evaluate(stepIf("preflight", "Is any package version missing from the registry"), { + "steps.pending.outputs.has-changesets": push.hasChangesets, + }); + const outputs: Record = { + "needs.preflight.outputs.has-changesets": push.hasChangesets, + "needs.preflight.outputs.is-release": push.isRelease, + }; + if (asked && push.hasUnpublished !== undefined) { + outputs["needs.preflight.outputs.has-unpublished"] = push.hasUnpublished; + } + return { + asked, + version: evaluate(jobIf("version"), outputs), + publish: evaluate(jobIf("publish"), outputs), + }; +} + +describe("the evaluator itself", () => { + it("follows GitHub's rules for the subset it implements", () => { + expect(evaluate("${{ needs.preflight.outputs.x == 'true' }}", {})).toBe(false); + expect(evaluate("${{ needs.preflight.outputs.x != 'false' }}", {})).toBe(true); + expect( + evaluate("${{ needs.preflight.outputs.x == 'TRUE' }}", { + "needs.preflight.outputs.x": "true", + }), + ).toBe(true); + expect(evaluate("${{ 'a' == 'b' || ('c' == 'c' && 'd' != 'e') }}", {})).toBe(true); + expect(evaluate("${{ 'a' == 'a' && ('c' == 'd' || 'd' == 'e') }}", {})).toBe(false); + }); + + it("refuses a condition it does not understand rather than guessing", () => { + expect(() => evaluate("${{ contains(needs.preflight.outputs.x, 'y') }}", {})).toThrow(); + expect(() => evaluate("${{ github.event_name == 'push' }}", {})).toThrow(/context/); + expect(() => evaluate("${{ !cancelled() }}", {})).toThrow(); + }); +}); + +describe("the job that asks for the release approval", () => { + it("is the publish job and only the publish job", () => { + const gated = [...(jobs?.entries ?? new Map())] + .filter(([, job]) => child(job, "environment") !== undefined) + .map(([id, job]) => [id, scalar(child(job, "environment"))]); + expect(gated).toEqual([["publish", "release"]]); + expect(scalar(child(child(jobs, "publish"), "needs"))).toBe("preflight"); + }); + + it("grants id-token write and runs on a GitHub-hosted runner", () => { + const publish = child(jobs, "publish"); + expect(scalar(child(child(publish, "permissions"), "id-token"))).toBe("write"); + expect(scalar(child(publish, "runs-on"))).toBe("ubuntu-latest"); + }); +}); + +describe("which job runs on which push", () => { + it("a push carrying a pending changeset: version runs, publish is skipped, nothing is asked", () => { + for (const isRelease of ["true", "false"] as const) { + const decision = decide({ hasChangesets: "true", isRelease }); + expect(decision).toEqual({ asked: false, version: true, publish: false }); + } + }); + + it("a version commit: publish runs, whatever the registry says", () => { + // Including `false`: a re-run after a partial release finds every version published and still + // owes tags and GitHub releases, which only the publish job creates. + for (const hasUnpublished of ["true", "false", "unknown", undefined] as const) { + const decision = decide({ hasChangesets: "false", isRelease: "true", hasUnpublished }); + expect(decision, `has-unpublished=${hasUnpublished}`).toEqual({ + asked: true, + version: false, + publish: true, + }); + } + }); + + it("no changesets and a version the registry lacks: publish runs", () => { + // A cancelled or failed publish, or a version merged by hand: what the old classifier-only rule + // would have withheld silently. + expect(decide({ hasChangesets: "false", isRelease: "false", hasUnpublished: "true" })).toEqual({ + asked: true, + version: false, + publish: true, + }); + }); + + it("no changesets and a registry that could not be read: publish runs", () => { + expect( + decide({ hasChangesets: "false", isRelease: "false", hasUnpublished: "unknown" }), + ).toEqual({ asked: true, version: false, publish: true }); + }); + + it("no changesets and a check that did not report: publish runs", () => { + expect(decide({ hasChangesets: "false", isRelease: "false" })).toEqual({ + asked: true, + version: false, + publish: true, + }); + }); + + it("an ordinary push with every version on npm: neither arm runs, so no deployment is requested", () => { + expect(decide({ hasChangesets: "false", isRelease: "false", hasUnpublished: "false" })).toEqual( + { + asked: true, + version: false, + publish: false, + }, + ); + }); + + it("skips publish on exactly one combination of outputs", () => { + const skipped: Push[] = []; + for (const hasChangesets of ["true", "false"] as const) { + for (const isRelease of ["true", "false"] as const) { + for (const hasUnpublished of ["true", "false", "unknown", undefined] as const) { + const push = { hasChangesets, isRelease, hasUnpublished }; + if (hasChangesets === "false" && !decide(push).publish) skipped.push(push); + } + } + } + expect(skipped).toEqual([ + { hasChangesets: "false", isRelease: "false", hasUnpublished: "false" }, + ]); + }); + + it("asks the registry with the shipped command, writing to the job's outputs", () => { + const steps = child(child(jobs, "preflight"), "steps")?.items ?? []; + const step = steps.find( + (candidate) => + scalar(child(candidate, "name")) === "Is any package version missing from the registry", + ); + expect(scalar(child(step, "id"))).toBe("unpublished"); + expect(scalar(child(step, "run"))).toBe( + 'node scripts/registry-presence.mjs pending --output "$GITHUB_OUTPUT"', + ); + expect(scalar(child(child(child(jobs, "preflight"), "outputs"), "has-unpublished"))).toBe( + "${{ steps.unpublished.outputs.has-unpublished }}", + ); + }); +}); From 11150eb8a402b11fc41493a9e21d74fc67c57169 Mon Sep 17 00:00:00 2001 From: Noah Schatz Date: Mon, 19 Oct 2026 23:18:15 +0000 Subject: [PATCH 2/2] Read each package manifest once, with no existence check before it CodeQL flagged the stat-then-read in workspaceVersions as a file system race. The manifest is now read directly; a missing one (ENOENT, or ENOTDIR for a stray file under packages/) is skipped, and any other read or parse failure refuses as before. Harness-Task: 108 Co-Authored-By: Claude Opus 5.5 --- scripts/registry-presence.mjs | 16 ++++++++++------ test/registry-presence.test.ts | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/scripts/registry-presence.mjs b/scripts/registry-presence.mjs index 0200e84..b2338de 100644 --- a/scripts/registry-presence.mjs +++ b/scripts/registry-presence.mjs @@ -66,7 +66,7 @@ // non-zero either way, so the release stops regardless. import { spawn } from "node:child_process"; -import { appendFileSync, readdirSync, readFileSync, statSync, writeFileSync } from "node:fs"; +import { appendFileSync, readdirSync, readFileSync, writeFileSync } from "node:fs"; import { join, resolve } from "node:path"; import { isCliEntrypoint } from "../packages/script-utils/index.js"; @@ -449,16 +449,20 @@ export function workspaceVersions(root) { const found = []; for (const entry of entries) { const manifestPath = join(packagesDir, entry, "package.json"); + // Read once, with no existence check first: a check and a later read can disagree about the + // same path. An entry with no manifest (or a file under packages/) is not a package. + let text; try { - if (!statSync(manifestPath).isFile()) continue; - } catch { - continue; + text = readFileSync(manifestPath, "utf8"); + } catch (error) { + if (error.code === "ENOENT" || error.code === "ENOTDIR") continue; + throw new InvocationError(`cannot read ${manifestPath}: ${error.message}`); } let manifest; try { - manifest = JSON.parse(readFileSync(manifestPath, "utf8")); + manifest = JSON.parse(text); } catch (error) { - throw new InvocationError(`cannot read ${manifestPath}: ${error.message}`); + throw new InvocationError(`cannot parse ${manifestPath}: ${error.message}`); } if (manifest.private === true) continue; if (typeof manifest.name !== "string" || manifest.name === "") { diff --git a/test/registry-presence.test.ts b/test/registry-presence.test.ts index 1d380cb..dccd85b 100644 --- a/test/registry-presence.test.ts +++ b/test/registry-presence.test.ts @@ -941,6 +941,20 @@ describe("pending: the work list is what `changeset publish` would consider", () expect(() => workspaceVersions(root)).toThrow(/no publishable packages/); }); + it("skips a directory with no manifest and a stray file under packages/", () => { + const root = workspace({ real: { name: `${SCOPE}/real`, version: "1.0.0" } }); + mkdirSync(join(root, "packages", "empty"), { recursive: true }); + writeFileSync(join(root, "packages", "README.md"), "not a package\n", "utf8"); + expect(workspaceVersions(root)).toEqual([{ name: `${SCOPE}/real`, version: "1.0.0" }]); + }); + + it("refuses a manifest it cannot parse rather than skipping that package", () => { + const root = workspace({}); + mkdirSync(join(root, "packages", "broken"), { recursive: true }); + writeFileSync(join(root, "packages", "broken", "package.json"), "{ nope", "utf8"); + expect(() => workspaceVersions(root)).toThrow(/cannot parse/); + }); + it("refuses a manifest with no version rather than probing `name@undefined`", () => { const root = workspace({ broken: { name: `${SCOPE}/broken` } }); expect(() => workspaceVersions(root)).toThrow(/has no version/);