ci: sign and notarize releases in a final job on GitHub's runner#26

merged3
main
ci/release-signing
opened2d agobyKKainoa Newton
View on GitHubTwo-way sync enabledPR last imported 11h ago

Problem.

  • Every compile phase received the Developer ID certificate and the notarization password, and the release job, which holds the Sparkle private key, ran on the compile runner. Moving the compile to Depot (D4) would have moved the keys.
  • The DMG was signed but never notarized or stapled.
  • Release builds contain rac's dev switches, and nothing checks for them (gate G6).
  • generate_sparkle_deltas.py assumes every earlier release has both architectures, so the release after an arm64-only 0.1.0 would have crashed.

Change.

  • Compile phases only compile. They receive the Sparkle public key and a plain true/false for branded entitlements; the true/false is computed in a small settings job. They receive no certificate, notarization password or provisioning profile. The finishing phase uploads the unsigned app as tar+zstd with a sha256 file.
  • New per-architecture sign job on GitHub-hosted macos-latest:
    • It downloads and checks the app with retries, then imports the certificate into a temporary keychain (set-key-partition-list, so no dialog appears). The keychain is deleted in an always() step.
    • It runs the G6 gate, then signs, notarizes and staples the app.
    • It signs the DMG with a timestamp, then notarizes and staples it, retrying on Apple's transient staple errors (exit codes 65 and 68) and on upload failures.
    • It mounts the DMG and verifies it: codesign --verify --deep --strict; spctl with assessments enabled and source=Notarized Developer ID for both the app and the DMG (the DMG with -t open --context context:primary-signature); stapler validate; the signing team; and no profile-only entitlements without a profile.
    • It makes the deltas and uploads the same DMG artifact the release job already expects.
  • Release job. It is pinned to macos-latest because it holds the Sparkle key. It re-verifies each notarized DMG before signing it into the appcast.
  • G6 gate. github_check_release_binary.sh runs strings over the app's executables and frameworks and looks for rac-agent-debug, rac-agent-mcp-echo, rac-window-screenshot and rac-launch-in-background. The new Developer switches (G6) input defaults to warn. Switch it to enforce once Core's BUILDFLAG(RAC_DEV_SWITCHES) change merges.
  • Deltas. Architectures an earlier release lacks are skipped, and the script prints "made N deltas".
  • Secrets stay out of logs. Tracing is off wherever a secret is used, and the sign log is no longer written to disk.
  • Docs. docs/operations/releases.md and upstream-sync.md are updated.

Preflight findings.

  • The empty MAC_TEAM_ID (patches/rac/branding/identity.patch) doesn't affect signing or notarization, which use the certificate name and PROD_MACOS_NOTARIZATION_TEAM_ID. It only matters if branded entitlements are turned on: the app would then claim .me.kainoa.rac and fail to launch. github_verify_dmg.sh now fails the build in that case.
  • Today's entitlements (hardened-runtime keys, and allow-jit in the helpers) are allowed for a Developer ID app without a profile. The restricted set (application-identifier, keychain-access-groups, web-browser public-key credential) is off.

Verified.

  • 14 unit tests for the G6 gate on fake Mach-O binaries, including a locale bug that a real universal binary exposed.
  • shellcheck is clean on the changed scripts. actionlint reports only the existing description error in prepare-environment.
  • A local ad hoc dry run with a fake app covered packing, the download check, unpacking (framework symlinks survive), the gate, sign_and_package_app.sh, the DMG, the hashes and the outputs.
  • The verify script passed a good ad hoc image and correctly failed on a restricted entitlement without a profile, a mismatched team, an ad hoc image that was expected to be notarized, and an unmountable image.
  • The retry logic was tested with a stub xcrun.
  • An Opus reviewer that hadn't seen the design tried to break it and found no secret reaching a compile job. Its six findings are fixed in the second commit.
  • Not proven: real certificate import, notarization, stapling and Gatekeeper on a runner. The first proof is build 1 (0.1.0), which fires when the sync merges.

Patch surface (D17): no change, because this PR touches no patches.

Part of plan 06 M0 task 5 and M1 (the ci stream).

🤖 Generated with Claude Code (Claude Opus 5.5 and Claude Sonnet 5.5)


Note

High Risk Restructures where Apple signing/notarization and Sparkle release secrets run, and changes what ships if verification or the new sign job fails—core release security infrastructure.

Overview Splits compile from signing so build phases never receive the Developer ID certificate, notarization credentials, or provisioning profile. Finished phases upload an unsigned rac.app (tar+zstd + checksum) instead of a DMG; a new per-arch sign job on macos-latest runs G6, signs/notarizes/staples the app and DMG, verifies the mounted image, and publishes the same release artifacts as before.

Secret exposure is reduced: a settings job exposes only a boolean for branded entitlements to compile jobs; the release job is pinned to GitHub's runner alongside sign. sign_and_package_app.sh now notarizes and staples the DMG (with retries), and generate_sparkle_deltas.py skips architectures missing from older releases.

G6 gate: new github_check_release_binary.sh scans Mach-O binaries for developer switch strings, controlled by a Developer switches (G6) workflow input (warn / enforce). The release job re-runs github_verify_dmg.sh on downloaded DMGs before appcast signing. Unit tests cover the gate and verify script behavior; release docs and upstream-sync notes describe the new flow.

Reviewed by Cursor Bugbot for commit 9f9e5a672251359b3c45f566ffc70ca2780917a9. Bugbot is set up for automated code reviews on this repo. Configure here.


Summary by cubic

Moves code signing, notarization, and release packaging out of the build phases and into a final sign job that always runs on GitHub's own macos-latest runner, so signing keys never reach build runners. Notarizes and staples the DMG, which was previously signed but never notarized.

  • Build phases now only compile and receive just the Sparkle public key plus a true/false flag for branded entitlements computed by a small settings job; the finished phase uploads the unsigned app as a tar.zst with a sha256.
  • The sign job imports the certificate into a temporary keychain (deleted in an always() step), runs a Gatekeeper check on the mounted DMG, signs, notarizes, and staples both app and DMG with retries on Apple's transient errors, and makes the update deltas.
  • Tracing is disabled wherever a secret is read or decoded, so signing and notarization credentials never appear in logs.
  • The release job, which holds the Sparkle private key, is pinned to macos-latest and re-verifies each notarized DMG before writing it into the appcast.
  • The team ID and certificate name are stripped of stray whitespace before comparison, and github_verify_dmg.sh fails if codesign can't read the entitlements instead of treating it as none.

Written for commit 9f9e5a672251359b3c45f566ffc70ca2780917a9. Summary will update on new commits.

View guided diff Turn on auto-fix

Checks

2 failing · 7 successful · 2 neutral · 1 skipped
Some checks failed

Failing2

  • pullfrogGitHub
    Failed
  • VercelGitHub
    Failed
9f9e5a6
Synced from GitHub

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

External image Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 2469baf4-964e-40f6-bd4d-46966c2f4382

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Synced from GitHub

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: 09b66ebd-487a-410f-885e-e1e87b20e174)

Synced from GitHub

RetriggerView in GreptileConfidence Score: 4/5

[High impact] The PR changes the path used to produce and publish every macOS release.

The PR appears safe to merge, with two non-blocking improvements to release checks.

Findings

  1. P2 Team ID cleanup differs ▶
  2. P2 Failed entitlement reads pass ▶
Fix with agent prompt
markdown
1### Issue 1
2.github/workflows/build.yml:126
3If `PROD_MACOS_NOTARIZATION_TEAM_ID` contains a trailing newline, signing succeeds because `github_sign_app.sh` removes whitespace. The release job passes the original secret to `RAC_EXPECT_TEAM_ID`, so its exact team comparison then fails. Apply the same cleanup before the release check.
4
5### Issue 2
6.github/scripts/github_verify_dmg.sh:51
7`github_verify_dmg.sh` ignores errors from the command that reads entitlements, then treats empty output as an empty entitlement list. If the command fails without writing output, the profile and application-ID checks silently do nothing. Fail when the command cannot read entitlements instead of treating that failure as a successful empty result.
8
9```suggestion
10if ! codesign -d --entitlements - --xml "$_app" > "$_mnt.entitlements.plist"; then
11 echo "::error::Could not read the app entitlements"
12 exit 1
13fi
14```
15
16---
17
18For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Moves signing and notarization out of compile phases and onto GitHub-hosted runners.

  • MacOS releases are signed, notarized, and checked on GitHub runners.
  • Release builds can check for rac's developer switches before signing.
  • Delta generation skips older releases that lack the selected architecture.

Acknowledgment: notkainoa explicitly deferred enforcing G6 until the Core build flag lands. The default warning mode is intentional. notkainoa also stated that real certificate import and notarization remain unproven; this review does not claim runtime verification.

Diagram

mermaid
1%%{init: {'theme': 'neutral'}}%%
2flowchart LR
3 Settings[Optional profile setting] --> Compile[Compile phases]
4 Compile --> Archive[Unsigned app and checksum]
5 Archive --> Gate[Check developer switches]
6 Gate --> Sign[Sign and notarize app]
7 Sign --> DMG[Package and notarize disk image]
8 DMG --> Verify[Verify image and make deltas]
9 Verify --> Release[Recheck images and sign update feeds]
10 Release --> Publish[Publish release]

Reviews (1) · Last reviewed commit: "ci: harden the signing job after review" · Reviewed by Greptile

.github/workflows/build.ymlline 126Outdated · 8515176Open thread
……@@ -85,6 +96,34 @@ jobs:
123+ env:
124+ # Only whether the certificate exists, and the (public) team ID.
125+ RAC_NOTARIZED: ${{ secrets.PROD_MACOS_CERTIFICATE != '' && '1' || '0' }}
126+ RAC_EXPECT_TEAM_ID: ${{ secrets.PROD_MACOS_NOTARIZATION_TEAM_ID }}
Ggreptile-appsBoton line 1262d ago

P2 Team ID cleanup differs

If PROD_MACOS_NOTARIZATION_TEAM_ID contains a trailing newline, signing succeeds because github_sign_app.sh removes whitespace. The release job passes the original secret to RAC_EXPECT_TEAM_ID, so its exact team comparison then fails. Apply the same cleanup before the release check.

Prompt To Fix With AI
markdown
1This is a comment left during a code review.
2Path: .github/workflows/build.yml
3Line: 126
4
5Comment:
6**Team ID cleanup differs**
7
8If `PROD_MACOS_NOTARIZATION_TEAM_ID` contains a trailing newline, signing succeeds because `github_sign_app.sh` removes whitespace. The release job passes the original secret to `RAC_EXPECT_TEAM_ID`, so its exact team comparison then fails. Apply the same cleanup before the release check.
9
10---
11
12For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
.github/scripts/github_verify_dmg.shline 51Outdated · 8515176Open thread
……@@ -0,0 +1,131 @@
48+# An app that claims a restricted entitlement without a provisioning profile
49+# that grants it is killed at launch, and notarization doesn't catch that.
50+echo "== entitlements (main executable)"
51+codesign -d --entitlements - --xml "$_app" 2>/dev/null > "$_mnt.entitlements.plist" || true
Ggreptile-appsBoton line 512d ago

P2 Failed entitlement reads pass

github_verify_dmg.sh ignores errors from the command that reads entitlements, then treats empty output as an empty entitlement list. If the command fails without writing output, the profile and application-ID checks silently do nothing. Fail when the command cannot read entitlements instead of treating that failure as a successful empty result.

suggestion
1if ! codesign -d --entitlements - --xml "$_app" > "$_mnt.entitlements.plist"; then
2 echo "::error::Could not read the app entitlements"
3 exit 1
4fi
Prompt To Fix With AI
markdown
1This is a comment left during a code review.
2Path: .github/scripts/github_verify_dmg.sh
3Line: 51
4
5Comment:
6**Failed entitlement reads pass**
7
8`github_verify_dmg.sh` ignores errors from the command that reads entitlements, then treats empty output as an empty entitlement list. If the command fails without writing output, the profile and application-ID checks silently do nothing. Fail when the command cannot read entitlements instead of treating that failure as a successful empty result.
9
10```suggestion
11if ! codesign -d --entitlements - --xml "$_app" > "$_mnt.entitlements.plist"; then
12 echo "::error::Could not read the app entitlements"
13 exit 1
14fi
15```
16
17---
18
19For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Added 2 commits

  • ci: harden the signing job after review272e4b3
  • ci: sign and notarize releases in a final job on GitHub's runnere82a0b5
Synced from GitHub

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: 23ea93f8-f4ae-4488-b2b4-86800d07d107)

Added 1 commit

  • ci: fix review findings in the signing job9f9e5a6
Synced from GitHub

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: 3b6246cb-3999-43bf-8d89-879c704a8449)