Skip to content

Commit d1f500e

Browse files
G36maidkubkon
andauthored
Fix binary name resolution against custom PATH on macOS (zed-industries#55672)
Closes zed-industries#50536 ## Summary Addresses zed-industries#50536 - On macOS, `posix_spawnp` resolves programs against the parent process's `cwd` and `environ` (via `getcwd` and `getenv("PATH")`), ignoring the child's `current_dir` and `envp`. This causes two classes of failures when Zed spawns external commands via the custom `posix_spawnp`-based `Command`: - **Bare names** (e.g. `"black"`): resolved via the parent's `PATH`, not the child's — binaries only available in a project-specific PATH (Nix/direnv) are not found. - **Relative paths** (e.g. `"./script.sh"`, `"bin/test.sh"`): resolved against the parent's `cwd`, not `current_dir` — only works when Zed's cwd happens to match the project root. - Added program resolution in `spawn_posix_spawn` (`crates/util/src/command/darwin.rs`): before calling `posix_spawnp`, resolve the program to an absolute path — bare names via `which::which_in` against the child's PATH, relative paths via `Path::join(current_dir)`. Falls back to the original program if resolution fails. ## Root Cause Apple's Libc implementation of `posix_spawnp` resolves the program path using the parent process's context — `getcwd()` for relative paths and `getenv("PATH")` for bare names — rather than the `current_dir` (set via `posix_spawn_file_actions_addchdir_np`) or `envp` argument. The child's working directory and environment only take effect **after** the binary has already been located. This is a well-documented macOS behavior that Rust's own `std::process::Command` works around by bypassing `posix_spawn` when PATH is modified (see [rust-lang/rust#48624](rust-lang/rust#48624)). The regression was introduced when PR zed-industries#49090 switched macOS from `std::process::Command` (which uses fork+execvp, correctly using the child's cwd and PATH) to a custom posix_spawnp-based implementation (which does not). ## Testing - `test_bare_program_resolved_via_custom_path` — bare name resolves via child's custom PATH - `test_bare_program_with_custom_path_falls_back_when_not_found` — non-existent binary still errors - `test_bare_program_with_custom_env_no_path_key` — custom env without PATH key falls back gracefully - `test_relative_path_skips_resolution` — relative path resolves against `current_dir` instead of parent's cwd Note: The fix and tests are in `darwin.rs` which is macOS-only. The Linux path uses `smol::process::Command` (via fork+execve) and is unaffected. Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed external formatters and language servers failing to launch on macOS when specified as a bare binary name or relative path and only available in the project's PATH (e.g. Nix, direnv) --------- Co-authored-by: Jakub Konka <kubkon@jakubkonka.com>
1 parent ef38a41 commit d1f500e

1 file changed

Lines changed: 129 additions & 3 deletions

File tree

crates/util/src/command/darwin.rs

Lines changed: 129 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use std::collections::BTreeMap;
88
use std::ffi::{CString, OsStr, OsString};
99
use std::io;
1010
use std::os::fd::AsRawFd;
11-
use std::os::unix::ffi::OsStrExt;
11+
use std::os::unix::ffi::{OsStrExt, OsStringExt};
1212
use std::os::unix::io::FromRawFd;
1313
use std::path::{Path, PathBuf};
1414
use std::process::{ExitStatus, Output};
@@ -300,12 +300,30 @@ fn spawn_posix_spawn(
300300
stderr_cfg: Stdio,
301301
kill_on_drop: bool,
302302
) -> io::Result<Child> {
303-
let program_cstr = CString::new(program.as_bytes()).map_err(|_| invalid_input_error())?;
303+
// posix_spawnp resolves programs against the parent's cwd/PATH, not the child's.
304+
let resolved_program = if program.as_bytes().contains(&b'/') {
305+
std::path::absolute(current_dir.join(program)).map_or_else(
306+
|_| program.as_bytes().to_vec(),
307+
|p| p.into_os_string().into_vec(),
308+
)
309+
} else {
310+
envs.and_then(|e| {
311+
e.iter()
312+
.find(|(k, _)| k.as_os_str() == OsStr::new("PATH"))
313+
.and_then(|(_, v)| which::which_in(program, Some(v.as_os_str()), current_dir).ok())
314+
})
315+
.map_or_else(
316+
|| program.as_bytes().to_vec(),
317+
|path| path.into_os_string().into_vec(),
318+
)
319+
};
320+
let program_cstr = CString::new(resolved_program).map_err(|_| invalid_input_error())?;
321+
let argv0_cstr = CString::new(program.as_bytes()).map_err(|_| invalid_input_error())?;
304322

305323
let current_dir_cstr =
306324
CString::new(current_dir.as_os_str().as_bytes()).map_err(|_| invalid_input_error())?;
307325

308-
let mut argv_cstrs = vec![program_cstr.clone()];
326+
let mut argv_cstrs = vec![argv0_cstr];
309327
for arg in args {
310328
let cstr = CString::new(arg.as_bytes()).map_err(|_| invalid_input_error())?;
311329
argv_cstrs.push(cstr);
@@ -909,6 +927,114 @@ mod tests {
909927
});
910928
}
911929

930+
#[test]
931+
fn test_bare_program_resolved_via_custom_path() {
932+
let temp_dir = tempfile::tempdir().expect("failed to create temp dir");
933+
934+
let link_path = temp_dir.path().join("zed-test-echo");
935+
std::os::unix::fs::symlink("/bin/echo", &link_path).expect("failed to create symlink");
936+
937+
let custom_path = temp_dir.path().to_string_lossy().into_owned();
938+
939+
smol::block_on(async {
940+
let output = Command::new("zed-test-echo")
941+
.args(["-n", "from-custom-path"])
942+
.env("PATH", &custom_path)
943+
.output()
944+
.await
945+
.expect("failed to spawn with custom PATH");
946+
947+
assert!(output.status.success());
948+
assert_eq!(output.stdout, b"from-custom-path");
949+
});
950+
}
951+
952+
#[test]
953+
fn test_bare_program_preserves_argv0_when_resolved_via_custom_path() {
954+
let temp_dir = tempfile::tempdir().expect("failed to create temp dir");
955+
956+
let link_path = temp_dir.path().join("zed-test-sh");
957+
std::os::unix::fs::symlink("/bin/sh", &link_path).expect("failed to create symlink");
958+
959+
let custom_path = temp_dir.path().to_string_lossy().into_owned();
960+
961+
smol::block_on(async {
962+
let output = Command::new("zed-test-sh")
963+
.args(["-c", "printf %s \"$0\""])
964+
.env("PATH", &custom_path)
965+
.output()
966+
.await
967+
.expect("failed to spawn with custom PATH");
968+
969+
assert!(output.status.success());
970+
assert_eq!(output.stdout, b"zed-test-sh");
971+
});
972+
}
973+
974+
#[test]
975+
fn test_bare_program_with_custom_path_falls_back_when_not_found() {
976+
smol::block_on(async {
977+
let result = Command::new("zed-nonexistent-binary-xyz")
978+
.env("PATH", "/nonexistent/path")
979+
.spawn();
980+
981+
assert!(result.is_err());
982+
});
983+
}
984+
985+
#[test]
986+
fn test_bare_program_with_custom_env_no_path_key() {
987+
smol::block_on(async {
988+
let output = Command::new("echo")
989+
.args(["-n", "from-inherited-path"])
990+
.env("ZED_TEST_VAR", "test")
991+
.output()
992+
.await
993+
.expect("failed to spawn with custom env but no PATH override");
994+
995+
assert!(output.status.success());
996+
assert_eq!(output.stdout, b"from-inherited-path");
997+
});
998+
}
999+
1000+
#[test]
1001+
fn test_relative_path_resolved_against_current_dir() {
1002+
let temp_dir = tempfile::tempdir().expect("failed to create temp dir");
1003+
1004+
let link_path = temp_dir.path().join("zed-test-echo");
1005+
std::os::unix::fs::symlink("/bin/echo", &link_path).expect("failed to create symlink");
1006+
1007+
let relative_path = "./zed-test-echo";
1008+
1009+
smol::block_on(async {
1010+
let output = Command::new(relative_path)
1011+
.args(["-n", "from-relative-path"])
1012+
.current_dir(temp_dir.path())
1013+
.env("PATH", "/nonexistent/path")
1014+
.output()
1015+
.await
1016+
.expect("failed to spawn with relative path");
1017+
1018+
assert!(output.status.success());
1019+
assert_eq!(output.stdout, b"from-relative-path");
1020+
});
1021+
}
1022+
1023+
#[test]
1024+
fn test_absolute_path_passes_through_unchanged() {
1025+
smol::block_on(async {
1026+
let output = Command::new("/bin/echo")
1027+
.args(["-n", "from-absolute-path"])
1028+
.env("PATH", "/nonexistent/path")
1029+
.output()
1030+
.await
1031+
.expect("failed to spawn with absolute path");
1032+
1033+
assert!(output.status.success());
1034+
assert_eq!(output.stdout, b"from-absolute-path");
1035+
});
1036+
}
1037+
9121038
#[test]
9131039
fn test_stdio_inherit_keeps_stdio_open() {
9141040
smol::block_on(async {

0 commit comments

Comments
 (0)