From 926db132cec21170c6e63f4a763a5a72707fdd91 Mon Sep 17 00:00:00 2001 From: Majid Feiz Date: Sun, 14 Jun 2026 13:35:55 +0330 Subject: [PATCH 1/2] Fix XPC connection failure when home directory is on external volume MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit launchd cannot register Mach services from plists located on external or removable volumes. When the user's home directory (and therefore the default ApplicationRoot) resides on such a volume, `launchctl bootstrap` silently fails, causing the XPC connection to be reported as invalid. Introduce ServiceRoot, a new root type whose default path resolves to a subdirectory of FileManager.temporaryDirectory — which macOS always places on the internal system volume regardless of where the home directory lives. ServiceRoot follows the same pattern as ApplicationRoot, InstallRoot, and LogRoot: it can be overridden via CONTAINER_SERVICE_ROOT or the new --service-root CLI flag on `container system start`. - Add Sources/ContainerPlugin/ServiceRoot.swift - Write apiserver.plist under serviceRoot instead of appRoot in SystemStart - Propagate CONTAINER_SERVICE_ROOT through the plist environment so the daemon's PluginLoader also writes plugin plists to the system volume - Add serviceRoot parameter to PluginLoader.init (nil = read env var) - Add ServiceRootTests and update PluginLoaderTest to cover the separation Fixes #1721 --- .../System/SystemStart.swift | 19 +++++- Sources/ContainerPlugin/PluginLoader.swift | 19 ++++-- Sources/ContainerPlugin/ServiceRoot.swift | 58 +++++++++++++++++++ .../PluginLoaderTest.swift | 46 +++++++++++++++ .../ContainerPluginTests/RootPathTests.swift | 18 ++++++ 5 files changed, 154 insertions(+), 6 deletions(-) create mode 100644 Sources/ContainerPlugin/ServiceRoot.swift diff --git a/Sources/ContainerCommands/System/SystemStart.swift b/Sources/ContainerCommands/System/SystemStart.swift index 4b3a96243..2cf347272 100644 --- a/Sources/ContainerCommands/System/SystemStart.swift +++ b/Sources/ContainerCommands/System/SystemStart.swift @@ -50,6 +50,12 @@ extension Application { transform: { FilePath(FileManager.default.currentDirectoryPath).resolve($0, defaultPath: FilePath($0)) }) var logRoot: FilePath? = nil + @Option( + name: .long, + help: "Path to the root directory for launchd service registration files; must reside on an internal volume", + transform: { FilePath(FileManager.default.currentDirectoryPath).resolve($0, defaultPath: FilePath($0)) }) + var serviceRoot = ServiceRoot.defaultPath + @Flag( name: .long, inversion: .prefixedEnableDisable, @@ -109,6 +115,7 @@ extension Application { var env = PluginLoader.filterEnvironment() env[ApplicationRoot.environmentName] = appRoot.string env[InstallRoot.environmentName] = installRoot.string + env[ServiceRoot.environmentName] = serviceRoot.string if let logRoot { env[LogRoot.environmentName] = logRoot.string } @@ -121,8 +128,16 @@ extension Application { machServices: ["com.apple.container.apiserver"] ) - let plistPath = apiServerDataPath.appending(FilePath.Component("apiserver.plist")) - let plistURL = URL(fileURLWithPath: plistPath.string) + // Write the plist under serviceRoot, which must be on an internal system + // volume. launchd cannot register Mach services from plists on external or + // removable volumes, so serviceRoot is kept separate from appRoot. + // CONTAINER_SERVICE_ROOT is propagated to the daemon via the plist + // environment so that plugin registration also uses this root. + let plistDir = serviceRoot + .appending(FilePath.Component("apiserver")) + let plistDirURL = URL(fileURLWithPath: plistDir.string) + try FileManager.default.createDirectory(at: plistDirURL, withIntermediateDirectories: true) + let plistURL = plistDirURL.appendingPathComponent("apiserver.plist") let data = try plist.encode() try data.write(to: plistURL) diff --git a/Sources/ContainerPlugin/PluginLoader.swift b/Sources/ContainerPlugin/PluginLoader.swift index 0c156a5a0..18053a22d 100644 --- a/Sources/ContainerPlugin/PluginLoader.swift +++ b/Sources/ContainerPlugin/PluginLoader.swift @@ -35,21 +35,32 @@ public struct PluginLoader: Sendable { public typealias PluginQualifier = ((Plugin) -> Bool) // A path on disk managed by the PluginLoader, where it stores - // runtime data for loaded plugins. This includes the launchd plists - // and logs files. + // runtime data for loaded plugins. This includes log files. private let pluginResourceRoot: URL + // A path on the system (internal) volume where launchd plists are written. + // Derived from serviceRoot, which is always on an internal volume, so that + // plist registration succeeds even when appRoot is on an external or + // removable volume — launchd cannot bootstrap Mach services from plists on + // such volumes. + private let plistResourceRoot: URL + public init( appRoot: URL, installRoot: URL, logRoot: FilePath?, pluginDirectories: [URL], pluginFactories: [PluginFactory], - log: Logger? = nil + log: Logger? = nil, + serviceRoot: URL? = nil ) throws { let pluginResourceRoot = appRoot.appendingPathComponent("plugin-state") try FileManager.default.createDirectory(at: pluginResourceRoot, withIntermediateDirectories: true) self.pluginResourceRoot = pluginResourceRoot + let resolvedServiceRoot = serviceRoot ?? URL(fileURLWithPath: ServiceRoot.pathname, isDirectory: true) + let plistResourceRoot = resolvedServiceRoot.appendingPathComponent("plugin-state", isDirectory: true) + try FileManager.default.createDirectory(at: plistResourceRoot, withIntermediateDirectories: true) + self.plistResourceRoot = plistResourceRoot self.appRoot = appRoot self.installRoot = installRoot self.logRoot = logRoot @@ -222,7 +233,7 @@ extension PluginLoader { let id = plugin.getLaunchdLabel(instanceId: instanceId) log?.info("Registering plugin", metadata: ["id": "\(id)"]) - let rootURL = pluginStateRoot ?? self.pluginResourceRoot.appending(path: plugin.name) + let rootURL = pluginStateRoot ?? self.plistResourceRoot.appending(path: plugin.name) let resourceURL = plugin.resourceURL try FileManager.default.createDirectory(at: rootURL, withIntermediateDirectories: true) diff --git a/Sources/ContainerPlugin/ServiceRoot.swift b/Sources/ContainerPlugin/ServiceRoot.swift new file mode 100644 index 000000000..37fe9d9d4 --- /dev/null +++ b/Sources/ContainerPlugin/ServiceRoot.swift @@ -0,0 +1,58 @@ +//===----------------------------------------------------------------------===// +// Copyright © 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 SystemPackage + +/// Provides the root path for launchd service registration files (plists). +/// +/// This root is intentionally separate from ``ApplicationRoot`` so that service +/// registration succeeds even when the application data directory is on an +/// external or removable volume. launchd cannot register Mach services from +/// plists on such volumes, so registration files must always reside on the +/// internal system volume. +/// +/// The default path resolves to a `com.apple.container` subdirectory under the +/// per-user temporary directory, which macOS always places on the internal +/// system volume regardless of where the user's home directory lives. +public struct ServiceRoot { + /// The environment variable that, if set, determines the root directory for + /// launchd service registration files. + public static let environmentName = "CONTAINER_SERVICE_ROOT" + + /// The default root directory used when ``environmentName`` is not set: + /// a `com.apple.container` subdirectory under the user's temporary directory. + /// + /// The temporary directory (`NSTemporaryDirectory()`) is always located on + /// the internal system volume on macOS, making it safe for launchd plist + /// registration even when ``ApplicationRoot`` is on an external volume. + public static let defaultPath = FilePath( + FileManager.default.temporaryDirectory.path(percentEncoded: false) + ).appending(FilePath.Component("com.apple.container")) + + /// The resolved root directory path, always lexically normalized. + /// + /// If the environment variable is set to an absolute path, that path is used directly. + /// If it is set to a relative path, the path is resolved against the working directory. + /// Otherwise, ``defaultPath`` is used. + public static let path = FilePath(FileManager.default.currentDirectoryPath).resolve( + ProcessInfo.processInfo.environment[environmentName], + defaultPath: defaultPath + ) + + /// The pathname to the root directory. + public static let pathname = path.string +} diff --git a/Tests/ContainerPluginTests/PluginLoaderTest.swift b/Tests/ContainerPluginTests/PluginLoaderTest.swift index 6433a36a5..fe5ef6f96 100644 --- a/Tests/ContainerPluginTests/PluginLoaderTest.swift +++ b/Tests/ContainerPluginTests/PluginLoaderTest.swift @@ -257,6 +257,52 @@ struct PluginLoaderTest { #expect(!programArguments.contains("--debug")) } + // MARK: - External volume plist placement tests + + /// When serviceRoot is separate from appRoot, the plist must land under + /// serviceRoot and NOT under appRoot — which might be on an external or + /// removable volume where launchd cannot register Mach services. + @Test + func testRegisterWithLaunchdPlistNotUnderAppRoot() async throws { + let appRootURL = FileManager.default.temporaryDirectory + .appendingPathComponent("app-root-\(UUID().uuidString)", isDirectory: true) + defer { try? FileManager.default.removeItem(at: appRootURL) } + + // serviceRoot is distinct from appRoot, simulating the case where appRoot + // is on an external volume but serviceRoot is on the internal system volume. + let serviceRootURL = FileManager.default.temporaryDirectory + .appendingPathComponent("service-root-\(UUID().uuidString)", isDirectory: true) + defer { try? FileManager.default.removeItem(at: serviceRootURL) } + + let factory = try setupMock(tempURL: appRootURL) + let loader = try PluginLoader( + appRoot: appRootURL, + installRoot: URL(filePath: "/usr/local/"), + logRoot: nil, + pluginDirectories: [appRootURL], + pluginFactories: [factory], + serviceRoot: serviceRootURL + ) + + let plugin = loader.findPlugin(name: "service")! + try loader.registerWithLaunchd(plugin: plugin) + + let expectedPlistURL = serviceRootURL + .appendingPathComponent("plugin-state", isDirectory: true) + .appending(path: plugin.name) + .appendingPathComponent("service.plist") + + // Plist exists under serviceRoot (internal volume) + #expect(FileManager.default.fileExists(atPath: expectedPlistURL.path)) + + // Plist must NOT exist under appRoot (which could be an external volume) + let appRootPlistURL = appRootURL + .appendingPathComponent("plugin-state", isDirectory: true) + .appending(path: plugin.name) + .appendingPathComponent("service.plist") + #expect(!FileManager.default.fileExists(atPath: appRootPlistURL.path)) + } + private func setupMock(tempURL: URL) throws -> MockPluginFactory { let cliConfig = PluginConfig(abstract: "cli", author: "CLI", servicesConfig: nil) let cliPlugin: Plugin = Plugin(binaryURL: URL(filePath: "/bin/cli"), config: cliConfig) diff --git a/Tests/ContainerPluginTests/RootPathTests.swift b/Tests/ContainerPluginTests/RootPathTests.swift index 8efcae926..aadc605b5 100644 --- a/Tests/ContainerPluginTests/RootPathTests.swift +++ b/Tests/ContainerPluginTests/RootPathTests.swift @@ -14,6 +14,7 @@ // limitations under the License. //===----------------------------------------------------------------------===// +import Foundation import SystemPackage import Testing @@ -45,3 +46,20 @@ struct LogRootTests { #expect(LogRoot.path == nil) } } + +struct ServiceRootTests { + @Test func defaultPathIsAbsolute() { + #expect(ServiceRoot.defaultPath.isAbsolute) + } + + @Test func defaultPathEndsWithContainerComponent() { + #expect(ServiceRoot.defaultPath.lastComponent?.string == "com.apple.container") + } + + @Test func defaultPathIsUnderTemporaryDirectory() { + let tmpDir = FilePath(FileManager.default.temporaryDirectory.path(percentEncoded: false)) + .lexicallyNormalized() + let serviceRoot = ServiceRoot.defaultPath.lexicallyNormalized() + #expect(serviceRoot.string.hasPrefix(tmpDir.string)) + } +} From 62b732031b3f092a71d40a8505ae60fbf7a28765 Mon Sep 17 00:00:00 2001 From: Majid Feiz Date: Mon, 15 Jun 2026 13:12:34 +0330 Subject: [PATCH 2/2] Address review feedback: defensive check and clearer plist placement comment - ServiceRoot.defaultPath: convert to a closure-based static let so a precondition fires if NSTemporaryDirectory() ever returns an empty path, making the safety assumption explicit and catchable at startup rather than silently producing a broken path. - SystemStart: expand the comment around plistDir construction to make the intentional appRoot/serviceRoot separation explicit, noting both why the split exists (external-volume restriction) and how CONTAINER_SERVICE_ROOT propagates to the daemon without additional wiring. --- .../ContainerCommands/System/SystemStart.swift | 13 ++++++++----- Sources/ContainerPlugin/ServiceRoot.swift | 18 ++++++++++++------ 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/Sources/ContainerCommands/System/SystemStart.swift b/Sources/ContainerCommands/System/SystemStart.swift index 2cf347272..b0c813728 100644 --- a/Sources/ContainerCommands/System/SystemStart.swift +++ b/Sources/ContainerCommands/System/SystemStart.swift @@ -128,11 +128,14 @@ extension Application { machServices: ["com.apple.container.apiserver"] ) - // Write the plist under serviceRoot, which must be on an internal system - // volume. launchd cannot register Mach services from plists on external or - // removable volumes, so serviceRoot is kept separate from appRoot. - // CONTAINER_SERVICE_ROOT is propagated to the daemon via the plist - // environment so that plugin registration also uses this root. + // Write the plist under serviceRoot, intentionally separate from appRoot. + // launchd cannot register Mach services from plists on external or removable + // volumes; keeping service files on the internal system volume (serviceRoot) + // while application data lives in appRoot preserves that guarantee even when + // the user's home directory is on an external volume. + // CONTAINER_SERVICE_ROOT is forwarded into the plist environment so the + // daemon's PluginLoader writes plugin plists to the same root without + // requiring any additional wiring at other call sites. let plistDir = serviceRoot .appending(FilePath.Component("apiserver")) let plistDirURL = URL(fileURLWithPath: plistDir.string) diff --git a/Sources/ContainerPlugin/ServiceRoot.swift b/Sources/ContainerPlugin/ServiceRoot.swift index 37fe9d9d4..f76e4b3cd 100644 --- a/Sources/ContainerPlugin/ServiceRoot.swift +++ b/Sources/ContainerPlugin/ServiceRoot.swift @@ -36,12 +36,18 @@ public struct ServiceRoot { /// The default root directory used when ``environmentName`` is not set: /// a `com.apple.container` subdirectory under the user's temporary directory. /// - /// The temporary directory (`NSTemporaryDirectory()`) is always located on - /// the internal system volume on macOS, making it safe for launchd plist - /// registration even when ``ApplicationRoot`` is on an external volume. - public static let defaultPath = FilePath( - FileManager.default.temporaryDirectory.path(percentEncoded: false) - ).appending(FilePath.Component("com.apple.container")) + /// `NSTemporaryDirectory()` is guaranteed by macOS to reside on the internal + /// system volume (typically `/private/var/folders/…`), making it safe for + /// launchd plist registration even when ``ApplicationRoot`` is on an external + /// or removable volume. + public static let defaultPath: FilePath = { + let tmpPath = FileManager.default.temporaryDirectory.path(percentEncoded: false) + precondition( + !tmpPath.isEmpty, + "NSTemporaryDirectory() returned an empty path; cannot determine a safe location for launchd plists" + ) + return FilePath(tmpPath).appending(FilePath.Component("com.apple.container")) + }() /// The resolved root directory path, always lexically normalized. ///