fix(panw_prisma_airs): scan tool call args as plain text, not a tool_event - #37038
Conversation
…event Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Greptile SummaryThe PR changes Prisma AIRS tool-call scanning from unsupported OpenAI
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| litellm/proxy/guardrails/guardrail_hooks/panw_prisma_airs/panw_prisma_airs.py | Reworks tool-call extraction and AIRS scanning to use side-aware plain text, with typed helper parameters addressing the previous review thread. |
| tests/test_litellm/proxy/guardrails/guardrail_hooks/test_panw_prisma_airs.py | Updates and expands mocked regression coverage for plain-text tool-call scanning, masked-data routing, malformed inputs, and MCP behavior. |
Reviews (5): Last reviewed commit: "fix(panw_prisma_airs): a wrong-typed too..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
|
|
@greptile review again |
…ng paths Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
|
Ran a base-vs-head live diff of this PR on a real proxy (real Postgres, real Gemini upstream, identical requests to both sides). The core fix is solid and I could reproduce it cleanly — the LIT-5279 repro goes 500 on base → 200 on head, and the MCP path correctly keeps Three things I'd like a closer look at before merge. I'm not asking for immediate fixes — a couple of these may be intentional, and one of them I genuinely can't settle without a PANW tenant. 1. Tool names no longer reach AIRS, and empty-arg tool calls aren't scanned at allThe new caller does Assistant tool call And for a tool call that is scanned: I understand the old Two related things to weigh:
2. Two shapes that base tolerated now raiseThe old extractor used To be straight about reachability: I ran this directly against each tree's handler, and I could not find a live HTTP route that emits a custom tool call today — so treat the crash as confirmed but the route as latent. A 3. Two behaviors from the original guardrail PR are reversedBoth came from #22999 and were labelled as fixes there. Running the base test file against this branch, 8 previously-pinned behaviors fail while this branch's own 232 tests pass:
Here's the part I can't settle: which masked-data key real AIRS returns for a Caveat on my rig: no PANW tenant available, so the AIRS endpoint was a local stand-in replaying the documented AIRS schema and enforcing its documented validation — it rejects any |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 380c449. Configure here.
…tool calls Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
a477c3e sends name and args newline-joined, scans name-only calls again, and parses shapes via Pydantic so custom or missing functions skip safely. Masked-key question still needs your call. |
3d07677 to
a477c3e
Compare
|
@greptileai review latest head |
|
bugbot run |
…arsed The tool call slice types arguments as a string, so a client posting parsed JSON failed validation and the whole tool call, name included, read as unscannable and was skipped without ever reaching AIRS. The OpenAI request path forwards client-supplied tool_calls verbatim, so that shape is reachable. Coerce non-string arguments instead of rejecting them, so the content is scanned.
|
Your First, an apology. I force-pushed over What I added on top:
|
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit a477c3e. Configure here.
… by key name Merging #37036 (already on staging) with this PR produces no conflict and a silent bug. #37036 withholds prompt_masked_data on response-side tool blocks, which was right while tool calls went out as a request-side tool_event: AIRS reported the model's arguments under that key. This PR scans tool calls as ordinary prompt/response text, so the side of the scan now decides which key holds what. The model's arguments arrive under response_masked_data, already covered by _CLIENT_HIDDEN_SCAN_FIELDS, and prompt_masked_data goes back to being the caller's own input -- one of the audit fields LIT-5638 asks for. Left as merged, a response-side tool block drops that field with nothing to flag it. - Tool-path block branch calls _build_error_detail without also_hide - also_hide parameter removed; after this change it has no callers - Regression test asserts both directions: model output withheld, caller input preserved. It fails against the auto-merged combination. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@greptileai review latest head |
|
bugbot run |
…scan _ToolCallFunctionSlice types name as str, and _get_tool_call_function turns any ValidationError into (None, None), which _scan_tool_calls_for_guardrail reads as an unscannable tool call and skips. So a client posting "name": 123 keeps its arguments off the wire to AIRS entirely -- no error, no log, no block. The OpenAI request path forwards client tool_calls verbatim, so this is reachable by any caller holding a valid key. _coerce_arguments already existed for exactly this failure mode on the sibling field. Widening it to cover name closes the gap: name='transfer_funds' AIRS called: 1x args scanned: True name=123 (int) AIRS called: 0x args scanned: False <- before name=123 (int) AIRS called: 1x args scanned: True <- after Reported by Cursor Bugbot on fd9f639. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@greptileai review latest head |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 887be41. Configure here.
74a1bed
into
litellm_internal_staging
TLDR
Problem this solves:
fallback_on_error: allowcannot rescue a malformed requestHow it solves it:
tool_eventschemaresponse_masked_datacorrectlySecond problem, found after #37036 landed on staging:
prompt_masked_dataon response-side tool blockstool_eventThird problem, reported by Cursor Bugbot on
fd9f6396e5:function.namefailed slice validation, so the tool call was skipped entirelytool_callsverbatimUser Flow
Before: a developer whose app lets the model call tools cannot get a single answer once the guardrail is on, every request dies with a 500
mode: [pre_call, post_call]andfallback_on_error: allowtoolsarray holdingget_weatherand"tool_choice": "auto""message": "Security scan failed - request blocked for safety","code": "panw_prisma_airs_scan_failed"and"category": "http_400_error"toolsstill return 200, so tool calling is the only broken flow, and turning the guardrail off is the only workaroundAfter: the same request completes, and both the tool name and the arguments the model produced are still scanned
mode: [pre_call, post_call]andfallback_on_error: allowtoolsarray holdingget_weatherand"tool_choice": "auto""finish_reason": "tool_calls"and theget_weathercall carrying{"city": "San Francisco"}Behavior changes
tool_event. MCP gateway invocations keeptool_event.response_masked_data;prompt_masked_datais the caller's own input on both sides._build_error_detail'salso_hideparameter, added by fix: send whisper timestamp_granularities as bracketed array field #36036/fix(guardrails): return the full PANW AIRS scan response on blocked requests #37036 for the oldtool_eventrouting, is removed. It has no callers after this change. Model-generated content stays withheld through_CLIENT_HIDDEN_SCAN_FIELDS, which already coversresponse_masked_data.prompt_masked_datareaches the caller again, as LIT-5638 intends._ToolCallFunctionSlicecoercesnameas well asargumentsto scannable text. Previously onlyargumentshad a coercing validator, so a wrong-typednamefailed validation for the whole slice and the tool call was skipped unscanned.Relevant issues
Interacts with #37036 (merged). This branch carries a merge of current
litellm_internal_stagingso that interaction is resolved here rather than left to whoever rebases.Linear ticket
Resolves LIT-5279
Pre-Submission checklist
@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
Part 1 — the 500 (original fix, at
a477c3e841)Real
anthropic/claude-sonnet-4-6upstream on a live proxy. AIRS itself stands in for the vendor service because we have no AIRS credentials, and it only enforces the one documented rule that causes the bug:tool_event.metadata.ecosystemmust bemcpGuardrail config used for both runs:
Same command for both runs:
Before, at 7a5b98e:
and the AIRS side logged the rejection:
After, at a477c3e:
and the two scans AIRS received are plain text, the prompt on the way in and the tool call on the way out, name first then arguments:
Part 2 — the #37036 interaction
Live A/B on a real proxy: separate Postgres per leg, separate ports, real Gemini upstream,
POST /v1/messageswith a tool the model actually calls. The AIRS stand-in returns both masked keys with deliberately distinguishable values so the client body shows which one was withheld:First, the merge itself is silent:
The scan that reaches AIRS confirms the routing change — response-side, plain text, no
tool_event:Response-side tool-call block,
error.provider_specific_fields.error:prompt_masked_data(caller input)response_masked_data(model output)also_hideremoved (this commit)CALLER-INPUTModel output stays withheld either way; the only difference is whether the caller gets their own masked input back.
MODEL-TOOLARGSnever appears in any body on either leg.Part 3 — the wrong-typed tool name
_get_tool_call_functionturns anyValidationErrorinto(None, None), and_scan_tool_calls_for_guardrailreads that as an unscannable tool call and skips it. Sincenamewas typedstrwith no coercing validator, a client could suppress the scan on a tool call by sending a non-string.The tool call a caller posts to
/v1/chat/completions, which the OpenAI path forwards verbatim:{"id": "call_1", "type": "function", "function": {"name": 123, "arguments": "{\"to_account\": \"ATTACKER-001\", \"ssn\": \"123-45-6789\"}"}}Counting calls to
_call_panw_apiand checking whether the arguments reached it:function.name"transfer_funds"123{"x": 1}NoneNonewas always fine, so the trigger is specifically a wrong type — the shape aValidationError-to-(None, None)fallback produces. The fix widens the existing_coerce_argumentsvalidator to covername, rather than adding a second near-identical one.Unit
211 passed. Two regression guards, each verified to fail against the defect it covers.
TestPanwAirsToolCallBlockMaskedDataRoutingdrives_scan_tool_calls_for_guardrailend to end rather than calling_build_error_detaildirectly. Restoring the auto-merged combination fails it:test_non_string_tool_name_does_not_suppress_the_scanis parametrized over123,{"x": 1},["a"]andTrue. Reverting the validator toarguments-only fails all four:Type
🐛 Bug Fix
Caveats (if any)
tool_eventreportinglitellm_internal_staging, so the diff is larger than the original change. Rebase instead if you prefer a linear history; the fix is one call site plus the removed parameter.ecosystemrule, and Part 2's accepts any payload, so neither run proves what a real AIRS returns. Both exercise everything downstream ofresponse.json(), which is all either change touches./v1/messagesthe block detail is delivered as a Python-repr string rather than structured JSON ("400: {'error': {...}}"). Pre-existing, reproduces before both PRs, and worth its own ticket.Final Attestation
Link to Devin session: https://app.devin.ai/sessions/0dd1270c67bc4a8393052f161740fc32
Note
Cursor Bugbot is generating a summary for commit 887be41. Configure here.