From 5808784f07e936194d5b4c7a204e56fc5ddf926a Mon Sep 17 00:00:00 2001 From: sanyekana Date: Sun, 8 Mar 2026 23:46:32 +0800 Subject: [PATCH] fix: prevent crash on malformed MCP server config (#5666) (#5673) * fix: prevent crash on malformed MCP server config (#5666) * fix: prevent crash on malformed MCP server config (#5666) * fix: validate MCP connection before persisting server config * fix: guard mcpServers type before iterating server list * refactor: use typed empty-config error and extract MCP rollback helper * fix: translate error messages and comments to English for consistency --------- Co-authored-by: Soulter <905617992@qq.com> --- astrbot/dashboard/routes/tools.py | 253 +++++++++++++----- .../extension/McpServersSection.vue | 8 + 2 files changed, 193 insertions(+), 68 deletions(-) diff --git a/astrbot/dashboard/routes/tools.py b/astrbot/dashboard/routes/tools.py index b19385c28..84f8dcc6d 100644 --- a/astrbot/dashboard/routes/tools.py +++ b/astrbot/dashboard/routes/tools.py @@ -12,6 +12,32 @@ from .route import Response, Route, RouteContext DEFAULT_MCP_CONFIG = {"mcpServers": {}} +class EmptyMcpServersError(ValueError): + """Raised when mcpServers is empty.""" + + pass + + +def _extract_mcp_server_config(mcp_servers_value: object) -> dict: + """Extract server configuration from user-submitted mcpServers field. + + Raises: + ValueError: Invalid configuration + """ + if not isinstance(mcp_servers_value, dict): + raise ValueError("mcpServers must be a JSON object") + if not mcp_servers_value: + raise EmptyMcpServersError("mcpServers configuration cannot be empty") + key_0 = next(iter(mcp_servers_value)) + extracted = mcp_servers_value[key_0] + if not isinstance(extracted, dict): + raise ValueError( + "Invalid mcpServers format. Ensure each key in mcpServers is a server name, " + "and each value is an object containing fields like command/url." + ) + return extracted + + class ToolsRoute(Route): def __init__( self, @@ -33,13 +59,37 @@ class ToolsRoute(Route): self.register_routes() self.tool_mgr = self.core_lifecycle.provider_manager.llm_tools + def _rollback_mcp_server(self, name: str) -> bool: + try: + rollback_config = self.tool_mgr.load_mcp_config() + if name in rollback_config["mcpServers"]: + rollback_config["mcpServers"].pop(name) + return self.tool_mgr.save_mcp_config(rollback_config) + return True + except Exception: + logger.error(traceback.format_exc()) + return False + async def get_mcp_servers(self): try: config = self.tool_mgr.load_mcp_config() servers = [] + mcp_servers = config.get("mcpServers", {}) + + if not isinstance(mcp_servers, dict): + logger.warning( + f"Invalid MCP server config type: {type(mcp_servers).__name__}. Expected object/dict; skipped all MCP servers." + ) + mcp_servers = {} # 获取所有服务器并添加它们的工具列表 - for name, server_config in config["mcpServers"].items(): + for name, server_config in mcp_servers.items(): + if not isinstance(server_config, dict): + logger.warning( + f"Invalid config for MCP server '{name}' (type: {type(server_config).__name__}); skipped." + ) + continue + server_info = { "name": name, "active": server_config.get("active", True), @@ -65,7 +115,7 @@ class ToolsRoute(Route): return Response().ok(servers).__dict__ except Exception as e: logger.error(traceback.format_exc()) - return Response().error(f"获取 MCP 服务器列表失败: {e!s}").__dict__ + return Response().error(f"Failed to get MCP server list: {e!s}").__dict__ async def add_mcp_server(self): try: @@ -75,7 +125,7 @@ class ToolsRoute(Route): # 检查必填字段 if not name: - return Response().error("服务器名称不能为空").__dict__ + return Response().error("Server name cannot be empty").__dict__ # 移除特殊字段并检查配置是否有效 has_valid_config = False @@ -85,21 +135,33 @@ class ToolsRoute(Route): for key, value in server_data.items(): if key not in ["name", "active", "tools", "errlogs"]: # 排除特殊字段 if key == "mcpServers": - key_0 = list(server_data["mcpServers"].keys())[ - 0 - ] # 不考虑为空的情况 - server_config = server_data["mcpServers"][key_0] + try: + server_config = _extract_mcp_server_config( + server_data["mcpServers"] + ) + except ValueError as e: + return Response().error(f"{e!s}").__dict__ else: server_config[key] = value has_valid_config = True if not has_valid_config: - return Response().error("必须提供有效的服务器配置").__dict__ + return ( + Response() + .error("A valid server configuration is required") + .__dict__ + ) config = self.tool_mgr.load_mcp_config() if name in config["mcpServers"]: - return Response().error(f"服务器 {name} 已存在").__dict__ + return Response().error(f"Server {name} already exists").__dict__ + + try: + await self.tool_mgr.test_mcp_server_connection(server_config) + except Exception as e: + logger.error(traceback.format_exc()) + return Response().error(f"MCP connection test failed: {e!s}").__dict__ config["mcpServers"][name] = server_config @@ -111,17 +173,27 @@ class ToolsRoute(Route): timeout=30, ) except TimeoutError: - return Response().error(f"启用 MCP 服务器 {name} 超时。").__dict__ + rollback_ok = self._rollback_mcp_server(name) + err_msg = f"Timed out while enabling MCP server {name}." + if not rollback_ok: + err_msg += " Configuration rollback failed. Please check the config manually." + return Response().error(err_msg).__dict__ except Exception as e: logger.error(traceback.format_exc()) - return ( - Response().error(f"启用 MCP 服务器 {name} 失败: {e!s}").__dict__ - ) - return Response().ok(None, f"成功添加 MCP 服务器 {name}").__dict__ - return Response().error("保存配置失败").__dict__ + rollback_ok = self._rollback_mcp_server(name) + err_msg = f"Failed to enable MCP server {name}: {e!s}" + if not rollback_ok: + err_msg += " Configuration rollback failed. Please check the config manually." + return Response().error(err_msg).__dict__ + return ( + Response() + .ok(None, f"Successfully added MCP server {name}") + .__dict__ + ) + return Response().error("Failed to save configuration").__dict__ except Exception as e: logger.error(traceback.format_exc()) - return Response().error(f"添加 MCP 服务器失败: {e!s}").__dict__ + return Response().error(f"Failed to add MCP server: {e!s}").__dict__ async def update_mcp_server(self): try: @@ -131,23 +203,25 @@ class ToolsRoute(Route): old_name = server_data.get("oldName") or name if not name: - return Response().error("服务器名称不能为空").__dict__ + return Response().error("Server name cannot be empty").__dict__ config = self.tool_mgr.load_mcp_config() if old_name not in config["mcpServers"]: - return Response().error(f"服务器 {old_name} 不存在").__dict__ + return Response().error(f"Server {old_name} does not exist").__dict__ is_rename = name != old_name if name in config["mcpServers"] and is_rename: - return Response().error(f"服务器 {name} 已存在").__dict__ + return Response().error(f"Server {name} already exists").__dict__ # 获取活动状态 - active = server_data.get( - "active", - config["mcpServers"][old_name].get("active", True), - ) + old_config = config["mcpServers"][old_name] + if isinstance(old_config, dict): + old_active = old_config.get("active", True) + else: + old_active = True + active = server_data.get("active", old_active) # 创建新的配置对象 server_config = {"active": active} @@ -165,17 +239,19 @@ class ToolsRoute(Route): "oldName", ]: # 排除特殊字段 if key == "mcpServers": - key_0 = list(server_data["mcpServers"].keys())[ - 0 - ] # 不考虑为空的情况 - server_config = server_data["mcpServers"][key_0] + try: + server_config = _extract_mcp_server_config( + server_data["mcpServers"] + ) + except ValueError as e: + return Response().error(f"{e!s}").__dict__ else: server_config[key] = value only_update_active = False # 如果只更新活动状态，保留原始配置 - if only_update_active: - for key, value in config["mcpServers"][old_name].items(): + if only_update_active and isinstance(old_config, dict): + for key, value in old_config.items(): if key != "active": # 除了active之外的所有字段都保留 server_config[key] = value @@ -200,7 +276,7 @@ class ToolsRoute(Route): return ( Response() .error( - f"启用前停用 MCP 服务器时 {old_name} 超时: {e!s}" + f"Timed out while disabling MCP server {old_name} before enabling: {e!s}" ) .__dict__ ) @@ -209,7 +285,7 @@ class ToolsRoute(Route): return ( Response() .error( - f"启用前停用 MCP 服务器时 {old_name} 失败: {e!s}" + f"Failed to disable MCP server {old_name} before enabling: {e!s}" ) .__dict__ ) @@ -221,13 +297,15 @@ class ToolsRoute(Route): ) except TimeoutError: return ( - Response().error(f"启用 MCP 服务器 {name} 超时。").__dict__ + Response() + .error(f"Timed out while enabling MCP server {name}.") + .__dict__ ) except Exception as e: logger.error(traceback.format_exc()) return ( Response() - .error(f"启用 MCP 服务器 {name} 失败: {e!s}") + .error(f"Failed to enable MCP server {name}: {e!s}") .__dict__ ) # 如果要停用服务器 @@ -237,22 +315,26 @@ class ToolsRoute(Route): except TimeoutError: return ( Response() - .error(f"停用 MCP 服务器 {old_name} 超时。") + .error(f"Timed out while disabling MCP server {old_name}.") .__dict__ ) except Exception as e: logger.error(traceback.format_exc()) return ( Response() - .error(f"停用 MCP 服务器 {old_name} 失败: {e!s}") + .error(f"Failed to disable MCP server {old_name}: {e!s}") .__dict__ ) - return Response().ok(None, f"成功更新 MCP 服务器 {name}").__dict__ - return Response().error("保存配置失败").__dict__ + return ( + Response() + .ok(None, f"Successfully updated MCP server {name}") + .__dict__ + ) + return Response().error("Failed to save configuration").__dict__ except Exception as e: logger.error(traceback.format_exc()) - return Response().error(f"更新 MCP 服务器失败: {e!s}").__dict__ + return Response().error(f"Failed to update MCP server: {e!s}").__dict__ async def delete_mcp_server(self): try: @@ -260,12 +342,12 @@ class ToolsRoute(Route): name = server_data.get("name", "") if not name: - return Response().error("服务器名称不能为空").__dict__ + return Response().error("Server name cannot be empty").__dict__ config = self.tool_mgr.load_mcp_config() if name not in config["mcpServers"]: - return Response().error(f"服务器 {name} 不存在").__dict__ + return Response().error(f"Server {name} does not exist").__dict__ del config["mcpServers"][name] @@ -275,51 +357,76 @@ class ToolsRoute(Route): await self.tool_mgr.disable_mcp_server(name, timeout=10) except TimeoutError: return ( - Response().error(f"停用 MCP 服务器 {name} 超时。").__dict__ + Response() + .error(f"Timed out while disabling MCP server {name}.") + .__dict__ ) except Exception as e: logger.error(traceback.format_exc()) return ( Response() - .error(f"停用 MCP 服务器 {name} 失败: {e!s}") + .error(f"Failed to disable MCP server {name}: {e!s}") .__dict__ ) - return Response().ok(None, f"成功删除 MCP 服务器 {name}").__dict__ - return Response().error("保存配置失败").__dict__ + return ( + Response() + .ok(None, f"Successfully deleted MCP server {name}") + .__dict__ + ) + return Response().error("Failed to save configuration").__dict__ except Exception as e: logger.error(traceback.format_exc()) - return Response().error(f"删除 MCP 服务器失败: {e!s}").__dict__ + return Response().error(f"Failed to delete MCP server: {e!s}").__dict__ async def test_mcp_connection(self): - """测试 MCP 服务器连接""" + """Test MCP server connection.""" try: server_data = await request.json config = server_data.get("mcp_server_config", None) if not isinstance(config, dict) or not config: - return Response().error("无效的 MCP 服务器配置").__dict__ + return Response().error("Invalid MCP server configuration").__dict__ if "mcpServers" in config: - keys = list(config["mcpServers"].keys()) - if not keys: - return Response().error("MCP 服务器配置不能为空").__dict__ - if len(keys) > 1: - return Response().error("一次只能配置一个 MCP 服务器配置").__dict__ - config = config["mcpServers"][keys[0]] + mcp_servers = config["mcpServers"] + if isinstance(mcp_servers, dict) and len(mcp_servers) > 1: + return ( + Response() + .error( + "Only one MCP server configuration can be tested at a time" + ) + .__dict__ + ) + try: + config = _extract_mcp_server_config(mcp_servers) + except EmptyMcpServersError: + return ( + Response() + .error("MCP server configuration cannot be empty") + .__dict__ + ) + except ValueError as e: + return Response().error(f"{e!s}").__dict__ elif not config: - return Response().error("MCP 服务器配置不能为空").__dict__ + return ( + Response() + .error("MCP server configuration cannot be empty") + .__dict__ + ) tools_name = await self.tool_mgr.test_mcp_server_connection(config) return ( - Response().ok(data=tools_name, message="🎉 MCP 服务器可用！").__dict__ + Response() + .ok(data=tools_name, message="🎉 MCP server is available!") + .__dict__ ) except Exception as e: logger.error(traceback.format_exc()) - return Response().error(f"测试 MCP 连接失败: {e!s}").__dict__ + return Response().error(f"Failed to test MCP connection: {e!s}").__dict__ async def get_tool_list(self): - """获取所有注册的工具列表""" + """Get all registered tools.""" try: tools = self.tool_mgr.func_list tools_dict = [] @@ -349,36 +456,44 @@ class ToolsRoute(Route): return Response().ok(data=tools_dict).__dict__ except Exception as e: logger.error(traceback.format_exc()) - return Response().error(f"获取工具列表失败: {e!s}").__dict__ + return Response().error(f"Failed to get tool list: {e!s}").__dict__ async def toggle_tool(self): - """启用或停用指定的工具""" + """Activate or deactivate a specified tool.""" try: data = await request.json tool_name = data.get("name") action = data.get("activate") # True or False if not tool_name or action is None: - return Response().error("缺少必要参数: name 或 action").__dict__ + return ( + Response() + .error("Missing required parameters: name or activate") + .__dict__ + ) if action: try: ok = self.tool_mgr.activate_llm_tool(tool_name, star_map=star_map) except ValueError as e: - return Response().error(f"启用工具失败: {e!s}").__dict__ + return Response().error(f"Failed to activate tool: {e!s}").__dict__ else: ok = self.tool_mgr.deactivate_llm_tool(tool_name) if ok: - return Response().ok(None, "操作成功。").__dict__ - return Response().error(f"工具 {tool_name} 不存在或操作失败。").__dict__ + return Response().ok(None, "Operation successful.").__dict__ + return ( + Response() + .error(f"Tool {tool_name} does not exist or the operation failed.") + .__dict__ + ) except Exception as e: logger.error(traceback.format_exc()) - return Response().error(f"操作工具失败: {e!s}").__dict__ + return Response().error(f"Failed to operate tool: {e!s}").__dict__ async def sync_provider(self): - """同步 MCP 提供者配置""" + """Sync MCP provider configuration.""" try: data = await request.json provider_name = data.get("name") # modelscope, or others @@ -387,9 +502,11 @@ class ToolsRoute(Route): access_token = data.get("access_token", "") await self.tool_mgr.sync_modelscope_mcp_servers(access_token) case _: - return Response().error(f"未知: {provider_name}").__dict__ + return ( + Response().error(f"Unknown provider: {provider_name}").__dict__ + ) - return Response().ok(message="同步成功").__dict__ + return Response().ok(message="Sync completed").__dict__ except Exception as e: logger.error(traceback.format_exc()) - return Response().error(f"同步失败: {e!s}").__dict__ + return Response().error(f"Sync failed: {e!s}").__dict__ diff --git a/dashboard/src/components/extension/McpServersSection.vue b/dashboard/src/components/extension/McpServersSection.vue index 95b679580..d24bcec58 100644 --- a/dashboard/src/components/extension/McpServersSection.vue +++ b/dashboard/src/components/extension/McpServersSection.vue @@ -300,6 +300,10 @@ export default { this.loadingGettingServers = true; axios.get('/api/tools/mcp/servers') .then(response => { + if (response.data.status === 'error') { + this.showError(response.data.message || this.tm('messages.getServersError', { error: 'Unknown error' })); + return; + } this.mcpServers = response.data.data || []; this.mcpServers.forEach(server => { if (!this.mcpServerUpdateLoaders[server.name]) { @@ -372,6 +376,10 @@ export default { axios.post(endpoint, serverData) .then(response => { this.loading = false; + if (response.data.status === 'error') { + this.showError(response.data.message || this.tm('messages.saveError', { error: 'Unknown error' })); + return; + } this.showMcpServerDialog = false; this.addServerDialogMessage = ''; this.getServers();