Repository navigation
fix: allow rm -rf on nested paths while still blocking top-level and wildcard wipes - #21
Merged
Merged
Conversation
…wildcard wipes #18's fallback 'Remove arbitrary absolute path' rule blocked EVERY absolute-path rm -rf, including /root/myapp/build or /tmp/build — this tool's normal daily use. Worse, it trained callers to bypass the validator entirely via 'cd /root && rm -rf subdir'. Rework the hardening rules: - block rm -rf targeting any TOP-LEVEL absolute path (any argument, optional trailing slash): rm -rf /, /home, /etc/, multi-arg variants - block wildcard wipes (rm -rf /home/*, /*) and critical system files (/etc/shadow, /etc/passwd, /etc/sudoers, /etc/fstab, /etc/group, /etc/gshadow, /boot/*) - ALLOW nested paths (depth >= 2): rm -rf /root/otherdirectory, /tmp/build, /home/user/project, /etc/nginx Also fix the keyword-layer 'rm -rf /root\b' pattern whose word boundary between 't' and '/' made it match /root/child too. All issue #17 payloads remain blocked (verified in tests).
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
Follow-up to #19 (and the #17/#18 hardening): the fallback rule added in #18
blocks every absolute-path
rm -rf, including ordinary nested paths:These are this tool's bread-and-butter operations — an SSH agent that can't clean a build directory isn't usable. Worse, the block is trivially bypassed (
cd /root && rm -rf otherdirectorypasses), so the rule adds friction without adding safety. There is also a second, subtler instance: the keyword-layer pattern\brm\s+-rf\s+/root\bmatches/root/childbecause\bholds betweentand/.Fix
Rework the
rm -rfhardening to be depth-aware:Blocked (any argument position, optional trailing slash):
rm -rf /,/home,/etc/,rm -rf /home /tmp,rm -rf /root/ /etc/rm -rf /home/*,rm -rf /*,rm -rf */etc/shadow,/etc/passwd,/etc/sudoers,/etc/group,/etc/gshadow,/etc/fstab,/boot/*rm -rf ..(unchanged)Allowed (depth ≥ 2 — specific content, this tool's normal use case):
rm -rf /root/otherdirectory,/tmp/build,/home/user/project,/var/log/app/,/opt/mytool/cache,/etc/nginx,/usr/share/doc,./buildRegression gate: all issue #17 payloads (
rm -rf /home,/home/*,/etc/passwd,/home ; echo PWNED,cat /etc/shadow,cp /etc/shadow /home/leak,tar czf ... /etc/shadow) remain blocked — verified in tests.Tests
Extended both block/allow lists (now 15 blocked + 13 allowed cases covering multi-argument commands, trailing slashes, wildcards, critical files). Full suite: 12 passed.