From 23cd70c3ee429e942a39a29e3215453de9ac487b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= Date: Sun, 9 Nov 2025 17:04:14 +0100 Subject: [PATCH] Keep order of paths fixed --- .../Platforms/Subprocess+Unix.swift | 48 ++++++--------- Tests/SubprocessTests/IntegrationTests.swift | 60 +++++++++++++++++++ Tests/SubprocessTests/UnixTests.swift | 52 ++++++++++++++++ 3 files changed, 129 insertions(+), 31 deletions(-) diff --git a/Sources/Subprocess/Platforms/Subprocess+Unix.swift b/Sources/Subprocess/Platforms/Subprocess+Unix.swift index e66cd074..07c07247 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Unix.swift +++ b/Sources/Subprocess/Platforms/Subprocess+Unix.swift @@ -271,15 +271,13 @@ extension Arguments { // MARK: - Executable Searching extension Executable { - internal static var defaultSearchPaths: Set { - return Set([ - "/usr/bin", - "/bin", - "/usr/sbin", - "/sbin", - "/usr/local/bin", - ]) - } + internal static let defaultSearchPaths = [ + "/usr/bin", + "/bin", + "/usr/sbin", + "/sbin", + "/usr/local/bin", + ] internal func resolveExecutablePath(withPathValue pathValue: String?) throws -> String { switch self.storage { @@ -288,21 +286,10 @@ extension Executable { if Configuration.pathAccessible(executableName, mode: X_OK) { return executableName } - // Get $PATH from environment - let searchPaths: Set - if let pathValue = pathValue { - let localSearchPaths = pathValue.split(separator: ":").map { String($0) } - searchPaths = Set(localSearchPaths).union(Self.defaultSearchPaths) - } else { - searchPaths = Self.defaultSearchPaths - } - - for path in searchPaths { - let fullPath = "\(path)/\(executableName)" - let fileExists = Configuration.pathAccessible(fullPath, mode: X_OK) - if fileExists { - return fullPath - } + let firstAccessibleExecutable = possibleExecutablePaths(withPathValue: pathValue) + .first { Configuration.pathAccessible($0, mode: X_OK) } + if let firstAccessibleExecutable { + return firstAccessibleExecutable } throw SubprocessError( code: .init(.executableNotFound(executableName)), @@ -323,13 +310,12 @@ extension Executable { // executableName could be a full path results.insert(executableName) // Get $PATH from environment - let searchPaths: Set - if let pathValue = pathValue { - let localSearchPaths = pathValue.split(separator: ":").map { String($0) } - searchPaths = Set(localSearchPaths).union(Self.defaultSearchPaths) - } else { - searchPaths = Self.defaultSearchPaths - } + let searchPaths = + if let pathValue = pathValue { + pathValue.split(separator: ":").map { String($0) } + Self.defaultSearchPaths + } else { + Self.defaultSearchPaths + } for path in searchPaths { results.insert( FilePath(path).appending(executableName).string diff --git a/Tests/SubprocessTests/IntegrationTests.swift b/Tests/SubprocessTests/IntegrationTests.swift index 104ef2b1..4b63429a 100644 --- a/Tests/SubprocessTests/IntegrationTests.swift +++ b/Tests/SubprocessTests/IntegrationTests.swift @@ -136,6 +136,66 @@ extension SubprocessIntegrationTests { _ = try await Subprocess.run(.path(fakePath), output: .discarded) } } + + #if !os(Windows) + /// Integration test verifying that subprocess execution respects PATH ordering. + @Test func testPathOrderingIsRespected() async throws { + // Create temporary directories to simulate a PATH with multiple executables. + let tempDir = FileManager.default.temporaryDirectory + let firstDir = tempDir.appendingPathComponent("path-test-first-\(UUID().uuidString)") + let secondDir = tempDir.appendingPathComponent("path-test-second-\(UUID().uuidString)") + + try FileManager.default.createDirectory(at: firstDir, withIntermediateDirectories: true) + try FileManager.default.createDirectory(at: secondDir, withIntermediateDirectories: true) + + defer { + try? FileManager.default.removeItem(at: firstDir) + try? FileManager.default.removeItem(at: secondDir) + } + + // Create two different "test-executable" scripts that output different values. + let executableName = "test-executable-\(UUID().uuidString)" + + // First + let firstExecutable = firstDir.appendingPathComponent(executableName) + try """ + #!/bin/sh + echo "FIRST" + """.write(to: firstExecutable, atomically: true, encoding: .utf8) + try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: firstExecutable.path()) + + // Second + let secondExecutable = secondDir.appendingPathComponent(executableName) + try """ + #!/bin/sh + echo "SECOND" + """.write(to: secondExecutable, atomically: true, encoding: .utf8) + try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: secondExecutable.path()) + + // Run repeatedly to increase chance of catching any ordering issues. + for _ in 0..<10 { + let first = try await Subprocess.run( + .name(executableName), + environment: .inherit.updating([ + "PATH": "\(firstDir.path()):\(secondDir.path())" + ]), + output: .string(limit: 8) + ) + #expect(first.terminationStatus.isSuccess) + #expect(first.standardOutput?.trimmingNewLineAndQuotes() == "FIRST") + + let second = try await Subprocess.run( + .name(executableName), + environment: .inherit.updating([ + "PATH": "\(secondDir.path()):\(firstDir.path())" + ]), + output: .string(limit: 8) + ) + #expect(second.terminationStatus.isSuccess) + #expect(second.standardOutput?.trimmingNewLineAndQuotes() == "SECOND") + } + } + #endif } // MARK: - Argument Tests diff --git a/Tests/SubprocessTests/UnixTests.swift b/Tests/SubprocessTests/UnixTests.swift index 490cc373..5db8c5d4 100644 --- a/Tests/SubprocessTests/UnixTests.swift +++ b/Tests/SubprocessTests/UnixTests.swift @@ -201,6 +201,58 @@ extension SubprocessUnixTests { } } +// MARK: - PATH Resolution Tests +extension SubprocessUnixTests { + @Test func testExecutablePathsPreserveOrder() throws { + let executable = Executable.name("test-bin") + let pathValue = "/first/path:/second/path:/third/path" + + let paths = 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) + + #expect(Array(duplicatePaths).count == Set(duplicatePaths).count) + } + + @Test func testPossibleExecutablePathsWithNilPATH() throws { + let executable = Executable.name("test-bin") + let paths = executable.possibleExecutablePaths(withPathValue: nil) + let pathsArray = Array(paths) + + #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", + ]) + } +} + // MARK: - Misc extension SubprocessUnixTests { @Test func testExitSignal() async throws {