Handle Windows environment overrides safely

codex-rs/app-server/README.md

@@ -1090,7 +1090,7 @@ Notes:
 
 - Empty `command` arrays are rejected.
 - Prefer `permissionProfile` for command permission overrides. It selects an active profile by id (for example `:read-only`, `:workspace`, or a user-defined `[permissions.<id>]` profile) rather than accepting low-level filesystem/network permissions. The legacy `sandboxPolicy` field accepts the same shape used by `turn/start` (e.g., `dangerFullAccess`, `readOnly`, `workspaceWrite` with flags, `externalSandbox` with `networkAccess` `restricted|enabled`), but cannot be combined with `permissionProfile`.
-- `env` merges into the environment produced by the server's shell environment policy. Matching names are overridden; unspecified variables are left intact.
+- `env` merges into the environment produced by the server's shell environment policy. Matching names are overridden; unspecified variables are left intact. On Windows, ASCII names are matched case-insensitively, and requests containing multiple ASCII-case-equivalent names are rejected.
 - When omitted, `timeoutMs` falls back to the server default.
 - When omitted, `outputBytesCap` falls back to the server default of 1 MiB per stream.
 - `disableOutputCap: true` disables stdout/stderr capture truncation for that `command/exec` request. It cannot be combined with `outputBytesCap`.
@@ -1225,6 +1225,7 @@ For interactive or streaming processes, set `tty: true` or `streamStdoutStderr:
 - Empty `command` arrays and empty `processHandle` strings are rejected.
 - `cwd` is required and must be absolute.
 - `process/spawn` is intentionally unsandboxed and does not define sandbox-selection fields such as `sandboxPolicy` or `permissionProfile`.
+- `env` overrides and unsets inherited variables by exact name on Unix. On Windows, ASCII names are matched case-insensitively, and requests containing multiple ASCII-case-equivalent names are rejected.
 - Duplicate active `processHandle` values are rejected on the same connection; the same handle can be reused after the prior process exits.
 - `tty: true` implies PTY mode plus `streamStdin: true` and `streamStdoutStderr: true`.
 - `process/writeStdin` accepts either `deltaBase64`, `closeStdin`, or both.

codex-rs/app-server/src/request_processors.rs

@@ -493,6 +493,7 @@ mod apps_processor;
 mod catalog_processor;
 mod command_exec_processor;
 mod config_processor;
+mod environment_overrides;
 mod environment_processor;
 mod external_agent_config_processor;
 mod external_agent_session_import;

codex-rs/app-server/src/request_processors/command_exec_processor.rs

@@ -1,3 +1,4 @@
+use super::environment_overrides::apply_environment_overrides;
 use super::*;
 
 #[derive(Clone)]
@@ -152,16 +153,8 @@ impl CommandExecRequestProcessor {
             /*thread_id*/ None,
         );
         if let Some(env_overrides) = env_overrides {
-            for (key, value) in env_overrides {
-                match value {
-                    Some(value) => {
-                        env.insert(key, value);
-                    }
-                    None => {
-                        env.remove(&key);
-                    }
-                }
-            }
+            apply_environment_overrides(&mut env, env_overrides)
+                .map_err(|error| invalid_params(format!("command/exec env: {error}")))?;
         }
         let timeout_ms = match timeout_ms {
             Some(timeout_ms) => match u64::try_from(timeout_ms) {

codex-rs/app-server/src/request_processors/environment_overrides.rs

@@ -0,0 +1,98 @@
+use std::collections::HashMap;
+use std::fmt;
+
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+enum EnvironmentNameSemantics {
+    CaseSensitive,
+    WindowsAsciiCaseInsensitive,
+}
+
+impl EnvironmentNameSemantics {
+    const fn for_target() -> Self {
+        if cfg!(windows) {
+            Self::WindowsAsciiCaseInsensitive
+        } else {
+            Self::CaseSensitive
+        }
+    }
+}
+
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub(super) struct EnvironmentOverrideError {
+    first_key: String,
+    second_key: String,
+}
+
+impl fmt::Display for EnvironmentOverrideError {
+    fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(
+            formatter,
+            "environment override keys '{}' and '{}' are equivalent on Windows",
+            self.first_key, self.second_key
+        )
+    }
+}
+
+pub(super) fn apply_environment_overrides(
+    env: &mut HashMap<String, String>,
+    overrides: HashMap<String, Option<String>>,
+) -> Result<(), EnvironmentOverrideError> {
+    apply_environment_overrides_with_semantics(
+        env,
+        overrides,
+        EnvironmentNameSemantics::for_target(),
+    )
+}
+
+fn apply_environment_overrides_with_semantics(
+    env: &mut HashMap<String, String>,
+    overrides: HashMap<String, Option<String>>,
+    semantics: EnvironmentNameSemantics,
+) -> Result<(), EnvironmentOverrideError> {
+    match semantics {
+        EnvironmentNameSemantics::CaseSensitive => {
+            for (key, value) in overrides {
+                match value {
+                    Some(value) => {
+                        env.insert(key, value);
+                    }
+                    None => {
+                        env.remove(&key);
+                    }
+                }
+            }
+        }
+        EnvironmentNameSemantics::WindowsAsciiCaseInsensitive => {
+            validate_windows_override_keys(&overrides)?;
+            for (key, value) in overrides {
+                env.retain(|existing_key, _| !existing_key.eq_ignore_ascii_case(&key));
+                if let Some(value) = value {
+                    env.insert(key, value);
+                }
+            }
+        }
+    }
+    Ok(())
+}
+
+fn validate_windows_override_keys(
+    overrides: &HashMap<String, Option<String>>,
+) -> Result<(), EnvironmentOverrideError> {
+    let mut keys = overrides.keys().collect::<Vec<_>>();
+    keys.sort_unstable();
+    for (index, first_key) in keys.iter().enumerate() {
+        for second_key in &keys[index + 1..] {
+            if first_key.eq_ignore_ascii_case(second_key) {
+                return Err(EnvironmentOverrideError {
+                    first_key: (*first_key).clone(),
+                    second_key: (*second_key).clone(),
+                });
+            }
+        }
+    }
+    Ok(())
+}
+
+#[cfg(test)]
+#[path = "environment_overrides_tests.rs"]
+mod tests;

codex-rs/app-server/src/request_processors/environment_overrides_tests.rs

@@ -0,0 +1,180 @@
+use super::*;
+use pretty_assertions::assert_eq;
+
+fn string_map<const N: usize>(entries: [(&str, &str); N]) -> HashMap<String, String> {
+    entries
+        .into_iter()
+        .map(|(key, value)| (key.to_string(), value.to_string()))
+        .collect()
+}
+
+fn override_map<const N: usize>(
+    entries: [(&str, Option<&str>); N],
+) -> HashMap<String, Option<String>> {
+    entries
+        .into_iter()
+        .map(|(key, value)| (key.to_string(), value.map(std::string::ToString::to_string)))
+        .collect()
+}
+
+fn apply_legacy_case_sensitive_overrides(
+    env: &mut HashMap<String, String>,
+    overrides: HashMap<String, Option<String>>,
+) {
+    for (key, value) in overrides {
+        match value {
+            Some(value) => {
+                env.insert(key, value);
+            }
+            None => {
+                env.remove(&key);
+            }
+        }
+    }
+}
+
+#[test]
+fn windows_present_override_replaces_every_case_variant() {
+    let mut env = string_map([
+        ("git_allow_protocol", "ext"),
+        ("Git_Allow_Protocol", "file"),
+        ("OTHER", "keep"),
+    ]);
+
+    apply_environment_overrides_with_semantics(
+        &mut env,
+        override_map([("GIT_ALLOW_PROTOCOL", Some(""))]),
+        EnvironmentNameSemantics::WindowsAsciiCaseInsensitive,
+    )
+    .expect("Windows override should be unambiguous");
+
+    assert_eq!(
+        env,
+        string_map([("GIT_ALLOW_PROTOCOL", ""), ("OTHER", "keep")])
+    );
+}
+
+#[test]
+fn windows_unset_removes_every_case_variant() {
+    let mut env = string_map([("Path", "first"), ("PATH", "second"), ("OTHER", "keep")]);
+
+    apply_environment_overrides_with_semantics(
+        &mut env,
+        override_map([("path", None)]),
+        EnvironmentNameSemantics::WindowsAsciiCaseInsensitive,
+    )
+    .expect("Windows unset should be unambiguous");
+
+    assert_eq!(env, string_map([("OTHER", "keep")]));
+}
+
+#[test]
+fn windows_local_only_overrides_replace_both_permissive_controls() {
+    let mut env = string_map([
+        ("git_allow_protocol", "ext"),
+        ("git_no_lazy_fetch", "0"),
+        ("OTHER", "keep"),
+    ]);
+
+    apply_environment_overrides_with_semantics(
+        &mut env,
+        override_map([
+            ("GIT_ALLOW_PROTOCOL", Some("")),
+            ("GIT_NO_LAZY_FETCH", Some("1")),
+        ]),
+        EnvironmentNameSemantics::WindowsAsciiCaseInsensitive,
+    )
+    .expect("local-only overrides should be unambiguous");
+
+    assert_eq!(
+        env,
+        string_map([
+            ("GIT_ALLOW_PROTOCOL", ""),
+            ("GIT_NO_LAZY_FETCH", "1"),
+            ("OTHER", "keep"),
+        ])
+    );
+}
+
+#[test]
+fn windows_rejects_equivalent_request_keys_before_mutation() {
+    let initial = string_map([("Path", "old"), ("OTHER", "keep")]);
+    let mut env = initial.clone();
+
+    let error = apply_environment_overrides_with_semantics(
+        &mut env,
+        override_map([("PATH", Some("first")), ("Path", Some("second"))]),
+        EnvironmentNameSemantics::WindowsAsciiCaseInsensitive,
+    )
+    .expect_err("equivalent Windows request keys should be rejected");
+
+    assert_eq!(
+        error,
+        EnvironmentOverrideError {
+            first_key: "PATH".to_string(),
+            second_key: "Path".to_string(),
+        }
+    );
+    assert_eq!(env, initial);
+}
+
+#[test]
+fn case_sensitive_mode_preserves_differently_cased_names() {
+    let mut env = string_map([("Path", "old"), ("PATH", "replace"), ("OTHER", "keep")]);
+
+    apply_environment_overrides_with_semantics(
+        &mut env,
+        override_map([("PATH", None), ("path", Some("new"))]),
+        EnvironmentNameSemantics::CaseSensitive,
+    )
+    .expect("case-sensitive overrides should not conflict");
+
+    assert_eq!(
+        env,
+        string_map([("Path", "old"), ("path", "new"), ("OTHER", "keep")])
+    );
+}
+
+#[test]
+fn case_sensitive_mode_matches_legacy_merge_behavior() {
+    let fixtures = [
+        (
+            string_map([("Path", "old"), ("PATH", "replace"), ("OTHER", "keep")]),
+            override_map([("PATH", None), ("path", Some("new"))]),
+        ),
+        (
+            string_map([("FOO", "old"), ("BAR", "keep"), ("EMPTY", "old")]),
+            override_map([
+                ("FOO", Some("new")),
+                ("ADDED", Some("value")),
+                ("EMPTY", None),
+            ]),
+        ),
+    ];
+
+    for (initial, overrides) in fixtures {
+        let mut expected = initial.clone();
+        apply_legacy_case_sensitive_overrides(&mut expected, overrides.clone());
+        let mut actual = initial;
+
+        apply_environment_overrides_with_semantics(
+            &mut actual,
+            overrides,
+            EnvironmentNameSemantics::CaseSensitive,
+        )
+        .expect("case-sensitive overrides should not conflict");
+
+        assert_eq!(actual, expected);
+    }
+}
+
+#[test]
+fn target_semantics_matches_platform() {
+    let expected = if cfg!(windows) {
+        EnvironmentNameSemantics::WindowsAsciiCaseInsensitive
+    } else {
+        EnvironmentNameSemantics::CaseSensitive
+    };
+
+    assert_eq!(EnvironmentNameSemantics::for_target(), expected);
+}

codex-rs/app-server/src/request_processors/process_exec_processor.rs

@@ -42,6 +42,7 @@ use crate::error_code::invalid_request;
 use crate::outgoing_message::ConnectionId;
 use crate::outgoing_message::ConnectionRequestId;
 use crate::outgoing_message::OutgoingMessageSender;
+use crate::request_processors::environment_overrides::apply_environment_overrides;
 
 const EXEC_TIMEOUT_EXIT_CODE: i32 = 124;
 const OUTPUT_CHUNK_SIZE_HINT: usize = 64 * 1024;
@@ -96,16 +97,8 @@ impl ProcessExecRequestProcessor {
         }
         let mut env = std::env::vars().collect::<HashMap<_, _>>();
         if let Some(env_overrides) = env_overrides {
-            for (key, value) in env_overrides {
-                match value {
-                    Some(value) => {
-                        env.insert(key, value);
-                    }
-                    None => {
-                        env.remove(&key);
-                    }
-                }
-            }
+            apply_environment_overrides(&mut env, env_overrides)
+                .map_err(|error| invalid_params(format!("{method_name} env: {error}")))?;
         }
         let expiration = match timeout_ms {
             Some(Some(timeout_ms)) => match u64::try_from(timeout_ms) {

codex-rs/app-server/tests/suite/v2/command_exec.rs

@@ -37,6 +37,17 @@ use super::connection_handling_websocket::send_initialize_request;
 use super::connection_handling_websocket::send_request;
 use super::connection_handling_websocket::spawn_websocket_server;
 
+#[cfg(windows)]
+fn require_absent_windows_environment(names: &[&str]) -> Result<()> {
+    for name in names {
+        anyhow::ensure!(
+            std::env::var_os(name).is_none(),
+            "cannot establish lowercase Windows environment fixture because {name} is already set"
+        );
+    }
+    Ok(())
+}
+
 #[tokio::test]
 async fn command_exec_without_streams_can_be_terminated() -> Result<()> {
     let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
@@ -193,6 +204,268 @@ async fn command_exec_env_overrides_merge_with_server_environment_and_support_un
     Ok(())
 }
 
+#[cfg(windows)]
+#[tokio::test]
+async fn command_exec_env_overrides_use_windows_case_folding() -> Result<()> {
+    let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
+    let codex_home = TempDir::new()?;
+    create_config_toml(codex_home.path(), &server.uri(), "never")?;
+    require_absent_windows_environment(&[
+        "GIT_ALLOW_PROTOCOL",
+        "GIT_NO_LAZY_FETCH",
+        "CASEFOLD_REMOVE_ME",
+    ])?;
+    let mut mcp = TestAppServer::new_with_env(
+        codex_home.path(),
+        &[
+            ("git_allow_protocol", Some("ext")),
+            ("git_no_lazy_fetch", Some("0")),
+            ("casefold_remove_me", Some("inherited")),
+        ],
+    )
+    .await?;
+    timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
+
+    let system_root = std::env::var_os("SystemRoot").context("SystemRoot should be set")?;
+    let powershell = Path::new(&system_root)
+        .join("System32")
+        .join("WindowsPowerShell")
+        .join("v1.0")
+        .join("powershell.exe");
+    let inherited_script = r#"
+$entries = [System.Environment]::GetEnvironmentVariables('Process')
+function Exact-Keys([string] $name) {
+    @($entries.Keys | Where-Object {
+        [string]::Equals(
+            [string] $_,
+            $name,
+            [System.StringComparison]::Ordinal
+        )
+    })
+}
+$allowLower = @(Exact-Keys 'git_allow_protocol')
+$allowUpper = @(Exact-Keys 'GIT_ALLOW_PROTOCOL')
+$noLazyLower = @(Exact-Keys 'git_no_lazy_fetch')
+$noLazyUpper = @(Exact-Keys 'GIT_NO_LAZY_FETCH')
+$removedLower = @(Exact-Keys 'casefold_remove_me')
+$removedUpper = @(Exact-Keys 'CASEFOLD_REMOVE_ME')
+if ($allowLower.Count -ne 1 -or [string] $entries[$allowLower[0]] -ne 'ext') { exit 21 }
+if ($allowUpper.Count -ne 0) { exit 22 }
+if ($noLazyLower.Count -ne 1 -or [string] $entries[$noLazyLower[0]] -ne '0') { exit 23 }
+if ($noLazyUpper.Count -ne 0) { exit 24 }
+if ($removedLower.Count -ne 1 -or [string] $entries[$removedLower[0]] -ne 'inherited') { exit 25 }
+if ($removedUpper.Count -ne 0) { exit 26 }
+[Console]::Out.Write('lowercase-inherited')
+"#;
+    let inherited_request_id = mcp
+        .send_command_exec_request(CommandExecParams {
+            command: vec![
+                powershell.display().to_string(),
+                "-NoProfile".to_string(),
+                "-NonInteractive".to_string(),
+                "-Command".to_string(),
+                inherited_script.to_string(),
+            ],
+            process_id: None,
+            tty: false,
+            stream_stdin: false,
+            stream_stdout_stderr: false,
+            output_bytes_cap: None,
+            disable_output_cap: false,
+            disable_timeout: false,
+            timeout_ms: None,
+            cwd: None,
+            env: None,
+            size: None,
+            sandbox_policy: None,
+            permission_profile: None,
+        })
+        .await?;
+
+    let inherited_response = mcp
+        .read_stream_until_response_message(RequestId::Integer(inherited_request_id))
+        .await?;
+    let inherited_response: CommandExecResponse = to_response(inherited_response)?;
+    assert_eq!(
+        inherited_response,
+        CommandExecResponse {
+            exit_code: 0,
+            stdout: "lowercase-inherited".to_string(),
+            stderr: String::new(),
+        }
+    );
+
+    let overridden_script = r#"
+$entries = [System.Environment]::GetEnvironmentVariables('Process')
+function Matching-Keys([string] $name) {
+    @($entries.Keys | Where-Object {
+        [string]::Equals(
+            [string] $_,
+            $name,
+            [System.StringComparison]::OrdinalIgnoreCase
+        )
+    })
+}
+$allow = @(Matching-Keys 'GIT_ALLOW_PROTOCOL')
+$noLazy = @(Matching-Keys 'GIT_NO_LAZY_FETCH')
+$removed = @(Matching-Keys 'CASEFOLD_REMOVE_ME')
+if ($allow.Count -ne 1 -or [string] $entries[$allow[0]] -ne '') { exit 11 }
+if ($noLazy.Count -ne 1 -or [string] $entries[$noLazy[0]] -ne '1') { exit 12 }
+if ($removed.Count -ne 0) { exit 13 }
+[Console]::Out.Write('casefold-ok')
+"#;
+    let command_request_id = mcp
+        .send_command_exec_request(CommandExecParams {
+            command: vec![
+                powershell.display().to_string(),
+                "-NoProfile".to_string(),
+                "-NonInteractive".to_string(),
+                "-Command".to_string(),
+                overridden_script.to_string(),
+            ],
+            process_id: None,
+            tty: false,
+            stream_stdin: false,
+            stream_stdout_stderr: false,
+            output_bytes_cap: None,
+            disable_output_cap: false,
+            disable_timeout: false,
+            timeout_ms: None,
+            cwd: None,
+            env: Some(HashMap::from([
+                ("GIT_ALLOW_PROTOCOL".to_string(), Some(String::new())),
+                ("GIT_NO_LAZY_FETCH".to_string(), Some("1".to_string())),
+                ("CASEFOLD_REMOVE_ME".to_string(), None),
+            ])),
+            size: None,
+            sandbox_policy: None,
+            permission_profile: None,
+        })
+        .await?;
+
+    let response = mcp
+        .read_stream_until_response_message(RequestId::Integer(command_request_id))
+        .await?;
+    let response: CommandExecResponse = to_response(response)?;
+    assert_eq!(
+        response,
+        CommandExecResponse {
+            exit_code: 0,
+            stdout: "casefold-ok".to_string(),
+            stderr: String::new(),
+        }
+    );
+
+    Ok(())
+}
+
+#[cfg(windows)]
+#[tokio::test]
+async fn command_exec_empty_git_allowlist_blocks_local_file_transport_on_windows() -> Result<()> {
+    let repository_root = TempDir::new()?;
+    let repository = repository_root.path().join("remote.git");
+    let init = std::process::Command::new("git")
+        .args(["init", "-q", "--bare"])
+        .arg(&repository)
+        .output()
+        .context("initialize bare Git repository")?;
+    anyhow::ensure!(
+        init.status.success(),
+        "git init failed: {}",
+        String::from_utf8_lossy(&init.stderr)
+    );
+    let repository_url = url::Url::from_file_path(&repository)
+        .map_err(|()| anyhow::anyhow!("convert repository path to file URL"))?
+        .to_string();
+
+    let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
+    let codex_home = TempDir::new()?;
+    create_config_toml(codex_home.path(), &server.uri(), "never")?;
+    require_absent_windows_environment(&["GIT_ALLOW_PROTOCOL", "GIT_NO_LAZY_FETCH"])?;
+    let mut mcp = TestAppServer::new_with_env(
+        codex_home.path(),
+        &[
+            ("git_allow_protocol", Some("file")),
+            ("git_no_lazy_fetch", Some("0")),
+        ],
+    )
+    .await?;
+    timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
+
+    let control_request_id = mcp
+        .send_command_exec_request(CommandExecParams {
+            command: vec![
+                "git".to_string(),
+                "ls-remote".to_string(),
+                repository_url.clone(),
+            ],
+            process_id: None,
+            tty: false,
+            stream_stdin: false,
+            stream_stdout_stderr: false,
+            output_bytes_cap: None,
+            disable_output_cap: false,
+            disable_timeout: false,
+            timeout_ms: None,
+            cwd: None,
+            env: None,
+            size: None,
+            sandbox_policy: None,
+            permission_profile: None,
+        })
+        .await?;
+
+    let control_response = mcp
+        .read_stream_until_response_message(RequestId::Integer(control_request_id))
+        .await?;
+    let control_response: CommandExecResponse = to_response(control_response)?;
+    assert_eq!(
+        control_response,
+        CommandExecResponse {
+            exit_code: 0,
+            stdout: String::new(),
+            stderr: String::new(),
+        },
+        "inherited lowercase allowlist should permit file:// through command/exec"
+    );
+
+    let command_request_id = mcp
+        .send_command_exec_request(CommandExecParams {
+            command: vec!["git".to_string(), "ls-remote".to_string(), repository_url],
+            process_id: None,
+            tty: false,
+            stream_stdin: false,
+            stream_stdout_stderr: false,
+            output_bytes_cap: None,
+            disable_output_cap: false,
+            disable_timeout: false,
+            timeout_ms: None,
+            cwd: None,
+            env: Some(HashMap::from([
+                ("GIT_ALLOW_PROTOCOL".to_string(), Some(String::new())),
+                ("GIT_NO_LAZY_FETCH".to_string(), Some("1".to_string())),
+            ])),
+            size: None,
+            sandbox_policy: None,
+            permission_profile: None,
+        })
+        .await?;
+
+    let response = mcp
+        .read_stream_until_response_message(RequestId::Integer(command_request_id))
+        .await?;
+    let response: CommandExecResponse = to_response(response)?;
+    assert_ne!(response.exit_code, 0, "empty allowlist should deny file://");
+    assert_eq!(response.stdout, "");
+    assert!(
+        response.stderr.contains("transport 'file' not allowed"),
+        "expected Git transport denial, got: {}",
+        response.stderr
+    );
+
+    Ok(())
+}
+
 #[tokio::test]
 async fn command_exec_accepts_permission_profile() -> Result<()> {
     let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;