From a6211f498a8fa2848cf417301b80d3a580465003 Mon Sep 17 00:00:00 2001 From: Chris George Date: Sat, 9 May 2026 09:20:33 -0700 Subject: [PATCH 1/3] Use FilePath for PublishSocket. Closes CHAOS-1459. Unblocks CHAOS-1463 (Parser). --- .../Container/PublishSocket.swift | 87 ++++++- .../ContainerAPIService/Client/Parser.swift | 5 +- .../RuntimeLinux/Server/RuntimeService.swift | 5 +- .../PublishSocketTests.swift | 224 ++++++++++++++++++ 4 files changed, 313 insertions(+), 8 deletions(-) create mode 100644 Tests/ContainerResourceTests/PublishSocketTests.swift diff --git a/Sources/ContainerResource/Container/PublishSocket.swift b/Sources/ContainerResource/Container/PublishSocket.swift index 08ea752bf..1763b01ca 100644 --- a/Sources/ContainerResource/Container/PublishSocket.swift +++ b/Sources/ContainerResource/Container/PublishSocket.swift @@ -20,21 +20,100 @@ import SystemPackage /// Represents a socket that should be published from container to host. public struct PublishSocket: Sendable, Codable { /// The path to the socket in the container. - public var containerPath: URL + public var containerPath: FilePath /// The path where the socket should appear on the host. - public var hostPath: URL + public var hostPath: FilePath /// File permissions for the socket on the host. public var permissions: FilePermissions? public init( - containerPath: URL, - hostPath: URL, + containerPath: FilePath, + hostPath: FilePath, permissions: FilePermissions? = nil ) { self.containerPath = containerPath self.hostPath = hostPath self.permissions = permissions } + + private enum CodingKeys: String, CodingKey { + case containerPath + case hostPath + case permissions + } + + /// Encode paths as file-URL absolute strings (e.g. `"file:///var/run/docker.sock"`). + /// + /// These fields were previously typed `URL`; `JSONEncoder` special-cases + /// `URL` to emit `absoluteString`. `FilePath`'s synthesized `Codable` + /// would instead emit a keyed container (`{"_storage": "..."}`), changing + /// the on-disk and XPC wire format. We therefore encode each path as the + /// equivalent `URL.absoluteString` so the byte form remains compatible + /// with persisted bundles and any readers (e.g. an older service binary) + /// that still decode these fields as `URL`. + public func encode(to encoder: any Encoder) throws { + var container = encoder.container(keyedBy: CodingKeys.self) + try container.encode(Self.encodePath(containerPath), forKey: .containerPath) + try container.encode(Self.encodePath(hostPath), forKey: .hostPath) + try container.encodeIfPresent(permissions, forKey: .permissions) + } + + public init(from decoder: any Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + self.containerPath = try Self.decodePath(from: container, forKey: .containerPath) + self.hostPath = try Self.decodePath(from: container, forKey: .hostPath) + self.permissions = try container.decodeIfPresent(FilePermissions.self, forKey: .permissions) + } + + /// Encode a `FilePath` as a file-URL `absoluteString` to match the prior + /// `URL`-typed wire format byte-for-byte. + private static func encodePath(_ path: FilePath) -> String { + URL(filePath: path.string).absoluteString + } + + /// Decode a `FilePath` from either the canonical file-URL form + /// (e.g. `"file:///foo"`) emitted by `encodePath(_:)` and the legacy + /// `URL`-typed wire format, or a plain absolute path string. Throws + /// `DecodingError.dataCorrupted` on malformed, empty, or non-absolute + /// inputs so corrupt persisted state fails loudly rather than silently + /// producing an invalid socket path. + private static func decodePath( + from container: KeyedDecodingContainer, + forKey key: CodingKeys + ) throws -> FilePath { + let raw = try container.decode(String.self, forKey: key) + + let path: String + if raw.hasPrefix("file:") { + guard let url = URL(string: raw), url.isFileURL else { + throw DecodingError.dataCorruptedError( + forKey: key, + in: container, + debugDescription: "malformed file URL: \(raw)" + ) + } + if let host = url.host(), !host.isEmpty, host != "localhost" { + throw DecodingError.dataCorruptedError( + forKey: key, + in: container, + debugDescription: "file URL host must be empty or 'localhost': \(raw)" + ) + } + path = url.path(percentEncoded: false) + } else { + path = raw + } + + guard path.hasPrefix("/") else { + throw DecodingError.dataCorruptedError( + forKey: key, + in: container, + debugDescription: "socket path must be absolute: \(raw)" + ) + } + + return FilePath(path) + } } diff --git a/Sources/Services/ContainerAPIService/Client/Parser.swift b/Sources/Services/ContainerAPIService/Client/Parser.swift index 7cbe7e0b0..16cb94e72 100644 --- a/Sources/Services/ContainerAPIService/Client/Parser.swift +++ b/Sources/Services/ContainerAPIService/Client/Parser.swift @@ -22,6 +22,7 @@ import ContainerizationExtras import ContainerizationOCI import ContainerizationOS import Foundation +import SystemPackage /// A parsed volume specification from user input public struct ParsedVolume { @@ -787,8 +788,8 @@ public struct Parser { // Create and return PublishSocket object with validated paths return PublishSocket( - containerPath: URL(fileURLWithPath: containerPath), - hostPath: URL(fileURLWithPath: absoluteHostPath), + containerPath: FilePath(containerPath), + hostPath: FilePath(absoluteHostPath), permissions: nil ) diff --git a/Sources/Services/RuntimeLinux/Server/RuntimeService.swift b/Sources/Services/RuntimeLinux/Server/RuntimeService.swift index 187de7af6..d4d468108 100644 --- a/Sources/Services/RuntimeLinux/Server/RuntimeService.swift +++ b/Sources/Services/RuntimeLinux/Server/RuntimeService.swift @@ -999,9 +999,10 @@ public actor RuntimeService { } for publishedSocket in config.publishedSockets { + // UnixSocketConfiguration (Containerization) takes URL; convert from FilePath at the boundary. let socketConfig = UnixSocketConfiguration( - source: publishedSocket.containerPath, - destination: publishedSocket.hostPath, + source: URL(filePath: publishedSocket.containerPath.string), + destination: URL(filePath: publishedSocket.hostPath.string), permissions: publishedSocket.permissions, direction: .outOf ) diff --git a/Tests/ContainerResourceTests/PublishSocketTests.swift b/Tests/ContainerResourceTests/PublishSocketTests.swift new file mode 100644 index 000000000..4e7ab32ac --- /dev/null +++ b/Tests/ContainerResourceTests/PublishSocketTests.swift @@ -0,0 +1,224 @@ +//===----------------------------------------------------------------------===// +// 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 +import Testing + +@testable import ContainerResource + +/// Tests covering the custom `Codable` implementation on ``PublishSocket``. +/// +/// `containerPath` and `hostPath` were migrated from `URL` to `FilePath`. +/// Because `URL` is encoded by `JSONEncoder` as its `absoluteString` (e.g. +/// `"file:///var/run/docker.sock"`), persisted bundles and any service +/// binaries still typed against `URL` expect that exact byte format on the +/// wire. The tests below pin the encoder to that legacy shape and verify the +/// decoder accepts both the legacy file-URL form and a plain absolute path. +struct PublishSocketTests { + // MARK: - Encoding + + @Test + func testEncodeProducesFileURLAbsoluteString() throws { + let socket = PublishSocket( + containerPath: FilePath("/var/run/docker.sock"), + hostPath: FilePath("/Users/me/docker.sock") + ) + let json = try JSONEncoder().encode(socket) + let decoded = try JSONSerialization.jsonObject(with: json) as? [String: Any] + let container = try #require(decoded) + #expect(container["containerPath"] as? String == "file:///var/run/docker.sock") + #expect(container["hostPath"] as? String == "file:///Users/me/docker.sock") + #expect(container["permissions"] == nil) + } + + @Test + func testEncodeIsByteCompatibleWithLegacyURLEncoding() throws { + // Pre-migration, these fields were `URL` and JSON-encoded via + // `URL.absoluteString`. Encoding must produce the exact same bytes + // for any reader still decoding them as `URL`. + let containerPath = "/var/run/docker.sock" + let hostPath = "/Users/me/docker.sock" + let legacy = LegacyPublishSocket( + containerPath: URL(fileURLWithPath: containerPath), + hostPath: URL(fileURLWithPath: hostPath) + ) + let current = PublishSocket( + containerPath: FilePath(containerPath), + hostPath: FilePath(hostPath) + ) + let encoder = JSONEncoder() + encoder.outputFormatting = [.sortedKeys] + #expect(try encoder.encode(current) == encoder.encode(legacy)) + } + + @Test + func testEncodePercentEncodesSpacesAndSpecialCharacters() throws { + let socket = PublishSocket( + containerPath: FilePath("/tmp/a b.sock"), + hostPath: FilePath("/tmp/dir with spaces/sock") + ) + let json = try JSONEncoder().encode(socket) + let decoded = try #require(try JSONSerialization.jsonObject(with: json) as? [String: Any]) + #expect(decoded["containerPath"] as? String == "file:///tmp/a%20b.sock") + #expect(decoded["hostPath"] as? String == "file:///tmp/dir%20with%20spaces/sock") + } + + // MARK: - Decoding (canonical / legacy file-URL form) + + @Test + func testDecodeFileURLForm() throws { + let json = """ + {"containerPath":"file:///var/run/docker.sock","hostPath":"file:///Users/me/docker.sock"} + """.data(using: .utf8)! + let socket = try JSONDecoder().decode(PublishSocket.self, from: json) + #expect(socket.containerPath == FilePath("/var/run/docker.sock")) + #expect(socket.hostPath == FilePath("/Users/me/docker.sock")) + #expect(socket.permissions == nil) + } + + @Test + func testDecodeFileURLPercentEncodingIsResolved() throws { + // Persisted bundles created via `URL(fileURLWithPath:)` percent-encode + // spaces; decoding must yield the original literal path. + let json = """ + {"containerPath":"file:///tmp/a%20b.sock","hostPath":"file:///tmp/x%2Fy.sock"} + """.data(using: .utf8)! + let socket = try JSONDecoder().decode(PublishSocket.self, from: json) + #expect(socket.containerPath == FilePath("/tmp/a b.sock")) + // `%2F` decodes to a literal `/` inside the path component. + #expect(socket.hostPath == FilePath("/tmp/x/y.sock")) + } + + @Test + func testDecodeFileURLWithLocalhostHost() throws { + let json = """ + {"containerPath":"file://localhost/var/run/docker.sock","hostPath":"file:///host.sock"} + """.data(using: .utf8)! + let socket = try JSONDecoder().decode(PublishSocket.self, from: json) + #expect(socket.containerPath == FilePath("/var/run/docker.sock")) + #expect(socket.hostPath == FilePath("/host.sock")) + } + + // MARK: - Decoding (forward-compat plain path form) + + @Test + func testDecodePlainAbsolutePath() throws { + // Forward-compat: if some other encoder emits a clean path string, + // accept it rather than rejecting at the boundary. + let json = """ + {"containerPath":"/var/run/docker.sock","hostPath":"/Users/me/docker.sock"} + """.data(using: .utf8)! + let socket = try JSONDecoder().decode(PublishSocket.self, from: json) + #expect(socket.containerPath == FilePath("/var/run/docker.sock")) + #expect(socket.hostPath == FilePath("/Users/me/docker.sock")) + } + + // MARK: - Round-trip + + @Test + func testRoundTrip() throws { + let original = PublishSocket( + containerPath: FilePath("/var/run/docker.sock"), + hostPath: FilePath("/tmp/socket with spaces.sock"), + permissions: FilePermissions(rawValue: 0o660) + ) + let encoder = JSONEncoder() + let decoder = JSONDecoder() + let data = try encoder.encode(original) + let decoded = try decoder.decode(PublishSocket.self, from: data) + #expect(decoded.containerPath == original.containerPath) + #expect(decoded.hostPath == original.hostPath) + #expect(decoded.permissions == original.permissions) + } + + // MARK: - Decoding errors + + @Test + func testDecodeEmptyStringThrows() { + let json = """ + {"containerPath":"","hostPath":"/host.sock"} + """.data(using: .utf8)! + #expect(throws: DecodingError.self) { + try JSONDecoder().decode(PublishSocket.self, from: json) + } + } + + @Test + func testDecodeFileColonOnlyThrows() { + // `"file:"` parses as a URL but yields an empty path; reject loudly + // rather than silently producing `FilePath("")`. + let json = """ + {"containerPath":"file:","hostPath":"/host.sock"} + """.data(using: .utf8)! + #expect(throws: DecodingError.self) { + try JSONDecoder().decode(PublishSocket.self, from: json) + } + } + + @Test + func testDecodeFileSchemeNoPathThrows() { + let json = """ + {"containerPath":"file://","hostPath":"/host.sock"} + """.data(using: .utf8)! + #expect(throws: DecodingError.self) { + try JSONDecoder().decode(PublishSocket.self, from: json) + } + } + + @Test + func testDecodeRelativePathThrows() { + let json = """ + {"containerPath":"relative/path.sock","hostPath":"/host.sock"} + """.data(using: .utf8)! + #expect(throws: DecodingError.self) { + try JSONDecoder().decode(PublishSocket.self, from: json) + } + } + + @Test + func testDecodeNonLocalHostFileURLThrows() { + // file URLs with a non-empty / non-localhost host are unsafe to + // interpret as a local path. + let json = """ + {"containerPath":"file://example.com/etc/passwd","hostPath":"/host.sock"} + """.data(using: .utf8)! + #expect(throws: DecodingError.self) { + try JSONDecoder().decode(PublishSocket.self, from: json) + } + } + + @Test + func testDecodeMissingRequiredKeyThrows() { + let json = """ + {"hostPath":"/host.sock"} + """.data(using: .utf8)! + #expect(throws: DecodingError.self) { + try JSONDecoder().decode(PublishSocket.self, from: json) + } + } +} + +// MARK: - Helpers + +/// Mirrors the pre-migration `URL`-typed `PublishSocket` shape, used only to +/// confirm the new encoder produces byte-identical output for any reader +/// still decoding these fields as `URL`. +private struct LegacyPublishSocket: Encodable { + var containerPath: URL + var hostPath: URL + var permissions: FilePermissions? +} From c572ff18adfb520b3d941a0b96d83e198bb65f00 Mon Sep 17 00:00:00 2001 From: Chris George Date: Wed, 27 May 2026 12:09:29 -0700 Subject: [PATCH 2/3] Address PR #1594 review feedback - PublishSocket.init now throws and validates absolute paths so objects are correct by construction; adds DocC for parameters and constraints (addresses comment on PublishSocket.swift:33 and :72). - Adds deprecation/migration note on the struct documenting that the decoder accepts both new `FilePath` and legacy `URL` forms for one release (addresses comment on PublishSocket.swift:21). - Wire format pre-1.0 breaking change: encoder now emits the plain absolute path (e.g. "/var/run/docker.sock") instead of the legacy file-URL form ("file:///var/run/docker.sock"). The decoder still accepts both forms so persisted bundles from earlier releases continue to load; that compatibility will be removed in a later release (addresses comment on PublishSocket.swift:58). - Bumps containerization to 0.33.2 and replaces URL(fileURLWithPath:).absoluteURL.path with ContainerizationOS.FilePathOps.absolutePath in Parser.publishSocket, removing the duplicate "must be absolute" check now that init() enforces it (addresses comments on Parser.swift:754 and :762). --- Package.resolved | 6 +- Package.swift | 2 +- .../Container/PublishSocket.swift | 91 +++++++---- .../ContainerAPIService/Client/Parser.swift | 31 ++-- .../PublishSocketTests.swift | 143 +++++++++--------- 5 files changed, 148 insertions(+), 125 deletions(-) diff --git a/Package.resolved b/Package.resolved index 6d19fb55d..3fdbb381c 100644 --- a/Package.resolved +++ b/Package.resolved @@ -1,5 +1,5 @@ { - "originHash" : "d9d032f8b1ad94cf9006bd4b441ce1dad52802b04b1d7f7b30baedd32cee0e8b", + "originHash" : "92e9d30c1bb3bc52ba8c855f83a17db4752940b9438a9f5d20fe5b7c1328aae0", "pins" : [ { "identity" : "async-http-client", @@ -15,8 +15,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/apple/containerization.git", "state" : { - "revision" : "9205a766302a18e06a0ce43f8b7e483625e3e50f", - "version" : "0.33.1" + "revision" : "2550dd49f1890702f6fe0171212050bbce9d3825", + "version" : "0.33.2" } }, { diff --git a/Package.swift b/Package.swift index 2cc2e7092..f6dda67b9 100644 --- a/Package.swift +++ b/Package.swift @@ -23,7 +23,7 @@ import PackageDescription let releaseVersion = ProcessInfo.processInfo.environment["RELEASE_VERSION"] ?? "0.0.0" let gitCommit = ProcessInfo.processInfo.environment["GIT_COMMIT"] ?? "unspecified" let builderShimVersion = "0.12.0" -let scVersion = "0.33.1" +let scVersion = "0.33.2" let package = Package( name: "container", diff --git a/Sources/ContainerResource/Container/PublishSocket.swift b/Sources/ContainerResource/Container/PublishSocket.swift index 1763b01ca..4b66c9076 100644 --- a/Sources/ContainerResource/Container/PublishSocket.swift +++ b/Sources/ContainerResource/Container/PublishSocket.swift @@ -14,25 +14,52 @@ // limitations under the License. //===----------------------------------------------------------------------===// +import ContainerizationError import Foundation import SystemPackage /// Represents a socket that should be published from container to host. +/// +/// - Deprecated: New for 1.0.0, path types changed from `URL` to `FilePath`. +/// - Note: Decoder handles `FilePath` and `URL` for persistent data compatibility; +/// this compatibility will be removed in a later release. public struct PublishSocket: Sendable, Codable { - /// The path to the socket in the container. + /// Absolute path to the socket inside the container. public var containerPath: FilePath - /// The path where the socket should appear on the host. + /// Absolute path where the socket appears on the host. public var hostPath: FilePath /// File permissions for the socket on the host. public var permissions: FilePermissions? + /// Creates a `PublishSocket` with validated absolute paths. + /// + /// - Parameters: + /// - containerPath: Absolute path to the socket inside the container. + /// Must begin with `/`. + /// - hostPath: Absolute path where the socket appears on the host. + /// Must begin with `/`. + /// - permissions: File permissions applied to the socket on the host. + /// - Throws: `ContainerizationError` with code `.invalidArgument` if + /// either path is not absolute. public init( containerPath: FilePath, hostPath: FilePath, permissions: FilePermissions? = nil - ) { + ) throws { + guard containerPath.isAbsolute else { + throw ContainerizationError( + .invalidArgument, + message: "containerPath must be absolute: \(containerPath)" + ) + } + guard hostPath.isAbsolute else { + throw ContainerizationError( + .invalidArgument, + message: "hostPath must be absolute: \(hostPath)" + ) + } self.containerPath = containerPath self.hostPath = hostPath self.permissions = permissions @@ -44,41 +71,45 @@ public struct PublishSocket: Sendable, Codable { case permissions } - /// Encode paths as file-URL absolute strings (e.g. `"file:///var/run/docker.sock"`). + /// Encodes each path as its plain absolute string (e.g. `"/var/run/docker.sock"`). /// - /// These fields were previously typed `URL`; `JSONEncoder` special-cases - /// `URL` to emit `absoluteString`. `FilePath`'s synthesized `Codable` - /// would instead emit a keyed container (`{"_storage": "..."}`), changing - /// the on-disk and XPC wire format. We therefore encode each path as the - /// equivalent `URL.absoluteString` so the byte form remains compatible - /// with persisted bundles and any readers (e.g. an older service binary) - /// that still decode these fields as `URL`. + /// Pre-1.0 wire-format change from the prior `URL`-typed encoding which + /// emitted `URL.absoluteString` (`"file:///var/run/docker.sock"`). The + /// decoder accepts both forms for compatibility with persisted bundles + /// from earlier releases; that compatibility will be removed in a later + /// release. public func encode(to encoder: any Encoder) throws { var container = encoder.container(keyedBy: CodingKeys.self) - try container.encode(Self.encodePath(containerPath), forKey: .containerPath) - try container.encode(Self.encodePath(hostPath), forKey: .hostPath) + try container.encode(containerPath.string, forKey: .containerPath) + try container.encode(hostPath.string, forKey: .hostPath) try container.encodeIfPresent(permissions, forKey: .permissions) } public init(from decoder: any Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) - self.containerPath = try Self.decodePath(from: container, forKey: .containerPath) - self.hostPath = try Self.decodePath(from: container, forKey: .hostPath) - self.permissions = try container.decodeIfPresent(FilePermissions.self, forKey: .permissions) - } - - /// Encode a `FilePath` as a file-URL `absoluteString` to match the prior - /// `URL`-typed wire format byte-for-byte. - private static func encodePath(_ path: FilePath) -> String { - URL(filePath: path.string).absoluteString + let containerPath = try Self.decodePath(from: container, forKey: .containerPath) + let hostPath = try Self.decodePath(from: container, forKey: .hostPath) + let permissions = try container.decodeIfPresent(FilePermissions.self, forKey: .permissions) + do { + try self.init( + containerPath: containerPath, + hostPath: hostPath, + permissions: permissions + ) + } catch let error as ContainerizationError { + throw DecodingError.dataCorruptedError( + forKey: .containerPath, + in: container, + debugDescription: String(describing: error) + ) + } } - /// Decode a `FilePath` from either the canonical file-URL form - /// (e.g. `"file:///foo"`) emitted by `encodePath(_:)` and the legacy - /// `URL`-typed wire format, or a plain absolute path string. Throws - /// `DecodingError.dataCorrupted` on malformed, empty, or non-absolute - /// inputs so corrupt persisted state fails loudly rather than silently - /// producing an invalid socket path. + /// Decodes a `FilePath` accepting either the new plain-path form + /// (`"/var/run/docker.sock"`) or the legacy file-URL form emitted by + /// older releases (`"file:///var/run/docker.sock"`). Throws + /// `DecodingError.dataCorrupted` on a malformed file URL or empty input. + /// Absoluteness is enforced in `init(containerPath:hostPath:permissions:)`. private static func decodePath( from container: KeyedDecodingContainer, forKey key: CodingKeys @@ -106,11 +137,11 @@ public struct PublishSocket: Sendable, Codable { path = raw } - guard path.hasPrefix("/") else { + guard !path.isEmpty else { throw DecodingError.dataCorruptedError( forKey: key, in: container, - debugDescription: "socket path must be absolute: \(raw)" + debugDescription: "decoded socket path is empty: \(raw)" ) } diff --git a/Sources/Services/ContainerAPIService/Client/Parser.swift b/Sources/Services/ContainerAPIService/Client/Parser.swift index 16cb94e72..61779498e 100644 --- a/Sources/Services/ContainerAPIService/Client/Parser.swift +++ b/Sources/Services/ContainerAPIService/Client/Parser.swift @@ -740,7 +740,6 @@ public struct Parser { let hostPath = String(parts[0]) let containerPath = String(parts[1]) - // Validate paths are not empty if hostPath.isEmpty { throw ContainerizationError( .invalidArgument, message: "host socket path cannot be empty") @@ -750,28 +749,18 @@ public struct Parser { .invalidArgument, message: "container socket path cannot be empty") } - // Ensure container path must start with / - if !containerPath.hasPrefix("/") { - throw ContainerizationError( - .invalidArgument, - message: "container socket path must be absolute: \(containerPath)") - } - - // Convert host path to absolute path for consistency - let hostURL = URL(fileURLWithPath: hostPath) - let absoluteHostPath = hostURL.absoluteURL.path + let absoluteHostPath = FilePathOps.absolutePath(FilePath(hostPath)) - // Check if host socket already exists and might be in use - if FileManager.default.fileExists(atPath: absoluteHostPath) { + if FileManager.default.fileExists(atPath: absoluteHostPath.string) { do { - let attrs = try FileManager.default.attributesOfItem(atPath: absoluteHostPath) + let attrs = try FileManager.default.attributesOfItem(atPath: absoluteHostPath.string) if let fileType = attrs[.type] as? FileAttributeType, fileType == .typeSocket { throw ContainerizationError( .invalidArgument, message: "host socket \(absoluteHostPath) already exists and may be in use") } // If it exists but is not a socket, we can remove it and create socket - try FileManager.default.removeItem(atPath: absoluteHostPath) + try FileManager.default.removeItem(atPath: absoluteHostPath.string) } catch let error as ContainerizationError { throw error } catch { @@ -779,17 +768,15 @@ public struct Parser { } } - // Create host directory if it doesn't exist - let hostDir = hostURL.deletingLastPathComponent() - if !FileManager.default.fileExists(atPath: hostDir.path) { + let hostDir = absoluteHostPath.removingLastComponent() + if !FileManager.default.fileExists(atPath: hostDir.string) { try FileManager.default.createDirectory( - at: hostDir, withIntermediateDirectories: true) + atPath: hostDir.string, withIntermediateDirectories: true) } - // Create and return PublishSocket object with validated paths - return PublishSocket( + return try PublishSocket( containerPath: FilePath(containerPath), - hostPath: FilePath(absoluteHostPath), + hostPath: absoluteHostPath, permissions: nil ) diff --git a/Tests/ContainerResourceTests/PublishSocketTests.swift b/Tests/ContainerResourceTests/PublishSocketTests.swift index 4e7ab32ac..2bfb0639c 100644 --- a/Tests/ContainerResourceTests/PublishSocketTests.swift +++ b/Tests/ContainerResourceTests/PublishSocketTests.swift @@ -14,73 +14,101 @@ // limitations under the License. //===----------------------------------------------------------------------===// +import ContainerizationError import Foundation import SystemPackage import Testing @testable import ContainerResource -/// Tests covering the custom `Codable` implementation on ``PublishSocket``. +/// Tests covering the custom `Codable` implementation and validating +/// initializer on ``PublishSocket``. /// -/// `containerPath` and `hostPath` were migrated from `URL` to `FilePath`. -/// Because `URL` is encoded by `JSONEncoder` as its `absoluteString` (e.g. -/// `"file:///var/run/docker.sock"`), persisted bundles and any service -/// binaries still typed against `URL` expect that exact byte format on the -/// wire. The tests below pin the encoder to that legacy shape and verify the -/// decoder accepts both the legacy file-URL form and a plain absolute path. +/// `containerPath` and `hostPath` were migrated from `URL` to `FilePath`. The +/// wire format was simultaneously changed from `URL.absoluteString` +/// (e.g. `"file:///var/run/docker.sock"`) to the plain absolute path string +/// (e.g. `"/var/run/docker.sock"`). The decoder retains compatibility with +/// the legacy file-URL form so persisted bundles from earlier releases +/// continue to load. struct PublishSocketTests { - // MARK: - Encoding + // MARK: - init validation @Test - func testEncodeProducesFileURLAbsoluteString() throws { - let socket = PublishSocket( + func testInitAcceptsAbsolutePaths() throws { + let socket = try PublishSocket( containerPath: FilePath("/var/run/docker.sock"), hostPath: FilePath("/Users/me/docker.sock") ) - let json = try JSONEncoder().encode(socket) - let decoded = try JSONSerialization.jsonObject(with: json) as? [String: Any] - let container = try #require(decoded) - #expect(container["containerPath"] as? String == "file:///var/run/docker.sock") - #expect(container["hostPath"] as? String == "file:///Users/me/docker.sock") - #expect(container["permissions"] == nil) - } - - @Test - func testEncodeIsByteCompatibleWithLegacyURLEncoding() throws { - // Pre-migration, these fields were `URL` and JSON-encoded via - // `URL.absoluteString`. Encoding must produce the exact same bytes - // for any reader still decoding them as `URL`. - let containerPath = "/var/run/docker.sock" - let hostPath = "/Users/me/docker.sock" - let legacy = LegacyPublishSocket( - containerPath: URL(fileURLWithPath: containerPath), - hostPath: URL(fileURLWithPath: hostPath) - ) - let current = PublishSocket( - containerPath: FilePath(containerPath), - hostPath: FilePath(hostPath) + #expect(socket.containerPath == FilePath("/var/run/docker.sock")) + #expect(socket.hostPath == FilePath("/Users/me/docker.sock")) + #expect(socket.permissions == nil) + } + + @Test + func testInitRejectsRelativeContainerPath() { + #expect(throws: ContainerizationError.self) { + try PublishSocket( + containerPath: FilePath("relative/path.sock"), + hostPath: FilePath("/host.sock") + ) + } + } + + @Test + func testInitRejectsRelativeHostPath() { + #expect(throws: ContainerizationError.self) { + try PublishSocket( + containerPath: FilePath("/var/run/docker.sock"), + hostPath: FilePath("relative/host.sock") + ) + } + } + + // MARK: - Encoding (plain absolute path) + + @Test + func testEncodeProducesPlainAbsolutePath() throws { + let socket = try PublishSocket( + containerPath: FilePath("/var/run/docker.sock"), + hostPath: FilePath("/Users/me/docker.sock") ) - let encoder = JSONEncoder() - encoder.outputFormatting = [.sortedKeys] - #expect(try encoder.encode(current) == encoder.encode(legacy)) + let json = try JSONEncoder().encode(socket) + let decoded = try #require(try JSONSerialization.jsonObject(with: json) as? [String: Any]) + #expect(decoded["containerPath"] as? String == "/var/run/docker.sock") + #expect(decoded["hostPath"] as? String == "/Users/me/docker.sock") + #expect(decoded["permissions"] == nil) } @Test - func testEncodePercentEncodesSpacesAndSpecialCharacters() throws { - let socket = PublishSocket( + func testEncodeDoesNotPercentEncode() throws { + // Plain-path encoding preserves spaces and special characters verbatim + // (no URL percent-encoding layer). + let socket = try PublishSocket( containerPath: FilePath("/tmp/a b.sock"), hostPath: FilePath("/tmp/dir with spaces/sock") ) let json = try JSONEncoder().encode(socket) let decoded = try #require(try JSONSerialization.jsonObject(with: json) as? [String: Any]) - #expect(decoded["containerPath"] as? String == "file:///tmp/a%20b.sock") - #expect(decoded["hostPath"] as? String == "file:///tmp/dir%20with%20spaces/sock") + #expect(decoded["containerPath"] as? String == "/tmp/a b.sock") + #expect(decoded["hostPath"] as? String == "/tmp/dir with spaces/sock") } - // MARK: - Decoding (canonical / legacy file-URL form) + // MARK: - Decoding (canonical plain-path form) @Test - func testDecodeFileURLForm() throws { + func testDecodePlainAbsolutePath() throws { + let json = """ + {"containerPath":"/var/run/docker.sock","hostPath":"/Users/me/docker.sock"} + """.data(using: .utf8)! + let socket = try JSONDecoder().decode(PublishSocket.self, from: json) + #expect(socket.containerPath == FilePath("/var/run/docker.sock")) + #expect(socket.hostPath == FilePath("/Users/me/docker.sock")) + } + + // MARK: - Decoding (legacy file-URL form, compat) + + @Test + func testDecodeLegacyFileURLForm() throws { let json = """ {"containerPath":"file:///var/run/docker.sock","hostPath":"file:///Users/me/docker.sock"} """.data(using: .utf8)! @@ -91,7 +119,7 @@ struct PublishSocketTests { } @Test - func testDecodeFileURLPercentEncodingIsResolved() throws { + func testDecodeLegacyFileURLResolvesPercentEncoding() throws { // Persisted bundles created via `URL(fileURLWithPath:)` percent-encode // spaces; decoding must yield the original literal path. let json = """ @@ -104,7 +132,7 @@ struct PublishSocketTests { } @Test - func testDecodeFileURLWithLocalhostHost() throws { + func testDecodeLegacyFileURLWithLocalhostHost() throws { let json = """ {"containerPath":"file://localhost/var/run/docker.sock","hostPath":"file:///host.sock"} """.data(using: .utf8)! @@ -113,25 +141,11 @@ struct PublishSocketTests { #expect(socket.hostPath == FilePath("/host.sock")) } - // MARK: - Decoding (forward-compat plain path form) - - @Test - func testDecodePlainAbsolutePath() throws { - // Forward-compat: if some other encoder emits a clean path string, - // accept it rather than rejecting at the boundary. - let json = """ - {"containerPath":"/var/run/docker.sock","hostPath":"/Users/me/docker.sock"} - """.data(using: .utf8)! - let socket = try JSONDecoder().decode(PublishSocket.self, from: json) - #expect(socket.containerPath == FilePath("/var/run/docker.sock")) - #expect(socket.hostPath == FilePath("/Users/me/docker.sock")) - } - // MARK: - Round-trip @Test func testRoundTrip() throws { - let original = PublishSocket( + let original = try PublishSocket( containerPath: FilePath("/var/run/docker.sock"), hostPath: FilePath("/tmp/socket with spaces.sock"), permissions: FilePermissions(rawValue: 0o660) @@ -181,6 +195,8 @@ struct PublishSocketTests { @Test func testDecodeRelativePathThrows() { + // Reject non-absolute paths — `init` enforces absoluteness and the + // decoder maps the validation error to `DecodingError`. let json = """ {"containerPath":"relative/path.sock","hostPath":"/host.sock"} """.data(using: .utf8)! @@ -211,14 +227,3 @@ struct PublishSocketTests { } } } - -// MARK: - Helpers - -/// Mirrors the pre-migration `URL`-typed `PublishSocket` shape, used only to -/// confirm the new encoder produces byte-identical output for any reader -/// still decoding these fields as `URL`. -private struct LegacyPublishSocket: Encodable { - var containerPath: URL - var hostPath: URL - var permissions: FilePermissions? -} From 605408b41e553e0f1898604e8d14c665b74859c0 Mon Sep 17 00:00:00 2001 From: Chris George Date: Thu, 28 May 2026 13:00:34 -0700 Subject: [PATCH 3/3] Validate absolute paths in PublishSocket.decodePath Enforce absoluteness directly in the decode helper (in addition to the by-construction check in init) so decoded PublishSocket paths are lexically correct even when read from manually edited or corrupt persisted configs. Non-absolute decoded paths now throw DecodingError.dataCorrupted at the decode layer. Adds testDecodeRelativeHostPathThrows covering a relative hostPath on decode and clarifies the existing relative-containerPath test. Addresses PR #1594 review comments on PublishSocket.swift:90 and PublishSocketTests.swift:99. --- .../Container/PublishSocket.swift | 17 ++++++++++++++--- .../PublishSocketTests.swift | 16 ++++++++++++++-- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/Sources/ContainerResource/Container/PublishSocket.swift b/Sources/ContainerResource/Container/PublishSocket.swift index 4b66c9076..3d53cd475 100644 --- a/Sources/ContainerResource/Container/PublishSocket.swift +++ b/Sources/ContainerResource/Container/PublishSocket.swift @@ -108,8 +108,10 @@ public struct PublishSocket: Sendable, Codable { /// Decodes a `FilePath` accepting either the new plain-path form /// (`"/var/run/docker.sock"`) or the legacy file-URL form emitted by /// older releases (`"file:///var/run/docker.sock"`). Throws - /// `DecodingError.dataCorrupted` on a malformed file URL or empty input. - /// Absoluteness is enforced in `init(containerPath:hostPath:permissions:)`. + /// `DecodingError.dataCorrupted` on a malformed file URL, empty input, or a + /// non-absolute path — validating decoded paths here guards against + /// manually edited or corrupt persisted configs, complementing the + /// by-construction check in `init(containerPath:hostPath:permissions:)`. private static func decodePath( from container: KeyedDecodingContainer, forKey key: CodingKeys @@ -145,6 +147,15 @@ public struct PublishSocket: Sendable, Codable { ) } - return FilePath(path) + let filePath = FilePath(path) + guard filePath.isAbsolute else { + throw DecodingError.dataCorruptedError( + forKey: key, + in: container, + debugDescription: "decoded socket path must be absolute: \(raw)" + ) + } + + return filePath } } diff --git a/Tests/ContainerResourceTests/PublishSocketTests.swift b/Tests/ContainerResourceTests/PublishSocketTests.swift index 2bfb0639c..6d40d2fdb 100644 --- a/Tests/ContainerResourceTests/PublishSocketTests.swift +++ b/Tests/ContainerResourceTests/PublishSocketTests.swift @@ -195,8 +195,9 @@ struct PublishSocketTests { @Test func testDecodeRelativePathThrows() { - // Reject non-absolute paths — `init` enforces absoluteness and the - // decoder maps the validation error to `DecodingError`. + // Reject non-absolute paths. `decodePath` validates absoluteness at the + // decode layer (and `init` enforces it by construction), surfacing the + // failure as a `DecodingError`. let json = """ {"containerPath":"relative/path.sock","hostPath":"/host.sock"} """.data(using: .utf8)! @@ -205,6 +206,17 @@ struct PublishSocketTests { } } + @Test + func testDecodeRelativeHostPathThrows() { + // A relative `hostPath` is likewise rejected at the decode layer. + let json = """ + {"containerPath":"/var/run/docker.sock","hostPath":"relative/host.sock"} + """.data(using: .utf8)! + #expect(throws: DecodingError.self) { + try JSONDecoder().decode(PublishSocket.self, from: json) + } + } + @Test func testDecodeNonLocalHostFileURLThrows() { // file URLs with a non-empty / non-localhost host are unsafe to