diff --git a/src/channels/wasm/setup.rs b/src/channels/wasm/setup.rs index 64e559f450..83cde31afa 100644 --- a/src/channels/wasm/setup.rs +++ b/src/channels/wasm/setup.rs @@ -141,6 +141,7 @@ pub async fn setup_wasm_channels( }) } +#[allow(clippy::too_many_arguments)] async fn register_startup_channels( loaded_channels: Vec, config: &Config, diff --git a/src/channels/web/server.rs b/src/channels/web/server.rs index 3c4f0ed19d..c57cf03c0c 100644 --- a/src/channels/web/server.rs +++ b/src/channels/web/server.rs @@ -2896,6 +2896,20 @@ async fn pending_gate_extension_name( ); } + if matches!( + tool_name, + "tool_install" + | "tool-install" + | "tool_activate" + | "tool-activate" + | "tool_auth" + | "tool-auth" + ) && let Some(name) = parsed_parameters.get("name").and_then(|v| v.as_str()) + && !name.trim().is_empty() + { + return Some(name.to_string()); + } + if let Some(tools) = state.tool_registry.as_ref() && let Some(name) = tools.provider_extension_for_tool(tool_name).await { diff --git a/src/tools/mcp/config.rs b/src/tools/mcp/config.rs index 011a12842b..94b08a71ef 100644 --- a/src/tools/mcp/config.rs +++ b/src/tools/mcp/config.rs @@ -165,6 +165,27 @@ impl McpServerConfig { }); } + // Allowlist: alphanumeric, dash, underscore. + // Rejects shell metacharacters (;|&`$), path separators (/\), + // dots (LLM providers require tool names match ^[a-zA-Z0-9_-]+$ + // and server names are used as tool name prefixes), null bytes, + // spaces, and other dangerous characters that could cause injection + // when names are interpolated into secret keys, tool name prefixes, + // or provider tags. + if !self + .name + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_') + { + return Err(ConfigError::InvalidConfig { + reason: format!( + "Server name '{}' contains invalid characters \ + (only alphanumeric, dash, underscore are allowed)", + self.name + ), + }); + } + match self.effective_transport() { EffectiveTransport::Http => { if self.url.is_empty() { @@ -543,14 +564,23 @@ pub async fn load_mcp_servers_from(path: impl AsRef) -> Result Result { match store.get_setting(user_id, "mcp_servers").await { Ok(Some(value)) => { - let config: McpServersFile = serde_json::from_value(value)?; - // Validate every server on load so corrupted DB configs are caught early - for server in &config.servers { - server.validate().map_err(|e| ConfigError::InvalidConfig { - reason: format!("Server '{}': {}", server.name, e), - })?; - } + let mut config: McpServersFile = serde_json::from_value(value)?; + // Validate every server on load. Invalid entries are skipped + // with a warning to avoid breaking all MCP integrations when + // legacy names don't pass tightened validation. + config.servers.retain(|server| { + if let Err(e) = server.validate() { + tracing::warn!( + server_name = %server.name, + "Skipping MCP server with invalid DB config: {e}" + ); + false + } else { + true + } + }); Ok(config) } Ok(None) => { @@ -891,31 +929,36 @@ mod tests { } #[tokio::test] - async fn test_load_rejects_corrupted_headers() { + async fn test_load_skips_corrupted_headers() { let dir = tempdir().unwrap(); let path = dir.path().join("mcp-servers.json"); // Write a config with an invalid header name directly to disk, // bypassing the add_mcp_server() validation path. let corrupted = serde_json::json!({ - "servers": [{ - "name": "bad-server", - "url": "https://mcp.example.com", - "enabled": true, - "headers": { "X Bad": "value" } - }] + "servers": [ + { + "name": "bad-server", + "url": "https://mcp.example.com", + "enabled": true, + "headers": { "X Bad": "value" } + }, + { + "name": "good-server", + "url": "https://mcp.good.com", + "enabled": true, + "headers": {} + } + ] }); tokio::fs::write(&path, corrupted.to_string()) .await .unwrap(); - let result = load_mcp_servers_from(&path).await; - assert!(result.is_err(), "Load should reject corrupted headers"); - let err = result.unwrap_err().to_string(); - assert!( - err.contains("bad-server"), - "Error should name the offending server, got: {err}" - ); + // Invalid entries are skipped, valid ones are kept + let result = load_mcp_servers_from(&path).await.unwrap(); + assert_eq!(result.servers.len(), 1); + assert_eq!(result.servers[0].name, "good-server"); } #[test] @@ -1452,6 +1495,134 @@ mod tests { } } + #[test] + fn test_server_name_valid_characters_accepted() { + // Alphanumeric, dashes, and underscores are all valid + for name in ["notion", "my-server", "my_server", "MCP-1"] { + let config = McpServerConfig::new(name, "https://mcp.example.com"); + assert!( + config.validate().is_ok(), + "Name '{}' should be accepted", + name + ); + } + } + + #[test] + fn test_server_name_shell_metacharacters_rejected() { + let dangerous_names = [ + "server; rm -rf /", + "server$(whoami)", + "server`id`", + "server|cat /etc/passwd", + "server&bg", + "server>out", + "server