Resolve Executable.name(_:) against PATH only on every platform

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 {

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()!

Sources/Subprocess/Platforms/Subprocess+Unix.swift

@@ -155,20 +155,26 @@ 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]
         case .custom(let fullEnvironment):
             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,22 +365,57 @@ 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 `<paths.h>` 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",
         "/sbin",
         "/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<String> {
+    ) throws(SubprocessError) -> _OrderedSet<String> {
         switch self.storage {
         case .executable(let executableName):
+            try Self.validate(name: executableName)
             var results: _OrderedSet<String> = .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,18 +592,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()!