fix(disk-hygiene): detect WindowsApps python3 alias stub fail-open in setup check#1124
fix(disk-hygiene): detect WindowsApps python3 alias stub fail-open in setup check#1124kyle-sexton wants to merge 2 commits into
Conversation
… setup check The clean skill's destructive-action guard hook launches the literal command `python3`. On stock Windows that name resolves to a zero-length `WindowsApps\python3.exe` App Execution Alias stub that opens the Microsoft Store instead of running an interpreter, so the guard process never starts. A PreToolUse hook blocks a tool call only by emitting exit code 2 or a `deny` decision; a guard that never runs emits neither, so the destructive Bash/PowerShell command proceeds ungated — a silent fail-open in a safety guard. setup check now runs a bundled inspect-only probe (python3_alias_probe.py) that classifies the python3 resolution — zero length under a WindowsApps path component is the stub — without executing it. The skill fails closed on every verdict except `ok`: `store-alias-stub` and `indeterminate` (identity unreadable) both FAIL, `not-found` folds into the floor's absent-interpreter FAIL. Adds cross-platform unit tests (including the stat-failure indeterminate path), a README requirements gotcha, and bumps the plugin to 0.6.5 with a CHANGELOG entry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Claude encountered an error after 1s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 1s —— View job I'll analyze this and get back to you. |
|
Warning Automated security review did not complete — this is an infrastructure failure, not a review verdict. Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."
Re-running the job, or pushing a new commit, will retry the review. |
|
🔒 babysit-prs lane claiming this PR for on-branch fix work this cycle (amendment-round: 16, safe tier). Will fix clear branch-owned findings and push; will not resolve threads or merge (safe tier). |
…boundary, doc polish (F7/F9/F10) (#1148) ## Summary dh **0.8.1** — the consumer-audit declarative/doc batch (findings F7, F9, F10). Docs-only; no engine or guard code changes. - **F7 — `--execute` contract for the manual lane (decision recorded, not silently resolved).** The flag now explicitly gates every deletion lane: the gated engine lane where supported, the manual handoff elsewhere. This is named in the doc as a *deliberate semantic unification* — the flag previously read as "offer the gated ENGINE lane" (which can never apply on Windows/macOS), leaving the manual lane's gate ambiguous; the audit observed consumer sessions reading it both ways, including one that proceeded to manual deletion without `--execute`. The issue's adversarial-review comment asked that the semantic shift be named rather than passed off as clarification — done at the argument definition and in the manual-handoff precondition (lane symmetry, per the issue's recommendation). - **F9 — hint-prose trust boundary.** Safety-model trust boundaries now name standing-policy `additional_hints[].reason` prose as untrusted claims requiring independent evidence: additive-only design means hints can't authorize anything, but the prose reached triage reasoning unlabeled. - **F10(a).** Setup SKILL.md + README corrected: `preview` *reports `execution-platform-unsupported` as a per-candidate blocker*, not a top-level status — verified against `preview()`'s payload shape (blockers live in `candidates[].blockers`; top-level status is `blocked`/`ready-for-explicit-approval`). - **F10(b).** README states once that the Recycle-Bin / Trash naming is a model-layer distinction only; the engine treats Windows and macOS identically (execution unsupported). - **F10(c) — DECLINED with evidence.** The hub-restructure suggestion rested on an MD013 line-length complaint; the repo's `.markdownlint-cli2.jsonc` sets `"MD013": false` ("No hard line-length limit"), so the complaint came from an out-of-repo lint run, and the skill-quality gate passes the hub at its current length. **Scheduling note:** #1113 was queued behind draft PR #1124 (file overlap on setup SKILL.md / README / CHANGELOG). That draft has been stale for ~4h with CI green; the actual hunk overlap is disjoint (different sections), so this batch proceeds — whichever merges second rebases trivially (CHANGELOG entry ordering + version bump). ## Test plan - [x] `check-changelog-parity.sh --check-bump origin/main` → pass (0.8.1 + entry) - [x] `check-changed-skills.sh` → 2 skills checked, 0 failed - [x] Docs-only: no engine tests affected (suite untouched) - [ ] CI green ## Related Closes #1113. Source: handoff-inbox item `20260723-021058-disk-hygiene-0-6-4-consumer-audit`, findings F7/F9/F10. F7 decision context: the adversarial-review comment on #1113. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Claude finished @kyle-sexton's task in 2m 58s —— View job PR Review
SummaryThis is a correct, well-structured fix for a real fail-open gap. The detection logic is sound, the fail-closed semantics are consistently applied, and the test suite is thorough. Three minor findings below — none blocking.
|
Summary
Closes a fail-open vector in the
disk-hygienedestructive-action guard on Windows. Thecleanskill registers its PreToolUse guard as the literal hook commandpython3(skills/clean/SKILL.md). On stock Windows,python3resolves to%LOCALAPPDATA%\Microsoft\WindowsApps\python3.exe— a zero-length App Execution Alias reparse stub that opens the Microsoft Store (or exits) instead of running an interpreter. The guard process never starts, so it emits neither of the two signals that let a PreToolUse hook block a tool call — exit code 2 or adenydecision — and Claude Code lets the destructiveBash/PowerShellcommand proceed ungated. This is the same fail-open shape as the 0.6.3 fix (a failed hook launch treated as non-blocking), reached through a new vector: the guard's launch name resolving to the Store stub rather than a real interpreter.This is a
DEFER-FORBIDDENfail-open gap in a safety guard — detection, tests, and docs are all part of the fix, not follow-ups.Fix
skills/setup/scripts/python3_alias_probe.py: classifies what the namepython3resolves to (viashutil.which,--pathoverride for tests) without executing it (running the stub pops the Store / hangs). Verdicts:store-alias-stub(zero-length file under aWindowsAppspath component),ok(real interpreter — including the Store's genuine Python under a versionedWindowsApps\PythonSoftwareFoundation...\subdir, which has non-zero size),not-found,indeterminate(identity unreadable). On Windows it also reports theIO_REPARSE_TAG_APPEXECLINKreparse attribute as corroborating evidence, but the cross-platform verdict uses the portable zero-length +WindowsApps-component signal.setup check(SKILL.md step 1) now runs the probe after locating the working interpreter and fails closed on every verdict exceptok:store-alias-stubandindeterminateboth FAIL with remediation (disable thepython3App execution alias, or install real Python ahead of WindowsApps onPATH);not-foundfolds into the floor's absent-interpreter FAIL. A barecommand -v python3success is explicitly called out as insufficient — it matches the stub too.0.6.5→0.6.6inplugin.json+ matchingCHANGELOG.mdentry. (Mergedorigin/mainin after PR fix(disk-hygiene): require immediate re-enumeration before container-wide manual deletions #1118 landed the fix(disk-hygiene): manual-handoff lane — require immediate re-enumeration before container-wide deletions (F2) #1108 fix as 0.6.5; re-based the bump to 0.6.6 and reordered the CHANGELOG entry above 0.6.5. Merge, not rebase, per branch policy.)Verification
test_python3_alias_probe.py(10 cases, run viapython3_alias_probe.test.sh) — all pass. They fabricate a zero-length.../WindowsApps/python3.exein a tempdir so detection is exercised cross-platform without a real Windows box: stub detected; case-insensitiveWindowsAppsmatch; real interpreter under a versioned WindowsApps subdir →ok(no false positive); zero-length outside WindowsApps →ok;not-found; theindeterminatestat-failure path (fail-closed); and an assertion that the probe never invokessubprocesson the candidate.kill_switch_probeunaffected, clean enginetest_hygiene.py112 passed (4 skipped).markdownlint-cli2,shellcheck, andtyposclean on all changed files; new scripts tracked100755(exec-bit lane parity with the existingkill_switch_probe.*).%LOCALAPPDATA%\Microsoft\WindowsApps— all 35 App-Execution-Alias.exestubs areLength 0with the ReparsePoint attribute set, confirming the zero-length detection signal against ground truth (not asserted from memory).PreToolUseblocks the tool call"; "Any other exit code is a non-blocking error … Execution continues"; PreToolUse can alternatively block via exit-0 JSONpermissionDecision: deny. A guard that never launches emits neither exit 2 nor adeny, so the non-blocking path is taken — the doc-grounded basis for the fail-open, stated as that logical consequence rather than as a documented launch-failure rule.Related
20260723-021058-disk-hygiene-0-6-4-consumer-audit, finding F3.python3(skills/clean/SKILL.md), which changing the hook's launch command would address — that is guard-registration territory (operator-gated, fix(disk-hygiene): implement guard registration decision — kill-switch delivery + data-root authority (F1/F5) #1107), so this PR is the assigned mitigation (detect + document + fail closed insetup check), not a change to the guard's registration.python3_alias_probe.test.sh'scommand -v python3fallback could itself resolve to a Store stub on a stub-only machine (kept at parity with the siblingkill_switch_probe.test.shwrapper rather than diverging); a symlink whose own path is outside WindowsApps but whose target is inside would evade the path-component check (AppExecLink aliases are reparse points, not symlinks, so not the real-world shape).Closes #1110