Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/agents/util/_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Comment on lines +36 to +39

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Respect tool redaction for handoff JSON

When this helper is reached from handoff(), json_str is the handoff tool-call arguments, so an app that enables model diagnostics but keeps tool-data redaction on (DONT_LOG_MODEL_DATA=False, DONT_LOG_TOOL_DATA=True) still gets the raw handoff payload and chained Pydantic ValidationError. Use a caller-specific or mixed model/tool redaction policy here instead of keying every validate_json failure only off the model flag.

AGENTS.md reference: AGENTS.md:L95-L95

Useful? React with 👍 / 👎.


# 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}")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Remove payload from redacted traceback locals

When model-data logging is disabled, this removes the message/cause leak but the redacted ModelBehaviorError traceback still includes the validate_json frame, whose locals contain json_str with the raw model output. Any traceback renderer or telemetry integration that captures locals can still expose the payload in the default redacted path; raise from a helper that never receives the payload or clear sensitive locals before raising.

AGENTS.md reference: AGENTS.md:L95-L95

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Scrub raw responses from redacted run errors

For a Runner.run() structured-output validation failure with model-data logging disabled, this redacted ModelBehaviorError is later populated with run_data, and that RunErrorDetails.raw_responses still contains the original model message text. Any handler or telemetry exporter that inspects the exception object can therefore recover the same payload even though the message and cause were redacted; mark these redacted validation errors so runner error details omit or sanitize raw model responses.

AGENTS.md reference: AGENTS.md:L95-L95

Useful? React with 👍 / 👎.



def _to_dump_compatible(obj: Any) -> Any:
Expand Down
90 changes: 89 additions & 1 deletion tests/test_error_logging_redaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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,
Expand All @@ -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"

Expand Down Expand Up @@ -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