Fix container system status false negative under a Seatbelt sandbox.

[renlord]

Probe with launchctl print <domain>/<label> instead, which launchd does allow a sandboxed process, and key off exit codes: 0 means registered, 113 (ENOSERVICE) means not registered, and anything else is an error rather than being mistranslated into a negative. The probe is advisory in system status, so an indeterminate answer falls through to the XPC health check, whose success is authoritative.

Important

All commits must be signed and verified. Pull requests containing unsigned or unverified commits cannot be built or merged. See the GitHub documentation for instructions.

For all but trivial fixes, make sure to first create a GitHub issue that concisely describes the bug or desired enhancement as justification for the change. Large PRs with no justifying issue will be closed.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update

Motivation and Context

system status gated its output on a launchd registration probe that shelled out to the legacy launchctl list. launchd applies an unconditional anti-sandbox check to the entire legacy domain-IPC family, so inside any Seatbelt sandbox launchctl list exits 1 with empty stdout and stderr -- indistinguishable from "the label is not registered", and not fixable by any sandbox profile. system status therefore reported unregistered and exited 1 while the apiserver was running and answering XPC.

Testing

• Tested locally
• Added/updated tests
• Added/updated docs

[renlord]

Closes #2317

[github-actions]

Code Coverage

Tier	Line Coverage
Unit	25.77%
Integration	66.26%
Combined	75.95%

[jglogan]

@renlord Sorry for the long wait on this one! I had just one comment on the change to the isRegistered API.

[jglogan]

Regarding this API change - could we continue using fullServiceLabel for all this API so that it doesn't diverge from deregister, kickstart, and kill?

Something along the lines of this patch to the current change:

diff --git a/Sources/ContainerCommands/System/SystemStatus.swift b/Sources/ContainerCommands/System/SystemStatus.swift
index ff634957..6e83c0ba 100644
--- a/Sources/ContainerCommands/System/SystemStatus.swift
+++ b/Sources/ContainerCommands/System/SystemStatus.swift
@@ -42,7 +42,8 @@ extension Application {
         public init() {}
 
         public func run() async throws {
-            let isRegistered = try ServiceManager.isRegistered(fullServiceLabel: "\(prefix)apiserver")
+            let domain = try ServiceManager.getDomainString()
+            let isRegistered = (try? ServiceManager.isRegistered(fullServiceLabel: "\(domain)/\(prefix)apiserver")) ?? true
             if !isRegistered {
                 try Output.render(payload: StatusPayload(status: "unregistered"), format: format) {
                     "apiserver is not running and not registered with launchd"
diff --git a/Sources/ContainerPlugin/ServiceManager.swift b/Sources/ContainerPlugin/ServiceManager.swift
index 4d2c3624..8abc1630 100644
--- a/Sources/ContainerPlugin/ServiceManager.swift
+++ b/Sources/ContainerPlugin/ServiceManager.swift
@@ -18,6 +18,10 @@ import ContainerizationError
 import Foundation
 
 public struct ServiceManager {
+    enum LaunchctlStatus {
+        static let noSuchService: Int32 = 113
+    }
+
     private static func runLaunchctlCommand(args: [String]) throws -> Int32 {
         let launchctl = Foundation.Process()
         launchctl.executableURL = URL(fileURLWithPath: "/bin/launchctl")
@@ -94,8 +98,40 @@ public struct ServiceManager {
 
     /// Check if a service has been registered or not.
     public static func isRegistered(fullServiceLabel label: String) throws -> Bool {
-        let exitStatus = try runLaunchctlCommand(args: ["list", label])
-        return exitStatus == 0
+        let result = try runLaunchctlPrint(target: label)
+        return try Self.interpretPrintStatus(result.status, target: label, standardError: result.standardError)
+    }
+
+    private static func runLaunchctlPrint(target: String) throws -> (status: Int32, standardError: String) {
+        let launchctl = Foundation.Process()
+        launchctl.executableURL = URL(fileURLWithPath: "/bin/launchctl")
+        launchctl.arguments = ["print", target]
+
+        let stderrPipe = Pipe()
+        launchctl.standardOutput = FileHandle.nullDevice
+        launchctl.standardError = stderrPipe
+
+        try launchctl.run()
+        let errorData = stderrPipe.fileHandleForReading.readDataToEndOfFile()
+        launchctl.waitUntilExit()
+
+        return (launchctl.terminationStatus, String(decoding: errorData, as: UTF8.self))
+    }
+
+    static func interpretPrintStatus(_ status: Int32, target: String = "", standardError: String = "") throws -> Bool {
+        switch status {
+        case 0:
+            return true
+        case LaunchctlStatus.noSuchService:
+            return false
+        default:
+            var message = "command `launchctl print \(target)` failed with status \(status)"
+            let details = standardError.trimmingCharacters(in: .whitespacesAndNewlines)
+            if !details.isEmpty {
+                message += ", message: \(details)"
+            }
+            throw ContainerizationError(.internalError, message: message)
+        }
     }
 
     private static func getLaunchdSessionType() throws -> String {

Docc for the public APIs could make it more clear what is expected for the calls. Maybe better in the long run would be to create a domain type for service labels so that they're correct by construction.

[jglogan]

@renlord lgtm, thank you for the contribution!