From f70630fda78ed337d9e1075f2eb2bff14cd7ebff Mon Sep 17 00:00:00 2001 From: Charles Hu Date: Fri, 31 Jul 2026 00:24:30 -0700 Subject: [PATCH] Resolve Executable.name(_:) against PATH only on every platform Executable.name(_:) is documented as a PATH lookup, but it also searched a current directory ahead of PATH. This behavior is opposite of what every POSIX runtime agreed on, and it allows potential arbitrary execution. Update Executable.name(_:) such that an executable name now resolves the same way on all platforms: by walking the PATH the subprocess will receive, falling back to the current process's value and then to the directories the platform reports as standard (confstr(_CS_PATH) on Unix, the CreateProcessW search list without the current directory on Windows). Empty and relative entries are skipped, so every resolved path is absolute and resolveExecutablePath(in:) agrees with the spawn path, and a name containing a path separator is now rejected with spawnFailed rather than resolved against a current directory. --- Sources/Subprocess/Configuration.swift | 78 ++++ .../Platforms/Subprocess+Darwin.swift | 16 +- .../Platforms/Subprocess+Unix.swift | 98 +++-- .../Platforms/Subprocess+Windows.swift | 412 ++++++++---------- .../_SubprocessCShims/include/process_shims.h | 8 + Sources/_SubprocessCShims/process_shims.c | 41 ++ Tests/SubprocessTests/IntegrationTests.swift | 36 +- Tests/SubprocessTests/UnixTests.swift | 238 ++++++++-- Tests/SubprocessTests/WindowsTests.swift | 255 ++++++++++- 9 files changed, 860 insertions(+), 322 deletions(-) diff --git a/Sources/Subprocess/Configuration.swift b/Sources/Subprocess/Configuration.swift index dce1cbc9..86aeae25 100644 --- a/Sources/Subprocess/Configuration.swift +++ b/Sources/Subprocess/Configuration.swift @@ -332,6 +332,18 @@ public struct Executable: Sendable, Hashable { /// Subprocess searches for the executable by name in the directories listed /// in the `PATH` environment variable, in order, and runs the first match it /// finds. To run an executable at a known location instead, use ``path(_:)``. + /// + /// The search uses the `PATH` of the environment you pass to the subprocess, + /// falling back to the `PATH` of the current process when that environment + /// doesn't set one, and to the directories the platform considers standard + /// when neither sets one. Only absolute directories are searched: Subprocess + /// skips empty and relative `PATH` entries, and never searches the current + /// working directory of either process. The working directory you pass to + /// `run` therefore has no effect on which executable runs. + /// + /// The executable name must be a name, not a path: passing a name that + /// contains a path separator throws a ``SubprocessError`` with the code + /// ``SubprocessError/Code/spawnFailed``. Use ``path(_:)`` for a location. public static func name(_ executableName: String) -> Self { return .init(_config: .executable(executableName)) } @@ -342,6 +354,17 @@ public struct Executable: Sendable, Hashable { return .init(_config: .path(filePath)) } /// Resolves the full executable path using the environment you provide. + /// + /// For an executable created with ``name(_:)``, this searches `PATH` the + /// same way running the subprocess does, and returns an absolute path. + /// An executable created with ``path(_:)`` is returned unchanged, without + /// checking whether anything exists there. + /// + /// - Throws: ``SubprocessError`` with the code + /// ``SubprocessError/Code/executableNotFound`` when no directory on + /// `PATH` holds a matching executable, or + /// ``SubprocessError/Code/spawnFailed`` when the name contains a path + /// separator. public func resolveExecutablePath(in environment: Environment) async throws(SubprocessError) -> FilePath { try await runOnBackgroundThread { () throws(SubprocessError) -> FilePath in let path = try self.resolveExecutablePath(withPathValue: environment.pathValue()) @@ -350,6 +373,61 @@ public struct Executable: Sendable, Hashable { } } +// MARK: - Executable Name Resolution Policy + +extension Executable { + #if os(Windows) + /// The character that separates one directory from the next in a `PATH` + /// value. + internal static let pathValueSeparator: Character = ";" + /// The characters that name a location, and so may not appear in a name + /// passed to ``name(_:)``. + /// + /// Windows accepts either slash as a separator, and `:` separates a drive + /// from a drive-relative path, so `C:tool` names a location in the same way + /// `.\tool` does. + internal static let pathSeparators: [Character] = ["/", "\\", ":"] + #else + /// The character that separates one directory from the next in a `PATH` + /// value. + internal static let pathValueSeparator: Character = ":" + /// The characters that name a location, and so may not appear in a name + /// passed to ``name(_:)``. + internal static let pathSeparators: [Character] = ["/"] + #endif + + /// Rejects a name that names a location rather than an executable. + /// + /// A name containing a path separator is ambiguous: it reads as a name to + /// look up on `PATH` and as a path to resolve against some current + /// directory, and the two answers differ. ``path(_:)`` expresses the + /// second intent unambiguously. + internal static func validate(name: String) throws(SubprocessError) { + guard let separator = name.first(where: { Self.pathSeparators.contains($0) }) else { + return + } + throw SubprocessError.spawnFailed( + withUnderlyingError: nil, + reason: """ + Executable name "\(name)" must not contain the path separator "\(separator)". \ + Executable.name(_:) looks a name up in the directories listed in PATH; \ + use Executable.path(_:) to run an executable at a known location. + """ + ) + } + + internal static func searchPaths(withPathValue pathValue: String?) -> [String] { + guard let pathValue else { + return Self.defaultSearchPaths + } + // `split` omits empty subsequences, dropping the empty entries that a + // leading, trailing, or doubled separator introduces. + return pathValue.split(separator: Self.pathValueSeparator) + .map(String.init) + .filter { FilePath($0).isAbsolute } + } +} + extension Executable: CustomStringConvertible, CustomDebugStringConvertible { /// A textual representation of this executable. public var description: String { diff --git a/Sources/Subprocess/Platforms/Subprocess+Darwin.swift b/Sources/Subprocess/Platforms/Subprocess+Darwin.swift index acfb8315..08a20fa8 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Darwin.swift +++ b/Sources/Subprocess/Platforms/Subprocess+Darwin.swift @@ -199,17 +199,23 @@ extension Configuration { // Ensure the waiter thread is running. _setupMonitorSignalHandler() - // Instead of checking if every possible executable path - // is valid, spawn each directly and catch ENOENT - let possiblePaths = self.executable.possibleExecutablePaths( - withPathValue: self.environment.pathValue() - ) var inputPipeBox: CreatedPipe? = consume inputPipe var outputPipeBox: CreatedPipe? = consume outputPipe var errorPipeBox: CreatedPipe? = consume errorPipe func spawnFunc(_ args: PreSpawnArgs) async throws -> SpawnResult { let (env, uidPtr, gidPtr, supplementaryGroups) = args + + // Resolve before taking the pipes out of their boxes, so that a + // rejected executable name leaves them for the caller's `catch` + // below to close. + // + // Instead of checking if every possible executable path + // is valid, spawn each directly and catch ENOENT + let possiblePaths = try self.executable.possibleExecutablePaths( + withPathValue: self.environment.pathValue() + ) + var _inputPipe = inputPipeBox.take()! var _outputPipe = outputPipeBox.take()! var _errorPipe = errorPipeBox.take()! diff --git a/Sources/Subprocess/Platforms/Subprocess+Unix.swift b/Sources/Subprocess/Platforms/Subprocess+Unix.swift index 41bf8a13..792840c0 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Unix.swift +++ b/Sources/Subprocess/Platforms/Subprocess+Unix.swift @@ -155,12 +155,18 @@ extension Execution { // MARK: - Environment Resolution and Validation extension Environment { + /// The `PATH` value to resolve ``Executable/name(_:)`` against. + /// + /// The value the subprocess receives wins, so a name resolves in the + /// environment it will run in. When that environment carries no `PATH`, + /// this process's own value is used; `nil` means neither defines one. internal func pathValue() -> String? { switch self.config { case .inherit(let overrides): - // If PATH value exists in overrides, use it - if let value = overrides[.path] { - return value + // If PATH value exists in overrides, use it. An override maps to + // `nil` to unset the value, which falls through to this process. + if let overridden = overrides[.path], let overridden { + return overridden } // Fall back to current process return Self.currentEnvironmentValues()[.path] @@ -168,7 +174,7 @@ extension Environment { if let value = fullEnvironment[.path] { return value } - return nil + return Self.currentEnvironmentValues()[.path] case .rawBytes(let rawBytesArray): let needle: [UInt8] = Array("\(Key.path.rawValue)=".utf8) for row in rawBytesArray { @@ -179,7 +185,7 @@ extension Environment { let pathValue = row.dropFirst(needle.count) return String(decoding: pathValue, as: UTF8.self) } - return nil + return Self.currentEnvironmentValues()[.path] } } @@ -359,7 +365,26 @@ extension Arguments { // MARK: - Executable Searching extension Executable { - internal static let defaultSearchPaths = [ + /// The directories searched when neither the environment passed to the + /// subprocess nor the current process defines `PATH`. This is the system's own + /// standard path,`confstr(_CS_PATH)`, falling back to the `` macro fixed + /// at compile time. + /// + /// The list is filtered like a `PATH` value, so a relative or empty entry in + /// the system's answer is skipped too. If the system reports nothing usable, + /// the historical hard-coded list stands in as a last resort. + internal static let defaultSearchPaths: [String] = { + guard let systemPathValue = Self.systemStandardPathValue() else { + return Self.fallbackSearchPaths + } + let searchPaths = systemPathValue.split(separator: Self.pathValueSeparator) + .map(String.init) + .filter { FilePath($0).isAbsolute } + return searchPaths.isEmpty ? Self.fallbackSearchPaths : searchPaths + }() + + /// The directories to search when the system reports no standard path. + private static let fallbackSearchPaths = [ "/usr/bin", "/bin", "/usr/sbin", @@ -367,14 +392,30 @@ extension Executable { "/usr/local/bin", ] + /// Asks the system for its standard `PATH` value, or returns `nil` when it + /// reports none. + private static func systemStandardPathValue() -> String? { + // Two-call `confstr` protocol: size the buffer, then fill it. A second + // call that needs more room than the first reported means the value + // changed underneath us, which cannot happen for a system constant, so + // a short answer is simply used as-is. + let size = _subprocess_default_search_path(nil, 0) + guard size > 1 else { + return nil + } + let pathValue = withUnsafeTemporaryAllocation(of: CChar.self, capacity: Int(size)) { buffer in + guard _subprocess_default_search_path(buffer.baseAddress, size) > 0 else { + return "" + } + return String(cString: buffer.baseAddress!) + } + return pathValue.isEmpty ? nil : pathValue + } + internal func resolveExecutablePath(withPathValue pathValue: String?) throws(SubprocessError) -> String { switch self.storage { case .executable(let executableName): - // If the executableName in is already a full path, return it directly - if Configuration.executableAccessible(executableName) { - return executableName - } - let firstAccessibleExecutable = possibleExecutablePaths(withPathValue: pathValue) + let firstAccessibleExecutable = try possibleExecutablePaths(withPathValue: pathValue) .first { Configuration.executableAccessible($0) } if let firstAccessibleExecutable { return firstAccessibleExecutable @@ -386,22 +427,20 @@ extension Executable { } } + /// The paths to try, in order, for this executable. + /// + /// A name is joined to each directory returned by + /// `Executable.searchPaths(withPathValue:)`; the name itself is never a + /// candidate, so `execvp`-style resolution against a current working + /// directory cannot happen. A path is its own only candidate. internal func possibleExecutablePaths( withPathValue pathValue: String? - ) -> _OrderedSet { + ) throws(SubprocessError) -> _OrderedSet { switch self.storage { case .executable(let executableName): + try Self.validate(name: executableName) var results: _OrderedSet = .init() - // executableName could be a full path - results.insert(executableName) - // Get $PATH from environment - let searchPaths = - if let pathValue = pathValue { - pathValue.split(separator: ":").map { String($0) } + Self.defaultSearchPaths - } else { - Self.defaultSearchPaths - } - for path in searchPaths { + for path in Self.searchPaths(withPathValue: pathValue) { results.insert( FilePath(path).appending(executableName).string ) @@ -553,11 +592,6 @@ extension Configuration { // Ensure the waiter thread is running. _setupMonitorSignalHandler() - // Instead of checking if every possible executable path - // is valid, spawn each directly and catch ENOENT - let possiblePaths = self.executable.possibleExecutablePaths( - withPathValue: self.environment.pathValue() - ) var inputPipeBox: CreatedPipe? = consume inputPipe var outputPipeBox: CreatedPipe? = consume outputPipe var errorPipeBox: CreatedPipe? = consume errorPipe @@ -565,6 +599,16 @@ extension Configuration { func spawnFunc(_ args: PreSpawnArgs) async throws -> SpawnResult { let (env, uidPtr, gidPtr, supplementaryGroups) = args + // Resolve before taking the pipes out of their boxes, so that a + // rejected executable name leaves them for the caller's `catch` + // below to close. + // + // Instead of checking if every possible executable path + // is valid, spawn each directly and catch ENOENT + let possiblePaths = try self.executable.possibleExecutablePaths( + withPathValue: self.environment.pathValue() + ) + var _inputPipe = inputPipeBox.take()! var _outputPipe = outputPipeBox.take()! var _errorPipe = errorPipeBox.take()! diff --git a/Sources/Subprocess/Platforms/Subprocess+Windows.swift b/Sources/Subprocess/Platforms/Subprocess+Windows.swift index cbb4a2de..ae003be5 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Windows.swift +++ b/Sources/Subprocess/Platforms/Subprocess+Windows.swift @@ -59,6 +59,48 @@ extension Configuration { throw error } + // Resolve the executable before spawning rather than letting + // `CreateProcessW` search for it. Its built-in search runs only when the + // executable is named on `lpCommandLine`, and it does not match the + // contract of `Executable.name(_:)` in two ways: it consults the + // directory the application loaded from, the current directory, and the + // system directories ahead of `PATH`, and it reads `PATH` from *this* + // process's environment rather than from the `lpEnvironment` block the + // subprocess receives. Resolving here keeps a name resolving against the + // subprocess's `PATH` and nothing else, on every platform. + // + // https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw + let possibleExecutablePaths: [String] + do { + switch self.executable.storage { + case .executable: + possibleExecutablePaths = try self.executable.possibleExecutablePaths( + withPathValue: self.environment.pathValue() + ).filter { + // `GetFileAttributesW` is bound by `MAX_PATH`, while + // `CreateProcessW` receives a `\\?\`-prefixed path from + // `withNTPathRepresentation` and is not. Leave a long + // candidate for `CreateProcessW` to judge rather than + // rejecting a spawn it would have accepted. + $0.utf16.count >= Int(MAX_PATH) || Configuration.executableAccessible($0) + } + case .path(let path): + // A path is used exactly as given, so `CreateProcessW` reports + // whatever is wrong with it. + possibleExecutablePaths = [path.string] + } + } catch { + try self.safelyCloseMultiple( + inputRead: inputReadFileDescriptor, + inputWrite: inputWriteFileDescriptor, + outputRead: outputReadFileDescriptor, + outputWrite: outputWriteFileDescriptor, + errorRead: errorReadFileDescriptor, + errorWrite: errorWriteFileDescriptor + ) + throw error + } + // Create the Job Object up front. It persists across all candidate path // attempts, and is owned by this function until ownership transfers to // the `ProcessIdentifier` on the success path. @@ -70,29 +112,6 @@ extension Configuration { } } - // CreateProcessW supports using `lpApplicationName` as well as `lpCommandLine` to - // specify executable path. However, only `lpCommandLine` supports PATH looking up, - // whereas `lpApplicationName` does not. In general we should rely on `lpCommandLine`'s - // automatic PATH lookup so we only need to call `CreateProcessW` once. However, if - // user wants to override executable path in arguments, we have to use `lpApplicationName` - // to specify the executable path. In this case, manually loop over all possible paths. - let possibleExecutablePaths: _OrderedSet - if _fastPath(self.arguments.executablePathOverride == nil) { - // Fast path: we can rely on `CreateProcessW`'s built in Path searching - switch self.executable.storage { - case .executable(let executable): - possibleExecutablePaths = _OrderedSet([executable]) - case .path(let path): - possibleExecutablePaths = _OrderedSet([path.string]) - } - } else { - // Slow path: user requested arg0 override, therefore we must manually - // traverse through all possible executable paths - possibleExecutablePaths = self.executable.possibleExecutablePaths( - withPathValue: self.environment.pathValue() - ) - } - for executablePath in possibleExecutablePaths { let applicationName: String? let commandAndArgs: String @@ -327,7 +346,20 @@ extension Configuration { errorWrite: errorWriteFileDescriptor ) - // If we reached this point, all possible executable paths have failed + // If we reached this point, all possible executable paths have failed. + // When a name resolved to nothing at all, `CreateProcessW` never ran and + // so never reported an unusable working directory. Check it here, so the + // error names the problem the caller can act on this is the same best-effort + // guess the Unix spawn path makes. + if let workingDirectory = self.workingDirectory?.string, + !Configuration.isDirectory(workingDirectory) + { + throw SubprocessError.failedToChangeWorkingDirectory( + workingDirectory, + underlyingError: SubprocessError.WindowsError(win32Error: DWORD(ERROR_DIRECTORY)) + ) + } + throw SubprocessError.executableNotFound( self.executable.description, underlyingError: SubprocessError.WindowsError(win32Error: DWORD(ERROR_FILE_NOT_FOUND)) @@ -701,35 +733,79 @@ extension Execution { // MARK: - Executable Searching extension Executable { - // Technically not needed for CreateProcess since - // it takes process name. It's here to support - // Executable.resolveExecutablePath + /// The directories searched when neither the environment passed to the + /// subprocess nor the current process defines `PATH`. + /// + /// Windows has no `confstr(_CS_PATH)` to ask. The closest equivalent is the + /// set of directories `CreateProcessW` searches by itself, so those are + /// queried here, in the order it documents: the directory the application + /// loaded from, the 32-bit system directory, the 16-bit system directory, + /// and the Windows directory. + /// + /// The one step of that order deliberately left out is "the current + /// directory for the calling process". ``Executable/name(_:)`` never + /// searches a current directory to avoid potential security issues. + /// + /// https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw + internal static let defaultSearchPaths: [String] = { + var directories: [String] = [] + + // 1. The directory from which the application loaded. + let applicationPath = try? fillNullTerminatedWideStringBuffer( + initialSize: DWORD(MAX_PATH), + maxSize: DWORD(Int16.max) + ) { + return GetModuleFileNameW(nil, $0.baseAddress, DWORD($0.count)) + } + if let applicationPath { + directories.append(FilePath(applicationPath).removingLastComponent().string) + } + + // 2. The 32-bit Windows system directory. + let systemDirectorySize = GetSystemDirectoryW(nil, 0) + let systemDirectory = try? fillNullTerminatedWideStringBuffer( + initialSize: systemDirectorySize, + maxSize: DWORD(Int16.max) + ) { + return GetSystemDirectoryW($0.baseAddress, DWORD($0.count)) + } + if let systemDirectory { + directories.append(systemDirectory) + } + + let windowsDirectorySize = GetWindowsDirectoryW(nil, 0) + let windowsDirectory = try? fillNullTerminatedWideStringBuffer( + initialSize: windowsDirectorySize, + maxSize: DWORD(Int16.max) + ) { + return GetWindowsDirectoryW($0.baseAddress, DWORD($0.count)) + } + if let windowsDirectory { + // 3. The 16-bit Windows system directory. Windows documentation + // states that "No such standard function (similar to + // GetSystemDirectory) exists for the 16-bit system folder", so use + // "\(windowsDirectory)\System" instead. + directories.append(FilePath(windowsDirectory).appending("System").string) + // 4. The Windows directory. + directories.append(windowsDirectory) + } + + // Filtered the way a `PATH` value is, so the absolute-only rule holds + // however the system answered. + return directories.filter { FilePath($0).isAbsolute } + }() + internal func resolveExecutablePath(withPathValue pathValue: String?) throws(SubprocessError) -> String { switch self.storage { case .executable(let executableName): - let (searchResult, searchError) = try Self.searchPath( - for: executableName, - withPathValue: pathValue - ) - if let searchResult { - if Configuration.executableAccessible(searchResult) { - return searchResult - } - // `SearchPathW` matches directories as well as files, and it - // stops at its first match with no way to resume, so a - // directory named like the executable hides every later - // candidate. A directory is never runnable; continue the - // search over the candidate paths, which replicate the same - // search order, and take the first one that is a real file. - let firstAccessibleExecutable = possibleExecutablePaths(withPathValue: pathValue) - .first { Configuration.executableAccessible($0) } - if let firstAccessibleExecutable { - return firstAccessibleExecutable - } + let firstAccessibleExecutable = try possibleExecutablePaths(withPathValue: pathValue) + .first { Configuration.executableAccessible($0) } + if let firstAccessibleExecutable { + return firstAccessibleExecutable } throw SubprocessError.executableNotFound( executableName, - underlyingError: SubprocessError.WindowsError(win32Error: searchError) + underlyingError: SubprocessError.WindowsError(win32Error: DWORD(ERROR_FILE_NOT_FOUND)) ) case .path(let executablePath): // Use path directly @@ -737,189 +813,30 @@ extension Executable { } } - /// Runs `SearchPathW` for `executableName`, returning the path it matched - /// and, when it matched nothing, the reason it reported. + /// The paths to try, in order, for this executable. /// - /// The `withCString` helpers this uses carry a typed `throws`, so this is - /// declared throwing even though `SearchPathW` itself reports failure in - /// its return value rather than by throwing. - private static func searchPath( - for executableName: String, - withPathValue pathValue: String? - ) throws(SubprocessError) -> (path: String?, error: DWORD) { - return try executableName._withCString( - encodedAs: UTF16.self - ) { exeName throws(SubprocessError) -> (String?, DWORD) in - return try pathValue.withOptionalCString( - encodedAs: UTF16.self - ) { path throws(SubprocessError) -> (String?, DWORD) in - let pathLength = SearchPathW( - path, - exeName, - nil, - 0, - nil, - nil - ) - guard pathLength > 0 else { - return (nil, GetLastError()) - } - let resolved = withUnsafeTemporaryAllocation( - of: WCHAR.self, - capacity: Int(pathLength) + 1 - ) { - _ = SearchPathW( - path, - exeName, - nil, - pathLength + 1, - $0.baseAddress, - nil - ) - return String(decodingCString: $0.baseAddress!, as: UTF16.self) - } - return (resolved, DWORD(ERROR_FILE_NOT_FOUND)) - } - } - } - - /// `CreateProcessW` allows users to specify the executable path via - /// `lpApplicationName` or via `lpCommandLine`. - /// - /// However, only `lpCommandLine` supports - /// path searching, whereas `lpApplicationName` does not. To support the - /// "argument 0 override" feature, Subprocess must use `lpApplicationName` instead of - /// relying on `lpCommandLine` (so we can potentially set a different value for `lpCommandLine`). + /// A name is joined to each directory returned by + /// `Executable.searchPaths(withPathValue:)`, and only to those: unlike + /// `CreateProcessW`'s own search, the directory the application loaded + /// from, the current directory, and the system directories are not + /// consulted, and the `PATH` that is walked is the subprocess's rather than + /// this process's. See ``Executable/name(_:)`` for the contract this implements. /// - /// This method replicates the executable searching behavior of `CreateProcessW`'s - /// `lpCommandLine`. Specifically, it follows the steps listed in `CreateProcessW`'s documentation: - /// - /// 1. The directory from which the application loaded. - /// 2. The current directory for the calling process. - /// 3. The 32-bit Windows system directory. - /// 4. The 16-bit Windows system directory. - /// 5. The Windows directory. - /// 6. The directories that are listed in the PATH environment variable. - /// - /// For more information: - /// https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw + /// Each directory is tried with every extension in `PATHEXT`, since + /// `CreateProcessW` appends only `.exe` and a name such as `npm` is + /// commonly a `.cmd`. internal func possibleExecutablePaths( withPathValue pathValue: String? - ) -> _OrderedSet { - func insertExecutableAddingExtension( - _ name: String, - currentPath: String, - pathExtensions: _OrderedSet, - storage: inout _OrderedSet - ) { - let fullPath = FilePath(currentPath).appending(name) - if !name.hasExtension() { - for ext in pathExtensions { - var path = fullPath - path.extension = ext - storage.insert(path.string) - } - } else { - storage.insert(fullPath.string) - } - } - + ) throws(SubprocessError) -> _OrderedSet { switch self.storage { case .executable(let name): + try Self.validate(name: name) var possiblePaths: _OrderedSet = .init() - let currentEnvironmentValues = Environment.currentEnvironmentValues() - // If `name` does not include extensions, we need to try these extensions - var pathExtensions: _OrderedSet = _OrderedSet(["com", "exe", "bat", "cmd"]) - if let extensionList = currentEnvironmentValues["PATHEXT"] { - for var ext in extensionList.split(separator: ";") { - ext.removeFirst(1) - pathExtensions.insert(String(ext).lowercased()) - } - } - // 1. The directory from which the application loaded. - let applicationDirectory = try? fillNullTerminatedWideStringBuffer( - initialSize: DWORD(MAX_PATH), maxSize: DWORD(Int16.max) - ) { - return GetModuleFileNameW(nil, $0.baseAddress, DWORD($0.count)) - } - if let applicationDirectory { - insertExecutableAddingExtension( - name, - currentPath: FilePath(applicationDirectory).removingLastComponent().string, - pathExtensions: pathExtensions, - storage: &possiblePaths - ) - } - // 2. Current directory - let directorySize = GetCurrentDirectoryW(0, nil) - let currentDirectory = try? fillNullTerminatedWideStringBuffer( - initialSize: directorySize, - maxSize: DWORD(Int16.max) - ) { - return GetCurrentDirectoryW(DWORD($0.count), $0.baseAddress) - } - if let currentDirectory { - insertExecutableAddingExtension( - name, - currentPath: currentDirectory, - pathExtensions: pathExtensions, - storage: &possiblePaths - ) - } - // 3. System directory (System32) - let systemDirectorySize = GetSystemDirectoryW(nil, 0) - let systemDirectory = try? fillNullTerminatedWideStringBuffer( - initialSize: systemDirectorySize, - maxSize: DWORD(Int16.max) - ) { - return GetSystemDirectoryW($0.baseAddress, DWORD($0.count)) - } - if let systemDirectory { - insertExecutableAddingExtension( - name, - currentPath: systemDirectory, - pathExtensions: pathExtensions, - storage: &possiblePaths - ) - } - // 4. The Windows directory - let windowsDirectorySize = GetWindowsDirectoryW(nil, 0) - let windowsDirectory = try? fillNullTerminatedWideStringBuffer( - initialSize: windowsDirectorySize, - maxSize: DWORD(Int16.max) - ) { - return GetWindowsDirectoryW($0.baseAddress, DWORD($0.count)) - } - if let windowsDirectory { - insertExecutableAddingExtension( - name, - currentPath: windowsDirectory, - pathExtensions: pathExtensions, - storage: &possiblePaths - ) - - // 5. 16 bit System Directory - // Windows documentation stats that "No such standard function - // (similar to GetSystemDirectory) exists for the 16-bit system folder". - // Use "\(windowsDirectory)\System" instead - let systemDirectory16 = FilePath(windowsDirectory).appending("System") - insertExecutableAddingExtension( - name, - currentPath: systemDirectory16.string, - pathExtensions: pathExtensions, - storage: &possiblePaths - ) - } - // 6. The directories that are listed in the PATH environment variable - if let pathValue { - let searchPaths = pathValue.split(separator: ";").map { String($0) } - for possiblePath in searchPaths { - insertExecutableAddingExtension( - name, - currentPath: possiblePath, - pathExtensions: pathExtensions, - storage: &possiblePaths - ) + let fileNames = Self.candidateFileNames(for: name) + for directory in Self.searchPaths(withPathValue: pathValue) { + let directoryPath = FilePath(directory) + for fileName in fileNames { + possiblePaths.insert(directoryPath.appending(fileName).string) } } return possiblePaths @@ -927,16 +844,45 @@ extension Executable { return _OrderedSet([path.string]) } } + + /// The file names to look for, in order, for the executable named `name`. + /// + /// A name that already carries an extension is tried as written first, + /// matching `CreateProcessW`, which appends `.exe` only to a name that has + /// no extension at all. Every `PATHEXT` extension is then *appended*, never + /// substituted, so a dotted name still resolves: `python3.11` looks for + /// `python3.11` and then `python3.11.exe`, not `python3.exe`. A name that + /// resolves to a `.bat` or `.cmd` runs through the hardened `cmd.exe` + /// invocation in `generateWindowsCommandAndArguments(withPossibleExecutablePath:)`. + private static func candidateFileNames(for name: String) -> [String] { + // If `name` does not include extensions, we need to try these extensions + var pathExtensions: _OrderedSet = _OrderedSet(["com", "exe", "bat", "cmd"]) + if let extensionList = Environment.currentEnvironmentValues()["PATHEXT"] { + for var ext in extensionList.split(separator: ";") { + ext.removeFirst(1) + pathExtensions.insert(String(ext).lowercased()) + } + } + var fileNames: [String] = FilePath(name).extension == nil ? [] : [name] + fileNames.append(contentsOf: pathExtensions.map { "\(name).\($0)" }) + return fileNames + } } // MARK: - Environment Resolution extension Environment { + /// The `PATH` value to resolve ``Executable/name(_:)`` against. + /// + /// The value the subprocess receives wins, so a name resolves in the + /// environment it will run in. When that environment carries no `PATH`, + /// this process's own value is used.`nil` means neither defines one. internal func pathValue() -> String? { switch self.config { case .inherit(let overrides): - // If PATH value exists in overrides, use it - if let value = overrides[.path] { - return value + // If PATH value exists in overrides, use it. An override maps to + // `nil` to unset the value, which falls through to this process. + if let overridden = overrides[.path], let overridden { + return overridden } // Fall back to current process return Self.currentEnvironmentValues()[.path] @@ -944,7 +890,7 @@ extension Environment { if let value = fullEnvironment[.path] { return value } - return nil + return Self.currentEnvironmentValues()[.path] } } @@ -1584,9 +1530,10 @@ extension Configuration { /// Returns whether `path` names something this process can execute: it must /// exist, and it must not be a directory. /// - /// Existence alone is not enough. Both `SearchPathW` and the - /// `CreateProcessW` candidate loop otherwise accept a directory whose name - /// matches the executable, and a directory can never be run. + /// Existence alone is not enough. The candidate paths that + /// `possibleExecutablePaths(withPathValue:)` produces otherwise accept a + /// directory whose name matches the executable, and a directory can never + /// be run. internal static func executableAccessible(_ path: String) -> Bool { return path.withCString(encodedAs: UTF16.self) { let attrs = GetFileAttributesW($0) @@ -1757,11 +1704,6 @@ extension String { } } - internal func hasExtension() -> Bool { - let components = self.split(separator: ".") - return components.count > 1 && components.last?.count == 3 - } - /// Returns `true` if this path names a Windows batch file (`.bat` or /// `.cmd`), which `CreateProcessW` executes by launching `cmd.exe`. /// diff --git a/Sources/_SubprocessCShims/include/process_shims.h b/Sources/_SubprocessCShims/include/process_shims.h index ec7dde9a..46b5613b 100644 --- a/Sources/_SubprocessCShims/include/process_shims.h +++ b/Sources/_SubprocessCShims/include/process_shims.h @@ -111,6 +111,14 @@ int _was_process_suspended(int status); /// correct type regardless of how the Swift Glibc/Darwin overlay imports it. uint64_t _subprocess_nofile_soft_limit(void); +/// Writes the system's standard `PATH` value into `buffer`. +/// +/// Follows the `confstr(3)` protocol: returns the buffer size required to hold +/// the value including its null terminator, and writes at most `size` bytes, +/// truncating and null-terminating if the value does not fit. Returns 0 when +/// the platform reports no standard path, in which case nothing is written. +size_t _subprocess_default_search_path(char * _Nullable buffer, size_t size); + void _subprocess_lock_environ(void); void _subprocess_unlock_environ(void); char * _Nullable * _Nullable _subprocess_get_environ(void); diff --git a/Sources/_SubprocessCShims/process_shims.c b/Sources/_SubprocessCShims/process_shims.c index b26d0932..51b39f0c 100644 --- a/Sources/_SubprocessCShims/process_shims.c +++ b/Sources/_SubprocessCShims/process_shims.c @@ -45,6 +45,11 @@ #include #include +#if __has_include() +// For _PATH_STDPATH / _PATH_DEFPATH +#include +#endif + #if __has_include() #include #elif defined(_WIN32) @@ -86,6 +91,42 @@ uint64_t _subprocess_nofile_soft_limit(void) { return (uint64_t)rl.rlim_cur; } +size_t _subprocess_default_search_path(char * _Nullable buffer, size_t size) { + // `confstr(_CS_PATH)` is the POSIX query for the standard path, and is + // preferred because the C library answers for the running system. Bionic + // only declares it from API level 26, so Android uses its `` + // macro instead rather than raising this package's minimum API level. +#if defined(_CS_PATH) && !defined(__ANDROID__) + size_t queried = confstr(_CS_PATH, buffer, size); + if (queried > 0) { + return queried; + } +#endif + + // Fall back to the standard path fixed at compile time. `_PATH_STDPATH` is + // the system utility path on Darwin and the BSDs and with glibc; + // `_PATH_DEFPATH` is what Bionic and musl provide. +#if defined(_PATH_STDPATH) + const char *standardPath = _PATH_STDPATH; +#elif defined(_PATH_DEFPATH) + const char *standardPath = _PATH_DEFPATH; +#else + const char *standardPath = NULL; +#endif + + if (standardPath == NULL) { + return 0; + } + size_t required = strlen(standardPath) + 1; + if (buffer != NULL && size > 0) { + // Truncate and terminate the way `confstr` does, so the caller can + // detect the short buffer from the returned size alone. + strncpy(buffer, standardPath, size - 1); + buffer[size - 1] = '\0'; + } + return required; +} + int _subprocess_pthread_create( #if TARGET_OS_MAC || defined(__FreeBSD__) || defined(__OpenBSD__) || TARGET_LIBC_MUSL pthread_t _Nullable * _Nonnull ptr, diff --git a/Tests/SubprocessTests/IntegrationTests.swift b/Tests/SubprocessTests/IntegrationTests.swift index c2123e3e..a4ed5a68 100644 --- a/Tests/SubprocessTests/IntegrationTests.swift +++ b/Tests/SubprocessTests/IntegrationTests.swift @@ -92,6 +92,39 @@ extension SubprocessIntegrationTests { } } + /// `name(_:)` takes a name to look up on `PATH`, so a value that names a + /// location instead is rejected rather than resolved against a current + /// directory. `/` is a path separator on every platform Subprocess supports. + @Test func testExecutableNameWithPathSeparatorIsRejected() async throws { + let error = await #expect(throws: SubprocessError.self) { + _ = try await Subprocess.run(.name("bin/do-not-exist"), output: .discarded) + } + #expect(error?.code == .spawnFailed) + + let resolveError = await #expect(throws: SubprocessError.self) { + _ = try await Executable.name("bin/do-not-exist").resolveExecutablePath(in: .inherit) + } + #expect(resolveError?.code == .spawnFailed) + } + + /// A `PATH` that is set but empty lists no directories, so a name resolves + /// to nothing rather than falling back to a built-in list of directories. + @Test func testExecutableNameWithEmptyPathValue() async throws { + #if os(Windows) + let name = "cmd.exe" + #else + let name = "echo" + #endif + let error = await #expect(throws: SubprocessError.self) { + _ = try await Subprocess.run( + .name(name), + environment: .custom(["PATH": ""]), + output: .discarded + ) + } + #expect(error?.code == .executableNotFound) + } + @Test func testExecutableAtPath() async throws { #if os(Windows) let cmdExe = ProcessInfo.processInfo.environment["COMSPEC"] ?? ProcessInfo.processInfo.environment["ComSpec"] ?? ProcessInfo.processInfo.environment["comspec"] @@ -613,12 +646,13 @@ extension SubprocessIntegrationTests { // PATH="$PATH:$HOME/Desktop/.DS_Store" // posix_spawn returns ENOTDIR in this case because `.DS_Store` is a valid // file, but not a directory so it can't append the executable name. + // Such an entry must be skipped, letting a later entry win. let setup = TestSetup( executable: .name("echo"), arguments: ["testEnvironmentPathWithNonDirectoryPaths"], environment: .inherit.updating([ // /bin/ls is a valid file, but not directory - "PATH": "/bin/ls" + "PATH": "/bin/ls:/bin" ]) ) diff --git a/Tests/SubprocessTests/UnixTests.swift b/Tests/SubprocessTests/UnixTests.swift index ea598ab2..4f3be629 100644 --- a/Tests/SubprocessTests/UnixTests.swift +++ b/Tests/SubprocessTests/UnixTests.swift @@ -343,51 +343,233 @@ extension SubprocessUnixTests { let executable = Executable.name("test-bin") let pathValue = "/first/path:/second/path:/third/path" - let paths = executable.possibleExecutablePaths(withPathValue: pathValue) + let paths = try executable.possibleExecutablePaths(withPathValue: pathValue) let pathsArray = Array(paths) #expect( pathsArray == [ - "test-bin", "/first/path/test-bin", "/second/path/test-bin", "/third/path/test-bin", - - // Default search paths - "/usr/bin/test-bin", - "/bin/test-bin", - "/usr/sbin/test-bin", - "/sbin/test-bin", - "/usr/local/bin/test-bin", ]) } @Test func testNoDuplicatedExecutablePaths() throws { let executable = Executable.name("test-bin") let duplicatePath = "/first/path:/first/path:/second/path" - let duplicatePaths = executable.possibleExecutablePaths(withPathValue: duplicatePath) + let duplicatePaths = try executable.possibleExecutablePaths(withPathValue: duplicatePath) #expect(Array(duplicatePaths).count == Set(duplicatePaths).count) } + /// The system's standard directories are a last resort for a `PATH`-less + /// environment, not an addition to a `PATH` that exists. @Test func testPossibleExecutablePathsWithNilPATH() throws { let executable = Executable.name("test-bin") - let paths = executable.possibleExecutablePaths(withPathValue: nil) - let pathsArray = Array(paths) + let paths = try executable.possibleExecutablePaths(withPathValue: nil) #expect( - pathsArray == [ - "test-bin", - - // Default search paths - "/usr/bin/test-bin", - "/bin/test-bin", - "/usr/sbin/test-bin", - "/sbin/test-bin", - "/usr/local/bin/test-bin", + Array(paths) == Executable.defaultSearchPaths.map { "\($0)/test-bin" } + ) + } + + /// That last resort is queried from the system rather than hard-coded, so it + /// is whatever this platform considers standard — and it is filtered the way + /// a `PATH` value is. + @Test func testDefaultSearchPathsComeFromTheSystem() throws { + let defaultSearchPaths = Executable.defaultSearchPaths + #expect(!defaultSearchPaths.isEmpty) + for directory in defaultSearchPaths { + #expect(FilePath(directory).isAbsolute, "\(directory) is not absolute") + } + } + + /// The standard directories are the ones `confstr(_CS_PATH)` reports, which + /// is what `getconf PATH` prints and what `execvp(3)` searches when `PATH` + /// is unset. + @Test( + .enabled( + if: FileManager.default.isExecutableFile(atPath: "/usr/bin/getconf"), + "This test requires getconf" + ) + ) + func testDefaultSearchPathsMatchTheSystemStandardPath() async throws { + let result = try await Subprocess.run( + .path("/usr/bin/getconf"), + arguments: ["PATH"], + output: .string(limit: 4096) + ) + try #require(result.terminationStatus.isSuccess) + let systemStandardPath = result.standardOutput.trimmingNewLineAndQuotes() + #expect( + Executable.defaultSearchPaths == systemStandardPath.split(separator: ":").map(String.init) + ) + } + + /// An empty `PATH` is a `PATH` that lists no directories, so nothing is + /// searched — the built-in directories do not come back. + @Test func testPossibleExecutablePathsWithEmptyPATH() throws { + let executable = Executable.name("test-bin") + #expect(Array(try executable.possibleExecutablePaths(withPathValue: "")).isEmpty) + } + + /// Empty entries, which a leading, trailing, or doubled `:` introduces, and + /// relative entries are both a search of a current working directory, and + /// are skipped. + @Test func testExecutablePathsSkipEmptyAndRelativeEntries() throws { + let executable = Executable.name("test-bin") + let paths = try executable.possibleExecutablePaths( + withPathValue: ":/first/path::relative/path:.:..:/second/path:" + ) + + #expect( + Array(paths) == [ + "/first/path/test-bin", + "/second/path/test-bin", ]) } + /// A name containing a path separator is a path, and is rejected rather + /// than resolved against some current directory. + @Test func testNameWithPathSeparatorIsRejected() async throws { + for name in ["bin/test-bin", "./test-bin", "/usr/bin/test-bin", "../test-bin"] { + #expect(throws: SubprocessError.self) { + _ = try Executable.name(name).possibleExecutablePaths(withPathValue: "/usr/bin") + } + let error = await #expect(throws: SubprocessError.self) { + _ = try await Executable.name(name).resolveExecutablePath(in: .inherit) + } + #expect(error?.code == .spawnFailed) + // The same name is rejected at spawn time, not just when resolving. + let spawnError = await #expect(throws: SubprocessError.self) { + _ = try await Subprocess.run(.name(name), output: .discarded) + } + #expect(spawnError?.code == .spawnFailed) + } + } + + /// An existing executable named without a separator still resolves through + /// `PATH` only, so the name of a real binary that happens to sit in the + /// current directory is not enough to run it. + #if !os(Android) // Exit tests are not supported on Android + @Test func testCurrentWorkingDirectoryIsNotSearched() async throws { + await #expect(processExitsWith: .success) { + // Runs in an isolated process because it changes the current + // working directory, which is process-wide state that sibling + // suites in the same process would otherwise see. + let fixture = FileManager.default.temporaryDirectory + .appendingPathComponent("cwd-probe-\(UUID().uuidString)") + let searchDirectory = fixture.appendingPathComponent("bin") + try FileManager.default.createDirectory( + at: searchDirectory, withIntermediateDirectories: true + ) + defer { try? FileManager.default.removeItem(at: fixture) } + + // The executable exists *only* in the current directory, and the + // current directory is not on PATH. + let name = "test-executable-\(UUID().uuidString)" + let executable = fixture.appendingPathComponent(name) + try """ + #!/bin/sh + echo "CWD" + """.write(to: executable, atomically: true, encoding: .utf8) + try FileManager.default.setAttributes( + [.posixPermissions: 0o755], ofItemAtPath: executable._fileSystemPath + ) + #expect(FileManager.default.changeCurrentDirectoryPath(fixture._fileSystemPath)) + + let environment = Environment.custom(["PATH": searchDirectory._fileSystemPath]) + let resolveError = await #expect(throws: SubprocessError.self) { + _ = try await Executable.name(name).resolveExecutablePath(in: environment) + } + #expect(resolveError?.code == .executableNotFound) + + let spawnError = await #expect(throws: SubprocessError.self) { + _ = try await Subprocess.run( + .name(name), + environment: environment, + output: .discarded + ) + } + #expect(spawnError?.code == .executableNotFound) + } + } + #endif // !os(Android) + + /// The working directory handed to `run` is where the subprocess starts, + /// not a directory the executable is looked for in. Resolution happens in + /// the parent, before the child changes directory, so the two agree. + @Test func testWorkingDirectoryIsNotSearched() async throws { + try await withExecutableSearchFixture { fixture in + let binDirectory = fixture.appendingPathComponent("bin") + try FileManager.default.createDirectory( + at: binDirectory, withIntermediateDirectories: true + ) + let name = "test-executable-\(UUID().uuidString)" + // The executable exists only in the working directory, which is + // not on PATH. + try Self.writeExecutableScript( + at: fixture.appendingPathComponent(name), echoing: "WORKING-DIRECTORY" + ) + + let spawnError = await #expect(throws: SubprocessError.self) { + _ = try await Subprocess.run( + .name(name), + environment: .custom(["PATH": binDirectory._fileSystemPath]), + workingDirectory: FilePath(fixture._fileSystemPath), + output: .discarded + ) + } + #expect(spawnError?.code == .executableNotFound) + } + } + + /// The `PATH` the subprocess receives is what a name resolves against, so + /// the executable that resolution picks is the one that runs. + @Test func testResolutionUsesTheSubprocessPathValue() async throws { + try await withExecutableSearchFixture { fixture in + let childDirectory = fixture.appendingPathComponent("child") + try FileManager.default.createDirectory( + at: childDirectory, withIntermediateDirectories: true + ) + let name = "test-executable-\(UUID().uuidString)" + let executable = childDirectory.appendingPathComponent(name) + try Self.writeExecutableScript(at: executable, echoing: "CHILD") + + // The directory is reachable only through the environment passed to + // the subprocess; it is on neither the current process's PATH nor + // the built-in list. + let environment = Environment.custom(["PATH": childDirectory._fileSystemPath]) + let resolved = try await Executable.name(name).resolveExecutablePath(in: environment) + #expect(resolved.string == executable._fileSystemPath) + + let result = try await Subprocess.run( + .name(name), + environment: environment, + output: .string(limit: 32) + ) + #expect(result.standardOutput.trimmingNewLineAndQuotes() == "CHILD") + } + } + + /// When the environment passed to the subprocess sets no `PATH` of its own, + /// the current process's value is what gets searched. + @Test func testPathValueFallsBackToCurrentProcess() throws { + let currentPathValue = try #require(ProcessInfo.processInfo.environment["PATH"]) + + // A PATH for the subprocess is preferred, however it is expressed. + #expect(Environment.custom(["PATH": "/child/bin"]).pathValue() == "/child/bin") + #expect(Environment.inherit.updating(["PATH": "/child/bin"]).pathValue() == "/child/bin") + #expect(Environment.custom([Array("PATH=/child/bin".utf8)]).pathValue() == "/child/bin") + + // Without one, the current process's value is used — including when the + // subprocess environment explicitly unsets `PATH`. + #expect(Environment.custom(["MARKER": "no-path-here"]).pathValue() == currentPathValue) + #expect(Environment.custom([Array("MARKER=no-path-here".utf8)]).pathValue() == currentPathValue) + #expect(Environment.inherit.updating(["PATH": nil]).pathValue() == currentPathValue) + #expect(Environment.inherit.pathValue() == currentPathValue) + } + /// A `PATH` entry that contains a *directory* whose name matches the /// executable must be skipped: the execute bit on a directory only means /// "searchable", not "runnable". @@ -429,20 +611,6 @@ extension SubprocessUnixTests { } } - /// A name that is itself the path of a directory must not resolve to that - /// directory, even though the directory is "executable" to `access(X_OK)`. - @Test func testNameThatIsADirectoryPathIsNotResolved() async throws { - try await withExecutableSearchFixture { fixture in - let directory = fixture.appendingPathComponent("test-executable-\(UUID().uuidString)") - try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) - - await #expect(throws: SubprocessError.self) { - _ = try await Executable.name(directory._fileSystemPath) - .resolveExecutablePath(in: .inherit) - } - } - } - /// The regular-file requirement must not weaken the permission check: a /// non-executable file that shares the name is still skipped. @Test func testNameResolutionSkipsNonExecutableRegularFile() async throws { diff --git a/Tests/SubprocessTests/WindowsTests.swift b/Tests/SubprocessTests/WindowsTests.swift index 210fa438..bf65aad4 100644 --- a/Tests/SubprocessTests/WindowsTests.swift +++ b/Tests/SubprocessTests/WindowsTests.swift @@ -629,10 +629,240 @@ extension SubprocessWindowsTests { // MARK: - Executable Resolution extension SubprocessWindowsTests { + /// Only the directories on `PATH` are searched, and only with the + /// extensions `PATHEXT` names. `CreateProcessW`'s own search would also + /// cover the application directory, the current directory, and the system + /// directories; `name(_:)` covers none of them. + @Test func testExecutablePathsAreLimitedToThePathValue() throws { + let paths = try Executable.name("test-bin").possibleExecutablePaths( + withPathValue: #"C:\first\path;C:\second\path"# + ) + let pathsArray = Array(paths) + + // Every candidate sits directly in one of the two `PATH` entries. + func isUnderAPathEntry(_ path: String) -> Bool { + let normalized = path.replacingOccurrences(of: "/", with: #"\"#).lowercased() + return normalized.hasPrefix(#"c:\first\path\"#) + || normalized.hasPrefix(#"c:\second\path\"#) + } + for path in pathsArray { + #expect(isUnderAPathEntry(path), "\(path) is outside the PATH value") + } + + // `.exe` in the first `PATH` entry is among them, and the first entry + // is searched before the second. + let firstExe = try #require( + pathsArray.firstIndex(where: { Self.isSamePath($0, #"C:\first\path\test-bin.exe"#) }) + ) + let secondExe = try #require( + pathsArray.firstIndex(where: { Self.isSamePath($0, #"C:\second\path\test-bin.exe"#) }) + ) + #expect(firstExe < secondExe) + + // Neither the current directory nor the system directory is a + // candidate, whatever they happen to be. + let systemDirectory = try fillNullTerminatedWideStringBuffer( + initialSize: DWORD(MAX_PATH), + maxSize: DWORD(Int16.max) + ) { + GetSystemDirectoryW($0.baseAddress, DWORD($0.count)) + } + for directory in [FileManager.default.currentDirectoryPath, systemDirectory] { + let shadowed = FilePath(directory).appending("test-bin.exe").string + #expect(!pathsArray.contains(where: { Self.isSamePath($0, shadowed) })) + } + } + + /// A `PATHEXT` extension is appended to the name, never substituted for an + /// extension the name already has, and a name that carries an extension is + /// tried as written first. + @Test func testExecutablePathsAppendExtensionsToDottedNames() throws { + let paths = try Executable.name("python3.11").possibleExecutablePaths( + withPathValue: #"C:\bin"# + ) + let pathsArray = Array(paths) + + let first = try #require(pathsArray.first) + #expect(Self.isSamePath(first, #"C:\bin\python3.11"#)) + #expect(pathsArray.contains(where: { Self.isSamePath($0, #"C:\bin\python3.11.exe"#) })) + // `.11` must not be mistaken for the extension and replaced. + #expect(!pathsArray.contains(where: { Self.isSamePath($0, #"C:\bin\python3.exe"#) })) + + // A name with no extension is only tried with extensions appended. + let extensionless = Array( + try Executable.name("test-bin").possibleExecutablePaths(withPathValue: #"C:\bin"#) + ) + #expect(!extensionless.contains(where: { Self.isSamePath($0, #"C:\bin\test-bin"#) })) + #expect(extensionless.contains(where: { Self.isSamePath($0, #"C:\bin\test-bin.exe"#) })) + #expect(extensionless.contains(where: { Self.isSamePath($0, #"C:\bin\test-bin.cmd"#) })) + } + + /// Empty entries, which a leading, trailing, or doubled `;` introduces, and + /// entries that aren't fully qualified are both a search of a current + /// directory, and are skipped. A drive-relative `C:bin` and a + /// root-relative `\bin` each depend on a current directory too. + @Test func testExecutablePathsSkipEmptyAndRelativeEntries() throws { + let relativeOnly = try Executable.name("test-bin").possibleExecutablePaths( + withPathValue: #";;relative\path;.;C:bin;\bin"# + ) + #expect(Array(relativeOnly).isEmpty) + + let empty = try Executable.name("test-bin").possibleExecutablePaths(withPathValue: "") + #expect(Array(empty).isEmpty) + } + + /// `name(_:)` takes a name, so anything that names a location is rejected + /// rather than resolved against a current directory. Windows accepts either + /// slash, and `:` separates a drive from a drive-relative path. + @Test func testNameWithPathSeparatorIsRejected() async throws { + let names = [ + #"bin\test-bin.exe"#, + #"bin/test-bin.exe"#, + #".\test-bin.exe"#, + #"C:\Windows\System32\cmd.exe"#, + "C:cmd.exe", + ] + for name in names { + #expect(throws: SubprocessError.self) { + _ = try Executable.name(name).possibleExecutablePaths(withPathValue: #"C:\bin"#) + } + let resolveError = await #expect(throws: SubprocessError.self) { + _ = try await Executable.name(name).resolveExecutablePath(in: .inherit) + } + #expect(resolveError?.code == .spawnFailed) + let spawnError = await #expect(throws: SubprocessError.self) { + _ = try await Subprocess.run(.name(name), output: .discarded) + } + #expect(spawnError?.code == .spawnFailed) + } + } + + /// The current directory is not searched, so an executable that sits there + /// and nowhere on `PATH` is not found — not by resolution, and not by + /// `CreateProcessW`, whose own search would have found it. + @Test func testCurrentDirectoryIsNotSearched() async throws { + await #expect(processExitsWith: .success) { + // Runs in an isolated process because it changes the current + // directory, which is process-wide state that sibling suites + // running in the same process would otherwise observe. + try await SubprocessWindowsTests.withExecutableSearchFixture { shadow, bin, name in + // The executable exists only in what becomes the current directory. + try SubprocessWindowsTests.copyCmdExe(to: shadow.appendingPathComponent(name)) + let originalDirectory = FileManager.default.currentDirectoryPath + // Restore before the fixture is removed: Windows refuses to + // delete a directory that is a process's current directory. + defer { _ = FileManager.default.changeCurrentDirectoryPath(originalDirectory) } + #expect(FileManager.default.changeCurrentDirectoryPath(shadow._fileSystemPath)) + + let environment = Environment.inherit.updating(["PATH": bin._fileSystemPath]) + let resolveError = await #expect(throws: SubprocessError.self) { + _ = try await Executable.name(name).resolveExecutablePath(in: environment) + } + #expect(resolveError?.code == .executableNotFound) + + let spawnError = await #expect(throws: SubprocessError.self) { + _ = try await Subprocess.run( + .name(name), + arguments: ["/c", "echo CURRENT-DIRECTORY"], + environment: environment, + output: .discarded + ) + } + #expect(spawnError?.code == .executableNotFound) + } + } + } + + /// The working directory handed to `run` is where the subprocess starts, + /// not a directory the executable is looked for in. + @Test func testWorkingDirectoryIsNotSearched() async throws { + try await Self.withExecutableSearchFixture { shadowDirectory, binDirectory, name in + // The executable exists only in the working directory. + try Self.copyCmdExe(to: shadowDirectory.appendingPathComponent(name)) + + let spawnError = await #expect(throws: SubprocessError.self) { + _ = try await Subprocess.run( + .name(name), + arguments: ["/c", "echo WORKING-DIRECTORY"], + environment: .inherit.updating(["PATH": binDirectory._fileSystemPath]), + workingDirectory: FilePath(shadowDirectory._fileSystemPath), + output: .discarded + ) + } + #expect(spawnError?.code == .executableNotFound) + } + } + + /// The `PATH` that a name resolves against is the subprocess's own, not + /// this process's. `CreateProcessW` reads `PATH` from the calling process + /// when it performs its own search, which is why Subprocess resolves the + /// name itself before calling it. + @Test func testResolutionUsesTheSubprocessPathValue() async throws { + try await Self.withExecutableSearchFixture { _, binDirectory, name in + let executable = binDirectory.appendingPathComponent(name) + try Self.copyCmdExe(to: executable) + + // `binDirectory` is reachable only through the environment passed + // to the subprocess. + let environment = Environment.inherit.updating([ + "PATH": binDirectory._fileSystemPath + ]) + let resolved = try await Executable.name(name).resolveExecutablePath(in: environment) + #expect(Self.isSamePath(resolved.string, executable._fileSystemPath)) + + let result = try await Subprocess.run( + .name(name), + arguments: ["/c", "echo CHILD"], + environment: environment, + output: .string(limit: 32) + ) + #expect(result.terminationStatus.isSuccess) + #expect(result.standardOutput.trimmingCharacters(in: .whitespacesAndNewlines) == "CHILD") + } + } + + /// With no `PATH` anywhere, the fallback is the set of directories + /// `CreateProcessW` searches on its own — minus the current directory, which + /// `name(_:)` never searches. + @Test func testDefaultSearchPathsAreTheSystemDirectories() throws { + let defaultSearchPaths = Executable.defaultSearchPaths + #expect(!defaultSearchPaths.isEmpty) + for directory in defaultSearchPaths { + #expect(FilePath(directory).isAbsolute, "\(directory) is not absolute") + } + + let systemDirectory = try fillNullTerminatedWideStringBuffer( + initialSize: DWORD(MAX_PATH), + maxSize: DWORD(Int16.max) + ) { + GetSystemDirectoryW($0.baseAddress, DWORD($0.count)) + } + #expect(defaultSearchPaths.contains(where: { Self.isSamePath($0, systemDirectory) })) + #expect( + !defaultSearchPaths.contains(where: { + Self.isSamePath($0, FileManager.default.currentDirectoryPath) + }) + ) + + // Those directories, and only those, are what a `PATH`-less lookup walks. + func directoryPrefix(_ directory: String) -> String { + let normalized = directory.replacingOccurrences(of: "/", with: #"\"#).lowercased() + return normalized.hasSuffix(#"\"#) ? normalized : normalized + #"\"# + } + let paths = try Executable.name("test-bin").possibleExecutablePaths(withPathValue: nil) + for path in paths { + let normalized = path.replacingOccurrences(of: "/", with: #"\"#).lowercased() + #expect( + defaultSearchPaths.contains(where: { normalized.hasPrefix(directoryPrefix($0)) }), + "\(path) is outside the default search paths" + ) + } + } + /// A `PATH` entry that contains a *directory* whose name matches the /// executable must be skipped. `GetFileAttributesW` succeeds for a - /// directory and `SearchPathW` happily matches one, so the real executable - /// later on `PATH` would otherwise be shadowed. + /// directory, so the real executable later on `PATH` would otherwise be + /// shadowed. @Test func testNameResolutionSkipsDirectoryInPathEntry() async throws { try await Self.withExecutableSearchFixture { shadowDirectory, binDirectory, name in // A directory that shares the executable's name, earlier on PATH @@ -652,23 +882,10 @@ extension SubprocessWindowsTests { } } - /// A name that is itself the path of a directory must not resolve to that - /// directory. - @Test func testNameThatIsADirectoryPathIsNotResolved() async throws { - try await Self.withExecutableSearchFixture { shadowDirectory, _, name in - let directory = shadowDirectory.appendingPathComponent(name) - try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) - - await #expect(throws: SubprocessError.self) { - _ = try await Executable.name(directory._fileSystemPath) - .resolveExecutablePath(in: .inherit) - } - } - } - - /// The spawn-time candidate loop is only used when argument 0 is - /// overridden. `CreateProcessW` fails with ERROR_ACCESS_DENIED on a - /// directory, which must not abort the spawn while real candidates remain. + /// A directory candidate is dropped before it reaches `CreateProcessW`, and + /// the `ERROR_ACCESS_DENIED` branch in the spawn loop remains as a backstop + /// for a candidate that becomes a directory after the check. Neither must + /// abort the spawn while real candidates remain. @Test func testSpawnSkipsDirectoryCandidateWithArgumentZeroOverride() async throws { try await Self.withExecutableSearchFixture { shadowDirectory, binDirectory, name in try FileManager.default.createDirectory(