From eac6d33bf34d42fe2d41c60fb31048c38b0b2658 Mon Sep 17 00:00:00 2001 From: abhay-codes07 Date: Wed, 5 Aug 2026 17:13:28 +0530 Subject: [PATCH] fix: redact model output from JSON validation errors validate_json embedded the offending JSON in its ModelBehaviorError message and chained the pydantic ValidationError, which repeats the same payload in its own input_value. Both surfaced regardless of DONT_LOG_MODEL_DATA, so structured output that failed schema validation leaked into the exception, its cause chain and any traceback the application logged. This is the model side of the tool argument redaction added in #4182, and it covers the three callers that pass model generated JSON: output schema validation, handoff input validation and its realtime equivalent. Follow the same shape as that fix: keep the detailed error when the flag allows it, and otherwise raise outside the except block so neither __cause__ nor __context__ keeps the ValidationError reachable. The span error was already payload free and is unchanged. --- src/agents/util/_json.py | 14 ++++- tests/test_error_logging_redaction.py | 90 ++++++++++++++++++++++++++- 2 files changed, 100 insertions(+), 4 deletions(-) diff --git a/src/agents/util/_json.py b/src/agents/util/_json.py index 67186328cd..b584f80676 100644 --- a/src/agents/util/_json.py +++ b/src/agents/util/_json.py @@ -6,6 +6,7 @@ from pydantic import TypeAdapter, ValidationError from typing_extensions import TypeVar +from .. import _debug from ..exceptions import ModelBehaviorError from ..tracing import SpanError from ._error_tracing import attach_error_to_current_span @@ -32,9 +33,16 @@ def validate_json( data={}, ) ) - raise ModelBehaviorError( - f"Invalid JSON when parsing {json_str} for {type_adapter}; {e}" - ) from e + if not _debug.DONT_LOG_MODEL_DATA: + raise ModelBehaviorError( + f"Invalid JSON when parsing {json_str} for {type_adapter}; {e}" + ) from e + + # Only reachable when the model-data policy suppressed the detailed error above. This is + # raised outside the except block on purpose: chaining it would keep the ValidationError + # reachable through __cause__ and __context__, and that error embeds the offending model + # output in its own message. + raise ModelBehaviorError(f"Invalid JSON when parsing model output for {type_adapter}") def _to_dump_compatible(obj: Any) -> Any: diff --git a/tests/test_error_logging_redaction.py b/tests/test_error_logging_redaction.py index b194c6f41b..70b3b42b0b 100644 --- a/tests/test_error_logging_redaction.py +++ b/tests/test_error_logging_redaction.py @@ -20,7 +20,7 @@ import httpx import pytest from openai import AsyncOpenAI -from pydantic import BaseModel, ValidationError +from pydantic import BaseModel, TypeAdapter, ValidationError import agents._debug as _debug from agents import ( @@ -31,9 +31,11 @@ OpenAIResponsesModel, RunConfig, RunContextWrapper, + Runner, function_tool, trace, ) +from agents.agent_output import AgentOutputSchema from agents.logger import ( log_model_action_debug, log_model_action_error, @@ -56,6 +58,10 @@ from agents.tracing.provider import SynchronousMultiTracingProcessor from agents.tracing.spans import Span from agents.tracing.traces import Trace +from agents.util._json import validate_json + +from .fake_model import FakeModel +from .test_responses import get_text_message _SECRET = "super secret prompt content" @@ -858,3 +864,85 @@ async def test_agent_tool_validation_error_preserves_diagnostics_when_tool_data_ error = exc_info.value assert _TOOL_ARGUMENT_SECRET in str(error) assert isinstance(error.__cause__, ValidationError) + + +_MODEL_OUTPUT_SECRET = "SECRET_MODEL_OUTPUT_123" + + +class _RequiredOutput(BaseModel): + answer: str + count: int + + +def _output_schema() -> AgentOutputSchema: + return AgentOutputSchema(_RequiredOutput) + + +def test_output_schema_validation_error_redacts_payload_when_model_data_disabled( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The model output must not reach the error when model-data logging is disabled. + + The message embedded the raw JSON, and the chained ValidationError carries the same + payload again in its own ``input_value``, so both have to go. + """ + monkeypatch.setattr(_debug, "DONT_LOG_MODEL_DATA", True) + payload = f'{{"answer": "{_MODEL_OUTPUT_SECRET}"}}' + + with pytest.raises(ModelBehaviorError) as exc_info: + _output_schema().validate_json(payload) + + error = exc_info.value + assert _MODEL_OUTPUT_SECRET not in str(error) + assert error.__cause__ is None + assert error.__context__ is None + + +def test_output_schema_validation_error_preserves_diagnostics_when_model_data_enabled( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(_debug, "DONT_LOG_MODEL_DATA", False) + payload = f'{{"answer": "{_MODEL_OUTPUT_SECRET}"}}' + + with pytest.raises(ModelBehaviorError) as exc_info: + _output_schema().validate_json(payload) + + error = exc_info.value + assert _MODEL_OUTPUT_SECRET in str(error) + assert isinstance(error.__cause__, ValidationError) + + +def test_handoff_input_validation_error_redacts_payload_when_model_data_disabled( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Handoff arguments are model output too, so they follow the same policy.""" + monkeypatch.setattr(_debug, "DONT_LOG_MODEL_DATA", True) + payload = f'{{"answer": "{_MODEL_OUTPUT_SECRET}"}}' + + with pytest.raises(ModelBehaviorError) as exc_info: + validate_json(payload, TypeAdapter(_RequiredOutput), partial=False) + + error = exc_info.value + assert _MODEL_OUTPUT_SECRET not in str(error) + assert error.__cause__ is None + assert error.__context__ is None + + +@pytest.mark.asyncio +async def test_run_surfaces_redacted_output_validation_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """End to end: a run whose model output fails validation must not leak it.""" + monkeypatch.setattr(_debug, "DONT_LOG_MODEL_DATA", True) + + model = FakeModel() + agent = Agent(name="A", model=model, output_type=_RequiredOutput) + model.set_next_output([get_text_message(f'{{"answer": "{_MODEL_OUTPUT_SECRET}"}}')]) + + with pytest.raises(ModelBehaviorError) as exc_info: + await Runner.run(agent, "go") + + error = exc_info.value + assert _MODEL_OUTPUT_SECRET not in str(error) + assert error.__cause__ is None + assert error.__context__ is None