fix(guard): detect catastrophic 'rm -rf .', '~', '*', '$HOME' (#470) - #480
Open
Rubyglask wants to merge 2 commits into
Open
fix(guard): detect catastrophic 'rm -rf .', '~', '*', '$HOME' (#470)#480Rubyglask wants to merge 2 commits into
Rubyglask wants to merge 2 commits into
Conversation
…obotics#470) The sensitive-command guard's rm pattern required a '/' in the target, so rm -rf . / ~ / * were never prompted (and it never fired anyway -- filtered by the destructive binary post-check). Adds _is_catastrophic_rm(); prompts on the catastrophic forms, leaves named sub-paths alone. Test-only additions to tests/tools/.
Add tests for catastrophic 'rm' command detection in the guard.
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.
Summary
Fixes #470. CAI's sensitive-command guard is meant to prompt before dangerous
shell commands, but it never prompted on the most common way to wipe data:
rm -rf .,rm -rf ~,rm -rf *. A user reported CAI runningrm -rf .and deleting their home directory with no confirmation.
Root cause (
src/cai/util/user_prompts.py)Two things meant
rmwas effectively unguarded:rmentry in_SENSITIVE_PATTERNSrequired the target to contain/:rm -rf ./~/*/$HOME(no/) were never matched.rm -rf /) never actually prompted:the
destructivecategory is refined by a binary post-check indetect_sensitive_command(_DESTRUCTIVE_TOOL_NAMES = {mkfs, wipefs, fdisk, parted, shred}), andrmisn't in that set, so the match is filtered outbefore it can prompt. (Your own
test_sensitive_guard_false_positives.pydocuments this post-check.) In practice the
rmpattern never fired.Net effect: no
rmcommand prompted, includingrm -rf .in #470.Fix — a small predicate, not a regex
Add
_is_catastrophic_rm(command), checked after the pattern loop sosudo rm -rf /still resolves to thesudocategory (it matches earlier). Itshlex-tokenizes, splits on shell operators (
&&,||,;,|), and flags arecursive
rmwhose target is a catastrophic root (.,..,~,$HOME,*,/) or a top-level system directory (/etc,/home, …), including a globof their contents (
~/*,./*,/home/*).It still prompts (never hard-blocks), and the guard still fails safe on the
headless-menu timeout — so this changes detection only, consistent with every
other pattern.
Coverage (before → after)
rm -rf .(the #470 command)rm -rf ~·rm -rf *·rm -rf $HOMErm -rf ~/*·rm -rf /home/*rm -rf /etc·rm -rf /rm -rf ./build·rm -rf /tmp/scratch(legit)rm -rf ./build && cd ~(legit compound)sudo rm -rf /¹ matched the old regex textually but was filtered by the
destructivebinary post-check, so it never prompted.
The fix adds a prompt for the bare catastrophic forms (that's the point of
#470) while leaving legitimate named sub-paths (
./build,/tmp/scratch,node_modules, …) unflagged — no new false positives, verified by the tests.Tests
tests/tools/test_catastrophic_rm_guard.py— parametrized catastrophic vs.legitimate commands, asserted through both
_is_catastrophic_rmanddetect_sensitive_command(categorydestructive), plussudo rm -rf /stillsudoand the disabled-guard path. The existingtest_sensitive_guard_false_positives.pykeeps passing.Scope / notes
obfuscated forms are out of scope, left to other guards / the model's
judgement: command substitution (
$(pwd)), wrapper binaries (env rm,xargs rm), and non-rmtree deleters (find … -delete).rm -rf */rm -rf .now prompt in cleanup flows too — intended; thoseare exactly the forms that wiped a home directory. Named targets don't prompt.
narrow it if you'd prefer a smaller set.
Fixes #470