Skip to content

Commit 071ac94

Browse files
fix(adapters): parse() crashed on a malformed tool_input, and a crash is an allow (#63)
parse() raised AttributeError whenever tool_input was a truthy non-dict, on six adapters, and whenever edits contained non-dicts, on three. dispatch.run wraps only the JSON decode, so the exception killed the hook with exit 1 -- a non-blocking error on almost every vendor in the matrix. The tool call proceeded. A guard that crashes is a guard that allows. Harden every site with an isinstance check, and add a fourth CI invariant: parse() never raises, over nine hostile payload shapes x every adapter, 108 cases. Suite 857 -> 965 passing.
1 parent c726541 commit 071ac94

8 files changed

Lines changed: 106 additions & 20 deletions

File tree

CHANGELOG.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,21 @@ versioning: [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
8787
the explicit allow**, because `updatedInput` is the only way to express a rewrite and
8888
approving the *substituted* call is what the handler asked for. `codex_cli` already
8989
behaved this way, for a different vendor reason.
90+
- **Six adapters crashed on a malformed `tool_input`, and a crash is an allow.** `parse()`
91+
raised `AttributeError` whenever `tool_input` was a truthy non-dict (a string, a list, a
92+
number), and three raised on an `edits` list holding non-dicts. `dispatch.run` wraps only
93+
the JSON decode — everything after it runs unprotected — so the exception killed the hook
94+
with exit 1, which almost every vendor here treats as a non-blocking error. The call
95+
proceeds. The one failure this library exists to prevent, caused by the library.
96+
97+
Five adapters already hardened with `isinstance`; the inconsistency is what kept it
98+
invisible. `tool_input` is whatever the agent chose to serialise, and its shape is not
99+
ours to assume.
100+
101+
### Added
102+
- **A fourth invariant: `parse()` never raises.** Nine hostile payload shapes × every
103+
adapter, 108 cases. None exotic — a vendor adding a scalar-argument tool, or a serialiser
104+
that flattens, produces them.
90105

91106
- **`installed()` answered a substring question, not an ownership one.** It was
92107
`owner in json.dumps(config)`, a text search over the whole serialised file. Two ways that

src/agentseam/adapters/claude_code.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,13 +111,18 @@ def claims(raw):
111111

112112

113113
def parse(raw):
114-
ti = raw.get("tool_input") or {}
114+
ti = raw.get("tool_input")
115+
# A guard that crashes is a guard that allows: dispatch only wraps the JSON decode, so
116+
# an exception here kills the hook with exit 1, which most vendors treat as a
117+
# non-blocking error and let the call through. tool_input is whatever the agent chose
118+
# to serialise, so it is not ours to assume the shape of.
119+
ti = ti if isinstance(ti, dict) else {}
115120
tool = raw.get("tool_name")
116121
# new_source is NotebookEdit's cell body -- the tool is in WRITE_TOOLS, so claiming to
117122
# handle it while dropping its content is an internal contradiction, not a vendor guess.
118123
content = ti.get("content") or ti.get("new_string") or ti.get("new_source") or None
119124
if content is None and isinstance(ti.get("edits"), list):
120-
joined = "\n".join(str(e.get("new_string", "")) for e in ti["edits"])
125+
joined = "\n".join(str(e.get("new_string", "")) for e in ti["edits"] if isinstance(e, dict))
121126
content = joined or None
122127
out = raw.get("tool_output")
123128
if isinstance(out, (dict, list)):

src/agentseam/adapters/devin.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,12 @@ def claims(raw):
105105

106106

107107
def parse(raw):
108-
ti = raw.get("tool_input") or {}
108+
ti = raw.get("tool_input")
109+
# A guard that crashes is a guard that allows: dispatch only wraps the JSON decode, so
110+
# an exception here kills the hook with exit 1, which most vendors treat as a
111+
# non-blocking error and let the call through. tool_input is whatever the agent chose
112+
# to serialise, so it is not ours to assume the shape of.
113+
ti = ti if isinstance(ti, dict) else {}
109114
content = ti.get("content") or ti.get("new_string") or None
110115
out = raw.get("tool_output")
111116
if isinstance(out, (dict, list)):

src/agentseam/adapters/gemini_cli.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,12 @@ def claims(raw):
9090

9191

9292
def parse(raw):
93-
ti = raw.get("tool_input") or {}
93+
ti = raw.get("tool_input")
94+
# A guard that crashes is a guard that allows: dispatch only wraps the JSON decode, so
95+
# an exception here kills the hook with exit 1, which most vendors treat as a
96+
# non-blocking error and let the call through. tool_input is whatever the agent chose
97+
# to serialise, so it is not ours to assume the shape of.
98+
ti = ti if isinstance(ti, dict) else {}
9499
tool = raw.get("tool_name")
95100
content = None
96101
if tool in WRITE_TOOLS:

src/agentseam/adapters/junie.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ def parse(raw):
9595
# the same fallback chain claude_code.parse uses -- not a guess, a claim we already made.
9696
content = ti.get("content") or ti.get("new_string") or ti.get("new_source") or None
9797
if content is None and isinstance(ti.get("edits"), list):
98-
joined = "\n".join(str(e.get("new_string", "")) for e in ti["edits"])
98+
joined = "\n".join(str(e.get("new_string", "")) for e in ti["edits"] if isinstance(e, dict))
9999
content = joined or None
100100
return Event(
101101
AGENT,

src/agentseam/adapters/kimi_code.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ def parse(raw):
116116
# same fallback chain claude_code.parse uses -- not a guess, a claim we already made.
117117
content = ti.get("content") or ti.get("new_string") or ti.get("new_source") or None
118118
if content is None and isinstance(ti.get("edits"), list):
119-
joined = "\n".join(str(e.get("new_string", "")) for e in ti["edits"])
119+
joined = "\n".join(str(e.get("new_string", "")) for e in ti["edits"] if isinstance(e, dict))
120120
content = joined or None
121121
out = raw.get("tool_output")
122122
if isinstance(out, (dict, list)):

src/agentseam/adapters/vscode_copilot.py

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,7 @@
7878
_CURSOR_MARKERS = ("model", "cursor_version", "conversation_id", "generation_id", "workspace_roots")
7979

8080
#: Copilot CLI's camelCase names, which no other vendor uses -- the name alone identifies
81-
#: these. Derived from EVENT_MAP: a hand-kept list drifted, leaving payloads claimed by
82-
#: the matrix but by no adapter, and so allowed through.
81+
#: these. Derived from EVENT_MAP: a hand-kept list drifted and left payloads unclaimed.
8382
_CLAIMABLE = tuple(name for name in EVENT_MAP if name[:1].islower())
8483

8584
#: executeHook merges {timestamp, hook_event_name, session_id?, transcript_path?} into
@@ -115,12 +114,14 @@ def claims(raw):
115114
if name in _CLAIMABLE:
116115
return True
117116
# memory-tool payloads are unmistakable
118-
ti = raw.get("tool_input") or {}
117+
# The isinstance guard also keeps a non-dict tool_input from raising here.
118+
ti = raw.get("tool_input")
119119
return raw.get("tool_name") in MEMORY_TOOLS and isinstance(ti, dict) and "command" in ti
120120

121121

122122
def parse(raw):
123-
ti = raw.get("tool_input") or {}
123+
ti = raw.get("tool_input")
124+
ti = ti if isinstance(ti, dict) else {}
124125
tool = raw.get("tool_name") or raw.get("toolName")
125126
path = content = None
126127
if tool in MEMORY_TOOLS:
@@ -158,15 +159,15 @@ def parse(raw):
158159

159160
def is_memory_write(event):
160161
"""True when this event is a memory-tool content write (VS Code's memory surface)."""
161-
ti = event.raw.get("tool_input") or {}
162+
ti = event.raw.get("tool_input")
163+
ti = ti if isinstance(ti, dict) else {}
162164
return event.tool in MEMORY_TOOLS and ti.get("command") in MEMORY_WRITE_COMMANDS
163165

164166

165-
#: Events whose block verdict is a TOP-LEVEL {decision: "block", reason}.
166-
#: UserPromptSubmitHookOutput declares exactly those two fields and
167-
#: defaultIntentRequestHandler reads `typedOutput.decision` off the root object;
168-
#: executePostToolUseHook reads the same two off a PostToolUse response's root, where a
169-
#: block feeds the reason back to the model instead of the tool result.
167+
#: Block verdict at the TOP LEVEL: {decision: "block", reason}.
168+
#: UserPromptSubmitHookOutput declares exactly those two and
169+
#: defaultIntentRequestHandler reads `typedOutput.decision` off the root;
170+
#: executePostToolUseHook reads the same two off a PostToolUse response's root.
170171
_TOP_LEVEL_BLOCK = (PROMPT_SUBMIT, POST_TOOL)
171172

172173
#: Block verdict NESTED in hookSpecificOutput. executeStopHook requires BOTH decision and
@@ -253,10 +254,9 @@ def respond(decision, event):
253254
#:
254255
#: PascalCase, i.e. VS Code's spelling, because CONFIG_PATH is VS Code's file.
255256
#: `parseCopilotHooks` resolves each key with `resolveCopilotCliHookType(id) ?? toHookType
256-
#: (id)`, so both spellings work there -- but only one may be written: it does
257-
#: `result.set(hookType, ...)`, an overwrite, so two keys resolving to the same HookType
258-
#: silently drop one. SESSION_END is therefore unwireable: `sessionEnd` is a Copilot CLI
259-
#: name VS Code never fires, claimed for parsing and not offered for installing.
257+
#: (id)`, so both spellings work -- but only one may be written: it does `result.set(...)`,
258+
#: an overwrite, so two keys resolving to the same HookType silently drop one. SESSION_END
259+
#: is therefore unwireable: a Copilot CLI name VS Code never fires.
260260
REVERSE_EVENT_MAP = {
261261
PRE_TOOL: "PreToolUse",
262262
POST_TOOL: "PostToolUse",

tests/test_parse_is_total.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
"""`parse()` must not raise, whatever the payload looks like.
2+
3+
A guard that crashes is a guard that allows. `dispatch.run` wraps only the JSON decode --
4+
everything after it, including `parse()`, runs unprotected -- so an exception here kills the
5+
hook process with exit 1, and exit 1 is a non-blocking error on almost every vendor here.
6+
The call proceeds. The one thing this library exists to prevent, caused by the library.
7+
8+
Six adapters crashed on a non-dict `tool_input` and three on an `edits` list containing
9+
non-dicts, while five others already hardened with `isinstance` -- the inconsistency is what
10+
made it invisible. `tool_input` is whatever the agent chose to serialise; its shape is not
11+
ours to assume.
12+
"""
13+
14+
from __future__ import annotations
15+
16+
import sys
17+
from pathlib import Path
18+
19+
import pytest
20+
21+
sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "src"))
22+
23+
from agentseam import adapters # noqa: E402
24+
25+
#: Shapes a payload could take that are not the one the adapter expects. Not exotic: a
26+
#: vendor adding a scalar-argument tool, or a serialiser that flattens, produces these.
27+
_HOSTILE = {
28+
"tool_input is a string": {"tool_input": "rm -rf /"},
29+
"tool_input is a list": {"tool_input": ["a"]},
30+
"tool_input is a number": {"tool_input": 7},
31+
"tool_input is null": {"tool_input": None},
32+
"edits holds non-dicts": {"tool_input": {"edits": ["oops", None, 3]}},
33+
"edits is not a list": {"tool_input": {"edits": "nope"}},
34+
"content is a dict": {"tool_input": {"content": {"nested": True}}},
35+
"tool_output is a list": {"tool_input": {}, "tool_output": [1, 2]},
36+
"no tool_input at all": {},
37+
}
38+
39+
40+
@pytest.mark.parametrize("agent", sorted(adapters.ADAPTERS))
41+
@pytest.mark.parametrize("label", sorted(_HOSTILE))
42+
def test_parse_never_raises(agent, label):
43+
mod = adapters.get(agent)
44+
raw = dict(
45+
{
46+
"hook_event_name": "PreToolUse",
47+
"hookEventName": "preToolUse",
48+
"tool_name": "Write",
49+
"client_type": getattr(mod, "CLIENT_TYPE", None),
50+
},
51+
**_HOSTILE[label],
52+
)
53+
try:
54+
mod.parse(raw)
55+
except Exception as exc: # noqa: BLE001 -- the whole point is that nothing escapes
56+
pytest.fail("%s.parse raised %s on %s: %s" % (agent, type(exc).__name__, label, exc))

0 commit comments

Comments
 (0)