Swap app bundles with APFS atomic swap if team IDs match (sparkle-pro…

…ject#2516)

Autoupdate/SUCodeSigningVerifier.h

@@ -11,6 +11,8 @@
 
 #import <Foundation/Foundation.h>
 
+NS_ASSUME_NONNULL_BEGIN
+
 #ifndef BUILDING_SPARKLE_TESTS
 SPU_OBJC_DIRECT_MEMBERS
 #endif
@@ -25,8 +27,10 @@ SPU_OBJC_DIRECT_MEMBERS
 
 + (BOOL)bundleAtURLIsCodeSigned:(NSURL *)bundleURL;
 
-+ (BOOL)teamIdentifierAtURL:(NSURL *)url1 matchesTeamIdentifierAtURL:(NSURL *)url2;
++ (NSString * _Nullable)teamIdentifierAtURL:(NSURL *)url;
 
 @end
 
+NS_ASSUME_NONNULL_END
+
 #endif

Autoupdate/SUCodeSigningVerifier.m

@@ -258,7 +258,7 @@ + (BOOL)bundleAtURLIsCodeSigned:(NSURL *)bundleURL
     return (result == 0);
 }
 
-static NSString * _Nullable teamIdentifierAtURL(NSURL *url)
++ (NSString * _Nullable)teamIdentifierAtURL:(NSURL *)url
 {
     SecStaticCodeRef staticCode = NULL;
     OSStatus staticCodeResult = SecStaticCodeCreateWithPath((__bridge CFURLRef)url, kSecCSDefaultFlags, &staticCode);
@@ -282,19 +282,4 @@ + (BOOL)bundleAtURLIsCodeSigned:(NSURL *)bundleURL
     return signingInformation[(NSString *)kSecCodeInfoTeamIdentifier];
 }
 
-+ (BOOL)teamIdentifierAtURL:(NSURL *)url1 matchesTeamIdentifierAtURL:(NSURL *)url2
-{
-    NSString *teamIdentifierForURL1 = teamIdentifierAtURL(url1);
-    if (teamIdentifierForURL1 == nil) {
-        return NO;
-    }
-
-    NSString *teamIdentifierForURL2 = teamIdentifierAtURL(url2);
-    if (teamIdentifierForURL2 == nil) {
-        return NO;
-    }
-
-    return [teamIdentifierForURL1 isEqualToString:teamIdentifierForURL2];
-}
-
 @end

Autoupdate/SUPlainInstaller.m

@@ -29,6 +29,7 @@ @implementation SUPlainInstaller
     NSURL *_temporaryNewDirectory;
 
     BOOL _newAndOldBundlesOnSameVolume;
+    BOOL _canPerformSafeAtomicSwap;
 }
 
 - (instancetype)initWithHost:(SUHost *)host bundlePath:(NSString *)bundlePath installationPath:(NSString *)installationPath
@@ -42,7 +43,7 @@ - (instancetype)initWithHost:(SUHost *)host bundlePath:(NSString *)bundlePath in
     return self;
 }
 
-- (void)_performInitialInstallationWithFileManager:(SUFileManager *)fileManager oldBundleURL:(NSURL *)oldBundleURL newBundleURL:(NSURL *)newBundleURL skipGatekeeperScan:(BOOL)skipGatekeeperScan progressBlock:(nullable void(^)(double))progress SPU_OBJC_DIRECT
+- (void)_performInitialInstallationWithFileManager:(SUFileManager *)fileManager oldBundleURL:(NSURL *)oldBundleURL newBundleURL:(NSURL *)newBundleURL performGatekeeperScan:(BOOL)performGatekeeperScan progressBlock:(nullable void(^)(double))progress SPU_OBJC_DIRECT
 {
     // Release our new app from quarantine
     NSError *quarantineError = nil;
@@ -90,7 +91,7 @@ - (void)_performInitialInstallationWithFileManager:(SUFileManager *)fileManager
         progress(8/11.0);
     }
 
-    if (!skipGatekeeperScan) {
+    if (performGatekeeperScan) {
         // Perform a Gatekeeper scan to pre-warm the app launch
         // This avoids users seeing a "Verifying..." dialog when the installed update is launched
         // Note the tool we use to perform the Gatekeeper scan (gktool) is technically available on macOS 14.0,
@@ -100,32 +101,23 @@ - (void)_performInitialInstallationWithFileManager:(SUFileManager *)fileManager
             // Only perform Gatekeeper scan if we're updating an app bundle
             NSString *newBundlePath = newBundleURL.path;
             if ([newBundlePath.pathExtension caseInsensitiveCompare:@"app"] == NSOrderedSame) {
-                // We only invoke gktool if Autoupdate is signed with the same team identifier as the new update bundle
-                // Otherwise we may unfortunately run into some Privacy & Security prompt bugs in the OS (note this is *not* a security check)
-                // This does overall imply that for an app to test the gktool path, this path may often skipped for most common development workflows that don't
-                // re-sign Sparkle's Autoupdate helper
-                NSURL *mainExecutableURL = NSBundle.mainBundle.executableURL;
-                if (mainExecutableURL != nil && [SUCodeSigningVerifier teamIdentifierAtURL:mainExecutableURL matchesTeamIdentifierAtURL:newBundleURL]) {
-                    NSURL *gktoolURL = [NSURL fileURLWithPath:@"/usr/bin/gktool" isDirectory:NO];
-                    if ([gktoolURL checkResourceIsReachableAndReturnError:NULL]) {
-                        NSTask *gatekeeperScanTask = [[NSTask alloc] init];
-                        gatekeeperScanTask.executableURL = gktoolURL;
-                        gatekeeperScanTask.arguments = @[@"scan", newBundlePath];
+                NSURL *gktoolURL = [NSURL fileURLWithPath:@"/usr/bin/gktool" isDirectory:NO];
+                if ([gktoolURL checkResourceIsReachableAndReturnError:NULL]) {
+                    NSTask *gatekeeperScanTask = [[NSTask alloc] init];
+                    gatekeeperScanTask.executableURL = gktoolURL;
+                    gatekeeperScanTask.arguments = @[@"scan", newBundlePath];
 
-                        NSError *taskError;
-                        if (![gatekeeperScanTask launchAndReturnError:&taskError]) {
-                            // Not a fatal error
-                            SULog(SULogLevelError, @"Failed to perform GateKeeper scan on '%@' with error %@", newBundlePath, taskError);
-                        } else {
-                            [gatekeeperScanTask waitUntilExit];
-
-                            if (gatekeeperScanTask.terminationStatus != 0) {
-                                SULog(SULogLevelError, @"gktool failed and returned exit status %d", gatekeeperScanTask.terminationStatus);
-                            }
+                    NSError *taskError;
+                    if (![gatekeeperScanTask launchAndReturnError:&taskError]) {
+                        // Not a fatal error
+                        SULog(SULogLevelError, @"Failed to perform GateKeeper scan on '%@' with error %@", newBundlePath, taskError);
+                    } else {
+                        [gatekeeperScanTask waitUntilExit];
+
+                        if (gatekeeperScanTask.terminationStatus != 0) {
+                            SULog(SULogLevelError, @"gktool failed and returned exit status %d", gatekeeperScanTask.terminationStatus);
                         }
                     }
-                } else {
-                    SULog(SULogLevelDefault, @"Skipping invocation of gktool because Autoupdate is not signed with same identity as the new update %@", newBundleURL.lastPathComponent);
                 }
             }
         }
@@ -233,7 +225,7 @@ - (BOOL)startInstallationToURL:(NSURL *)installationURL fromUpdateAtURL:(NSURL *
     if (!_newAndOldBundlesOnSameVolume) {
         // If we're updating a bundle on another volume, the install process can be pretty slow.
         // In this case let's get out of the way and skip the Gatekeeper scan
-        [self _performInitialInstallationWithFileManager:fileManager oldBundleURL:oldURL newBundleURL:newFinalURL skipGatekeeperScan:YES progressBlock:progress];
+        [self _performInitialInstallationWithFileManager:fileManager oldBundleURL:oldURL newBundleURL:newFinalURL performGatekeeperScan:NO progressBlock:progress];
     }
 
     if (progress) {
@@ -243,8 +235,8 @@ - (BOOL)startInstallationToURL:(NSURL *)installationURL fromUpdateAtURL:(NSURL *
     // First try swapping the application atomically
     NSError *swapError = nil;
     BOOL swappedApp;
-    // If the app is normalized and the installation path differs, go through the old swap path
-    if (SPARKLE_NORMALIZE_INSTALLED_APPLICATION_NAME && ![oldURL.path isEqual:installationURL.path]) {
+    // If we can not safely perform an atomic swap, or if the app is normalized and the installation path differs, go through the old swap path
+    if (!_canPerformSafeAtomicSwap || (SPARKLE_NORMALIZE_INSTALLED_APPLICATION_NAME && ![oldURL.path isEqual:installationURL.path])) {
         swappedApp = NO;
     } else {
         // We will be cleaning up the temporary directory later in -performCleanup:
@@ -352,11 +344,33 @@ - (BOOL)performInitialInstallation:(NSError * __autoreleasing *)error
 
     SUFileManager *fileManager = [[SUFileManager alloc] init];
 
+    if (@available(macOS 13.0, *)) {
+        NSURL *mainExecutableURL = NSBundle.mainBundle.executableURL;
+        if (mainExecutableURL == nil) {
+            // This shouldn't happen
+            _canPerformSafeAtomicSwap = NO;
+        } else {
+            NSString *installerTeamIdentifier = [SUCodeSigningVerifier teamIdentifierAtURL:mainExecutableURL];
+            NSString *bundleTeamIdentifier = [SUCodeSigningVerifier teamIdentifierAtURL:bundle.bundleURL];
+
+            // If the new update is code signed and Autoupdate is not signed with the same Team ID as the new update,
+            // then we may run into Privacy & Security prompt issues from the OS
+            // To avoid these, we skip the gatekeeper scan and skip performing an atomic swap during install
+            _canPerformSafeAtomicSwap = (bundleTeamIdentifier == nil || (installerTeamIdentifier != nil && [installerTeamIdentifier isEqualToString:bundleTeamIdentifier]));
+        }
+    } else {
+        _canPerformSafeAtomicSwap = YES;
+    }
+
+    if (!_canPerformSafeAtomicSwap) {
+        SULog(SULogLevelDefault, @"Skipping atomic rename/swap and gatekeeper scan because Autoupdate is not signed with same identity as the new update %@", bundle.bundleURL.lastPathComponent);
+    }
+
     _newAndOldBundlesOnSameVolume = [fileManager itemAtURL:bundle.bundleURL isOnSameVolumeItemAsURL:_host.bundle.bundleURL];
 
     // We can do a lot of the installation work ahead of time if the new app update does not need to be copied to another volume
     if (_newAndOldBundlesOnSameVolume) {
-        [self _performInitialInstallationWithFileManager:fileManager oldBundleURL:_host.bundle.bundleURL newBundleURL:bundle.bundleURL skipGatekeeperScan:NO progressBlock:NULL];
+        [self _performInitialInstallationWithFileManager:fileManager oldBundleURL:_host.bundle.bundleURL newBundleURL:bundle.bundleURL performGatekeeperScan:_canPerformSafeAtomicSwap progressBlock:NULL];
     }
 
     return YES;