From 984a2c702c7ea42436c914d0fdf434cf2dd3f45c Mon Sep 17 00:00:00 2001 From: Abhay Singh Date: Sun, 12 Jul 2026 21:04:23 +0530 Subject: [PATCH] fix(ccr): don't crash parse_tool_call on non-object tool arguments (#2071) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description `parse_tool_call` (`headroom/ccr/tool_injection.py`) extracts the retrieval hash from a CCR tool call. For the OpenAI and `openai_responses` shapes it decodes the `arguments` string with `json.loads` and catches only `JSONDecodeError`: ```python args_str = function.get("arguments", "{}") try: input_data = json.loads(args_str) except json.JSONDecodeError: input_data = {} ... hash_key = input_data.get("hash") # assumes input_data is a dict ``` If a (confused) model emits `arguments='[]'` / `'"abc"'` / `'123'`, `json.loads` succeeds and returns a **list / str / number**, so `input_data.get("hash")` raises `AttributeError`. A null value (`arguments: null` → `json.loads(None)`) raises an uncaught `TypeError`. The Anthropic branch has the same hazard if `tool_call["input"]` is present but not a dict. `parse_tool_call` is called from `parse_ccr_tool_calls` (`ccr/tool_calls.py`) and the server CCR path with no guard for this, so a malformed CCR-named tool call **crashes CCR response processing** instead of being ignored. Closes: no issue filed — found while auditing the CCR tool-call parsing. ## Fix - Catch `TypeError` as well as `JSONDecodeError` around `json.loads` (covers `arguments: null`). - Return `None` when `input_data` is not a `dict` — a non-object tool call simply isn't a valid CCR call. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - `headroom/ccr/tool_injection.py`: widen the decode `except` to `(json.JSONDecodeError, TypeError)`; return `None` for non-dict `input_data`. - `tests/test_ccr_tool_injection.py`: add tests for non-object OpenAI arguments (`[]`/`"abc"`/`123`), null arguments, and a non-dict Anthropic `input`. ## Testing - [x] New regression tests added (`tests/test_ccr_tool_injection.py`) - [x] Linting/formatting clean — run with the CI-pinned `ruff==0.15.17` - [ ] Full `pytest` deferred to CI (local-OOM reason below). ```text $ uvx ruff@0.15.17 check headroom/ccr/tool_injection.py tests/test_ccr_tool_injection.py All checks passed! ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.10, headroom from this branch. Importing `headroom` pulls in the torch/transformers stack and a full `pytest` gets OOM-killed on this box, so I verified the parse logic with a dependency-free script and left the full pytest to CI. - Exact command / steps: ran the four crash vectors (openai `[]`, `"abc"`, `null`; anthropic non-dict `input`) plus a valid CCR call and a non-CCR call through the old and new logic. - Observed result: the old parser crashes on every malformed case; the new one returns `None` and still parses a valid call: ```text OK [openai] '[]': old CRASHED -> new None OK [openai] '"abc"': old CRASHED -> new None OK [openai] None: old CRASHED -> new None OK [anthropic] ['not', 'a', 'dict']: old CRASHED -> new None PARSE_TOOL_CALL NON-DICT FIX VERIFIED (old crashes; new returns None; valid still parses) ``` - Not tested: a full CCR response round-trip with a malformed tool call (needs the heavy stack). The fix is confined to `parse_tool_call` and the new tests drive it directly. Full local `pytest` deferred to CI (OOM, per above). ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes — ran lint + a standalone logic check; full pytest deferred to CI (local OOM, disclosed above) - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes - Two-line hardening plus tests; no new dependencies. - @JerrettDavis tagging you — a malformed CCR-named tool call currently crashes CCR response processing; quick one. Thanks! --- CHANGELOG.md | 1 + headroom/ccr/tool_injection.py | 12 ++++++++++-- tests/test_ccr_tool_injection.py | 17 +++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 82fe80f67..22dce9efb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Bug Fixes +* **ccr:** don't crash `parse_tool_call` on a CCR tool call whose arguments aren't an object. For the OpenAI/`openai_responses` shape the arguments are `json.loads`-decoded and only `JSONDecodeError` was caught, so a model that emitted `arguments='[]'`/`'"abc"'`/`'123'` (decoding to a list/str/number) — or a non-dict Anthropic `input` — reached `input_data.get("hash")` and raised `AttributeError`; a null `arguments` raised an uncaught `TypeError` from `json.loads(None)`. Both are now handled: the decode also catches `TypeError`, and a non-dict `input_data` returns `None` (not a valid CCR call) instead of crashing CCR response processing. * **cache/semantic:** key entries by the full-context hash, not the trailing query text. `SemanticCache.put` stored each response under `sha256(query)[:16]` where `query` is only the last user message, and the exact-match branch of `get` returned the slot without checking the stored entry's `messages_hash`. Two requests that share a trailing message ("continue", "yes", "run the tests") but differ in earlier context therefore collided on one slot — the second overwrote the first, and the first's hash then resolved to the second's cached response (wrong data served). Entries are now keyed by `messages_hash` when present, and `get` verifies `entry.messages_hash` before returning. * **proxy/openai:** stop PRE_SEND from reintroducing `tools: []` after the direct #728 fix. The OpenAI request handler now mirrors the existing `tools or _original_tools is not None` body-write guard during PRE_SEND write-back, so providers that reject empty tool arrays no longer see a tools field when the client omitted it, while explicit client `tools: []` remains preserved ([#1983](https://github.com/headroomlabs-ai/headroom/issues/1983)). * **proxy/openai:** keep the exact Responses function name `terminal` resident during OpenAI tool-search deferral so cache-mode optimization stops forwarding `terminal.terminal` and triggering the reserved-namespace 400 on Codex Responses ([#1946](https://github.com/headroomlabs-ai/headroom/issues/1946)). diff --git a/headroom/ccr/tool_injection.py b/headroom/ccr/tool_injection.py index d8e744535..118cec849 100644 --- a/headroom/ccr/tool_injection.py +++ b/headroom/ccr/tool_injection.py @@ -465,7 +465,8 @@ def parse_tool_call( args_str = function.get("arguments", "{}") try: input_data = json.loads(args_str) - except json.JSONDecodeError: + except (json.JSONDecodeError, TypeError): + # TypeError covers a null/None `arguments` value (json.loads(None)). input_data = {} elif provider == "google": # Google/Gemini format: {"functionCall": {"name": "...", "args": {...}}} @@ -480,7 +481,8 @@ def parse_tool_call( args_str = tool_call.get("arguments", "{}") try: input_data = json.loads(args_str) - except json.JSONDecodeError: + except (json.JSONDecodeError, TypeError): + # TypeError covers a null/None `arguments` value (json.loads(None)). input_data = {} else: # Generic fallback @@ -490,6 +492,12 @@ def parse_tool_call( if name != CCR_TOOL_NAME: return None + # A CCR-named tool call whose decoded arguments/input are not an object + # (a JSON array/string/number, or a non-dict Anthropic `input`) is simply + # not a valid CCR call — return None instead of crashing on `.get`. + if not isinstance(input_data, dict): + return None + hash_key = input_data.get("hash") if hash_key is None: return None diff --git a/tests/test_ccr_tool_injection.py b/tests/test_ccr_tool_injection.py index 294b725e5..7a781f7db 100644 --- a/tests/test_ccr_tool_injection.py +++ b/tests/test_ccr_tool_injection.py @@ -313,6 +313,23 @@ class TestParseToolCall: assert hash_key is None + def test_parse_openai_non_object_arguments_returns_none(self): + """OpenAI arguments that decode to a non-object (array/string/number) + must return None, not crash on `.get`.""" + for args in ("[]", '"abc"', "123"): + tool_call = {"function": {"name": CCR_TOOL_NAME, "arguments": args}} + assert parse_tool_call(tool_call, "openai") is None + + def test_parse_openai_null_arguments_returns_none(self): + """A null `arguments` value (json.loads(None) -> TypeError) is handled.""" + tool_call = {"function": {"name": CCR_TOOL_NAME, "arguments": None}} + assert parse_tool_call(tool_call, "openai") is None + + def test_parse_anthropic_non_dict_input_returns_none(self): + """A non-dict Anthropic `input` must return None, not crash.""" + tool_call = {"name": CCR_TOOL_NAME, "input": ["not", "a", "dict"]} + assert parse_tool_call(tool_call, "anthropic") is None + class TestHashSecurityValidation: """Test hash validation security measures.