chore: add OIDC Trusted Publisher for NPM #13
Conversation
- Add OIDC permissions (id-token: write, contents: read) - Update to actions/setup-node@v4 and Node 20 - Add npm update step to ensure npm 11.5.1+ - Remove NODE_AUTH_TOKEN (OIDC handles authentication) - Add @types/prettier to fix build errors
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe PR adds TypeScript type definitions for Prettier as a development dependency and updates the npm publishing GitHub Actions workflow to use OIDC-based authentication with Node.js 20, npm 11.5.1+, and provenance-based publishing. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related issues
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Comment |
There was a problem hiding this comment.
Pull Request Overview
Switch publishing to npm OIDC Trusted Publisher, modernize the workflow to Node 20, ensure a compatible npm version for OIDC, and add missing type dependency.
- Add OIDC permissions and remove token-based publishing in GitHub Actions
- Upgrade runner to Node 20 and update npm during CI
- Add @types/prettier to devDependencies
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| package.json | Adds @types/prettier to devDependencies to address missing types. |
| .github/workflow/npm-publish.yaml | Configures OIDC permissions, upgrades setup-node to v4 with Node 20, updates npm, and removes NODE_AUTH_TOKEN from publish step. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflow/npm-publish.yaml (1)
26-28: Avoid using npm@latest for version pinning.The step
npm install -g npm@latestis too permissive and could introduce breaking changes. The PR states npm 11.5.1+ is required for OIDC support, so consider pinning to a specific minimum version instead.Apply this diff to pin npm to a safe, tested version:
- - name: Update npm - run: npm install -g npm@latest + - name: Update npm + run: npm install -g npm@11.5.1Alternatively, if you want to allow minor/patch updates:
- - name: Update npm - run: npm install -g npm@latest + - name: Update npm + run: npm install -g 'npm@^11.5.1'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
.github/workflow/npm-publish.yaml(2 hunks)package.json(1 hunks)
🔇 Additional comments (4)
package.json (1)
21-21: Verify @types/prettier is actually required by the codebase.
@types/prettieris being added as a devDependency, but the PR description and workflow changes don't clarify why this package is needed. If Prettier isn't used in the codebase (e.g., not imported, not configured in tsconfig, or not part of build tooling), this is an unnecessary dependency.Please confirm:
- Is Prettier actually used in this project for code formatting or as part of the build process?
- If so, is there an actual Prettier import that requires the type definitions?
- Is this addition necessary for the TypeScript build to succeed during the publish workflow?
.github/workflow/npm-publish.yaml (3)
9-11: Permissions block correctly configured for OIDC.The
id-token: writepermission enables GitHub's OIDC provider to issue a token, andcontents: readallows repository access. This is the correct permission model for OIDC authentication.
48-50: OIDC Trusted Publisher must be pre-configured on npmjs.com.The removal of
NODE_AUTH_TOKENis correct for OIDC authentication. However, this workflow requires that you have already configured therequest-injectorpackage as a Trusted Publisher on npmjs.com before this workflow will succeed. This is a critical prerequisite that cannot be skipped.Verify the setup at: https://docs.npmjs.com/creating-and-viewing-access-tokens#creating-granular-access-tokens-on-the-web
Confirm that:
- You have admin access to the
request-injectorpackage on npmjs.com- The package is registered as a Trusted Publisher with GitHub (organization: RequestNetwork, repository: request-cli, workflow: npm-publish.yaml)
- The package on npmjs.com allows publishing from this GitHub Actions workflow via OIDC
Without this external configuration, the
npm publishstep will fail with authentication errors.
21-21: The review comment's premise about version upgrades is inaccurate and does not reflect reality.setup-node@v3 ran on Node 16, not Node 18, and setup-node@v4 upgraded the action runtime to Node 20. More importantly, verification shows this upgrade is entirely safe for this project:
- No engine constraint exists: package.json has no explicit Node version requirement, providing full compatibility flexibility
- Dependencies are compatible: @types/node is already at ^22.4.1 (targeting Node 22+), well-aligned with Node 20 LTS
- @types/prettier has no issues: It's a simple typing package with no known Node 20 compatibility concerns
- Existing dependencies are modern: typescript ^5.5.4 and other dev dependencies have no Node 20 incompatibilities
The upgrade proceeds without risk. No compatibility verification is needed.
bassgeta
left a comment
bassgeta
left a comment
There was a problem hiding this comment.
Looking good 😎
Although Copilot is on to something here, why do we have a directory named workflow instead of workflows? 🤔
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Problem
NPM package publishing uses long-lived tokens (NODE_AUTH_TOKEN) which pose security risks if compromised or exposed.
Solution
Considerations
Summary by CodeRabbit