Fix rm hook bypass via inline shell and script execution#31
Open
yurukusa wants to merge 1 commit into
Open
Conversation
Replace regex-based flag detection with shlex.split tokenization to properly distinguish flags from path components (fixes disler#28), and add detection for indirect rm execution patterns (fixes disler#4): - bash/sh/zsh/dash -c "rm -rf /" (inline shell wrappers) - bash/sh script.sh where the script contains dangerous rm commands - source script.sh and . script.sh patterns - Chained commands via &&, ||, ;, and | The old regex approach matched substrings like '-enrollment' inside path names as recursive flags, causing false positives. The new token-based approach only interprets actual option tokens as flags. Script file inspection fails open (allows execution) when the file doesn't exist or can't be read, avoiding false blocks on legitimate commands. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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 #4 and #28 by rewriting
is_dangerous_rm_command()inpre_tool_use.py.Problem 1 — Bypass (#4): The hook only inspected the top-level command string for
rmpatterns. Wrappingrminbash -c "rm -rf /", writing it to a script file, or usingsource/.completely bypassed detection.Problem 2 — False positives (#28): The regex
\brm\s+.*-[a-z]*rmatched path components like-enrollmentas recursive flags, blocking safe commands likerm /path/soft-hold-enrollment/file.rb.Changes
shlex.splittokenization — flags are identified by token position, not substring matching. Path components like-enrollmentare correctly treated as operands, not flags.bash -c "...",sh -c "...",zsh -c "...",dash -c "..."are parsed and the inline script is recursively checked.bash script.sh,sh script.sh,source script.sh,. script.sh, and./script.shtrigger inspection of the script file contents. Fails open (allows execution) if the file doesn't exist or can't be read.&&,||,;,|separators are split soecho hi && rm -rf /is caught.--end-of-options —rm -- -file-starting-with-dashcorrectly treats the operand as a filename.Test plan
rm -rf /— blockedbash -c "rm -rf /"— blocked (was bypassing before)sh -c "rm -r ~"— blocked (was bypassing before)rm -rf /executed viabash script.sh— blocked (was bypassing before)source script.shand. script.shwith dangerous rm — blockedecho hi && rm -rf /— blockedrm /path/soft-hold-enrollment/migrate/foo.rb— allowed (was falsely blocked before)rm file1.txt file2.txt— allowedbash -c "echo hello"— allowedbash nonexistent-script.sh— allowed (fail open)