From 3bed830a1f9ac7ef1569bfca27fd61e835fe3094 Mon Sep 17 00:00:00 2001 From: Haru Kim Date: Tue, 10 Jun 2025 11:37:43 +0900 Subject: [PATCH] Use unix pipe to capture environment variables (#32136) The use of `NamedTempFile` in #31799 was not secure and could potentially cause write permission issues (see [this comment](https://github.com/zed-industries/zed/issues/29528#issuecomment-2939672433)). Therefore, it has been replaced with a Unix pipe. Release Notes: - N/A --- Cargo.lock | 23 ++++++++ crates/project/src/environment.rs | 2 +- crates/util/Cargo.toml | 1 + crates/util/src/shell_env.rs | 88 +++++++++++++++++++++---------- crates/util/src/util.rs | 2 +- 5 files changed, 87 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 99b432985d..bf9c02e972 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3161,6 +3161,16 @@ dependencies = [ "memchr", ] +[[package]] +name = "command-fds" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2ec1052629a80c28594777d1252efc8a6b005d13f9edfd8c3fc0f44d5b32489a" +dependencies = [ + "nix 0.30.1", + "thiserror 2.0.12", +] + [[package]] name = "command_palette" version = "0.1.0" @@ -10132,6 +10142,18 @@ dependencies = [ "memoffset", ] +[[package]] +name = "nix" +version = "0.30.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "74523f3a35e05aba87a1d978330aef40f67b0304ac79c1c00b294c9830543db6" +dependencies = [ + "bitflags 2.9.0", + "cfg-if", + "cfg_aliases 0.2.1", + "libc", +] + [[package]] name = "node_runtime" version = "0.1.0" @@ -17124,6 +17146,7 @@ dependencies = [ "async-fs", "async_zip", "collections", + "command-fds", "dirs 4.0.0", "dunce", "futures 0.3.31", diff --git a/crates/project/src/environment.rs b/crates/project/src/environment.rs index 14e3ecb4c0..6ecfae9163 100644 --- a/crates/project/src/environment.rs +++ b/crates/project/src/environment.rs @@ -249,7 +249,7 @@ async fn load_shell_environment( use util::shell_env; let dir_ = dir.to_owned(); - let mut envs = match smol::unblock(move || shell_env::capture(Some(dir_))).await { + let mut envs = match smol::unblock(move || shell_env::capture(&dir_)).await { Ok(envs) => envs, Err(err) => { util::log_err(&err); diff --git a/crates/util/Cargo.toml b/crates/util/Cargo.toml index 328442cddb..ff79126ade 100644 --- a/crates/util/Cargo.toml +++ b/crates/util/Cargo.toml @@ -42,6 +42,7 @@ walkdir.workspace = true workspace-hack.workspace = true [target.'cfg(unix)'.dependencies] +command-fds = "0.3.1" libc.workspace = true [target.'cfg(windows)'.dependencies] diff --git a/crates/util/src/shell_env.rs b/crates/util/src/shell_env.rs index 9b42d5c0b1..68d32c6376 100644 --- a/crates/util/src/shell_env.rs +++ b/crates/util/src/shell_env.rs @@ -1,16 +1,21 @@ +#![cfg_attr(not(unix), allow(unused))] + use anyhow::{Context as _, Result}; -use collections::HashMap; use std::borrow::Cow; -use std::ffi::OsStr; -use std::io::Read; -use std::path::{Path, PathBuf}; -use std::process::Command; -use tempfile::NamedTempFile; /// Capture all environment variables from the login shell. -pub fn capture(change_dir: Option>) -> Result> { - let shell_path = std::env::var("SHELL").map(PathBuf::from)?; - let shell_name = shell_path.file_name().and_then(OsStr::to_str); +#[cfg(unix)] +pub fn capture(directory: &std::path::Path) -> Result> { + use std::os::unix::process::CommandExt; + use std::process::Stdio; + + let shell_path = std::env::var("SHELL").map(std::path::PathBuf::from)?; + let shell_name = shell_path.file_name().and_then(std::ffi::OsStr::to_str); + + let mut command = std::process::Command::new(&shell_path); + command.stdin(Stdio::null()); + command.stdout(Stdio::piped()); + command.stderr(Stdio::piped()); let mut command_string = String::new(); @@ -18,10 +23,7 @@ pub fn capture(change_dir: Option>) -> Result>) -> Result "", }); - let mut env_output_file = NamedTempFile::new()?; - command_string.push_str(&format!( - "sh -c 'export -p' > '{}';", - env_output_file.path().to_string_lossy(), - )); - - let mut command = Command::new(&shell_path); + // In some shells, file descriptors greater than 2 cannot be used in interactive mode, + // so file descriptor 0 is used instead. + const ENV_OUTPUT_FD: std::os::fd::RawFd = 0; + command_string.push_str(&format!("sh -c 'export -p >&{ENV_OUTPUT_FD}';")); // For csh/tcsh, the login shell option is set by passing `-` as // the 0th argument instead of using `-l`. if let Some("tcsh" | "csh") = shell_name { - #[cfg(unix)] - std::os::unix::process::CommandExt::arg0(&mut command, "-"); + command.arg0("-"); } else { command.arg("-l"); } command.args(["-i", "-c", &command_string]); - let process_output = super::set_pre_exec_to_start_new_session(&mut command).output()?; + super::set_pre_exec_to_start_new_session(&mut command); + + let (env_output, process_output) = spawn_and_read_fd(command, ENV_OUTPUT_FD)?; + let env_output = String::from_utf8_lossy(&env_output); + anyhow::ensure!( process_output.status.success(), "login shell exited with {}. stdout: {:?}, stderr: {:?}", @@ -58,15 +60,36 @@ pub fn capture(change_dir: Option>) -> Result Some(Ok((name.into(), value?.into()))), Err(err) => Some(Err(err)), }) - .collect::>>() + .collect::>() +} + +#[cfg(unix)] +fn spawn_and_read_fd( + mut command: std::process::Command, + child_fd: std::os::fd::RawFd, +) -> anyhow::Result<(Vec, std::process::Output)> { + use command_fds::{CommandFdExt, FdMapping}; + use std::io::Read; + + let (mut reader, writer) = std::io::pipe()?; + + command.fd_mappings(vec![FdMapping { + parent_fd: writer.into(), + child_fd, + }])?; + + let process = command.spawn()?; + drop(command); + + let mut buffer = Vec::new(); + reader.read_to_end(&mut buffer)?; + + Ok((buffer, process.wait_with_output()?)) } /// Parse the result of calling `sh -c 'export -p'`. @@ -154,6 +177,17 @@ fn parse_literal_double_quoted(input: &str) -> Option<(String, &str)> { mod tests { use super::*; + #[cfg(unix)] + #[test] + fn test_spawn_and_read_fd() -> anyhow::Result<()> { + let mut command = std::process::Command::new("sh"); + super::super::set_pre_exec_to_start_new_session(&mut command); + command.args(["-lic", "printf 'abc%.0s' $(seq 1 65536) >&0"]); + let (bytes, _) = spawn_and_read_fd(command, 0)?; + assert_eq!(bytes.len(), 65536 * 3); + Ok(()) + } + #[test] fn test_parse() { let input = indoc::indoc! {r#" diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs index db95f70347..b450e4900f 100644 --- a/crates/util/src/util.rs +++ b/crates/util/src/util.rs @@ -315,7 +315,7 @@ pub fn load_login_shell_environment() -> Result<()> { // into shell's `cd` command (and hooks) to manipulate env. // We do this so that we get the env a user would have when spawning a shell // in home directory. - for (name, value) in shell_env::capture(Some(paths::home_dir()))? { + for (name, value) in shell_env::capture(paths::home_dir())? { unsafe { env::set_var(&name, &value) }; }