From 95851ae9da5b2783e7999c01cc95804653b597c5 Mon Sep 17 00:00:00 2001 From: Michael Chavez Date: Fri, 13 Feb 2026 19:59:11 -0800 Subject: [PATCH 1/3] Potential bug fix to issue #1189 --- Package.swift | 1 + .../Container/ContainerConfiguration.swift | 4 + .../ContainerAPIService/Client/Utility.swift | 3 + .../Server/SandboxService.swift | 108 +++++++++++++++++- .../SSHConfigTests.swift | 98 ++++++++++++++++ 5 files changed, 211 insertions(+), 3 deletions(-) create mode 100644 Tests/ContainerResourceTests/SSHConfigTests.swift diff --git a/Package.swift b/Package.swift index 3a24b7971..28e63d15d 100644 --- a/Package.swift +++ b/Package.swift @@ -353,6 +353,7 @@ let package = Package( dependencies: [ .product(name: "Containerization", package: "containerization"), .product(name: "ContainerizationExtras", package: "containerization"), + .product(name: "ContainerizationOCI", package: "containerization"), "ContainerAPIService", "ContainerResource", ] diff --git a/Sources/ContainerResource/Container/ContainerConfiguration.swift b/Sources/ContainerResource/Container/ContainerConfiguration.swift index c4c3da6b9..87c8c0587 100644 --- a/Sources/ContainerResource/Container/ContainerConfiguration.swift +++ b/Sources/ContainerResource/Container/ContainerConfiguration.swift @@ -49,6 +49,8 @@ public struct ContainerConfiguration: Sendable, Codable { public var virtualization: Bool = false /// Enable SSH agent socket forwarding from host to container. public var ssh: Bool = false + /// Optiional preferred SSH agent socket path captured from client-side environment + public var sshAuthSocketPath: String? = nil /// Whether to mount the rootfs as read-only. public var readOnly: Bool = false /// Whether to use a minimal init process inside the container. @@ -71,6 +73,7 @@ public struct ContainerConfiguration: Sendable, Codable { case runtimeHandler case virtualization case ssh + case sshAuthSocketPath case readOnly case useInit } @@ -102,6 +105,7 @@ public struct ContainerConfiguration: Sendable, Codable { runtimeHandler = try container.decodeIfPresent(String.self, forKey: .runtimeHandler) ?? "container-runtime-linux" virtualization = try container.decodeIfPresent(Bool.self, forKey: .virtualization) ?? false ssh = try container.decodeIfPresent(Bool.self, forKey: .ssh) ?? false + sshAuthSocketPath = try container.decodeIfPresent(String.self, forKey: .sshAuthSocketPath) readOnly = try container.decodeIfPresent(Bool.self, forKey: .readOnly) ?? false useInit = try container.decodeIfPresent(Bool.self, forKey: .useInit) ?? false } diff --git a/Sources/Services/ContainerAPIService/Client/Utility.swift b/Sources/Services/ContainerAPIService/Client/Utility.swift index 8ce9086be..f92fc3fd0 100644 --- a/Sources/Services/ContainerAPIService/Client/Utility.swift +++ b/Sources/Services/ContainerAPIService/Client/Utility.swift @@ -248,6 +248,9 @@ public struct Utility { config.publishedSockets = try Parser.publishSockets(management.publishSockets) config.ssh = management.ssh + if management.ssh { + config.sshAuthSocketPath = ProcessInfo.processInfo.environment["SSH_AUTH_SOCK"] + } config.readOnly = management.readOnly config.useInit = management.useInit diff --git a/Sources/Services/ContainerSandboxService/Server/SandboxService.swift b/Sources/Services/ContainerSandboxService/Server/SandboxService.swift index db8acd03c..78aebaa8b 100644 --- a/Sources/Services/ContainerSandboxService/Server/SandboxService.swift +++ b/Sources/Services/ContainerSandboxService/Server/SandboxService.swift @@ -75,13 +75,82 @@ public actor SandboxService { } } - private static func sshAuthSocketHostUrl(config: ContainerConfiguration) -> URL? { - if config.ssh, let sshSocket = Foundation.ProcessInfo.processInfo.environment[Self.sshAuthSocketEnvVar] { - return URL(fileURLWithPath: sshSocket) + private enum SSHAuthSocketSource: String { + case config = "config" + case runtimeEnv = "runtimeEnv" + case launchctl = "launchctl" + } + + private static func isUnixSocket(path: String) -> Bool { + (try? File.info(path).isSocket) ?? false + } + + private static func launchctlSSHAuthSock() -> String? { + let proc = Foundation.Process() + proc.executableURL = URL(fileURLWithPath: "/bin/launchctl") + proc.arguments = ["getenv", Self.sshAuthSocketEnvVar] + + let out = Pipe() + proc.standardOutput = out + proc.standardError = Pipe() + + do { + try proc.run() + proc.waitUntilExit() + guard proc.terminationStatus == 0 else { + return nil + } + let data = out.fileHandleForReading.readDataToEndOfFile() + guard var value = String(data: data, encoding: .utf8) else { + return nil + } + value = value.trimmingCharacters(in: .whitespacesAndNewlines) + return value.isEmpty ? nil : value + } catch { + return nil + } + } + + private static func resolveSSHAuthSocketHostPath(config: ContainerConfiguration) -> (path: String, source: SSHAuthSocketSource)? { + guard config.ssh else { + return nil + } + + if let configuredPath = config.sshAuthSocketPath, + Self.isUnixSocket(path: configuredPath) + { + return (configuredPath, .config) } + + if let envPath = Foundation.ProcessInfo.processInfo.environment[Self.sshAuthSocketEnvVar], + Self.isUnixSocket(path: envPath) + { + return (envPath, .runtimeEnv) + } + + if let launchctlPath = Self.launchctlSSHAuthSock(), + Self.isUnixSocket(path: launchctlPath) + { + return (launchctlPath, .launchctl) + } + return nil } + private static func sshAuthSocketHostUrl(config: ContainerConfiguration) -> URL? { + guard let resolved = Self.resolveSSHAuthSocketHostPath(config: config) else { + return nil + } + return URL(fileURLWithPath: resolved.path) + } + + /// Create an instance with a bundle that describes the container. + /// + /// - Parameters: + /// - root: The file URL for the bundle root. + /// - interfaceStrategy: The strategy for producing network interface + /// objects for each network to which the container attaches. + /// - log: The destination for log messages. public init( root: URL, interfaceStrategies: [NetworkPluginInfo: InterfaceStrategy], @@ -145,6 +214,39 @@ public actor SandboxService { var config = try bundle.configuration + if config.ssh { + let runtimeSshAuthSock = Foundation.ProcessInfo.processInfo.environment[Self.sshAuthSocketEnvVar] + let runtimeSocketIsValid = runtimeSshAuthSock.map { Self.isUnixSocket(path: $0) } ?? false + let configuredSshAuthSock = config.sshAuthSocketPath + let configuredSocketIsValid = configuredSshAuthSock.map { Self.isUnixSocket(path: $0) } ?? false + if let resolved = Self.resolveSSHAuthSocketHostPath(config: config) { + self.log.info( + "ssh agent forwarding requested", + metadata: [ + "hostSocketPath": "\(resolved.path)", + "hostSocketSource": "\(resolved.source.rawValue)", + "hostSocketIsSocket": "true", + "guestSocketPath": "\(Self.sshAuthSocketGuestPath)", + "configuredSocketPath": "\(configuredSshAuthSock ?? "")", + "configuredSocketIsValid": "\(configuredSocketIsValid)", + "runtimeEnvSocketPath": "\(runtimeSshAuthSock ?? "")", + "runtimeEnvSocketIsValid": "\(runtimeSocketIsValid)", + ] + ) + } else { + self.log.warning( + "ssh agent forwarding requested but no valid SSH_AUTH_SOCK source found", + metadata: [ + "envVar": "\(Self.sshAuthSocketEnvVar)", + "configuredSocketPath": "\(configuredSshAuthSock ?? "")", + "configuredSocketIsValid": "\(configuredSocketIsValid)", + "runtimeEnvSocketPath": "\(runtimeSshAuthSock ?? "")", + "runtimeEnvSocketIsValid": "\(runtimeSocketIsValid)", + ] + ) + } + } + var kernel = try bundle.kernel kernel.commandLine.kernelArgs.append("oops=panic") kernel.commandLine.kernelArgs.append("lsm=lockdown,capability,landlock,yama,apparmor") diff --git a/Tests/ContainerResourceTests/SSHConfigTests.swift b/Tests/ContainerResourceTests/SSHConfigTests.swift new file mode 100644 index 000000000..7ad85f5e4 --- /dev/null +++ b/Tests/ContainerResourceTests/SSHConfigTests.swift @@ -0,0 +1,98 @@ +//===----------------------------------------------------------------------===// +// 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 ContainerizationOCI +import Foundation +import Testing + +@testable import ContainerResource + +/// Unit tests for SSH agent forwarding configuration (`ssh`, `sshAuthSocketPath`). +/// Ensures the config model correctly persists and restores the caller's socket path +/// so runtime resolution (config → runtime env → launchctl) is not regressed. +struct SSHConfigTests { + + @Test("SSH config round-trip: ssh and sshAuthSocketPath are preserved after encode/decode") + func sshConfigRoundTripPreservesSocketPath() throws { + var config = makeMinimalConfig() + config.ssh = true + config.sshAuthSocketPath = "/path/to/agent.sock" + + let encoded = try JSONEncoder().encode(config) + let decoded = try JSONDecoder().decode(ContainerConfiguration.self, from: encoded) + + #expect(decoded.ssh == true) + #expect(decoded.sshAuthSocketPath == "/path/to/agent.sock") + } + + @Test("SSH config round-trip: ssh false and nil path are preserved") + func sshConfigRoundTripPreservesFalseAndNil() throws { + let config = makeMinimalConfig() + #expect(config.ssh == false) + #expect(config.sshAuthSocketPath == nil) + + let encoded = try JSONEncoder().encode(config) + let decoded = try JSONDecoder().decode(ContainerConfiguration.self, from: encoded) + + #expect(decoded.ssh == false) + #expect(decoded.sshAuthSocketPath == nil) + } + + @Test("SSH config decode: missing ssh and sshAuthSocketPath default to false and nil") + func sshConfigDecodeDefaults() throws { + let minimalJSON = """ + { + "id": "test", + "image": {"reference": "alpine", "descriptor": {"digest": "sha256:test", "mediaType": "application/vnd.oci.image.manifest.v1+json", "size": 0}}, + "initProcess": { + "executable": "/bin/sh", + "arguments": [], + "environment": [], + "workingDirectory": "/", + "terminal": false, + "user": {"id": {"uid": 0, "gid": 0}}, + "supplementalGroups": [], + "rlimits": [] + } + } + """ + let data = minimalJSON.data(using: .utf8)! + let decoded = try JSONDecoder().decode(ContainerConfiguration.self, from: data) + + #expect(decoded.ssh == false) + #expect(decoded.sshAuthSocketPath == nil) + } + + private func makeMinimalConfig() -> ContainerConfiguration { + let descriptor = Descriptor( + mediaType: "application/vnd.oci.image.manifest.v1+json", + digest: "sha256:test", + size: 0 + ) + let image = ImageDescription(reference: "alpine", descriptor: descriptor) + let process = ProcessConfiguration( + executable: "/bin/sh", + arguments: [], + environment: [], + workingDirectory: "/", + terminal: false, + user: .id(uid: 0, gid: 0), + supplementalGroups: [], + rlimits: [] + ) + return ContainerConfiguration(id: "test", image: image, process: process) + } +} From a9f33a9da91f576a70ca60eb04dde95bddee8675 Mon Sep 17 00:00:00 2001 From: Michael Chavez Date: Thu, 26 Feb 2026 16:59:49 -0800 Subject: [PATCH 2/3] Pass SSH_AUTH_SOCK at bootstrap instead of in container config --- .../Container/ContainerRun.swift | 3 +- .../Container/ContainerStart.swift | 3 +- .../Container/ContainerConfiguration.swift | 4 - .../Client/ContainerClient.swift | 6 +- .../ContainerAPIService/Client/Utility.swift | 3 - .../ContainerAPIService/Client/XPC+.swift | 3 + .../Server/Containers/ContainersHarness.swift | 3 +- .../Client/SandboxClient.swift | 4 + .../Server/SandboxService.swift | 92 +++++-------------- .../SSHConfigTests.swift | 20 ++-- 10 files changed, 50 insertions(+), 91 deletions(-) diff --git a/Sources/ContainerCommands/Container/ContainerRun.swift b/Sources/ContainerCommands/Container/ContainerRun.swift index c83fbf790..146f754ba 100644 --- a/Sources/ContainerCommands/Container/ContainerRun.swift +++ b/Sources/ContainerCommands/Container/ContainerRun.swift @@ -128,7 +128,8 @@ extension Application { try? io.close() } - let process = try await client.bootstrap(id: id, stdio: io.stdio) + let sshAuthSocketPath = ck.0.ssh ? ProcessInfo.processInfo.environment["SSH_AUTH_SOCK"] : nil + let process = try await client.bootstrap(id: id, stdio: io.stdio, sshAuthSocketPath: sshAuthSocketPath) progress.finish() if !self.managementFlags.cidfile.isEmpty { diff --git a/Sources/ContainerCommands/Container/ContainerStart.swift b/Sources/ContainerCommands/Container/ContainerStart.swift index 4067df8cf..1418b75f3 100644 --- a/Sources/ContainerCommands/Container/ContainerStart.swift +++ b/Sources/ContainerCommands/Container/ContainerStart.swift @@ -87,7 +87,8 @@ extension Application { try? io.close() } - let process = try await client.bootstrap(id: container.id, stdio: io.stdio) + let sshAuthSocketPath = container.configuration.ssh ? ProcessInfo.processInfo.environment["SSH_AUTH_SOCK"] : nil + let process = try await client.bootstrap(id: container.id, stdio: io.stdio, sshAuthSocketPath: sshAuthSocketPath) progress.finish() if detach { diff --git a/Sources/ContainerResource/Container/ContainerConfiguration.swift b/Sources/ContainerResource/Container/ContainerConfiguration.swift index 87c8c0587..c4c3da6b9 100644 --- a/Sources/ContainerResource/Container/ContainerConfiguration.swift +++ b/Sources/ContainerResource/Container/ContainerConfiguration.swift @@ -49,8 +49,6 @@ public struct ContainerConfiguration: Sendable, Codable { public var virtualization: Bool = false /// Enable SSH agent socket forwarding from host to container. public var ssh: Bool = false - /// Optiional preferred SSH agent socket path captured from client-side environment - public var sshAuthSocketPath: String? = nil /// Whether to mount the rootfs as read-only. public var readOnly: Bool = false /// Whether to use a minimal init process inside the container. @@ -73,7 +71,6 @@ public struct ContainerConfiguration: Sendable, Codable { case runtimeHandler case virtualization case ssh - case sshAuthSocketPath case readOnly case useInit } @@ -105,7 +102,6 @@ public struct ContainerConfiguration: Sendable, Codable { runtimeHandler = try container.decodeIfPresent(String.self, forKey: .runtimeHandler) ?? "container-runtime-linux" virtualization = try container.decodeIfPresent(Bool.self, forKey: .virtualization) ?? false ssh = try container.decodeIfPresent(Bool.self, forKey: .ssh) ?? false - sshAuthSocketPath = try container.decodeIfPresent(String.self, forKey: .sshAuthSocketPath) readOnly = try container.decodeIfPresent(Bool.self, forKey: .readOnly) ?? false useInit = try container.decodeIfPresent(Bool.self, forKey: .useInit) ?? false } diff --git a/Sources/Services/ContainerAPIService/Client/ContainerClient.swift b/Sources/Services/ContainerAPIService/Client/ContainerClient.swift index ec50d91c0..9fa212429 100644 --- a/Sources/Services/ContainerAPIService/Client/ContainerClient.swift +++ b/Sources/Services/ContainerAPIService/Client/ContainerClient.swift @@ -113,7 +113,8 @@ public struct ContainerClient: Sendable { } /// Bootstrap the container's init process. - public func bootstrap(id: String, stdio: [FileHandle?]) async throws -> ClientProcess { + /// - Parameter sshAuthSocketPath: Optional path to the current shell's SSH agent socket, supplied at bootstrap time when SSH forwarding is enabled. + public func bootstrap(id: String, stdio: [FileHandle?], sshAuthSocketPath: String? = nil) async throws -> ClientProcess { let request = XPCMessage(route: .containerBootstrap) for (i, h) in stdio.enumerated() { @@ -134,6 +135,9 @@ public struct ContainerClient: Sendable { do { request.set(key: .id, value: id) + if let sshAuthSocketPath { + request.set(key: .sshAuthSocketPath, value: sshAuthSocketPath) + } try await xpcClient.send(request) return ClientProcessImpl(containerId: id, xpcClient: xpcClient) } catch { diff --git a/Sources/Services/ContainerAPIService/Client/Utility.swift b/Sources/Services/ContainerAPIService/Client/Utility.swift index f92fc3fd0..8ce9086be 100644 --- a/Sources/Services/ContainerAPIService/Client/Utility.swift +++ b/Sources/Services/ContainerAPIService/Client/Utility.swift @@ -248,9 +248,6 @@ public struct Utility { config.publishedSockets = try Parser.publishSockets(management.publishSockets) config.ssh = management.ssh - if management.ssh { - config.sshAuthSocketPath = ProcessInfo.processInfo.environment["SSH_AUTH_SOCK"] - } config.readOnly = management.readOnly config.useInit = management.useInit diff --git a/Sources/Services/ContainerAPIService/Client/XPC+.swift b/Sources/Services/ContainerAPIService/Client/XPC+.swift index 495a227fd..b25f11db9 100644 --- a/Sources/Services/ContainerAPIService/Client/XPC+.swift +++ b/Sources/Services/ContainerAPIService/Client/XPC+.swift @@ -112,6 +112,9 @@ public enum XPCKeys: String { /// Init image reference case initImage + /// SSH agent socket path supplied at bootstrap time (current client shell). + case sshAuthSocketPath + /// Volume case volume case volumes diff --git a/Sources/Services/ContainerAPIService/Server/Containers/ContainersHarness.swift b/Sources/Services/ContainerAPIService/Server/Containers/ContainersHarness.swift index c315c3e08..11831ceef 100644 --- a/Sources/Services/ContainerAPIService/Server/Containers/ContainersHarness.swift +++ b/Sources/Services/ContainerAPIService/Server/Containers/ContainersHarness.swift @@ -55,7 +55,8 @@ public struct ContainersHarness: Sendable { ) } let stdio = message.stdio() - try await service.bootstrap(id: id, stdio: stdio) + let sshAuthSocketPath = message.string(key: .sshAuthSocketPath) + try await service.bootstrap(id: id, stdio: stdio, sshAuthSocketPath: sshAuthSocketPath) return message.reply() } diff --git a/Sources/Services/ContainerSandboxService/Client/SandboxClient.swift b/Sources/Services/ContainerSandboxService/Client/SandboxClient.swift index 6f1cdd2d8..c5ee49dae 100644 --- a/Sources/Services/ContainerSandboxService/Client/SandboxClient.swift +++ b/Sources/Services/ContainerSandboxService/Client/SandboxClient.swift @@ -96,6 +96,10 @@ extension SandboxClient { } } + if let sshAuthSocketPath { + request.set(key: SandboxKeys.sshAuthSocketPath.rawValue, value: sshAuthSocketPath) + } + do { try request.setAllocatedAttachments(allocatedAttachments) try await self.client.send(request) diff --git a/Sources/Services/ContainerSandboxService/Server/SandboxService.swift b/Sources/Services/ContainerSandboxService/Server/SandboxService.swift index 78aebaa8b..e4ddf32c0 100644 --- a/Sources/Services/ContainerSandboxService/Server/SandboxService.swift +++ b/Sources/Services/ContainerSandboxService/Server/SandboxService.swift @@ -76,69 +76,33 @@ public actor SandboxService { } private enum SSHAuthSocketSource: String { - case config = "config" - case runtimeEnv = "runtimeEnv" - case launchctl = "launchctl" + /// Path supplied by client at bootstrap time (current shell's SSH_AUTH_SOCK). + case bootstrap = "bootstrap" } private static func isUnixSocket(path: String) -> Bool { (try? File.info(path).isSocket) ?? false } - private static func launchctlSSHAuthSock() -> String? { - let proc = Foundation.Process() - proc.executableURL = URL(fileURLWithPath: "/bin/launchctl") - proc.arguments = ["getenv", Self.sshAuthSocketEnvVar] - - let out = Pipe() - proc.standardOutput = out - proc.standardError = Pipe() - - do { - try proc.run() - proc.waitUntilExit() - guard proc.terminationStatus == 0 else { - return nil - } - let data = out.fileHandleForReading.readDataToEndOfFile() - guard var value = String(data: data, encoding: .utf8) else { - return nil - } - value = value.trimmingCharacters(in: .whitespacesAndNewlines) - return value.isEmpty ? nil : value - } catch { - return nil - } - } - - private static func resolveSSHAuthSocketHostPath(config: ContainerConfiguration) -> (path: String, source: SSHAuthSocketSource)? { + /// Resolves the host path for SSH agent socket forwarding. Uses only the path passed by the + /// client at bootstrap time; the sandbox does not read SSH_AUTH_SOCK from its own environment + /// or launchctl (the client is responsible for passing the current value). + private static func resolveSSHAuthSocketHostPath(config: ContainerConfiguration, bootstrapOverridePath: String? = nil) -> (path: String, source: SSHAuthSocketSource)? { guard config.ssh else { return nil } - if let configuredPath = config.sshAuthSocketPath, - Self.isUnixSocket(path: configuredPath) - { - return (configuredPath, .config) - } - - if let envPath = Foundation.ProcessInfo.processInfo.environment[Self.sshAuthSocketEnvVar], - Self.isUnixSocket(path: envPath) - { - return (envPath, .runtimeEnv) - } - - if let launchctlPath = Self.launchctlSSHAuthSock(), - Self.isUnixSocket(path: launchctlPath) + if let bootstrapOverridePath, + Self.isUnixSocket(path: bootstrapOverridePath) { - return (launchctlPath, .launchctl) + return (bootstrapOverridePath, .bootstrap) } return nil } - private static func sshAuthSocketHostUrl(config: ContainerConfiguration) -> URL? { - guard let resolved = Self.resolveSSHAuthSocketHostPath(config: config) else { + private static func sshAuthSocketHostUrl(config: ContainerConfiguration, bootstrapOverridePath: String? = nil) -> URL? { + guard let resolved = Self.resolveSSHAuthSocketHostPath(config: config, bootstrapOverridePath: bootstrapOverridePath) else { return nil } return URL(fileURLWithPath: resolved.path) @@ -213,24 +177,19 @@ public actor SandboxService { try bundle.createLogFile() var config = try bundle.configuration + // Extract sshAuthSocketPath from the XPC request; if present and config.ssh is true, + // we mount the socket and set SSH_AUTH_SOCK in the process environment (replacing the + // previous behavior that used the sandbox's launch env). + let bootstrapSshAuthPath = message.string(key: SandboxKeys.sshAuthSocketPath.rawValue) if config.ssh { - let runtimeSshAuthSock = Foundation.ProcessInfo.processInfo.environment[Self.sshAuthSocketEnvVar] - let runtimeSocketIsValid = runtimeSshAuthSock.map { Self.isUnixSocket(path: $0) } ?? false - let configuredSshAuthSock = config.sshAuthSocketPath - let configuredSocketIsValid = configuredSshAuthSock.map { Self.isUnixSocket(path: $0) } ?? false - if let resolved = Self.resolveSSHAuthSocketHostPath(config: config) { + if let resolved = Self.resolveSSHAuthSocketHostPath(config: config, bootstrapOverridePath: bootstrapSshAuthPath) { self.log.info( "ssh agent forwarding requested", metadata: [ "hostSocketPath": "\(resolved.path)", "hostSocketSource": "\(resolved.source.rawValue)", - "hostSocketIsSocket": "true", "guestSocketPath": "\(Self.sshAuthSocketGuestPath)", - "configuredSocketPath": "\(configuredSshAuthSock ?? "")", - "configuredSocketIsValid": "\(configuredSocketIsValid)", - "runtimeEnvSocketPath": "\(runtimeSshAuthSock ?? "")", - "runtimeEnvSocketIsValid": "\(runtimeSocketIsValid)", ] ) } else { @@ -238,10 +197,7 @@ public actor SandboxService { "ssh agent forwarding requested but no valid SSH_AUTH_SOCK source found", metadata: [ "envVar": "\(Self.sshAuthSocketEnvVar)", - "configuredSocketPath": "\(configuredSshAuthSock ?? "")", - "configuredSocketIsValid": "\(configuredSocketIsValid)", - "runtimeEnvSocketPath": "\(runtimeSshAuthSock ?? "")", - "runtimeEnvSocketIsValid": "\(runtimeSocketIsValid)", + "bootstrapPath": "\(bootstrapSshAuthPath ?? "")", ] ) } @@ -317,7 +273,7 @@ public actor SandboxService { let id = config.id let rootfs = try bundle.containerRootfs.asMount let container = try LinuxContainer(id, rootfs: rootfs, vmm: vmm, logger: self.log) { czConfig in - try Self.configureContainer(czConfig: &czConfig, config: config) + try Self.configureContainer(czConfig: &czConfig, config: config, bootstrapOverridePath: bootstrapSshAuthPath) czConfig.interfaces = interfaces czConfig.process.stdout = stdout czConfig.process.stderr = stderr @@ -942,7 +898,8 @@ public actor SandboxService { private static func configureContainer( czConfig: inout LinuxContainer.Configuration, - config: ContainerConfiguration + config: ContainerConfiguration, + bootstrapOverridePath: String? = nil ) throws { czConfig.cpus = config.resources.cpus czConfig.memoryInBytes = config.resources.memoryInBytes @@ -975,7 +932,7 @@ public actor SandboxService { czConfig.sockets.append(socketConfig) } - if let socketUrl = Self.sshAuthSocketHostUrl(config: config) { + if let socketUrl = Self.sshAuthSocketHostUrl(config: config, bootstrapOverridePath: bootstrapOverridePath) { let socketPath = socketUrl.path(percentEncoded: false) let attrs = try? FileManager.default.attributesOfItem(atPath: socketPath) let permissions = (attrs?[.posixPermissions] as? NSNumber) @@ -1001,7 +958,7 @@ public actor SandboxService { searchDomains: dns.searchDomains, options: dns.options) } - try Self.configureInitialProcess(czConfig: &czConfig, config: config) + try Self.configureInitialProcess(czConfig: &czConfig, config: config, bootstrapOverridePath: bootstrapOverridePath) } private func getDefaultNameservers(allocatedAttachments: [AllocatedAttachment]) async throws -> [String] { @@ -1018,14 +975,15 @@ public actor SandboxService { private static func configureInitialProcess( czConfig: inout LinuxContainer.Configuration, - config: ContainerConfiguration + config: ContainerConfiguration, + bootstrapOverridePath: String? = nil ) throws { let process = config.initProcess czConfig.process.arguments = [process.executable] + process.arguments czConfig.process.environmentVariables = process.environment - if Self.sshAuthSocketHostUrl(config: config) != nil { + if Self.sshAuthSocketHostUrl(config: config, bootstrapOverridePath: bootstrapOverridePath) != nil { if !czConfig.process.environmentVariables.contains(where: { $0.starts(with: "\(Self.sshAuthSocketEnvVar)=") }) { czConfig.process.environmentVariables.append("\(Self.sshAuthSocketEnvVar)=\(Self.sshAuthSocketGuestPath)") } diff --git a/Tests/ContainerResourceTests/SSHConfigTests.swift b/Tests/ContainerResourceTests/SSHConfigTests.swift index 7ad85f5e4..1073c3760 100644 --- a/Tests/ContainerResourceTests/SSHConfigTests.swift +++ b/Tests/ContainerResourceTests/SSHConfigTests.swift @@ -20,38 +20,33 @@ import Testing @testable import ContainerResource -/// Unit tests for SSH agent forwarding configuration (`ssh`, `sshAuthSocketPath`). -/// Ensures the config model correctly persists and restores the caller's socket path -/// so runtime resolution (config → runtime env → launchctl) is not regressed. +/// Unit tests for SSH agent forwarding configuration (`ssh`). +/// The SSH agent socket path is supplied at bootstrap time by the client, not stored in config. struct SSHConfigTests { - @Test("SSH config round-trip: ssh and sshAuthSocketPath are preserved after encode/decode") - func sshConfigRoundTripPreservesSocketPath() throws { + @Test("SSH config round-trip: ssh true is preserved after encode/decode") + func sshConfigRoundTripPreservesTrue() throws { var config = makeMinimalConfig() config.ssh = true - config.sshAuthSocketPath = "/path/to/agent.sock" let encoded = try JSONEncoder().encode(config) let decoded = try JSONDecoder().decode(ContainerConfiguration.self, from: encoded) #expect(decoded.ssh == true) - #expect(decoded.sshAuthSocketPath == "/path/to/agent.sock") } - @Test("SSH config round-trip: ssh false and nil path are preserved") - func sshConfigRoundTripPreservesFalseAndNil() throws { + @Test("SSH config round-trip: ssh false is preserved") + func sshConfigRoundTripPreservesFalse() throws { let config = makeMinimalConfig() #expect(config.ssh == false) - #expect(config.sshAuthSocketPath == nil) let encoded = try JSONEncoder().encode(config) let decoded = try JSONDecoder().decode(ContainerConfiguration.self, from: encoded) #expect(decoded.ssh == false) - #expect(decoded.sshAuthSocketPath == nil) } - @Test("SSH config decode: missing ssh and sshAuthSocketPath default to false and nil") + @Test("SSH config decode: missing ssh defaults to false") func sshConfigDecodeDefaults() throws { let minimalJSON = """ { @@ -73,7 +68,6 @@ struct SSHConfigTests { let decoded = try JSONDecoder().decode(ContainerConfiguration.self, from: data) #expect(decoded.ssh == false) - #expect(decoded.sshAuthSocketPath == nil) } private func makeMinimalConfig() -> ContainerConfiguration { From c396115203b1a799d2a9c9753632850110b4e3e7 Mon Sep 17 00:00:00 2001 From: Michael Chavez Date: Fri, 13 Mar 2026 20:42:48 -0700 Subject: [PATCH 3/3] General bootstrap env that overrides with dynamicEnv for SSH forwarding --- Package.swift | 3 +- .../Container/ContainerRun.swift | 7 +- .../Container/ContainerStart.swift | 7 +- .../Client/ContainerClient.swift | 9 ++- .../ContainerAPIService/Client/XPC+.swift | 2 +- .../Server/Containers/ContainersHarness.swift | 9 ++- .../Server/Containers/ContainersService.swift | 5 +- .../Client/SandboxClient.swift | 8 +- .../Client/SandboxKeys.swift | 3 + .../Server/SandboxService.swift | 75 ++++++++++++++++--- .../Subcommands/Run/TestCLIRunLifecycle.swift | 40 ++++++++++ .../DynamicEnvMergeTests.swift | 50 +++++++++++++ 12 files changed, 189 insertions(+), 29 deletions(-) create mode 100644 Tests/ContainerSandboxServiceTests/DynamicEnvMergeTests.swift diff --git a/Package.swift b/Package.swift index 28e63d15d..09d366b76 100644 --- a/Package.swift +++ b/Package.swift @@ -354,8 +354,8 @@ let package = Package( .product(name: "Containerization", package: "containerization"), .product(name: "ContainerizationExtras", package: "containerization"), .product(name: "ContainerizationOCI", package: "containerization"), - "ContainerAPIService", "ContainerResource", + "ContainerAPIService", ] ), .target( @@ -394,6 +394,7 @@ let package = Package( dependencies: [ .product(name: "Containerization", package: "containerization"), "ContainerResource", + "ContainerSandboxService", "ContainerSandboxServiceClient", ] ), diff --git a/Sources/ContainerCommands/Container/ContainerRun.swift b/Sources/ContainerCommands/Container/ContainerRun.swift index 146f754ba..2a586d588 100644 --- a/Sources/ContainerCommands/Container/ContainerRun.swift +++ b/Sources/ContainerCommands/Container/ContainerRun.swift @@ -128,8 +128,11 @@ extension Application { try? io.close() } - let sshAuthSocketPath = ck.0.ssh ? ProcessInfo.processInfo.environment["SSH_AUTH_SOCK"] : nil - let process = try await client.bootstrap(id: id, stdio: io.stdio, sshAuthSocketPath: sshAuthSocketPath) + var dynamicEnv: [String: String] = [:] + if ck.0.ssh, let sshAuthSock = ProcessInfo.processInfo.environment["SSH_AUTH_SOCK"] { + dynamicEnv["SSH_AUTH_SOCK"] = sshAuthSock + } + let process = try await client.bootstrap(id: id, stdio: io.stdio, dynamicEnv: dynamicEnv) progress.finish() if !self.managementFlags.cidfile.isEmpty { diff --git a/Sources/ContainerCommands/Container/ContainerStart.swift b/Sources/ContainerCommands/Container/ContainerStart.swift index 1418b75f3..bfa07a44c 100644 --- a/Sources/ContainerCommands/Container/ContainerStart.swift +++ b/Sources/ContainerCommands/Container/ContainerStart.swift @@ -87,8 +87,11 @@ extension Application { try? io.close() } - let sshAuthSocketPath = container.configuration.ssh ? ProcessInfo.processInfo.environment["SSH_AUTH_SOCK"] : nil - let process = try await client.bootstrap(id: container.id, stdio: io.stdio, sshAuthSocketPath: sshAuthSocketPath) + var dynamicEnv: [String: String] = [:] + if container.configuration.ssh, let sshAuthSock = ProcessInfo.processInfo.environment["SSH_AUTH_SOCK"] { + dynamicEnv["SSH_AUTH_SOCK"] = sshAuthSock + } + let process = try await client.bootstrap(id: container.id, stdio: io.stdio, dynamicEnv: dynamicEnv) progress.finish() if detach { diff --git a/Sources/Services/ContainerAPIService/Client/ContainerClient.swift b/Sources/Services/ContainerAPIService/Client/ContainerClient.swift index 9fa212429..217303d39 100644 --- a/Sources/Services/ContainerAPIService/Client/ContainerClient.swift +++ b/Sources/Services/ContainerAPIService/Client/ContainerClient.swift @@ -113,8 +113,8 @@ public struct ContainerClient: Sendable { } /// Bootstrap the container's init process. - /// - Parameter sshAuthSocketPath: Optional path to the current shell's SSH agent socket, supplied at bootstrap time when SSH forwarding is enabled. - public func bootstrap(id: String, stdio: [FileHandle?], sshAuthSocketPath: String? = nil) async throws -> ClientProcess { + /// - Parameter dynamicEnv: Optional start-time environment overrides passed through bootstrap. + public func bootstrap(id: String, stdio: [FileHandle?], dynamicEnv: [String: String] = [:]) async throws -> ClientProcess { let request = XPCMessage(route: .containerBootstrap) for (i, h) in stdio.enumerated() { @@ -135,8 +135,9 @@ public struct ContainerClient: Sendable { do { request.set(key: .id, value: id) - if let sshAuthSocketPath { - request.set(key: .sshAuthSocketPath, value: sshAuthSocketPath) + if !dynamicEnv.isEmpty { + let encodedDynamicEnv = try JSONEncoder().encode(dynamicEnv) + request.set(key: .dynamicEnv, value: encodedDynamicEnv) } try await xpcClient.send(request) return ClientProcessImpl(containerId: id, xpcClient: xpcClient) diff --git a/Sources/Services/ContainerAPIService/Client/XPC+.swift b/Sources/Services/ContainerAPIService/Client/XPC+.swift index b25f11db9..39a23a138 100644 --- a/Sources/Services/ContainerAPIService/Client/XPC+.swift +++ b/Sources/Services/ContainerAPIService/Client/XPC+.swift @@ -113,7 +113,7 @@ public enum XPCKeys: String { case initImage /// SSH agent socket path supplied at bootstrap time (current client shell). - case sshAuthSocketPath + case dynamicEnv /// Volume case volume diff --git a/Sources/Services/ContainerAPIService/Server/Containers/ContainersHarness.swift b/Sources/Services/ContainerAPIService/Server/Containers/ContainersHarness.swift index 11831ceef..38922430f 100644 --- a/Sources/Services/ContainerAPIService/Server/Containers/ContainersHarness.swift +++ b/Sources/Services/ContainerAPIService/Server/Containers/ContainersHarness.swift @@ -55,8 +55,13 @@ public struct ContainersHarness: Sendable { ) } let stdio = message.stdio() - let sshAuthSocketPath = message.string(key: .sshAuthSocketPath) - try await service.bootstrap(id: id, stdio: stdio, sshAuthSocketPath: sshAuthSocketPath) + let dynamicEnv: [String: String] = + if let data = message.dataNoCopy(key: .dynamicEnv) { + try JSONDecoder().decode([String: String].self, from: data) + } else { + [:] + } + try await service.bootstrap(id: id, stdio: stdio, dynamicEnv: dynamicEnv) return message.reply() } diff --git a/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift b/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift index b266116fb..a4e7660f0 100644 --- a/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift +++ b/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift @@ -398,7 +398,8 @@ public actor ContainersService { } /// Bootstrap the init process of the container. - public func bootstrap(id: String, stdio: [FileHandle?]) async throws { + /// - Parameter dynamicEnv: Optional start-time environment overrides passed from the client. + public func bootstrap(id: String, stdio: [FileHandle?], dynamicEnv: [String: String] = [:]) async throws { log.debug( "ContainersService: enter", metadata: [ @@ -473,7 +474,7 @@ public actor ContainersService { id: id, runtime: runtime ) - try await sandboxClient.bootstrap(stdio: stdio, allocatedAttachments: allocatedAttachments) + try await sandboxClient.bootstrap(stdio: stdio, allocatedAttachments: allocatedAttachments, dynamicEnv: dynamicEnv) try await self.exitMonitor.registerProcess( id: id, diff --git a/Sources/Services/ContainerSandboxService/Client/SandboxClient.swift b/Sources/Services/ContainerSandboxService/Client/SandboxClient.swift index c5ee49dae..125563a27 100644 --- a/Sources/Services/ContainerSandboxService/Client/SandboxClient.swift +++ b/Sources/Services/ContainerSandboxService/Client/SandboxClient.swift @@ -77,7 +77,8 @@ public struct SandboxClient: Sendable { // Runtime Methods extension SandboxClient { - public func bootstrap(stdio: [FileHandle?], allocatedAttachments: [AllocatedAttachment]) async throws { + /// - Parameter dynamicEnv: Optional start-time environment overrides passed from the API service. + public func bootstrap(stdio: [FileHandle?], allocatedAttachments: [AllocatedAttachment], dynamicEnv: [String: String] = [:]) async throws { let request = XPCMessage(route: SandboxRoutes.bootstrap.rawValue) for (i, h) in stdio.enumerated() { @@ -96,8 +97,9 @@ extension SandboxClient { } } - if let sshAuthSocketPath { - request.set(key: SandboxKeys.sshAuthSocketPath.rawValue, value: sshAuthSocketPath) + if !dynamicEnv.isEmpty { + let encodedDynamicEnv = try JSONEncoder().encode(dynamicEnv) + request.set(key: SandboxKeys.dynamicEnv.rawValue, value: encodedDynamicEnv) } do { diff --git a/Sources/Services/ContainerSandboxService/Client/SandboxKeys.swift b/Sources/Services/ContainerSandboxService/Client/SandboxKeys.swift index e207cb049..77f3ddb55 100644 --- a/Sources/Services/ContainerSandboxService/Client/SandboxKeys.swift +++ b/Sources/Services/ContainerSandboxService/Client/SandboxKeys.swift @@ -43,6 +43,9 @@ public enum SandboxKeys: String { /// Container statistics case statistics + /// SSH agent socket path supplied at bootstrap time (current client shell). + case dynamicEnv + /// Network resource keys. case allocatedAttachments case networkAdditionalData diff --git a/Sources/Services/ContainerSandboxService/Server/SandboxService.swift b/Sources/Services/ContainerSandboxService/Server/SandboxService.swift index e4ddf32c0..c9fc9561b 100644 --- a/Sources/Services/ContainerSandboxService/Server/SandboxService.swift +++ b/Sources/Services/ContainerSandboxService/Server/SandboxService.swift @@ -108,6 +108,38 @@ public actor SandboxService { return URL(fileURLWithPath: resolved.path) } + /// Merges start-time environment overrides into a base environment list. + /// Existing keys are replaced in-place and new keys are appended. + /// Made static for unit testability. + public static func mergedEnvironmentVariables(base: [String], overrides: [String: String]) -> [String] { + guard !overrides.isEmpty else { + return base + } + + var env = base + var pending = overrides + + for (index, entry) in env.enumerated() { + guard let separator = entry.firstIndex(of: "=") else { + continue + } + + let key = String(entry[.. [String] { @@ -976,17 +1024,20 @@ public actor SandboxService { private static func configureInitialProcess( czConfig: inout LinuxContainer.Configuration, config: ContainerConfiguration, - bootstrapOverridePath: String? = nil + bootstrapOverridePath: String? = nil, + dynamicEnv: [String: String] = [:] ) throws { let process = config.initProcess czConfig.process.arguments = [process.executable] + process.arguments - czConfig.process.environmentVariables = process.environment + czConfig.process.environmentVariables = Self.mergedEnvironmentVariables( + base: process.environment, + overrides: dynamicEnv + ) if Self.sshAuthSocketHostUrl(config: config, bootstrapOverridePath: bootstrapOverridePath) != nil { - if !czConfig.process.environmentVariables.contains(where: { $0.starts(with: "\(Self.sshAuthSocketEnvVar)=") }) { - czConfig.process.environmentVariables.append("\(Self.sshAuthSocketEnvVar)=\(Self.sshAuthSocketGuestPath)") - } + czConfig.process.environmentVariables.removeAll(where: { $0.starts(with: "\(Self.sshAuthSocketEnvVar)=") }) + czConfig.process.environmentVariables.append("\(Self.sshAuthSocketEnvVar)=\(Self.sshAuthSocketGuestPath)") } czConfig.process.terminal = process.terminal diff --git a/Tests/CLITests/Subcommands/Run/TestCLIRunLifecycle.swift b/Tests/CLITests/Subcommands/Run/TestCLIRunLifecycle.swift index e5448b45b..510dcb6eb 100644 --- a/Tests/CLITests/Subcommands/Run/TestCLIRunLifecycle.swift +++ b/Tests/CLITests/Subcommands/Run/TestCLIRunLifecycle.swift @@ -15,6 +15,7 @@ //===----------------------------------------------------------------------===// import ContainerizationError +import Foundation import Testing class TestCLIRunLifecycle: CLITest { @@ -138,4 +139,43 @@ class TestCLIRunLifecycle: CLITest { ) } } + + @Test func testRunStartWithSSHSetsGuestAuthSockPath() throws { + guard ProcessInfo.processInfo.environment["SSH_AUTH_SOCK"] != nil else { + return + } + + let name = getTestName() + defer { + try? doStop(name: name) + try? doRemove(name: name, force: true) + } + + try doLongRun(name: name, args: ["--ssh"], autoRemove: false) + try waitForContainerRunning(name) + + var output = try doExec(name: name, cmd: ["sh", "-lc", "echo -n ${SSH_AUTH_SOCK:-}"]) + #expect(output.trimmingCharacters(in: .whitespacesAndNewlines) == "/run/host-services/ssh-auth.sock") + + try doStop(name: name) + try doStart(name: name) + try waitForContainerRunning(name) + + output = try doExec(name: name, cmd: ["sh", "-lc", "echo -n ${SSH_AUTH_SOCK:-}"]) + #expect(output.trimmingCharacters(in: .whitespacesAndNewlines) == "/run/host-services/ssh-auth.sock") + } + + @Test func testRunWithoutSSHDoesNotSetGuestAuthSockPath() throws { + let name = getTestName() + defer { + try? doStop(name: name) + try? doRemove(name: name, force: true) + } + + try doLongRun(name: name, autoRemove: false) + try waitForContainerRunning(name) + + let output = try doExec(name: name, cmd: ["sh", "-lc", "echo -n ${SSH_AUTH_SOCK:-}"]) + #expect(output.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty) + } } diff --git a/Tests/ContainerSandboxServiceTests/DynamicEnvMergeTests.swift b/Tests/ContainerSandboxServiceTests/DynamicEnvMergeTests.swift new file mode 100644 index 000000000..1cd9e1548 --- /dev/null +++ b/Tests/ContainerSandboxServiceTests/DynamicEnvMergeTests.swift @@ -0,0 +1,50 @@ +//===----------------------------------------------------------------------===// +// 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 ContainerSandboxService +import Testing + +struct DynamicEnvMergeTests { + @Test + func testDynamicEnvMergeAddsMissingKey() { + let overrides = ["SSH_AUTH_SOCK": "/run/host-services/ssh-auth.sock"] + let base = ["PATH=/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin"] + + let mergedEnv = SandboxService.mergedEnvironmentVariables(base: base, overrides: overrides) + + #expect(mergedEnv.contains("PATH=/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin")) + #expect(mergedEnv.contains("SSH_AUTH_SOCK=/run/host-services/ssh-auth.sock")) + } + + @Test + func testDynamicEnvMergeOverridesExistingKey() { + let overrides = ["FOO": "updated"] + let base = ["FOO=original", "PATH=/usr/bin"] + + let mergedEnv = SandboxService.mergedEnvironmentVariables(base: base, overrides: overrides) + + #expect(mergedEnv.contains("FOO=updated")) + #expect(!mergedEnv.contains("FOO=original")) + #expect(mergedEnv.contains("PATH=/usr/bin")) + } + + @Test + func testDynamicEnvMergeNoOverridesLeavesBaseUnchanged() { + let base = ["FOO=bar", "PATH=/usr/bin"] + let mergedEnv = SandboxService.mergedEnvironmentVariables(base: base, overrides: [:]) + #expect(mergedEnv == base) + } +}