ci(publish): keep the dev toolchain out of the job that can mint the npm OIDC token (PHARN-07) - #201
Merged
Merged
Conversation
…npm OIDC token (PHARN-07) publish.yml ran `npm ci` (dev-dependency install scripts) and `npm publish` (prepublishOnly: prettier, eslint, markdownlint, tsc, vitest; prepack: esbuild) in ONE job holding `id-token: write`, so any of ~280 dev dependencies could request the OIDC token and publish a malicious version with valid provenance. Now two jobs: - build (no id-token): strict `^vX.Y.Z$` tag == package.json version on a commit contained in main, checked BEFORE npm ci; then check + test:coverage, npm pack, smoke-install of the tarball, upload as an artifact. - publish (the only job with id-token + npm-publish env): Assert npm floor, download the tarball, `npm publish pkg/pharn-dev-pharn-<version>.tgz --provenance --access public --ignore-scripts`. No checkout, no install. check-run-pins.test.mjs pins that the grant appears once, inside `publish`, which installs nothing; the live install count goes 9 -> 10. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TvcuVhk8hTeDskp5pAJhnc
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
8 of 11 tasks
PrzemekGalarowicz
added a commit
that referenced
this pull request
Sep 24, 2026
… workflow (#213) publish.yml's build job ran `npm pack --pack-destination "$RUNNER_TEMP/pkg"` without creating `pkg/`, and npm does not create the destination (ENOENT on npm 10.9.7 and 11.20.0). Every Release run would fail at Pack and never reach the publish job. The defect came in with the build/publish split (#201) and was invisible to PR CI, because publish.yml only runs on a published Release. The Pack step now runs `mkdir -p` first. A live test in check-run-pins.test.mjs pins that every workflow's pack destination is either the runner temp root or created earlier in the same job, with a positive control against the live publish.yml. Claude-Session: https://claude.ai/code/session_0199owRmYfskqYQVQrVP679o Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
publish.ymldid everything in one job that hasid-token: write:npm ci(which runs dev-dependency install scripts) and thennpm publish, which in turn ran theprepublishOnlygates (prettier, eslint, markdownlint, tsc, vitest) and theprepackesbuild step. Withid-token: write, any step in the job can request the OIDC token that npm exchanges for publish rights. So a single compromised dev dependency, out of about 280, could have published a malicious version, and it would have carried a valid provenance attestation.The workflow is now split into two jobs:
build— noid-token. Before installing anything it checks three things:^v[0-9]+\.[0-9]+\.[0-9]+$;package.jsonversion;git merge-base --is-ancestor "$GITHUB_SHA" origin/main, i.e. the tagged commit is onmain.Then it runs
npm ci,npm run check,npm run test:coverageandnpm pack(the build runs as part of pack). It installs the packed tarball into a scratch directory, runspharn --version, and uploads the tarball as an artifact.publish— the only job withid-token: writeand thenpm-publishenvironment. It does no checkout and no install. It runs the existing Assert npm floor step, downloads the artifact, and runsnpm publish pkg/pharn-dev-pharn-<version>.tgz --provenance --access public --ignore-scripts.<version>is the one thebuildjob verified.Supporting changes:
.dev/floor/check-run-pins.test.mjs:id-token: writemust appear exactly once, inside thepublishjob, and that job must not check out or install anything and must use--ignore-scripts. It fails on the old file. The same file's count of workflow installs goes from 9 to 10.docs/RELEASING.mdstep 5 is rewritten for the two jobs;CLAUDE.mdReleasing section updated.Built with
/pharn-dev-ship; stage artifacts are in.dev/features/publish-split-oidc/. Results:no-regressionsPASSType of change
feat— new stack option, wizard step, or command capabilityfix— bug fixdocs— docs-only changechore/refactor— tooling or internal restructure, no behavior changeArea(s) touched
repo tooling (publish.yml, floor test) | docs
Checklist
docs/RELEASING.md.uses:is pinned to a SHA;check-action-pinsandcheck-run-pinsreport no violations.Quality gates
npm run checkpasses locally (1335/1335; non-root user, node 22).npm run build/npm run test:coverage(left to CI).Notes for the reviewer
actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2andactions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0are pinned from memory. I had no GitHub access outside this repo to check them, and you asked me to assume they work. The first release run is their real test. If a SHA is wrong, the run fails before anything is published. Please check them against the upstream tags before the next release.buildcan still change the contents of the tarball. What it can no longer do is get the OIDC token or publish by itself.🤖 Generated with Claude Code
https://claude.ai/code/session_01TvcuVhk8hTeDskp5pAJhnc
Generated by Claude Code