mirror of
https://github.com/openai/codex.git
synced 2026-04-24 22:54:54 +00:00
feat: return an error if the image sent by the user is a bad image (#9146)
## Before When we detect an `InvalidImageRequest`, we replace the image by a placeholder and keep going ## Now In such `InvalidImageRequest`, we check if the image is due to a user message or a tool call output. For tool call output we still replace it with a placeholder to avoid breaking the agentic loop bu tif this is because of a user message, we send an error to the user
This commit is contained in:
@@ -2629,9 +2629,18 @@ pub(crate) async fn run_turn(
|
|||||||
Err(CodexErr::InvalidImageRequest()) => {
|
Err(CodexErr::InvalidImageRequest()) => {
|
||||||
let mut state = sess.state.lock().await;
|
let mut state = sess.state.lock().await;
|
||||||
error_or_panic(
|
error_or_panic(
|
||||||
"Invalid image detected, replacing it in the last turn to prevent poisoning",
|
"Invalid image detected; sanitizing tool output to prevent poisoning",
|
||||||
);
|
);
|
||||||
state.history.replace_last_turn_images("Invalid image");
|
if state.history.replace_last_turn_images("Invalid image") {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
let event = EventMsg::Error(ErrorEvent {
|
||||||
|
message: "Invalid image in your last message. Please remove it and try again."
|
||||||
|
.to_string(),
|
||||||
|
codex_error_info: Some(CodexErrorInfo::BadRequest),
|
||||||
|
});
|
||||||
|
sess.send_event(&turn_context, event).await;
|
||||||
|
break;
|
||||||
}
|
}
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
info!("Turn error: {e:#}");
|
info!("Turn error: {e:#}");
|
||||||
|
|||||||
@@ -124,34 +124,35 @@ impl ContextManager {
|
|||||||
self.items = items;
|
self.items = items;
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) fn replace_last_turn_images(&mut self, placeholder: &str) {
|
/// Replace image content in the last turn if it originated from a tool output.
|
||||||
let Some(last_item) = self.items.last_mut() else {
|
/// Returns true when a tool image was replaced, false otherwise.
|
||||||
return;
|
pub(crate) fn replace_last_turn_images(&mut self, placeholder: &str) -> bool {
|
||||||
|
let Some(index) = self.items.iter().rposition(|item| {
|
||||||
|
matches!(item, ResponseItem::FunctionCallOutput { .. })
|
||||||
|
|| matches!(item, ResponseItem::Message { role, .. } if role == "user")
|
||||||
|
}) else {
|
||||||
|
return false;
|
||||||
};
|
};
|
||||||
|
|
||||||
match last_item {
|
match &mut self.items[index] {
|
||||||
ResponseItem::Message { role, content, .. } if role == "user" => {
|
|
||||||
for item in content.iter_mut() {
|
|
||||||
if matches!(item, ContentItem::InputImage { .. }) {
|
|
||||||
*item = ContentItem::InputText {
|
|
||||||
text: placeholder.to_string(),
|
|
||||||
};
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
ResponseItem::FunctionCallOutput { output, .. } => {
|
ResponseItem::FunctionCallOutput { output, .. } => {
|
||||||
let Some(content_items) = output.content_items.as_mut() else {
|
let Some(content_items) = output.content_items.as_mut() else {
|
||||||
return;
|
return false;
|
||||||
};
|
};
|
||||||
|
let mut replaced = false;
|
||||||
|
let placeholder = placeholder.to_string();
|
||||||
for item in content_items.iter_mut() {
|
for item in content_items.iter_mut() {
|
||||||
if matches!(item, FunctionCallOutputContentItem::InputImage { .. }) {
|
if matches!(item, FunctionCallOutputContentItem::InputImage { .. }) {
|
||||||
*item = FunctionCallOutputContentItem::InputText {
|
*item = FunctionCallOutputContentItem::InputText {
|
||||||
text: placeholder.to_string(),
|
text: placeholder.clone(),
|
||||||
};
|
};
|
||||||
|
replaced = true;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
replaced
|
||||||
}
|
}
|
||||||
_ => {}
|
ResponseItem::Message { role, .. } if role == "user" => false,
|
||||||
|
_ => false,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ use crate::truncate;
|
|||||||
use crate::truncate::TruncationPolicy;
|
use crate::truncate::TruncationPolicy;
|
||||||
use codex_git::GhostCommit;
|
use codex_git::GhostCommit;
|
||||||
use codex_protocol::models::ContentItem;
|
use codex_protocol::models::ContentItem;
|
||||||
|
use codex_protocol::models::FunctionCallOutputContentItem;
|
||||||
use codex_protocol::models::FunctionCallOutputPayload;
|
use codex_protocol::models::FunctionCallOutputPayload;
|
||||||
use codex_protocol::models::LocalShellAction;
|
use codex_protocol::models::LocalShellAction;
|
||||||
use codex_protocol::models::LocalShellExecAction;
|
use codex_protocol::models::LocalShellExecAction;
|
||||||
@@ -209,6 +210,58 @@ fn remove_first_item_removes_matching_call_for_output() {
|
|||||||
assert_eq!(h.raw_items(), vec![]);
|
assert_eq!(h.raw_items(), vec![]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn replace_last_turn_images_replaces_tool_output_images() {
|
||||||
|
let items = vec![
|
||||||
|
user_input_text_msg("hi"),
|
||||||
|
ResponseItem::FunctionCallOutput {
|
||||||
|
call_id: "call-1".to_string(),
|
||||||
|
output: FunctionCallOutputPayload {
|
||||||
|
content: "ok".to_string(),
|
||||||
|
content_items: Some(vec![FunctionCallOutputContentItem::InputImage {
|
||||||
|
image_url: "data:image/png;base64,AAA".to_string(),
|
||||||
|
}]),
|
||||||
|
success: Some(true),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
];
|
||||||
|
let mut history = create_history_with_items(items);
|
||||||
|
|
||||||
|
assert!(history.replace_last_turn_images("Invalid image"));
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
history.raw_items(),
|
||||||
|
vec![
|
||||||
|
user_input_text_msg("hi"),
|
||||||
|
ResponseItem::FunctionCallOutput {
|
||||||
|
call_id: "call-1".to_string(),
|
||||||
|
output: FunctionCallOutputPayload {
|
||||||
|
content: "ok".to_string(),
|
||||||
|
content_items: Some(vec![FunctionCallOutputContentItem::InputText {
|
||||||
|
text: "Invalid image".to_string(),
|
||||||
|
}]),
|
||||||
|
success: Some(true),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
]
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn replace_last_turn_images_does_not_touch_user_images() {
|
||||||
|
let items = vec![ResponseItem::Message {
|
||||||
|
id: None,
|
||||||
|
role: "user".to_string(),
|
||||||
|
content: vec![ContentItem::InputImage {
|
||||||
|
image_url: "data:image/png;base64,AAA".to_string(),
|
||||||
|
}],
|
||||||
|
}];
|
||||||
|
let mut history = create_history_with_items(items.clone());
|
||||||
|
|
||||||
|
assert!(!history.replace_last_turn_images("Invalid image"));
|
||||||
|
assert_eq!(history.raw_items(), items);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn remove_first_item_handles_local_shell_pair() {
|
fn remove_first_item_handles_local_shell_pair() {
|
||||||
let items = vec![
|
let items = vec![
|
||||||
|
|||||||
Reference in New Issue
Block a user