fix: redact invalid tool argument errors (#4182)
Co-authored-by: Illia Oleksiuk <ilya.oleksiuk@gmail.com>
This commit is contained in:
+9
-1
@@ -13,6 +13,7 @@ from openai.types.responses.response_prompt_param import ResponsePromptParam
|
||||
from pydantic import BaseModel, TypeAdapter, ValidationError
|
||||
from typing_extensions import NotRequired, TypedDict
|
||||
|
||||
from . import _debug
|
||||
from ._tool_identity import get_function_tool_approval_keys
|
||||
from .agent_output import AgentOutputSchemaBase
|
||||
from .agent_tool_input import (
|
||||
@@ -681,10 +682,17 @@ class Agent(AgentBase, Generic[TContext]):
|
||||
)
|
||||
_log_function_tool_invocation(tool_name=tool_name, input_json=input_json)
|
||||
|
||||
base_message = f"Invalid JSON input for tool {tool_name}"
|
||||
validation_failed = False
|
||||
try:
|
||||
parsed_params = params_adapter.validate_python(json_data)
|
||||
except ValidationError as exc:
|
||||
raise ModelBehaviorError(f"Invalid JSON input for tool {tool_name}: {exc}") from exc
|
||||
if not _debug.DONT_LOG_TOOL_DATA:
|
||||
raise ModelBehaviorError(f"{base_message}: {exc}") from exc
|
||||
validation_failed = True
|
||||
|
||||
if validation_failed:
|
||||
raise ModelBehaviorError(base_message)
|
||||
|
||||
params_data = _normalize_tool_input(parsed_params, tool_name)
|
||||
resolved_input = await resolve_agent_tool_input(
|
||||
|
||||
@@ -530,19 +530,27 @@ def _validate_default_run_context_thread_id_suffix(value: str) -> str:
|
||||
|
||||
|
||||
def _parse_tool_input(parameters_model: type[BaseModel], input_json: str) -> BaseModel:
|
||||
base_message = "Invalid JSON input for codex tool"
|
||||
decode_failed = False
|
||||
try:
|
||||
json_data = json.loads(input_json) if input_json else {}
|
||||
except Exception as exc:
|
||||
if _debug.DONT_LOG_TOOL_DATA:
|
||||
logger.debug("Invalid JSON input for codex tool")
|
||||
else:
|
||||
logger.debug("Invalid JSON input for codex tool: %s", input_json)
|
||||
raise ModelBehaviorError(f"Invalid JSON input for codex tool: {input_json}") from exc
|
||||
if not _debug.DONT_LOG_TOOL_DATA:
|
||||
logger.debug("%s: %s", base_message, input_json)
|
||||
raise ModelBehaviorError(f"{base_message}: {input_json}") from exc
|
||||
logger.debug(base_message)
|
||||
decode_failed = True
|
||||
|
||||
if decode_failed:
|
||||
raise ModelBehaviorError(base_message)
|
||||
|
||||
try:
|
||||
return parameters_model.model_validate(json_data)
|
||||
except ValidationError as exc:
|
||||
raise ModelBehaviorError(f"Invalid JSON input for codex tool: {exc}") from exc
|
||||
if not _debug.DONT_LOG_TOOL_DATA:
|
||||
raise ModelBehaviorError(f"{base_message}: {exc}") from exc
|
||||
|
||||
raise ModelBehaviorError(base_message)
|
||||
|
||||
|
||||
def _normalize_parameters(params: BaseModel) -> CodexToolCallArguments:
|
||||
|
||||
+8
-1
@@ -2600,6 +2600,8 @@ def function_tool(
|
||||
json_data = _parse_function_tool_json_input(tool_name=tool_name, input_json=input)
|
||||
_log_function_tool_invocation(tool_name=tool_name, input_json=input)
|
||||
|
||||
base_message = f"Invalid JSON input for tool {tool_name}"
|
||||
validation_failed = False
|
||||
try:
|
||||
parsed = (
|
||||
schema.params_pydantic_model(**json_data)
|
||||
@@ -2607,7 +2609,12 @@ def function_tool(
|
||||
else schema.params_pydantic_model()
|
||||
)
|
||||
except ValidationError as e:
|
||||
raise ModelBehaviorError(f"Invalid JSON input for tool {tool_name}: {e}") from e
|
||||
if not _debug.DONT_LOG_TOOL_DATA:
|
||||
raise ModelBehaviorError(f"{base_message}: {e}") from e
|
||||
validation_failed = True
|
||||
|
||||
if validation_failed:
|
||||
raise ModelBehaviorError(base_message)
|
||||
|
||||
args, kwargs_dict = schema.to_call_args(parsed)
|
||||
|
||||
|
||||
@@ -12,7 +12,7 @@ from typing import Any, cast
|
||||
|
||||
import pytest
|
||||
from openai.types.responses import ResponseFunctionToolCall
|
||||
from pydantic import BaseModel, ConfigDict
|
||||
from pydantic import BaseModel, ConfigDict, ValidationError
|
||||
|
||||
import agents._debug as _debug
|
||||
from agents import Agent, function_tool
|
||||
@@ -2055,3 +2055,56 @@ def test_codex_tool_coerce_options_rejects_empty_run_context_key() -> None:
|
||||
"run_context_thread_id_key": " ",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
_CODEX_TOOL_ARGUMENT_SECRET = "SECRET_CODEX_TOOL_ARGUMENT_123"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"input_json, cause_type",
|
||||
[
|
||||
(
|
||||
f'{{"inputs": "{_CODEX_TOOL_ARGUMENT_SECRET}"}}',
|
||||
ValidationError,
|
||||
),
|
||||
(
|
||||
f"not valid json {_CODEX_TOOL_ARGUMENT_SECRET}",
|
||||
json.JSONDecodeError,
|
||||
),
|
||||
],
|
||||
ids=["validation", "json_decode"],
|
||||
)
|
||||
@pytest.mark.parametrize("redact", [True, False], ids=["redacted", "diagnostic"])
|
||||
@pytest.mark.asyncio
|
||||
async def test_codex_tool_argument_errors_respect_tool_data_redaction(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
input_json: str,
|
||||
cause_type: type[Exception],
|
||||
redact: bool,
|
||||
) -> None:
|
||||
monkeypatch.setattr(_debug, "DONT_LOG_TOOL_DATA", redact)
|
||||
tool = codex_tool(
|
||||
CodexToolOptions(
|
||||
codex=cast(Codex, FakeCodex(CodexMockState())),
|
||||
failure_error_function=None,
|
||||
)
|
||||
)
|
||||
context = ToolContext(
|
||||
None,
|
||||
tool_name=tool.name,
|
||||
tool_call_id="call-1",
|
||||
tool_arguments=input_json,
|
||||
)
|
||||
|
||||
with pytest.raises(ModelBehaviorError) as exc_info:
|
||||
await tool.on_invoke_tool(context, input_json)
|
||||
|
||||
error = exc_info.value
|
||||
if redact:
|
||||
assert str(error) == "Invalid JSON input for codex tool"
|
||||
assert _CODEX_TOOL_ARGUMENT_SECRET not in str(error)
|
||||
assert error.__cause__ is None
|
||||
assert error.__context__ is None
|
||||
else:
|
||||
assert _CODEX_TOOL_ARGUMENT_SECRET in str(error)
|
||||
assert isinstance(error.__cause__, cause_type)
|
||||
|
||||
@@ -20,15 +20,18 @@ from unittest.mock import patch
|
||||
import httpx
|
||||
import pytest
|
||||
from openai import AsyncOpenAI
|
||||
from pydantic import BaseModel, ValidationError
|
||||
|
||||
import agents._debug as _debug
|
||||
from agents import (
|
||||
Agent,
|
||||
ModelBehaviorError,
|
||||
ModelSettings,
|
||||
ModelTracing,
|
||||
OpenAIResponsesModel,
|
||||
RunConfig,
|
||||
RunContextWrapper,
|
||||
function_tool,
|
||||
trace,
|
||||
)
|
||||
from agents.logger import (
|
||||
@@ -48,6 +51,7 @@ from agents.run_internal.tool_execution import (
|
||||
resolve_approval_rejection_message,
|
||||
)
|
||||
from agents.run_state import _deserialize_items
|
||||
from agents.tool_context import ToolContext
|
||||
from agents.tracing.processor_interface import TracingProcessor
|
||||
from agents.tracing.provider import SynchronousMultiTracingProcessor
|
||||
from agents.tracing.spans import Span
|
||||
@@ -753,3 +757,104 @@ async def test_approval_rejection_formatter_error_logs_full_when_enabled(
|
||||
assert record.__dict__["openai_agents_diagnostic_context"] == {"tool_name": tool_name}
|
||||
assert record.exc_info is not None
|
||||
assert "SECRET_FMT_123" in caplog.text
|
||||
|
||||
|
||||
_TOOL_ARGUMENT_SECRET = "SECRET_TOOL_ARGUMENT_123"
|
||||
|
||||
|
||||
def _requires_integer_argument(value: int) -> str:
|
||||
return str(value)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_function_tool_validation_error_redacts_payload_when_tool_data_disabled(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.setattr(_debug, "DONT_LOG_TOOL_DATA", True)
|
||||
tool = function_tool(_requires_integer_argument, failure_error_function=None)
|
||||
payload = f'{{"value": "{_TOOL_ARGUMENT_SECRET}"}}'
|
||||
|
||||
with pytest.raises(ModelBehaviorError) as exc_info:
|
||||
await tool.on_invoke_tool(
|
||||
ToolContext(None, tool_name=tool.name, tool_call_id="1", tool_arguments=payload),
|
||||
payload,
|
||||
)
|
||||
|
||||
error = exc_info.value
|
||||
assert str(error) == f"Invalid JSON input for tool {tool.name}"
|
||||
assert _TOOL_ARGUMENT_SECRET not in str(error)
|
||||
assert error.__cause__ is None
|
||||
assert error.__context__ is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_function_tool_validation_error_preserves_diagnostics_when_tool_data_enabled(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.setattr(_debug, "DONT_LOG_TOOL_DATA", False)
|
||||
tool = function_tool(_requires_integer_argument, failure_error_function=None)
|
||||
payload = f'{{"value": "{_TOOL_ARGUMENT_SECRET}"}}'
|
||||
|
||||
with pytest.raises(ModelBehaviorError) as exc_info:
|
||||
await tool.on_invoke_tool(
|
||||
ToolContext(None, tool_name=tool.name, tool_call_id="1", tool_arguments=payload),
|
||||
payload,
|
||||
)
|
||||
|
||||
error = exc_info.value
|
||||
assert _TOOL_ARGUMENT_SECRET in str(error)
|
||||
assert isinstance(error.__cause__, ValidationError)
|
||||
|
||||
|
||||
class _AgentToolParameters(BaseModel):
|
||||
value: int
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_agent_tool_validation_error_redacts_payload_when_tool_data_disabled(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.setattr(_debug, "DONT_LOG_TOOL_DATA", True)
|
||||
tool = Agent(name="worker").as_tool(
|
||||
tool_name="worker_tool",
|
||||
tool_description="Runs the worker agent.",
|
||||
parameters=_AgentToolParameters,
|
||||
failure_error_function=None,
|
||||
)
|
||||
payload = f'{{"value": "{_TOOL_ARGUMENT_SECRET}"}}'
|
||||
|
||||
with pytest.raises(ModelBehaviorError) as exc_info:
|
||||
await tool.on_invoke_tool(
|
||||
ToolContext(None, tool_name=tool.name, tool_call_id="1", tool_arguments=payload),
|
||||
payload,
|
||||
)
|
||||
|
||||
error = exc_info.value
|
||||
assert str(error) == f"Invalid JSON input for tool {tool.name}"
|
||||
assert _TOOL_ARGUMENT_SECRET not in str(error)
|
||||
assert error.__cause__ is None
|
||||
assert error.__context__ is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_agent_tool_validation_error_preserves_diagnostics_when_tool_data_enabled(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.setattr(_debug, "DONT_LOG_TOOL_DATA", False)
|
||||
tool = Agent(name="worker").as_tool(
|
||||
tool_name="worker_tool",
|
||||
tool_description="Runs the worker agent.",
|
||||
parameters=_AgentToolParameters,
|
||||
failure_error_function=None,
|
||||
)
|
||||
payload = f'{{"value": "{_TOOL_ARGUMENT_SECRET}"}}'
|
||||
|
||||
with pytest.raises(ModelBehaviorError) as exc_info:
|
||||
await tool.on_invoke_tool(
|
||||
ToolContext(None, tool_name=tool.name, tool_call_id="1", tool_arguments=payload),
|
||||
payload,
|
||||
)
|
||||
|
||||
error = exc_info.value
|
||||
assert _TOOL_ARGUMENT_SECRET in str(error)
|
||||
assert isinstance(error.__cause__, ValidationError)
|
||||
|
||||
@@ -487,7 +487,8 @@ async def test_schema_backed_direct_tool_preserves_argument_error_formatter() ->
|
||||
result = await failing_tool.on_invoke_tool(context, "{}")
|
||||
|
||||
assert result.startswith("An error occurred while running the tool. Please try again. Error:")
|
||||
assert "sku" in result
|
||||
assert "Invalid JSON input for tool failing_tool" in result
|
||||
assert "sku" not in result
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
Reference in New Issue
Block a user