Skip to content

Commit f42321f

Browse files
Merge branch 'fix/adk-handle-invalid-app-name' of https://github.com/AbhishekMauryaGEEK/adk-python into fix/adk-handle-invalid-app-name
2 parents 8b8ee9b + 0516e3f commit f42321f

7 files changed

Lines changed: 136 additions & 14 deletions

File tree

src/google/adk/agents/invocation_context.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
from ..apps.app import EventsCompactionConfig
2929
from ..apps.app import ResumabilityConfig
3030
from ..artifacts.base_artifact_service import BaseArtifactService
31+
from ..auth.auth_credential import AuthCredential
3132
from ..auth.credential_service.base_credential_service import BaseCredentialService
3233
from ..events.event import Event
3334
from ..memory.base_memory_service import BaseMemoryService
@@ -214,6 +215,9 @@ class InvocationContext(BaseModel):
214215
canonical_tools_cache: Optional[list[BaseTool]] = None
215216
"""The cache of canonical tools for this invocation."""
216217

218+
credential_by_key: dict[str, AuthCredential] = Field(default_factory=dict)
219+
"""The resolved credentials for this invocation, keyed by credential_key."""
220+
217221
_invocation_cost_manager: _InvocationCostManager = PrivateAttr(
218222
default_factory=_InvocationCostManager
219223
)

src/google/adk/agents/readonly_context.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
if TYPE_CHECKING:
2323
from google.genai import types
2424

25+
from ..auth.auth_credential import AuthCredential
2526
from ..sessions.session import Session
2627
from .invocation_context import InvocationContext
2728
from .run_config import RunConfig
@@ -69,3 +70,7 @@ def user_id(self) -> str:
6970
def run_config(self) -> Optional[RunConfig]:
7071
"""The run config of the current invocation. READONLY field."""
7172
return self._invocation_context.run_config
73+
74+
def get_credential(self, key: str) -> Optional[AuthCredential]:
75+
"""Gets a resolved credential by key for this invocation."""
76+
return self._invocation_context.credential_by_key.get(key)

src/google/adk/flows/llm_flows/base_llm_flow.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -143,10 +143,11 @@ async def _resolve_toolset_auth(
143143
if not auth_config:
144144
continue
145145

146+
auth_config_copy = auth_config.model_copy(deep=True)
146147
try:
147-
credential = await CredentialManager(auth_config).get_auth_credential(
148-
callback_context
149-
)
148+
credential = await CredentialManager(
149+
auth_config_copy
150+
).get_auth_credential(callback_context)
150151
except ValueError as e:
151152
# Validation errors from CredentialManager should be logged but not
152153
# block the flow - the toolset may still work without auth
@@ -158,14 +159,16 @@ async def _resolve_toolset_auth(
158159
credential = None
159160

160161
if credential:
161-
# Populate in-place for toolset to use in get_tools()
162-
auth_config.exchanged_auth_credential = credential
162+
# Store in invocation context to avoid data leakage and race conditions
163+
invocation_context.credential_by_key[auth_config.credential_key] = (
164+
credential
165+
)
163166
else:
164167
# Need auth - will interrupt
165168
toolset_id = (
166169
f'{TOOLSET_AUTH_CREDENTIAL_ID_PREFIX}{type(tool_union).__name__}'
167170
)
168-
pending_auth_requests[toolset_id] = auth_config
171+
pending_auth_requests[toolset_id] = auth_config_copy
169172

170173
if not pending_auth_requests:
171174
return

src/google/adk/tools/mcp_tool/mcp_tool.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import base64
1919
import inspect
2020
import logging
21+
import os
2122
from typing import Any
2223
from typing import Callable
2324
from typing import Dict
@@ -169,6 +170,16 @@ def __init__(
169170
Raises:
170171
ValueError: If mcp_tool or mcp_session_manager is None.
171172
"""
173+
174+
# --- BEGIN BOUND TOKEN PATCH ---
175+
# Set GOOGLE_API_PREVENT_AGENT_TOKEN_SHARING_FOR_GCP_SERVICES to false
176+
# to disable bound token sharing. Tracking on
177+
# https://github.com/google/adk-python/issues/5361
178+
os.environ["GOOGLE_API_PREVENT_AGENT_TOKEN_SHARING_FOR_GCP_SERVICES"] = (
179+
"false"
180+
)
181+
# --- END BOUND TOKEN PATCH ---
182+
172183
super().__init__(
173184
name=mcp_tool.name,
174185
description=mcp_tool.description if mcp_tool.description else "",

src/google/adk/tools/mcp_tool/mcp_toolset.py

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import asyncio
1818
import base64
1919
import logging
20+
import os
2021
import sys
2122
from typing import Any
2223
from typing import Awaitable
@@ -158,6 +159,15 @@ def __init__(
158159
sampling_capabilities: Optional capabilities for sampling.
159160
"""
160161

162+
# --- BEGIN BOUND TOKEN PATCH ---
163+
# Set GOOGLE_API_PREVENT_AGENT_TOKEN_SHARING_FOR_GCP_SERVICES to false
164+
# to disable bound token sharing. Tracking on
165+
# https://github.com/google/adk-python/issues/5361
166+
os.environ["GOOGLE_API_PREVENT_AGENT_TOKEN_SHARING_FOR_GCP_SERVICES"] = (
167+
"false"
168+
)
169+
# --- END BOUND TOKEN PATCH ---
170+
161171
super().__init__(tool_filter=tool_filter, tool_name_prefix=tool_name_prefix)
162172

163173
self._sampling_callback = sampling_callback
@@ -193,16 +203,31 @@ def __init__(
193203
)
194204
self._use_mcp_resources = use_mcp_resources
195205

196-
def _get_auth_headers(self) -> Optional[Dict[str, str]]:
206+
def _get_auth_headers(
207+
self, readonly_context: Optional[ReadonlyContext] = None
208+
) -> Optional[Dict[str, str]]:
197209
"""Build authentication headers from exchanged credential.
198210
211+
Args:
212+
readonly_context: Readonly context to get credentials from.
213+
199214
Returns:
200215
Dictionary of auth headers, or None if no auth configured.
201216
"""
202-
if not self._auth_config or not self._auth_config.exchanged_auth_credential:
217+
if not self._auth_config:
203218
return None
204219

205-
credential = self._auth_config.exchanged_auth_credential
220+
credential = None
221+
if readonly_context:
222+
credential = readonly_context.get_credential(
223+
self._auth_config.credential_key
224+
)
225+
226+
if not credential:
227+
credential = self._auth_config.exchanged_auth_credential
228+
229+
if not credential:
230+
return None
206231
headers: Optional[Dict[str, str]] = None
207232

208233
if credential.oauth2:
@@ -279,7 +304,7 @@ async def _execute_with_session(
279304
headers.update(provider_headers)
280305

281306
# Add auth headers from exchanged credential if available
282-
auth_headers = self._get_auth_headers()
307+
auth_headers = self._get_auth_headers(readonly_context)
283308
if auth_headers:
284309
headers.update(auth_headers)
285310

tests/unittests/auth/test_toolset_auth.py

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ def mock_invocation_context(self):
110110
ctx.credential_service = None
111111
ctx.app_name = "test-app"
112112
ctx.user_id = "test-user"
113+
ctx.credential_by_key = {}
113114
return ctx
114115

115116
@pytest.fixture
@@ -154,10 +155,10 @@ async def test_toolset_without_auth_config_skipped(
154155
assert mock_invocation_context.end_invocation is False
155156

156157
@pytest.mark.asyncio
157-
async def test_toolset_with_credential_available_populates_config(
158+
async def test_toolset_with_credential_available_populates_context(
158159
self, mock_invocation_context, mock_agent
159160
):
160-
"""Test that credential is populated in auth_config when available."""
161+
"""Test that credential is stored in invocation context when available."""
161162
auth_config = create_oauth2_auth_config()
162163
toolset = MockToolset(auth_config=auth_config)
163164
mock_agent.tools = [toolset]
@@ -184,8 +185,52 @@ async def test_toolset_with_credential_available_populates_config(
184185
# No auth request events - credential was available
185186
assert len(events) == 0
186187
assert mock_invocation_context.end_invocation is False
187-
# Credential should be populated in auth_config
188-
assert auth_config.exchanged_auth_credential == mock_credential
188+
# Credential should be stored in invocation context, not auth_config
189+
assert (
190+
mock_invocation_context.credential_by_key[auth_config.credential_key]
191+
== mock_credential
192+
)
193+
assert auth_config.exchanged_auth_credential is None
194+
195+
@pytest.mark.asyncio
196+
async def test_toolset_auth_uses_copy_and_does_not_mutate_shared_config(
197+
self, mock_invocation_context, mock_agent
198+
):
199+
"""Test that _resolve_toolset_auth uses a copy and does not mutate shared config."""
200+
auth_config = create_oauth2_auth_config()
201+
toolset = MockToolset(auth_config=auth_config)
202+
mock_agent.tools = [toolset]
203+
204+
def create_mock_cm(cfg):
205+
m = AsyncMock()
206+
m._auth_config = cfg
207+
208+
async def get_cred(ctx):
209+
cfg.exchanged_auth_credential = AuthCredential(
210+
auth_type=AuthCredentialTypes.OAUTH2,
211+
oauth2=OAuth2Auth(auth_uri="https://example.com/consent"),
212+
)
213+
return None
214+
215+
m.get_auth_credential = AsyncMock(side_effect=get_cred)
216+
return m
217+
218+
with patch(
219+
"google.adk.flows.llm_flows.base_llm_flow.CredentialManager",
220+
side_effect=create_mock_cm,
221+
):
222+
events = []
223+
async for event in _resolve_toolset_auth(
224+
mock_invocation_context, mock_agent
225+
):
226+
events.append(event)
227+
228+
# Should yield one auth request event
229+
assert len(events) == 1
230+
assert mock_invocation_context.end_invocation is True
231+
232+
# The shared auth_config should NOT be mutated
233+
assert auth_config.exchanged_auth_credential is None
189234

190235
@pytest.mark.asyncio
191236
async def test_toolset_without_credential_yields_auth_event(

tests/unittests/tools/mcp_tool/test_mcp_toolset_auth.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,3 +267,32 @@ def test_get_auth_headers_api_key_non_header_logs_warning(self, caplog):
267267

268268
# Should return None for non-header API key
269269
assert headers is None
270+
271+
def test_get_auth_headers_reads_from_readonly_context(
272+
self, toolset_with_oauth2
273+
):
274+
"""Test that _get_auth_headers reads from ReadonlyContext first."""
275+
from google.adk.agents.readonly_context import ReadonlyContext
276+
277+
mock_readonly_context = Mock(spec=ReadonlyContext)
278+
mock_credential = AuthCredential(
279+
auth_type=AuthCredentialTypes.OAUTH2,
280+
oauth2=OAuth2Auth(access_token="token-from-context"),
281+
)
282+
mock_readonly_context.get_credential.return_value = mock_credential
283+
284+
# Even if exchanged_auth_credential has a different value
285+
toolset_with_oauth2._auth_config.exchanged_auth_credential = AuthCredential(
286+
auth_type=AuthCredentialTypes.OAUTH2,
287+
oauth2=OAuth2Auth(access_token="token-from-config"),
288+
)
289+
290+
headers = toolset_with_oauth2._get_auth_headers(
291+
readonly_context=mock_readonly_context
292+
)
293+
294+
assert headers is not None
295+
assert headers["Authorization"] == "Bearer token-from-context"
296+
mock_readonly_context.get_credential.assert_called_once_with(
297+
toolset_with_oauth2._auth_config.credential_key
298+
)

0 commit comments

Comments
 (0)