mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-09 22:31:41 +00:00
fix(responses): lift additional_tools input items into tools on the chat bridge
An additional_tools input item carries tool definitions but no content, so the Responses -> Chat Completions conversion dropped it silently and the model was offered no tools at all. Lift the nested tools into the converted tool list. Codex CLI emits this shape, so a Codex session against any provider without a native Responses config lost its entire toolset with no error and no log line. Lifting makes those tools live, so the /v1/responses allowlist and guardrail extractor has to see them too; otherwise a nested tool reaches the model without passing tool authorization. The bridge and the extractor now share one parser so they cannot disagree about what the effective tool list is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Leonardo Freitas dos Santos
parent
b03e913ccf
commit
9b9e67052e
6 changed files with 576 additions and 45 deletions
|
|
@ -1,5 +1,6 @@
|
|||
import types
|
||||
from abc import ABC, abstractmethod
|
||||
from collections.abc import Iterable, Mapping, Sequence
|
||||
from typing import TYPE_CHECKING, Any, Final, cast
|
||||
|
||||
import httpx
|
||||
|
|
@ -14,6 +15,65 @@ from litellm.types.responses.main import *
|
|||
from litellm.types.router import GenericLiteLLMParams
|
||||
from litellm.types.utils import LlmProviders
|
||||
|
||||
# Codex CLI ships tool definitions inside an input item of this type rather than the
|
||||
# top-level ``tools`` array. One parser is shared by every caller so they cannot
|
||||
# disagree: the chat bridge lifts these tools so the model sees them, and the
|
||||
# authorization/guardrail extractors read them so a nested tool cannot slip past an
|
||||
# allowlist that only inspects ``tools``.
|
||||
ADDITIONAL_TOOLS_INPUT_ITEM_TYPE: Final = "additional_tools"
|
||||
NAMESPACE_TOOL_TYPE: Final = "namespace"
|
||||
|
||||
|
||||
def _tools_held_by(item: object, container_type: str) -> tuple[object, ...] | None:
|
||||
"""Tools held by ``item`` when it is a container of ``container_type``, else ``None``.
|
||||
|
||||
Returns an empty tuple for a malformed container, so callers can still recognise it.
|
||||
"""
|
||||
if not isinstance(item, Mapping):
|
||||
return None
|
||||
entry: Final = cast("Mapping[str, object]", item) # cast-ok: request items reach here as untyped mappings
|
||||
if entry.get("type") != container_type:
|
||||
return None
|
||||
nested: Final = entry.get("tools")
|
||||
if isinstance(nested, Sequence) and not isinstance(nested, (str, bytes)):
|
||||
return tuple(cast("Sequence[object]", nested)) # cast-ok: nested tool entries are untyped
|
||||
return ()
|
||||
|
||||
|
||||
def additional_tools_of(item: object) -> tuple[object, ...] | None:
|
||||
"""Nested tools when ``item`` is an ``additional_tools`` input item, else ``None``."""
|
||||
return _tools_held_by(item, ADDITIONAL_TOOLS_INPUT_ITEM_TYPE)
|
||||
|
||||
|
||||
def flatten_namespace_tools(tools: Iterable[object]) -> tuple[object, ...]:
|
||||
"""Expand ``namespace`` containers into the tools they hold, leaving others as-is.
|
||||
|
||||
Codex groups its tools under namespaces (``functions``, ``collaboration``). A
|
||||
namespace entry carries only the group name, so anything reading tool *names* --
|
||||
allowlist and guardrail extraction -- must look at the leaves or it sees nothing
|
||||
enforceable at all.
|
||||
"""
|
||||
flattened: Final[list[object]] = [] # mutable-ok: accumulator, returned as a tuple
|
||||
for tool in tools:
|
||||
members = _tools_held_by(tool, NAMESPACE_TOOL_TYPE) # rebind-ok: loop-local; Final is invalid in a loop
|
||||
if members is None:
|
||||
flattened.append(tool)
|
||||
else:
|
||||
flattened.extend(members)
|
||||
return tuple(flattened)
|
||||
|
||||
|
||||
def additional_tools_in(input: object) -> tuple[object, ...]:
|
||||
"""Every tool nested in ``additional_tools`` items, leaving ``input`` untouched.
|
||||
|
||||
For callers that must see the effective tool list without rewriting the request,
|
||||
such as allowlist and guardrail extraction.
|
||||
"""
|
||||
if not isinstance(input, list):
|
||||
return ()
|
||||
return tuple(tool for item in input for tool in (additional_tools_of(item) or ()))
|
||||
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from litellm.litellm_core_utils.litellm_logging import Logging as _LiteLLMLoggingObj
|
||||
|
||||
|
|
|
|||
|
|
@ -28,7 +28,7 @@ Output: response.output is List[GenericResponseOutputItem] where each has:
|
|||
- text: str
|
||||
"""
|
||||
|
||||
from collections.abc import Sequence
|
||||
from collections.abc import Mapping, Sequence
|
||||
from typing import TYPE_CHECKING, Any, Final, Union, cast
|
||||
|
||||
from openai.types.responses.response_function_tool_call import ResponseFunctionToolCall
|
||||
|
|
@ -41,6 +41,10 @@ from litellm.completion_extras.litellm_responses_transformation.transformation i
|
|||
OpenAiResponsesToChatCompletionStreamIterator,
|
||||
)
|
||||
from litellm.llms.base_llm.guardrail_translation.base_translation import BaseTranslation
|
||||
from litellm.llms.base_llm.responses.transformation import (
|
||||
additional_tools_in,
|
||||
flatten_namespace_tools,
|
||||
)
|
||||
from litellm.responses.litellm_completion_transformation.transformation import (
|
||||
LiteLLMCompletionResponsesConfig,
|
||||
)
|
||||
|
|
@ -174,9 +178,7 @@ class OpenAIResponsesHandler(BaseTranslation):
|
|||
task_mappings=task_mappings,
|
||||
)
|
||||
|
||||
# Extract and transform tools if present
|
||||
if "tools" in data and data["tools"]:
|
||||
self._extract_and_transform_tools(data["tools"], tools_to_check)
|
||||
mergeable_tool_count: Final = self._collect_tools_for_guardrail(data, input_data, tools_to_check)
|
||||
|
||||
# Step 2: Apply guardrail to all texts in batch
|
||||
if texts_to_check:
|
||||
|
|
@ -203,6 +205,8 @@ class OpenAIResponsesHandler(BaseTranslation):
|
|||
data,
|
||||
original_tools_list,
|
||||
guardrailed_inputs.get("tools"),
|
||||
mergeable_tool_count=mergeable_tool_count,
|
||||
inspected_tool_count=len(tools_to_check),
|
||||
)
|
||||
|
||||
# Step 3: Map guardrail responses back to original input structure
|
||||
|
|
@ -218,9 +222,21 @@ class OpenAIResponsesHandler(BaseTranslation):
|
|||
|
||||
def extract_request_tool_names(self, data: dict) -> list[str]:
|
||||
"""Extract tool names from Responses API request (tools[].name for function
|
||||
and custom, tools[].server_label for mcp)."""
|
||||
and custom, tools[].server_label for mcp).
|
||||
|
||||
Covers tools nested in ``additional_tools`` input items as well: the chat
|
||||
bridge lifts those into the effective tool list, so an allowlist that read
|
||||
only ``tools`` could be walked past by nesting a disallowed tool in ``input``.
|
||||
|
||||
``namespace`` containers are expanded to their leaves. Codex groups its tools
|
||||
that way, and a namespace entry carries only the group name -- reading it
|
||||
directly yields no enforceable tool name at all.
|
||||
"""
|
||||
names: Final[list[str]] = []
|
||||
for tool in data.get("tools") or []:
|
||||
candidates: Final = flatten_namespace_tools(
|
||||
(*(data.get("tools") or ()), *additional_tools_in(data.get("input")))
|
||||
)
|
||||
for tool in candidates:
|
||||
if not isinstance(tool, dict):
|
||||
continue
|
||||
if tool.get("type") in ("function", "custom") and tool.get("name"):
|
||||
|
|
@ -248,6 +264,52 @@ class OpenAIResponsesHandler(BaseTranslation):
|
|||
) = LiteLLMCompletionResponsesConfig.transform_responses_api_tools_to_chat_completion_tools(tools)
|
||||
tools_to_check.extend(cast(list[ChatCompletionToolParam], transformed_tools))
|
||||
|
||||
def _collect_tools_for_guardrail(
|
||||
self,
|
||||
data: Mapping[str, object],
|
||||
input_data: object,
|
||||
tools_to_check: list[ChatCompletionToolParam], # mutable-ok: extended in place for the caller
|
||||
) -> int:
|
||||
"""Fill ``tools_to_check`` for the guardrail and return the mergeable boundary.
|
||||
|
||||
Top-level ``data["tools"]`` go in first, then tools nested in ``additional_tools``
|
||||
input items. The nested ones matter because the chat bridge lifts them into the
|
||||
live tool list, so a guardrail that only saw ``data["tools"]`` would never get to
|
||||
block them -- and the previous gate skipped tool extraction entirely when
|
||||
``tools`` was empty, which is exactly the shape Codex sends.
|
||||
|
||||
The return value is how many leading entries came from ``data["tools"]`` and may
|
||||
therefore be merged back into the request.
|
||||
"""
|
||||
if data.get("tools"):
|
||||
self._extract_and_transform_tools(data["tools"], tools_to_check)
|
||||
mergeable: Final = len(tools_to_check)
|
||||
nested: Final = additional_tools_in(input_data)
|
||||
if nested:
|
||||
nested_list: Final = list(nested) # mutable-ok: the transform takes a list
|
||||
self._extract_and_transform_tools(
|
||||
cast("list[FunctionToolParam | OpenAIMcpServerTool]", nested_list), # cast-ok: untyped request data
|
||||
tools_to_check,
|
||||
)
|
||||
return mergeable
|
||||
|
||||
@staticmethod
|
||||
def _mergeable_guardrailed_tools(
|
||||
guardrailed_tools: list[ChatCompletionToolParam], # mutable-ok: matches the guardrail payload type
|
||||
mergeable_tool_count: int | None,
|
||||
inspected_tool_count: int | None,
|
||||
) -> list[ChatCompletionToolParam]: # mutable-ok: fed straight into _remap_tools_to_responses_api_format
|
||||
"""Guardrail output with the inspection-only nested tools removed.
|
||||
|
||||
Merging those back would hoist them into the request and hand the model two
|
||||
copies, since the chat bridge lifts them as well. Tools the guardrail appended
|
||||
past ``inspected_tool_count`` are kept, matching ``_merge_tools_after_guardrail``.
|
||||
"""
|
||||
if mergeable_tool_count is None:
|
||||
return guardrailed_tools
|
||||
tail: Final = guardrailed_tools[inspected_tool_count:] if inspected_tool_count is not None else ()
|
||||
return [*guardrailed_tools[:mergeable_tool_count], *tail] # mutable-ok: drops inspection-only
|
||||
|
||||
def _remap_tools_to_responses_api_format(self, guardrailed_tools: list[Any]) -> list[dict[str, object]]:
|
||||
"""
|
||||
Remap guardrail-returned tools (Chat Completion format) back to
|
||||
|
|
@ -291,11 +353,25 @@ class OpenAIResponsesHandler(BaseTranslation):
|
|||
data: dict,
|
||||
original_tools: list[dict[str, object]],
|
||||
guardrailed_tools: list[ChatCompletionToolParam] | None,
|
||||
mergeable_tool_count: int | None = None,
|
||||
inspected_tool_count: int | None = None,
|
||||
) -> None:
|
||||
"""Remap guardrailed tools to Responses API format and merge with original, then set data['tools']."""
|
||||
if guardrailed_tools is not None:
|
||||
remapped: Final = self._remap_tools_to_responses_api_format(guardrailed_tools)
|
||||
data["tools"] = self._merge_tools_after_guardrail(original_tools, remapped)
|
||||
"""Remap guardrailed tools to Responses API format and merge with original, then set data['tools'].
|
||||
|
||||
``mergeable_tool_count`` is how many leading entries came from ``data["tools"]``.
|
||||
Anything after it was a tool nested in an ``additional_tools`` input item, sent to
|
||||
the guardrail for inspection only: merging those would hoist them into the request
|
||||
and hand the model two copies of each, since the chat bridge lifts them as well.
|
||||
Tools the guardrail appended past ``inspected_tool_count`` are still kept, matching
|
||||
``_merge_tools_after_guardrail``.
|
||||
"""
|
||||
if guardrailed_tools is None:
|
||||
return
|
||||
checked: Final = self._mergeable_guardrailed_tools(
|
||||
guardrailed_tools, mergeable_tool_count, inspected_tool_count
|
||||
)
|
||||
remapped: Final = self._remap_tools_to_responses_api_format(checked)
|
||||
data["tools"] = self._merge_tools_after_guardrail(original_tools, remapped)
|
||||
|
||||
def _extract_input_text_and_images(
|
||||
self,
|
||||
|
|
|
|||
|
|
@ -37,6 +37,7 @@ from litellm.litellm_core_utils.get_supported_openai_params import (
|
|||
get_supported_openai_params,
|
||||
)
|
||||
from litellm.litellm_core_utils.litellm_logging import Logging as LiteLLMLoggingObj
|
||||
from litellm.llms.base_llm.responses.transformation import additional_tools_of
|
||||
from litellm.responses.litellm_completion_transformation.session_handler import (
|
||||
ResponsesSessionHandler,
|
||||
)
|
||||
|
|
@ -271,6 +272,28 @@ class LiteLLMCompletionResponsesConfig:
|
|||
)
|
||||
return supported_params is not None and "web_search_options" not in supported_params
|
||||
|
||||
@staticmethod
|
||||
def _lift_additional_tools(
|
||||
input: str | ResponseInputParam,
|
||||
) -> tuple[str | ResponseInputParam, tuple[object, ...]]:
|
||||
"""Pull tool definitions out of ``additional_tools`` input items.
|
||||
|
||||
Such items carry no ``content``, so input-to-messages conversion below drops
|
||||
them and their tools never reach the provider. Codex CLI emits them.
|
||||
"""
|
||||
if not isinstance(input, list):
|
||||
return input, ()
|
||||
|
||||
nested_per_item: Final = tuple(additional_tools_of(item) for item in input)
|
||||
if all(nested is None for nested in nested_per_item):
|
||||
return input, ()
|
||||
|
||||
# mutable-ok: the input->messages conversion narrows on isinstance(input, list),
|
||||
# so handing it a tuple silently yields no messages at all.
|
||||
kept: Final = [item for item, nested in zip(input, nested_per_item) if nested is None]
|
||||
lifted: Final = tuple(tool for nested in nested_per_item if nested is not None for tool in nested)
|
||||
return kept, lifted
|
||||
|
||||
@staticmethod
|
||||
def transform_responses_api_request_to_chat_completion_request(
|
||||
model: str,
|
||||
|
|
@ -284,11 +307,16 @@ class LiteLLMCompletionResponsesConfig:
|
|||
"""
|
||||
Transform a Responses API request into a Chat Completion request
|
||||
"""
|
||||
input, lifted_tools = LiteLLMCompletionResponsesConfig._lift_additional_tools(input)
|
||||
|
||||
(
|
||||
tools,
|
||||
web_search_options,
|
||||
) = LiteLLMCompletionResponsesConfig.transform_responses_api_tools_to_chat_completion_tools(
|
||||
responses_api_request.get("tools") or []
|
||||
cast( # cast-ok: additional_tools sits outside the input-item union, so lifted entries are untyped
|
||||
"list[FunctionToolParam | OpenAIMcpServerTool]",
|
||||
(*(responses_api_request.get("tools") or ()), *lifted_tools),
|
||||
)
|
||||
)
|
||||
|
||||
if web_search_options is not None and LiteLLMCompletionResponsesConfig._should_drop_derived_web_search_options(
|
||||
|
|
|
|||
|
|
@ -146,15 +146,9 @@ class TestOpenAIResponsesHandlerInputProcessing:
|
|||
|
||||
result = await handler.process_input_messages(data, guardrail)
|
||||
|
||||
assert (
|
||||
result["input"][0]["content"][0]["text"]
|
||||
== "Describe this image [GUARDRAILED]"
|
||||
)
|
||||
assert result["input"][0]["content"][0]["text"] == "Describe this image [GUARDRAILED]"
|
||||
# Image URL should remain unchanged
|
||||
assert (
|
||||
result["input"][0]["content"][1]["image_url"]["url"]
|
||||
== "https://example.com/image.jpg"
|
||||
)
|
||||
assert result["input"][0]["content"][1]["image_url"]["url"] == "https://example.com/image.jpg"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_process_input_with_empty_content(self):
|
||||
|
|
@ -572,10 +566,7 @@ class TestOpenAIResponsesHandlerToolCallExtraction:
|
|||
assert tool_call["id"] == "call_4SjsMeA6DUHwGKaE87ZojgOF"
|
||||
assert tool_call["type"] == "function"
|
||||
assert tool_call["function"]["name"] == "get_current_weather"
|
||||
assert (
|
||||
tool_call["function"]["arguments"]
|
||||
== '{"location":"Boston, MA","unit":"celsius"}'
|
||||
)
|
||||
assert tool_call["function"]["arguments"] == '{"location":"Boston, MA","unit":"celsius"}'
|
||||
assert tool_call["index"] == 0
|
||||
|
||||
def test_extract_tool_call_from_dict_format(self):
|
||||
|
|
@ -616,10 +607,7 @@ class TestOpenAIResponsesHandlerToolCallExtraction:
|
|||
assert tool_call["id"] == "call_4SjsMeA6DUHwGKaE87ZojgOF"
|
||||
assert tool_call["type"] == "function"
|
||||
assert tool_call["function"]["name"] == "get_current_weather"
|
||||
assert (
|
||||
tool_call["function"]["arguments"]
|
||||
== '{"location":"Boston, MA","unit":"celsius"}'
|
||||
)
|
||||
assert tool_call["function"]["arguments"] == '{"location":"Boston, MA","unit":"celsius"}'
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_process_output_response_with_tool_calls(self):
|
||||
|
|
@ -1020,9 +1008,7 @@ class TestOpenAIResponsesHandlerStreamingOutputProcessing:
|
|||
logging_obj: Optional[Any] = None,
|
||||
) -> GenericGuardrailAPIInputs:
|
||||
texts = inputs.get("texts", [])
|
||||
inputs["texts"] = [
|
||||
t.replace("<TOKEN_1>", "john@example.com") for t in texts
|
||||
]
|
||||
inputs["texts"] = [t.replace("<TOKEN_1>", "john@example.com") for t in texts]
|
||||
return inputs
|
||||
|
||||
handler = OpenAIResponsesHandler()
|
||||
|
|
@ -1058,15 +1044,11 @@ class TestOpenAIResponsesHandlerStreamingOutputProcessing:
|
|||
litellm_logging_obj=None,
|
||||
)
|
||||
|
||||
completed_chunk = next(
|
||||
c
|
||||
for c in result
|
||||
if isinstance(c, dict) and c.get("type") == "response.completed"
|
||||
)
|
||||
completed_chunk = next(c for c in result if isinstance(c, dict) and c.get("type") == "response.completed")
|
||||
output_text = completed_chunk["response"]["output"][0]["content"][0]["text"]
|
||||
assert (
|
||||
output_text == "send to john@example.com"
|
||||
), f"Expected PII token to be unmasked in response.completed output, got: {output_text!r}"
|
||||
assert output_text == "send to john@example.com", (
|
||||
f"Expected PII token to be unmasked in response.completed output, got: {output_text!r}"
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_process_output_streaming_response_pass_through_unchanged(self):
|
||||
|
|
@ -1145,9 +1127,7 @@ class TestGetStructuredMessages:
|
|||
}
|
||||
result = handler.get_structured_messages(data)
|
||||
assert result is not None
|
||||
has_system = any(
|
||||
isinstance(msg, dict) and msg.get("role") == "system" for msg in result
|
||||
)
|
||||
has_system = any(isinstance(msg, dict) and msg.get("role") == "system" for msg in result)
|
||||
assert has_system, f"Expected system message from instructions, got: {result}"
|
||||
|
||||
def test_should_return_none_when_no_input(self):
|
||||
|
|
@ -1229,3 +1209,134 @@ class TestOpenAIResponsesHandlerToolInjection:
|
|||
names = [t.get("name") for t in result["tools"]]
|
||||
assert "get_weather" in names
|
||||
assert "injected_tool" in names
|
||||
|
||||
|
||||
class ToolRecordingGuardrail(CustomGuardrail):
|
||||
"""Records the tools the handler hands to the guardrail, and returns them unchanged."""
|
||||
|
||||
def __init__(self, guardrail_name: str = "recorder"):
|
||||
super().__init__(guardrail_name=guardrail_name)
|
||||
self.seen_tools: List[Any] = []
|
||||
|
||||
async def apply_guardrail(
|
||||
self,
|
||||
inputs: GenericGuardrailAPIInputs,
|
||||
request_data: dict,
|
||||
input_type: Literal["request", "response"],
|
||||
logging_obj: Optional[Any] = None,
|
||||
) -> GenericGuardrailAPIInputs:
|
||||
self.seen_tools = list(inputs.get("tools") or [])
|
||||
return inputs
|
||||
|
||||
|
||||
def _additional_tools_item(name: str = "restricted_tool") -> dict:
|
||||
"""The Codex "responses lite" shape: tools nested in an input item, top-level empty."""
|
||||
return {
|
||||
"type": "additional_tools",
|
||||
"role": "developer",
|
||||
"tools": [
|
||||
{
|
||||
"type": "namespace",
|
||||
"name": "functions",
|
||||
"description": "Local tools",
|
||||
"tools": [
|
||||
{
|
||||
"type": "function",
|
||||
"name": name,
|
||||
"description": "x",
|
||||
"parameters": {"type": "object", "properties": {}},
|
||||
}
|
||||
],
|
||||
}
|
||||
],
|
||||
}
|
||||
|
||||
|
||||
class TestOpenAIResponsesHandlerAdditionalToolsGuardrailing:
|
||||
"""Tools nested in an ``additional_tools`` input item must reach the guardrail.
|
||||
|
||||
The Chat Completions bridge lifts them into the live tool list, so a guardrail that
|
||||
only inspected ``data["tools"]`` never got the chance to block them -- and the old
|
||||
gate skipped tool extraction entirely when ``tools`` was empty, which is exactly the
|
||||
shape Codex sends (VERIA finding on PR #38388).
|
||||
"""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_nested_tools_are_sent_to_the_guardrail(self):
|
||||
handler = OpenAIResponsesHandler()
|
||||
guardrail = ToolRecordingGuardrail()
|
||||
data = {
|
||||
"input": [
|
||||
_additional_tools_item(),
|
||||
{"role": "user", "content": "hi", "type": "message"},
|
||||
],
|
||||
"tools": [],
|
||||
"model": "gpt-4",
|
||||
}
|
||||
|
||||
await handler.process_input_messages(data, guardrail)
|
||||
|
||||
names = [(t.get("function") or {}).get("name") for t in guardrail.seen_tools]
|
||||
# The name is namespace-prefixed because the tool transform expands a namespace
|
||||
# container into "<namespace>__<tool>", the same form the bridge sends the model.
|
||||
assert "functions__restricted_tool" in names, "guardrail must see tools nested in input"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_nested_tools_are_not_merged_into_request_tools(self):
|
||||
"""Inspection only. Merging them here would hoist them at the proxy layer and the
|
||||
bridge would lift them again, handing the model two copies."""
|
||||
handler = OpenAIResponsesHandler()
|
||||
guardrail = ToolRecordingGuardrail()
|
||||
data = {
|
||||
"input": [
|
||||
_additional_tools_item(),
|
||||
{"role": "user", "content": "hi", "type": "message"},
|
||||
],
|
||||
"tools": [],
|
||||
"model": "gpt-4",
|
||||
}
|
||||
|
||||
result = await handler.process_input_messages(data, guardrail)
|
||||
|
||||
names = [t.get("name") for t in (result.get("tools") or [])]
|
||||
assert names == [], "nested tools must not be hoisted into the request here"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_top_level_tools_still_survive_alongside_nested_ones(self):
|
||||
handler = OpenAIResponsesHandler()
|
||||
guardrail = ToolRecordingGuardrail()
|
||||
data = {
|
||||
"input": [
|
||||
_additional_tools_item(),
|
||||
{"role": "user", "content": "hi", "type": "message"},
|
||||
],
|
||||
"tools": [{"type": "function", "name": "get_weather", "parameters": {"type": "object", "properties": {}}}],
|
||||
"model": "gpt-4",
|
||||
}
|
||||
|
||||
result = await handler.process_input_messages(data, guardrail)
|
||||
|
||||
seen = [(t.get("function") or {}).get("name") for t in guardrail.seen_tools]
|
||||
assert "get_weather" in seen and "functions__restricted_tool" in seen
|
||||
assert [t.get("name") for t in result["tools"]] == ["get_weather"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_guardrail_appended_tool_still_survives_with_nested_tools_present(self):
|
||||
"""The trim that drops inspection-only tools must not also drop injected ones."""
|
||||
handler = OpenAIResponsesHandler()
|
||||
guardrail = ToolAppendingGuardrail(guardrail_name="test")
|
||||
data = {
|
||||
"input": [
|
||||
_additional_tools_item(),
|
||||
{"role": "user", "content": "hi", "type": "message"},
|
||||
],
|
||||
"tools": [{"type": "function", "name": "get_weather", "parameters": {"type": "object", "properties": {}}}],
|
||||
"model": "gpt-4",
|
||||
}
|
||||
|
||||
result = await handler.process_input_messages(data, guardrail)
|
||||
|
||||
names = [t.get("name") for t in result["tools"]]
|
||||
assert "get_weather" in names
|
||||
assert "injected_tool" in names
|
||||
assert "restricted_tool" not in names
|
||||
|
|
|
|||
|
|
@ -58,10 +58,95 @@ class TestExtractRequestToolNames:
|
|||
{"type": "function", "name": "get_current_weather", "description": "x"},
|
||||
]
|
||||
}
|
||||
assert extract_request_tool_names("/v1/responses", data) == ["get_current_weather"]
|
||||
|
||||
def test_openai_responses_additional_tools_input_items(self):
|
||||
"""Codex CLI nests its tool definitions in an ``additional_tools`` input item
|
||||
and leaves top-level ``tools`` empty. The Chat Completions bridge lifts those
|
||||
into the effective tool list, so extraction must see them; otherwise a
|
||||
restricted key walks past the allowlist by nesting a disallowed tool -- an
|
||||
MCP reference with require_approval "never" included (VERIA finding on
|
||||
PR #38388)."""
|
||||
data = {
|
||||
"tools": [{"type": "function", "name": "get_current_weather"}],
|
||||
"input": [
|
||||
{"role": "user", "content": "hi"},
|
||||
{
|
||||
"type": "additional_tools",
|
||||
"role": "developer",
|
||||
"tools": [
|
||||
{"type": "custom", "name": "exec", "description": "x"},
|
||||
{"type": "mcp", "server_label": "dmcp", "require_approval": "never"},
|
||||
],
|
||||
},
|
||||
],
|
||||
}
|
||||
assert extract_request_tool_names("/v1/responses", data) == [
|
||||
"get_current_weather"
|
||||
"get_current_weather",
|
||||
"exec",
|
||||
"dmcp",
|
||||
]
|
||||
|
||||
def test_openai_responses_codex_namespaced_tools_are_extracted(self):
|
||||
"""The real Codex 0.149 wire shape: nine tools grouped under two ``namespace``
|
||||
containers inside an ``additional_tools`` item, with top-level ``tools`` empty.
|
||||
A namespace entry carries only the group name, so without descending into it the
|
||||
allowlist extracts nothing enforceable at all."""
|
||||
data = {
|
||||
"tools": [],
|
||||
"input": [
|
||||
{
|
||||
"type": "additional_tools",
|
||||
"role": "developer",
|
||||
"tools": [
|
||||
{
|
||||
"type": "namespace",
|
||||
"name": "functions",
|
||||
"tools": [
|
||||
{"type": "custom", "name": "exec"},
|
||||
{"type": "function", "name": "wait"},
|
||||
{"type": "function", "name": "request_user_input"},
|
||||
],
|
||||
},
|
||||
{
|
||||
"type": "namespace",
|
||||
"name": "collaboration",
|
||||
"tools": [
|
||||
{"type": "function", "name": "followup_task"},
|
||||
{"type": "function", "name": "interrupt_agent"},
|
||||
{"type": "function", "name": "list_agents"},
|
||||
{"type": "function", "name": "send_message"},
|
||||
{"type": "function", "name": "spawn_agent"},
|
||||
{"type": "function", "name": "wait_agent"},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
{"role": "user", "content": "hi"},
|
||||
],
|
||||
}
|
||||
assert extract_request_tool_names("/v1/responses", data) == [
|
||||
"exec",
|
||||
"wait",
|
||||
"request_user_input",
|
||||
"followup_task",
|
||||
"interrupt_agent",
|
||||
"list_agents",
|
||||
"send_message",
|
||||
"spawn_agent",
|
||||
"wait_agent",
|
||||
]
|
||||
|
||||
def test_openai_responses_top_level_namespace_tools_are_extracted(self):
|
||||
data = {
|
||||
"tools": [{"type": "namespace", "name": "functions", "tools": [{"type": "function", "name": "read_file"}]}]
|
||||
}
|
||||
assert extract_request_tool_names("/v1/responses", data) == ["read_file"]
|
||||
|
||||
def test_openai_responses_string_input_is_ignored(self):
|
||||
data = {"tools": [{"type": "function", "name": "get_current_weather"}], "input": "hi"}
|
||||
assert extract_request_tool_names("/v1/responses", data) == ["get_current_weather"]
|
||||
|
||||
def test_openai_responses_mcp_tools(self):
|
||||
data = {
|
||||
"tools": [
|
||||
|
|
@ -103,9 +188,7 @@ class TestExtractRequestToolNames:
|
|||
},
|
||||
]
|
||||
}
|
||||
assert extract_request_tool_names("/generate_content", data) == [
|
||||
"schedule_meeting"
|
||||
]
|
||||
assert extract_request_tool_names("/generate_content", data) == ["schedule_meeting"]
|
||||
|
||||
def test_mcp_call_tool_name(self):
|
||||
data = {"name": "my_tool", "arguments": {}}
|
||||
|
|
@ -219,3 +302,58 @@ class TestCheckToolsAllowlist:
|
|||
team_object=None,
|
||||
route="/v1/chat/completions",
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_disallowed_tool_nested_in_additional_tools_raises_on_responses_route(self):
|
||||
"""The bridge lifts nested tools into the request, so nesting must not be a
|
||||
way around the allowlist (VERIA finding on PR #38388)."""
|
||||
token = _token(metadata={"allowed_tools": ["other_tool"]})
|
||||
body = {
|
||||
"input": [
|
||||
{
|
||||
"type": "additional_tools",
|
||||
"role": "developer",
|
||||
"tools": [{"type": "custom", "name": "restricted_tool"}],
|
||||
},
|
||||
],
|
||||
}
|
||||
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 "restricted_tool" in str(exc_info.value.message)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_disallowed_tool_inside_a_namespace_raises_on_responses_route(self):
|
||||
"""The shape Codex actually sends: disallowed tool inside a namespace, inside an
|
||||
additional_tools input item (VERIA finding on PR #38388)."""
|
||||
token = _token(metadata={"allowed_tools": ["other_tool"]})
|
||||
body = {
|
||||
"tools": [],
|
||||
"input": [
|
||||
{
|
||||
"type": "additional_tools",
|
||||
"role": "developer",
|
||||
"tools": [
|
||||
{
|
||||
"type": "namespace",
|
||||
"name": "collaboration",
|
||||
"tools": [{"type": "function", "name": "spawn_agent"}],
|
||||
}
|
||||
],
|
||||
}
|
||||
],
|
||||
}
|
||||
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 "spawn_agent" in str(exc_info.value.message)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,118 @@
|
|||
"""``additional_tools`` input items carry tool definitions that belong in ``tools``.
|
||||
|
||||
The item has no ``content``, so input-to-messages conversion drops it and the tools
|
||||
never reach the provider. Codex CLI emits this shape.
|
||||
"""
|
||||
|
||||
import json
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
|
||||
from litellm.responses.litellm_completion_transformation.transformation import (
|
||||
LiteLLMCompletionResponsesConfig,
|
||||
)
|
||||
|
||||
lift = LiteLLMCompletionResponsesConfig._lift_additional_tools
|
||||
|
||||
|
||||
def _fn(name: str) -> dict[str, Any]:
|
||||
return {
|
||||
"type": "function",
|
||||
"name": name,
|
||||
"description": f"{name} tool",
|
||||
"parameters": {"type": "object", "properties": {}},
|
||||
"strict": False,
|
||||
}
|
||||
|
||||
|
||||
def _item() -> dict[str, Any]:
|
||||
"""Shaped like a real Codex 0.149 request: namespaces wrapping nested tools."""
|
||||
return {
|
||||
"type": "additional_tools",
|
||||
"role": "developer",
|
||||
"tools": [
|
||||
{"type": "namespace", "name": "functions", "description": "Local", "tools": [_fn("wait")]},
|
||||
{"type": "namespace", "name": "collaboration", "description": "Agents", "tools": [_fn("spawn_agent")]},
|
||||
],
|
||||
}
|
||||
|
||||
|
||||
class TestLiftAdditionalTools:
|
||||
def test_lifts_tools_and_strips_the_item(self):
|
||||
item = _item()
|
||||
kept, lifted = lift([{"role": "user", "content": "hi"}, item])
|
||||
assert kept == [{"role": "user", "content": "hi"}]
|
||||
assert lifted == tuple(item["tools"])
|
||||
|
||||
def test_multiple_items_lift_in_order(self):
|
||||
first = {"type": "additional_tools", "role": "developer", "tools": [_fn("a")]}
|
||||
second = {"type": "additional_tools", "role": "developer", "tools": [_fn("b")]}
|
||||
kept, lifted = lift([first, {"role": "user", "content": "x"}, second])
|
||||
assert kept == [{"role": "user", "content": "x"}]
|
||||
assert [t["name"] for t in lifted] == ["a", "b"]
|
||||
|
||||
def test_string_input_passes_through(self):
|
||||
assert lift("just a prompt") == ("just a prompt", ())
|
||||
|
||||
def test_input_without_the_item_is_returned_unchanged(self):
|
||||
original = [{"role": "user", "content": "hi"}]
|
||||
kept, lifted = lift(original)
|
||||
assert kept is original
|
||||
assert lifted == ()
|
||||
|
||||
def test_non_mapping_items_are_left_alone(self):
|
||||
"""Input lists can carry non-mapping entries; they are not tool containers."""
|
||||
original = ["a bare string", 42, None, {"role": "user", "content": "hi"}]
|
||||
kept, lifted = lift(original)
|
||||
assert kept is original
|
||||
assert lifted == ()
|
||||
|
||||
@pytest.mark.parametrize("malformed", [{}, {"tools": None}, {"tools": "not-a-list"}])
|
||||
def test_malformed_item_is_still_stripped(self, malformed):
|
||||
kept, lifted = lift([{"type": "additional_tools", **malformed}, {"role": "user", "content": "hi"}])
|
||||
assert kept == [{"role": "user", "content": "hi"}]
|
||||
assert lifted == ()
|
||||
|
||||
|
||||
class TestAdditionalToolsThroughTheBridge:
|
||||
@staticmethod
|
||||
def _bridge(input_, tools=None):
|
||||
return LiteLLMCompletionResponsesConfig.transform_responses_api_request_to_chat_completion_request(
|
||||
model="gpt-5.6-sol",
|
||||
input=input_,
|
||||
responses_api_request={"tools": tools} if tools is not None else {},
|
||||
custom_llm_provider="bedrock",
|
||||
)
|
||||
|
||||
def test_nested_tools_reach_the_chat_request(self):
|
||||
request = self._bridge([_item()], tools=[])
|
||||
assert request.get("tools"), "lifted tools must reach the chat request"
|
||||
serialized = json.dumps(request["tools"])
|
||||
assert "wait" in serialized
|
||||
assert "spawn_agent" in serialized
|
||||
|
||||
def test_top_level_tools_are_preserved_alongside_lifted_ones(self):
|
||||
request = self._bridge([_item()], tools=[_fn("already_here")])
|
||||
names = json.dumps(request["tools"])
|
||||
assert "already_here" in names
|
||||
assert "spawn_agent" in names
|
||||
|
||||
def test_request_without_tools_still_sends_none(self):
|
||||
"""A request that genuinely has no tools must not gain ``tools: []`` —
|
||||
some providers reject an empty array."""
|
||||
request = self._bridge([{"role": "user", "content": "hi"}])
|
||||
assert "tools" not in request
|
||||
assert "tool_choice" not in request
|
||||
|
||||
def test_user_messages_survive_the_lift(self):
|
||||
"""Regression: the surviving input must stay a list. The input-to-messages
|
||||
conversion narrows on isinstance(input, list), so returning a tuple produced
|
||||
zero messages and Bedrock rejected the request outright."""
|
||||
request = self._bridge(
|
||||
[_item(), {"role": "user", "content": "read a file for me"}],
|
||||
tools=[],
|
||||
)
|
||||
contents = json.dumps(request["messages"])
|
||||
assert request["messages"], "the user turn must survive"
|
||||
assert "read a file for me" in contents
|
||||
Loading…
Add table
Reference in a new issue