From 3ca0e7673b77303db6e0d686c1d9d34fc2ed63e0 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 24 Feb 2026 10:31:08 -0800 Subject: [PATCH] feat: run zsh fork shell tool via shell-escalation (#12649) ## Why This PR switches the `shell_command` zsh-fork path over to `codex-shell-escalation` so the new shell tool can use the shared exec-wrapper/escalation protocol instead of the `zsh_exec_bridge` implementation that was introduced in https://github.com/openai/codex/pull/12052. `zsh_exec_bridge` relied on UNIX domain sockets, which is not as tamper-proof as the FD-based approach in `codex-shell-escalation`. ## What Changed - Added a Unix zsh-fork runtime adapter in `core` (`core/src/tools/runtimes/shell/unix_escalation.rs`) that: - runs zsh-fork commands through `codex_shell_escalation::run_escalate_server` - bridges exec-policy / approval decisions into `ShellActionProvider` - executes escalated commands via a `ShellCommandExecutor` that calls `process_exec_tool_call` - Updated `ShellRuntime` / `ShellCommandHandler` / tool spec wiring to select a `shell_command` backend (`classic` vs `zsh-fork`) while leaving the generic `shell` tool path unchanged. - Removed the `zsh_exec_bridge`-based session service and deleted `core/src/zsh_exec_bridge/mod.rs`. - Moved exec-wrapper entrypoint dispatch to `arg0` by handling the `codex-execve-wrapper` arg0 alias there, and removed the old `codex_core::maybe_run_zsh_exec_wrapper_mode()` hooks from `cli` and `app-server` mains. - Added the needed `codex-shell-escalation` dependencies for `core` and `arg0`. ## Tests - `cargo test -p codex-core shell_zsh_fork_prefers_shell_command_over_unified_exec` - `cargo test -p codex-app-server turn_start_shell_zsh_fork -- --nocapture` - verifies zsh-fork command execution and approval flows through the new backend - includes subcommand approve/decline coverage using the shared zsh DotSlash fixture in `app-server/tests/suite/zsh` - To test manually, I added the following to `~/.codex/config.toml`: ```toml zsh_path = "/Users/mbolin/code/codex3/codex-rs/app-server/tests/suite/zsh" [features] shell_zsh_fork = true ``` Then I ran `just c` to run the dev build of Codex with these changes and sent it the message: ``` run `echo $0` ``` And it replied with: ``` echo $0 printed: /Users/mbolin/code/codex3/codex-rs/app-server/tests/suite/zsh In this tool context, $0 reflects the script path used to invoke the shell, not just zsh. ``` so the tool appears to be wired up correctly. ## Notes - The zsh subcommand-decline integration test now uses `rm` under a `WorkspaceWrite` sandbox. The previous `/usr/bin/true` scenario is auto-allowed by the new `shell-escalation` policy path, which no longer produces subcommand approval prompts. --- codex-rs/Cargo.lock | 2 + codex-rs/Cargo.toml | 1 + codex-rs/app-server/src/main.rs | 5 - .../tests/suite/v2/turn_start_zsh_fork.rs | 113 ++-- codex-rs/arg0/Cargo.toml | 1 + codex-rs/arg0/src/lib.rs | 30 + codex-rs/cli/src/main.rs | 5 - codex-rs/core/Cargo.toml | 3 + codex-rs/core/src/codex.rs | 17 +- codex-rs/core/src/lib.rs | 23 +- codex-rs/core/src/sandboxing/mod.rs | 8 +- codex-rs/core/src/state/service.rs | 5 +- codex-rs/core/src/tools/handlers/shell.rs | 44 +- codex-rs/core/src/tools/runtimes/shell.rs | 91 +-- .../tools/runtimes/shell/unix_escalation.rs | 463 ++++++++++++++ codex-rs/core/src/tools/spec.rs | 20 +- codex-rs/core/src/zsh_exec_bridge/mod.rs | 570 ------------------ codex-rs/shell-escalation/src/lib.rs | 4 +- .../src/unix/core_shell_escalation.rs | 1 - .../src/unix/escalate_client.rs | 5 +- .../src/unix/escalate_server.rs | 12 +- .../src/unix/execve_wrapper.rs | 2 +- codex-rs/shell-escalation/src/unix/mod.rs | 2 +- .../tui/src/app/pending_interactive_replay.rs | 2 + 24 files changed, 706 insertions(+), 723 deletions(-) create mode 100644 codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs delete mode 100644 codex-rs/core/src/zsh_exec_bridge/mod.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index a4f5568a7..4f815078c 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1468,6 +1468,7 @@ dependencies = [ "anyhow", "codex-apply-patch", "codex-linux-sandbox", + "codex-shell-escalation", "codex-utils-home-dir", "dotenvy", "tempfile", @@ -1713,6 +1714,7 @@ dependencies = [ "codex-rmcp-client", "codex-secrets", "codex-shell-command", + "codex-shell-escalation", "codex-skills", "codex-state", "codex-utils-absolute-path", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index d61ae1ee2..baed07814 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -113,6 +113,7 @@ codex-responses-api-proxy = { path = "responses-api-proxy" } codex-rmcp-client = { path = "rmcp-client" } codex-secrets = { path = "secrets" } codex-shell-command = { path = "shell-command" } +codex-shell-escalation = { path = "shell-escalation" } codex-skills = { path = "skills" } codex-state = { path = "state" } codex-stdio-to-uds = { path = "stdio-to-uds" } diff --git a/codex-rs/app-server/src/main.rs b/codex-rs/app-server/src/main.rs index c56c3c8f9..5c4e5eacc 100644 --- a/codex-rs/app-server/src/main.rs +++ b/codex-rs/app-server/src/main.rs @@ -24,11 +24,6 @@ struct AppServerArgs { fn main() -> anyhow::Result<()> { arg0_dispatch_or_else(|codex_linux_sandbox_exe| async move { - // Run wrapper mode only after arg0 dispatch so `codex-linux-sandbox` - // invocations don't get misclassified as zsh exec-wrapper calls. - if codex_core::maybe_run_zsh_exec_wrapper_mode()? { - return Ok(()); - } let args = AppServerArgs::parse(); let managed_config_path = managed_config_path_from_debug_env(); let loader_overrides = LoaderOverrides { diff --git a/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs b/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs index aee9f153d..340a5e156 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs @@ -35,7 +35,6 @@ use core_test_support::responses; use core_test_support::skip_if_no_network; use pretty_assertions::assert_eq; use std::collections::BTreeMap; -use std::os::unix::fs::PermissionsExt; use std::path::Path; use tempfile::TempDir; use tokio::time::timeout; @@ -444,32 +443,17 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2() return Ok(()); } eprintln!("using zsh path for zsh-fork test: {}", zsh_path.display()); - let zsh_path_for_config = { - // App-server config accepts only a zsh path, not extra argv. Use a - // wrapper so this test can force `-df` and downgrade `-lc` to `-c` - // to avoid rc/login-shell startup noise. - let path = workspace.join("zsh-no-rc"); - std::fs::write( - &path, - format!( - r#"#!/bin/sh -if [ "$1" = "-lc" ]; then - shift - set -- -c "$@" -fi -exec "{}" -df "$@" -"#, - zsh_path.display() - ), - )?; - let mut permissions = std::fs::metadata(&path)?.permissions(); - permissions.set_mode(0o755); - std::fs::set_permissions(&path, permissions)?; - path - }; - + let first_file = workspace.join("first.txt"); + let second_file = workspace.join("second.txt"); + std::fs::write(&first_file, "one")?; + std::fs::write(&second_file, "two")?; + let shell_command = format!( + "/bin/rm {} && /bin/rm {}", + first_file.display(), + second_file.display() + ); let tool_call_arguments = serde_json::to_string(&serde_json::json!({ - "command": "/usr/bin/true && /usr/bin/true", + "command": shell_command, "workdir": serde_json::Value::Null, "timeout_ms": 5000 }))?; @@ -495,13 +479,13 @@ exec "{}" -df "$@" create_config_toml( &codex_home, &server.uri(), - "on-request", + "untrusted", &BTreeMap::from([ (Feature::ShellZshFork, true), (Feature::UnifiedExec, false), (Feature::ShellSnapshot, false), ]), - &zsh_path_for_config, + &zsh_path, )?; let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace).await?; @@ -525,21 +509,17 @@ exec "{}" -df "$@" .send_turn_start_request(TurnStartParams { thread_id: thread.id.clone(), input: vec![V2UserInput::Text { - text: "run true true".to_string(), + text: "remove both files".to_string(), text_elements: Vec::new(), }], cwd: Some(workspace.clone()), - approval_policy: Some(codex_app_server_protocol::AskForApproval::OnRequest), - sandbox_policy: Some(if cfg!(target_os = "linux") { - // The zsh exec-bridge wrapper uses a Unix socket back to the parent - // process. Linux restricted sandbox seccomp denies connect(2), so use - // full access here; this test is validating zsh approval/decline - // behavior, not Linux sandboxing. - codex_app_server_protocol::SandboxPolicy::DangerFullAccess - } else { - codex_app_server_protocol::SandboxPolicy::ReadOnly { - access: codex_app_server_protocol::ReadOnlyAccess::FullAccess, - } + approval_policy: Some(codex_app_server_protocol::AskForApproval::UnlessTrusted), + sandbox_policy: Some(codex_app_server_protocol::SandboxPolicy::WorkspaceWrite { + writable_roots: vec![workspace.clone().try_into()?], + read_only_access: codex_app_server_protocol::ReadOnlyAccess::FullAccess, + network_access: false, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, }), model: Some("mock-model".to_string()), effort: Some(codex_protocol::openai_models::ReasoningEffort::Medium), @@ -554,7 +534,8 @@ exec "{}" -df "$@" .await??; let TurnStartResponse { turn } = to_response::(turn_resp)?; - let mut approval_ids = Vec::new(); + let mut approved_subcommand_strings = Vec::new(); + let mut approved_subcommand_ids = Vec::new(); let mut saw_parent_approval = false; let target_decisions = [ CommandExecutionApprovalDecision::Accept, @@ -573,38 +554,45 @@ exec "{}" -df "$@" }; assert_eq!(params.item_id, "call-zsh-fork-subcommand-decline"); assert_eq!(params.thread_id, thread.id); - let is_target_subcommand = params.command.as_deref() == Some("/usr/bin/true"); + let approval_command = params + .command + .as_deref() + .expect("approval command should be present"); + let is_target_subcommand = (approval_command.starts_with("/bin/rm ") + || approval_command.starts_with("/usr/bin/rm ")) + && (approval_command.contains(&first_file.display().to_string()) + || approval_command.contains(&second_file.display().to_string())); if is_target_subcommand { - approval_ids.push( + assert!( + approval_command.contains(&first_file.display().to_string()) + || approval_command.contains(&second_file.display().to_string()), + "expected zsh subcommand approval for one of the rm commands, got: {approval_command}" + ); + approved_subcommand_ids.push( params .approval_id .clone() .expect("approval_id must be present for zsh subcommand approvals"), ); + approved_subcommand_strings.push(approval_command.to_string()); } + let is_parent_approval = approval_command.contains(&zsh_path.display().to_string()) + && approval_command.contains(&shell_command); let decision = if is_target_subcommand { let decision = target_decisions[target_decision_index].clone(); target_decision_index += 1; decision - } else { - let command = params - .command - .as_deref() - .expect("approval command should be present"); + } else if is_parent_approval { assert!( !saw_parent_approval, - "unexpected extra non-target approval: {command}" - ); - assert!( - command.contains("zsh-no-rc"), - "expected parent zsh wrapper approval, got: {command}" - ); - assert!( - command.contains("/usr/bin/true && /usr/bin/true"), - "expected tool command in parent approval, got: {command}" + "unexpected extra non-target approval: {approval_command}" ); saw_parent_approval = true; CommandExecutionApprovalDecision::Accept + } else { + // Login shells may run startup helpers (for example path_helper on macOS) + // before the parent shell command or target subcommands are reached. + CommandExecutionApprovalDecision::Accept }; mcp.send_response( request_id, @@ -613,8 +601,15 @@ exec "{}" -df "$@" .await?; } - assert_eq!(approval_ids.len(), 2); - assert_ne!(approval_ids[0], approval_ids[1]); + assert!( + saw_parent_approval, + "expected parent shell approval request" + ); + assert_eq!(approved_subcommand_ids.len(), 2); + assert_ne!(approved_subcommand_ids[0], approved_subcommand_ids[1]); + assert_eq!(approved_subcommand_strings.len(), 2); + assert!(approved_subcommand_strings[0].contains(&first_file.display().to_string())); + assert!(approved_subcommand_strings[1].contains(&second_file.display().to_string())); let parent_completed_command_execution = timeout(DEFAULT_READ_TIMEOUT, async { loop { let completed_notif = mcp diff --git a/codex-rs/arg0/Cargo.toml b/codex-rs/arg0/Cargo.toml index c5d968132..abe7277d9 100644 --- a/codex-rs/arg0/Cargo.toml +++ b/codex-rs/arg0/Cargo.toml @@ -15,6 +15,7 @@ workspace = true anyhow = { workspace = true } codex-apply-patch = { workspace = true } codex-linux-sandbox = { workspace = true } +codex-shell-escalation = { workspace = true } codex-utils-home-dir = { workspace = true } dotenvy = { workspace = true } tempfile = { workspace = true } diff --git a/codex-rs/arg0/src/lib.rs b/codex-rs/arg0/src/lib.rs index 3ac7e017f..fca681d5e 100644 --- a/codex-rs/arg0/src/lib.rs +++ b/codex-rs/arg0/src/lib.rs @@ -12,6 +12,8 @@ use tempfile::TempDir; const LINUX_SANDBOX_ARG0: &str = "codex-linux-sandbox"; const APPLY_PATCH_ARG0: &str = "apply_patch"; const MISSPELLED_APPLY_PATCH_ARG0: &str = "applypatch"; +#[cfg(unix)] +const EXECVE_WRAPPER_ARG0: &str = "codex-execve-wrapper"; const LOCK_FILENAME: &str = ".lock"; const TOKIO_WORKER_STACK_SIZE_BYTES: usize = 16 * 1024 * 1024; @@ -39,6 +41,32 @@ pub fn arg0_dispatch() -> Option { .and_then(|s| s.to_str()) .unwrap_or(""); + #[cfg(unix)] + if exe_name == EXECVE_WRAPPER_ARG0 { + let mut args = std::env::args(); + let _ = args.next(); + let file = match args.next() { + Some(file) => file, + None => std::process::exit(1), + }; + let argv = args.collect::>(); + + let runtime = match tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + { + Ok(runtime) => runtime, + Err(_) => std::process::exit(1), + }; + let exit_code = runtime.block_on( + codex_shell_escalation::run_shell_escalation_execve_wrapper(file, argv), + ); + match exit_code { + Ok(exit_code) => std::process::exit(exit_code), + Err(_) => std::process::exit(1), + } + } + if exe_name == LINUX_SANDBOX_ARG0 { // Safety: [`run_main`] never returns. codex_linux_sandbox::run_main(); @@ -227,6 +255,8 @@ pub fn prepend_path_entry_for_codex_aliases() -> std::io::Result &'static str { fn main() -> anyhow::Result<()> { arg0_dispatch_or_else(|codex_linux_sandbox_exe| async move { - // Run wrapper mode only after arg0 dispatch so `codex-linux-sandbox` - // invocations don't get misclassified as zsh exec-wrapper calls. - if codex_core::maybe_run_zsh_exec_wrapper_mode()? { - return Ok(()); - } cli_main(codex_linux_sandbox_exe).await?; Ok(()) }) diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 5425b819d..5d132d58b 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -137,6 +137,9 @@ windows-sys = { version = "0.52", features = [ [target.'cfg(any(target_os = "freebsd", target_os = "openbsd"))'.dependencies] keyring = { workspace = true, features = ["sync-secret-service"] } +[target.'cfg(unix)'.dependencies] +codex-shell-escalation = { workspace = true } + [dev-dependencies] assert_cmd = { workspace = true } assert_matches = { workspace = true } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index ef8ea368d..dd4379459 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -257,7 +257,6 @@ use crate::turn_diff_tracker::TurnDiffTracker; use crate::unified_exec::UnifiedExecProcessManager; use crate::util::backoff; use crate::windows_sandbox::WindowsSandboxLevelExt; -use crate::zsh_exec_bridge::ZshExecBridge; use codex_async_utils::OrCancelExt; use codex_otel::OtelManager; use codex_otel::TelemetryAuthMode; @@ -1234,7 +1233,8 @@ impl Session { "zsh fork feature enabled, but `zsh_path` is not configured; set `zsh_path` in config.toml" ) })?; - shell::get_shell(shell::ShellType::Zsh, Some(zsh_path)).ok_or_else(|| { + 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 zsh_path `{}` is not usable; set `zsh_path` to a valid zsh executable", zsh_path.display() @@ -1313,12 +1313,6 @@ impl Session { (None, None) }; - let zsh_exec_bridge = - ZshExecBridge::new(config.zsh_path.clone(), config.codex_home.clone()); - zsh_exec_bridge - .initialize_for_session(&conversation_id.to_string()) - .await; - let services = SessionServices { // Initialize the MCP connection manager with an uninitialized // instance. It will be replaced with one created via @@ -1334,7 +1328,7 @@ impl Session { unified_exec_manager: UnifiedExecProcessManager::new( config.background_terminal_max_timeout, ), - zsh_exec_bridge, + shell_zsh_path: config.zsh_path.clone(), analytics_events_client: AnalyticsEventsClient::new( Arc::clone(&config), Arc::clone(&auth_manager), @@ -4495,7 +4489,6 @@ mod handlers { .unified_exec_manager .terminate_all_processes() .await; - sess.services.zsh_exec_bridge.shutdown().await; info!("Shutting down Codex instance"); let history = sess.clone_history().await; let turn_count = history @@ -8213,7 +8206,7 @@ mod tests { unified_exec_manager: UnifiedExecProcessManager::new( config.background_terminal_max_timeout, ), - zsh_exec_bridge: ZshExecBridge::default(), + shell_zsh_path: None, analytics_events_client: AnalyticsEventsClient::new( Arc::clone(&config), Arc::clone(&auth_manager), @@ -8368,7 +8361,7 @@ mod tests { unified_exec_manager: UnifiedExecProcessManager::new( config.background_terminal_max_timeout, ), - zsh_exec_bridge: ZshExecBridge::default(), + shell_zsh_path: None, analytics_events_client: AnalyticsEventsClient::new( Arc::clone(&config), Arc::clone(&auth_manager), diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index e40905e31..a9ca37e70 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -109,7 +109,6 @@ pub mod terminal; mod tools; pub mod turn_diff_tracker; mod turn_metadata; -mod zsh_exec_bridge; pub use rollout::ARCHIVED_SESSIONS_SUBDIR; pub use rollout::INTERACTIVE_SESSION_SOURCES; pub use rollout::RolloutRecorder; @@ -144,7 +143,17 @@ pub(crate) use codex_shell_command::is_safe_command; pub(crate) use codex_shell_command::parse_command; pub(crate) use codex_shell_command::powershell; +pub use client::ModelClient; +pub use client::ModelClientSession; +pub use client::ResponsesWebsocketVersion; pub use client::X_CODEX_TURN_METADATA_HEADER; +pub use client::ws_version_from_features; +pub use client_common::Prompt; +pub use client_common::REVIEW_PROMPT; +pub use client_common::ResponseEvent; +pub use client_common::ResponseStream; +pub use compact::content_items_to_text; +pub use event_mapping::parse_turn_item; pub use exec_policy::ExecPolicyError; pub use exec_policy::check_execpolicy_for_warnings; pub use exec_policy::format_exec_policy_error_with_source; @@ -153,18 +162,6 @@ pub use file_watcher::FileWatcherEvent; pub use safety::get_platform_sandbox; pub use tools::spec::parse_tool_input_schema; pub use turn_metadata::build_turn_metadata_header; -pub use zsh_exec_bridge::maybe_run_zsh_exec_wrapper_mode; - -pub use client::ModelClient; -pub use client::ModelClientSession; -pub use client::ResponsesWebsocketVersion; -pub use client::ws_version_from_features; -pub use client_common::Prompt; -pub use client_common::REVIEW_PROMPT; -pub use client_common::ResponseEvent; -pub use client_common::ResponseStream; -pub use compact::content_items_to_text; -pub use event_mapping::parse_turn_item; pub mod compact; pub mod memory_trace; pub mod otel_init; diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index 061b709b6..61f9c82b6 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -347,19 +347,13 @@ impl SandboxManager { SandboxType::MacosSeatbelt => { let mut seatbelt_env = HashMap::new(); seatbelt_env.insert(CODEX_SANDBOX_ENV_VAR.to_string(), "seatbelt".to_string()); - let zsh_exec_bridge_wrapper_socket = env - .get(crate::zsh_exec_bridge::ZSH_EXEC_BRIDGE_WRAPPER_SOCKET_ENV_VAR) - .map(PathBuf::from); - let zsh_exec_bridge_allowed_unix_sockets = zsh_exec_bridge_wrapper_socket - .as_ref() - .map_or_else(Vec::new, |path| vec![path.clone()]); let mut args = create_seatbelt_command_args( command.clone(), &effective_policy, sandbox_policy_cwd, enforce_managed_network, network, - &zsh_exec_bridge_allowed_unix_sockets, + &[], ); let mut full_command = Vec::with_capacity(1 + args.len()); full_command.push(MACOS_PATH_TO_SEATBELT_EXECUTABLE.to_string()); diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index fe82ec84c..14ac226c5 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -15,9 +15,9 @@ use crate::state_db::StateDbHandle; use crate::tools::network_approval::NetworkApprovalService; use crate::tools::sandboxing::ApprovalStore; use crate::unified_exec::UnifiedExecProcessManager; -use crate::zsh_exec_bridge::ZshExecBridge; use codex_hooks::Hooks; use codex_otel::OtelManager; +use std::path::PathBuf; use tokio::sync::Mutex; use tokio::sync::RwLock; use tokio::sync::watch; @@ -27,7 +27,8 @@ pub(crate) struct SessionServices { pub(crate) mcp_connection_manager: Arc>, pub(crate) mcp_startup_cancellation_token: Mutex, pub(crate) unified_exec_manager: UnifiedExecProcessManager, - pub(crate) zsh_exec_bridge: ZshExecBridge, + #[cfg_attr(not(unix), allow(dead_code))] + pub(crate) shell_zsh_path: Option, pub(crate) analytics_events_client: AnalyticsEventsClient, pub(crate) hooks: Hooks, pub(crate) rollout: Mutex>, diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 84329b81e..79db57e2e 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -28,12 +28,22 @@ use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; use crate::tools::runtimes::shell::ShellRequest; use crate::tools::runtimes::shell::ShellRuntime; +use crate::tools::runtimes::shell::ShellRuntimeBackend; use crate::tools::sandboxing::ToolCtx; +use crate::tools::spec::ShellCommandBackendConfig; use codex_protocol::models::AdditionalPermissions; pub struct ShellHandler; -pub struct ShellCommandHandler; +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum ShellCommandBackend { + Classic, + ZshFork, +} + +pub struct ShellCommandHandler { + backend: ShellCommandBackend, +} struct RunExecLikeArgs { tool_name: String, @@ -45,6 +55,7 @@ struct RunExecLikeArgs { tracker: crate::tools::context::SharedTurnDiffTracker, call_id: String, freeform: bool, + shell_runtime_backend: ShellRuntimeBackend, } impl ShellHandler { @@ -68,6 +79,13 @@ impl ShellHandler { } impl ShellCommandHandler { + fn shell_runtime_backend(&self) -> ShellRuntimeBackend { + match self.backend { + ShellCommandBackend::Classic => ShellRuntimeBackend::ShellCommandClassic, + ShellCommandBackend::ZshFork => ShellRuntimeBackend::ShellCommandZshFork, + } + } + fn resolve_use_login_shell( login: Option, allow_login_shell: bool, @@ -110,6 +128,16 @@ impl ShellCommandHandler { } } +impl From for ShellCommandHandler { + fn from(config: ShellCommandBackendConfig) -> Self { + let backend = match config { + ShellCommandBackendConfig::Classic => ShellCommandBackend::Classic, + ShellCommandBackendConfig::ZshFork => ShellCommandBackend::ZshFork, + }; + Self { backend } + } +} + #[async_trait] impl ToolHandler for ShellHandler { fn kind(&self) -> ToolKind { @@ -161,6 +189,7 @@ impl ToolHandler for ShellHandler { tracker, call_id, freeform: false, + shell_runtime_backend: ShellRuntimeBackend::Generic, }) .await } @@ -177,6 +206,7 @@ impl ToolHandler for ShellHandler { tracker, call_id, freeform: false, + shell_runtime_backend: ShellRuntimeBackend::Generic, }) .await } @@ -260,6 +290,7 @@ impl ToolHandler for ShellCommandHandler { tracker, call_id, freeform: true, + shell_runtime_backend: self.shell_runtime_backend(), }) .await } @@ -277,6 +308,7 @@ impl ShellHandler { tracker, call_id, freeform, + shell_runtime_backend, } = args; let mut exec_params = exec_params; @@ -368,7 +400,15 @@ impl ShellHandler { exec_approval_requirement, }; let mut orchestrator = ToolOrchestrator::new(); - let mut runtime = ShellRuntime::new(); + let mut runtime = { + use ShellRuntimeBackend::*; + match shell_runtime_backend { + Generic => ShellRuntime::new(), + backend @ (ShellCommandClassic | ShellCommandZshFork) => { + ShellRuntime::for_shell_command(backend) + } + } + }; let tool_ctx = ToolCtx { session: session.clone(), turn: turn.clone(), diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index 300e8f437..0a5353755 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -4,6 +4,9 @@ Runtime: shell Executes shell requests under the orchestrator: asks for approval when needed, builds a CommandSpec, and runs it under the current SandboxAttempt. */ +#[cfg(unix)] +mod unix_escalation; + use crate::command_canonicalization::canonicalize_command_for_approval; use crate::exec::ExecToolCallOutput; use crate::features::Feature; @@ -27,11 +30,11 @@ use crate::tools::sandboxing::ToolError; use crate::tools::sandboxing::ToolRuntime; use crate::tools::sandboxing::sandbox_override_for_first_attempt; use crate::tools::sandboxing::with_cached_approval; -use crate::zsh_exec_bridge::ZSH_EXEC_BRIDGE_WRAPPER_SOCKET_ENV_VAR; use codex_network_proxy::NetworkProxy; use codex_protocol::models::AdditionalPermissions; use codex_protocol::protocol::ReviewDecision; use futures::future::BoxFuture; +use std::collections::HashMap; use std::path::PathBuf; #[derive(Clone, Debug)] @@ -39,8 +42,8 @@ pub struct ShellRequest { pub command: Vec, pub cwd: PathBuf, pub timeout_ms: Option, - pub env: std::collections::HashMap, - pub explicit_env_overrides: std::collections::HashMap, + pub env: HashMap, + pub explicit_env_overrides: HashMap, pub network: Option, pub sandbox_permissions: SandboxPermissions, pub additional_permissions: Option, @@ -48,8 +51,38 @@ pub struct ShellRequest { pub exec_approval_requirement: ExecApprovalRequirement, } +/// Selects `ShellRuntime` behavior for different callers. +/// +/// Note: `Generic` is not the same as `ShellCommandClassic`. +/// `Generic` means "no `shell_command`-specific backend behavior" (used by the +/// generic `shell` tool path). The `ShellCommand*` variants are only for the +/// `shell_command` tool family. +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +pub(crate) enum ShellRuntimeBackend { + /// Tool-agnostic/default runtime path. + /// + /// Uses the normal `ShellRuntime` execution flow without enabling any + /// `shell_command`-specific backend selection. + #[default] + Generic, + /// Legacy backend for the `shell_command` tool. + /// + /// Keeps `shell_command` on the standard shell runtime flow without the + /// zsh-fork shell-escalation adapter. + ShellCommandClassic, + /// zsh-fork backend for the `shell_command` tool. + /// + /// On Unix, attempts to run via the zsh-fork + `codex-shell-escalation` + /// adapter, with fallback to the standard shell runtime flow if + /// prerequisites are not met. + ShellCommandZshFork, +} + #[derive(Default)] -pub struct ShellRuntime; +pub struct ShellRuntime { + #[cfg_attr(not(unix), allow(dead_code))] + backend: ShellRuntimeBackend, +} #[derive(serde::Serialize, Clone, Debug, Eq, PartialEq, Hash)] pub(crate) struct ApprovalKey { @@ -61,7 +94,13 @@ pub(crate) struct ApprovalKey { impl ShellRuntime { pub fn new() -> Self { - Self + Self { + backend: ShellRuntimeBackend::Generic, + } + } + + pub(crate) fn for_shell_command(backend: ShellRuntimeBackend) -> Self { + Self { backend } } fn stdout_stream(ctx: &ToolCtx) -> Option { @@ -159,10 +198,9 @@ impl ToolRuntime for ShellRuntime { attempt: &SandboxAttempt<'_>, ctx: &ToolCtx, ) -> Result { - let base_command = &req.command; let session_shell = ctx.session.user_shell(); let command = maybe_wrap_shell_lc_with_snapshot( - base_command, + &req.command, session_shell.as_ref(), &req.cwd, &req.explicit_env_overrides, @@ -175,35 +213,16 @@ impl ToolRuntime for ShellRuntime { command }; - if ctx.session.features().enabled(Feature::ShellZshFork) { - let wrapper_socket_path = ctx - .session - .services - .zsh_exec_bridge - .next_wrapper_socket_path(); - let mut zsh_fork_env = req.env.clone(); - zsh_fork_env.insert( - ZSH_EXEC_BRIDGE_WRAPPER_SOCKET_ENV_VAR.to_string(), - wrapper_socket_path.to_string_lossy().to_string(), - ); - let spec = build_command_spec( - &command, - &req.cwd, - &zsh_fork_env, - req.timeout_ms.into(), - req.sandbox_permissions, - req.additional_permissions.clone(), - req.justification.clone(), - )?; - let env = attempt - .env_for(spec, req.network.as_ref()) - .map_err(|err| ToolError::Codex(err.into()))?; - return ctx - .session - .services - .zsh_exec_bridge - .execute_shell_request(&env, &ctx.session, &ctx.turn, &ctx.call_id) - .await; + #[cfg(unix)] + if self.backend == ShellRuntimeBackend::ShellCommandZshFork { + match unix_escalation::try_run_zsh_fork(req, attempt, ctx, &command).await? { + Some(out) => return Ok(out), + None => { + tracing::warn!( + "ZshFork backend specified, but conditions for using it were not met, falling back to normal execution", + ); + } + } } let spec = build_command_spec( diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs new file mode 100644 index 000000000..2fd7f49e2 --- /dev/null +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -0,0 +1,463 @@ +use super::ShellRequest; +use crate::error::CodexErr; +use crate::error::SandboxErr; +use crate::exec::ExecExpiration; +use crate::exec::ExecToolCallOutput; +use crate::exec::SandboxType; +use crate::exec::is_likely_sandbox_denied; +use crate::features::Feature; +use crate::sandboxing::SandboxPermissions; +use crate::shell::ShellType; +use crate::tools::runtimes::build_command_spec; +use crate::tools::sandboxing::SandboxAttempt; +use crate::tools::sandboxing::ToolCtx; +use crate::tools::sandboxing::ToolError; +use codex_execpolicy::Decision; +use codex_execpolicy::Policy; +use codex_execpolicy::RuleMatch; +use codex_protocol::config_types::WindowsSandboxLevel; +use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::NetworkPolicyRuleAction; +use codex_protocol::protocol::RejectConfig; +use codex_protocol::protocol::ReviewDecision; +use codex_protocol::protocol::SandboxPolicy; +use codex_shell_command::bash::parse_shell_lc_plain_commands; +use codex_shell_command::bash::parse_shell_lc_single_command_prefix; +use codex_shell_escalation::EscalateAction; +use codex_shell_escalation::ExecParams; +use codex_shell_escalation::ExecResult; +use codex_shell_escalation::ShellActionProvider; +use codex_shell_escalation::ShellCommandExecutor; +use codex_shell_escalation::ShellPolicyFactory; +use codex_shell_escalation::Stopwatch; +use codex_shell_escalation::run_escalate_server; +use std::collections::HashMap; +use std::path::Path; +use std::path::PathBuf; +use std::sync::Arc; +use std::time::Duration; +use tokio::sync::RwLock; +use tokio_util::sync::CancellationToken; +use uuid::Uuid; + +pub(super) async fn try_run_zsh_fork( + req: &ShellRequest, + attempt: &SandboxAttempt<'_>, + ctx: &ToolCtx, + command: &[String], +) -> Result, ToolError> { + let Some(shell_zsh_path) = ctx.session.services.shell_zsh_path.as_ref() else { + tracing::warn!("ZshFork backend specified, but shell_zsh_path is not configured."); + return Ok(None); + }; + if !ctx.session.features().enabled(Feature::ShellZshFork) { + tracing::warn!("ZshFork backend specified, but ShellZshFork feature is not enabled."); + return Ok(None); + } + if !matches!(ctx.session.user_shell().shell_type, ShellType::Zsh) { + tracing::warn!("ZshFork backend specified, but user shell is not Zsh."); + return Ok(None); + } + + let spec = build_command_spec( + command, + &req.cwd, + &req.env, + req.timeout_ms.into(), + req.sandbox_permissions, + req.additional_permissions.clone(), + req.justification.clone(), + )?; + let sandbox_exec_request = attempt + .env_for(spec, req.network.as_ref()) + .map_err(|err| ToolError::Codex(err.into()))?; + // Keep env/network/sandbox metadata from `attempt.env_for()`, but build the + // script from the original shell argv. `attempt.env_for()` may wrap the + // command with `sandbox-exec` on macOS, and passing those wrapper flags + // (`-p`, `-D...`) through zsh breaks the zsh-fork path before subcommand + // approval runs. + let crate::sandboxing::ExecRequest { + command: _sandbox_command, + cwd: _sandbox_cwd, + env: sandbox_env, + network: sandbox_network, + expiration: _sandbox_expiration, + sandbox, + windows_sandbox_level, + sandbox_permissions, + sandbox_policy, + justification, + arg0, + } = sandbox_exec_request; + let ParsedShellCommand { script, login } = extract_shell_script(command)?; + let effective_timeout = Duration::from_millis( + req.timeout_ms + .unwrap_or(crate::exec::DEFAULT_EXEC_COMMAND_TIMEOUT_MS), + ); + let exec_policy = Arc::new(RwLock::new( + ctx.session.services.exec_policy.current().as_ref().clone(), + )); + let command_executor = CoreShellCommandExecutor { + sandbox_policy, + sandbox, + env: sandbox_env, + network: sandbox_network, + windows_sandbox_level, + sandbox_permissions, + justification, + arg0, + }; + let exec_result = run_escalate_server( + ExecParams { + command: script, + workdir: req.cwd.to_string_lossy().to_string(), + timeout_ms: Some(effective_timeout.as_millis() as u64), + login: Some(login), + }, + shell_zsh_path.clone(), + shell_execve_wrapper().map_err(|err| ToolError::Rejected(format!("{err}")))?, + exec_policy.clone(), + ShellPolicyFactory::new(CoreShellActionProvider { + policy: Arc::clone(&exec_policy), + session: Arc::clone(&ctx.session), + turn: Arc::clone(&ctx.turn), + call_id: ctx.call_id.clone(), + approval_policy: ctx.turn.approval_policy.value(), + sandbox_policy: attempt.policy.clone(), + sandbox_permissions: req.sandbox_permissions, + }), + effective_timeout, + &command_executor, + ) + .await + .map_err(|err| ToolError::Rejected(err.to_string()))?; + + map_exec_result(attempt.sandbox, exec_result).map(Some) +} + +struct CoreShellActionProvider { + policy: Arc>, + session: Arc, + turn: Arc, + call_id: String, + approval_policy: AskForApproval, + sandbox_policy: SandboxPolicy, + sandbox_permissions: SandboxPermissions, +} + +impl CoreShellActionProvider { + fn decision_driven_by_policy(matched_rules: &[RuleMatch], decision: Decision) -> bool { + matched_rules.iter().any(|rule_match| { + !matches!(rule_match, RuleMatch::HeuristicsRuleMatch { .. }) + && rule_match.decision() == decision + }) + } + + async fn prompt( + &self, + command: &[String], + workdir: &Path, + stopwatch: &Stopwatch, + ) -> anyhow::Result { + let command = command.to_vec(); + let workdir = workdir.to_path_buf(); + let session = self.session.clone(); + let turn = self.turn.clone(); + let call_id = self.call_id.clone(); + let approval_id = Some(Uuid::new_v4().to_string()); + Ok(stopwatch + .pause_for(async move { + session + .request_command_approval( + &turn, + call_id, + approval_id, + command, + workdir, + None, + None, + None, + None, + ) + .await + }) + .await) + } +} + +#[async_trait::async_trait] +impl ShellActionProvider for CoreShellActionProvider { + async fn determine_action( + &self, + file: &Path, + argv: &[String], + workdir: &Path, + stopwatch: &Stopwatch, + ) -> anyhow::Result { + let command = std::iter::once(file.to_string_lossy().to_string()) + .chain(argv.iter().cloned()) + .collect::>(); + let (commands, used_complex_parsing) = + if let Some(commands) = parse_shell_lc_plain_commands(&command) { + (commands, false) + } else if let Some(single_command) = parse_shell_lc_single_command_prefix(&command) { + (vec![single_command], true) + } else { + (vec![command.clone()], false) + }; + + let fallback = |cmd: &[String]| { + crate::exec_policy::render_decision_for_unmatched_command( + self.approval_policy, + &self.sandbox_policy, + cmd, + self.sandbox_permissions, + used_complex_parsing, + ) + }; + let evaluation = { + let policy = self.policy.read().await; + policy.check_multiple(commands.iter(), &fallback) + }; + // When true, means the Evaluation was due to *.rules, not the + // fallback function. + let decision_driven_by_policy = + Self::decision_driven_by_policy(&evaluation.matched_rules, evaluation.decision); + let needs_escalation = + self.sandbox_permissions.requires_escalated_permissions() || decision_driven_by_policy; + + Ok(match evaluation.decision { + Decision::Forbidden => EscalateAction::Deny { + reason: Some("Execution forbidden by policy".to_string()), + }, + Decision::Prompt => { + if matches!( + self.approval_policy, + AskForApproval::Never + | AskForApproval::Reject(RejectConfig { rules: true, .. }) + ) { + EscalateAction::Deny { + reason: Some("Execution forbidden by policy".to_string()), + } + } else { + match self.prompt(&command, workdir, stopwatch).await? { + ReviewDecision::Approved + | ReviewDecision::ApprovedExecpolicyAmendment { .. } + | ReviewDecision::ApprovedForSession => { + if needs_escalation { + EscalateAction::Escalate + } else { + EscalateAction::Run + } + } + ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment, + } => match network_policy_amendment.action { + NetworkPolicyRuleAction::Allow => { + if needs_escalation { + EscalateAction::Escalate + } else { + EscalateAction::Run + } + } + NetworkPolicyRuleAction::Deny => EscalateAction::Deny { + reason: Some("User denied execution".to_string()), + }, + }, + ReviewDecision::Denied => EscalateAction::Deny { + reason: Some("User denied execution".to_string()), + }, + ReviewDecision::Abort => EscalateAction::Deny { + reason: Some("User cancelled execution".to_string()), + }, + } + } + } + Decision::Allow => EscalateAction::Escalate, + }) + } +} + +struct CoreShellCommandExecutor { + sandbox_policy: SandboxPolicy, + sandbox: SandboxType, + env: HashMap, + network: Option, + windows_sandbox_level: WindowsSandboxLevel, + sandbox_permissions: SandboxPermissions, + justification: Option, + arg0: Option, +} + +#[async_trait::async_trait] +impl ShellCommandExecutor for CoreShellCommandExecutor { + async fn run( + &self, + command: Vec, + cwd: PathBuf, + env: HashMap, + cancel_rx: CancellationToken, + ) -> anyhow::Result { + let mut exec_env = self.env.clone(); + for var in ["CODEX_ESCALATE_SOCKET", "EXEC_WRAPPER", "BASH_EXEC_WRAPPER"] { + if let Some(value) = env.get(var) { + exec_env.insert(var.to_string(), value.clone()); + } + } + + let result = crate::sandboxing::execute_env( + crate::sandboxing::ExecRequest { + command, + cwd, + env: exec_env, + network: self.network.clone(), + expiration: ExecExpiration::Cancellation(cancel_rx), + sandbox: self.sandbox, + windows_sandbox_level: self.windows_sandbox_level, + sandbox_permissions: self.sandbox_permissions, + sandbox_policy: self.sandbox_policy.clone(), + justification: self.justification.clone(), + arg0: self.arg0.clone(), + }, + None, + ) + .await?; + + Ok(ExecResult { + exit_code: result.exit_code, + stdout: result.stdout.text, + stderr: result.stderr.text, + output: result.aggregated_output.text, + duration: result.duration, + timed_out: result.timed_out, + }) + } +} + +// TODO(mbolin): This should be passed down from codex-arg0 like codex_linux_sandbox_exe. +fn shell_execve_wrapper() -> anyhow::Result { + const EXECVE_WRAPPER: &str = "codex-execve-wrapper"; + + if let Some(path) = std::env::var_os("PATH") { + for dir in std::env::split_paths(&path) { + let candidate = dir.join(EXECVE_WRAPPER); + if candidate.is_file() { + return Ok(candidate); + } + } + } + + let exe = std::env::current_exe()?; + let sibling = exe + .parent() + .map(|parent| parent.join(EXECVE_WRAPPER)) + .ok_or_else(|| anyhow::anyhow!("failed to determine codex-execve-wrapper path"))?; + if sibling.is_file() { + return Ok(sibling); + } + + Err(anyhow::anyhow!( + "failed to locate {EXECVE_WRAPPER} in PATH or next to current executable ({})", + exe.display() + )) +} + +#[derive(Debug, Eq, PartialEq)] +struct ParsedShellCommand { + script: String, + login: bool, +} + +fn extract_shell_script(command: &[String]) -> Result { + match command { + [_, flag, script, ..] if flag == "-c" => Ok(ParsedShellCommand { + script: script.clone(), + login: false, + }), + [_, flag, script, ..] if flag == "-lc" => Ok(ParsedShellCommand { + script: script.clone(), + login: true, + }), + _ => Err(ToolError::Rejected( + "unexpected shell command format for zsh-fork execution".to_string(), + )), + } +} + +fn map_exec_result( + sandbox: SandboxType, + result: ExecResult, +) -> Result { + let output = ExecToolCallOutput { + exit_code: result.exit_code, + stdout: crate::exec::StreamOutput::new(result.stdout.clone()), + stderr: crate::exec::StreamOutput::new(result.stderr.clone()), + aggregated_output: crate::exec::StreamOutput::new(result.output.clone()), + duration: result.duration, + timed_out: result.timed_out, + }; + + if result.timed_out { + return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Timeout { + output: Box::new(output), + }))); + } + + if is_likely_sandbox_denied(sandbox, &output) { + return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { + output: Box::new(output), + network_policy_decision: None, + }))); + } + + Ok(output) +} + +#[cfg(test)] +mod tests { + use super::ParsedShellCommand; + use super::extract_shell_script; + use super::map_exec_result; + use crate::exec::SandboxType; + use codex_shell_escalation::ExecResult; + use pretty_assertions::assert_eq; + use std::time::Duration; + + #[test] + fn extract_shell_script_preserves_login_flag() { + assert_eq!( + extract_shell_script(&["/bin/zsh".into(), "-lc".into(), "echo hi".into()]).unwrap(), + ParsedShellCommand { + script: "echo hi".to_string(), + login: true, + } + ); + assert_eq!( + extract_shell_script(&["/bin/zsh".into(), "-c".into(), "echo hi".into()]).unwrap(), + ParsedShellCommand { + script: "echo hi".to_string(), + login: false, + } + ); + } + + #[test] + fn map_exec_result_preserves_stdout_and_stderr() { + let out = map_exec_result( + SandboxType::None, + ExecResult { + exit_code: 0, + stdout: "out".to_string(), + stderr: "err".to_string(), + output: "outerr".to_string(), + duration: Duration::from_millis(1), + timed_out: false, + }, + ) + .unwrap(); + + assert_eq!(out.stdout.text, "out"); + assert_eq!(out.stderr.text, "err"); + assert_eq!(out.aggregated_output.text, "outerr"); + } +} diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 0c36675f5..ea9ccb3ea 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -32,9 +32,16 @@ use std::collections::HashMap; const SEARCH_TOOL_BM25_DESCRIPTION_TEMPLATE: &str = include_str!("../../templates/search_tool/tool_description.md"); +#[derive(Debug, Clone, Copy, Eq, PartialEq)] +pub enum ShellCommandBackendConfig { + Classic, + ZshFork, +} + #[derive(Debug, Clone)] pub(crate) struct ToolsConfig { pub shell_type: ConfigShellToolType, + shell_command_backend: ShellCommandBackendConfig, pub allow_login_shell: bool, pub apply_patch_tool_type: Option, pub web_search_mode: Option, @@ -69,6 +76,12 @@ impl ToolsConfig { let include_collaboration_modes_tools = true; let include_search_tool = features.enabled(Feature::Apps); let request_permission_enabled = features.enabled(Feature::RequestPermissions); + let shell_command_backend = + if features.enabled(Feature::ShellTool) && features.enabled(Feature::ShellZshFork) { + ShellCommandBackendConfig::ZshFork + } else { + ShellCommandBackendConfig::Classic + }; let shell_type = if !features.enabled(Feature::ShellTool) { ConfigShellToolType::Disabled @@ -99,6 +112,7 @@ impl ToolsConfig { Self { shell_type, + shell_command_backend, allow_login_shell: true, apply_patch_tool_type, web_search_mode: *web_search_mode, @@ -1500,7 +1514,7 @@ pub(crate) fn build_specs( let view_image_handler = Arc::new(ViewImageHandler); let mcp_handler = Arc::new(McpHandler); let mcp_resource_handler = Arc::new(McpResourceHandler); - let shell_command_handler = Arc::new(ShellCommandHandler); + let shell_command_handler = Arc::new(ShellCommandHandler::from(config.shell_command_backend)); let request_user_input_handler = Arc::new(RequestUserInputHandler); let search_tool_handler = Arc::new(SearchToolBm25Handler); let js_repl_handler = Arc::new(JsReplHandler); @@ -2326,6 +2340,10 @@ mod tests { }); assert_eq!(tools_config.shell_type, ConfigShellToolType::ShellCommand); + assert_eq!( + tools_config.shell_command_backend, + ShellCommandBackendConfig::ZshFork + ); } #[test] diff --git a/codex-rs/core/src/zsh_exec_bridge/mod.rs b/codex-rs/core/src/zsh_exec_bridge/mod.rs deleted file mode 100644 index 4d8b51f68..000000000 --- a/codex-rs/core/src/zsh_exec_bridge/mod.rs +++ /dev/null @@ -1,570 +0,0 @@ -use crate::exec::ExecToolCallOutput; -use crate::tools::sandboxing::ToolError; -use std::path::PathBuf; -use tokio::sync::Mutex; -use uuid::Uuid; - -#[cfg(unix)] -use crate::error::CodexErr; -#[cfg(unix)] -use crate::error::SandboxErr; -#[cfg(unix)] -use crate::protocol::EventMsg; -#[cfg(unix)] -use crate::protocol::ExecCommandOutputDeltaEvent; -#[cfg(unix)] -use crate::protocol::ExecOutputStream; -#[cfg(unix)] -use crate::protocol::NetworkPolicyRuleAction; -#[cfg(unix)] -use crate::protocol::ReviewDecision; -#[cfg(unix)] -use anyhow::Context as _; -#[cfg(unix)] -use codex_protocol::approvals::ExecPolicyAmendment; -#[cfg(unix)] -use codex_utils_pty::process_group::kill_child_process_group; -#[cfg(unix)] -use serde::Deserialize; -#[cfg(unix)] -use serde::Serialize; -#[cfg(unix)] -use std::io::Read; -#[cfg(unix)] -use std::io::Write; -#[cfg(unix)] -use std::time::Instant; -#[cfg(unix)] -use tokio::io::AsyncReadExt; -#[cfg(unix)] -use tokio::net::UnixListener; -#[cfg(unix)] -use tokio::net::UnixStream; - -pub(crate) const ZSH_EXEC_BRIDGE_WRAPPER_SOCKET_ENV_VAR: &str = - "CODEX_ZSH_EXEC_BRIDGE_WRAPPER_SOCKET"; -pub(crate) const ZSH_EXEC_WRAPPER_MODE_ENV_VAR: &str = "CODEX_ZSH_EXEC_WRAPPER_MODE"; -#[cfg(unix)] -pub(crate) const EXEC_WRAPPER_ENV_VAR: &str = "EXEC_WRAPPER"; - -#[derive(Debug, Clone, PartialEq, Eq, Default)] -pub(crate) struct ZshExecBridgeSessionState { - pub(crate) initialized_session_id: Option, -} - -#[derive(Debug, Default)] -pub(crate) struct ZshExecBridge { - zsh_path: Option, - state: Mutex, -} - -#[cfg(unix)] -#[derive(Debug, Deserialize, Serialize)] -#[serde(tag = "type", rename_all = "snake_case")] -enum WrapperIpcRequest { - ExecRequest { - request_id: String, - file: String, - argv: Vec, - cwd: String, - }, -} - -#[cfg(unix)] -#[derive(Debug, Deserialize, Serialize)] -#[serde(tag = "type", rename_all = "snake_case")] -enum WrapperIpcResponse { - ExecResponse { - request_id: String, - action: WrapperExecAction, - reason: Option, - }, -} - -#[cfg(unix)] -#[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] -#[serde(rename_all = "snake_case")] -enum WrapperExecAction { - Run, - Deny, -} - -impl ZshExecBridge { - pub(crate) fn new(zsh_path: Option, _codex_home: PathBuf) -> Self { - Self { - zsh_path, - state: Mutex::new(ZshExecBridgeSessionState::default()), - } - } - - pub(crate) async fn initialize_for_session(&self, session_id: &str) { - let mut state = self.state.lock().await; - state.initialized_session_id = Some(session_id.to_string()); - } - - pub(crate) async fn shutdown(&self) { - let mut state = self.state.lock().await; - state.initialized_session_id = None; - } - - pub(crate) fn next_wrapper_socket_path(&self) -> PathBuf { - let socket_id = Uuid::new_v4().as_simple().to_string(); - let temp_dir = std::env::temp_dir(); - let canonical_temp_dir = temp_dir.canonicalize().unwrap_or(temp_dir); - canonical_temp_dir.join(format!("czs-{}.sock", &socket_id[..12])) - } - - #[cfg(not(unix))] - pub(crate) async fn execute_shell_request( - &self, - _req: &crate::sandboxing::ExecRequest, - _session: &crate::codex::Session, - _turn: &crate::codex::TurnContext, - _call_id: &str, - ) -> Result { - let _ = &self.zsh_path; - Err(ToolError::Rejected( - "shell_zsh_fork is only supported on unix".to_string(), - )) - } - - #[cfg(unix)] - pub(crate) async fn execute_shell_request( - &self, - req: &crate::sandboxing::ExecRequest, - session: &crate::codex::Session, - turn: &crate::codex::TurnContext, - call_id: &str, - ) -> Result { - let zsh_path = self.zsh_path.clone().ok_or_else(|| { - ToolError::Rejected( - "shell_zsh_fork enabled, but zsh_path is not configured".to_string(), - ) - })?; - - let command = req.command.clone(); - if command.is_empty() { - return Err(ToolError::Rejected("command args are empty".to_string())); - } - - let wrapper_socket_path = req - .env - .get(ZSH_EXEC_BRIDGE_WRAPPER_SOCKET_ENV_VAR) - .map(PathBuf::from) - .unwrap_or_else(|| self.next_wrapper_socket_path()); - - let listener = { - let _ = std::fs::remove_file(&wrapper_socket_path); - UnixListener::bind(&wrapper_socket_path).map_err(|err| { - ToolError::Rejected(format!( - "bind wrapper socket at {}: {err}", - wrapper_socket_path.display() - )) - })? - }; - - let wrapper_path = std::env::current_exe().map_err(|err| { - ToolError::Rejected(format!("resolve current executable path: {err}")) - })?; - - let mut cmd = tokio::process::Command::new(&command[0]); - #[cfg(unix)] - if let Some(arg0) = &req.arg0 { - cmd.arg0(arg0); - } - if command.len() > 1 { - cmd.args(&command[1..]); - } - cmd.current_dir(&req.cwd); - cmd.stdin(std::process::Stdio::null()); - cmd.stdout(std::process::Stdio::piped()); - cmd.stderr(std::process::Stdio::piped()); - cmd.kill_on_drop(true); - cmd.env_clear(); - cmd.envs(&req.env); - cmd.env( - ZSH_EXEC_BRIDGE_WRAPPER_SOCKET_ENV_VAR, - wrapper_socket_path.to_string_lossy().to_string(), - ); - cmd.env(EXEC_WRAPPER_ENV_VAR, &wrapper_path); - cmd.env(ZSH_EXEC_WRAPPER_MODE_ENV_VAR, "1"); - - let mut child = cmd.spawn().map_err(|err| { - ToolError::Rejected(format!( - "failed to start zsh fork command {} with zsh_path {}: {err}", - command[0], - zsh_path.display() - )) - })?; - - let (stream_tx, mut stream_rx) = - tokio::sync::mpsc::unbounded_channel::<(ExecOutputStream, Vec)>(); - - if let Some(mut out) = child.stdout.take() { - let tx = stream_tx.clone(); - tokio::spawn(async move { - let mut buf = [0_u8; 8192]; - loop { - let read = match out.read(&mut buf).await { - Ok(0) => break, - Ok(n) => n, - Err(err) => { - tracing::warn!("zsh fork stdout read error: {err}"); - break; - } - }; - let _ = tx.send((ExecOutputStream::Stdout, buf[..read].to_vec())); - } - }); - } - - if let Some(mut err) = child.stderr.take() { - let tx = stream_tx.clone(); - tokio::spawn(async move { - let mut buf = [0_u8; 8192]; - loop { - let read = match err.read(&mut buf).await { - Ok(0) => break, - Ok(n) => n, - Err(err) => { - tracing::warn!("zsh fork stderr read error: {err}"); - break; - } - }; - let _ = tx.send((ExecOutputStream::Stderr, buf[..read].to_vec())); - } - }); - } - drop(stream_tx); - - let mut stdout_bytes = Vec::new(); - let mut stderr_bytes = Vec::new(); - let mut child_exit = None; - let mut timed_out = false; - let mut stream_open = true; - let mut user_rejected = false; - let start = Instant::now(); - - let expiration = req.expiration.clone().wait(); - tokio::pin!(expiration); - - while child_exit.is_none() || stream_open { - tokio::select! { - result = child.wait(), if child_exit.is_none() => { - child_exit = Some(result.map_err(|err| ToolError::Rejected(format!("wait for zsh fork command exit: {err}")))?); - } - stream = stream_rx.recv(), if stream_open => { - if let Some((output_stream, chunk)) = stream { - match output_stream { - ExecOutputStream::Stdout => stdout_bytes.extend_from_slice(&chunk), - ExecOutputStream::Stderr => stderr_bytes.extend_from_slice(&chunk), - } - session - .send_event( - turn, - EventMsg::ExecCommandOutputDelta(ExecCommandOutputDeltaEvent { - call_id: call_id.to_string(), - stream: output_stream, - chunk, - }), - ) - .await; - } else { - stream_open = false; - } - } - accept_result = listener.accept(), if child_exit.is_none() => { - let (stream, _) = accept_result.map_err(|err| { - ToolError::Rejected(format!("failed to accept wrapper request: {err}")) - })?; - if self - .handle_wrapper_request(stream, req.justification.clone(), session, turn, call_id) - .await? - { - user_rejected = true; - } - } - _ = &mut expiration, if child_exit.is_none() => { - timed_out = true; - kill_child_process_group(&mut child).map_err(|err| { - ToolError::Rejected(format!("kill zsh fork command process group: {err}")) - })?; - child.start_kill().map_err(|err| { - ToolError::Rejected(format!("kill zsh fork command process: {err}")) - })?; - } - } - } - - let _ = std::fs::remove_file(&wrapper_socket_path); - - let status = child_exit.ok_or_else(|| { - ToolError::Rejected("zsh fork command did not return exit status".to_string()) - })?; - - if user_rejected { - return Err(ToolError::Rejected("rejected by user".to_string())); - } - - let stdout_text = crate::text_encoding::bytes_to_string_smart(&stdout_bytes); - let stderr_text = crate::text_encoding::bytes_to_string_smart(&stderr_bytes); - let output = ExecToolCallOutput { - exit_code: status.code().unwrap_or(-1), - stdout: crate::exec::StreamOutput::new(stdout_text.clone()), - stderr: crate::exec::StreamOutput::new(stderr_text.clone()), - aggregated_output: crate::exec::StreamOutput::new(format!( - "{stdout_text}{stderr_text}" - )), - duration: start.elapsed(), - timed_out, - }; - - Self::map_exec_result(req.sandbox, output) - } - - #[cfg(unix)] - async fn handle_wrapper_request( - &self, - mut stream: UnixStream, - approval_reason: Option, - session: &crate::codex::Session, - turn: &crate::codex::TurnContext, - call_id: &str, - ) -> Result { - let mut request_buf = Vec::new(); - stream.read_to_end(&mut request_buf).await.map_err(|err| { - ToolError::Rejected(format!("read wrapper request from socket: {err}")) - })?; - let request_line = String::from_utf8(request_buf).map_err(|err| { - ToolError::Rejected(format!("decode wrapper request as utf-8: {err}")) - })?; - let request = parse_wrapper_request_line(request_line.trim())?; - - let (request_id, file, argv, cwd) = match request { - WrapperIpcRequest::ExecRequest { - request_id, - file, - argv, - cwd, - } => (request_id, file, argv, cwd), - }; - - let command_for_approval = if argv.is_empty() { - vec![file.clone()] - } else { - argv.clone() - }; - - let approval_id = Uuid::new_v4().to_string(); - let decision = session - .request_command_approval( - turn, - call_id.to_string(), - Some(approval_id), - command_for_approval, - PathBuf::from(cwd), - approval_reason, - None, - None::, - None, - ) - .await; - - let (action, reason, user_rejected) = match decision { - ReviewDecision::Approved - | ReviewDecision::ApprovedForSession - | ReviewDecision::ApprovedExecpolicyAmendment { .. } => { - (WrapperExecAction::Run, None, false) - } - ReviewDecision::NetworkPolicyAmendment { - network_policy_amendment, - } => match network_policy_amendment.action { - NetworkPolicyRuleAction::Allow => (WrapperExecAction::Run, None, false), - NetworkPolicyRuleAction::Deny => ( - WrapperExecAction::Deny, - Some("command denied by host approval policy".to_string()), - true, - ), - }, - ReviewDecision::Denied => ( - WrapperExecAction::Deny, - Some("command denied by host approval policy".to_string()), - true, - ), - ReviewDecision::Abort => ( - WrapperExecAction::Deny, - Some("command aborted by host approval policy".to_string()), - true, - ), - }; - - write_json_line( - &mut stream, - &WrapperIpcResponse::ExecResponse { - request_id, - action, - reason, - }, - ) - .await?; - - Ok(user_rejected) - } - - #[cfg(unix)] - fn map_exec_result( - sandbox: crate::exec::SandboxType, - output: ExecToolCallOutput, - ) -> Result { - if output.timed_out { - return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Timeout { - output: Box::new(output), - }))); - } - - if crate::exec::is_likely_sandbox_denied(sandbox, &output) { - return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { - output: Box::new(output), - network_policy_decision: None, - }))); - } - - Ok(output) - } -} - -pub fn maybe_run_zsh_exec_wrapper_mode() -> anyhow::Result { - if std::env::var_os(ZSH_EXEC_WRAPPER_MODE_ENV_VAR).is_none() { - return Ok(false); - } - - run_exec_wrapper_mode()?; - Ok(true) -} - -fn run_exec_wrapper_mode() -> anyhow::Result<()> { - #[cfg(not(unix))] - { - anyhow::bail!("zsh exec wrapper mode is only supported on unix"); - } - - #[cfg(unix)] - { - use std::os::unix::net::UnixStream as StdUnixStream; - - let args: Vec = std::env::args().collect(); - if args.len() < 2 { - anyhow::bail!("exec wrapper mode requires target executable path"); - } - let file = args[1].clone(); - let argv = if args.len() > 2 { - args[2..].to_vec() - } else { - vec![file.clone()] - }; - let cwd = std::env::current_dir() - .context("resolve wrapper cwd")? - .to_string_lossy() - .to_string(); - let socket_path = std::env::var(ZSH_EXEC_BRIDGE_WRAPPER_SOCKET_ENV_VAR) - .context("missing wrapper socket path env var")?; - - let request_id = Uuid::new_v4().to_string(); - let request = WrapperIpcRequest::ExecRequest { - request_id: request_id.clone(), - file: file.clone(), - argv: argv.clone(), - cwd, - }; - let mut stream = StdUnixStream::connect(&socket_path) - .with_context(|| format!("connect to wrapper socket at {socket_path}"))?; - let encoded = serde_json::to_string(&request).context("serialize wrapper request")?; - stream - .write_all(encoded.as_bytes()) - .context("write wrapper request")?; - stream - .write_all(b"\n") - .context("write wrapper request newline")?; - stream - .shutdown(std::net::Shutdown::Write) - .context("shutdown wrapper write")?; - - let mut response_buf = String::new(); - stream - .read_to_string(&mut response_buf) - .context("read wrapper response")?; - let response: WrapperIpcResponse = - serde_json::from_str(response_buf.trim()).context("parse wrapper response")?; - - let (response_request_id, action, reason) = match response { - WrapperIpcResponse::ExecResponse { - request_id, - action, - reason, - } => (request_id, action, reason), - }; - if response_request_id != request_id { - anyhow::bail!( - "wrapper response request_id mismatch: expected {request_id}, got {response_request_id}" - ); - } - - if action == WrapperExecAction::Deny { - if let Some(reason) = reason { - tracing::warn!("execution denied: {reason}"); - } else { - tracing::warn!("execution denied"); - } - std::process::exit(1); - } - - let mut command = std::process::Command::new(&file); - if argv.len() > 1 { - command.args(&argv[1..]); - } - command.env_remove(ZSH_EXEC_WRAPPER_MODE_ENV_VAR); - command.env_remove(ZSH_EXEC_BRIDGE_WRAPPER_SOCKET_ENV_VAR); - command.env_remove(EXEC_WRAPPER_ENV_VAR); - let status = command.status().context("spawn wrapped executable")?; - std::process::exit(status.code().unwrap_or(1)); - } -} - -#[cfg(unix)] -fn parse_wrapper_request_line(request_line: &str) -> Result { - serde_json::from_str(request_line) - .map_err(|err| ToolError::Rejected(format!("parse wrapper request payload: {err}"))) -} - -#[cfg(unix)] -async fn write_json_line( - writer: &mut W, - message: &T, -) -> Result<(), ToolError> { - let encoded = serde_json::to_string(message) - .map_err(|err| ToolError::Rejected(format!("serialize wrapper message: {err}")))?; - tokio::io::AsyncWriteExt::write_all(writer, encoded.as_bytes()) - .await - .map_err(|err| ToolError::Rejected(format!("write wrapper message: {err}")))?; - tokio::io::AsyncWriteExt::write_all(writer, b"\n") - .await - .map_err(|err| ToolError::Rejected(format!("write wrapper newline: {err}")))?; - tokio::io::AsyncWriteExt::flush(writer) - .await - .map_err(|err| ToolError::Rejected(format!("flush wrapper message: {err}")))?; - Ok(()) -} - -#[cfg(all(test, unix))] -mod tests { - use super::*; - - #[test] - fn parse_wrapper_request_line_rejects_malformed_json() { - let err = parse_wrapper_request_line("this-is-not-json").unwrap_err(); - let ToolError::Rejected(message) = err else { - panic!("expected ToolError::Rejected"); - }; - assert!(message.starts_with("parse wrapper request payload:")); - } -} diff --git a/codex-rs/shell-escalation/src/lib.rs b/codex-rs/shell-escalation/src/lib.rs index 45ed7c799..98d33fc59 100644 --- a/codex-rs/shell-escalation/src/lib.rs +++ b/codex-rs/shell-escalation/src/lib.rs @@ -22,6 +22,6 @@ pub use unix::Stopwatch; #[cfg(unix)] pub use unix::main_execve_wrapper; #[cfg(unix)] -pub use unix::run; -#[cfg(unix)] pub use unix::run_escalate_server; +#[cfg(unix)] +pub use unix::run_shell_escalation_execve_wrapper; diff --git a/codex-rs/shell-escalation/src/unix/core_shell_escalation.rs b/codex-rs/shell-escalation/src/unix/core_shell_escalation.rs index a56624cd9..3f2d5ad23 100644 --- a/codex-rs/shell-escalation/src/unix/core_shell_escalation.rs +++ b/codex-rs/shell-escalation/src/unix/core_shell_escalation.rs @@ -42,7 +42,6 @@ impl ShellPolicyFactory { /// Public only because it is the associated `Policy` type in the public /// `EscalationPolicyFactory` impl for `ShellPolicyFactory`. -#[doc(hidden)] pub struct ShellEscalationPolicy { provider: Arc, stopwatch: Stopwatch, diff --git a/codex-rs/shell-escalation/src/unix/escalate_client.rs b/codex-rs/shell-escalation/src/unix/escalate_client.rs index 17d8e6577..5c363f7d6 100644 --- a/codex-rs/shell-escalation/src/unix/escalate_client.rs +++ b/codex-rs/shell-escalation/src/unix/escalate_client.rs @@ -27,7 +27,10 @@ fn get_escalate_client() -> anyhow::Result { Ok(unsafe { AsyncDatagramSocket::from_raw_fd(client_fd) }?) } -pub async fn run(file: String, argv: Vec) -> anyhow::Result { +pub async fn run_shell_escalation_execve_wrapper( + file: String, + argv: Vec, +) -> anyhow::Result { let handshake_client = get_escalate_client()?; let (server, client) = AsyncSocket::pair()?; const HANDSHAKE_MESSAGE: [u8; 1] = [0]; diff --git a/codex-rs/shell-escalation/src/unix/escalate_server.rs b/codex-rs/shell-escalation/src/unix/escalate_server.rs index dbdba645b..7e373f4fb 100644 --- a/codex-rs/shell-escalation/src/unix/escalate_server.rs +++ b/codex-rs/shell-escalation/src/unix/escalate_server.rs @@ -45,7 +45,7 @@ pub trait ShellCommandExecutor: Send + Sync { #[derive(Debug, serde::Deserialize, serde::Serialize)] pub struct ExecParams { - /// The bash string to execute. + /// The the string of Zsh/shell to execute. pub command: String, /// The working directory to execute the command in. Must be an absolute path. pub workdir: String, @@ -58,20 +58,22 @@ pub struct ExecParams { #[derive(Debug, serde::Serialize, serde::Deserialize)] pub struct ExecResult { pub exit_code: i32, + pub stdout: String, + pub stderr: String, + /// Aggregated stdout+stderr output for compatibility with existing callers. pub output: String, pub duration: Duration, pub timed_out: bool, } -#[allow(clippy::module_name_repetitions)] -pub struct EscalateServer { +struct EscalateServer { bash_path: PathBuf, execve_wrapper: PathBuf, policy: Arc, } impl EscalateServer { - pub fn new

(bash_path: PathBuf, execve_wrapper: PathBuf, policy: P) -> Self + fn new

(bash_path: PathBuf, execve_wrapper: PathBuf, policy: P) -> Self where P: EscalationPolicy + Send + Sync + 'static, { @@ -82,7 +84,7 @@ impl EscalateServer { } } - pub async fn exec( + async fn exec( &self, params: ExecParams, cancel_rx: CancellationToken, diff --git a/codex-rs/shell-escalation/src/unix/execve_wrapper.rs b/codex-rs/shell-escalation/src/unix/execve_wrapper.rs index ac865bf9f..58f8d3706 100644 --- a/codex-rs/shell-escalation/src/unix/execve_wrapper.rs +++ b/codex-rs/shell-escalation/src/unix/execve_wrapper.rs @@ -20,6 +20,6 @@ pub async fn main_execve_wrapper() -> anyhow::Result<()> { .init(); let ExecveWrapperCli { file, argv } = ExecveWrapperCli::parse(); - let exit_code = crate::run(file, argv).await?; + let exit_code = crate::run_shell_escalation_execve_wrapper(file, argv).await?; std::process::exit(exit_code); } diff --git a/codex-rs/shell-escalation/src/unix/mod.rs b/codex-rs/shell-escalation/src/unix/mod.rs index 42091791f..c69c8b942 100644 --- a/codex-rs/shell-escalation/src/unix/mod.rs +++ b/codex-rs/shell-escalation/src/unix/mod.rs @@ -64,7 +64,7 @@ pub mod stopwatch; pub use self::core_shell_escalation::ShellActionProvider; pub use self::core_shell_escalation::ShellPolicyFactory; -pub use self::escalate_client::run; +pub use self::escalate_client::run_shell_escalation_execve_wrapper; pub use self::escalate_protocol::EscalateAction; pub use self::escalate_server::EscalationPolicyFactory; pub use self::escalate_server::ExecParams; diff --git a/codex-rs/tui/src/app/pending_interactive_replay.rs b/codex-rs/tui/src/app/pending_interactive_replay.rs index 1cfb12f5f..bc7fed644 100644 --- a/codex-rs/tui/src/app/pending_interactive_replay.rs +++ b/codex-rs/tui/src/app/pending_interactive_replay.rs @@ -377,6 +377,7 @@ mod tests { network_approval_context: None, proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, + additional_permissions: None, parsed_cmd: Vec::new(), }, ), @@ -518,6 +519,7 @@ mod tests { network_approval_context: None, proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, + additional_permissions: None, parsed_cmd: Vec::new(), }, ),