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"}}"#)