fix(audit): stop misclassifying manual installs as brew and surface removal failures - #133
Merged
Merged
Conversation
…emoval failures `make reconcile-all` confirmed removals then reported "Conflicts resolved: 0" with no explanation. Three defects: - Path heuristic labeled any /usr/local/bin or /opt/homebrew binary as 'brew' even when brew is not installed, so removal ran a nonexistent `brew uninstall` and always failed. Classify as 'manual' when brew is absent (the shell-side capability.sh already gated on `command -v brew`). - Manual removal of root-owned binaries failed with a bare "Permission denied"; now surfaces actionable guidance (`sudo rm <path>`), matching the apt/dnf branch. - The `--reconcile --all --apply` non-JSON path printed only the summary; per-tool failures, declines, and protected skips were vlog-only. Print them to stderr unconditionally so a confirmed-but-failed removal is never silent. Claude-Session: https://claude.ai/code/session_01MH3EaniXCnJdwqNvrMB4Ym Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
|
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
CybotTM
added a commit
that referenced
this pull request
Jul 30, 2026
…alls (#134) ## Problem Follow-up to #133. With failures now visible, a full `make reconcile-all` run surfaced three remaining issues: 1. Dozens of individual sudo hints — the user has to collect `sudo apt remove X` / `sudo rm <path>` commands one by one from the output. 2. `~/go/bin` binaries (golangci-lint, shfmt) fell through the generic `/bin` heuristic to `system`, producing nonsense guidance: `sudo system remove golangci-lint`. They are user-owned `go install` binaries — removal is deleting the file, no sudo needed. 3. `cargo uninstall delta` / `watchexec` failed with "package ID specification did not match any packages": the owning crates are `git-delta` and `watchexec-cli`. ## Fix - `--reconcile --all --apply` now ends with one copy-paste command per remedy: a single `sudo apt remove <pkg…>` per system package manager and a single `sudo rm -f <path…>` for unmanaged binaries (deduped, sorted; only for conflicts the user confirmed). - Path heuristic classifies `/go/bin` as `go` (before the generic `/bin` fallback); `go` binaries are removed like manual installs and rank tier 2 alongside cargo/pip/npm. Shell-side `capability.sh` already handled this. - Cargo uninstall maps binary → owning crate via `cargo install --list` (fallback: tool name). - `system`-classified binaries get `sudo rm <path>` guidance instead of the nonexistent `sudo system remove <tool>`. ## Test plan - 5 new tests (written first, watched fail): GOPATH classification, go-method file removal, system-method rm guidance, cargo crate mapping, and the aggregated command lines in `--all --apply` output. - Full suite: 781 passed, 1 skipped; smoke test OK; pre-commit (flake8, isort, black) green. - Real-machine spot checks: `classify_install_method('/home/sme/go/bin/shfmt') == 'go'`; `_cargo_package_for('delta') == 'git-delta'`; `_cargo_package_for('watchexec') == 'watchexec-cli'`. https://claude.ai/code/session_01MH3EaniXCnJdwqNvrMB4Ym
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
make reconcile-allprompted for removals, the user confirmed withy, yet the run ended withConflicts resolved: 0and exit 1 — with no indication of what went wrong. Nothing was actually removed.Root cause chain:
_classify_via_pathlabeled any/usr/local/bin(or/opt/homebrew) binary asbrew, so removal ran a nonexistentbrew uninstalland failed withFileNotFoundError./usr/local/binare typically root-owned, andos.removefails with a barePermission deniedgiving no path forward.vlog-only, and the--reconcile --all --applynon-JSON path printed just the summary — so every failure after a confirmed prompt was silent.Fix
brewfor/usr/local/bin//opt/homebrewonly whenbrewis installed; otherwisemanual. (The shell-sidescripts/lib/capability.shandreconcile.shalready gate oncommand -v brew; only the Python side was affected.)PermissionErrorduring manual removal now surfaces actionable guidance:Permission denied — remove manually: sudo rm <path>, matching the existing apt/dnf behavior.--reconcile --all --applyprints per-tool outcomes to stderr before the summary:✗ tool: <error>for failures,skipped (declined),skipped (protected system tool).Test plan
/usr/local/binand/opt/homebrew, sudo guidance onPermissionError, and per-tool stderr output for the--all --applypath../scripts/test_smoke.shpasses.audit.py --reconcile yq --apply --yesnow reportsFailed to remove /usr/local/bin/yq: Permission denied — remove manually: sudo rm /usr/local/bin/yqinstead of failing invisibly.https://claude.ai/code/session_01MH3EaniXCnJdwqNvrMB4Ym