From cba716c0f1caa4f2528dedbb1075b50126709d7e Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Wed, 7 Oct 2026 21:37:44 +0000 Subject: [PATCH] Run sandbox integrity checks before commands and filesystem operations (#51843) ## What changed Wire existing sandbox integrity checks into local command preparation, exec-server process preparation, and sandboxed filesystem helper and file-open operations. Make execution request preparation asynchronous so callers can await the checks. The checks remain observational: failures and timeouts do not prevent command execution. ## Testing Add regression tests that launch a real command after checker preparation fails or exceeds its timeout, verifying successful output and exit status. GitOrigin-RevId: a82399f8aeb8b08f9965e6121b4c7f460552af3f --- .../command_exec_processor.rs | 1 + codex-rs/core/src/exec.rs | 7 +- codex-rs/core/src/exec_tests.rs | 115 +++++++++++++++++- codex-rs/core/src/sandboxing/mod.rs | 3 +- codex-rs/core/src/shell_snapshot_sandbox.rs | 1 + codex-rs/core/src/tools/runtimes/mod_tests.rs | 1 + .../core/src/tools/runtimes/unified_exec.rs | 1 + .../runtimes/zsh_fork/unix_escalation.rs | 14 ++- codex-rs/core/src/tools/sandboxing.rs | 13 +- codex-rs/core/src/tools/sandboxing_tests.rs | 13 +- .../core/src/unified_exec/process_manager.rs | 6 +- codex-rs/exec-server/src/fs_sandbox.rs | 1 + codex-rs/exec-server/src/process_sandbox.rs | 1 + .../exec-server/src/sandboxed_file_system.rs | 1 + codex-rs/exec/tests/suite/sandbox.rs | 1 + 15 files changed, 151 insertions(+), 28 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/command_exec_processor.rs b/codex-rs/app-server/src/request_processors/command_exec_processor.rs index 0115a13bd6..1d5c3c7056 100644 --- a/codex-rs/app-server/src/request_processors/command_exec_processor.rs +++ b/codex-rs/app-server/src/request_processors/command_exec_processor.rs @@ -324,6 +324,7 @@ impl CommandExecRequestProcessor { self.config.effective_local_windows_sandbox_type(), use_legacy_landlock, ) + .await .map_err(|err| internal_error(format!("exec failed: {err}")))?; self.command_exec_manager .start(StartCommandExecParams { diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index fa458ed1b7..f72396e383 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -329,7 +329,8 @@ pub async fn process_exec_tool_call( codex_self_exe, windows_sandbox_type, use_legacy_landlock, - )?; + ) + .await?; // Route through the sandboxing module for a single, unified execution path. crate::sandboxing::execute_env(exec_req, stdout_stream).await @@ -338,7 +339,7 @@ pub async fn process_exec_tool_call( /// Transform a portable exec request into the concrete argv/env that should be /// spawned under the requested sandbox policy. #[allow(clippy::too_many_arguments)] -pub fn build_exec_request( +pub async fn build_exec_request( params: ExecParams, permission_profile: &PermissionProfile, sandbox_cwd: &AbsolutePathBuf, @@ -439,7 +440,7 @@ pub fn build_exec_request( } else { Vec::new() }; - ExecRequest::from_sandbox_exec_request(request, options, windows_sandbox_workspace_roots) + ExecRequest::from_sandbox_exec_request(request, options, windows_sandbox_workspace_roots).await } pub(crate) async fn execute_exec_request( diff --git a/codex-rs/core/src/exec_tests.rs b/codex-rs/core/src/exec_tests.rs index db07135e3b..f86df094b8 100644 --- a/codex-rs/core/src/exec_tests.rs +++ b/codex-rs/core/src/exec_tests.rs @@ -1,7 +1,13 @@ use super::*; use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::FileSystemAccessMode; +use codex_protocol::permissions::FileSystemPath; +use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::sandbox::SandboxOverride; +use codex_sandboxing::FileContentsChecker; +use codex_sandboxing::SandboxExecRequest; use codex_sandboxing::SandboxType; use core_test_support::PathBufExt; use core_test_support::PathExt; @@ -1197,11 +1203,11 @@ fn process_exec_tool_call_uses_platform_sandbox_for_network_only_restrictions() ); } -#[test] -fn build_exec_request_projects_workspace_roots_only_for_windows_sandbox() -> Result<()> { +#[tokio::test] +async fn build_exec_request_projects_workspace_roots_only_for_windows_sandbox() -> Result<()> { let temp_dir = tempfile::TempDir::new()?; let cwd = temp_dir.path().abs(); - let build_request = |profile: &PermissionProfile, roots: &[PathUri]| { + let build_request = async |profile: &PermissionProfile, roots: &[PathUri]| { build_exec_request( ExecParams { command: vec!["echo".to_string(), "ok".to_string()], @@ -1224,6 +1230,7 @@ fn build_exec_request_projects_workspace_roots_only_for_windows_sandbox() -> Res SandboxType::WindowsRestrictedToken, /*use_legacy_landlock*/ false, ) + .await }; let native_roots = vec![cwd.clone(), temp_dir.path().join("additional").abs()]; let request = build_request( @@ -1232,7 +1239,8 @@ fn build_exec_request_projects_workspace_roots_only_for_windows_sandbox() -> Res .iter() .map(PathUri::from_abs_path) .collect::>(), - )?; + ) + .await?; assert_eq!( request.windows_sandbox_workspace_roots, if cfg!(windows) { @@ -1250,10 +1258,12 @@ fn build_exec_request_projects_workspace_roots_only_for_windows_sandbox() -> Res .expect("foreign workspace URI"); let roots = [PathUri::from_abs_path(&cwd), foreign_root]; assert_eq!( - build_request(&PermissionProfile::Disabled, &roots)?.windows_sandbox_workspace_roots, + build_request(&PermissionProfile::Disabled, &roots) + .await? + .windows_sandbox_workspace_roots, Vec::new(), ); - let request = build_request(&PermissionProfile::read_only(), &roots); + let request = build_request(&PermissionProfile::read_only(), &roots).await; if cfg!(windows) { let error = request.expect_err("native Windows sandbox must reject foreign roots"); assert!(matches!( @@ -1267,6 +1277,99 @@ fn build_exec_request_projects_workspace_roots_only_for_windows_sandbox() -> Res Ok(()) } +#[test_case(false; "checker_error")] +#[test_case(true; "checker_timeout")] +fn command_runs_after_integrity_check_failure(time_out: bool) -> anyhow::Result<()> { + let directory = tempfile::tempdir()?; + let cwd = directory.path().abs(); + let mut policy = FileSystemSandboxPolicy::read_only(); + policy.entries.push(FileSystemSandboxEntry::new( + FileSystemPath::GlobPattern { + pattern: format!("{}/[z-a]", cwd.display()), + }, + FileSystemAccessMode::Deny, + )); + assert!(FileContentsChecker::new(&policy, &cwd).is_err()); + let permission_profile = + PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Restricted); + // This boundary receives prepared argv. Exercise a real child without requiring + // a native sandbox installation to test the checker's failure isolation. + let request = SandboxExecRequest { + command: if cfg!(windows) { + vec![ + "cmd.exe".into(), + "/D".into(), + "/C".into(), + "echo integrity-ok".into(), + ] + } else { + vec!["/bin/echo".into(), "integrity-ok".into()] + }, + cwd: PathUri::from_abs_path(&cwd), + sandbox_policy_cwd: PathUri::from_abs_path(&cwd), + env: std::env::vars().collect(), + network: None, + network_environment_id: None, + sandbox: select_process_exec_tool_sandbox_type( + &permission_profile, + SandboxType::WindowsMxc, + /*enforce_managed_network*/ false, + ), + sandbox_override: SandboxOverride::NoOverride, + windows_sandbox_level: WindowsSandboxLevel::Disabled, + permission_profile, + arg0: None, + }; + tokio::runtime::Builder::new_current_thread() + .enable_all() + .max_blocking_threads(/*val*/ 1) + .build()? + .block_on(async { + let mut prepared = Box::pin(ExecRequest::from_sandbox_exec_request( + request, + ExecOptions { + expiration: ExecExpiration::DefaultTimeout, + capture_policy: ExecCapturePolicy::ShellTool, + }, + Vec::new(), + )); + let request = if time_out { + tokio::time::pause(); + // Keep checker work queued past the caller's deadline. Dropping + // release unblocks the worker even if an assertion fails. + let (release, wait) = std::sync::mpsc::channel::<()>(); + let (started, ready) = tokio::sync::oneshot::channel(); + let blocker = tokio::task::spawn_blocking(move || { + let _ = started.send(()); + let _ = wait.recv(); + }); + ready.await?; + assert!(futures::poll!(&mut prepared).is_pending()); + tokio::time::advance(Duration::from_secs(/*secs*/ 6)).await; + let result = futures::poll!(&mut prepared); + drop(release); + blocker.await?; + tokio::time::resume(); + let std::task::Poll::Ready(result) = result else { + panic!("command preparation must stop waiting for the checker"); + }; + result? + } else { + prepared.await? + }; + let output = crate::sandboxing::execute_env(request, /*stdout_stream*/ None).await?; + assert_eq!( + ( + output.exit_code, + output.stdout.text.trim(), + output.timed_out + ), + (0, "integrity-ok", false), + ); + Ok(()) + }) +} + #[cfg(unix)] #[test] fn sandbox_detection_flags_sigsys_exit_code() { diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index 69f6ff9044..e3ff8053b0 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -114,11 +114,12 @@ impl ExecRequest { } } - pub(crate) fn from_sandbox_exec_request( + pub(crate) async fn from_sandbox_exec_request( request: SandboxExecRequest, options: ExecOptions, windows_sandbox_workspace_roots: Vec, ) -> Result { + codex_exec_server::run_integrity_checks(&request).await; let SandboxExecRequest { command, cwd, diff --git a/codex-rs/core/src/shell_snapshot_sandbox.rs b/codex-rs/core/src/shell_snapshot_sandbox.rs index 1da6854995..0735470154 100644 --- a/codex-rs/core/src/shell_snapshot_sandbox.rs +++ b/codex-rs/core/src/shell_snapshot_sandbox.rs @@ -197,6 +197,7 @@ impl ShellSnapshotSandbox { }, workspace_roots, ) + .await .context("failed to prepare shell snapshot execution")?; // Caller cancellation must leave execution alive long enough to clean up its process group. let output = tokio::spawn(execute_env(request, /*stdout_stream*/ None)) diff --git a/codex-rs/core/src/tools/runtimes/mod_tests.rs b/codex-rs/core/src/tools/runtimes/mod_tests.rs index f3cdac6a5d..3ee918cb58 100644 --- a/codex-rs/core/src/tools/runtimes/mod_tests.rs +++ b/codex-rs/core/src/tools/runtimes/mod_tests.rs @@ -147,6 +147,7 @@ async fn explicit_escalation_prepares_exec_without_managed_network() -> anyhow:: ), /*environment_id*/ None, ) + .await .expect("prepare exec request"); assert_eq!(exec_request.cwd, PathUri::from_abs_path(&command_cwd)); diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 2642a9c1c0..57320382fb 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -688,6 +688,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt managed_network, Some(&req.turn_environment.selection.environment_id), ) + .await .map_err(ToolError::Codex)?; exec_env.exec_server_env_config = req.exec_server_env_config.clone(); match zsh_fork::maybe_prepare_unified_exec(req, attempt, ctx, exec_env, zsh_fork_config) diff --git a/codex-rs/core/src/tools/runtimes/zsh_fork/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/zsh_fork/unix_escalation.rs index f0aaf9d5f4..ef7d30e35d 100644 --- a/codex-rs/core/src/tools/runtimes/zsh_fork/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/zsh_fork/unix_escalation.rs @@ -717,7 +717,8 @@ impl CoreShellCommandExecutor { env, permission_profile: &self.permission_profile, additional_permissions: None, - })? + }) + .await? } EscalationExecution::Permissions( EscalationPermissions::AdditionalPermissionProfile(permission_profile), @@ -729,7 +730,8 @@ impl CoreShellCommandExecutor { env, permission_profile: &self.permission_profile, additional_permissions: Some(permission_profile), - })? + }) + .await? } EscalationExecution::Permissions(EscalationPermissions::ResolvedPermissionProfile( permissions, @@ -741,7 +743,8 @@ impl CoreShellCommandExecutor { env, permission_profile: &permissions.permission_profile, additional_permissions: None, - })? + }) + .await? } }; @@ -749,7 +752,7 @@ impl CoreShellCommandExecutor { } #[allow(clippy::too_many_arguments)] - fn prepare_sandboxed_exec( + async fn prepare_sandboxed_exec( &self, params: PrepareSandboxedExecParams<'_>, ) -> anyhow::Result { @@ -800,7 +803,8 @@ impl CoreShellCommandExecutor { exec_request, options, self.windows_sandbox_workspace_roots.clone(), - )?; + ) + .await?; if let Some(network) = exec_request.network.as_ref() { network .apply_to_env_for_optional_environment( diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 0e4184eae9..d49273beb3 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -487,7 +487,7 @@ impl<'a> SandboxAttempt<'a> { } } - pub fn env_for( + pub async fn env_for( &self, command: SandboxCommand, options: ExecOptions, @@ -517,9 +517,10 @@ impl<'a> SandboxAttempt<'a> { .map(PathUri::to_abs_path) .collect::>>()?; crate::sandboxing::ExecRequest::from_sandbox_exec_request(request, options, workspace_roots) + .await } - pub fn env_for_exec_server( + pub async fn env_for_exec_server( &self, command: SandboxCommand, options: ExecOptions, @@ -546,11 +547,9 @@ impl<'a> SandboxAttempt<'a> { }) .map_err(CodexErr::from)?; request.sandbox_override = self.sandbox_override; - let mut exec_request = crate::sandboxing::ExecRequest::from_sandbox_exec_request( - request, - options, - Vec::new(), - )?; + let mut exec_request = + crate::sandboxing::ExecRequest::from_sandbox_exec_request(request, options, Vec::new()) + .await?; exec_request.exec_server_managed_network = managed_network; if self.sandbox_requested { exec_request.exec_server_sandbox = Some(FileSystemSandboxContext { diff --git a/codex-rs/core/src/tools/sandboxing_tests.rs b/codex-rs/core/src/tools/sandboxing_tests.rs index ee7afc16a5..a4839cfd33 100644 --- a/codex-rs/core/src/tools/sandboxing_tests.rs +++ b/codex-rs/core/src/tools/sandboxing_tests.rs @@ -250,8 +250,8 @@ fn deny_read_preserves_the_sandbox_for_explicit_escalation_and_blocks_policy_byp ); } -#[test] -fn windows_sandbox_env_preserves_denied_reads_or_rejects_unsupported_backend() { +#[tokio::test] +async fn windows_sandbox_env_preserves_denied_reads_or_rejects_unsupported_backend() { let temp_dir = tempfile::TempDir::new().expect("create sandbox workspace"); let cwd = AbsolutePathBuf::from_absolute_path( dunce::canonicalize(temp_dir.path()).expect("canonicalize sandbox workspace"), @@ -329,6 +329,7 @@ fn windows_sandbox_env_preserves_denied_reads_or_rejects_unsupported_backend() { /*network*/ None, /*environment_id*/ None, ) + .await .expect("prepare elevated Windows sandbox request"); let overrides = request .windows_sandbox_filesystem_overrides @@ -345,6 +346,7 @@ fn windows_sandbox_env_preserves_denied_reads_or_rejects_unsupported_backend() { /*network*/ None, /*environment_id*/ None, ) + .await .expect_err("restricted-token Windows sandbox cannot enforce deny-read restrictions"); assert_eq!( error.to_string(), @@ -352,8 +354,8 @@ fn windows_sandbox_env_preserves_denied_reads_or_rejects_unsupported_backend() { ); } -#[test] -fn exec_server_env_keeps_command_native_and_carries_sandbox_context() { +#[tokio::test] +async fn exec_server_env_keeps_command_native_and_carries_sandbox_context() { let cwd: AbsolutePathBuf = std::env::current_dir() .expect("current dir") .try_into() @@ -400,6 +402,7 @@ fn exec_server_env_keeps_command_native_and_carries_sandbox_context() { }; let request = attempt .env_for_exec_server(command(), options()) + .await .expect("prepare remote exec request"); assert!(!attempt.is_escalated()); @@ -440,6 +443,7 @@ fn exec_server_env_keeps_command_native_and_carries_sandbox_context() { attempt.sandbox_override = SandboxOverride::EscalatedSandboxWithRestrictions; let escalated_request = attempt .env_for_exec_server(command(), options()) + .await .expect("prepare approved escalation with retained sandbox restrictions"); let mut expected_context = request.exec_server_sandbox.expect("sandbox context"); expected_context.sandbox_override = SandboxOverride::EscalatedSandboxWithRestrictions; @@ -451,6 +455,7 @@ fn exec_server_env_keeps_command_native_and_carries_sandbox_context() { attempt.sandbox_requested = false; let request = attempt .env_for_exec_server(command(), options()) + .await .expect("prepare unsandboxed remote exec request"); assert!(attempt.is_escalated()); diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 2940235c6a..4743e34f15 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -1268,9 +1268,11 @@ impl UnifiedExecProcessManager { environment: &codex_exec_server::Environment, ) -> Result { let mut request = if environment.is_remote() || shell_snapshot.is_some() { - attempt.env_for_exec_server(command, options) + attempt.env_for_exec_server(command, options).await } else { - attempt.env_for(command, options, network, environment_id) + attempt + .env_for(command, options, network, environment_id) + .await } .map_err(ToolError::Codex)?; let network_policy_decider = network_proxy_launch diff --git a/codex-rs/exec-server/src/fs_sandbox.rs b/codex-rs/exec-server/src/fs_sandbox.rs index ed5cb9cb1f..f3e39c4e94 100644 --- a/codex-rs/exec-server/src/fs_sandbox.rs +++ b/codex-rs/exec-server/src/fs_sandbox.rs @@ -88,6 +88,7 @@ impl FileSystemSandboxRunner { request: FsHelperRequest, ) -> Result { let command = self.sandbox_command(sandbox)?; + crate::run_integrity_checks(&command).await; let request_json = serde_json::to_vec(&request).map_err(json_error)?; run_command(command, request_json).await } diff --git a/codex-rs/exec-server/src/process_sandbox.rs b/codex-rs/exec-server/src/process_sandbox.rs index 767fc4ab82..cb7f061961 100644 --- a/codex-rs/exec-server/src/process_sandbox.rs +++ b/codex-rs/exec-server/src/process_sandbox.rs @@ -298,6 +298,7 @@ pub(crate) async fn prepare_exec_request_with_telemetry( } .map_err(|err| invalid_params(format!("failed to prepare process sandbox: {err}")))?; request.sandbox_override = sandbox_context.sandbox_override; + crate::run_integrity_checks(&request).await; let windows_sandbox = if sandbox == SandboxType::WindowsRestrictedToken { let windows_sandbox_level = windows_sandbox_level.ok_or_else(|| { invalid_params("restricted token sandbox requires a sandbox level".to_string()) diff --git a/codex-rs/exec-server/src/sandboxed_file_system.rs b/codex-rs/exec-server/src/sandboxed_file_system.rs index 374058f0d0..f592edfbf3 100644 --- a/codex-rs/exec-server/src/sandboxed_file_system.rs +++ b/codex-rs/exec-server/src/sandboxed_file_system.rs @@ -93,6 +93,7 @@ impl SandboxedFileSystem { .sandbox_runner .sandbox_command(sandbox) .map_err(map_sandbox_error)?; + crate::run_integrity_checks(&command).await; crate::sandboxed_file_open::open(command, path.clone(), mode) .await .map_err(map_sandbox_error) diff --git a/codex-rs/exec/tests/suite/sandbox.rs b/codex-rs/exec/tests/suite/sandbox.rs index aa438c0403..d2eb241e9e 100644 --- a/codex-rs/exec/tests/suite/sandbox.rs +++ b/codex-rs/exec/tests/suite/sandbox.rs @@ -56,6 +56,7 @@ pub(super) async fn spawn_command_under_sandbox( codex_protocol::sandbox::SandboxType::None, /*use_legacy_landlock*/ false, ) + .await .map_err(|err| io::Error::other(err.to_string()))?; let (program, args) = exec_request