diff --git a/Sources/ContainerCommands/System/SystemStatus.swift b/Sources/ContainerCommands/System/SystemStatus.swift index bfdd76527..984339254 100644 --- a/Sources/ContainerCommands/System/SystemStatus.swift +++ b/Sources/ContainerCommands/System/SystemStatus.swift @@ -71,7 +71,10 @@ extension Application { } public func run() async throws { - let isRegistered = try ServiceManager.isRegistered(fullServiceLabel: "\(prefix)apiserver") + // Use the same launchd domain target as register/stop so a system-domain + // bootstrap is visible to status (plain `launchctl list` is domain-scoped). + let domain = try ServiceManager.getDomainString() + let isRegistered = try ServiceManager.isRegistered(fullServiceLabel: "\(domain)/\(prefix)apiserver") if !isRegistered { try Output.render(payload: PrintableStatus(status: "unregistered"), format: format) { "apiserver is not running and not registered with launchd" diff --git a/Sources/ContainerCommands/System/SystemStop.swift b/Sources/ContainerCommands/System/SystemStop.swift index 164dd90fd..c2e853cae 100644 --- a/Sources/ContainerCommands/System/SystemStop.swift +++ b/Sources/ContainerCommands/System/SystemStop.swift @@ -96,9 +96,12 @@ extension Application { // Note: The assumption here is that we would have registered the launchd services // in the same domain as `launchdDomainString`. This is a fairly sane assumption since // if somehow the launchd domain changed, XPC interactions would not be possible. + // Compare bare labels from `launchctl list` against the apiserver label, then + // re-qualify with the domain used at register time before bootout. + let apiserverLabel = "\(prefix)apiserver" try ServiceManager.enumerate() .filter { $0.hasPrefix(prefix) } - .filter { $0 != fullLabel } + .filter { $0 != apiserverLabel } .map { "\(launchdDomainString)/\($0)" } .forEach { log.info("stopping service", metadata: ["label": "\($0)"]) diff --git a/Sources/ContainerPlugin/ServiceManager.swift b/Sources/ContainerPlugin/ServiceManager.swift index 4d2c36246..9dd567165 100644 --- a/Sources/ContainerPlugin/ServiceManager.swift +++ b/Sources/ContainerPlugin/ServiceManager.swift @@ -18,25 +18,46 @@ import ContainerizationError import Foundation public struct ServiceManager { - private static func runLaunchctlCommand(args: [String]) throws -> Int32 { + private static func runLaunchctlCommand(args: [String]) throws -> (status: Int32, stderr: String) { let launchctl = Foundation.Process() launchctl.executableURL = URL(fileURLWithPath: "/bin/launchctl") launchctl.arguments = args let null = FileHandle.nullDevice + let stderrPipe = Pipe() launchctl.standardOutput = null - launchctl.standardError = null + launchctl.standardError = stderrPipe try launchctl.run() + let stderrData = stderrPipe.fileHandleForReading.readDataToEndOfFile() launchctl.waitUntilExit() - return launchctl.terminationStatus + let stderr = String(data: stderrData, encoding: .utf8) ?? "" + return (launchctl.terminationStatus, stderr) } /// Register a service by providing the path to a plist. public static func register(plistPath: String) throws { let domain = try Self.getDomainString() - _ = try runLaunchctlCommand(args: ["bootstrap", domain, plistPath]) + let command = "launchctl bootstrap \(domain) \(plistPath)" + let (status, stderr) = try runLaunchctlCommand(args: ["bootstrap", domain, plistPath]) + guard status == 0 else { + // `container system start` is idempotent: if the service is already + // bootstrapped, launchctl returns non-zero. Treat that as success. + // Use try? so a launchctl spawn failure does not replace the bootstrap error. + // Query the same domain we bootstrapped into. + if let label = try? launchdLabel(fromPlistAt: plistPath), + (try? isRegistered(fullServiceLabel: "\(domain)/\(label)")) == true + { + return + } + let detail = stderr.trimmingCharacters(in: .whitespacesAndNewlines) + let message = + detail.isEmpty + ? "command `\(command)` failed with status \(status)" + : "command `\(command)` failed with status \(status), message: \(detail)" + throw ContainerizationError(.internalError, message: message) + } } /// Deregister a service by a launchd label. @@ -46,7 +67,7 @@ public struct ServiceManager { /// Deregister a service and pass return status public static func deregister(fullServiceLabel label: String, status: inout Int32) throws { - status = try runLaunchctlCommand(args: ["bootout", label]) + status = try runLaunchctlCommand(args: ["bootout", label]).status } /// Restart a service by a launchd label. @@ -93,8 +114,13 @@ public struct ServiceManager { } /// Check if a service has been registered or not. + /// + /// Prefer a domain-qualified service target (`gui/501/label`, `system/label`) + /// so the lookup agrees with the domain used by `register`. Bare labels still + /// use `launchctl list` for compatibility with existing callers. public static func isRegistered(fullServiceLabel label: String) throws -> Bool { - let exitStatus = try runLaunchctlCommand(args: ["list", label]) + let args = label.contains("/") ? ["print", label] : ["list", label] + let exitStatus = try runLaunchctlCommand(args: args).status return exitStatus == 0 } @@ -122,16 +148,38 @@ public struct ServiceManager { } public static func getDomainString() throws -> String { - let currentSessionType = try getLaunchdSessionType() - switch currentSessionType { + try domainString(sessionType: getLaunchdSessionType(), uid: getuid(), euid: geteuid()) + } + + /// Compute the launchd domain target for the given session and credentials. + /// + /// When running as root outside an Aqua session (for example `sudo` on a CI + /// runner), bootstrap into the `system` domain. `user/0` and `gui/0` are not + /// valid bootstrap targets in that context. + static func domainString(sessionType: String, uid: uid_t, euid: uid_t) throws -> String { + if euid == 0 && sessionType != LaunchPlist.Domain.Aqua.rawValue { + return LaunchPlist.Domain.System.rawValue.lowercased() + } + switch sessionType { case LaunchPlist.Domain.System.rawValue: return LaunchPlist.Domain.System.rawValue.lowercased() case LaunchPlist.Domain.Background.rawValue: - return "user/\(getuid())" + return "user/\(uid)" case LaunchPlist.Domain.Aqua.rawValue: - return "gui/\(getuid())" + return "gui/\(uid)" default: - throw ContainerizationError(.internalError, message: "unsupported session type \(currentSessionType)") + throw ContainerizationError(.internalError, message: "unsupported session type \(sessionType)") + } + } + + private static func launchdLabel(fromPlistAt path: String) throws -> String { + let data = try Data(contentsOf: URL(fileURLWithPath: path)) + let plist = try PropertyListSerialization.propertyList(from: data, format: nil) + guard let dict = plist as? [String: Any], + let label = dict[LaunchPlist.CodingKeys.label.rawValue] as? String + else { + throw ContainerizationError(.internalError, message: "launchd plist at \(path) is missing Label") } + return label } } diff --git a/Tests/ContainerPluginTests/ServiceManagerTests.swift b/Tests/ContainerPluginTests/ServiceManagerTests.swift new file mode 100644 index 000000000..94d55dabd --- /dev/null +++ b/Tests/ContainerPluginTests/ServiceManagerTests.swift @@ -0,0 +1,70 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2025-2026 Apple Inc. and the container project authors. +// +// 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 +// +// https://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 Testing + +@testable import ContainerPlugin + +struct ServiceManagerTests { + @Test + func domainStringAquaUsesGuiDomain() throws { + #expect(try ServiceManager.domainString(sessionType: "Aqua", uid: 501, euid: 501) == "gui/501") + } + + @Test + func domainStringBackgroundUsesUserDomain() throws { + #expect(try ServiceManager.domainString(sessionType: "Background", uid: 501, euid: 501) == "user/501") + } + + @Test + func domainStringSystemSessionUsesSystemDomain() throws { + #expect(try ServiceManager.domainString(sessionType: "System", uid: 0, euid: 0) == "system") + } + + @Test + func domainStringRootOutsideAquaUsesSystemDomain() throws { + #expect(try ServiceManager.domainString(sessionType: "Background", uid: 0, euid: 0) == "system") + #expect(try ServiceManager.domainString(sessionType: "System", uid: 0, euid: 0) == "system") + } + + @Test + func domainStringRootInAquaKeepsGuiDomain() throws { + // Preserve existing Aqua behavior for interactive sudo sessions. + #expect(try ServiceManager.domainString(sessionType: "Aqua", uid: 0, euid: 0) == "gui/0") + } + + @Test + func domainStringUnsupportedSessionThrows() { + #expect(throws: (any Error).self) { + try ServiceManager.domainString(sessionType: "LoginWindow", uid: 501, euid: 501) + } + } + + @Test + func registerSurfacesLaunchctlBootstrapFailure() throws { + let missingPlist = "/tmp/container-issue-2008-missing-\(UUID().uuidString).plist" + do { + try ServiceManager.register(plistPath: missingPlist) + Issue.record("expected register to throw for missing plist") + } catch { + let message = String(describing: error) + #expect(message.contains("launchctl bootstrap")) + #expect(message.contains(missingPlist)) + #expect(message.contains("failed with status")) + } + } +}