Resolving automatic update edge cases

Configuration/BuildNumber.xcconfig

@@ -1 +1 @@
-CURRENT_PROJECT_VERSION = 247
+CURRENT_PROJECT_VERSION = 246

DuckDuckGo.xcodeproj/project.pbxproj

@@ -44,6 +44,8 @@
 		1D220BF92B86192200F8BBC6 /* PreferencesEmailProtectionView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1D220BF72B86192200F8BBC6 /* PreferencesEmailProtectionView.swift */; };
 		1D220BFC2B87AACF00F8BBC6 /* PrivacyProtectionStatus.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1D220BFB2B87AACF00F8BBC6 /* PrivacyProtectionStatus.swift */; };
 		1D220BFD2B87AACF00F8BBC6 /* PrivacyProtectionStatus.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1D220BFB2B87AACF00F8BBC6 /* PrivacyProtectionStatus.swift */; };
+		1D232E942C7860DA0043840D /* BinaryOwnershipChecker.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1D232E932C7860DA0043840D /* BinaryOwnershipChecker.swift */; };
+		1D232E992C7870D90043840D /* BinaryOwnershipCheckerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1D232E962C786E7D0043840D /* BinaryOwnershipCheckerTests.swift */; };
 		1D26EBAC2B74BECB0002A93F /* NSImageSendable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1D26EBAB2B74BECB0002A93F /* NSImageSendable.swift */; };
 		1D26EBAD2B74BECB0002A93F /* NSImageSendable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1D26EBAB2B74BECB0002A93F /* NSImageSendable.swift */; };
 		1D26EBB02B74DB600002A93F /* TabSnapshotCleanupService.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1D26EBAF2B74DB600002A93F /* TabSnapshotCleanupService.swift */; };
@@ -2969,6 +2971,8 @@
 		1D1C36E529FB019C001FA40C /* HistoryTabExtensionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HistoryTabExtensionTests.swift; sourceTree = "<group>"; };
 		1D220BF72B86192200F8BBC6 /* PreferencesEmailProtectionView.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PreferencesEmailProtectionView.swift; sourceTree = "<group>"; };
 		1D220BFB2B87AACF00F8BBC6 /* PrivacyProtectionStatus.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PrivacyProtectionStatus.swift; sourceTree = "<group>"; };
+		1D232E932C7860DA0043840D /* BinaryOwnershipChecker.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BinaryOwnershipChecker.swift; sourceTree = "<group>"; };
+		1D232E962C786E7D0043840D /* BinaryOwnershipCheckerTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BinaryOwnershipCheckerTests.swift; sourceTree = "<group>"; };
 		1D26EBAB2B74BECB0002A93F /* NSImageSendable.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NSImageSendable.swift; sourceTree = "<group>"; };
 		1D26EBAF2B74DB600002A93F /* TabSnapshotCleanupService.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TabSnapshotCleanupService.swift; sourceTree = "<group>"; };
 		1D36E657298AA3BA00AA485D /* InternalUserDeciderStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InternalUserDeciderStore.swift; sourceTree = "<group>"; };
@@ -4729,6 +4733,7 @@
 				1D72D59B2BFF61B200AEDE36 /* UpdateNotificationPresenter.swift */,
 				1D9297BE2C1B062900A38521 /* ApplicationUpdateDetector.swift */,
 				1D0DE93D2C3BA9840037ABC2 /* AppRestarter.swift */,
+				1D232E932C7860DA0043840D /* BinaryOwnershipChecker.swift */,
 				1D39E5762C2BFD5700757339 /* ReleaseNotesTabExtension.swift */,
 				1D39E5792C2C0F3700757339 /* ReleaseNotesUserScript.swift */,
 				1D0DE9402C3BB9CC0037ABC2 /* ReleaseNotesParser.swift */,
@@ -4769,6 +4774,7 @@
 			children = (
 				1D838A312C44F0180078373F /* ReleaseNotesParserTests.swift */,
 				1D638D602C44F2BA00530DD5 /* ApplicationUpdateDetectorTests.swift */,
+				1D232E962C786E7D0043840D /* BinaryOwnershipCheckerTests.swift */,
 			);
 			path = Updates;
 			sourceTree = "<group>";
@@ -12003,6 +12009,7 @@
 				37BF3F21286F0A7A00BD9014 /* PinnedTabsViewModel.swift in Sources */,
 				EEC4A6692B2C87D300F7C0AA /* VPNLocationView.swift in Sources */,
 				AAC5E4D225D6A709007F5990 /* BookmarkList.swift in Sources */,
+				1D232E942C7860DA0043840D /* BinaryOwnershipChecker.swift in Sources */,
 				B602E81D2A1E25B1006D261F /* NEOnDemandRuleExtension.swift in Sources */,
 				56A0543E2C215FB3007D8FAB /* OnboardingUserScript.swift in Sources */,
 				C1372EF42BBC5BAD003F8793 /* SecureTextField.swift in Sources */,
@@ -12314,6 +12321,7 @@
 				56A054302C2043C8007D8FAB /* OnboardingTabExtensionTests.swift in Sources */,
 				986189E62A7CFB3E001B4519 /* LocalBookmarkStoreSavingTests.swift in Sources */,
 				AA652CD325DDA6E9009059CC /* LocalBookmarkManagerTests.swift in Sources */,
+				1D232E992C7870D90043840D /* BinaryOwnershipCheckerTests.swift in Sources */,
 				CBDD5DE329A67F2700832877 /* MockConfigurationStore.swift in Sources */,
 				9F3910692B68D87B00CB5112 /* ProgressExtensionTests.swift in Sources */,
 				B63ED0DC26AE7B1E00A9DAD1 /* WebViewMock.swift in Sources */,

DuckDuckGo/Info.plist

@@ -554,6 +554,8 @@
 	<integer>10800</integer>
 	<key>ViewBridgeService</key>
 	<false/>
+	<key>APP_CONFIGURATION_APP_GROUP</key>
+	<string>$(APP_CONFIGURATION_APP_GROUP)</string>
 	<key>IPC_APP_GROUP</key>
 	<string>$(IPC_APP_GROUP)</string>
 </dict>

DuckDuckGo/Updates/BinaryOwnershipChecker.swift

@@ -0,0 +1,67 @@
+//
+//  BinaryOwnershipChecker.swift
+//
+//  Copyright © 2024 DuckDuckGo. All rights reserved.
+//
+//  Licensed under the Apache License, Version 2.0 (the "License");
+//  you may not use this file except in compliance with the License.
+//  You may obtain a copy of the License at
+//
+//  http://www.apache.org/licenses/LICENSE-2.0
+//
+//  Unless required by applicable law or agreed to in writing, software
+//  distributed under the License is distributed on an "AS IS" BASIS,
+//  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//  See the License for the specific language governing permissions and
+//  limitations under the License.
+//
+
+import Foundation
+import Common
+
+protocol BinaryOwnershipChecking {
+    func isCurrentUserOwner() -> Bool
+}
+
+/// A class responsible for checking whether the current user owns the binary of the app.
+/// The result is cached after the first check to avoid repeated file system access.
+final class BinaryOwnershipChecker: BinaryOwnershipChecking {
+
+    private let fileManager: FileManager
+    private var ownershipCache: Bool?
+
+    init(fileManager: FileManager = .default) {
+        self.fileManager = fileManager
+    }
+
+    /// Checks if the current user owns the binary of the currently running app.
+    /// The method caches the result after the first check to improve performance on subsequent calls.
+    /// - Returns: `true` if the current user is the owner, `false` otherwise.
+    func isCurrentUserOwner() -> Bool {
+        if let cachedResult = ownershipCache {
+            return cachedResult
+        }
+
+        guard let binaryPath = Bundle.main.executablePath else {
+            os_log("Failed to get the binary path", log: .updates)
+            ownershipCache = false
+            return false
+        }
+
+        do {
+            let attributes = try fileManager.attributesOfItem(atPath: binaryPath)
+            if let ownerID = attributes[FileAttributeKey.ownerAccountID] as? NSNumber {
+                let isOwner = ownerID.intValue == getuid()
+                ownershipCache = isOwner
+                return isOwner
+            }
+        } catch {
+            os_log("Failed to get binary file attributes: %{public}@",
+                   log: .updates,
+                   error.localizedDescription)
+        }
+
+        ownershipCache = false
+        return false
+    }
+}

DuckDuckGo/Updates/UpdateController.swift

@@ -58,27 +58,26 @@ final class UpdateController: NSObject, UpdateControllerProtocol {
     lazy var notificationPresenter = UpdateNotificationPresenter()
     let willRelaunchAppPublisher: AnyPublisher<Void, Never>
 
-    init(internalUserDecider: InternalUserDecider,
-         appRestarter: AppRestarting = AppRestarter()) {
-        willRelaunchAppPublisher = willRelaunchAppSubject.eraseToAnyPublisher()
-        self.internalUserDecider = internalUserDecider
-        self.appRestarter = appRestarter
-        super.init()
-
-        configureUpdater()
-    }
-
     @Published private(set) var isUpdateBeingLoaded = false
     var isUpdateBeingLoadedPublisher: Published<Bool>.Publisher { $isUpdateBeingLoaded }
 
+    // Struct used to cache data until the updater finishes checking for updates
+    struct UpdateCheckResult {
+        let item: SUAppcastItem
+        let isInstalled: Bool
+    }
+    private var updateCheckResult: UpdateCheckResult?
+
     @Published private(set) var latestUpdate: Update? {
         didSet {
             if let latestUpdate, !latestUpdate.isInstalled {
-                switch latestUpdate.type {
-                case .critical:
-                    notificationPresenter.showUpdateNotification(icon: NSImage.criticalUpdateNotificationInfo, text: UserText.criticalUpdateNotification, presentMultiline: true)
-                case .regular:
-                    notificationPresenter.showUpdateNotification(icon: NSImage.updateNotificationInfo, text: UserText.updateAvailableNotification, presentMultiline: true)
+                if !shouldShowManualUpdateDialog {
+                    switch latestUpdate.type {
+                    case .critical:
+                        notificationPresenter.showUpdateNotification(icon: NSImage.criticalUpdateNotificationInfo, text: UserText.criticalUpdateNotification, presentMultiline: true)
+                    case .regular:
+                        notificationPresenter.showUpdateNotification(icon: NSImage.updateNotificationInfo, text: UserText.updateAvailableNotification, presentMultiline: true)
+                    }
                 }
                 isUpdateAvailableToInstall = !latestUpdate.isInstalled
             } else {
@@ -112,15 +111,34 @@ final class UpdateController: NSObject, UpdateControllerProtocol {
         }
     }
 
+    var automaticUpdateFlow: Bool {
+        // In case the current user is not the owner of the binary, we have to switch
+        // to manual update flow because the authentication is required.
+        return areAutomaticUpdatesEnabled && binaryOwnershipChecker.isCurrentUserOwner()
+    }
+
     var shouldShowManualUpdateDialog = false
 
     private(set) var updater: SPUStandardUpdaterController!
     private var appRestarter: AppRestarting
     private let willRelaunchAppSubject = PassthroughSubject<Void, Never>()
     private var internalUserDecider: InternalUserDecider
+    private let binaryOwnershipChecker: BinaryOwnershipChecking
 
     // MARK: - Public
 
+    init(internalUserDecider: InternalUserDecider,
+         appRestarter: AppRestarting = AppRestarter(),
+         binaryOwnershipChecker: BinaryOwnershipChecking = BinaryOwnershipChecker()) {
+        willRelaunchAppPublisher = willRelaunchAppSubject.eraseToAnyPublisher()
+        self.internalUserDecider = internalUserDecider
+        self.appRestarter = appRestarter
+        self.binaryOwnershipChecker = binaryOwnershipChecker
+        super.init()
+
+        configureUpdater()
+    }
+
     func checkNewApplicationVersion() {
         let updateStatus = ApplicationUpdateDetector.isApplicationUpdated()
         switch updateStatus {
@@ -144,42 +162,40 @@ final class UpdateController: NSObject, UpdateControllerProtocol {
         updater.updater.checkForUpdatesInBackground()
     }
 
+    @objc func runUpdate() {
+        PixelKit.fire(DebugEvent(GeneralPixel.updaterDidRunUpdate))
+
+        if automaticUpdateFlow {
+            appRestarter.restart()
+        } else {
+            updater.userDriver.activeUpdateAlert?.hideUnnecessaryUpdateButtons()
+            shouldShowManualUpdateDialog = true
+            checkForUpdate()
+        }
+    }
+
     // MARK: - Private
 
     private func configureUpdater() {
         // The default configuration of Sparkle updates is in Info.plist
         updater = SPUStandardUpdaterController(updaterDelegate: self, userDriverDelegate: self)
         shouldShowManualUpdateDialog = false
 
-        if updater.updater.automaticallyDownloadsUpdates != areAutomaticUpdatesEnabled {
-            updater.updater.automaticallyDownloadsUpdates = areAutomaticUpdatesEnabled
+        if updater.updater.automaticallyDownloadsUpdates != automaticUpdateFlow {
+            updater.updater.automaticallyDownloadsUpdates = automaticUpdateFlow
         }
 
 #if DEBUG
         updater.updater.automaticallyChecksForUpdates = false
         updater.updater.automaticallyDownloadsUpdates = false
         updater.updater.updateCheckInterval = 0
 #endif
-
-        checkForUpdateInBackground()
     }
 
-    @objc func openUpdatesPage() {
+    @objc private func openUpdatesPage() {
         notificationPresenter.openUpdatesPage()
     }
 
-    @objc func runUpdate() {
-        PixelKit.fire(DebugEvent(GeneralPixel.updaterDidRunUpdate))
-
-        if areAutomaticUpdatesEnabled {
-            appRestarter.restart()
-        } else {
-            updater.userDriver.activeUpdateAlert?.hideUnnecessaryUpdateButtons()
-            shouldShowManualUpdateDialog = true
-            checkForUpdate()
-        }
-    }
-
 }
 
 extension UpdateController: SPUStandardUserDriverDelegate {
@@ -201,6 +217,7 @@ extension UpdateController: SPUUpdaterDelegate {
     }
 
     private func onUpdateCheckStart() {
+        updateCheckResult = nil
         isUpdateBeingLoaded = true
     }
 
@@ -217,7 +234,9 @@ extension UpdateController: SPUUpdaterDelegate {
     }
 
     func updater(_ updater: SPUUpdater, didAbortWithError error: Error) {
-        os_log("Updater did abort with error: \(error.localizedDescription)", log: .updates)
+        os_log("Updater did abort with error: %{public}@",
+               log: .updates,
+               error.localizedDescription)
 
         let errorCode = (error as NSError).code
         guard ![Int(Sparkle.SUError.noUpdateError.rawValue),
@@ -231,51 +250,69 @@ extension UpdateController: SPUUpdaterDelegate {
     }
 
     func updater(_ updater: SPUUpdater, didFindValidUpdate item: SUAppcastItem) {
-        os_log("Updater did find valid update: \(item.displayVersionString)(\(item.versionString))", log: .updates)
+        os_log("Updater did find valid update: %{public}@",
+               log: .updates,
+               "\(item.displayVersionString)(\(item.versionString))")
 
         PixelKit.fire(DebugEvent(GeneralPixel.updaterDidFindUpdate))
 
-        guard !areAutomaticUpdatesEnabled else {
-            // If automatic updates are enabled, we are waiting until the update is downloaded
-            return
+        if !automaticUpdateFlow {
+            // For manual updates, we can present the available update without waiting for the update cycle to finish. The Sparkle flow downloads the update later
+            updateCheckResult = UpdateCheckResult(item: item, isInstalled: false)
+            onUpdateCheckEnd()
         }
-        // For manual updates, show the available update without downloading
-        onUpdateCheckEnd(item: item, isInstalled: false)
     }
 
     func updaterDidNotFindUpdate(_ updater: SPUUpdater, error: any Error) {
         let item = (error as NSError).userInfo["SULatestAppcastItemFound"] as? SUAppcastItem
-        os_log("Updater did not find update: \(String(describing: item?.displayVersionString))(\(String(describing: item?.versionString)))", log: .updates)
-
-        onUpdateCheckEnd(item: item, isInstalled: true)
+        os_log("Updater did not find update: %{public}@",
+               log: .updates,
+               "\(item?.displayVersionString ?? "")(\(item?.versionString ?? ""))")
+        if let item {
+            // User is running the latest version
+            updateCheckResult = UpdateCheckResult(item: item, isInstalled: true)
+        }
 
         PixelKit.fire(DebugEvent(GeneralPixel.updaterDidNotFindUpdate, error: error))
     }
 
     func updater(_ updater: SPUUpdater, didDownloadUpdate item: SUAppcastItem) {
-        os_log("Updater did download update: \(item.displayVersionString)(\(item.versionString))", log: .updates)
+        os_log("Updater did download update: %{public}@",
+               log: .updates,
+               "\(item.displayVersionString)(\(item.versionString))")
 
-        guard areAutomaticUpdatesEnabled else {
-            // If manual are enabled, we don't download
+        if automaticUpdateFlow {
+            // For automatic updates, the available item has to be downloaded
+            updateCheckResult = UpdateCheckResult(item: item, isInstalled: false)
             return
         }
-        // Automatic updates present the available update after it's downloaded
-        onUpdateCheckEnd(item: item, isInstalled: false)
 
         PixelKit.fire(DebugEvent(GeneralPixel.updaterDidDownloadUpdate))
     }
 
-    private func onUpdateCheckEnd(item: SUAppcastItem?, isInstalled: Bool) {
-        if let item {
-            latestUpdate = Update(appcastItem: item, isInstalled: isInstalled)
+    func updater(_ updater: SPUUpdater, didFinishUpdateCycleFor updateCheck: SPUUpdateCheck, error: (any Error)?) {
+        os_log("Updater did finish update cycle", log: .updates)
+
+        onUpdateCheckEnd()
+    }
+
+    private func onUpdateCheckEnd() {
+        guard isUpdateBeingLoaded else {
+            // The update check end is already handled
+            return
+        }
+
+        // If the update is available, present it
+        if let updateCheckResult = updateCheckResult {
+            latestUpdate = Update(appcastItem: updateCheckResult.item,
+                                  isInstalled: updateCheckResult.isInstalled)
         } else {
             latestUpdate = nil
         }
-        isUpdateBeingLoaded = false
-    }
 
-    func updater(_ updater: SPUUpdater, didFinishUpdateCycleFor updateCheck: SPUUpdateCheck, error: (any Error)?) {
-        os_log("Updater did finish update cycle", log: .updates)
+        // Clear cache
+        isUpdateBeingLoaded = false
+        updateCheckResult = nil
     }
 
 }

LocalPackages/BuildToolPlugins/Plugins/InputFilesChecker/InputFilesChecker.swift

@@ -21,6 +21,7 @@ import PackagePlugin
 import XcodeProjectPlugin
 
 let nonSandboxedExtraInputFiles: Set<InputFile> = [
+    .init("BinaryOwnershipChecker.swift", .source),
     .init("BWEncryption.m", .source),
     .init("BWEncryptionOutput.m", .source),
     .init("BWManager.swift", .source),
@@ -49,6 +50,7 @@ let extraInputFiles: [TargetName: Set<InputFile>] = [
     "DuckDuckGo Privacy Pro": nonSandboxedExtraInputFiles,
 
     "Unit Tests": [
+        .init("BinaryOwnershipCheckerTests.swift", .source),
         .init("BWEncryptionTests.swift", .source),
         .init("WKWebViewPrivateMethodsAvailabilityTests.swift", .source)
     ],