| 1 | //! Runtime MCP server management. |
| 2 | //! |
| 3 | //! Provides `StartRuntimeMcpServer` — the entry tool for LLM to dynamically |
| 4 | //! connect to MCP servers from conversation context. Also contains parsing |
| 5 | //! and naming helpers used by the tool. |
| 6 | |
| 7 | use std::collections::HashMap; |
| 8 | use std::sync::Arc; |
| 9 | |
| 10 | use anyhow::Result; |
| 11 | use serde_json::{Value, json}; |
| 12 | use tokio::sync::Mutex as AsyncMutex; |
| 13 | |
| 14 | use crate::mcp::{McpPool, McpServerConfig}; |
| 15 | use crate::tools::spec::{ |
| 16 | ApprovalRequirement, ToolCapability, ToolContext, ToolError, ToolResult, ToolSpec, |
| 17 | }; |
| 18 | |
| 19 | // === Parsing Functions === |
| 20 | |
| 21 | #[derive(Debug, Clone)] |
| 22 | pub struct ParsedMcpServer { |
| 23 | pub name: String, |
| 24 | pub config: McpServerConfig, |
| 25 | } |
| 26 | |
| 27 | /// Parse a command string or URL into an MCP server configuration. |
| 28 | /// |
| 29 | /// - Local command: `npx @modelcontextprotocol/server-filesystem /tmp` |
| 30 | /// - Remote URL: `https://huggingface.co/mcp` |
| 31 | pub fn parse_mcp_command(input: &str) -> Result<ParsedMcpServer> { |
| 32 | let input = input.trim(); |
| 33 | if input.is_empty() { |
| 34 | anyhow::bail!("MCP command cannot be empty"); |
| 35 | } |
| 36 | |
| 37 | if input.starts_with("http://") || input.starts_with("https://") { |
| 38 | let name = extract_name_from_url(input)?; |
| 39 | return Ok(ParsedMcpServer { |
| 40 | name, |
| 41 | config: McpServerConfig { |
| 42 | command: None, |
| 43 | args: Vec::new(), |
| 44 | env: HashMap::new(), |
| 45 | cwd: None, |
| 46 | url: Some(input.to_string()), |
| 47 | transport: None, |
| 48 | connect_timeout: None, |
| 49 | execute_timeout: None, |
| 50 | read_timeout: None, |
| 51 | disabled: false, |
| 52 | enabled: true, |
| 53 | required: false, |
| 54 | enabled_tools: Vec::new(), |
| 55 | disabled_tools: Vec::new(), |
| 56 | headers: HashMap::new(), |
| 57 | env_headers: HashMap::new(), |
| 58 | bearer_token_env_var: None, |
| 59 | scopes: Vec::new(), |
| 60 | oauth: None, |
| 61 | oauth_resource: None, |
| 62 | reviewed_plugin: None, |
| 63 | runtime_added: false, |
| 64 | allow_private_network: false, |
| 65 | }, |
| 66 | }); |
| 67 | } |
| 68 | |
| 69 | let parts: Vec<String> = shell_words::split(input).unwrap_or_default(); |
| 70 | if parts.is_empty() { |
| 71 | anyhow::bail!("MCP command cannot be empty"); |
| 72 | } |
| 73 | |
| 74 | let command = parts[0].clone(); |
| 75 | let args: Vec<String> = parts[1..].to_vec(); |
| 76 | let name = infer_server_name(&command, &args)?; |
| 77 | |
| 78 | Ok(ParsedMcpServer { |
| 79 | name, |
| 80 | config: McpServerConfig { |
| 81 | command: Some(command), |
| 82 | args, |
| 83 | env: HashMap::new(), |
| 84 | cwd: None, |
| 85 | url: None, |
| 86 | transport: None, |
| 87 | connect_timeout: None, |
| 88 | execute_timeout: None, |
| 89 | read_timeout: None, |
| 90 | disabled: false, |
| 91 | enabled: true, |
| 92 | required: false, |
| 93 | enabled_tools: Vec::new(), |
| 94 | disabled_tools: Vec::new(), |
| 95 | headers: HashMap::new(), |
| 96 | env_headers: HashMap::new(), |
| 97 | bearer_token_env_var: None, |
| 98 | scopes: Vec::new(), |
| 99 | oauth: None, |
| 100 | oauth_resource: None, |
| 101 | reviewed_plugin: None, |
| 102 | runtime_added: false, |
| 103 | allow_private_network: false, |
| 104 | }, |
| 105 | }) |
| 106 | } |
| 107 | |
| 108 | pub fn extract_name_from_url(url: &str) -> Result<String> { |
| 109 | let parsed = reqwest::Url::parse(url)?; |
| 110 | let host = parsed.host_str().unwrap_or("remote"); |
| 111 | let path = parsed.path().trim_matches('/'); |
| 112 | |
| 113 | // Replace dots with dashes in hostname for better readability |
| 114 | let host_part = host.replace('.', "-"); |
| 115 | |
| 116 | // Combine host and path, replacing slashes with underscores |
| 117 | let name = if path.is_empty() { |
| 118 | host_part |
| 119 | } else { |
| 120 | format!("{}_{}", host_part, path.replace('/', "_")) |
| 121 | }; |
| 122 | |
| 123 | Ok(sanitize_name(&name)) |
| 124 | } |
| 125 | |
| 126 | fn infer_server_name(command: &str, args: &[String]) -> Result<String> { |
| 127 | let cmd_path = std::path::Path::new(command); |
| 128 | let cmd_base = cmd_path.file_stem().unwrap_or_default().to_string_lossy(); |
| 129 | |
| 130 | // Windows cmd /c prefix: skip "cmd /c" and recurse on the remaining args |
| 131 | // e.g. ["cmd", "/c", "npx", "-y", "@modelcontextprotocol/server-memory"] |
| 132 | if cmd_base.as_ref() == "cmd" |
| 133 | && args.len() >= 2 |
| 134 | && (args[0] == "/c" || args[0] == "/C" || args[0] == "/k" || args[0] == "/K") |
| 135 | { |
| 136 | let inner_cmd = &args[1]; |
| 137 | let inner_args: Vec<String> = args[2..].to_vec(); |
| 138 | return infer_server_name(inner_cmd, &inner_args); |
| 139 | } |
| 140 | |
| 141 | // Package managers: extract the package name (first non-flag arg) |
| 142 | if matches!( |
| 143 | cmd_base.as_ref(), |
| 144 | "npx" | "npm" | "pnpm" | "yarn" | "bunx" | "bun" |
| 145 | ) { |
| 146 | for arg in args { |
| 147 | if !arg.starts_with('-') && arg != "exec" && arg != "run" && arg != "start" { |
| 148 | // e.g. "@modelcontextprotocol/server-filesystem" → "filesystem" |
| 149 | if let Some(name) = arg.split('/').next_back() { |
| 150 | if let Some(short) = name.strip_prefix("server-") { |
| 151 | return Ok(sanitize_name(short)); |
| 152 | } |
| 153 | return Ok(sanitize_name(name)); |
| 154 | } |
| 155 | } |
| 156 | } |
| 157 | } |
| 158 | |
| 159 | // Script interpreters: extract the script path (first non-flag arg) |
| 160 | if matches!( |
| 161 | cmd_base.as_ref(), |
| 162 | "node" | "python" | "python3" | "uvx" | "uv" | "ruby" | "deno" |
| 163 | ) && let Some(script) = args.iter().find(|a| !a.starts_with('-')) |
| 164 | { |
| 165 | let script_path = std::path::Path::new(script); |
| 166 | if let Some(stem) = script_path.file_stem() { |
| 167 | return Ok(sanitize_name(&stem.to_string_lossy())); |
| 168 | } |
| 169 | } |
| 170 | |
| 171 | // Fallback: first non-flag argument (script or file) |
| 172 | if let Some(script) = args.iter().find(|a| !a.starts_with('-')) { |
| 173 | let script_path = std::path::Path::new(script); |
| 174 | if let Some(stem) = script_path.file_stem() { |
| 175 | return Ok(sanitize_name(&stem.to_string_lossy())); |
| 176 | } |
| 177 | } |
| 178 | |
| 179 | // Last resort: command name itself |
| 180 | Ok(sanitize_name(&cmd_base)) |
| 181 | } |
| 182 | |
| 183 | pub fn sanitize_name(name: &str) -> String { |
| 184 | name.chars() |
| 185 | .map(|c| { |
| 186 | if c.is_ascii_alphanumeric() || c == '-' { |
| 187 | c |
| 188 | } else { |
| 189 | '-' |
| 190 | } |
| 191 | }) |
| 192 | .collect::<String>() |
| 193 | .trim_matches('-') |
| 194 | .to_string() |
| 195 | } |
| 196 | |
| 197 | // === Tool: StartRuntimeMcpServer === |
| 198 | |
| 199 | /// Entry tool for dynamically adding MCP servers from conversation context. |
| 200 | /// |
| 201 | /// LLM calls this to start a local MCP server (stdio) or connect to a remote |
| 202 | /// one (HTTP). The server config is added to `McpPool.dynamic_servers` and |
| 203 | /// tools are discovered via the existing `McpConnection` / `StdioTransport` flow. |
| 204 | pub struct StartRuntimeMcpServer { |
| 205 | pool: Arc<AsyncMutex<McpPool>>, |
| 206 | } |
| 207 | |
| 208 | impl StartRuntimeMcpServer { |
| 209 | pub fn new(pool: Arc<AsyncMutex<McpPool>>) -> Self { |
| 210 | Self { pool } |
| 211 | } |
| 212 | } |
| 213 | |
| 214 | #[async_trait::async_trait] |
| 215 | impl ToolSpec for StartRuntimeMcpServer { |
| 216 | fn name(&self) -> &str { |
| 217 | "start_mcp_server" |
| 218 | } |
| 219 | |
| 220 | fn description(&self) -> &str { |
| 221 | "When a user provides an MCP server command (like 'npx ...') or URL \ |
| 222 | (like 'https://...'), call this tool immediately to start the server \ |
| 223 | and register its tools. Do NOT suggest editing config files. \ |
| 224 | Accepts a local command (stdio) or a remote URL (HTTP/SSE). \ |
| 225 | To reconnect an existing configured server after login, pass only its exact name \ |
| 226 | and omit server; this keeps its saved credentials and configuration. \ |
| 227 | After the server starts, the response lists each tool's callable name. \ |
| 228 | You MUST copy those exact names when calling the tools. \ |
| 229 | Do NOT construct or guess tool names yourself." |
| 230 | } |
| 231 | |
| 232 | fn input_schema(&self) -> Value { |
| 233 | json!({ |
| 234 | "type": "object", |
| 235 | "properties": { |
| 236 | "server": { |
| 237 | "type": "string", |
| 238 | "description": "New MCP server command or URL; omit to reconnect a configured server by name" |
| 239 | }, |
| 240 | "name": { |
| 241 | "type": "string", |
| 242 | "description": "Exact configured name for reconnect; optional name for a new server" |
| 243 | } |
| 244 | }, |
| 245 | "anyOf": [{"required": ["server"]}, {"required": ["name"]}] |
| 246 | }) |
| 247 | } |
| 248 | |
| 249 | fn capabilities(&self) -> Vec<ToolCapability> { |
| 250 | vec![ToolCapability::Network, ToolCapability::ExecutesCode] |
| 251 | } |
| 252 | |
| 253 | fn approval_requirement(&self) -> ApprovalRequirement { |
| 254 | ApprovalRequirement::Required |
| 255 | } |
| 256 | |
| 257 | async fn execute(&self, input: Value, context: &ToolContext) -> Result<ToolResult, ToolError> { |
| 258 | let custom_name = input.get("name").and_then(|v| v.as_str()); |
| 259 | if input.get("server").is_none() { |
| 260 | let name = custom_name |
| 261 | .filter(|name| !name.trim().is_empty()) |
| 262 | .ok_or_else(|| { |
| 263 | ToolError::invalid_input("Provide server or an existing configured name") |
| 264 | })?; |
| 265 | // The exact configured key owns its credentials and trust. Do not |
| 266 | // sanitize it into an alias, replace its config, or reconnect siblings. |
| 267 | if McpPool::server_denied_by(&context.disallowed_tools, name) { |
| 268 | return Err(ToolError::not_available(format!( |
| 269 | "Failed to find MCP server: {name}" |
| 270 | ))); |
| 271 | } |
| 272 | let mut pool = self.pool.lock().await; |
| 273 | let conn = pool.retry_connection(name).await.map_err(|error| { |
| 274 | ToolError::execution_failed(connect_failure_message(name, &error)) |
| 275 | })?; |
| 276 | let transport = if conn.config().url.is_some() { |
| 277 | "http" |
| 278 | } else { |
| 279 | "stdio" |
| 280 | }; |
| 281 | return Ok(connected_result(&pool, name, transport, context)); |
| 282 | } |
| 283 | let server = input |
| 284 | .get("server") |
| 285 | .and_then(|v| v.as_str()) |
| 286 | .ok_or_else(|| ToolError::invalid_input("server must be a command or URL string"))?; |
| 287 | let mut parsed = |
| 288 | parse_mcp_command(server).map_err(|e| ToolError::invalid_input(e.to_string()))?; |
| 289 | // Host-supplied override (used by the Registry launcher, whose |
| 290 | // packages cold-start via npx/uvx downloads). Not exposed on the |
| 291 | // model-facing schema, so the model cannot widen its own timeouts. |
| 292 | if let Some(timeout) = input.get("connect_timeout").and_then(Value::as_u64) { |
| 293 | parsed.config.connect_timeout = Some(timeout); |
| 294 | } |
| 295 | |
| 296 | // Refuse shell wrappers and anything outside the runtime allowlist. |
| 297 | if let Some(ref cmd) = parsed.config.command { |
| 298 | validate_runtime_command(cmd)?; |
| 299 | } |
| 300 | |
| 301 | // Reject shell metacharacters in arguments to prevent injection. |
| 302 | // Extracted to `reject_shell_metacharacters` so it is reachable from |
| 303 | // tests: the `reject_metachar_*` tests used to assert only that their |
| 304 | // own input string contained the metacharacter and never that this |
| 305 | // guard refused it, so deleting the guard left them green |
| 306 | // (2026-08-04 audit). |
| 307 | reject_shell_metacharacters(&parsed.config.args)?; |
| 308 | |
| 309 | let server_name = custom_name |
| 310 | .map(sanitize_name) |
| 311 | .unwrap_or(parsed.name) |
| 312 | .replace('_', "-"); |
| 313 | |
| 314 | // Underscores in server names would cause tool name collision. |
| 315 | // Tool names are formatted as mcp_{server}_{tool}; underscores in |
| 316 | // server names would make it ambiguous (server "foo" + tool "bar_x" |
| 317 | // vs server "foo_bar" + tool "x" both → mcp_foo_bar_x). |
| 318 | // sanitize_name already converts non-alphanumeric chars to hyphens, |
| 319 | // but underscores from the original input need explicit conversion. |
| 320 | |
| 321 | let transport = if parsed.config.url.is_some() { |
| 322 | "http" |
| 323 | } else { |
| 324 | "stdio" |
| 325 | }; |
| 326 | |
| 327 | // Ancestor restrictions stay request-local because children share the pool. |
| 328 | if McpPool::server_denied_by(&context.disallowed_tools, &server_name) { |
| 329 | return Err(ToolError::not_available(format!( |
| 330 | "Failed to find MCP server: {server_name}" |
| 331 | ))); |
| 332 | } |
| 333 | // Register server config, connect, and collect tool info |
| 334 | let mut pool = self.pool.lock().await; |
| 335 | pool.add_runtime_server_config(server_name.clone(), parsed.config) |
| 336 | .map_err(ToolError::invalid_input)?; |
| 337 | let conn = match pool.get_or_connect(&server_name).await { |
| 338 | Ok(conn) => conn, |
| 339 | Err(error) => { |
| 340 | let message = connect_failure_message(&server_name, &error); |
| 341 | pool.remove_runtime_server_config(&server_name); |
| 342 | return Err(ToolError::execution_failed(message)); |
| 343 | } |
| 344 | }; |
| 345 | |
| 346 | let _ = conn; |
| 347 | Ok(connected_result(&pool, &server_name, transport, context)) |
| 348 | } |
| 349 | } |
| 350 | |
| 351 | /// Shared receipt for both new servers and configured-name reconnects. |
| 352 | fn connected_result( |
| 353 | pool: &McpPool, |
| 354 | server_name: &str, |
| 355 | transport: &str, |
| 356 | context: &ToolContext, |
| 357 | ) -> ToolResult { |
| 358 | let owners = pool.resolved_tool_servers(); |
| 359 | let tools_list: Vec<String> = pool |
| 360 | .all_tools() |
| 361 | .into_iter() |
| 362 | .filter(|(name, _)| { |
| 363 | owners.get(name).map(String::as_str) == Some(server_name) |
| 364 | && !crate::core::engine::tool_catalog::tool_matches_any_rule( |
| 365 | &context.disallowed_tools, |
| 366 | name, |
| 367 | ) |
| 368 | }) |
| 369 | .map(|(name, tool)| { |
| 370 | format!( |
| 371 | "- {} → {}", |
| 372 | name, |
| 373 | tool.description.as_deref().unwrap_or("no description") |
| 374 | ) |
| 375 | }) |
| 376 | .collect(); |
| 377 | let result = serde_json::to_string(&json!({ |
| 378 | "status": "connected", |
| 379 | "transport": transport, |
| 380 | "server": server_name, |
| 381 | "new_tools": tools_list.len(), |
| 382 | "total_mcp_tools": pool.all_tools().iter().filter(|(name, _)| !crate::core::engine::tool_catalog::tool_matches_any_rule(&context.disallowed_tools, name)).count(), |
| 383 | "message": format!( |
| 384 | "MCP server '{}' connected via {}. {} tools discovered.\n\nCallable tools (use these exact names):\n{}", |
| 385 | server_name, transport, tools_list.len(), tools_list.join("\n") |
| 386 | ) |
| 387 | })).unwrap_or_else(|_| "{}".to_string()); |
| 388 | let mut output = ToolResult::success(result); |
| 389 | output.metadata = Some(json!({ "mcp_catalog_changed": true })); |
| 390 | output |
| 391 | } |
| 392 | |
| 393 | /// Shell interpreters that would run an arbitrary script as the "server". |
| 394 | const SHELL_WRAPPERS: &[&str] = &["bash", "sh", "zsh", "cmd", "powershell"]; |
| 395 | |
| 396 | /// Known MCP server runtimes and package managers. Anything else is refused |
| 397 | /// to prevent arbitrary execution. |
| 398 | const ALLOWED_COMMANDS: &[&str] = &[ |
| 399 | "npx", "npm", "pnpm", "yarn", "bunx", "bun", "node", "python", "python3", "uvx", "uv", "deno", |
| 400 | "ruby", "cargo", |
| 401 | ]; |
| 402 | |
| 403 | /// Refuse a runtime MCP command that is a shell wrapper or is not a known |
| 404 | /// server runtime. Both checks read the command's lowercased file stem, so |
| 405 | /// `/bin/zsh` and `powershell.exe` are named for what they are. Kept out of |
| 406 | /// `execute` (as [`reject_shell_metacharacters`] is) so tests exercise the |
| 407 | /// real guard rather than a copy of its list. |
| 408 | fn validate_runtime_command(cmd: &str) -> Result<(), ToolError> { |
| 409 | let cmd_base = std::path::Path::new(cmd) |
| 410 | .file_stem() |
| 411 | .unwrap_or_default() |
| 412 | .to_string_lossy() |
| 413 | .to_lowercase(); |
| 414 | if SHELL_WRAPPERS.contains(&cmd_base.as_str()) { |
| 415 | return Err(ToolError::invalid_input(format!( |
| 416 | "Shell wrapper commands ({cmd}) are not allowed. \ |
| 417 | Provide the actual MCP server command directly, \ |
| 418 | e.g. 'npx @modelcontextprotocol/server-filesystem /tmp'" |
| 419 | ))); |
| 420 | } |
| 421 | if !ALLOWED_COMMANDS.contains(&cmd_base.as_str()) { |
| 422 | return Err(ToolError::invalid_input(format!( |
| 423 | "Command '{cmd}' is not in the allowed list. \ |
| 424 | Permitted commands: {}", |
| 425 | ALLOWED_COMMANDS.join(", ") |
| 426 | ))); |
| 427 | } |
| 428 | Ok(()) |
| 429 | } |
| 430 | |
| 431 | /// Refuse MCP server arguments carrying shell metacharacters. |
| 432 | /// |
| 433 | /// Redirects (`>`), pipes (`|`), chaining (`;`, `&`), subshells (`` ` ``), and |
| 434 | /// variable expansion (`$`) are all dangerous in an argv that may reach a |
| 435 | /// shell. Kept as a free function rather than inline in `execute` so it is |
| 436 | /// directly testable: the `reject_metachar_*` tests previously asserted only |
| 437 | /// that their own input contained the metacharacter, so deleting the guard |
| 438 | /// left every one of them green (2026-08-04 audit). |
| 439 | fn reject_shell_metacharacters(args: &[String]) -> Result<(), ToolError> { |
| 440 | for arg in args { |
| 441 | if arg.contains(['>', '|', ';', '&', '`', '$']) { |
| 442 | return Err(ToolError::invalid_input(format!( |
| 443 | "Argument contains shell metacharacters: '{arg}'. \ |
| 444 | MCP server arguments must not contain redirects, pipes, \ |
| 445 | command chaining, or variable expansion." |
| 446 | ))); |
| 447 | } |
| 448 | } |
| 449 | Ok(()) |
| 450 | } |
| 451 | |
| 452 | /// Build the connect-failure message returned to the model. A spawned |
| 453 | /// package that prints its CLI help and exits (the classic |
| 454 | /// missing-subcommand case, e.g. `npx -y agentic-mermaid@0.1.2` without |
| 455 | /// `mcp`) surfaces as `Stdio transport closed` before the handshake |
| 456 | /// completes — a bare transport error gives the model no signal about |
| 457 | /// *why*, and it tends to abandon the MCP route after one failed server. |
| 458 | /// Classify that early-exit shape, note when the captured output looks |
| 459 | /// like usage help, and point recovery at the registry: verify the exact |
| 460 | /// structured arguments returned by `registry_sync`, then fall through to |
| 461 | /// the next candidate from the search results instead of giving up. |
| 462 | fn connect_failure_message(server_name: &str, err: &anyhow::Error) -> String { |
| 463 | let text = format!("{err:#}"); |
| 464 | let base = format!("Failed to connect to MCP server '{server_name}': {text}"); |
| 465 | let early_exit = |
| 466 | text.contains("Stdio transport closed") || text.contains("Stdio transport read error"); |
| 467 | if !early_exit { |
| 468 | return base; |
| 469 | } |
| 470 | let looks_like_help = text.contains("usage") |
| 471 | || text.contains("Usage") |
| 472 | || text.contains("--help") |
| 473 | || text.contains("Commands:"); |
| 474 | let help_note = if looks_like_help { |
| 475 | " Its output above looks like CLI usage help." |
| 476 | } else { |
| 477 | "" |
| 478 | }; |
| 479 | format!( |
| 480 | "{base}\n\nThe server process exited before completing the MCP handshake.{help_note} The launch arguments are usually incomplete in this case (missing subcommand or required argument). For Registry-discovered servers, verify the structured required_args returned by registry_sync and retry; if this server still will not start, try the next candidate from the Registry catalog." |
| 481 | ) |
| 482 | } |
| 483 | |
| 484 | #[cfg(test)] |
| 485 | mod tests { |
| 486 | use super::*; |
| 487 | |
| 488 | #[tokio::test] |
| 489 | async fn mcp_ceiling_runtime_registration_respects_child_policy_before_connection() { |
| 490 | let directory = tempfile::tempdir().unwrap(); |
| 491 | let pool = Arc::new(AsyncMutex::new(McpPool::new( |
| 492 | crate::mcp::McpConfig::default(), |
| 493 | ))); |
| 494 | let tool = StartRuntimeMcpServer::new(Arc::clone(&pool)); |
| 495 | let mut context = ToolContext::new(directory.path()); |
| 496 | context.disallowed_tools = vec!["mcp_private-*".to_string()]; |
| 497 | // The command would execute if the child ceiling were ignored. |
| 498 | let error = tool |
| 499 | .execute( |
| 500 | json!({"server":"node nonexistent-mcp.js", "name":"private_a"}), |
| 501 | &context, |
| 502 | ) |
| 503 | .await |
| 504 | .unwrap_err(); |
| 505 | assert!( |
| 506 | error |
| 507 | .to_string() |
| 508 | .contains("Failed to find MCP server: private-a") |
| 509 | ); |
| 510 | assert!(pool.lock().await.server_names().is_empty()); |
| 511 | assert!(pool.lock().await.connected_servers().is_empty()); |
| 512 | } |
| 513 | |
| 514 | #[test] |
| 515 | fn parse_command_stdio() { |
| 516 | let parsed = parse_mcp_command("npx @modelcontextprotocol/server-filesystem /tmp").unwrap(); |
| 517 | assert!(parsed.config.command.is_some()); |
| 518 | assert!(parsed.config.url.is_none()); |
| 519 | } |
| 520 | |
| 521 | #[test] |
| 522 | fn parse_command_url() { |
| 523 | let parsed = parse_mcp_command("https://huggingface.co/mcp").unwrap(); |
| 524 | assert!(parsed.config.command.is_none()); |
| 525 | assert!(parsed.config.url.is_some()); |
| 526 | assert_eq!(parsed.name, "huggingface-co-mcp"); |
| 527 | } |
| 528 | |
| 529 | #[test] |
| 530 | fn parse_command_url_with_subdomain() { |
| 531 | let parsed = parse_mcp_command("https://api.example.com/mcp").unwrap(); |
| 532 | assert!(parsed.config.command.is_none()); |
| 533 | assert!(parsed.config.url.is_some()); |
| 534 | assert_eq!(parsed.name, "api-example-com-mcp"); |
| 535 | } |
| 536 | |
| 537 | #[test] |
| 538 | fn parse_command_empty() { |
| 539 | assert!(parse_mcp_command("").is_err()); |
| 540 | assert!(parse_mcp_command(" ").is_err()); |
| 541 | } |
| 542 | |
| 543 | #[test] |
| 544 | fn extract_name_from_url_with_path() { |
| 545 | assert_eq!( |
| 546 | extract_name_from_url("https://huggingface.co/mcp").unwrap(), |
| 547 | "huggingface-co-mcp" |
| 548 | ); |
| 549 | } |
| 550 | |
| 551 | #[test] |
| 552 | fn extract_name_from_url_with_subdomain() { |
| 553 | assert_eq!( |
| 554 | extract_name_from_url("https://api.example.com/mcp").unwrap(), |
| 555 | "api-example-com-mcp" |
| 556 | ); |
| 557 | } |
| 558 | |
| 559 | #[test] |
| 560 | fn extract_name_from_url_no_path() { |
| 561 | assert_eq!( |
| 562 | extract_name_from_url("https://example.com").unwrap(), |
| 563 | "example-com" |
| 564 | ); |
| 565 | } |
| 566 | |
| 567 | #[test] |
| 568 | fn extract_name_from_url_empty_path() { |
| 569 | assert_eq!( |
| 570 | extract_name_from_url("https://example.com/").unwrap(), |
| 571 | "example-com" |
| 572 | ); |
| 573 | } |
| 574 | |
| 575 | #[test] |
| 576 | fn connect_failure_message_flags_early_exit_with_help_output() { |
| 577 | let err = anyhow::anyhow!( |
| 578 | "Stdio transport closed\nMCP server stderr (last 2 lines):\nUsage: agentic-mermaid [OPTIONS] <COMMAND>" |
| 579 | ); |
| 580 | let msg = connect_failure_message("agentic-mermaid", &err); |
| 581 | assert!(msg.contains("Failed to connect to MCP server 'agentic-mermaid'")); |
| 582 | assert!(msg.contains("exited before completing the MCP handshake")); |
| 583 | assert!(msg.contains("looks like CLI usage help")); |
| 584 | assert!(msg.contains("required_args")); |
| 585 | assert!(msg.contains("next candidate")); |
| 586 | } |
| 587 | |
| 588 | #[test] |
| 589 | fn connect_failure_message_flags_early_exit_without_help_output() { |
| 590 | let err = anyhow::anyhow!("Stdio transport closed"); |
| 591 | let msg = connect_failure_message("x", &err); |
| 592 | assert!(msg.contains("exited before completing the MCP handshake")); |
| 593 | assert!(!msg.contains("usage help")); |
| 594 | assert!(msg.contains("required_args")); |
| 595 | } |
| 596 | |
| 597 | #[test] |
| 598 | fn connect_failure_message_passes_other_errors_through() { |
| 599 | let err = anyhow::anyhow!("connection refused"); |
| 600 | let msg = connect_failure_message("x", &err); |
| 601 | assert_eq!( |
| 602 | msg, |
| 603 | "Failed to connect to MCP server 'x': connection refused" |
| 604 | ); |
| 605 | } |
| 606 | |
| 607 | // === shell_words split tests === |
| 608 | |
| 609 | #[test] |
| 610 | fn shell_words_simple() { |
| 611 | assert_eq!( |
| 612 | shell_words::split("npx server /tmp").unwrap(), |
| 613 | vec!["npx", "server", "/tmp"] |
| 614 | ); |
| 615 | } |
| 616 | |
| 617 | #[test] |
| 618 | fn shell_words_double_quotes() { |
| 619 | assert_eq!( |
| 620 | shell_words::split(r#"npx server --env="MY KEY""#).unwrap(), |
| 621 | vec!["npx", "server", "--env=MY KEY"] |
| 622 | ); |
| 623 | } |
| 624 | |
| 625 | #[test] |
| 626 | fn shell_words_single_quotes() { |
| 627 | assert_eq!( |
| 628 | shell_words::split("npx server --env='MY KEY'").unwrap(), |
| 629 | vec!["npx", "server", "--env=MY KEY"] |
| 630 | ); |
| 631 | } |
| 632 | |
| 633 | #[test] |
| 634 | fn shell_words_mixed_quotes() { |
| 635 | assert_eq!( |
| 636 | shell_words::split(r#"cmd --opt="hello world" --flag 'single'"#).unwrap(), |
| 637 | vec!["cmd", "--opt=hello world", "--flag", "single"] |
| 638 | ); |
| 639 | } |
| 640 | |
| 641 | #[test] |
| 642 | fn shell_words_escaped_quote() { |
| 643 | assert_eq!( |
| 644 | shell_words::split(r#"cmd arg\"with\"quotes"#).unwrap(), |
| 645 | vec!["cmd", r#"arg"with"quotes"#] |
| 646 | ); |
| 647 | } |
| 648 | |
| 649 | #[test] |
| 650 | fn shell_words_empty() { |
| 651 | assert!(shell_words::split("").unwrap().is_empty()); |
| 652 | assert!(shell_words::split(" ").unwrap().is_empty()); |
| 653 | } |
| 654 | |
| 655 | #[test] |
| 656 | fn shell_words_postgres_url() { |
| 657 | assert_eq!( |
| 658 | shell_words::split( |
| 659 | r#"npx -y @modelcontextprotocol/server-postgres "postgresql://user:pass@host/db""# |
| 660 | ) |
| 661 | .unwrap(), |
| 662 | vec![ |
| 663 | "npx", |
| 664 | "-y", |
| 665 | "@modelcontextprotocol/server-postgres", |
| 666 | "postgresql://user:pass@host/db" |
| 667 | ] |
| 668 | ); |
| 669 | } |
| 670 | |
| 671 | #[test] |
| 672 | fn parse_command_with_quoted_args() { |
| 673 | let parsed = |
| 674 | parse_mcp_command(r#"npx @modelcontextprotocol/server-filesystem /tmp --env="MY KEY""#) |
| 675 | .unwrap(); |
| 676 | assert_eq!(parsed.config.command, Some("npx".to_string())); |
| 677 | assert_eq!( |
| 678 | parsed.config.args, |
| 679 | vec![ |
| 680 | "@modelcontextprotocol/server-filesystem", |
| 681 | "/tmp", |
| 682 | "--env=MY KEY" |
| 683 | ] |
| 684 | ); |
| 685 | } |
| 686 | |
| 687 | // === infer_server_name tests === |
| 688 | |
| 689 | #[test] |
| 690 | fn infer_name_npx_package() { |
| 691 | let parsed = parse_mcp_command("npx @modelcontextprotocol/server-filesystem /tmp").unwrap(); |
| 692 | assert_eq!(parsed.name, "filesystem"); |
| 693 | } |
| 694 | |
| 695 | #[test] |
| 696 | fn infer_name_npx_simple() { |
| 697 | let parsed = parse_mcp_command("npx my-mcp-server").unwrap(); |
| 698 | assert_eq!(parsed.name, "my-mcp-server"); |
| 699 | } |
| 700 | |
| 701 | #[test] |
| 702 | fn infer_name_pnpm_exec() { |
| 703 | let parsed = parse_mcp_command("pnpm exec @modelcontextprotocol/server-postgres").unwrap(); |
| 704 | assert_eq!(parsed.name, "postgres"); |
| 705 | } |
| 706 | |
| 707 | #[test] |
| 708 | fn infer_name_node_script() { |
| 709 | let parsed = parse_mcp_command("node ./my-mcp-server.js").unwrap(); |
| 710 | assert_eq!(parsed.name, "my-mcp-server"); |
| 711 | } |
| 712 | |
| 713 | #[test] |
| 714 | fn infer_name_python_script() { |
| 715 | let parsed = parse_mcp_command("python3 mcp_server.py").unwrap(); |
| 716 | assert_eq!(parsed.name, "mcp-server"); |
| 717 | } |
| 718 | |
| 719 | #[test] |
| 720 | fn infer_name_uvx_package() { |
| 721 | let parsed = parse_mcp_command("uvx mcp-server-git").unwrap(); |
| 722 | assert_eq!(parsed.name, "mcp-server-git"); |
| 723 | } |
| 724 | |
| 725 | #[test] |
| 726 | fn infer_name_bare_command() { |
| 727 | let parsed = parse_mcp_command("/usr/local/bin/my-server").unwrap(); |
| 728 | assert_eq!(parsed.name, "my-server"); |
| 729 | } |
| 730 | |
| 731 | #[test] |
| 732 | fn infer_name_windows_cmd_prefix() { |
| 733 | let parsed = |
| 734 | parse_mcp_command("cmd /c npx -y @modelcontextprotocol/server-memory").unwrap(); |
| 735 | assert_eq!(parsed.name, "memory"); |
| 736 | } |
| 737 | |
| 738 | #[test] |
| 739 | fn infer_name_windows_cmd_uppercase() { |
| 740 | let parsed = |
| 741 | parse_mcp_command("cmd /C npx @modelcontextprotocol/server-filesystem /tmp").unwrap(); |
| 742 | assert_eq!(parsed.name, "filesystem"); |
| 743 | } |
| 744 | |
| 745 | #[test] |
| 746 | fn infer_name_only_command_no_args() { |
| 747 | // No args at all — falls through to last resort: command name itself |
| 748 | let parsed = parse_mcp_command("my-server").unwrap(); |
| 749 | assert_eq!(parsed.name, "my-server"); |
| 750 | } |
| 751 | |
| 752 | #[test] |
| 753 | fn infer_name_only_command_no_args_path() { |
| 754 | // Absolute path, no args — uses file_stem of command |
| 755 | let parsed = parse_mcp_command("/usr/local/bin/my-server").unwrap(); |
| 756 | assert_eq!(parsed.name, "my-server"); |
| 757 | } |
| 758 | |
| 759 | // === sanitize_name tests === |
| 760 | |
| 761 | #[test] |
| 762 | fn sanitize_name_preserves_hyphens() { |
| 763 | assert_eq!(sanitize_name("my-server"), "my-server"); |
| 764 | } |
| 765 | |
| 766 | #[test] |
| 767 | fn sanitize_name_converts_underscores_to_hyphens() { |
| 768 | assert_eq!(sanitize_name("my_server"), "my-server"); |
| 769 | } |
| 770 | |
| 771 | #[test] |
| 772 | fn sanitize_name_converts_special_chars_to_hyphens() { |
| 773 | assert_eq!(sanitize_name("my@server!"), "my-server"); |
| 774 | } |
| 775 | |
| 776 | #[test] |
| 777 | fn sanitize_name_trims_leading_trailing_hyphens() { |
| 778 | assert_eq!(sanitize_name("_my_server_"), "my-server"); |
| 779 | } |
| 780 | |
| 781 | #[test] |
| 782 | fn sanitize_name_preserves_alphanumeric() { |
| 783 | assert_eq!(sanitize_name("server123"), "server123"); |
| 784 | } |
| 785 | |
| 786 | #[test] |
| 787 | fn sanitize_name_empty_input() { |
| 788 | assert_eq!(sanitize_name(""), ""); |
| 789 | } |
| 790 | |
| 791 | // === command validation tests === |
| 792 | |
| 793 | #[test] |
| 794 | fn shell_wrappers_and_unlisted_commands_are_refused() { |
| 795 | for (cmd, reason) in [ |
| 796 | ("bash", "Shell wrapper"), |
| 797 | ("sh", "Shell wrapper"), |
| 798 | ("/bin/zsh", "Shell wrapper"), |
| 799 | ("powershell", "Shell wrapper"), |
| 800 | ("PowerShell.exe", "Shell wrapper"), |
| 801 | ("curl", "not in the allowed list"), |
| 802 | ] { |
| 803 | let err = |
| 804 | super::validate_runtime_command(cmd).expect_err(&format!("{cmd} must be refused")); |
| 805 | assert!(err.to_string().contains(reason), "{cmd}: {err}"); |
| 806 | } |
| 807 | // The parsed command, not the raw string, is what gets checked. |
| 808 | let parsed = parse_mcp_command("bash -c 'npx server'").expect("parses"); |
| 809 | let cmd = parsed.config.command.expect("stdio command"); |
| 810 | assert!(super::validate_runtime_command(&cmd).is_err()); |
| 811 | } |
| 812 | |
| 813 | #[test] |
| 814 | fn known_runtimes_pass_the_command_guard() { |
| 815 | for cmd in ["npx", "/usr/local/bin/node", "python3", "uvx", "cargo"] { |
| 816 | assert!(super::validate_runtime_command(cmd).is_ok(), "{cmd}"); |
| 817 | } |
| 818 | } |
| 819 | |
| 820 | /// These used to assert only that their own input string contained the |
| 821 | /// metacharacter — never that the guard refused it — so deleting the |
| 822 | /// defense left all four green (2026-08-04 audit). They now call the |
| 823 | /// guard. |
| 824 | #[test] |
| 825 | fn shell_metacharacters_in_args_are_refused() { |
| 826 | for bad in [ |
| 827 | "--out>file", |
| 828 | "arg|cat", |
| 829 | "a;rm -rf /", |
| 830 | "a&&b", |
| 831 | "`whoami`", |
| 832 | "$HOME", |
| 833 | ] { |
| 834 | let args = vec!["server".to_string(), bad.to_string()]; |
| 835 | let err = super::reject_shell_metacharacters(&args) |
| 836 | .expect_err("metacharacter must be refused: {bad}"); |
| 837 | assert!( |
| 838 | err.to_string().contains("shell metacharacters"), |
| 839 | "refusal must name the reason for {bad}: {err}" |
| 840 | ); |
| 841 | } |
| 842 | } |
| 843 | |
| 844 | #[test] |
| 845 | fn ordinary_args_pass_the_metacharacter_guard() { |
| 846 | let args = vec![ |
| 847 | "@modelcontextprotocol/server-filesystem".to_string(), |
| 848 | "/tmp/workspace".to_string(), |
| 849 | "--read-only".to_string(), |
| 850 | ]; |
| 851 | assert!(super::reject_shell_metacharacters(&args).is_ok()); |
| 852 | } |
| 853 | |
| 854 | // === approval-gate contract === |
| 855 | |
| 856 | #[test] |
| 857 | fn start_mcp_server_declares_required_approval() { |
| 858 | // Security invariant (#3866): spawning a runtime MCP server is |
| 859 | // side-effecting (child process / network connection), so the tool |
| 860 | // spec itself must declare `ApprovalRequirement::Required`. Combined |
| 861 | // with the engine's non-bypassable gate (see engine tests), this |
| 862 | // guarantees an unapproved start is rejected before `execute` runs. |
| 863 | let pool = Arc::new(AsyncMutex::new(McpPool::new( |
| 864 | crate::mcp::McpConfig::default(), |
| 865 | ))); |
| 866 | let tool = StartRuntimeMcpServer::new(pool); |
| 867 | assert_eq!(tool.name(), "start_mcp_server"); |
| 868 | assert!( |
| 869 | matches!(tool.approval_requirement(), ApprovalRequirement::Required), |
| 870 | "start_mcp_server must require approval before spawning" |
| 871 | ); |
| 872 | } |
| 873 | } |
| 874 |