diff --git a/tests/tools/test_approval.py b/tests/tools/test_approval.py index 1ebb3efe6d98..d79ec64e786f 100644 --- a/tests/tools/test_approval.py +++ b/tests/tools/test_approval.py @@ -119,6 +119,41 @@ class TestDetectDangerousRm: assert key is not None assert "delete" in desc.lower() + def test_rm_flags_after_operands_detected(self): + # GNU rm permutes options: `rm build/ -rf` == `rm -rf build/`. + # Port of openai/codex#33464. + for cmd in ( + "rm build/ -rf", + "rm build/ -r -f", + "rm build/ -fR", + "rm build/ --recursive --force", + "rm build/ --force --recursive", + "rm ~/projects -rf", + "sudo rm build/ -rf", + "rm -f build/ -r", + "rm one two three -rf", + ): + is_dangerous, key, desc = detect_dangerous_command(cmd) + assert is_dangerous is True, f"{cmd!r} should require approval" + assert "delete" in desc.lower() + + def test_rm_flags_after_operands_no_false_positives(self): + for cmd in ( + # after a bare `--`, -rf-looking tokens are literal filenames + "rm -- -weird-r-file", + "rm -f -- -r-file", + # a later pipeline/command segment's flags don't belong to rm + "rm foo | grep -r bar", + "rm foo; ls -lart", + # long options whose `r` is not whitespace-anchored + "npm rm somepkg --registry=https://registry.npmjs.org", + "rm old.log --verbose", + # plain multi-operand deletes stay safe + "rm build/file.txt other.txt", + ): + is_dangerous, key, desc = detect_dangerous_command(cmd) + assert is_dangerous is False, f"{cmd!r} should be safe, got: {desc}" + def test_nonrecursive_verification_artifact_cleanup_is_not_dangerous(self): with mock_patch("tempfile.gettempdir", return_value="/tmp"): for prefix in ("hermes-verify-", "hermes-ad-hoc-"): diff --git a/tools/approval.py b/tools/approval.py index 9962bf851d8b..cf19b12f2f8c 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -607,6 +607,24 @@ DANGEROUS_PATTERNS = [ (r'\brm\s+(-[^\s]*\s+)*/', "delete in root path"), (r'\brm\s+-[^\s]*r', "recursive delete"), (r'\brm\s+--recursive\b', "recursive delete (long flag)"), + # GNU rm permutes options, so a recursive flag group may legally FOLLOW + # the operands: `rm build/ -rf`, `rm build/ -r -f`, and `rm build/ + # --recursive --force` are all equivalent to the flags-first spellings the + # two patterns above catch — without this rule they run with no approval + # prompt at all. The operand run is tempered: it cannot cross a command + # separator (`;`, `|`, `&`, newline — so a later pipeline segment's flags, + # e.g. `rm foo | grep -r bar`, are not attributed to `rm`), cannot cross a + # quote (so `git commit -m "rm x" --amend` style data can't bridge an `rm` + # word to an unrelated dash token), and cannot cross a bare ` -- ` + # end-of-options separator (after `--`, POSIX rm treats `-rf` as a literal + # filename, not flags; guarded both leading and mid-run). The flag token + # itself must start right after whitespace so the `r` inside long options + # like `--registry` (preceded by `-`, not whitespace) does not count. + # Port of openai/codex#33464 ("recognize force options when they follow + # operands"). + (r'\brm\s+(?!--(?:\s|$))(?:(?!\s--(?:\s|$))[^\n"\';|&])*\s' + r'(?:-[a-z]*r[a-z]*\b|--recursive\b)', + "recursive delete (flags after operands)"), # Windows shell front-ends have destructive built-ins that do not look like # Unix `rm`. Gate only when they are executed through cmd/powershell so # ordinary prose or filenames containing "del"/"rd" do not trip the guard.