-
Notifications
You must be signed in to change notification settings - Fork 41
chore: allow provisioning server to launch in sandboxed processes #221
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughA new macOS entitlements key Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| <key>com.apple.security.temporary-exception.mach-lookup.global-name</key> | ||
| <array> | ||
| <string>com.setapp.ProvisioningService</string> | ||
| </array> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
The entitlement syntax looks correct.
The plist format and key name follow Apple's conventions properly. The scope is appropriately limited to the specific Setapp provisioning service.
However, please verify the following:
- Functional requirement: Confirm through testing that this entitlement is actually required for Setapp integration to work correctly in sandboxed processes.
- Distribution impact: If you plan to distribute via the Mac App Store, temporary exception entitlements may require additional justification during review. Setapp distribution should be fine.
- Documentation: Consider adding a comment in a relevant configuration file or README explaining why this exception is needed, to help future maintainers understand the Setapp integration requirements.
Run the following to check if there's any documentation about Setapp integration:
🏁 Script executed:
#!/bin/bash
# Description: Search for existing Setapp-related documentation or configuration
# Search for Setapp references in documentation files
rg -i "setapp" --type=md -C 3
# Search for Setapp references in configuration files
fd -e json -e yaml -e yml -e toml | xargs rg -i "setapp" -C 2
# Check for any other entitlements documentation
fd -g "*entitlement*" -g "*README*" -g "*CHANGELOG*"Length of output: 6018
🏁 Script executed:
#!/bin/bash
# Get full context of the entitlements file and how it's used
echo "=== Full entitlements.mac.plist content ==="
cat assets/entitlements.mac.plist
echo -e "\n=== Check if entitlements file is referenced in build configs ==="
rg "entitlements" --type=json -C 2
echo -e "\n=== Search for entitlements references in all build configs ==="
fd -e json -path "*build*" -o -path "*electron*" | xargs rg -l "entitlements" 2>/dev/null || trueLength of output: 1641
🏁 Script executed:
#!/bin/bash
# Search for any Setapp-specific entitlements files and check build config paths
echo "=== Look for alternative entitlements files ==="
fd "entitlements" assets/
echo -e "\n=== Check if electron-builder-setapp.json overrides entitlements path ==="
cat electron-builder-setapp.json | grep -A 5 -B 5 "entitlements"
echo -e "\n=== Verify the exact build structure for Setapp ==="
cat electron-builder-setapp.json | head -50Length of output: 1766
Fix the entitlements file architecture—separate Setapp from general builds.
The Setapp-specific entitlement has been added to assets/entitlements.mac.plist, which is shared by both general builds and Setapp builds. This means the general/App Store build now includes the Setapp provisioning service entitlement, which is problematic:
- App Store review may reject it as an unnecessary or unexpected entitlement
- Non-Setapp users shouldn't have Setapp service lookup capabilities
- Security boundary violation: mixing Setapp-specific permissions with general release build
Required fix:
- Create
assets/entitlements-setapp.mac.plistcontaining the Setapp-specific entitlements (including the newcom.setapp.ProvisioningServiceentry) - Move the new entitlement to this Setapp-specific file
- Update
electron-builder-setapp.jsonto reference"entitlements": "assets/entitlements-setapp.mac.plist" - Restore
assets/entitlements.mac.plistto only contain the general entitlements (com.apple.security.cs.allow-unsigned-executable-memoryandcom.apple.security.cs.allow-jit)
🤖 Prompt for AI Agents
In assets/entitlements.mac.plist around lines 9 to 12, the Setapp-specific key
com.setapp.ProvisioningService was added to the shared entitlements file; to
fix, create a new file assets/entitlements-setapp.mac.plist and move the entire
com.setapp.ProvisioningService array entry into that file, update
electron-builder-setapp.json to set "entitlements":
"assets/entitlements-setapp.mac.plist", and restore
assets/entitlements.mac.plist so it only contains the general entitlements
(com.apple.security.cs.allow-unsigned-executable-memory and
com.apple.security.cs.allow-jit).
Summary by CodeRabbit