From 09489cba45e8ff4b08e0a8b04e0522fbb763be8a Mon Sep 17 00:00:00 2001 From: adityabagchi24 Date: Tue, 23 Jun 2026 00:24:00 +0530 Subject: [PATCH] fix(cp): resolve relative host paths against current directory (#1741) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1738 `container cp` fails when the host source path is relative (e.g. `container cp file foo:/root/`), because `NSString.standardizingPath` only canonicalizes paths but does not make them absolute. The unchanged relative path is then interpreted as `/file` (root-absolute) by `URL(fileURLWithPath:)` on the runtime side. Fixed by resolving relative paths against the current working directory before use, matching the pattern already used by `container export`, `container image save`, and `container image load`. The same fix was also applied to the copy-out destination path (line 68), which had the same issue. ## Type of Change - [x] Bug fix - [ ] New feature - [ ] Breaking change - [ ] Documentation update ## Motivation and Context `container cp file foo:/root/` fails with `"copyIn: source not found '/file'"` because the relative path `file` is never expanded to an absolute path. Using `$PWD/file` works, but relative paths should work too — every other command in the codebase handles this correctly. ## Testing - [x] Tested locally — builds and all existing tests pass - [ ] Added/updated tests - [ ] Added/updated docs --------- Co-authored-by: jwhur <57657645+JaewonHur@users.noreply.github.com> --- Makefile | 3 +- .../Container/ContainerCopy.swift | 17 +++++-- .../Subcommands/Containers/TestCLICopy.swift | 50 +++++++++++++++++++ 3 files changed, 65 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 23a26877..fb9a78e5 100644 --- a/Makefile +++ b/Makefile @@ -221,7 +221,8 @@ INTEGRATION_TEST_SUITES ?= \ TestCLISystemDF \ TestCLIMachineCommand \ TestCLIMachineRuntime \ - TestCLINoParallelCases + TestCLINoParallelCases \ + TestCLICopyCommand empty := space := $(empty) $(empty) diff --git a/Sources/ContainerCommands/Container/ContainerCopy.swift b/Sources/ContainerCommands/Container/ContainerCopy.swift index c6bdef07..83023316 100644 --- a/Sources/ContainerCommands/Container/ContainerCopy.swift +++ b/Sources/ContainerCommands/Container/ContainerCopy.swift @@ -65,16 +65,17 @@ extension Application { switch (srcRef, dstRef) { case (.container(let id, let path), .local(let localPath)): let srcPath = FilePath(path) - let destPath = FilePath((localPath as NSString).standardizingPath) + let destPath = FilePath(URL(fileURLWithPath: localPath, relativeTo: .currentDirectory()).absoluteURL.path(percentEncoded: false)) var isDirectory: ObjCBool = false let exists = FileManager.default.fileExists(atPath: destPath.string, isDirectory: &isDirectory) + var finalDestPath = destPath if exists && isDirectory.boolValue { guard let lastComponent = srcPath.lastComponent else { throw ContainerizationError(.invalidArgument, message: "source path has no last component: \(path)") } - let finalDest = destPath.appending(lastComponent) - try await client.copyOut(id: id, source: path, destination: finalDest.string) + finalDestPath = destPath.appending(lastComponent) + try await client.copyOut(id: id, source: path, destination: finalDestPath.string) } else if localPath.hasSuffix("/") { try await client.copyOut(id: id, source: path, destination: destPath.string) var resultIsDir: ObjCBool = false @@ -89,9 +90,15 @@ extension Application { } else { try await client.copyOut(id: id, source: path, destination: destPath.string) } + print(finalDestPath.string) case (.local(let localPath), .container(let id, let path)): - let srcPath = FilePath((localPath as NSString).standardizingPath) + let srcPath = FilePath(URL(fileURLWithPath: localPath, relativeTo: .currentDirectory()).absoluteURL.path(percentEncoded: false)) var isDirectory: ObjCBool = false + + guard let lastComponent = srcPath.lastComponent else { + throw ContainerizationError(.invalidArgument, message: "source path has no last component: \(localPath)") + } + guard FileManager.default.fileExists(atPath: srcPath.string, isDirectory: &isDirectory) else { throw ContainerizationError(.notFound, message: "source path does not exist: \(localPath)") } @@ -100,6 +107,8 @@ extension Application { } try await client.copyIn(id: id, source: srcPath.string, destination: path, createParents: true) + let printedDest = path.hasSuffix("/") ? "\(id):\(path)\(lastComponent.string)" : "\(id):\(path)" + print(printedDest) case (.container, .container): throw ContainerizationError(.invalidArgument, message: "copying between containers is not supported") case (.local, .local): diff --git a/Tests/CLITests/Subcommands/Containers/TestCLICopy.swift b/Tests/CLITests/Subcommands/Containers/TestCLICopy.swift index 3ad485ad..f7db9a82 100644 --- a/Tests/CLITests/Subcommands/Containers/TestCLICopy.swift +++ b/Tests/CLITests/Subcommands/Containers/TestCLICopy.swift @@ -905,4 +905,54 @@ class TestCLICopyCommand: CLITest { Issue.record("testCopyInDirectoryContentsToExistingDirectoryTrailingSlash failed: \(error)") } } + + // MARK: - Relative path resolution + + @Test func testCopyInRelativeSourcePath() throws { + do { + let name = getTestName() + try doCreate(name: name) + defer { try? doStop(name: name) } + try doStart(name: name) + try waitForContainerRunning(name) + + let content = "relative source" + try content.write(to: testDir.appendingPathComponent("relfile.txt"), atomically: true, encoding: .utf8) + + let (_, _, error, status) = try run( + arguments: ["copy", "./relfile.txt", "\(name):/tmp/"], + currentDirectory: testDir) + if status != 0 { throw CLIError.executionFailed("copy failed: \(error)") } + + let result = try doExec(name: name, cmd: ["cat", "/tmp/relfile.txt"]) + #expect(result.trimmingCharacters(in: .whitespacesAndNewlines) == content) + try doStop(name: name) + } catch { + Issue.record("testCopyInRelativeSourcePath failed: \(error)") + } + } + + @Test func testCopyOutRelativeDestinationPath() throws { + do { + let name = getTestName() + try doCreate(name: name) + defer { try? doStop(name: name) } + try doStart(name: name) + try waitForContainerRunning(name) + + let content = "relative dest" + _ = try doExec(name: name, cmd: ["sh", "-c", "echo -n '\(content)' > /tmp/relfile.txt"]) + + let (_, _, error, status) = try run( + arguments: ["copy", "\(name):/tmp/relfile.txt", "./"], + currentDirectory: testDir) + if status != 0 { throw CLIError.executionFailed("copy failed: \(error)") } + + let result = try String(contentsOfFile: testDir.appendingPathComponent("relfile.txt").path, encoding: .utf8) + #expect(result == content) + try doStop(name: name) + } catch { + Issue.record("testCopyOutRelativeDestinationPath failed: \(error)") + } + } }