rtk init -g --uninstall removes foreign PreToolUse hooks sharing a matcher group #3036

Summary

rtk init -g --uninstall removes the entire PreToolUse matcher group that rtk's hook lives in, instead of removing only rtk's own hook entry. If a user's rtk hook shares a matcher group with any foreign hook, that foreign hook is silently deleted too.

This is a data-loss bug for anything security-relevant: in my case the shared group held a PreToolUse: Bash guard that blocks commands from dumping secrets to stdout. Uninstalling rtk would silently delete it.

Version: 0.43.0 (Homebrew bottle, matches current master).

Repro

~/.claude/settings.json with rtk's hook in the same matcher group as a foreign hook:

{
  "hooks": {
    "PreToolUse": [
      {
        "matcher": "Bash",
        "hooks": [
          { "type": "command", "command": "node ~/.claude/hooks/my-security-guard.js" },
          { "type": "command", "command": "rtk hook claude" }
        ]
      }
    ]
  }
}

Run:

rtk init -g --uninstall

Expected: only rtk hook claude is removed; my-security-guard.js survives.
Actual: the whole "matcher": "Bash" entry is gone, including my-security-guard.js.

Root cause

src/hooks/init.rs → remove_hook_from_json (line 529), the retain at lines 544-556:

pre_tool_use_array.retain(|entry| {
    if let Some(hooks_array) = entry.get("hooks").and_then(|h| h.as_array()) {
        for hook in hooks_array {
            if let Some(command) = hook.get("command").and_then(|c| c.as_str()) {
                // Match both legacy script path and new binary command
                if command.contains(REWRITE_HOOK_FILE) || command == CLAUDE_HOOK_COMMAND {
                    return false;
                }
            }
        }
    }
    true
});

retain iterates over entries (matcher groups), but the predicate returns false as soon as any hook inside the entry matches rtk. So one matching hook discards the entry and every sibling hook in it.

This looks unintended, not by design

Two things in the codebase point the same way:

1. remove_legacy_hook_entries_from_json (line 1322) already gets this right — it only drops an entry when all of its hooks are rtk hooks:

   let dominated_by_legacy = entry
       .get("hooks")
       .and_then(|h| h.as_array())
       .map(|hooks| {
           hooks.iter().all(|hook| { /* ... contains(REWRITE_HOOK_FILE) ... */ })
       })
       .unwrap_or(false);

   So remove_hook_from_json is inconsistent with its sibling.

2. The existing tests assert that foreign hooks must survive — test_remove_hook_from_json (line 5496) ends with:

   // Should have only one hook left
   assert_eq!(pre_tool_use.len(), 1);
   // Check it's the other hook
   assert_eq!(command, "/some/other/hook.sh");

   The intent is clearly "don't touch foreign hooks". The bug survives only because both existing tests (5496, 5531) put rtk in its own entry, where dropping the entry happens to be equivalent to dropping the hook. The mixed-entry case is untested.

Note this is easy to hit in practice: install_hook_into_json appends rtk as a new "matcher": "Bash" group, so a user who already has a Bash group ends up with two. Consolidating them by hand — which looks like harmless tidying — is what arms the bug.

Proposed fix

Strip rtk's own hooks first, then drop only the entries that were emptied:

    let mut removed_any = false;

    // Remove rtk's own hooks, so foreign hooks sharing an entry survive.
    for entry in pre_tool_use_array.iter_mut() {
        if let Some(hooks_array) = entry.get_mut("hooks").and_then(|h| h.as_array_mut()) {
            let before = hooks_array.len();
            hooks_array.retain(|hook| {
                !hook
                    .get("command")
                    .and_then(|c| c.as_str())
                    .is_some_and(|cmd| {
                        cmd.contains(REWRITE_HOOK_FILE) || cmd == CLAUDE_HOOK_COMMAND
                    })
            });
            if hooks_array.len() < before {
                removed_any = true;
            }
        }
    }

    // Drop only entries that rtk emptied out.
    pre_tool_use_array.retain(|entry| {
        entry
            .get("hooks")
            .and_then(|h| h.as_array())
            .map_or(true, |hooks| !hooks.is_empty())
    });

    removed_any

Returning removed_any rather than pre_tool_use_array.len() < original_len matters: when rtk's hook shared an entry, the array length is unchanged even though a hook was removed, and the old return value would report false.

The existing tests at 5496/5531 still pass — rtk's solo entry ends up empty and is dropped, so len() goes 2 → 1 as asserted.

Suggested regression test

#[test]
fn test_remove_hook_from_json_preserves_foreign_hook_in_shared_entry() {
    let mut json_content = serde_json::json!({
        "hooks": {
            "PreToolUse": [
                {
                    "matcher": "Bash",
                    "hooks": [
                        { "type": "command", "command": "/some/other/hook.sh" },
                        { "type": "command", "command": CLAUDE_HOOK_COMMAND }
                    ]
                }
            ]
        }
    });

    let removed = remove_hook_from_json(&mut json_content);
    assert!(removed);

    let pre_tool_use = json_content["hooks"]["PreToolUse"].as_array().unwrap();
    assert_eq!(pre_tool_use.len(), 1, "entry must survive: it still holds a foreign hook");

    let hooks = pre_tool_use[0]["hooks"].as_array().unwrap();
    assert_eq!(hooks.len(), 1, "only rtk's hook should be removed");
    assert_eq!(hooks[0]["command"].as_str().unwrap(), "/some/other/hook.sh");
}

Notes

The proposed patch is untested locally — I don't have a Rust toolchain on this machine, so I'm filing this as an issue with a diff rather than a drive-by PR. Happy to open one if you'd prefer.

Same shape may be worth a look in the Codex/Cursor uninstall paths (lines ~3199, ~3281), though those entries appear to carry command directly rather than a nested hooks array, so the bug likely doesn't apply there.