-
Notifications
You must be signed in to change notification settings - Fork 905
macOS App Sandbox #9023
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
macOS App Sandbox #9023
Conversation
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.
Pull Request Overview
This PR migrates from using team-identifier-prefixed app groups to the standard group. prefix for macOS app group identifiers, and updates code signing configuration to use automatic signing with a specific development team.
- Changed app group identifier format from
$(TEAM_ID)$(BUNDLE_ID)togroup.$(BUNDLE_ID) - Updated socket paths to use
Library/Application Support/subdirectories - Switched from manual to automatic code signing with
NKUJUXUJ3Bdevelopment team - Updated bundle identifiers from
com.owncloud.*tocom.nextcloud.*
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/gui/socketapi/socketapi_mac.mm | Updated socket path to use group prefix and Application Support directory |
| src/gui/macOS/fileproviderutils_mac.mm | Added whitespace formatting improvements |
| src/gui/macOS/fileprovidersocketserver_mac.mm | Updated file provider socket path to use group prefix and Application Support directory |
| shell_integration/MacOSX/NextcloudIntegration/NextcloudIntegration.xcodeproj/project.pbxproj | Changed code signing from manual to automatic, updated development team ID, and changed bundle IDs from owncloud to nextcloud |
| shell_integration/MacOSX/NextcloudIntegration/FinderSyncExt/FinderSyncExt.entitlements | Updated app group identifier to use group prefix |
| shell_integration/MacOSX/NextcloudIntegration/FinderSyncExt/FinderSync.m | Updated socket path to use Application Support subdirectory |
| shell_integration/MacOSX/NextcloudIntegration/FileProviderUIExt/FileProviderUIExt.entitlements | Updated app group identifier to use group prefix |
| shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Info.plist | Updated NCFPKAppGroupIdentifier to use group prefix |
| shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExt.entitlements | Updated app group identifier to use group prefix |
| cmake/modules/MacOSXBundleInfo.plist.in | Updated NCFPKAppGroupIdentifier template to use group prefix |
| admin/osx/macosx.entitlements.cmake | Updated app group identifier template to use group prefix and added sandbox entitlements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Info.plist
Outdated
Show resolved
Hide resolved
shell_integration/MacOSX/NextcloudIntegration/NextcloudIntegration.xcodeproj/project.pbxproj
Outdated
Show resolved
Hide resolved
38661b3 to
a7f4af5
Compare
636553d to
e37dada
Compare
|
I need to fix how the build settings are passed through now before it is ready for review. |
38055c2 to
a78b990
Compare
- These changes are related to making the main app bundle of the macOS build adopt the app sandbox entitlement which imposes certain restrictions on its file system access (nextcloud/desktop#9023). - Logs are now written to the app group container because they are not accessible by the main app for creating debug archives otherwise. - Databases are now written to the app group container because they are not accessible by the main app for creating debug archives otherwise. - Removed legacy database migration code because it cannot work with the new app group identifier of the now sandboxed app anymore. - Simplified database location assembly. Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
- These changes are related to making the main app bundle of the macOS build adopt the app sandbox entitlement which imposes certain restrictions on its file system access (nextcloud/desktop#9023). - Logs are now written to the app group container because they are not accessible by the main app for creating debug archives otherwise. - Databases are now written to the app group container because they are not accessible by the main app for creating debug archives otherwise. - Removed legacy database migration code because it cannot work with the new app group identifier of the now sandboxed app anymore. - Simplified database location assembly. Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
76982cb to
1534ff2
Compare
- These changes are related to making the main app bundle of the macOS build adopt the app sandbox entitlement which imposes certain restrictions on its file system access (nextcloud/desktop#9023). - Logs are now written to the app group container because they are not accessible by the main app for creating debug archives otherwise. - Databases are now written to the app group container because they are not accessible by the main app for creating debug archives otherwise. - Removed legacy database migration code because it cannot work with the new app group identifier of the now sandboxed app anymore. - Simplified database location assembly. Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
- These changes are related to making the main app bundle of the macOS build adopt the app sandbox entitlement which imposes certain restrictions on its file system access (nextcloud/desktop#9023). - Logs are now written to the app group container because they are not accessible by the main app for creating debug archives otherwise. - Databases are now written to the app group container because they are not accessible by the main app for creating debug archives otherwise. - Removed legacy database migration code because it cannot work with the new app group identifier of the now sandboxed app anymore. - Simplified database location assembly. Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
- These changes are related to making the main app bundle of the macOS build adopt the app sandbox entitlement which imposes certain restrictions on its file system access (nextcloud/desktop#9023). - Logs are now written to the app group container because they are not accessible by the main app for creating debug archives otherwise. - Databases are now written to the app group container because they are not accessible by the main app for creating debug archives otherwise. - Removed legacy database migration code because it cannot work with the new app group identifier of the now sandboxed app anymore. - Simplified database location assembly. Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
1534ff2 to
67ab6f5
Compare
67ab6f5 to
4581341
Compare
Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
…ion. Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
To have more predictable flow, the removal methods wait for completion of their dispatched and asynchronous methods first. Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
…upported macOS 11. We require at least macOS 12 anyway. Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
…eproviderdomainmanager.mm Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
…Domain synchronous. To avoid race conditions. Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
To avoid race conditions. Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
- The app sandbox migration of file provider domains now wipes the mapping groups from the configuration file. - The Account type now has "fileProviderDomainIdentifier" property and appropriate getters and setters in case the file provider module is built. - The AccountManager is extended accordingly to handle the new "fileProviderDomainIdentifier" property on Account. - The AccountManager now also provides a lookup method to fetch the account belonging to a file provider domain identifier. This was previously implemented by the FileProviderDomainManager but not within its scope of responsibility. - The FileProviderSettingsController now provides a method to migrate the enabled accounts from UserDefaults to the configuration file. - The FileProviderDomainManager contained mapping methods between account identifiers and file provider domain identifiers and UUIDs which were obsolete and removed. - Refactored FileProviderDomainManager to separate individual steps into smaller methods like the synchronous wrappers for NSFileProviderManager. This simplifies the code a lot by avoiding nested completion handler code. - Removed FileProviderDomainManager::accountStateFromFileProviderDomainIdentifier() which is redundant with the previously added AccountManager lookup method. - Removed dead code here and there. Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
…ain identifier instead of account identifier. Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
…n settings. Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
…st Xcode. Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
- Moved some logic (most notably removeOrphanedDomains and restoreMissingDomains) from FileProviderDomainManager into the more appropriate FileProviderSettingsController - Dissolved updateFileProviderDomains() - Removed domainSetupComplete signal - Introduced migrateToAppSandbox() implementation FileProviderSettingsController - Improved logging messages Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
ba98f26 to
6dcc1fd
Compare
|




Originally, I set out to avoid the appearance of a system prompt of our app wanting to access data from other apps when it actually just tried to create a local socket file in its own group container for interprocess communication.
group.prefix) so the main app can access it for creation of a debug archive. This renders the shared legacy account database migration code obsolete because it could not be accessed anyway.NSUserDefaultsno longer is used for storing file provider enabled accounts in other facilities.😵 Caveats
Breaking File Provider Change
This is a breaking change for our file provider extension. Currently, I do not see a way to enable the app sandbox without setting up file provider domains for accounts from scratch. The compliance with the app sandbox requires to use a proper group container identifier and app sandbox migration manifests only support containers (not group containers), to my knowledge.
Should a 4.1 build look for the file provider extension data, it will look in the new group container initially and not find anything. This is equal to a complete reset.
UNIX Sockets
Long identifiers may break the UNIX socket-based IPC. The app sandbox makes long path prefixes inevitable. In example:
It is about 122 characters long and there is a problem with that:
Apple XNU kernel source code for reference.
The solution for now is to keep it as short as possible but this may break with branded identifiers which are significantly longer than this reference case.
⚡️ Impact
This is not a simple bug fix but foundational changes to how security is implemented by our client on macOS. It requires extensive testing and cannot be delivered or ported back as a patch release. This is not just flipping a switch in some settings. The debug archive creation feature is an example how the technical debt of a missing app sandbox requires code refactoring.
🔗 Dependencies
This pull request requires these changes to NextcloudFileProviderKit.With release 4.0.0 of the package, all necessary changes are available.🔬 Testing
When upgrading from a previous release:
UserDefaultsare no longer used by the file provider extensionfileProviderDomainsAppSandboxMigrationCompleted=true