Compare commits

..

1 Commits

Author SHA1 Message Date
Michael Bolin
16588ffc6d core: route view_image through a sandbox-backed fs helper 2026-03-19 23:44:57 -07:00
3 changed files with 132 additions and 36 deletions

View File

@@ -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>,

View File

@@ -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

View File

@@ -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_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;
@@ -1075,7 +1082,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!(
@@ -1229,7 +1239,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}`"
@@ -1243,6 +1256,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(()));