From 14f104be28ddc6e21afc0afe9e8a4f2992fada7b Mon Sep 17 00:00:00 2001 From: CorgiBoyG <111257566+CorgiBoyG@users.noreply.github.com> Date: Wed, 21 Oct 2026 14:49:20 +0800 Subject: [PATCH] fix(mcp): keep empty OAuth2 token on its own branch in McpTool McpTool._get_headers used `if credential.oauth2 and credential.oauth2.access_token`, so an OAuth2 credential with an empty token fell through to the `elif credential.http` branch and emitted an Authorization header from an unrelated HTTP field on the same object. The discovery helper build_auth_headers keeps the credential on the OAuth2 branch and emits no header for an empty token. Restore the nested structure here so both paths resolve the same credential to the same header and therefore the same pooled MCP session. Fixes #7646 --- src/google/adk/tools/mcp_tool/mcp_tool.py | 10 +++- .../unittests/tools/mcp_tool/test_mcp_tool.py | 47 +++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/google/adk/tools/mcp_tool/mcp_tool.py b/src/google/adk/tools/mcp_tool/mcp_tool.py index ef147c4a91e..330703db9b0 100644 --- a/src/google/adk/tools/mcp_tool/mcp_tool.py +++ b/src/google/adk/tools/mcp_tool/mcp_tool.py @@ -859,8 +859,14 @@ async def _get_headers( """ headers: dict[str, str] | None = None if credential: - if credential.oauth2 and credential.oauth2.access_token: - headers = {"Authorization": f"Bearer {credential.oauth2.access_token}"} + if credential.oauth2: + # Mirror build_auth_headers: an OAuth2 credential stays on this branch + # even without a token, so an empty token yields no header rather than + # falling through to an unrelated HTTP credential on the same object. + if credential.oauth2.access_token: + headers = { + "Authorization": f"Bearer {credential.oauth2.access_token}" + } elif credential.http: # Handle HTTP authentication schemes if ( diff --git a/tests/unittests/tools/mcp_tool/test_mcp_tool.py b/tests/unittests/tools/mcp_tool/test_mcp_tool.py index 7775146cba3..f840a3969f4 100644 --- a/tests/unittests/tools/mcp_tool/test_mcp_tool.py +++ b/tests/unittests/tools/mcp_tool/test_mcp_tool.py @@ -1927,6 +1927,53 @@ async def test_run_async_impl_provider_survives_unusable_oauth_credential( headers={"Authorization": "Bearer provider-token"} ) + @pytest.mark.asyncio + @pytest.mark.parametrize("empty_token", [None, ""]) + async def test_run_async_impl_empty_oauth_does_not_fall_through_to_http( + self, empty_token + ): + """An empty OAuth2 token must not fall through to the HTTP field. + + The discovery path (``build_auth_headers``) keeps an OAuth2 credential on + the OAuth2 branch and emits no header when its token is empty. The + invocation path must match so that both paths resolve the same credential + to the same ``Authorization`` header and therefore the same pooled MCP + session. Both an absent (``None``) and an empty-string token take this + path. + """ + header_provider = Mock( + return_value={"Authorization": "Bearer provider-token"} + ) + tool = MCPTool( + mcp_tool=self.mock_mcp_tool, + mcp_session_manager=self.mock_session_manager, + header_provider=header_provider, + ) + credential = AuthCredential( + auth_type=AuthCredentialTypes.OAUTH2, + oauth2=OAuth2Auth(access_token=empty_token), + http=HttpAuth( + scheme="bearer", + credentials=HttpCredentials(token="stale-http-token"), + ), + ) + mcp_response = CallToolResult( + content=[TextContent(type="text", text="success")] + ) + self.mock_session.call_tool = AsyncMock(return_value=mcp_response) + tool_context = Mock(spec=ToolContext) + tool_context._invocation_context = Mock() + + await tool._run_async_impl( + args={"param1": "test_value"}, + tool_context=tool_context, + credential=credential, + ) + + self.mock_session_manager.create_session.assert_awaited_once_with( + headers={"Authorization": "Bearer provider-token"} + ) + def test_init_with_progress_callback(self): """Test initialization with progress_callback."""