From 091373c046b6b9d230c3e01405beaef9750f7ea2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Wed, 2 Sep 2026 10:50:31 +0200 Subject: [PATCH] fix(translation): fail closed on malformed Responses input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Responses-to-Chat Completions translation silently dropped or synthesized malformed input instead of rejecting it: scalar input became an empty message list, non-object items were skipped, missing message role/content were defaulted, and function calls and outputs missing call_id, name, arguments, or output were dropped. The backend then received a different, valid-looking Chat Completions request and could not reject the client's invalid Responses request. Validate translation-required structure before canonical state erases the original scalar input, and return the existing TranslationError to HTTP 400 path for unsupported input types and missing required item fields. Valid requests are unchanged. Closes #552 Signed-off-by: Sébastien Han --- .../responses_to_chat_completions/tests.rs | 52 +++++++++ .../openai/translation/chat_completions.rs | 108 ++++++++++++------ apis/src/openai/translation/mod.rs | 96 +++++++++++----- .../examples/responses_to_chat_completions.rs | 32 ++++++ 4 files changed, 224 insertions(+), 64 deletions(-) diff --git a/apis/src/openai/responses/responses_to_chat_completions/tests.rs b/apis/src/openai/responses/responses_to_chat_completions/tests.rs index 859d37c5c..24e837f40 100644 --- a/apis/src/openai/responses/responses_to_chat_completions/tests.rs +++ b/apis/src/openai/responses/responses_to_chat_completions/tests.rs @@ -313,6 +313,58 @@ async fn canonical_state_is_translated_and_arms_response() { assert_eq!(context.get_metadata(CREATED_AT_KEY), Some("1700000000")); } +#[tokio::test] +async fn malformed_responses_input_is_rejected_before_request_translation() { + let cases = [ + ( + "scalar input", + json!({"model": "m", "input": 42}), + "unsupported Responses input type for Chat Completions translation: number", + ), + ( + "non-object input item", + json!({"model": "m", "input": [42]}), + "Responses input item must be a JSON object", + ), + ( + "function call without call_id", + json!({"model": "m", "input": [{"type": "function_call", "name": "lookup", "arguments": "{}"}]}), + "Responses function_call input item is missing required field `call_id`", + ), + ]; + + for (case, request_body, expected_message) in cases { + let filter = ResponsesToChatCompletionsFilter::from_config(&serde_yaml::Value::Null).unwrap(); + let request = crate::test_utils::make_request(http::Method::POST, "/v1/responses"); + let mut context = crate::test_utils::make_filter_context(&request); + context.set_metadata("openai_responses_format.format", "openai_responses"); + context + .extensions + .insert(ResponsesState::from_request_body(request_body.clone())); + let original = Bytes::from(serde_json::to_vec(&request_body).unwrap()); + let mut body = Some(original.clone()); + + let action = filter.on_request_body(&mut context, &mut body, true).await.unwrap(); + + let FilterAction::Reject(rejection) = action else { + panic!("{case} should be rejected"); + }; + assert_eq!(rejection.status, 400, "{case} should produce a client error"); + let parsed: serde_json::Value = serde_json::from_slice(rejection.body.as_deref().unwrap()).unwrap(); + assert_eq!(parsed["error"]["code"], "invalid_request_error", "{case}"); + assert_eq!(parsed["error"]["message"], expected_message, "{case}"); + assert_eq!( + body.as_deref(), + Some(original.as_ref()), + "{case} should not rewrite the body" + ); + assert!( + context.get_metadata(ARMED_KEY).is_none(), + "{case} must not arm response processing" + ); + } +} + #[tokio::test] async fn rehydrated_previous_response_id_translates_full_history() { let filter = ResponsesToChatCompletionsFilter::from_config(&serde_yaml::Value::Null).unwrap(); diff --git a/apis/src/openai/translation/chat_completions.rs b/apis/src/openai/translation/chat_completions.rs index 0351cc5ee..95d917b71 100644 --- a/apis/src/openai/translation/chat_completions.rs +++ b/apis/src/openai/translation/chat_completions.rs @@ -5,7 +5,6 @@ use serde_json::{Map, Number, Value, json}; use thiserror::Error; -use tracing::warn; // ----------------------------------------------------------------------------- // Constants @@ -173,6 +172,25 @@ pub(crate) enum TranslationError { /// The provided JSON value was not the expected object type. #[error("{0} must be a JSON object")] ExpectedObject(&'static str), + /// A Responses input value has no valid Chat Completions representation. + #[error("unsupported Responses input type for Chat Completions translation: {0}")] + UnsupportedInputType(&'static str), + /// A Responses input item omitted a field required for faithful translation. + #[error("Responses {item_type} input item is missing required field `{field}`")] + MissingInputItemField { + /// Stable Responses input item type. + item_type: &'static str, + /// Required field that was absent. + field: &'static str, + }, + /// A Responses input item field has the wrong type for translation. + #[error("Responses {item_type} input item field `{field}` must be a string")] + InvalidInputItemStringField { + /// Stable Responses input item type. + item_type: &'static str, + /// String field whose value had another JSON type. + field: &'static str, + }, /// A Responses input item has no Chat Completions-compatible representation. #[error("unsupported Responses input item type for Chat Completions translation: {0}")] UnsupportedInputItemType(String), @@ -232,6 +250,7 @@ fn translate_responses_request(request: &Value, overrides: RequestOverrides<'_>) let obj = request .as_object() .ok_or(TranslationError::ExpectedObject("Responses request"))?; + validate_input_container(obj.get("input"))?; let mut chat = Map::new(); map_request_parameters(obj, &mut chat); @@ -397,12 +416,8 @@ fn append_input_messages(messages: &mut Vec, input: &Value) -> Result<(), Value::String(text) => messages.push(json!({"role": "user", "content": text})), Value::Array(items) => append_input_item_sequence(messages, items)?, Value::Object(_) => append_input_item_sequence(messages, std::slice::from_ref(input))?, - _ => { - warn!( - input_type = json_type_name(input), - "dropping unsupported Responses input during Chat Completions translation" - ); - }, + Value::Null => {}, + _ => return Err(unsupported_input_type(input)), } Ok(()) @@ -415,9 +430,7 @@ fn append_input_item_sequence(messages: &mut Vec, items: &[Value]) -> Res if let Some(obj) = item.as_object() && obj.get("type").and_then(Value::as_str) == Some("function_call") { - if let Some(tool_call) = function_call_tool_call(obj) { - pending_tool_calls.push(tool_call); - } + pending_tool_calls.push(function_call_tool_call(obj)?); continue; } @@ -444,11 +457,11 @@ fn flush_pending_function_calls(messages: &mut Vec, pending_tool_calls: & /// Convert a single `Responses` input item into one Chat Completions message. fn append_input_item(messages: &mut Vec, item: &Value) -> Result<(), TranslationError> { let Some(obj) = item.as_object() else { - return Ok(()); + return Err(TranslationError::ExpectedObject("Responses input item")); }; match obj.get("type").and_then(Value::as_str) { - Some("function_call_output") => append_tool_output(messages, obj), + Some("function_call_output") => append_tool_output(messages, obj)?, Some("message") => append_message_item(messages, obj)?, Some("compaction") => append_compaction_item(messages, obj), None if obj.contains_key("role") || obj.contains_key("content") => append_message_item(messages, obj)?, @@ -461,10 +474,12 @@ fn append_input_item(messages: &mut Vec, item: &Value) -> Result<(), Tran /// Convert a Responses message item into a Chat Completions message. fn append_message_item(messages: &mut Vec, obj: &Map) -> Result<(), TranslationError> { - let role = obj.get("role").and_then(Value::as_str).unwrap_or("user"); - let content = obj - .get("content") - .map_or_else(|| Ok(json!("")), convert_input_content)?; + let role = required_input_item_string(obj, "message", "role")?; + let content = obj.get("content").ok_or(TranslationError::MissingInputItemField { + item_type: "message", + field: "content", + })?; + let content = convert_input_content(content)?; messages.push(json!({"role": role, "content": content})); Ok(()) } @@ -490,38 +505,65 @@ fn append_compaction_item(messages: &mut Vec, obj: &Map) { } /// Convert one Responses function-call item to a Chat tool-call object. -fn function_call_tool_call(obj: &Map) -> Option { - let Some(call_id) = obj.get("call_id").and_then(Value::as_str) else { - warn!("dropping Responses function_call without call_id during Chat Completions translation"); - return None; - }; - let Some(name) = obj.get("name").and_then(Value::as_str) else { - warn!("dropping Responses function_call without name during Chat Completions translation"); - return None; - }; +fn function_call_tool_call(obj: &Map) -> Result { + let call_id = required_input_item_string(obj, "function_call", "call_id")?; + let name = required_input_item_string(obj, "function_call", "name")?; + let arguments = obj.get("arguments").ok_or(TranslationError::MissingInputItemField { + item_type: "function_call", + field: "arguments", + })?; - Some(json!({ + Ok(json!({ "id": call_id, "type": "function", "function": { "name": name, - "arguments": chat_string_field(obj.get("arguments")), + "arguments": chat_string_field(Some(arguments)), } })) } /// Convert a `Responses` function call output item into a Chat tool message. -fn append_tool_output(messages: &mut Vec, obj: &Map) { - let Some(call_id) = obj.get("call_id").and_then(Value::as_str) else { - warn!("dropping Responses function_call_output without call_id during Chat Completions translation"); - return; - }; +fn append_tool_output(messages: &mut Vec, obj: &Map) -> Result<(), TranslationError> { + let call_id = required_input_item_string(obj, "function_call_output", "call_id")?; + let output = obj.get("output").ok_or(TranslationError::MissingInputItemField { + item_type: "function_call_output", + field: "output", + })?; messages.push(json!({ "role": "tool", "tool_call_id": call_id, - "content": chat_string_field(obj.get("output")) + "content": chat_string_field(Some(output)) })); + Ok(()) +} + +/// Validate the outer Responses input shape before canonical state overrides +/// can hide an invalid scalar value. +fn validate_input_container(input: Option<&Value>) -> Result<(), TranslationError> { + match input { + None | Some(Value::Null | Value::String(_) | Value::Array(_) | Value::Object(_)) => Ok(()), + Some(input) => Err(unsupported_input_type(input)), + } +} + +/// Build a stable error for an unsupported outer input value. +fn unsupported_input_type(input: &Value) -> TranslationError { + TranslationError::UnsupportedInputType(json_type_name(input)) +} + +/// Read a required string field from a Responses input item. +fn required_input_item_string<'a>( + obj: &'a Map, + item_type: &'static str, + field: &'static str, +) -> Result<&'a str, TranslationError> { + match obj.get(field) { + Some(Value::String(value)) => Ok(value), + Some(_) => Err(TranslationError::InvalidInputItemStringField { item_type, field }), + None => Err(TranslationError::MissingInputItemField { item_type, field }), + } } /// Convert an optional JSON field to Chat's string-valued history fields. diff --git a/apis/src/openai/translation/mod.rs b/apis/src/openai/translation/mod.rs index 08fcb2698..9595f1b2d 100644 --- a/apis/src/openai/translation/mod.rs +++ b/apis/src/openai/translation/mod.rs @@ -152,11 +152,11 @@ mod tests { } #[test] - fn simple_inputs_map_or_drop_cleanly() { + fn simple_inputs_map_cleanly() { let string_input = map(&json!({"model": "gpt-4o-mini", "instructions": "", "input": "Hello"})); let object_input = map(&json!({"model": "gpt-4o-mini", "input": {"role": "developer", "content": "terse"}})); let no_input = map(&json!({"model": "gpt-4o-mini"})); - let unsupported_input = map(&json!({"model": "gpt-4o-mini", "input": 42})); + let null_input = map(&json!({"model": "gpt-4o-mini", "input": null})); assert_eq!(string_input["messages"], json!([{"role": "user", "content": "Hello"}])); assert_eq!( @@ -164,7 +164,17 @@ mod tests { json!([{"role": "developer", "content": "terse"}]) ); assert_eq!(no_input["messages"], Value::Array(Vec::new())); - assert_eq!(unsupported_input["messages"], Value::Array(Vec::new())); + assert_eq!(null_input["messages"], Value::Array(Vec::new())); + } + + #[test] + fn scalar_input_returns_error() { + let error = map_error(&json!({"model": "gpt-4o-mini", "input": 42})); + + assert_eq!( + error, + "unsupported Responses input type for Chat Completions translation: number" + ); } #[test] @@ -871,13 +881,16 @@ mod tests { // ------------------------------------------------------------------------- #[test] - fn message_item_without_content_gets_empty_content() { - let mapped = map(&json!({ + fn message_item_without_content_returns_error() { + let error = map_error(&json!({ "model": "m", "input": [{"role": "user"}] })); - assert_eq!(mapped["messages"][0]["content"], ""); + assert_eq!( + error, + "Responses message input item is missing required field `content`" + ); } #[test] @@ -894,25 +907,23 @@ mod tests { } #[test] - fn untyped_item_with_content_key_maps_as_message() { - let mapped = map(&json!({ + fn untyped_item_without_role_returns_error() { + let error = map_error(&json!({ "model": "m", "input": [{"content": "implicit user"}] })); - assert_eq!(mapped["messages"][0]["role"], "user"); - assert_eq!(mapped["messages"][0]["content"], "implicit user"); + assert_eq!(error, "Responses message input item is missing required field `role`"); } #[test] - fn non_object_input_items_are_skipped() { - let mapped = map(&json!({ + fn non_object_input_items_return_error() { + let error = map_error(&json!({ "model": "m", "input": [42, "bare string", {"role": "user", "content": "real"}] })); - assert_eq!(mapped["messages"].as_array().unwrap().len(), 1); - assert_eq!(mapped["messages"][0]["content"], "real"); + assert_eq!(error, "Responses input item must be a JSON object"); } // ------------------------------------------------------------------------- @@ -1027,8 +1038,8 @@ mod tests { } #[test] - fn function_call_without_arguments_uses_empty_string() { - let mapped = map(&json!({ + fn function_call_without_arguments_returns_error() { + let error = map_error(&json!({ "model": "m", "input": [{ "type": "function_call", @@ -1037,24 +1048,31 @@ mod tests { }] })); - assert_eq!(mapped["messages"][0]["tool_calls"][0]["function"]["arguments"], ""); + assert_eq!( + error, + "Responses function_call input item is missing required field `arguments`" + ); } #[test] - fn malformed_function_calls_without_call_id_or_name_are_dropped() { - let mapped = map(&json!({ + fn malformed_function_calls_without_call_id_or_name_return_errors() { + let missing_id = map_error(&json!({ "model": "m", - "input": [ - {"type": "function_call", "name": "missing_id", "arguments": "{}"}, - {"type": "function_call", "call_id": "missing_name", "arguments": "{}"}, - {"type": "function_call", "call_id": "valid_call", "name": "valid_function", "arguments": "{}"} - ] + "input": [{"type": "function_call", "name": "missing_id", "arguments": "{}"}] + })); + let missing_name = map_error(&json!({ + "model": "m", + "input": [{"type": "function_call", "call_id": "missing_name", "arguments": "{}"}] })); - let tool_calls = mapped["messages"][0]["tool_calls"].as_array().unwrap(); - assert_eq!(tool_calls.len(), 1); - assert_eq!(tool_calls[0]["id"], "valid_call"); - assert_eq!(tool_calls[0]["function"]["name"], "valid_function"); + assert_eq!( + missing_id, + "Responses function_call input item is missing required field `call_id`" + ); + assert_eq!( + missing_name, + "Responses function_call input item is missing required field `name`" + ); } #[test] @@ -1071,8 +1089,8 @@ mod tests { } #[test] - fn function_call_output_without_output_uses_empty_string() { - let mapped = map(&json!({ + fn function_call_output_without_output_returns_error() { + let error = map_error(&json!({ "model": "m", "input": [ {"type": "function_call", "call_id": "c1", "name": "f", "arguments": "{}"}, @@ -1080,7 +1098,23 @@ mod tests { ] })); - assert_eq!(mapped["messages"][1]["content"], ""); + assert_eq!( + error, + "Responses function_call_output input item is missing required field `output`" + ); + } + + #[test] + fn function_call_output_without_call_id_returns_error() { + let error = map_error(&json!({ + "model": "m", + "input": [{"type": "function_call_output", "output": "done"}] + })); + + assert_eq!( + error, + "Responses function_call_output input item is missing required field `call_id`" + ); } #[test] diff --git a/tests/integration/tests/suite/examples/responses_to_chat_completions.rs b/tests/integration/tests/suite/examples/responses_to_chat_completions.rs index b934d73c4..c35c22e3b 100644 --- a/tests/integration/tests/suite/examples/responses_to_chat_completions.rs +++ b/tests/integration/tests/suite/examples/responses_to_chat_completions.rs @@ -82,6 +82,38 @@ fn responses_to_chat_completions_translates_request_and_response() { assert_eq!(response["usage"]["output_tokens"], 3); } +#[test] +fn responses_to_chat_completions_rejects_malformed_input_before_upstream() { + let backend = StatefulCapturingBackend::new(vec![(200, r#"{}"#.to_owned())]).start_with_shutdown(); + let proxy_port = free_port(); + let (config, _db) = load_test_config( + "malformed_input", + proxy_port, + &HashMap::from([("127.0.0.1:3001", backend.port())]), + ); + let proxy = start_proxy(&config); + + let raw = http_send( + proxy.addr(), + &json_post( + "/v1/responses", + r#"{"model":"gpt-4.1-mini","input":42,"stream":false,"store":false}"#, + ), + ); + let response: serde_json::Value = serde_json::from_str(&parse_body(&raw)).expect("error response should be JSON"); + + assert_eq!(parse_status(&raw), 400); + assert_eq!(response["error"]["type"], "invalid_request_error"); + assert_eq!( + response["error"]["message"], + "unsupported Responses input type for Chat Completions translation: number" + ); + assert!( + backend.requests().is_empty(), + "malformed input must not reach the backend" + ); +} + #[test] fn responses_to_chat_completions_normalizes_finite_provider_error() { let backend = Backend::status(429, r#"{"error":{"code":"rate_limit_exceeded","message":"slow down"}}"#)