fix(mcp): parse gate-exec-write input with python json+shlex#69
Open
phjlljp wants to merge 1 commit into
Open
Conversation
The bash-regex JSON parser silently failed open on five command shapes that the underlying MCP server's tokenizer happily accepts: - leading whitespace before `call` - tab separator between `call` and the tool name - leading newline inside the JSON command value - JSON-escaped quotes around the tool name (`call "experiment-update"`) - upper-case `CALL` Each let an agent issue a destructive PostHog write (experiment-update, notebooks-destroy, cdp-functions-delete, ...) without surfacing the permission prompt the hook was added for. The hardcoded write-verb list was also incomplete — `purge`, `revoke`, `truncate`, and `terminate` matched no entry and silently fell through. Switch JSON parsing and command tokenization to python3 (already required by the SessionEnd hook), so any command-shape the server accepts is parsed identically by the gate. Default-deny on parse failure or on a `call` with no extractable tool name. Add the missing write verbs. Tests: 11 new cases (5 bypass regressions, 4 new write verbs, 2 default-deny shapes). All 32 pass; 21 prior cases unchanged. Bumps .claude-plugin/plugin.json to 1.1.21 (Claude-only change).
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the bash-regex JSON parser inside
hooks/gate-exec-write.shwithpython3 + json.loads + shlex.split. python3 is already required by the SessionEnd hook so this introduces no new dependency. Adds four write verbs that the hardcoded list was missing (purge,revoke,truncate,terminate,kill,void,expire,flush,clear,grant).The previous regex parser silently fell through to
exit 0(no prompt) on five command shapes that the MCP server's underlying CLI tokenizer accepts as ordinarycallinvocations:"command":" call experiment-update {}"(leading whitespace)"command":"call\texperiment-update {}"(tab separator)"command":"\ncall experiment-update {}"(leading newline)"command":"CALL experiment-update {}"(uppercase verb)"command":"call \"experiment-update\" {}"(JSON-escaped quotes around the tool name)Each one let an agent issue a destructive PostHog write —
experiment-update,notebooks-destroy,cdp-functions-delete, etc. — without surfacing the permission prompt this hook exists to enforce. Once a user allow-listsmcp__posthog__exec, the gate is the only thing standing between an agent and a state-changing call. Closing the bypasses restores the property the hook was added in #42 to provide.The existing
POSTHOG_MCP_EXEC_GATE_ALLOWenv-var contract is preserved (no behaviour change for users who allow-list specific tools).Why python3 instead of more bash
Bash regex over raw JSON conflated value-content with field-structure. Each bypass exploited a place where the regex assumed structural simplicity that JSON doesn't actually guarantee — leading whitespace inside a value, JSON
\n/\tescapes, alternate field ordering, JSON-escaped inner quotes.json.loads+shlex.splitlets the gate parse the command exactly the way a kebab-case CLI would, so any input the server tokenizes the same way is classified the same way.The hook also now default-denies (returns
permissionDecision: ask) when the command can't be cleanly tokenized at all (unbalanced quotes, etc.) or whencallis invoked without a recognizable tool name. Better to over-prompt on a malformed payload than silently let it through.Bash 3.2 compatibility
mapfilewould have been the natural way to read the four-line python output, but macOS still ships bash 3.2. The portableread -rchain stays compatible with the plugin's existing local-dev story.Test plan
./tests/test_gate_exec_write.sh— 32/32 pass (21 original cases unchanged + 11 new: 5 bypass regressions, 4 newly-classified write verbs, 2 default-deny shapes)pytest tests/test_session_parser.py tests/test_posthog_llma.py— 51/51 pass (untouched by this PR)CALL, JSON-quoted tool name → all promptcall data-purge,api-key-revoke,event-truncate,session-terminate→ all promptcall "unterminated quote,callwith no tool → default-deny promptNotes
This is one of three security PRs from a strict-zero-trust audit; the other two cover
POSTHOG_HOSTvalidation andsync-skills.ymlzip-slip / auto-merge. They land independently of this one.Bumps
.claude-plugin/plugin.jsonto 1.1.21 (Claude-only change, matching the convention used by #59 and #42).