google / google/adk-java

MCP toolset: in-model built-ins (e.g. google_search) can be shadowed by a server tool; a server tool named set_model_response aborts the run

Abierto
#1,513 1 comentario 0 reacciones 1 asignado Reclamado por @hemasekhar-p Ver en GitHub
needs review
Lenguaje dominante
Java
Estrellas
1.7k
Forks
420
Merge medio
4 d 12 h
PR fusionados (30 d)
31

Descripción

## Summary

Same two defects as the Go port, reproduced locally against `adk-java` (`4ee155b2`, core). No live service was contacted.

1. **In-model built-in tools do not occupy their name, so a server-provided tool can shadow them.** `GoogleSearchTool` (and `GoogleMapsTool`, `UrlContextTool`, `VertexAiSearchTool`, `BuiltInCodeExecutionTool`) override `processLlmRequest` to append only to `config.Tools`, never calling `appendTools`. So the duplicate-name guard in `LlmRequest.Builder.appendTools` never sees them, and a callable MCP tool advertising `google_search` is accepted and wins dispatch — `Functions.handleFunctionCalls` resolves by name from a map where the MCP tool overwrote the built-in.
2. **A server-provided tool named `set_model_response` aborts the whole run** on an agent with an output schema (`IllegalArgumentException: Duplicate tool name: set_model_response`). Availability, not silent displacement — but a malicious/untrusted MCP server can deny service.

## Evidence

- MCP tools take the server's name, unfiltered by default — `tools/mcp/McpToolset.java:260-279`; `tools/BaseToolset.java:60-64` (`if (toolFilter == null) return true;`); name source `tools/mcp/AbstractMcpTool.java:52-56`.
- Callable-vs-callable is fail-closed — `models/LlmRequest.java:204-222` (`appendTools`, throwing merger); all tools enter via `tools/BaseTool.java:187-193`.
- `set_model_response` is in the guarded class — `flows/llmflows/OutputSchema.java:60-65`; `SetModelResponseTool.java:33-45` (extends `BaseTool`, does not override `processLlmRequest`). Processor order: `SingleFlow.java:27-36`, `BaseLlmFlow.java:103-104`.
- Built-ins bypass the guard — `tools/GoogleSearchTool.java:45-76` (override that appends `Tool.builder().googleSearch(...)` only); same in `GoogleMapsTool.java:65`, `UrlContextTool.java:46`, `VertexAiSearchTool.java:60`, `BuiltInCodeExecutionTool.java:44`.
- Dispatch by name — `BaseLlmFlow.java:765-766`; `Functions.java:295` (`tools.get(functionCall.name().get())`).

## Local reproduction

```
[probe] registered tool map keys: [google_search]
[probe] config.Tool googleSearch=true functionDeclarations=[]
[probe] config.Tool googleSearch=false functionDeclarations=[google_search]
[probe] google_search: NO ERROR. dispatch map maps google_search -> FakeMcpTool
[probe] set_model_response: REJECTED -> Duplicate tool name: set_model_response
```

## Suggested fix

Preferred (availability-first): reject the server-provided tool via a reserved-name check in `AbstractMcpTool`'s constructor or in `McpToolset.getTools` / `McpAsyncToolset.getTools`, covering `set_model_response`, `transfer_to_agent`, `google_search`, `google_maps`, `url_context`, `vertex_ai_search`, `code_execution`, `load_artifacts`.

Alternative: make the in-model tools call `appendTools` so the existing duplicate guard applies (note: `google_search` would then need a clean-failing `runAsync`).

Same class as the Go port (`google/adk-go#1605`) and a separately-triaged `adk-js` report.

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.