ci: notarize macOS release candidates - #2073
Merged
Merged
Conversation
|
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 now supports stable and beta versions, validates protected tag provenance, builds signed and notarized candidates, manages draft GitHub releases, verifies macOS artifacts, and publishes or verifies npm packages. ChangesRelease pipeline
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 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 #2073 +/- ##
=======================================
Coverage 75.69% 75.69%
=======================================
Files 942 942
Lines 100079 100079
=======================================
Hits 75750 75750
Misses 18537 18537
Partials 5792 5792 ☔ 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@55aedd44aaab46a28c1f19f10ce949c27dcff0aa🧩 Skill updatenpx skills add larksuite/cli#ci/macos-signing-notarization -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: 4
🧹 Nitpick comments (4)
scripts/release-candidate.js (1)
427-442: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
compareReleaseVersionssilently assumes valid, same-channel inputs.Callers validate today, but a
nullmatch or a stable/beta mix would fail as a rawTypeError(BigInt(undefined)) rather than the script's structured failure. A guard would keep future callers honest.♻️ Optional hardening
function compareReleaseVersions(left, right) { - const leftMatch = RELEASE_VERSION_PATTERN.exec(left); - const rightMatch = RELEASE_VERSION_PATTERN.exec(right); + const leftMatch = RELEASE_VERSION_PATTERN.exec(left); + const rightMatch = RELEASE_VERSION_PATTERN.exec(right); + if (!leftMatch || !rightMatch) { + fail("release versions must be comparable Stable or Beta versions"); + } + if ((leftMatch[4] === undefined) !== (rightMatch[4] === undefined)) { + fail("release versions must belong to the same channel to be compared"); + }🤖 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-candidate.js` around lines 427 - 442, Harden compareReleaseVersions by validating both RELEASE_VERSION_PATTERN matches before accessing capture groups, and reject stable/beta channel mismatches with the script’s established structured failure mechanism rather than allowing TypeError or BigInt(undefined). Preserve the existing numeric comparison behavior for valid versions from the same channel..github/workflows/release.yml (3)
31-33: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winInconsistent
setup-nodecaching across release jobs. These threesetup-nodesteps omitpackage-manager-cache: false, which the build/publish jobs set explicitly; zizmor flags exactly these lines for cache poisoning of runtime artifacts. Since none of these jobs installs dependencies, disabling the cache is free and keeps the release path uniform.
.github/workflows/release.yml#L31-L33: addpackage-manager-cache: falseto the preflightsetup-node..github/workflows/release.yml#L333-L335: addpackage-manager-cache: falseto thecreate-draft-releasesetup-node..github/workflows/release.yml#L1138-L1140: addpackage-manager-cache: falseto theverify-macossetup-node.🤖 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 31 - 33, Add package-manager-cache: false to the setup-node steps in the preflight job (.github/workflows/release.yml#L31-L33), create-draft-release job (.github/workflows/release.yml#L333-L335), and verify-macos job (.github/workflows/release.yml#L1138-L1140), matching the existing configuration in the build and publish jobs.Source: Linters/SAST tools
985-992: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
verifyReleaseinvocation and brittle error-string branching.Lines 985-988 and 989-992 are identical, so the full release download/verify runs twice for no added guarantee, and the draft/published fallback keys off the substring
state, or type changedfromassertRelease— any wording change silently turns the fallback into a hard failure. Return an explicit draft state instead of parsing messages, and call it once.🤖 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 985 - 992, Refactor the release verification flow around verifyRelease so it is invoked once, removing the duplicated beforePublic assignment. Replace the error-message substring check with an explicit draft-state result from the release verification/assertRelease path, and use that state to select the published or draft fallback without relying on error wording.
874-898: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExtract the repeated provenance/manifest validation into a script.
This block, the
create-draft-releaseequivalent (Lines 373-456), and thepublish-npmblock (Lines 1086-1104) implement the same artifact-lookup, digest, tag-chain, and manifest checks with divergent formatting and slightly different error messages. Moving it intoscripts/(e.g. arelease-candidate.jssubcommand) would make the gates testable and keep the three jobs in sync.🤖 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 874 - 898, Extract the shared artifact provenance, digest, tag-chain, and candidate-manifest validation from this inline Node block, the create-draft-release equivalent, and publish-npm into a reusable scripts/release-candidate.js entry point or subcommand. Update all three workflow jobs to invoke that shared implementation with their existing inputs and preserve the current validation gates and failure behavior, so the checks and error handling remain synchronized and testable.
🤖 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 663-687: The release validation flow must use the commit resolved
by resolveRemoteTag, including its annotated-tag chain, as the expected source
instead of sourceSha when checking or creating an existing release. Update the
assertRelease calls and related release payload in this branch so reruns for
existing tags validate against the resolved tag commit while preserving the
draft and alreadyPublished behavior.
- Around line 1122-1127: Update the verify-macos job permissions to grant
contents: write access, replacing its current contents: read permission so
GITHUB_TOKEN can fetch the draft release and macOS asset through the release
verification flow.
- Around line 1046-1051: Update the publish-npm job after the setup-node step
and before the publish command to globally install the pinned npm@11.16.0,
matching the existing installation used by build-sign-notarize. Ensure npm
publish --provenance runs with that installed version.
In `@scripts/release-workflow.test.sh`:
- Around line 19-24: Update the require-based assertions in the release workflow
test so security-critical checks validate the actual YAML fields and steps
rather than arbitrary text matches. Replace broad haystack grep usage with
field/step-aware extraction or YAML parsing, ensuring commented lines and
unrelated run-command content cannot satisfy the assertions.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 31-33: Add package-manager-cache: false to the setup-node steps in
the preflight job (.github/workflows/release.yml#L31-L33), create-draft-release
job (.github/workflows/release.yml#L333-L335), and verify-macos job
(.github/workflows/release.yml#L1138-L1140), matching the existing configuration
in the build and publish jobs.
- Around line 985-992: Refactor the release verification flow around
verifyRelease so it is invoked once, removing the duplicated beforePublic
assignment. Replace the error-message substring check with an explicit
draft-state result from the release verification/assertRelease path, and use
that state to select the published or draft fallback without relying on error
wording.
- Around line 874-898: Extract the shared artifact provenance, digest,
tag-chain, and candidate-manifest validation from this inline Node block, the
create-draft-release equivalent, and publish-npm into a reusable
scripts/release-candidate.js entry point or subcommand. Update all three
workflow jobs to invoke that shared implementation with their existing inputs
and preserve the current validation gates and failure behavior, so the checks
and error handling remain synchronized and testable.
In `@scripts/release-candidate.js`:
- Around line 427-442: Harden compareReleaseVersions by validating both
RELEASE_VERSION_PATTERN matches before accessing capture groups, and reject
stable/beta channel mismatches with the script’s established structured failure
mechanism rather than allowing TypeError or BigInt(undefined). Preserve the
existing numeric comparison behavior for valid versions from the same channel.
🪄 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 Plus
Run ID: af54810b-5ae0-43fa-9e3d-71b7f4ef99a6
📒 Files selected for processing (10)
.github/workflows/release.yml.goreleaser.ymlMakefilescripts/install.jsscripts/install.test.jsscripts/release-candidate.jsscripts/release-candidate.test.jsscripts/release-preflight.jsscripts/release-preflight.test.jsscripts/release-workflow.test.sh
github-actions
Bot
added
size/L
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
scripts/release-workflow.test.sh (2)
13-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid shadowing
Kernel#fail.Redefining
failat top level replaces theraisealias for the whole script, so any futurefailused with raise semantics would silentlyabortinstead. A distinct name keeps intent clear.♻️ Rename
-def fail(message) +def contract_error(message) abort("release workflow contract: #{message}") end🤖 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-workflow.test.sh` around lines 13 - 15, Rename the top-level helper method fail to a distinct name, such as report_failure, and update every call site in the release workflow test script to use the new name, preserving its current abort message behavior without shadowing Kernel#fail.
113-127: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAssert the notarize
enabledguard too.The contract pins signing/notarization inputs but not
enabled. Flipping it tofalsewould silently ship unsigned macOS builds with this test still green.🛡️ Add assertion
expect_equal(macos_notarize.fetch("ids"), ["lark-cli"], "notarized build IDs") +expect_equal( + macos_notarize.fetch("enabled"), + '{{ isEnvSet "MACOS_SIGN_P12" }}', + "macOS notarization enablement", +)🤖 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-workflow.test.sh` around lines 113 - 127, Add an assertion in the macOS notarization checks around macos_notarize to require its enabled setting to be true, alongside the existing IDs, signing, and notarization input assertions..github/workflows/release.yml (1)
246-250: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTwo brittle assertions in the notarization gate.
Line 246 pins the exact codesign flag word
flags=0x10000(runtime); any additional code-directory flag (e.g. library validation) changes the rendered value and fails a legitimately hardened binary. Line 250 uses a substring match, so1.2.3also matches a binary reporting1.2.30.🛡️ Suggested tightening
- grep -Fq 'flags=0x10000(runtime)' <<<"$details" + grep -Eq '^CodeDirectory .*flags=0x[0-9a-f]+\(([a-z-]+,)*runtime(,[a-z-]+)*\)' <<<"$details" grep -Eq '^Timestamp=.+' <<<"$details" spctl --assess --type execute --verbose=4 "$binary" 2>&1 | tee "$work/spctl.txt" grep -Fq 'source=Notarized Developer ID' "$work/spctl.txt" - "$binary" --version | grep -Fq "$VERSION" + "$binary" --version | grep -Eq "(^|[^0-9A-Za-z.])${VERSION//./\\.}([^0-9A-Za-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 @.github/workflows/release.yml around lines 246 - 250, Update the notarization checks around the codesign-details and binary-version assertions: validate that the runtime flag is present without requiring the exact complete flags value, and require the --version output to equal VERSION rather than merely contain it. Preserve the existing assessment and notarization checks.
🤖 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 185-201: Update the existing-draft reuse path around the gh
release view logic to reconcile the draft before upload: call gh release edit
for TAG with target SOURCE_SHA and prerelease set from PRERELEASE. Keep
published-release validation and new-release creation unchanged, and apply the
edit only when reusing a draft.
- Around line 321-335: Update the integrity gate around the local integrity
calculation and published lookup so it does not require byte-identical npm
tarballs across workflow reruns. Use the already-generated package/checksums.txt
and extracted package content as the stable comparison, or otherwise make npm
pack deterministic; retain a hard failure only when published content differs,
and downgrade metadata-only tarball differences to a warning.
In `@scripts/release-workflow.test.sh`:
- Around line 9-11: Ensure the environment running
scripts/release-workflow.test.sh has Ruby available before its YAML parsing
commands execute. Update the CI job that invokes make script-test, or explicitly
install Ruby within the test setup, while preserving the existing YAML
validation behavior.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 246-250: Update the notarization checks around the
codesign-details and binary-version assertions: validate that the runtime flag
is present without requiring the exact complete flags value, and require the
--version output to equal VERSION rather than merely contain it. Preserve the
existing assessment and notarization checks.
In `@scripts/release-workflow.test.sh`:
- Around line 13-15: Rename the top-level helper method fail to a distinct name,
such as report_failure, and update every call site in the release workflow test
script to use the new name, preserving its current abort message behavior
without shadowing Kernel#fail.
- Around line 113-127: Add an assertion in the macOS notarization checks around
macos_notarize to require its enabled setting to be true, alongside the existing
IDs, signing, and notarization input assertions.
🪄 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 Plus
Run ID: 82ab5c02-ca21-4718-8d52-6cbc23905f29
📒 Files selected for processing (2)
.github/workflows/release.ymlscripts/release-workflow.test.sh
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release.yml (1)
332-338: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDo not treat every
npm viewfailure as “version absent.”
npm viewreturning non-zero includes registry, authentication, or transient lookup failures, which still fall into the publish branch. Only proceed to publish after a clear missing-version response; otherwise fail closed.🤖 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 332 - 338, Update the version-existence check surrounding npm view so only an explicit not-found response enters the npm publish branch. Capture and inspect the command’s failure status/output, while treating registry, authentication, and transient errors as fatal and exiting without publishing; preserve the existing integrity and dist-tag validation for versions that exist.
🤖 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 51-52: Restore the main-ancestry validation in the release
workflow before stable publication, using the existing git merge-base check to
ensure the protected tag commit is contained in origin/main and fail otherwise.
Keep any beta or rehearsal bypass explicitly scoped to that release condition
rather than disabling the gate globally, and add coverage verifying both stable
rejection and permitted beta behavior.
---
Outside diff comments:
In @.github/workflows/release.yml:
- Around line 332-338: Update the version-existence check surrounding npm view
so only an explicit not-found response enters the npm publish branch. Capture
and inspect the command’s failure status/output, while treating registry,
authentication, and transient errors as fatal and exiting without publishing;
preserve the existing integrity and dist-tag validation for versions that exist.
🪄 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 Plus
Run ID: fd649bb8-c68e-4065-8561-ae6030dd89bb
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
.github/workflows/release.ymlpackage.json
Tantanz20020918
temporarily deployed
to
npm-production
July 29, 2026 03:30 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Tantanz20020918
had a problem deploying
to
npm-production
July 29, 2026 07:39 — with
GitHub Actions
Failure
GitHub Actions
Failure
Tantanz20020918
temporarily deployed
to
npm-production
July 31, 2026 03:03 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Tantanz20020918
temporarily deployed
to
npm-production
July 31, 2026 03:52 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
This reverts commit 9b1e76e.
liangshuo-1
force-pushed
the
ci/macos-signing-notarization
branch
from
August 3, 2026 16:58
fdfac79 to
55aedd4
Compare
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
Add a guarded macOS release-candidate pipeline that signs and notarizes the two macOS CLI binaries before publication. The release path gains beta-version support, immutable candidate validation, safe recovery after an interrupted release, and protection against npm channel rollback.
Changes
.github/workflows/release.ymlinto preflight, build/sign/notarize, Draft Release, macOS verification, GitHub publish, and npm publish jobs.goreleaser.yml, with short-lived secret files and cleanup in the release workflowscripts/release-preflight.jsandscripts/install.jsfor beta versions; add release workflow and npm-publish policy contract testsmain; allow beta tags from a separate branch and warn when a beta tag points to the currentmainHEADTest Plan
node --test scripts/release-preflight.test.js scripts/release-publish-policy.test.js(16/16),bash scripts/release-workflow.test.sh, andgit diff --check1.0.79-beta.5, and beta.0 is absent from npmRelated Issues
N/A
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores