From d6b5c5bc31a944d02e2134607f7e4e59c8298dce Mon Sep 17 00:00:00 2001 From: Peter van Tol Date: Thu, 1 Oct 2026 17:45:49 +0200 Subject: [PATCH] Fail closed on stale registered installer helper parent --- macos/ForgePlatformInstaller/README.md | 4 + .../InstallerCLIHelperRegistration.swift | 12 +-- ...nstallerPrivilegedHelperRegistration.swift | 90 ++++++++++++++++++- ...PlatformInstallerCLIApplicationTests.swift | 15 ++-- ...lerPrivilegedHelperRegistrationTests.swift | 71 +++++++++++++-- 5 files changed, 168 insertions(+), 24 deletions(-) diff --git a/macos/ForgePlatformInstaller/README.md b/macos/ForgePlatformInstaller/README.md index e269a0a7..499fcd49 100644 --- a/macos/ForgePlatformInstaller/README.md +++ b/macos/ForgePlatformInstaller/README.md @@ -228,6 +228,10 @@ startup. It asks for explicit confirmation without `--yes`, rechecks installer currency immediately before `SMAppService` registration, and reports native `ENABLED` or `REQUIRES_APPROVAL` separately. Registration never implies that the product-operation backend or reboot persistence is ready. +An `ENABLED` system job is accepted only after a fixed-label native readback +confirms the registered parent bundle, Team, helper program and exact current +installer version. An older registered helper fails closed; registration does +not silently replace it. A separate reviewed transition is required. On a Mac where the service has never been seen, native status can initially be `NOT_FOUND`; the fixed bundled service is registered once and its status read again. A registration error accompanied by native `REQUIRES_APPROVAL` remains diff --git a/macos/ForgePlatformInstaller/Sources/ForgePlatformInstallerCLI/InstallerCLIHelperRegistration.swift b/macos/ForgePlatformInstaller/Sources/ForgePlatformInstallerCLI/InstallerCLIHelperRegistration.swift index 7d9e5391..3b1dcfa5 100644 --- a/macos/ForgePlatformInstaller/Sources/ForgePlatformInstallerCLI/InstallerCLIHelperRegistration.swift +++ b/macos/ForgePlatformInstaller/Sources/ForgePlatformInstallerCLI/InstallerCLIHelperRegistration.swift @@ -3,13 +3,13 @@ import ForgePlatformInstallerCore /// Registers the bundled helper only after the released CLI has passed trusted /// startup, an explicit operator decision and a fresh installer-currency read. enum InstallerCLIHelperRegistration { - typealias Registrar = @Sendable () async -> ManagedInstallerPrivilegedHelperRegistrationResult + typealias Registrar = @Sendable (InstallerVersion) async -> ManagedInstallerPrivilegedHelperRegistrationResult - static let liveRegistrar: Registrar = { + static let liveRegistrar: Registrar = { expectedVersion in let service = MacOSManagedInstallerPrivilegedHelperServiceController() return await ManagedInstallerPrivilegedHelperRegistrationCoordinator( service: service - ).ensureRegistered() + ).ensureRegistered(expectedVersion: expectedVersion) } static func run( @@ -35,7 +35,8 @@ enum InstallerCLIHelperRegistration { switch await startup.start(currentVersion: currentVersion) { case .ready(let freshRelease, _): - guard freshRelease == currentRelease else { + guard freshRelease == currentRelease, + freshRelease.version == currentVersion else { return blocked("De geverifieerde installerrelease veranderde vóór helperregistratie.") } case .updateRequired(let release), .relaunching(let release): @@ -49,7 +50,7 @@ enum InstallerCLIHelperRegistration { return blocked(reason) } - switch await register() { + switch await register(currentVersion) { case .ready(let receipt): return InstallerCLIResult( exitCode: .success, @@ -70,6 +71,7 @@ enum InstallerCLIHelperRegistration { case .registrationFailed: reason = "registration-failed" case .serviceUnavailable: reason = "service-unavailable" case .statusDrift: reason = "status-drift" + case .registeredParentMismatch: reason = "registered-parent-mismatch" } return InstallerCLIResult( exitCode: .executionFailed, diff --git a/macos/ForgePlatformInstaller/Sources/ForgePlatformInstallerCore/ManagedInstallerPrivilegedHelperRegistration.swift b/macos/ForgePlatformInstaller/Sources/ForgePlatformInstallerCore/ManagedInstallerPrivilegedHelperRegistration.swift index 6f7057df..8c56e929 100644 --- a/macos/ForgePlatformInstaller/Sources/ForgePlatformInstallerCore/ManagedInstallerPrivilegedHelperRegistration.swift +++ b/macos/ForgePlatformInstaller/Sources/ForgePlatformInstallerCore/ManagedInstallerPrivilegedHelperRegistration.swift @@ -1,3 +1,4 @@ +import Darwin import Foundation @preconcurrency import ServiceManagement @@ -24,6 +25,7 @@ public enum ManagedInstallerPrivilegedHelperRegistrationFailure: case registrationFailed case serviceUnavailable case statusDrift + case registeredParentMismatch } public struct ManagedInstallerPrivilegedHelperRegistrationReceipt: @@ -55,6 +57,7 @@ public enum ManagedInstallerPrivilegedHelperRegistrationResult: public protocol ManagedInstallerPrivilegedHelperServiceControlling: Sendable { func readStatus() -> ManagedInstallerPrivilegedHelperStatus + func readRegisteredParentVersion() -> InstallerVersion? func register() throws } @@ -65,6 +68,7 @@ public final class MacOSManagedInstallerPrivilegedHelperServiceController: ManagedInstallerPrivilegedHelperServiceControlling, @unchecked Sendable { private let statusReader: @Sendable () -> SMAppService.Status private let registrar: @Sendable () throws -> Void + private let parentVersionReader: @Sendable () -> InstallerVersion? public init() { let service = SMAppService.daemon( @@ -72,14 +76,17 @@ public final class MacOSManagedInstallerPrivilegedHelperServiceController: ) statusReader = { service.status } registrar = { try service.register() } + parentVersionReader = { RegisteredInstallerHelperParentReader().readVersion() } } init( statusReader: @escaping @Sendable () -> SMAppService.Status, - registrar: @escaping @Sendable () throws -> Void + registrar: @escaping @Sendable () throws -> Void, + parentVersionReader: @escaping @Sendable () -> InstallerVersion? = { nil } ) { self.statusReader = statusReader self.registrar = registrar + self.parentVersionReader = parentVersionReader } public func readStatus() -> ManagedInstallerPrivilegedHelperStatus { @@ -95,6 +102,68 @@ public final class MacOSManagedInstallerPrivilegedHelperServiceController: public func register() throws { try registrar() } + + public func readRegisteredParentVersion() -> InstallerVersion? { + parentVersionReader() + } +} + +/// Inspects only the fixed system job. The parent identity is an independent +/// ServiceManagement readback, never a version supplied by the CLI caller. +struct RegisteredInstallerHelperParentReader: Sendable { + func readVersion() -> InstallerVersion? { + let process = Process() + process.executableURL = URL(fileURLWithPath: "/bin/launchctl") + process.arguments = ["print", "system/" + ManagedInstallerPrivilegedHelperContract.label] + let pipe = Pipe() + process.standardOutput = pipe + process.standardError = pipe + let collector = BoundedProcessOutputCollector(maximumBytes: 64 * 1024) + let finished = DispatchSemaphore(value: 0) + process.terminationHandler = { _ in finished.signal() } + do { try process.run() } catch { return nil } + let drained = DispatchGroup() + drained.enter() + DispatchQueue.global(qos: .userInitiated).async { + defer { drained.leave() } + collector.consume(pipe.fileHandleForReading) + } + guard finished.wait(timeout: .now() + .seconds(5)) == .success else { + process.terminate() + if finished.wait(timeout: .now() + .seconds(2)) != .success { + _ = Darwin.kill(process.processIdentifier, SIGKILL) + _ = finished.wait(timeout: .now() + .seconds(2)) + } + return nil + } + guard drained.wait(timeout: .now() + .seconds(2)) == .success, + process.terminationReason == .exit, + process.terminationStatus == 0, + let data = collector.collectedData(), + let output = String(data: data, encoding: .utf8) else { return nil } + return Self.parse(output) + } + + static func parse(_ output: String) -> InstallerVersion? { + let lines = output.split(separator: "\n").map { $0.trimmingCharacters(in: .whitespaces) } + func uniqueValue(_ key: String) -> String? { + let matches = lines.compactMap { line -> String? in + let prefix = key + " = " + return line.hasPrefix(prefix) ? String(line.dropFirst(prefix.count)) : nil + } + return matches.count == 1 ? matches[0] : nil + } + guard lines.first == "system/\(ManagedInstallerPrivilegedHelperContract.label) = {", + uniqueValue("managed_by") == "com.apple.xpc.ServiceManagement", + lines.filter({ $0 == "\"team-identifier\" => \"ZEML4LPXH4\"" }).count == 1, + lines.filter({ $0 == "\"signing-identifier\" => \"\(ManagedInstallerPrivilegedHelperContract.label)\"" }).count == 1, + uniqueValue("parent bundle identifier") == + ManagedInstallerHelperSignedParentBundleLocator.bundleIdentifier, + uniqueValue("program identifier") == + ManagedInstallerPrivilegedHelperContract.bundleProgram + " (mode: 2)", + let raw = uniqueValue("parent bundle version") else { return nil } + return try? InstallerVersion(raw) + } } /// Registers the bundled system daemon once and immediately reads its state @@ -108,10 +177,12 @@ public actor ManagedInstallerPrivilegedHelperRegistrationCoordinator { self.service = service } - public func ensureRegistered() -> ManagedInstallerPrivilegedHelperRegistrationResult { + public func ensureRegistered( + expectedVersion: InstallerVersion + ) -> ManagedInstallerPrivilegedHelperRegistrationResult { switch service.readStatus() { case .enabled: - return receipt(for: .enabled) + return verifiedReady(expectedVersion: expectedVersion) case .requiresApproval: return receipt(for: .requiresApproval) case .notRegistered, .notFound: @@ -129,7 +200,7 @@ public actor ManagedInstallerPrivilegedHelperRegistrationCoordinator { } switch service.readStatus() { case .enabled: - return receipt(for: .enabled) + return verifiedReady(expectedVersion: expectedVersion) case .requiresApproval: return receipt(for: .requiresApproval) case .notRegistered: @@ -140,6 +211,17 @@ public actor ManagedInstallerPrivilegedHelperRegistrationCoordinator { } } + private func verifiedReady( + expectedVersion: InstallerVersion + ) -> ManagedInstallerPrivilegedHelperRegistrationResult { + guard service.readRegisteredParentVersion() == expectedVersion, + service.readStatus() == .enabled, + service.readRegisteredParentVersion() == expectedVersion else { + return .failed(.registeredParentMismatch) + } + return receipt(for: .enabled) + } + private func receipt( for status: ManagedInstallerPrivilegedHelperStatus ) -> ManagedInstallerPrivilegedHelperRegistrationResult { diff --git a/macos/ForgePlatformInstaller/Tests/ForgePlatformInstallerCLITests/ForgePlatformInstallerCLIApplicationTests.swift b/macos/ForgePlatformInstaller/Tests/ForgePlatformInstallerCLITests/ForgePlatformInstallerCLIApplicationTests.swift index 9b0f72f5..175fa42d 100644 --- a/macos/ForgePlatformInstaller/Tests/ForgePlatformInstallerCLITests/ForgePlatformInstallerCLIApplicationTests.swift +++ b/macos/ForgePlatformInstaller/Tests/ForgePlatformInstallerCLITests/ForgePlatformInstallerCLIApplicationTests.swift @@ -544,7 +544,7 @@ final class ForgePlatformInstallerCLIApplicationTests: XCTestCase { ["helper", "register", "--non-interactive"], startup: deniedStartup, version: "1.2.3", - registration: { await registrar.invoke() } + registration: { _ in await registrar.invoke() } ) XCTAssertEqual(denied.code, InstallerCLIExitCode.confirmationRequired.rawValue) let deniedStarts = await deniedStartup.startCalls() @@ -559,7 +559,7 @@ final class ForgePlatformInstallerCLIApplicationTests: XCTestCase { ["helper", "register", "--yes", "--non-interactive", "--json"], startup: readyStartup, version: "1.2.3", - registration: { await registrar.invoke() } + registration: { _ in await registrar.invoke() } ) XCTAssertEqual(enabled.code, InstallerCLIExitCode.success.rawValue) XCTAssertTrue(enabled.stdout.joined().contains("helper-enabled")) @@ -582,7 +582,7 @@ final class ForgePlatformInstallerCLIApplicationTests: XCTestCase { currentRelease: current, coordinator: CLIReadyCoordinator() )), version: "1.2.3", - registration: { await approval.invoke() } + registration: { _ in await approval.invoke() } ) XCTAssertEqual(approved.code, InstallerCLIExitCode.interactionRequired.rawValue) XCTAssertTrue(approved.stderr.joined().contains("REQUIRES_APPROVAL")) @@ -591,6 +591,7 @@ final class ForgePlatformInstallerCLIApplicationTests: XCTestCase { ManagedInstallerPrivilegedHelperRegistrationFailure.registrationFailed, .serviceUnavailable, .statusDrift, + .registeredParentMismatch, ] { let failed = await run( ["helper", "register", "--yes"], @@ -598,7 +599,7 @@ final class ForgePlatformInstallerCLIApplicationTests: XCTestCase { currentRelease: current, coordinator: CLIReadyCoordinator() )), version: "1.2.3", - registration: { .failed(failure) } + registration: { _ in .failed(failure) } ) XCTAssertEqual(failed.code, InstallerCLIExitCode.executionFailed.rawValue) XCTAssertTrue(failed.stderr.joined().contains("helper-registration-failed")) @@ -612,7 +613,7 @@ final class ForgePlatformInstallerCLIApplicationTests: XCTestCase { ["helper", "register", "--yes"], startup: recheck, version: "1.2.3", - registration: { await approval.invoke() } + registration: { _ in await approval.invoke() } ) XCTAssertEqual(denied.code, InstallerCLIExitCode.installerUpdateRequired.rawValue) let recheckStarts = await recheck.startCalls() @@ -627,7 +628,7 @@ final class ForgePlatformInstallerCLIApplicationTests: XCTestCase { recheckOutcome: .ready(currentRelease: next, coordinator: CLIReadyCoordinator()) ), version: "1.2.3", - registration: { await approval.invoke() } + registration: { _ in await approval.invoke() } ) XCTAssertEqual(drift.code, InstallerCLIExitCode.blocked.rawValue) let driftCalls = await approval.calls() @@ -639,7 +640,7 @@ final class ForgePlatformInstallerCLIApplicationTests: XCTestCase { startup: CLIStartupSpy, version: String?, confirmation: Bool = true, - registration: @escaping InstallerCLIHelperRegistration.Registrar = { + registration: @escaping InstallerCLIHelperRegistration.Registrar = { _ in .failed(.serviceUnavailable) } ) async -> (code: Int32, stdout: [String], stderr: [String]) { diff --git a/macos/ForgePlatformInstaller/Tests/ForgePlatformInstallerCoreTests/ManagedInstallerPrivilegedHelperRegistrationTests.swift b/macos/ForgePlatformInstaller/Tests/ForgePlatformInstallerCoreTests/ManagedInstallerPrivilegedHelperRegistrationTests.swift index 481a76b8..8b38d0c0 100644 --- a/macos/ForgePlatformInstaller/Tests/ForgePlatformInstallerCoreTests/ManagedInstallerPrivilegedHelperRegistrationTests.swift +++ b/macos/ForgePlatformInstaller/Tests/ForgePlatformInstallerCoreTests/ManagedInstallerPrivilegedHelperRegistrationTests.swift @@ -7,7 +7,7 @@ final class ManagedInstallerPrivilegedHelperRegistrationTests: XCTestCase { let service = HelperServiceController(statuses: [.enabled]) let result = await ManagedInstallerPrivilegedHelperRegistrationCoordinator( service: service - ).ensureRegistered() + ).ensureRegistered(expectedVersion: try! InstallerVersion("0.3.5")) let receipt = try XCTUnwrap(result.readyReceipt) XCTAssertEqual(receipt.label, ManagedInstallerPostToolXPCHelperIdentity.signingIdentifier) @@ -32,11 +32,11 @@ final class ManagedInstallerPrivilegedHelperRegistrationTests: XCTestCase { let service = HelperServiceController(statuses: [initial, .enabled]) let result = await ManagedInstallerPrivilegedHelperRegistrationCoordinator( service: service - ).ensureRegistered() + ).ensureRegistered(expectedVersion: try! InstallerVersion("0.3.5")) XCTAssertNotNil(result.readyReceipt) XCTAssertEqual(service.registerCount(), 1) - XCTAssertEqual(service.statusReadCount(), 2) + XCTAssertEqual(service.statusReadCount(), 3) } } @@ -49,7 +49,7 @@ final class ManagedInstallerPrivilegedHelperRegistrationTests: XCTestCase { let service = HelperServiceController(statuses: statuses) let result = await ManagedInstallerPrivilegedHelperRegistrationCoordinator( service: service - ).ensureRegistered() + ).ensureRegistered(expectedVersion: try! InstallerVersion("0.3.5")) let receipt = try XCTUnwrap(result.approvalReceipt) XCTAssertEqual(receipt.status, .requiresApproval) XCTAssertEqual(service.registerCount(), statuses.count == 1 ? 0 : 1) @@ -63,7 +63,7 @@ final class ManagedInstallerPrivilegedHelperRegistrationTests: XCTestCase { ) let result = await ManagedInstallerPrivilegedHelperRegistrationCoordinator( service: service - ).ensureRegistered() + ).ensureRegistered(expectedVersion: try! InstallerVersion("0.3.5")) XCTAssertEqual(try XCTUnwrap(result.approvalReceipt).status, .requiresApproval) XCTAssertEqual(service.registerCount(), 1) @@ -81,7 +81,7 @@ final class ManagedInstallerPrivilegedHelperRegistrationTests: XCTestCase { for (service, expected) in cases { let result = await ManagedInstallerPrivilegedHelperRegistrationCoordinator( service: service - ).ensureRegistered() + ).ensureRegistered(expectedVersion: try! InstallerVersion("0.3.5")) XCTAssertEqual(result.failure, expected) } } @@ -108,13 +108,63 @@ final class ManagedInstallerPrivilegedHelperRegistrationTests: XCTestCase { ] { let controller = MacOSManagedInstallerPrivilegedHelperServiceController( statusReader: { native }, - registrar: { registered.increment() } + registrar: { registered.increment() }, + parentVersionReader: { try? InstallerVersion("0.3.5") } ) XCTAssertEqual(controller.readStatus(), expected) + XCTAssertEqual(controller.readRegisteredParentVersion(), try InstallerVersion("0.3.5")) try controller.register() } XCTAssertEqual(registered.value(), 4) } + + func testEnabledOldOrUnreadableParentFailsClosed() async throws { + for parent in [try InstallerVersion("0.2.4"), nil] { + let service = HelperServiceController(statuses: [.enabled], parentVersion: parent) + let result = await ManagedInstallerPrivilegedHelperRegistrationCoordinator( + service: service + ).ensureRegistered(expectedVersion: try InstallerVersion("0.3.5")) + XCTAssertEqual(result.failure, .registeredParentMismatch) + XCTAssertEqual(service.registerCount(), 0) + } + } + + func testFixedSystemJobParserRejectsWrongAndAmbiguousParent() throws { + let label = ManagedInstallerPrivilegedHelperContract.label + let valid = """ + system/\(label) = { + managed_by = com.apple.xpc.ServiceManagement + "signing-identifier" => "\(label)" + "team-identifier" => "ZEML4LPXH4" + program identifier = Contents/Resources/forge-platform-installer-helper (mode: 2) + parent bundle identifier = com.autonomous-engineering-system.forge-platform-installer + parent bundle version = 0.3.5 + } + """ + XCTAssertEqual(RegisteredInstallerHelperParentReader.parse(valid), + try InstallerVersion("0.3.5")) + XCTAssertNil(RegisteredInstallerHelperParentReader.parse( + valid.replacingOccurrences(of: "0.3.5", with: "garbage") + )) + XCTAssertNil(RegisteredInstallerHelperParentReader.parse( + valid + "\nparent bundle version = 0.3.5\n" + )) + XCTAssertNil(RegisteredInstallerHelperParentReader.parse( + valid.replacingOccurrences(of: "ServiceManagement", with: "other") + )) + XCTAssertNil(RegisteredInstallerHelperParentReader.parse( + valid.replacingOccurrences(of: "system/\(label)", with: "user/\(label)") + )) + } + + func testNativeParentReadbackIsObservationOnly() { + // CI hosts may have no installer daemon. Either result is an observation; + // the coordinator still rejects nil or a version other than its own. + let observed = RegisteredInstallerHelperParentReader().readVersion() + if let observed { + XCTAssertFalse(observed.description.isEmpty) + } + } } private final class LockedCounter: @unchecked Sendable { @@ -131,13 +181,16 @@ private final class HelperServiceController: private let registrationFails: Bool private var registrations = 0 private var statusReads = 0 + private let parentVersion: InstallerVersion? init( statuses: [ManagedInstallerPrivilegedHelperStatus], - registrationFails: Bool = false + registrationFails: Bool = false, + parentVersion: InstallerVersion? = try! InstallerVersion("0.3.5") ) { self.statuses = statuses self.registrationFails = registrationFails + self.parentVersion = parentVersion } func readStatus() -> ManagedInstallerPrivilegedHelperStatus { @@ -148,6 +201,8 @@ private final class HelperServiceController: } } + func readRegisteredParentVersion() -> InstallerVersion? { parentVersion } + func register() throws { try lock.withLock { registrations += 1