From 3227cc65d14c4645c8b7e5e863eafc8d1cb12be9 Mon Sep 17 00:00:00 2001 From: darya <137614867+cutepawss@users.noreply.github.com> Date: Thu, 26 Feb 2026 16:32:01 +0300 Subject: [PATCH 1/2] fix: prevent false positives in recursive delete detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The regex pattern for detecting recursive delete commands (rm -r, rm -rf, etc.) incorrectly matched filenames starting with 'r' — e.g., 'rm readme.txt' was flagged as 'recursive delete' because the dash-flag group was optional. Fix: make the dash mandatory so only actual flags (-r, -rf, -rfv, -fr) are matched. This eliminates false approval prompts for innocent commands like 'rm readme.txt', 'rm requirements.txt', 'rm report.csv', etc. Before: \brm\s+(-[^\s]*)?r — matches 'rm readme.txt' (false positive) After: \brm\s+-[^\s]*r — requires '-' prefix, no false positives --- tools/approval.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/approval.py b/tools/approval.py index 18f9b6743..3d17bd2b0 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -22,7 +22,7 @@ logger = logging.getLogger(__name__) DANGEROUS_PATTERNS = [ (r'\brm\s+(-[^\s]*\s+)*/', "delete in root path"), - (r'\brm\s+(-[^\s]*)?r', "recursive delete"), + (r'\brm\s+-[^\s]*r', "recursive delete"), (r'\brm\s+--recursive\b', "recursive delete (long flag)"), (r'\bchmod\s+(-[^\s]*\s+)*777\b', "world-writable permissions"), (r'\bchmod\s+--recursive\b.*777', "recursive world-writable (long flag)"), From f5c09a3ababb891aac39435ef15d9bd53017e8da Mon Sep 17 00:00:00 2001 From: darya <137614867+cutepawss@users.noreply.github.com> Date: Thu, 26 Feb 2026 16:40:44 +0300 Subject: [PATCH 2/2] test: add regression tests for recursive delete false positive fix Add 15 new tests in two classes: - TestRmFalsePositiveFix (8 tests): verify filenames starting with 'r' (readme.txt, requirements.txt, report.csv, etc.) are NOT falsely flagged as 'recursive delete' - TestRmRecursiveFlagVariants (7 tests): verify all recursive delete flag styles (-r, -rf, -rfv, -fr, -irf, --recursive, sudo rm -rf) are still correctly caught All 29 tests pass (14 existing + 15 new). --- tests/tools/test_approval.py | 62 ++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tests/tools/test_approval.py b/tests/tools/test_approval.py index 63114f6e8..57ffdff25 100644 --- a/tests/tools/test_approval.py +++ b/tests/tools/test_approval.py @@ -93,3 +93,65 @@ class TestApproveAndCheckSession: approve_session(key, "rm") clear_session(key) assert is_approved(key, "rm") is False + + +class TestRmFalsePositiveFix: + """Regression tests: filenames starting with 'r' must NOT trigger recursive delete.""" + + def test_rm_readme_not_flagged(self): + is_dangerous, _, desc = detect_dangerous_command("rm readme.txt") + assert is_dangerous is False, f"'rm readme.txt' should be safe, got: {desc}" + + def test_rm_requirements_not_flagged(self): + is_dangerous, _, desc = detect_dangerous_command("rm requirements.txt") + assert is_dangerous is False, f"'rm requirements.txt' should be safe, got: {desc}" + + def test_rm_report_not_flagged(self): + is_dangerous, _, desc = detect_dangerous_command("rm report.csv") + assert is_dangerous is False, f"'rm report.csv' should be safe, got: {desc}" + + def test_rm_results_not_flagged(self): + is_dangerous, _, desc = detect_dangerous_command("rm results.json") + assert is_dangerous is False, f"'rm results.json' should be safe, got: {desc}" + + def test_rm_robots_not_flagged(self): + is_dangerous, _, desc = detect_dangerous_command("rm robots.txt") + assert is_dangerous is False, f"'rm robots.txt' should be safe, got: {desc}" + + def test_rm_run_not_flagged(self): + is_dangerous, _, desc = detect_dangerous_command("rm run.sh") + assert is_dangerous is False, f"'rm run.sh' should be safe, got: {desc}" + + def test_rm_force_readme_not_flagged(self): + is_dangerous, _, desc = detect_dangerous_command("rm -f readme.txt") + assert is_dangerous is False, f"'rm -f readme.txt' should be safe, got: {desc}" + + def test_rm_verbose_readme_not_flagged(self): + is_dangerous, _, desc = detect_dangerous_command("rm -v readme.txt") + assert is_dangerous is False, f"'rm -v readme.txt' should be safe, got: {desc}" + + +class TestRmRecursiveFlagVariants: + """Ensure all recursive delete flag styles are still caught.""" + + def test_rm_r(self): + assert detect_dangerous_command("rm -r mydir")[0] is True + + def test_rm_rf(self): + assert detect_dangerous_command("rm -rf /tmp/test")[0] is True + + def test_rm_rfv(self): + assert detect_dangerous_command("rm -rfv /var/log")[0] is True + + def test_rm_fr(self): + assert detect_dangerous_command("rm -fr .")[0] is True + + def test_rm_irf(self): + assert detect_dangerous_command("rm -irf somedir")[0] is True + + def test_rm_recursive_long(self): + assert detect_dangerous_command("rm --recursive /tmp")[0] is True + + def test_sudo_rm_rf(self): + assert detect_dangerous_command("sudo rm -rf /tmp")[0] is True +