From b88e4ec7bce1d5ab70c679c5cfc86cdae414bf76 Mon Sep 17 00:00:00 2001 From: Roy Han Date: Fri, 22 May 2026 17:55:51 -0700 Subject: [PATCH] reject empty base64 image inputs --- codex-rs/app-server/src/error_code.rs | 1 + codex-rs/app-server/src/request_processors.rs | 1 + .../src/request_processors/turn_processor.rs | 26 ++++ .../app-server/tests/suite/v2/turn_start.rs | 63 ++++++++++ codex-rs/codex-api/src/api_bridge.rs | 10 +- codex-rs/codex-api/src/api_bridge_tests.rs | 42 +++++++ codex-rs/protocol/src/models.rs | 119 +++++++++++++++--- 7 files changed, 238 insertions(+), 24 deletions(-) diff --git a/codex-rs/app-server/src/error_code.rs b/codex-rs/app-server/src/error_code.rs index 48e401f7bc..bec3cf708d 100644 --- a/codex-rs/app-server/src/error_code.rs +++ b/codex-rs/app-server/src/error_code.rs @@ -5,6 +5,7 @@ pub(crate) const METHOD_NOT_FOUND_ERROR_CODE: i64 = -32601; pub const INVALID_PARAMS_ERROR_CODE: i64 = -32602; pub(crate) const INTERNAL_ERROR_CODE: i64 = -32603; pub(crate) const OVERLOADED_ERROR_CODE: i64 = -32001; +pub(crate) const EMPTY_BASE64_IMAGE_INPUT_ERROR_CODE: &str = "empty_base64_image_input"; pub const INPUT_TOO_LARGE_ERROR_CODE: &str = "input_too_large"; pub(crate) fn invalid_request(message: impl Into) -> JSONRPCErrorError { diff --git a/codex-rs/app-server/src/request_processors.rs b/codex-rs/app-server/src/request_processors.rs index d0958e80ab..bbb36f28c3 100644 --- a/codex-rs/app-server/src/request_processors.rs +++ b/codex-rs/app-server/src/request_processors.rs @@ -3,6 +3,7 @@ use crate::bespoke_event_handling::maybe_emit_hook_prompt_item_completed; use crate::command_exec::CommandExecManager; use crate::command_exec::StartCommandExecParams; use crate::config_manager::ConfigManager; +use crate::error_code::EMPTY_BASE64_IMAGE_INPUT_ERROR_CODE; use crate::error_code::INPUT_TOO_LARGE_ERROR_CODE; use crate::error_code::invalid_params; use crate::models::supported_models; diff --git a/codex-rs/app-server/src/request_processors/turn_processor.rs b/codex-rs/app-server/src/request_processors/turn_processor.rs index ff2825be67..578df15e98 100644 --- a/codex-rs/app-server/src/request_processors/turn_processor.rs +++ b/codex-rs/app-server/src/request_processors/turn_processor.rs @@ -46,6 +46,14 @@ struct ThreadSettingsBuildParams { personality: Option, } +fn is_empty_base64_image_input(item: &V2UserInput) -> bool { + matches!( + item, + V2UserInput::Image { url, .. } + if codex_protocol::models::is_empty_base64_image_data_url(url) + ) +} + impl TurnRequestProcessor { #[allow(clippy::too_many_arguments)] pub(crate) fn new( @@ -344,6 +352,14 @@ impl TurnRequestProcessor { error } + fn empty_base64_image_input_error() -> JSONRPCErrorError { + let mut error = invalid_params("Could not attach image: empty image data."); + error.data = Some(serde_json::json!({ + "input_error_code": EMPTY_BASE64_IMAGE_INPUT_ERROR_CODE, + })); + error + } + fn validate_v2_input_limit(items: &[V2UserInput]) -> Result<(), JSONRPCErrorError> { let actual_chars: usize = items.iter().map(V2UserInput::text_char_count).sum(); if actual_chars > MAX_USER_INPUT_TEXT_CHARS { @@ -367,6 +383,11 @@ impl TurnRequestProcessor { ); return Err(error); } + if params.input.iter().any(is_empty_base64_image_input) { + let error = Self::empty_base64_image_input_error(); + self.track_error_response(&request_id, &error, /*error_type*/ None); + return Err(error); + } let (thread_id, thread) = self.load_thread(¶ms.thread_id) .await @@ -740,6 +761,11 @@ impl TurnRequestProcessor { ); return Err(error); } + if params.input.iter().any(is_empty_base64_image_input) { + let error = Self::empty_base64_image_input_error(); + self.track_error_response(request_id, &error, /*error_type*/ None); + return Err(error); + } let mapped_items: Vec = params .input diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index 236d57a0ee..84c441678f 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -954,6 +954,69 @@ async fn turn_start_rejects_combined_oversized_text_input() -> Result<()> { Ok(()) } +#[tokio::test] +async fn turn_start_rejects_empty_base64_image_before_starting_turn() -> Result<()> { + let codex_home = TempDir::new()?; + create_config_toml( + codex_home.path(), + "http://localhost/unused", + "never", + &BTreeMap::from([(Feature::Personality, true)]), + )?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let thread_req = mcp + .send_thread_start_request(ThreadStartParams { + model: Some("mock-model".to_string()), + ..Default::default() + }) + .await?; + let thread_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(thread_req)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(thread_resp)?; + + let turn_req = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id, + input: vec![V2UserInput::Image { + detail: None, + url: "data:image/png;base64,".to_string(), + }], + ..Default::default() + }) + .await?; + let err: JSONRPCError = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_error_message(RequestId::Integer(turn_req)), + ) + .await??; + + assert_eq!(err.error.code, INVALID_PARAMS_ERROR_CODE); + assert_eq!( + err.error.message, + "Could not attach image: empty image data." + ); + let data = err.error.data.expect("expected structured error data"); + assert_eq!(data["input_error_code"], "empty_base64_image_input"); + + let turn_started = tokio::time::timeout( + std::time::Duration::from_millis(250), + mcp.read_stream_until_notification_message("turn/started"), + ) + .await; + assert!( + turn_started.is_err(), + "did not expect a turn/started notification for rejected input" + ); + + Ok(()) +} + #[tokio::test] async fn turn_start_rejects_invalid_permission_selection_before_starting_turn() -> Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/codex-api/src/api_bridge.rs b/codex-rs/codex-api/src/api_bridge.rs index 1c34d8bbf2..adbde94848 100644 --- a/codex-rs/codex-api/src/api_bridge.rs +++ b/codex-rs/codex-api/src/api_bridge.rs @@ -57,8 +57,8 @@ pub fn map_api_error(err: ApiError) -> CodexErr { } if status == http::StatusCode::BAD_REQUEST { - if let Ok(parsed) = serde_json::from_str::(&body_text) - && let Some(error) = parsed.get("error") + let parsed = serde_json::from_str::(&body_text).ok(); + if let Some(error) = parsed.as_ref().and_then(|parsed| parsed.get("error")) && error.get("code").and_then(Value::as_str) == Some(CYBER_POLICY_ERROR_CODE) { @@ -69,8 +69,8 @@ pub fn map_api_error(err: ApiError) -> CodexErr { .map(str::to_string) .unwrap_or_else(|| CYBER_POLICY_FALLBACK_MESSAGE.to_string()); CodexErr::CyberPolicy { message } - } else if body_text - .contains("The image data you provided does not represent a valid image") + } else if body_text.contains(IMAGE_PARSE_ERROR) + || body_text.contains(IMAGE_INVALID_BASE64) { CodexErr::InvalidImageRequest() } else { @@ -145,6 +145,8 @@ const X_ERROR_JSON_HEADER: &str = "x-error-json"; const CYBER_POLICY_ERROR_CODE: &str = "cyber_policy"; const CYBER_POLICY_FALLBACK_MESSAGE: &str = "This request has been flagged for possible cybersecurity risk."; +const IMAGE_PARSE_ERROR: &str = "The image data you provided does not represent a valid image"; +const IMAGE_INVALID_BASE64: &str = "Expected a base64-encoded data URL with an image MIME type"; #[cfg(test)] #[path = "api_bridge_tests.rs"] diff --git a/codex-rs/codex-api/src/api_bridge_tests.rs b/codex-rs/codex-api/src/api_bridge_tests.rs index 101e5566fe..73159bb2cd 100644 --- a/codex-rs/codex-api/src/api_bridge_tests.rs +++ b/codex-rs/codex-api/src/api_bridge_tests.rs @@ -124,6 +124,48 @@ fn map_api_error_keeps_unknown_400_errors_generic() { assert_eq!(message, body); } +#[test] +fn map_api_error_maps_empty_base64_image_error_message() { + const EMPTY_IMAGE_MESSAGE: &str = "Expected a base64-encoded data URL with an image MIME type \ + (e.g. 'data:image/png;base64,aW1nIGJ5dGVzIGhlcmU='), but got empty base64-encoded bytes."; + + let cases = [ + ("input[0].content[2].image_url", EMPTY_IMAGE_MESSAGE, true), + ("input[111].output[1].image_url", EMPTY_IMAGE_MESSAGE, true), + ("input[0].name", "Expected a valid tool name.", false), + ]; + + for (param, message, maps_to_invalid_image) in cases { + let body = serde_json::json!({ + "error": { + "type": "invalid_request_error", + "code": "invalid_value", + "message": format!("Invalid '{param}'. {message}"), + "param": param + } + }) + .to_string(); + let err = map_api_error(ApiError::Transport(TransportError::Http { + status: http::StatusCode::BAD_REQUEST, + url: Some("http://example.com/v1/responses".to_string()), + headers: None, + body: Some(body.clone()), + })); + + if maps_to_invalid_image { + assert!( + matches!(err, CodexErr::InvalidImageRequest()), + "expected InvalidImageRequest for {param}, got {err:?}" + ); + } else { + let CodexErr::InvalidRequest(message) = err else { + panic!("expected CodexErr::InvalidRequest for {param}, got {err:?}"); + }; + assert_eq!(message, body); + } + } +} + #[test] fn map_api_error_maps_usage_limit_limit_name_header() { let mut headers = HeaderMap::new(); diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 28a7d6522c..72e3bb98e9 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -1062,6 +1062,22 @@ fn unsupported_image_error_placeholder(path: &std::path::Path, mime: &str) -> Co } } +const EMPTY_BASE64_TOOL_IMAGE_TEXT: &str = + "Tool returned an empty image; skipped sending it to the model."; + +pub fn is_empty_base64_image_data_url(image_url: &str) -> bool { + let Some((metadata, payload)) = image_url.split_once(',') else { + return false; + }; + let metadata = metadata.trim().to_ascii_lowercase(); + + metadata.starts_with("data:image/") + && metadata + .split(';') + .any(|segment| segment.trim() == "base64") + && payload.trim().is_empty() +} + pub fn local_image_content_items_with_label_number( path: &std::path::Path, file_bytes: Vec, @@ -1231,14 +1247,15 @@ impl From> for ResponseInputItem { UserInput::Image { image_url, detail } => { image_index += 1; let detail = detail.unwrap_or(DEFAULT_IMAGE_DETAIL); + let image_item = ContentItem::InputImage { + image_url, + detail: Some(detail), + }; vec![ ContentItem::InputText { text: image_open_tag_text(), }, - ContentItem::InputImage { - image_url, - detail: Some(detail), - }, + image_item, ContentItem::InputText { text: image_close_tag_text(), }, @@ -1361,9 +1378,15 @@ impl From Self::InputText { text } } crate::dynamic_tools::DynamicToolCallOutputContentItem::InputImage { image_url } => { - Self::InputImage { - image_url, - detail: Some(DEFAULT_IMAGE_DETAIL), + if is_empty_base64_image_data_url(&image_url) { + Self::InputText { + text: EMPTY_BASE64_TOOL_IMAGE_TEXT.to_string(), + } + } else { + Self::InputImage { + image_url, + detail: Some(DEFAULT_IMAGE_DETAIL), + } } } } @@ -1592,19 +1615,23 @@ fn convert_mcp_content_to_items( let mime_type = mime_type.unwrap_or_else(|| "application/octet-stream".into()); format!("data:{mime_type};base64,{data}") }; - FunctionCallOutputContentItem::InputImage { - image_url, - detail: meta - .as_ref() - .and_then(serde_json::Value::as_object) - .and_then(|meta| meta.get(CODEX_IMAGE_DETAIL_META_KEY)) - .and_then(serde_json::Value::as_str) - .and_then(|detail| match detail { - "high" => Some(ImageDetail::High), - "original" => Some(ImageDetail::Original), - _ => None, - }) - .or(Some(DEFAULT_IMAGE_DETAIL)), + let detail = meta + .as_ref() + .and_then(serde_json::Value::as_object) + .and_then(|meta| meta.get(CODEX_IMAGE_DETAIL_META_KEY)) + .and_then(serde_json::Value::as_str) + .and_then(|detail| match detail { + "high" => Some(ImageDetail::High), + "original" => Some(ImageDetail::Original), + _ => None, + }) + .or(Some(DEFAULT_IMAGE_DETAIL)); + if is_empty_base64_image_data_url(&image_url) { + FunctionCallOutputContentItem::InputText { + text: EMPTY_BASE64_TOOL_IMAGE_TEXT.to_string(), + } + } else { + FunctionCallOutputContentItem::InputImage { image_url, detail } } } Ok(McpContent::Unknown) | Err(_) => FunctionCallOutputContentItem::InputText { @@ -2210,6 +2237,24 @@ mod tests { Ok(()) } + #[test] + fn empty_base64_dynamic_tool_image_output_is_replaced_with_text() -> Result<()> { + let item = FunctionCallOutputContentItem::from( + crate::dynamic_tools::DynamicToolCallOutputContentItem::InputImage { + image_url: "data:image/png;base64,".to_string(), + }, + ); + + assert_eq!( + item, + FunctionCallOutputContentItem::InputText { + text: EMPTY_BASE64_TOOL_IMAGE_TEXT.to_string(), + } + ); + + Ok(()) + } + #[test] fn serializes_image_outputs_as_array() -> Result<()> { let call_tool_result = CallToolResult { @@ -2255,6 +2300,40 @@ mod tests { Ok(()) } + #[test] + fn empty_base64_mcp_image_outputs_are_replaced_with_text() -> Result<()> { + for data in ["", "data:image/png;base64,"] { + let call_tool_result = CallToolResult { + content: vec![ + serde_json::json!({"type":"text","text":"caption"}), + serde_json::json!({"type":"image","data":data,"mimeType":"image/png"}), + ], + structured_content: None, + is_error: Some(false), + meta: None, + }; + + let payload = call_tool_result.into_function_call_output_payload(); + assert_eq!(payload.success, Some(true)); + let Some(items) = payload.content_items() else { + panic!("expected content items for {data:?}"); + }; + assert_eq!( + items, + [ + FunctionCallOutputContentItem::InputText { + text: "caption".into(), + }, + FunctionCallOutputContentItem::InputText { + text: EMPTY_BASE64_TOOL_IMAGE_TEXT.to_string(), + }, + ] + ); + } + + Ok(()) + } + #[test] fn serializes_custom_tool_image_outputs_as_array() -> Result<()> { let item = ResponseInputItem::CustomToolCallOutput {