From b39f132510197ea58a35d0a66b58f410d0fbd21f Mon Sep 17 00:00:00 2001 From: Danny Canter Date: Thu, 12 Jun 2025 07:19:31 -0700 Subject: [PATCH] Vminitd: Always parse /etc/passwd (#104) Fixes #103 (and should fix https://github.com/apple/container/issues/108) Today we only parse if the username field of the runtime spec is set, but this misses if someone just supplied the integer uid and gid fields only. --- Sources/Integration/ProcessTests.swift | 38 +++++++++++++++++++++++ Sources/Integration/Suite.swift | 1 + Sources/cctl/RunCommand.swift | 5 +-- vminitd/Sources/vminitd/Server+GRPC.swift | 30 ++++++++++-------- 4 files changed, 57 insertions(+), 17 deletions(-) diff --git a/Sources/Integration/ProcessTests.swift b/Sources/Integration/ProcessTests.swift index 7b8194f8..d2f60d93 100644 --- a/Sources/Integration/ProcessTests.swift +++ b/Sources/Integration/ProcessTests.swift @@ -251,6 +251,44 @@ extension IntegrationSuite { } } + // Make sure we set HOME by default if we can find it in /etc/passwd in the guest. + func testProcessHomeEnvvar() async throws { + let id = "test-process-home-envvar" + + let bs = try await bootstrap() + let container = LinuxContainer( + id, + rootfs: bs.rootfs, + vmm: bs.vmm + ) + container.arguments = ["env"] + container.user = .init(uid: 0, gid: 0) + + let buffer = BufferWriter() + container.stdout = buffer + + try await container.create() + try await container.start() + + let status = try await container.wait() + try await container.stop() + + guard status == 0 else { + throw IntegrationError.assert(msg: "process status \(status) != 0") + } + + guard let str = String(data: buffer.data, encoding: .utf8) else { + throw IntegrationError.assert( + msg: "failed to convert standard output to a UTF8 string") + } + + let homeEnvvar = "HOME=/root" + guard str.contains(homeEnvvar) else { + throw IntegrationError.assert( + msg: "process should have HOME environment variable defined") + } + } + func testHostname() async throws { let id = "test-container-hostname" diff --git a/Sources/Integration/Suite.swift b/Sources/Integration/Suite.swift index 3a70b2c4..cc49943c 100644 --- a/Sources/Integration/Suite.swift +++ b/Sources/Integration/Suite.swift @@ -198,6 +198,7 @@ struct IntegrationSuite: AsyncParsableCommand { "process false": testProcessFalse, "process echo hi": testProcessEchoHi, "process user": testProcessUser, + "process home envvar": testProcessHomeEnvvar, "multiple concurrent processes": testMultipleConcurrentProcesses, "multiple concurrent processes with output": testMultipleConcurrentProcessesOutput, "container hostname": testHostname, diff --git a/Sources/cctl/RunCommand.swift b/Sources/cctl/RunCommand.swift index 0c2b9ce2..823898b3 100644 --- a/Sources/cctl/RunCommand.swift +++ b/Sources/cctl/RunCommand.swift @@ -100,10 +100,7 @@ extension Application { container.terminalDevice = current container.arguments = arguments - container.environment.append(contentsOf: [ - "HOME=/", - "TERM=xterm", - ]) + container.environment.append("TERM=xterm") container.workingDirectory = cwd for mount in self.mounts { diff --git a/vminitd/Sources/vminitd/Server+GRPC.swift b/vminitd/Sources/vminitd/Server+GRPC.swift index 3af5559c..53c3cc0f 100644 --- a/vminitd/Sources/vminitd/Server+GRPC.swift +++ b/vminitd/Sources/vminitd/Server+GRPC.swift @@ -824,10 +824,16 @@ extension Initd: Com_Apple_Containerization_Sandbox_V3_SandboxContextAsyncProvid extension Initd { func ociAlterations(ociSpec: inout ContainerizationOCI.Spec) throws { guard var process = ociSpec.process else { - throw ContainerizationError(.invalidArgument, message: "runtime spec without process field present") + throw ContainerizationError( + .invalidArgument, + message: "runtime spec without process field present" + ) } guard let root = ociSpec.root else { - throw ContainerizationError(.invalidArgument, message: "runtime spec without root field present") + throw ContainerizationError( + .invalidArgument, + message: "runtime spec without root field present" + ) } try FileManager.default.createDirectory( @@ -839,17 +845,15 @@ extension Initd { process.cwd = "/" } - // This is truthfully a Windows field, but it's fairly common for vm runtimes - // to fill this in as a way to defer username lookup until we hit the guest. - let username = process.user.username - if !username.isEmpty { - let parsedUser = try User.parseUser(root: root.path, userString: username) - process.user.uid = parsedUser.uid - process.user.gid = parsedUser.gid - process.user.additionalGids.append(contentsOf: parsedUser.sgids) - if !process.env.contains("HOME") { - process.env.append("HOME=\(parsedUser.home)") - } + // Username is truthfully a Windows field, but we use this as away to passthrough + // the exact string representation of a username a client may have given us. + let username = process.user.username.isEmpty ? "\(process.user.uid):\(process.user.gid)" : process.user.username + let parsedUser = try User.parseUser(root: root.path, userString: username) + process.user.uid = parsedUser.uid + process.user.gid = parsedUser.gid + process.user.additionalGids = parsedUser.sgids + if !process.env.contains("HOME") { + process.env.append("HOME=\(parsedUser.home)") } ociSpec.process = process }