From fe910bbcb1791c4b3e9670e42389ff644e30f404 Mon Sep 17 00:00:00 2001 From: Chris George Date: Fri, 8 May 2026 17:14:11 -0700 Subject: [PATCH] Use FilePath for CLI image/container path-arg parsing. - Closes CHAOS-1457. - Adds shared FilePath.absolute(_:relativeTo:) helper. - Sibling of #1480 (HostDNSResolver) and #1518 (PacketFilter), same pattern. --- Package.swift | 1 + .../Container/ContainerExport.swift | 7 +- .../ContainerCommands/FilePath+Absolute.swift | 31 +++++++ .../ContainerCommands/Image/ImageLoad.swift | 8 +- .../ContainerCommands/Image/ImageSave.swift | 8 +- .../FilePathAbsoluteTests.swift | 80 +++++++++++++++++++ 6 files changed, 117 insertions(+), 18 deletions(-) create mode 100644 Sources/ContainerCommands/FilePath+Absolute.swift create mode 100644 Tests/ContainerCommandsTests/FilePathAbsoluteTests.swift diff --git a/Package.swift b/Package.swift index 442aa8b9a..9956f2a2d 100644 --- a/Package.swift +++ b/Package.swift @@ -145,6 +145,7 @@ let package = Package( .testTarget( name: "ContainerCommandsTests", dependencies: [ + .product(name: "SystemPackage", package: "swift-system"), "ContainerCommands", "ContainerResource", ] diff --git a/Sources/ContainerCommands/Container/ContainerExport.swift b/Sources/ContainerCommands/Container/ContainerExport.swift index a7394f268..df4f9ec60 100644 --- a/Sources/ContainerCommands/Container/ContainerExport.swift +++ b/Sources/ContainerCommands/Container/ContainerExport.swift @@ -18,6 +18,7 @@ import ArgumentParser import ContainerAPIClient import ContainerizationError import Foundation +import SystemPackage import TerminalProgress extension Application { @@ -35,9 +36,7 @@ extension Application { @Option( name: .shortAndLong, help: "Pathname for the saved container filesystem (defaults to stdout)", completion: .file(), - transform: { str in - URL(fileURLWithPath: str, relativeTo: .currentDirectory()).absoluteURL.path(percentEncoded: false) - }) + transform: { str in FilePath.absolute(str).string }) var output: String? @Argument(help: "container ID") @@ -67,7 +66,7 @@ extension Application { } try fileHandle.close() } else { - try FileManager.default.moveItem(at: archive, to: URL(fileURLWithPath: output!)) + try FileManager.default.moveItem(atPath: archive.path(), toPath: output!) } } } diff --git a/Sources/ContainerCommands/FilePath+Absolute.swift b/Sources/ContainerCommands/FilePath+Absolute.swift new file mode 100644 index 000000000..99c90f448 --- /dev/null +++ b/Sources/ContainerCommands/FilePath+Absolute.swift @@ -0,0 +1,31 @@ +//===----------------------------------------------------------------------===// +// 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 Foundation +import SystemPackage + +extension FilePath { + /// Resolve `str` to an absolute path. If already absolute, returns it lexically normalized. + /// Otherwise resolves against `cwd` (defaults to the current working directory). + static func absolute( + _ str: String, + relativeTo cwd: FilePath = FilePath(FileManager.default.currentDirectoryPath) + ) -> FilePath { + let p = FilePath(str) + if p.isAbsolute { return p.lexicallyNormalized() } + return cwd.appending(p.components).lexicallyNormalized() + } +} diff --git a/Sources/ContainerCommands/Image/ImageLoad.swift b/Sources/ContainerCommands/Image/ImageLoad.swift index 08e66d4f7..10c6ec9f1 100644 --- a/Sources/ContainerCommands/Image/ImageLoad.swift +++ b/Sources/ContainerCommands/Image/ImageLoad.swift @@ -32,13 +32,7 @@ extension Application { @Option( name: .shortAndLong, help: "Path to the image tar archive", completion: .file(), - transform: { str in - let path = FilePath(str) - guard path.isRelative else { return path.lexicallyNormalized() } - return FilePath(FileManager.default.currentDirectoryPath) - .pushing(path) - .lexicallyNormalized() - }) + transform: { str in FilePath.absolute(str) }) var input: FilePath? @Flag(name: .shortAndLong, help: "Load images even if the archive contains invalid files") diff --git a/Sources/ContainerCommands/Image/ImageSave.swift b/Sources/ContainerCommands/Image/ImageSave.swift index d1cab7965..7d06f4f1d 100644 --- a/Sources/ContainerCommands/Image/ImageSave.swift +++ b/Sources/ContainerCommands/Image/ImageSave.swift @@ -47,13 +47,7 @@ extension Application { @Option( name: .shortAndLong, help: "Pathname for the saved image", completion: .file(), - transform: { str in - let path = FilePath(str) - guard path.isRelative else { return path.lexicallyNormalized() } - return FilePath(FileManager.default.currentDirectoryPath) - .pushing(path) - .lexicallyNormalized() - }) + transform: { str in FilePath.absolute(str) }) var output: FilePath? @Option( diff --git a/Tests/ContainerCommandsTests/FilePathAbsoluteTests.swift b/Tests/ContainerCommandsTests/FilePathAbsoluteTests.swift new file mode 100644 index 000000000..636ada8a7 --- /dev/null +++ b/Tests/ContainerCommandsTests/FilePathAbsoluteTests.swift @@ -0,0 +1,80 @@ +//===----------------------------------------------------------------------===// +// 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 SystemPackage +import Testing + +@testable import ContainerCommands + +struct FilePathAbsoluteTests { + @Test + func absoluteInputReturnedAsIs() { + let result = FilePath.absolute("/usr/local/bin/tool") + #expect(result.string == "/usr/local/bin/tool") + } + + @Test + func absoluteInputWithDotDotNormalized() { + let result = FilePath.absolute("/usr/local/../bin/tool") + #expect(result.string == "/usr/bin/tool") + } + + @Test + func relativeInputResolvedAgainstCwd() { + let result = FilePath.absolute("images/foo.tar", relativeTo: FilePath("/tmp")) + #expect(result.string == "/tmp/images/foo.tar") + } + + @Test + func relativeInputWithDotDotNormalized() { + let result = FilePath.absolute("../foo.tar", relativeTo: FilePath("/tmp/sub")) + #expect(result.string == "/tmp/foo.tar") + } + + @Test + func singleFilenameResolvedAgainstCwd() { + let result = FilePath.absolute("archive.tar", relativeTo: FilePath("/home/user")) + #expect(result.string == "/home/user/archive.tar") + } + + @Test + func dotDotPastRootClampsAtRoot() { + // POSIX lexicallyNormalized() clamps excess `..` components at `/` + let result = FilePath.absolute("../../../../../../etc/passwd", relativeTo: FilePath("/tmp")) + #expect(result.string == "/etc/passwd") + } + + @Test + func emptyStringInputResolvesToCwd() { + // `--output ""` is silently treated as "use current directory" + let result = FilePath.absolute("", relativeTo: FilePath("/tmp")) + #expect(result.string == "/tmp") + } + + @Test + func rootPathPreservedThroughNormalization() { + // An absolute `/` input should survive lexical normalization unchanged + let result = FilePath.absolute("/", relativeTo: FilePath("/tmp")) + #expect(result.string == "/") + } + + @Test + func trailingSlashDroppedByNormalization() { + // FilePath drops trailing slashes during lexical normalization + let result = FilePath.absolute("/tmp/", relativeTo: FilePath("/somewhere")) + #expect(result.string == "/tmp") + } +}