Stream tar over a file descriptor for cp -, no host staging

The first pass wrapped the existing path based cp: it shelled out to
/usr/bin/tar, unpacked the stdin archive into a host temp directory, then
copied the extracted files in, and did the reverse on the way out. That gave up
the two things tar streaming is for. Ownership and mode came from whatever the
host filesystem allowed rather than from the tar headers, and every entry had
to fit the host's path length limits under a temp directory even when its path
was valid inside the archive.

Hand the descriptor down instead. `cp -` now passes the CLI's stdin or stdout
through both XPC hops on a new archiveFd key, using the same file handle
passing that dial and logs already rely on, to LinuxContainer's stream based
copyIn(archive:)/copyOut(to:). The bytes go straight to the guest over vsock
and the guest extracts them as root, so the ownership, mode and symlink targets
in the headers are applied verbatim and nothing is unpacked on the host. Path
traversal is rejected during extraction, where the archive is actually read,
rather than by pre-scanning entry names on the host.

Each hop closes the descriptor it owns and duplicates for the outgoing message.
Setting a file handle on an XPC message closes the descriptor it is given, and
the container lookups on the receiving side can throw before the handle is ever
forwarded, which otherwise leaked an fd in the apiserver on every copy to a
stopped container.

Behavior now follows docker and podman: output is uncompressed tar named
relative to the source's parent so the source's basename is the top level
entry, for files as well as directories, and input may be uncompressed or
gzip, bzip2 or xz compressed.

The integration tests assert metadata rather than content: a stream carrying
uid, gid and modes the host could not reproduce unprivileged, a traversal entry
that must not escape, entry naming on the way out, and a round trip.

Requires the stream based copy API from apple/containerization#812; the
containerization pin still needs bumping once that lands.

Author: yash-mandaviya

Sources/ContainerCommands/Container/ContainerCopy.swift

@@ -62,41 +62,18 @@ extension Application {
             let srcRef = try Self.parsePathRef(source)
             let dstRef = try Self.parsePathRef(destination)
 
-            if destination == "-" {
-                guard case .container(let id, let path) = srcRef else {
-                    throw ContainerizationError(
-                        .invalidArgument,
-                        message: "when destination is '-', source must be a container reference"
-                    )
-                }
-                guard case .local(let localDash) = dstRef, localDash == "-" else {
-                    throw ContainerizationError(
-                        .invalidArgument,
-                        message: "destination '-' is only supported for container-to-host tar streams"
-                    )
-                }
-                try await Self.streamTarFromContainer(client: client, id: id, sourcePath: path)
-                return
-            }
-
-            if source == "-" {
-                guard case .local(let localDash) = srcRef, localDash == "-" else {
-                    throw ContainerizationError(
-                        .invalidArgument,
-                        message: "source '-' is only supported for host-to-container tar streams"
-                    )
-                }
-                guard case .container(let id, let path) = dstRef else {
-                    throw ContainerizationError(
-                        .invalidArgument,
-                        message: "when source is '-', destination must be a container reference"
-                    )
-                }
-                try await Self.streamTarToContainer(client: client, id: id, destinationPath: path)
-                return
-            }
-
             switch (srcRef, dstRef) {
+            // `-` is an uncompressed tar stream on stdin/stdout, matching
+            // `docker cp` and `podman cp`. The descriptor is handed to the
+            // runtime and forwarded to the guest unmodified: nothing is staged
+            // on the host, so host permission rules and path length limits do
+            // not apply, and the ownership and mode recorded in the tar headers
+            // are what land in the container. Neither case prints, so stdout
+            // carries only archive bytes.
+            case (.container(let id, let path), .local("-")):
+                try await client.copyOut(id: id, source: path, archive: FileHandle.standardOutput)
+            case (.local("-"), .container(let id, let path)):
+                try await client.copyIn(id: id, archive: FileHandle.standardInput, destination: path)
             case (.container(let id, let path), .local(let localPath)):
                 let srcPath = FilePath(path)
                 let destPath = FilePath(URL(fileURLWithPath: localPath, relativeTo: .currentDirectory()).absoluteURL.path(percentEncoded: false))
@@ -151,149 +128,5 @@ extension Application {
                     message: "one of source or destination must be a container reference (container_id:path)")
             }
         }
-
-        private static func streamTarFromContainer(client: ContainerClient, id: String, sourcePath: String) async throws {
-            let tempDir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
-            try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true)
-            defer { try? FileManager.default.removeItem(at: tempDir) }
-
-            let sourceFilePath = FilePath(sourcePath)
-            let fallbackName = "copy"
-            let leafName = sourceFilePath.lastComponent?.string ?? fallbackName
-            let stagingPath = tempDir.appendingPathComponent(leafName)
-
-            try await client.copyOut(id: id, source: sourcePath, destination: stagingPath.path(percentEncoded: false), createParents: true)
-
-            var isDirectory: ObjCBool = false
-            guard FileManager.default.fileExists(atPath: stagingPath.path(percentEncoded: false), isDirectory: &isDirectory) else {
-                throw ContainerizationError(.internalError, message: "failed to stage container copy source for tar streaming")
-            }
-
-            let tarArgs: [String] = ["-C", tempDir.path(percentEncoded: false), "-cf", "-", leafName]
-
-            _ = isDirectory  // kept for parity if future behavior diverges by source type
-
-            try runTar(args: tarArgs, stdinData: nil, outputToStdout: true)
-        }
-
-        private static func streamTarToContainer(client: ContainerClient, id: String, destinationPath: String) async throws {
-            let tempDir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
-            let extractDir = tempDir.appendingPathComponent("extract")
-            let archivePath = tempDir.appendingPathComponent("stdin.tar")
-            try FileManager.default.createDirectory(at: extractDir, withIntermediateDirectories: true)
-            FileManager.default.createFile(atPath: archivePath.path(percentEncoded: false), contents: nil)
-            defer { try? FileManager.default.removeItem(at: tempDir) }
-
-            let archiveHandle = try FileHandle(forWritingTo: archivePath)
-            defer { try? archiveHandle.close() }
-
-            var totalBytesRead = 0
-            while let chunk = try FileHandle.standardInput.read(upToCount: 64 * 1024), !chunk.isEmpty {
-                archiveHandle.write(chunk)
-                totalBytesRead += chunk.count
-            }
-
-            if totalBytesRead == 0 {
-                throw ContainerizationError(.invalidArgument, message: "empty tar stream on stdin")
-            }
-
-            let listed = try runTar(args: ["-tf", archivePath.path(percentEncoded: false)], stdinData: nil, outputToStdout: false)
-            let entries =
-                listed
-                .split(separator: "\n", omittingEmptySubsequences: true)
-                .map(String.init)
-
-            guard !entries.isEmpty else {
-                throw ContainerizationError(.invalidArgument, message: "tar stream has no entries")
-            }
-
-            for entry in entries {
-                let normalized = entry.trimmingCharacters(in: .whitespacesAndNewlines)
-                if normalized.isEmpty {
-                    continue
-                }
-                if normalized.hasPrefix("/") {
-                    throw ContainerizationError(.invalidArgument, message: "tar stream contains absolute path: \(normalized)")
-                }
-                let parts = normalized.split(separator: "/")
-                if parts.contains("..") {
-                    throw ContainerizationError(.invalidArgument, message: "tar stream contains parent traversal: \(normalized)")
-                }
-            }
-
-            _ = try runTar(
-                args: ["-xf", archivePath.path(percentEncoded: false), "-C", extractDir.path(percentEncoded: false)],
-                stdinData: nil,
-                outputToStdout: false
-            )
-
-            let topLevelNames = Set(
-                entries.compactMap { entry -> String? in
-                    let trimmed = entry.trimmingCharacters(in: .whitespacesAndNewlines)
-                    guard !trimmed.isEmpty else { return nil }
-                    return String(trimmed.split(separator: "/", maxSplits: 1).first ?? "")
-                }
-            ).sorted()
-
-            guard !topLevelNames.isEmpty else {
-                throw ContainerizationError(.invalidArgument, message: "tar stream has no copyable top-level entries")
-            }
-
-            if topLevelNames.count == 1 {
-                let name = topLevelNames[0]
-                let src = extractDir.appendingPathComponent(name).path(percentEncoded: false)
-                try await client.copyIn(id: id, source: src, destination: destinationPath, createParents: true)
-                return
-            }
-
-            let destinationDirPath = destinationPath.hasSuffix("/") ? destinationPath : destinationPath + "/"
-            for name in topLevelNames {
-                let src = extractDir.appendingPathComponent(name).path(percentEncoded: false)
-                try await client.copyIn(id: id, source: src, destination: destinationDirPath, createParents: true)
-            }
-        }
-
-        @discardableResult
-        private static func runTar(args: [String], stdinData: Data?, outputToStdout: Bool) throws -> String {
-            let process = Process()
-            process.executableURL = URL(filePath: "/usr/bin/tar")
-            process.arguments = args
-
-            let errPipe = Pipe()
-            process.standardError = errPipe
-            if outputToStdout {
-                process.standardOutput = FileHandle.standardOutput
-            } else {
-                process.standardOutput = Pipe()
-            }
-
-            if let stdinData {
-                let inputPipe = Pipe()
-                process.standardInput = inputPipe
-                try process.run()
-                inputPipe.fileHandleForWriting.write(stdinData)
-                inputPipe.fileHandleForWriting.closeFile()
-            } else {
-                try process.run()
-            }
-
-            process.waitUntilExit()
-
-            let stderrData = errPipe.fileHandleForReading.readDataToEndOfFile()
-            let stderrText = String(decoding: stderrData, as: UTF8.self)
-
-            if process.terminationStatus != 0 {
-                let errorText = stderrText.isEmpty ? "tar failed with status \(process.terminationStatus)" : stderrText
-                throw ContainerizationError(.internalError, message: errorText)
-            }
-
-            if outputToStdout {
-                return ""
-            }
-
-            let outPipe = process.standardOutput as? Pipe
-            let stdoutData = outPipe?.fileHandleForReading.readDataToEndOfFile() ?? Data()
-            return String(decoding: stdoutData, as: UTF8.self)
-        }
     }
 }

Sources/Services/ContainerAPIService/Client/ContainerClient.swift

@@ -350,6 +350,53 @@ public struct ContainerClient: Sendable {
         }
     }
 
+    /// Extract a tar stream read from `archive` into a container directory.
+    ///
+    /// The stream is handed to the runtime as a file descriptor and forwarded to
+    /// the guest unmodified, so the ownership and mode recorded in the tar
+    /// headers are applied verbatim inside the container.
+    public func copyIn(id: String, archive: FileHandle, destination: String, createParents: Bool = true) async throws {
+        let request = XPCMessage(route: .containerCopyIn)
+        request.set(key: .id, value: id)
+        request.set(key: .destinationPath, value: destination)
+        request.set(key: .createParents, value: createParents)
+        request.set(key: .archiveFd, value: Self.duplicate(archive))
+
+        do {
+            try await xpcSend(message: request, timeout: .seconds(300))
+        } catch {
+            throw ContainerizationError(
+                .internalError,
+                message: "failed to copy tar stream into container \(id)",
+                cause: error
+            )
+        }
+    }
+
+    /// Write a container path to `archive` as a tar stream.
+    public func copyOut(id: String, source: String, archive: FileHandle) async throws {
+        let request = XPCMessage(route: .containerCopyOut)
+        request.set(key: .id, value: id)
+        request.set(key: .sourcePath, value: source)
+        request.set(key: .archiveFd, value: Self.duplicate(archive))
+
+        do {
+            try await xpcSend(message: request, timeout: .seconds(300))
+        } catch {
+            throw ContainerizationError(
+                .internalError,
+                message: "failed to copy tar stream from container \(id)",
+                cause: error
+            )
+        }
+    }
+
+    /// `XPCMessage.set(key:value:)` takes ownership of the descriptor and closes
+    /// it, so hand it a duplicate and leave the caller's handle intact.
+    private static func duplicate(_ handle: FileHandle) -> FileHandle {
+        FileHandle(fileDescriptor: dup(handle.fileDescriptor), closeOnDealloc: false)
+    }
+
     /// Get resource usage statistics for a container.
     public func stats(id: String) async throws -> ContainerStats {
         let request = XPCMessage(route: .containerStats)

Sources/Services/ContainerAPIService/Client/XPC+.swift

@@ -143,6 +143,8 @@ public enum XPCKeys: String {
     case destinationPath
     case fileMode
     case createParents
+    /// FD carrying a tar stream for `cp -`. Replaces the host side of the copy.
+    case archiveFd
 }
 
 public enum XPCRoute: String {

Sources/Services/ContainerAPIService/Server/Containers/ContainersHarness.swift

@@ -303,20 +303,30 @@ public struct ContainersHarness: Sendable {
                 message: "id cannot be empty"
             )
         }
-        guard let sourcePath = message.string(key: .sourcePath) else {
+        guard let destinationPath = message.string(key: .destinationPath) else {
             throw ContainerizationError(
                 .invalidArgument,
-                message: "source path cannot be empty"
+                message: "destination path cannot be empty"
             )
         }
-        guard let destinationPath = message.string(key: .destinationPath) else {
+        let createParents = message.bool(key: .createParents)
+
+        // fileHandle(key:) hands back a duplicate this process owns, so close it
+        // however the copy ends. The container lookups underneath can throw
+        // before the descriptor is ever forwarded.
+        if let archive = message.fileHandle(key: .archiveFd) {
+            defer { try? archive.close() }
+            try await service.copyIn(id: id, archive: archive, destination: destinationPath, createParents: createParents)
+            return message.reply()
+        }
+
+        guard let sourcePath = message.string(key: .sourcePath) else {
             throw ContainerizationError(
                 .invalidArgument,
-                message: "destination path cannot be empty"
+                message: "source path cannot be empty"
             )
         }
         let mode = UInt32(message.uint64(key: .fileMode))
-        let createParents = message.bool(key: .createParents)
 
         try await service.copyIn(id: id, source: sourcePath, destination: destinationPath, mode: mode, createParents: createParents)
         return message.reply()
@@ -336,6 +346,13 @@ public struct ContainersHarness: Sendable {
                 message: "source path cannot be empty"
             )
         }
+
+        if let archive = message.fileHandle(key: .archiveFd) {
+            defer { try? archive.close() }
+            try await service.copyOut(id: id, source: sourcePath, archive: archive)
+            return message.reply()
+        }
+
         guard let destinationPath = message.string(key: .destinationPath) else {
             throw ContainerizationError(
                 .invalidArgument,

Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift

@@ -786,6 +786,30 @@ public actor ContainersService {
         try await client.copyOut(source: source, destination: destination, createParents: createParents)
     }
 
+    /// Extract a tar stream read from `archive` into a container directory.
+    public func copyIn(id: String, archive: FileHandle, destination: String, createParents: Bool = true) async throws {
+        self.log.debug("\(#function)")
+
+        let state = try self._getContainerState(id: id)
+        guard state.snapshot.status == .running else {
+            throw ContainerizationError(.invalidState, message: "container \(id) is not running")
+        }
+        let client = try state.getClient()
+        try await client.copyIn(archive: archive, destination: destination, createParents: createParents)
+    }
+
+    /// Write a container path to `archive` as a tar stream.
+    public func copyOut(id: String, source: String, archive: FileHandle) async throws {
+        self.log.debug("\(#function)")
+
+        let state = try self._getContainerState(id: id)
+        guard state.snapshot.status == .running else {
+            throw ContainerizationError(.invalidState, message: "container \(id) is not running")
+        }
+        let client = try state.getClient()
+        try await client.copyOut(source: source, archive: archive)
+    }
+
     /// Get statistics for the container.
     public func stats(id: String) async throws -> ContainerStats {
         log.debug(

Sources/Services/Runtime/RuntimeClient/RuntimeClient.swift

@@ -319,6 +319,46 @@ extension RuntimeClient {
         }
     }
 
+    /// Extract a tar stream read from `archive` into a container directory.
+    ///
+    /// `archive` stays owned by the caller; a duplicate is sent over XPC, since
+    /// setting a file handle on a message closes the descriptor it is given.
+    public func copyIn(archive: FileHandle, destination: String, createParents: Bool = true) async throws {
+        let request = XPCMessage(route: RuntimeRoutes.copyIn.rawValue)
+        request.set(key: RuntimeKeys.destinationPath.rawValue, value: destination)
+        request.set(key: RuntimeKeys.createParents.rawValue, value: createParents)
+        request.set(key: RuntimeKeys.archiveFd.rawValue, value: FileHandle(fileDescriptor: dup(archive.fileDescriptor), closeOnDealloc: false))
+
+        do {
+            try await self.client.send(request, responseTimeout: .seconds(300))
+        } catch {
+            throw ContainerizationError(
+                .internalError,
+                message: "failed to copy tar stream into container \(self.id)",
+                cause: error
+            )
+        }
+    }
+
+    /// Write a container path to `archive` as a tar stream.
+    ///
+    /// `archive` stays owned by the caller, as in ``copyIn(archive:destination:createParents:)``.
+    public func copyOut(source: String, archive: FileHandle) async throws {
+        let request = XPCMessage(route: RuntimeRoutes.copyOut.rawValue)
+        request.set(key: RuntimeKeys.sourcePath.rawValue, value: source)
+        request.set(key: RuntimeKeys.archiveFd.rawValue, value: FileHandle(fileDescriptor: dup(archive.fileDescriptor), closeOnDealloc: false))
+
+        do {
+            try await self.client.send(request, responseTimeout: .seconds(300))
+        } catch {
+            throw ContainerizationError(
+                .internalError,
+                message: "failed to copy tar stream from container \(self.id)",
+                cause: error
+            )
+        }
+    }
+
     public func statistics() async throws -> ContainerStats {
         let request = XPCMessage(route: RuntimeRoutes.statistics.rawValue)