fix(redaction): redact assistant tool call arguments in spend logs - #33109
fix(redaction): redact assistant tool call arguments in spend logs#33109rs-03 wants to merge 4 commits into
Conversation
When message redaction is enabled (turn_off_message_logging / store_prompts_in_spend_logs=false), message content, reasoning, thinking and audio were redacted but assistant tool_calls were left untouched, so the function arguments, which can echo sensitive conversation data, were still written to spend logs in the clear. Redact the arguments of every assistant tool call in both the object and dict redaction paths, for message and streaming delta choices, keeping the tool name and call structure so token counts and metrics stay intact. Fixes BerriAI#33107
Greptile SummaryThis PR fixes a redaction gap where assistant tool-call
Confidence Score: 4/5The change is safe to merge — it adds argument redaction to two helper functions and two new unit tests, with no modifications to existing logic or test assertions. The fix is correctly applied to all four choice paths and is consistent with how the rest of The streaming path in
|
| Filename | Overview |
|---|---|
| litellm/litellm_core_utils/redact_messages.py | Adds _redact_tool_call_arguments and _redact_tool_call_arguments_dict helpers that blank function.arguments in all four choice paths (object Choices, object StreamingChoices, dict message, dict delta); logic is correct and consistent with existing redaction style. |
| tests/test_litellm/litellm_core_utils/test_redact_messages.py | Two new tests cover the ModelResponse object path and the dict message/delta paths; the StreamingChoices object path (complete_streaming_response with StreamingChoices containing tool_calls) is not exercised by the new tests. |
Reviews (1): Last reviewed commit: "fix(redaction): redact assistant tool ca..." | Re-trigger Greptile
| def test_redacts_tool_call_arguments(self): | ||
| tool_call = { | ||
| "id": "call_1", | ||
| "type": "function", | ||
| "function": {"name": "get_weather", "arguments": '{"city": "sensitive city"}'}, | ||
| } | ||
| result = litellm.ModelResponse( | ||
| choices=[ | ||
| litellm.Choices( | ||
| message=litellm.Message( | ||
| content="message content", | ||
| role="assistant", | ||
| tool_calls=[dict(tool_call)], | ||
| ) | ||
| ) | ||
| ] | ||
| ) | ||
|
|
||
| redacted = perform_redaction({}, result) | ||
|
|
||
| redacted_call = redacted.choices[0].message.tool_calls[0] | ||
| assert redacted_call.function.arguments == "redacted-by-litellm" | ||
| assert redacted_call.function.name == "get_weather" | ||
|
|
||
| def test_redacts_tool_call_arguments_in_dict_choices(self): | ||
| tool_call = { | ||
| "id": "call_1", | ||
| "type": "function", | ||
| "function": {"name": "get_weather", "arguments": '{"city": "sensitive city"}'}, | ||
| } | ||
| result = { | ||
| "choices": [ | ||
| {"message": {"content": "message content", "tool_calls": [dict(tool_call)]}}, | ||
| {"delta": {"content": "delta content", "tool_calls": [dict(tool_call)]}}, | ||
| ] | ||
| } | ||
|
|
||
| redacted = perform_redaction({}, result) | ||
|
|
||
| message_call = redacted["choices"][0]["message"]["tool_calls"][0] | ||
| assert message_call["function"]["arguments"] == "redacted-by-litellm" | ||
| assert message_call["function"]["name"] == "get_weather" | ||
|
|
||
| delta_call = redacted["choices"][1]["delta"]["tool_calls"][0] | ||
| assert delta_call["function"]["arguments"] == "redacted-by-litellm" |
There was a problem hiding this comment.
Missing test for
StreamingChoices object-form tool calls
The PR exercises three of four redaction paths for tool calls: litellm.ModelResponse (object), dict "message", and dict "delta". The fourth path — complete_streaming_response with litellm.utils.StreamingChoices objects containing tool_calls — is covered by the new _redact_tool_call_arguments call added to _redact_choice_content's StreamingChoices branch, but no test drives it. A test passing model_call_details={"stream": True, "complete_streaming_response": ...} with StreamingChoices that have delta.tool_calls set would close this gap and guard against future regressions on the streaming path.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Add a regression test that drives perform_redaction over a complete_streaming_response whose StreamingChoices carry delta tool_calls, closing the one tool-call redaction path the earlier tests did not exercise directly.
|
Good catch, added est_redacts_tool_call_arguments_in_streaming_choices in 9ece9c9. It drives perform_redaction over a complete_streaming_response whose StreamingChoices carry delta.tool_calls, so the streaming object path is now covered directly. It fails on the pre-fix code and passes with the fix, same as the other three paths |
Assert redaction skips non-dict and function-less tool_call entries without error while still redacting valid ones, guarding the tool_calls redaction against malformed provider responses.
| choice["message"]["thinking_blocks"] = None | ||
| if "audio" in choice["message"]: | ||
| choice["message"]["audio"] = None | ||
| _redact_tool_call_arguments_dict(choice["message"].get("tool_calls"), redacted_str) |
There was a problem hiding this comment.
Medium: Alternate function-call arguments bypass redaction
This only handles message.tool_calls[].function.arguments. An untrusted prompt can place sensitive data in legacy message.function_call.arguments or a Responses API output item’s top-level arguments; both survive perform_redaction() and are serialized into spend logs. Redact those representations in both the object and dictionary helpers as well.
PR overviewThis pull request updates spend-log redaction logic so assistant tool call arguments are removed before messages are logged. The changes focus on the message redaction helpers in There is still an open gap in the redaction coverage: alternate function-call argument formats can bypass the current handling and be written to spend logs. That means sensitive data supplied through legacy or Responses API-style fields may remain exposed in logging paths until those representations are redacted as well. No issues have been addressed yet, so the PR still needs a targeted fix before the redaction behavior is complete. Open issues (1)
Fixed/addressed: 0 · PR risk: 6/10 |
The tool_calls fix left the legacy function_call field untouched, so a response using the older single function_call format still wrote its arguments to spend logs in the clear under message redaction. Redact function_call.arguments alongside tool_calls in every choice path, object and dict, message and streaming delta.
|
Good catch, addressed in 08cd10a. Extended the fix to redact the legacy message.function_call.arguments (and delta.function_call.arguments) alongside ool_calls, in both the object and dict paths, since a response using the older single function_call format had the same leak. Added est_redacts_legacy_function_call_arguments_object and est_redacts_legacy_function_call_arguments_dict, both mutation-checked against the pre-fix code |
|
Friendly ping, this has been open about a week and is mergeable. Same unrelated systemic |
|
Closing this as redundant. #33111 landed the same assistant tool-call argument redaction and is already in the daily branches, including the Responses API function_call path, so this PR no longer adds anything on top. Thanks to those who reviewed. A separate follow-up to also redact the tool name (the selection), requested on #33107, would build on the merged code rather than this branch |
Relevant issues
Fixes #33107
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
This is a redaction bug, so the observable difference is what ends up in the spend log for an assistant response that contains tool calls, with message redaction enabled (
turn_off_message_logging: true, orstore_prompts_in_spend_logs: false, or the per-request header)Before this change, the message content is redacted but the tool call is left intact, so the arguments the model generated (which routinely echo user data) are written to the log in the clear:
After this change the arguments are redacted while the tool name and call structure stay intact, so token counts and metrics are unaffected:
I verified this at the redaction layer rather than against a live proxy, since reproducing the spend-log entry end to end needs the proxy running with a real tool-calling model. The behavior is pinned by the added regression tests, which fail on the current code (arguments are not redacted) and pass with the fix, across the object response path, the dict response path, and the streaming delta path
Type
🐛 Bug Fix
Changes
_redact_choice_contentand_redact_model_response_dict_choicesinlitellm/litellm_core_utils/redact_messages.pyredactedcontent,reasoning_content,thinking_blocksandaudio, but nevertool_calls. Every redaction path funnels through these two helpers (the direct result, the streamingcomplete_streaming_response, and thestandard_logging_objectresponse), so assistant tool call arguments survived redaction everywhereThis adds
tool_callsargument redaction to both helpers, for the message choice and the streaming delta, in both the object form (litellm.Choices/StreamingChoices) and the dict form. Onlyfunction.argumentsis blanked; the tool name, id and overall structure are preserved, consistent with howcontentis replaced with a placeholder rather than droppedQA runbook
The regression tests are
TestPerformRedaction::test_redacts_tool_call_arguments(objectModelResponsepath) andTestPerformRedaction::test_redacts_tool_call_arguments_in_dict_choices(dict message and delta paths) intests/test_litellm/litellm_core_utils/test_redact_messages.pyFinal Attestation