ci: harden npm release publishing#1918
Merged
Merged
Conversation
Tantanz20020918
added
size/L
github-actions
Bot
added
size/XL
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe release pipeline adds stable version and tag preflight validation, stricter tag creation gates, a preflight-gated publishing job, expanded workflow tests, and fail-closed installer checksum verification. ChangesRelease integrity and publishing
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1918 +/- ##
==========================================
+ Coverage 74.98% 75.05% +0.06%
==========================================
Files 895 901 +6
Lines 94379 95391 +1012
==========================================
+ Hits 70767 71591 +824
- Misses 18197 18307 +110
- Partials 5415 5493 +78 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@4cab173f8b3d2ca49cbad521cea7bbea42c7437f🧩 Skill updatenpx skills add larksuite/cli#ci/npm-secure-publishing -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
scripts/release-preflight.test.js (1)
326-355: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise each release gate behaviorally, not only through source-text ordering.
Add subprocess failure cases for a non-
mainbranch, staged/unstaged metadata, andHEAD != FETCH_HEAD, asserting that no tag query, creation, or push occurs. The fakediffcommand will need configurable exit statuses.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/release-preflight.test.js` around lines 326 - 355, Extend the release preflight tests around the existing tag-release subprocess harness to execute failure cases for a non-main branch, staged or unstaged package metadata, and HEAD differing from FETCH_HEAD. Make the fake diff command support configurable exit statuses, assert each gate rejects the release, and verify no tag query, creation, or push command is reached in every case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 78-83: Update the release artifact naming in the upload and
corresponding download steps to use only github.run_id, removing
github.run_attempt so reruns reference the same artifact. Apply this
consistently to the artifact references near the upload/download steps at the
indicated locations, and add overwrite: true to the upload configuration if the
producing job can rerun.
In `@scripts/release-preflight.js`:
- Line 8: Update the version validation around STABLE_VERSION_PATTERN and the
release:check flow to reject any major, minor, or patch component exceeding
Number.MAX_SAFE_INTEGER, matching node-semver behavior. Apply the same
semver-compatible bound check to package fields and the generated --tag vX.Y.Z
value while preserving acceptance of valid stable versions.
In `@scripts/tag-release.sh`:
- Line 15: Bind all Git operations in the release script, including
CURRENT_BRANCH and the commands in the affected preflight, tagging, and push
flow, to the repository root derived from SCRIPT_DIR rather than the caller’s
working directory. Preserve existing release behavior while ensuring absolute
invocations and non-root working directories operate on the script’s checkout.
Add a regression test that invokes the script from a non-root cwd and verifies
Git operations target that repository.
---
Nitpick comments:
In `@scripts/release-preflight.test.js`:
- Around line 326-355: Extend the release preflight tests around the existing
tag-release subprocess harness to execute failure cases for a non-main branch,
staged or unstaged package metadata, and HEAD differing from FETCH_HEAD. Make
the fake diff command support configurable exit statuses, assert each gate
rejects the release, and verify no tag query, creation, or push command is
reached in every case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f45e910c-1f08-4585-b41d-4f892df52801
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
.github/workflows/release.ymlMakefilepackage.jsonscripts/install.jsscripts/install.test.jsscripts/release-preflight.jsscripts/release-preflight.test.jsscripts/release-workflow.test.jsscripts/tag-release.shscripts/verify-release-assets.jsscripts/verify-release-assets.test.js
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 159-164: Update the existing release-state parsing in
.github/workflows/release.yml at lines 159-164 to validate both draft and
prerelease fields, rejecting the release when either state is true while
preserving the current error-and-exit behavior. Add a contract assertion in
scripts/release-workflow.test.js at lines 166-177 covering rejection of an
existing prerelease release.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2ebb1004-d0d3-404d-8bba-cc086ce174f0
📒 Files selected for processing (2)
.github/workflows/release.ymlscripts/release-workflow.test.js
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/verify-release-assets.js (1)
86-92: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify array operations.
archiveEntriesis only used to generatearchiveNames, and sorting is performed twice. You can chain these operations and eliminate the redundant intermediate sort.♻️ Proposed refactor
- const archiveEntries = entries - .filter((entry) => entry.isFile() && isReleaseArchive(entry.name)) - .sort((left, right) => left.name.localeCompare(right.name)); - - const archiveNames = archiveEntries - .map((entry) => entry.name) - .sort(); + const archiveNames = entries + .filter((entry) => entry.isFile() && isReleaseArchive(entry.name)) + .map((entry) => entry.name) + .sort();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/verify-release-assets.js` around lines 86 - 92, In the archive-name construction flow, remove the intermediate archiveEntries sort and chain the file filtering, name mapping, and final sorting directly into archiveNames. Preserve the existing file and release-archive filters and the alphabetical ordering of the resulting names.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/verify-release-assets.js`:
- Around line 86-92: In the archive-name construction flow, remove the
intermediate archiveEntries sort and chain the file filtering, name mapping, and
final sorting directly into archiveNames. Preserve the existing file and
release-archive filters and the alphabetical ordering of the resulting names.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d62d8ab0-1689-4c9e-8823-2a359a162388
📒 Files selected for processing (6)
.github/workflows/release.ymlscripts/release-preflight.jsscripts/release-preflight.test.jsscripts/release-workflow.test.jsscripts/verify-release-assets.jsscripts/verify-release-assets.test.js
💤 Files with no reviewable changes (3)
- .github/workflows/release.yml
- scripts/release-workflow.test.js
- scripts/release-preflight.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/release-preflight.js
github-actions
Bot
added
size/L
There was a problem hiding this comment.
🧹 Nitpick comments (2)
scripts/release-preflight.js (1)
19-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify property access with optional chaining.
Since the project uses Node 22, you can use optional chaining (
?.) to make these nested property accesses much more concise and easier to read.♻️ Proposed refactor
- const packageVersion = packageJson && packageJson.version; - const lockVersion = packageLockJson && packageLockJson.version; - const lockRootVersion = packageLockJson && packageLockJson.packages && - packageLockJson.packages[""] && packageLockJson.packages[""].version; + const packageVersion = packageJson?.version; + const lockVersion = packageLockJson?.version; + const lockRootVersion = packageLockJson?.packages?.[""]?.version;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/release-preflight.js` around lines 19 - 22, Update the version assignments for packageVersion, lockVersion, and lockRootVersion to use optional chaining instead of repeated && checks, preserving their current undefined behavior when any parent property is absent..github/workflows/release.yml (1)
21-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInconsistent
setup-nodeversions across jobs.preflightpinsactions/setup-node@…# v4whilepublish-releaseuses# v6. Both target Node22.14.0, so behavior is equivalent, but aligning the two avoids drift and matches the contract test that expects v6.Also applies to: 64-64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml at line 21, Align the actions/setup-node version in the preflight job with the v6 pin already used by publish-release, updating the referenced commit and version comment while preserving the existing Node 22.14.0 configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/release.yml:
- Line 21: Align the actions/setup-node version in the preflight job with the v6
pin already used by publish-release, updating the referenced commit and version
comment while preserving the existing Node 22.14.0 configuration.
In `@scripts/release-preflight.js`:
- Around line 19-22: Update the version assignments for packageVersion,
lockVersion, and lockRootVersion to use optional chaining instead of repeated &&
checks, preserving their current undefined behavior when any parent property is
absent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2fc0a26e-722e-4ee7-896e-7df3206cf56e
📒 Files selected for processing (6)
.github/workflows/release.ymlMakefilescripts/release-preflight.jsscripts/release-preflight.test.jsscripts/release-workflow.test.jsscripts/tag-release.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- Makefile
github-actions
Bot
added
size/M
# Conflicts: # package.json
liangshuo-1
approved these changes
Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrate npm releases from a long-lived token to trusted publishing, and add fail-closed validation before release side effects.
Changes
npm-productionenvironmentTest Plan
make unit-testpassedmake script-testpassed (184/184)go vet ./...,gofmt -l .,go mod tidy, and incremental lint passednode scripts/release-preflight.js,bash -n scripts/tag-release.sh, andgit diff --checkRelated Issues
N/A
Summary by CodeRabbit
Security
Release Process
mainbranch and rejects existing local or remote tags.Testing