diff --git a/Sources/APIServer/APIServer+Start.swift b/Sources/APIServer/APIServer+Start.swift index 0c1b82a91..3e245ec20 100644 --- a/Sources/APIServer/APIServer+Start.swift +++ b/Sources/APIServer/APIServer+Start.swift @@ -64,9 +64,13 @@ extension APIServer { let pluginLoader = try initializePluginLoader(log: log) try await initializePlugins(pluginLoader: pluginLoader, log: log, routes: &routes, debug: debug) + // Shared by the containers and volumes services so a volume's secret is written, + // read, and removed through the same keychain domain. + let credentialStore = KeychainVolumeCredentialStore() let containersService = try initializeContainersService( pluginLoader: pluginLoader, containerSystemConfig: containerSystemConfig, + credentialStore: credentialStore, log: log, routes: &routes ) @@ -80,7 +84,12 @@ extension APIServer { await containersService.setNetworksService(networkService) initializeHealthCheckService(log: log, routes: &routes) try initializeKernelService(log: log, routes: &routes) - let volumesService = try await initializeVolumeService(containersService: containersService, log: log, routes: &routes) + let volumesService = try await initializeVolumeService( + containersService: containersService, + credentialStore: credentialStore, + log: log, + routes: &routes + ) try initializeDiskUsageService( containersService: containersService, volumesService: volumesService, @@ -274,6 +283,7 @@ extension APIServer { private func initializeContainersService( pluginLoader: PluginLoader, containerSystemConfig: ContainerSystemConfig, + credentialStore: VolumeCredentialStore, log: Logger, routes: inout [XPCRoute: XPCServer.RouteHandler] ) throws -> ContainersService { @@ -286,7 +296,8 @@ extension APIServer { pluginLoader: pluginLoader, containerSystemConfig: containerSystemConfig, log: log, - debugHelpers: debug + debugHelpers: debug, + credentialStore: credentialStore ) let harness = ContainersHarness(service: service, log: log) @@ -360,13 +371,19 @@ extension APIServer { private func initializeVolumeService( containersService: ContainersService, + credentialStore: VolumeCredentialStore, log: Logger, routes: inout [XPCRoute: XPCServer.RouteHandler] ) async throws -> VolumesService { log.info("initializing volume service") let resourceRoot = appRoot.appending(FilePath.Component("volumes")) - let service = try await VolumesService(resourceRoot: resourceRoot, containersService: containersService, log: log) + let service = try await VolumesService( + resourceRoot: resourceRoot, + containersService: containersService, + log: log, + credentialStore: credentialStore + ) let harness = VolumesHarness(service: service, log: log) routes[XPCRoute.volumeCreate] = XPCServer.route(harness.create) diff --git a/Sources/ContainerCommands/Volume/VolumeCreate.swift b/Sources/ContainerCommands/Volume/VolumeCreate.swift index cc41fd215..2965c4b86 100644 --- a/Sources/ContainerCommands/Volume/VolumeCreate.swift +++ b/Sources/ContainerCommands/Volume/VolumeCreate.swift @@ -16,6 +16,7 @@ import ArgumentParser import ContainerAPIClient +import ContainerResource import Foundation extension Application.VolumeCommand { @@ -25,6 +26,9 @@ extension Application.VolumeCommand { abstract: "Create a new volume" ) + @Option(name: .customLong("driver"), help: "Volume driver: local, smb, or nfs (default: local; smb and nfs are experimental)") + var driver: String = "local" + @Option(name: .customLong("label"), help: "Set metadata for a volume") var labels: [String] = [] @@ -51,9 +55,18 @@ extension Application.VolumeCommand { parsedDriverOpts["size"] = size } + // The server validates the options too, but checking here fails fast with a CLI error. + if driver == "smb" || driver == "nfs" { + do { + try MountOptionValidator.validate(parsedDriverOpts) + } catch let violation as MountOptionValidator.Violation { + throw ValidationError(violation.description) + } + } + let volume = try await ClientVolume.create( name: name, - driver: "local", + driver: driver, driverOpts: parsedDriverOpts, labels: parsedLabels ) diff --git a/Sources/ContainerCommands/Volume/VolumeResource+ListDisplayable.swift b/Sources/ContainerCommands/Volume/VolumeResource+ListDisplayable.swift index 70cc5a1bd..f832cb260 100644 --- a/Sources/ContainerCommands/Volume/VolumeResource+ListDisplayable.swift +++ b/Sources/ContainerCommands/Volume/VolumeResource+ListDisplayable.swift @@ -26,7 +26,13 @@ extension VolumeResource: ListDisplayable { name, isAnonymous ? "anonymous" : "named", configuration.driver, - configuration.options.isEmpty ? "" : configuration.options.sorted(by: { $0.key < $1.key }).map { "\($0.key)=\($0.value)" }.joined(separator: ","), + // The SMB password is kept in the keychain and never stored in the options, so this + // redaction is defence in depth for a volume written by something other than the API server. + configuration.options.isEmpty + ? "" + : configuration.options.sorted(by: { $0.key < $1.key }) + .map { "\($0.key)=\($0.key == "password" ? "***" : $0.value)" } + .joined(separator: ","), ] } diff --git a/Sources/ContainerResource/Container/Filesystem.swift b/Sources/ContainerResource/Container/Filesystem.swift index 445a87621..25da0d6f4 100644 --- a/Sources/ContainerResource/Container/Filesystem.swift +++ b/Sources/ContainerResource/Container/Filesystem.swift @@ -58,6 +58,8 @@ public struct Filesystem: Sendable, Codable { case block(format: String, cache: CacheMode, sync: SyncMode) case volume(name: String, format: String, cache: CacheMode, sync: SyncMode) + case smb(name: String? = nil, share: String, mountOptions: [String: String]) + case nfs(name: String? = nil, share: String, mountOptions: [String: String]) case virtiofs case tmpfs } @@ -115,6 +117,30 @@ public struct Filesystem: Sendable, Codable { ) } + /// An SMB share mounted directly inside the guest via CIFS. + public static func smb( + name: String? = nil, share: String, mountOptions: [String: String], destination: String, options: MountOptions + ) -> Filesystem { + .init( + type: .smb(name: name, share: share, mountOptions: mountOptions), + source: share, + destination: destination, + options: options + ) + } + + /// An NFS share mounted directly inside the guest. + public static func nfs( + name: String? = nil, share: String, mountOptions: [String: String], destination: String, options: MountOptions + ) -> Filesystem { + .init( + type: .nfs(name: name, share: share, mountOptions: mountOptions), + source: share, + destination: destination, + options: options + ) + } + /// A vritiofs backed filesystem providing a directory. public static func virtiofs(source: String, destination: String, options: MountOptions) -> Filesystem { .init( @@ -134,6 +160,22 @@ public struct Filesystem: Sendable, Codable { ) } + /// Returns true if the Filesystem is an SMB volume. + public var isSMB: Bool { + switch type { + case .smb(_, _, _): true + default: false + } + } + + /// Returns true if the Filesystem is an NFS volume. + public var isNFS: Bool { + switch type { + case .nfs(_, _, _): true + default: false + } + } + /// Returns true if the Filesystem is backed by a block device. public var isBlock: Bool { switch type { @@ -145,20 +187,80 @@ public struct Filesystem: Sendable, Codable { /// Returns true if the Filesystem is a named volume. public var isVolume: Bool { - switch type { - case .volume(_, _, _, _): true - default: false - } + volumeName != nil } /// Returns the volume name if this is a volume filesystem, nil otherwise. public var volumeName: String? { switch type { case .volume(let name, _, _, _): name + case .smb(let name, _, _): name + case .nfs(let name, _, _): name default: nil } } + /// Returns the combined mount options for this filesystem, merging any + /// driver-specific mount options (excluding `share`) in deterministic key order + /// with the general mount options. + package var resolvedMountOptions: MountOptions { + switch type { + case .smb(_, _, let mountOptions), .nfs(_, _, let mountOptions): + return mountOptions.filter { $0.key != MountOptionValidator.shareKey } + .sorted { $0.key < $1.key } + .map { $0.value.isEmpty ? $0.key : "\($0.key)=\($0.value)" } + self.options + default: + return self.options + } + } + + /// Checks that the share and driver mount options of an SMB or NFS filesystem are syntactically valid. + /// + /// The share is checked as a mount source and the remaining options as the values that are joined + /// into the mount options string, matching how `resolvedMountOptions` treats them. Other filesystem + /// types carry no driver mount options and always pass. + /// + /// - Throws: `MountOptionValidator.Violation` for the first value that breaks a rule. + package func validateMountOptions() throws { + switch type { + case .smb(_, let share, let mountOptions), .nfs(_, let share, let mountOptions): + try MountOptionValidator.validateShare(share) + try MountOptionValidator.validate(mountOptions) + default: + break + } + } + + /// Checks that an SMB or NFS filesystem carries no secret in its mount options. + /// + /// Stored configurations must never hold a password; for SMB it lives in the credential store and + /// is added to the mount options in memory at bootstrap, and NFS has no password option at all. + /// Other filesystem types always pass. + /// + /// - Throws: `MountOptionValidator.Violation` naming the offending key. + package func validateNoSecretOptions() throws { + switch type { + case .smb(_, _, let mountOptions), .nfs(_, _, let mountOptions): + try MountOptionValidator.validateNoSecrets(mountOptions) + default: + break + } + } + + /// Returns a copy of an SMB filesystem with the password added to its mount options. + /// + /// This is used at bootstrap to combine the stored, secret-free configuration with the password + /// read from the credential store. Other filesystem types are returned unchanged. + package func addingSMBPassword(_ password: String) -> Filesystem { + guard case .smb(let name, let share, var mountOptions) = type else { + return self + } + mountOptions[MountOptionValidator.passwordKey] = password + var copy = self + copy.type = .smb(name: name, share: share, mountOptions: mountOptions) + return copy + } + /// Returns true if the Filesystem is backed by a in-memory mount type. public var isTmpfs: Bool { switch type { diff --git a/Sources/ContainerResource/Volume/MountOptionValidator.swift b/Sources/ContainerResource/Volume/MountOptionValidator.swift new file mode 100644 index 000000000..df4a63fe7 --- /dev/null +++ b/Sources/ContainerResource/Volume/MountOptionValidator.swift @@ -0,0 +1,127 @@ +//===----------------------------------------------------------------------===// +// 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. +//===----------------------------------------------------------------------===// + +/// Checks that the share and driver options of a network-backed volume are syntactically valid. +/// +/// The options are joined into a comma-separated `key=value` string before they reach the guest +/// kernel's mount, so a value that contains a comma, whitespace, or a control character could smuggle +/// in extra options. The share is different: it is the mount source, which is passed to `mount(2)` as +/// its own argument and never joined into the options string, so it only needs to be a well-formed +/// string. The API server checks both when a volume is created and again when a container is created, +/// and the runtime checks them once more before mounting, so none of them takes values read from a +/// client or from storage on faith. +package enum MountOptionValidator { + /// The key under which the share is carried in the driver options. + package static let shareKey = "share" + + /// The most characters a key may have. + package static let keyLengthMax = 64 + + /// The most characters a value may have. + package static let valueLengthMax = 1024 + + /// The most characters a share may have. + package static let shareLengthMax = 4096 + + /// The option key under which an SMB password is given to `container volume create`. + package static let passwordKey = "password" + + /// The SMB mount option keys that carry or point at a secret. + /// + /// `pass` is an alias of `password`, and `credentials`/`cred` name a file of credentials. None of + /// them may appear in a stored volume or container configuration; the password is kept in the + /// credential store instead and added to the mount options only at bootstrap. + package static let secretKeys: Set = [passwordKey, "pass", "credentials", "cred"] + + /// A rule that an option breaks. The description says which option and why. + /// + /// The description never includes the value, since a value may be a credential. + package struct Violation: Error, CustomStringConvertible, Equatable { + package let description: String + + init(_ description: String) { + self.description = description + } + } + + /// Checks that the share of a network mount is well formed. + /// + /// A share may contain spaces and commas, which are common in SMB share names, since it is never + /// joined into the options string. It must not be empty or contain control characters, which would + /// truncate or corrupt the C string handed to `mount(2)`. + /// + /// - Parameter share: The share, such as `//server/share` or `server:/export/path`. + /// - Throws: `Violation` if the share breaks a rule. + package static func validateShare(_ share: String) throws { + guard !share.isEmpty else { + throw Violation("share must not be empty") + } + let sharePattern = #/^[^\p{Cc}]{1,4096}$/# + guard share.wholeMatch(of: sharePattern) != nil else { + throw Violation("invalid share: must not contain control characters, with at most \(Self.shareLengthMax) characters") + } + } + + /// Checks that `options` carries no secret. + /// + /// - Parameter options: The mount options of an SMB or NFS filesystem. + /// - Throws: `Violation` naming the first secret key found, in key order. + package static func validateNoSecrets(_ options: [String: String]) throws { + if let key = options.keys.sorted().first(where: Self.secretKeys.contains) { + throw Violation( + "mount option \"\(key)\" is not allowed here; give the password to `container volume create --driver smb` so it is kept in the keychain, or delete and recreate the volume if it was created by an earlier build" + ) + } + } + + /// Checks that every key and value in `options` follows the rules. + /// + /// - A key starts with a letter and contains only letters, digits, `_`, and `-`, with at most + /// `keyLengthMax` characters. + /// - A value contains no commas, whitespace, or control characters, with at most `valueLengthMax` + /// characters. An empty value denotes a bare flag such as `nolock`. + /// + /// The share, if present under `shareKey`, is checked with `validateShare` instead, since it is + /// not joined into the options string. Options are checked in key order, so the same input always + /// produces the same error. + /// + /// - Parameter options: The driver options, as parsed from `--opt key=value`. + /// - Throws: `Violation` for the first option that breaks a rule. + package static func validate(_ options: [String: String]) throws { + let keyPattern = #/^[A-Za-z][A-Za-z0-9_-]{0,63}$/# + let valuePattern = #/^[^,\s\p{Cc}]{0,1024}$/# + + for key in options.keys.sorted() { + guard let value = options[key] else { + continue + } + if key == Self.shareKey { + try Self.validateShare(value) + continue + } + guard key.wholeMatch(of: keyPattern) != nil else { + throw Violation( + "invalid mount option key \"\(key)\": must start with a letter and contain only letters, digits, '_', or '-', with at most \(Self.keyLengthMax) characters" + ) + } + guard value.wholeMatch(of: valuePattern) != nil else { + throw Violation( + "invalid value for mount option \"\(key)\": must not contain commas, whitespace, or control characters, with at most \(Self.valueLengthMax) characters" + ) + } + } + } +} diff --git a/Sources/ContainerResource/Volume/VolumeConfiguration.swift b/Sources/ContainerResource/Volume/VolumeConfiguration.swift index e7c1a668e..761acb1fd 100644 --- a/Sources/ContainerResource/Volume/VolumeConfiguration.swift +++ b/Sources/ContainerResource/Volume/VolumeConfiguration.swift @@ -113,6 +113,7 @@ public enum VolumeError: Error, LocalizedError { case volumeInUse(String) case invalidVolumeName(String) case driverNotSupported(String) + case invalidOption(String) case storageError(String) public var errorDescription: String? { @@ -127,6 +128,8 @@ public enum VolumeError: Error, LocalizedError { return "invalid volume name '\(name)'" case .driverNotSupported(let driver): return "volume driver '\(driver)' is not supported" + case .invalidOption(let message): + return "invalid volume option: \(message)" case .storageError(let message): return "storage error: \(message)" } diff --git a/Sources/Services/ContainerAPIService/Client/Constants.swift b/Sources/Services/ContainerAPIService/Client/Constants.swift index c5ab8fe63..05ad1ea35 100644 --- a/Sources/Services/ContainerAPIService/Client/Constants.swift +++ b/Sources/Services/ContainerAPIService/Client/Constants.swift @@ -18,4 +18,7 @@ public enum Constants { /// The keychain ID to use for registry credentials. public static let keychainID = "com.apple.container.registry" + + /// The keychain ID to use for volume credentials, such as SMB passwords. + public static let volumeKeychainID = "com.apple.container.volume" } diff --git a/Sources/Services/ContainerAPIService/Client/Utility.swift b/Sources/Services/ContainerAPIService/Client/Utility.swift index bc503ef32..ea3adcf47 100644 --- a/Sources/Services/ContainerAPIService/Client/Utility.swift +++ b/Sources/Services/ContainerAPIService/Client/Utility.swift @@ -175,13 +175,38 @@ public struct Utility { resolvedMounts.append(fs) case .volume(let parsed): let volume = try await getOrCreateVolume(parsed: parsed, log: log) - let volumeMount = Filesystem.volume( - name: parsed.name, - format: volume.format, - source: volume.source, - destination: parsed.destination, - options: parsed.options - ) + let volumeMount: Filesystem + switch volume.driver { + case "smb": + volumeMount = Filesystem.smb( + name: parsed.name, + share: volume.source, + mountOptions: volume.options, + destination: parsed.destination, + options: parsed.options + ) + case "nfs": + volumeMount = Filesystem.nfs( + name: parsed.name, + share: volume.source, + mountOptions: volume.options, + destination: parsed.destination, + options: parsed.options + ) + case "local": + volumeMount = Filesystem.volume( + name: parsed.name, + format: volume.format, + source: volume.source, + destination: parsed.destination, + options: parsed.options + ) + default: + throw ContainerizationError( + .invalidArgument, + message: "volume '\(parsed.name)' uses unsupported driver '\(volume.driver)'" + ) + } resolvedMounts.append(volumeMount) } } diff --git a/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift b/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift index b947212f5..145e8d273 100644 --- a/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift +++ b/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift @@ -58,6 +58,7 @@ public actor ContainersService { private let runtimePlugins: [Plugin] private let exitMonitor: ExitMonitor private let containerSystemConfig: ContainerSystemConfig + private let credentialStore: VolumeCredentialStore private let lock: AsyncLock private var containers: [String: ContainerState] @@ -70,7 +71,8 @@ public actor ContainersService { pluginLoader: PluginLoader, containerSystemConfig: ContainerSystemConfig, log: Logger, - debugHelpers: Bool = false + debugHelpers: Bool = false, + credentialStore: VolumeCredentialStore = KeychainVolumeCredentialStore() ) throws { let containerRoot = appRoot.appendingPathComponent("containers") try FileManager.default.createDirectory(at: containerRoot, withIntermediateDirectories: true) @@ -79,6 +81,7 @@ public actor ContainersService { self.containerRoot = containerRoot self.pluginLoader = pluginLoader self.containerSystemConfig = containerSystemConfig + self.credentialStore = credentialStore self.log = log self.debugHelpers = debugHelpers self.runtimePlugins = pluginLoader.findPlugins().filter { $0.hasType(.runtime) } @@ -318,6 +321,21 @@ public actor ContainersService { ) } + // Volumes are validated when they are created, but a client can also attach an SMB + // or NFS filesystem directly, so check the mount options here as well. An SMB password + // is never accepted here; it belongs in the credential store, keyed by volume name. + for mount in configuration.mounts { + do { + try mount.validateMountOptions() + try mount.validateNoSecretOptions() + } catch let violation as MountOptionValidator.Violation { + throw ContainerizationError( + .invalidArgument, + message: "invalid mount options for \(mount.destination): \(violation)" + ) + } + } + let path = self.containerRoot.appendingPathComponent(configuration.id) let systemPlatform = kernel.platform @@ -414,6 +432,10 @@ public actor ContainersService { networkBootstrapInfos.append(NetworkBootstrapInfo(plugin: plugin)) } + // Resolve SMB passwords here, where the credential store is, and hand them to the + // runtime for this bootstrap only. They are never written to the bundle or logged. + let mountCredentials = try self.resolveMountCredentials(for: config) + do { try Self.registerService( plugin: self.runtimePlugins.first { $0.name == config.runtimeHandler }!, @@ -428,7 +450,12 @@ public actor ContainersService { id: id, runtime: runtime ) - try await runtimeClient.bootstrap(stdio: stdio, networkBootstrapInfos: networkBootstrapInfos, dynamicEnv: dynamicEnv) + try await runtimeClient.bootstrap( + stdio: stdio, + networkBootstrapInfos: networkBootstrapInfos, + dynamicEnv: dynamicEnv, + mountCredentials: mountCredentials + ) try await self.exitMonitor.registerProcess( id: id, @@ -450,6 +477,33 @@ public actor ContainersService { } } + /// Returns the SMB passwords for the volumes a container mounts, keyed by volume name. + /// + /// A volume without a stored credential is left out, so the share is mounted without a + /// password (for example, a guest share). Any other failure to read the credential store + /// fails the bootstrap, since starting the container would otherwise fail later with a + /// less useful permission error from the guest. + private nonisolated func resolveMountCredentials(for config: ContainerConfiguration) throws -> [String: String] { + var credentials = [String: String]() + for mount in config.mounts { + guard mount.isSMB, let name = mount.volumeName, credentials[name] == nil else { + continue + } + do { + if let credential = try self.credentialStore.lookup(volume: name) { + credentials[name] = credential.password + } + } catch { + throw ContainerizationError( + .internalError, + message: "failed to read the credential for volume \(name) from the keychain", + cause: error + ) + } + } + return credentials + } + /// Create a new process in the container. public func createProcess( id: String, diff --git a/Sources/Services/ContainerAPIService/Server/Volumes/VolumeCredentialStore.swift b/Sources/Services/ContainerAPIService/Server/Volumes/VolumeCredentialStore.swift new file mode 100644 index 000000000..584178c0e --- /dev/null +++ b/Sources/Services/ContainerAPIService/Server/Volumes/VolumeCredentialStore.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 ContainerAPIClient +import ContainerizationOS +import Foundation + +/// A credential stored for a volume, such as the username and password of an SMB share. +public struct VolumeCredential: Sendable, Equatable { + public let username: String + public let password: String + + public init(username: String, password: String) { + self.username = username + self.password = password + } +} + +/// Stores the secrets that a volume needs at mount time, keyed by volume name. +/// +/// The password of an SMB volume is kept here rather than in `VolumeConfiguration.options`, so it +/// is never written to the volume store, the container bundle, or shown by `volume inspect`. The +/// API server writes the secret when the volume is created, reads it back when a container that +/// mounts the volume is bootstrapped, and removes it when the volume is deleted. +public protocol VolumeCredentialStore: Sendable { + /// Saves the credential for a volume, replacing any existing credential. + func save(volume: String, credential: VolumeCredential) throws + + /// Returns the credential for a volume, or nil if none is stored. + func lookup(volume: String) throws -> VolumeCredential? + + /// Removes the credential for a volume. Removing a credential that does not exist is not an error. + func delete(volume: String) throws +} + +/// A `VolumeCredentialStore` backed by the macOS keychain. +/// +/// Items are stored as internet passwords under `Constants.volumeKeychainID`, with the volume name as +/// the server attribute, which keeps them apart from the registry credentials that `container registry +/// login` stores under `Constants.keychainID`. +public struct KeychainVolumeCredentialStore: VolumeCredentialStore { + private let securityDomain: String + + public init(securityDomain: String = Constants.volumeKeychainID) { + self.securityDomain = securityDomain + } + + public func save(volume: String, credential: VolumeCredential) throws { + try KeychainQuery().save( + securityDomain: self.securityDomain, + hostname: volume, + username: credential.username, + password: credential.password + ) + } + + public func lookup(volume: String) throws -> VolumeCredential? { + guard let result = try KeychainQuery().get(securityDomain: self.securityDomain, hostname: volume) else { + return nil + } + return VolumeCredential(username: result.username, password: result.password) + } + + public func delete(volume: String) throws { + try KeychainQuery().delete(securityDomain: self.securityDomain, hostname: volume) + } +} diff --git a/Sources/Services/ContainerAPIService/Server/Volumes/VolumesService.swift b/Sources/Services/ContainerAPIService/Server/Volumes/VolumesService.swift index fe607e762..96eb8bff0 100644 --- a/Sources/Services/ContainerAPIService/Server/Volumes/VolumesService.swift +++ b/Sources/Services/ContainerAPIService/Server/Volumes/VolumesService.swift @@ -29,6 +29,7 @@ import SystemPackage public actor VolumesService { private let resourceRoot: FilePath private let store: ContainerPersistence.FilesystemEntityStore + private let credentialStore: VolumeCredentialStore private let log: Logger private let lock = AsyncLock() private let containersService: ContainersService @@ -37,10 +38,16 @@ public actor VolumesService { private static let entityFile = "entity.json" private static let blockFile = "volume.img" - public init(resourceRoot: FilePath, containersService: ContainersService, log: Logger) async throws { + public init( + resourceRoot: FilePath, + containersService: ContainersService, + log: Logger, + credentialStore: VolumeCredentialStore = KeychainVolumeCredentialStore() + ) async throws { try FileManager.default.createDirectory(atPath: resourceRoot.string, withIntermediateDirectories: true) self.resourceRoot = resourceRoot self.store = try FilesystemEntityStore(path: resourceRoot, type: "volumes", log: log) + self.credentialStore = credentialStore self.containersService = containersService self.log = log @@ -286,6 +293,31 @@ public actor VolumesService { return EXT4.JournalConfig(size: size, defaultMode: mode) } + /// Validates the driver options of an `smb` or `nfs` volume. + /// + /// - Parameters: + /// - driverOpts: The options passed with `--opt key=value`. + /// - driver: The driver name, used in error messages. + /// - shareExample: An example `share` value for the driver, used in error messages. + /// - Returns: The non-empty `share` option. + /// - Throws: `VolumeError.invalidOption` if `share` is missing or empty, if `size` is present, + /// or if the share or any option is not syntactically valid. + static func validateNetworkDriverOptions(_ driverOpts: [String: String], driver: String, shareExample: String) throws -> String { + guard let share = driverOpts[MountOptionValidator.shareKey], !share.isEmpty else { + throw VolumeError.invalidOption("\(driver) driver requires --opt share=\(shareExample)") + } + if driverOpts["size"] != nil { + throw VolumeError.invalidOption("size option is not supported for \(driver) driver") + } + do { + // Checks the share as a mount source and the remaining options as mount options. + try MountOptionValidator.validate(driverOpts) + } catch let violation as MountOptionValidator.Violation { + throw VolumeError.invalidOption(violation.description) + } + return share + } + private func createVolumeImage(for name: String, sizeInBytes: UInt64 = VolumeStorage.defaultVolumeSizeBytes, journal: EXT4.JournalConfig? = nil) throws { let blockPath = blockPath(for: name) @@ -325,31 +357,82 @@ public actor VolumesService { throw VolumeError.volumeAlreadyExists(name) } - try createVolumeDirectory(for: name) - - // Parse size from driver options (default 512GB) - let sizeInBytes: UInt64 - if let sizeString = driverOpts["size"] { - sizeInBytes = try parseSize(sizeString) - } else { - sizeInBytes = VolumeStorage.defaultVolumeSizeBytes - } + let volume: VolumeConfiguration + var credential: VolumeCredential? + switch driver { + case "smb": + let share = try Self.validateNetworkDriverOptions(driverOpts, driver: driver, shareExample: "//server/share") + var options = driverOpts + credential = try Self.extractSMBCredential(from: &options) + volume = VolumeConfiguration( + name: name, + driver: "smb", + format: "cifs", + source: share, + labels: labels, + options: options, + sizeInBytes: nil + ) + case "nfs": + let share = try Self.validateNetworkDriverOptions(driverOpts, driver: driver, shareExample: "server:/export/path") + // NFS has no password option, so a secret here could only end up stored in plain text. + if let key = driverOpts.keys.sorted().first(where: MountOptionValidator.secretKeys.contains) { + throw VolumeError.invalidOption("nfs driver does not support option '\(key)'") + } + volume = VolumeConfiguration( + name: name, + driver: "nfs", + format: "nfs", + source: share, + labels: labels, + options: driverOpts, + sizeInBytes: nil + ) + case "local": + try createVolumeDirectory(for: name) + + // Parse size from driver options, or use the system default value + let sizeInBytes: UInt64 + if let sizeString = driverOpts["size"] { + sizeInBytes = try parseSize(sizeString) + } else { + sizeInBytes = VolumeStorage.defaultVolumeSizeBytes + } - let journalConfig = try driverOpts["journal"].map { try Self.parseJournalConfig($0) } + let journalConfig = try driverOpts["journal"].map { try Self.parseJournalConfig($0) } - try createVolumeImage(for: name, sizeInBytes: sizeInBytes, journal: journalConfig) + try createVolumeImage(for: name, sizeInBytes: sizeInBytes, journal: journalConfig) - let volume = VolumeConfiguration( - name: name, - driver: driver, - format: "ext4", - source: blockPath(for: name), - labels: labels, - options: driverOpts, - sizeInBytes: sizeInBytes - ) + volume = VolumeConfiguration( + name: name, + driver: "local", + format: "ext4", + source: blockPath(for: name), + labels: labels, + options: driverOpts, + sizeInBytes: sizeInBytes + ) + default: + throw VolumeError.driverNotSupported(driver) + } - try await store.create(volume) + // Store the secret before the volume so a volume never exists without its credential. + // If the volume cannot then be stored, remove the credential again. + if let credential { + do { + try credentialStore.save(volume: name, credential: credential) + } catch { + throw VolumeError.storageError("failed to store the password for volume '\(name)' in the keychain: \(error)") + } + } + do { + try await store.create(volume) + } catch { + if credential != nil { + try? credentialStore.delete(volume: name) + } + throw error + } log.info( "created volume", @@ -357,10 +440,34 @@ public actor VolumesService { "name": "\(name)", "driver": "\(driver)", "isAnonymous": "\(volume.isAnonymous)", + "hasCredential": "\(credential != nil)", ]) return volume } + /// Removes the SMB password from `options` and returns it with its username as a credential. + /// + /// Only `password` is accepted as the key for the secret. The other keys that cifs treats as a + /// password or a credentials file are rejected so a secret cannot slip into the stored options + /// under another name. + /// + /// - Returns: The credential, or nil if no password was given. + /// - Throws: `VolumeError.invalidOption` if a password is given without a username, or if a + /// secret key other than `password` is present. + static func extractSMBCredential(from options: inout [String: String]) throws -> VolumeCredential? { + let otherSecretKeys = MountOptionValidator.secretKeys.subtracting([MountOptionValidator.passwordKey]) + if let key = options.keys.sorted().first(where: otherSecretKeys.contains) { + throw VolumeError.invalidOption("smb option '\(key)' is not supported; use --opt \(MountOptionValidator.passwordKey)=") + } + guard let password = options.removeValue(forKey: MountOptionValidator.passwordKey) else { + return nil + } + guard let username = options["username"], !username.isEmpty else { + throw VolumeError.invalidOption("smb driver requires --opt username= when a password is given") + } + return VolumeCredential(username: username, password: password) + } + private func _delete(name: String) async throws { guard VolumeStorage.isValidVolumeName(name) else { throw VolumeError.invalidVolumeName("invalid volume name '\(name)': must match \(VolumeStorage.volumeNamePattern)") @@ -368,7 +475,7 @@ public actor VolumesService { // Check if volume exists by trying to list and finding it let existingVolumes = try await store.list() - guard existingVolumes.contains(where: { $0.name == name }) else { + guard let volume = existingVolumes.first(where: { $0.name == name }) else { throw VolumeError.volumeNotFound(name) } @@ -386,6 +493,20 @@ public actor VolumesService { try self.removeVolumeDirectory(for: name) } + // The credential is removed last so a failure to delete the volume never leaves a volume + // without its credential. A failure here is logged rather than thrown, since the volume is + // already gone and the next create under the same name replaces the item anyway. + if volume.driver == "smb" { + do { + try credentialStore.delete(volume: name) + } catch { + log.warning( + "failed to remove the credential for a deleted volume from the keychain", + metadata: ["name": "\(name)", "error": "\(error)"] + ) + } + } + log.info("deleted volume", metadata: ["name": "\(name)"]) } diff --git a/Sources/Services/Runtime/RuntimeClient/RuntimeClient.swift b/Sources/Services/Runtime/RuntimeClient/RuntimeClient.swift index e05ef2ba2..6017e05d3 100644 --- a/Sources/Services/Runtime/RuntimeClient/RuntimeClient.swift +++ b/Sources/Services/Runtime/RuntimeClient/RuntimeClient.swift @@ -80,7 +80,8 @@ extension RuntimeClient { public func bootstrap( stdio: [FileHandle?], networkBootstrapInfos: [NetworkBootstrapInfo], - dynamicEnv: [String: String] = [:] + dynamicEnv: [String: String] = [:], + mountCredentials: [String: String] = [:] ) async throws { let request = XPCMessage(route: RuntimeRoutes.bootstrap.rawValue) @@ -106,6 +107,11 @@ extension RuntimeClient { let infosData = try JSONEncoder().encode(networkBootstrapInfos) request.set(key: RuntimeKeys.networkBootstrapInfos.rawValue, value: infosData) + + if !mountCredentials.isEmpty { + let credentialsData = try JSONEncoder().encode(mountCredentials) + request.set(key: RuntimeKeys.mountCredentials.rawValue, value: credentialsData) + } try await self.client.send(request) } catch { throw ContainerizationError( diff --git a/Sources/Services/Runtime/RuntimeClient/RuntimeKeys.swift b/Sources/Services/Runtime/RuntimeClient/RuntimeKeys.swift index 1d3548cfe..4e8bad774 100644 --- a/Sources/Services/Runtime/RuntimeClient/RuntimeKeys.swift +++ b/Sources/Services/Runtime/RuntimeClient/RuntimeKeys.swift @@ -56,4 +56,7 @@ public enum RuntimeKeys: String { /// Per-network connection info passed to the runtime so it can allocate directly. case networkBootstrapInfos + + /// Secrets for volume mounts, keyed by volume name, resolved by the API server at bootstrap. + case mountCredentials } diff --git a/Sources/Services/RuntimeLinux/Server/RuntimeService.swift b/Sources/Services/RuntimeLinux/Server/RuntimeService.swift index 8ecc932d4..c0f45fa86 100644 --- a/Sources/Services/RuntimeLinux/Server/RuntimeService.swift +++ b/Sources/Services/RuntimeLinux/Server/RuntimeService.swift @@ -153,11 +153,13 @@ public actor RuntimeService { } let dynamicEnv = try message.dynamicEnv() + let mountCredentials = try message.mountCredentials() let bundle = ContainerResource.Bundle(path: self.root) try bundle.createLogFile() var config = try bundle.configuration + try Self.validateStoredMountsCarryNoSecrets(config) var kernel = try bundle.kernel // Built-in defaults keyed by arg name. Each is applied only if the user did not already @@ -271,7 +273,13 @@ public actor RuntimeService { memoryInBytes: config.resources.memoryInBytes + VMResources.guestMemoryOverhead ) let container = try LinuxContainer(id, rootfs: rootfs, vmm: vmm, vm: vmResources, logger: self.log) { czConfig in - try Self.configureContainer(czConfig: &czConfig, config: config, dynamicEnv: dynamicEnv, log: self.log) + try Self.configureContainer( + czConfig: &czConfig, + config: config, + dynamicEnv: dynamicEnv, + mountCredentials: mountCredentials, + log: self.log + ) czConfig.interfaces = interfaces czConfig.process.stdout = stdout czConfig.process.stderr = stderr @@ -1116,10 +1124,41 @@ public actor RuntimeService { } } + /// Checks that no mount in the stored configuration carries a secret. + /// + /// The password of an SMB volume lives in the API server's credential store and arrives with + /// the bootstrap message; it is never written to the bundle. A bundle that carries one has been + /// tampered with or written by something other than the API server, so the container does not start. + static func validateStoredMountsCarryNoSecrets(_ config: ContainerConfiguration) throws { + for mount in config.mounts { + do { + try mount.validateNoSecretOptions() + } catch let violation as MountOptionValidator.Violation { + throw ContainerizationError( + .invalidState, + message: "the stored configuration of container \(config.id) carries a secret for \(mount.destination): \(violation)" + ) + } + } + } + + /// Returns the mount with the password that the API server resolved for its volume, if any. + /// + /// Only an SMB mount with a volume name that has a credential is changed; every other mount is + /// returned as-is. The password is added to a copy that becomes the guest mount, not to the + /// configuration the runtime keeps and reports through its state. + static func applyMountCredential(from credentials: [String: String], to mount: Filesystem) -> Filesystem { + guard mount.isSMB, let name = mount.volumeName, let password = credentials[name] else { + return mount + } + return mount.addingSMBPassword(password) + } + private static func configureContainer( czConfig: inout LinuxContainer.Configuration, config: ContainerConfiguration, dynamicEnv: [String: String] = [:], + mountCredentials: [String: String] = [:], log: Logger? = nil, ) throws { czConfig.cpus = config.resources.cpus @@ -1152,6 +1191,17 @@ public actor RuntimeService { } for mount in config.mounts { + // The stored configuration is not trusted to be what the API server accepted, + // since the file can be damaged or edited, so check the mount options again. + do { + try mount.validateMountOptions() + } catch let violation as MountOptionValidator.Violation { + throw ContainerizationError( + .invalidState, + message: "the stored configuration of container \(config.id) has invalid mount options for \(mount.destination): \(violation)" + ) + } + if try mount.isSocket() { let attrs = try? FileManager.default.attributesOfItem(atPath: mount.source) let permissions = (attrs?[.posixPermissions] as? NSNumber) @@ -1164,7 +1214,9 @@ public actor RuntimeService { ) czConfig.sockets.append(socket) } else { - czConfig.mounts.append(mount.asMount) + // The SMB password is added only to the guest mount that is built here. `config` + // stays as stored, so the runtime never holds or reports the secret elsewhere. + czConfig.mounts.append(Self.applyMountCredential(from: mountCredentials, to: mount).asMount) } } @@ -1472,6 +1524,11 @@ extension XPCMessage { return dynamicEnv } + fileprivate func mountCredentials() throws -> [String: String] { + let data = self.dataNoCopy(key: RuntimeKeys.mountCredentials.rawValue) + return try data.map { try JSONDecoder().decode([String: String].self, from: $0) } ?? [:] + } + } extension ContainerResource.Bundle { @@ -1524,6 +1581,20 @@ extension Filesystem { "\(Filesystem.SyncMode.vzRuntimeOptionKey)=\(syncMode.asVZRuntimeOption)", ], ) + case .smb(_, let share, _): + return .any( + type: "cifs", + source: share, + destination: self.destination, + options: self.resolvedMountOptions + ) + case .nfs(_, let share, _): + return .any( + type: "nfs", + source: share, + destination: self.destination, + options: self.resolvedMountOptions + ) } } diff --git a/Tests/ContainerAPIClientTests/NFSVolumeTests.swift b/Tests/ContainerAPIClientTests/NFSVolumeTests.swift new file mode 100644 index 000000000..3363cb017 --- /dev/null +++ b/Tests/ContainerAPIClientTests/NFSVolumeTests.swift @@ -0,0 +1,112 @@ +//===----------------------------------------------------------------------===// +// 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 ContainerResource +import ContainerizationError +import Foundation +import Testing + +@testable import ContainerAPIClient + +struct NFSVolumeTests { + + // MARK: - Parser: named volume syntax still works for NFS volumes + + @Test("Named volume syntax resolves as a volume, not a filesystem") + func testNamedVolumeIsParsedAsVolume() throws { + let result = try Parser.volume("myexport:/data") + guard case .volume(let parsed) = result else { + Issue.record("Expected .volume, got .filesystem") + return + } + #expect(parsed.name == "myexport") + #expect(parsed.destination == "/data") + #expect(!parsed.isAnonymous) + } + + @Test("Named volume with options is parsed correctly") + func testNamedVolumeWithOptions() throws { + let result = try Parser.volume("myexport:/data:ro") + guard case .volume(let parsed) = result else { + Issue.record("Expected .volume") + return + } + #expect(parsed.name == "myexport") + #expect(parsed.destination == "/data") + #expect(parsed.options == ["ro"]) + } + + // MARK: - Volume model: NFS driver fields + + @Test("NFS volume stores share path as source") + func testNFSVolumeSource() { + let volume = VolumeConfiguration( + name: "myexport", + driver: "nfs", + format: "nfs", + source: "nas.local:/exports/data", + labels: [:], + options: ["vers": "4"], + sizeInBytes: nil + ) + + #expect(volume.driver == "nfs") + #expect(volume.format == "nfs") + #expect(volume.source == "nas.local:/exports/data") + #expect(volume.sizeInBytes == nil) + #expect(volume.options["vers"] == "4") + } + + @Test("NFS volume is not anonymous") + func testNFSVolumeIsNotAnonymous() { + let volume = VolumeConfiguration( + name: "myexport", + driver: "nfs", + format: "nfs", + source: "nas.local:/exports/data", + labels: [:] + ) + #expect(!volume.isAnonymous) + } + + @Test("Local volume driver defaults remain unchanged") + func testLocalVolumeDefaults() { + let volume = VolumeConfiguration( + name: "mydata", + source: "/var/lib/container/volumes/mydata/volume.img" + ) + #expect(volume.driver == "local") + #expect(volume.format == "ext4") + } + + // MARK: - Utility: parseKeyValuePairs for NFS driver opts + + @Test("NFS driver opts are parsed from --opt flags including bare flags") + func testNFSDriverOptsKeyValueParsing() { + let raw = ["share=nas.local:/exports/data", "vers=4", "nolock"] + let parsed = Utility.parseKeyValuePairs(raw) + + #expect(parsed["share"] == "nas.local:/exports/data") + #expect(parsed["vers"] == "4") + #expect(parsed["nolock"] == "") + } + + @Test("Missing share opt is detectable") + func testMissingNFSShareOpt() { + let opts = Utility.parseKeyValuePairs(["vers=4"]) + #expect(opts["share"] == nil) + } +} diff --git a/Tests/ContainerAPIClientTests/SMBVolumeTests.swift b/Tests/ContainerAPIClientTests/SMBVolumeTests.swift new file mode 100644 index 000000000..c9bbdcd1e --- /dev/null +++ b/Tests/ContainerAPIClientTests/SMBVolumeTests.swift @@ -0,0 +1,115 @@ +//===----------------------------------------------------------------------===// +// 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 ContainerResource +import ContainerizationError +import Foundation +import Testing + +@testable import ContainerAPIClient + +struct SMBVolumeTests { + + // MARK: - Parser: named volume syntax still works for SMB volumes + + @Test("Named volume syntax resolves as a volume, not a filesystem") + func testNamedVolumeIsParsedAsVolume() throws { + let result = try Parser.volume("myshare:/data") + guard case .volume(let parsed) = result else { + Issue.record("Expected .volume, got .filesystem") + return + } + #expect(parsed.name == "myshare") + #expect(parsed.destination == "/data") + #expect(!parsed.isAnonymous) + } + + @Test("Named volume with options is parsed correctly") + func testNamedVolumeWithOptions() throws { + let result = try Parser.volume("myshare:/data:ro") + guard case .volume(let parsed) = result else { + Issue.record("Expected .volume") + return + } + #expect(parsed.name == "myshare") + #expect(parsed.destination == "/data") + #expect(parsed.options == ["ro"]) + } + + // MARK: - Volume model: SMB driver fields + + @Test("SMB volume stores share path as source") + func testSMBVolumeSource() { + let volume = VolumeConfiguration( + name: "myshare", + driver: "smb", + format: "cifs", + source: "//server/share", + labels: [:], + options: ["username": "user", "password": "secret"], + sizeInBytes: nil + ) + + #expect(volume.driver == "smb") + #expect(volume.format == "cifs") + #expect(volume.source == "//server/share") + #expect(volume.sizeInBytes == nil) + #expect(volume.options["username"] == "user") + #expect(volume.options["password"] == "secret") + } + + @Test("SMB volume is not anonymous") + func testSMBVolumeIsNotAnonymous() { + let volume = VolumeConfiguration( + name: "myshare", + driver: "smb", + format: "cifs", + source: "//server/share", + labels: [:] + ) + #expect(!volume.isAnonymous) + } + + @Test("Local volume driver defaults remain unchanged") + func testLocalVolumeDefaults() { + let volume = VolumeConfiguration( + name: "mydata", + source: "/var/lib/container/volumes/mydata/volume.img" + ) + #expect(volume.driver == "local") + #expect(volume.format == "ext4") + } + + // MARK: - Utility: parseKeyValuePairs for SMB driver opts + + @Test("SMB driver opts are parsed from --opt flags including bare flags") + func testSMBDriverOptsKeyValueParsing() { + let raw = ["share=//server/share", "username=user", "password=secret", "domain=corp", "guest"] + let parsed = Utility.parseKeyValuePairs(raw) + + #expect(parsed["share"] == "//server/share") + #expect(parsed["username"] == "user") + #expect(parsed["password"] == "secret") + #expect(parsed["domain"] == "corp") + #expect(parsed["guest"] == "") + } + + @Test("Missing share opt is detectable") + func testMissingSMBShareOpt() { + let opts = Utility.parseKeyValuePairs(["username=user"]) + #expect(opts["share"] == nil) + } +} diff --git a/Tests/ContainerAPIServiceTests/InMemoryVolumeCredentialStore.swift b/Tests/ContainerAPIServiceTests/InMemoryVolumeCredentialStore.swift new file mode 100644 index 000000000..330d576d2 --- /dev/null +++ b/Tests/ContainerAPIServiceTests/InMemoryVolumeCredentialStore.swift @@ -0,0 +1,49 @@ +//===----------------------------------------------------------------------===// +// 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 ContainerAPIService +import Synchronization + +/// A `VolumeCredentialStore` that keeps credentials in memory, for tests that must not touch the keychain. +final class InMemoryVolumeCredentialStore: VolumeCredentialStore, Sendable { + struct Failure: Error {} + + private let credentials = Mutex<[String: VolumeCredential]>([:]) + private let failSave: Bool + + init(failSave: Bool = false) { + self.failSave = failSave + } + + func save(volume: String, credential: VolumeCredential) throws { + if failSave { + throw Failure() + } + credentials.withLock { $0[volume] = credential } + } + + func lookup(volume: String) throws -> VolumeCredential? { + credentials.withLock { $0[volume] } + } + + func delete(volume: String) throws { + credentials.withLock { $0[volume] = nil } + } + + var count: Int { + credentials.withLock { $0.count } + } +} diff --git a/Tests/ContainerAPIServiceTests/VolumesServiceTests.swift b/Tests/ContainerAPIServiceTests/VolumesServiceTests.swift new file mode 100644 index 000000000..1a7a50df6 --- /dev/null +++ b/Tests/ContainerAPIServiceTests/VolumesServiceTests.swift @@ -0,0 +1,357 @@ +//===----------------------------------------------------------------------===// +// 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 ContainerAPIClient +import ContainerAPIService +import ContainerPersistence +import ContainerPlugin +import ContainerResource +import ContainerTestSupport +import ContainerizationError +import Foundation +import Logging +import SystemPackage +import Testing + +struct VolumesServiceTests { + private let log = Logger(label: "test") + + private func makeVolumesService( + appRoot: FilePath, + credentialStore: VolumeCredentialStore = InMemoryVolumeCredentialStore() + ) async throws -> VolumesService { + let appRootURL = URL(fileURLWithPath: appRoot.string) + let pluginLoader = try PluginLoader( + appRoot: appRootURL, + installRoot: appRootURL, + logRoot: nil, + pluginDirectories: [], + pluginFactories: [] + ) + let containersService = try ContainersService( + appRoot: appRootURL, + pluginLoader: pluginLoader, + containerSystemConfig: ContainerSystemConfig(), + log: log, + credentialStore: credentialStore + ) + return try await VolumesService( + resourceRoot: appRoot.appending("volumes"), + containersService: containersService, + log: log, + credentialStore: credentialStore + ) + } + + @Test("VolumesService creates, inspects, and deletes an SMB volume without a disk image") + func testCreateInspectDeleteSMBVolume() async throws { + try await TemporaryStorage.withTempDir { appRoot in + let service = try await makeVolumesService(appRoot: appRoot) + + let created = try await service.create( + name: "myshare", + driver: "smb", + driverOpts: ["share": "//server/share", "username": "user", "password": "secret"], + labels: ["env": "test"] + ) + + #expect(created.name == "myshare") + #expect(created.driver == "smb") + #expect(created.format == "cifs") + #expect(created.source == "//server/share") + #expect(created.sizeInBytes == nil) + #expect(created.options["username"] == "user") + #expect(created.labels["env"] == "test") + + let blockImagePath = appRoot.appending("volumes/myshare/volume.img").string + #expect(!FileManager.default.fileExists(atPath: blockImagePath)) + + let inspected = try await service.inspect("myshare") + #expect(inspected.name == "myshare") + #expect(inspected.driver == "smb") + #expect(inspected.source == "//server/share") + + try await service.delete(name: "myshare") + await #expect(throws: VolumeError.self) { + _ = try await service.inspect("myshare") + } + } + } + + @Test("VolumesService creates, inspects, and deletes an NFS volume without a disk image") + func testCreateInspectDeleteNFSVolume() async throws { + try await TemporaryStorage.withTempDir { appRoot in + let service = try await makeVolumesService(appRoot: appRoot) + + let created = try await service.create( + name: "myexport", + driver: "nfs", + driverOpts: ["share": "nas.local:/exports/data", "vers": "4", "nolock": ""], + labels: ["env": "test"] + ) + + #expect(created.name == "myexport") + #expect(created.driver == "nfs") + #expect(created.format == "nfs") + #expect(created.source == "nas.local:/exports/data") + #expect(created.sizeInBytes == nil) + #expect(created.options["vers"] == "4") + #expect(created.options["nolock"] == "") + #expect(created.labels["env"] == "test") + + let blockImagePath = appRoot.appending("volumes/myexport/volume.img").string + #expect(!FileManager.default.fileExists(atPath: blockImagePath)) + + let inspected = try await service.inspect("myexport") + #expect(inspected.name == "myexport") + #expect(inspected.driver == "nfs") + #expect(inspected.source == "nas.local:/exports/data") + + try await service.delete(name: "myexport") + await #expect(throws: VolumeError.self) { + _ = try await service.inspect("myexport") + } + } + } + + @Test( + "VolumesService rejects SMB and NFS volumes with missing or empty share option", + arguments: ["smb", "nfs"] + ) + func testRejectsMissingOrEmptyShare(_ driver: String) async throws { + try await TemporaryStorage.withTempDir { appRoot in + let service = try await makeVolumesService(appRoot: appRoot) + + await #expect(throws: VolumeError.self) { + _ = try await service.create(name: "vol1", driver: driver, driverOpts: [:]) + } + + await #expect(throws: VolumeError.self) { + _ = try await service.create(name: "vol2", driver: driver, driverOpts: ["share": ""]) + } + } + } + + @Test( + "VolumesService rejects unsupported size option for SMB and NFS volumes", + arguments: [ + ("smb", "//server/share"), + ("nfs", "nas.local:/exports/data"), + ] + ) + func testRejectsSizeOption(_ driver: String, _ share: String) async throws { + try await TemporaryStorage.withTempDir { appRoot in + let service = try await makeVolumesService(appRoot: appRoot) + + await #expect(throws: VolumeError.self) { + _ = try await service.create( + name: "vol1", + driver: driver, + driverOpts: ["share": share, "size": "10G"] + ) + } + } + } + + @Test( + "VolumesService rejects SMB and NFS volumes with syntactically invalid options", + arguments: [ + ("smb", "//server/share"), + ("nfs", "nas.local:/exports/data"), + ] + ) + func testRejectsInvalidMountOptions(_ driver: String, _ share: String) async throws { + try await TemporaryStorage.withTempDir { appRoot in + let service = try await makeVolumesService(appRoot: appRoot) + + // A comma in a value could inject additional mount options. + await #expect(throws: VolumeError.self) { + _ = try await service.create( + name: "vol1", + driver: driver, + driverOpts: ["share": share, "username": "user,uid=0"] + ) + } + + // A key that does not start with a letter. + await #expect(throws: VolumeError.self) { + _ = try await service.create( + name: "vol2", + driver: driver, + driverOpts: ["share": share, "1bad": "value"] + ) + } + + // A control character in the share. + await #expect(throws: VolumeError.self) { + _ = try await service.create( + name: "vol3", + driver: driver, + driverOpts: ["share": share + "\n"] + ) + } + + // Nothing was persisted. + let volumes = try await service.list() + #expect(volumes.isEmpty) + } + } + + @Test( + "VolumesService accepts shares that contain spaces", + arguments: [ + ("smb", "//server/My Share"), + ("nfs", "nas.local:/exports/my data"), + ] + ) + func testAcceptsShareWithSpaces(_ driver: String, _ share: String) async throws { + try await TemporaryStorage.withTempDir { appRoot in + let service = try await makeVolumesService(appRoot: appRoot) + + let created = try await service.create(name: "vol1", driver: driver, driverOpts: ["share": share]) + #expect(created.source == share) + #expect(created.options["share"] == share) + } + } + + @Test("VolumesService rejects unsupported volume driver") + func testRejectsUnsupportedDriver() async throws { + try await TemporaryStorage.withTempDir { appRoot in + let service = try await makeVolumesService(appRoot: appRoot) + + await #expect(throws: VolumeError.self) { + _ = try await service.create(name: "vol1", driver: "ceph", driverOpts: [:]) + } + } + } + + // MARK: - SMB credentials + + @Test("VolumesService keeps the SMB password in the credential store, not in the volume") + func testSMBPasswordIsStoredInCredentialStore() async throws { + try await TemporaryStorage.withTempDir { appRoot in + let credentialStore = InMemoryVolumeCredentialStore() + let service = try await makeVolumesService(appRoot: appRoot, credentialStore: credentialStore) + + let created = try await service.create( + name: "myshare", + driver: "smb", + driverOpts: [ + "share": "//server/share", + "username": "alice", + "password": "hunter2", + "domain": "CORP", + ] + ) + + // The returned, persisted, and inspected configurations all omit the password. + #expect(created.options["password"] == nil) + #expect(created.options["username"] == "alice") + #expect(created.options["domain"] == "CORP") + let inspected = try await service.inspect("myshare") + #expect(inspected.options["password"] == nil) + let onDisk = try String(contentsOfFile: appRoot.appending("volumes/myshare/entity.json").string, encoding: .utf8) + #expect(!onDisk.contains("hunter2")) + + // The credential store holds it, keyed by volume name. + #expect(try credentialStore.lookup(volume: "myshare") == VolumeCredential(username: "alice", password: "hunter2")) + + // Deleting the volume removes the credential. + try await service.delete(name: "myshare") + #expect(try credentialStore.lookup(volume: "myshare") == nil) + #expect(credentialStore.count == 0) + } + } + + @Test("VolumesService does not store a credential for an SMB volume without a password") + func testSMBWithoutPasswordStoresNoCredential() async throws { + try await TemporaryStorage.withTempDir { appRoot in + let credentialStore = InMemoryVolumeCredentialStore() + let service = try await makeVolumesService(appRoot: appRoot, credentialStore: credentialStore) + + _ = try await service.create(name: "guest", driver: "smb", driverOpts: ["share": "//server/public", "guest": ""]) + #expect(credentialStore.count == 0) + + // Deleting is still fine when there is nothing to remove. + try await service.delete(name: "guest") + } + } + + @Test("VolumesService rejects an SMB password without a username") + func testSMBPasswordRequiresUsername() async throws { + try await TemporaryStorage.withTempDir { appRoot in + let credentialStore = InMemoryVolumeCredentialStore() + let service = try await makeVolumesService(appRoot: appRoot, credentialStore: credentialStore) + + await #expect(throws: VolumeError.self) { + _ = try await service.create(name: "vol1", driver: "smb", driverOpts: ["share": "//server/share", "password": "hunter2"]) + } + #expect(credentialStore.count == 0) + #expect(try await service.list().isEmpty) + } + } + + @Test( + "VolumesService rejects SMB secret keys other than password", + arguments: ["pass", "credentials", "cred"] + ) + func testSMBRejectsOtherSecretKeys(_ key: String) async throws { + try await TemporaryStorage.withTempDir { appRoot in + let credentialStore = InMemoryVolumeCredentialStore() + let service = try await makeVolumesService(appRoot: appRoot, credentialStore: credentialStore) + + await #expect(throws: VolumeError.self) { + _ = try await service.create(name: "vol1", driver: "smb", driverOpts: ["share": "//server/share", "username": "u", key: "x"]) + } + #expect(credentialStore.count == 0) + #expect(try await service.list().isEmpty) + } + } + + @Test( + "VolumesService rejects secret keys for NFS volumes, which have no password option", + arguments: ["password", "pass", "credentials", "cred"] + ) + func testNFSRejectsSecretKeys(_ key: String) async throws { + try await TemporaryStorage.withTempDir { appRoot in + let credentialStore = InMemoryVolumeCredentialStore() + let service = try await makeVolumesService(appRoot: appRoot, credentialStore: credentialStore) + + await #expect(throws: VolumeError.self) { + _ = try await service.create(name: "vol1", driver: "nfs", driverOpts: ["share": "nas.local:/exports/data", key: "x"]) + } + #expect(credentialStore.count == 0) + #expect(try await service.list().isEmpty) + } + } + + @Test("VolumesService does not create the volume if the credential cannot be stored") + func testSMBCreateFailsClosedWhenCredentialStoreFails() async throws { + try await TemporaryStorage.withTempDir { appRoot in + let credentialStore = InMemoryVolumeCredentialStore(failSave: true) + let service = try await makeVolumesService(appRoot: appRoot, credentialStore: credentialStore) + + await #expect(throws: VolumeError.self) { + _ = try await service.create( + name: "vol1", + driver: "smb", + driverOpts: ["share": "//server/share", "username": "alice", "password": "hunter2"] + ) + } + #expect(try await service.list().isEmpty) + } + } +} diff --git a/Tests/ContainerCommandsTests/ListFormattingTests.swift b/Tests/ContainerCommandsTests/ListFormattingTests.swift index 5709a685a..41e7926f6 100644 --- a/Tests/ContainerCommandsTests/ListFormattingTests.swift +++ b/Tests/ContainerCommandsTests/ListFormattingTests.swift @@ -39,3 +39,29 @@ struct NetworkResourceDisplayTests { #expect(NetworkResource.tableHeader == ["NETWORK", "SUBNET"]) } } + +// MARK: - VolumeResource ListDisplayable conformance tests + +struct VolumeResourceDisplayTests { + @Test + func tableRowRedactsPasswordOption() { + let config = VolumeConfiguration( + name: "myshare", + driver: "smb", + format: "cifs", + source: "//server/share", + labels: [:], + options: [ + "share": "//server/share", + "username": "user", + "password": "supersecretpassword", + ], + sizeInBytes: nil + ) + let resource = VolumeResource(configuration: config) + let row = resource.tableRow + + #expect(row == ["myshare", "named", "smb", "password=***,share=//server/share,username=user"]) + #expect(!row[3].contains("supersecretpassword")) + } +} diff --git a/Tests/ContainerResourceTests/FilesystemNFSTests.swift b/Tests/ContainerResourceTests/FilesystemNFSTests.swift new file mode 100644 index 000000000..3f54dc026 --- /dev/null +++ b/Tests/ContainerResourceTests/FilesystemNFSTests.swift @@ -0,0 +1,149 @@ +//===----------------------------------------------------------------------===// +// 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 Testing + +@testable import ContainerResource + +struct FilesystemNFSTests { + + @Test("NFS factory produces correct FSType") + func testNFSFactory() { + let fs = Filesystem.nfs( + share: "nas.local:/exports/data", + mountOptions: ["vers": "4"], + destination: "/data", + options: [] + ) + + guard case .nfs(let name, let share, let opts) = fs.type else { + Issue.record("Expected .nfs FSType") + return + } + #expect(name == nil) + #expect(share == "nas.local:/exports/data") + #expect(opts["vers"] == "4") + #expect(fs.source == "nas.local:/exports/data") + #expect(fs.destination == "/data") + } + + @Test("isNFS returns true only for NFS filesystems") + func testIsNFS() { + let nfs = Filesystem.nfs( + share: "nas.local:/exports/data", + mountOptions: [:], + destination: "/data", + options: [] + ) + let virtiofs = Filesystem.virtiofs(source: "/host/path", destination: "/data", options: []) + let tmpfs = Filesystem.tmpfs(destination: "/tmp", options: []) + + #expect(nfs.isNFS) + #expect(!virtiofs.isNFS) + #expect(!tmpfs.isNFS) + } + + @Test("NFS filesystem is not a block device") + func testNFSIsNotBlock() { + let fs = Filesystem.nfs( + share: "nas.local:/exports/data", + mountOptions: [:], + destination: "/data", + options: [] + ) + #expect(!fs.isBlock) + #expect(!fs.isVolume) + #expect(fs.volumeName == nil) + #expect(!fs.isTmpfs) + #expect(!fs.isVirtiofs) + #expect(!fs.isSMB) + } + + @Test("Named NFS volume reports volumeName and isVolume without isBlock") + func testNamedNFSVolume() { + let fs = Filesystem.nfs( + name: "myexport", + share: "nas.local:/exports/data", + mountOptions: ["vers": "4"], + destination: "/data", + options: [] + ) + #expect(fs.isNFS) + #expect(fs.isVolume) + #expect(fs.volumeName == "myexport") + #expect(!fs.isBlock) + } + + @Test("NFS FSType round-trips through Codable") + func testNFSCodable() throws { + let original = Filesystem.nfs( + name: "myexport", + share: "nas.local:/exports/data", + mountOptions: ["vers": "4"], + destination: "/mnt/data", + options: ["ro"] + ) + + let data = try JSONEncoder().encode(original) + let decoded = try JSONDecoder().decode(Filesystem.self, from: data) + + guard case .nfs(let name, let share, let opts) = decoded.type else { + Issue.record("Expected .nfs after decoding") + return + } + #expect(name == "myexport") + #expect(share == "nas.local:/exports/data") + #expect(opts["vers"] == "4") + #expect(decoded.destination == "/mnt/data") + #expect(decoded.options == ["ro"]) + } + + @Test("NFS mount options are preserved with extra options") + func testNFSWithExtraOptions() { + let fs = Filesystem.nfs( + share: "nas.local:/exports/backup", + mountOptions: ["vers": "3", "timeo": "600"], + destination: "/backup", + options: ["ro", "noatime"] + ) + + guard case .nfs(_, _, let opts) = fs.type else { + Issue.record("Expected .nfs FSType") + return + } + #expect(opts.count == 2) + #expect(fs.options == ["ro", "noatime"]) + } + + @Test("NFS resolvedMountOptions strips share, formats empty values as bare flags, and sorts keys") + func testNFSResolvedMountOptions() { + let fs = Filesystem.nfs( + name: "myexport", + share: "nas.local:/exports/backup", + mountOptions: [ + "share": "nas.local:/exports/backup", + "vers": "3", + "nolock": "", + "rsize": "1048576", + ], + destination: "/backup", + options: ["ro"] + ) + + #expect(fs.resolvedMountOptions == ["nolock", "rsize=1048576", "vers=3", "ro"]) + } +} diff --git a/Tests/ContainerResourceTests/FilesystemSMBTests.swift b/Tests/ContainerResourceTests/FilesystemSMBTests.swift new file mode 100644 index 000000000..c9f7e89f9 --- /dev/null +++ b/Tests/ContainerResourceTests/FilesystemSMBTests.swift @@ -0,0 +1,266 @@ +//===----------------------------------------------------------------------===// +// 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 Testing + +@testable import ContainerResource + +struct FilesystemSMBTests { + + @Test("SMB factory produces correct FSType") + func testSMBFactory() { + let fs = Filesystem.smb( + share: "//server/share", + mountOptions: ["username": "user", "password": "secret"], + destination: "/data", + options: [] + ) + + guard case .smb(let name, let share, let opts) = fs.type else { + Issue.record("Expected .smb FSType") + return + } + #expect(name == nil) + #expect(share == "//server/share") + #expect(opts["username"] == "user") + #expect(opts["password"] == "secret") + #expect(fs.source == "//server/share") + #expect(fs.destination == "/data") + } + + @Test("isSMB returns true only for SMB filesystems") + func testIsSMB() { + let smb = Filesystem.smb( + share: "//server/share", + mountOptions: [:], + destination: "/data", + options: [] + ) + let virtiofs = Filesystem.virtiofs(source: "/host/path", destination: "/data", options: []) + let tmpfs = Filesystem.tmpfs(destination: "/tmp", options: []) + + #expect(smb.isSMB) + #expect(!virtiofs.isSMB) + #expect(!tmpfs.isSMB) + } + + @Test("SMB filesystem is not a block device") + func testSMBIsNotBlock() { + let fs = Filesystem.smb( + share: "//server/share", + mountOptions: [:], + destination: "/data", + options: [] + ) + #expect(!fs.isBlock) + #expect(!fs.isVolume) + #expect(fs.volumeName == nil) + #expect(!fs.isTmpfs) + #expect(!fs.isVirtiofs) + #expect(!fs.isNFS) + } + + @Test("Named SMB volume reports volumeName and isVolume without isBlock") + func testNamedSMBVolume() { + let fs = Filesystem.smb( + name: "myshare", + share: "//server/share", + mountOptions: ["username": "user"], + destination: "/data", + options: [] + ) + #expect(fs.isSMB) + #expect(fs.isVolume) + #expect(fs.volumeName == "myshare") + #expect(!fs.isBlock) + } + + @Test("SMB FSType round-trips through Codable") + func testSMBCodable() throws { + let original = Filesystem.smb( + name: "myshare", + share: "//server/share", + mountOptions: ["username": "user", "domain": "corp"], + destination: "/mnt/data", + options: ["ro"] + ) + + let data = try JSONEncoder().encode(original) + let decoded = try JSONDecoder().decode(Filesystem.self, from: data) + + guard case .smb(let name, let share, let opts) = decoded.type else { + Issue.record("Expected .smb after decoding") + return + } + #expect(name == "myshare") + #expect(share == "//server/share") + #expect(opts["username"] == "user") + #expect(opts["domain"] == "corp") + #expect(decoded.destination == "/mnt/data") + #expect(decoded.options == ["ro"]) + } + + @Test("SMB mount options are preserved with extra options") + func testSMBWithExtraOptions() { + let fs = Filesystem.smb( + share: "//nas/backup", + mountOptions: ["username": "admin", "password": "pass", "domain": "corp"], + destination: "/backup", + options: ["ro", "noatime"] + ) + + guard case .smb(_, _, let opts) = fs.type else { + Issue.record("Expected .smb FSType") + return + } + #expect(opts.count == 3) + #expect(fs.options == ["ro", "noatime"]) + } + + @Test("SMB resolvedMountOptions strips share, formats empty values as bare flags, and sorts keys") + func testSMBResolvedMountOptions() { + let fs = Filesystem.smb( + name: "myshare", + share: "//nas/backup", + mountOptions: [ + "share": "//nas/backup", + "username": "admin", + "password": "secret", + "domain": "corp", + "guest": "", + ], + destination: "/backup", + options: ["ro", "noatime"] + ) + + #expect(fs.resolvedMountOptions == ["domain=corp", "guest", "password=secret", "username=admin", "ro", "noatime"]) + } + + @Test("validateMountOptions checks the share as a source, the options as options, and ignores other types") + func testValidateMountOptions() throws { + let validSMB = Filesystem.smb( + share: "//server/My Share", + mountOptions: ["share": "//server/My Share", "username": "user", "password": "secret"], + destination: "/data", + options: [] + ) + try validSMB.validateMountOptions() + + let injectedSMB = Filesystem.smb( + share: "//server/share", + mountOptions: ["username": "user,uid=0"], + destination: "/data", + options: [] + ) + #expect(throws: MountOptionValidator.Violation.self) { + try injectedSMB.validateMountOptions() + } + + let controlCharacterShare = Filesystem.smb( + share: "//server/share\u{0}", + mountOptions: [:], + destination: "/data", + options: [] + ) + #expect(throws: MountOptionValidator.Violation.self) { + try controlCharacterShare.validateMountOptions() + } + + let injectedNFS = Filesystem.nfs( + share: "nas.local:/exports/data", + mountOptions: ["vers": "4,nolock"], + destination: "/data", + options: [] + ) + #expect(throws: MountOptionValidator.Violation.self) { + try injectedNFS.validateMountOptions() + } + + // Other filesystem types have no driver options and always pass. + try Filesystem.virtiofs(source: "/host/path", destination: "/data", options: []).validateMountOptions() + try Filesystem.tmpfs(destination: "/tmp", options: []).validateMountOptions() + } + + @Test("validateNoSecretOptions rejects a password on SMB and NFS filesystems and ignores other types") + func testValidateNoSecretOptions() throws { + let withPassword = Filesystem.smb( + share: "//server/share", + mountOptions: ["username": "user", "password": "secret"], + destination: "/data", + options: [] + ) + #expect(throws: MountOptionValidator.Violation.self) { + try withPassword.validateNoSecretOptions() + } + + let withoutPassword = Filesystem.smb( + share: "//server/share", + mountOptions: ["username": "user", "domain": "corp"], + destination: "/data", + options: [] + ) + try withoutPassword.validateNoSecretOptions() + + // NFS has no password option, so a secret there could only be stored in plain text. + let nfs = Filesystem.nfs( + share: "nas.local:/exports/data", + mountOptions: ["password": "irrelevant"], + destination: "/data", + options: [] + ) + #expect(throws: MountOptionValidator.Violation.self) { + try nfs.validateNoSecretOptions() + } + try Filesystem.tmpfs(destination: "/tmp", options: []).validateNoSecretOptions() + } + + @Test("addingSMBPassword adds the password to an SMB mount without touching anything else") + func testAddingSMBPassword() { + let original = Filesystem.smb( + name: "myshare", + share: "//server/share", + mountOptions: ["share": "//server/share", "username": "user", "domain": "corp"], + destination: "/data", + options: ["ro"] + ) + + let withPassword = original.addingSMBPassword("hunter2") + + guard case .smb(let name, let share, let opts) = withPassword.type else { + Issue.record("Expected .smb FSType") + return + } + #expect(name == "myshare") + #expect(share == "//server/share") + #expect(opts["password"] == "hunter2") + #expect(opts["username"] == "user") + #expect(opts["domain"] == "corp") + #expect(withPassword.source == original.source) + #expect(withPassword.destination == original.destination) + #expect(withPassword.options == original.options) + #expect(withPassword.resolvedMountOptions == ["domain=corp", "password=hunter2", "username=user", "ro"]) + + // The original is unchanged and non-SMB filesystems are returned as-is. + guard case .smb(_, _, let originalOpts) = original.type else { + Issue.record("Expected .smb FSType") + return + } + #expect(originalOpts["password"] == nil) + let tmpfs = Filesystem.tmpfs(destination: "/tmp", options: []) + #expect(tmpfs.addingSMBPassword("x").type == tmpfs.type) + } +} diff --git a/Tests/ContainerResourceTests/MountOptionValidatorTests.swift b/Tests/ContainerResourceTests/MountOptionValidatorTests.swift new file mode 100644 index 000000000..84f7e46a6 --- /dev/null +++ b/Tests/ContainerResourceTests/MountOptionValidatorTests.swift @@ -0,0 +1,182 @@ +//===----------------------------------------------------------------------===// +// 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 ContainerResource +import Testing + +struct MountOptionValidatorTests { + @Test("Accepts typical SMB options") + func acceptsSMBOptions() throws { + try MountOptionValidator.validate([ + "share": "//fileserver/share", + "username": "alice", + "password": "s3cr3t!#$%", + "domain": "CORP", + ]) + } + + @Test("Accepts typical NFS options including bare flags") + func acceptsNFSOptions() throws { + try MountOptionValidator.validate([ + "share": "nas.local:/exports/data", + "addr": "192.168.1.10", + "vers": "4.1", + "proto": "tcp", + "nolock": "", + "timeo": "600", + ]) + } + + @Test("Accepts an empty option set") + func acceptsEmptyOptions() throws { + try MountOptionValidator.validate([:]) + } + + @Test( + "Accepts shares with spaces and commas, since the share is the mount source", + arguments: [ + "//fileserver/My Share", + "//fileserver/Reports, 2026", + #"\\fileserver\share"#, + "nas.local:/exports/my data", + "[fd00::1]:/exports/data", + ] + ) + func acceptsShareSourceCharacters(_ share: String) throws { + try MountOptionValidator.validateShare(share) + try MountOptionValidator.validate(["share": share, "username": "user"]) + } + + @Test( + "Rejects shares that are empty or contain control characters", + arguments: [ + "", + "//server/share\u{0}", + "//server/share\n", + "//server/\u{1b}[0mshare", + String(repeating: "s", count: MountOptionValidator.shareLengthMax + 1), + ] + ) + func rejectsInvalidShares(_ share: String) { + #expect(throws: MountOptionValidator.Violation.self) { + try MountOptionValidator.validateShare(share) + } + #expect(throws: MountOptionValidator.Violation.self) { + try MountOptionValidator.validate(["share": share]) + } + } + + @Test("Share leniency does not extend to other options") + func shareLeniencyIsScoped() { + #expect(throws: MountOptionValidator.Violation.self) { + try MountOptionValidator.validate(["share": "//server/My Share", "username": "my user"]) + } + } + + @Test("Accepts keys and values at their maximum length") + func acceptsMaximumLengths() throws { + let key = "k" + String(repeating: "a", count: MountOptionValidator.keyLengthMax - 1) + let value = String(repeating: "v", count: MountOptionValidator.valueLengthMax) + try MountOptionValidator.validate([key: value]) + } + + @Test( + "Rejects keys that are malformed or too long", + arguments: [ + "", + "1vers", + "-vers", + "_vers", + "ver.s", + "ver s", + "ver=s", + "ver,s", + "k" + String(repeating: "a", count: MountOptionValidator.keyLengthMax), + ] + ) + func rejectsInvalidKeys(_ key: String) { + #expect(throws: MountOptionValidator.Violation.self) { + try MountOptionValidator.validate([key: "value"]) + } + } + + @Test( + "Rejects values that could inject options or are too long", + arguments: [ + "a,b", + "a,uid=0", + "a b", + "a\tb", + "a\nb", + "\u{0}", + "a\u{1b}[0m", + String(repeating: "v", count: MountOptionValidator.valueLengthMax + 1), + ] + ) + func rejectsInvalidValues(_ value: String) { + #expect(throws: MountOptionValidator.Violation.self) { + try MountOptionValidator.validate(["username": value]) + } + } + + @Test("Violation names the offending key but not its value") + func violationOmitsValue() { + let secret = "hunter2,uid=0" + do { + try MountOptionValidator.validate(["password": secret]) + Issue.record("Expected a violation") + } catch let violation as MountOptionValidator.Violation { + #expect(violation.description.contains("password")) + #expect(!violation.description.contains(secret)) + } catch { + Issue.record("Unexpected error: \(error)") + } + } + + @Test("Checks options in key order so the first error is deterministic") + func reportsFirstKeyInOrder() { + do { + try MountOptionValidator.validate(["zeta": "a,b", "alpha": "c d"]) + Issue.record("Expected a violation") + } catch let violation as MountOptionValidator.Violation { + #expect(violation.description.contains("alpha")) + } catch { + Issue.record("Unexpected error: \(error)") + } + } + + @Test( + "validateNoSecrets rejects every cifs key that carries or points at a secret", + arguments: ["password", "pass", "credentials", "cred"] + ) + func rejectsSecretKeys(_ key: String) { + do { + try MountOptionValidator.validateNoSecrets(["username": "alice", key: "x"]) + Issue.record("Expected a violation") + } catch let violation as MountOptionValidator.Violation { + #expect(violation.description.contains("\"\(key)\"")) + #expect(!violation.description.contains("x\"")) + } catch { + Issue.record("Unexpected error: \(error)") + } + } + + @Test("validateNoSecrets accepts options without a secret") + func acceptsOptionsWithoutSecrets() throws { + try MountOptionValidator.validateNoSecrets([:]) + try MountOptionValidator.validateNoSecrets(["share": "//server/share", "username": "alice", "domain": "CORP", "guest": ""]) + } +} diff --git a/docs/command-reference.md b/docs/command-reference.md index 58edd7147..19cbecaad 100644 --- a/docs/command-reference.md +++ b/docs/command-reference.md @@ -914,7 +914,7 @@ Creates a new named volume with an optional size and driver-specific options. **Usage** ```bash -container volume create [--label