Sanitize Unicode tags in MCP resources (#10746)
This commit is contained in:
@@ -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(
|
||||
|
||||
@@ -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![
|
||||
|
||||
@@ -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!!!");
|
||||
|
||||
@@ -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<Option<bedrock::DocumentBlock>> {
|
||||
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;
|
||||
|
||||
Reference in New Issue
Block a user