From 8ae0c837f0fbc090bef94c6323d0fa3803e55258 Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Wed, 13 May 2026 14:37:50 -0700 Subject: [PATCH] Avoid PowerShell profiles in elevated Windows sandbox (#21400) ## Why On Windows, elevated sandboxed commands run under a dedicated sandbox account while `HOME` / `USERPROFILE` can still point at the real user's profile directory. For PowerShell login shells, that combination can make the sandbox account try to load the real user's PowerShell profile script. If the sandbox account's execution policy differs from the real user's policy, startup can emit profile-loading errors before the requested command runs. For this backend, loading the profile is not a faithful user login shell: it is cross-account profile execution. Treating these PowerShell invocations as non-login shells avoids that invalid startup path. ## Why This Happens Late The normal `login` decision is resolved when shell argv is created, but that point is too early to make this Windows sandbox-specific decision. At argv creation time we do not yet know the actual sandbox attempt that will run the command. A turn can include sandboxed and unsandboxed attempts, and a broad turn-level override would also affect Full Access commands where the user's profile should remain available. Instead, this change carries the selected `ShellType` alongside the argv and applies the `-NoProfile` adjustment in the shell runtimes once the `SandboxAttempt` is known. That keeps the override scoped to actual `WindowsRestrictedToken` attempts with `WindowsSandboxLevel::Elevated`. The runtime uses the selected shell metadata rather than re-detecting PowerShell from argv. That avoids brittle parsing and covers PowerShell invocation shapes such as `-EncodedCommand`. ## What Changed - Carry selected shell metadata through `exec_command` / unified exec requests and shell tool requests. - Insert `-NoProfile` for PowerShell commands only when the runtime is about to execute a sandboxed elevated Windows attempt. - Add focused unit coverage for elevated Windows PowerShell, `-EncodedCommand`, existing `-NoProfile`, legacy restricted-token attempts, unsandboxed attempts, and non-PowerShell commands. ## Verification - `cargo test -p codex-core disable_powershell_profile_tests` - `cargo test -p codex-core test_get_command` - `cargo clippy --fix --tests --allow-dirty --allow-no-vcs -p codex-core` A full `cargo test -p codex-core` run was also attempted during development, but it still hit an unrelated stack overflow in `agent::control` tests before reaching this area. --- codex-rs/core/src/memory_usage.rs | 2 +- codex-rs/core/src/tools/handlers/shell.rs | 4 + .../src/tools/handlers/shell/shell_command.rs | 2 + .../core/src/tools/handlers/unified_exec.rs | 27 ++- .../handlers/unified_exec/exec_command.rs | 5 +- .../src/tools/handlers/unified_exec_tests.rs | 39 ++++- codex-rs/core/src/tools/runtimes/mod.rs | 163 ++++++++++++++++++ codex-rs/core/src/tools/runtimes/shell.rs | 8 + .../core/src/tools/runtimes/unified_exec.rs | 9 + codex-rs/core/src/unified_exec/mod.rs | 2 + .../core/src/unified_exec/process_manager.rs | 1 + .../src/unified_exec/process_manager_tests.rs | 1 + 12 files changed, 245 insertions(+), 18 deletions(-) diff --git a/codex-rs/core/src/memory_usage.rs b/codex-rs/core/src/memory_usage.rs index b5bc6eef5..baa64459a 100644 --- a/codex-rs/core/src/memory_usage.rs +++ b/codex-rs/core/src/memory_usage.rs @@ -71,7 +71,7 @@ fn shell_command_for_invocation(invocation: &ToolInvocation) -> Option<(Vec None, } diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 84f400854..c8c93bd6c 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -7,6 +7,7 @@ use crate::exec::ExecParams; use crate::exec_policy::ExecApprovalRequest; use crate::function_tool::FunctionCallError; use crate::session::turn_context::TurnContext; +use crate::shell::ShellType; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolPayload; use crate::tools::events::ToolEmitter; @@ -44,6 +45,7 @@ struct RunExecLikeArgs { tool_name: ToolName, exec_params: ExecParams, hook_command: String, + shell_type: Option, additional_permissions: Option, prefix_rule: Option>, session: Arc, @@ -59,6 +61,7 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result Result for ShellCommandHandler { session.conversation_id, turn.tools_config.allow_login_shell, )?; + let shell_type = Some(session.user_shell().shell_type.clone()); run_exec_like(RunExecLikeArgs { tool_name, exec_params, hook_command: params.command, + shell_type, additional_permissions: params.additional_permissions.clone(), prefix_rule, session, diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index c97f5bb6f..22e0a16dd 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -1,5 +1,6 @@ use crate::sandboxing::SandboxPermissions; use crate::shell::Shell; +use crate::shell::ShellType; use crate::shell::get_shell_by_model_provided_path; use crate::tools::context::ExecCommandToolOutput; use crate::tools::context::ToolInvocation; @@ -79,6 +80,12 @@ fn effective_max_output_tokens( resolve_max_tokens(max_output_tokens).min(truncation_policy.token_budget()) } +#[derive(Debug)] +pub(crate) struct ResolvedCommand { + pub(crate) command: Vec, + pub(crate) shell_type: ShellType, +} + fn post_unified_exec_tool_use_payload( invocation: &ToolInvocation, result: &ExecCommandToolOutput, @@ -107,7 +114,7 @@ pub(crate) fn get_command( session_shell: Arc, shell_mode: &UnifiedExecShellMode, allow_login_shell: bool, -) -> Result, String> { +) -> Result { let use_login_shell = match args.login { Some(true) if !allow_login_shell => { return Err( @@ -126,13 +133,19 @@ pub(crate) fn get_command( shell }); let shell = model_shell.as_ref().unwrap_or(session_shell.as_ref()); - Ok(shell.derive_exec_args(&args.cmd, use_login_shell)) + Ok(ResolvedCommand { + command: shell.derive_exec_args(&args.cmd, use_login_shell), + shell_type: shell.shell_type.clone(), + }) } - UnifiedExecShellMode::ZshFork(zsh_fork_config) => Ok(vec![ - zsh_fork_config.shell_zsh_path.to_string_lossy().to_string(), - if use_login_shell { "-lc" } else { "-c" }.to_string(), - args.cmd.clone(), - ]), + UnifiedExecShellMode::ZshFork(zsh_fork_config) => Ok(ResolvedCommand { + command: vec![ + zsh_fork_config.shell_zsh_path.to_string_lossy().to_string(), + if use_login_shell { "-lc" } else { "-c" }.to_string(), + args.cmd.clone(), + ], + shell_type: ShellType::Zsh, + }), } } diff --git a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs index cef066ad5..e27c56576 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs @@ -138,13 +138,15 @@ impl ToolExecutor for ExecCommandHandler { ) .await; let process_id = manager.allocate_process_id().await; - let command = get_command( + let resolved_command = get_command( &args, session.user_shell(), &turn.tools_config.unified_exec_shell_mode, turn.tools_config.allow_login_shell, ) .map_err(FunctionCallError::RespondToModel)?; + let command = resolved_command.command; + let shell_type = resolved_command.shell_type; let command_for_display = codex_shell_command::parse_command::shlex_join(&command); let ExecCommandArgs { @@ -249,6 +251,7 @@ impl ToolExecutor for ExecCommandHandler { .exec_command( ExecCommandRequest { command, + shell_type, hook_command: hook_command.clone(), process_id, yield_time_ms, diff --git a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs index 02123c4b6..6ab2752b9 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::shell::ShellType; use crate::shell::default_user_shell; use codex_tools::UnifiedExecShellMode; use codex_tools::ZshForkConfig; @@ -42,13 +43,14 @@ fn test_get_command_uses_default_shell_when_unspecified() -> anyhow::Result<()> assert!(args.shell.is_none()); - let command = get_command( + let resolved = get_command( &args, Arc::new(default_user_shell()), &UnifiedExecShellMode::Direct, /*allow_login_shell*/ true, ) .map_err(anyhow::Error::msg)?; + let command = resolved.command; assert_eq!(command.len(), 3); assert_eq!(command[2], "echo hello"); @@ -63,13 +65,14 @@ fn test_get_command_respects_explicit_bash_shell() -> anyhow::Result<()> { assert_eq!(args.shell.as_deref(), Some("/bin/bash")); - let command = get_command( + let resolved = get_command( &args, Arc::new(default_user_shell()), &UnifiedExecShellMode::Direct, /*allow_login_shell*/ true, ) .map_err(anyhow::Error::msg)?; + let command = resolved.command; assert_eq!(command.last(), Some(&"echo hello".to_string())); if command @@ -83,21 +86,37 @@ fn test_get_command_respects_explicit_bash_shell() -> anyhow::Result<()> { #[test] fn test_get_command_respects_explicit_powershell_shell() -> anyhow::Result<()> { - let json = r#"{"cmd": "echo hello", "shell": "powershell"}"#; + let temp_dir = tempfile::tempdir()?; + let powershell_path = temp_dir.path().join(if cfg!(windows) { + "powershell.exe" + } else { + "powershell" + }); + std::fs::write(&powershell_path, "")?; + let json = serde_json::json!({ + "cmd": "echo hello", + "shell": powershell_path, + }) + .to_string(); - let args: ExecCommandArgs = parse_arguments(json)?; + let args: ExecCommandArgs = parse_arguments(&json)?; - assert_eq!(args.shell.as_deref(), Some("powershell")); + assert_eq!( + args.shell.as_deref(), + Some(powershell_path.to_string_lossy().as_ref()) + ); - let command = get_command( + let resolved = get_command( &args, Arc::new(default_user_shell()), &UnifiedExecShellMode::Direct, /*allow_login_shell*/ true, ) .map_err(anyhow::Error::msg)?; + let command = resolved.command; assert_eq!(command[2], "echo hello"); + assert_eq!(resolved.shell_type, ShellType::PowerShell); Ok(()) } @@ -109,13 +128,14 @@ fn test_get_command_respects_explicit_cmd_shell() -> anyhow::Result<()> { assert_eq!(args.shell.as_deref(), Some("cmd")); - let command = get_command( + let resolved = get_command( &args, Arc::new(default_user_shell()), &UnifiedExecShellMode::Direct, /*allow_login_shell*/ true, ) .map_err(anyhow::Error::msg)?; + let command = resolved.command; assert_eq!(command[2], "echo hello"); Ok(()) @@ -159,7 +179,7 @@ fn test_get_command_ignores_explicit_shell_in_zsh_fork_mode() -> anyhow::Result< })?, }); - let command = get_command( + let resolved = get_command( &args, Arc::new(default_user_shell()), &shell_mode, @@ -168,13 +188,14 @@ fn test_get_command_ignores_explicit_shell_in_zsh_fork_mode() -> anyhow::Result< .map_err(anyhow::Error::msg)?; assert_eq!( - command, + resolved.command, vec![ shell_zsh_path.to_string_lossy().to_string(), "-lc".to_string(), "echo hello".to_string() ] ); + assert_eq!(resolved.shell_type, ShellType::Zsh); Ok(()) } diff --git a/codex-rs/core/src/tools/runtimes/mod.rs b/codex-rs/core/src/tools/runtimes/mod.rs index 073fda8ec..bba1c572e 100644 --- a/codex-rs/core/src/tools/runtimes/mod.rs +++ b/codex-rs/core/src/tools/runtimes/mod.rs @@ -8,6 +8,7 @@ use crate::exec_env::CODEX_THREAD_ID_ENV_VAR; use crate::path_utils; use crate::sandboxing::SandboxPermissions; use crate::shell::Shell; +use crate::shell::ShellType; use crate::tools::sandboxing::ToolError; #[cfg(target_os = "macos")] use codex_network_proxy::CODEX_PROXY_GIT_SSH_COMMAND_MARKER; @@ -15,8 +16,10 @@ use codex_network_proxy::PROXY_ACTIVE_ENV_KEY; use codex_network_proxy::PROXY_ENV_KEYS; #[cfg(target_os = "macos")] use codex_network_proxy::PROXY_GIT_SSH_COMMAND_ENV_KEY; +use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::AdditionalPermissionProfile; use codex_sandboxing::SandboxCommand; +use codex_sandboxing::SandboxType; use codex_utils_absolute_path::AbsolutePathBuf; use std::collections::HashMap; @@ -67,6 +70,35 @@ pub(crate) fn exec_env_for_sandbox_permissions( env } +pub(crate) fn disable_powershell_profile_for_elevated_windows_sandbox( + command: &[String], + shell_type: Option<&ShellType>, + sandbox: SandboxType, + windows_sandbox_level: WindowsSandboxLevel, +) -> Vec { + if shell_type != Some(&ShellType::PowerShell) + || sandbox != SandboxType::WindowsRestrictedToken + || windows_sandbox_level != WindowsSandboxLevel::Elevated + || command.is_empty() + { + return command.to_vec(); + } + + if command[1..] + .iter() + .any(|arg| arg.eq_ignore_ascii_case("-NoProfile")) + { + return command.to_vec(); + } + + // The elevated Windows sandbox runs as a dedicated sandbox account while + // HOME/USERPROFILE may still point at the real user profile. Loading + // PowerShell profiles in that mixed context is not a valid login shell. + let mut command = command.to_vec(); + command.insert(1, "-NoProfile".to_string()); + command +} + /// POSIX-only helper: for commands produced by `Shell::derive_exec_args` /// for Bash/Zsh/sh of the form `[shell_path, "-lc", "