fix: make more PATH reads OsString-aware

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.

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));
+    }
 }

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<String, String>
     // 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);
             }

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<String, String>) {
 // Keep PATH and PATHEXT stable for callers that rely on inheriting the parent process env.
 pub fn inherit_path_env(env_map: &mut HashMap<String, String>) {
     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<String, String>) {
     }
 }
 
-fn prepend_path(env_map: &mut HashMap<String, String>, prefix: &str) {
+fn prepend_path(env_map: &mut HashMap<String, String>, 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<String> = existing.split(';').map(|s| s.to_string()).collect();
+    let parts: Vec<PathBuf> = 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<String, String>) {
@@ -168,7 +172,7 @@ pub fn apply_no_network_to_env(env_map: &mut HashMap<String, String>) -> Result<
             }
         }
     }
-    prepend_path(env_map, &base.to_string_lossy());
+    prepend_path(env_map, &base)?;
     reorder_pathext_for_stubs(env_map);
     Ok(())
 }