Compare commits

...

2 Commits

Author SHA1 Message Date
Michael Bolin
01e0584979 fix: make more PATH reads OsString-aware 2026-03-20 17:31:47 -07:00
Michael Bolin
a73d681558 fix: build env var using OsString instead of String 2026-03-20 17:05:24 -07:00
5 changed files with 72 additions and 31 deletions

View File

@@ -309,14 +309,16 @@ pub fn prepend_path_entry_for_codex_aliases() -> std::io::Result<Arg0PathEntryGu
#[cfg(windows)]
const PATH_SEPARATOR: &str = ";";
let path_element = path.display();
let updated_path_env_var = match std::env::var("PATH") {
Ok(existing_path) => {
format!("{path_element}{PATH_SEPARATOR}{existing_path}")
}
Err(_) => {
format!("{path_element}")
let updated_path_env_var = match std::env::var_os("PATH") {
Some(existing_path) => {
let mut path_env_var =
std::ffi::OsString::with_capacity(path.as_os_str().len() + 1 + existing_path.len());
path_env_var.push(path);
path_env_var.push(PATH_SEPARATOR);
path_env_var.push(existing_path);
path_env_var
}
None => path.as_os_str().to_owned(),
};
unsafe {

View File

@@ -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.

View File

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

View File

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

View File

@@ -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(())
}