std::process::Command sometimes ignores PATH env variable on Windows about rust HOT 4 OPEN

Okay I kinda fell down a rabbithole with this, apologies in advance.

I think Windows should probably follow the Unix-ish variant of using the child's PATH (if set by env, since Windows lacks a fork-like mechanism that can modify PATH before exec), else the parent's one.

However:

1. Unix has execvp, which searches PATH, and if PATH is not set, the behaviour is implementation defined.
   On Linux (and other glibc systems) this means calling confstr(_SC_PATH) which returns some path which MAY (but as of glibc 2.24, does not) include the current directory (.)
   It will also try to run the file with sh if executing it as a binary file fails. This is also defined by POSIX.
   Rust doesn't use this, however it roughly approximates what Windows does. (In different order, however.)
2. Further complicating this, the variants of exec that don't search path (e.g. execve) CAN execute things in the current directory (of the child!). (The path argument works as-if passed to open)

I think PATH-only is a reasonable default that is relatively easy to get consistent across platforms, especially since Rust provides ways to query both the executable directory, and the working directory, so the behaviour can be implemented by the user, should they need it (Although adding them to PATH manually is maybe a little clunky.)

In a perfect world, what I would want is:

1. Only search PATH (child's if applicable, else parent's), consistently on all platforms
2. A method on std::os::windows::process::CommandExt to opt into the regular, pass-it-straight-to-CreateProcess behaviour, i.e. fn use_system_search_path(&mut self, use: bool) -> &mut Command. Adding this to Command to have the same apply to execve on Unix doesn't seem worth it, as it restricts relative paths to only the current directory, and std wants as little platform-specificity in its APIs as possible outside of OS-specific extension traits.

Needless to say, all of this is a pretty big mess. I'm advocating for following the principle of least surprise here as much as possible. Notably, it makes "check the value of PATH" be the correct first troubleshooting step on all platforms.

from rust.

Ooph yeah, this is semi-intentional but for historic reasons. So please bare with me while I give a history lesson 🙂

The search order in CreateProcessW (the underlying API) searches the current directory and system directories before PATH.

At some point some code was added to try to mimic Linux's search order but it was super buggy, Maybe it worked originally but broke over time due to later refactoring that went untested. This lead to (amongst other things) the behaviour you've seen.

When I came along, it was to fix a security issue. We didn't want to ever search the current directory so I rewrote the search manually. I opted to (mostly) preserve the existing behaviour except for removing the current directory because I wanted to avoid too many technically breaking changes at once. I did admittedly fix some bugs though.

The issue now is deciding which behaviour we want to use consistently. This may involve breaking someone's workflow. Our options are:

• Just search the PATH environment variable. No more, no less. This is my preference.
• Try to mimic the Linux behaviour again. The only difference with the first bullet point is that Linux APIs search the child's PATH whereas Windows APIs, by default, searches the current process' PATH. Obviously this only matters when the two PATHs differ.
• We could try to do both (which is closest to the current behaviour). That is search the child's PATH first then fallback to the current process if not found.

Additionally there's the question of what to do about the system paths that CreateProcessW API searches and I preserved in the rewrite. We could just remove them (again this is my preference) but I'm not sure if that would break anyone's expectations.

In any case this is ultimately an API question so tagging appropriately.

from rust.

In case the above wasn't entirely clear I'll try to explictly list the current state and future options.

Current state:

• child's PATH environment variable (but only if the child's environment is not inherited)
• application's path
• system directories
• PATH environment variable

Option 1:

• PATH only

Option 2:

• application's path
• PATH

Option 3:

• application's path
• system directories
• PATH

Option 4:

• child's PATH

Option 5:

• child's PATH
• PATH

Option 6:

• application's path
• child's PATH
• PATH

Option 7:

• application's path
• system directories
• child's PATH
• PATH

from rust.

Additionally there's the question of what to do about the system paths that CreateProcessW API searches and I preserved in the rewrite. We could just remove them (again this is my preference) but I'm not sure if that would break anyone's expectations.

By default, the system path %SystemRoot%\System32 is usually the first entry in the PATH env variable. So, we could theoretically just search PATH, as you have suggested. But, since the PATH can be modified, it might be safer to also search the system path afterwards as a fallback.

Searching PATH before the system directory seems to me like a better approach because it gives users better control over the search order (see my use case with C:\Program Files\Git\bin\bash.exe vs C:\Windows\System32\bash.exe). I'm not sure if this can have some security implications, though.

Regarding the options, it would be convenient (for me, as Rust developer) if the std::process::Command behavior was consistent across platforms (unless there is some good reason for it not to be).

This seems like a situation where we will definitely break someone's workflow, no matter what approach we choose.

from rust.