From 6babc092d45a68852be16bba4cda1eeaefb2f41d Mon Sep 17 00:00:00 2001 From: CodeWhale Bot Date: Tue, 15 Sep 2026 21:27:49 -0700 Subject: [PATCH] perf(tui): compile an MCP resource template once instead of per call (#6213 T7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `resource_uri_matches_template` is an authorization check that runs per URI per advertised template, and it rebuilt the anchored pattern string and compiled a fresh `Regex` on every one of those calls — while the template itself is fixed by the server's listing. Split the pattern builder out as `resource_template_pattern` and add `compiled_resource_template`, which caches the compiled program by template string. `None` still means "matches nothing", so a template using an operator this RFC 6570 subset does not implement stays uncallable rather than over-matching. Part of #6213 (item T7). T5, T6 and T4 remain, as does T1's id-keying half. Verification: cargo check -p codewhale-tui --all-targets --all-features --locked (clean) cargo clippy -p codewhale-tui --all-targets --all-features --locked -- \ -D warnings -A clippy::uninlined_format_args -A clippy::too_many_arguments \ -A clippy::unnecessary_map_or (clean) test result: ok. 275 passed; 0 failed; 1 ignored; 0 measured; 12531 filtered out (mcp::) test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 12807 filtered out (resource_uri_template_matching_is_anchored_and_fail_closed) Signed-off-by: CodeWhale Bot --- crates/tui/src/mcp.rs | 42 ++++++++++++++++++++++++++++++------- crates/tui/src/mcp/tests.rs | 40 +++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 7 deletions(-) diff --git a/crates/tui/src/mcp.rs b/crates/tui/src/mcp.rs index 90f18d1f21..20ec7baf35 100644 --- a/crates/tui/src/mcp.rs +++ b/crates/tui/src/mcp.rs @@ -1236,14 +1236,15 @@ pub struct McpResourceTemplate { /// simple (`{id}`), and reserved (`{+path}`) expansions cover the common MCP /// resource templates. More elaborate operators remain listable but are not /// callable until their expansion semantics are implemented exactly. -fn resource_uri_matches_template(uri: &str, template: &str) -> bool { +/// +/// `None` is the fail-closed answer: a template this subset cannot express +/// matches nothing. +fn resource_template_pattern(template: &str) -> Option { let mut pattern = String::from("^"); let mut rest = template; while let Some(start) = rest.find('{') { pattern.push_str(®ex::escape(&rest[..start])); - let Some(end) = rest[start + 1..].find('}') else { - return false; - }; + let end = rest[start + 1..].find('}')?; let expression = &rest[start + 1..start + 1 + end]; let (reserved, variables) = match expression.strip_prefix('+') { Some(variables) => (true, variables), @@ -1257,7 +1258,7 @@ fn resource_uri_matches_template(uri: &str, template: &str) -> bool { .all(|ch| ch.is_ascii_alphanumeric() || matches!(ch, '_' | '-' | '.')) }) { - return false; + return None; } let atom = if reserved { ".+" } else { "[^/?#]+" }; for (index, _) in variables.split(',').enumerate() { @@ -1269,11 +1270,38 @@ fn resource_uri_matches_template(uri: &str, template: &str) -> bool { rest = &rest[start + end + 2..]; } if rest.contains('}') { - return false; + return None; } pattern.push_str(®ex::escape(rest)); pattern.push('$'); - regex::Regex::new(&pattern).is_ok_and(|regex| regex.is_match(uri)) + Some(pattern) +} + +/// `template`'s anchored pattern, compiled once and reused. +/// +/// This runs per URI per advertised template, while the template itself is +/// fixed by the server's listing, so compiling it on every call was pure +/// repetition. `None` still means "matches nothing" (#6213 T7). +fn compiled_resource_template(template: &str) -> Option> { + static CACHE: std::sync::OnceLock< + std::sync::Mutex>>>, + > = std::sync::OnceLock::new(); + let cache = CACHE.get_or_init(|| std::sync::Mutex::new(HashMap::new())); + let mut cache = cache + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + cache + .entry(template.to_string()) + .or_insert_with(|| { + resource_template_pattern(template) + .and_then(|pattern| regex::Regex::new(&pattern).ok()) + .map(Arc::new) + }) + .clone() +} + +fn resource_uri_matches_template(uri: &str, template: &str) -> bool { + compiled_resource_template(template).is_some_and(|regex| regex.is_match(uri)) } /// Prompt discovered from an MCP server diff --git a/crates/tui/src/mcp/tests.rs b/crates/tui/src/mcp/tests.rs index 2693769d1c..5cc6a50331 100644 --- a/crates/tui/src/mcp/tests.rs +++ b/crates/tui/src/mcp/tests.rs @@ -8149,3 +8149,43 @@ async fn mcp_ceiling_preserves_ordinary_tool_result_tools_field() { } } } + +/// #6213 T7: the resource-URI template check is an authorization decision that +/// runs per URI per advertised template. Pin what it accepts, what it refuses, +/// and that the anchored pattern is compiled once rather than per call. +#[test] +fn resource_uri_template_matching_is_anchored_and_fail_closed() { + // Literal templates are anchored: no suffix may sneak past. + assert!(resource_uri_matches_template( + "file:///readme", + "file:///readme" + )); + assert!(!resource_uri_matches_template( + "file:///readme/extra", + "file:///readme" + )); + + // `{id}` is a simple expansion, so it must not cross a path separator. + assert!(resource_uri_matches_template("file:///a", "file:///{id}")); + assert!(!resource_uri_matches_template( + "file:///a/b", + "file:///{id}" + )); + + // `{+path}` is a reserved expansion, so it may. + assert!(resource_uri_matches_template( + "file:///a/b/c", + "file:///{+path}" + )); + + // An operator this subset does not implement, and a template that never + // closes its expression, both stay uncallable rather than over-matching. + assert!(!resource_uri_matches_template("x", "x{?query}")); + assert!(!resource_uri_matches_template("x", "x{id")); + + // The compile happens once per template and is reused. + let first = compiled_resource_template("file:///{path}").expect("template compiles"); + let second = compiled_resource_template("file:///{path}").expect("template compiles"); + assert!(Arc::ptr_eq(&first, &second)); + assert!(compiled_resource_template("x{?query}").is_none()); +}