From f988471e77ddb8e912eba155bb4f41c09a94398e Mon Sep 17 00:00:00 2001 From: Illia Oleksiuk Date: Wed, 29 Jul 2026 19:25:46 -0700 Subject: [PATCH] fix: redact tool-argument values from invalid-input errors When DONT_LOG_TOOL_DATA is set, the ModelBehaviorError raised for an invalid tool argument still embedded the raw values: pydantic's ValidationError string carries input_value=..., and `raise ... from e` attached the payload-bearing exception as __cause__/__context__, so the data surfaced in tracebacks and telemetry even though the adjacent log calls are already redacted. Follow the existing _parse_function_tool_json_input hardening: under DONT_LOG_TOOL_DATA, raise a fixed base message from outside the except block, so the ValidationError is neither interpolated into the message nor attached as __context__. The redacted path also avoids copying the exception to an outer local, so it is not retained in the raising frame where telemetry that captures frame locals could recover it. Full validation detail and exception chaining are preserved when DONT_LOG_TOOL_DATA is disabled. This covers all three tool-argument parsing surfaces: function tools (tool.py), agent tools (Agent.as_tool), and the experimental Codex tool. Add caller-level regression tests for each surface asserting the raw value is absent and __cause__/__context__ are None in redacted mode (and that no ValidationError is retained in the traceback frame locals), and present with chaining when enabled. --- src/agents/agent.py | 13 +- .../experimental/codex/codex_tool.py | 25 +++- src/agents/tool.py | 13 +- .../experiemental/codex/test_codex_tool.py | 88 ++++++++++++ tests/test_error_logging_redaction.py | 126 ++++++++++++++++++ tests/test_programmatic_tool_calling.py | 5 +- 6 files changed, 261 insertions(+), 9 deletions(-) diff --git a/src/agents/agent.py b/src/agents/agent.py index c4899b2a8b..9e187c6100 100644 --- a/src/agents/agent.py +++ b/src/agents/agent.py @@ -11,6 +11,7 @@ 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 ( @@ -657,10 +658,20 @@ async def _run_agent_impl(context: ToolContext, input_json: str) -> Any: ) _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 + # Under DONT_LOG_TOOL_DATA, do not copy the payload-bearing ValidationError to + # an outer local; the redacted error below is raised with no ValidationError in + # this frame's locals and no ``__cause__``/``__context__``. + 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( diff --git a/src/agents/extensions/experimental/codex/codex_tool.py b/src/agents/extensions/experimental/codex/codex_tool.py index 2c252e3d00..6e9c636a5c 100644 --- a/src/agents/extensions/experimental/codex/codex_tool.py +++ b/src/agents/extensions/experimental/codex/codex_tool.py @@ -530,19 +530,32 @@ 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) + # Under DONT_LOG_TOOL_DATA, do not copy the payload-bearing JSONDecodeError to an + # outer local; the redacted error below is raised with no exception in this frame's + # locals and no ``__cause__``/``__context__``. + 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 + + # Reached only when validation failed under DONT_LOG_TOOL_DATA; raised outside the + # ``except`` block with no ValidationError retained in this frame's locals. + raise ModelBehaviorError(base_message) def _normalize_parameters(params: BaseModel) -> CodexToolCallArguments: diff --git a/src/agents/tool.py b/src/agents/tool.py index babf56d6ee..5b96d80d74 100644 --- a/src/agents/tool.py +++ b/src/agents/tool.py @@ -2535,6 +2535,8 @@ async def _on_invoke_tool_impl(ctx: ToolContext[Any], input: str) -> Any: 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) @@ -2542,7 +2544,16 @@ async def _on_invoke_tool_impl(ctx: ToolContext[Any], input: str) -> Any: 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 + # Under DONT_LOG_TOOL_DATA, do not copy the payload-bearing ValidationError to + # an outer local. Python clears the ``except`` binding on exit, so the redacted + # error below is raised with no ValidationError in this frame's locals and no + # ``__cause__``/``__context__``. + validation_failed = True + + if validation_failed: + raise ModelBehaviorError(base_message) args, kwargs_dict = schema.to_call_args(parsed) diff --git a/tests/extensions/experiemental/codex/test_codex_tool.py b/tests/extensions/experiemental/codex/test_codex_tool.py index 3aeb7db73d..338fdc42cb 100644 --- a/tests/extensions/experiemental/codex/test_codex_tool.py +++ b/tests/extensions/experiemental/codex/test_codex_tool.py @@ -2055,3 +2055,91 @@ def test_codex_tool_coerce_options_rejects_empty_run_context_key() -> None: "run_context_thread_id_key": " ", } ) + + +class _CodexRedactionModel(BaseModel): + value: int + + +_CODEX_ARG_SECRET = "SECRET_SSN_123-45-6789" + + +@pytest.mark.parametrize( + "input_json", + [ + '{"value": "SECRET_SSN_123-45-6789"}', + "not valid json SECRET_SSN_123-45-6789", + ], + ids=["validation_error", "json_decode_error"], +) +def test_codex_parse_tool_input_redacts_payload_when_tool_data_disabled( + monkeypatch: pytest.MonkeyPatch, input_json: str +) -> None: + # Both the JSON-decode and the pydantic validation failure paths embed the raw model + # payload in their underlying exception. Neither may leak into the raised + # ModelBehaviorError (message, __cause__, or __context__) when tool-data logging is off. + monkeypatch.setattr(_debug, "DONT_LOG_TOOL_DATA", True) + + with pytest.raises(ModelBehaviorError) as exc_info: + codex_tool_module._parse_tool_input(_CodexRedactionModel, input_json) + + error = exc_info.value + assert _CODEX_ARG_SECRET not in str(error) + assert error.__cause__ is None + assert error.__context__ is None + + +@pytest.mark.parametrize( + "input_json", + [ + '{"value": "SECRET_SSN_123-45-6789"}', + "not valid json SECRET_SSN_123-45-6789", + ], + ids=["validation_error", "json_decode_error"], +) +def test_codex_parse_tool_input_includes_payload_when_tool_data_enabled( + monkeypatch: pytest.MonkeyPatch, input_json: str +) -> None: + monkeypatch.setattr(_debug, "DONT_LOG_TOOL_DATA", False) + + with pytest.raises(ModelBehaviorError) as exc_info: + codex_tool_module._parse_tool_input(_CodexRedactionModel, input_json) + + error = exc_info.value + assert _CODEX_ARG_SECRET in str(error) + assert error.__cause__ is not None + + +@pytest.mark.parametrize( + "input_json", + [ + '{"inputs": "SECRET_SSN_123-45-6789"}', + "not valid json SECRET_SSN_123-45-6789", + ], + ids=["validation_error", "json_decode_error"], +) +@pytest.mark.asyncio +async def test_codex_tool_on_invoke_redacts_payload_when_tool_data_disabled( + monkeypatch: pytest.MonkeyPatch, input_json: str +) -> None: + # Caller-level: the ModelBehaviorError escaping on_invoke_tool must be a fixed message + # with no payload in the message, __cause__, or __context__ when tool logging is off. + monkeypatch.setattr(_debug, "DONT_LOG_TOOL_DATA", True) + tool = codex_tool( + CodexToolOptions( + codex=cast(Codex, FakeCodex(CodexMockState())), + failure_error_function=None, + ) + ) + context = ToolContext( + context=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 + assert str(error) == "Invalid JSON input for codex tool" + assert _CODEX_ARG_SECRET not in str(error) + assert error.__cause__ is None + assert error.__context__ is None diff --git a/tests/test_error_logging_redaction.py b/tests/test_error_logging_redaction.py index 85ebd221b3..149e5a78af 100644 --- a/tests/test_error_logging_redaction.py +++ b/tests/test_error_logging_redaction.py @@ -20,15 +20,18 @@ 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 @@ 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,125 @@ def boom(_args): 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_ARG_SECRET = "SECRET_SSN_123-45-6789" + + +def _requires_int_tool_arg(value: int) -> str: + return str(value) + + +def _validation_errors_in_traceback_locals(error: BaseException) -> list[ValidationError]: + """Collect any ValidationError retained in the raised error's traceback frame locals. + + Telemetry that captures frame locals could otherwise recover the raw tool arguments from + the payload-bearing ValidationError even after ``__cause__``/``__context__`` are cleared. + """ + found: list[ValidationError] = [] + tb = error.__traceback__ + while tb is not None: + found.extend(v for v in tb.tb_frame.f_locals.values() if isinstance(v, ValidationError)) + tb = tb.tb_next + return found + + +@pytest.mark.asyncio +async def test_function_tool_validation_error_redacts_payload_when_tool_data_disabled( + monkeypatch, +) -> None: + # A mistyped tool argument raises a pydantic ValidationError whose string form embeds + # the raw offending value. That exception must not leak into the raised + # ModelBehaviorError (message, __cause__, or __context__) when tool-data logging is off. + monkeypatch.setattr(_debug, "DONT_LOG_TOOL_DATA", True) + tool = function_tool(_requires_int_tool_arg, failure_error_function=None) + payload = '{"value": "SECRET_SSN_123-45-6789"}' + + 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_ARG_SECRET not in str(error) + assert error.__cause__ is None + assert error.__context__ is None + assert _validation_errors_in_traceback_locals(error) == [] + + +@pytest.mark.asyncio +async def test_function_tool_validation_error_includes_payload_when_tool_data_enabled( + monkeypatch, +) -> None: + monkeypatch.setattr(_debug, "DONT_LOG_TOOL_DATA", False) + tool = function_tool(_requires_int_tool_arg, failure_error_function=None) + payload = '{"value": "SECRET_SSN_123-45-6789"}' + + 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_ARG_SECRET in str(error) + assert isinstance(error.__cause__, ValidationError) + + +class _AgentToolParams(BaseModel): + value: int + + +@pytest.mark.asyncio +async def test_agent_as_tool_validation_error_redacts_payload_when_tool_data_disabled( + monkeypatch, +) -> None: + # Agent.as_tool validates model-supplied arguments too; in redacted mode the raised error + # must be a fixed message with no value in the message, __cause__, or __context__. + 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=_AgentToolParams, + failure_error_function=None, + ) + payload = '{"value": "SECRET_SSN_123-45-6789"}' + + 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_ARG_SECRET not in str(error) + assert error.__cause__ is None + assert error.__context__ is None + assert _validation_errors_in_traceback_locals(error) == [] + + +@pytest.mark.asyncio +async def test_agent_as_tool_validation_error_includes_payload_when_tool_data_enabled( + 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=_AgentToolParams, + failure_error_function=None, + ) + payload = '{"value": "SECRET_SSN_123-45-6789"}' + + 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_ARG_SECRET in str(error) + assert isinstance(error.__cause__, ValidationError) diff --git a/tests/test_programmatic_tool_calling.py b/tests/test_programmatic_tool_calling.py index 3236a7fbc0..bbe9aaef66 100644 --- a/tests/test_programmatic_tool_calling.py +++ b/tests/test_programmatic_tool_calling.py @@ -487,7 +487,10 @@ def failing_tool(sku: str) -> InventoryOutput: 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 + # In redacted mode (DONT_LOG_TOOL_DATA default) the formatter surfaces the fixed base + # message without the argument field or value. + assert "Invalid JSON input for tool failing_tool" in result + assert "sku" not in result @pytest.mark.asyncio