-
-
Notifications
You must be signed in to change notification settings - Fork 1
infra: enable trusted publishing #28
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
|
WalkthroughUpdated the GitHub Actions release workflow to add an "Update npm" step before installing dependencies, change the changesets action reference to a specific commit, and remove Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer (labels PR)
participant GH as GitHub Actions (release.yml)
participant Registry as npm Registry
Dev->>GH: Label PR / trigger workflow
GH->>GH: Update npm (npm install -g npm@latest)
GH->>GH: Install dependencies
GH->>Registry: Publish using GITHUB_TOKEN / OIDC (no NPM_TOKEN)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
ae65b32
to
5ce0e85
Compare
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: 0
🔭 Outside diff range comments (1)
.github/workflows/release.yml (1)
54-56
: Re-enable provenance to attach SLSA metadata to npm publishesYou removed NPM_CONFIG_PROVENANCE. While OIDC auth enables trusted publishing without NPM_TOKEN, you still need provenance enabled to embed build provenance in the package. The safest way is to set the npm config via env so all internal
npm publish
invocations inherit it.Apply this diff:
env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + NPM_CONFIG_PROVENANCE: true
Alternatively, ensure your publish command includes
--provenance
or run a prior stepnpm config set provenance true
.
🧹 Nitpick comments (3)
.github/workflows/release.yml (3)
7-9
: Guard PR-labeled runs to a specific label to avoid unintended releasesTriggering on any label event can fire this workflow frequently. If the intent is to only act on a specific release label, add a job-level guard.
Apply this diff to restrict the job to either pushes (main) or PRs labeled with "release":
jobs: release: name: Release + if: ${{ github.event_name == 'push' || (github.event_name == 'pull_request' && github.event.action == 'labeled' && github.event.label.name == 'release') }} runs-on: ubuntu-latest
40-43
: Pin npm to a minimum version instead of floating to latestUsing npm@latest harms reproducibility and may introduce breaking changes unexpectedly. Since trusted publishing requires a minimum npm version, pin to a semver range that satisfies it.
Apply this diff:
- - name: Update npm - run: npm install -g npm@latest + - name: Update npm + run: npm install -g npm@^11.5.1
34-39
: Set npm registry in setup-node for trusted publishingExplicitly configuring the npm registry ensures the CLI talks to the public npm registry and sets auth appropriately in the environment. This is a common prerequisite in trusted publisher setups.
Apply this diff:
- name: Setup Node.js LTS uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: node-version: lts/* cache: pnpm + registry-url: 'https://registry.npmjs.org' + always-auth: true
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/release.yml
(2 hunks)
5ce0e85
to
31c6543
Compare
31c6543
to
3fd7750
Compare
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: 0
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
37-40
: Pin npm to a minimum version instead of using latest.Using npm@latest makes the workflow fragile to upstream breaking changes. Since the comment states “Ensure npm 11.5.1 or later,” pin to a caret range and log the version.
Apply this diff:
- # Ensure npm 11.5.1 or later is installed - - name: Update npm - run: npm install -g npm@latest + # Ensure npm 11.5.1 or later is installed + - name: Update npm + run: | + npm i -g npm@^11.5.1 + npm --version
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/release.yml
(1 hunks)
🔇 Additional comments (2)
.github/workflows/release.yml (2)
46-46
: Good: Pinning changesets/action to a specific commit.Pinning to the v1.5.3 commit hash improves supply-chain security and reproducibility. LGTM.
44-53
: Publish step uses npm CLI via Changesets – no updates neededVerified that your workflow’s
publish: pnpm release
invokes the rootrelease
script (pnpm build && changeset publish
), andchangeset publish
shells out tonpm publish
by default. Trusted Publishing’s OIDC token will be applied as expected.
closes #27
Summary by CodeRabbit