diff --git a/backend/client_config.py b/backend/client_config.py index c9772b4..c3291b2 100644 --- a/backend/client_config.py +++ b/backend/client_config.py @@ -269,14 +269,34 @@ def mcp_entry(platform: str) -> dict: def _hooks_command_claude() -> str: - """Claude/WorkBuddy: curl the hook with $CLAUDE_TOOL_USE_INPUT.""" + """Claude/WorkBuddy: curl the hook, forwarding stdin as the POST body. + + Claude Code writes the PreToolUse JSON payload to the hook command's + **stdin** — there is no ``$CLAUDE_TOOL_USE_INPUT`` env var (that was a + mistaken assumption from an earlier version; confirmed against Claude + Code's hooks docs). A single-quoted ``'$CLAUDE_TOOL_USE_INPUT'`` is never + shell-expanded either way, so the old command always POSTed that literal + string — invalid JSON, so the backend 422'd on *every* call and curl's + non-2xx exit code (0) meant Claude Code silently treated it as "allow". + ``-d @-`` tells curl to read the POST body from stdin instead. + """ return ( f"curl -s -X POST {HOOK_ENDPOINT} " "-H 'Content-Type: application/json' " - "-d '$CLAUDE_TOOL_USE_INPUT'" + "-d @-" ) +# Old, broken Claude command (see _hooks_command_claude docstring) — kept only +# so _matcher_is_mine still recognises an install written before this fix and +# upgrades it in place instead of appending a second, duplicate matcher. +_LEGACY_CLAUDE_HOOK_COMMANDS = ( + f"curl -s -X POST {HOOK_ENDPOINT} " + "-H 'Content-Type: application/json' " + "-d '$CLAUDE_TOOL_USE_INPUT'", +) + + def _hooks_command_codebuddy() -> str: """CodeBuddy: forward stdin JSON via the ``hooks_forward`` helper. @@ -369,21 +389,24 @@ def hooks_matcher(platform: str) -> dict: def _matcher_is_mine(matcher: dict, cmd: str) -> bool: """True if a PreToolUse matcher is the MyKnowledge hook (by command signature). - Recognises our hook by its command (claude/curl-$CLAUDE_TOOL_USE_INPUT, - codebuddy/hooks_forward.py, or cursor's direct-entry form) so we never - mistake a user's own hook for ours. Handles both the Claude/CodeBuddy - nested ``hooks[0].command`` shape and Cursor's direct ``command`` entry. + Recognises our hook by its command (claude/curl-@-, codebuddy/hooks_forward.py, + or cursor's direct-entry form) so we never mistake a user's own hook for + ours. Handles both the Claude/CodeBuddy nested ``hooks[0].command`` shape + and Cursor's direct ``command`` entry. Also matches + ``_LEGACY_CLAUDE_HOOK_COMMANDS`` so an install written before the + stdin/``-d @-`` fix is upgraded in place rather than duplicated. """ if not isinstance(matcher, dict): return False + recognised = (cmd, *_LEGACY_CLAUDE_HOOK_COMMANDS) # Cursor entries carry the command directly on the entry. - if matcher.get("command") == cmd: + if matcher.get("command") in recognised: return True hooks = matcher.get("hooks") if not isinstance(hooks, list) or not hooks: return False first = hooks[0] - return isinstance(first, dict) and first.get("command") == cmd + return isinstance(first, dict) and first.get("command") in recognised def _merge_hook_entry(matchers: list, entry: dict, cmd: str) -> list: @@ -391,16 +414,16 @@ def _merge_hook_entry(matchers: list, entry: dict, cmd: str) -> list: The matcher is identified by **command signature** (``_matcher_is_mine``), so a user's own hook sharing a matcher string is never touched. If our hook already - exists but its ``matcher`` string is outdated (e.g. the old Claude ``Bash`` that - skipped Write/Edit), we replace it with the current entry instead of leaving the - stale one — otherwise re-running ``write_kind`` on an existing install would keep - the broken matcher and the fix would never take effect. + exists but is stale — an outdated ``matcher`` (e.g. the old Claude ``Bash`` that + skipped Write/Edit) or an outdated ``command`` (e.g. the old + ``$CLAUDE_TOOL_USE_INPUT`` payload bug) — we overwrite it with the current entry + unconditionally. Comparing only ``matcher`` here previously let a stale + ``command`` survive re-installs whenever the matcher string already matched, + so re-running ``write_kind`` silently kept the broken command forever. """ for i, m in enumerate(matchers): if _matcher_is_mine(m, cmd): - if isinstance(m, dict) and m.get("matcher") == entry.get("matcher"): - return matchers # already current — nothing to do - matchers[i] = entry # upgrade stale matcher in place + matchers[i] = entry # upgrade stale matcher/command in place return matchers matchers.append(entry) return matchers diff --git a/tests/test_client_config.py b/tests/test_client_config.py index fe6b7b6..038177e 100644 --- a/tests/test_client_config.py +++ b/tests/test_client_config.py @@ -146,15 +146,33 @@ def test_upgrades_stale_matcher_in_place(self, fake_home: Path) -> None: write_kind("ClaudeCode", "hooks") data = _read_json(s) matchers = data["hooks"]["PreToolUse"] - # 我们的钩子 matcher 已升级,且仍是唯一的 MyKnowledge 钩子(不重复追加)。 + # 我们的钩子已升级到当前命令(-d @-),且仍是唯一的 MyKnowledge 钩子(不重复追加)。 my = [m for m in matchers - if "$CLAUDE_TOOL_USE_INPUT" in m["hooks"][0]["command"]] + if "-d @-" in m["hooks"][0]["command"]] assert len(my) == 1 assert my[0]["matcher"] == "Bash|Write|Edit" + assert "$CLAUDE_TOOL_USE_INPUT" not in my[0]["hooks"][0]["command"] # 用户的钩子(不同 command)不受影响。 user = [m for m in matchers if m["hooks"][0]["command"] == "user-hook"] assert user and user[0]["matcher"] == "Edit|Write" + def test_upgrades_legacy_command_not_duplicated(self, fake_home: Path) -> None: + """Writing twice over a legacy-command install never yields 2+ of our matchers.""" + s = fake_home / ".claude" / "settings.json" + _write_json(s, {"hooks": { + "PreToolUse": [ + {"matcher": "Bash|Write|Edit", + "hooks": [{"type": "command", + "command": "curl -s -X POST http://127.0.0.1:8080/hooks/pre-tool-use " + "-H 'Content-Type: application/json' -d '$CLAUDE_TOOL_USE_INPUT'"}]}, + ], + }}) + write_kind("ClaudeCode", "hooks") + write_kind("ClaudeCode", "hooks") + matchers = _read_json(s)["hooks"]["PreToolUse"] + assert len(matchers) == 1 + assert "-d @-" in matchers[0]["hooks"][0]["command"] + class TestAgent: def test_creates_agent_file(self, fake_home: Path) -> None: @@ -414,11 +432,12 @@ def test_bad_kind(self, fake_home: Path) -> None: class TestHooksMatcher: def test_commands_differ_by_platform(self, fake_home: Path) -> None: - """ClaudeCode uses curl/$CLAUDE_TOOL_USE_INPUT; CodeBuddyIDE uses helper script.""" + """ClaudeCode uses curl reading stdin (-d @-); CodeBuddyIDE uses helper script.""" claude = hooks_matcher("ClaudeCode") codebuddy = hooks_matcher("CodeBuddyIDE") assert claude["hooks"][0]["command"] != codebuddy["hooks"][0]["command"] - assert "$CLAUDE_TOOL_USE_INPUT" in claude["hooks"][0]["command"] + assert "-d @-" in claude["hooks"][0]["command"] + assert "$CLAUDE_TOOL_USE_INPUT" not in claude["hooks"][0]["command"] assert "backend.hooks_forward" in codebuddy["hooks"][0]["command"] def test_codebuddy_matcher_all(self, fake_home: Path) -> None: @@ -515,7 +534,7 @@ def test_remove_hooks_keeps_others(self, fake_home: Path) -> None: data = _read_json(s) cmds = [m["hooks"][0]["command"] for m in data["hooks"]["PreToolUse"]] # our exact MyKnowledge command gone, user's own hooks preserved - assert "$CLAUDE_TOOL_USE_INPUT" not in "\n".join(cmds) + assert _hooks_command_claude() not in cmds assert user_bash_cmd in cmds assert "fmt" in cmds