From e4ee6621167c9be062028bfc55c7c64377b3bfde Mon Sep 17 00:00:00 2001 From: George Weale Date: Mon, 17 Aug 2026 22:47:35 +0000 Subject: [PATCH 1/3] fix(mcp): reject stdio MCP servers declared in agent configs by default (v1) Loading an agent config that declared a stdio MCP server launched the config-supplied `command` as a local process, before the model was ever contacted. `McpToolset.from_config()` now rejects `stdio_server_params` and `stdio_connection_params` unless the operator opts in by setting `ADK_ALLOW_CONFIG_STDIO_MCP_SERVERS=1`. This is a behaviour change for existing 1.x users: an agent config that declares a stdio MCP server now raises `ValueError` at load time until the environment variable is set. Remote transports (`sse_connection_params`, `streamable_http_connection_params`) and toolsets constructed in Python code are unaffected. Port of upstream a61d8ecf. Unlike the upstream version, this port does not add the in-process `_set_allow_config_stdio_servers()` override, so the environment variable is the only opt-in and no new module state lands on the maintenance branch. --- .../tool_mcp_stdio_notion_config/README.md | 15 +++++- .../root_agent.yaml | 2 + src/google/adk/tools/mcp_tool/mcp_toolset.py | 23 ++++++++ .../tools/mcp_tool/test_mcp_toolset.py | 54 ++++++++++++++++++- 4 files changed, 92 insertions(+), 2 deletions(-) diff --git a/contributing/samples/tool_mcp_stdio_notion_config/README.md b/contributing/samples/tool_mcp_stdio_notion_config/README.md index 41544a19c7e..c67399429d2 100644 --- a/contributing/samples/tool_mcp_stdio_notion_config/README.md +++ b/contributing/samples/tool_mcp_stdio_notion_config/README.md @@ -30,7 +30,20 @@ env: 1. Click "Edit access" 1. Add pages or databases as needed -### 4. Run the Agent +### 4. Opt In to Stdio MCP Servers + +This sample declares a stdio MCP server in `root_agent.yaml`, which means +loading the config launches `npx` as a local process. ADK rejects that by +default, because an agent config obtained from someone else would then be able +to run arbitrary commands. Opt in before running the sample: + +```bash +export ADK_ALLOW_CONFIG_STDIO_MCP_SERVERS=1 +``` + +Only set this when you trust every agent config the process will load. + +### 5. Run the Agent Use the `adk web` to run the agent and interact with your Notion workspace. diff --git a/contributing/samples/tool_mcp_stdio_notion_config/root_agent.yaml b/contributing/samples/tool_mcp_stdio_notion_config/root_agent.yaml index 7cb9e1cea35..f59418a7f23 100644 --- a/contributing/samples/tool_mcp_stdio_notion_config/root_agent.yaml +++ b/contributing/samples/tool_mcp_stdio_notion_config/root_agent.yaml @@ -4,6 +4,8 @@ model: gemini-2.5-flash instruction: | You are my workspace assistant. Use the provided tools to read, search, comment on, or create Notion pages. Ask clarifying questions when unsure. +# Declaring a stdio MCP server launches `command` as a local process when this +# config loads, so it requires ADK_ALLOW_CONFIG_STDIO_MCP_SERVERS=1. See README. tools: - name: MCPToolset args: diff --git a/src/google/adk/tools/mcp_tool/mcp_toolset.py b/src/google/adk/tools/mcp_tool/mcp_toolset.py index 71f7cd16458..357260e8f5b 100644 --- a/src/google/adk/tools/mcp_tool/mcp_toolset.py +++ b/src/google/adk/tools/mcp_tool/mcp_toolset.py @@ -43,6 +43,7 @@ from ...auth.auth_credential import AuthCredential from ...auth.auth_schemes import AuthScheme from ...auth.auth_tool import AuthConfig +from ...utils.env_utils import is_env_enabled from ..base_tool import BaseTool from ..base_toolset import BaseToolset from ..base_toolset import ToolPredicate @@ -59,6 +60,13 @@ logger = logging.getLogger("google_adk." + __name__) +ALLOW_CONFIG_STDIO_SERVERS_ENV_VAR = "ADK_ALLOW_CONFIG_STDIO_MCP_SERVERS" + + +def _allow_config_stdio_servers_enabled() -> bool: + """Returns whether agent configs may declare stdio MCP servers.""" + return is_env_enabled(ALLOW_CONFIG_STDIO_SERVERS_ENV_VAR) + T = TypeVar("T") @@ -456,6 +464,21 @@ def from_config( """Creates an McpToolset from a configuration object.""" mcp_toolset_config = McpToolsetConfig.model_validate(config.model_dump()) + if ( + mcp_toolset_config.stdio_server_params + or mcp_toolset_config.stdio_connection_params + ) and not _allow_config_stdio_servers_enabled(): + raise ValueError( + "Stdio MCP servers are not allowed in agent configs: the" + " config-supplied 'command' is launched as a local process when the" + " agent starts, so an untrusted config would be able to run" + " arbitrary code. Construct the McpToolset in Python code instead," + " use a remote transport (sse_connection_params or" + " streamable_http_connection_params), or set" + f" {ALLOW_CONFIG_STDIO_SERVERS_ENV_VAR}=1 if this application only" + " loads agent configs it trusts." + ) + if mcp_toolset_config.stdio_server_params: connection_params = mcp_toolset_config.stdio_server_params elif mcp_toolset_config.stdio_connection_params: diff --git a/tests/unittests/tools/mcp_tool/test_mcp_toolset.py b/tests/unittests/tools/mcp_tool/test_mcp_toolset.py index 7edd1b29575..7a9682c48e1 100644 --- a/tests/unittests/tools/mcp_tool/test_mcp_toolset.py +++ b/tests/unittests/tools/mcp_tool/test_mcp_toolset.py @@ -30,6 +30,7 @@ from google.adk.auth.auth_credential import OAuth2Auth from google.adk.auth.auth_tool import AuthConfig from google.adk.tools.load_mcp_resource_tool import LoadMcpResourceTool +from google.adk.tools.mcp_tool import mcp_toolset as mcp_toolset_module from google.adk.tools.mcp_tool.mcp_session_manager import MCPSessionManager from google.adk.tools.mcp_tool.mcp_session_manager import SseConnectionParams from google.adk.tools.mcp_tool.mcp_session_manager import StdioConnectionParams @@ -176,8 +177,11 @@ def test_init_with_auth_and_credential_key(self): assert toolset._auth_credential == auth_credential assert toolset._auth_config.credential_key == "my_custom_key" - def test_from_config_with_credential_key(self): + def test_from_config_with_credential_key(self, monkeypatch): """Test that from_config correctly parses credential_key.""" + monkeypatch.setenv( + mcp_toolset_module.ALLOW_CONFIG_STDIO_SERVERS_ENV_VAR, "1" + ) auth_scheme = OAuth2(flows={}) @@ -191,6 +195,54 @@ def test_from_config_with_credential_key(self): assert isinstance(toolset._auth_scheme, OAuth2) assert toolset._auth_config.credential_key == "my_custom_key" + def test_from_config_rejects_stdio_server_params(self): + """Config-supplied stdio servers are rejected by default.""" + config = ToolArgsConfig(stdio_server_params=self.mock_stdio_params) + + with pytest.raises(ValueError, match="not allowed in agent configs"): + McpToolset.from_config(config, "") + + def test_from_config_rejects_stdio_connection_params(self): + """The stdio_connection_params spelling is rejected the same way.""" + config = ToolArgsConfig( + stdio_connection_params=StdioConnectionParams( + server_params=self.mock_stdio_params + ) + ) + + with pytest.raises(ValueError, match="not allowed in agent configs"): + McpToolset.from_config(config, "") + + def test_from_config_rejection_names_the_env_var(self): + """The error tells the operator how to opt in.""" + config = ToolArgsConfig(stdio_server_params=self.mock_stdio_params) + + with pytest.raises( + ValueError, match=mcp_toolset_module.ALLOW_CONFIG_STDIO_SERVERS_ENV_VAR + ): + McpToolset.from_config(config, "") + + def test_from_config_allows_stdio_when_env_var_set(self, monkeypatch): + """The environment variable opts a whole process in.""" + monkeypatch.setenv( + mcp_toolset_module.ALLOW_CONFIG_STDIO_SERVERS_ENV_VAR, "1" + ) + config = ToolArgsConfig(stdio_server_params=self.mock_stdio_params) + + toolset = McpToolset.from_config(config, "") + + assert isinstance(toolset, McpToolset) + + def test_from_config_allows_remote_connection_params(self): + """Remote MCP servers are unaffected: they launch no local process.""" + config = ToolArgsConfig( + sse_connection_params=SseConnectionParams(url="https://example.com/sse") + ) + + toolset = McpToolset.from_config(config, "") + + assert isinstance(toolset, McpToolset) + def test_init_missing_connection_params(self): """Test initialization with missing connection params raises error.""" with pytest.raises(ValueError, match="Missing connection params"): From 71e21d4417f733649bcf5c56f43085ec1e3304f3 Mon Sep 17 00:00:00 2001 From: George Weale Date: Mon, 17 Aug 2026 22:50:24 +0000 Subject: [PATCH 2/3] fix(mcp): refuse MCP tools that take a reserved ADK tool name (v1) An MCP tool was registered under the verbatim name the remote server advertised, with no check against the names the framework itself puts on the wire. A server that advertised `adk_request_credential`, `adk_request_confirmation`, `adk_request_input` or `transfer_to_agent` therefore had its own tool dispatched in place of the framework's. `McpToolset.get_tools` now drops a tool carrying one of those four names and logs a warning, and `McpTool.__init__` refuses the name outright. The listing skips rather than raises so that one reserved name does not fail the whole `list_tools` call and take the server's honest tools with it; the constructor check is the backstop for anything that builds an `McpTool` directly. Behaviour change: a server that legitimately serves a tool under one of those four names loses that tool, with only a warning log as the signal. Only exact matches are refused, so `transfer_to_agent_v2` still registers. Because `tool_name_prefix` is applied after `get_tools` returns, a prefixed toolset also drops such a tool even though the prefix would have made the final name unique; this matches upstream. Port of upstream 77d4647c, which is itself the reland of an earlier attempt. Only the reland's net content is ported: the constructor's handling of a `None` tool or session manager and the `mcp_session_manager` annotation are left exactly as they were. --- src/google/adk/tools/mcp_tool/mcp_tool.py | 23 ++++++++++++- src/google/adk/tools/mcp_tool/mcp_toolset.py | 11 ++++++ .../unittests/tools/mcp_tool/test_mcp_tool.py | 34 +++++++++++++++++++ .../tools/mcp_tool/test_mcp_toolset.py | 21 ++++++++++++ 4 files changed, 88 insertions(+), 1 deletion(-) diff --git a/src/google/adk/tools/mcp_tool/mcp_tool.py b/src/google/adk/tools/mcp_tool/mcp_tool.py index 7c7a2bdd9f5..5b2e7ce7634 100644 --- a/src/google/adk/tools/mcp_tool/mcp_tool.py +++ b/src/google/adk/tools/mcp_tool/mcp_tool.py @@ -46,6 +46,9 @@ from ...events.ui_widget import UiWidget from ...features import FeatureName from ...features import is_feature_enabled +from ...flows.llm_flows.functions import REQUEST_CONFIRMATION_FUNCTION_CALL_NAME +from ...flows.llm_flows.functions import REQUEST_EUC_FUNCTION_CALL_NAME +from ...flows.llm_flows.functions import REQUEST_INPUT_FUNCTION_CALL_NAME from ...utils.context_utils import find_context_parameter # `is_feature_enabled(FeatureName._MCP_GRACEFUL_ERROR_HANDLING)` gates the # error-boundary and transport-crash-detection behavior added in this module. @@ -56,12 +59,23 @@ from .._gemini_schema_util import _to_gemini_schema from ..base_authenticated_tool import BaseAuthenticatedTool from ..tool_context import ToolContext +from ..transfer_to_agent_tool import transfer_to_agent from .mcp_session_manager import MCPSessionManager from .mcp_session_manager import retry_on_errors from .session_context import SessionContext logger = logging.getLogger("google_adk." + __name__) +# Tool names the framework itself puts on the wire. A server advertising one of +# these would have its tool dispatched in place of the framework's own, so the +# name is refused at registration. +_RESERVED_TOOL_NAMES = frozenset({ + REQUEST_EUC_FUNCTION_CALL_NAME, + REQUEST_CONFIRMATION_FUNCTION_CALL_NAME, + REQUEST_INPUT_FUNCTION_CALL_NAME, + transfer_to_agent.__name__, +}) + @runtime_checkable class ProgressCallbackFactory(Protocol): @@ -177,7 +191,8 @@ def __init__( and modify runtime context like session state. Raises: - ValueError: If mcp_tool or mcp_session_manager is None. + ValueError: If the MCP tool name collides with a reserved ADK tool + name. """ # --- BEGIN BOUND TOKEN PATCH --- @@ -189,6 +204,12 @@ def __init__( ) # --- END BOUND TOKEN PATCH --- + if mcp_tool.name in _RESERVED_TOOL_NAMES: + raise ValueError( + f"MCP tool name '{mcp_tool.name}' collides with a reserved ADK tool" + " name." + ) + super().__init__( name=mcp_tool.name, description=mcp_tool.description if mcp_tool.description else "", diff --git a/src/google/adk/tools/mcp_tool/mcp_toolset.py b/src/google/adk/tools/mcp_tool/mcp_toolset.py index 357260e8f5b..f741529b3eb 100644 --- a/src/google/adk/tools/mcp_tool/mcp_toolset.py +++ b/src/google/adk/tools/mcp_tool/mcp_toolset.py @@ -55,6 +55,7 @@ from .mcp_session_manager import SseConnectionParams from .mcp_session_manager import StdioConnectionParams from .mcp_session_manager import StreamableHTTPConnectionParams +from .mcp_tool import _RESERVED_TOOL_NAMES from .mcp_tool import MCPTool from .mcp_tool import ProgressCallbackFactory @@ -362,6 +363,16 @@ async def get_tools( # Apply filtering based on context and tool_filter tools = [] for tool in tools_response.tools: + # Skip rather than let McpTool raise: one reserved name would otherwise + # fail the whole listing and take the server's honest tools down with it. + if tool.name in _RESERVED_TOOL_NAMES: + logger.warning( + "Skipping MCP tool '%s' because it collides with a reserved ADK" + " framework tool name.", + tool.name, + ) + continue + mcp_tool = MCPTool( mcp_tool=tool, mcp_session_manager=self._mcp_session_manager, diff --git a/tests/unittests/tools/mcp_tool/test_mcp_tool.py b/tests/unittests/tools/mcp_tool/test_mcp_tool.py index 6643547df94..13876d2273a 100644 --- a/tests/unittests/tools/mcp_tool/test_mcp_tool.py +++ b/tests/unittests/tools/mcp_tool/test_mcp_tool.py @@ -240,6 +240,40 @@ def test_init_with_empty_description(self): assert tool.description == "" + @pytest.mark.parametrize( + "reserved_name", + [ + "adk_request_credential", + "adk_request_confirmation", + "adk_request_input", + "transfer_to_agent", + ], + ) + def test_init_reserved_name(self, reserved_name): + """A tool named after a framework function call is refused.""" + mock_tool = MockMCPTool(name=reserved_name) + with pytest.raises( + ValueError, + match=( + f"MCP tool name '{reserved_name}' collides with a reserved ADK tool" + " name." + ), + ): + MCPTool( + mcp_tool=mock_tool, + mcp_session_manager=self.mock_session_manager, + ) + + def test_init_reserved_name_prefix_allowed(self): + """Only exact collisions are refused, not names that merely look alike.""" + mock_tool = MockMCPTool(name="transfer_to_agent_v2") + tool = MCPTool( + mcp_tool=mock_tool, + mcp_session_manager=self.mock_session_manager, + ) + + assert tool.name == "transfer_to_agent_v2" + @pytest.mark.asyncio async def test_run_async_impl_no_auth(self): """Test running tool without authentication.""" diff --git a/tests/unittests/tools/mcp_tool/test_mcp_toolset.py b/tests/unittests/tools/mcp_tool/test_mcp_toolset.py index 7a9682c48e1..da8f9bf9c6f 100644 --- a/tests/unittests/tools/mcp_tool/test_mcp_toolset.py +++ b/tests/unittests/tools/mcp_tool/test_mcp_toolset.py @@ -277,6 +277,27 @@ async def test_get_tools_basic(self): assert tools[2].name == "tool3" assert tools[3].name == "load_mcp_resource" + @pytest.mark.asyncio + async def test_get_tools_skips_reserved_names(self): + """A server advertising reserved names loses those, not the whole list.""" + mock_tools = [ + MockMCPTool("valid_tool"), + MockMCPTool("transfer_to_agent"), + MockMCPTool("adk_request_credential"), + MockMCPTool("adk_request_confirmation"), + MockMCPTool("adk_request_input"), + ] + self.mock_session.list_tools = AsyncMock( + return_value=MockListToolsResult(mock_tools) + ) + + toolset = McpToolset(connection_params=self.mock_stdio_params) + toolset._mcp_session_manager = self.mock_session_manager + + tools = await toolset.get_tools() + + assert [tool.name for tool in tools] == ["valid_tool"] + @pytest.mark.asyncio async def test_get_tools_with_list_filter(self): """Test getting tools with list-based filtering.""" From 1a195082ab6e0447884fafaae686158e211f2971 Mon Sep 17 00:00:00 2001 From: George Weale Date: Tue, 18 Aug 2026 23:46:33 +0000 Subject: [PATCH 3/3] chore(samples): add the license header to the Notion config sample (v1) The header-check bot compares a pull request against `main`, and this sample directory exists only on `v1`, so both of its files read as newly added and the YAML was reported as missing a license header. Adds the standard header; no functional change. --- .../tool_mcp_stdio_notion_config/root_agent.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/contributing/samples/tool_mcp_stdio_notion_config/root_agent.yaml b/contributing/samples/tool_mcp_stdio_notion_config/root_agent.yaml index f59418a7f23..74c360bdb17 100644 --- a/contributing/samples/tool_mcp_stdio_notion_config/root_agent.yaml +++ b/contributing/samples/tool_mcp_stdio_notion_config/root_agent.yaml @@ -1,3 +1,17 @@ +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + # yaml-language-server: $schema=https://raw.githubusercontent.com/google/adk-python/refs/heads/main/src/google/adk/agents/config_schemas/AgentConfig.json name: notion_agent model: gemini-2.5-flash