Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down