ci: harden npm release publishing#1918
Conversation
Tantanz20020918
added
size/L
github-actions
Bot
added
size/XL
📝 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.96% 74.96%
=======================================
Files 892 892
Lines 94058 94058
=======================================
Hits 70506 70506
Misses 18139 18139
Partials 5413 5413 ☔ 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@15e41759869afaee7f3e045947c2766923df3dbc🧩 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
| - name: Upload build artifact | ||
| uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 | ||
| with: | ||
| name: release-assets-${{ github.run_id }}-${{ github.run_attempt }} | ||
| path: release-assets/ | ||
| if-no-files-found: error |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
workflow="$(fd -a '^release\.yml$' .github/workflows | head -n1)"
rg -n -C2 'github\.run_attempt|release-assets-' "$workflow"Repository: larksuite/cli
Length of output: 1103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
workflow=".github/workflows/release.yml"
echo "== workflow outline =="
ast-grep outline "$workflow" --view expanded || true
echo
echo "== relevant excerpt =="
sed -n '1,260p' "$workflow" | nl -ba | sed -n '1,260p'
echo
echo "== artifact references =="
rg -n 'release-assets-|upload-artifact|download-artifact|run_attempt|run_id' "$workflow"Repository: larksuite/cli
Length of output: 283
🌐 Web query:
GitHub Actions rerun selected jobs successful upstream jobs do not rerun github.run_attempt increments artifact upload download rerun workflow jobs
💡 Result:
When you perform a partial re-run in GitHub Actions—by selecting either "re-run failed jobs" or specific jobs—the workflow execution behavior is as follows: Upstream/Successful Jobs: Jobs that previously completed successfully are not re-run [1]. They remain in their finished state, and the workflow preserves their outputs and results [1]. Only the selected failed jobs and their dependent jobs are re-executed [2][3]. Run Attempt Increment: Every time a re-run is triggered (regardless of whether you re-run all jobs, failed jobs, or specific jobs), it initiates a new attempt for the overall workflow run [4]. The github.run_attempt context variable increments by 1 for this new attempt [5][6]. Artifacts: GitHub Actions maintains a single set of artifacts for a workflow run [7][8]. When a re-run occurs, artifacts from the new attempt are associated with the run, and existing artifacts from previous attempts are overwritten if they share the same name [7][8]. Because a workflow run only ever holds one set of artifacts at a time, you cannot independently maintain artifacts from both the original run and a re-run if they have overlapping names [7][8].
Citations:
- 1: https://github.blog/news-insights/product-news/save-time-partial-re-runs-github-actions/
- 2: https://docs.github.com/en/actions/how-tos/manage-workflow-runs/re-run-workflows-and-jobs
- 3: https://docs.github.com/en/rest/actions/workflow-runs?apiVersion=2026-03-10
- 4: https://github.com/marketplace/actions/rerun-failed-jobs
- 5: https://github.com/orgs/community/discussions/10462
- 6: https://pygithub.readthedocs.io/en/v1.57/github_objects/WorkflowRun.html
- 7: https://github.com/orgs/community/discussions/17854
- 8: [bug] Aritifact getting deleted on rerun actions/upload-artifact#323
🏁 Script executed:
#!/bin/bash
set -euo pipefail
workflow=".github/workflows/release.yml"
sed -n '1,240p' "$workflow" | cat -n | sed -n '1,240p'Repository: larksuite/cli
Length of output: 9488
Keep the artifact name stable across reruns. github.run_attempt changes on reruns, so retrying a failed downstream job looks for release-assets-${{ github.run_id }}-${{ github.run_attempt }} even though the upload step wasn’t rerun. Switch the upload/download names at 81, 96, 124, and 179 to github.run_id; add overwrite: true on upload if the producer itself can rerun.
🤖 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 around lines 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.
| const fs = require("node:fs"); | ||
| const path = require("node:path"); | ||
|
|
||
| const STABLE_VERSION_PATTERN = /^(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)$/; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C2 --hidden --glob '!node_modules/**' \
'npm(@[0-9]| install .*npm)|"semver"\s*:' \
package.json package-lock.json .github Makefile 2>/dev/null || true
curl -fsSL https://raw.githubusercontent.com/npm/node-semver/main/classes/semver.js |
rg 'MAX_SAFE_INTEGER|Invalid (major|minor|patch) version'Repository: larksuite/cli
Length of output: 905
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline scripts/release-preflight.js --view expanded || true
printf '\n---\n'
cat -n scripts/release-preflight.js | sed -n '1,220p'
printf '\n---\n'
rg -n "release-preflight|STABLE_VERSION_PATTERN|tag.slice\\(1\\)|version" .github scripts -g '!node_modules/**'Repository: larksuite/cli
Length of output: 39460
Reject versions with numeric components above node-semver’s limit.
This regex still accepts values like 9007199254740992.0.0; npm/node-semver rejects components above Number.MAX_SAFE_INTEGER, so release:check can pass and the publish step fails later. Use a semver-compatible bound check for the package fields and --tag vX.Y.Z.
🤖 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` at 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.
| if git rev-parse "$TAG" >/dev/null 2>&1; then | ||
| echo "Tag ${TAG} already exists locally, skipping." | ||
| exit 0 | ||
| CURRENT_BRANCH=$(git branch --show-current) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Bind Git operations to the repository containing this script.
Preflight reads ${SCRIPT_DIR}/.., but Git uses the caller's working directory. An absolute invocation from another checkout could tag and push that checkout's origin; invocation from a subdirectory can also miss dirty root metadata.
Proposed fix
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+REPO_ROOT="$(cd "${SCRIPT_DIR}/.." && pwd)"
+cd "${REPO_ROOT}"Add a regression test that invokes the script with a non-root cwd.
Also applies to: 21-85
🤖 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/tag-release.sh` at 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.
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
| IS_DRAFT="$(node -e 'const r=require(process.argv[1]); if(typeof r.draft!=="'"'boolean"'"') process.exit(2); process.stdout.write(String(r.draft))' "$RELEASE_JSON")" | ||
| if [[ "$IS_DRAFT" == "true" ]]; then | ||
| echo "An existing mutable Draft Release for ${TAG} is not trusted; remove it and rerun." >&2 | ||
| exit 1 | ||
| fi | ||
| fi |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject existing prerelease releases for stable versions.
The verifier requires stable X.Y.Z metadata, but an existing release with prerelease: true currently passes and can be paired with a stable npm publication.
.github/workflows/release.yml#L159-L164: parse and reject bothdraftandprereleaserelease states.scripts/release-workflow.test.js#L166-L177: add a contract assertion covering prerelease rejection.
📍 Affects 2 files
.github/workflows/release.yml#L159-L164(this comment)scripts/release-workflow.test.js#L166-L177
🤖 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 around lines 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.
1 participant
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
New Features
vX.Y.Ztag validation.checksums.txtSHA-256 integrity.Bug Fixes
checksums.txtis missing or if expected SHA-256 digests are missing/invalid.Tests
script-testcoverage and added workflow contract tests for both production and preview isolation.