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>
This commit is contained in:
sanyekana
2026-03-08 23:46:32 +08:00
committed by GitHub
parent 537849c1e7
commit 5808784f07
2 changed files with 193 additions and 68 deletions
+185 -68
View File
@@ -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__
@@ -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();