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