-
-
Notifications
You must be signed in to change notification settings - Fork 40
feat: add support for npm Trusted Publishers with NPM_ID_TOKEN #227
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 1e3d823 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughREADME adds two npm publish authentication methods: Trusted Publishers using NPM_ID_TOKEN and Classic Automation Token using NPM_TOKEN. Code now detects NPM_ID_TOKEN to skip .npmrc creation, falls back to NPM_TOKEN to create .npmrc, or errors if neither is present. Types and a changeset were added. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CI as CI Runner
participant Action as Publish Action
participant Env as Environment
participant FS as File System
participant NPM as npm Registry
CI->>Action: Invoke publish
Action->>Env: Read NPM_ID_TOKEN, NPM_TOKEN
alt Existing .npmrc present
Action->>FS: Use existing .npmrc
Action->>NPM: npm publish
else No .npmrc
alt NPM_ID_TOKEN present (Trusted Publishers)
Note over Action: Trusted Publishers detected — skip .npmrc
Action->>NPM: npm publish (OIDC/ID token)
else NPM_TOKEN present (Classic)
Action->>FS: Create .npmrc with //registry.npmjs.org/:_authToken=${NPM_TOKEN}
Action->>NPM: npm publish
else Neither token present
Action-->>CI: Fail (no NPM_TOKEN/NPM_ID_TOKEN and no .npmrc)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to d8321bc in 1 minute and 1 seconds. Click for details.
- Reviewed
84lines of code in3files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. README.md:92
- Draft comment:
Clear documentation for npm Trusted Publishers added. Consider clarifying what happens if both NPM_ID_TOKEN and NPM_TOKEN are set (i.e. that NPM_ID_TOKEN takes precedence). - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. src/main.ts:25
- Draft comment:
NPM_ID_TOKEN is correctly destructured from env. Add a brief inline comment noting that if both tokens are provided, NPM_ID_TOKEN is prioritized. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. src/main.ts:72
- Draft comment:
The conditional branch for .npmrc creation handles Trusted Publishers mode well. Consider logging a warning if both NPM_ID_TOKEN and NPM_TOKEN are set, to make the precedence explicit for users. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
4. src/types.ts:31
- Draft comment:
Addition of the NPM_ID_TOKEN field in the Env type is clear and properly documented. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_o3nypJU0XSTncZRE
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main.ts (1)
69-88: Consider extracting .npmrc handling to reduce cognitive complexity.The cognitive complexity is slightly above the threshold (16 vs 15). While the code is readable, extracting the
.npmrchandling logic into a separate function could improve maintainability.Example refactor:
async function ensureNpmrc( npmrcPath: string, NPM_ID_TOKEN: string | undefined, NPM_TOKEN: string | undefined ): Promise<void> { if (fs.existsSync(npmrcPath)) { console.log('Found existing .npmrc file') } else if (NPM_ID_TOKEN) { console.log( 'Detected `NPM_ID_TOKEN`; skipping `.npmrc` creation (Trusted Publishers mode).', ) } else if (NPM_TOKEN) { console.log('No .npmrc file found, creating one with `NPM_TOKEN`') await fs.promises.writeFile( npmrcPath, `//registry.npmjs.org/:_authToken=${NPM_TOKEN}`, ) } else { setFailed( 'No `.npmrc` found and neither `NPM_TOKEN` nor `NPM_ID_TOKEN` provided, unable to publish packages', ) throw new Error('No npm authentication available') } }Then in the main function:
try { await ensureNpmrc(npmrcPath, NPM_ID_TOKEN, NPM_TOKEN) } catch { return }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(2 hunks)src/main.ts(2 hunks)src/types.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main.ts (1)
src/env.ts (2)
env(10-33)GITLAB_TOKEN(24-32)
🪛 GitHub Actions: CI
src/main.ts
[error] 24-29: SonarJS cognitive complexity limit exceeded. Refactor this function to reduce cognitive complexity from 16 to ≤15.
README.md
[warning] 1-1: Unexpected GFM autolink literal, expected regular autolink; add '<' before and '>' after
🪛 GitHub Check: Lint and Test with Node.js 18
README.md
[warning] 94-94:
{"reason":"Unexpected GFM autolink literal, expected regular autolink, add < before and > after","source":"remark-lint","ruleId":"no-literal-urls","severity":1}
🪛 GitHub Check: Lint and Test with Node.js 20
README.md
[warning] 94-94:
{"reason":"Unexpected GFM autolink literal, expected regular autolink, add < before and > after","source":"remark-lint","ruleId":"no-literal-urls","severity":1}
🪛 GitHub Check: Lint and Test with Node.js 22
README.md
[warning] 94-94:
{"reason":"Unexpected GFM autolink literal, expected regular autolink, add < before and > after","source":"remark-lint","ruleId":"no-literal-urls","severity":1}
🪛 markdownlint-cli2 (0.18.1)
README.md
94-94: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (8)
src/types.ts (1)
30-32: LGTM! Clear type definition with helpful comments.The addition of
NPM_ID_TOKENis well-documented and aligns with the implementation inmain.tswhere this token is used to skip.npmrccreation in Trusted Publishers mode.src/main.ts (4)
25-25: LGTM!Correct destructuring of the new
NPM_ID_TOKENenvironment variable.
72-76: LGTM! Correct Trusted Publishers detection.The logic correctly detects
NPM_ID_TOKENand skips.npmrccreation, allowing npm to handle the OIDC token exchange internally.
78-78: LGTM! Improved log message clarity.Explicitly mentioning
NPM_TOKENhelps users understand which authentication method is being used.
85-85: LGTM! Clear error message.The error message now correctly indicates both authentication options, helping users understand what's missing.
README.md (3)
92-93: LGTM! Clear documentation of authentication methods.The documentation accurately describes both authentication approaches and provides helpful guidance for users.
Also applies to: 95-96
122-122: LGTM! Clear clarification of .npmrc behavior.The note accurately explains when the
.npmrcfile is created in classic token mode.
129-129: LGTM! Helpful note about Trusted Publishers.The note clearly explains that no
.npmrcis required when using Trusted Publishers withNPM_ID_TOKEN.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to 1e3d823 in 38 seconds. Click for details.
- Reviewed
95lines of code in4files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .changeset/yummy-hairs-enjoy.md:2
- Draft comment:
Changeset file for a minor update looks good. Ensure that the minor version bump is intended. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. README.md:92
- Draft comment:
The README update clearly explains both authentication methods. Consider verifying the formatting of the providedcat << EOFcode block to avoid unintended whitespace in the generated .npmrc. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. src/main.ts:72
- Draft comment:
The token handling logic now prioritizes NPM_ID_TOKEN over NPM_TOKEN by skipping .npmrc creation when NPM_ID_TOKEN is set. Consider adding a code comment to document that if both tokens are present, Trusted Publishers mode takes precedence. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. src/types.ts:29
- Draft comment:
The addition of NPM_ID_TOKEN in the Env type is clear and aligns with the updated authentication methods. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_9lLxGRUIOmOg3VuU
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.



Important
Add support for npm Trusted Publishers using
NPM_ID_TOKEN, modifying.npmrccreation and updating documentation.NPM_ID_TOKENinsrc/main.ts..npmrccreation ifNPM_ID_TOKENis detected..npmrccreation withNPM_TOKENif no.npmrcexists.src/main.tsfor missing tokens.README.mdto include setup for Trusted Publishers and classic token flows..npmrccreation behavior inREADME.md.This description was created by
for 1e3d823. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Documentation