diff --git a/astrbot/core/agent/mcp_client.py b/astrbot/core/agent/mcp_client.py index aae1aaa77..fcad60d0a 100644 --- a/astrbot/core/agent/mcp_client.py +++ b/astrbot/core/agent/mcp_client.py @@ -1,9 +1,11 @@ import asyncio import logging import os +import re import sys from contextlib import AsyncExitStack from datetime import timedelta +from pathlib import Path, PureWindowsPath from typing import Generic from tenacity import ( @@ -21,6 +23,75 @@ from astrbot.core.utils.log_pipe import LogPipe from .run_context import TContext from .tool import FunctionTool +_DEFAULT_STDIO_COMMAND_ALLOWLIST = frozenset( + { + "python", + "python3", + "py", + "node", + "npx", + "npm", + "pnpm", + "yarn", + "bun", + "bunx", + "deno", + "uv", + "uvx", + } +) +_DENIED_STDIO_COMMANDS = frozenset( + { + "bash", + "sh", + "zsh", + "fish", + "cmd", + "cmd.exe", + "powershell", + "powershell.exe", + "pwsh", + "pwsh.exe", + "osascript", + "open", + "curl", + "wget", + "nc", + "netcat", + "telnet", + "ssh", + "scp", + "rm", + "mv", + "cp", + "dd", + "mkfs", + "sudo", + "su", + "chmod", + "chown", + "kill", + "killall", + "shutdown", + "reboot", + "poweroff", + "halt", + } +) +_SHELL_META_RE = re.compile(r"[\r\n\x00;&|<>`$()]") +_PYTHON_INLINE_CODE_FLAGS = frozenset({"-c"}) +_JS_INLINE_CODE_FLAGS = frozenset({"-e", "--eval", "-p", "--print"}) +_DENIED_DOCKER_ARGS = frozenset( + { + "--privileged", + "--pid=host", + "--network=host", + "--net=host", + "--ipc=host", + } +) +_STDIO_ALLOWLIST_ENV = "ASTRBOT_MCP_STDIO_ALLOWED_COMMANDS" + try: import anyio import mcp @@ -42,11 +113,107 @@ def _prepare_config(config: dict) -> dict: """Prepare configuration, handle nested format""" if config.get("mcpServers"): first_key = next(iter(config["mcpServers"])) - config = config["mcpServers"][first_key] + config = dict(config["mcpServers"][first_key]) + else: + config = dict(config) config.pop("active", None) return config +def _normalize_stdio_command_name(command: str) -> str: + command = command.strip() + if "\\" in command: + command_name = PureWindowsPath(command).name + else: + command_name = Path(command).name + command_name = command_name.lower() + for suffix in (".exe", ".cmd", ".bat"): + if command_name.endswith(suffix): + return command_name[: -len(suffix)] + return command_name + + +def _get_stdio_command_allowlist() -> set[str]: + allowed = set(_DEFAULT_STDIO_COMMAND_ALLOWLIST) + configured = os.environ.get(_STDIO_ALLOWLIST_ENV, "") + if configured.strip(): + allowed = { + _normalize_stdio_command_name(item) + for item in configured.split(",") + if item.strip() + } + return allowed + + +def _is_stdio_config(config: dict) -> bool: + cfg = _prepare_config(config.copy()) + return "url" not in cfg + + +def _validate_stdio_args(command_name: str, args: object) -> None: + if args is None: + return + if not isinstance(args, list) or not all(isinstance(arg, str) for arg in args): + raise ValueError("MCP stdio args must be a list of strings.") + + for arg in args: + if "\x00" in arg or "\r" in arg or "\n" in arg: + raise ValueError("MCP stdio args cannot contain control characters.") + + if command_name in {"python", "python3", "py"}: + if any(arg in _PYTHON_INLINE_CODE_FLAGS for arg in args): + raise ValueError( + "MCP stdio Python servers must be launched from a module or file; inline code flags such as `-c` are not allowed." + ) + elif command_name in {"node", "deno", "bun"}: + if any(arg in _JS_INLINE_CODE_FLAGS for arg in args): + raise ValueError( + "MCP stdio JavaScript servers must be launched from a package or file; inline eval flags are not allowed." + ) + elif command_name == "docker": + denied = [arg for arg in args if arg in _DENIED_DOCKER_ARGS] + if denied: + raise ValueError( + f"MCP stdio Docker args are unsafe and not allowed: {', '.join(denied)}." + ) + + +def validate_mcp_stdio_config(config: dict) -> None: + """Validate stdio MCP config before any subprocess can be spawned.""" + cfg = _prepare_config(config.copy()) + if "url" in cfg: + return + + command = cfg.get("command") + if not isinstance(command, str) or not command.strip(): + raise ValueError("MCP stdio server requires a non-empty command.") + if _SHELL_META_RE.search(command): + raise ValueError("MCP stdio command contains unsafe shell metacharacters.") + + command_name = _normalize_stdio_command_name(command) + if command_name in _DENIED_STDIO_COMMANDS: + raise ValueError(f"MCP stdio command `{command_name}` is not allowed.") + + allowed = _get_stdio_command_allowlist() + if command_name not in allowed: + allowed_display = ", ".join(sorted(allowed)) + raise ValueError( + f"MCP stdio command `{command_name}` is not allowed. " + f"Allowed commands: {allowed_display}. " + f"Set {_STDIO_ALLOWLIST_ENV} to override this list if you trust another launcher." + ) + + _validate_stdio_args(command_name, cfg.get("args")) + + env = cfg.get("env") + if env is not None and not isinstance(env, dict): + raise ValueError("MCP stdio env must be an object.") + if isinstance(env, dict) and not all( + isinstance(key, str) and isinstance(value, str) for key, value in env.items() + ): + raise ValueError("MCP stdio env keys and values must be strings.") + + def _prepare_stdio_env(config: dict) -> dict: """Preserve Windows executable resolution for stdio subprocesses.""" if sys.platform != "win32": @@ -243,6 +410,7 @@ class MCPClient: ) else: + validate_mcp_stdio_config(cfg) cfg = _prepare_stdio_env(cfg) server_params = mcp.StdioServerParameters( **cfg, diff --git a/astrbot/dashboard/routes/tools.py b/astrbot/dashboard/routes/tools.py index 7273c5176..157b4d75b 100644 --- a/astrbot/dashboard/routes/tools.py +++ b/astrbot/dashboard/routes/tools.py @@ -3,7 +3,7 @@ import traceback from quart import request from astrbot.core import logger -from astrbot.core.agent.mcp_client import MCPTool +from astrbot.core.agent.mcp_client import MCPTool, validate_mcp_stdio_config from astrbot.core.core_lifecycle import AstrBotCoreLifecycle from astrbot.core.star import star_map from astrbot.core.tools.registry import get_builtin_tool_config_statuses @@ -153,6 +153,11 @@ class ToolsRoute(Route): .__dict__ ) + try: + validate_mcp_stdio_config(server_config) + except ValueError as e: + return Response().error(f"{e!s}").__dict__ + config = self.tool_mgr.load_mcp_config() if name in config["mcpServers"]: @@ -256,6 +261,11 @@ class ToolsRoute(Route): if key != "active": # 除了active之外的所有字段都保留 server_config[key] = value + try: + validate_mcp_stdio_config(server_config) + except ValueError as e: + return Response().error(f"{e!s}").__dict__ + # config["mcpServers"][name] = server_config if is_rename: config["mcpServers"].pop(old_name) @@ -415,6 +425,11 @@ class ToolsRoute(Route): .__dict__ ) + try: + validate_mcp_stdio_config(config) + except ValueError as e: + return Response().error(f"{e!s}").__dict__ + tools_name = await self.tool_mgr.test_mcp_server_connection(config) return ( Response() diff --git a/dashboard/src/components/extension/McpServersSection.vue b/dashboard/src/components/extension/McpServersSection.vue index ae42d8ca6..8784e8511 100644 --- a/dashboard/src/components/extension/McpServersSection.vue +++ b/dashboard/src/components/extension/McpServersSection.vue @@ -110,6 +110,10 @@ *{{ tm('dialogs.addServer.tips.timeoutConfig') }} + + {{ tm('dialogs.addServer.tips.transportRecommendation') }} + +