Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions Xcodes/Backend/AppState+Install.swift
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,9 @@ extension AppState {
try await xcodePostInstallWorkflowService.performPostInstallSteps(for: xcode)
} catch {
Logger.appState.error("Performing post-install steps failed: \(error.legibleLocalizedDescription)")
throw InstallationError.postInstallStepsNotPerformed(version: xcode.version, helperInstallState: helperInstallState)
// Keep the underlying reason (unless the user simply declined), so the alert can say why it failed.
let reason: String? = if case InstallationError.postInstallStepsNotPerformed = error { nil } else { error.legibleLocalizedDescription }
throw InstallationError.postInstallStepsNotPerformed(version: xcode.version, helperInstallState: helperInstallState, reason: reason)
}
}

Expand Down Expand Up @@ -411,7 +413,8 @@ extension AppState {
helperConsent.resume(
throwing: InstallationError.postInstallStepsNotPerformed(
version: version,
helperInstallState: self.helperInstallState
helperInstallState: self.helperInstallState,
reason: nil
)
)
}
Expand Down Expand Up @@ -511,7 +514,8 @@ public enum InstallationError: LocalizedError, Equatable {
case versionAlreadyInstalled(InstalledXcode)
case invalidVersion(String)
case versionNotInstalled(Version)
case postInstallStepsNotPerformed(version: Version, helperInstallState: HelperInstallState)
/// `reason` describes the underlying failure, if there was one besides the user declining to install the helper
case postInstallStepsNotPerformed(version: Version, helperInstallState: HelperInstallState, reason: String?)

public var errorDescription: String? {
switch self {
Expand Down Expand Up @@ -545,13 +549,16 @@ public enum InstallationError: LocalizedError, Equatable {
return String(format: localizeString("InstallationError.InvalidVersion"), version)
case let .versionNotInstalled(version):
return String(format: localizeString("InstallationError.VersionNotInstalled"), version.appleDescription)
case let .postInstallStepsNotPerformed(version, helperInstallState):
case let .postInstallStepsNotPerformed(version, helperInstallState, reason):
let message: String
switch helperInstallState {
case .installed:
return String(format: localizeString("InstallationError.PostInstallStepsNotPerformed.Installed"), version.appleDescription)
message = String(format: localizeString("InstallationError.PostInstallStepsNotPerformed.Installed"), version.appleDescription)
case .notInstalled, .unknown:
return String(format: localizeString("InstallationError.PostInstallStepsNotPerformed.NotInstalled"), version.appleDescription)
message = String(format: localizeString("InstallationError.PostInstallStepsNotPerformed.NotInstalled"), version.appleDescription)
}
guard let reason else { return message }
return "\(message)\n\n\(reason)"
}
}
}
Expand Down
32 changes: 28 additions & 4 deletions Xcodes/Backend/AppState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ class AppState: ObservableObject {
@Published var presentedAlert: XcodesAlert?
@Published var presentedPreferenceAlert: XcodesPreferencesAlert?
@Published var helperInstallState: HelperInstallState = .notInstalled
/// Delay between attempts to reach a newly installed helper
var helperInstallRetryDelay: Duration = .milliseconds(500)
/// Whether the user is being prepared for the helper installation alert with an explanation.
/// This closure will be performed after the user chooses whether or not to proceed.
@Published var isPreparingUserForActionRequiringHelper: ((Bool) -> Void)?
Expand Down Expand Up @@ -546,15 +548,37 @@ class AppState: ObservableObject {
try Task.checkCancellation()
try await Current.helper.install()
try Task.checkCancellation()
await checkIfHelperIsInstalled()
// launchd starts a newly blessed helper asynchronously, so give it a moment to answer.
let connectionError = await checkIfHelperIsInstalled(attempts: 5)
try Task.checkCancellation()
guard helperInstallState == .installed else {
throw HelperClientError.unreachableAfterInstall(underlyingError: connectionError)
}
}
}

private func checkIfHelperIsInstalled() async {
/// Asks the helper for its version, retrying up to `attempts` times.
/// - Returns: The last connection error, if the helper couldn't be reached.
@discardableResult
private func checkIfHelperIsInstalled(attempts: Int = 1) async -> Error? {
helperInstallState = .unknown

let installed = (try? await Current.helper.checkIfLatestHelperIsInstalledAsync()) ?? false
helperInstallState = installed ? .installed : .notInstalled
var lastError: Error?
for attempt in 1...max(1, attempts) {
do {
if try await Current.helper.checkIfLatestHelperIsInstalledAsync() {
helperInstallState = .installed
return nil
}
lastError = nil
} catch {
lastError = error
}
guard attempt < attempts, !Task.isCancelled else { break }
try? await Task.sleep(for: helperInstallRetryDelay)
}
helperInstallState = .notInstalled
return lastError
}

@discardableResult
Expand Down
33 changes: 32 additions & 1 deletion Xcodes/Backend/HelperClient.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Foundation
import LegibleError
import os.log
import ServiceManagement
import XcodesKit
Expand Down Expand Up @@ -235,7 +236,20 @@ final class HelperClient {
let authRef = try authorizationRef(&authRights, nil, [.interactionAllowed, .extendRights, .preAuthorize])
var cfError: Unmanaged<CFError>?
SMJobBless(kSMDomainSystemLaunchd, machServiceName as CFString, authRef, &cfError)
if let error = cfError?.takeRetainedValue() { throw error }
if let error = cfError?.takeRetainedValue() {
// kSMErrorDomainLaunchd is deprecated, but SMJobBless still reports its errors in that domain
if CFErrorGetDomain(error) as String == "CFErrorDomainLaunchd" {
switch CFErrorGetCode(error) {
case kSMErrorInvalidSignature:
throw HelperClientError.invalidSignature(underlyingError: error)
case kSMErrorAuthorizationFailure:
throw HelperClientError.authorizationFailed(underlyingError: error)
default:
break
}
}
throw error
}

self.connection?.invalidate()
self.connection = nil
Expand Down Expand Up @@ -271,13 +285,30 @@ final class HelperClient {
enum HelperClientError: LocalizedError {
case failedToCreateRemoteObjectProxy
case message(String)
/// SMJobBless rejected the helper because its signature doesn't satisfy the app's SMPrivilegedExecutables requirement
case invalidSignature(underlyingError: Error)
/// SMJobBless wasn't authorized, e.g. the administrator prompt was cancelled or couldn't be shown
case authorizationFailed(underlyingError: Error)
/// The helper was blessed but doesn't answer, e.g. it rejects this app's signature via SMAuthorizedClients
case unreachableAfterInstall(underlyingError: Error?)

var errorDescription: String? {
switch self {
case .failedToCreateRemoteObjectProxy:
return localizeString("HelperClient.error")
case let .message(message):
return message
case let .invalidSignature(underlyingError):
return Self.withDetails(localizeString("HelperClient.error.InvalidSignature"), underlyingError)
case let .authorizationFailed(underlyingError):
return Self.withDetails(localizeString("HelperClient.error.AuthorizationFailed"), underlyingError)
case let .unreachableAfterInstall(underlyingError):
return Self.withDetails(localizeString("HelperClient.error.UnreachableAfterInstall"), underlyingError)
}
}

private static func withDetails(_ message: String, _ underlyingError: Error?) -> String {
guard let underlyingError else { return message }
return "\(message)\n\n\(underlyingError.legibleLocalizedDescription)"
}
}
33 changes: 33 additions & 0 deletions Xcodes/Resources/Localizable.xcstrings
Original file line number Diff line number Diff line change
Expand Up @@ -11157,6 +11157,39 @@
}
}
},
"HelperClient.error.AuthorizationFailed" : {
"extractionState" : "manual",
"localizations" : {
"en" : {
"stringUnit" : {
"state" : "translated",
"value" : "macOS didn't authorize installing the privileged helper. Try again and enter an administrator's name and password when asked; the helper can also be installed from Settings > Advanced."
}
}
}
},
"HelperClient.error.InvalidSignature" : {
"extractionState" : "manual",
"localizations" : {
"en" : {
"stringUnit" : {
"state" : "translated",
"value" : "macOS refused to install the privileged helper because its code signature doesn't match what Xcodes expects. The app and helper must be signed by the same developer team."
}
}
}
},
"HelperClient.error.UnreachableAfterInstall" : {
"extractionState" : "manual",
"localizations" : {
"en" : {
"stringUnit" : {
"state" : "translated",
"value" : "The privileged helper was installed, but Xcodes couldn't connect to it. This usually means the helper and Xcodes are signed by different developer teams, for example a local development build running alongside a helper installed by the released app."
}
}
}
},
"HelperInstalled" : {
"localizations" : {
"ar" : {
Expand Down
59 changes: 59 additions & 0 deletions XcodesTests/AppStateTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,49 @@ class AppStateTests: XCTestCase {
subject = AppState()
}

func test_InstallHelper_RetriesUntilNewlyInstalledHelperAnswers() async throws {
subject.helperInstallState = .notInstalled
subject.helperInstallRetryDelay = .zero
let checks = AppStateTestsCounter()
Current.helper.install = { }
Current.helper.checkIfLatestHelperIsInstalledAsync = {
// launchd hasn't started the helper for the first couple of checks
checks.increment() >= 3
}

try await subject.installHelperIfNecessaryAsync()

XCTAssertEqual(subject.helperInstallState, .installed)
XCTAssertEqual(checks.value, 3)
}

func test_InstallHelper_ThrowsWithReasonWhenInstalledHelperIsUnreachable() async throws {
subject.helperInstallState = .notInstalled
subject.helperInstallRetryDelay = .zero
Current.helper.install = { }
Current.helper.checkIfLatestHelperIsInstalledAsync = {
throw NSError(domain: NSCocoaErrorDomain, code: NSXPCConnectionInvalid)
}

do {
try await subject.installHelperIfNecessaryAsync()
XCTFail("Expected an unreachable helper to throw")
} catch let HelperClientError.unreachableAfterInstall(underlyingError) {
XCTAssertEqual((underlyingError as NSError?)?.code, NSXPCConnectionInvalid)
}
XCTAssertEqual(subject.helperInstallState, .notInstalled)
}

func test_PostInstallStepsError_IncludesUnderlyingReason() {
let error = InstallationError.postInstallStepsNotPerformed(
version: Version("27.2.0")!,
helperInstallState: .notInstalled,
reason: HelperClientError.unreachableAfterInstall(underlyingError: nil).localizedDescription
)

XCTAssertTrue(error.errorDescription?.contains(localizeString("HelperClient.error.UnreachableAfterInstall")) == true)
}

func test_InstallError_Network401IsUnauthorized() {
let error = NetworkError.non200StatusCode(statusCode: 401, data: Data())

Expand Down Expand Up @@ -1046,3 +1089,19 @@ private extension HTTPCookie {
]))
}
}

private final class AppStateTestsCounter: @unchecked Sendable {
private let lock = NSLock()
private var count = 0

var value: Int {
lock.withLock { count }
}

func increment() -> Int {
lock.withLock {
count += 1
return count
}
}
}