fix(adk): stop leaking Authorization headers in MCP tool error responses

python/packages/kagent-adk/src/kagent/adk/_mcp_toolset.py

@@ -71,6 +71,28 @@ def _enrich_cancelled_error(error: BaseException) -> asyncio.CancelledError:
     return asyncio.CancelledError(message)
 
 
+def _extract_http_status(error: BaseException) -> Optional[int]:
+    """Return the HTTP status code carried by an httpx.HTTPStatusError found
+    anywhere in the exception's chain or group, without stringifying the error
+    (which could leak Authorization headers). Returns None if absent."""
+    seen: set[int] = set()
+    stack: list[BaseException] = [error]
+    while stack:
+        current = stack.pop()
+        if id(current) in seen:
+            continue
+        seen.add(id(current))
+        if isinstance(current, httpx.HTTPStatusError):
+            return current.response.status_code
+        if isinstance(current, BaseExceptionGroup):
+            stack.extend(current.exceptions)
+        if current.__cause__ is not None:
+            stack.append(current.__cause__)
+        if current.__context__ is not None:
+            stack.append(current.__context__)
+    return None
+
+
 class ConnectionSafeMcpTool(McpTool):
     """McpTool wrapper that catches connection errors and returns them as
     error text to the LLM instead of raising.
@@ -96,14 +118,30 @@ def __init__(self, inner_tool: McpTool):
     def __getattr__(self, name: str) -> Any:
         return getattr(self._inner_tool, name)
 
-    def _connection_error_response(self, error: Exception) -> dict[str, Any]:
+    def _connection_error_response(self, error: BaseException) -> dict[str, Any]:
         error_message = (
-            f"MCP tool '{self.name}' failed due to a connection error: "
-            f"{type(error).__name__}: {error}. "
+            f"MCP tool '{self.name}' failed due to a connection error "
+            f"({type(error).__name__}). "
             "The MCP server may be unreachable. "
             "Do not retry this tool — inform the user about the failure."
         )
-        logger.error(error_message, exc_info=error)
+        logger.error(
+            "MCP tool '%s' failed due to a connection error (%s)",
+            self.name,
+            type(error).__name__,
+        )
+        return {"error": error_message}
+
+    def _http_status_error_response(self, status_code: int) -> dict[str, Any]:
+        error_message = (
+            f"MCP tool '{self.name}' failed because the upstream returned HTTP "
+            f"{status_code}. Do not retry this tool — inform the user about the failure."
+        )
+        logger.warning(
+            "MCP tool '%s' received upstream HTTP status %s",
+            self.name,
+            status_code,
+        )
         return {"error": error_message}
 
     async def run_async(
@@ -116,10 +154,20 @@ async def run_async(
             return await self._inner_tool.run_async(args=args, tool_context=tool_context)
         except _CONNECTION_ERROR_TYPES as error:
             return self._connection_error_response(error)
+        except httpx.HTTPStatusError as error:
+            return self._http_status_error_response(error.response.status_code)
         except McpError as error:
+            status_code = _extract_http_status(error)
+            if status_code is not None:
+                return self._http_status_error_response(status_code)
             if not _is_transport_mcp_error(error):
                 raise
             return self._connection_error_response(error)
+        except BaseExceptionGroup as error:
+            status_code = _extract_http_status(error)
+            if status_code is not None:
+                return self._http_status_error_response(status_code)
+            raise
 
 
 class KAgentMcpToolset(McpToolset):

python/packages/kagent-adk/tests/unittests/test_mcp_connection_error_handling.py

@@ -49,7 +49,7 @@ async def test_connection_reset_error_returns_error_dict():
 
     assert "error" in result
     assert "ConnectionResetError" in result["error"]
-    assert "Connection reset by peer" in result["error"]
+    assert "Connection reset by peer" not in result["error"]
     assert "Do not retry" in result["error"]
 
 
@@ -117,7 +117,7 @@ async def test_transport_mcp_error_returns_error_dict():
 
     assert "error" in result
     assert "McpError" in result["error"]
-    assert "session read timeout" in result["error"]
+    assert "session read timeout" not in result["error"]
 
 
 @pytest.mark.asyncio
@@ -197,3 +197,103 @@ async def mock_super_get_tools(self_arg, readonly_context=None):
     # Model-visible app tool is recorded; app-only tool is not.
     assert "get_weather" in app_tool_names
     assert "refresh_dashboard" not in app_tool_names
+
+
+@pytest.mark.asyncio
+async def test_http_status_error_returns_sanitized_error_dict_and_logs_warning():
+    """HTTP status failures are returned without leaking request metadata."""
+    credential = "super-secret-token"
+    request = httpx.Request(
+        "POST",
+        "http://x",
+        headers={"Authorization": f"Bearer {credential}"},
+    )
+    response = httpx.Response(401, request=request)
+    tool = _make_connection_safe_tool(
+        httpx.HTTPStatusError("Unauthorized", request=request, response=response)
+    )
+
+    with patch("kagent.adk._mcp_toolset.logger") as mock_logger:
+        result = await tool.run_async(args={}, tool_context=MagicMock())
+
+    assert isinstance(result, dict)
+    assert "error" in result
+    assert "401" in result["error"]
+    assert credential not in result["error"]
+    assert "Authorization" not in result["error"]
+    mock_logger.warning.assert_called_once()
+    assert credential not in str(mock_logger.warning.call_args)
+    assert "Authorization" not in str(mock_logger.warning.call_args)
+
+
+@pytest.mark.asyncio
+async def test_mcp_error_wrapping_http_status_returns_sanitized_dict():
+    """An McpError whose cause is an httpx.HTTPStatusError is reported as an HTTP
+    status failure without leaking the Authorization header."""
+    credential = "super-secret-token"
+    request = httpx.Request(
+        "POST",
+        "http://x",
+        headers={"Authorization": f"Bearer {credential}"},
+    )
+    response = httpx.Response(403, request=request)
+    http_error = httpx.HTTPStatusError("Forbidden", request=request, response=response)
+    mcp_error = McpError(ErrorData(code=-1, message="upstream error"))
+    mcp_error.__cause__ = http_error
+    tool = _make_connection_safe_tool(mcp_error)
+
+    result = await tool.run_async(args={}, tool_context=MagicMock())
+
+    assert "error" in result
+    assert "403" in result["error"]
+    assert credential not in result["error"]
+    assert "Authorization" not in result["error"]
+
+
+@pytest.mark.asyncio
+async def test_exception_group_wrapping_http_status_returns_sanitized_dict():
+    """An ExceptionGroup carrying an httpx.HTTPStatusError is reported as an HTTP
+    status failure without leaking the Authorization header."""
+    credential = "super-secret-token"
+    request = httpx.Request(
+        "POST",
+        "http://x",
+        headers={"Authorization": f"Bearer {credential}"},
+    )
+    response = httpx.Response(502, request=request)
+    http_error = httpx.HTTPStatusError("Bad Gateway", request=request, response=response)
+    group = ExceptionGroup("mcp session failed", [http_error])
+    tool = _make_connection_safe_tool(group)
+
+    result = await tool.run_async(args={}, tool_context=MagicMock())
+
+    assert "error" in result
+    assert "502" in result["error"]
+    assert credential not in result["error"]
+    assert "Authorization" not in result["error"]
+
+
+@pytest.mark.asyncio
+async def test_connection_error_response_does_not_leak_wrapped_credentials():
+    """A connection error carrying an httpx request with an Authorization header
+    must not leak that header via str(error) or exc_info."""
+    credential = "super-secret-token"
+    request = httpx.Request(
+        "GET",
+        "http://x",
+        headers={"Authorization": f"Bearer {credential}"},
+    )
+    error = httpx.ConnectError("connection refused", request=request)
+
+    tool = _make_connection_safe_tool(error)
+    with patch("kagent.adk._mcp_toolset.logger") as mock_logger:
+        result = await tool.run_async(args={}, tool_context=MagicMock())
+
+    assert "error" in result
+    assert "ConnectError" in result["error"]
+    assert credential not in result["error"]
+    assert "Authorization" not in result["error"]
+    mock_logger.error.assert_called_once()
+    assert mock_logger.error.call_args.kwargs.get("exc_info") is None
+    assert credential not in str(mock_logger.error.call_args)
+    assert "Authorization" not in str(mock_logger.error.call_args)