From ee32374e511eb7660f550ddf6441716dd9aef70b Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Wed, 4 Jun 2025 18:22:48 +0000 Subject: [PATCH] Add `Command::resolve_in_parent_path` --- library/std/src/process.rs | 16 +++++ library/std/src/process/tests.rs | 43 ++++++++++- library/std/src/sys/process/uefi.rs | 4 ++ library/std/src/sys/process/unix/common.rs | 9 +++ library/std/src/sys/process/unix/unix.rs | 84 +++++++++++++++++++--- library/std/src/sys/process/unsupported.rs | 6 ++ library/std/src/sys/process/windows.rs | 11 ++- 7 files changed, 160 insertions(+), 13 deletions(-) diff --git a/library/std/src/process.rs b/library/std/src/process.rs index 373584d0117ce..61a3cf96b725d 100644 --- a/library/std/src/process.rs +++ b/library/std/src/process.rs @@ -1195,6 +1195,22 @@ impl Command { pub fn get_current_dir(&self) -> Option<&Path> { self.inner.get_current_dir() } + + /// Resolve the program given in [`new`](Self::new) using the parent's PATH. + /// + /// Usually when a `env` is used to set `PATH` on a child process, + /// that new `PATH` will be used to discover the process. + /// With `resolve_in_parent_path` to `true`, the parent's `PATH` will be used instead. + /// + /// # Platform-specific behavior + /// + /// On Unix the process is executed after fork and after setting up the rest of the new environment. + /// So if `chroot`, `uid`, `gid` or `groups` are used then the way `PATH` is interpreted may be changed. + #[unstable(feature = "command_resolve", issue = "none")] + pub fn resolve_in_parent_path(&mut self, use_parent: bool) -> &mut Self { + self.inner.resolve_in_parent_path(use_parent); + self + } } #[stable(feature = "rust1", since = "1.0.0")] diff --git a/library/std/src/process/tests.rs b/library/std/src/process/tests.rs index 5879914ca206a..626c676152c49 100644 --- a/library/std/src/process/tests.rs +++ b/library/std/src/process/tests.rs @@ -2,10 +2,10 @@ use super::{Command, Output, Stdio}; use crate::io::prelude::*; use crate::io::{BorrowedBuf, ErrorKind}; use crate::mem::MaybeUninit; -use crate::str; +use crate::{fs, str}; fn known_command() -> Command { - if cfg!(windows) { Command::new("help") } else { Command::new("echo") } + if cfg!(windows) { Command::new("help.exe") } else { Command::new("echo") } } #[cfg(target_os = "android")] @@ -603,3 +603,42 @@ fn terminate_exited_process() { assert!(p.kill().is_ok()); assert!(p.kill().is_ok()); } + +#[test] +fn resolve_in_parent_path() { + let tempdir = crate::test_helpers::tmpdir(); + let mut cmd = if cfg!(windows) { + // On Windows std prepends the system paths when searching for executables + // but not if PATH is set on the command. Therefore adding a broken exe + // using PATH will cause spawn to fail if it's invoked. + let mut cmd = known_command(); + fs::write(tempdir.join(cmd.get_program().to_str().unwrap()), "").unwrap(); + cmd.env("PATH", tempdir.path()); + cmd + } else { + let mut cmd = known_command(); + cmd.env("PATH", "/a/b/c"); + cmd + }; + + assert!(cmd.spawn().is_err()); + cmd.resolve_in_parent_path(true); + cmd.spawn().unwrap(); + cmd.resolve_in_parent_path(false); + assert!(cmd.spawn().is_err()); + + // If the platform is a unix and has posix_spawn then that will be used in the test above. + // So we also test the fork/exec path by setting a pre_exec function. + #[cfg(unix)] + { + use crate::os::unix::process::CommandExt; + unsafe { + cmd.pre_exec(|| Ok(())); + } + assert!(cmd.spawn().is_err()); + cmd.resolve_in_parent_path(true); + assert!(cmd.status().unwrap().success()); + cmd.resolve_in_parent_path(false); + assert!(cmd.spawn().is_err()); + } +} diff --git a/library/std/src/sys/process/uefi.rs b/library/std/src/sys/process/uefi.rs index 4864c58698817..2f89b778b45dd 100644 --- a/library/std/src/sys/process/uefi.rs +++ b/library/std/src/sys/process/uefi.rs @@ -78,6 +78,10 @@ impl Command { self.stderr = Some(stderr); } + pub fn resolve_in_parent_path(&mut self, _use_parent: bool) -> &mut Self { + self + } + pub fn get_program(&self) -> &OsStr { self.prog.as_ref() } diff --git a/library/std/src/sys/process/unix/common.rs b/library/std/src/sys/process/unix/common.rs index b6777b76668d5..3ad998d7e7752 100644 --- a/library/std/src/sys/process/unix/common.rs +++ b/library/std/src/sys/process/unix/common.rs @@ -98,6 +98,7 @@ pub struct Command { #[cfg(target_os = "linux")] create_pidfd: bool, pgroup: Option, + resolve_in_parent_path: bool, } // passed back to std::process with the pipes connected to the child, if any @@ -185,6 +186,7 @@ impl Command { #[cfg(target_os = "linux")] create_pidfd: false, pgroup: None, + resolve_in_parent_path: false, } } @@ -220,6 +222,9 @@ impl Command { self.cwd(&OsStr::new("/")); } } + pub fn resolve_in_parent_path(&mut self, use_parent: bool) { + self.resolve_in_parent_path = use_parent; + } #[cfg(target_os = "linux")] pub fn create_pidfd(&mut self, val: bool) { @@ -298,6 +303,10 @@ impl Command { pub fn get_chroot(&self) -> Option<&CStr> { self.chroot.as_deref() } + #[allow(dead_code)] + pub fn get_resolve_in_parent_path(&self) -> bool { + self.resolve_in_parent_path + } pub fn get_closures(&mut self) -> &mut Vec io::Result<()> + Send + Sync>> { &mut self.closures diff --git a/library/std/src/sys/process/unix/unix.rs b/library/std/src/sys/process/unix/unix.rs index 4f595ac9a1c5f..687d6388d427c 100644 --- a/library/std/src/sys/process/unix/unix.rs +++ b/library/std/src/sys/process/unix/unix.rs @@ -378,27 +378,89 @@ impl Command { callback()?; } + let mut _reset = None; // Although we're performing an exec here we may also return with an // error from this function (without actually exec'ing) in which case we // want to be sure to restore the global environment back to what it // once was, ensuring that our temporary override, when free'd, doesn't // corrupt our process's environment. - let mut _reset = None; - if let Some(envp) = maybe_envp { - struct Reset(*const *const libc::c_char); + struct Reset(*const *const libc::c_char); - impl Drop for Reset { - fn drop(&mut self) { - unsafe { - *sys::env::environ() = self.0; - } + impl Drop for Reset { + fn drop(&mut self) { + unsafe { + *sys::env::environ() = self.0; } } + } + if let Some(envp) = maybe_envp + && self.get_resolve_in_parent_path() + && self.env_saw_path() + && self.get_program_kind() == ProgramKind::PathLookup + { + use crate::ffi::CStr; + // execvpe is a gnu extension... + #[cfg(all(target_os = "linux", target_env = "gnu"))] + unsafe fn exec_with_env( + program: &CStr, + args: &CStringArray, + envp: &CStringArray, + ) -> io::Error { + unsafe { libc::execvpe(program.as_ptr(), args.as_ptr(), envp.as_ptr()) }; + io::Error::last_os_error() + } + + // ...so if we're not gnu then use our own implementation. + #[cfg(not(all(target_os = "linux", target_env = "gnu")))] + unsafe fn exec_with_env( + program: &CStr, + args: &CStringArray, + envp: &CStringArray, + ) -> io::Error { + unsafe { + let name = program.to_bytes(); + let mut buffer = + [const { mem::MaybeUninit::::uninit() }; libc::PATH_MAX as usize]; + let mut environ = *sys::env::environ(); + // Search the environment for PATH and, if found, + // search the paths for the executable by trying to execve each candidate. + while !(*environ).is_null() { + let kv = CStr::from_ptr(*environ); + if let Some(value) = kv.to_bytes().strip_prefix(b"PATH=") { + for path in value.split(|&b| b == b':') { + if buffer.len() - 2 >= path.len().saturating_add(name.len()) { + let buf_ptr = buffer.as_mut_ptr().cast::(); + let mut offset = 0; + if !path.is_empty() { + buf_ptr.copy_from(path.as_ptr(), path.len()); + offset += path.len(); + if path.last() != Some(&b'/') { + *buf_ptr.add(path.len()) = b'/'; + offset += 1; + } + } + buf_ptr.add(offset).copy_from(name.as_ptr(), name.len()); + offset += name.len(); + *buf_ptr.add(offset) = 0; + libc::execve(buf_ptr.cast(), args.as_ptr(), envp.as_ptr()); + } + } + break; + } + environ = environ.add(1); + } + // If execve is successful then it'll never return, + // thus we only reach this point on failure.. + io::Error::from_raw_os_error(libc::ENOENT) + } + } + _reset = Some(Reset(*sys::env::environ())); + return Err(exec_with_env(self.get_program_cstr(), self.get_argv(), envp)); + } else if let Some(envp) = maybe_envp { _reset = Some(Reset(*sys::env::environ())); *sys::env::environ() = envp.as_ptr(); } - libc::execvp(self.get_program_cstr().as_ptr(), self.get_argv().as_ptr()); Err(io::Error::last_os_error()) } @@ -453,7 +515,9 @@ impl Command { if self.get_gid().is_some() || self.get_uid().is_some() - || (self.env_saw_path() && !self.program_is_path()) + || (!self.get_resolve_in_parent_path() + && self.env_saw_path() + && !self.program_is_path()) || !self.get_closures().is_empty() || self.get_groups().is_some() || self.get_chroot().is_some() diff --git a/library/std/src/sys/process/unsupported.rs b/library/std/src/sys/process/unsupported.rs index 469922c78aca2..9bbb485483a53 100644 --- a/library/std/src/sys/process/unsupported.rs +++ b/library/std/src/sys/process/unsupported.rs @@ -21,6 +21,7 @@ pub struct Command { stdin: Option, stdout: Option, stderr: Option, + force_parent_path: bool, } // passed back to std::process with the pipes connected to the child, if any @@ -52,6 +53,7 @@ impl Command { stdin: None, stdout: None, stderr: None, + force_parent_path: false, } } @@ -79,6 +81,10 @@ impl Command { self.stderr = Some(stderr); } + pub fn resolve_in_parent_path(&mut self, use_parent: bool) { + self.force_parent_path = use_parent; + } + pub fn get_program(&self) -> &OsStr { &self.program } diff --git a/library/std/src/sys/process/windows.rs b/library/std/src/sys/process/windows.rs index 1ee3fbd285f52..107f74a3e8c89 100644 --- a/library/std/src/sys/process/windows.rs +++ b/library/std/src/sys/process/windows.rs @@ -158,6 +158,7 @@ pub struct Command { startupinfo_fullscreen: bool, startupinfo_untrusted_source: bool, startupinfo_force_feedback: Option, + force_parent_path: bool, } pub enum Stdio { @@ -192,6 +193,7 @@ impl Command { startupinfo_fullscreen: false, startupinfo_untrusted_source: false, startupinfo_force_feedback: None, + force_parent_path: false, } } @@ -224,6 +226,10 @@ impl Command { self.force_quotes_enabled = enabled; } + pub fn resolve_in_parent_path(&mut self, use_parent: bool) { + self.force_parent_path = use_parent; + } + pub fn raw_arg(&mut self, command_str_to_append: &OsStr) { self.args.push(Arg::Raw(command_str_to_append.to_os_string())) } @@ -274,7 +280,10 @@ impl Command { let env_saw_path = self.env.have_changed_path(); let maybe_env = self.env.capture_if_changed(); - let child_paths = if env_saw_path && let Some(env) = maybe_env.as_ref() { + let child_paths = if env_saw_path + && !self.force_parent_path + && let Some(env) = maybe_env.as_ref() + { env.get(&EnvKey::new("PATH")).map(|s| s.as_os_str()) } else { None