From 49b85cbfbc636bad8ddd75eeee8d38745006374a Mon Sep 17 00:00:00 2001 From: prasanna8585 <65734642+prasanna8585@users.noreply.github.com> Date: Tue, 11 Aug 2026 10:09:07 +0530 Subject: [PATCH] fix(tools): percent-encode path parameter values in RestApiTool Path parameter values passed to RestApiTool ultimately originate from the model's tool-call arguments and were substituted into the request URL via str.format() with no escaping. An unescaped value could: - Contain '/' or '..' segments, redirecting the request to a different, undeclared path on the same host than the one the OpenAPI spec's path template (and this tool's configured auth credentials) were scoped to. - Contain '?' or '#', which the existing query-string-recovery logic a few lines below would then promote into a real query parameter sent on the wire. Both are now closed by percent-encoding each path parameter value with urllib.parse.quote(value, safe="") -- including '/' -- before substitution, so a value can never introduce a new path segment, query string, or fragment. Adds a regression test covering both cases; full openapi_tool suite (260 tests) passes unchanged. --- .../openapi_spec_parser/rest_api_tool.py | 20 +++++-- .../openapi_spec_parser/test_rest_api_tool.py | 54 +++++++++++++++++++ 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/src/google/adk/tools/openapi_tool/openapi_spec_parser/rest_api_tool.py b/src/google/adk/tools/openapi_tool/openapi_spec_parser/rest_api_tool.py index b6ec5d8553..bd0034682a 100644 --- a/src/google/adk/tools/openapi_tool/openapi_spec_parser/rest_api_tool.py +++ b/src/google/adk/tools/openapi_tool/openapi_spec_parser/rest_api_tool.py @@ -25,6 +25,7 @@ from typing import Tuple from typing import Union from urllib.parse import parse_qs +from urllib.parse import quote from urllib.parse import urlparse from urllib.parse import urlunparse @@ -392,7 +393,17 @@ def _prepare_request_params( param_location = param_obj.param_location if param_location == "path": - path_params[original_k] = v + # Percent-encode path parameter values (including '/') before they + # are substituted into the URL template below. Path parameter + # values ultimately originate from the model's tool-call + # arguments, so an unescaped value (e.g. containing '/', '..', + # '?', or '#') could redirect the request to a different, + # undeclared path -- or undeclared query parameters -- on the + # same host than the one the OpenAPI spec's path template and + # this tool's configured auth credentials were intended for. + # `safe=""` ensures '/' is escaped too, so a value can never + # introduce a new path segment. + path_params[original_k] = quote(str(v), safe="") elif param_location == "query": if v is not None: query_params[original_k] = v @@ -406,8 +417,11 @@ def _prepare_request_params( base_url = base_url[:-1] if base_url.endswith("/") else base_url url = f"{base_url}{self.endpoint.path.format(**path_params)}" - # Move query params embedded in the path into query_params, since httpx - # replaces (rather than merges) the URL query string when `params` is set. + # Move query params embedded in the path template itself (now that path + # parameter values are percent-encoded above, only a spec-authored + # literal query string in `self.endpoint.path` can still produce one + # here) into query_params, since httpx replaces (rather than merges) + # the URL query string when `params` is set. parsed_url = urlparse(url) if parsed_url.query or parsed_url.fragment: for key, values in parse_qs(parsed_url.query).items(): diff --git a/tests/unittests/tools/openapi_tool/openapi_spec_parser/test_rest_api_tool.py b/tests/unittests/tools/openapi_tool/openapi_spec_parser/test_rest_api_tool.py index 6f3743af06..46943df391 100644 --- a/tests/unittests/tools/openapi_tool/openapi_spec_parser/test_rest_api_tool.py +++ b/tests/unittests/tools/openapi_tool/openapi_spec_parser/test_rest_api_tool.py @@ -854,6 +854,60 @@ def test_prepare_request_params_path_param( request_params["url"] == "https://example.com/test/123" ) # Path param replaced + def test_prepare_request_params_path_param_is_percent_encoded( + self, sample_endpoint, sample_auth_credential, sample_auth_scheme + ): + """A path parameter value must not be able to introduce new path + segments, a query string, or a fragment into the constructed URL. + + Path parameter values ultimately come from the model's tool-call + arguments. Without percent-encoding, a value such as '../../admin' + could redirect the request -- along with this tool's configured auth + credentials -- to an endpoint the OpenAPI spec never declared. + """ + mock_operation = Operation(operationId="test_op") + tool = RestApiTool( + name="test_tool", + description="Test Tool", + endpoint=sample_endpoint, + operation=mock_operation, + auth_credential=sample_auth_credential, + auth_scheme=sample_auth_scheme, + ) + params = [ + ApiParameter( + original_name="user_id", + py_name="user_id", + param_location="path", + param_schema=OpenAPISchema(type="string"), + ) + ] + endpoint_with_path = OperationEndpoint( + base_url="https://example.com", + path="/users/{user_id}/messages", + method="get", + ) + tool.endpoint = endpoint_with_path + + # Path traversal attempt: '/' must be escaped so this cannot leave the + # {user_id} path segment. + request_params = tool._prepare_request_params( + params, {"user_id": "../../admin/v1/tenants"} + ) + assert request_params["url"] == ( + "https://example.com/users/..%2F..%2Fadmin%2Fv1%2Ftenants/messages" + ) + + # Query/fragment smuggling attempt: '?' and '#' must be escaped so a + # path parameter value cannot introduce a query string or fragment. + request_params = tool._prepare_request_params( + params, {"user_id": "me?impersonate=other-user#"} + ) + assert request_params["url"] == ( + "https://example.com/users/me%3Fimpersonate%3Dother-user%23/messages" + ) + assert request_params["params"] == {} # nothing smuggled into query params + def test_prepare_request_params_header_param( self, sample_endpoint,