From 9efac7747ae19667367d8a6e4d9bbfd70612daa3 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Fri, 6 Nov 2026 19:34:54 -0800 Subject: [PATCH] fix(tool-choice): refuse an allowed_tools entry whose name is not a string Backport of #50467 to lts/1.108.x. Cherry-picked from 2ac020c070c7fdd47ec912f5ae334923ec85356c (litellm_allowed_tools_flat_tool_choice). Test hunks that need main-only test helpers are carried by the line's test adaptation commit. --- .../allowed_tools_tool_choice.py | 8 ++++++-- .../test_allowed_tools_tool_choice.py | 16 ++++++++++++++++ tests/unit/test_main.py | 14 ++++++++++++-- tests/unit/test_tool_loop.py | 10 ++++++++-- 4 files changed, 42 insertions(+), 6 deletions(-) diff --git a/litellm/litellm_core_utils/allowed_tools_tool_choice.py b/litellm/litellm_core_utils/allowed_tools_tool_choice.py index 0d836eb8aba..7ee53dcddab 100644 --- a/litellm/litellm_core_utils/allowed_tools_tool_choice.py +++ b/litellm/litellm_core_utils/allowed_tools_tool_choice.py @@ -92,6 +92,10 @@ def _has_name(selector: Mapping[str, object]) -> bool: return isinstance(name, str) and name != "" +def _names_on_other_tool_types_are_strings(tools: Iterable[Mapping[str, object]]) -> bool: + return all(tool.get("type") in ("function", "custom") or "name" not in tool or _has_name(tool) for tool in tools) + + def _chat_allowed_tool(tool: Mapping[str, object]) -> Mapping[str, object] | None: tool_type: Final = tool.get("type") if not isinstance(tool_type, str) or tool_type not in ("function", "custom"): @@ -120,9 +124,9 @@ def parse_allowed_tools_tool_choice(tool_choice: object) -> ChatAllowedToolsTool return None match parsed: case ChatAllowedToolsToolChoice(): - return parsed + return parsed if _names_on_other_tool_types_are_strings(parsed.allowed_tools.tools) else None case ResponsesAllowedToolsToolChoice(): - return _chat_layout(parsed) + return _chat_layout(parsed) if _names_on_other_tool_types_are_strings(parsed.tools) else None def azure_api_version_takes_tool_choice(api_version: str) -> bool: diff --git a/tests/unit/litellm_core_utils/test_allowed_tools_tool_choice.py b/tests/unit/litellm_core_utils/test_allowed_tools_tool_choice.py index a32ebe3e9b5..8b124818d6a 100644 --- a/tests/unit/litellm_core_utils/test_allowed_tools_tool_choice.py +++ b/tests/unit/litellm_core_utils/test_allowed_tools_tool_choice.py @@ -14,10 +14,20 @@ from litellm.litellm_core_utils.allowed_tools_tool_choice import ( parse_allowed_tools_tool_choice, refuse_a_malformed_allowed_tools_tool_choice, refuse_tool_calls_the_choice_excludes, + tool_calls_the_choice_excludes, unenforced_allowed_tools_tool_choice, responses_allowed_tools_tool_choice, ) +_CHOICES_WITH_A_NAME_THAT_IS_NOT_A_STRING: Final = ( + {"type": "allowed_tools", "mode": "auto", "tools": [{"type": "mcp", "name": ["get_time"]}]}, + {"type": "allowed_tools", "mode": "auto", "tools": [{"type": "mcp", "name": {"first": "get_time"}}]}, + {"type": "allowed_tools", "mode": "auto", "tools": [{"type": "mcp", "name": 7}]}, + {"type": "allowed_tools", "mode": "auto", "tools": [{"type": "mcp", "name": ""}]}, + {"type": "allowed_tools", "allowed_tools": {"mode": "auto", "tools": [{"type": "mcp", "name": ["get_time"]}]}}, + {"type": "allowed_tools", "allowed_tools": {"mode": "auto", "tools": [{"type": "mcp", "name": 7}]}}, +) + _CHAT_LAYOUT: Final[dict[str, object]] = { "type": "allowed_tools", "allowed_tools": { @@ -245,6 +255,7 @@ def test_a_choice_the_provider_cannot_take_answers_400_without_drop_params(): [ {"type": "allowed_tools", "mode": "required", "tools": []}, {"type": "allowed_tools", "tools": [{"type": "function", "name": "get_time"}]}, + *_CHOICES_WITH_A_NAME_THAT_IS_NOT_A_STRING, ], ) def test_a_malformed_choice_is_refused(tool_choice: object): @@ -254,6 +265,11 @@ def test_a_malformed_choice_is_refused(tool_choice: object): assert exc.value.status_code == 400 +@pytest.mark.parametrize("tool_choice", _CHOICES_WITH_A_NAME_THAT_IS_NOT_A_STRING) +def test_a_name_that_is_not_a_string_lists_no_tool(tool_choice: object): + assert tool_calls_the_choice_excludes(tool_choice, ("get_time",)) == ("get_time",) + + @pytest.mark.parametrize("tool_choice", [_FLAT_CHOICE, _CHAT_LAYOUT, "required", None]) def test_a_valid_choice_or_any_other_choice_is_not_called_malformed(tool_choice: object): assert refuse_a_malformed_allowed_tools_tool_choice(tool_choice, "the-model") is None diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index 3ed9452d0fd..3fb23afbef1 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -517,13 +517,23 @@ def test_completion_drop_params_drops_an_allowed_tools_tool_choice_it_cannot_sen @pytest.mark.asyncio -async def test_a_malformed_allowed_tools_tool_choice_stays_a_400_on_mcp_gateway_tools(respx_mock: respx.MockRouter): +@pytest.mark.parametrize( + "tool_choice", + [ + {"type": "allowed_tools", "mode": "required", "tools": []}, + {"type": "allowed_tools", "mode": "auto", "tools": [{"type": "mcp", "name": ["get_time"]}]}, + {"type": "allowed_tools", "allowed_tools": {"mode": "auto", "tools": [{"type": "mcp", "name": ["get_time"]}]}}, + ], +) +async def test_a_malformed_allowed_tools_tool_choice_stays_a_400_on_mcp_gateway_tools( + tool_choice: dict[str, object], respx_mock: respx.MockRouter +): with pytest.raises(litellm.BadRequestError, match="Invalid tool choice") as exc: await litellm.acompletion( model="openai/gpt-5.5", messages=[{"role": "user", "content": "What time is it?"}], tools=[{"type": "mcp", "server_url": "litellm_proxy/mcp/demo", "require_approval": "never"}], - tool_choice={"type": "allowed_tools", "mode": "required", "tools": []}, + tool_choice=tool_choice, drop_params=True, api_base="http://localhost:12346/v1", api_key="fake_api_key", diff --git a/tests/unit/test_tool_loop.py b/tests/unit/test_tool_loop.py index e9bf4a82e9c..a4cec7ec0bd 100644 --- a/tests/unit/test_tool_loop.py +++ b/tests/unit/test_tool_loop.py @@ -336,14 +336,20 @@ def test_a_tool_the_choice_excludes_never_runs_when_the_model_ignored_the_choice assert not _tools_run_before_the_refusal(_ALLOWED_WEATHER_ONLY, model="openai/gpt-5-mini") -def test_a_malformed_choice_is_refused_before_any_request(respx_mock: respx.MockRouter) -> None: +@pytest.mark.parametrize( + "tool_choice", + [_EMPTY_ALLOWED_TOOLS, {"type": "allowed_tools", "mode": "auto", "tools": [{"type": "mcp", "name": ["get_time"]}]}], +) +def test_a_malformed_choice_is_refused_before_any_request( + tool_choice: dict[str, object], respx_mock: respx.MockRouter +) -> None: with pytest.raises(litellm.BadRequestError, match="Invalid tool choice"): litellm.run_tool_loop( model="openai/gpt-5-mini", messages=[{"role": "user", "content": "hi"}], tools=WEATHER_TOOLS, execute_tool=_tool_result, - tool_choice=_EMPTY_ALLOWED_TOOLS, + tool_choice=tool_choice, drop_params=True, api_key="sk-test", )