Skip safe atomic swap if update has custom update security policy (#2593)

Also emit a warning when checking if the updater is configured correctly.

Author: zorgiepoo

Autoupdate/SUPlainInstaller.m

@@ -344,26 +344,42 @@ - (BOOL)performInitialInstallation:(NSError * __autoreleasing *)error
 
     SUFileManager *fileManager = [[SUFileManager alloc] init];
 
+    BOOL updateHasCustomUpdateSecurityPolicy = NO;
     if (@available(macOS 13.0, *)) {
+        // If the new update is notarized / developer ID 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
+        // which think we are modifying the update (but we're not)
+        // To avoid these, we skip the gatekeeper scan and skip performing an atomic swap during install
+        // If the update has a custom update security policy, the same team ID policy may not apply,
+        // so in that case we will also skip performing an atomic swap
+        
         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]));
+            updateHasCustomUpdateSecurityPolicy = updateHost.hasUpdateSecurityPolicy;
+            if (updateHasCustomUpdateSecurityPolicy) {
+                // We don't handle working around a custom update security policy
+                _canPerformSafeAtomicSwap = NO;
+            } else {
+                NSString *installerTeamIdentifier = [SUCodeSigningVerifier teamIdentifierAtURL:mainExecutableURL];
+                NSString *bundleTeamIdentifier = [SUCodeSigningVerifier teamIdentifierAtURL:bundle.bundleURL];
+                
+                // If bundleTeamIdentifier is nil, then the update isn't code signed so atomic swap is okay
+                _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);
+        if (updateHasCustomUpdateSecurityPolicy) {
+            SULog(SULogLevelDefault, @"Skipping atomic rename/swap and gatekeeper scan because new update %@ has a custom NSUpdateSecurityPolicy", bundle.bundleURL.lastPathComponent);
+        } else {
+            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];

Sparkle/SPUUpdater.m

@@ -76,6 +76,7 @@ @implementation SPUUpdater
     BOOL _showingPermissionRequest;
     BOOL _loggedATSWarning;
     BOOL _loggedNoSecureKeyWarning;
+    BOOL _loggedUpdateSecurityPolicyWarning;
     BOOL _updatingMainBundle;
 }
 
@@ -361,6 +362,14 @@ - (BOOL)checkIfConfiguredProperlyAndRequireFeedURL:(BOOL)requireFeedURL validate
         }
     }
 
+    if (_updatingMainBundle) {
+        if (!_loggedUpdateSecurityPolicyWarning && mainBundleHost.hasUpdateSecurityPolicy) {
+            SULog(SULogLevelDefault, @"Warning: %@ has a custom NSUpdateSecurityPolicy in its Info.plist. This may cause issues when installing updates. Please consider removing this key for your builds using Sparkle if you do not really require a custom update security policy.", hostName);
+            
+            _loggedUpdateSecurityPolicyWarning = YES;
+        }
+    }
+    
     return YES;
 }
 

Sparkle/SUHost.h

@@ -33,6 +33,8 @@ SPU_OBJC_DIRECT_MEMBERS
 @property (getter=isRunningTranslocated, nonatomic, readonly) BOOL runningTranslocated;
 @property (readonly, nonatomic, copy, nullable) NSString *publicDSAKeyFileKey;
 
+@property (nonatomic, readonly) BOOL hasUpdateSecurityPolicy;
+
 - (nullable id)objectForInfoDictionaryKey:(NSString *)key;
 - (BOOL)boolForInfoDictionaryKey:(NSString *)key;
 - (nullable id)objectForUserDefaultsKey:(NSString *)defaultName;

Sparkle/SUHost.m

@@ -161,6 +161,13 @@ - (NSString *_Nullable)publicDSAKey SPU_OBJC_DIRECT
     return key;
 }
 
+- (BOOL)hasUpdateSecurityPolicy
+{
+    NSDictionary<NSString *, id> *updateSecurityPolicy = [self objectForInfoDictionaryKey:@"NSUpdateSecurityPolicy"];
+    
+    return (updateSecurityPolicy != nil);
+}
+
 - (SUPublicKeys *)publicKeys
 {
     return [[SUPublicKeys alloc] initWithEd:[self publicEDKey]