From bb81182d5212d60fbfe27a7ad6267e253367c4f4 Mon Sep 17 00:00:00 2001 From: Jake Petroules Date: Tue, 28 Jul 2026 23:59:00 -0400 Subject: [PATCH 1/4] Don't resolve a directory as an executable on Unix `Executable.name(_:)` resolution accepted any path that `access(_, X_OK)` succeeded on. On a directory the execute bit means "searchable", not "runnable", so a directory whose name matched the executable was returned as a resolved executable path. A `PATH` entry containing a `docker/` subdirectory, for example, would shadow the real `docker` binary and the returned path could then only fail at spawn time. Add `Configuration.executableAccessible(_:)`, which requires a candidate to be a regular file in addition to being executable, and use it for every candidate in `resolveExecutablePath(withPathValue:)`. --- .../Platforms/Subprocess+Unix.swift | 20 +++- Tests/SubprocessTests/UnixTests.swift | 111 ++++++++++++++++++ 2 files changed, 129 insertions(+), 2 deletions(-) diff --git a/Sources/Subprocess/Platforms/Subprocess+Unix.swift b/Sources/Subprocess/Platforms/Subprocess+Unix.swift index d0c4f330..41bf8a13 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Unix.swift +++ b/Sources/Subprocess/Platforms/Subprocess+Unix.swift @@ -371,11 +371,11 @@ extension Executable { switch self.storage { case .executable(let executableName): // If the executableName in is already a full path, return it directly - if Configuration.pathAccessible(executableName, mode: X_OK) { + if Configuration.executableAccessible(executableName) { return executableName } let firstAccessibleExecutable = possibleExecutablePaths(withPathValue: pathValue) - .first { Configuration.pathAccessible($0, mode: X_OK) } + .first { Configuration.executableAccessible($0) } if let firstAccessibleExecutable { return firstAccessibleExecutable } @@ -468,6 +468,22 @@ extension Configuration { return access($0, mode) == 0 } } + + /// Returns whether `path` refers to a regular file this process may execute. + /// + /// `access(_:X_OK)` alone is not enough: on a directory the execute bit + /// means "searchable", so a directory would otherwise be reported as a + /// runnable executable. + internal static func executableAccessible(_ path: String) -> Bool { + guard Self.pathAccessible(path, mode: X_OK) else { + return false + } + var status = stat() + guard path.withCString({ stat($0, &status) == 0 }) else { + return false + } + return (status.st_mode & mode_t(S_IFMT)) == mode_t(S_IFREG) + } } // MARK: - FileDescriptor extensions diff --git a/Tests/SubprocessTests/UnixTests.swift b/Tests/SubprocessTests/UnixTests.swift index ab4ab7a6..fa352127 100644 --- a/Tests/SubprocessTests/UnixTests.swift +++ b/Tests/SubprocessTests/UnixTests.swift @@ -387,6 +387,117 @@ extension SubprocessUnixTests { "/usr/local/bin/test-bin", ]) } + + /// 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". + @Test func testNameResolutionSkipsDirectoryInPathEntry() async throws { + try await withExecutableSearchFixture { fixture in + let shadowDirectory = fixture.appendingPathComponent("shadow") + let binDirectory = fixture.appendingPathComponent("bin") + try FileManager.default.createDirectory( + at: shadowDirectory, withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: binDirectory, withIntermediateDirectories: true + ) + let name = "test-executable-\(UUID().uuidString)" + // A directory that shares the executable's name, earlier on PATH + try FileManager.default.createDirectory( + at: shadowDirectory.appendingPathComponent(name), + withIntermediateDirectories: true + ) + let executable = binDirectory.appendingPathComponent(name) + try Self.writeExecutableScript(at: executable, echoing: "REAL") + + let environment = Environment.inherit.updating([ + "PATH": "\(shadowDirectory._fileSystemPath):\(binDirectory._fileSystemPath)" + ]) + + // Eager resolution + let resolved = try await Executable.name(name).resolveExecutablePath(in: environment) + #expect(resolved.string == executable._fileSystemPath) + + // Spawn-time resolution + let result = try await Subprocess.run( + .name(name), + environment: environment, + output: .string(limit: 16) + ) + #expect(result.terminationStatus.isSuccess) + #expect(result.standardOutput.trimmingNewLineAndQuotes() == "REAL") + } + } + + /// 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 { + try await withExecutableSearchFixture { fixture in + let shadowDirectory = fixture.appendingPathComponent("shadow") + let binDirectory = fixture.appendingPathComponent("bin") + try FileManager.default.createDirectory( + at: shadowDirectory, withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: binDirectory, withIntermediateDirectories: true + ) + let name = "test-executable-\(UUID().uuidString)" + // A regular file that shares the executable's name but is not + // executable, earlier on PATH + let shadow = shadowDirectory.appendingPathComponent(name) + try Data("not executable".utf8).write(to: shadow) + try FileManager.default.setAttributes( + [.posixPermissions: 0o644], ofItemAtPath: shadow._fileSystemPath + ) + let executable = binDirectory.appendingPathComponent(name) + try Self.writeExecutableScript(at: executable, echoing: "REAL") + + let resolved = try await Executable.name(name).resolveExecutablePath( + in: .inherit.updating([ + "PATH": "\(shadowDirectory._fileSystemPath):\(binDirectory._fileSystemPath)" + ]) + ) + #expect(resolved.string == executable._fileSystemPath) + } + } + + // MARK: Fixture helpers + + /// Creates a unique temporary directory, passes it to `body`, and removes + /// it afterwards. + private func withExecutableSearchFixture( + _ body: (URL) async throws -> Void + ) async throws { + let fixture = FileManager.default.temporaryDirectory + .appendingPathComponent("executable-search-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: fixture, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: fixture) } + try await body(fixture) + } + + private static func writeExecutableScript(at url: URL, echoing marker: String) throws { + try """ + #!/bin/sh + echo "\(marker)" + """.write(to: url, atomically: true, encoding: .utf8) + try FileManager.default.setAttributes( + [.posixPermissions: 0o755], ofItemAtPath: url._fileSystemPath + ) + } } // MARK: - Misc From 14d26c0c25b08d64263120365c9b0c86c0fb2217 Mon Sep 17 00:00:00 2001 From: Jake Petroules Date: Wed, 29 Jul 2026 00:33:56 -0400 Subject: [PATCH 2/4] Don't resolve a directory as an executable on Windows `GetFileAttributesW` succeeds for a directory and `SearchPathW` matches directories as well as files, so the Windows resolver had the same false positive as the Unix one: a `tool.exe` *directory* earlier on `PATH` shadowed the real `tool.exe`, and `resolveExecutablePath` handed back a path that can never be run. Verified on Windows 11 arm64: SearchPathW name="tool.exe" ext=nil -> E:\probe\shadow\tool.exe (a directory) CreateProcessW lpApplicationName= -> 5 (ERROR_ACCESS_DENIED) Add `Configuration.executableAccessible(_:)` (exists and is not a directory) and `Configuration.isDirectory(_:)`, replacing the unused private `pathAccessible(_:)`, then: - `resolveExecutablePath(withPathValue:)` rejects a `SearchPathW` match that is a directory. `SearchPathW` cannot be resumed past a match, so it falls back to walking `possibleExecutablePaths(withPathValue:)`, which replicates the same search order, and takes the first real file. Behavior is unchanged whenever `SearchPathW` matches a file. - the spawn-time candidate loop, which is only used when argument 0 is overridden, treats ERROR_ACCESS_DENIED on a directory as a miss and tries the next candidate. Genuine permission failures on a real file still throw. `CreateProcessW`'s own search, used by the fast path, already skips directories, so the fast path needs no change: CWD=, PATH= CreateProcessW(nil, "tool.exe") -> created -> E:\probe\real\tool.exe --- .../Platforms/Subprocess+Windows.swift | 126 +++++++++++++----- Tests/SubprocessTests/WindowsTests.swift | 106 +++++++++++++++ 2 files changed, 200 insertions(+), 32 deletions(-) diff --git a/Sources/Subprocess/Platforms/Subprocess+Windows.swift b/Sources/Subprocess/Platforms/Subprocess+Windows.swift index 3c6a8e69..cbb4a2de 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Windows.swift +++ b/Sources/Subprocess/Platforms/Subprocess+Windows.swift @@ -230,6 +230,18 @@ extension Configuration { continue } + // `CreateProcessW` reports ERROR_ACCESS_DENIED when the path + // names a directory, so a directory sharing the executable's + // name would otherwise fail the spawn outright instead of + // falling through to the remaining candidates. A directory is + // never runnable, so treat it as a miss. Genuine permission + // failures on a real file still throw below. + if windowsError == DWORD(ERROR_ACCESS_DENIED), + Configuration.isDirectory(executablePath) + { + continue + } + try self.safelyCloseMultiple( inputRead: inputReadFileDescriptor, inputWrite: inputWriteFileDescriptor, @@ -695,45 +707,79 @@ extension Executable { internal func resolveExecutablePath(withPathValue pathValue: String?) throws(SubprocessError) -> String { switch self.storage { case .executable(let executableName): - return try executableName._withCString( + 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 + } + } + throw SubprocessError.executableNotFound( + executableName, + underlyingError: SubprocessError.WindowsError(win32Error: searchError) + ) + case .path(let executablePath): + // Use path directly + return executablePath.string + } + } + + /// Runs `SearchPathW` for `executableName`, returning the path it matched + /// and, when it matched nothing, the reason it reported. + /// + /// 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 - ) { exeName throws(SubprocessError) -> String in - return try pathValue.withOptionalCString( - encodedAs: UTF16.self - ) { path throws(SubprocessError) -> String in - let pathLength = SearchPathW( + ) { 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, - 0, - nil, + pathLength + 1, + $0.baseAddress, nil ) - guard pathLength > 0 else { - throw SubprocessError.executableNotFound( - executableName, - underlyingError: SubprocessError.WindowsError(win32Error: GetLastError()) - ) - } - return 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 String(decodingCString: $0.baseAddress!, as: UTF16.self) } + return (resolved, DWORD(ERROR_FILE_NOT_FOUND)) } - case .path(let executablePath): - // Use path directly - return executablePath.string } } @@ -1535,10 +1581,26 @@ extension Configuration { return quoted } - private static func pathAccessible(_ path: String) -> Bool { + /// 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. + internal static func executableAccessible(_ path: String) -> Bool { + return path.withCString(encodedAs: UTF16.self) { + let attrs = GetFileAttributesW($0) + return attrs != INVALID_FILE_ATTRIBUTES + && (attrs & DWORD(FILE_ATTRIBUTE_DIRECTORY)) == 0 + } + } + + /// Returns whether `path` exists and is a directory. + internal static func isDirectory(_ path: String) -> Bool { return path.withCString(encodedAs: UTF16.self) { let attrs = GetFileAttributesW($0) return attrs != INVALID_FILE_ATTRIBUTES + && (attrs & DWORD(FILE_ATTRIBUTE_DIRECTORY)) != 0 } } } diff --git a/Tests/SubprocessTests/WindowsTests.swift b/Tests/SubprocessTests/WindowsTests.swift index 222d731b..02d0523e 100644 --- a/Tests/SubprocessTests/WindowsTests.swift +++ b/Tests/SubprocessTests/WindowsTests.swift @@ -627,6 +627,112 @@ extension SubprocessWindowsTests { } } +// MARK: - Executable Resolution +extension SubprocessWindowsTests { + /// 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. + @Test func testNameResolutionSkipsDirectoryInPathEntry() async throws { + try await Self.withExecutableSearchFixture { shadowDirectory, binDirectory, name in + // A directory that shares the executable's name, earlier on PATH + try FileManager.default.createDirectory( + at: shadowDirectory.appendingPathComponent(name), + withIntermediateDirectories: true + ) + let executable = binDirectory.appendingPathComponent(name) + try Self.copyCmdExe(to: executable) + + let resolved = try await Executable.name(name).resolveExecutablePath( + in: .inherit.updating([ + "PATH": "\(shadowDirectory._fileSystemPath);\(binDirectory._fileSystemPath)" + ]) + ) + #expect(Self.isSamePath(resolved.string, executable._fileSystemPath)) + } + } + + /// 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. + @Test func testSpawnSkipsDirectoryCandidateWithArgumentZeroOverride() async throws { + try await Self.withExecutableSearchFixture { shadowDirectory, binDirectory, name in + try FileManager.default.createDirectory( + at: shadowDirectory.appendingPathComponent(name), + withIntermediateDirectories: true + ) + try Self.copyCmdExe(to: binDirectory.appendingPathComponent(name)) + + let result = try await Subprocess.run( + .name(name), + arguments: .init( + executablePathOverride: name, + remainingValues: ["/c", "echo REAL"] + ), + environment: .inherit.updating([ + "PATH": "\(shadowDirectory._fileSystemPath);\(binDirectory._fileSystemPath)" + ]), + output: .string(limit: 32) + ) + #expect(result.terminationStatus.isSuccess) + #expect(result.standardOutput.trimmingCharacters(in: .whitespacesAndNewlines) == "REAL") + } + } + + /// Compares two Windows paths, which may mix `/` and `\` separators and + /// differ in case. + private static func isSamePath(_ lhs: String, _ rhs: String) -> Bool { + func normalized(_ path: String) -> String { + path.replacingOccurrences(of: "/", with: "\\") + } + return normalized(lhs).caseInsensitiveCompare(normalized(rhs)) == .orderedSame + } + + /// Creates a throwaway `shadow` and `bin` directory pair plus a unique + /// executable name, and removes them afterwards. + private static func withExecutableSearchFixture( + _ body: (URL, URL, String) async throws -> Void + ) async throws { + let fixture = URL.temporaryDirectory + .appendingPathComponent("executable-search-\(UUID().uuidString)") + let shadowDirectory = fixture.appendingPathComponent("shadow") + let binDirectory = fixture.appendingPathComponent("bin") + try FileManager.default.createDirectory(at: shadowDirectory, withIntermediateDirectories: true) + try FileManager.default.createDirectory(at: binDirectory, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: fixture) } + try await body(shadowDirectory, binDirectory, "test-executable-\(UUID().uuidString).exe") + } + + /// Copies `cmd.exe` to `destination` to stand in for an arbitrary + /// executable that the tests can both resolve and run. + private static func copyCmdExe(to destination: URL) throws { + let systemDirectory = try fillNullTerminatedWideStringBuffer( + initialSize: DWORD(MAX_PATH), + maxSize: DWORD(Int16.max) + ) { + GetSystemDirectoryW($0.baseAddress, DWORD($0.count)) + } + try FileManager.default.copyItem( + at: URL(filePath: systemDirectory).appendingPathComponent("cmd.exe"), + to: destination + ) + } +} + // MARK: - User Utils extension SubprocessWindowsTests { private func withTemporaryUser( From 2efeb93009e5ea3c2d11e931079fa524c5e7caaa Mon Sep 17 00:00:00 2001 From: Jake Petroules Date: Wed, 29 Jul 2026 01:00:10 -0400 Subject: [PATCH 3/4] Cover symlinks and other non-regular files in resolution tests The Unix check requires a regular file while the Windows one requires "not a directory", which raises the question of how each treats symlinks. Measured on both platforms; both are already correct, so this adds tests rather than changing either check. Unix uses `stat`, which follows symlinks, so a symlink is classified by its target: a link to an executable resolves, a link to a directory is rejected, and a dangling link fails the `access` check. realexe access(X_OK)= 0 stat S_ISREG=1 S_ISDIR=0 link-to-exe access(X_OK)= 0 stat S_ISREG=1 S_ISDIR=0 lstat S_ISLNK=1 link-to-dir access(X_OK)= 0 stat S_ISREG=0 S_ISDIR=1 lstat S_ISLNK=1 dangling access(X_OK)=-1 stat failed Requiring a regular file rather than merely a non-directory also matches what `execve` itself accepts. A FIFO with the execute bit set passes `access(_, X_OK)` and is not a directory, yet cannot be executed: myfifo: access(X_OK)=0 S_ISREG=0 S_ISFIFO=1 posix_spawn -> 13 (EACCES) On Windows `GetFileAttributesW` reports reparse points by what they point at, so the existing directory bit test is sufficient there: link-to-exe -> REPARSE_POINT CreateProcessW -> created link-to-dir -> DIRECTORY|REPARSE_POINT CreateProcessW -> 5 (ERROR_ACCESS_DENIED) junction-to-dir -> DIRECTORY|REPARSE_POINT Add tests for each case. On Unix, the non-regular-file and symlink-to-directory tests fail before the fix; the symlink-to-executable test passes either way and guards against switching `stat` for `lstat`. The Windows test needs SeCreateSymbolicLinkPrivilege, so it is gated on a `requiresSymbolicLinkPrivilege` condition trait. --- Tests/SubprocessTests/UnixTests.swift | 91 ++++++++++++++++++++++++ Tests/SubprocessTests/WindowsTests.swift | 73 +++++++++++++++++++ 2 files changed, 164 insertions(+) diff --git a/Tests/SubprocessTests/UnixTests.swift b/Tests/SubprocessTests/UnixTests.swift index fa352127..3c1d6462 100644 --- a/Tests/SubprocessTests/UnixTests.swift +++ b/Tests/SubprocessTests/UnixTests.swift @@ -475,6 +475,97 @@ extension SubprocessUnixTests { } } + /// Only a *regular* file can be executed. A FIFO with the execute bit set + /// satisfies `access(_, X_OK)` and is not a directory, yet `execve` rejects + /// it with `EACCES`, so it must not shadow the real executable either. + @Test func testNameResolutionSkipsNonRegularFile() async throws { + try await withExecutableSearchFixture { fixture in + let shadowDirectory = fixture.appendingPathComponent("shadow") + let binDirectory = fixture.appendingPathComponent("bin") + try FileManager.default.createDirectory( + at: shadowDirectory, withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: binDirectory, withIntermediateDirectories: true + ) + let name = "test-executable-\(UUID().uuidString)" + let fifo = shadowDirectory.appendingPathComponent(name) + try #require(fifo._fileSystemPath.withCString { mkfifo($0, 0o755) } == 0) + // `mkfifo` honors the umask, so set the execute bits explicitly. + try FileManager.default.setAttributes( + [.posixPermissions: 0o755], ofItemAtPath: fifo._fileSystemPath + ) + let executable = binDirectory.appendingPathComponent(name) + try Self.writeExecutableScript(at: executable, echoing: "REAL") + + let resolved = try await Executable.name(name).resolveExecutablePath( + in: .inherit.updating([ + "PATH": "\(shadowDirectory._fileSystemPath):\(binDirectory._fileSystemPath)" + ]) + ) + #expect(resolved.string == executable._fileSystemPath) + } + } + + /// Symlinks are resolved, not rejected: the check follows the link with + /// `stat` rather than inspecting the link itself, so a symlink to an + /// executable on `PATH` still resolves and runs. + @Test func testNameResolutionFollowsSymlinkToExecutable() async throws { + try await withExecutableSearchFixture { fixture in + let binDirectory = fixture.appendingPathComponent("bin") + try FileManager.default.createDirectory( + at: binDirectory, withIntermediateDirectories: true + ) + let target = fixture.appendingPathComponent("target-\(UUID().uuidString)") + try Self.writeExecutableScript(at: target, echoing: "LINKED") + let name = "test-executable-\(UUID().uuidString)" + let link = binDirectory.appendingPathComponent(name) + try FileManager.default.createSymbolicLink(at: link, withDestinationURL: target) + + let environment = Environment.inherit.updating([ + "PATH": binDirectory._fileSystemPath + ]) + let resolved = try await Executable.name(name).resolveExecutablePath(in: environment) + #expect(resolved.string == link._fileSystemPath) + + let result = try await Subprocess.run( + .name(name), + environment: environment, + output: .string(limit: 16) + ) + #expect(result.standardOutput.trimmingNewLineAndQuotes() == "LINKED") + } + } + + /// A symlink that points at a *directory* is still a directory, and must be + /// skipped like any other. + @Test func testNameResolutionSkipsSymlinkToDirectory() async throws { + try await withExecutableSearchFixture { fixture in + let shadowDirectory = fixture.appendingPathComponent("shadow") + let binDirectory = fixture.appendingPathComponent("bin") + let target = fixture.appendingPathComponent("target-directory") + for directory in [shadowDirectory, binDirectory, target] { + try FileManager.default.createDirectory( + at: directory, withIntermediateDirectories: true + ) + } + let name = "test-executable-\(UUID().uuidString)" + try FileManager.default.createSymbolicLink( + at: shadowDirectory.appendingPathComponent(name), + withDestinationURL: target + ) + let executable = binDirectory.appendingPathComponent(name) + try Self.writeExecutableScript(at: executable, echoing: "REAL") + + let resolved = try await Executable.name(name).resolveExecutablePath( + in: .inherit.updating([ + "PATH": "\(shadowDirectory._fileSystemPath):\(binDirectory._fileSystemPath)" + ]) + ) + #expect(resolved.string == executable._fileSystemPath) + } + } + // MARK: Fixture helpers /// Creates a unique temporary directory, passes it to `body`, and removes diff --git a/Tests/SubprocessTests/WindowsTests.swift b/Tests/SubprocessTests/WindowsTests.swift index 02d0523e..210fa438 100644 --- a/Tests/SubprocessTests/WindowsTests.swift +++ b/Tests/SubprocessTests/WindowsTests.swift @@ -693,6 +693,45 @@ extension SubprocessWindowsTests { } } + /// Reparse points must be classified by what they point at, not rejected + /// outright: `GetFileAttributesW` reports `REPARSE_POINT` alone for a file + /// symlink but `DIRECTORY|REPARSE_POINT` for a directory symlink or + /// junction. So a symlink to an executable still resolves, while one + /// pointing at a directory is skipped like any other directory. + @Test(.requiresSymbolicLinkPrivilege) + func testNameResolutionClassifiesSymlinksByTarget() async throws { + try await Self.withExecutableSearchFixture { shadowDirectory, binDirectory, name in + let target = binDirectory.appendingPathComponent("target-\(UUID().uuidString).exe") + try Self.copyCmdExe(to: target) + let targetDirectory = shadowDirectory.appendingPathComponent("target-directory") + try FileManager.default.createDirectory( + at: targetDirectory, withIntermediateDirectories: true + ) + try FileManager.default.createSymbolicLink( + at: shadowDirectory.appendingPathComponent(name), + withDestinationURL: targetDirectory + ) + try FileManager.default.createSymbolicLink( + at: binDirectory.appendingPathComponent(name), + withDestinationURL: target + ) + + let resolved = try await Executable.name(name).resolveExecutablePath( + in: .inherit.updating([ + "PATH": "\(shadowDirectory._fileSystemPath);\(binDirectory._fileSystemPath)" + ]) + ) + // The directory symlink was skipped; the symlink to the executable + // resolved without being followed to its target. + #expect( + Self.isSamePath( + resolved.string, + binDirectory.appendingPathComponent(name)._fileSystemPath + ) + ) + } + } + /// Compares two Windows paths, which may mix `/` and `\` separators and /// differ in case. private static func isSamePath(_ lhs: String, _ rhs: String) -> Bool { @@ -1039,4 +1078,38 @@ extension SubprocessWindowsTests { } } +extension Trait where Self == ConditionTrait { + /// Creating a symbolic link on Windows requires + /// `SeCreateSymbolicLinkPrivilege`, which an unelevated process only holds + /// when Developer Mode is enabled. + static var requiresSymbolicLinkPrivilege: Self { + enabled( + "This test requires the privilege to create symbolic links (enable Developer Mode)", + { + let directory = URL.temporaryDirectory + .appendingPathComponent("symlink-probe-\(UUID().uuidString)") + guard + let _ = try? FileManager.default.createDirectory( + at: directory, + withIntermediateDirectories: true + ) + else { + return false + } + defer { try? FileManager.default.removeItem(at: directory) } + let link = directory.appendingPathComponent("link") + do { + try FileManager.default.createSymbolicLink( + at: link, + withDestinationURL: directory + ) + return true + } catch { + return false + } + } + ) + } +} + #endif // canImport(WinSDK) From 16553a508eaeef518f210637a3764266ac2e0fd2 Mon Sep 17 00:00:00 2001 From: Jake Petroules Date: Wed, 29 Jul 2026 01:16:30 -0400 Subject: [PATCH 4/4] Gate the FIFO resolution test on being able to create a FIFO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Android CI fails the test at the setup step: `mkfifo` in the temporary directory returns -1, so the fixture can never be built there. ✘ testNameResolutionSkipsNonRegularFile Expectation failed: fifo._fileSystemPath.withCString { mkfifo($0, 0o755) } == 0 → -1 Gate the test on a `requiresFIFOCreation` condition trait that probes `mkfifo` in the temporary directory, mirroring the `requiresSymbolicLinkPrivilege` trait the Windows symlink test uses. Testing the capability rather than checking for `os(Android)` also covers sandboxes elsewhere that deny FIFO creation. Also report `errno` when the `mkfifo` inside the test fails, so a future failure at that line says why. Verified that the trait skips (rather than fails) when the probe fails, and that the test target still cross-compiles for aarch64-unknown-linux-android24. --- Tests/SubprocessTests/UnixTests.swift | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/Tests/SubprocessTests/UnixTests.swift b/Tests/SubprocessTests/UnixTests.swift index 3c1d6462..ea598ab2 100644 --- a/Tests/SubprocessTests/UnixTests.swift +++ b/Tests/SubprocessTests/UnixTests.swift @@ -478,7 +478,8 @@ extension SubprocessUnixTests { /// Only a *regular* file can be executed. A FIFO with the execute bit set /// satisfies `access(_, X_OK)` and is not a directory, yet `execve` rejects /// it with `EACCES`, so it must not shadow the real executable either. - @Test func testNameResolutionSkipsNonRegularFile() async throws { + @Test(.requiresFIFOCreation) + func testNameResolutionSkipsNonRegularFile() async throws { try await withExecutableSearchFixture { fixture in let shadowDirectory = fixture.appendingPathComponent("shadow") let binDirectory = fixture.appendingPathComponent("bin") @@ -490,7 +491,8 @@ extension SubprocessUnixTests { ) let name = "test-executable-\(UUID().uuidString)" let fifo = shadowDirectory.appendingPathComponent(name) - try #require(fifo._fileSystemPath.withCString { mkfifo($0, 0o755) } == 0) + let result = fifo._fileSystemPath.withCString { mkfifo($0, 0o755) } + try #require(result == 0, "mkfifo failed: \(Errno(rawValue: errno))") // `mkfifo` honors the umask, so set the execute bits explicitly. try FileManager.default.setAttributes( [.posixPermissions: 0o755], ofItemAtPath: fifo._fileSystemPath @@ -1390,4 +1392,24 @@ extension SubprocessUnixTests { } } +extension Trait where Self == ConditionTrait { + /// Creating a FIFO is not permitted everywhere: on Android `mkfifo` fails + /// in the temporary directory, and a sandbox can deny it anywhere. + static var requiresFIFOCreation: Self { + enabled( + "This test requires creating a FIFO in the temporary directory", + { + let path = FileManager.default.temporaryDirectory + .appendingPathComponent("fifo-probe-\(UUID().uuidString)") + ._fileSystemPath + guard path.withCString({ mkfifo($0, 0o600) }) == 0 else { + return false + } + try? FileManager.default.removeItem(atPath: path) + return true + } + ) + } +} + #endif // !os(Windows)