Skip to content

Commit fa7b65b

Browse files
authored
Merge branch 'main' into fix/eval-skip-agent-only-invocations
2 parents a0037fd + cbcb5e6 commit fa7b65b

6 files changed

Lines changed: 204 additions & 1 deletion

File tree

src/google/adk/artifacts/file_artifact_service.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,13 +138,39 @@ def _is_user_scoped(session_id: Optional[str], filename: str) -> bool:
138138
return session_id is None or _file_has_user_namespace(filename)
139139

140140

141+
def _validate_path_segment(value: str, field_name: str) -> None:
142+
"""Rejects values that could alter the constructed filesystem path.
143+
144+
Args:
145+
value: The caller-supplied identifier (e.g. user_id or session_id).
146+
field_name: Human-readable name used in the error message.
147+
148+
Raises:
149+
InputValidationError: If the value contains path separators, traversal
150+
segments, or null bytes.
151+
"""
152+
if not value:
153+
raise InputValidationError(f"{field_name} must not be empty.")
154+
if "\x00" in value:
155+
raise InputValidationError(f"{field_name} must not contain null bytes.")
156+
if "/" in value or "\\" in value:
157+
raise InputValidationError(
158+
f"{field_name} {value!r} must not contain path separators."
159+
)
160+
if value in (".", "..") or ".." in value.split("/"):
161+
raise InputValidationError(
162+
f"{field_name} {value!r} must not contain traversal segments."
163+
)
164+
165+
141166
def _user_artifacts_dir(base_root: Path) -> Path:
142167
"""Returns the path that stores user-scoped artifacts."""
143168
return base_root / "artifacts"
144169

145170

146171
def _session_artifacts_dir(base_root: Path, session_id: str) -> Path:
147172
"""Returns the path that stores session-scoped artifacts."""
173+
_validate_path_segment(session_id, "session_id")
148174
return base_root / "sessions" / session_id / "artifacts"
149175

150176

@@ -220,6 +246,7 @@ def __init__(self, root_dir: Path | str):
220246

221247
def _base_root(self, user_id: str, /) -> Path:
222248
"""Returns the artifacts root directory for a user."""
249+
_validate_path_segment(user_id, "user_id")
223250
return self.root_dir / "users" / user_id
224251

225252
def _scope_root(

src/google/adk/cli/cli_deploy.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -808,6 +808,24 @@ def to_cloud_run(
808808
shutil.rmtree(temp_folder)
809809

810810

811+
def _print_agent_engine_url(resource_name: str) -> None:
812+
"""Prints the Google Cloud Console URL for the deployed agent."""
813+
parts = resource_name.split('/')
814+
if len(parts) >= 6 and parts[0] == 'projects' and parts[2] == 'locations':
815+
project_id = parts[1]
816+
region = parts[3]
817+
engine_id = parts[5]
818+
819+
url = (
820+
'https://console.cloud.google.com/agent-platform/runtimes'
821+
f'/locations/{region}/agent-engines/{engine_id}/playground'
822+
f'?project={project_id}'
823+
)
824+
click.secho(
825+
f'\n🎉 View your deployed agent here:\n{url}\n', fg='cyan', bold=True
826+
)
827+
828+
811829
def to_agent_engine(
812830
*,
813831
agent_folder: str,
@@ -1150,11 +1168,13 @@ def to_agent_engine(
11501168
f'✅ Created agent engine: {agent_engine.api_resource.name}',
11511169
fg='green',
11521170
)
1171+
_print_agent_engine_url(agent_engine.api_resource.name)
11531172
else:
11541173
if project and region and not agent_engine_id.startswith('projects/'):
11551174
agent_engine_id = f'projects/{project}/locations/{region}/reasoningEngines/{agent_engine_id}'
11561175
client.agent_engines.update(name=agent_engine_id, config=agent_config)
11571176
click.secho(f'✅ Updated agent engine: {agent_engine_id}', fg='green')
1177+
_print_agent_engine_url(agent_engine_id)
11581178
finally:
11591179
click.echo(f'Cleaning up the temp folder: {temp_folder}')
11601180
shutil.rmtree(agent_src_path)

src/google/adk/integrations/agent_registry/agent_registry.py

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
from google.adk.agents.remote_a2a_agent import RemoteA2aAgent
3737
from google.adk.auth.auth_credential import AuthCredential
3838
from google.adk.auth.auth_schemes import AuthScheme
39+
from google.adk.integrations.agent_identity.gcp_auth_provider_scheme import GcpAuthProviderScheme
3940
from google.adk.telemetry.tracing import GCP_MCP_SERVER_DESTINATION_ID
4041
from google.adk.tools.base_tool import BaseTool
4142
from google.adk.tools.mcp_tool.mcp_session_manager import SseConnectionParams
@@ -292,8 +293,24 @@ def get_mcp_toolset(
292293
mcp_server_name: str,
293294
auth_scheme: AuthScheme | None = None,
294295
auth_credential: AuthCredential | None = None,
296+
*,
297+
continue_uri: str | None = None,
295298
) -> McpToolset:
296-
"""Constructs an McpToolset instance from a registered MCP Server."""
299+
"""Constructs an McpToolset from a registered MCP Server.
300+
301+
If `auth_scheme` is omitted, it is automatically resolved from the server's
302+
IAM bindings via `GcpAuthProviderScheme`.
303+
304+
Args:
305+
mcp_server_name: Resource name of the MCP Server.
306+
auth_scheme: Optional auth scheme. Resolved via bindings if omitted.
307+
auth_credential: Optional auth credential.
308+
continue_uri: Optional continue URI to override what is in the auth
309+
provider.
310+
311+
Returns:
312+
An McpToolset for the MCP server.
313+
"""
297314
server_details = self.get_mcp_server(mcp_server_name)
298315
name = self._clean_name(server_details.get("displayName", mcp_server_name))
299316
mcp_server_id = server_details.get("mcpServerId")
@@ -313,6 +330,23 @@ def get_mcp_toolset(
313330
)
314331

315332
headers = self._get_auth_headers() if _is_google_api(endpoint_uri) else None
333+
if mcp_server_id and not auth_scheme:
334+
try:
335+
bindings_data = self._make_request("bindings")
336+
for b in bindings_data.get("bindings", []):
337+
target_id = b.get("target", {}).get("identifier", "")
338+
if target_id.endswith(mcp_server_id):
339+
auth_provider = b.get("authProviderBinding", {}).get("authProvider")
340+
if auth_provider:
341+
auth_scheme = GcpAuthProviderScheme(
342+
name=auth_provider, continue_uri=continue_uri
343+
)
344+
break
345+
except Exception as e:
346+
logger.warning(
347+
f"Failed to fetch bindings for MCP Server {mcp_server_name}: {e}"
348+
)
349+
316350
connection_params = StreamableHTTPConnectionParams(
317351
url=endpoint_uri,
318352
headers=headers,

tests/unittests/artifacts/test_artifact_service.py

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -744,6 +744,69 @@ async def test_file_save_artifact_rejects_out_of_scope_paths(
744744
)
745745

746746

747+
@pytest.mark.asyncio
748+
@pytest.mark.parametrize(
749+
"user_id",
750+
[
751+
"../escape",
752+
"../../etc",
753+
"foo/../../bar",
754+
"valid/../..",
755+
"..",
756+
".",
757+
"has/slash",
758+
"back\\slash",
759+
"null\x00byte",
760+
"",
761+
],
762+
)
763+
async def test_file_save_artifact_rejects_traversal_in_user_id(
764+
tmp_path, user_id
765+
):
766+
"""FileArtifactService rejects user_id values that escape root_dir."""
767+
artifact_service = FileArtifactService(root_dir=tmp_path / "artifacts")
768+
part = types.Part(text="content")
769+
with pytest.raises(InputValidationError):
770+
await artifact_service.save_artifact(
771+
app_name="myapp",
772+
user_id=user_id,
773+
session_id="sess123",
774+
filename="safe.txt",
775+
artifact=part,
776+
)
777+
778+
779+
@pytest.mark.asyncio
780+
@pytest.mark.parametrize(
781+
"session_id",
782+
[
783+
"../escape",
784+
"../../tmp",
785+
"foo/../../bar",
786+
"..",
787+
".",
788+
"has/slash",
789+
"back\\slash",
790+
"null\x00byte",
791+
"",
792+
],
793+
)
794+
async def test_file_save_artifact_rejects_traversal_in_session_id(
795+
tmp_path, session_id
796+
):
797+
"""FileArtifactService rejects session_id values that escape root_dir."""
798+
artifact_service = FileArtifactService(root_dir=tmp_path / "artifacts")
799+
part = types.Part(text="content")
800+
with pytest.raises(InputValidationError):
801+
await artifact_service.save_artifact(
802+
app_name="myapp",
803+
user_id="user123",
804+
session_id=session_id,
805+
filename="safe.txt",
806+
artifact=part,
807+
)
808+
809+
747810
@pytest.mark.asyncio
748811
async def test_file_save_artifact_rejects_absolute_path_within_scope(tmp_path):
749812
"""Absolute filenames are rejected even when they point inside the scope."""

tests/unittests/cli/utils/test_cli_deploy.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,20 @@ def test_agent_engine_app_template_compiles_with_windows_paths() -> None:
240240
compile(rendered, "<agent_engine_app.py>", "exec")
241241

242242

243+
def test_print_agent_engine_url() -> None:
244+
"""It should print the correct URL for a fully-qualified resource name."""
245+
with mock.patch("click.secho") as mocked_secho:
246+
cli_deploy._print_agent_engine_url(
247+
"projects/my-project/locations/us-central1/reasoningEngines/123456"
248+
)
249+
mocked_secho.assert_called_once()
250+
call_args = mocked_secho.call_args[0][0]
251+
assert "my-project" in call_args
252+
assert "us-central1" in call_args
253+
assert "123456" in call_args
254+
assert "playground" in call_args
255+
256+
243257
@pytest.mark.parametrize("include_requirements", [True, False])
244258
def test_to_agent_engine_happy_path(
245259
monkeypatch: pytest.MonkeyPatch,

tests/unittests/integrations/agent_registry/test_agent_registry.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -637,3 +637,48 @@ def test_get_model_name_raises_value_error_if_no_uri(
637637
mock_get_endpoint.return_value = {}
638638
with pytest.raises(ValueError, match="Connection URI not found"):
639639
registry.get_model_name("test-endpoint")
640+
641+
@patch.object(AgentRegistry, "_make_request")
642+
def test_get_mcp_toolset_with_binding(self, mock_make_request, registry):
643+
def side_effect(*args, **kwargs):
644+
if args[0] == "test-mcp":
645+
return {
646+
"displayName": "TestPrefix",
647+
"mcpServerId": "server-456",
648+
"interfaces": [{
649+
"url": "https://mcp.com",
650+
"protocolBinding": "JSONRPC",
651+
}],
652+
}
653+
if args[0] == "bindings":
654+
return {
655+
"bindings": [{
656+
"target": {
657+
"identifier": (
658+
"urn:mcp:projects-123:projects:123:locations:l:mcpServers:server-456"
659+
)
660+
},
661+
"authProviderBinding": {
662+
"authProvider": (
663+
"projects/123/locations/l/authProviders/ap-789"
664+
)
665+
},
666+
}]
667+
}
668+
return {}
669+
670+
mock_make_request.side_effect = side_effect
671+
672+
registry._credentials.token = "token"
673+
registry._credentials.refresh = MagicMock()
674+
675+
toolset = registry.get_mcp_toolset(
676+
"test-mcp", continue_uri="https://override.com/continue"
677+
)
678+
assert isinstance(toolset, McpToolset)
679+
assert toolset._auth_scheme is not None
680+
assert (
681+
toolset._auth_scheme.name
682+
== "projects/123/locations/l/authProviders/ap-789"
683+
)
684+
assert toolset._auth_scheme.continue_uri == "https://override.com/continue"

0 commit comments

Comments
 (0)