diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index f4015a7624..dcc07cccfb 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -14,6 +14,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::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; @@ -93,36 +94,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 = @@ -135,14 +106,23 @@ 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 image = - load_for_prompt_bytes(abs_path.as_path(), file_bytes, image_mode).map_err(|error| { + let image = load_for_prompt_bytes(abs_path.as_path(), image_bytes, image_mode).map_err( + |error| { FunctionCallError::RespondToModel(format!( "unable to process image at `{}`: {error}", abs_path.display() )) - })?; + }, + )?; let image_url = image.into_data_url(); session @@ -201,6 +181,33 @@ impl ToolOutput for ViewImageOutput { } } +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() + ) + } + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/codex-rs/core/tests/suite/view_image.rs b/codex-rs/core/tests/suite/view_image.rs index 8f1d0a5fe7..e24610b7cb 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; @@ -1243,6 +1250,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(()));