Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion codex-rs/core/src/sandboxed_fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Session>,
Expand Down
47 changes: 14 additions & 33 deletions codex-rs/core/src/tools/handlers/view_image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 =
Expand All @@ -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
Expand Down
120 changes: 118 additions & 2 deletions codex-rs/core/tests/suite/view_image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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}`"
Expand All @@ -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(()));
Expand Down
Loading