From 030d4caa0e48b52ddf59ce675ce0020e8720aaee Mon Sep 17 00:00:00 2001 From: KoNit-K <124019182+KoNit-K@users.noreply.github.com> Date: Tue, 15 Sep 2026 22:18:33 +0800 Subject: [PATCH] fix(mcp): bound nested connection error traversal --- tests/tools/test_mcp_tool_issue_948.py | 22 ++++++++++ tools/mcp_tool_errors.py | 59 +++++++++++++++++++------- 2 files changed, 66 insertions(+), 15 deletions(-) diff --git a/tests/tools/test_mcp_tool_issue_948.py b/tests/tools/test_mcp_tool_issue_948.py index 230668b4a1..d31c02be7c 100644 --- a/tests/tools/test_mcp_tool_issue_948.py +++ b/tests/tools/test_mcp_tool_issue_948.py @@ -20,6 +20,28 @@ if not _MCP_AVAILABLE: _mcp_mod.ClientSession = MagicMock +def test_format_connect_error_finds_missing_executable_in_deep_exception_chain(): + """A deeply wrapped missing binary must not recurse while formatting it.""" + missing = FileNotFoundError(2, "No such file or directory", "/opt/bin/removed-mcp") + current = missing + for _ in range(sys.getrecursionlimit() + 10): + wrapper = RuntimeError("stdio startup failed") + wrapper.__cause__ = current + current = wrapper + + assert _format_connect_error(current) == "missing executable '/opt/bin/removed-mcp'" + + +def test_format_connect_error_handles_cyclic_exception_chain(): + """Malformed exception chains must fall back to a finite, sanitized message.""" + first = RuntimeError("first failure") + second = RuntimeError("second failure") + first.__cause__ = second + second.__cause__ = first + + assert _format_connect_error(first) == "first failure; second failure" + + def test_resolve_stdio_command_falls_back_to_hermes_node_bin(tmp_path): node_bin = tmp_path / "node" / "bin" node_bin.mkdir(parents=True) diff --git a/tools/mcp_tool_errors.py b/tools/mcp_tool_errors.py index 13167317e5..c0cb5b299f 100644 --- a/tools/mcp_tool_errors.py +++ b/tools/mcp_tool_errors.py @@ -307,23 +307,52 @@ def _exc_children(exc: BaseException) -> List[BaseException]: def _format_connect_error(exc: BaseException) -> str: """Render nested MCP connection errors into an actionable short message.""" - def _find_missing(current: BaseException) -> Optional[str]: - if isinstance(current, FileNotFoundError): - if getattr(current, "filename", None): - return str(current.filename) - match = re.search(r"No such file or directory: '([^']+)'", str(current)) - if match: - return match.group(1) - return next(filter(None, map(_find_missing, _exc_children(current))), None) + def _find_missing() -> Optional[str]: + """Find a missing executable without recursing through malformed chains.""" + stack = [exc] + seen: set[int] = set() + budget = _EXC_TRAVERSAL_MAX_NODES + while stack and budget > 0: + current = stack.pop() + if id(current) in seen: + continue + seen.add(id(current)) + budget -= 1 + if isinstance(current, FileNotFoundError): + if getattr(current, "filename", None): + return str(current.filename) + match = re.search(r"No such file or directory: '([^']+)'", str(current)) + if match: + return match.group(1) + # Reverse preserves the former left-to-right depth-first order. + stack.extend(reversed(_exc_children(current))) + return None - def _flatten_messages(current: BaseException) -> List[str]: - # A group's own str() is opaque — only its children speak. - text = "" if getattr(current, "exceptions", None) else str(current).strip() - messages = ([text] if text else []) + [m for child in _exc_children(current) for m in _flatten_messages(child)] - return messages or [current.__class__.__name__] - missing = _find_missing(exc) + def _flatten_messages() -> List[str]: + """Collect a short, cycle-safe rendering of an exception chain.""" + stack = [exc] + seen: set[int] = set() + messages: List[str] = [] + budget = _EXC_TRAVERSAL_MAX_NODES + while stack and budget > 0: + current = stack.pop() + if id(current) in seen: + continue + seen.add(id(current)) + budget -= 1 + children = _exc_children(current) + # A group's own str() is opaque — only its children speak. + text = "" if getattr(current, "exceptions", None) else str(current).strip() + if text: + messages.append(text) + elif not children: + messages.append(current.__class__.__name__) + stack.extend(reversed(children)) + return messages or [exc.__class__.__name__] + + missing = _find_missing() if not missing: - return _sanitize_error("; ".join(list(dict.fromkeys(_flatten_messages(exc)))[:3])) + return _sanitize_error("; ".join(list(dict.fromkeys(_flatten_messages()))[:3])) message = f"missing executable '{missing}'" if os.path.basename(missing) in {"npx", "npm", "node"}: message += (" (ensure Node.js is installed and PATH includes its bin directory, "