From b38bfeaf233390846edeca898d6b11dbeb24db3b Mon Sep 17 00:00:00 2001 From: lr90 Date: Mon, 29 Jun 2026 16:19:58 +0800 Subject: [PATCH 1/2] fix: parameters validation of mcp interface --- memoria/.DS_Store | Bin 0 -> 8196 bytes memoria/crates/memoria-api/tests/api_e2e.rs | 110 ++++++++++++++++++ memoria/crates/memoria-mcp/src/tools.rs | 95 +++++++++++++-- .../memoria-mcp/tests/core_tools_e2e.rs | 51 ++++++++ .../crates/memoria-mcp/tests/tools_unit.rs | 52 +++++++++ 5 files changed, 301 insertions(+), 7 deletions(-) create mode 100644 memoria/.DS_Store diff --git a/memoria/.DS_Store b/memoria/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..6a91148ff5caae92785905837bc1826d3710abf9 GIT binary patch literal 8196 zcmeHMU2GIp6u#eWX=h=iQ=n3C2R0NUAjJw2{*&#WQcEdPYPSU`yE|h$IGrgwv)dwQ zHYUEPXnfN67kTn8@}Q!|7flpJAGJPUj1L%%`l1QGpgwr+y|W-KycvTsH<^3x`MY=S zneWWqGs_r5ThY3Wu|~$2Mwg(*9O|yngkD@T8jmDZL_z+H*(`K}QhUzzb2F@CK^%cN z0&xW52*eSHBXBK5fX-~uw5y!^N*nia1mXx>mk|)}hZJ3crXyNbRK7Z>lO+JrY7)yv zW2ytBPb#A6h?W(Vt~95r9uT=ADlwqkNuLttMAH#1E2`WXlsiMTGNK9va&_`cNpps@ zqH!NbAdbMy2#DIfhJ`H43T))^`n{?zH|{z?*wXSfcvbbRnp!O}dyamCIgl9~+NTd2 z^Gc3A(K{YGz8BnM7XqTgMqM!KPjuydZ+F2K`hmRf7RS6`*r?BE-F(r_hIXs#_BjvO zB&!SZ1>bdd`+*~-8O((CWGHkqRGFc`FNE!KvW!FXD=VWI*n4c6)G65=Gc)TB$TYxY&Q>xWp`$Suy@?{#4Vj69c}H7Wu`eO zbgh;pZ)kDjqB^tN?9q%bw4q=$Td;d4Ji9Oz$?w)Q`K(DUX6v+^(7&kc7fQqNN=`@* zh8`7>VxTou9!*nvwHh{kXqDwi2D&NTqQnV03{19;b+Dc6AOZ6zJIp_z1WRC=)-=D!oeW~n8Xnr#W6gBNAVbr z;|V;C7w{rp!U?>FH}Muu;T?Q{Gx!Xj<4c^wclaJZ;1^uL@A!jP^I5!>YkWRWa+5FN zEqpWI!ng8Wyq9NqmLK9ipHv&*(J965ReYuAM`$!X-Nx1DGJf*P>6W~6N7r55|DAL@ zJ69&yf`%IxrdF)J_4ZBMTQ34pA-4k7zo&r2{bkuAaW8Q(4YN}+6ZIuaE2(!W8C7!j zQ^MGjn#fzDHnMJ7u2I4mvm$ooDq_2h8pS4?*J_lU2A8a!T(>?!DQHwhZ1YB9p%{JZ zwge@kQ5~^M(!?^!B$FMTG|d@9`=^k8mYriivR~Psg!H+Xk4D^#7Q%NMc3_u8cpv(4 z0K+(l94w5%gO9_6^<#Jt521{Q3F(g$)}O>vcoxs$dBXe4cm=QGb-Y1ne;4oJeSCzE zaT=dSxc`Pw|1-`{Lvl|A!;0d!=}0d16+GAXMo3;k|K%<*ca23roc+`qMg8A-_3!`E zDDL72#1Z&^MgY|veI0GIx2wIkMXjBp>mXgC3A?hQ(uF!%juVyTIMIuL7}7XJroty3 b(Xyf>q4uwT2oOKx_4xdc&wmNO>$dv~|DgIi literal 0 HcmV?d00001 diff --git a/memoria/crates/memoria-api/tests/api_e2e.rs b/memoria/crates/memoria-api/tests/api_e2e.rs index 55c9071d..4ea9ca94 100644 --- a/memoria/crates/memoria-api/tests/api_e2e.rs +++ b/memoria/crates/memoria-api/tests/api_e2e.rs @@ -8703,6 +8703,116 @@ async fn test_mcp_tools_call_memory_store() { println!("✅ POST /mcp tools/call memory_store: {text}"); } +#[tokio::test] +async fn test_mcp_memory_store_rejects_empty_content() { + let (base, client, _server) = spawn_server().await; + let uid = uid(); + + for (id, args) in [ + (4, json!({})), + (5, json!({"content": ""})), + (6, json!({"content": " \t"})), + ] { + let resp = mcp_post_with_headers( + &client, + &base, + json!({ + "jsonrpc": "2.0", + "id": id, + "method": "tools/call", + "params": { + "name": "memory_store", + "arguments": args + } + }), + &[("X-User-Id", uid.as_str())], + ) + .await; + + assert_eq!(resp["jsonrpc"], "2.0"); + assert_eq!(resp["id"], id); + assert!( + resp["error"].is_null(), + "validation should return tool text, not RPC error: {}", + resp["error"] + ); + let text = resp["result"]["content"][0]["text"].as_str().unwrap_or(""); + assert!( + text.contains("content is required"), + "unexpected response for args={args}: {text}" + ); + } + + let list = client + .get(format!("{base}/v1/memories")) + .header("X-User-Id", &uid) + .send() + .await + .unwrap(); + assert_eq!(list.status(), 200); + assert!( + list.json::().await.unwrap()["items"] + .as_array() + .unwrap() + .is_empty(), + "empty MCP store must not create memories" + ); + println!("✅ POST /mcp memory_store rejects empty content"); +} + +#[tokio::test] +async fn test_mcp_memory_retrieve_and_search_reject_missing_query() { + let (base, client, _server) = spawn_server().await; + let uid = uid(); + + client + .post(format!("{base}/v1/memories")) + .header("X-User-Id", &uid) + .json(&json!({"content": "seed memory for query validation", "memory_type": "semantic"})) + .send() + .await + .unwrap(); + + for (id, tool, args) in [ + (7, "memory_retrieve", json!({})), + (8, "memory_retrieve", json!({"query": ""})), + (9, "memory_retrieve", json!({"query": " \t"})), + (10, "memory_search", json!({})), + (11, "memory_search", json!({"query": ""})), + (12, "memory_search", json!({"query": " \t"})), + ] { + let resp = mcp_post_with_headers( + &client, + &base, + json!({ + "jsonrpc": "2.0", + "id": id, + "method": "tools/call", + "params": { + "name": tool, + "arguments": args + } + }), + &[("X-User-Id", uid.as_str())], + ) + .await; + + assert_eq!(resp["jsonrpc"], "2.0"); + assert_eq!(resp["id"], id); + assert!( + resp["error"].is_null(), + "validation should return tool text, not RPC error: {}", + resp["error"] + ); + let text = resp["result"]["content"][0]["text"].as_str().unwrap_or(""); + assert!( + text.contains("query is required"), + "{tool} unexpected response for args={args}: {text}" + ); + } + println!("✅ POST /mcp memory_retrieve/search reject missing query"); +} + #[tokio::test] async fn test_mcp_memory_retrieve_session_scope_end_to_end() { let (base, client, _server) = diff --git a/memoria/crates/memoria-mcp/src/tools.rs b/memoria/crates/memoria-mcp/src/tools.rs index cd126212..38702197 100644 --- a/memoria/crates/memoria-mcp/src/tools.rs +++ b/memoria/crates/memoria-mcp/src/tools.rs @@ -94,6 +94,23 @@ fn branch_arg(args: &Value) -> Option<&str> { .filter(|branch| !branch.is_empty()) } +fn parse_required_str(args: &Value, key: &str, err: &'static str) -> Result { + let raw = args[key].as_str().unwrap_or(""); + if raw.trim().is_empty() { + Err(err) + } else { + Ok(raw.to_string()) + } +} + +fn parse_store_content(args: &Value) -> Result { + parse_required_str(args, "content", "content is required") +} + +fn parse_retrieve_query(args: &Value) -> Result { + parse_required_str(args, "query", "query is required") +} + enum ToolCallName { MemoryStore, MemoryRetrieve, @@ -358,7 +375,10 @@ pub async fn call( }; match tool { ToolCallName::MemoryStore => { - let content = args["content"].as_str().unwrap_or("").to_string(); + let content = match parse_store_content(&args) { + Ok(content) => content, + Err(msg) => return Ok(mcp_text(msg)), + }; let memory_type = args["memory_type"].as_str().unwrap_or("semantic"); let session_id = args["session_id"].as_str().map(String::from); let trust_tier = args["trust_tier"] @@ -447,7 +467,10 @@ pub async fn call( } ToolCallName::MemoryRetrieve | ToolCallName::MemorySearch => { - let query = args["query"].as_str().unwrap_or("").to_string(); + let query = match parse_retrieve_query(&args) { + Ok(query) => query, + Err(msg) => return Ok(mcp_text(msg)), + }; let top_k = if matches!(tool, ToolCallName::MemorySearch) { args["top_k"].as_i64().unwrap_or(10) } else { @@ -511,10 +534,10 @@ pub async fn call( } ToolCallName::MemoryCorrect => { - let new_content = args["new_content"].as_str().unwrap_or(""); - if new_content.is_empty() { - return Ok(mcp_text("new_content is required")); - } + let new_content = match parse_required_str(&args, "new_content", "new_content is required") { + Ok(s) => s, + Err(msg) => return Ok(mcp_text(msg)), + }; let memory_id = args["memory_id"].as_str().unwrap_or(""); let query = args["query"].as_str().unwrap_or(""); @@ -541,7 +564,7 @@ pub async fn call( }; let m = service - .correct_on_branch(user_id, branch_arg(&args), &old_mid, new_content) + .correct_on_branch(user_id, branch_arg(&args), &old_mid, &new_content) .await?; Ok(mcp_text(&format!( @@ -1436,6 +1459,64 @@ pub fn entity_extract_prompt(text: &str) -> String { mod tests { use super::*; + #[test] + fn parse_required_str_rejects_missing_and_blank() { + for val in [json!({}), json!({"k": ""}), json!({"k": " \t\n"})] { + assert!(parse_required_str(&val, "k", "k is required").is_err()); + } + } + + #[test] + fn parse_required_str_preserves_whitespace_when_valid() { + assert_eq!( + parse_required_str(&json!({"k": " hello "}), "k", "k is required").unwrap(), + " hello " + ); + } + + #[test] + fn parse_store_content_rejects_missing_and_blank() { + assert_eq!(parse_store_content(&json!({})), Err("content is required")); + assert_eq!( + parse_store_content(&json!({"content": ""})), + Err("content is required") + ); + assert_eq!( + parse_store_content(&json!({"content": " \t\n"})), + Err("content is required") + ); + } + + #[test] + fn parse_retrieve_query_rejects_missing_and_blank() { + assert_eq!(parse_retrieve_query(&json!({})), Err("query is required")); + assert_eq!( + parse_retrieve_query(&json!({"query": ""})), + Err("query is required") + ); + assert_eq!( + parse_retrieve_query(&json!({"query": " \t\n"})), + Err("query is required") + ); + } + + #[test] + fn parse_correct_new_content_rejects_blank() { + // memory_correct now uses parse_required_str — verify whitespace is rejected + assert_eq!( + parse_required_str(&json!({"new_content": ""}), "new_content", "new_content is required"), + Err("new_content is required") + ); + assert_eq!( + parse_required_str(&json!({"new_content": " "}), "new_content", "new_content is required"), + Err("new_content is required") + ); + assert_eq!( + parse_required_str(&json!({"new_content": "ok"}), "new_content", "new_content is required").unwrap(), + "ok" + ); + } + #[test] fn entity_extract_prompt_truncates_at_char_boundary() { // 2000 ASCII bytes then a 3-byte Chinese char — must not panic diff --git a/memoria/crates/memoria-mcp/tests/core_tools_e2e.rs b/memoria/crates/memoria-mcp/tests/core_tools_e2e.rs index 32e8d922..aad7d8ce 100644 --- a/memoria/crates/memoria-mcp/tests/core_tools_e2e.rs +++ b/memoria/crates/memoria-mcp/tests/core_tools_e2e.rs @@ -123,6 +123,26 @@ async fn test_store_rejects_invalid_trust_tier() { println!("✅ invalid trust_tier rejected explicitly"); } +// ── 2b. memory_store: reject missing/blank content ─────────────────────────── + +#[tokio::test] +async fn test_store_rejects_empty_content() { + let (svc, uid, _ctx) = setup().await; + for args in [json!({}), json!({"content": ""}), json!({"content": " \t"})] { + let r = call("memory_store", args, &svc, &uid).await; + assert!( + text(&r).contains("content is required"), + "unexpected response: {}", + text(&r) + ); + } + assert!( + svc.list_active(&uid, 10).await.unwrap().is_empty(), + "empty content must not create memories" + ); + println!("✅ store rejects empty content"); +} + // ── 3. memory_retrieve: returns relevant memories ──────────────────────────── #[tokio::test] @@ -170,6 +190,37 @@ async fn test_retrieve_empty() { println!("✅ retrieve empty: {}", text(&r)); } +// ── 3b. memory_retrieve/search: reject missing/blank query ─────────────────── + +#[tokio::test] +async fn test_retrieve_and_search_reject_missing_query() { + let (svc, uid, _ctx) = setup().await; + call( + "memory_store", + json!({"content": "rust ownership model"}), + &svc, + &uid, + ) + .await; + + for (tool, args) in [ + ("memory_retrieve", json!({})), + ("memory_retrieve", json!({"query": ""})), + ("memory_retrieve", json!({"query": " \t"})), + ("memory_search", json!({})), + ("memory_search", json!({"query": ""})), + ("memory_search", json!({"query": " \t"})), + ] { + let r = call(tool, args, &svc, &uid).await; + assert!( + text(&r).contains("query is required"), + "{tool} unexpected response: {}", + text(&r) + ); + } + println!("✅ retrieve/search reject missing query"); +} + #[tokio::test] async fn test_retrieve_session_scope_only() { let (svc, uid, _ctx) = setup().await; diff --git a/memoria/crates/memoria-mcp/tests/tools_unit.rs b/memoria/crates/memoria-mcp/tests/tools_unit.rs index b0123c87..1c86124b 100644 --- a/memoria/crates/memoria-mcp/tests/tools_unit.rs +++ b/memoria/crates/memoria-mcp/tests/tools_unit.rs @@ -156,6 +156,26 @@ async fn test_tool_memory_store() { println!("✅ tool memory_store: {text}"); } +#[tokio::test] +async fn test_tool_memory_store_rejects_empty_content() { + let svc = make_service(); + for args in [json!({}), json!({"content": ""}), json!({"content": " "})] { + let result = memoria_mcp::tools::call("memory_store", args, &svc, "u1") + .await + .unwrap(); + let text = result["content"][0]["text"].as_str().unwrap(); + assert!( + text.contains("content is required"), + "unexpected response: {text}" + ); + } + assert!( + svc.list_active("u1", 10).await.unwrap().is_empty(), + "empty content must not create memories" + ); + println!("✅ tool memory_store rejects empty content"); +} + #[tokio::test] async fn test_tool_memory_retrieve_empty() { let svc = make_service(); @@ -172,6 +192,38 @@ async fn test_tool_memory_retrieve_empty() { println!("✅ tool memory_retrieve empty: {text}"); } +#[tokio::test] +async fn test_tool_memory_retrieve_rejects_missing_query() { + let svc = make_service(); + memoria_mcp::tools::call( + "memory_store", + json!({"content": "rust programming"}), + &svc, + "u1", + ) + .await + .unwrap(); + + for (tool, args) in [ + ("memory_retrieve", json!({})), + ("memory_retrieve", json!({"query": ""})), + ("memory_retrieve", json!({"query": " "})), + ("memory_search", json!({})), + ("memory_search", json!({"query": ""})), + ("memory_search", json!({"query": " "})), + ] { + let result = memoria_mcp::tools::call(tool, args, &svc, "u1") + .await + .unwrap(); + let text = result["content"][0]["text"].as_str().unwrap(); + assert!( + text.contains("query is required"), + "{tool} unexpected response: {text}" + ); + } + println!("✅ tool memory_retrieve/search reject missing query"); +} + #[tokio::test] async fn test_tool_memory_retrieve_finds() { let svc = make_service(); From a1cf2eeb89e356ec9a9c306d6cc17b8c750f6cd9 Mon Sep 17 00:00:00 2001 From: lr90 Date: Mon, 29 Jun 2026 17:18:49 +0800 Subject: [PATCH 2/2] fix: refine test cases --- memoria/crates/memoria-api/tests/api_e2e.rs | 8 -------- memoria/crates/memoria-mcp/tests/core_tools_e2e.rs | 7 ------- memoria/crates/memoria-mcp/tests/tools_unit.rs | 8 -------- 3 files changed, 23 deletions(-) diff --git a/memoria/crates/memoria-api/tests/api_e2e.rs b/memoria/crates/memoria-api/tests/api_e2e.rs index 4ea9ca94..98dbdb5f 100644 --- a/memoria/crates/memoria-api/tests/api_e2e.rs +++ b/memoria/crates/memoria-api/tests/api_e2e.rs @@ -8765,14 +8765,6 @@ async fn test_mcp_memory_retrieve_and_search_reject_missing_query() { let (base, client, _server) = spawn_server().await; let uid = uid(); - client - .post(format!("{base}/v1/memories")) - .header("X-User-Id", &uid) - .json(&json!({"content": "seed memory for query validation", "memory_type": "semantic"})) - .send() - .await - .unwrap(); - for (id, tool, args) in [ (7, "memory_retrieve", json!({})), (8, "memory_retrieve", json!({"query": ""})), diff --git a/memoria/crates/memoria-mcp/tests/core_tools_e2e.rs b/memoria/crates/memoria-mcp/tests/core_tools_e2e.rs index aad7d8ce..15e6d5c7 100644 --- a/memoria/crates/memoria-mcp/tests/core_tools_e2e.rs +++ b/memoria/crates/memoria-mcp/tests/core_tools_e2e.rs @@ -195,13 +195,6 @@ async fn test_retrieve_empty() { #[tokio::test] async fn test_retrieve_and_search_reject_missing_query() { let (svc, uid, _ctx) = setup().await; - call( - "memory_store", - json!({"content": "rust ownership model"}), - &svc, - &uid, - ) - .await; for (tool, args) in [ ("memory_retrieve", json!({})), diff --git a/memoria/crates/memoria-mcp/tests/tools_unit.rs b/memoria/crates/memoria-mcp/tests/tools_unit.rs index 1c86124b..1ed12dbe 100644 --- a/memoria/crates/memoria-mcp/tests/tools_unit.rs +++ b/memoria/crates/memoria-mcp/tests/tools_unit.rs @@ -195,14 +195,6 @@ async fn test_tool_memory_retrieve_empty() { #[tokio::test] async fn test_tool_memory_retrieve_rejects_missing_query() { let svc = make_service(); - memoria_mcp::tools::call( - "memory_store", - json!({"content": "rust programming"}), - &svc, - "u1", - ) - .await - .unwrap(); for (tool, args) in [ ("memory_retrieve", json!({})),