diff --git a/backend/packages/harness/deerflow/mcp/tools.py b/backend/packages/harness/deerflow/mcp/tools.py index 0d24aa563a..e04e473ee6 100644 --- a/backend/packages/harness/deerflow/mcp/tools.py +++ b/backend/packages/harness/deerflow/mcp/tools.py @@ -1086,9 +1086,14 @@ async def _discover_server_tools( except (asyncio.CancelledError, StaleMCPBindingError, McpTaskConfigurationError): raise except Exception as e: + # Never interpolate ``e`` nor attach its traceback: ``ExtensionsConfig`` + # resolves ``$VAR`` before validation, so a discovery error message can + # embed a resolved-credential request URL. Keep the server name and the + # exception type for diagnosis. logger.warning( - f"Skipping MCP server '{server_name}' after tool discovery failed: {e}", - exc_info=True, + "Skipping MCP server '%s' after tool discovery failed (%s)", + server_name, + type(e).__name__, ) return server_name, None @@ -1432,5 +1437,7 @@ async def get_mcp_tools( except (McpTaskConfigurationError, StaleMCPBindingError): raise except Exception as e: - logger.error(f"Failed to load MCP tools: {e}", exc_info=True) + # Only the exception type is logged: the message/traceback can carry a + # resolved-credential request URL (see ``_discover_server_tools``). + logger.error("Failed to load MCP tools (%s)", type(e).__name__) return [] diff --git a/backend/tests/test_mcp_per_server_discovery.py b/backend/tests/test_mcp_per_server_discovery.py index 1f0ed69e28..edefa792bb 100644 --- a/backend/tests/test_mcp_per_server_discovery.py +++ b/backend/tests/test_mcp_per_server_discovery.py @@ -7,6 +7,7 @@ import builtins import logging from unittest.mock import AsyncMock, MagicMock +import httpx import pytest from langchain_core.tools import StructuredTool @@ -711,3 +712,37 @@ async def test_discovery_and_wrapper_use_the_same_session_init_timeout(monkeypat assert len(groups["A"].tools) == 1 assert observed["discovery"] == 5.0 assert observed["wrapper"] == observed["discovery"] + + +@pytest.mark.asyncio +async def test_per_server_discovery_failure_log_redacts_resolved_credentials(monkeypatch, caplog): + """A failed per-server discovery must not log a credential-bearing URL.""" + from langchain_mcp_adapters import client as adapter_client + + secret = "s3cr3t-pass-123" + config = _config({"A": {"enabled": True, "type": "http", "url": "https://alice@remote.test/mcp"}}) + _patch_common(monkeypatch) + + class FakeClient: + def __init__(self, connections, **kwargs) -> None: + self.callbacks = kwargs.get("callbacks") or [] + self.tool_interceptors = kwargs.get("tool_interceptors") or [] + self.tool_name_prefix = kwargs.get("tool_name_prefix", True) + + async def get_tools(self, *, server_name=None): + request = httpx.Request("GET", f"https://alice:{secret}@remote.test/mcp") + raise httpx.HTTPStatusError( + f"Client error for url '{request.url}'", + request=request, + response=httpx.Response(401, request=request), + ) + + monkeypatch.setattr(adapter_client, "MultiServerMCPClient", FakeClient) + + with caplog.at_level("WARNING", logger="deerflow.mcp.tools"): + result = await get_mcp_tools_by_server(config, server_names={"A"}) + + assert result == {} + assert secret not in caplog.text # the resolved credential never reaches the log + assert "server 'A'" in caplog.text # the offending server name is retained + assert "HTTPStatusError" in caplog.text # exception type retained diff --git a/backend/tests/test_mcp_sync_wrapper.py b/backend/tests/test_mcp_sync_wrapper.py index 30197b8219..e729f962bf 100644 --- a/backend/tests/test_mcp_sync_wrapper.py +++ b/backend/tests/test_mcp_sync_wrapper.py @@ -130,7 +130,11 @@ def test_mcp_tool_loading_skips_failed_server(): assert [tool.name for tool in tools] == ["good-server_search"] assert tools[0].func is not None mock_warning.assert_called_once() - assert "bad-server" in mock_warning.call_args[0][0] + args, kwargs = mock_warning.call_args + assert args[1] == "bad-server" + assert args[2] == "RuntimeError" + assert "server 'bad-server'" in args[0] % args[1:] + assert kwargs.get("exc_info") is None def test_mcp_tool_sync_wrapper_in_running_loop():