diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 41221841ef..c4bf9ed5a1 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -634,9 +634,11 @@ impl EscalationPolicy for CoreShellActionProvider { let decision_driven_by_policy = Self::decision_driven_by_policy(&evaluation.matched_rules, evaluation.decision); let unsandboxed_allowed = unsandboxed_execution_allowed(&self.file_system_sandbox_policy); - let needs_escalation = unsandboxed_allowed + let needs_unsandboxed_escalation = unsandboxed_allowed && (self.sandbox_permissions.requires_escalated_permissions() || decision_driven_by_policy); + let needs_escalation = + self.sandbox_permissions.uses_additional_permissions() || needs_unsandboxed_escalation; let decision_source = if decision_driven_by_policy { DecisionSource::PrefixRule @@ -862,7 +864,7 @@ impl ShellCommandExecutor for CoreShellCommandExecutor { EscalationExecution::Unsandboxed => PreparedExec { command, cwd: workdir.to_path_buf(), - env, + env: exec_env_for_sandbox_permissions(&env, SandboxPermissions::RequireEscalated), arg0: Some(first_arg.clone()), }, EscalationExecution::TurnDefault => { diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs index ecdf015fd5..87e0854176 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs @@ -1,4 +1,5 @@ use super::CoreShellActionProvider; +use super::CoreShellCommandExecutor; use super::InterceptedExecPolicyContext; use super::ParsedShellCommand; use super::commands_for_intercepted_exec_policy; @@ -16,6 +17,9 @@ use codex_execpolicy::PolicyParser; use codex_execpolicy::RuleMatch; use codex_hooks::Hooks; use codex_hooks::HooksConfig; +use codex_network_proxy::PROXY_ACTIVE_ENV_KEY; +use codex_network_proxy::PROXY_ENV_KEYS; +use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::AdditionalPermissionProfile; use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::PermissionProfile; @@ -29,13 +33,16 @@ use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::GranularApprovalConfig; use codex_protocol::protocol::GuardianCommandSource; use codex_sandboxing::SandboxType; +use codex_sandboxing::policy_transforms::effective_permission_profile; use codex_shell_escalation::EscalationExecution; use codex_shell_escalation::EscalationPermissions; use codex_shell_escalation::ExecResult; use codex_shell_escalation::ResolvedPermissionProfile; +use codex_shell_escalation::ShellCommandExecutor; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use serde_json::Value; +use std::collections::HashMap; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; @@ -348,6 +355,100 @@ fn shell_request_escalation_execution_is_explicit() { ); } +#[tokio::test] +async fn unsandboxed_intercepted_exec_strips_managed_network_env() -> anyhow::Result<()> { + let workdir = test_sandbox_cwd(); + let executor = CoreShellCommandExecutor { + command: Vec::new(), + cwd: workdir.clone(), + permission_profile: PermissionProfile::workspace_write(), + file_system_sandbox_policy: read_only_file_system_sandbox_policy(), + network_sandbox_policy: NetworkSandboxPolicy::Restricted, + sandbox: SandboxType::None, + env: HashMap::new(), + network: None, + windows_sandbox_level: WindowsSandboxLevel::Disabled, + arg0: None, + sandbox_policy_cwd: workdir.clone(), + windows_sandbox_workspace_roots: vec![workdir.clone()], + codex_linux_sandbox_exe: None, + use_legacy_landlock: false, + }; + let mut env = HashMap::new(); + env.insert(PROXY_ACTIVE_ENV_KEY.to_string(), "1".to_string()); + for key in PROXY_ENV_KEYS { + env.insert((*key).to_string(), format!("proxy-{key}")); + } + + let prepared = executor + .prepare_escalated_exec( + &AbsolutePathBuf::from_absolute_path("/usr/bin/curl")?, + &["curl".to_string(), "example.com".to_string()], + &workdir, + env, + EscalationExecution::Unsandboxed, + ) + .await?; + + assert!(!prepared.env.contains_key(PROXY_ACTIVE_ENV_KEY)); + for key in PROXY_ENV_KEYS { + assert!(!prepared.env.contains_key(*key)); + } + + Ok(()) +} + +#[tokio::test] +async fn preapproved_additional_permissions_escalate_intercepted_exec() -> anyhow::Result<()> { + let (session, turn_context) = make_session_and_context().await; + let requested_permissions = AdditionalPermissionProfile { + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![ + AbsolutePathBuf::from_absolute_path("/tmp/output").unwrap(), + ]), + )), + ..Default::default() + }; + let workdir = test_sandbox_cwd(); + let permission_profile = effective_permission_profile( + &PermissionProfile::workspace_write(), + Some(&requested_permissions), + ); + let provider = CoreShellActionProvider { + policy: Arc::new(RwLock::new(codex_execpolicy::Policy::empty())), + session: Arc::new(session), + turn: Arc::new(turn_context), + call_id: "preapproved-additional-permissions".to_string(), + tool_name: GuardianCommandSource::Shell, + approval_policy: AskForApproval::OnRequest, + permission_profile: permission_profile.clone(), + file_system_sandbox_policy: read_only_file_system_sandbox_policy(), + sandbox_policy_cwd: workdir.clone(), + sandbox_permissions: SandboxPermissions::WithAdditionalPermissions, + approval_sandbox_permissions: SandboxPermissions::UseDefault, + prompt_permissions: Some(requested_permissions), + stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)), + }; + + let action = codex_shell_escalation::EscalationPolicy::determine_action( + &provider, + &AbsolutePathBuf::from_absolute_path("/usr/bin/printf")?, + &["printf".to_string(), "hello".to_string()], + &workdir, + ) + .await?; + + let expected = codex_shell_escalation::EscalationDecision::Escalate( + EscalationExecution::Permissions(EscalationPermissions::ResolvedPermissionProfile( + ResolvedPermissionProfile { permission_profile }, + )), + ); + assert_eq!(action, expected); + + Ok(()) +} + #[tokio::test(flavor = "current_thread")] async fn execve_permission_request_hook_short_circuits_prompt() -> anyhow::Result<()> { let (session, mut turn_context) = make_session_and_context().await; diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index bf04a6fe35..454c966c54 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -265,17 +265,15 @@ impl<'a> ToolRuntime for UnifiedExecRunt let base_command = &req.command; let session_shell = ctx.session.user_shell(); let (file_system_sandbox_policy, _) = attempt.permissions.to_runtime_permissions(); - let sandbox_permissions = sandbox_permissions_preserving_denied_reads( + let launch_sandbox_permissions = sandbox_permissions_preserving_denied_reads( req.sandbox_permissions, &file_system_sandbox_policy, ); - let req = &UnifiedExecRequest { - sandbox_permissions, - ..req.clone() - }; - let managed_network = - managed_network_for_sandbox_permissions(req.network.as_ref(), req.sandbox_permissions); - let mut env = exec_env_for_sandbox_permissions(&req.env, req.sandbox_permissions); + let managed_network = managed_network_for_sandbox_permissions( + req.network.as_ref(), + launch_sandbox_permissions, + ); + let mut env = exec_env_for_sandbox_permissions(&req.env, launch_sandbox_permissions); if let Some(network) = managed_network { network.apply_to_env(&mut env); } @@ -412,6 +410,7 @@ mod tests { use crate::exec::DEFAULT_EXEC_COMMAND_TIMEOUT_MS; use crate::tools::sandboxing::ToolRuntime; use codex_exec_server::Environment; + use codex_tools::ZshForkConfig; use std::time::Duration; use tempfile::tempdir; @@ -473,4 +472,103 @@ mod tests { assert_eq!(runtime.sandbox_cwd(&request), Some(&sandbox_cwd)); } + + #[tokio::test] + async fn zsh_fork_first_attempt_preserves_parent_sandbox_override() { + let manager = UnifiedExecProcessManager::default(); + let request = test_request( + SandboxPermissions::RequireEscalated, + ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: None, + }, + ); + let direct_runtime = UnifiedExecRuntime::new(&manager, UnifiedExecShellMode::Direct); + let zsh_fork_runtime = UnifiedExecRuntime::new(&manager, zsh_fork_mode()); + + assert_eq!( + direct_runtime.sandbox_permissions(&request), + SandboxPermissions::RequireEscalated + ); + assert_eq!( + zsh_fork_runtime.sandbox_permissions(&request), + SandboxPermissions::RequireEscalated + ); + } + + #[tokio::test] + async fn zsh_fork_first_attempt_preserves_additional_permissions_request() { + let manager = UnifiedExecProcessManager::default(); + let request = test_request( + SandboxPermissions::WithAdditionalPermissions, + ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: None, + }, + ); + let zsh_fork_runtime = UnifiedExecRuntime::new(&manager, zsh_fork_mode()); + + assert_eq!( + zsh_fork_runtime.sandbox_permissions(&request), + SandboxPermissions::WithAdditionalPermissions + ); + } + + #[tokio::test] + async fn zsh_fork_execpolicy_allow_preserves_parent_sandbox_override() { + let manager = UnifiedExecProcessManager::default(); + let request = test_request( + SandboxPermissions::UseDefault, + ExecApprovalRequirement::Skip { + bypass_sandbox: true, + proposed_execpolicy_amendment: None, + }, + ); + let runtime = UnifiedExecRuntime::new(&manager, zsh_fork_mode()); + + assert_eq!( + runtime.exec_approval_requirement(&request), + Some(ExecApprovalRequirement::Skip { + bypass_sandbox: true, + proposed_execpolicy_amendment: None, + }) + ); + } + + fn test_request( + sandbox_permissions: SandboxPermissions, + exec_approval_requirement: ExecApprovalRequirement, + ) -> UnifiedExecRequest { + let cwd = AbsolutePathBuf::try_from(std::env::current_dir().unwrap()) + .expect("current dir is absolute"); + UnifiedExecRequest { + command: vec!["zsh".to_string(), "-c".to_string(), "echo hi".to_string()], + shell_type: ShellType::Zsh, + hook_command: "echo hi".to_string(), + process_id: 1000, + cwd: cwd.clone(), + sandbox_cwd: cwd, + environment: Arc::new(Environment::default_for_tests()), + env: HashMap::new(), + exec_server_env_config: None, + explicit_env_overrides: HashMap::new(), + network: None, + tty: false, + sandbox_permissions, + additional_permissions: None, + #[cfg(unix)] + additional_permissions_preapproved: false, + justification: None, + exec_approval_requirement, + } + } + + fn zsh_fork_mode() -> UnifiedExecShellMode { + let cwd = std::env::current_dir().expect("read current dir"); + UnifiedExecShellMode::ZshFork(ZshForkConfig { + shell_zsh_path: AbsolutePathBuf::try_from(cwd.join("zsh")).expect("absolute zsh path"), + main_execve_wrapper_exe: AbsolutePathBuf::try_from(cwd.join("execve-wrapper")) + .expect("absolute wrapper path"), + }) + } } diff --git a/codex-rs/core/tests/common/zsh_fork.rs b/codex-rs/core/tests/common/zsh_fork.rs index e58ebd81dd..5abfc753b9 100644 --- a/codex-rs/core/tests/common/zsh_fork.rs +++ b/codex-rs/core/tests/common/zsh_fork.rs @@ -94,6 +94,33 @@ where builder.build(server).await } +pub async fn build_unified_exec_zsh_fork_test( + server: &wiremock::MockServer, + runtime: ZshForkRuntime, + approval_policy: AskForApproval, + permission_profile: PermissionProfile, + pre_build_hook: F, +) -> Result +where + F: FnOnce(&Path) + Send + 'static, +{ + let mut builder = test_codex() + .with_pre_build_hook(pre_build_hook) + .with_config(move |config| { + runtime.apply_to_config(config, approval_policy, permission_profile); + config.use_experimental_unified_exec_tool = true; + config + .features + .enable(Feature::UnifiedExec) + .expect("test config should allow feature update"); + config + .features + .enable(Feature::UnifiedExecZshFork) + .expect("test config should allow feature update"); + }); + builder.build(server).await +} + fn find_test_zsh_path() -> Result> { let repo_root = codex_utils_cargo_bin::repo_root()?; let dotslash_zsh = repo_root.join("codex-rs/app-server/tests/suite/zsh"); diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index aceac7320e..96ac47cf45 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -15,6 +15,7 @@ use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::ApplyPatchApprovalRequestEvent; use codex_protocol::protocol::AskForApproval; @@ -43,6 +44,7 @@ use core_test_support::test_codex::test_codex; use core_test_support::test_codex::turn_permission_fields; use core_test_support::wait_for_event; use core_test_support::wait_for_event_with_timeout; +use core_test_support::zsh_fork::build_unified_exec_zsh_fork_test; use core_test_support::zsh_fork::build_zsh_fork_test; use core_test_support::zsh_fork::restrictive_workspace_write_profile; use core_test_support::zsh_fork::zsh_fork_runtime; @@ -2800,6 +2802,146 @@ async fn matched_prefix_rule_runs_unsandboxed_under_zsh_fork() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[cfg(unix)] +async fn unified_exec_zsh_fork_parent_approval_preserves_denied_reads() -> Result<()> { + skip_if_no_network!(Ok(())); + + let Some(runtime) = zsh_fork_runtime("unified-exec zsh-fork denied-read approval test")? else { + return Ok(()); + }; + + let denied_dir = tempfile::tempdir_in(std::env::current_dir()?)?; + let denied_path = denied_dir.path().join("secret.env"); + let secret = "unified-exec-zsh-fork-denied-read-secret"; + fs::write(&denied_path, format!("{secret}\n"))?; + let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Read, + }, + FileSystemSandboxEntry { + path: FileSystemPath::GlobPattern { + pattern: denied_path.to_string_lossy().to_string(), + }, + access: FileSystemAccessMode::Deny, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::project_roots(/*subpath*/ None), + }, + access: FileSystemAccessMode::Write, + }, + ]); + assert!( + file_system_sandbox_policy.has_denied_read_restrictions(), + "test must exercise a permission profile with denied reads" + ); + let permission_profile = PermissionProfile::from_runtime_permissions( + &file_system_sandbox_policy, + NetworkSandboxPolicy::Restricted, + ); + + let approval_policy = AskForApproval::OnRequest; + let command = format!("cat {denied_path:?}"); + + let server = start_mock_server().await; + let test = build_unified_exec_zsh_fork_test( + &server, + runtime, + approval_policy, + permission_profile, + move |_home| {}, + ) + .await?; + + let call_id = "uexec-zsh-fork-parent-approval-denied-read"; + let event = exec_command_event( + call_id, + &command, + Some(30_000), + SandboxPermissions::RequireEscalated, + Some("attempt a denied read for the test"), + )?; + let _ = mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-uexec-zsh-fork-denied-read-1"), + event, + ev_completed("resp-uexec-zsh-fork-denied-read-1"), + ]), + ) + .await; + let results = mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("msg-uexec-zsh-fork-denied-read-1", "done"), + ev_completed("resp-uexec-zsh-fork-denied-read-2"), + ]), + ) + .await; + + let session_model = test.session_configured.model.clone(); + let (sandbox_policy, permission_profile) = turn_permission_fields( + test.session_configured.permission_profile.clone(), + test.cwd.path(), + ); + test.codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "run approved unified exec denied read through zsh fork".into(), + text_elements: Vec::new(), + }], + environments: None, + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: codex_protocol::protocol::ThreadSettingsOverrides { + cwd: Some(test.cwd.path().to_path_buf()), + approval_policy: Some(approval_policy), + approvals_reviewer: Some(ApprovalsReviewer::User), + sandbox_policy: Some(sandbox_policy), + permission_profile, + collaboration_mode: Some(codex_protocol::config_types::CollaborationMode { + mode: codex_protocol::config_types::ModeKind::Default, + settings: codex_protocol::config_types::Settings { + model: session_model, + reasoning_effort: None, + developer_instructions: None, + }, + }), + ..Default::default() + }, + }) + .await?; + + let approval = expect_exec_approval(&test, &command).await; + test.codex + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: None, + decision: ReviewDecision::Approved, + }) + .await?; + wait_for_completion_without_approval(&test).await; + + let result = parse_result(&results.single_request().function_call_output(call_id)); + assert_ne!( + result.exit_code.unwrap_or(0), + 0, + "denied-read command should stay sandboxed after parent approval" + ); + assert!( + !result.stdout.contains(secret), + "denied-read command unexpectedly printed the secret: {}", + result.stdout + ); + + Ok(()) +} + #[tokio::test(flavor = "current_thread")] #[cfg(unix)] async fn invalid_requested_prefix_rule_falls_back_for_compound_command() -> Result<()> {