Repository navigation
Fix codesign: path filter excluded all files inside outer .app - #2680
Conversation
The find filter `-not -path '*.app/*'` matched the OUTER app bundle (cmux NIGHTLY.app/) in every file's full path, so Pass 2 found zero files and Autoupdate was never signed. Fix: use `-prune` on nested .app directories instead. This skips Updater.app's contents (already signed in Pass 1) while still finding standalone executables like Autoupdate. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughTwo GitHub Actions workflow files are updated to modify macOS app codesigning Pass 2 file enumeration. The change replaces Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
✨ 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 |
Greptile SummaryThis PR fixes a broken Confidence Score: 5/5Safe to merge — the fix is correct and the only findings are P2 style suggestions The root-cause analysis is accurate, the -prune expression is verified correct on BSD find (macOS), the three-pass signing order is sound, and --verify --deep --strict provides a safety net. Both P2 comments are non-blocking style suggestions that don't represent regressions. No files require special attention; both nightly.yml and release.yml apply an identical, correct fix Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["for DIR in PlugIns / Frameworks"] --> B
B["Pass 1: Sign nested .app & .xpc bundles\nfind -depth -type d '*.app' or '*.xpc'\n(deepest-first, e.g. Updater.app)"] --> C
C["Pass 2: Sign standalone Mach-O executables\nfind with -prune on .app dirs\n(e.g. Autoupdate — previously skipped!)"] --> D
D["Pass 3: Sign .framework / .plugin / .appex\nfind -depth -type d\n(deepest-first, e.g. Sparkle.framework)"] --> E
E{More DIRs?} -- Yes --> A
E -- No --> F[Sign outer .app bundle]
F --> G["codesign --verify --deep --strict\n(safety net catches any ordering error)"]
G --> H[assert-passkey-entitlement.sh]
Reviews (1): Last reviewed commit: "Fix codesign: -path '*.app/*' excluded e..." | Re-trigger Greptile |
| /usr/bin/codesign --force --options runtime --timestamp --sign "$APPLE_SIGNING_IDENTITY" --entitlements "$EMBEDDED_ENTITLEMENTS" "$f" | ||
| fi | ||
| done < <(find "$DIR" -type f -not -path '*.app/*' -print0) | ||
| done < <(find "$DIR" \( -type d -name '*.app' -prune \) -o \( -type f -print0 \)) |
There was a problem hiding this comment.
.xpc contents not pruned in Pass 2
Pass 1 signs .xpc bundles as whole bundles (line 419), but Pass 2 only prunes .app directories. If any .xpc bundles sit directly inside Frameworks (not nested within Updater.app), Pass 2 would re-sign individual files inside them after Pass 1 already committed a bundle signature — invalidating it. This isn't a regression from the original code, and the --verify --deep --strict check on line 435 would catch any resulting invalidity. For correctness, consider also pruning .xpc dirs:
| done < <(find "$DIR" \( -type d -name '*.app' -prune \) -o \( -type f -print0 \)) | |
| done < <(find "$DIR" \( -type d \( -name '*.app' -o -name '*.xpc' \) -prune \) -o \( -type f -print0 \)) |
| /usr/bin/codesign --force --options runtime --timestamp --sign "$APPLE_SIGNING_IDENTITY" --entitlements "$EMBEDDED_ENTITLEMENTS" "$f" | ||
| fi | ||
| done < <(find "$DIR" -type f -not -path '*.app/*' -print0) | ||
| done < <(find "$DIR" \( -type d -name '*.app' -prune \) -o \( -type f -print0 \)) |
There was a problem hiding this comment.
Same
.xpc pruning gap as in nightly.yml
Same observation as nightly.yml line 426: Pass 2 prunes only .app directories, leaving .xpc bundle contents reachable. Consider adding .xpc to the prune list:
| done < <(find "$DIR" \( -type d -name '*.app' -prune \) -o \( -type f -print0 \)) | |
| done < <(find "$DIR" \( -type d \( -name '*.app' -o -name '*.xpc' \) -prune \) -o \( -type f -print0 \)) |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is kicking off a free cloud agent to fix this issue. This run is complimentary, but you can enable autofix for all future PRs in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e2d5cff. Configure here.
| /usr/bin/codesign --force --options runtime --timestamp --sign "$APPLE_SIGNING_IDENTITY" --entitlements "$EMBEDDED_ENTITLEMENTS" "$f" | ||
| fi | ||
| done < <(find "$DIR" -type f -not -path '*.app/*' -print0) | ||
| done < <(find "$DIR" \( -type d -name '*.app' -prune \) -o \( -type f -print0 \)) |
There was a problem hiding this comment.
Pass 2 prune misses .xpc bundles signed in Pass 1
High Severity
Pass 1 signs both *.app and *.xpc bundles, but the new find in Pass 2 only prunes *.app directories. Since Sparkle 2.x bundles Installer.xpc and Downloader.xpc inside the framework (each containing a Mach-O executable), Pass 2 will descend into those .xpc bundles and re-sign their internal executables individually. This invalidates the .xpc bundle signatures created by Pass 1, and Pass 3 (which signs *.framework) does not re-sign nested .xpc bundles — causing codesign --verify --deep --strict to fail. The -prune clause needs to include -name '*.xpc' to match what Pass 1 handles.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit e2d5cff. Configure here.


Summary
-not -path '*.app/*'in Pass 2's find matched the outercmux NIGHTLY.app/in every path, excluding ALL files — Autoupdate was never reachedfind -pruneon nested.appdirs, which correctly skips onlyUpdater.app/contents while finding standalone executables likeAutoupdateRoot cause
find "$DIR" -type f -not -path '*.app/*'where$DIRiscmux NIGHTLY.app/Contents/Frameworks— every file's full path containscmux NIGHTLY.app/, so the glob*.app/*matches everything and Pass 2 signs nothing.Fix
-pruneon*.appdirectories prevents descending into nested app bundles (likeUpdater.app, already signed in Pass 1) without filtering by full path.🤖 Generated with Claude Code
Note
Medium Risk
Touches the macOS release/nightly signing pipeline; a mistake could cause unsigned binaries or failed notarization, but the change is a small, targeted
findfilter correction.Overview
Fixes Pass 2 of the macOS codesigning loops in
nightly.ymlandrelease.ymlso standalone Mach-O files insideContents/Frameworks/Contents/PlugInsare actually discovered and signed.Replaces the previous
find ... -not -path '*.app/*'filter (which could exclude everything under an outer.app) with afind-pruneon nested*.appdirectories, skipping only embedded app bundles while still signing executables like Sparkle’sAutoupdate.Reviewed by Cursor Bugbot for commit e2d5cff. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fix codesign filter in GitHub workflows so Pass 2 signs standalone executables (e.g., Autoupdate) instead of skipping everything inside the outer .app. This restores proper signing for nightly and release builds.
-not -path '*.app/*'withfind ... -pruneto skip only nested.appbundles (e.g.,Updater.app).Contents/Frameworksare found and signed in Pass 2..github/workflows/nightly.ymland.github/workflows/release.yml.Written for commit e2d5cff. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
Note: These are internal build system improvements with no user-facing changes.