From 74a4425cae6776b4c171b83482fd5e9484050751 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Wed, 2 Sep 2026 10:44:54 +0200 Subject: [PATCH 1/2] fix(translation): fail-closed on malformed Chat Completions success MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Responses translator accepted any JSON object as a successful Chat Completions response. A backend returning HTTP 200 with a malformed body such as `{}` was translated into a fake successful Responses object (`status: "completed"`, `error: null`, `output: []`), hiding the upstream failure from the client. Validate the minimum successful shape before translating: a non-empty `choices` array, a first choice object, a supported `finish_reason` (stop/length/tool_calls/content_filter), an assistant `message`, and at least one translatable output (content, refusal, or a well-formed function tool call). A `tool_calls` finish reason now requires function tool calls. Malformed responses fail closed as HTTP 500 instead of surfacing a counterfeit success. Add unit coverage for each rejected shape, a filter-boundary test that aborts after headers are sent, and an end-to-end regression asserting a finite HTTP 200 `{}` becomes a 500. Closes #804 Signed-off-by: Sébastien Han --- .../responses_to_chat_completions/tests.rs | 33 +++++ .../openai/translation/chat_completions.rs | 137 +++++++++++++++++- apis/src/openai/translation/mod.rs | 78 ++++++++-- .../examples/responses_to_chat_completions.rs | 22 +++ 4 files changed, 252 insertions(+), 18 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..954d38ed6 100644 --- a/apis/src/openai/responses/responses_to_chat_completions/tests.rs +++ b/apis/src/openai/responses/responses_to_chat_completions/tests.rs @@ -792,6 +792,39 @@ async fn malformed_success_aborts_after_headers_are_sent() { assert_eq!(body.as_deref(), Some(b"not-json".as_slice())); } +#[tokio::test] +async fn malformed_success_shape_aborts_after_headers_are_sent() { + let yaml = serde_yaml::from_str("{}").unwrap(); + let filter = ResponsesToChatCompletionsFilter::from_config(&yaml).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(ARMED_KEY, "true"); + context.set_metadata(CREATED_AT_KEY, "1700000000"); + context.set_metadata("responses.response_id", "resp_test_123"); + context.extensions.insert(ResponsesState::from_request_body(json!({ + "model": "gpt-4.1-mini", + "input": "hello" + }))); + let response = Box::leak(Box::new(crate::test_utils::make_response())); + response.headers.insert( + http::header::CONTENT_TYPE, + http::HeaderValue::from_static("application/json"), + ); + context.response_header = Some(response); + assert!(matches!( + filter.on_response(&mut context).await.unwrap(), + FilterAction::Continue + )); + context.response_header = None; + let original = Bytes::from_static(b"{}"); + let mut body = Some(original.clone()); + + let error = filter.on_response_body(&mut context, &mut body, true).unwrap_err(); + + assert!(error.to_string().contains("choices must be an array")); + assert_eq!(body.as_deref(), Some(original.as_ref())); +} + #[tokio::test] async fn finite_provider_error_uses_captured_status_without_mutable_headers() { let yaml = serde_yaml::from_str("{}").unwrap(); diff --git a/apis/src/openai/translation/chat_completions.rs b/apis/src/openai/translation/chat_completions.rs index 0351cc5ee..9fc288247 100644 --- a/apis/src/openai/translation/chat_completions.rs +++ b/apis/src/openai/translation/chat_completions.rs @@ -188,6 +188,9 @@ pub(crate) enum TranslationError { /// A Responses tool choice has no Chat Completions-compatible representation. #[error("unsupported Responses tool_choice type for Chat Completions translation: {0}")] UnsupportedToolChoiceType(String), + /// A successful Chat Completions response is missing required translation state. + #[error("invalid Chat Completions response: {0}")] + InvalidChatResponse(&'static str), } /// Borrowed canonical request fields that supersede their original request values. @@ -754,9 +757,7 @@ pub(crate) fn chat_response_to_response_resource( .as_object() .ok_or(TranslationError::ExpectedObject("Chat Completions response"))?; - let finish_reason = first_choice(obj) - .and_then(|choice| choice.get("finish_reason")) - .and_then(Value::as_str); + let finish_reason = validate_chat_response(obj)?; let status = response_status(finish_reason); let incomplete_details = incomplete_details(finish_reason); let output = build_output_items(obj, context, status); @@ -773,6 +774,126 @@ pub(crate) fn chat_response_to_response_resource( Ok(response_resource(context, parts)) } +/// Validate the minimum successful Chat Completions shape used by translation. +fn validate_chat_response(obj: &Map) -> Result<&str, TranslationError> { + let choices = obj + .get("choices") + .and_then(Value::as_array) + .ok_or(TranslationError::InvalidChatResponse("choices must be an array"))?; + let choice = choices + .first() + .and_then(Value::as_object) + .ok_or(TranslationError::InvalidChatResponse("choices must contain an object"))?; + let finish_reason = + choice + .get("finish_reason") + .and_then(Value::as_str) + .ok_or(TranslationError::InvalidChatResponse( + "first choice must contain a string finish_reason", + ))?; + if !matches!(finish_reason, "stop" | "length" | "tool_calls" | "content_filter") { + return Err(TranslationError::InvalidChatResponse( + "first choice contains an unsupported finish_reason", + )); + } + + validate_chat_message(choice, finish_reason)?; + Ok(finish_reason) +} + +/// Validate the assistant message fields that the translator consumes. +fn validate_chat_message(choice: &Map, finish_reason: &str) -> Result<(), TranslationError> { + let message = choice + .get("message") + .and_then(Value::as_object) + .ok_or(TranslationError::InvalidChatResponse( + "first choice must contain a message object", + ))?; + if message.get("role").and_then(Value::as_str) != Some("assistant") { + return Err(TranslationError::InvalidChatResponse( + "first choice message must have the assistant role", + )); + } + + let has_content = validate_chat_content(message)?; + let has_refusal = validate_chat_refusal(message)?; + let has_tool_calls = validate_chat_tool_calls(message, finish_reason)?; + if !has_content && !has_refusal && !has_tool_calls { + return Err(TranslationError::InvalidChatResponse( + "first choice message has no supported output", + )); + } + Ok(()) +} + +/// Validate optional assistant content and report whether it is present. +fn validate_chat_content(message: &Map) -> Result { + match message.get("content") { + Some(Value::Null | Value::String(_)) => Ok(true), + Some(Value::Array(parts)) if parts.iter().all(is_supported_text_part) => Ok(true), + Some(_) => Err(TranslationError::InvalidChatResponse( + "first choice message contains unsupported content", + )), + None => Ok(false), + } +} + +/// Validate optional assistant refusal content and report whether it is present. +fn validate_chat_refusal(message: &Map) -> Result { + match message.get("refusal") { + Some(Value::String(_)) => Ok(true), + Some(Value::Null) | None => Ok(false), + Some(_) => Err(TranslationError::InvalidChatResponse( + "first choice message contains an invalid refusal", + )), + } +} + +/// Return whether one provider-specific content part can be translated as text. +fn is_supported_text_part(part: &Value) -> bool { + part.get("text").is_some_and(Value::is_string) +} + +/// Validate optional function calls and require them for a tool-call terminal. +fn validate_chat_tool_calls(message: &Map, finish_reason: &str) -> Result { + let tool_calls = match message.get("tool_calls") { + None | Some(Value::Null) => &[][..], + Some(Value::Array(tool_calls)) => tool_calls.as_slice(), + Some(_) => { + return Err(TranslationError::InvalidChatResponse( + "message tool_calls must be an array", + )); + }, + }; + if tool_calls.is_empty() { + if finish_reason == "tool_calls" { + return Err(TranslationError::InvalidChatResponse( + "tool_calls finish_reason requires function tool calls", + )); + } + return Ok(false); + } + if !tool_calls.iter().all(is_supported_function_call) { + return Err(TranslationError::InvalidChatResponse( + "message contains an invalid function tool call", + )); + } + Ok(true) +} + +/// Return whether one Chat Completions tool call has the fields we emit. +fn is_supported_function_call(tool_call: &Value) -> bool { + tool_call.get("id").is_some_and(Value::is_string) + && tool_call.get("type").and_then(Value::as_str) == Some("function") + && tool_call + .get("function") + .and_then(Value::as_object) + .is_some_and(|function| { + function.get("name").is_some_and(Value::is_string) + && function.get("arguments").is_some_and(Value::is_string) + }) +} + /// Values that vary between response resource snapshots. #[derive(Debug)] struct ResponseResourceParts<'a> { @@ -871,18 +992,18 @@ fn chat_logprobs_content(choice: &Value) -> &[Value] { } /// Map a Chat Completions finish reason to a `Responses` status. -fn response_status(finish_reason: Option<&str>) -> &'static str { +fn response_status(finish_reason: &str) -> &'static str { match finish_reason { - Some("length" | "content_filter") => "incomplete", + "length" | "content_filter" => "incomplete", _ => "completed", } } /// Build `Responses` incomplete details from a Chat Completions finish reason. -fn incomplete_details(finish_reason: Option<&str>) -> Value { +fn incomplete_details(finish_reason: &str) -> Value { match finish_reason { - Some("length") => json!({"reason": "max_output_tokens"}), - Some("content_filter") => json!({"reason": "content_filter"}), + "length" => json!({"reason": "max_output_tokens"}), + "content_filter" => json!({"reason": "content_filter"}), _ => Value::Null, } } diff --git a/apis/src/openai/translation/mod.rs b/apis/src/openai/translation/mod.rs index 08fcb2698..116374b7e 100644 --- a/apis/src/openai/translation/mod.rs +++ b/apis/src/openai/translation/mod.rs @@ -1406,7 +1406,7 @@ mod tests { // ------------------------------------------------------------------------- #[test] - fn response_with_no_choices_produces_empty_output() { + fn response_with_no_choices_is_rejected() { let request = json!({"model": "m", "input": "hello"}); let context = make_response_context(&request); let response = json!({ @@ -1417,13 +1417,13 @@ mod tests { "choices": [] }); - let mapped = super::chat_completions::chat_response_to_response_resource(&response, &context).unwrap(); + let error = super::chat_completions::chat_response_to_response_resource(&response, &context).unwrap_err(); - assert_eq!(mapped["output"], json!([])); + assert!(error.to_string().contains("choices must contain an object")); } #[test] - fn response_without_choices_key_produces_empty_output() { + fn response_without_choices_key_is_rejected() { let request = json!({"model": "m", "input": "hello"}); let context = make_response_context(&request); let response = json!({ @@ -1433,9 +1433,64 @@ mod tests { "model": "m" }); - let mapped = super::chat_completions::chat_response_to_response_resource(&response, &context).unwrap(); + let error = super::chat_completions::chat_response_to_response_resource(&response, &context).unwrap_err(); - assert_eq!(mapped["output"], json!([])); + assert!(error.to_string().contains("choices must be an array")); + } + + #[test] + fn malformed_first_choices_are_rejected() { + let request = json!({"model": "m", "input": "hello"}); + let context = make_response_context(&request); + let malformed = [ + ( + "non-object choice", + json!({"choices": [null]}), + "choices must contain an object", + ), + ( + "missing finish reason", + json!({"choices": [{"message": {"role": "assistant", "content": "ok"}}]}), + "first choice must contain a string finish_reason", + ), + ( + "unsupported finish reason", + json!({ + "choices": [{ + "finish_reason": "unknown", + "message": {"role": "assistant", "content": "ok"} + }] + }), + "first choice contains an unsupported finish_reason", + ), + ( + "missing message", + json!({"choices": [{"finish_reason": "stop"}]}), + "first choice must contain a message object", + ), + ( + "invalid role", + json!({ + "choices": [{ + "finish_reason": "stop", + "message": {"role": "user", "content": "ok"} + }] + }), + "first choice message must have the assistant role", + ), + ( + "missing output", + json!({"choices": [{"finish_reason": "stop", "message": {"role": "assistant"}}]}), + "first choice message has no supported output", + ), + ]; + + for (case, response, expected) in malformed { + let error = + super::chat_completions::chat_response_to_response_resource(&response, &context).expect_err(case); + + assert!(error.to_string().contains(expected), "{case}: {error}"); + } } // ------------------------------------------------------------------------- @@ -1789,7 +1844,7 @@ mod tests { } #[test] - fn tool_call_missing_function_fields_uses_defaults() { + fn tool_call_missing_function_fields_is_rejected() { let request = json!({"model": "m", "input": "hello"}); let context = make_response_context(&request); let response = json!({ @@ -1808,10 +1863,13 @@ mod tests { }] }); - let mapped = super::chat_completions::chat_response_to_response_resource(&response, &context).unwrap(); + let error = super::chat_completions::chat_response_to_response_resource(&response, &context).unwrap_err(); - assert_eq!(mapped["output"][0]["name"], ""); - assert_eq!(mapped["output"][0]["arguments"], "{}"); + assert!( + error + .to_string() + .contains("message contains an invalid function tool call") + ); } // ------------------------------------------------------------------------- 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..8c88d4d34 100644 --- a/tests/integration/tests/suite/examples/responses_to_chat_completions.rs +++ b/tests/integration/tests/suite/examples/responses_to_chat_completions.rs @@ -105,6 +105,28 @@ fn responses_to_chat_completions_normalizes_finite_provider_error() { assert_eq!(response["error"]["message"], "slow down"); } +#[test] +fn responses_to_chat_completions_rejects_malformed_finite_success() { + let backend = Backend::fixed("{}") + .header("content-type", "application/json") + .start_with_shutdown(); + let proxy_port = free_port(); + let (config, _db) = load_test_config( + "malformed_finite_success", + proxy_port, + &HashMap::from([("127.0.0.1:3001", backend.port())]), + ); + let proxy = start_proxy(&config); + let request = r#"{"model":"gpt-4.1-mini","input":"Hello","stream":false,"store":false}"#; + + let raw = http_send(proxy.addr(), &json_post("/v1/responses", request)); + let response: serde_json::Value = serde_json::from_str(&parse_body(&raw)).expect("error response should be JSON"); + + assert_eq!(parse_status(&raw), 500); + assert_eq!(response["title"], "Internal Server Error"); + assert_eq!(response["status"], 500); +} + #[test] fn responses_to_chat_completions_leaves_sse_for_stream_converter() { let sse = "data: {\"id\":\"chatcmpl_1\",\"choices\":[{\"delta\":{\"content\":\"Hi\"}}]}\n\ndata: [DONE]\n\n"; From 104de60c3336079e3b0696d31f103bd39cc823d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Wed, 2 Sep 2026 18:16:28 +0200 Subject: [PATCH 2/2] fix(translation): reject null assistant content on completed terminals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found that validate_chat_content treated `content: null` as present. A completed terminal (finish_reason `stop`) with null content, no refusal, and no tool calls therefore passed validation while build_output_items emitted nothing, letting the translator synthesize a counterfeit `completed` Responses object with an empty output array. Treat null content as absent and require at least one translatable output only for completed terminals. Incomplete terminals (length, content_filter) truthfully carry empty output and remain valid. Add a regression test for the rejected completed case and one for the preserved incomplete terminals. Signed-off-by: Sébastien Han --- .../openai/translation/chat_completions.rs | 15 +++++-- apis/src/openai/translation/mod.rs | 41 +++++++++++++++++-- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/apis/src/openai/translation/chat_completions.rs b/apis/src/openai/translation/chat_completions.rs index 9fc288247..321c3e19b 100644 --- a/apis/src/openai/translation/chat_completions.rs +++ b/apis/src/openai/translation/chat_completions.rs @@ -818,7 +818,12 @@ fn validate_chat_message(choice: &Map, finish_reason: &str) -> Re let has_content = validate_chat_content(message)?; let has_refusal = validate_chat_refusal(message)?; let has_tool_calls = validate_chat_tool_calls(message, finish_reason)?; - if !has_content && !has_refusal && !has_tool_calls { + // A completed terminal must carry at least one translatable output; without + // one the translator would synthesize a counterfeit `completed` response with + // an empty output array. Incomplete terminals (length, content_filter) + // truthfully carry empty output, so they are exempt. + let has_output = has_content || has_refusal || has_tool_calls; + if response_status(finish_reason) == "completed" && !has_output { return Err(TranslationError::InvalidChatResponse( "first choice message has no supported output", )); @@ -827,14 +832,18 @@ fn validate_chat_message(choice: &Map, finish_reason: &str) -> Re } /// Validate optional assistant content and report whether it is present. +/// +/// `null` is treated as absent: the emitter produces no output for it, so +/// counting it as content would let a completed terminal translate into a +/// counterfeit success with an empty output array. fn validate_chat_content(message: &Map) -> Result { match message.get("content") { - Some(Value::Null | Value::String(_)) => Ok(true), + None | Some(Value::Null) => Ok(false), + Some(Value::String(_)) => Ok(true), Some(Value::Array(parts)) if parts.iter().all(is_supported_text_part) => Ok(true), Some(_) => Err(TranslationError::InvalidChatResponse( "first choice message contains unsupported content", )), - None => Ok(false), } } diff --git a/apis/src/openai/translation/mod.rs b/apis/src/openai/translation/mod.rs index 116374b7e..f0ae81fd3 100644 --- a/apis/src/openai/translation/mod.rs +++ b/apis/src/openai/translation/mod.rs @@ -1509,7 +1509,7 @@ mod tests { } #[test] - fn response_with_null_content_produces_no_message_output() { + fn response_with_null_content_on_completed_is_rejected() { let request = json!({"model": "m", "input": "hello"}); let context = make_response_context(&request); let response = json!({ @@ -1520,9 +1520,44 @@ mod tests { "choices": [{"finish_reason": "stop", "index": 0, "message": {"role": "assistant", "content": null}}] }); - let mapped = super::chat_completions::chat_response_to_response_resource(&response, &context).unwrap(); + let error = super::chat_completions::chat_response_to_response_resource(&response, &context).unwrap_err(); - assert_eq!(mapped["output"], json!([])); + assert!( + error + .to_string() + .contains("first choice message has no supported output"), + "{error}" + ); + } + + #[test] + fn response_with_null_content_on_incomplete_terminal_is_preserved() { + let request = json!({"model": "m", "input": "hello"}); + let context = make_response_context(&request); + for (finish_reason, reason) in [("length", "max_output_tokens"), ("content_filter", "content_filter")] { + let response = json!({ + "id": "chatcmpl-1", + "object": "chat.completion", + "created": 0, + "model": "m", + "choices": [{ + "finish_reason": finish_reason, + "index": 0, + "message": {"role": "assistant", "content": null} + }] + }); + + let mapped = + super::chat_completions::chat_response_to_response_resource(&response, &context).expect(finish_reason); + + assert_eq!(mapped["status"], "incomplete", "{finish_reason}"); + assert_eq!( + mapped["incomplete_details"], + json!({"reason": reason}), + "{finish_reason}" + ); + assert_eq!(mapped["output"], json!([]), "{finish_reason}"); + } } // -------------------------------------------------------------------------