fix(hooks): remove quoted/launcher-wrapped hook entries on clear & dedup (WEB-4737)

[MohamedAklamaash]

What & why

setup --clear / nuke and the install-side dedup identified Unbound hook entries by exact string equality against an unquoted path (cmd == str(script_path)). Hook commands that wrap the path in quotes or a launcher — the forms MDM/managed installs write on Unix ("/Users/x/.claude/hooks/unbound.py") and the Windows installer writes (py -3 "C:\...\unbound.py") — never matched.

Consequences (reproduced 2026-06-09, see WEB-4737):

• macOS/Linux clear/nuke left the hooks block intact → broken half-state: entries point at a now-deleted script, the settings diagnostic PASSes while Claude Code logs a hook error on every event.
• The same flaw in install dedup appended a duplicate entry instead of recognizing the existing one → hooks fire twice.

Fix

Single shared _command_targets_hook(command, target) helper: tokenizes the command with shlex (dropping surrounding quotes and any py -3 / python launcher prefix), normalizes each token, and matches against the resolved hook target — with a normalized-substring fallback for unparseable commands. Used in both the install dedup and the removal path so they can never disagree.

Applied to every selective matcher:

• claude-code/hooks/setup.py — install dedup + remove_hooks_from_settings
• claude-code/hooks/mdm/setup.py — remove_user_level_hooks_for_user
• codex/hooks/setup.py — install dedup + remove_hooks_from_config
• codex/hooks/mdm/setup.py — user-level strip + install dedup
• binary/src/unbound_hook/setup_cmd.py — codex merge dedup (target = HOOK_BINARY)

copilot/cursor are unaffected — they own their config files and clear them wholesale, so there is no selective matcher to fix.

Per the vendored-by-file-path constraint in the binary bundle, the helper is duplicated byte-identically per tree (not cross-imported) and pinned by a parity test.

Tests

claude-code/hooks/test_setup.py and binary/tests/test_hook_command_matcher.py:

• matcher: bare / double-quoted / single-quoted / py -3 & python launcher / paths with spaces; foreign command and empty string do not match
• remove_hooks_from_settings: removes quoted + bare + launcher forms, preserves foreign hooks, prunes emptied events
• mixed quoted + bare in one item → both removed
• install dedup against a pre-existing quoted entry → no duplicate appended
• binary codex merge dedup against a quoted entry → no duplicate
• cross-tree matcher byte-identical parity

Local run: claude-code/hooks 30 passed (+3 subtests); binary/tests matcher 2 passed; migration suite 17 passed.

Acceptance (WEB-4737)

• Seeded quoted + py -3 entries → --clear removes them all (nuke shares this path)
• Install dedup prevents duplicates for both quoted and unquoted forms
• Unit tests covering quoted, unquoted, and launcher-prefixed command forms

Notes / out of scope

• The two minor residue items in the ticket are not in this repo's clear path: the ~/.unbound/identity.json non-removal is a documented parity note in unbound-hook clear, and the --clear mtime-churn claim does not originate in remove_hooks_from_settings (it only writes when modified). Suggest tracking separately.

Closes WEB-4737

🤖 Generated with Claude Code

---

Note

Low Risk
Localized hook-config matching logic with broad unit tests and no auth or data-path changes; wrong matches would only affect which JSON hook entries are removed or deduplicated.

Overview
Hook install dedup and clear/nuke paths used exact string equality against an unquoted script path, so MDM-style quoted commands ("/path/unbound.py") and Windows launcher forms (py -3 "...") were never recognized—leaving stale hook JSON after clear or appending duplicates on reinstall.

This PR adds _command_targets_hook, duplicated identically in five hook setup modules (Claude/Codex user + MDM + binary Codex merge). It tokenizes with shlex, strips optional python/py launchers and flags, normalizes paths, and compares only the invoked program to the target hook path. That logic now drives both dedup (configure_*, _merge_codex_hooks_json, MDM user-level strip) and removal (remove_hooks_from_settings, remove_hooks_from_config, remove_user_level_hooks_for_user).

Tests cover matcher edge cases, clear preserving foreign hooks, install dedup against quoted entries, binary Codex merge dedup, and a byte-identical parity check across all copies.

^{Reviewed by Cursor Bugbot for commit c60c743. Bugbot is set up for automated code reviews on this repo. Configure here.}

Greptile Summary

This PR fixes hook entry matching in --clear/nuke and install dedup paths that previously relied on exact string equality, breaking for MDM-managed installs (which quote the path) and Windows launcher forms (py -3 \"...\"). A shared _command_targets_hook(command, target) helper is introduced that uses shlex.split, strips launcher prefixes, and normalizes paths before comparing.

• The helper is duplicated byte-identically across all five affected trees (claude-code/hooks, claude-code/hooks/mdm, codex/hooks, codex/hooks/mdm, binary/src) due to the vendored-binary constraint, and a new parity test (TestMatcherParityAcrossTrees) enforces this invariant.
• The substring fallback that existed in the prior review iteration has been removed; unparseable commands now return False and parseable commands use exact first-token path equality only, correctly preventing false-positive matches.

Confidence Score: 5/5

Safe to merge. The change narrows the existing removal/dedup logic to a well-tested, exact-match helper; it cannot accidentally delete foreign hooks or miss installs in any of the covered command forms.

Every changed call site replaces a demonstrably broken comparator with the new helper. The helper tokenizes, strips launcher prefixes, and does normcase+normpath comparison — covering all documented input forms. The removal of the substring fallback eliminates the false-positive path flagged in the previous review. Unit tests cover bare, double-quoted, single-quoted, launcher-prefixed, and path-with-spaces forms as well as false-positive cases. The byte-identical parity test prevents future drift across the five duplicated copies.

No files require special attention. All five modified setup files apply the helper consistently, and both test files exercise the critical matcher and dedup paths.

Important Files Changed

Filename	Overview
claude-code/hooks/setup.py	Adds _command_targets_hook helper and replaces exact-string/Windows-substring matching in both configure_claude_settings (dedup) and remove_hooks_from_settings (clear) with the new helper.
claude-code/hooks/mdm/setup.py	Byte-identical helper copy added; remove_user_level_hooks_for_user _is_unbound closure now delegates to _command_targets_hook.
codex/hooks/setup.py	Parallel change to the Codex variant: helper added, configure_codex_hooks dedup and remove_hooks_from_config both updated consistently.
codex/hooks/mdm/setup.py	Codex MDM variant updated; both remove_user_level_hooks_for_user and configure_codex_hooks_for_user dedup now use _command_targets_hook.
binary/src/unbound_hook/setup_cmd.py	Binary tree copy of helper added; _merge_codex_hooks_json dedup updated to use _command_targets_hook with Path(hook_command) as target.
claude-code/hooks/test_setup.py	New test classes cover matcher edge cases, remove_hooks_from_settings end-to-end, install dedup against quoted entries, and byte-identical parity check across all five trees.
binary/tests/test_hook_command_matcher.py	New binary-side test validates matcher against HOOK_BINARY and verifies no duplicate written when existing entry is quoted.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["_command_targets_hook(command, target)"] --> B{command empty?}
    B -->|Yes| C[return False]
    B -->|No| D["shlex.split with posix flag"]
    D -->|ValueError| E[return False]
    D -->|OK| F["strip quotes from each token, filter empty"]
    F --> G{any tokens?}
    G -->|No| H[return False]
    G -->|Yes| I{"tokens[0] is py/python launcher?"}
    I -->|Yes| J["drop launcher, drop leading flag tokens"]
    J --> K{any tokens left?}
    K -->|No| L[return False]
    K -->|Yes| M["normcase + normpath compare tokens[0] vs target"]
    I -->|No| M
    M --> N[return True or False]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["_command_targets_hook(command, target)"] --> B{command empty?}
    B -->|Yes| C[return False]
    B -->|No| D["shlex.split with posix flag"]
    D -->|ValueError| E[return False]
    D -->|OK| F["strip quotes from each token, filter empty"]
    F --> G{any tokens?}
    G -->|No| H[return False]
    G -->|Yes| I{"tokens[0] is py/python launcher?"}
    I -->|Yes| J["drop launcher, drop leading flag tokens"]
    J --> K{any tokens left?}
    K -->|No| L[return False]
    K -->|Yes| M["normcase + normpath compare tokens[0] vs target"]
    I -->|No| M
    M --> N[return True or False]

Loading

_{Reviews (3): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile}

[vigneshsubbiah16]

🛡️ Automated Security Review (consensus)

1 finding — 1 high-confidence, 0 to triage. Reviewers: Lead, Cursor, Claude, Semgrep, Gitleaks.

Over-broad _command_targets_hook matching can remove foreign hooks or skip Unbound install

Severity: Medium · Confidence: 🔴 HIGH
Location: claude-code/hooks/setup.py:382-387 (byte-identical copies in codex/hooks/setup.py:397-398, claude-code/hooks/mdm/setup.py:656-660, codex/hooks/mdm/setup.py:668-673, binary/src/unbound_hook/setup_cmd.py:315-316)

Impact: Matcher returns true if the normalized hook path equals any parsed token (not just the invoked executable) or appears as a substring of the full command — so --clear/MDM strip can delete unrelated third-party hooks (e.g. /opt/other/hook.py --config "/…/unbound.py" or a sibling …/unbound.py.backup), and install dedup can treat a coincidental match as “already installed,” silently leaving Unbound unhooked (integrity/availability; not attacker-controlled RCE).

Fix: Drop the normalized_target in os.path.normcase(command) substring fallback; after stripping a known py/-3/python launcher prefix, compare only the first remaining executable token to the normalized target (still covers bare/quoted/launcher forms).

Flagged by: Lead, Cursor (2 inline threads), Claude

---

Clean scans: Gitleaks — no secrets. Semgrep — no new issues in changed logic (remaining hits are pre-existing pickle/permission/urllib patterns outside this diff).

---

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head 9b6bce5b · 2026-06-23T10:14Z

[MohamedAklamaash]

Addressed in 631cb28 (consensus finding — over-broad _command_targets_hook).

The matcher now compares only the invoked program token:

1. shlex.split the command (unparseable → return False, no substring scan).
2. Strip a leading py/python/python2/python3 launcher and its option flags.
3. Exact-match the normalized first remaining token against the target.

The normalized_target in os.path.normcase(command) substring fallback is removed. Consequences:

• Foreign hooks passing our path as an argument (/opt/other/hook.py --config "/…/unbound.py") no longer match.
• Sibling/suffix paths (unbound.py.backup, mirrored-home /opt/mirror/…/unbound.py) no longer match.
• Bare / quoted / single-quoted / launcher-wrapped forms are still covered.

Regression tests added in both suites: test_target_as_argument_does_not_match, test_sibling_path_does_not_match (claude-code), plus arg/sibling negatives in the binary matcher test. Full run: 51 passed (+3 subtests). Replied inline to the Cursor and Greptile threads too.

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 631cb28. Configure here.}

[vigneshsubbiah16]

🛡️ Automated Security Review (consensus)

0 findings — 0 high-confidence, 0 to triage. Reviewers: Lead, Cursor, Claude, Semgrep, Gitleaks.

✅ Security consensus: no issues found.

Previously acknowledged (not re-flagged)

• Over-broad _command_targets_hook matching (substring fallback, any-token match, sibling/argument false-positives) — fixed in 631cb28; matcher now exact-matches only the invoked program token after launcher stripping; regression tests added.
• Missing matcher debug_print logging — intentionally omitted to preserve byte-identical parity across five vendored trees; removal call sites already log per stripped hook.

---

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head 631cb288 · 2026-06-23T10:26Z

[vigneshsubbiah16]

🛡️ Automated Security Review (consensus)

0 findings — 0 high-confidence, 0 to triage. Reviewers: Lead, Cursor, Claude, Semgrep, Gitleaks.

✅ Security consensus: no issues found. (reviewers: Lead, Cursor, Claude, Semgrep, Gitleaks)

Previously acknowledged (not re-flagged)

• Over-broad _command_targets_hook matching (substring fallback, path-as-argument, sibling/suffix paths) — Fixed in 631cb28; matcher now exact-compares only the invoked program token after launcher/flag stripping; regression tests added.
• Missing debug_print in _command_targets_hook — Intentionally omitted to preserve byte-identical parity across five vendored copies; removal call sites already log each stripped hook.

---

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head c60c7433 · 2026-06-23T11:41Z