mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-07 08:26:10 +00:00
fix(guardrails): extract tool names nested in additional_tools input items
Codex's responses-lite wire mode declares tools inside an `additional_tools` input item rather than in top-level `tools`, and the Mantle transformation hoists them back out before dispatch. The key/team allowed_tools check and ToolPolicyGuardrail both read only `tools`, so a restricted key could place any tool, web_search included, in `input` and have it forwarded unchecked. Walk `input` for those items during extraction so nested tools face the same allowlist as declared ones. A missing or non-list `tools` slot, and a plain string `input`, yield nothing rather than raising on the auth hot path.
This commit is contained in:
parent
a632e02920
commit
53d7e124b7
2 changed files with 74 additions and 1 deletions
|
|
@ -233,8 +233,28 @@ class OpenAIResponsesHandler(BaseTranslation):
|
|||
return str(server_label) if server_label else None
|
||||
return str(tool_type) if tool_type else None
|
||||
|
||||
@staticmethod
|
||||
def _tools_nested_in_input_item(item: object) -> tuple[object, ...]:
|
||||
"""Tools declared inside an ``additional_tools`` input item. Codex's responses-lite wire
|
||||
mode ships tool definitions there instead of in top-level ``tools``, and providers hoist
|
||||
them back out before dispatch, so reading only ``tools`` would miss them."""
|
||||
if not isinstance(item, dict) or item.get("type") != "additional_tools":
|
||||
return ()
|
||||
tools: Final = item.get("tools")
|
||||
return tuple(tools) if isinstance(tools, list) else ()
|
||||
|
||||
def extract_request_tool_names(self, data: dict) -> list[str]:
|
||||
return [name for tool in data.get("tools") or [] if (name := self._request_tool_name(tool)) is not None]
|
||||
input_items: Final = data.get("input")
|
||||
nested: Final = (
|
||||
tuple(tool for item in input_items for tool in self._tools_nested_in_input_item(item))
|
||||
if isinstance(input_items, list)
|
||||
else ()
|
||||
)
|
||||
return [
|
||||
name
|
||||
for tool in (*(data.get("tools") or []), *nested)
|
||||
if (name := self._request_tool_name(tool)) is not None
|
||||
]
|
||||
|
||||
def _extract_and_transform_tools(
|
||||
self,
|
||||
|
|
|
|||
|
|
@ -105,6 +105,41 @@ class TestExtractRequestToolNames:
|
|||
"dmcp",
|
||||
]
|
||||
|
||||
def test_openai_responses_tools_nested_in_additional_tools_input_item(self):
|
||||
"""Codex's responses-lite wire mode declares tools inside an `additional_tools` input
|
||||
item, and providers hoist them into top-level `tools` before dispatch. Reading only
|
||||
`tools` would let a restricted key smuggle any tool through `input`
|
||||
(VERIA finding on PR #37995)."""
|
||||
data = {
|
||||
"input": [
|
||||
{"type": "message", "role": "user", "content": "hi"},
|
||||
{
|
||||
"type": "additional_tools",
|
||||
"role": "developer",
|
||||
"tools": [{"type": "web_search"}, {"type": "function", "name": "run_sql"}],
|
||||
},
|
||||
],
|
||||
"tools": [{"type": "function", "name": "declared_up_front"}],
|
||||
}
|
||||
assert extract_request_tool_names("/v1/responses", data) == [
|
||||
"declared_up_front",
|
||||
"web_search",
|
||||
"run_sql",
|
||||
]
|
||||
|
||||
def test_openai_responses_malformed_additional_tools_yields_no_name(self):
|
||||
"""An `additional_tools` item with a missing or non-list `tools` slot, and a plain string
|
||||
input, must not raise on the auth hot path."""
|
||||
data = {
|
||||
"input": [
|
||||
{"type": "additional_tools"},
|
||||
{"type": "additional_tools", "tools": "not-a-list"},
|
||||
"junk",
|
||||
]
|
||||
}
|
||||
assert extract_request_tool_names("/v1/responses", data) == []
|
||||
assert extract_request_tool_names("/v1/responses", {"input": "plain string"}) == []
|
||||
|
||||
def test_openai_responses_unnamed_tool_yields_no_name(self):
|
||||
"""A function or custom tool missing its name must not fall back to the bare type:
|
||||
that would let "function" satisfy an allowlist that never granted the real tool."""
|
||||
|
|
@ -280,6 +315,24 @@ class TestCheckToolsAllowlist:
|
|||
)
|
||||
assert body["tools"] == tools
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_disallowed_tool_nested_in_input_raises_on_responses_route(self):
|
||||
token = _token(metadata={"allowed_tools": ["run_sql"]})
|
||||
body = {
|
||||
"input": [
|
||||
{"type": "additional_tools", "role": "developer", "tools": [{"type": "web_search"}]},
|
||||
]
|
||||
}
|
||||
with pytest.raises(ProxyException) as exc_info:
|
||||
await check_tools_allowlist(
|
||||
request_body=body,
|
||||
valid_token=token,
|
||||
team_object=None,
|
||||
route="/v1/responses",
|
||||
)
|
||||
assert exc_info.value.type == ProxyErrorTypes.tool_access_denied
|
||||
assert "web_search" in str(exc_info.value.message)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_team_allowlist_used_when_key_empty(self):
|
||||
token = _token(
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue