From 8e99522152c9f6e9ad3326d84d8a7187b78f69c7 Mon Sep 17 00:00:00 2001 From: Chris George Date: Fri, 8 May 2026 16:38:22 -0700 Subject: [PATCH 1/2] Use FilePath for SnapshotStore. - Closes CHAOS-1454. - Internal cluster; URL boundary preserved at `directorySize` for `URLResourceKey.totalFileAllocatedSizeKey`. - Sibling of #1480 (HostDNSResolver) and #1518 (PacketFilter), same pattern. --- Package.swift | 1 + Sources/Plugins/CoreImages/ImagesHelper.swift | 3 +- .../Server/SnapshotStore.swift | 85 +++++++++---------- 3 files changed, 43 insertions(+), 46 deletions(-) diff --git a/Package.swift b/Package.swift index 442aa8b9a..17790948b 100644 --- a/Package.swift +++ b/Package.swift @@ -266,6 +266,7 @@ let package = Package( .product(name: "ContainerizationExtras", package: "containerization"), .product(name: "ContainerizationOCI", package: "containerization"), .product(name: "ContainerizationOS", package: "containerization"), + .product(name: "SystemPackage", package: "swift-system"), "ContainerAPIClient", "ContainerImagesServiceClient", "ContainerLog", diff --git a/Sources/Plugins/CoreImages/ImagesHelper.swift b/Sources/Plugins/CoreImages/ImagesHelper.swift index 6ba3318b5..30b479fa1 100644 --- a/Sources/Plugins/CoreImages/ImagesHelper.swift +++ b/Sources/Plugins/CoreImages/ImagesHelper.swift @@ -25,6 +25,7 @@ import ContainerXPC import Containerization import Foundation import Logging +import SystemPackage @main struct ImagesHelper: AsyncParsableCommand { @@ -94,7 +95,7 @@ extension ImagesHelper { let contentStore = RemoteContentStoreClient() let imageStore = try ImageStore(path: root, contentStore: contentStore) let unpackStrategy = SnapshotStore.defaultUnpackStrategy(initImage: containerSystemConfig.vminit.image) - let snapshotStore = try SnapshotStore(path: root, unpackStrategy: unpackStrategy, log: log) + let snapshotStore = try SnapshotStore(path: FilePath(root.absolutePath()), unpackStrategy: unpackStrategy, log: log) let service = try ImagesService(contentStore: contentStore, imageStore: imageStore, snapshotStore: snapshotStore, log: log) let harness = ImagesServiceHarness(service: service, log: log) diff --git a/Sources/Services/ContainerImagesService/Server/SnapshotStore.swift b/Sources/Services/ContainerImagesService/Server/SnapshotStore.swift index 282569a9d..fe52fd871 100644 --- a/Sources/Services/ContainerImagesService/Server/SnapshotStore.swift +++ b/Sources/Services/ContainerImagesService/Server/SnapshotStore.swift @@ -20,9 +20,9 @@ import Containerization import ContainerizationError import ContainerizationExtras import ContainerizationOCI -import ContainerizationOS import Foundation import Logging +import SystemPackage import TerminalProgress public actor SnapshotStore { @@ -47,20 +47,20 @@ public actor SnapshotStore { } } - let path: URL + let path: FilePath let fm = FileManager.default - let ingestDir: URL + let ingestDir: FilePath let unpackStrategy: UnpackStrategy let log: Logger? - public init(path: URL, unpackStrategy: @escaping UnpackStrategy, log: Logger?) throws { - let root = path.appendingPathComponent("snapshots") + public init(path: FilePath, unpackStrategy: @escaping UnpackStrategy, log: Logger?) throws { + let root = path.appending("snapshots") self.path = root - self.ingestDir = self.path.appendingPathComponent(Self.ingestDirName) + self.ingestDir = self.path.appending(Self.ingestDirName) self.unpackStrategy = unpackStrategy self.log = log - try self.fm.createDirectory(at: root, withIntermediateDirectories: true) - try self.fm.createDirectory(at: self.ingestDir, withIntermediateDirectories: true) + try self.fm.createDirectory(atPath: root.string, withIntermediateDirectories: true, attributes: nil) + try self.fm.createDirectory(atPath: self.ingestDir.string, withIntermediateDirectories: true, attributes: nil) } public func unpack(image: Containerization.Image, platform: Platform? = nil, progressUpdate: ProgressUpdateHandler?) async throws { @@ -78,7 +78,7 @@ public actor SnapshotStore { for desc in toUnpack { try Task.checkCancellation() let snapshotDir = self.snapshotDir(desc) - guard !self.fm.fileExists(atPath: snapshotDir.absolutePath()) else { + guard !self.fm.fileExists(atPath: snapshotDir.string) else { // We have already unpacked this image + platform. Skip continue } @@ -100,30 +100,30 @@ public actor SnapshotStore { let tempDir = try self.tempUnpackDir() - let tempSnapshotPath = tempDir.appendingPathComponent(Self.snapshotFileName, isDirectory: false) - let infoPath = tempDir.appendingPathComponent(Self.snapshotInfoFileName, isDirectory: false) + let tempSnapshotPath = URL(fileURLWithPath: tempDir.appending(Self.snapshotFileName).string, isDirectory: false) + let infoPath = tempDir.appending(Self.snapshotInfoFileName) do { let progress = ContainerizationProgressAdapter.handler(from: taskUpdateProgress) let mount = try await unpacker.unpack(image, for: platform, at: tempSnapshotPath, progress: progress) let fs = Filesystem.block( format: mount.type, - source: self.snapshotPath(desc).absolutePath(), + source: self.snapshotPath(desc).string, destination: mount.destination, options: mount.options ) let snapshotInfo = try JSONEncoder().encode(fs) - self.fm.createFile(atPath: infoPath.absolutePath(), contents: snapshotInfo) + self.fm.createFile(atPath: infoPath.string, contents: snapshotInfo) } catch { - try? self.fm.removeItem(at: tempDir) + try? self.fm.removeItem(atPath: tempDir.string) throw error } do { - try fm.moveItem(at: tempDir, to: snapshotDir) + try fm.moveItem(atPath: tempDir.string, toPath: snapshotDir.string) } catch let err as NSError { guard err.code == NSFileWriteFileExistsError else { throw err } - try? self.fm.removeItem(at: tempDir) + try? self.fm.removeItem(atPath: tempDir.string) } } await taskManager.finish() @@ -139,10 +139,10 @@ public actor SnapshotStore { } for desc in toDelete { let p = self.snapshotDir(desc) - guard self.fm.fileExists(atPath: p.absolutePath()) else { + guard self.fm.fileExists(atPath: p.string) else { continue } - try self.fm.removeItem(at: p) + try self.fm.removeItem(atPath: p.string) } } @@ -151,13 +151,13 @@ public actor SnapshotStore { let infoPath = snapshotInfoPath(desc) let fsPath = snapshotPath(desc) - guard self.fm.fileExists(atPath: infoPath.absolutePath()), - self.fm.fileExists(atPath: fsPath.absolutePath()) + guard self.fm.fileExists(atPath: infoPath.string), + self.fm.fileExists(atPath: fsPath.string) else { throw ContainerizationError(.notFound, message: "image snapshot for \(image.reference) with platform \(platform.description)") } let decoder = JSONDecoder() - let data = try Data(contentsOf: infoPath) + let data = try Data(contentsOf: URL(filePath: infoPath.string)) let fs = try decoder.decode(Filesystem.self, from: data) return fs } @@ -173,49 +173,42 @@ public actor SnapshotStore { toKeep.append(desc.digest.trimmingDigestPrefix) } } - let all = try self.fm.contentsOfDirectory(at: self.path, includingPropertiesForKeys: [.totalFileAllocatedSizeKey]).map { - $0.lastPathComponent - } + let all = try self.fm.contentsOfDirectory(atPath: self.path.string) let delete = Set(all).subtracting(Set(toKeep)) var deletedBytes: UInt64 = 0 for dir in delete { - let unpackedPath = self.path.appending(path: dir, directoryHint: .isDirectory) - guard self.fm.fileExists(atPath: unpackedPath.absolutePath()) else { + let unpackedPath = self.path.appending(dir) + guard self.fm.fileExists(atPath: unpackedPath.string) else { continue } deletedBytes += (try? self.fm.directorySize(dir: unpackedPath)) ?? 0 - try self.fm.removeItem(at: unpackedPath) + try self.fm.removeItem(atPath: unpackedPath.string) } return deletedBytes } - private func snapshotDir(_ desc: Descriptor) -> URL { - let p = self.path.appendingPathComponent(desc.digest.trimmingDigestPrefix, isDirectory: true) - return p + private func snapshotDir(_ desc: Descriptor) -> FilePath { + self.path.appending(desc.digest.trimmingDigestPrefix) } - private func snapshotPath(_ desc: Descriptor) -> URL { - let p = self.snapshotDir(desc) - .appendingPathComponent(Self.snapshotFileName, isDirectory: false) - return p + private func snapshotPath(_ desc: Descriptor) -> FilePath { + self.snapshotDir(desc).appending(Self.snapshotFileName) } - private func snapshotInfoPath(_ desc: Descriptor) -> URL { - let p = self.snapshotDir(desc) - .appendingPathComponent(Self.snapshotInfoFileName, isDirectory: false) - return p + private func snapshotInfoPath(_ desc: Descriptor) -> FilePath { + self.snapshotDir(desc).appending(Self.snapshotInfoFileName) } - private func tempUnpackDir() throws -> URL { - let uniqueDirectoryURL = ingestDir.appendingPathComponent(UUID().uuidString, isDirectory: true) - try self.fm.createDirectory(at: uniqueDirectoryURL, withIntermediateDirectories: true, attributes: nil) - return uniqueDirectoryURL + private func tempUnpackDir() throws -> FilePath { + let uniqueDir = ingestDir.appending(UUID().uuidString) + try self.fm.createDirectory(atPath: uniqueDir.string, withIntermediateDirectories: true, attributes: nil) + return uniqueDir } /// Get the disk size for a specific snapshot descriptor public func getSnapshotSize(descriptor: Descriptor) throws -> UInt64 { let snapshotPath = self.snapshotDir(descriptor) - guard self.fm.fileExists(atPath: snapshotPath.path) else { + guard self.fm.fileExists(atPath: snapshotPath.string) else { return 0 } return try self.fm.directorySize(dir: snapshotPath) @@ -223,13 +216,15 @@ public actor SnapshotStore { } extension FileManager { - fileprivate func directorySize(dir: URL) throws -> UInt64 { + fileprivate func directorySize(dir: FilePath) throws -> UInt64 { var size: UInt64 = 0 let resourceKeys: [URLResourceKey] = [.totalFileAllocatedSizeKey] + // URL boundary: URLResourceKey.totalFileAllocatedSizeKey is a Foundation API + let dirURL = URL(fileURLWithPath: dir.string, isDirectory: true) guard let enumerator = self.enumerator( - at: dir, + at: dirURL, includingPropertiesForKeys: resourceKeys, options: [.skipsHiddenFiles] ) From 8b2c540a12ba6976a213c3e6e7a95255d18e89f4 Mon Sep 17 00:00:00 2001 From: Chris George Date: Fri, 8 May 2026 17:18:23 -0700 Subject: [PATCH 2/2] Refine URL boundary comment in directorySize. Clarifies that the URL leak is required by the URLEnumerator API and has no FilePath equivalent (vs. the prior "is a Foundation API" wording, which underspecified the constraint). --- .../Services/ContainerImagesService/Server/SnapshotStore.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Services/ContainerImagesService/Server/SnapshotStore.swift b/Sources/Services/ContainerImagesService/Server/SnapshotStore.swift index fe52fd871..4f170c6ed 100644 --- a/Sources/Services/ContainerImagesService/Server/SnapshotStore.swift +++ b/Sources/Services/ContainerImagesService/Server/SnapshotStore.swift @@ -220,7 +220,7 @@ extension FileManager { var size: UInt64 = 0 let resourceKeys: [URLResourceKey] = [.totalFileAllocatedSizeKey] - // URL boundary: URLResourceKey.totalFileAllocatedSizeKey is a Foundation API + // URL boundary: required by URLEnumerator API; no FilePath equivalent let dirURL = URL(fileURLWithPath: dir.string, isDirectory: true) guard let enumerator = self.enumerator(