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
13 changes: 12 additions & 1 deletion src/agents/agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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(
Expand Down
25 changes: 19 additions & 6 deletions src/agents/extensions/experimental/codex/codex_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
13 changes: 12 additions & 1 deletion src/agents/tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -2535,14 +2535,25 @@ 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)
if json_data
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)

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 Clear raw argument locals before redacted raises

When DONT_LOG_TOOL_DATA is true and validation fails after JSON decoding, this redacted raise still happens in _on_invoke_tool_impl while input, ctx.tool_arguments, and json_data remain live in the traceback frame, so traceback-with-locals telemetry can recover the model-supplied arguments even though the message and exception chain are scrubbed. Fresh evidence beyond the prior exception-local comments is the decoded payload/raw-input locals that remain at the redacted raise; clear or avoid carrying those locals before raising the redacted ModelBehaviorError.

AGENTS.md reference: AGENTS.md:L89-L89

Useful? React with 👍 / 👎.


args, kwargs_dict = schema.to_call_args(parsed)

Expand Down
88 changes: 88 additions & 0 deletions tests/extensions/experiemental/codex/test_codex_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
126 changes: 126 additions & 0 deletions tests/test_error_logging_redaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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
Expand Down Expand Up @@ -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)
5 changes: 4 additions & 1 deletion tests/test_programmatic_tool_calling.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down