Validate images before returning view_image output (#37939)

## What changed

- Reject invalid or unsupported image data before producing tool output, so
  non-image file contents are not exposed through code mode.
- Preserve valid image bytes and metadata while leaving image preparation and
  resizing to the history insertion path.

## Testing

- Cover invalid images in both code mode and standard `view_image` calls.
- Verify that code mode preserves PNG bytes, dimensions, EXIF orientation, and
  metadata.

GitOrigin-RevId: 07343ed10b781d0ce0ffb5b24432658be4c046e3
This commit is contained in:
rka-oai
2026-08-11 05:26:16 +00:00
committed by copyberry
parent 2cc9dbb984
commit 41ece455b7
3 changed files with 163 additions and 12 deletions

View File

@@ -49,6 +49,8 @@ impl ViewImageHandler {
const VIEW_IMAGE_UNSUPPORTED_MESSAGE: &str =
"view_image is not allowed because you do not support image inputs";
const VIEW_IMAGE_INVALID_MESSAGE: &str =
"unable to process image: invalid or unsupported image data";
#[derive(Deserialize)]
struct ViewImageArgs {
@@ -173,6 +175,11 @@ impl ViewImageHandler {
"unable to read image at `{model_visible_path}`: {error}"
))
})?;
// Reject non-images before their bytes can reach code mode without changing
// valid image bytes, metadata, or centralized image preparation.
image::load_from_memory(&file_bytes).map_err(|_| {
FunctionCallError::RespondToModel(VIEW_IMAGE_INVALID_MESSAGE.to_string())
})?;
let can_request_original_detail = can_request_original_image_detail(&turn.model_info);
let use_original_detail = self.options.unified_image_budget
@@ -183,7 +190,7 @@ impl ViewImageHandler {
DEFAULT_IMAGE_DETAIL
};
// The history insertion path owns image decoding and resizing.
// The history insertion path owns image preparation and resizing.
let image_url = data_url_from_bytes("application/octet-stream", &file_bytes);
let item = TurnItem::ImageView(ImageViewItem {
@@ -262,8 +269,12 @@ mod tests {
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use core_test_support::TempDirExt;
use image::ImageBuffer;
use image::ImageFormat;
use image::Rgba;
use pretty_assertions::assert_eq;
use serde_json::json;
use std::io::Cursor;
use std::sync::Arc;
use tokio::sync::Mutex;
@@ -284,6 +295,19 @@ mod tests {
));
}
fn tiny_png() -> Vec<u8> {
let image = ImageBuffer::from_pixel(
/*width*/ 1,
/*height*/ 1,
Rgba([255u8, 0, 0, 255]),
);
let mut bytes = Vec::new();
image
.write_to(&mut Cursor::new(&mut bytes), ImageFormat::Png)
.expect("encode test image");
bytes
}
#[test]
fn log_preview_omits_image_data() {
let output = ViewImageOutput {
@@ -324,7 +348,7 @@ mod tests {
replace_primary_environment_cwd(&mut turn, image_cwd.clone());
let image_path = image_cwd.join("image.png");
std::fs::write(image_path.as_path(), b"not a real image").expect("write test image");
std::fs::write(image_path.as_path(), tiny_png()).expect("write test image");
Arc::make_mut(&mut turn.config)
.permissions
.set_permission_profile(PermissionProfile::Disabled)
@@ -400,7 +424,7 @@ mod tests {
replace_primary_environment_cwd(&mut turn, image_cwd.clone());
let image_path = image_cwd.join("image.png");
std::fs::write(image_path.as_path(), b"not a real image").expect("write test image");
std::fs::write(image_path.as_path(), tiny_png()).expect("write test image");
let TurnEnvironmentState::Ready(environment) = &mut turn.environments.environments[0]
else {
panic!("primary environment should be ready");
@@ -427,4 +451,47 @@ mod tests {
result.expect("explicit high detail should be accepted");
}
#[tokio::test(flavor = "multi_thread")]
async fn handle_rejects_invalid_image_before_returning_output_to_code_mode() {
let (session, mut turn) = make_session_and_context().await;
let image_dir = tempfile::tempdir().expect("create image temp dir");
let image_cwd = image_dir.abs();
replace_primary_environment_cwd(&mut turn, image_cwd.clone());
let image_path = image_cwd.join("not-an-image.txt");
std::fs::write(image_path.as_path(), b"arbitrary file contents")
.expect("write invalid image");
let TurnEnvironmentState::Ready(environment) = &mut turn.environments.environments[0]
else {
panic!("primary environment should be ready");
};
environment.config.permission_profile =
PermissionProfileSnapshot::legacy(PermissionProfile::Disabled);
let turn = Arc::new(turn);
let result = ViewImageHandler::default()
.handle(ToolInvocation {
session: Arc::new(session),
step_context: StepContext::for_test(Arc::clone(&turn)),
turn,
cancellation_token: tokio_util::sync::CancellationToken::new(),
tracker: Arc::new(Mutex::new(TurnDiffTracker::new())),
call_id: "call-view-image".to_string(),
tool_name: codex_tools::ToolName::plain("view_image"),
source: ToolCallSource::CodeMode {
cell_id: "cell-1".to_string(),
runtime_tool_call_id: "tool-1".to_string(),
},
payload: ToolPayload::Function {
arguments: json!({ "path": "not-an-image.txt" }).to_string(),
},
})
.await;
let Err(FunctionCallError::RespondToModel(message)) = result else {
panic!("expected invalid image error");
};
assert_eq!(message, VIEW_IMAGE_INVALID_MESSAGE);
}
}

View File

@@ -76,7 +76,13 @@ use core_test_support::wait_for_mcp_server;
use image::DynamicImage;
use image::GenericImageView;
use image::ImageBuffer;
use image::ImageDecoder;
use image::ImageEncoder;
use image::ImageFormat;
use image::ImageReader;
use image::Rgba;
use image::codecs::png::PngEncoder;
use image::metadata::Orientation;
use pretty_assertions::assert_eq;
use serde_json::Value;
use std::collections::HashMap;
@@ -4038,6 +4044,53 @@ image(s.trim(), "original");
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn code_mode_view_image_rejects_invalid_file_without_exposing_contents() -> Result<()> {
skip_if_no_network!(Ok(()));
const INVALID_IMAGE_CONTENTS: &str = "private-file-contents-must-not-be-exposed";
let server = responses::start_mock_server().await;
let builder = test_codex()
.with_model("gpt-5.4")
.with_config(|config| {
let _ = config.features.enable(Feature::CodeMode);
})
.with_workspace_setup(|cwd, _fs| async move {
fs::write(
cwd.join("not-an-image.txt").as_path(),
INVALID_IMAGE_CONTENTS,
)?;
Ok(())
});
let (_test, second_mock) = run_code_mode_turn_with_builder(
&server,
"use exec to call view_image on a non-image file",
r#"await tools.view_image({ path: "not-an-image.txt" });"#,
builder,
)
.await?;
let request = second_mock.single_request();
let (output, success) = custom_tool_output_body_and_success(&request, "call-1");
assert_ne!(
success,
Some(true),
"code-mode view_image unexpectedly accepted a non-image file"
);
assert!(
output.contains("unable to process image: invalid or unsupported image data"),
"unexpected code-mode failure: {output}"
);
let model_visible_output = serde_json::to_string(&request.custom_tool_call_output("call-1"))?;
assert!(
!model_visible_output.contains(INVALID_IMAGE_CONTENTS),
"invalid file contents leaked into model-visible output: {model_visible_output}"
);
Ok(())
}
#[test_case(false; "legacy detail")]
#[test_case(true; "unified image budget")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
@@ -4057,11 +4110,26 @@ async fn code_mode_can_use_view_image_result_with_image_helper(
});
let test = builder.build(&server).await?;
let image_bytes = BASE64_STANDARD.decode(
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==",
let image = ImageBuffer::from_pixel(
/*width*/ 2,
/*height*/ 1,
Rgba([255u8, 0, 0, 255]),
);
let rotate_90_exif = vec![
0x49, 0x49, 0x2a, 0x00, 0x08, 0x00, 0x00, 0x00, 0x01, 0x00, 0x12, 0x01, 0x03, 0x00, 0x01,
0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
];
let mut image_bytes = Vec::new();
let mut encoder = PngEncoder::new(&mut image_bytes);
encoder.set_exif_metadata(rotate_90_exif.clone())?;
encoder.write_image(
image.as_raw(),
image.width(),
image.height(),
image::ColorType::Rgba8.into(),
)?;
let image_path = test.cwd_path().join("code_mode_view_image.png");
fs::write(&image_path, image_bytes)?;
fs::write(&image_path, &image_bytes)?;
let image_path_json = serde_json::to_string(&image_path.to_string_lossy().to_string())?;
let expected_output_keys = if unified_image_budget {
@@ -4129,6 +4197,21 @@ image(out);
.and_then(Value::as_str)
.expect("image helper should emit an input_image item with image_url");
assert!(emitted_image_url.starts_with("data:image/png;base64,"));
let (_, emitted_image_base64) = emitted_image_url
.split_once(',')
.expect("emitted image should contain a data URL prefix");
let emitted_image_bytes = BASE64_STANDARD.decode(emitted_image_base64)?;
assert_eq!(emitted_image_bytes, image_bytes);
let mut decoder = ImageReader::with_format(Cursor::new(&emitted_image_bytes), ImageFormat::Png)
.into_decoder()?;
assert_eq!(
(
decoder.dimensions(),
decoder.orientation()?,
decoder.exif_metadata()?
),
((2, 1), Orientation::Rotate90, Some(rotate_90_exif))
);
assert_eq!(
items[1].get("detail").and_then(Value::as_str),
Some("original")

View File

@@ -1443,7 +1443,7 @@ async fn view_image_tool_errors_when_path_is_directory() -> anyhow::Result<()> {
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn view_image_tool_turns_invalid_image_into_placeholder() -> anyhow::Result<()> {
async fn view_image_tool_rejects_invalid_image_before_tool_output() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
@@ -1496,12 +1496,13 @@ async fn view_image_tool_turns_invalid_image_into_placeholder() -> anyhow::Resul
.await;
let request = second_mock.single_request();
let output_text = request
.function_call_output_content_and_success(call_id)
.and_then(|(content, _)| content)
.context("invalid view_image error text present")?;
assert_eq!(
request.function_call_output(call_id).get("output"),
Some(&serde_json::json!([{
"type": "input_text",
"text": "image content omitted because it could not be processed"
}]))
output_text,
"unable to process image: invalid or unsupported image data"
);
Ok(())
}