Browse Source

fix(install): harden hook wiring per adversarial review (2 findings)

GLM refuter (lane v5-refute-g5) refuted the first cut:
1. The jq merge used walk/1 (jq >=1.6) behind a command -v existence
   probe - a jq 1.5 host passes the probe, crashes mid-install under
   set -e, and exits with hooks copied but settings.json UNWIRED (the
   exact regression the feature fixes) plus everything after the
   block skipped. Now a capability probe: echo '{}' | jq -e 'walk(.)'.
2. Idempotency dedup matched the resolved script path, so a hook
   already wired in plugin form (${CLAUDE_PLUGIN_ROOT}/hooks/...)
   was re-added by a script install - advisory hooks fired twice per
   tool call in mixed-method setups. Dedup now keys on the script
   name under hooks/ (separator-tolerant: Join-Path yields backslash
   forms on Windows), both installers.
Replayed: fresh=7 wired, re-run=7, plugin-form pre-wired stays 1.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
0xDarkMatter 6 days ago
parent
commit
ebce5714da
2 changed files with 19 additions and 8 deletions
  1. 7 2
      scripts/install.ps1
  2. 12 6
      scripts/install.sh

+ 7 - 2
scripts/install.ps1

@@ -211,11 +211,16 @@ foreach ($eventProperty in $desired.hooks.PSObject.Properties) {
         $missingHooks = @()
         $missingHooks = @()
         foreach ($hook in @($group.hooks)) {
         foreach ($hook in @($group.hooks)) {
             $command = $hook.command.Replace('${CLAUDE_PLUGIN_ROOT}/hooks', (Join-Path $claudeDir "hooks"))
             $command = $hook.command.Replace('${CLAUDE_PLUGIN_ROOT}/hooks', (Join-Path $claudeDir "hooks"))
-            $hookPath = $command -replace '^bash "', '' -replace '"$', ''
+            # Dedup on the script NAME under hooks/, not the resolved path: a
+            # hook wired by a plugin install carries the
+            # ${CLAUDE_PLUGIN_ROOT}/hooks/ form and must still count as
+            # already-wired (mixed-method double-fire). Separator-tolerant:
+            # Join-Path yields \hooks while plugin form uses /hooks.
+            $hookName = ($command -replace '^.*hooks[/\\]', '') -replace '"$', ''
             $alreadyWired = $false
             $alreadyWired = $false
             foreach ($existingGroup in $eventGroups) {
             foreach ($existingGroup in $eventGroups) {
                 foreach ($existingHook in @($existingGroup.hooks)) {
                 foreach ($existingHook in @($existingGroup.hooks)) {
-                    if ($existingHook.command -and $existingHook.command.Contains($hookPath)) {
+                    if ($existingHook.command -and ($existingHook.command.Contains("hooks/$hookName") -or $existingHook.command.Contains("hooks\$hookName"))) {
                         $alreadyWired = $true
                         $alreadyWired = $true
                     }
                     }
                 }
                 }

+ 12 - 6
scripts/install.sh

@@ -228,13 +228,19 @@ for file in "$PROJECT_ROOT/hooks"/*.sh; do
     make_executable "$CLAUDE_DIR/hooks/$(basename "$file")"
     make_executable "$CLAUDE_DIR/hooks/$(basename "$file")"
 done
 done
 
 
-if command -v jq >/dev/null 2>&1; then
+# Capability probe, not existence probe: walk/1 needs jq >= 1.6, and a 1.5 jq
+# passes `command -v` then crashes mid-install under set -e, leaving hooks
+# copied but settings.json unwired (adversarial-review finding, 2026-07).
+if echo '{}' | jq -e 'walk(.) | true' >/dev/null 2>&1; then
     settings_path="$CLAUDE_DIR/settings.json"
     settings_path="$CLAUDE_DIR/settings.json"
     [ -f "$settings_path" ] || printf '{}\n' > "$settings_path"
     [ -f "$settings_path" ] || printf '{}\n' > "$settings_path"
     tmp_settings="$(mktemp)"
     tmp_settings="$(mktemp)"
     jq --arg hook_dir "$CLAUDE_DIR/hooks" --slurpfile desired "$PROJECT_ROOT/hooks/hooks.json" '
     jq --arg hook_dir "$CLAUDE_DIR/hooks" --slurpfile desired "$PROJECT_ROOT/hooks/hooks.json" '
-      def installed_path:
-        .command | sub("^bash \\\""; "") | sub("\\\"$"; "");
+      # Dedup on the script NAME under hooks/, not the resolved path: a hook
+      # wired by a plugin install carries the ${CLAUDE_PLUGIN_ROOT}/hooks/
+      # form and must still count as already-wired (mixed-method double-fire).
+      def script_name:
+        .command | sub("^.*hooks[/\\\\]"; "") | sub("\\\"$"; "");
       ($desired[0]
       ($desired[0]
         | walk(if type == "string" then gsub("\\$\\{CLAUDE_PLUGIN_ROOT\\}/hooks"; $hook_dir) else . end)
         | walk(if type == "string" then gsub("\\$\\{CLAUDE_PLUGIN_ROOT\\}/hooks"; $hook_dir) else . end)
       ) as $wanted
       ) as $wanted
@@ -245,8 +251,8 @@ if command -v jq >/dev/null 2>&1; then
             |
             |
             ($group.hooks | map(
             ($group.hooks | map(
               . as $hook
               . as $hook
-              | ($hook | installed_path) as $path
-              | select(any($existing[]; contains($path)) | not)
+              | ($hook | script_name) as $name
+              | select(any($existing[]; contains("hooks/" + $name) or contains("hooks\\" + $name)) | not)
             )) as $missing
             )) as $missing
             | if ($missing | length) > 0 then
             | if ($missing | length) > 0 then
                 .hooks[$event.key] = ((.hooks[$event.key] // []) + [($group | .hooks = $missing)])
                 .hooks[$event.key] = ((.hooks[$event.key] // []) + [($group | .hooks = $missing)])
@@ -257,7 +263,7 @@ if command -v jq >/dev/null 2>&1; then
     mv "$tmp_settings" "$settings_path"
     mv "$tmp_settings" "$settings_path"
     echo -e "  ${GREEN}Security and peer-guard hooks wired in settings.json${NC}"
     echo -e "  ${GREEN}Security and peer-guard hooks wired in settings.json${NC}"
 else
 else
-    echo -e "  ${YELLOW}jq not found, skipping hook wiring (plugin installs unaffected)${NC}"
+    echo -e "  ${YELLOW}jq with walk/1 (>=1.6) not available, skipping hook wiring (plugin installs unaffected)${NC}"
 fi
 fi
 echo ""
 echo ""