Skip to content

Commit a8009bc

Browse files
authored
fix(proxy): normalize leaked GLM/qwen tool-call arg dialect from responses (#85) (#86)
Some open-weight backends (z-ai/glm-5.2, qwen family) intermittently fail to decode their own tool-call argument encoding, leaving raw markup inside the structured `arguments` a well-formed native tool_calls entry returns: GLM: <arg_key>NAME</arg_key> <arg_value>VALUE</arg_value> qwen: <parameter=NAME>VALUE</parameter> The intended payload is intact inside the wrapper, so it's a parse gap not lost data. Normalize both the value-level leak (dialect inside one value of an otherwise-valid JSON object, the #85 symptom) and the whole-string leak, applied to each successful response before it is returned or logged, so no downstream consumer (client or log) sees the markup. Fully defensive: any parse failure leaves the value untouched. Repair count recorded as tool_call_dialect_repaired on the response log so the leak rate stays measurable. Durable server-side fix for the class geo-agent#276 defends against client-side.
1 parent e2551cf commit a8009bc

3 files changed

Lines changed: 261 additions & 3 deletions

File tree

CHANGELOG.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,22 @@ See [Releases](README.md#releases) for how a release is cut.
88

99
## [Unreleased]
1010

11+
### Fixed
12+
- **Strip leaked `<arg_key>`/`<arg_value>` (GLM) and `<parameter=…>` (qwen) tool-call
13+
arg dialect from responses (#85).** Some open-weight backends (`z-ai/glm-5.2`, the
14+
qwen family) intermittently fail to decode their own tool-call argument encoding,
15+
leaving raw markup inside the structured `arguments` a well-formed native
16+
`tool_calls` entry returns — e.g. a `value_stats` value arriving as
17+
`<arg_key>value_stats</arg_key> <arg_value>{…}</arg_value>` instead of the parsed
18+
object. The proxy now normalizes both the value-level leak (dialect inside one value
19+
of an otherwise-valid JSON object) and the whole-string leak (the entire `arguments`
20+
is raw dialect) in `_normalize_response_tool_calls`, applied to each successful
21+
response *before* it is returned or logged, so no downstream consumer (client or log)
22+
ever sees the markup. Fully defensive — any parse failure leaves the value untouched.
23+
The repair count is recorded as `tool_call_dialect_repaired` on the response log so
24+
the leak rate stays measurable. Durable server-side fix for the leak class that
25+
geo-agent#276 was defending against client-side.
26+
1127
### Added
1228
- **Multi-key client auth — accept more than one `PROXY_KEY` so eval/dev keys are
1329
independently revocable.** New `PROXY_KEYS_EXTRA` env (comma-separated, wired from

llm_proxy.py

Lines changed: 109 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,98 @@ def _cap(s: Optional[str], limit: int) -> str:
9595
return s[:limit]
9696
return s
9797

98+
# --- Tool-call arg-dialect normalization (#85) -------------------------------
99+
# Some open-weight backends intermittently fail to decode their own tool-call
100+
# argument encoding, leaving raw markup inside the structured `arguments` string
101+
# that an otherwise well-formed native `tool_calls` entry hands back:
102+
# GLM (z-ai/glm-5.2): <arg_key>NAME</arg_key> <arg_value>VALUE</arg_value>
103+
# qwen / hermes: <parameter=NAME>VALUE</parameter>
104+
# The intended payload is intact *inside* the wrapper, so this is a
105+
# serialization/parse gap, not lost data (#85; qwen precedent geo-agent#276).
106+
# We repair it here — before the response is returned or logged — so no
107+
# downstream consumer (client or log) ever sees the dialect. Fully defensive:
108+
# any parse failure leaves the value untouched.
109+
_ARG_DIALECT_MARKERS = ("<arg_key>", "<arg_value>", "<parameter=")
110+
_GLM_ARG_PAIR_RE = re.compile(r"<arg_key>(.*?)</arg_key>\s*<arg_value>(.*?)(?:</arg_value>|$)", re.DOTALL)
111+
_QWEN_ARG_PAIR_RE = re.compile(r"<parameter=(.*?)>(.*?)(?:</parameter>|$)", re.DOTALL)
112+
_GLM_ARG_VALUE_RE = re.compile(r"<arg_value>(.*?)(?:</arg_value>|$)", re.DOTALL)
113+
_QWEN_ARG_VALUE_RE = re.compile(r"<parameter=[^>]*>(.*?)(?:</parameter>|$)", re.DOTALL)
114+
115+
116+
def _coerce_json(s: str):
117+
"""Parse `s` as JSON when it is structured data, else return it stripped.
118+
Lets an unwrapped `{...}`/`[...]`/number value come back structured while a
119+
bare scalar string stays a string."""
120+
s = s.strip()
121+
try:
122+
return json.loads(s)
123+
except Exception:
124+
return s
125+
126+
127+
def _unwrap_dialect_value(val):
128+
"""If a single tool-call argument *value* carries leaked arg-dialect markup,
129+
extract the real payload from inside the <arg_value>/<parameter=…> wrapper.
130+
Returns (value, changed)."""
131+
if not isinstance(val, str) or not any(m in val for m in _ARG_DIALECT_MARKERS):
132+
return val, False
133+
m = _GLM_ARG_VALUE_RE.search(val) or _QWEN_ARG_VALUE_RE.search(val)
134+
if not m:
135+
return val, False
136+
return _coerce_json(m.group(1)), True
137+
138+
139+
def _normalize_tool_call_arguments(arguments):
140+
"""Strip leaked arg-dialect markup from a tool call's `arguments` string.
141+
Handles both the value-level leak (dialect inside one value of an otherwise
142+
valid JSON object, the #85 symptom) and the whole-string leak (the entire
143+
`arguments` is raw dialect). Returns (arguments_string, changed)."""
144+
if not isinstance(arguments, str) or not any(m in arguments for m in _ARG_DIALECT_MARKERS):
145+
return arguments, False
146+
# Case 1: arguments is valid JSON; dialect leaked into individual values.
147+
try:
148+
obj = json.loads(arguments)
149+
except Exception:
150+
obj = None
151+
if isinstance(obj, dict):
152+
changed = False
153+
for k, v in list(obj.items()):
154+
new_v, ch = _unwrap_dialect_value(v)
155+
if ch:
156+
obj[k] = new_v
157+
changed = True
158+
return (json.dumps(obj), True) if changed else (arguments, False)
159+
# Case 2: the whole arguments string is raw dialect — rebuild the object by
160+
# pairing each key tag with the value tag that follows it.
161+
pairs = _GLM_ARG_PAIR_RE.findall(arguments) or _QWEN_ARG_PAIR_RE.findall(arguments)
162+
if pairs:
163+
return json.dumps({k.strip(): _coerce_json(v) for k, v in pairs}), True
164+
return arguments, False
165+
166+
167+
def _normalize_response_tool_calls(result) -> int:
168+
"""Repair leaked tool-call arg dialect (#85) in an upstream response, in
169+
place. Returns the number of tool-call `arguments` repaired. Fully
170+
defensive: any error leaves `result` untouched and returns 0 —
171+
normalization must never corrupt a response or break serving."""
172+
repaired = 0
173+
try:
174+
for choice in result.get("choices") or []:
175+
message = (choice or {}).get("message") or {}
176+
for tc in message.get("tool_calls") or []:
177+
fn = (tc or {}).get("function")
178+
if not isinstance(fn, dict):
179+
continue
180+
new_args, changed = _normalize_tool_call_arguments(fn.get("arguments"))
181+
if changed:
182+
fn["arguments"] = new_args
183+
repaired += 1
184+
except Exception as e: # pragma: no cover - defensive
185+
print(f"⚠️ tool-call dialect normalization skipped: "
186+
f"{type(e).__name__}: {e}", flush=True)
187+
return 0
188+
return repaired
189+
98190
# --- Credential scrubbing ----------------------------------------------------
99191
# Credentials reach the logs because the geo-agent `query` MCP tool accepts
100192
# s3_key/s3_secret in its arguments, which flow through `tool_calls`, tool
@@ -405,7 +497,7 @@ def log_request(provider: str, model: str, messages: List[Dict], tools_count: in
405497
_emit(log_entry)
406498

407499
@_never_raises
408-
def log_response(provider: str, model: str, response_data: dict, latency_ms: int, error: str = None, origin: str = None, request_id: str = None, session_id: str = None, client: str = None, upstream_headers: dict = None):
500+
def log_response(provider: str, model: str, response_data: dict, latency_ms: int, error: str = None, origin: str = None, request_id: str = None, session_id: str = None, client: str = None, upstream_headers: dict = None, dialect_repaired: int = 0):
409501
"""Log response in structured JSON format"""
410502
log_entry = {
411503
"timestamp": datetime.utcnow().isoformat() + "Z",
@@ -453,6 +545,12 @@ def log_response(provider: str, model: str, response_data: dict, latency_ms: int
453545
for tc in message["tool_calls"]
454546
]
455547

548+
# How many tool-call arguments were repaired of leaked arg dialect (#85).
549+
# Kept queryable so the leak rate stays measurable even though the markup
550+
# itself no longer reaches the logs.
551+
if dialect_repaired:
552+
log_entry["tool_call_dialect_repaired"] = dialect_repaired
553+
456554
# Extract token usage if available
457555
if "usage" in response_data:
458556
log_entry["tokens"] = response_data["usage"]
@@ -595,10 +693,18 @@ async def proxy_chat(request: ChatRequest, http_request: Request, authorization:
595693
response = await http_client.post(endpoint, json=payload, headers=headers)
596694
response.raise_for_status()
597695
result = response.json()
598-
696+
697+
# Repair leaked tool-call arg dialect (#85) before returning OR
698+
# logging, so neither the client nor the log ever sees the markup.
699+
dialect_repaired = _normalize_response_tool_calls(result)
700+
if dialect_repaired:
701+
print(f"🧹 Normalized {dialect_repaired} tool-call argument(s) with "
702+
f"leaked arg dialect (model={request.model}, request_id={request_id})",
703+
flush=True)
704+
599705
# Log successful response
600706
latency_ms = int((time.time() - start_time) * 1000)
601-
log_response(provider_name, request.model, result, latency_ms, origin=origin, request_id=request_id, session_id=session_id, client=client)
707+
log_response(provider_name, request.model, result, latency_ms, origin=origin, request_id=request_id, session_id=session_id, client=client, dialect_repaired=dialect_repaired)
602708

603709
return result
604710

test_logging.py

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,142 @@ def test_openrouter_only_knobs():
498498
assert "usage" not in nrp_payload, "usage must not leak to non-OpenRouter"
499499

500500

501+
# --- Tool-call arg-dialect normalization (#85) -------------------------------
502+
# glm-5.2 (and the qwen family) intermittently leak their tool-call arg encoding
503+
# into the structured `arguments`. Verbatim symptom from the issue: a valid outer
504+
# JSON object whose `value_stats` value is wrapped in the GLM XML arg dialect.
505+
506+
def test_normalize_glm_value_level_leak():
507+
"""#85: dialect leaked into one value of an otherwise-valid JSON object.
508+
The wrapper is stripped and the intended JSON payload comes back structured."""
509+
p = importlib.reload(llm_proxy)
510+
inner = {"by_res": {"2": {"max": 9.45, "min": 0.1}}}
511+
args = json.dumps({
512+
"layer_id": "hardwood",
513+
"value_stats": f'<arg_key>value_stats</arg_key> <arg_value>{json.dumps(inner)}</arg_value>',
514+
})
515+
out, changed = p._normalize_tool_call_arguments(args)
516+
assert changed
517+
parsed = json.loads(out)
518+
assert parsed["value_stats"] == inner # structured, not a string
519+
assert parsed["layer_id"] == "hardwood" # untouched
520+
assert "<arg_key>" not in out and "<arg_value>" not in out
521+
522+
523+
def test_normalize_glm_value_level_leak_unterminated():
524+
"""The leaked value may arrive without a closing </arg_value> tag (as the
525+
issue's truncated capture showed). We still recover the payload up to end."""
526+
p = importlib.reload(llm_proxy)
527+
inner = {"by_res": {"2": {"max": 9.45}}}
528+
args = json.dumps({
529+
"value_stats": f'<arg_key>value_stats</arg_key> <arg_value>{json.dumps(inner)}',
530+
})
531+
out, changed = p._normalize_tool_call_arguments(args)
532+
assert changed
533+
assert json.loads(out)["value_stats"] == inner
534+
535+
536+
def test_normalize_whole_string_glm_dialect():
537+
"""The entire `arguments` string is raw GLM dialect (no valid outer JSON)."""
538+
p = importlib.reload(llm_proxy)
539+
raw = ('<arg_key>layer_id</arg_key> <arg_value>hardwood</arg_value> '
540+
'<arg_key>opacity</arg_key> <arg_value>0.5</arg_value>')
541+
out, changed = p._normalize_tool_call_arguments(raw)
542+
assert changed
543+
parsed = json.loads(out)
544+
assert parsed == {"layer_id": "hardwood", "opacity": 0.5}
545+
546+
547+
def test_normalize_qwen_parameter_dialect():
548+
"""The qwen/hermes `<parameter=NAME>VALUE</parameter>` form of the same leak."""
549+
p = importlib.reload(llm_proxy)
550+
args = json.dumps({"sql": "<parameter=sql>SELECT 1</parameter>"})
551+
out, changed = p._normalize_tool_call_arguments(args)
552+
assert changed
553+
assert json.loads(out)["sql"] == "SELECT 1"
554+
555+
556+
def test_normalize_leaves_clean_arguments_untouched():
557+
"""No dialect markers → byte-identical passthrough, no wasted re-serialize."""
558+
p = importlib.reload(llm_proxy)
559+
args = json.dumps({"sql": "SELECT * FROM t WHERE a < 5", "n": 3})
560+
out, changed = p._normalize_tool_call_arguments(args)
561+
assert not changed
562+
assert out == args
563+
564+
565+
def test_normalize_response_tool_calls_in_place_and_counts():
566+
"""The response-level pass mutates result in place and returns a repair count;
567+
a clean sibling tool call in the same response is left alone."""
568+
p = importlib.reload(llm_proxy)
569+
result = {"choices": [{"message": {"tool_calls": [
570+
{"function": {"name": "add_hex_tile_layer", "arguments": json.dumps(
571+
{"value_stats": '<arg_value>{"by_res": {"2": {"max": 1}}}</arg_value>'})}},
572+
{"function": {"name": "get_schema", "arguments": '{"dataset": "ca"}'}},
573+
]}}]}
574+
n = p._normalize_response_tool_calls(result)
575+
assert n == 1
576+
tcs = result["choices"][0]["message"]["tool_calls"]
577+
assert json.loads(tcs[0]["function"]["arguments"])["value_stats"] == {"by_res": {"2": {"max": 1}}}
578+
assert tcs[1]["function"]["arguments"] == '{"dataset": "ca"}'
579+
580+
581+
def test_normalize_response_is_defensive_on_garbage():
582+
"""Malformed shapes never raise — normalization must not break serving."""
583+
p = importlib.reload(llm_proxy)
584+
for junk in ({}, {"choices": None}, {"choices": [None]},
585+
{"choices": [{"message": {"tool_calls": [{"function": None}]}}]},
586+
{"choices": [{"message": {"tool_calls": "nope"}}]}):
587+
assert p._normalize_response_tool_calls(junk) == 0
588+
589+
590+
def test_handler_repairs_dialect_and_logs_count():
591+
"""End-to-end: a glm-5.2 response with a leaked value is repaired before it
592+
is returned to the client, and the repair count is recorded in the log."""
593+
import asyncio
594+
from unittest.mock import patch
595+
596+
p = _reload(PROXY_KEY="testkey")
597+
p._log_buffer.clear()
598+
599+
leaked = json.dumps({"value_stats": '<arg_key>value_stats</arg_key> <arg_value>{"by_res": {"2": {"max": 9.45}}}</arg_value>'})
600+
601+
class _FakeResp:
602+
def raise_for_status(self):
603+
pass
604+
def json(self):
605+
return {"choices": [{"message": {
606+
"content": None,
607+
"tool_calls": [{"function": {"name": "add_hex_tile_layer",
608+
"arguments": leaked}}]}}],
609+
"usage": {"total_tokens": 5}}
610+
611+
class _FakeAsyncClient:
612+
def __init__(self, *a, **k):
613+
pass
614+
async def __aenter__(self):
615+
return self
616+
async def __aexit__(self, *a):
617+
return False
618+
async def post(self, *a, **k):
619+
return _FakeResp()
620+
621+
class _FakeRequest:
622+
headers = {"origin": "https://ca-30x30.nrp-nautilus.io"}
623+
624+
req = p.ChatRequest(model="z-ai/glm-5.2", messages=[{"role": "user", "content": "what fraction of ca hardwood is protected?"}])
625+
with patch.object(p, "get_provider_for_model",
626+
return_value=("openrouter", {"endpoint": "http://or", "api_key": "k"})), \
627+
patch.object(p.httpx, "AsyncClient", _FakeAsyncClient):
628+
result = asyncio.run(p.proxy_chat(req, _FakeRequest(), authorization="Bearer testkey"))
629+
630+
returned = json.loads(result["choices"][0]["message"]["tool_calls"][0]["function"]["arguments"])
631+
assert returned["value_stats"] == {"by_res": {"2": {"max": 9.45}}} # client gets structured data
632+
responses = [e for e in p._log_buffer if e.get("type") == "response"]
633+
assert responses[0]["tool_call_dialect_repaired"] == 1
634+
assert "<arg_key>" not in json.dumps(responses[0]) # log is clean too
635+
636+
501637
if __name__ == "__main__":
502638
import sys
503639
fns = [v for k, v in sorted(globals().items()) if k.startswith("test_") and callable(v)]

0 commit comments

Comments
 (0)