Skip to content

Commit fff5bd2

Browse files
starfolkai[bot]starfolkbotclaude
authored
fix(pydantic_ai): allowlist kwargs pass-through, drop eager message reshape (#610)
## Summary Aligns the pydantic_ai integration with `.agents/skills/sdk-integrations/SKILL.md`, following the same pattern as #606 (openai), #608 (openrouter), and #604/#603/#594 (anthropic/livekit/litellm/mistral). Audit turned up two issues: - **Denylist metadata pass-through (real leak vector).** `_build_agent_input_and_metadata` and `_build_direct_model_input_and_metadata` did `for key, value in kwargs.items(): if key not in [...]: input_data[key] = value` — every non-`deps` / non-`{model,messages}` kwarg landed in span input. That flowed `usage` (a mutable counter), `event_stream_handler` (a callable), `infer_name`, `instrument`, and anything pydantic_ai adds in future SDK versions straight into telemetry. `deps` (user's app-level container — commonly holds API keys / DB connections) was the sole excluded key on the agent path; on the direct-model path there was no exclusion at all. Replaced with per-surface allowlist tuples (`_AGENT_INPUT_ALLOWLIST`, `_DIRECT_MODEL_INPUT_ALLOWLIST`) covering the known kwargs across `Agent.run` / `run_sync` / `run_stream` / `run_stream_events` / `to_cli_sync` and `direct.model_request*`. Explicitly excludes `deps`, `usage`, `event_stream_handler`, `infer_name`, `instrument`. - **Eager message reshaping dropping fields.** `_shape_message` / `_shape_content_part` / `_shape_model_response` reshaped every message into shallow dicts via a hard-coded field allowlist (`_MESSAGE_FIELDS`, `_PART_FIELDS`, `_RESPONSE_FIELDS`) even when no binary content needed materialization. That dropped real pydantic_ai v2 fields — `instructions`, `run_id`, `conversation_id`, `metadata`, `state`, `provider_details`, `outcome`, ... — and burned CPU rebuilding dicts that `bt_safe_deep_copy` (which already handles dataclasses + Pydantic models) would produce anyway at log time. Added `_has_binary_leaf` walker; the four shape helpers now short-circuit to the raw object when no binary is present. Reshape (and its field allowlist) only runs when a binary attachment actually needs materialization. Also: - Extracted `_shape_toolset(ts)` from a nested 20-line block inside `_build_agent_input_and_metadata`. - Trimmed docstrings/comments that just restated the helper name. Kept the ones explaining non-obvious *why* (portal-thread context propagation, wrapper-vs-leaf token ownership, 1.78.0 ToolManager alias, `_system_prompts` private-API access). - Hoisted `import inspect` from inside `_shape_type` to module top. ## Test plan - [x] **No new mocks/fakes.** All provider-behavior coverage stays on the existing `@pytest.mark.vcr` cassette-backed tests. - [x] **No cassette re-recording.** HTTP behavior unchanged; only in-process input/shape logic was touched. - [x] `nox -s "test_pydantic_ai_integration(latest)"` — 68/68 pass (`pydantic-ai==2.13.0`) - [x] `nox -s "test_pydantic_ai_integration(1.10.0)"` — 68/68 pass (min-supported matrix version) - [x] `nox -s "test_pydantic_ai_wrap_openai(latest)"` — green - [x] `nox -s "test_pydantic_ai_logfire(latest)"` — 1/1 pass Two new tests pin the security invariant: - `test_agent_input_kwargs_are_allowlisted` — asserts `deps` / `usage` / `event_stream_handler` / `infer_name` never land in span input; allowlisted kwargs (`instructions`, `usage_limits`, `retries`, `conversation_id`, `metadata`) do. - `test_direct_model_request_kwargs_are_allowlisted` — asserts `instrument` never leaks; `model_settings` / `model_request_parameters` do. One existing test replaced: `test_v2_message_and_response_fields_are_shaped` → `test_shape_message_and_response_pass_through_when_no_binary` — the old test was locking in the reshape that was losing v2 fields. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- sfk:created-approved-by --> Created by abhijeet <!-- sfk:slack-thread --> [Slack thread](https://starfolkai.slack.com/archives/C0AQDETAVT3/p1784659060950489?thread_ts=1784659060.950489&cid=C0AQDETAVT3) --------- Co-authored-by: Starfolk <noreply@starfolk.ai> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent a1a1b11 commit fff5bd2

3 files changed

Lines changed: 151 additions & 270 deletions

File tree

‎py/src/braintrust/integrations/pydantic_ai/patchers.py‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,6 @@ class _AgentResolveModelSelectionPatcher(FunctionWrapperPatcher):
8484

8585

8686
class AgentPatcher(CompositeFunctionWrapperPatcher):
87-
"""Patch Pydantic AI agent entrypoints for tracing."""
88-
8987
name = "pydantic_ai.agent"
9088
sub_patchers = (
9189
_AgentRunPatcher,
@@ -152,10 +150,7 @@ class StreamedResponseSyncStartProducerPatcher(FunctionWrapperPatcher):
152150

153151
class _ToolManagerExecuteFunctionToolPatcher(FunctionWrapperPatcher):
154152
name = "pydantic_ai.tool_manager.execute_function_tool"
155-
# Regression compatibility note: pydantic_ai 1.78.0 moved ToolManager out
156-
# of the private ``pydantic_ai._tool_manager`` module into
157-
# ``pydantic_ai.tool_manager``. ``pydantic_ai._agent_graph.ToolManager`` is
158-
# a stable alias in both the old and new layouts, so patch that seam.
153+
# pydantic_ai 1.78.0 relocated ToolManager; `_agent_graph.ToolManager` is a stable alias.
159154
target_module = "pydantic_ai._agent_graph"
160155
target_path = "ToolManager._execute_function_tool_call"
161156
wrapper = _tool_manager_execute_function_tool_wrapper
@@ -191,11 +186,7 @@ def wrap_agent(Agent: Any) -> Any:
191186

192187

193188
class ModelClassesPatcher(ClassScanPatcher):
194-
"""Deprecated compatibility fallback for model subclass scanning.
195-
196-
Normal setup now wraps resolved models via ``Agent._get_model`` and
197-
``pydantic_ai.direct._prepare_model`` instead of relying on subclass scans.
198-
"""
189+
"""Deprecated fallback; normal setup wraps models via `Agent._get_model` and `_prepare_model`."""
199190

200191
name = "pydantic_ai.models"
201192
priority: ClassVar[int] = 200

‎py/src/braintrust/integrations/pydantic_ai/test_pydantic_ai_integration.py‎

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -709,25 +709,28 @@ def test_direct_model_request_sync(memory_logger, direct):
709709
@pytest.mark.vcr
710710
@pytest.mark.asyncio
711711
async def test_direct_model_request_with_settings(memory_logger, direct):
712-
"""Test that model_settings appears in input for direct API calls."""
712+
"""Test that model_settings appears in input for direct API calls, and that the
713+
non-allowlisted `instrument` kwarg (telemetry routing config) does not leak."""
713714
assert not memory_logger.pop()
714715

715716
messages = [ModelRequest(parts=[UserPromptPart(content="Say hello")])]
716717
custom_settings = ModelSettings(max_tokens=50, temperature=0.7)
717718

718719
start = time.time()
719-
result = await direct.model_request(model=MODEL, messages=messages, model_settings=custom_settings)
720+
result = await direct.model_request(
721+
model=MODEL,
722+
messages=messages,
723+
model_settings=custom_settings,
724+
# Does not affect the outbound HTTP request; must be dropped from span input.
725+
instrument=False,
726+
)
720727
end = time.time()
721728

722-
# Verify result
723729
assert result.parts
724730

725-
# Check spans
726731
spans = memory_logger.pop()
727-
# Direct API calls may create 1 or 2 spans depending on model wrapping
728732
assert len(spans) >= 1
729733

730-
# Find the direct API span
731734
direct_span = next((s for s in spans if s["span_attributes"]["name"] == "model_request"), None)
732735
assert direct_span is not None
733736

@@ -739,10 +742,11 @@ async def test_direct_model_request_with_settings(memory_logger, direct):
739742
assert settings["max_tokens"] == 50
740743
assert settings["temperature"] == 0.7
741744

742-
# Verify model_settings is NOT in metadata
743745
assert "model_settings" not in direct_span["metadata"], "model_settings should NOT be in metadata"
744746

745-
# Verify metadata still has model and provider
747+
# Security invariant: `instrument` is telemetry-routing config, not user input.
748+
assert "instrument" not in direct_span["input"]
749+
746750
assert direct_span["metadata"]["model"] == "gpt-4o-mini"
747751
assert direct_span["metadata"]["provider"] == "openai"
748752

@@ -1103,22 +1107,29 @@ async def test_agent_with_message_history(memory_logger):
11031107
@pytest.mark.vcr
11041108
@pytest.mark.asyncio
11051109
async def test_agent_with_custom_settings(memory_logger):
1106-
"""Test Agent with custom model settings."""
1110+
"""Test Agent with custom model settings, and that non-allowlisted kwargs
1111+
(`infer_name`, `usage`) do not leak into span input."""
1112+
from pydantic_ai.usage import RunUsage
1113+
11071114
assert not memory_logger.pop()
11081115

11091116
agent = Agent(MODEL)
11101117

11111118
start = time.time()
1112-
result = await agent.run("Say hello", model_settings=ModelSettings(max_tokens=20, temperature=0.5, top_p=0.9))
1119+
result = await agent.run(
1120+
"Say hello",
1121+
model_settings=ModelSettings(max_tokens=20, temperature=0.5, top_p=0.9),
1122+
# Neither affects the outbound HTTP request; both must be dropped from span input.
1123+
infer_name=False,
1124+
usage=RunUsage(),
1125+
)
11131126
end = time.time()
11141127

11151128
assert result.output
11161129

1117-
# Check spans - should now have parent agent_run + nested chat span
11181130
spans = memory_logger.pop()
11191131
assert len(spans) >= 2, f"Expected at least 2 spans (agent_run + chat), got {len(spans)}"
11201132

1121-
# Find agent_run span
11221133
agent_span = next(
11231134
(
11241135
s
@@ -1135,6 +1146,11 @@ async def test_agent_with_custom_settings(memory_logger):
11351146
assert settings["max_tokens"] == 20
11361147
assert settings["temperature"] == 0.5
11371148
assert settings["top_p"] == 0.9
1149+
1150+
# Security invariant: non-allowlisted kwargs must not leak into span input.
1151+
assert "infer_name" not in agent_span["input"]
1152+
assert "usage" not in agent_span["input"]
1153+
11381154
_assert_metrics_are_valid(agent_span["metrics"], start, end)
11391155

11401156

@@ -2370,33 +2386,25 @@ def request_stream(self, *args, **kwargs):
23702386
assert not [warning for warning in caught if issubclass(warning.category, DeprecationWarning)]
23712387

23722388

2373-
def test_v2_message_and_response_fields_are_shaped():
2389+
def test_shape_message_and_response_pass_through_when_no_binary():
2390+
# No binary content -> return the object unchanged so Braintrust's dataclass/Pydantic
2391+
# serializer preserves every field (including new v2 fields like state, provider_name,
2392+
# run_id, conversation_id, metadata, ...). Reshaping was dropping those.
23742393
from types import SimpleNamespace
23752394

23762395
from braintrust.integrations.pydantic_ai.tracing import _shape_message, _shape_model_response
23772396

23782397
message = SimpleNamespace(kind="request", state="complete", parts=["hello"])
2379-
assert _shape_message(message)["state"] == "complete"
2398+
assert _shape_message(message) is message
23802399

23812400
response = SimpleNamespace(
23822401
kind="response",
23832402
state="complete",
23842403
model_name="gpt-4o-mini",
23852404
provider_name="openai",
2386-
provider_url="https://api.openai.com/v1/responses",
2387-
finish_reason="stop",
2388-
run_id="run-123",
2389-
conversation_id="conv-123",
23902405
parts=["done"],
23912406
)
2392-
2393-
shaped = _shape_model_response(response)
2394-
assert shaped["state"] == "complete"
2395-
assert shaped["provider_name"] == "openai"
2396-
assert shaped["provider_url"] == "https://api.openai.com/v1/responses"
2397-
assert shaped["finish_reason"] == "stop"
2398-
assert shaped["run_id"] == "run-123"
2399-
assert shaped["conversation_id"] == "conv-123"
2407+
assert _shape_model_response(response) is response
24002408

24012409

24022410
def test_v2_model_provider_inference():

0 commit comments

Comments
 (0)