From 7b71ffafec2727009d86466a2f8568a83ae956bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Wed, 2 Sep 2026 12:34:45 +0200 Subject: [PATCH] fix(anthropic_response): fail translation on malformed tool arguments (#550) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When translating an OpenAI Chat Completions response into Anthropic Messages format, tool call arguments that were not a valid JSON-encoded object were silently replaced with an empty object (`{}`). That could cause an Anthropic client to execute a tool with missing or default parameters. Translation now fails instead of fabricating an executable `tool_use` block. Because upstream status headers are already committed by the time the body is parsed, the original upstream response is preserved unchanged rather than emitting a misleading error, which avoids accidental tool execution without inventing a semantically incorrect response. Adds unit, functional proxy, and controlled synthetic inference fixture coverage. Signed-off-by: Sébastien Han --- apis/src/anthropic/to_openai/response.rs | 46 +++--- .../integration/fixtures/inference/README.md | 3 +- .../fixtures/inference/coverage.yaml | 9 + .../messages/malformed-tool-arguments.json | 155 ++++++++++++++++++ .../messages/malformed-tool-arguments.yaml | 32 ++++ .../suite/examples/anthropic_messages.rs | 51 ++++++ .../tests/suite/inference_fixtures.rs | 36 ++++ tests/utils/src/inference_fixture/coverage.rs | 52 ++++-- 8 files changed, 344 insertions(+), 40 deletions(-) create mode 100644 tests/integration/fixtures/inference/recordings/synthetic/messages/malformed-tool-arguments.json create mode 100644 tests/integration/fixtures/inference/scenarios/messages/malformed-tool-arguments.yaml diff --git a/apis/src/anthropic/to_openai/response.rs b/apis/src/anthropic/to_openai/response.rs index 2bd129c761..e13d351c05 100644 --- a/apis/src/anthropic/to_openai/response.rs +++ b/apis/src/anthropic/to_openai/response.rs @@ -78,7 +78,7 @@ pub(crate) fn transform_response(body: &[u8], request_model: &str) -> Result &'static str { // ----------------------------------------------------------------------------- /// Extract content blocks from the first choice. -fn build_content_blocks<'a>(obj: &'a Map) -> Vec> { +fn build_content_blocks<'a>(obj: &'a Map) -> Result>, String> { let mut blocks = Vec::new(); let choice = obj.get("choices").and_then(Value::as_array).and_then(|c| c.first()); let Some(choice) = choice else { - return blocks; + return Ok(blocks); }; let message = choice.get("message"); extract_text_block(message, &mut blocks); - extract_tool_call_blocks(message, &mut blocks); + extract_tool_call_blocks(message, &mut blocks)?; - blocks + Ok(blocks) } /// Extract a text content block from the message if present. @@ -176,9 +176,9 @@ fn extract_text_block<'a>(message: Option<&'a Value>, blocks: &mut Vec(message: Option<&'a Value>, blocks: &mut Vec>) { +fn extract_tool_call_blocks<'a>(message: Option<&'a Value>, blocks: &mut Vec>) -> Result<(), String> { let Some(Value::Array(tool_calls)) = message.and_then(|m| m.get("tool_calls")) else { - return; + return Ok(()); }; for tc in tool_calls { @@ -192,11 +192,14 @@ fn extract_tool_call_blocks<'a>(message: Option<&'a Value>, blocks: &mut Vec>(args_str).unwrap_or_default(); + .ok_or_else(|| "tool call arguments must be a JSON-encoded object string".to_owned())?; + let input = serde_json::from_str::>(args_str) + .map_err(|error| format!("invalid tool call arguments: {error}"))?; blocks.push(ContentBlock::tool_use(id, input, name)); } + + Ok(()) } // ----------------------------------------------------------------------------- @@ -589,21 +592,18 @@ mod tests { } #[test] - fn invalid_tool_call_arguments_fallback_to_empty_object() { + fn invalid_tool_call_arguments_fail_transformation() { let body = br#"{"id":"chatcmpl-1","model":"gpt-4","choices":[{"message":{"role":"assistant","tool_calls":[{"id":"call_1","type":"function","function":{"name":"get_weather","arguments":"not{json"}}]},"finish_reason":"tool_calls"}],"usage":{"prompt_tokens":10,"completion_tokens":5}}"#; - let tr = transform_response(body, "gpt-4").unwrap(); - let parsed: Value = serde_json::from_slice(&tr.body).unwrap(); + let error = transform_response(body, "gpt-4").err().unwrap(); - assert_eq!(parsed["content"][0]["type"], "tool_use"); - assert_eq!( - parsed["content"][0]["input"], - json!({}), - "invalid JSON arguments should fallback to empty object" + assert!( + error.contains("invalid tool call arguments"), + "malformed arguments should fail response transformation: {error}" ); } #[test] - fn non_object_tool_call_arguments_fallback_to_empty_object() { + fn non_object_tool_call_arguments_fail_transformation() { for arguments in ["[]", "null", "\"text\""] { let body = json!({ "id": "chatcmpl-1", @@ -625,13 +625,11 @@ mod tests { "usage": {"prompt_tokens": 10, "completion_tokens": 5} }); let encoded = serde_json::to_vec(&body).unwrap(); - let transformed = transform_response(&encoded, "gpt-4").unwrap(); - let parsed: Value = serde_json::from_slice(&transformed.body).unwrap(); + let error = transform_response(&encoded, "gpt-4").err().unwrap(); - assert_eq!( - parsed["content"][0]["input"], - json!({}), - "{arguments} should not produce a non-object tool input" + assert!( + error.contains("invalid tool call arguments"), + "non-object arguments {arguments} should fail response transformation: {error}" ); } } diff --git a/tests/integration/fixtures/inference/README.md b/tests/integration/fixtures/inference/README.md index 13831e1152..568d9253ca 100644 --- a/tests/integration/fixtures/inference/README.md +++ b/tests/integration/fixtures/inference/README.md @@ -19,13 +19,14 @@ than editing the table. -The manifest declares **13 features** across **5 scopes**, linked to **11 scenarios**. +The manifest declares **14 features** across **5 scopes**, linked to **12 scenarios**. | Scope | Feature | Status | Scenarios | Provider coverage | | --- | --- | --- | --- | --- | | `messages_to_chat_completions` | `messages.request.minimal` | `live_covered` | `messages/basic-nonstream`
`messages/basic-stream` | `openai`: `covered`
`vllm`: `live_covered` | | `messages_to_chat_completions` | `messages.response.text` | `live_covered` | `messages/basic-nonstream`
`messages/basic-stream` | `openai`: `covered`
`vllm`: `live_covered` | | `messages_to_chat_completions` | `messages.error.upstream` | `synthetic_only` | `messages/upstream-error` | `synthetic`: `synthetic_only` | +| `messages_to_chat_completions` | `messages.response.malformed_tool_arguments` | `synthetic_only` | `messages/malformed-tool-arguments` | `synthetic`: `synthetic_only` | | `messages_native_passthrough` | `messages.native.request` | `live_covered` | `messages/native-basic-nonstream`
`messages/native-basic-stream`
`messages/native-tool-use` | `anthropic`: `live_covered` | | `messages_native_passthrough` | `messages.native.response.text` | `live_covered` | `messages/native-basic-nonstream`
`messages/native-basic-stream` | `anthropic`: `live_covered` | | `messages_native_passthrough` | `messages.native.tool_use` | `live_covered` | `messages/native-tool-use` | `anthropic`: `live_covered` | diff --git a/tests/integration/fixtures/inference/coverage.yaml b/tests/integration/fixtures/inference/coverage.yaml index 99b53c819d..8b696ff2d6 100644 --- a/tests/integration/fixtures/inference/coverage.yaml +++ b/tests/integration/fixtures/inference/coverage.yaml @@ -39,6 +39,15 @@ features: providers: synthetic: status: synthetic_only + - id: messages.response.malformed_tool_arguments + scopes: + - messages_to_chat_completions + status: synthetic_only + scenarios: + - messages/malformed-tool-arguments + providers: + synthetic: + status: synthetic_only - id: messages.native.request scopes: - messages_native_passthrough diff --git a/tests/integration/fixtures/inference/recordings/synthetic/messages/malformed-tool-arguments.json b/tests/integration/fixtures/inference/recordings/synthetic/messages/malformed-tool-arguments.json new file mode 100644 index 0000000000..39e170dda3 --- /dev/null +++ b/tests/integration/fixtures/inference/recordings/synthetic/messages/malformed-tool-arguments.json @@ -0,0 +1,155 @@ +{ + "version": 1, + "scenario_id": "messages/malformed-tool-arguments", + "protocol": "anthropic_messages", + "provenance": { + "kind": "synthetic", + "provider": "synthetic", + "model": "synthetic-malformed-tool-model", + "source_id": "controlled-malformed-tool-arguments" + }, + "normalization": { + "version": 1, + "linked_ids": { + "call_1": "call_recorded_0001", + "chatcmpl-malformed-tool": "chatcmpl-recorded-0002" + } + }, + "turns": [ + { + "name": "initial", + "client": { + "request": { + "method": "POST", + "path": "/v1/messages", + "headers": { + "content-type": [ + "application/json" + ] + }, + "body": { + "kind": "json", + "value": { + "max_tokens": 64, + "messages": [ + { + "content": "Use the weather tool.", + "role": "user" + } + ], + "model": "synthetic-malformed-tool-model", + "stream": false + } + } + }, + "response": { + "status": 200, + "headers": { + "content-type": [ + "application/json" + ] + }, + "body": { + "kind": "json", + "value": { + "choices": [ + { + "finish_reason": "tool_calls", + "index": 0, + "message": { + "content": null, + "role": "assistant", + "tool_calls": [ + { + "function": { + "arguments": "not{json", + "name": "get_weather" + }, + "id": "call_recorded_0001", + "type": "function" + } + ] + } + } + ], + "id": "chatcmpl-recorded-0002", + "model": "synthetic-malformed-tool-model", + "object": "chat.completion", + "usage": { + "completion_tokens": 5, + "prompt_tokens": 10, + "total_tokens": 15 + } + } + } + } + }, + "upstream": { + "request": { + "method": "POST", + "path": "/v1/chat/completions", + "headers": { + "content-type": [ + "application/json" + ] + }, + "body": { + "kind": "json", + "value": { + "max_completion_tokens": 64, + "messages": [ + { + "content": "Use the weather tool.", + "role": "user" + } + ], + "model": "synthetic-malformed-tool-model", + "stream": false + } + } + }, + "response": { + "status": 200, + "headers": { + "content-type": [ + "application/json" + ] + }, + "body": { + "kind": "json", + "value": { + "choices": [ + { + "finish_reason": "tool_calls", + "index": 0, + "message": { + "content": null, + "role": "assistant", + "tool_calls": [ + { + "function": { + "arguments": "not{json", + "name": "get_weather" + }, + "id": "call_recorded_0001", + "type": "function" + } + ] + } + } + ], + "id": "chatcmpl-recorded-0002", + "model": "synthetic-malformed-tool-model", + "object": "chat.completion", + "usage": { + "completion_tokens": 5, + "prompt_tokens": 10, + "total_tokens": 15 + } + } + } + } + } + } + ] +} diff --git a/tests/integration/fixtures/inference/scenarios/messages/malformed-tool-arguments.yaml b/tests/integration/fixtures/inference/scenarios/messages/malformed-tool-arguments.yaml new file mode 100644 index 0000000000..83e2a28eb4 --- /dev/null +++ b/tests/integration/fixtures/inference/scenarios/messages/malformed-tool-arguments.yaml @@ -0,0 +1,32 @@ +version: 1 +id: messages/malformed-tool-arguments +description: Malformed Chat Completions tool arguments fail response translation without emitting Anthropic tool_use. +protocol: anthropic_messages +example_config: anthropic/messages-to-openai.yaml +upstream_authority: 127.0.0.1:8000 +features: + - messages.response.malformed_tool_arguments +turns: + - name: initial + request: + method: POST + path: /v1/messages + headers: + content-type: + - application/json + body: + kind: json + value: + model: ${MODEL} + max_tokens: 64 + stream: false + messages: + - role: user + content: Use the weather tool. + expect: + client_status: 200 + client_body_kind: json + upstream_path: /v1/chat/completions + upstream_body_kind: json + client_sse_events: [] + upstream_sse_events: [] diff --git a/tests/integration/tests/suite/examples/anthropic_messages.rs b/tests/integration/tests/suite/examples/anthropic_messages.rs index be35fa63ea..61a7e02ced 100644 --- a/tests/integration/tests/suite/examples/anthropic_messages.rs +++ b/tests/integration/tests/suite/examples/anthropic_messages.rs @@ -166,6 +166,57 @@ fn anthropic_to_openai_transforms_response_body() { ); } +#[test] +fn anthropic_to_openai_preserves_response_with_malformed_tool_arguments() { + let response = serde_json::json!({ + "id": "chatcmpl-malformed-tool", + "object": "chat.completion", + "model": "synthetic-malformed-tool-model", + "choices": [{ + "index": 0, + "message": { + "role": "assistant", + "content": null, + "tool_calls": [{ + "id": "call_1", + "type": "function", + "function": { + "name": "get_weather", + "arguments": "not{json" + } + }] + }, + "finish_reason": "tool_calls" + }], + "usage": {"prompt_tokens": 10, "completion_tokens": 5, "total_tokens": 15} + }); + let backend = Backend::fixed(&response.to_string()) + .header("content-type", "application/json") + .start_with_shutdown(); + let proxy_port = free_port(); + let config = load_example_config( + "anthropic/messages-to-openai.yaml", + proxy_port, + HashMap::from([("127.0.0.1:8000", backend.port())]), + ); + let proxy = start_proxy(&config); + let request = serde_json::json!({ + "model": "synthetic-malformed-tool-model", + "max_tokens": 64, + "messages": [{"role": "user", "content": "Use the weather tool."}] + }); + + let raw = http_send(proxy.addr(), &json_post("/v1/messages", &request.to_string())); + let client_body: serde_json::Value = + serde_json::from_str(&parse_body(&raw)).expect("preserved response should remain JSON"); + + assert_eq!(parse_status(&raw), 200, "upstream status should be preserved"); + assert_eq!( + client_body, response, + "failed transformation must preserve the upstream response instead of emitting tool_use" + ); +} + fn run_anthropic_to_openai_error(status: u16, response_body: &str, stream: bool) -> (u16, serde_json::Value) { let backend = Backend::status(status, response_body) .header("content-type", "application/json") diff --git a/tests/integration/tests/suite/inference_fixtures.rs b/tests/integration/tests/suite/inference_fixtures.rs index 942578cb72..a066df51b2 100644 --- a/tests/integration/tests/suite/inference_fixtures.rs +++ b/tests/integration/tests/suite/inference_fixtures.rs @@ -14,6 +14,8 @@ const STREAM_SCENARIO: &str = "messages/basic-stream"; const STREAM_PROVIDER: &str = "openai"; const ERROR_SCENARIO: &str = "messages/upstream-error"; const ERROR_PROVIDER: &str = "synthetic"; +const MALFORMED_TOOL_ARGUMENTS_SCENARIO: &str = "messages/malformed-tool-arguments"; +const MALFORMED_TOOL_ARGUMENTS_PROVIDER: &str = "synthetic"; const NATIVE_ANTHROPIC_PROVIDER: &str = "anthropic"; const NATIVE_ANTHROPIC_SCENARIOS: [&str; 3] = [ "messages/native-basic-nonstream", @@ -50,6 +52,7 @@ async fn all_inference_fixtures_replay() { let mut native_responses_recordings = BTreeSet::new(); let mut saw_stream_representative = false; let mut saw_error_representative = false; + let mut saw_malformed_tool_arguments = false; let mut saw_agentic_parallel_tool_calls = false; for recording in recordings { @@ -88,6 +91,10 @@ async fn all_inference_fixtures_replay() { assert_synthetic_rate_limit(&report.actual, scenario_id, provider); saw_error_representative = true; } + if scenario_id == MALFORMED_TOOL_ARGUMENTS_SCENARIO && provider == MALFORMED_TOOL_ARGUMENTS_PROVIDER { + assert_malformed_tool_arguments_preserved(&report.actual, scenario_id, provider); + saw_malformed_tool_arguments = true; + } if provider == NATIVE_ANTHROPIC_PROVIDER && NATIVE_ANTHROPIC_SCENARIOS.contains(&scenario_id) { native_anthropic_scenarios.insert(scenario_id.to_owned()); if scenario_id == NATIVE_TOOL_USE_SCENARIO { @@ -129,6 +136,10 @@ async fn all_inference_fixtures_replay() { saw_error_representative, "missing representative recording for scenario `{ERROR_SCENARIO}` and provider `{ERROR_PROVIDER}`" ); + assert!( + saw_malformed_tool_arguments, + "missing representative recording for scenario `{MALFORMED_TOOL_ARGUMENTS_SCENARIO}` and provider `{MALFORMED_TOOL_ARGUMENTS_PROVIDER}`" + ); assert!( saw_agentic_parallel_tool_calls, "missing representative recording for scenario `{AGENTIC_PARALLEL_TOOL_CALLS_SCENARIO}` and provider `{AGENTIC_PARALLEL_TOOL_CALLS_PROVIDER}`" @@ -225,6 +236,31 @@ fn assert_synthetic_rate_limit(actual: &WireFixture, scenario_id: &str, provider ); } +fn assert_malformed_tool_arguments_preserved(actual: &WireFixture, scenario_id: &str, provider: &str) { + let turn = actual.turns.first().unwrap_or_else(|| { + panic!("scenario `{scenario_id}` and provider `{provider}` replayed without a turn"); + }); + assert_eq!( + turn.client.response.status, 200, + "upstream status changed for scenario `{scenario_id}` and provider `{provider}`" + ); + assert_eq!( + turn.client.response.body, turn.upstream.response.body, + "failed translation must preserve the upstream response for scenario `{scenario_id}` and provider `{provider}`" + ); + let RecordedBody::Json { value } = &turn.client.response.body else { + panic!("scenario `{scenario_id}` and provider `{provider}` must preserve a JSON response"); + }; + assert_eq!( + value["choices"][0]["message"]["tool_calls"][0]["function"]["arguments"], "not{json", + "malformed arguments changed for scenario `{scenario_id}` and provider `{provider}`" + ); + assert!( + value.get("content").is_none(), + "scenario `{scenario_id}` and provider `{provider}` must not emit an Anthropic tool_use block" + ); +} + fn assert_native_tool_use(actual: &WireFixture, scenario_id: &str, provider: &str) { let turn = actual.turns.first().unwrap_or_else(|| { panic!("scenario `{scenario_id}` and provider `{provider}` replayed without a turn"); diff --git a/tests/utils/src/inference_fixture/coverage.rs b/tests/utils/src/inference_fixture/coverage.rs index 9d734128e6..9c2f7dd7b3 100644 --- a/tests/utils/src/inference_fixture/coverage.rs +++ b/tests/utils/src/inference_fixture/coverage.rs @@ -1227,6 +1227,7 @@ mod tests { vec!["messages_to_chat_completions"], vec!["messages_to_chat_completions"], vec!["messages_to_chat_completions"], + vec!["messages_to_chat_completions"], vec!["messages_native_passthrough"], vec!["messages_native_passthrough"], vec!["messages_native_passthrough"], @@ -1249,6 +1250,7 @@ mod tests { CoverageStatus::LiveCovered, CoverageStatus::LiveCovered, CoverageStatus::SyntheticOnly, + CoverageStatus::SyntheticOnly, CoverageStatus::LiveCovered, CoverageStatus::LiveCovered, CoverageStatus::LiveCovered, @@ -1261,14 +1263,15 @@ mod tests { CoverageStatus::SyntheticOnly, ] ); - assert_eq!(report.features_total, 13); - assert_eq!(report.scenarios_total, 11); - assert_eq!(report.recordings_total, 16); + assert_eq!(report.features_total, 14); + assert_eq!(report.scenarios_total, 12); + assert_eq!(report.recordings_total, 17); assert_eq!( scenarios.keys().collect::>(), vec![ "messages/basic-nonstream", "messages/basic-stream", + "messages/malformed-tool-arguments", "messages/native-basic-nonstream", "messages/native-basic-stream", "messages/native-tool-use", @@ -1280,7 +1283,7 @@ mod tests { "responses/native-tool-call", ] ); - assert_eq!(manifest.features.len(), 13); + assert_eq!(manifest.features.len(), 14); assert_eq!(manifest.version, 1); assert_eq!( manifest @@ -1307,6 +1310,10 @@ mod tests { &"messages.error.upstream".to_owned(), &vec!["messages/upstream-error".to_owned()] ), + ( + &"messages.response.malformed_tool_arguments".to_owned(), + &vec!["messages/malformed-tool-arguments".to_owned()] + ), ( &"messages.native.request".to_owned(), &vec![ @@ -1385,15 +1392,17 @@ mod tests { ("vllm", CoverageStatus::LiveCovered), ] ); - assert_eq!( - manifest.features[2] - .providers - .iter() - .map(|(provider, coverage)| (provider.as_str(), coverage.status.clone())) - .collect::>(), - vec![("synthetic", CoverageStatus::SyntheticOnly)] - ); - for feature in &manifest.features[3..6] { + for feature in &manifest.features[2..4] { + assert_eq!( + feature + .providers + .iter() + .map(|(provider, coverage)| (provider.as_str(), coverage.status.clone())) + .collect::>(), + vec![("synthetic", CoverageStatus::SyntheticOnly)] + ); + } + for feature in &manifest.features[4..7] { assert_eq!( feature .providers @@ -1403,7 +1412,7 @@ mod tests { vec![("anthropic", CoverageStatus::LiveCovered)] ); } - for feature in &manifest.features[6..9] { + for feature in &manifest.features[7..10] { assert_eq!( feature .providers @@ -1416,7 +1425,7 @@ mod tests { ] ); } - for feature in &manifest.features[9..] { + for feature in &manifest.features[10..] { assert_eq!( feature .providers @@ -1473,6 +1482,19 @@ mod tests { 429, &[], ); + let malformed_tool_arguments = + InferenceScenario::load(&root.join("scenarios/messages/malformed-tool-arguments.yaml")).unwrap(); + assert_scenario( + &malformed_tool_arguments, + "messages/malformed-tool-arguments", + "Malformed Chat Completions tool arguments fail response translation without emitting Anthropic tool_use.", + &["messages.response.malformed_tool_arguments"], + "Use the weather tool.", + false, + BodyKind::Json, + 200, + &[], + ); let native_nonstream = InferenceScenario::load(&root.join("scenarios/messages/native-basic-nonstream.yaml")).unwrap();