diff --git a/codex-rs/rmcp-client/src/program_resolver.rs b/codex-rs/rmcp-client/src/program_resolver.rs index e62336e263..fe1e61d168 100644 --- a/codex-rs/rmcp-client/src/program_resolver.rs +++ b/codex-rs/rmcp-client/src/program_resolver.rs @@ -203,9 +203,13 @@ mod tests { /// Prepends the given directory to the system's PATH variable. fn build_path(dir: &Path) -> String { - let current = std::env::var("PATH").unwrap_or_default(); - let sep = if cfg!(windows) { ";" } else { ":" }; - format!("{}{sep}{current}", dir.to_string_lossy()) + let mut path = OsString::from(dir.as_os_str()); + if let Some(current) = std::env::var_os("PATH") { + let sep = if cfg!(windows) { ";" } else { ":" }; + path.push(sep); + path.push(current); + } + path.to_string_lossy().into_owned() } /// Ensures `.CMD` is in the `PATHEXT` variable on Windows for script discovery. diff --git a/codex-rs/rmcp-client/src/utils.rs b/codex-rs/rmcp-client/src/utils.rs index df09b0f8be..4da98b985f 100644 --- a/codex-rs/rmcp-client/src/utils.rs +++ b/codex-rs/rmcp-client/src/utils.rs @@ -14,7 +14,11 @@ pub(crate) fn create_env_for_mcp_server( .iter() .copied() .chain(env_vars.iter().map(String::as_str)) - .filter_map(|var| env::var(var).ok().map(|value| (var.to_string(), value))) + .filter_map(|var| match var { + "PATH" => env::var_os(var) + .map(|value| (var.to_string(), value.to_string_lossy().into_owned())), + _ => env::var(var).ok().map(|value| (var.to_string(), value)), + }) .chain(extra_env.unwrap_or_default()) .collect() } @@ -158,6 +162,18 @@ mod tests { original, } } + + #[cfg(unix)] + fn set_os(key: &str, value: &std::ffi::OsStr) -> Self { + let original = std::env::var_os(key); + unsafe { + std::env::set_var(key, value); + } + Self { + key: key.to_string(), + original, + } + } } impl Drop for EnvVarGuard { @@ -191,4 +207,19 @@ mod tests { let env = create_env_for_mcp_server(None, &[custom_var.to_string()]); assert_eq!(env.get(custom_var), Some(&value.to_string())); } + + #[cfg(unix)] + #[test] + #[serial(extra_rmcp_env)] + fn create_env_preserves_path_when_it_is_not_utf8() { + use std::os::unix::ffi::OsStrExt; + + let raw_path = std::ffi::OsStr::from_bytes(b"/tmp/codex-\xFF/bin"); + let expected = raw_path.to_string_lossy().into_owned(); + let _guard = EnvVarGuard::set_os("PATH", raw_path); + + let env = create_env_for_mcp_server(None, &[]); + + assert_eq!(env.get("PATH"), Some(&expected)); + } } diff --git a/codex-rs/windows-sandbox-rs/src/audit.rs b/codex-rs/windows-sandbox-rs/src/audit.rs index d85c5dea8e..040ba26723 100644 --- a/codex-rs/windows-sandbox-rs/src/audit.rs +++ b/codex-rs/windows-sandbox-rs/src/audit.rs @@ -11,7 +11,7 @@ use anyhow::anyhow; use anyhow::Result; use std::collections::HashSet; use std::ffi::c_void; -use std::ffi::OsStr; +use std::ffi::OsString; use std::path::Path; use std::path::PathBuf; use std::time::Duration; @@ -61,10 +61,10 @@ fn gather_candidates(cwd: &Path, env: &std::collections::HashMap // 4) PATH entries (best-effort) if let Some(path) = env .get("PATH") - .cloned() - .or_else(|| std::env::var("PATH").ok()) + .map(OsString::from) + .or_else(|| std::env::var_os("PATH")) { - for part in std::env::split_paths(OsStr::new(&path)) { + for part in std::env::split_paths(&path) { if !part.as_os_str().is_empty() { unique_push(&mut set, &mut out, part); } diff --git a/codex-rs/windows-sandbox-rs/src/env.rs b/codex-rs/windows-sandbox-rs/src/env.rs index c69a0033e9..b2d35bc0e3 100644 --- a/codex-rs/windows-sandbox-rs/src/env.rs +++ b/codex-rs/windows-sandbox-rs/src/env.rs @@ -2,6 +2,7 @@ use anyhow::{anyhow, Result}; use dirs_next::home_dir; use std::collections::HashMap; use std::env; +use std::ffi::OsString; use std::fs::{self, File}; use std::io::Write; use std::path::{Path, PathBuf}; @@ -31,8 +32,8 @@ pub fn ensure_non_interactive_pager(env_map: &mut HashMap) { // Keep PATH and PATHEXT stable for callers that rely on inheriting the parent process env. pub fn inherit_path_env(env_map: &mut HashMap) { if !env_map.contains_key("PATH") { - if let Ok(path) = env::var("PATH") { - env_map.insert("PATH".into(), path); + if let Some(path) = env::var_os("PATH") { + env_map.insert("PATH".into(), path.to_string_lossy().into_owned()); } } if !env_map.contains_key("PATHEXT") { @@ -42,27 +43,30 @@ pub fn inherit_path_env(env_map: &mut HashMap) { } } -fn prepend_path(env_map: &mut HashMap, prefix: &str) { +fn prepend_path(env_map: &mut HashMap, prefix: &Path) -> Result<()> { let existing = env_map .get("PATH") - .cloned() - .or_else(|| env::var("PATH").ok()) + .map(OsString::from) + .or_else(|| env::var_os("PATH")) .unwrap_or_default(); - let parts: Vec = existing.split(';').map(|s| s.to_string()).collect(); + let parts: Vec = env::split_paths(&existing).collect(); if parts .first() - .map(|p| p.eq_ignore_ascii_case(prefix)) + .map(|path| { + path.as_os_str() + .to_string_lossy() + .eq_ignore_ascii_case(&prefix.as_os_str().to_string_lossy()) + }) .unwrap_or(false) { - return; + return Ok(()); } - let mut new_path = String::new(); - new_path.push_str(prefix); - if !existing.is_empty() { - new_path.push(';'); - new_path.push_str(&existing); - } - env_map.insert("PATH".into(), new_path); + let new_path = env::join_paths( + std::iter::once(prefix.as_os_str()).chain(parts.iter().map(|path| path.as_os_str())), + ) + .map_err(|err| anyhow!("failed to construct PATH: {err}"))?; + env_map.insert("PATH".into(), new_path.to_string_lossy().into_owned()); + Ok(()) } fn reorder_pathext_for_stubs(env_map: &mut HashMap) { @@ -168,7 +172,7 @@ pub fn apply_no_network_to_env(env_map: &mut HashMap) -> Result< } } } - prepend_path(env_map, &base.to_string_lossy()); + prepend_path(env_map, &base)?; reorder_pathext_for_stubs(env_map); Ok(()) }