fix(tools): restrict concatenated-JSON salvage to complete objects

Address review feedback on #40603.

`split_concatenated_json_objects` deliberately keeps whatever prefix it
can parse before a malformed tail, so salvaging on its result meant
`{"a": 1}{"b":` or `{"a": 1} garbage` would invoke a tool with partial
arguments where parsing previously failed. Tool calls execute, so those
must keep failing. Salvage now requires the whole string to be consumed
as two or more complete JSON objects.

The recovery warning also no longer logs a raw prefix of the arguments,
which can carry PII or credentials; it records structural metadata only.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
deepak7lal 2026-09-11 00:49:58 +05:30
parent 47ce07253c
commit fe0607c16c
2 changed files with 56 additions and 10 deletions

View file

@ -2314,6 +2314,35 @@ def _attempt_json_repair(s: str) -> object | None:
return None
def _split_complete_json_objects(raw: str) -> list[dict[str, object]] | None:
"""
Split *raw* into JSON objects, requiring the entire string to be consumed.
Unlike :func:`split_concatenated_json_objects`, which deliberately salvages
whatever prefix it can before a malformed tail, this returns ``None`` unless
*raw* is exactly a sequence of complete JSON objects. Tool call arguments
are executed, so a truncated or trailing-garbage payload must keep failing
rather than invoke a tool with partial input.
"""
import json
decoder: Final = json.JSONDecoder()
objects: list[dict[str, object]] = []
index = 0
while index < len(raw):
if raw[index].isspace():
index += 1
continue
try:
obj, index = decoder.raw_decode(raw, index)
except json.JSONDecodeError:
return None
if not isinstance(obj, dict):
return None
objects.append(obj)
return objects or None
def parse_tool_call_arguments(
arguments: str | None,
tool_name: str | None = None,
@ -2364,21 +2393,20 @@ def parse_tool_call_arguments(
# arguments string, which ``json.loads`` reports as "Extra data" and
# ``_attempt_json_repair`` cannot fix because nothing is truncated.
# This is the same provider behaviour already repaired on the Bedrock
# request path (see ``_convert_to_bedrock_tool_call_invoke``), so the
# helper is reused here rather than dropping the call: returning ``{}``
# is indistinguishable from the model asking for nothing.
concatenated: Final = split_concatenated_json_objects(arguments)
if concatenated:
# request path (see ``_convert_to_bedrock_tool_call_invoke``), so it is
# salvaged here too rather than dropping the call: returning ``{}`` is
# indistinguishable from the model asking for nothing.
concatenated: Final = _split_complete_json_objects(arguments)
if concatenated is not None and len(concatenated) > 1:
# Structural metadata only - the arguments themselves may carry
# PII or credentials and must not reach warning logs.
verbose_logger.warning(
"Recovered %d concatenated JSON object(s) from tool call arguments for tool '%s' (%s); "
"using the first and discarding %d. Original (%d chars): %.200s%s",
"Recovered %d concatenated JSON objects from tool call arguments "
"for tool '%s' (%s); using the first and discarding %d.",
len(concatenated),
tool_name or "<unknown>",
context or "unknown context",
len(concatenated) - 1,
len(arguments),
arguments,
"..." if len(arguments) > 200 else "",
)
# Mirrors factory.py, where the first parsed object keeps the
# original tool call id.

View file

@ -309,6 +309,24 @@ def test_parse_tool_call_arguments_concatenated_is_not_dropped_silently():
assert result == {"a": 1}
@pytest.mark.parametrize(
"raw",
[
'{"a": 1}{"b":', # truncated tail
'{"a": 1} garbage', # trailing garbage
'{"a": 1}{"b": 2} x', # complete objects followed by junk
],
)
def test_parse_tool_call_arguments_rejects_incomplete_concatenation(raw):
"""
Salvage is restricted to input wholly consumed as complete JSON objects.
Tool call arguments are executed, so a truncated or trailing-garbage
payload must keep failing rather than invoke a tool with partial input.
"""
with pytest.raises(ValueError, match="Failed to parse tool call arguments"):
parse_tool_call_arguments(raw, tool_name="demo", context="chat completions")
# ---------------------------------------------------------------------------
# Regression tests for non-OpenAI file content blocks.
#