ci: harden npm release publishing#1918
Open
Tantanz20020918 wants to merge 5 commits into
Open
Conversation
Tantanz20020918
added
size/L
github-actions
Bot
added
size/XL
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThe release pipeline validates stable metadata, verifies generated assets, publishes npm artifacts idempotently, and creates or verifies matching GitHub Release assets. Tagging gains stricter repository checks, while installer checksum validation fails closed on missing or malformed data. ChangesRelease integrity and publishing
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ReleaseTrigger
participant Preflight
participant GoReleaser
participant AssetVerifier
participant PublishRelease
participant GitHubRelease
participant NpmRegistry
ReleaseTrigger->>Preflight: validate tag and package metadata
Preflight->>GoReleaser: permit non-publishing build
GoReleaser->>AssetVerifier: provide release-assets artifact
AssetVerifier->>PublishRelease: pass verified artifact
PublishRelease->>GitHubRelease: create or verify release and compare assets
PublishRelease->>NpmRegistry: compare integrity and publish if needed
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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% 74.98%
=======================================
Files 895 895
Lines 94379 94379
=======================================
Hits 70767 70767
Misses 18197 18197
Partials 5415 5415 ☔ 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@964c571063b1e10ad62a9fef838eb12c57062087🧩 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
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
1 participant
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
release:checkpreflight command for stable, synchronized package versions and optionalvX.Y.Ztag validation.checksums.txt.checksums.txtis missing and enforces strict SHA-256 checksum format validation.