Files
codex/codex-rs/core/src/exec_env_tests.rs
Michael Bolin faab4d39e1 fix: preserve platform-specific core shell env vars (#16707)
## Why

We were seeing failures in the following tests as part of trying to get
all the tests running under Bazel on Windows in CI
(https://github.com/openai/codex/pull/16528):

```
suite::shell_command::unicode_output::with_login
suite::shell_command::unicode_output::without_login
```

Certainly `PATHEXT` should have been included in the extra `CORE_VARS`
list, so we fix that up here, but also take things a step further for
now by forcibly ensuring it is set on Windows in the return value of
`create_env()`. Once we get the Windows Bazel build working reliably
(i.e., after #16528 is merged), we should come back to this and confirm
we can remove the special case in `create_env()`.

## What

- Split core env inheritance into `COMMON_CORE_VARS` plus
platform-specific allowlists for Windows and Unix in
[`exec_env.rs`](1b55c88fbf/codex-rs/core/src/exec_env.rs (L45-L81)).
- Preserve `PATHEXT`, `USERNAME`, and `USERPROFILE` on Windows, and
`HOME` / locale vars on Unix.
- Backfill a default `PATHEXT` in `create_env()` on Windows if the
parent env does not provide one, so child process launch still works in
stripped-down Bazel environments.
- Extend the Windows exec-env test to assert mixed-case `PathExt`
survives case-insensitive core filtering, and document why the
shell-command Unicode test goes through a child process.

## Verification

- `cargo test -p codex-core exec_env::tests`
2026-04-03 12:07:07 -07:00

260 lines
8.0 KiB
Rust

use super::*;
use codex_config::types::ShellEnvironmentPolicyInherit;
use maplit::hashmap;
use pretty_assertions::assert_eq;
fn make_vars(pairs: &[(&str, &str)]) -> Vec<(String, String)> {
pairs
.iter()
.map(|(k, v)| (k.to_string(), v.to_string()))
.collect()
}
#[test]
fn test_core_inherit_defaults_keep_sensitive_vars() {
let vars = make_vars(&[
("PATH", "/usr/bin"),
("HOME", "/home/user"),
("API_KEY", "secret"),
("SECRET_TOKEN", "t"),
]);
let policy = ShellEnvironmentPolicy::default(); // inherit All, default excludes ignored
let thread_id = ThreadId::new();
let result = populate_env(vars, &policy, Some(thread_id));
let mut expected: HashMap<String, String> = hashmap! {
"PATH".to_string() => "/usr/bin".to_string(),
"HOME".to_string() => "/home/user".to_string(),
"API_KEY".to_string() => "secret".to_string(),
"SECRET_TOKEN".to_string() => "t".to_string(),
};
expected.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
assert_eq!(result, expected);
}
#[test]
fn test_core_inherit_with_default_excludes_enabled() {
let vars = make_vars(&[
("PATH", "/usr/bin"),
("HOME", "/home/user"),
("API_KEY", "secret"),
("SECRET_TOKEN", "t"),
]);
let policy = ShellEnvironmentPolicy {
ignore_default_excludes: false, // apply KEY/SECRET/TOKEN filter
..Default::default()
};
let thread_id = ThreadId::new();
let result = populate_env(vars, &policy, Some(thread_id));
let mut expected: HashMap<String, String> = hashmap! {
"PATH".to_string() => "/usr/bin".to_string(),
"HOME".to_string() => "/home/user".to_string(),
};
expected.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
assert_eq!(result, expected);
}
#[test]
fn test_include_only() {
let vars = make_vars(&[("PATH", "/usr/bin"), ("FOO", "bar")]);
let policy = ShellEnvironmentPolicy {
// skip default excludes so nothing is removed prematurely
ignore_default_excludes: true,
include_only: vec![EnvironmentVariablePattern::new_case_insensitive("*PATH")],
..Default::default()
};
let thread_id = ThreadId::new();
let result = populate_env(vars, &policy, Some(thread_id));
let mut expected: HashMap<String, String> = hashmap! {
"PATH".to_string() => "/usr/bin".to_string(),
};
expected.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
assert_eq!(result, expected);
}
#[test]
fn test_set_overrides() {
let vars = make_vars(&[("PATH", "/usr/bin")]);
let mut policy = ShellEnvironmentPolicy {
ignore_default_excludes: true,
..Default::default()
};
policy.r#set.insert("NEW_VAR".to_string(), "42".to_string());
let thread_id = ThreadId::new();
let result = populate_env(vars, &policy, Some(thread_id));
let mut expected: HashMap<String, String> = hashmap! {
"PATH".to_string() => "/usr/bin".to_string(),
"NEW_VAR".to_string() => "42".to_string(),
};
expected.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
assert_eq!(result, expected);
}
#[test]
fn populate_env_inserts_thread_id() {
let vars = make_vars(&[("PATH", "/usr/bin")]);
let policy = ShellEnvironmentPolicy::default();
let thread_id = ThreadId::new();
let result = populate_env(vars, &policy, Some(thread_id));
let mut expected: HashMap<String, String> = hashmap! {
"PATH".to_string() => "/usr/bin".to_string(),
};
expected.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
assert_eq!(result, expected);
}
#[test]
fn populate_env_omits_thread_id_when_missing() {
let vars = make_vars(&[("PATH", "/usr/bin")]);
let policy = ShellEnvironmentPolicy::default();
let result = populate_env(vars, &policy, /*thread_id*/ None);
let expected: HashMap<String, String> = hashmap! {
"PATH".to_string() => "/usr/bin".to_string(),
};
assert_eq!(result, expected);
}
#[test]
fn test_inherit_all() {
let vars = make_vars(&[("PATH", "/usr/bin"), ("FOO", "bar")]);
let policy = ShellEnvironmentPolicy {
inherit: ShellEnvironmentPolicyInherit::All,
ignore_default_excludes: true, // keep everything
..Default::default()
};
let thread_id = ThreadId::new();
let result = populate_env(vars.clone(), &policy, Some(thread_id));
let mut expected: HashMap<String, String> = vars.into_iter().collect();
expected.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
assert_eq!(result, expected);
}
#[test]
fn test_inherit_all_with_default_excludes() {
let vars = make_vars(&[("PATH", "/usr/bin"), ("API_KEY", "secret")]);
let policy = ShellEnvironmentPolicy {
inherit: ShellEnvironmentPolicyInherit::All,
ignore_default_excludes: false,
..Default::default()
};
let thread_id = ThreadId::new();
let result = populate_env(vars, &policy, Some(thread_id));
let mut expected: HashMap<String, String> = hashmap! {
"PATH".to_string() => "/usr/bin".to_string(),
};
expected.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
assert_eq!(result, expected);
}
#[test]
#[cfg(target_os = "windows")]
fn test_core_inherit_respects_case_insensitive_names_on_windows() {
let vars = make_vars(&[
("Path", "C:\\Windows\\System32"),
("PathExt", ".COM;.EXE;.BAT;.CMD"),
("TEMP", "C:\\Temp"),
("FOO", "bar"),
]);
let policy = ShellEnvironmentPolicy {
inherit: ShellEnvironmentPolicyInherit::Core,
ignore_default_excludes: true,
..Default::default()
};
let thread_id = ThreadId::new();
let result = populate_env(vars, &policy, Some(thread_id));
let mut expected: HashMap<String, String> = hashmap! {
"Path".to_string() => "C:\\Windows\\System32".to_string(),
"PathExt".to_string() => ".COM;.EXE;.BAT;.CMD".to_string(),
"TEMP".to_string() => "C:\\Temp".to_string(),
};
expected.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
assert_eq!(result, expected);
}
#[test]
#[cfg(target_os = "windows")]
fn create_env_inserts_pathext_on_windows_when_missing() {
let vars = make_vars(&[]);
let policy = ShellEnvironmentPolicy {
inherit: ShellEnvironmentPolicyInherit::None,
ignore_default_excludes: true,
..Default::default()
};
let result = create_env_from_vars(vars, &policy, /*thread_id*/ None);
let expected: HashMap<String, String> = hashmap! {
"PATHEXT".to_string() => ".COM;.EXE;.BAT;.CMD".to_string(),
};
assert_eq!(result, expected);
}
#[test]
#[cfg(target_os = "windows")]
fn create_env_preserves_existing_pathext_case_insensitively_on_windows() {
let vars = make_vars(&[("PathExt", ".COM;.EXE;.BAT;.CMD;.PS1")]);
let policy = ShellEnvironmentPolicy {
inherit: ShellEnvironmentPolicyInherit::Core,
ignore_default_excludes: true,
..Default::default()
};
let result = create_env_from_vars(vars, &policy, /*thread_id*/ None);
let pathext_vars = result
.iter()
.filter(|(key, _)| key.eq_ignore_ascii_case("PATHEXT"))
.collect::<Vec<_>>();
assert_eq!(pathext_vars.len(), 1);
assert_eq!(pathext_vars[0].1, ".COM;.EXE;.BAT;.CMD;.PS1");
}
#[test]
fn test_inherit_none() {
let vars = make_vars(&[("PATH", "/usr/bin"), ("HOME", "/home")]);
let mut policy = ShellEnvironmentPolicy {
inherit: ShellEnvironmentPolicyInherit::None,
ignore_default_excludes: true,
..Default::default()
};
policy
.r#set
.insert("ONLY_VAR".to_string(), "yes".to_string());
let thread_id = ThreadId::new();
let result = populate_env(vars, &policy, Some(thread_id));
let mut expected: HashMap<String, String> = hashmap! {
"ONLY_VAR".to_string() => "yes".to_string(),
};
expected.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
assert_eq!(result, expected);
}