Skip to content

fix: use NPM_TOKEN for npm publish#21

Open
jack-arturo wants to merge 1 commit intomainfrom
fix/npm-publish-auth
Open

fix: use NPM_TOKEN for npm publish#21
jack-arturo wants to merge 1 commit intomainfrom
fix/npm-publish-auth

Conversation

@jack-arturo
Copy link
Member

OIDC trusted publishing isn't working. Fall back to token-based auth.

@coderabbitai
Copy link

coderabbitai bot commented Jan 6, 2026

📝 Walkthrough

Summary by CodeRabbit

  • Chores
    • Updated internal release workflow configuration.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Updates the npm publishing workflow in GitHub Actions to use OIDC-based authentication instead of implicit token handling. Replaces registry-url configuration with direct npmrc file generation containing the auth token from NODE_AUTH_TOKEN secret, and adds provenance flag to npm publish command.

Changes

Cohort / File(s) Summary
CI/CD Workflow Configuration
.github/workflows/release-please.yml
Changes npm publishing authentication from implicit/registry-url approach to OIDC-based method. Adds echo command to generate npmrc with auth token from NODE_AUTH_TOKEN secret. Updates npm publish step to include provenance flag. Modifies publishing step comment to reflect OIDC-based approach.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: switching from OIDC to token-based npm publishing using NPM_TOKEN.
Description check ✅ Passed The description clearly explains the reason for the change: OIDC trusted publishing wasn't working, necessitating a fallback to token-based authentication.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/npm-publish-auth

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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

Fix all issues with AI Agents 🤖
In @.github/workflows/release-please.yml:
- Around line 62-67: The workflow step that currently echoes an npm token into
~/.npmrc and runs `npm publish --access public --provenance` is inconsistent
with the PR objective/inline comment; update the inline comment to state this
step is a token-based fallback, ensure the env var and secret usage is explicit
(use NODE_AUTH_TOKEN from secrets.NPM_TOKEN) and confirm the echo line correctly
interpolates that token, and either verify that `--provenance` is supported with
token-based publishing (and add a short comment if verified) or remove the
`--provenance` flag if it is not supported.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
.github/workflows/release-please.yml (2)

18-19: Update comment to reflect token-based authentication.

The comment claims "Uses Trusted Publishing (OIDC) - no secrets needed!" but the implementation at lines 63-67 uses secrets.NPM_TOKEN for token-based authentication. This contradicts the PR objective to "fall back to token-based auth."

🔎 Proposed fix
-# npm Publishing:
-#   Uses Trusted Publishing (OIDC) - no secrets needed!
-#   Configure at: https://www.npmjs.com/package/@verygoodplugins/mcp-{name}/access
+# npm Publishing:
+#   Uses token-based authentication via NPM_TOKEN secret
+#   Configure the NPM_TOKEN secret in repository settings

48-50: Remove unnecessary id-token: write permission and correct the misleading comment.

The id-token: write permission is only required for OIDC trusted publishing. However, this workflow uses token-based authentication (NPM_TOKEN), making this permission unnecessary. Additionally, the comment on line 62 stating "Trusted Publishing via OIDC - no NPM_TOKEN needed" contradicts the actual implementation which explicitly uses NPM_TOKEN. Either migrate to proper OIDC trusted publishing (which requires removing NPM_TOKEN and configuring a trusted publisher on npmjs.com) or simplify to token-based auth by removing id-token: write and updating the comment.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 12ffa27 and b9c93f9.

📒 Files selected for processing (1)
  • .github/workflows/release-please.yml

Comment on lines +62 to +67
# Trusted Publishing via OIDC - no NPM_TOKEN needed
- run: |
echo "//registry.npmjs.org/:_authToken=\${NODE_AUTH_TOKEN}" > ~/.npmrc
npm publish --access public --provenance
env:
NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical inconsistency between PR objectives, comment, and implementation.

There are several contradictions here:

  1. PR objective states: "OIDC trusted publishing isn't working. Fall back to token-based auth"
  2. Line 62 comment claims: "Trusted Publishing via OIDC - no NPM_TOKEN needed"
  3. Line 67 implementation: Uses secrets.NPM_TOKEN for authentication

Additionally, the --provenance flag at line 65 is typically associated with OIDC trusted publishing. If you're truly falling back to token-based authentication, you should:

  • Update the comment to reflect token-based authentication
  • Verify that --provenance works with token-based auth (or remove it if it doesn't)
  • Ensure the npmrc configuration approach is correct for your use case
🔎 Proposed fix for consistent token-based authentication
-      # Trusted Publishing via OIDC - no NPM_TOKEN needed
+      # Token-based authentication using NPM_TOKEN
       - run: |
           echo "//registry.npmjs.org/:_authToken=\${NODE_AUTH_TOKEN}" > ~/.npmrc
-          npm publish --access public --provenance
+          npm publish --access public
         env:
           NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}

Note: If you want to keep --provenance for attestation generation, verify that it works with token-based auth and document this in the comment.

🤖 Prompt for AI Agents
In @.github/workflows/release-please.yml around lines 62-67, The workflow step
that currently echoes an npm token into ~/.npmrc and runs `npm publish --access
public --provenance` is inconsistent with the PR objective/inline comment;
update the inline comment to state this step is a token-based fallback, ensure
the env var and secret usage is explicit (use NODE_AUTH_TOKEN from
secrets.NPM_TOKEN) and confirm the echo line correctly interpolates that token,
and either verify that `--provenance` is supported with token-based publishing
(and add a short comment if verified) or remove the `--provenance` flag if it is
not supported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant