From 40ba3162cc96aeabb97b85b83c0833b88dfbaff3 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Thu, 19 Mar 2026 09:18:57 -0700 Subject: [PATCH 1/2] core: route view_image through a sandbox-backed fs helper --- codex-rs/Cargo.lock | 14 ++ codex-rs/Cargo.toml | 2 + codex-rs/arg0/Cargo.toml | 1 + codex-rs/arg0/src/lib.rs | 12 ++ codex-rs/core/Cargo.toml | 1 + codex-rs/core/src/exec.rs | 133 ++++++++++++ codex-rs/core/src/lib.rs | 1 + codex-rs/core/src/sandboxed_fs.rs | 197 ++++++++++++++++++ codex-rs/core/src/sandboxing/mod.rs | 16 ++ .../core/src/tools/handlers/view_image.rs | 69 +++--- codex-rs/core/tests/suite/view_image.rs | 110 ++++++++++ codex-rs/fs-ops/BUILD.bazel | 6 + codex-rs/fs-ops/Cargo.toml | 22 ++ codex-rs/fs-ops/src/command.rs | 38 ++++ codex-rs/fs-ops/src/command_tests.rs | 16 ++ codex-rs/fs-ops/src/constants.rs | 3 + codex-rs/fs-ops/src/error.rs | 70 +++++++ codex-rs/fs-ops/src/lib.rs | 13 ++ codex-rs/fs-ops/src/runner.rs | 55 +++++ codex-rs/fs-ops/src/runner_tests.rs | 76 +++++++ codex-rs/utils/image/src/lib.rs | 1 - 21 files changed, 823 insertions(+), 33 deletions(-) create mode 100644 codex-rs/core/src/sandboxed_fs.rs create mode 100644 codex-rs/fs-ops/BUILD.bazel create mode 100644 codex-rs/fs-ops/Cargo.toml create mode 100644 codex-rs/fs-ops/src/command.rs create mode 100644 codex-rs/fs-ops/src/command_tests.rs create mode 100644 codex-rs/fs-ops/src/constants.rs create mode 100644 codex-rs/fs-ops/src/error.rs create mode 100644 codex-rs/fs-ops/src/lib.rs create mode 100644 codex-rs/fs-ops/src/runner.rs create mode 100644 codex-rs/fs-ops/src/runner_tests.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index d6a13a3d4c..23d7d6107f 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1557,6 +1557,7 @@ version = "0.0.0" dependencies = [ "anyhow", "codex-apply-patch", + "codex-fs-ops", "codex-linux-sandbox", "codex-shell-escalation", "codex-utils-home-dir", @@ -1846,6 +1847,7 @@ dependencies = [ "codex-environment", "codex-execpolicy", "codex-file-search", + "codex-fs-ops", "codex-git", "codex-hooks", "codex-keyring-store", @@ -2077,6 +2079,18 @@ dependencies = [ "tokio", ] +[[package]] +name = "codex-fs-ops" +version = "0.0.0" +dependencies = [ + "anyhow", + "base64 0.22.1", + "pretty_assertions", + "serde", + "serde_json", + "tempfile", +] + [[package]] name = "codex-git" version = "0.0.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 35ff64195e..3508c38de4 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -11,6 +11,7 @@ members = [ "apply-patch", "arg0", "feedback", + "fs-ops", "codex-backend-openapi-models", "cloud-requirements", "cloud-tasks", @@ -109,6 +110,7 @@ codex-exec = { path = "exec" } codex-execpolicy = { path = "execpolicy" } codex-experimental-api-macros = { path = "codex-experimental-api-macros" } codex-feedback = { path = "feedback" } +codex-fs-ops = { path = "fs-ops" } codex-file-search = { path = "file-search" } codex-git = { path = "utils/git" } codex-hooks = { path = "hooks" } diff --git a/codex-rs/arg0/Cargo.toml b/codex-rs/arg0/Cargo.toml index abe7277d94..10c31eac5e 100644 --- a/codex-rs/arg0/Cargo.toml +++ b/codex-rs/arg0/Cargo.toml @@ -14,6 +14,7 @@ workspace = true [dependencies] anyhow = { workspace = true } codex-apply-patch = { workspace = true } +codex-fs-ops = { workspace = true } codex-linux-sandbox = { workspace = true } codex-shell-escalation = { workspace = true } codex-utils-home-dir = { workspace = true } diff --git a/codex-rs/arg0/src/lib.rs b/codex-rs/arg0/src/lib.rs index d8dbaaa30d..321a2c73a8 100644 --- a/codex-rs/arg0/src/lib.rs +++ b/codex-rs/arg0/src/lib.rs @@ -4,6 +4,7 @@ use std::path::Path; use std::path::PathBuf; use codex_apply_patch::CODEX_CORE_APPLY_PATCH_ARG1; +use codex_fs_ops::CODEX_CORE_FS_OPS_ARG1; use codex_utils_home_dir::find_codex_home; #[cfg(unix)] use std::os::unix::fs::symlink; @@ -105,6 +106,17 @@ pub fn arg0_dispatch() -> Option { }; std::process::exit(exit_code); } + if argv1 == CODEX_CORE_FS_OPS_ARG1 { + let mut stdin = std::io::stdin(); + let mut stdout = std::io::stdout(); + let mut stderr = std::io::stderr(); + let exit_code = + match codex_fs_ops::run_from_args(args, &mut stdin, &mut stdout, &mut stderr) { + Ok(()) => 0, + Err(_) => 1, + }; + std::process::exit(exit_code); + } // This modifies the environment, which is not thread-safe, so do this // before creating any threads/the Tokio runtime. diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index d11e209813..df8924018d 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -38,6 +38,7 @@ codex-environment = { workspace = true } codex-shell-command = { workspace = true } codex-skills = { workspace = true } codex-execpolicy = { workspace = true } +codex-fs-ops = { workspace = true } codex-file-search = { workspace = true } codex-git = { workspace = true } codex-hooks = { workspace = true } diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 3569917b5c..7d5d71d451 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -332,6 +332,58 @@ pub(crate) async fn execute_exec_request( finalize_exec_result(raw_output_result, sandbox, duration) } +pub(crate) async fn execute_exec_request_bytes( + exec_request: ExecRequest, + sandbox_policy: &SandboxPolicy, + stdout_stream: Option, + after_spawn: Option>, +) -> Result { + let ExecRequest { + command, + cwd, + env, + network, + expiration, + sandbox, + windows_sandbox_level, + windows_sandbox_private_desktop, + sandbox_permissions, + sandbox_policy: _sandbox_policy_from_env, + file_system_sandbox_policy, + network_sandbox_policy, + justification, + arg0, + } = exec_request; + let _ = _sandbox_policy_from_env; + + let params = ExecParams { + command, + cwd, + expiration, + env, + network: network.clone(), + sandbox_permissions, + windows_sandbox_level, + windows_sandbox_private_desktop, + justification, + arg0, + }; + + let start = Instant::now(); + let raw_output_result = exec( + params, + sandbox, + sandbox_policy, + &file_system_sandbox_policy, + network_sandbox_policy, + stdout_stream, + after_spawn, + ) + .await; + let duration = start.elapsed(); + finalize_exec_result_bytes(raw_output_result, sandbox, duration) +} + #[cfg(target_os = "windows")] fn extract_create_process_as_user_error_code(err: &str) -> Option { let marker = "CreateProcessAsUserW failed: "; @@ -574,6 +626,64 @@ fn finalize_exec_result( } } +fn finalize_exec_result_bytes( + raw_output_result: std::result::Result, + sandbox_type: SandboxType, + duration: Duration, +) -> Result { + match raw_output_result { + Ok(raw_output) => { + #[allow(unused_mut)] + let mut timed_out = raw_output.timed_out; + + #[cfg(target_family = "unix")] + { + if let Some(signal) = raw_output.exit_status.signal() { + if signal == TIMEOUT_CODE { + timed_out = true; + } else { + return Err(CodexErr::Sandbox(SandboxErr::Signal(signal))); + } + } + } + + let mut exit_code = raw_output.exit_status.code().unwrap_or(-1); + if timed_out { + exit_code = EXEC_TIMEOUT_EXIT_CODE; + } + + let exec_output = ExecToolCallOutputBytes { + exit_code, + stdout: raw_output.stdout, + stderr: raw_output.stderr, + aggregated_output: raw_output.aggregated_output, + duration, + timed_out, + }; + + if timed_out { + return Err(CodexErr::Sandbox(SandboxErr::Timeout { + output: Box::new(exec_output.to_utf8_lossy_output()), + })); + } + + let string_output = exec_output.to_utf8_lossy_output(); + if is_likely_sandbox_denied(sandbox_type, &string_output) { + return Err(CodexErr::Sandbox(SandboxErr::Denied { + output: Box::new(string_output), + network_policy_decision: None, + })); + } + + Ok(exec_output) + } + Err(err) => { + tracing::error!("exec error: {err}"); + Err(err) + } + } +} + pub(crate) mod errors { use super::CodexErr; use crate::sandboxing::SandboxTransformError; @@ -741,6 +851,16 @@ pub struct ExecToolCallOutput { pub timed_out: bool, } +#[derive(Clone, Debug)] +pub(crate) struct ExecToolCallOutputBytes { + pub exit_code: i32, + pub stdout: StreamOutput>, + pub stderr: StreamOutput>, + pub aggregated_output: StreamOutput>, + pub duration: Duration, + pub timed_out: bool, +} + impl Default for ExecToolCallOutput { fn default() -> Self { Self { @@ -754,6 +874,19 @@ impl Default for ExecToolCallOutput { } } +impl ExecToolCallOutputBytes { + fn to_utf8_lossy_output(&self) -> ExecToolCallOutput { + ExecToolCallOutput { + exit_code: self.exit_code, + stdout: self.stdout.from_utf8_lossy(), + stderr: self.stderr.from_utf8_lossy(), + aggregated_output: self.aggregated_output.from_utf8_lossy(), + duration: self.duration, + timed_out: self.timed_out, + } + } +} + #[cfg_attr(not(target_os = "windows"), allow(unused_variables))] async fn exec( params: ExecParams, diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 10a51b23ec..ef13966fb2 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -114,6 +114,7 @@ pub mod default_client; pub mod project_doc; mod rollout; pub(crate) mod safety; +mod sandboxed_fs; pub mod seatbelt; pub mod shell; pub mod shell_snapshot; diff --git a/codex-rs/core/src/sandboxed_fs.rs b/codex-rs/core/src/sandboxed_fs.rs new file mode 100644 index 0000000000..a7f51864d7 --- /dev/null +++ b/codex-rs/core/src/sandboxed_fs.rs @@ -0,0 +1,197 @@ +use crate::codex::Session; +use crate::codex::TurnContext; +use crate::error::CodexErr; +use crate::error::SandboxErr; +use crate::exec::ExecExpiration; +use crate::exec::ExecToolCallOutputBytes; +use crate::sandboxing::CommandSpec; +use crate::sandboxing::SandboxPermissions; +use crate::sandboxing::execute_env_bytes; +use crate::sandboxing::merge_permission_profiles; +use crate::tools::sandboxing::SandboxAttempt; +use crate::tools::sandboxing::SandboxablePreference; +use codex_fs_ops::CODEX_CORE_FS_OPS_ARG1; +use codex_fs_ops::FsError; +use codex_fs_ops::FsErrorKind; +use codex_protocol::models::PermissionProfile; +use std::collections::HashMap; +use std::path::Path; +use std::path::PathBuf; +use std::sync::Arc; +use std::time::Duration; + +const SANDBOXED_FS_TIMEOUT: Duration = Duration::from_secs(30); + +pub(crate) async fn read_file( + session: &Arc, + turn: &Arc, + path: &Path, +) -> Result, SandboxedFsError> { + let output = run_request(session, turn, path).await?; + Ok(output.stdout.text) +} + +async fn run_request( + session: &Arc, + turn: &Arc, + path: &Path, +) -> Result { + let exe = std::env::current_exe().map_err(|error| SandboxedFsError::ResolveExe { + message: error.to_string(), + })?; + let additional_permissions = effective_granted_permissions(session).await; + let sandbox_manager = crate::sandboxing::SandboxManager::new(); + let attempt = SandboxAttempt { + sandbox: sandbox_manager.select_initial( + &turn.file_system_sandbox_policy, + turn.network_sandbox_policy, + SandboxablePreference::Auto, + turn.windows_sandbox_level, + /*has_managed_network_requirements*/ false, + ), + policy: &turn.sandbox_policy, + file_system_policy: &turn.file_system_sandbox_policy, + network_policy: turn.network_sandbox_policy, + enforce_managed_network: false, + manager: &sandbox_manager, + sandbox_cwd: &turn.cwd, + codex_linux_sandbox_exe: turn.codex_linux_sandbox_exe.as_ref(), + use_legacy_landlock: turn.features.use_legacy_landlock(), + windows_sandbox_level: turn.windows_sandbox_level, + windows_sandbox_private_desktop: turn.config.permissions.windows_sandbox_private_desktop, + }; + let exec_request = attempt + .env_for( + CommandSpec { + program: exe.to_string_lossy().to_string(), + args: vec![ + CODEX_CORE_FS_OPS_ARG1.to_string(), + "read".to_string(), + path.to_string_lossy().to_string(), + ], + cwd: turn.cwd.clone(), + env: HashMap::new(), + expiration: ExecExpiration::Timeout(SANDBOXED_FS_TIMEOUT), + sandbox_permissions: SandboxPermissions::UseDefault, + additional_permissions, + justification: None, + }, + /*network*/ None, + ) + .map_err(|error| SandboxedFsError::ProcessFailed { + path: path.to_path_buf(), + exit_code: -1, + message: error.to_string(), + })?; + + let output = execute_env_bytes(exec_request, /*stdout_stream*/ None) + .await + .map_err(|error| map_exec_error(path, error))?; + if output.exit_code != 0 { + return Err(parse_helper_failure( + path, + output.exit_code, + &output.stderr.text, + &output.stdout.text, + )); + } + + Ok(output) +} + +async fn effective_granted_permissions(session: &Session) -> Option { + let granted_session_permissions = session.granted_session_permissions().await; + let granted_turn_permissions = session.granted_turn_permissions().await; + merge_permission_profiles( + granted_session_permissions.as_ref(), + granted_turn_permissions.as_ref(), + ) +} + +fn map_exec_error(path: &Path, error: CodexErr) -> SandboxedFsError { + match error { + CodexErr::Sandbox(SandboxErr::Timeout { .. }) => SandboxedFsError::TimedOut { + path: path.to_path_buf(), + }, + _ => SandboxedFsError::ProcessFailed { + path: path.to_path_buf(), + exit_code: -1, + message: error.to_string(), + }, + } +} + +fn parse_helper_failure( + path: &Path, + exit_code: i32, + stderr: &[u8], + stdout: &[u8], +) -> SandboxedFsError { + if let Ok(error) = serde_json::from_slice::(stderr) { + return SandboxedFsError::Operation { + path: path.to_path_buf(), + error, + }; + } + + let stderr = String::from_utf8_lossy(stderr); + let stdout = String::from_utf8_lossy(stdout); + let message = if !stderr.trim().is_empty() { + stderr.trim().to_string() + } else if !stdout.trim().is_empty() { + stdout.trim().to_string() + } else { + "no error details emitted".to_string() + }; + + SandboxedFsError::ProcessFailed { + path: path.to_path_buf(), + exit_code, + message, + } +} + +#[derive(Debug, thiserror::Error)] +pub(crate) enum SandboxedFsError { + #[error("failed to determine codex executable: {message}")] + ResolveExe { message: String }, + #[error("sandboxed fs helper timed out while accessing `{path}`")] + TimedOut { path: PathBuf }, + #[error("sandboxed fs helper exited with code {exit_code} while accessing `{path}`: {message}")] + ProcessFailed { + path: PathBuf, + exit_code: i32, + message: String, + }, + #[error("sandboxed fs helper could not access `{path}`: {error}")] + Operation { path: PathBuf, error: FsError }, +} + +impl SandboxedFsError { + pub(crate) fn operation_error_kind(&self) -> Option<&FsErrorKind> { + match self { + Self::Operation { error, .. } => Some(&error.kind), + _ => None, + } + } + + pub(crate) fn operation_error_message(&self) -> Option<&str> { + match self { + Self::Operation { error, .. } => Some(error.message.as_str()), + _ => None, + } + } + + #[allow(dead_code)] + pub(crate) fn to_io_error(&self) -> std::io::Error { + match self { + Self::Operation { error, .. } => error.to_io_error(), + Self::TimedOut { .. } => { + std::io::Error::new(std::io::ErrorKind::TimedOut, self.to_string()) + } + Self::ResolveExe { .. } | Self::ProcessFailed { .. } => { + std::io::Error::other(self.to_string()) + } + } + } +} diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index db88788814..baf18343e2 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -10,9 +10,11 @@ pub(crate) mod macos_permissions; use crate::exec::ExecExpiration; use crate::exec::ExecToolCallOutput; +use crate::exec::ExecToolCallOutputBytes; use crate::exec::SandboxType; use crate::exec::StdoutStream; use crate::exec::execute_exec_request; +use crate::exec::execute_exec_request_bytes; use crate::landlock::allow_network_for_proxy; use crate::landlock::create_linux_sandbox_command_args_for_policies; use crate::protocol::SandboxPolicy; @@ -738,6 +740,20 @@ pub async fn execute_env( .await } +pub(crate) async fn execute_env_bytes( + exec_request: ExecRequest, + stdout_stream: Option, +) -> crate::error::Result { + let effective_policy = exec_request.sandbox_policy.clone(); + execute_exec_request_bytes( + exec_request, + &effective_policy, + stdout_stream, + /*after_spawn*/ None, + ) + .await +} + pub async fn execute_exec_request_with_after_spawn( exec_request: ExecRequest, stdout_stream: Option, diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index 3957549d2d..db090a2728 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -1,5 +1,4 @@ use async_trait::async_trait; -use codex_environment::ExecutorFileSystem; use codex_protocol::models::ContentItem; use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::models::ImageDetail; @@ -13,6 +12,7 @@ use crate::function_tool::FunctionCallError; use crate::original_image_detail::can_request_original_image_detail; use crate::protocol::EventMsg; use crate::protocol::ViewImageToolCallEvent; +use crate::sandboxed_fs; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; @@ -92,36 +92,6 @@ impl ToolHandler for ViewImageHandler { AbsolutePathBuf::try_from(turn.resolve_path(Some(args.path))).map_err(|error| { FunctionCallError::RespondToModel(format!("unable to resolve image path: {error}")) })?; - - let metadata = turn - .environment - .get_filesystem() - .get_metadata(&abs_path) - .await - .map_err(|error| { - FunctionCallError::RespondToModel(format!( - "unable to locate image at `{}`: {error}", - abs_path.display() - )) - })?; - - if !metadata.is_file { - return Err(FunctionCallError::RespondToModel(format!( - "image path `{}` is not a file", - abs_path.display() - ))); - } - let file_bytes = turn - .environment - .get_filesystem() - .read_file(&abs_path) - .await - .map_err(|error| { - FunctionCallError::RespondToModel(format!( - "unable to read image at `{}`: {error}", - abs_path.display() - )) - })?; let event_path = abs_path.to_path_buf(); let can_request_original_detail = @@ -134,10 +104,18 @@ impl ToolHandler for ViewImageHandler { PromptImageMode::ResizeToFit }; let image_detail = use_original_detail.then_some(ImageDetail::Original); + let image_bytes = sandboxed_fs::read_file(&session, &turn, abs_path.as_path()) + .await + .map_err(|error| { + FunctionCallError::RespondToModel(render_view_image_read_error( + abs_path.as_path(), + &error, + )) + })?; let content = local_image_content_items_with_label_number( abs_path.as_path(), - file_bytes, + image_bytes, /*label_number*/ None, image_mode, ) @@ -165,3 +143,30 @@ impl ToolHandler for ViewImageHandler { Ok(FunctionToolOutput::from_content(content, Some(true))) } } + +fn render_view_image_read_error( + path: &std::path::Path, + error: &sandboxed_fs::SandboxedFsError, +) -> String { + let operation_message = error + .operation_error_message() + .map(str::to_owned) + .unwrap_or_else(|| error.to_string()); + match error.operation_error_kind() { + Some(codex_fs_ops::FsErrorKind::IsADirectory) => { + format!("image path `{}` is not a file", path.display()) + } + Some(codex_fs_ops::FsErrorKind::NotFound) => { + format!( + "unable to locate image at `{}`: {operation_message}", + path.display() + ) + } + Some(_) | None => { + format!( + "unable to read image at `{}`: {operation_message}", + path.display() + ) + } + } +} diff --git a/codex-rs/core/tests/suite/view_image.rs b/codex-rs/core/tests/suite/view_image.rs index 240620a272..aa9b651c2f 100644 --- a/codex-rs/core/tests/suite/view_image.rs +++ b/codex-rs/core/tests/suite/view_image.rs @@ -3,6 +3,7 @@ use base64::Engine; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; use codex_core::CodexAuth; +use codex_core::config::Constrained; use codex_core::features::Feature; use codex_protocol::config_types::ReasoningSummary; use codex_protocol::openai_models::ConfigShellToolType; @@ -13,9 +14,15 @@ use codex_protocol::openai_models::ModelsResponse; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::openai_models::ReasoningEffortPreset; use codex_protocol::openai_models::TruncationPolicyConfig; +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::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::Op; +use codex_protocol::protocol::ReadOnlyAccess; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::user_input::UserInput; use core_test_support::responses; @@ -1244,6 +1251,109 @@ async fn view_image_tool_errors_when_file_missing() -> anyhow::Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn view_image_tool_respects_filesystem_sandbox() -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let sandbox_policy_for_config = SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: true, + readable_roots: Vec::new(), + }, + network_access: false, + }; + let mut builder = test_codex().with_config({ + let sandbox_policy_for_config = sandbox_policy_for_config.clone(); + move |config| { + config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config); + config.permissions.file_system_sandbox_policy = + FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Minimal, + }, + access: FileSystemAccessMode::Read, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Read, + }, + ]); + } + }); + let TestCodex { + codex, + config, + cwd, + session_configured, + .. + } = builder.build(&server).await?; + + let outside_dir = tempfile::tempdir()?; + let abs_path = outside_dir.path().join("blocked.png"); + let image = ImageBuffer::from_pixel(256, 128, Rgba([10u8, 20, 30, 255])); + image.save(&abs_path)?; + + let call_id = "view-image-sandbox-denied"; + let arguments = serde_json::json!({ "path": abs_path }).to_string(); + + let first_response = sse(vec![ + ev_response_created("resp-1"), + ev_function_call(call_id, "view_image", &arguments), + ev_completed("resp-1"), + ]); + responses::mount_sse_once(&server, first_response).await; + + let second_response = sse(vec![ + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]); + let mock = responses::mount_sse_once(&server, second_response).await; + + let session_model = session_configured.model.clone(); + + codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "please attach the outside image".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + cwd: cwd.path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: config.permissions.sandbox_policy.get().clone(), + model: session_model, + effort: None, + summary: None, + service_tier: None, + collaboration_mode: None, + personality: None, + }) + .await?; + + wait_for_event(&codex, |event| matches!(event, EventMsg::TurnComplete(_))).await; + + let request = mock.single_request(); + assert!( + request.inputs_of_type("input_image").is_empty(), + "sandbox-denied image should not produce an input_image message" + ); + let output_text = request + .function_call_output_content_and_success(call_id) + .and_then(|(content, _)| content) + .expect("output text present"); + let expected_prefix = format!("unable to read image at `{}`:", abs_path.display()); + assert!( + output_text.starts_with(&expected_prefix), + "expected sandbox denial prefix `{expected_prefix}` but got `{output_text}`" + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn view_image_tool_returns_unsupported_message_for_text_only_model() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/fs-ops/BUILD.bazel b/codex-rs/fs-ops/BUILD.bazel new file mode 100644 index 0000000000..87fe4d5517 --- /dev/null +++ b/codex-rs/fs-ops/BUILD.bazel @@ -0,0 +1,6 @@ +load("//:defs.bzl", "codex_rust_crate") + +codex_rust_crate( + name = "fs-ops", + crate_name = "codex_fs_ops", +) diff --git a/codex-rs/fs-ops/Cargo.toml b/codex-rs/fs-ops/Cargo.toml new file mode 100644 index 0000000000..2b7495f0d2 --- /dev/null +++ b/codex-rs/fs-ops/Cargo.toml @@ -0,0 +1,22 @@ +[package] +name = "codex-fs-ops" +version.workspace = true +edition.workspace = true +license.workspace = true + +[lib] +name = "codex_fs_ops" +path = "src/lib.rs" + +[lints] +workspace = true + +[dependencies] +anyhow = { workspace = true } +base64 = { workspace = true } +serde = { workspace = true, features = ["derive"] } +serde_json = { workspace = true } + +[dev-dependencies] +pretty_assertions = { workspace = true } +tempfile = { workspace = true } diff --git a/codex-rs/fs-ops/src/command.rs b/codex-rs/fs-ops/src/command.rs new file mode 100644 index 0000000000..1c04c5fa1d --- /dev/null +++ b/codex-rs/fs-ops/src/command.rs @@ -0,0 +1,38 @@ +use std::ffi::OsString; +use std::path::PathBuf; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum FsCommand { + ReadFile { path: PathBuf }, +} + +pub fn parse_command_from_args( + mut args: impl Iterator, +) -> Result { + let Some(operation) = args.next() else { + return Err("missing operation".to_string()); + }; + let Some(operation) = operation.to_str() else { + return Err("operation must be valid UTF-8".to_string()); + }; + let Some(path) = args.next() else { + return Err(format!("missing path for operation `{operation}`")); + }; + if args.next().is_some() { + return Err(format!( + "unexpected extra arguments for operation `{operation}`" + )); + } + + let path = PathBuf::from(path); + match operation { + "read" => Ok(FsCommand::ReadFile { path }), + _ => Err(format!( + "unsupported filesystem operation `{operation}`; expected `read`" + )), + } +} + +#[cfg(test)] +#[path = "command_tests.rs"] +mod tests; diff --git a/codex-rs/fs-ops/src/command_tests.rs b/codex-rs/fs-ops/src/command_tests.rs new file mode 100644 index 0000000000..c68d5a4a18 --- /dev/null +++ b/codex-rs/fs-ops/src/command_tests.rs @@ -0,0 +1,16 @@ +use super::FsCommand; +use super::parse_command_from_args; +use pretty_assertions::assert_eq; + +#[test] +fn parse_read_command() { + let command = parse_command_from_args(["read", "/tmp/example.png"].into_iter().map(Into::into)) + .expect("command should parse"); + + assert_eq!( + command, + FsCommand::ReadFile { + path: "/tmp/example.png".into(), + } + ); +} diff --git a/codex-rs/fs-ops/src/constants.rs b/codex-rs/fs-ops/src/constants.rs new file mode 100644 index 0000000000..ae71a287e1 --- /dev/null +++ b/codex-rs/fs-ops/src/constants.rs @@ -0,0 +1,3 @@ +/// Special argv[1] flag used when the Codex executable self-invokes to run the +/// internal sandbox-backed filesystem helper path. +pub const CODEX_CORE_FS_OPS_ARG1: &str = "--codex-run-as-fs-ops"; diff --git a/codex-rs/fs-ops/src/error.rs b/codex-rs/fs-ops/src/error.rs new file mode 100644 index 0000000000..a914056823 --- /dev/null +++ b/codex-rs/fs-ops/src/error.rs @@ -0,0 +1,70 @@ +use serde::Deserialize; +use serde::Serialize; +use std::io::ErrorKind; + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum FsErrorKind { + NotFound, + PermissionDenied, + IsADirectory, + InvalidData, + Other, +} + +impl From for FsErrorKind { + fn from(value: ErrorKind) -> Self { + match value { + ErrorKind::NotFound => Self::NotFound, + ErrorKind::PermissionDenied => Self::PermissionDenied, + ErrorKind::IsADirectory => Self::IsADirectory, + ErrorKind::InvalidData => Self::InvalidData, + _ => Self::Other, + } + } +} + +impl FsErrorKind { + pub fn to_io_error_kind(&self) -> ErrorKind { + match self { + Self::NotFound => ErrorKind::NotFound, + Self::PermissionDenied => ErrorKind::PermissionDenied, + Self::IsADirectory => ErrorKind::IsADirectory, + Self::InvalidData => ErrorKind::InvalidData, + Self::Other => ErrorKind::Other, + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct FsError { + pub kind: FsErrorKind, + pub message: String, + pub raw_os_error: Option, +} + +impl std::fmt::Display for FsError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(&self.message) + } +} + +impl FsError { + pub fn to_io_error(&self) -> std::io::Error { + if let Some(raw_os_error) = self.raw_os_error { + std::io::Error::from_raw_os_error(raw_os_error) + } else { + std::io::Error::new(self.kind.to_io_error_kind(), self.message.clone()) + } + } +} + +impl From for FsError { + fn from(error: std::io::Error) -> Self { + Self { + kind: error.kind().into(), + message: error.to_string(), + raw_os_error: error.raw_os_error(), + } + } +} diff --git a/codex-rs/fs-ops/src/lib.rs b/codex-rs/fs-ops/src/lib.rs new file mode 100644 index 0000000000..a3be232648 --- /dev/null +++ b/codex-rs/fs-ops/src/lib.rs @@ -0,0 +1,13 @@ +mod command; +mod constants; +mod error; +mod runner; + +pub use command::FsCommand; +pub use command::parse_command_from_args; +pub use constants::CODEX_CORE_FS_OPS_ARG1; +pub use error::FsError; +pub use error::FsErrorKind; +pub use runner::execute; +pub use runner::run_from_args; +pub use runner::write_error; diff --git a/codex-rs/fs-ops/src/runner.rs b/codex-rs/fs-ops/src/runner.rs new file mode 100644 index 0000000000..bafdabba55 --- /dev/null +++ b/codex-rs/fs-ops/src/runner.rs @@ -0,0 +1,55 @@ +use crate::FsCommand; +use crate::FsError; +use crate::parse_command_from_args; +use anyhow::Context; +use anyhow::Result; +use std::ffi::OsString; +use std::io::Read; +use std::io::Write; + +pub fn run_from_args( + args: impl Iterator, + stdin: &mut impl Read, + stdout: &mut impl Write, + stderr: &mut impl Write, +) -> Result<()> { + let command = match parse_command_from_args(args) { + Ok(command) => command, + Err(error) => { + writeln!(stderr, "{error}").context("failed to write fs helper usage error")?; + anyhow::bail!("{error}"); + } + }; + + if let Err(error) = execute(command, stdin, stdout) { + write_error(stderr, &error)?; + anyhow::bail!("{error}"); + } + + Ok(()) +} + +pub fn execute( + command: FsCommand, + _stdin: &mut impl Read, + stdout: &mut impl Write, +) -> Result<(), FsError> { + match command { + FsCommand::ReadFile { path } => { + let mut file = std::fs::File::open(path).map_err(FsError::from)?; + std::io::copy(&mut file, stdout) + .map(|_| ()) + .map_err(FsError::from) + } + } +} + +pub fn write_error(stderr: &mut impl Write, error: &FsError) -> Result<()> { + serde_json::to_writer(&mut *stderr, error).context("failed to serialize fs error")?; + writeln!(stderr).context("failed to terminate fs error with newline")?; + Ok(()) +} + +#[cfg(test)] +#[path = "runner_tests.rs"] +mod tests; diff --git a/codex-rs/fs-ops/src/runner_tests.rs b/codex-rs/fs-ops/src/runner_tests.rs new file mode 100644 index 0000000000..ca7396b5d1 --- /dev/null +++ b/codex-rs/fs-ops/src/runner_tests.rs @@ -0,0 +1,76 @@ +use super::execute; +use crate::FsCommand; +use crate::FsErrorKind; +use crate::run_from_args; +use pretty_assertions::assert_eq; +use tempfile::tempdir; + +#[test] +fn run_from_args_streams_file_bytes_to_stdout() { + let tempdir = tempdir().expect("tempdir"); + let path = tempdir.path().join("image.bin"); + let expected = b"hello\x00world".to_vec(); + std::fs::write(&path, &expected).expect("write test file"); + + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + let mut stdin = std::io::empty(); + run_from_args( + ["read", path.to_str().expect("utf-8 test path")] + .into_iter() + .map(Into::into), + &mut stdin, + &mut stdout, + &mut stderr, + ) + .expect("read should succeed"); + + assert_eq!(stdout, expected); + assert_eq!(stderr, Vec::::new()); +} + +#[test] +fn read_reports_directory_error() { + let tempdir = tempdir().expect("tempdir"); + let mut stdout = Vec::new(); + let mut stdin = std::io::empty(); + + let error = execute( + FsCommand::ReadFile { + path: tempdir.path().to_path_buf(), + }, + &mut stdin, + &mut stdout, + ) + .expect_err("reading a directory should fail"); + + #[cfg(target_os = "windows")] + assert_eq!(error.kind, FsErrorKind::PermissionDenied); + #[cfg(not(target_os = "windows"))] + assert_eq!(error.kind, FsErrorKind::IsADirectory); +} + +#[test] +fn run_from_args_serializes_errors_to_stderr() { + let tempdir = tempdir().expect("tempdir"); + let missing = tempdir.path().join("missing.txt"); + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + let mut stdin = std::io::empty(); + + let result = run_from_args( + ["read", missing.to_str().expect("utf-8 test path")] + .into_iter() + .map(Into::into), + &mut stdin, + &mut stdout, + &mut stderr, + ); + + assert!(result.is_err(), "missing file should fail"); + assert_eq!(stdout, Vec::::new()); + + let error: crate::FsError = serde_json::from_slice(&stderr).expect("structured fs error"); + assert_eq!(error.kind, FsErrorKind::NotFound); + assert!(error.raw_os_error.is_some()); +} diff --git a/codex-rs/utils/image/src/lib.rs b/codex-rs/utils/image/src/lib.rs index 8fd0724263..e7ae1cff1e 100644 --- a/codex-rs/utils/image/src/lib.rs +++ b/codex-rs/utils/image/src/lib.rs @@ -59,7 +59,6 @@ pub fn load_for_prompt_bytes( mode: PromptImageMode, ) -> Result { let path_buf = path.to_path_buf(); - let key = ImageCacheKey { digest: sha1_digest(&file_bytes), mode, From a02c347a591fd207ae30ca13d04e91e9bb27c434 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Thu, 19 Mar 2026 09:27:59 -0700 Subject: [PATCH 2/2] core: add stdin-backed sandboxed fs writes --- codex-rs/cli/src/debug_sandbox.rs | 2 + codex-rs/core/src/codex_tests.rs | 2 + codex-rs/core/src/codex_tests_guardian.rs | 1 + codex-rs/core/src/exec.rs | 43 ++++++++++++++- codex-rs/core/src/exec_tests.rs | 52 +++++++++++++++++++ codex-rs/core/src/landlock.rs | 1 + codex-rs/core/src/sandboxed_fs.rs | 28 ++++++++-- codex-rs/core/src/sandboxing/mod.rs | 3 ++ codex-rs/core/src/seatbelt.rs | 1 + codex-rs/core/src/spawn.rs | 16 ++++-- codex-rs/core/src/tasks/user_shell.rs | 2 + codex-rs/core/src/tools/handlers/shell.rs | 3 ++ .../tools/runtimes/shell/unix_escalation.rs | 2 + codex-rs/core/tests/suite/exec.rs | 1 + codex-rs/fs-ops/src/command.rs | 4 +- codex-rs/fs-ops/src/command_tests.rs | 14 +++++ codex-rs/fs-ops/src/runner.rs | 8 ++- codex-rs/fs-ops/src/runner_tests.rs | 46 ++++++++++++++++ .../windows-sandbox-rs/src/elevated_impl.rs | 17 +++++- codex-rs/windows-sandbox-rs/src/lib.rs | 27 +++++++++- 20 files changed, 260 insertions(+), 13 deletions(-) diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index 64169327f5..8172546933 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -171,6 +171,7 @@ async fn run_command_under_sandbox( &cwd_clone, env_map, None, + None, config.permissions.windows_sandbox_private_desktop, ) } else { @@ -182,6 +183,7 @@ async fn run_command_under_sandbox( &cwd_clone, env_map, None, + None, config.permissions.windows_sandbox_private_desktop, ) } diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index f767c05f4e..4c43dc7375 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -4778,6 +4778,7 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() { expiration: timeout_ms.into(), env: HashMap::new(), network: None, + stdin: crate::exec::ExecStdin::Closed, sandbox_permissions, windows_sandbox_level: turn_context.windows_sandbox_level, windows_sandbox_private_desktop: turn_context @@ -4795,6 +4796,7 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() { expiration: timeout_ms.into(), env: HashMap::new(), network: None, + stdin: crate::exec::ExecStdin::Closed, windows_sandbox_level: turn_context.windows_sandbox_level, windows_sandbox_private_desktop: turn_context .config diff --git a/codex-rs/core/src/codex_tests_guardian.rs b/codex-rs/core/src/codex_tests_guardian.rs index 677456ab44..890e7db339 100644 --- a/codex-rs/core/src/codex_tests_guardian.rs +++ b/codex-rs/core/src/codex_tests_guardian.rs @@ -126,6 +126,7 @@ async fn guardian_allows_shell_additional_permissions_requests_past_policy_valid expiration: expiration_ms.into(), env: HashMap::new(), network: None, + stdin: crate::exec::ExecStdin::Closed, sandbox_permissions: SandboxPermissions::WithAdditionalPermissions, windows_sandbox_level: turn_context.windows_sandbox_level, windows_sandbox_private_desktop: turn_context diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 7d5d71d451..4d8d9e4341 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -12,6 +12,7 @@ use std::time::Instant; use async_channel::Sender; use tokio::io::AsyncRead; use tokio::io::AsyncReadExt; +use tokio::io::AsyncWriteExt; use tokio::io::BufReader; use tokio::process::Child; use tokio_util::sync::CancellationToken; @@ -80,6 +81,7 @@ pub struct ExecParams { pub expiration: ExecExpiration, pub env: HashMap, pub network: Option, + pub stdin: ExecStdin, pub sandbox_permissions: SandboxPermissions, pub windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel, pub windows_sandbox_private_desktop: bool, @@ -87,6 +89,13 @@ pub struct ExecParams { pub arg0: Option, } +#[derive(Clone, Debug, Default)] +pub enum ExecStdin { + #[default] + Closed, + Bytes(Vec), +} + fn select_process_exec_tool_sandbox_type( file_system_sandbox_policy: &FileSystemSandboxPolicy, network_sandbox_policy: NetworkSandboxPolicy, @@ -231,6 +240,7 @@ pub fn build_exec_request( mut env, expiration, network, + stdin: _stdin, sandbox_permissions, windows_sandbox_level, windows_sandbox_private_desktop, @@ -291,6 +301,7 @@ pub(crate) async fn execute_exec_request( cwd, env, network, + stdin, expiration, sandbox, windows_sandbox_level, @@ -310,6 +321,7 @@ pub(crate) async fn execute_exec_request( expiration, env, network: network.clone(), + stdin, sandbox_permissions, windows_sandbox_level, windows_sandbox_private_desktop, @@ -343,6 +355,7 @@ pub(crate) async fn execute_exec_request_bytes( cwd, env, network, + stdin, expiration, sandbox, windows_sandbox_level, @@ -362,6 +375,7 @@ pub(crate) async fn execute_exec_request_bytes( expiration, env, network: network.clone(), + stdin, sandbox_permissions, windows_sandbox_level, windows_sandbox_private_desktop, @@ -465,6 +479,7 @@ async fn exec_windows_sandbox( cwd, mut env, network, + stdin, expiration, windows_sandbox_level, windows_sandbox_private_desktop, @@ -501,6 +516,10 @@ async fn exec_windows_sandbox( command, &cwd, env, + match stdin { + ExecStdin::Closed => None, + ExecStdin::Bytes(bytes) => Some(bytes), + }, timeout_ms, windows_sandbox_private_desktop, ) @@ -512,6 +531,10 @@ async fn exec_windows_sandbox( command, &cwd, env, + match stdin { + ExecStdin::Closed => None, + ExecStdin::Bytes(bytes) => Some(bytes), + }, timeout_ms, windows_sandbox_private_desktop, ) @@ -917,6 +940,7 @@ async fn exec( mut env, network, arg0, + stdin, expiration, windows_sandbox_level: _, .. @@ -944,12 +968,13 @@ async fn exec( network: None, stdio_policy: StdioPolicy::RedirectForShellTool, env, + stdin_open: matches!(stdin, ExecStdin::Bytes(_)), }) .await?; if let Some(after_spawn) = after_spawn { after_spawn(); } - consume_truncated_output(child, expiration, stdout_stream).await + consume_truncated_output(child, stdin, expiration, stdout_stream).await } #[cfg_attr(not(target_os = "windows"), allow(dead_code))] @@ -1007,9 +1032,18 @@ fn windows_restricted_token_sandbox_support( /// use as the output of a `shell` tool call. Also enforces specified timeout. async fn consume_truncated_output( mut child: Child, + stdin: ExecStdin, expiration: ExecExpiration, stdout_stream: Option, ) -> Result { + let stdin_task = match (child.stdin.take(), stdin) { + (Some(mut child_stdin), ExecStdin::Bytes(bytes)) => Some(tokio::spawn(async move { + child_stdin.write_all(&bytes).await?; + child_stdin.shutdown().await + })), + _ => None, + }; + // Both stdout and stderr were configured with `Stdio::piped()` // above, therefore `take()` should normally return `Some`. If it doesn't // we treat it as an exceptional I/O error @@ -1089,6 +1123,13 @@ async fn consume_truncated_output( Duration::from_millis(IO_DRAIN_TIMEOUT_MS), ) .await?; + if let Some(stdin_task) = stdin_task { + match stdin_task.await { + Ok(Ok(())) => {} + Ok(Err(err)) => return Err(CodexErr::Io(err)), + Err(join_err) => return Err(CodexErr::Io(io::Error::other(join_err))), + } + } let aggregated_output = aggregate_output(&stdout, &stderr); Ok(RawExecToolCallOutput { diff --git a/codex-rs/core/src/exec_tests.rs b/codex-rs/core/src/exec_tests.rs index 0b5254f43d..ea0e29e42a 100644 --- a/codex-rs/core/src/exec_tests.rs +++ b/codex-rs/core/src/exec_tests.rs @@ -102,6 +102,56 @@ async fn read_capped_limits_retained_bytes() { assert_eq!(out.text.len(), EXEC_OUTPUT_MAX_BYTES); } +#[tokio::test] +async fn exec_passes_stdin_bytes_to_child() -> Result<()> { + let command = if cfg!(windows) { + vec![ + "cmd.exe".to_string(), + "/Q".to_string(), + "/D".to_string(), + "/C".to_string(), + "more".to_string(), + ] + } else { + vec!["/bin/cat".to_string()] + }; + let params = ExecParams { + command, + cwd: std::env::current_dir()?, + expiration: 1_000.into(), + env: std::env::vars().collect(), + network: None, + stdin: ExecStdin::Bytes(b"hello from stdin\n".to_vec()), + sandbox_permissions: SandboxPermissions::UseDefault, + windows_sandbox_level: WindowsSandboxLevel::Disabled, + windows_sandbox_private_desktop: false, + justification: None, + arg0: None, + }; + + let output = exec( + params, + SandboxType::None, + &SandboxPolicy::DangerFullAccess, + &FileSystemSandboxPolicy::from(&SandboxPolicy::DangerFullAccess), + NetworkSandboxPolicy::Enabled, + None, + None, + ) + .await?; + + let expected_stdout = if cfg!(windows) { + "hello from stdin\r\n" + } else { + "hello from stdin\n" + }; + assert_eq!(output.exit_status.code(), Some(0)); + assert_eq!(output.stdout.from_utf8_lossy().text, expected_stdout); + assert_eq!(output.stderr.from_utf8_lossy().text, ""); + + Ok(()) +} + #[test] fn aggregate_output_prefers_stderr_on_contention() { let stdout = StreamOutput { @@ -398,6 +448,7 @@ async fn kill_child_process_group_kills_grandchildren_on_timeout() -> Result<()> expiration: 500.into(), env, network: None, + stdin: ExecStdin::Closed, sandbox_permissions: SandboxPermissions::UseDefault, windows_sandbox_level: WindowsSandboxLevel::Disabled, windows_sandbox_private_desktop: false, @@ -455,6 +506,7 @@ async fn process_exec_tool_call_respects_cancellation_token() -> Result<()> { expiration: ExecExpiration::Cancellation(cancel_token), env, network: None, + stdin: ExecStdin::Closed, sandbox_permissions: SandboxPermissions::UseDefault, windows_sandbox_level: WindowsSandboxLevel::Disabled, windows_sandbox_private_desktop: false, diff --git a/codex-rs/core/src/landlock.rs b/codex-rs/core/src/landlock.rs index 19b3f7c6af..bd0e8a3984 100644 --- a/codex-rs/core/src/landlock.rs +++ b/codex-rs/core/src/landlock.rs @@ -56,6 +56,7 @@ where network, stdio_policy, env, + stdin_open: false, }) .await } diff --git a/codex-rs/core/src/sandboxed_fs.rs b/codex-rs/core/src/sandboxed_fs.rs index a7f51864d7..77b919e9a2 100644 --- a/codex-rs/core/src/sandboxed_fs.rs +++ b/codex-rs/core/src/sandboxed_fs.rs @@ -3,6 +3,7 @@ use crate::codex::TurnContext; use crate::error::CodexErr; use crate::error::SandboxErr; use crate::exec::ExecExpiration; +use crate::exec::ExecStdin; use crate::exec::ExecToolCallOutputBytes; use crate::sandboxing::CommandSpec; use crate::sandboxing::SandboxPermissions; @@ -27,14 +28,34 @@ pub(crate) async fn read_file( turn: &Arc, path: &Path, ) -> Result, SandboxedFsError> { - let output = run_request(session, turn, path).await?; + let output = run_request(session, turn, path, "read", ExecStdin::Closed).await?; Ok(output.stdout.text) } +#[allow(dead_code)] +pub(crate) async fn write_file( + session: &Arc, + turn: &Arc, + path: &Path, + contents: &[u8], +) -> Result<(), SandboxedFsError> { + run_request( + session, + turn, + path, + "write", + ExecStdin::Bytes(contents.to_vec()), + ) + .await?; + Ok(()) +} + async fn run_request( session: &Arc, turn: &Arc, path: &Path, + operation: &str, + stdin: ExecStdin, ) -> Result { let exe = std::env::current_exe().map_err(|error| SandboxedFsError::ResolveExe { message: error.to_string(), @@ -60,13 +81,13 @@ async fn run_request( windows_sandbox_level: turn.windows_sandbox_level, windows_sandbox_private_desktop: turn.config.permissions.windows_sandbox_private_desktop, }; - let exec_request = attempt + let mut exec_request = attempt .env_for( CommandSpec { program: exe.to_string_lossy().to_string(), args: vec![ CODEX_CORE_FS_OPS_ARG1.to_string(), - "read".to_string(), + operation.to_string(), path.to_string_lossy().to_string(), ], cwd: turn.cwd.clone(), @@ -83,6 +104,7 @@ async fn run_request( exit_code: -1, message: error.to_string(), })?; + exec_request.stdin = stdin; let output = execute_env_bytes(exec_request, /*stdout_stream*/ None) .await diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index baf18343e2..6d574f7b23 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -9,6 +9,7 @@ ready‑to‑spawn environment. pub(crate) mod macos_permissions; use crate::exec::ExecExpiration; +use crate::exec::ExecStdin; use crate::exec::ExecToolCallOutput; use crate::exec::ExecToolCallOutputBytes; use crate::exec::SandboxType; @@ -68,6 +69,7 @@ pub struct ExecRequest { pub cwd: PathBuf, pub env: HashMap, pub network: Option, + pub stdin: ExecStdin, pub expiration: ExecExpiration, pub sandbox: SandboxType, pub windows_sandbox_level: WindowsSandboxLevel, @@ -708,6 +710,7 @@ impl SandboxManager { cwd: spec.cwd, env, network: network.cloned(), + stdin: ExecStdin::Closed, expiration: spec.expiration, sandbox, windows_sandbox_level, diff --git a/codex-rs/core/src/seatbelt.rs b/codex-rs/core/src/seatbelt.rs index 2d1e0a7b5d..62ccf82300 100644 --- a/codex-rs/core/src/seatbelt.rs +++ b/codex-rs/core/src/seatbelt.rs @@ -63,6 +63,7 @@ pub async fn spawn_command_under_seatbelt( network, stdio_policy, env, + stdin_open: false, }) .await } diff --git a/codex-rs/core/src/spawn.rs b/codex-rs/core/src/spawn.rs index 575a2f777c..83da439d8b 100644 --- a/codex-rs/core/src/spawn.rs +++ b/codex-rs/core/src/spawn.rs @@ -45,6 +45,7 @@ pub(crate) struct SpawnChildRequest<'a> { pub network: Option<&'a NetworkProxy>, pub stdio_policy: StdioPolicy, pub env: HashMap, + pub stdin_open: bool, } pub(crate) async fn spawn_child_async(request: SpawnChildRequest<'_>) -> std::io::Result { @@ -57,6 +58,7 @@ pub(crate) async fn spawn_child_async(request: SpawnChildRequest<'_>) -> std::io network, stdio_policy, mut env, + stdin_open, } = request; trace!( @@ -105,11 +107,15 @@ pub(crate) async fn spawn_child_async(request: SpawnChildRequest<'_>) -> std::io match stdio_policy { StdioPolicy::RedirectForShellTool => { - // Do not create a file descriptor for stdin because otherwise some - // commands may hang forever waiting for input. For example, ripgrep has - // a heuristic where it may try to read from stdin as explained here: - // https://github.com/BurntSushi/ripgrep/blob/e2362d4d5185d02fa857bf381e7bd52e66fafc73/crates/core/flags/hiargs.rs#L1101-L1103 - cmd.stdin(Stdio::null()); + if stdin_open { + cmd.stdin(Stdio::piped()); + } else { + // Do not create a file descriptor for stdin because otherwise some + // commands may hang forever waiting for input. For example, ripgrep has + // a heuristic where it may try to read from stdin as explained here: + // https://github.com/BurntSushi/ripgrep/blob/e2362d4d5185d02fa857bf381e7bd52e66fafc73/crates/core/flags/hiargs.rs#L1101-L1103 + cmd.stdin(Stdio::null()); + } cmd.stdout(Stdio::piped()).stderr(Stdio::piped()); } diff --git a/codex-rs/core/src/tasks/user_shell.rs b/codex-rs/core/src/tasks/user_shell.rs index 83368efc95..b3233eb4ce 100644 --- a/codex-rs/core/src/tasks/user_shell.rs +++ b/codex-rs/core/src/tasks/user_shell.rs @@ -10,6 +10,7 @@ use tracing::error; use uuid::Uuid; use crate::codex::TurnContext; +use crate::exec::ExecStdin; use crate::exec::ExecToolCallOutput; use crate::exec::SandboxType; use crate::exec::StdoutStream; @@ -162,6 +163,7 @@ pub(crate) async fn execute_user_shell_command( Some(session.conversation_id), ), network: turn_context.network.clone(), + stdin: ExecStdin::Closed, // TODO(zhao-oai): Now that we have ExecExpiration::Cancellation, we // should use that instead of an "arbitrarily large" timeout here. expiration: USER_SHELL_TIMEOUT_MS.into(), diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 04b5c77c37..cf60c98aa1 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -6,6 +6,7 @@ use std::sync::Arc; use crate::codex::TurnContext; use crate::exec::ExecParams; +use crate::exec::ExecStdin; use crate::exec_env::create_env; use crate::exec_policy::ExecApprovalRequest; use crate::features::Feature; @@ -72,6 +73,7 @@ impl ShellHandler { expiration: params.timeout_ms.into(), env: create_env(&turn_context.shell_environment_policy, Some(thread_id)), network: turn_context.network.clone(), + stdin: ExecStdin::Closed, sandbox_permissions: params.sandbox_permissions.unwrap_or_default(), windows_sandbox_level: turn_context.windows_sandbox_level, windows_sandbox_private_desktop: turn_context @@ -126,6 +128,7 @@ impl ShellCommandHandler { expiration: params.timeout_ms.into(), env: create_env(&turn_context.shell_environment_policy, Some(thread_id)), network: turn_context.network.clone(), + stdin: ExecStdin::Closed, sandbox_permissions: params.sandbox_permissions.unwrap_or_default(), windows_sandbox_level: turn_context.windows_sandbox_level, windows_sandbox_private_desktop: turn_context 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 89bf4d424c..c3419bf334 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -123,6 +123,7 @@ pub(super) async fn try_run_zsh_fork( cwd: sandbox_cwd, env: sandbox_env, network: sandbox_network, + stdin: _stdin, expiration: _sandbox_expiration, sandbox, windows_sandbox_level, @@ -902,6 +903,7 @@ impl ShellCommandExecutor for CoreShellCommandExecutor { cwd: self.cwd.clone(), env: exec_env, network: self.network.clone(), + stdin: crate::exec::ExecStdin::Closed, expiration: ExecExpiration::Cancellation(cancel_rx), sandbox: self.sandbox, windows_sandbox_level: self.windows_sandbox_level, diff --git a/codex-rs/core/tests/suite/exec.rs b/codex-rs/core/tests/suite/exec.rs index fc1619b8b3..25a861dcb3 100644 --- a/codex-rs/core/tests/suite/exec.rs +++ b/codex-rs/core/tests/suite/exec.rs @@ -39,6 +39,7 @@ async fn run_test_cmd(tmp: TempDir, cmd: Vec<&str>) -> Result Ok(FsCommand::ReadFile { path }), + "write" => Ok(FsCommand::WriteFile { path }), _ => Err(format!( - "unsupported filesystem operation `{operation}`; expected `read`" + "unsupported filesystem operation `{operation}`; expected `read` or `write`" )), } } diff --git a/codex-rs/fs-ops/src/command_tests.rs b/codex-rs/fs-ops/src/command_tests.rs index c68d5a4a18..31629a4778 100644 --- a/codex-rs/fs-ops/src/command_tests.rs +++ b/codex-rs/fs-ops/src/command_tests.rs @@ -14,3 +14,17 @@ fn parse_read_command() { } ); } + +#[test] +fn parse_write_command() { + let command = + parse_command_from_args(["write", "/tmp/example.png"].into_iter().map(Into::into)) + .expect("command should parse"); + + assert_eq!( + command, + FsCommand::WriteFile { + path: "/tmp/example.png".into(), + } + ); +} diff --git a/codex-rs/fs-ops/src/runner.rs b/codex-rs/fs-ops/src/runner.rs index bafdabba55..602895c997 100644 --- a/codex-rs/fs-ops/src/runner.rs +++ b/codex-rs/fs-ops/src/runner.rs @@ -31,7 +31,7 @@ pub fn run_from_args( pub fn execute( command: FsCommand, - _stdin: &mut impl Read, + stdin: &mut impl Read, stdout: &mut impl Write, ) -> Result<(), FsError> { match command { @@ -41,6 +41,12 @@ pub fn execute( .map(|_| ()) .map_err(FsError::from) } + FsCommand::WriteFile { path } => { + let mut file = std::fs::File::create(path).map_err(FsError::from)?; + std::io::copy(stdin, &mut file) + .map(|_| ()) + .map_err(FsError::from) + } } } diff --git a/codex-rs/fs-ops/src/runner_tests.rs b/codex-rs/fs-ops/src/runner_tests.rs index ca7396b5d1..3ddd06e2cd 100644 --- a/codex-rs/fs-ops/src/runner_tests.rs +++ b/codex-rs/fs-ops/src/runner_tests.rs @@ -3,6 +3,7 @@ use crate::FsCommand; use crate::FsErrorKind; use crate::run_from_args; use pretty_assertions::assert_eq; +use std::io::Cursor; use tempfile::tempdir; #[test] @@ -74,3 +75,48 @@ fn run_from_args_serializes_errors_to_stderr() { assert_eq!(error.kind, FsErrorKind::NotFound); assert!(error.raw_os_error.is_some()); } + +#[test] +fn run_from_args_streams_stdin_bytes_to_file() { + let tempdir = tempdir().expect("tempdir"); + let path = tempdir.path().join("image.bin"); + let expected = b"hello\x00world".to_vec(); + + let mut stdin = Cursor::new(expected.clone()); + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + run_from_args( + ["write", path.to_str().expect("utf-8 test path")] + .into_iter() + .map(Into::into), + &mut stdin, + &mut stdout, + &mut stderr, + ) + .expect("write should succeed"); + + assert_eq!(std::fs::read(&path).expect("read test file"), expected); + assert_eq!(stdout, Vec::::new()); + assert_eq!(stderr, Vec::::new()); +} + +#[test] +fn write_reports_directory_error() { + let tempdir = tempdir().expect("tempdir"); + let mut stdin = Cursor::new(b"hello world".to_vec()); + let mut stdout = Vec::new(); + + let error = execute( + FsCommand::WriteFile { + path: tempdir.path().to_path_buf(), + }, + &mut stdin, + &mut stdout, + ) + .expect_err("writing to a directory should fail"); + + #[cfg(target_os = "windows")] + assert_eq!(error.kind, FsErrorKind::PermissionDenied); + #[cfg(not(target_os = "windows"))] + assert_eq!(error.kind, FsErrorKind::IsADirectory); +} diff --git a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs index f6c4563e17..fa6eef91b0 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs @@ -209,6 +209,7 @@ mod windows_impl { command: Vec, cwd: &Path, mut env_map: HashMap, + stdin_bytes: Option>, timeout_ms: Option, use_private_desktop: bool, ) -> Result { @@ -383,13 +384,26 @@ mod windows_impl { cap_sids, timeout_ms, tty: false, - stdin_open: false, + stdin_open: stdin_bytes.is_some(), use_private_desktop, }), }, }; write_frame(&mut pipe_write, &spawn_request)?; read_spawn_ready(&mut pipe_read)?; + if let Some(stdin_bytes) = stdin_bytes { + write_frame( + &mut pipe_write, + &FramedMessage { + version: 1, + message: Message::Stdin { + payload: crate::ipc_framed::StdinPayload { + data_b64: crate::ipc_framed::encode_bytes(&stdin_bytes), + }, + }, + }, + )?; + } drop(pipe_write); let mut stdout = Vec::new(); @@ -503,6 +517,7 @@ mod stub { _command: Vec, _cwd: &Path, _env_map: HashMap, + _stdin_bytes: Option>, _timeout_ms: Option, _use_private_desktop: bool, ) -> Result { diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index cb4be52750..6e6eb47d0a 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -265,6 +265,7 @@ mod windows_impl { command: Vec, cwd: &Path, mut env_map: HashMap, + stdin_bytes: Option>, timeout_ms: Option, use_private_desktop: bool, ) -> Result { @@ -406,7 +407,30 @@ mod windows_impl { unsafe { CloseHandle(in_r); - // Close the parent's stdin write end so the child sees EOF immediately. + if let Some(stdin_bytes) = stdin_bytes { + let mut offset = 0; + while offset < stdin_bytes.len() { + let remaining = stdin_bytes.len() - offset; + let chunk_len = remaining.min(u32::MAX as usize); + let mut written: u32 = 0; + let ok = windows_sys::Win32::Storage::FileSystem::WriteFile( + in_w, + stdin_bytes[offset..offset + chunk_len].as_ptr(), + chunk_len as u32, + &mut written, + std::ptr::null_mut(), + ); + if ok == 0 { + CloseHandle(in_w); + CloseHandle(out_w); + CloseHandle(err_w); + CloseHandle(pi.hThread); + CloseHandle(pi.hProcess); + return Err(std::io::Error::from_raw_os_error(GetLastError() as i32).into()); + } + offset += written as usize; + } + } CloseHandle(in_w); CloseHandle(out_w); CloseHandle(err_w); @@ -615,6 +639,7 @@ mod stub { _command: Vec, _cwd: &Path, _env_map: HashMap, + _stdin_bytes: Option>, _timeout_ms: Option, _use_private_desktop: bool, ) -> Result {