From 3d51cd3db264f01ff36bf973824e97abec4d8193 Mon Sep 17 00:00:00 2001 From: Martin Ball Date: Thu, 22 Oct 2026 15:13:28 +0100 Subject: [PATCH] #3692: MCP type_text types its paragraphs as one edit and one undo step Audit finding 18 (CWE-407). `type_text` with `paragraphs: true` ran a text.insert and a text.newParagraph per line, each a mutating command with its own whole-document undo snapshot, so a long text did quadratic work and filled the undo history. It now sends one raw text.insert with `\r` paragraph breaks (Enter's split): one edit, one undo step, all or nothing. More than MAX_TYPED_PARAGRAPHS (10 000) lines is refused before anything is typed. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/mcp/src/tests.rs | 43 +++++++++++++++++++++++++++++++++-------- crates/mcp/src/tools.rs | 30 ++++++++++++++-------------- 2 files changed, 49 insertions(+), 24 deletions(-) diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index 781f24d02..e00037134 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -120,17 +120,16 @@ fn headless_sessions_keep_the_dictionary_in_memory() { assert!(h.session.user_dict_path.is_none()); } -/// A backend whose paragraph break is refused while text insertion would succeed. +/// A backend that keeps every command it is sent and refuses text with a paragraph break. struct RefusesParagraph { - calls: std::sync::Arc>>, + calls: std::sync::Arc>>, } impl crate::backend::Backend for RefusesParagraph { fn call(&mut self, _method: &str, params: Value) -> Result { - let command = params["command"].as_str().unwrap_or("").to_string(); - self.calls.lock().unwrap().push(command.clone()); - match command.as_str() { - "text.newParagraph" => Err("paragraph break refused".into()), + self.calls.lock().unwrap().push(params.clone()); + match params["params"]["text"].as_str() { + Some(t) if t.contains('\r') => Err("paragraph break refused".into()), _ => Ok(json!({"ok": true})), } } @@ -142,7 +141,8 @@ impl crate::backend::Backend for RefusesParagraph { } } -/// #1356: a refused paragraph break is reported and the next line is not typed after it. +/// #1356: a refused paragraph break is reported, and nothing of the text goes in without it: the +/// lines are one command, all or nothing. #[test] fn type_text_stops_at_a_refused_paragraph_break() { let calls = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); @@ -150,5 +150,32 @@ fn type_text_stops_at_a_refused_paragraph_break() { let r = call(&mut s, 11, "tools/call", json!({"name": "type_text", "arguments": {"text": "one\ntwo", "paragraphs": true}})); assert_eq!(r["result"]["isError"], true, "{r}"); assert!(r["result"]["content"][0]["text"].as_str().unwrap().contains("paragraph break refused"), "{r}"); - assert_eq!(*calls.lock().unwrap(), vec!["text.insert", "text.newParagraph"]); + let calls = calls.lock().unwrap(); + assert_eq!(calls.len(), 1, "{calls:?}"); + assert_eq!(calls[0]["command"], "text.insert"); + assert_eq!(calls[0]["params"]["text"], "one\rtwo"); +} + +/// #3692: typed paragraphs are one edit and one undo step, not a command (and a whole-document +/// undo snapshot) per line; too many lines are refused before anything is typed. +#[test] +fn type_text_paragraphs_are_one_undo_step_with_a_line_cap() { + let mut s = Server::new(Box::new(Headless::default())); + tool(&mut s, "new_document", json!({})); + let lines: Vec = (0..2000).map(|i| format!("Line {i}")).collect(); + tool(&mut s, "type_text", json!({"text": lines.join("\r\n"), "paragraphs": true})); + let text = tool(&mut s, "get_text", json!({}))["text"].as_str().unwrap().to_string(); + assert!(text.contains("Line 0") && text.contains("Line 1999"), "{}", text.len()); + let paras = |s: &mut Server| tool(s, "inspect_document", json!({}))["blocks"].as_array().unwrap().len(); + assert_eq!(paras(&mut s), 2000); + tool(&mut s, "execute", json!({"command": "edit.undo"})); + assert_eq!(paras(&mut s), 1, "one Undo takes all the lines back out"); + assert!(!tool(&mut s, "get_text", json!({}))["text"].as_str().unwrap().contains("Line")); + + let calls = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let mut s = Server::new(Box::new(RefusesParagraph { calls: calls.clone() })); + let many = "x\n".repeat(crate::tools::MAX_TYPED_PARAGRAPHS); + let r = call(&mut s, 12, "tools/call", json!({"name": "type_text", "arguments": {"text": many, "paragraphs": true}})); + assert_eq!(r["result"]["isError"], true, "{r}"); + assert!(calls.lock().unwrap().is_empty(), "refused before any command"); } diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 47e162a0b..3a46629b2 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -102,7 +102,7 @@ pub fn tool_definitions() -> Value { tool( "type_text", "Type text", - "Type at the caret (replacing the selection). Use \\n... for line breaks inside text; use execute text.newParagraph for new paragraphs, or `paragraphs: true` to split lines into paragraphs.", + "Type at the caret (replacing the selection). Use \\n... for line breaks inside text; use execute text.newParagraph for new paragraphs, or `paragraphs: true` to split lines into paragraphs (one undo step, at most 10000 lines).", obj(json!({"text": s("Text"), "paragraphs": {"type": "boolean"}}), &["text"]), false ), @@ -154,6 +154,9 @@ pub fn tool_definitions() -> Value { ]) } +/// Most paragraphs `type_text` with `paragraphs: true` types in one call. +pub const MAX_TYPED_PARAGRAPHS: usize = 10_000; + fn exec(b: &mut dyn Backend, id: &str, params: Value) -> Result { b.call("engine.execute", json!({"command": id, "params": params})) } @@ -229,22 +232,17 @@ pub fn call_tool(b: &mut dyn Backend, name: &str, a: &Value) -> ToolResult { "type_text" => { let t = st("text").unwrap_or(""); if a.get("paragraphs").and_then(Value::as_bool).unwrap_or(false) { - let mut last = Ok(Value::Null); - for (i, line) in t.split('\n').enumerate() { - if i > 0 { - last = exec(b, "text.newParagraph", json!({})); - if last.is_err() { - break; - } - } - if !line.is_empty() { - last = exec(b, "text.insert", json!({"text": line, "raw": true})); - } - if last.is_err() { - break; - } + // One `text.insert`: `\r` is a paragraph break there (Enter's split), so the + // lines go in as one edit and one undo step, all or nothing, instead of a command + // (and a whole-document undo snapshot) per line. + let t = t.replace("\r\n", "\n").replace('\r', "\n"); + let lines = t.split(['\n', '\u{2029}']).count(); + if lines > MAX_TYPED_PARAGRAPHS { + return ToolResult::error(format!( + "`text` has {lines} lines; type at most {MAX_TYPED_PARAGRAPHS} paragraphs per call" + )); } - wrap(last) + wrap(exec(b, "text.insert", json!({"text": t.replace('\n', "\r"), "raw": true}))) } else { wrap(exec(b, "text.insert", json!({"text": t, "raw": true}))) }