Repository navigation
Sign nested plugins and frameworks explicitly (post-PR #2905 fix-forward) - #2906
lawrencecchen wants to merge 1 commit into
Conversation
PR #2905 removed --deep from the top-level sign to avoid clobbering the per-helper entitlements, but --deep was also what ensured nested plugins (PlugIns/CmuxDockTilePlugin.plugin) and the Sparkle / Sentry frameworks got signed. Without --deep the plugin is left unsigned and 'codesign --entitlements ... <app>' fails with 'code object is not signed at all' in subcomponent. Add explicit steps to sign each plugin and each framework with --deep (no custom entitlements) before signing the main app bundle. This matches Apple's documented inside-out signing flow: every nested code item is signed first, outer containers last.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughGitHub Actions workflows for nightly and release builds updated to implement "inside-out" macOS bundle codesigning. Nested components—CLI helpers, PlugIns, and Frameworks—are signed before the main app, with varying entitlements policies applied at each stage. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
310-330: Keep the helper-entitlement invariant checked in release CI.This step now signs the helpers correctly, but release still doesn't assert afterward that
cmuxandghosttydid not pick upapplication-identifier. That invariant is part of this PR's test plan, and nightly already guards it. Adding the same check here would fail fast before publishing a tagged build.Suggested guard
/usr/bin/codesign --force --options runtime --timestamp --sign "$APPLE_SIGNING_IDENTITY" --entitlements "$RELEASE_APP_ENT" "$APP_PATH" /usr/bin/codesign --verify --deep --strict --verbose=2 "$APP_PATH" /usr/bin/codesign -d --entitlements :- "$APP_PATH" 2>&1 | grep -q "com.apple.developer.web-browser.public-key-credential" /usr/bin/codesign -d --entitlements :- "$APP_PATH" 2>&1 | grep -q "7WLXT3NR37.com.cmuxterm.app" + for helper in "$CLI_PATH" "$HELPER_PATH"; do + if [ -f "$helper" ]; then + /usr/bin/codesign -d --entitlements :- "$helper" 2>&1 | grep -q "application-identifier" && { + echo "error: helper unexpectedly carries application-identifier: $helper" >&2 + exit 1 + } || true + fi + done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 310 - 330, Add a post-signing guard that asserts the helper binaries (the ones signed via CLI_PATH and HELPER_PATH, e.g., cmux and ghostty) do NOT contain the application-identifier entitlement: after the helper signing block (the section that signs CLI_PATH and HELPER_PATH) run codesign --display --entitlements - on each helper (reference CLI_PATH and HELPER_PATH or the actual helper filenames cmux and ghostty) and fail the script (exit 1) if the output contains "application-identifier"; this ensures the helper-entitlement invariant is enforced in release CI before packaging/publishing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 310-330: Add a post-signing guard that asserts the helper binaries
(the ones signed via CLI_PATH and HELPER_PATH, e.g., cmux and ghostty) do NOT
contain the application-identifier entitlement: after the helper signing block
(the section that signs CLI_PATH and HELPER_PATH) run codesign --display
--entitlements - on each helper (reference CLI_PATH and HELPER_PATH or the
actual helper filenames cmux and ghostty) and fail the script (exit 1) if the
output contains "application-identifier"; this ensures the helper-entitlement
invariant is enforced in release CI before packaging/publishing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 73a809cf-f4ad-4d95-ab32-70f482523da5
📒 Files selected for processing (2)
.github/workflows/nightly.yml.github/workflows/release.yml
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes a codesign failure introduced by PR #2905: removing Confidence Score: 5/5Safe to merge — the signing order and logic are correct; remaining findings are minor parity/style suggestions. The fix correctly restores signing for nested PlugIns and Frameworks using proper inside-out order. The find -mindepth 1 -maxdepth 1 -print0 + null-byte read loop handles spaces in paths correctly. Using --deep per-bundle is the right targeted approach to cover Sparkle's nested XPC services without reintroducing the helper entitlement overwrite. All remaining comments are P2 style/parity suggestions that do not block merge. No files require special attention beyond the P2 suggestions already noted. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Build app bundle\nCODE_SIGNING_ALLOWED=NO] --> B
subgraph "Inside-out signing order"
B["1. Sign CLI helpers\n(cmux + ghostty)\n--entitlements cmux-helper.entitlements"]
B --> C["2. Sign each PlugIns/* bundle\n--deep, no entitlements\n(e.g. CmuxDockTilePlugin.plugin)"]
C --> D["3. Sign each Frameworks/* bundle\n--deep, no entitlements\n(handles Sparkle XPC services,\nUpdater.app recursively)"]
D --> E["4. Sign main app bundle LAST\n--entitlements app.entitlements\nNO --deep"]
end
E --> F[codesign --verify --deep --strict]
F --> G{Assertions pass?}
G -->|application-identifier present in main app\nWebAuthn entitlement present\nhelpers do NOT carry app-identifier| H[Notarize]
G -->|Fail| X[❌ Step fails]
|
…itlements Two changes consolidate the inside-out signing work introduced by PRs #2902, #2905, and #2906 into something a future reader can understand without reading two 40-line YAML blocks: - Check in cmux.release.entitlements and cmux.nightly.entitlements, each with the right application-identifier and team-identifier baked in. Replaces the PlistBuddy-at-sign-time injection that copies cmux.entitlements and mutates it per workflow run. - Extract the five-step inside-out signing logic (helpers, plugins, frameworks, main bundle, verification) into scripts/sign-cmux-bundle.sh. Both nightly.yml and release.yml shrink to one line that calls the script with the right entitlements file. No behavior change versus PR #2906 at steady state: same order, same --deep boundaries, same grep-based post-sign asserts. The script also refuses to sign if a helper ends up with the main app's application-identifier, so future regressions surface at build time rather than on launch under amfi.
|
Superseded by #2908 which took the shared-script + checked-in-entitlements approach. Both PRs produced identical signed artifacts in CI; 2908 is significantly cleaner (scripts/sign-cmux-bundle.sh + cmux.{release,nightly}.entitlements). |
…itlements (#2908) Two changes consolidate the inside-out signing work introduced by PRs #2902, #2905, and #2906 into something a future reader can understand without reading two 40-line YAML blocks: - Check in cmux.release.entitlements and cmux.nightly.entitlements, each with the right application-identifier and team-identifier baked in. Replaces the PlistBuddy-at-sign-time injection that copies cmux.entitlements and mutates it per workflow run. - Extract the five-step inside-out signing logic (helpers, plugins, frameworks, main bundle, verification) into scripts/sign-cmux-bundle.sh. Both nightly.yml and release.yml shrink to one line that calls the script with the right entitlements file. No behavior change versus PR #2906 at steady state: same order, same --deep boundaries, same grep-based post-sign asserts. The script also refuses to sign if a helper ends up with the main app's application-identifier, so future regressions surface at build time rather than on launch under amfi. Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
…itlements (manaflow-ai#2908) Two changes consolidate the inside-out signing work introduced by PRs manaflow-ai#2902, manaflow-ai#2905, and manaflow-ai#2906 into something a future reader can understand without reading two 40-line YAML blocks: - Check in cmux.release.entitlements and cmux.nightly.entitlements, each with the right application-identifier and team-identifier baked in. Replaces the PlistBuddy-at-sign-time injection that copies cmux.entitlements and mutates it per workflow run. - Extract the five-step inside-out signing logic (helpers, plugins, frameworks, main bundle, verification) into scripts/sign-cmux-bundle.sh. Both nightly.yml and release.yml shrink to one line that calls the script with the right entitlements file. No behavior change versus PR manaflow-ai#2906 at steady state: same order, same --deep boundaries, same grep-based post-sign asserts. The script also refuses to sign if a helper ends up with the main app's application-identifier, so future regressions surface at build time rather than on launch under amfi. Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
This branch was successfully deployed
1 participant
Summary
Nightly run after #2905 failed at the Codesign step with:
PR 2905 dropped
--deepfrom the top-level sign so the main app's entitlements wouldn't overwrite the CLI helpers. But--deepwas also the only thing signing the nested plugin and the Sparkle / Sentry frameworks, so they're left unsigned and the top-level sign refuses.Proper inside-out order per Apple's docs: sign every nested code item first (deepest out), outer containers last.
Changes (to both
nightly.ymlandrelease.yml):cmux-helper.entitlements(unchanged).Contents/PlugIns/*and sign each with--deep(no custom entitlements).Contents/Frameworks/*and sign each with--deep(no custom entitlements). Handles Sparkle's nestedXPCServices/*.xpcandUpdater.apptoo.--deep(unchanged).Test plan
cmux NIGHTLY.applaunches on macOS 26 Tahoe.codesign -d --entitlementson the published artifact still shows main app withapplication-identifier+web-browser.public-key-credential, helpers withoutapplication-identifier.Summary by CodeRabbit
Summary by cubic
Fix macOS codesigning by explicitly signing nested PlugIns and Frameworks before the app bundle, following Apple’s inside-out order. Nightly and release workflows now sign subcomponents so notarization and launch succeed after removing
--deepfrom the top-level sign.Contents/PlugIns/*andContents/Frameworks/*with--deepand no entitlements; includesSparkle’sXPCServicesandUpdater.app.cmux-helper.entitlements.--deep.Written for commit 1b3b2a1. Summary will update on new commits.