Containerization: Always set TERM (apple#146)

Make sure we always set TERM for containers that ask for a tty. Right
now this handling was spread around in a bunch of spots, but I'd wager
setting it for the client on the host via LinuxContainer/Process is more
sane and already what we do for a lot of the other fields.

Author: dcantah

Package.resolved

@@ -411,12 +411,15 @@ extension LinuxContainer {
         set { config.spec.process!.rlimits = newValue }
     }
 
-    /// Set a pty device as the container's stdio.
+    /// Set a pty device as the container's stdio. This additionally will
+    /// set the TERM=xterm environment variable, and the OCI runtime specs
+    /// `process.terminal` field to true.
     public var terminalDevice: Terminal? {
         get { config.terminal }
         set {
             config.spec.process!.terminal = newValue != nil ? true : false
             config.terminal = newValue
+            config.spec.process!.env.append("TERM=xterm")
             config.ioHandlers.stdin = newValue
             config.ioHandlers.stdout = newValue
             config.ioHandlers.stderr = nil
@@ -426,7 +429,10 @@ extension LinuxContainer {
     /// If the container has a pty allocated.
     public var terminal: Bool {
         get { config.spec.process!.terminal }
-        set { config.spec.process!.terminal = newValue }
+        set {
+            config.spec.process!.terminal = newValue
+            config.spec.process!.env.append("TERM=xterm")
+        }
     }
 
     /// Set the stdin stream for the initial process of the container.

Sources/Containerization/LinuxContainer.swift

@@ -135,7 +135,12 @@ public final class LinuxProcess: Sendable {
     /// be attached to the Process's Standard I/O.
     public var terminal: Bool {
         get { state.withLock { $0.spec.process!.terminal } }
-        set { state.withLock { $0.spec.process!.terminal = newValue } }
+        set {
+            state.withLock {
+                $0.spec.process!.terminal = newValue
+                $0.spec.process!.env.append("TERM=xterm")
+            }
+        }
     }
 
     /// The User a Process should execute under.

Sources/Containerization/LinuxProcess.swift

@@ -17,6 +17,7 @@
 import ArgumentParser
 import Containerization
 import ContainerizationOCI
+import ContainerizationOS
 import Crypto
 import Foundation
 import Logging
@@ -261,6 +262,44 @@ extension IntegrationSuite {
         }
     }
 
+    // Ensure if we ask for a terminal we set TERM.
+    func testProcessTtyEnvvar() async throws {
+        let id = "test-process-tty-envvar"
+
+        let bs = try await bootstrap()
+        let container = LinuxContainer(
+            id,
+            rootfs: bs.rootfs,
+            vmm: bs.vmm
+        )
+        container.arguments = ["env"]
+        container.terminal = true
+
+        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 = "TERM=xterm"
+        guard str.contains(homeEnvvar) else {
+            throw IntegrationError.assert(
+                msg: "process should have TERM environment variable defined")
+        }
+    }
+
     // 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"

Sources/Integration/ProcessTests.swift

@@ -200,6 +200,7 @@ struct IntegrationSuite: AsyncParsableCommand {
             "process user": testProcessUser,
             "process home envvar": testProcessHomeEnvvar,
             "process custom home envvar": testProcessCustomHomeEnvvar,
+            "process tty ensure TERM": testProcessTtyEnvvar,
             "multiple concurrent processes": testMultipleConcurrentProcesses,
             "multiple concurrent processes with output stress": testMultipleConcurrentProcessesOutputStress,
             "container hostname": testHostname,

Sources/Integration/Suite.swift

@@ -100,7 +100,6 @@ extension Application {
 
             container.terminalDevice = current
             container.arguments = arguments
-            container.environment.append("TERM=xterm")
             container.workingDirectory = cwd
 
             for mount in self.mounts {

Sources/cctl/RunCommand.swift

@@ -104,7 +104,6 @@ final class ManagedProcess: Sendable {
             log.info("setting up terminal IO")
             let attrs = Command.Attrs(setsid: false, setctty: false)
             process.attrs = attrs
-            process.environment.append("TERM=xterm")
             io = try TerminalIO(
                 process: &process,
                 stdio: stdio,

vminitd/Package.resolved

@@ -850,6 +850,15 @@ extension Initd {
             process.env.append("HOME=\(parsedUser.home)")
         }
 
+        // Defensive programming a tad, but ensure we have TERM set if
+        // the client requested a pty.
+        if process.terminal {
+            let termEnv = "TERM="
+            if !process.env.contains(where: { $0.hasPrefix(termEnv) }) {
+                process.env.append("TERM=xterm")
+            }
+        }
+
         ociSpec.process = process
     }
 }

vminitd/Sources/vminitd/ManagedProcess.swift

@@ -43,9 +43,12 @@ final class TerminalIO: ManagedProcess.IO & Sendable {
         self.log = log
 
         let ptyHandle = child.handle
-        process.stdin = stdio.stdin != nil ? ptyHandle : nil
+        let useHandles = stdio.stdin != nil || stdio.stdout != nil
+        // We currently set stdin to the controlling terminal always, so
+        // it must be a valid pty descriptor.
+        process.stdin = useHandles ? ptyHandle : nil
 
-        let stdoutHandle = stdio.stdout != nil ? ptyHandle : nil
+        let stdoutHandle = useHandles ? ptyHandle : nil
         process.stdout = stdoutHandle
         process.stderr = stdoutHandle
     }