diff --git a/codex-rs/core/src/sandboxed_fs.rs b/codex-rs/core/src/sandboxed_fs.rs index bb5425cf572..5af97de6c7d 100644 --- a/codex-rs/core/src/sandboxed_fs.rs +++ b/codex-rs/core/src/sandboxed_fs.rs @@ -24,7 +24,6 @@ use std::time::Duration; /// FILE`, this function verifies that FILE is a regular file before reading, /// which means that if you pass `/dev/zero` as the path, it will error (rather /// than hang forever). -#[allow(dead_code)] pub(crate) async fn read_file( path: &AbsolutePathBuf, session: &Arc, diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index f4015a76245..6c361640d7b 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,24 @@ impl ToolHandler for ViewImageHandler { PromptImageMode::ResizeToFit }; let image_detail = use_original_detail.then_some(ImageDetail::Original); + let image_bytes = sandboxed_fs::read_file(&abs_path, &session, &turn) + .await + .map_err(|error| { + let full_error = format!( + "unable to read image file `{path}`: {error:?}", + path = abs_path.display() + ); + FunctionCallError::RespondToModel(full_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 diff --git a/codex-rs/core/tests/suite/view_image.rs b/codex-rs/core/tests/suite/view_image.rs index efc2e533242..e1d58e450af 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_exec_server::CreateDirectoryOptions; use codex_features::Feature; use codex_protocol::config_types::ReasoningSummary; @@ -14,9 +15,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; @@ -1146,7 +1153,10 @@ async fn view_image_tool_errors_when_path_is_directory() -> anyhow::Result<()> { .function_call_output_content_and_success(call_id) .and_then(|(content, _)| content) .expect("output text present"); - let expected_message = format!("image path `{}` is not a file", abs_path.display()); + let expected_message = format!( + r#"unable to read image file `{path}`: ProcessFailed {{ exit_code: 1, message: "error: `{path}` is not a regular file" }}"#, + path = abs_path.display() + ); assert_eq!(output_text, expected_message); assert!( @@ -1301,7 +1311,10 @@ async fn view_image_tool_errors_when_file_missing() -> anyhow::Result<()> { .function_call_output_content_and_success(call_id) .and_then(|(content, _)| content) .expect("output text present"); - let expected_prefix = format!("unable to locate image at `{}`:", abs_path.display()); + let expected_prefix = format!( + "unable to read image file `{path}`:", + path = abs_path.display() + ); assert!( output_text.starts_with(&expected_prefix), "expected error to start with `{expected_prefix}` but got `{output_text}`" @@ -1315,6 +1328,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 file `{}`:", 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(()));