From d182d90708435f8926e716bfca92e554c553de51 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 21 Jul 2026 17:13:03 -0700 Subject: [PATCH] fix(approval): detect recursive rm when flags follow operands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port from openai/codex#33464: GNU rm permutes options, so `rm build/ -rf`, `rm build/ -r -f`, and `rm build/ --recursive --force` are equivalent to the flags-first spellings — but every existing rm pattern required the flag group BEFORE the path, so these spellings ran with no approval prompt at all (proven live on main). The hardline floor was NOT affected: protected paths (/, system dirs, $HOME) match regardless of flag position because the hardline path matcher does not require flags. The gap was the approval-prompt layer for arbitrary paths. New DANGEROUS_PATTERNS entry with a tempered operand run: cannot cross command separators (; | & newline), quotes, or a bare -- end-of-options separator (after --, -rf is a literal filename). Flag token must be whitespace-anchored so the r inside long options like --registry does not count. 9 positive + 7 negative shapes in tests; approval cluster (868 tests) green. --- tests/tools/test_approval.py | 35 +++++++++++++++++++++++++++++++++++ tools/approval.py | 18 ++++++++++++++++++ 2 files changed, 53 insertions(+) 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.