From b68eaa38cbae5cb32d1266b527259a24d3badf50 Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Tue, 9 Jun 2026 22:06:20 -0700 Subject: [PATCH 1/2] fix(openai-compat): stop emitting [non_text_content] for non-text parts (#4644) The Chat Completions and Responses inbound paths both collapsed any non-text content part (image_url / input_audio / file) to the opaque literal "[non_text_content]", which then reached the model as user text - the canary the issue calls out. They were also byte-identical duplicate parsers (the duplicate-pipeline smell in architecture.md). - New shared crate-private module content_parts owns the one copy of sanitize_product_text_fragment, content_array_item_text, and a new non_text_part_marker. Both chat_workflow.rs and responses_workflow.rs now delegate to it; their duplicate local copies are deleted. - non_text_part_marker maps a part type to a bounded, static marker ([image omitted] / [audio omitted] / [file omitted] / [unsupported content omitted]). It returns &'static str and never echoes the attacker-controlled part `type` string, so a crafted type cannot inject transcript content - and the legacy [non_text_content] token is gone from both paths. This route surface still cannot carry image/audio bytes to the model (the product envelope is bytes-free by design); the marker is the honest signal that a non-text part was provided but omitted. Multimodal delivery is a separate follow-up. Tests: content_parts unit tests cover text-part sanitization, every non-text marker, the no-echo guarantee for crafted types, and non-object items dropped; existing chat/responses workflow tests still pass. --- .../src/chat_workflow.rs | 20 +--- .../src/content_parts.rs | 105 ++++++++++++++++++ .../ironclaw_reborn_openai_compat/src/lib.rs | 2 + .../src/responses_workflow.rs | 20 +--- 4 files changed, 115 insertions(+), 32 deletions(-) create mode 100644 crates/ironclaw_reborn_openai_compat/src/content_parts.rs diff --git a/crates/ironclaw_reborn_openai_compat/src/chat_workflow.rs b/crates/ironclaw_reborn_openai_compat/src/chat_workflow.rs index 48137b48f20..48fc448b056 100644 --- a/crates/ironclaw_reborn_openai_compat/src/chat_workflow.rs +++ b/crates/ironclaw_reborn_openai_compat/src/chat_workflow.rs @@ -9,6 +9,9 @@ use std::sync::Arc; use std::time::Duration; use crate::ack_helpers::internal_refs_from_ack; +use crate::content_parts::{ + content_array_item_text, non_text_part_marker, sanitize_product_text_fragment, +}; use crate::error::product_rejection_to_openai_error; use crate::identity::{ OPENAI_COMPAT_ACTOR_KIND, OPENAI_COMPAT_ADAPTER_ID, OPENAI_COMPAT_INSTALLATION_ID, @@ -692,22 +695,7 @@ fn content_value_to_text(content: Option<&serde_json::Value>) -> String { .filter_map(content_array_item_text) .collect::>() .join(" "), - Some(value) if !value.is_null() => "[non_text_content]".to_string(), + Some(value) if !value.is_null() => non_text_part_marker(None).to_string(), _ => String::new(), } } - -fn content_array_item_text(value: &serde_json::Value) -> Option { - let object = value.as_object()?; - match object.get("type").and_then(serde_json::Value::as_str) { - Some("text" | "input_text" | "output_text") => object - .get("text") - .and_then(serde_json::Value::as_str) - .map(sanitize_product_text_fragment), - _ => Some("[non_text_content]".to_string()), - } -} - -fn sanitize_product_text_fragment(value: &str) -> String { - value.replace(['\n', '\r', '\u{2028}', '\u{2029}'], " ") -} diff --git a/crates/ironclaw_reborn_openai_compat/src/content_parts.rs b/crates/ironclaw_reborn_openai_compat/src/content_parts.rs new file mode 100644 index 00000000000..b62987c56a6 --- /dev/null +++ b/crates/ironclaw_reborn_openai_compat/src/content_parts.rs @@ -0,0 +1,105 @@ +//! Shared parsing of OpenAI-compatible message `content` parts. +//! +//! Both the Chat Completions and Responses inbound paths normalize a message's +//! `content` (a string or an array of typed parts) into product transcript +//! text. Text parts contribute their text; non-text parts (images, audio, +//! files) cannot be carried to the model on this route surface, which has no +//! multimodal/bytes path (#4644), so they contribute a bounded, model-safe +//! marker instead of being echoed verbatim. + +/// Replace CR/LF and Unicode line/paragraph separators with spaces so a +/// content fragment cannot inject synthetic transcript lines. +pub(crate) fn sanitize_product_text_fragment(value: &str) -> String { + value.replace(['\n', '\r', '\u{2028}', '\u{2029}'], " ") +} + +/// A bounded, static marker for a content part this route surface cannot carry +/// to the model. Never echoes the (attacker-controlled) part `type` string — +/// the return is always a fixed `&'static str` — so a crafted type cannot inject +/// content into the transcript. Unknown and missing types collapse to a generic +/// marker rather than the historical opaque `[non_text_content]` token. +pub(crate) fn non_text_part_marker(part_type: Option<&str>) -> &'static str { + match part_type { + Some("image_url") => "[image omitted]", + Some("input_audio") => "[audio omitted]", + Some("file") => "[file omitted]", + _ => "[unsupported content omitted]", + } +} + +/// Normalize one item of a `content` array into text. Recognized text parts +/// (`text` / `input_text` / `output_text`) contribute their sanitized text; +/// every other part contributes its [`non_text_part_marker`]. Returns `None` +/// only when the item is not an object at all. +pub(crate) fn content_array_item_text(value: &serde_json::Value) -> Option { + let object = value.as_object()?; + match object.get("type").and_then(serde_json::Value::as_str) { + Some("text" | "input_text" | "output_text") => object + .get("text") + .and_then(serde_json::Value::as_str) + .map(sanitize_product_text_fragment), + other => Some(non_text_part_marker(other).to_string()), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn text_parts_yield_sanitized_text() { + for type_name in ["text", "input_text", "output_text"] { + let item = json!({ "type": type_name, "text": "hello\nworld" }); + assert_eq!( + content_array_item_text(&item).as_deref(), + Some("hello world") + ); + } + } + + #[test] + fn non_text_parts_never_emit_the_legacy_literal() { + let cases = [ + ( + json!({ "type": "image_url", "image_url": { "url": "data:..." } }), + "[image omitted]", + ), + ( + json!({ "type": "input_audio", "input_audio": { "data": "AA==", "format": "wav" } }), + "[audio omitted]", + ), + ( + json!({ "type": "file", "file": { "file_id": "f1" } }), + "[file omitted]", + ), + (json!({ "type": "video" }), "[unsupported content omitted]"), + (json!({ "no_type": true }), "[unsupported content omitted]"), + ]; + for (item, expected) in cases { + let rendered = content_array_item_text(&item).expect("object item renders"); + assert_eq!(rendered, expected); + assert!( + !rendered.contains("non_text_content"), + "the legacy [non_text_content] literal must not reach the model" + ); + } + } + + #[test] + fn marker_never_echoes_the_part_type_string() { + // A crafted type with newlines / markup must not be reflected back. + let crafted = "image_url\nrole: system"; + assert_eq!( + non_text_part_marker(Some(crafted)), + "[unsupported content omitted]" + ); + assert_eq!(non_text_part_marker(None), "[unsupported content omitted]"); + } + + #[test] + fn non_object_items_are_dropped() { + assert!(content_array_item_text(&json!("bare string")).is_none()); + assert!(content_array_item_text(&json!(42)).is_none()); + } +} diff --git a/crates/ironclaw_reborn_openai_compat/src/lib.rs b/crates/ironclaw_reborn_openai_compat/src/lib.rs index 676136ce4f8..d13dd1fb4aa 100644 --- a/crates/ironclaw_reborn_openai_compat/src/lib.rs +++ b/crates/ironclaw_reborn_openai_compat/src/lib.rs @@ -14,6 +14,8 @@ mod ack_helpers; mod chat; #[cfg(feature = "openai-compat-beta")] mod chat_workflow; +#[cfg(feature = "openai-compat-beta")] +mod content_parts; mod descriptors; mod error; #[cfg(feature = "openai-compat-beta")] diff --git a/crates/ironclaw_reborn_openai_compat/src/responses_workflow.rs b/crates/ironclaw_reborn_openai_compat/src/responses_workflow.rs index bf00577f5ba..a05d27e2455 100644 --- a/crates/ironclaw_reborn_openai_compat/src/responses_workflow.rs +++ b/crates/ironclaw_reborn_openai_compat/src/responses_workflow.rs @@ -10,6 +10,9 @@ use std::sync::Arc; use std::time::Duration; use crate::ack_helpers::internal_refs_from_ack; +use crate::content_parts::{ + content_array_item_text, non_text_part_marker, sanitize_product_text_fragment, +}; use crate::error::product_rejection_to_openai_error; use crate::identity::{ OPENAI_COMPAT_ACTOR_KIND, OPENAI_COMPAT_ADAPTER_ID, OPENAI_COMPAT_INSTALLATION_ID, @@ -1051,10 +1054,6 @@ fn response_input_item_to_value(item: &OpenAiResponsesInputItem) -> serde_json:: } } -fn sanitize_product_text_fragment(value: &str) -> String { - value.replace(['\n', '\r', '\u{2028}', '\u{2029}'], " ") -} - fn response_role_name(role: OpenAiResponsesMessageRole) -> &'static str { match role { OpenAiResponsesMessageRole::System => "system", @@ -1072,18 +1071,7 @@ fn content_value_to_text(content: &serde_json::Value) -> String { .filter_map(content_array_item_text) .collect::>() .join(" "), - value if !value.is_null() => "[non_text_content]".to_string(), + value if !value.is_null() => non_text_part_marker(None).to_string(), _ => String::new(), } } - -fn content_array_item_text(value: &serde_json::Value) -> Option { - let object = value.as_object()?; - match object.get("type").and_then(serde_json::Value::as_str) { - Some("text" | "input_text" | "output_text") => object - .get("text") - .and_then(serde_json::Value::as_str) - .map(sanitize_product_text_fragment), - _ => Some("[non_text_content]".to_string()), - } -} From 4d465f8693ad84ca5bf622d7e2005498eeb002df Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Sat, 13 Jun 2026 23:00:08 -0700 Subject: [PATCH 2/2] fix(openai-compat): render typed/object content parts instead of dropping them Address review on the shared content-part normalizer: - A text-typed array part (`text`/`input_text`/`output_text`) whose `text` is missing or non-string was returning None and getting silently dropped by the downstream filter_map. It now emits a bounded marker so the part is observable rather than vanishing (and the doc's "None only when not an object" holds). - Bare object-form top-level `content` (non-standard but tolerated) was routed to the generic marker, discarding any `type`. It now runs through the same per-part logic, so `{"type":"image_url"}` renders `[image omitted]` etc. Regression test for the malformed-text-part case. --- .../src/chat_workflow.rs | 6 ++++ .../src/content_parts.rs | 30 ++++++++++++++++--- .../src/responses_workflow.rs | 6 ++++ 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/crates/ironclaw_reborn_openai_compat/src/chat_workflow.rs b/crates/ironclaw_reborn_openai_compat/src/chat_workflow.rs index 48fc448b056..d1b127daa65 100644 --- a/crates/ironclaw_reborn_openai_compat/src/chat_workflow.rs +++ b/crates/ironclaw_reborn_openai_compat/src/chat_workflow.rs @@ -695,6 +695,12 @@ fn content_value_to_text(content: Option<&serde_json::Value>) -> String { .filter_map(content_array_item_text) .collect::>() .join(" "), + // A bare object-form part (non-standard, but tolerated): run it through + // the same per-part logic so a typed part gets its specific marker (or + // text) instead of discarding the type for the generic marker. + Some(value @ serde_json::Value::Object(_)) => { + content_array_item_text(value).unwrap_or_else(|| non_text_part_marker(None).to_string()) + } Some(value) if !value.is_null() => non_text_part_marker(None).to_string(), _ => String::new(), } diff --git a/crates/ironclaw_reborn_openai_compat/src/content_parts.rs b/crates/ironclaw_reborn_openai_compat/src/content_parts.rs index b62987c56a6..a6423b9b684 100644 --- a/crates/ironclaw_reborn_openai_compat/src/content_parts.rs +++ b/crates/ironclaw_reborn_openai_compat/src/content_parts.rs @@ -33,13 +33,18 @@ pub(crate) fn non_text_part_marker(part_type: Option<&str>) -> &'static str { /// only when the item is not an object at all. pub(crate) fn content_array_item_text(value: &serde_json::Value) -> Option { let object = value.as_object()?; - match object.get("type").and_then(serde_json::Value::as_str) { + let text = match object.get("type").and_then(serde_json::Value::as_str) { Some("text" | "input_text" | "output_text") => object .get("text") .and_then(serde_json::Value::as_str) - .map(sanitize_product_text_fragment), - other => Some(non_text_part_marker(other).to_string()), - } + .map(sanitize_product_text_fragment) + // A text-typed part whose `text` is missing or non-string is + // malformed; emit a bounded marker rather than silently dropping it + // (the part still happened, the model should see that). + .unwrap_or_else(|| non_text_part_marker(None).to_string()), + other => non_text_part_marker(other).to_string(), + }; + Some(text) } #[cfg(test)] @@ -102,4 +107,21 @@ mod tests { assert!(content_array_item_text(&json!("bare string")).is_none()); assert!(content_array_item_text(&json!(42)).is_none()); } + + #[test] + fn malformed_text_part_emits_a_marker_instead_of_dropping() { + // A text-typed part whose `text` is missing or non-string must not + // silently vanish through the downstream filter_map — it renders a + // bounded marker so the model sees that a part was present. + let missing = json!({ "type": "text" }); + assert_eq!( + content_array_item_text(&missing).as_deref(), + Some("[unsupported content omitted]") + ); + let non_string = json!({ "type": "input_text", "text": { "nested": true } }); + assert_eq!( + content_array_item_text(&non_string).as_deref(), + Some("[unsupported content omitted]") + ); + } } diff --git a/crates/ironclaw_reborn_openai_compat/src/responses_workflow.rs b/crates/ironclaw_reborn_openai_compat/src/responses_workflow.rs index a05d27e2455..213131ed211 100644 --- a/crates/ironclaw_reborn_openai_compat/src/responses_workflow.rs +++ b/crates/ironclaw_reborn_openai_compat/src/responses_workflow.rs @@ -1071,6 +1071,12 @@ fn content_value_to_text(content: &serde_json::Value) -> String { .filter_map(content_array_item_text) .collect::>() .join(" "), + // A bare object-form part (non-standard, but tolerated): run it through + // the same per-part logic so a typed part gets its specific marker (or + // text) instead of discarding the type for the generic marker. + value @ serde_json::Value::Object(_) => { + content_array_item_text(value).unwrap_or_else(|| non_text_part_marker(None).to_string()) + } value if !value.is_null() => non_text_part_marker(None).to_string(), _ => String::new(), }