fix(disk-hygiene): launch wired hooks through bash Python resolver (#1504)

[kyle-sexton]

Fixes #1504

Summary

Both wired disk-hygiene hooks now register as bash invoking hooks/run-python-hook.sh, which resolves a real Python 3 interpreter (rejecting the zero-length WindowsApps python3 alias stub) before exec'ing the target script. When no interpreter resolves, the guard still fails open (exit 0) and the Stop detector emits a systemMessage on stdout.

Related

• fix(disk-hygiene): surface silent destructive-guard launch failures #1465 — disclosed the blind spot this change closes
• disk-hygiene: the destructive-operation guard has never launched — 73 recorded runs, all hook_non_blocking_error #1416 — guard-launch monitor that benefits from independent launcher

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[claude]

Claude finished @kyle-sexton's task in 1m 38s —— View job

---

Security review complete ✅

• Gather context (PR diff, changed files)
• Skip-gate check — PR is open, change has a security-relevant surface (hook launcher for the destructive-command guard), no prior review on this head
• Review hooks/run-python-hook.sh for command/argument injection, interpreter resolution safety
• Review hooks.json wiring changes
• Review test file for anything security relevant
• Post findings

No security issues found.

Scope reviewed (files changed in this PR only):

• hooks/run-python-hook.sh — new bash launcher that resolves a real Python 3 interpreter before exec'ing the target guard/detector script
• hooks/hooks.json — wiring change from command: "python3" to command: "bash" + launcher script arg
• hooks/run-python-hook.test.sh, README.md, CHANGELOG.md, plugin.json — no security-relevant content

Notes from the review (informational, not blocking):

• $SCRIPT and its arguments in run-python-hook.sh#L18-L19 come only from the static args array in hooks.json, not from tool call input or model-controlled data, so there's no injection surface here — quoting through to exec "$PYTHON" "$SCRIPT" "$@" at L81 is correct (no unquoted expansion, no eval).
• Interpreter resolution still trusts PATH (via command -v), same trust boundary as the prior direct python3 registration — this launcher only adds a same-trust-level check (rejecting the zero-length WindowsApps alias stub) on top of it, so it's a strict improvement, not a new risk.
• The fail-open behavior (exit 0 when no interpreter resolves, at L78) is pre-existing/documented guard behavior, not introduced by this change — this PR only makes the failure observable via the Stop detector's systemMessage, which is a net visibility improvement for the exact blind spot fix(disk-hygiene): surface silent destructive-guard launch failures #1465 disclosed.
• No GitHub Actions workflow files, permissions, secrets, or dependency pins are touched in this PR.

[github-actions]

Last security-reviewed head: be9a311370af191f731eedc0dcb934b6a90c8db6. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[claude]

Claude finished @cursor[bot]'s task in 1m 16s —— View job

---

Security Review — complete

• Gather context (PR diff, changed files)
• Run skip gate
• Review changed files for security-relevant surface
• Report findings

Skip gate: PR is open, and the change touches a security-relevant surface (hook execution wiring), so a full review was performed.

Scope reviewed: plugins/disk-hygiene/hooks/hooks.json, plugins/disk-hygiene/hooks/run-python-hook.sh (new launcher), its test, and the plugin manifest/docs updates.

No security issues found.

Notes from the review (informational, not findings):

• Both wired hooks (PreToolUse guard, Stop monitor) invoke the new launcher via exec-form argv arrays in hooks.json — no shell is involved in dispatch, so there's no injection surface from ${CLAUDE_PLUGIN_ROOT}/${CLAUDE_PLUGIN_DATA} expansion into the command line.
• run-python-hook.sh quotes "$SCRIPT"/"$@" correctly in the final exec "$PYTHON" "$SCRIPT" "$@", so no word-splitting/injection risk there.
• The fail-open behavior for destructive_guard.py (silent exit 0 when no interpreter resolves) is called out in the PR description as pre-existing behavior being preserved, not introduced — the new part is that the Stop monitor now surfaces a systemMessage so the blind spot is observable instead of silent. This is a net improvement to the detective control, not a new gap.
• The WindowsApps stub detection (_is_store_alias_stub) is a robustness heuristic (skip a known-broken zero-byte alias), not a trust boundary — its exact matching logic doesn't affect the security posture either way, since a real interpreter is still required to pass the -c 'import sys; sys.exit(0)' smoke test before use.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a96b8d4304

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @cursor[bot]'s task in 5m 28s —— View job

---

Claude's Code Review

• Gather context (PR diff, changed files, CLAUDE.md)
• Understand the request (code review via /review:code-review)
• Review hooks/run-python-hook.sh
• Review hooks/run-python-hook.test.sh
• Review hooks/hooks.json and plugin.json/README/CHANGELOG updates
• Post findings

Summary: This closes the #1465/#1504 blind spot cleanly — both hooks.json registrations now launch through run-python-hook.sh, which rejects the zero-length WindowsApps alias stub before exec'ing the real script, and the guard/monitor fail-open behavior is preserved (destructive_guard.py exits 0 silently; guard_launch_monitor.py emits a systemMessage). I confirmed guard_launch_monitor.py's detector still works: it matches on a substring of the full recorded command line (destructive_guard.py), which survives the switch from command: python3 to command: bash + args, so detection isn't broken by this refactor.

Two findings posted inline:

1. hooks/hooks.json:9,30 — The fix trades a python3-resolvability dependency for a bash-resolvability one. Since the guard's matcher is Bash|PowerShell, a PowerShell-only Windows session could hit this hook without bash.exe ever having been resolved by Claude Code, silently reproducing the exact blind spot this PR closes (no systemMessage is possible if the launcher itself can't start). Neither the README nor /disk-hygiene:setup check were updated to document/probe for this new prerequisite.
2. hooks/run-python-hook.test.sh:83-85 — Dead VERSION_OUT probe left over from an earlier draft; never asserted, and the following comment explains it doesn't do what it looks like.

Additional (cross-file, no changed line to anchor to): skills/clean/reference/safety-model.md lines 273-277 still describes the pre-fix state — "wired with the same literal python3 command as the guard it watches... Closing that requires a launcher whose availability does not depend on the same lookup, which is tracked separately (#1504)." This PR is that launcher, so this paragraph is now stale and should be updated alongside the README/CHANGELOG changes already in this PR, or a follow-up filed.

Note: I wasn't able to execute run-python-hook.test.sh or the Python test suite in this sandbox (Bash execution requires approval not available in this review lane), so the above is from static review only — worth confirming CI runs the new .test.sh file.

Branch: cursor/fix-1504-guard-launch-launcher-2ae1

[github-actions]

Claude has reviewed this PR 4 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[claude]

Claude finished @cursor[bot]'s task in 2m 39s —— View job

---

Claude's Code Review

• Gather context (PR diff, CLAUDE.md, prior review comments)
• Determine skip gate applicability at new head 5779f1f
• Review new commit 5779f1f (addresses prior Python-2/bash-prereq feedback)
• Re-check outstanding items from prior reviews (Codex P1, bash-resolvability note, dead test scaffolding)
• Post findings

Skip gate: not applicable — the head advanced to 5779f1f since the last code-review pass (which reviewed be9a311), and this commit has real review surface (interpreter version-gating logic).

Summary: Commit 5779f1f correctly resolves the two prior actionable findings: the Codex P1 (legacy python 2.x silently accepted) is fixed by gating on sys.version_info >= (3, 11) at both probe call sites, and the dead VERSION_OUT scaffolding in run-python-hook.test.sh is removed. The bash-prerequisite documentation gap is also addressed (README + new setup check step 1).

One finding posted inline on hooks/run-python-hook.sh:52:

1. Hardcoded Python floor reintroduces the exact drift this plugin already fixed once. run-python-hook.sh:52,60 (and run-python-hook.test.sh:82) hardcode the floor as a literal (3, 11) instead of deriving it from MIN_PYTHON in hygiene.py, which CHANGELOG.md [0.6.1] established as the single origin specifically because hand-maintained copies of this number drift silently on a future bump. Three new hand-maintained copies land in this PR. See inline comment for the existing shell-parse pattern (skills/setup/scripts/python3_alias_probe.test.sh) to mirror instead.

Non-blocking observation: neither the new floor-gating logic nor the test suite adds a regression case simulating a present-but-too-old interpreter (e.g. Python 2.x or 3.6 on PATH) — the exact scenario the Codex P1 finding identified. The current tests only cover "no interpreter" and "modern interpreter" happy/sad paths. Worth a follow-up test case given this is the precise regression class that motivated the fix.

[claude]

Claude finished @cursor[bot]'s task in 1m 23s —— View job

---

Claude's Code Review

• Gather context (PR diff, CLAUDE.md, prior review comments)
• Determine skip gate applicability at new head 7afe7c3
• Review new commit 7afe7c3 (derives Python floor from MIN_PYTHON)
• Re-check outstanding items from prior reviews
• Post findings

Skip gate: not applicable — the head advanced to 7afe7c3 since the last code-review pass (which reviewed 5779f1f), and this commit has real review surface (the MIN_PYTHON-derivation fix).

Summary: Commit 7afe7c3 largely resolves the prior P1 finding (hardcoded (3, 11) floor drifting from hygiene.py's MIN_PYTHON) by parsing MIN_PYTHON out of hygiene.py via sed and building a shared PYTHON_VERSION_PROBE string, with a safe (3, 11) fallback if parsing fails. run-python-hook.test.sh was updated the same way and the previously-flagged dead VERSION_OUT scaffolding is gone.

One finding posted inline on hooks/run-python-hook.sh:70:

1. The fix missed one of the three hardcoded-floor call sites. resolve_python3's py -3 Windows-launcher fallback (line 70) still checks sys.version_info >= (3, 11) as an inline literal instead of using the newly-derived $PYTHON_VERSION_PROBE (used at line 62 for the python3/python loop). This reopens the exact single-source-of-truth gap this commit set out to close, just on the path CI doesn't exercise (run-python-hook.test.sh has no py -3 coverage) — if MIN_PYTHON is bumped past 3.11 in hygiene.py, this branch would keep accepting an interpreter too old to run the guard/monitor.
   · Branch: cursor/fix-1504-guard-launch-launcher-2ae1

[claude]

Claude finished @cursor[bot]'s task in 1m 1s —— View job

---

Claude's Code Review

• Gather context (PR diff, CLAUDE.md, prior review comments)
• Determine skip gate applicability at new head 952f4ae
• Review new commit 952f4ae (uses derived floor in py -3 launcher path)
• Re-check outstanding items from prior reviews
• Post findings

Skip gate: not applicable — the head advanced to 952f4ae since the last code-review pass (which reviewed 7afe7c3), and this commit has real review surface (the last remaining hardcoded-floor call site).

Summary: Commit 952f4ae is a one-line fix that resolves the P1 flagged in the previous review round. hooks/run-python-hook.sh:70 (the py -3 Windows-launcher fallback) now checks "$PYTHON_VERSION_PROBE" instead of the inline literal sys.version_info >= (3, 11), matching the python3/python candidate loop at line 62. Both interpreter-resolution paths now derive their version floor from the single PYTHON_VERSION_PROBE built off MIN_PYTHON in hygiene.py (lines 20–26), closing the single-source-of-truth gap this PR's review thread has been tracking.

I confirmed the diff between the last-reviewed head (7afe7c3) and this head (952f4ae) touches only that one line — no other regressions introduced.

No new findings. All previously raised items across this review's rounds have now been addressed:

• ✅ Codex P1 (legacy Python 2.x silently accepted) — fixed via sys.version_info gating (commit 5779f1f)
• ✅ Dead VERSION_OUT test scaffolding — removed (commit 5779f1f)
• ✅ Bash-resolvability prerequisite undocumented — README + setup check updated (commit 5779f1f)
• ✅ Hardcoded (3, 11) floor at the two python3/python call sites — derived from MIN_PYTHON (commit 7afe7c3)
• ✅ Hardcoded (3, 11) floor at the py -3 fallback call site (the one path the previous commit missed) — now derived from the same PYTHON_VERSION_PROBE (commit 952f4ae, this round)

Note: I was unable to execute run-python-hook.test.sh in this sandbox (Bash execution requires approval not available in this review lane) — worth confirming CI runs it green at this head, particularly the happy-path and monitor/guard-fail-open cases.

Branch: cursor/fix-1504-guard-launch-launcher-2ae1