From 186b449bc218ced20399bc950e23ca16cc3f9be3 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Wed, 19 Aug 2026 20:14:46 +0000 Subject: [PATCH] Resolve model-provided shells by type (#39607) ## Why A model-provided shell path should select the requested shell type without allowing that path to determine which executable Codex runs. ## What changed - Resolve model-provided shells through Codex's normal shell discovery and fallback logic after detecting their type. - Keep the configured packaged zsh executable when the zsh-fork feature is enabled and the file exists. - Update shell, unified exec, and network approval expectations to use the resolved local executable and arguments. GitOrigin-RevId: ebe6f7eec2cfd1c0548d5bf1a26b7a30dba02cc2 --- codex-rs/core/src/session/session.rs | 20 +- codex-rs/core/src/shell.rs | 4 +- codex-rs/core/src/shell_snapshot.rs | 4 +- codex-rs/core/src/shell_tests.rs | 18 +- .../src/tools/handlers/unified_exec_tests.rs | 14 +- .../core/tests/suite/executable_identity.rs | 349 ------------------ codex-rs/core/tests/suite/mod.rs | 16 - codex-rs/core/tests/suite/network_approval.rs | 15 +- codex-rs/shell-command/src/shell_detect.rs | 55 ++- 9 files changed, 73 insertions(+), 422 deletions(-) delete mode 100644 codex-rs/core/tests/suite/executable_identity.rs diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 1d999d63af..c934599a68 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -1130,13 +1130,19 @@ impl Session { "zsh fork feature enabled, but no packaged zsh fork is available for this install" ) })?; - let zsh_path = zsh_path.to_path_buf(); - shell::get_shell(shell::ShellType::Zsh, Some(&zsh_path)).ok_or_else(|| { - anyhow::anyhow!( - "zsh fork feature enabled, but packaged zsh fork `{}` is not usable", - zsh_path.display() - ) - })? + if zsh_path.is_file() { + shell::Shell { + shell_type: shell::ShellType::Zsh, + shell_path: zsh_path.clone(), + } + } else { + shell::get_shell(shell::ShellType::Zsh).ok_or_else(|| { + anyhow::anyhow!( + "zsh fork feature enabled, but packaged zsh fork `{}` is not usable", + zsh_path.display() + ) + })? + } } else { shell::default_user_shell() }; diff --git a/codex-rs/core/src/shell.rs b/codex-rs/core/src/shell.rs index ccd1ea20d2..37a35bfe3f 100644 --- a/codex-rs/core/src/shell.rs +++ b/codex-rs/core/src/shell.rs @@ -85,8 +85,8 @@ pub fn get_shell_by_model_provided_path(shell_path: &PathBuf) -> Shell { codex_shell_command::shell_detect::get_shell_by_model_provided_path(shell_path).into() } -pub fn get_shell(shell_type: ShellType, path: Option<&PathBuf>) -> Option { - codex_shell_command::shell_detect::get_shell(shell_type, path).map(Into::into) +pub fn get_shell(shell_type: ShellType) -> Option { + codex_shell_command::shell_detect::get_shell(shell_type).map(Into::into) } pub fn default_user_shell() -> Shell { diff --git a/codex-rs/core/src/shell_snapshot.rs b/codex-rs/core/src/shell_snapshot.rs index baeb2b69f4..b0782e5c5a 100644 --- a/codex-rs/core/src/shell_snapshot.rs +++ b/codex-rs/core/src/shell_snapshot.rs @@ -206,8 +206,8 @@ async fn write_shell_snapshot( if shell_type == ShellType::PowerShell || shell_type == ShellType::Cmd { bail!("Shell snapshot not supported yet for {shell_type:?}"); } - let shell = get_shell(shell_type, /*path*/ None) - .with_context(|| format!("No available shell for {shell_type:?}"))?; + let shell = + get_shell(shell_type).with_context(|| format!("No available shell for {shell_type:?}"))?; let raw_snapshot = capture_snapshot(&shell, cwd).await?; let snapshot = strip_snapshot_preamble(&raw_snapshot)?; diff --git a/codex-rs/core/src/shell_tests.rs b/codex-rs/core/src/shell_tests.rs index dea4a4f689..0963ef8364 100644 --- a/codex-rs/core/src/shell_tests.rs +++ b/codex-rs/core/src/shell_tests.rs @@ -5,7 +5,7 @@ use std::process::Command; #[test] #[cfg(target_os = "macos")] fn detects_zsh() { - let zsh_shell = get_shell(ShellType::Zsh, /*path*/ None).unwrap(); + let zsh_shell = get_shell(ShellType::Zsh).unwrap(); let shell_path = zsh_shell.shell_path; @@ -24,7 +24,7 @@ fn fish_fallback_to_zsh() { #[test] fn detects_bash() { - let bash_shell = get_shell(ShellType::Bash, /*path*/ None).unwrap(); + let bash_shell = get_shell(ShellType::Bash).unwrap(); let shell_path = bash_shell.shell_path; assert!( @@ -35,7 +35,7 @@ fn detects_bash() { #[test] fn detects_sh() { - let sh_shell = get_shell(ShellType::Sh, /*path*/ None).unwrap(); + let sh_shell = get_shell(ShellType::Sh).unwrap(); let shell_path = sh_shell.shell_path; assert!( shell_path.file_name().and_then(|name| name.to_str()) == Some("sh"), @@ -48,12 +48,12 @@ fn can_run_on_shell_test() { let cmd = "echo \"Works\""; if cfg!(windows) { assert!(shell_works( - get_shell(ShellType::PowerShell, /*path*/ None), + get_shell(ShellType::PowerShell), "Out-String 'Works'", /*required*/ true, )); assert!(shell_works( - get_shell(ShellType::Cmd, /*path*/ None), + get_shell(ShellType::Cmd), cmd, /*required*/ true, )); @@ -69,17 +69,17 @@ fn can_run_on_shell_test() { /*required*/ true )); assert!(shell_works( - get_shell(ShellType::Zsh, /*path*/ None), + get_shell(ShellType::Zsh), cmd, /*required*/ false )); assert!(shell_works( - get_shell(ShellType::Bash, /*path*/ None), + get_shell(ShellType::Bash), cmd, /*required*/ true )); assert!(shell_works( - get_shell(ShellType::Sh, /*path*/ None), + get_shell(ShellType::Sh), cmd, /*required*/ true )); @@ -181,7 +181,7 @@ fn finds_powershell() { return; } - let powershell_shell = get_shell(ShellType::PowerShell, /*path*/ None).unwrap(); + let powershell_shell = get_shell(ShellType::PowerShell).unwrap(); let shell_path = powershell_shell.shell_path; assert!(shell_path.ends_with("pwsh.exe") || shell_path.ends_with("powershell.exe")); 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 eee95fd43a..775d78cc1f 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs @@ -1,6 +1,7 @@ use super::*; use crate::shell::ShellType; use crate::shell::default_user_shell; +use crate::shell::get_shell; use codex_exec_server::Environment; use codex_tools::UnifiedExecShellMode; use codex_tools::ZshForkConfig; @@ -95,7 +96,7 @@ fn test_get_command_respects_explicit_bash_shell() -> anyhow::Result<()> { } #[test] -fn test_get_command_respects_explicit_powershell_shell() -> anyhow::Result<()> { +fn test_get_command_resolves_powershell_by_type() -> anyhow::Result<()> { let temp_dir = tempfile::tempdir()?; let powershell_path = temp_dir.path().join(if cfg!(windows) { "powershell.exe" @@ -123,10 +124,13 @@ fn test_get_command_respects_explicit_powershell_shell() -> anyhow::Result<()> { /*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); + let expected_shell = get_shell(ShellType::PowerShell) + .unwrap_or_else(|| codex_shell_command::shell_detect::ultimate_fallback_shell().into()); + assert_eq!( + resolved.command, + expected_shell.derive_exec_args("echo hello", /*use_login_shell*/ true) + ); + assert_eq!(resolved.shell_type, expected_shell.shell_type); Ok(()) } diff --git a/codex-rs/core/tests/suite/executable_identity.rs b/codex-rs/core/tests/suite/executable_identity.rs deleted file mode 100644 index 77f87d6e3a..0000000000 --- a/codex-rs/core/tests/suite/executable_identity.rs +++ /dev/null @@ -1,349 +0,0 @@ -use anyhow::Result; -use codex_config::Constrained; -use codex_core::TurnInputRequest; -use codex_features::Feature; -use codex_protocol::config_types::ApprovalsReviewer; -use codex_protocol::models::PermissionProfile; -use codex_protocol::protocol::AskForApproval; -use codex_protocol::protocol::EventMsg; -use codex_protocol::protocol::Op; -use codex_protocol::protocol::ReviewDecision; -use codex_protocol::protocol::ThreadSettingsOverrides; -use codex_protocol::user_input::UserInput; -use core_test_support::responses::ev_assistant_message; -use core_test_support::responses::ev_completed; -use core_test_support::responses::ev_function_call; -use core_test_support::responses::ev_response_created; -use core_test_support::responses::mount_sse_once; -use core_test_support::responses::sse; -use core_test_support::responses::start_mock_server; -use core_test_support::skip_if_remote; -use core_test_support::test_codex::test_codex; -use core_test_support::test_codex::turn_permission_fields; -use core_test_support::wait_for_event; -use pretty_assertions::assert_eq; -use serde_json::json; -use std::fs; -#[cfg(unix)] -use std::os::unix::fs::PermissionsExt; -use test_case::test_case; - -#[derive(Clone, Copy)] -enum ShellAttack { - ExactShellName, - ExactShellNameWithAllowedInnerCommand, - ExactShellNameWithForbiddenInnerCommand, - DangerousCommandOnRequest, - DangerousCommandNever, - ApprovedCustomShell, - SessionApprovalDoesNotTrustDifferentShell, - SpoofedShellExtension, -} - -#[test_case(ShellAttack::ExactShellName; "workspace shell requires approval")] -#[test_case(ShellAttack::ExactShellNameWithAllowedInnerCommand; "inner allow does not trust workspace shell")] -#[test_case(ShellAttack::ExactShellNameWithForbiddenInnerCommand; "inner forbidden rule still rejects workspace shell")] -#[test_case(ShellAttack::DangerousCommandOnRequest; "dangerous inner command requires approval")] -#[test_case(ShellAttack::DangerousCommandNever; "dangerous inner command is forbidden without approval")] -#[test_case(ShellAttack::ApprovedCustomShell; "approved custom shell still runs")] -#[test_case(ShellAttack::SessionApprovalDoesNotTrustDifferentShell; "session approval does not trust a different shell")] -#[test_case(ShellAttack::SpoofedShellExtension; "workspace shell with an extra extension requires approval")] -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn model_provided_shell_cannot_inherit_inner_command_trust( - attack: ShellAttack, -) -> Result<()> { - skip_if_remote!( - Ok(()), - "remote executors already replace requested shell paths with their reported shell" - ); - - let approval_policy = match attack { - ShellAttack::DangerousCommandOnRequest => AskForApproval::OnRequest, - ShellAttack::DangerousCommandNever => AskForApproval::Never, - ShellAttack::ExactShellName - | ShellAttack::ExactShellNameWithAllowedInnerCommand - | ShellAttack::ExactShellNameWithForbiddenInnerCommand - | ShellAttack::ApprovedCustomShell - | ShellAttack::SessionApprovalDoesNotTrustDifferentShell - | ShellAttack::SpoofedShellExtension => AskForApproval::UnlessTrusted, - }; - let server = start_mock_server().await; - let mut builder = test_codex().with_config(move |config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("enable unified exec"); - config.permissions.approval_policy = Constrained::allow_any(approval_policy); - config.approvals_reviewer = ApprovalsReviewer::User; - let inner_command_rule = match attack { - ShellAttack::ExactShellNameWithAllowedInnerCommand => { - Some("prefix_rule(pattern=[\"echo\"], decision=\"allow\")\n") - } - ShellAttack::ExactShellNameWithForbiddenInnerCommand => { - if cfg!(windows) { - Some( - r#"prefix_rule(pattern=["Remove-Item", "C:\\important"], decision="forbidden")"#, - ) - } else { - Some("prefix_rule(pattern=[\"rm\"], decision=\"forbidden\")\n") - } - } - ShellAttack::ExactShellName - | ShellAttack::DangerousCommandOnRequest - | ShellAttack::DangerousCommandNever - | ShellAttack::ApprovedCustomShell - | ShellAttack::SessionApprovalDoesNotTrustDifferentShell - | ShellAttack::SpoofedShellExtension => None, - }; - if let Some(inner_command_rule) = inner_command_rule { - let policy_path = config.codex_home.join("rules/default.rules"); - fs::create_dir_all(policy_path.parent().expect("rules directory")) - .expect("create rules directory"); - fs::write(policy_path, inner_command_rule).expect("write execution policy rule"); - } - }); - #[cfg(windows)] - if matches!( - attack, - ShellAttack::ExactShellNameWithForbiddenInnerCommand | ShellAttack::ApprovedCustomShell - ) { - let system_root = std::env::var_os("SystemRoot").expect("Windows SystemRoot"); - let configured_shell = std::path::Path::new(&system_root) - .join("System32") - .join("WindowsPowerShell") - .join("v1.0") - .join("powershell.exe"); - builder = builder.with_user_shell(codex_core::shell::get_shell_by_model_provided_path( - &configured_shell, - )); - } - let test = builder.build_with_auto_env(&server).await?; - let shell_name = match attack { - ShellAttack::ExactShellNameWithForbiddenInnerCommand | ShellAttack::ApprovedCustomShell - if cfg!(windows) => - { - "pwsh.exe" - } - ShellAttack::ExactShellName - | ShellAttack::ExactShellNameWithAllowedInnerCommand - | ShellAttack::ExactShellNameWithForbiddenInnerCommand - | ShellAttack::DangerousCommandOnRequest - | ShellAttack::DangerousCommandNever - | ShellAttack::ApprovedCustomShell - | ShellAttack::SessionApprovalDoesNotTrustDifferentShell => { - if cfg!(windows) { - "powershell.exe" - } else { - "bash" - } - } - ShellAttack::SpoofedShellExtension => { - if cfg!(windows) { - "powershell.evil" - } else { - "bash.evil" - } - } - }; - let shell = test.workspace_path(shell_name); - let marker = test.workspace_path("attacker-executed"); - #[cfg(unix)] - { - fs::write(&shell, "#!/bin/sh\nprintf ran > attacker-executed\n")?; - fs::set_permissions(&shell, fs::Permissions::from_mode(0o755))?; - } - #[cfg(windows)] - { - let test_executable = std::env::current_exe()?; - fs::hard_link(&test_executable, &shell) - .or_else(|_| fs::copy(&test_executable, &shell).map(|_| ()))?; - fs::write( - shell.with_file_name(".codex-executable-identity-fixture"), - b"fake shell", - )?; - } - let other_shell = if matches!( - attack, - ShellAttack::SessionApprovalDoesNotTrustDifferentShell - ) { - let other_shell = test.workspace_path("another").join(shell_name); - fs::create_dir_all(other_shell.parent().expect("alternate shell directory"))?; - fs::copy(&shell, &other_shell)?; - #[cfg(windows)] - fs::write( - other_shell.with_file_name(".codex-executable-identity-fixture"), - b"fake shell", - )?; - Some(other_shell) - } else { - None - }; - let call_id = "untrusted-shell-path"; - let other_call_id = "different-untrusted-shell-path"; - let command = match attack { - ShellAttack::ExactShellName if cfg!(windows) => "Write-Output $env:USERNAME", - ShellAttack::DangerousCommandOnRequest | ShellAttack::DangerousCommandNever => { - if cfg!(windows) { - "Remove-Item important -Force" - } else { - "rm -rf important" - } - } - ShellAttack::ExactShellNameWithForbiddenInnerCommand => { - if cfg!(windows) { - r"echo shell-safe && Remove-Item C:\important" - } else { - "echo shell-safe; rm important" - } - } - ShellAttack::ExactShellName - | ShellAttack::ExactShellNameWithAllowedInnerCommand - | ShellAttack::ApprovedCustomShell - | ShellAttack::SessionApprovalDoesNotTrustDifferentShell - | ShellAttack::SpoofedShellExtension => "echo shell-safe", - }; - - mount_sse_once( - &server, - sse(vec![ - ev_response_created("resp-untrusted-shell-1"), - ev_function_call( - call_id, - "exec_command", - &json!({ "cmd": command, "shell": shell }).to_string(), - ), - ev_completed("resp-untrusted-shell-1"), - ]), - ) - .await; - if let Some(other_shell) = other_shell.as_ref() { - mount_sse_once( - &server, - sse(vec![ - ev_response_created("resp-different-untrusted-shell"), - ev_function_call( - other_call_id, - "exec_command", - &json!({ "cmd": command, "shell": other_shell }).to_string(), - ), - ev_completed("resp-different-untrusted-shell"), - ]), - ) - .await; - } - let completed = mount_sse_once( - &server, - sse(vec![ - ev_assistant_message("msg-untrusted-shell", "done"), - ev_completed("resp-untrusted-shell-2"), - ]), - ) - .await; - - let (sandbox_policy, permission_profile) = - turn_permission_fields(PermissionProfile::Disabled, test.config.cwd.as_path()); - test.codex - .start_or_steer_turn( - TurnInputRequest::user_input(vec![UserInput::Text { - text: "inspect the repository".to_string(), - text_elements: Vec::new(), - }]) - .with_thread_settings(ThreadSettingsOverrides { - approval_policy: Some(approval_policy), - approvals_reviewer: Some(ApprovalsReviewer::User), - sandbox_policy: Some(sandbox_policy), - permission_profile, - ..Default::default() - }), - ) - .await?; - - let event = wait_for_event(&test.codex, |event| { - matches!( - event, - EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_) - ) - }) - .await; - if matches!( - attack, - ShellAttack::ExactShellNameWithForbiddenInnerCommand | ShellAttack::DangerousCommandNever - ) { - assert!(matches!(event, EventMsg::TurnComplete(_))); - let output = completed - .single_request() - .function_call_output_text(call_id) - .expect("forbidden command output"); - assert!( - output.contains("rejected"), - "the forbidden command should be rejected: {output}" - ); - #[cfg(windows)] - if matches!(attack, ShellAttack::ExactShellNameWithForbiddenInnerCommand) { - assert!( - output.contains("Remove-Item"), - "the forbidden PowerShell command should remain visible to policy: {output}" - ); - } - } else { - let EventMsg::ExecApprovalRequest(approval) = event else { - panic!("workspace shell bypassed approval"); - }; - assert_eq!(approval.call_id, call_id); - assert!(!marker.exists(), "the shell ran before approval"); - - test.codex - .submit(Op::ExecApproval { - id: approval.effective_approval_id(), - turn_id: None, - decision: match attack { - ShellAttack::ApprovedCustomShell => ReviewDecision::Approved, - ShellAttack::SessionApprovalDoesNotTrustDifferentShell => { - ReviewDecision::ApprovedForSession - } - _ => ReviewDecision::denied("untrusted shell"), - }, - }) - .await?; - if other_shell.is_some() { - let event = wait_for_event(&test.codex, |event| { - matches!( - event, - EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_) - ) - }) - .await; - let EventMsg::ExecApprovalRequest(approval) = event else { - panic!("a different workspace shell reused the first shell's session approval"); - }; - assert_eq!(approval.call_id, other_call_id); - assert!( - marker.exists(), - "the session-approved shell should have run" - ); - test.codex - .submit(Op::ExecApproval { - id: approval.effective_approval_id(), - turn_id: None, - decision: ReviewDecision::denied("different untrusted shell"), - }) - .await?; - } - wait_for_event(&test.codex, |event| { - matches!(event, EventMsg::TurnComplete(_)) - }) - .await; - } - - assert_eq!( - marker.exists(), - matches!( - attack, - ShellAttack::ApprovedCustomShell - | ShellAttack::SessionApprovalDoesNotTrustDifferentShell - ), - "only an explicitly approved custom shell should run" - ); - Ok(()) -} diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index ee7a971fd9..ac6014729e 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -16,21 +16,6 @@ use ctor::ctor; #[ctor] pub static CODEX_ALIASES_TEMP_DIR: Option = { configure_test_binary_dispatch("codex-core-tests", |exe_name, argv1| { - #[cfg(windows)] - if exe_name.eq_ignore_ascii_case("powershell.exe") - || exe_name.eq_ignore_ascii_case("powershell.evil") - || exe_name.eq_ignore_ascii_case("pwsh.exe") - { - let executable = std::env::current_exe().expect("locate fake PowerShell executable"); - if executable - .with_file_name(".codex-executable-identity-fixture") - .is_file() - { - let marker = executable.with_file_name("attacker-executed"); - std::fs::write(marker, b"ran").expect("record fake PowerShell execution"); - std::process::exit(0); - } - } if argv1 == Some(CODEX_CORE_APPLY_PATCH_ARG1) { return TestBinaryDispatchMode::DispatchArg0Only; } @@ -77,7 +62,6 @@ mod cyber_exec_policy; mod deprecation_notice; mod exec; mod exec_policy; -mod executable_identity; #[cfg(not(target_os = "windows"))] mod extension_sandbox; mod external_auth; diff --git a/codex-rs/core/tests/suite/network_approval.rs b/codex-rs/core/tests/suite/network_approval.rs index 03f4a8b650..6db81300d4 100644 --- a/codex-rs/core/tests/suite/network_approval.rs +++ b/codex-rs/core/tests/suite/network_approval.rs @@ -3,6 +3,8 @@ use anyhow::Result; use codex_config::types::ApprovalsReviewer; use codex_core::TurnInputRequest; use codex_core::config::Constrained; +use codex_core::shell::ShellType; +use codex_core::shell::get_shell; use codex_exec_server::CreateDirectoryOptions; use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_exec_server::REMOTE_ENVIRONMENT_ID; @@ -96,6 +98,9 @@ async fn guardian_network_approval_preserves_action_and_outcome_routing() -> Res .context("expected network command")? .to_string(); let second_command = first_command.clone(); + let expected_command = get_shell(ShellType::Sh) + .context("expected local sh")? + .derive_exec_args(&first_command, /*use_login_shell*/ false); let denial = "The destination is outside the approved test boundary."; let responses = mount_sse_sequence( &server, @@ -178,7 +183,7 @@ async fn guardian_network_approval_preserves_action_and_outcome_routing() -> Res "tool": "network_access", "trigger": { "callId": first_call_id, - "command": ["/bin/sh", "-c", first_command], + "command": expected_command, "cwd": test.config.cwd, "sandboxPermissions": "use_default", "toolName": "exec_command", @@ -1869,12 +1874,18 @@ async fn remote_guardian_network_decisions_are_scoped_to_each_request_and_enviro .await .context("remote session approval should bypass another network prompt")?; + let local_shell = get_shell(ShellType::Sh).context("expected local sh")?; let mut expected_actions = Vec::with_capacity(cases.len()); for (call_id, command, environment, _, _) in &cases { let cwd = environment .cwd .to_abs_path() .with_context(|| format!("resolve the environment cwd for {call_id}"))?; + let command = if environment.environment_id == LOCAL_ENVIRONMENT_ID { + local_shell.derive_exec_args(command, /*use_login_shell*/ false) + } else { + vec!["/bin/sh".to_string(), "-c".to_string(), command.clone()] + }; expected_actions.push(json!({ "host": NETWORK_TEST_HOST, "port": 80, @@ -1883,7 +1894,7 @@ async fn remote_guardian_network_decisions_are_scoped_to_each_request_and_enviro "tool": "network_access", "trigger": { "callId": call_id, - "command": ["/bin/sh", "-c", command], + "command": command, "cwd": cwd, "sandboxPermissions": "use_default", "toolName": "exec_command", diff --git a/codex-rs/shell-command/src/shell_detect.rs b/codex-rs/shell-command/src/shell_detect.rs index 69a66f0563..2545bf09b0 100644 --- a/codex-rs/shell-command/src/shell_detect.rs +++ b/codex-rs/shell-command/src/shell_detect.rs @@ -134,14 +134,9 @@ fn file_exists(path: &std::path::Path) -> Option { fn get_shell_path( shell_type: ShellType, - provided_path: Option<&PathBuf>, binary_name: &str, fallback_paths: &[&str], ) -> Option { - if let Some(path) = provided_path.and_then(|path| file_exists(path)) { - return Some(path); - } - let default_shell_path = get_user_shell_path(); if let Some(default_shell_path) = default_shell_path && detect_shell_type(&default_shell_path) == Some(shell_type) @@ -165,8 +160,8 @@ fn get_shell_path( const ZSH_FALLBACK_PATHS: &[&str] = &["/bin/zsh"]; -fn get_zsh_shell(path: Option<&PathBuf>) -> Option { - let shell_path = get_shell_path(ShellType::Zsh, path, "zsh", ZSH_FALLBACK_PATHS); +fn get_zsh_shell() -> Option { + let shell_path = get_shell_path(ShellType::Zsh, "zsh", ZSH_FALLBACK_PATHS); shell_path.map(|shell_path| DetectedShell { shell_type: ShellType::Zsh, @@ -176,8 +171,8 @@ fn get_zsh_shell(path: Option<&PathBuf>) -> Option { const BASH_FALLBACK_PATHS: &[&str] = &["/bin/bash", "/usr/bin/bash"]; -fn get_bash_shell(path: Option<&PathBuf>) -> Option { - let shell_path = get_shell_path(ShellType::Bash, path, "bash", BASH_FALLBACK_PATHS); +fn get_bash_shell() -> Option { + let shell_path = get_shell_path(ShellType::Bash, "bash", BASH_FALLBACK_PATHS); shell_path.map(|shell_path| DetectedShell { shell_type: ShellType::Bash, @@ -187,8 +182,8 @@ fn get_bash_shell(path: Option<&PathBuf>) -> Option { const SH_FALLBACK_PATHS: &[&str] = &["/bin/sh"]; -fn get_sh_shell(path: Option<&PathBuf>) -> Option { - let shell_path = get_shell_path(ShellType::Sh, path, "sh", SH_FALLBACK_PATHS); +fn get_sh_shell() -> Option { + let shell_path = get_shell_path(ShellType::Sh, "sh", SH_FALLBACK_PATHS); shell_path.map(|shell_path| DetectedShell { shell_type: ShellType::Sh, @@ -212,12 +207,11 @@ const POWERSHELL_FALLBACK_PATHS: &[&str] = #[cfg(not(windows))] const POWERSHELL_FALLBACK_PATHS: &[&str] = &[]; -fn get_powershell_shell(path: Option<&PathBuf>) -> Option { - let shell_path = get_shell_path(ShellType::PowerShell, path, "pwsh", PWSH_FALLBACK_PATHS) - .or_else(|| { +fn get_powershell_shell() -> Option { + let shell_path = + get_shell_path(ShellType::PowerShell, "pwsh", PWSH_FALLBACK_PATHS).or_else(|| { get_shell_path( ShellType::PowerShell, - path, "powershell", POWERSHELL_FALLBACK_PATHS, ) @@ -229,8 +223,8 @@ fn get_powershell_shell(path: Option<&PathBuf>) -> Option { }) } -fn get_cmd_shell(path: Option<&PathBuf>) -> Option { - let shell_path = get_shell_path(ShellType::Cmd, path, "cmd", &[]); +fn get_cmd_shell() -> Option { + let shell_path = get_shell_path(ShellType::Cmd, "cmd", &[]); shell_path.map(|shell_path| DetectedShell { shell_type: ShellType::Cmd, @@ -252,19 +246,20 @@ pub fn ultimate_fallback_shell() -> DetectedShell { } } +/// Uses the model-provided path only to select a shell type, then discovers its executable. pub fn get_shell_by_model_provided_path(shell_path: &PathBuf) -> DetectedShell { detect_shell_type(shell_path) - .and_then(|shell_type| get_shell(shell_type, Some(shell_path))) + .and_then(get_shell) .unwrap_or_else(ultimate_fallback_shell) } -pub fn get_shell(shell_type: ShellType, path: Option<&PathBuf>) -> Option { +pub fn get_shell(shell_type: ShellType) -> Option { match shell_type { - ShellType::Zsh => get_zsh_shell(path), - ShellType::Bash => get_bash_shell(path), - ShellType::PowerShell => get_powershell_shell(path), - ShellType::Sh => get_sh_shell(path), - ShellType::Cmd => get_cmd_shell(path), + ShellType::Zsh => get_zsh_shell(), + ShellType::Bash => get_bash_shell(), + ShellType::PowerShell => get_powershell_shell(), + ShellType::Sh => get_sh_shell(), + ShellType::Cmd => get_cmd_shell(), } } @@ -274,20 +269,20 @@ pub fn default_user_shell() -> DetectedShell { pub fn default_user_shell_from_path(user_shell_path: Option) -> DetectedShell { if cfg!(windows) { - get_shell(ShellType::PowerShell, /*path*/ None).unwrap_or_else(ultimate_fallback_shell) + get_shell(ShellType::PowerShell).unwrap_or_else(ultimate_fallback_shell) } else { let user_default_shell = user_shell_path .and_then(|shell| detect_shell_type(&shell)) - .and_then(|shell_type| get_shell(shell_type, /*path*/ None)); + .and_then(get_shell); let shell_with_fallback = if cfg!(target_os = "macos") { user_default_shell - .or_else(|| get_shell(ShellType::Zsh, /*path*/ None)) - .or_else(|| get_shell(ShellType::Bash, /*path*/ None)) + .or_else(|| get_shell(ShellType::Zsh)) + .or_else(|| get_shell(ShellType::Bash)) } else { user_default_shell - .or_else(|| get_shell(ShellType::Bash, /*path*/ None)) - .or_else(|| get_shell(ShellType::Zsh, /*path*/ None)) + .or_else(|| get_shell(ShellType::Bash)) + .or_else(|| get_shell(ShellType::Zsh)) }; shell_with_fallback.unwrap_or_else(ultimate_fallback_shell)