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