Skip to content

Commit d2e9fd4

Browse files
zen-agentkaavee315
andcommitted
fix(python-sdk): wrap proxy callable so caller cannot override account [PLEN-2345]
The previous PLEN-2345 commits used ``functools.partial`` to pre-bind ``connected_account_id`` onto the ``execute_request`` proxy callable. Codex flagged that ``partial`` lets a caller-supplied keyword override a pre-bound value: a custom tool calling ``execute_request(connected_account_id="ca_other")`` would run the proxy under that account while ``auth_credentials`` came from the trusted resolved account — a credential/identity mismatch (CWE-639). Replace the partial with a closure whose signature omits ``connected_account_id`` entirely. The id is fixed by the SDK to the account whose credentials were also passed to the tool function; any attempt to override it now raises ``TypeError`` at the call site rather than silently swapping the account out from under the credentials. Side effect: the public ``ExecuteRequestFn`` Protocol no longer declares ``connected_account_id`` (it would never have been honoured anyway given the new wrapper). Sentinel switched ``NotGiven``→``Omit`` on the same Protocol to match the underlying ``client.tools.proxy`` type signature — both sentinels are runtime-equivalent in Stainless' ``is_given``/``strip_not_given``, but the type alignment removes a mypy error in the wrapper forwarding code. Pinned by ``test_execute_request_rejects_caller_supplied_connected_account_id`` (new). Existing PLEN-2345 proxy-binding assertions adjusted to match the wrapper's ``body=omit, parameters=omit`` forwarding. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Karan Vaidya <karan@composio.dev>
1 parent 47d000f commit d2e9fd4

2 files changed

Lines changed: 97 additions & 20 deletions

File tree

python/composio/core/models/custom_tools.py

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,10 @@
3030
import inspect
3131
import typing as t
3232

33+
from composio_client import Omit, omit
3334
from pydantic import BaseModel
3435

35-
from composio.client import HttpClient, NotGiven
36+
from composio.client import HttpClient
3637
from composio.client.types import (
3738
Tool,
3839
tool_list_response,
@@ -45,15 +46,22 @@
4546

4647

4748
class ExecuteRequestFn(t.Protocol):
49+
"""Proxy callable handed to custom tools.
50+
51+
``connected_account_id`` is intentionally absent. The SDK binds the
52+
proxy call to the same connected account that supplied
53+
``auth_credentials``, so the proxy and credentials always address the
54+
same trusted account; a caller-supplied ``connected_account_id`` would
55+
introduce a credential/identity mismatch and is rejected with
56+
``TypeError`` at the call site.
57+
"""
58+
4859
def __call__(
4960
self,
5061
endpoint: str,
5162
method: t.Literal["GET", "POST", "PUT", "DELETE", "PATCH"],
52-
body: t.Dict | NotGiven = HttpClient.not_given,
53-
connected_account_id: str | NotGiven = HttpClient.not_given,
54-
parameters: (
55-
t.Iterable[tool_proxy_params.Parameter] | NotGiven
56-
) = HttpClient.not_given,
63+
body: t.Dict | Omit = omit,
64+
parameters: t.Iterable[tool_proxy_params.Parameter] | Omit = omit,
5765
) -> tool_proxy_response.ToolProxyResponse: ...
5866

5967

@@ -232,10 +240,30 @@ def invoke_trusted(
232240
user_id=user_id,
233241
connected_account_id=connected_account_id,
234242
)
235-
execute_request = functools.partial(
236-
self.client.tools.proxy,
237-
connected_account_id=account_id,
238-
)
243+
244+
# Bind the proxy to the resolved account via a wrapper rather than
245+
# ``functools.partial``: ``partial`` lets a caller-supplied keyword
246+
# override the pre-bound value, so a tool that does
247+
# ``execute_request(connected_account_id="ca_other")`` could run
248+
# the proxy under one account while ``auth_credentials`` came
249+
# from another (CWE-639). The wrapper omits ``connected_account_id``
250+
# from its signature so any attempt to override it raises TypeError.
251+
client = self.client
252+
253+
def execute_request(
254+
endpoint: str,
255+
method: t.Literal["GET", "POST", "PUT", "DELETE", "PATCH"],
256+
body: t.Dict | Omit = omit,
257+
parameters: t.Iterable[tool_proxy_params.Parameter] | Omit = omit,
258+
) -> tool_proxy_response.ToolProxyResponse:
259+
return client.tools.proxy(
260+
endpoint=endpoint,
261+
method=method,
262+
body=body,
263+
parameters=parameters,
264+
connected_account_id=account_id,
265+
)
266+
239267
return t.cast(CustomToolWithProxyProtocol, self.f)(
240268
request=request,
241269
execute_request=t.cast(ExecuteRequestFn, execute_request),

python/tests/test_custom_tools_security.py

Lines changed: 59 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,54 @@ def test_execute_with_connected_account_id_filters_list_by_envelope(
367367
assert result == {"issue_number": 7, "token": "explicit-token"}
368368

369369

370+
def test_execute_request_rejects_caller_supplied_connected_account_id(
371+
mock_http_client_with_explicit_account: MagicMock,
372+
) -> None:
373+
"""``execute_request`` MUST refuse a caller-supplied ``connected_account_id``.
374+
375+
``functools.partial`` would let a caller-supplied keyword override the
376+
SDK-bound id while ``auth_credentials`` still came from the trusted
377+
account — that would re-introduce a credential/identity mismatch
378+
(CWE-639) right at the proxy call site. The wrapper omits
379+
``connected_account_id`` from its signature so any attempt to set it
380+
raises ``TypeError`` rather than silently swapping the account.
381+
"""
382+
captured: dict = {}
383+
384+
def proxy_tool(request: _IssueInput, execute_request, auth_credentials):
385+
"""Try to override the SDK-bound account id from inside the tool."""
386+
try:
387+
execute_request(
388+
endpoint="/api/x",
389+
method="GET",
390+
connected_account_id="ca_other", # type: ignore[call-arg]
391+
)
392+
except TypeError as exc:
393+
captured["error"] = str(exc)
394+
return {"rejected": True}
395+
return {"rejected": False}
396+
397+
tool = CustomTool(
398+
f=proxy_tool,
399+
client=mock_http_client_with_explicit_account,
400+
toolkit="github",
401+
)
402+
tools = CustomTools(client=mock_http_client_with_explicit_account)
403+
tools.custom_tools_registry[tool.slug] = tool
404+
405+
result = tools.execute(
406+
slug=tool.slug,
407+
request={"issue_number": 1},
408+
user_id="trusted-user",
409+
connected_account_id="ca_explicit",
410+
)
411+
412+
assert result == {"rejected": True}
413+
assert "connected_account_id" in captured["error"]
414+
# Proxy was NOT called — wrapper rejected the override before reaching it.
415+
mock_http_client_with_explicit_account.tools.proxy.assert_not_called()
416+
417+
370418
def test_explicit_connected_account_id_outside_envelope_raises(
371419
mock_http_client_with_explicit_account: MagicMock,
372420
custom_tools: CustomTools,
@@ -427,11 +475,13 @@ def proxy_tool(request: _IssueInput, execute_request, auth_credentials):
427475
connected_account_id="ca_explicit",
428476
)
429477

430-
mock_http_client_with_explicit_account.tools.proxy.assert_called_once_with(
431-
endpoint="/api/x",
432-
method="GET",
433-
connected_account_id="ca_explicit",
434-
)
478+
# The wrapper forwards body/parameters as the Stainless ``NOT_GIVEN``
479+
# sentinel when the user's tool omits them — same semantics as if the
480+
# user passed the bare ``client.tools.proxy``, plus the SDK-bound id.
481+
proxy_call = mock_http_client_with_explicit_account.tools.proxy.call_args
482+
assert proxy_call.kwargs["endpoint"] == "/api/x"
483+
assert proxy_call.kwargs["method"] == "GET"
484+
assert proxy_call.kwargs["connected_account_id"] == "ca_explicit"
435485
assert captured["credentials"] == {"access_token": "explicit-token"}
436486

437487

@@ -469,11 +519,10 @@ def proxy_tool(request: _IssueInput, execute_request, auth_credentials):
469519
toolkit_slugs=["github"],
470520
user_ids=["trusted-user"],
471521
)
472-
mock_http_client_with_explicit_account.tools.proxy.assert_called_once_with(
473-
endpoint="/api/y",
474-
method="GET",
475-
connected_account_id="ca_fallback",
476-
)
522+
proxy_call = mock_http_client_with_explicit_account.tools.proxy.call_args
523+
assert proxy_call.kwargs["endpoint"] == "/api/y"
524+
assert proxy_call.kwargs["method"] == "GET"
525+
assert proxy_call.kwargs["connected_account_id"] == "ca_fallback"
477526

478527

479528
def test_explicit_connected_account_id_wins_over_smuggled_one(

0 commit comments

Comments
 (0)