ci: install npm via direct tarball (corepack doesn't replace PATH)#20
Merged
ci: install npm via direct tarball (corepack doesn't replace PATH)#20
Conversation
The prior fix (cf65d33) used `corepack prepare [email protected] --activate` to sidestep the broken bundled npm in Node 22.22.2. Corepack's prepare staged the new npm in its cache but did not actually replace the npm binary in the PATH — `npm --version` still printed 10.9.7 after the activation step. Since npm 10.x does not support OIDC trusted publishing, the subsequent `npm publish` fell back to token auth and failed with a 404 against the npm registry. Replace the corepack approach with a direct tarball download from registry.npmjs.org, extracting over the broken bundled install in $NODE_DIR/lib/node_modules/npm. This guarantees `npm --version` reports the expected 11.6.0 and OIDC trusted publishing works. Verified root cause from run 24176156466: publish > Install a working npm Preparing [email protected] for immediate activation... 10.9.7 publish > Publish to npm npm error 404 Not Found - PUT https://registry.npmjs.org/@jim80net%2fmemex-core Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/release-please.yml">
<violation number="1" location=".github/workflows/release-please.yml:76">
P2: The downloaded npm tarball has no integrity/checksum verification before being installed. This workflow publishes packages with OIDC trusted publishing (`id-token: write`), so a tampered tarball would gain the ability to push arbitrary code to the npm registry. Pin the expected `shasum` (from the registry's `dist.integrity` field) and verify it after download.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
Identified by cubic on PR #20: the direct tarball download had no integrity verification before being installed into $NODE_DIR/lib/node_modules/npm. Since this workflow holds id-token: write for OIDC trusted publishing, a tampered tarball would gain the ability to publish arbitrary code as @jim80net/memex-core. Fetch the expected sha512 integrity value from the registry's package metadata first, then compare it against openssl dgst of the downloaded tarball before extracting. Both requests target registry.npmjs.org so the trust model is unchanged, but this protects against in-transit tampering between the metadata fetch and the tarball fetch, and enforces that we install the exact bytes the registry says 11.6.0 should be. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
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.
Summary
Second attempt at fixing the v0.4.0 publish. PR #19 (merged as cf65d33) switched the broken
npm install -g npm@latesttocorepack prepare [email protected] --activate, but the subsequent manual publish dispatch still failed — with a different error.Root cause
From run 24176156466:
```
publish > Install a working npm
Preparing [email protected] for immediate activation...
10.9.7 ← npm --version still prints the OLD bundled version
publish > Publish to npm
npm error code E404
npm error 404 Not Found - PUT https://registry.npmjs.org/@jim80net%2fmemex-core - Not found
```
Corepack's `prepare --activate` stages the package manager in the corepack cache but does not replace the `npm` binary in PATH that the subsequent steps see. `npm --version` printed `10.9.7` — the original broken-bundled version. npm 10.x does not support OIDC trusted publishing (which requires 11.5.1+), so `npm publish` fell back to token auth and hit a 404 against the registry.
Fix
Replace corepack with a direct tarball download. Download `npm-11.6.0.tgz` from registry.npmjs.org, extract over `$NODE_DIR/lib/node_modules/npm`, and `hash -r` to flush the shell's command cache. Hacky but bulletproof — `npm --version` will now report `11.6.0` and OIDC trusted publishing will work.
Recovery plan after merge
Same as before — trigger the publish manually:
```bash
gh workflow run release-please.yml
```
Then verify with `npm view @jim80net/[email protected]`.
🤖 Generated with Claude Code
Summary by cubic
Fix release workflow by installing
[email protected]from a tarball with integrity verification. Ensures the PATHnpmis 11.6.0 so OIDC trusted publishing works.Bug Fixes
[email protected]via tarball over the bundlednpmin Node 22.22.2; flush command cache. Avoids token-auth fallback and 404; enables OIDCnpm publish.sha512integrity from the registry before extraction to prevent tampering on a job withid-token: write.Migration
gh workflow run release-please.ymlto publish v0.4.0.npm view @jim80net/[email protected].Written for commit 36d975f. Summary will update on new commits.