Update publish#50
Conversation
![constructor-claude-bedrock[bot]](https://avatars.githubusercontent.com/u/9828194?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This PR migrates NPM publishing from token-based authentication (NPM_TOKEN secret) to OIDC-based Trusted Publishing, which is a good security improvement. The approach is mostly correct but has a few issues to address before merging.
Inline comments: 3 discussions added
Overall Assessment:
| - name: Install npm 11 | ||
| run: npm install -g npm@11 | ||
|
|
||
| - name: Debug node and npm versions |
There was a problem hiding this comment.
Important Issue: This is a debug step that should not be committed to a production workflow. Remove it before merging. Version information is already surfaced implicitly by the runner logs and does not warrant a dedicated step in a persistent workflow.
| token: ${{ secrets.NPM_TOKEN }} | ||
|
|
||
| - name: Install npm 11 | ||
| run: npm install -g npm@11 |
There was a problem hiding this comment.
Important Issue: Pinning to a floating major version (npm@11) makes the workflow non-deterministic — patch/minor releases within npm v11 may introduce unexpected behaviour over time. Pin to a specific version (e.g. npm@11.4.1) to guarantee reproducible builds. Additionally, installing npm globally as a workflow step adds latency and a potential point of failure; consider whether the npm version bundled with Node 22.18.x is insufficient before adding this step.
| version: ${{ inputs.version }} | ||
|
|
||
| - uses: actions/setup-node@v3 | ||
| - uses: actions/setup-node@v4 |
There was a problem hiding this comment.
Critical Issue: When publishing to npm via OIDC (Trusted Publishers), actions/setup-node must be configured with registry-url: https://registry.npmjs.org/ so that the action writes the correct .npmrc configuration and sets NODE_AUTH_TOKEN from the OIDC token exchange. Without registry-url, the generated .npmrc is absent and npm publish will either fail to authenticate or silently skip provenance attestation. Add:
- uses: actions/setup-node@v4
with:
node-version: 22.18.x
registry-url: https://registry.npmjs.org/See: https://docs.npmjs.com/generating-provenance-statements#publishing-provenance-in-a-cicd-workflow
2 participants
No description provided.