From ff1d669893fea209785d26080decaab0de22d104 Mon Sep 17 00:00:00 2001 From: Jasper Date: Wed, 5 Aug 2026 08:22:55 -0600 Subject: [PATCH] Sanitize Unicode tags in MCP resources (#10746) --- .../src/formats/databricks.rs | 25 ++++++++---- .../src/formats/google.rs | 18 +++++++++ crates/goose-provider-types/src/mcp_utils.rs | 11 ++++-- crates/goose/src/providers/formats/bedrock.rs | 38 ++++++++++++++++++- 4 files changed, 79 insertions(+), 13 deletions(-) diff --git a/crates/goose-provider-types/src/formats/databricks.rs b/crates/goose-provider-types/src/formats/databricks.rs index e650dfb02..069cd42c7 100644 --- a/crates/goose-provider-types/src/formats/databricks.rs +++ b/crates/goose-provider-types/src/formats/databricks.rs @@ -10,10 +10,9 @@ use crate::formats::openai::{ openai_reasoning_effort_for_thinking, sanitize_function_name, validate_tool_schemas, }; use crate::images::{convert_image, detect_image_path, load_image_file, ImageFormat}; +use crate::mcp_utils::extract_text_from_resource; use anyhow::{anyhow, Error}; -use rmcp::model::{ - object, CallToolRequestParams, ContentBlock, ErrorCode, ErrorData, ResourceContents, Role, Tool, -}; +use rmcp::model::{object, CallToolRequestParams, ContentBlock, ErrorCode, ErrorData, Role, Tool}; use serde::Serialize; use serde_json::{json, Value}; use std::borrow::Cow; @@ -70,10 +69,7 @@ fn format_tool_response( }); } ContentBlock::Resource(resource) => { - let text = match &resource.resource { - ResourceContents::TextResourceContents { text, .. } => text.clone(), - _ => String::new(), - }; + let text = extract_text_from_resource(&resource.resource); tool_content.push(ContentBlock::text(text)); } _ => tool_content.push(content), @@ -720,6 +716,21 @@ mod tests { Ok(()) } + #[test] + fn test_format_messages_sanitizes_resource_tool_response() { + let message = Message::user().with_tool_response( + "tool1", + Ok(CallToolResult::success(vec![ContentBlock::embedded_text( + "file:///result.txt", + "visible\u{E0041}text", + )])), + ); + + let spec = format_messages(&[message], &ImageFormat::OpenAi); + + assert_eq!(spec[0].content, "visibletext"); + } + #[test] fn test_format_tools() -> anyhow::Result<()> { let tool = Tool::new( diff --git a/crates/goose-provider-types/src/formats/google.rs b/crates/goose-provider-types/src/formats/google.rs index 806e42e7f..144c30840 100644 --- a/crates/goose-provider-types/src/formats/google.rs +++ b/crates/goose-provider-types/src/formats/google.rs @@ -917,6 +917,24 @@ mod tests { ); } + #[test] + fn test_message_to_google_spec_sanitizes_resource_tool_response() { + let messages = vec![set_up_tool_response_message( + "response_id", + vec![ContentBlock::embedded_text( + "file:///result.txt", + "visible\u{E0041}text", + )], + )]; + + let payload = format_messages(&messages, false); + + assert_eq!( + payload[0]["parts"][0]["functionResponse"]["response"]["content"]["text"], + "visibletext" + ); + } + #[test] fn test_function_response_matches_function_call() { let messages = vec![ diff --git a/crates/goose-provider-types/src/mcp_utils.rs b/crates/goose-provider-types/src/mcp_utils.rs index 199a5bc46..771db8a69 100644 --- a/crates/goose-provider-types/src/mcp_utils.rs +++ b/crates/goose-provider-types/src/mcp_utils.rs @@ -1,16 +1,17 @@ +use crate::utils::sanitize_unicode_tags; use base64::Engine; use rmcp::model::ResourceContents; pub fn extract_text_from_resource(resource: &ResourceContents) -> String { match resource { - ResourceContents::TextResourceContents { text, .. } => text.clone(), + ResourceContents::TextResourceContents { text, .. } => sanitize_unicode_tags(text), ResourceContents::BlobResourceContents { blob, mime_type, .. } => match base64::engine::general_purpose::STANDARD.decode(blob) { Ok(bytes) => { let byte_len = bytes.len(); match String::from_utf8(bytes) { - Ok(text) => text, + Ok(text) => sanitize_unicode_tags(&text), Err(_) => { let mime = mime_type .as_ref() @@ -20,7 +21,7 @@ pub fn extract_text_from_resource(resource: &ResourceContents) -> String { } } } - Err(_) => blob.clone(), + Err(_) => sanitize_unicode_tags(blob), }, _ => String::new(), } @@ -33,6 +34,7 @@ mod tests { #[test_case("Hello, World!", "Hello, World!" ; "simple text")] #[test_case("Hello from GitHub!", "Hello from GitHub!" ; "github content")] + #[test_case("visible\u{E0041}\u{E0042}text", "visibletext" ; "unicode tags")] #[test_case("", "" ; "empty text")] fn test_extract_text_from_text_resource(input: &str, expected: &str) { let resource = ResourceContents::TextResourceContents { @@ -46,6 +48,7 @@ mod tests { #[test_case("Hello from GitHub!", "Hello from GitHub!" ; "utf8 markdown")] #[test_case("Simple text", "Simple text" ; "utf8 plain")] + #[test_case("visible\u{E0041}\u{E0042}text", "visibletext" ; "unicode tags")] fn test_extract_text_from_blob_utf8(input: &str, expected: &str) { let blob = base64::engine::general_purpose::STANDARD.encode(input.as_bytes()); let resource = ResourceContents::BlobResourceContents { @@ -98,7 +101,7 @@ mod tests { let resource = ResourceContents::BlobResourceContents { uri: "file:///test.txt".to_string(), mime_type: Some("text/plain".to_string()), - blob: "not valid base64!!!".to_string(), + blob: "not\u{E0041} valid base64!!!".to_string(), meta: None, }; assert_eq!(extract_text_from_resource(&resource), "not valid base64!!!"); diff --git a/crates/goose/src/providers/formats/bedrock.rs b/crates/goose/src/providers/formats/bedrock.rs index d076f8e58..a505dd77a 100644 --- a/crates/goose/src/providers/formats/bedrock.rs +++ b/crates/goose/src/providers/formats/bedrock.rs @@ -20,6 +20,7 @@ use crate::providers::formats::anthropic::{ adaptive_output_effort, model_supports_temperature, thinking_budget_tokens, thinking_type_for_provider, ThinkingType, ANTHROPIC_PROVIDER_NAME, MIN_ANSWER_TOKENS, }; +use crate::utils::sanitize_unicode_tags; use goose_providers::conversation::token_usage::Usage; use goose_providers::model::ModelConfig; use once_cell::sync::Lazy; @@ -303,7 +304,9 @@ pub fn to_bedrock_tool_result_content_block( ResourceContents::TextResourceContents { text, .. } => { match to_bedrock_document(tool_use_id, &resource.resource)? { Some(doc) => bedrock::ToolResultContentBlock::Document(doc), - None => bedrock::ToolResultContentBlock::Text(text.to_string()), + None => { + bedrock::ToolResultContentBlock::Text(sanitize_unicode_tags(text.as_str())) + } } } ResourceContents::BlobResourceContents { .. } => { @@ -419,7 +422,9 @@ fn to_bedrock_document( content: &ResourceContents, ) -> Result> { let (uri, text) = match content { - ResourceContents::TextResourceContents { uri, text, .. } => (uri, text), + ResourceContents::TextResourceContents { uri, text, .. } => { + (uri, sanitize_unicode_tags(text)) + } ResourceContents::BlobResourceContents { .. } => { bail!("Blob resource content is not supported by Bedrock provider yet") } @@ -821,6 +826,35 @@ mod tests { Ok(()) } + #[test] + fn test_to_bedrock_tool_result_sanitizes_text_resource_fallback() -> Result<()> { + let content = ContentBlock::embedded_text("file:///result.bin", "visible\u{E0041}text"); + let result = to_bedrock_tool_result_content_block("test_id", content)?; + + let bedrock::ToolResultContentBlock::Text(text) = result else { + panic!("expected text fallback"); + }; + assert_eq!(text, "visibletext"); + + Ok(()) + } + + #[test] + fn test_to_bedrock_tool_result_sanitizes_document_resource() -> Result<()> { + let content = ContentBlock::embedded_text("file:///result.txt", "visible\u{E0041}text"); + let result = to_bedrock_tool_result_content_block("test_id", content)?; + + let bedrock::ToolResultContentBlock::Document(document) = result else { + panic!("expected document"); + }; + let Some(bedrock::DocumentSource::Bytes(bytes)) = document.source() else { + panic!("expected document bytes"); + }; + assert_eq!(bytes.as_ref(), b"visibletext"); + + Ok(()) + } + #[test] fn test_to_bedrock_message_with_caching() -> Result<()> { use chrono::Utc;