diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index ed405dd8c42..7505ae1e4e2 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -949,7 +949,7 @@ "landlock_0.4.4": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"anyhow\",\"req\":\"^1.0\"},{\"name\":\"enumflags2\",\"req\":\"^0.7\"},{\"kind\":\"dev\",\"name\":\"lazy_static\",\"req\":\"^1\"},{\"name\":\"libc\",\"req\":\"^0.2.175\"},{\"kind\":\"dev\",\"name\":\"strum\",\"req\":\"^0.26\"},{\"kind\":\"dev\",\"name\":\"strum_macros\",\"req\":\"^0.26\"},{\"name\":\"thiserror\",\"req\":\"^2.0\"}],\"features\":{}}", "language-tags_0.3.2": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"bencher\",\"req\":\"^0.1\"},{\"name\":\"serde\",\"optional\":true,\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"serde_json\",\"req\":\"^1.0\"}],\"features\":{}}", "lazy_static_1.5.0": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"doc-comment\",\"req\":\"^0.3.1\"},{\"default_features\":false,\"features\":[\"once\"],\"name\":\"spin\",\"optional\":true,\"req\":\"^0.9.8\"},{\"kind\":\"dev\",\"name\":\"trybuild\",\"req\":\"^1\"}],\"features\":{\"spin_no_std\":[\"spin\"]}}", - "libc_0.2.180": "{\"dependencies\":[{\"name\":\"rustc-std-workspace-core\",\"optional\":true,\"req\":\"^1.0.1\"}],\"features\":{\"align\":[],\"const-extern-fn\":[],\"default\":[\"std\"],\"extra_traits\":[],\"rustc-dep-of-std\":[\"align\",\"rustc-std-workspace-core\"],\"std\":[],\"use_std\":[\"std\"]}}", + "libc_0.2.182": "{\"dependencies\":[{\"name\":\"rustc-std-workspace-core\",\"optional\":true,\"req\":\"^1.0.1\"}],\"features\":{\"align\":[],\"const-extern-fn\":[],\"default\":[\"std\"],\"extra_traits\":[],\"rustc-dep-of-std\":[\"align\",\"rustc-std-workspace-core\"],\"std\":[],\"use_std\":[\"std\"]}}", "libdbus-sys_0.2.7": "{\"dependencies\":[{\"kind\":\"build\",\"name\":\"cc\",\"optional\":true,\"req\":\"^1.0.78\"},{\"kind\":\"build\",\"name\":\"pkg-config\",\"optional\":true,\"req\":\"^0.3\"}],\"features\":{\"default\":[\"pkg-config\"],\"vendored\":[\"cc\"]}}", "libm_0.2.16": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"no-panic\",\"req\":\"^0.1.35\"}],\"features\":{\"arch\":[],\"default\":[\"arch\"],\"force-soft-floats\":[],\"unstable\":[\"unstable-intrinsics\",\"unstable-float\"],\"unstable-float\":[],\"unstable-intrinsics\":[],\"unstable-public-internals\":[]}}", "libredox_0.1.12": "{\"dependencies\":[{\"name\":\"bitflags\",\"req\":\"^2\"},{\"name\":\"ioslice\",\"optional\":true,\"req\":\"^0.6\"},{\"name\":\"libc\",\"req\":\"^0.2\"},{\"name\":\"redox_syscall\",\"optional\":true,\"req\":\"^0.7\"}],\"features\":{\"call\":[],\"default\":[\"call\",\"std\",\"redox_syscall\"],\"mkns\":[\"ioslice\"],\"std\":[]}}", @@ -1277,7 +1277,7 @@ "supports-color_2.1.0": "{\"dependencies\":[{\"name\":\"is-terminal\",\"req\":\"^0.4.0\"},{\"name\":\"is_ci\",\"req\":\"^1.1.1\"}],\"features\":{}}", "supports-color_3.0.2": "{\"dependencies\":[{\"name\":\"is_ci\",\"req\":\"^1.2.0\"}],\"features\":{}}", "syn_1.0.109": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"anyhow\",\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"automod\",\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"flate2\",\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"insta\",\"req\":\"^1.0\"},{\"default_features\":false,\"name\":\"proc-macro2\",\"req\":\"^1.0.46\"},{\"default_features\":false,\"name\":\"quote\",\"optional\":true,\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"rayon\",\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"ref-cast\",\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"regex\",\"req\":\"^1.0\"},{\"features\":[\"blocking\"],\"kind\":\"dev\",\"name\":\"reqwest\",\"req\":\"^0.11\"},{\"kind\":\"dev\",\"name\":\"syn-test-suite\",\"req\":\"^0\"},{\"kind\":\"dev\",\"name\":\"tar\",\"req\":\"^0.4.16\"},{\"kind\":\"dev\",\"name\":\"termcolor\",\"req\":\"^1.0\"},{\"name\":\"unicode-ident\",\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"walkdir\",\"req\":\"^2.1\"}],\"features\":{\"clone-impls\":[],\"default\":[\"derive\",\"parsing\",\"printing\",\"clone-impls\",\"proc-macro\"],\"derive\":[],\"extra-traits\":[],\"fold\":[],\"full\":[],\"parsing\":[],\"printing\":[\"quote\"],\"proc-macro\":[\"proc-macro2/proc-macro\",\"quote/proc-macro\"],\"test\":[\"syn-test-suite/all-features\"],\"visit\":[],\"visit-mut\":[]}}", - "syn_2.0.114": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"anyhow\",\"req\":\"^1\"},{\"kind\":\"dev\",\"name\":\"automod\",\"req\":\"^1\"},{\"kind\":\"dev\",\"name\":\"flate2\",\"req\":\"^1\",\"target\":\"cfg(not(miri))\"},{\"kind\":\"dev\",\"name\":\"insta\",\"req\":\"^1\"},{\"default_features\":false,\"name\":\"proc-macro2\",\"req\":\"^1.0.91\"},{\"default_features\":false,\"name\":\"quote\",\"optional\":true,\"req\":\"^1.0.35\"},{\"kind\":\"dev\",\"name\":\"rayon\",\"req\":\"^1\",\"target\":\"cfg(not(miri))\"},{\"kind\":\"dev\",\"name\":\"ref-cast\",\"req\":\"^1\"},{\"features\":[\"blocking\"],\"kind\":\"dev\",\"name\":\"reqwest\",\"req\":\"^0.13\",\"target\":\"cfg(not(miri))\"},{\"kind\":\"dev\",\"name\":\"rustversion\",\"req\":\"^1\"},{\"kind\":\"dev\",\"name\":\"syn-test-suite\",\"req\":\"^0\"},{\"kind\":\"dev\",\"name\":\"tar\",\"req\":\"^0.4.16\",\"target\":\"cfg(not(miri))\"},{\"kind\":\"dev\",\"name\":\"termcolor\",\"req\":\"^1\"},{\"name\":\"unicode-ident\",\"req\":\"^1\"},{\"kind\":\"dev\",\"name\":\"walkdir\",\"req\":\"^2.3.2\",\"target\":\"cfg(not(miri))\"}],\"features\":{\"clone-impls\":[],\"default\":[\"derive\",\"parsing\",\"printing\",\"clone-impls\",\"proc-macro\"],\"derive\":[],\"extra-traits\":[],\"fold\":[],\"full\":[],\"parsing\":[],\"printing\":[\"dep:quote\"],\"proc-macro\":[\"proc-macro2/proc-macro\",\"quote?/proc-macro\"],\"test\":[\"syn-test-suite/all-features\"],\"visit\":[],\"visit-mut\":[]}}", + "syn_2.0.117": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"anyhow\",\"req\":\"^1\"},{\"kind\":\"dev\",\"name\":\"automod\",\"req\":\"^1\"},{\"kind\":\"dev\",\"name\":\"flate2\",\"req\":\"^1\",\"target\":\"cfg(not(miri))\"},{\"kind\":\"dev\",\"name\":\"insta\",\"req\":\"^1\"},{\"default_features\":false,\"name\":\"proc-macro2\",\"req\":\"^1.0.91\"},{\"default_features\":false,\"name\":\"quote\",\"optional\":true,\"req\":\"^1.0.35\"},{\"kind\":\"dev\",\"name\":\"rayon\",\"req\":\"^1\",\"target\":\"cfg(not(miri))\"},{\"kind\":\"dev\",\"name\":\"ref-cast\",\"req\":\"^1\"},{\"features\":[\"blocking\"],\"kind\":\"dev\",\"name\":\"reqwest\",\"req\":\"^0.13\",\"target\":\"cfg(not(miri))\"},{\"kind\":\"dev\",\"name\":\"rustversion\",\"req\":\"^1\"},{\"kind\":\"dev\",\"name\":\"syn-test-suite\",\"req\":\"^0\"},{\"kind\":\"dev\",\"name\":\"tar\",\"req\":\"^0.4.16\",\"target\":\"cfg(not(miri))\"},{\"kind\":\"dev\",\"name\":\"termcolor\",\"req\":\"^1\"},{\"name\":\"unicode-ident\",\"req\":\"^1\"},{\"kind\":\"dev\",\"name\":\"walkdir\",\"req\":\"^2.3.2\",\"target\":\"cfg(not(miri))\"}],\"features\":{\"clone-impls\":[],\"default\":[\"derive\",\"parsing\",\"printing\",\"clone-impls\",\"proc-macro\"],\"derive\":[],\"extra-traits\":[],\"fold\":[],\"full\":[],\"parsing\":[],\"printing\":[\"dep:quote\"],\"proc-macro\":[\"proc-macro2/proc-macro\",\"quote?/proc-macro\"],\"test\":[\"syn-test-suite/all-features\"],\"visit\":[],\"visit-mut\":[]}}", "sync_wrapper_1.0.2": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"futures\",\"req\":\"^0.3\"},{\"default_features\":false,\"name\":\"futures-core\",\"optional\":true,\"req\":\"^0.3\"},{\"kind\":\"dev\",\"name\":\"pin-project-lite\",\"req\":\"^0.2.7\"}],\"features\":{\"futures\":[\"futures-core\"]}}", "synstructure_0.13.2": "{\"dependencies\":[{\"default_features\":false,\"name\":\"proc-macro2\",\"req\":\"^1.0.60\"},{\"default_features\":false,\"name\":\"quote\",\"req\":\"^1\"},{\"default_features\":false,\"features\":[\"derive\",\"parsing\",\"printing\",\"clone-impls\",\"visit\",\"extra-traits\"],\"name\":\"syn\",\"req\":\"^2\"},{\"kind\":\"dev\",\"name\":\"synstructure_test_traits\",\"req\":\"^0.1\"}],\"features\":{\"default\":[\"proc-macro\"],\"proc-macro\":[\"proc-macro2/proc-macro\",\"syn/proc-macro\",\"quote/proc-macro\"]}}", "syntect_5.3.0": "{\"dependencies\":[{\"name\":\"bincode\",\"optional\":true,\"req\":\"^1.0\"},{\"features\":[\"html_reports\"],\"kind\":\"dev\",\"name\":\"criterion\",\"req\":\"^0.3\"},{\"name\":\"fancy-regex\",\"optional\":true,\"req\":\"^0.16.2\"},{\"name\":\"flate2\",\"optional\":true,\"req\":\"^1.0\"},{\"name\":\"fnv\",\"optional\":true,\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"getopts\",\"req\":\"^0.2\"},{\"name\":\"once_cell\",\"req\":\"^1.8\"},{\"default_features\":false,\"name\":\"onig\",\"optional\":true,\"req\":\"^6.5.1\"},{\"name\":\"plist\",\"optional\":true,\"req\":\"^1.3\"},{\"kind\":\"dev\",\"name\":\"pretty_assertions\",\"req\":\"^0.6\"},{\"kind\":\"dev\",\"name\":\"public-api\",\"req\":\"^0.50.1\"},{\"kind\":\"dev\",\"name\":\"rayon\",\"req\":\"^1.0.0\"},{\"kind\":\"dev\",\"name\":\"regex\",\"req\":\"^1.0\"},{\"name\":\"regex-syntax\",\"optional\":true,\"req\":\"^0.8\"},{\"kind\":\"dev\",\"name\":\"rustdoc-json\",\"req\":\"^0.9.7\"},{\"kind\":\"dev\",\"name\":\"rustup-toolchain\",\"req\":\"^0.1.5\"},{\"name\":\"serde\",\"req\":\"^1.0\"},{\"name\":\"serde_derive\",\"req\":\"^1.0\"},{\"name\":\"serde_json\",\"optional\":true,\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"serde_json\",\"req\":\"^1.0\"},{\"name\":\"thiserror\",\"req\":\"^2.0.12\"},{\"name\":\"walkdir\",\"req\":\"^2.0\"},{\"name\":\"yaml-rust\",\"optional\":true,\"req\":\"^0.4.5\"}],\"features\":{\"default\":[\"default-onig\"],\"default-fancy\":[\"parsing\",\"default-syntaxes\",\"default-themes\",\"html\",\"plist-load\",\"yaml-load\",\"dump-load\",\"dump-create\",\"regex-fancy\"],\"default-onig\":[\"parsing\",\"default-syntaxes\",\"default-themes\",\"html\",\"plist-load\",\"yaml-load\",\"dump-load\",\"dump-create\",\"regex-onig\"],\"default-syntaxes\":[\"parsing\",\"dump-load\"],\"default-themes\":[\"dump-load\"],\"dump-create\":[\"flate2\",\"bincode\"],\"dump-load\":[\"flate2\",\"bincode\"],\"html\":[\"parsing\"],\"metadata\":[\"parsing\",\"plist-load\",\"dep:serde_json\"],\"parsing\":[\"regex-syntax\",\"fnv\",\"dump-create\",\"dump-load\"],\"plist-load\":[\"plist\",\"dep:serde_json\"],\"regex-fancy\":[\"fancy-regex\"],\"regex-onig\":[\"onig\"],\"yaml-load\":[\"yaml-rust\",\"parsing\"]}}", diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 7e475eeaaa7..ede51081d3f 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1408,6 +1408,7 @@ dependencies = [ "anyhow", "codex-apply-patch", "codex-linux-sandbox", + "codex-shell-escalation", "codex-utils-home-dir", "dotenvy", "tempfile", @@ -1653,6 +1654,7 @@ dependencies = [ "codex-rmcp-client", "codex-secrets", "codex-shell-command", + "codex-shell-escalation", "codex-skills", "codex-state", "codex-utils-absolute-path", @@ -1799,6 +1801,7 @@ dependencies = [ "shlex", "tempfile", "tokio", + "tokio-util", "tracing", "tracing-subscriber", ] @@ -2205,7 +2208,6 @@ version = "0.0.0" dependencies = [ "anyhow", "async-trait", - "codex-core", "codex-execpolicy", "codex-protocol", "libc", diff --git a/codex-rs/app-server/src/main.rs b/codex-rs/app-server/src/main.rs index c56c3c8f988..5c4e5eacc7a 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 e989807ea9d..59af940d703 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 @@ -13,6 +13,7 @@ use app_test_support::create_mock_responses_server_sequence; use app_test_support::create_mock_responses_server_sequence_unchecked; use app_test_support::create_shell_command_sse_response; use app_test_support::to_response; +use app_test_support::write_models_cache; use codex_app_server_protocol::CommandExecutionApprovalDecision; use codex_app_server_protocol::CommandExecutionRequestApprovalResponse; use codex_app_server_protocol::CommandExecutionStatus; @@ -35,7 +36,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; @@ -79,6 +79,7 @@ async fn turn_start_shell_zsh_fork_executes_command_v2() -> Result<()> { ]), &zsh_path, )?; + write_models_cache(&codex_home)?; let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace).await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; @@ -106,7 +107,9 @@ async fn turn_start_shell_zsh_fork_executes_command_v2() -> Result<()> { }], cwd: Some(workspace.clone()), approval_policy: Some(codex_app_server_protocol::AskForApproval::Never), - sandbox_policy: Some(codex_app_server_protocol::SandboxPolicy::DangerFullAccess), + sandbox_policy: Some(codex_app_server_protocol::SandboxPolicy::ReadOnly { + access: codex_app_server_protocol::ReadOnlyAccess::FullAccess, + }), model: Some("mock-model".to_string()), effort: Some(codex_protocol::openai_models::ReasoningEffort::Medium), summary: Some(codex_protocol::config_types::ReasoningSummary::Auto), @@ -196,6 +199,7 @@ async fn turn_start_shell_zsh_fork_exec_approval_decline_v2() -> Result<()> { ]), &zsh_path, )?; + write_models_cache(&codex_home)?; let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace).await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; @@ -329,6 +333,7 @@ async fn turn_start_shell_zsh_fork_exec_approval_cancel_v2() -> Result<()> { ]), &zsh_path, )?; + write_models_cache(&codex_home)?; let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace).await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; @@ -444,32 +449,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,14 +485,15 @@ 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, )?; + write_models_cache(&codex_home)?; let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace).await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; @@ -525,21 +516,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 +541,7 @@ 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 saw_parent_approval = false; let target_decisions = [ CommandExecutionApprovalDecision::Accept, @@ -573,35 +560,38 @@ 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( - params - .approval_id - .clone() - .expect("approval_id must be present for zsh subcommand approvals"), + 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_strings.push(approval_command.to_string()); } 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"); assert!( !saw_parent_approval, - "unexpected extra non-target approval: {command}" + "unexpected extra non-target approval: {approval_command}" ); assert!( - command.contains("zsh-no-rc"), - "expected parent zsh wrapper approval, got: {command}" + approval_command.contains(&zsh_path.display().to_string()), + "expected parent zsh approval, got: {approval_command}" ); assert!( - command.contains("/usr/bin/true && /usr/bin/true"), - "expected tool command in parent approval, got: {command}" + approval_command.contains(&shell_command), + "expected tool command in parent approval, got: {approval_command}" ); saw_parent_approval = true; CommandExecutionApprovalDecision::Accept @@ -613,8 +603,13 @@ 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_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 c5d9681327d..31cee359942 100644 --- a/codex-rs/arg0/Cargo.toml +++ b/codex-rs/arg0/Cargo.toml @@ -19,3 +19,6 @@ codex-utils-home-dir = { workspace = true } dotenvy = { workspace = true } tempfile = { workspace = true } tokio = { workspace = true, features = ["rt-multi-thread"] } + +[target.'cfg(unix)'.dependencies] +codex-shell-escalation = { workspace = true } diff --git a/codex-rs/arg0/src/lib.rs b/codex-rs/arg0/src/lib.rs index 3ac7e017f22..fa352d90678 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::unix::escalate_client::run( + 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 15f9d7d11c3..bd571248c79 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -138,6 +138,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 249cb99776d..e61b7af3209 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -249,7 +249,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; @@ -1208,7 +1207,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() @@ -1287,12 +1287,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 @@ -1308,7 +1302,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), @@ -4227,7 +4221,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 @@ -7895,7 +7888,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), @@ -8048,7 +8041,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/config/mod.rs b/codex-rs/core/src/config/mod.rs index ffd35e036ad..2d34577223a 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -372,7 +372,7 @@ pub struct Config { pub js_repl_node_module_dirs: Vec, /// Optional absolute path to patched zsh used by zsh-exec-bridge-backed shell execution. - pub zsh_path: Option, + pub zsh_path: Option, /// Value to use for `reasoning.effort` when making a request using the /// Responses API. @@ -1484,7 +1484,7 @@ pub struct ConfigOverrides { pub codex_linux_sandbox_exe: Option, pub js_repl_node_path: Option, pub js_repl_node_module_dirs: Option>, - pub zsh_path: Option, + pub zsh_path: Option, pub base_instructions: Option, pub developer_instructions: Option, pub personality: Option, @@ -1905,8 +1905,8 @@ impl Config { }) .unwrap_or_default(); let zsh_path = zsh_path_override - .or(config_profile.zsh_path.map(Into::into)) - .or(cfg.zsh_path.map(Into::into)); + .or(config_profile.zsh_path) + .or(cfg.zsh_path); let review_model = override_review_model.or(cfg.review_model); diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index e40905e3141..a9ca37e7000 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,20 +143,10 @@ 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::X_CODEX_TURN_METADATA_HEADER; -pub use exec_policy::ExecPolicyError; -pub use exec_policy::check_execpolicy_for_warnings; -pub use exec_policy::format_exec_policy_error_with_source; -pub use exec_policy::load_exec_policy; -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::X_CODEX_TURN_METADATA_HEADER; pub use client::ws_version_from_features; pub use client_common::Prompt; pub use client_common::REVIEW_PROMPT; @@ -165,6 +154,14 @@ 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; +pub use exec_policy::load_exec_policy; +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 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 f9c3c171e76..87f7298fd06 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -163,19 +163,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(), 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 fe82ec84c0d..96d420de053 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 codex_utils_absolute_path::AbsolutePathBuf; use tokio::sync::Mutex; use tokio::sync::RwLock; use tokio::sync::watch; @@ -27,7 +27,7 @@ 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, + 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 46a90681d2a..f1655d8bcbc 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -25,11 +25,20 @@ 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; 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, @@ -40,6 +49,7 @@ struct RunExecLikeArgs { tracker: crate::tools::context::SharedTurnDiffTracker, call_id: String, freeform: bool, + shell_runtime_backend: ShellRuntimeBackend, } impl ShellHandler { @@ -63,6 +73,25 @@ impl ShellHandler { } impl ShellCommandHandler { + pub(crate) fn classic() -> Self { + Self { + backend: ShellCommandBackend::Classic, + } + } + + pub(crate) fn zsh_fork() -> Self { + Self { + backend: ShellCommandBackend::ZshFork, + } + } + + 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, @@ -155,6 +184,7 @@ impl ToolHandler for ShellHandler { tracker, call_id, freeform: false, + shell_runtime_backend: ShellRuntimeBackend::Generic, }) .await } @@ -170,6 +200,7 @@ impl ToolHandler for ShellHandler { tracker, call_id, freeform: false, + shell_runtime_backend: ShellRuntimeBackend::Generic, }) .await } @@ -245,6 +276,7 @@ impl ToolHandler for ShellCommandHandler { tracker, call_id, freeform: true, + shell_runtime_backend: self.shell_runtime_backend(), }) .await } @@ -261,6 +293,7 @@ impl ShellHandler { tracker, call_id, freeform, + shell_runtime_backend, } = args; let mut exec_params = exec_params; @@ -341,7 +374,10 @@ impl ShellHandler { exec_approval_requirement, }; let mut orchestrator = ToolOrchestrator::new(); - let mut runtime = ShellRuntime::new(); + let mut runtime = match shell_runtime_backend { + ShellRuntimeBackend::Generic => ShellRuntime::new(), + backend => 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 11ad6691697..33588212075 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; @@ -26,10 +29,10 @@ use crate::tools::sandboxing::ToolCtx; use crate::tools::sandboxing::ToolError; use crate::tools::sandboxing::ToolRuntime; 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::protocol::ReviewDecision; use futures::future::BoxFuture; +use std::collections::HashMap; use std::path::PathBuf; #[derive(Clone, Debug)] @@ -37,16 +40,26 @@ 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 justification: Option, pub exec_approval_requirement: ExecApprovalRequirement, } +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +pub(crate) enum ShellRuntimeBackend { + #[default] + Generic, + ShellCommandClassic, + ShellCommandZshFork, +} + #[derive(Default)] -pub struct ShellRuntime; +pub struct ShellRuntime { + backend: ShellRuntimeBackend, +} #[derive(serde::Serialize, Clone, Debug, Eq, PartialEq, Hash)] pub(crate) struct ApprovalKey { @@ -57,7 +70,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 { @@ -73,6 +92,7 @@ impl Sandboxable for ShellRuntime { fn sandbox_preference(&self) -> SandboxablePreference { SandboxablePreference::Auto } + fn escalate_on_failure(&self) -> bool { true } @@ -165,15 +185,13 @@ 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, - session_shell.as_ref(), + &req.command, + ctx.session.user_shell().as_ref(), &req.cwd, &req.explicit_env_overrides, ); - let command = if matches!(session_shell.shell_type, ShellType::PowerShell) + let command = if matches!(ctx.session.user_shell().shell_type, ShellType::PowerShell) && ctx.session.features().enabled(Feature::PowershellUtf8) { prefix_powershell_script_with_utf8(&command) @@ -181,34 +199,12 @@ 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.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 + && let Some(out) = + unix_escalation::try_run_zsh_fork(req, attempt, ctx, &command).await? + { + return Ok(out); } 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 00000000000..bc15a34ea44 --- /dev/null +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -0,0 +1,332 @@ +use super::ShellRequest; +use crate::error::CodexErr; +use crate::error::SandboxErr; +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::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::unix::core_shell_escalation::ShellActionProvider; +use codex_shell_escalation::unix::core_shell_escalation::ShellPolicyFactory; +use codex_shell_escalation::unix::escalate_protocol::EscalateAction; +use codex_shell_escalation::unix::escalate_server::ExecParams; +use codex_shell_escalation::unix::escalate_server::ExecResult; +use codex_shell_escalation::unix::escalate_server::SandboxState; +use codex_shell_escalation::unix::escalate_server::ShellCommandExecutor; +use codex_shell_escalation::unix::escalate_server::run_escalate_server; +use codex_shell_escalation::unix::stopwatch::Stopwatch; +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; + +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 { + return Ok(None); + }; + if !ctx.session.features().enabled(Feature::ShellZshFork) { + return Ok(None); + } + if !matches!(ctx.session.user_shell().shell_type, ShellType::Zsh) { + return Ok(None); + } + + let spec = build_command_spec( + command, + &req.cwd, + &req.env, + req.timeout_ms.into(), + req.sandbox_permissions, + req.justification.clone(), + )?; + let _sandbox_exec_request = attempt + .env_for(spec, req.network.as_ref()) + .map_err(|err| ToolError::Codex(err.into()))?; + // Build the script from the original shell argv, not the sandbox-transformed + // command. `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 script = 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 sandbox_state = SandboxState { + sandbox_policy: ctx.turn.sandbox_policy.get().clone(), + codex_linux_sandbox_exe: attempt.codex_linux_sandbox_exe.cloned(), + sandbox_cwd: req.cwd.clone(), + use_linux_sandbox_bwrap: attempt.use_linux_sandbox_bwrap, + }; + 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(false), + }, + &sandbox_state, + shell_zsh_path.to_path_buf(), + 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, + &CoreShellCommandExecutor, + ) + .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(); + Ok(stopwatch + .pause_for(async move { + session + .request_command_approval( + &turn, call_id, None, command, workdir, 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 policy = self.policy.read().await; + 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 = policy.check_multiple(commands.iter(), &fallback); + 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 self.approval_policy == AskForApproval::Never { + EscalateAction::Deny { + reason: Some("Execution forbidden by policy".to_string()), + } + } else if decision_driven_by_policy { + EscalateAction::Escalate + } else { + match self.prompt(&command, workdir, stopwatch).await? { + ReviewDecision::Approved + | ReviewDecision::ApprovedExecpolicyAmendment { .. } + | ReviewDecision::ApprovedForSession => { + if needs_escalation { + EscalateAction::Escalate + } else { + EscalateAction::Run + } + } + 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::Run, + }) + } +} + +struct CoreShellCommandExecutor; + +#[async_trait::async_trait] +impl ShellCommandExecutor for CoreShellCommandExecutor { + async fn run( + &self, + command: Vec, + cwd: PathBuf, + env: HashMap, + cancel_rx: CancellationToken, + sandbox_state: &SandboxState, + ) -> anyhow::Result { + let result = crate::exec::process_exec_tool_call( + crate::exec::ExecParams { + command, + cwd, + expiration: crate::exec::ExecExpiration::Cancellation(cancel_rx), + env, + network: None, + sandbox_permissions: SandboxPermissions::UseDefault, + windows_sandbox_level: WindowsSandboxLevel::Disabled, + justification: None, + arg0: None, + }, + &sandbox_state.sandbox_policy, + &sandbox_state.sandbox_cwd, + &sandbox_state.codex_linux_sandbox_exe, + sandbox_state.use_linux_sandbox_bwrap, + None, + ) + .await?; + + Ok(ExecResult { + exit_code: result.exit_code, + output: result.aggregated_output.text, + duration: result.duration, + timed_out: result.timed_out, + }) + } +} + +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() + )) +} + +fn extract_shell_script(command: &[String]) -> Result { + match command { + [_, flag, script, ..] if flag == "-c" || flag == "-lc" => Ok(script.clone()), + _ => 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.output.clone()), + stderr: crate::exec::StreamOutput::new(String::new()), + 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) +} diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 60bd8bb03ec..54a7c4814ea 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)] +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, @@ -67,6 +74,12 @@ impl ToolsConfig { let include_collab_tools = features.enabled(Feature::Collab); let include_collaboration_modes_tools = true; let include_search_tool = features.enabled(Feature::Apps); + 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 @@ -97,6 +110,7 @@ impl ToolsConfig { Self { shell_type, + shell_command_backend, allow_login_shell: true, apply_patch_tool_type, web_search_mode: *web_search_mode, @@ -1459,7 +1473,10 @@ 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(match config.shell_command_backend { + ShellCommandBackendConfig::Classic => ShellCommandHandler::classic(), + ShellCommandBackendConfig::ZshFork => ShellCommandHandler::zsh_fork(), + }); let request_user_input_handler = Arc::new(RequestUserInputHandler); let search_tool_handler = Arc::new(SearchToolBm25Handler); let js_repl_handler = Arc::new(JsReplHandler); @@ -2281,6 +2298,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 8094a4b786c..00000000000 --- a/codex-rs/core/src/zsh_exec_bridge/mod.rs +++ /dev/null @@ -1,557 +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::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::, - ) - .await; - - let (action, reason, user_rejected) = match decision { - ReviewDecision::Approved - | ReviewDecision::ApprovedForSession - | ReviewDecision::ApprovedExecpolicyAmendment { .. } => { - (WrapperExecAction::Run, None, false) - } - 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/exec-server/Cargo.toml b/codex-rs/exec-server/Cargo.toml index a6a0721b985..74b252d97b1 100644 --- a/codex-rs/exec-server/Cargo.toml +++ b/codex-rs/exec-server/Cargo.toml @@ -32,6 +32,8 @@ codex-core = { workspace = true } codex-execpolicy = { workspace = true } codex-protocol = { workspace = true } codex-shell-command = { workspace = true } + +[target.'cfg(unix)'.dependencies] codex-shell-escalation = { workspace = true } rmcp = { workspace = true, default-features = false, features = [ "auth", @@ -51,6 +53,7 @@ serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } shlex = { workspace = true } tokio = { workspace = true, features = ["macros", "rt-multi-thread", "signal"] } +tokio-util = { workspace = true } tracing = { workspace = true } tracing-subscriber = { workspace = true, features = ["env-filter", "fmt"] } diff --git a/codex-rs/exec-server/src/unix.rs b/codex-rs/exec-server/src/unix.rs index 0e0355443bc..2bd892fd649 100644 --- a/codex-rs/exec-server/src/unix.rs +++ b/codex-rs/exec-server/src/unix.rs @@ -67,7 +67,7 @@ use codex_execpolicy::Decision; use codex_execpolicy::Policy; use codex_execpolicy::RuleMatch; use codex_shell_command::is_dangerous_command::command_might_be_dangerous; -use codex_shell_escalation as shell_escalation; +use codex_shell_escalation::unix::escalate_client::run; use rmcp::ErrorData as McpError; use tokio::sync::RwLock; use tracing_subscriber::EnvFilter; @@ -160,7 +160,7 @@ pub async fn main_execve_wrapper() -> anyhow::Result<()> { .init(); let ExecveWrapperCli { file, argv } = ExecveWrapperCli::parse(); - let exit_code = shell_escalation::run(file, argv).await?; + let exit_code = run(file, argv).await?; std::process::exit(exit_code); } diff --git a/codex-rs/exec-server/src/unix/mcp.rs b/codex-rs/exec-server/src/unix/mcp.rs index 547d055c144..30dbc4d8194 100644 --- a/codex-rs/exec-server/src/unix/mcp.rs +++ b/codex-rs/exec-server/src/unix/mcp.rs @@ -6,11 +6,19 @@ use anyhow::Context as _; use anyhow::Result; use codex_core::MCP_SANDBOX_STATE_CAPABILITY; use codex_core::MCP_SANDBOX_STATE_METHOD; -use codex_core::SandboxState; +use codex_core::SandboxState as CoreSandboxState; +use codex_core::exec::process_exec_tool_call; use codex_execpolicy::Policy; +use codex_protocol::config_types::WindowsSandboxLevel; +use codex_protocol::models::SandboxPermissions as ProtocolSandboxPermissions; use codex_protocol::protocol::SandboxPolicy; -use codex_shell_escalation::EscalationPolicyFactory; -use codex_shell_escalation::run_escalate_server; +use codex_shell_escalation::unix::escalate_server::EscalationPolicyFactory; +use codex_shell_escalation::unix::escalate_server::ExecParams as ShellExecParams; +use codex_shell_escalation::unix::escalate_server::ExecResult as ShellExecResult; +use codex_shell_escalation::unix::escalate_server::SandboxState as ShellEscalationSandboxState; +use codex_shell_escalation::unix::escalate_server::ShellCommandExecutor; +use codex_shell_escalation::unix::escalate_server::run_escalate_server; +use codex_shell_escalation::unix::stopwatch::Stopwatch; use rmcp::ErrorData as McpError; use rmcp::RoleServer; use rmcp::ServerHandler; @@ -27,7 +35,9 @@ use rmcp::tool_handler; use rmcp::tool_router; use rmcp::transport::stdio; use serde_json::json; +use std::collections::HashMap; use tokio::sync::RwLock; +use tokio_util::sync::CancellationToken; use crate::unix::mcp_escalation_policy::McpEscalationPolicy; @@ -50,8 +60,8 @@ pub struct ExecResult { pub timed_out: bool, } -impl From for ExecResult { - fn from(result: codex_shell_escalation::ExecResult) -> Self { +impl From for ExecResult { + fn from(result: ShellExecResult) -> Self { Self { exit_code: result.exit_code, output: result.output, @@ -68,7 +78,7 @@ pub struct ExecTool { execve_wrapper: PathBuf, policy: Arc>, preserve_program_paths: bool, - sandbox_state: Arc>>, + sandbox_state: Arc>>, } #[derive(Debug, serde::Serialize, serde::Deserialize, rmcp::schemars::JsonSchema)] @@ -83,7 +93,7 @@ pub struct ExecParams { pub login: Option, } -impl From for codex_shell_escalation::ExecParams { +impl From for ShellExecParams { fn from(inner: ExecParams) -> Self { Self { command: inner.command, @@ -99,14 +109,51 @@ struct McpEscalationPolicyFactory { preserve_program_paths: bool, } +struct McpShellCommandExecutor; + +#[async_trait::async_trait] +impl ShellCommandExecutor for McpShellCommandExecutor { + async fn run( + &self, + command: Vec, + cwd: PathBuf, + env: HashMap, + cancel_rx: CancellationToken, + sandbox_state: &ShellEscalationSandboxState, + ) -> anyhow::Result { + let result = process_exec_tool_call( + codex_core::exec::ExecParams { + command, + cwd, + expiration: codex_core::exec::ExecExpiration::Cancellation(cancel_rx), + env, + network: None, + sandbox_permissions: ProtocolSandboxPermissions::UseDefault, + windows_sandbox_level: WindowsSandboxLevel::Disabled, + justification: None, + arg0: None, + }, + &sandbox_state.sandbox_policy, + &sandbox_state.sandbox_cwd, + &sandbox_state.codex_linux_sandbox_exe, + sandbox_state.use_linux_sandbox_bwrap, + None, + ) + .await?; + + Ok(ShellExecResult { + exit_code: result.exit_code, + output: result.aggregated_output.text, + duration: result.duration, + timed_out: result.timed_out, + }) + } +} + impl EscalationPolicyFactory for McpEscalationPolicyFactory { type Policy = McpEscalationPolicy; - fn create_policy( - &self, - policy: Arc>, - stopwatch: codex_shell_escalation::Stopwatch, - ) -> Self::Policy { + fn create_policy(&self, policy: Arc>, stopwatch: Stopwatch) -> Self::Policy { McpEscalationPolicy::new( policy, self.context.clone(), @@ -151,15 +198,21 @@ impl ExecTool { .read() .await .clone() - .unwrap_or_else(|| SandboxState { + .unwrap_or_else(|| CoreSandboxState { sandbox_policy: SandboxPolicy::new_read_only_policy(), codex_linux_sandbox_exe: None, sandbox_cwd: PathBuf::from(¶ms.workdir), use_linux_sandbox_bwrap: false, }); + let shell_sandbox_state = ShellEscalationSandboxState { + sandbox_policy: sandbox_state.sandbox_policy.clone(), + codex_linux_sandbox_exe: sandbox_state.codex_linux_sandbox_exe.clone(), + sandbox_cwd: sandbox_state.sandbox_cwd.clone(), + use_linux_sandbox_bwrap: sandbox_state.use_linux_sandbox_bwrap, + }; let result = run_escalate_server( params.into(), - &sandbox_state, + &shell_sandbox_state, &self.bash_path, &self.execve_wrapper, self.policy.clone(), @@ -168,6 +221,7 @@ impl ExecTool { preserve_program_paths: self.preserve_program_paths, }, effective_timeout, + &McpShellCommandExecutor, ) .await .map_err(|e| McpError::internal_error(e.to_string(), None))?; @@ -236,7 +290,7 @@ impl ServerHandler for ExecTool { )); }; - let Ok(sandbox_state) = serde_json::from_value::(params.clone()) else { + let Ok(sandbox_state) = serde_json::from_value::(params.clone()) else { return Err(McpError::invalid_params( "failed to deserialize sandbox state".to_string(), Some(params), diff --git a/codex-rs/exec-server/src/unix/mcp_escalation_policy.rs b/codex-rs/exec-server/src/unix/mcp_escalation_policy.rs index 98638182619..73b2c34e428 100644 --- a/codex-rs/exec-server/src/unix/mcp_escalation_policy.rs +++ b/codex-rs/exec-server/src/unix/mcp_escalation_policy.rs @@ -2,9 +2,9 @@ use std::path::Path; use codex_core::sandboxing::SandboxPermissions; use codex_execpolicy::Policy; -use codex_shell_escalation::EscalateAction; -use codex_shell_escalation::EscalationPolicy; -use codex_shell_escalation::Stopwatch; +use codex_shell_escalation::unix::escalate_protocol::EscalateAction; +use codex_shell_escalation::unix::escalation_policy::EscalationPolicy; +use codex_shell_escalation::unix::stopwatch::Stopwatch; use rmcp::ErrorData as McpError; use rmcp::RoleServer; use rmcp::model::CreateElicitationRequestParams; diff --git a/codex-rs/shell-escalation/Cargo.toml b/codex-rs/shell-escalation/Cargo.toml index 49b8f10bd31..3cdd50250ab 100644 --- a/codex-rs/shell-escalation/Cargo.toml +++ b/codex-rs/shell-escalation/Cargo.toml @@ -7,20 +7,21 @@ license.workspace = true [dependencies] anyhow = { workspace = true } async-trait = { workspace = true } -codex-core = { workspace = true } codex-execpolicy = { workspace = true } codex-protocol = { workspace = true } libc = { workspace = true } serde_json = { workspace = true } path-absolutize = { workspace = true } serde = { workspace = true, features = ["derive"] } -socket2 = { workspace = true } +socket2 = { workspace = true, features = ["all"] } tokio = { workspace = true, features = [ "io-std", + "net", "macros", "process", "rt-multi-thread", "signal", + "time", ] } tokio-util = { workspace = true } tracing = { workspace = true } diff --git a/codex-rs/shell-escalation/src/lib.rs b/codex-rs/shell-escalation/src/lib.rs index 555d0f89e0d..fae4ef574b5 100644 --- a/codex-rs/shell-escalation/src/lib.rs +++ b/codex-rs/shell-escalation/src/lib.rs @@ -1,21 +1,5 @@ #[cfg(unix)] -mod unix { - mod escalate_client; - mod escalate_protocol; - mod escalate_server; - mod escalation_policy; - mod socket; - mod stopwatch; - - pub use self::escalate_client::run; - pub use self::escalate_protocol::EscalateAction; - pub use self::escalate_server::EscalationPolicyFactory; - pub use self::escalate_server::ExecParams; - pub use self::escalate_server::ExecResult; - pub use self::escalate_server::run_escalate_server; - pub use self::escalation_policy::EscalationPolicy; - pub use self::stopwatch::Stopwatch; -} +pub mod unix; #[cfg(unix)] pub use unix::*; 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 0be7af28fa1..ca6bd347daa 100644 --- a/codex-rs/shell-escalation/src/unix/core_shell_escalation.rs +++ b/codex-rs/shell-escalation/src/unix/core_shell_escalation.rs @@ -40,7 +40,7 @@ impl ShellPolicyFactory { } } -struct ShellEscalationPolicy { +pub struct ShellEscalationPolicy { provider: Arc, stopwatch: Stopwatch, } diff --git a/codex-rs/shell-escalation/src/unix/escalate_server.rs b/codex-rs/shell-escalation/src/unix/escalate_server.rs index 0ee5fc27c40..d437795bab7 100644 --- a/codex-rs/shell-escalation/src/unix/escalate_server.rs +++ b/codex-rs/shell-escalation/src/unix/escalate_server.rs @@ -7,8 +7,8 @@ use std::sync::Arc; use std::time::Duration; use anyhow::Context as _; -use codex_core::SandboxState; use codex_execpolicy::Policy; +use codex_protocol::protocol::SandboxPolicy; use path_absolutize::Absolutize as _; use tokio::process::Command; use tokio::sync::RwLock; @@ -27,6 +27,26 @@ use crate::unix::socket::AsyncDatagramSocket; use crate::unix::socket::AsyncSocket; use crate::unix::stopwatch::Stopwatch; +#[derive(Debug, Clone)] +pub struct SandboxState { + pub sandbox_policy: SandboxPolicy, + pub codex_linux_sandbox_exe: Option, + pub sandbox_cwd: PathBuf, + pub use_linux_sandbox_bwrap: bool, +} + +#[async_trait::async_trait] +pub trait ShellCommandExecutor: Send + Sync { + async fn run( + &self, + command: Vec, + cwd: PathBuf, + env: HashMap, + cancel_rx: CancellationToken, + sandbox_state: &SandboxState, + ) -> anyhow::Result; +} + #[derive(Debug, serde::Deserialize, serde::Serialize)] pub struct ExecParams { /// The bash string to execute. @@ -71,11 +91,10 @@ impl EscalateServer { params: ExecParams, cancel_rx: CancellationToken, sandbox_state: &SandboxState, + command_executor: &dyn ShellCommandExecutor, ) -> anyhow::Result { let (escalate_server, escalate_client) = AsyncDatagramSocket::pair()?; let client_socket = escalate_client.into_inner(); - client_socket.set_cloexec(false)?; - let escalate_task = tokio::spawn(escalate_task(escalate_server, self.policy.clone())); let mut env = std::env::vars().collect::>(); env.insert( @@ -91,47 +110,27 @@ impl EscalateServer { self.execve_wrapper.to_string_lossy().to_string(), ); - let ExecParams { - command, - workdir, - timeout_ms: _, - login, - } = params; - let result = codex_core::exec::process_exec_tool_call( - codex_core::exec::ExecParams { - command: vec![ - self.bash_path.to_string_lossy().to_string(), - if login == Some(false) { - "-c".to_string() - } else { - "-lc".to_string() - }, - command, - ], - cwd: PathBuf::from(&workdir), - expiration: codex_core::exec::ExecExpiration::Cancellation(cancel_rx), - env, - network: None, - sandbox_permissions: codex_core::sandboxing::SandboxPermissions::UseDefault, - windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel::Disabled, - justification: None, - arg0: None, + let command = vec![ + self.bash_path.to_string_lossy().to_string(), + if params.login == Some(false) { + "-c".to_string() + } else { + "-lc".to_string() }, - &sandbox_state.sandbox_policy, - &sandbox_state.sandbox_cwd, - &sandbox_state.codex_linux_sandbox_exe, - sandbox_state.use_linux_sandbox_bwrap, - None, - ) - .await?; + params.command, + ]; + let result = command_executor + .run( + command, + PathBuf::from(¶ms.workdir), + env, + cancel_rx, + sandbox_state, + ) + .await?; escalate_task.abort(); - Ok(ExecResult { - exit_code: result.exit_code, - output: result.aggregated_output.text, - duration: result.duration, - timed_out: result.timed_out, - }) + Ok(result) } } @@ -142,6 +141,7 @@ pub trait EscalationPolicyFactory { fn create_policy(&self, policy: Arc>, stopwatch: Stopwatch) -> Self::Policy; } +#[allow(clippy::too_many_arguments)] pub async fn run_escalate_server( exec_params: ExecParams, sandbox_state: &SandboxState, @@ -150,6 +150,7 @@ pub async fn run_escalate_server( policy: Arc>, escalation_policy_factory: impl EscalationPolicyFactory, effective_timeout: Duration, + command_executor: &dyn ShellCommandExecutor, ) -> anyhow::Result { let stopwatch = Stopwatch::new(effective_timeout); let cancel_token = stopwatch.cancellation_token(); @@ -160,7 +161,7 @@ pub async fn run_escalate_server( ); escalate_server - .exec(exec_params, cancel_token, sandbox_state) + .exec(exec_params, cancel_token, sandbox_state, command_executor) .await } @@ -272,6 +273,7 @@ async fn handle_escalate_session_with_policy( .await?; } } + Ok(()) } @@ -279,7 +281,6 @@ async fn handle_escalate_session_with_policy( mod tests { use super::*; use pretty_assertions::assert_eq; - use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; diff --git a/codex-rs/shell-escalation/src/unix/mod.rs b/codex-rs/shell-escalation/src/unix/mod.rs index 0ae7941da26..555bd7246eb 100644 --- a/codex-rs/shell-escalation/src/unix/mod.rs +++ b/codex-rs/shell-escalation/src/unix/mod.rs @@ -1,7 +1,7 @@ +pub mod core_shell_escalation; pub mod escalate_client; pub mod escalate_protocol; pub mod escalate_server; pub mod escalation_policy; pub mod socket; -pub mod core_shell_escalation; pub mod stopwatch; diff --git a/codex-rs/shell-escalation/src/unix/socket.rs b/codex-rs/shell-escalation/src/unix/socket.rs index 35292367a6b..fd3fd7cb0af 100644 --- a/codex-rs/shell-escalation/src/unix/socket.rs +++ b/codex-rs/shell-escalation/src/unix/socket.rs @@ -96,8 +96,8 @@ async fn read_frame_header( while filled < LENGTH_PREFIX_SIZE { let mut guard = async_socket.readable().await?; // The first read should come with a control message containing any FDs. - let result = if !captured_control { - guard.try_io(|inner| { + let read = if !captured_control { + match guard.try_io(|inner| { let mut bufs = [MaybeUninitSlice::new(&mut header[filled..])]; let (read, control_len) = { let mut msg = MsgHdrMut::new() @@ -109,16 +109,18 @@ async fn read_frame_header( control.truncate(control_len); captured_control = true; Ok(read) - }) + }) { + Ok(Ok(read)) => read, + Ok(Err(err)) => return Err(err), + Err(_would_block) => continue, + } } else { - guard.try_io(|inner| inner.get_ref().recv(&mut header[filled..])) - }; - let Ok(result) = result else { - // Would block, try again. - continue; + match guard.try_io(|inner| inner.get_ref().recv(&mut header[filled..])) { + Ok(Ok(read)) => read, + Ok(Err(err)) => return Err(err), + Err(_would_block) => continue, + } }; - - let read = result?; if read == 0 { return Err(std::io::Error::new( std::io::ErrorKind::UnexpectedEof, @@ -150,12 +152,11 @@ async fn read_frame_payload( let mut filled = 0; while filled < message_len { let mut guard = async_socket.readable().await?; - let result = guard.try_io(|inner| inner.get_ref().recv(&mut payload[filled..])); - let Ok(result) = result else { - // Would block, try again. - continue; + let read = match guard.try_io(|inner| inner.get_ref().recv(&mut payload[filled..])) { + Ok(Ok(read)) => read, + Ok(Err(err)) => return Err(err), + Err(_would_block) => continue, }; - let read = result?; if read == 0 { return Err(std::io::Error::new( std::io::ErrorKind::UnexpectedEof, @@ -261,7 +262,7 @@ impl AsyncSocket { } pub fn pair() -> std::io::Result<(AsyncSocket, AsyncSocket)> { - let (server, client) = Socket::pair(Domain::UNIX, Type::STREAM, None)?; + let (server, client) = Socket::pair_raw(Domain::UNIX, Type::STREAM, None)?; Ok((AsyncSocket::new(server)?, AsyncSocket::new(client)?)) } @@ -314,11 +315,11 @@ async fn send_stream_frame( let mut include_fds = !fds.is_empty(); while written < frame.len() { let mut guard = socket.writable().await?; - let result = guard.try_io(|inner| { - send_stream_chunk(inner.get_ref(), &frame[written..], fds, include_fds) - }); - let bytes_written = match result { - Ok(bytes_written) => bytes_written?, + let bytes_written = match guard + .try_io(|inner| send_stream_chunk(inner.get_ref(), &frame[written..], fds, include_fds)) + { + Ok(Ok(bytes_written)) => bytes_written, + Ok(Err(err)) => return Err(err), Err(_would_block) => continue, }; if bytes_written == 0 { @@ -370,7 +371,7 @@ impl AsyncDatagramSocket { } pub fn pair() -> std::io::Result<(Self, Self)> { - let (server, client) = Socket::pair(Domain::UNIX, Type::DGRAM, None)?; + let (server, client) = Socket::pair_raw(Domain::UNIX, Type::DGRAM, None)?; Ok((Self::new(server)?, Self::new(client)?)) } @@ -472,7 +473,7 @@ mod tests { #[test] fn send_datagram_bytes_rejects_excessive_fd_counts() -> std::io::Result<()> { - let (socket, _peer) = Socket::pair(Domain::UNIX, Type::DGRAM, None)?; + let (socket, _peer) = Socket::pair_raw(Domain::UNIX, Type::DGRAM, None)?; let fds = fd_list(MAX_FDS_PER_MESSAGE + 1)?; let err = send_datagram_bytes(&socket, b"hi", &fds).unwrap_err(); assert_eq!(std::io::ErrorKind::InvalidInput, err.kind()); @@ -481,7 +482,7 @@ mod tests { #[test] fn send_stream_chunk_rejects_excessive_fd_counts() -> std::io::Result<()> { - let (socket, _peer) = Socket::pair(Domain::UNIX, Type::STREAM, None)?; + let (socket, _peer) = Socket::pair_raw(Domain::UNIX, Type::STREAM, None)?; let fds = fd_list(MAX_FDS_PER_MESSAGE + 1)?; let err = send_stream_chunk(&socket, b"hello", &fds, true).unwrap_err(); assert_eq!(std::io::ErrorKind::InvalidInput, err.kind());