aboutsummaryrefslogtreecommitdiff
path: root/hooks/tests/test_destructive_bash_confirm.py
diff options
context:
space:
mode:
authorCraig Jennings <c@cjennings.net>2026-05-22 15:38:58 -0500
committerCraig Jennings <c@cjennings.net>2026-05-22 15:38:58 -0500
commit53d33d9cdd5c6d7fd7c4dc7315b04d225add94d6 (patch)
treea2cfe5331eed9e22cc9880c3b75e246d74e2239e /hooks/tests/test_destructive_bash_confirm.py
parentefcc8e5ffdd09f538fd2d83824f4c632264ad96c (diff)
downloadrulesets-53d33d9cdd5c6d7fd7c4dc7315b04d225add94d6.tar.gz
rulesets-53d33d9cdd5c6d7fd7c4dc7315b04d225add94d6.zip
feat(hooks): scan file-backed messages and harden rm parsing
Two audit gaps in the confirmation hooks, plus the test harness they were missing. The git-commit and gh-pr-create hooks scanned for AI attribution but only saw inline messages. A commit made with -F/--file or a PR made with --body-file slipped through, since the hook stored a placeholder instead of the file's text, and the publish flow uses -F constantly. A new read_referenced_file helper in _common.py reads the referenced local file (missing, oversized, or non-UTF-8 returns None, which means "couldn't inspect" and never "clean"), so attribution scanning now sees the real committed and posted text. An unreadable file falls through to the existing ask-anyway path. destructive-bash-confirm.py parsed rm flags by splitting on whitespace, which mangled quoted paths and missed flag variants. detect_rm_rf now tokenizes with shlex, so quoted or spaced paths and combined, separate, or reordered flags all parse. It fails toward asking (a sentinel that still fires the modal) on unbalanced quotes, or when a forced recursive rm sits alongside a pipeline, compound command, substitution, or redirect, since target attribution isn't trustworthy there. The supported and unsupported shell constructs are documented in the docstrings. These hooks had no tests and weren't in make test. Added a pytest harness under hooks/tests (an importlib-by-path loader, since the hook filenames are hyphenated) with 54 tests across the three hooks and the shared helper, and wired hooks/tests into make test. Full suite green.
Diffstat (limited to 'hooks/tests/test_destructive_bash_confirm.py')
-rw-r--r--hooks/tests/test_destructive_bash_confirm.py137
1 files changed, 137 insertions, 0 deletions
diff --git a/hooks/tests/test_destructive_bash_confirm.py b/hooks/tests/test_destructive_bash_confirm.py
new file mode 100644
index 0000000..50302e6
--- /dev/null
+++ b/hooks/tests/test_destructive_bash_confirm.py
@@ -0,0 +1,137 @@
+"""Tests for hooks/destructive-bash-confirm.py — shlex-based rm -rf parsing."""
+
+from conftest import load_hook
+
+hook = load_hook("destructive-bash-confirm.py")
+
+SENTINEL = "(unparsed — shell too complex to inspect safely)"
+
+
+# --- detection of flag forms (combined / separate / reordered) -------------
+
+def test_rf_combined_detected():
+ assert hook.detect_rm_rf("rm -rf build") == ["build"]
+
+
+def test_r_f_separate_detected():
+ assert hook.detect_rm_rf("rm -r -f build") == ["build"]
+
+
+def test_fr_reordered_detected():
+ assert hook.detect_rm_rf("rm -fr build") == ["build"]
+
+
+def test_capital_R_detected():
+ assert hook.detect_rm_rf("rm -Rf build") == ["build"]
+
+
+def test_long_flags_detected():
+ targets = hook.detect_rm_rf("rm --recursive --force build")
+ assert targets == ["build"]
+
+
+# --- quoted / spaced paths now parse correctly -----------------------------
+
+def test_quoted_path_with_space_parsed():
+ assert hook.detect_rm_rf('rm -rf "my dir"') == ["my dir"]
+
+
+def test_multiple_targets():
+ assert hook.detect_rm_rf("rm -rf a b c") == ["a", "b", "c"]
+
+
+def test_double_dash_separates_flags_from_paths():
+ assert hook.detect_rm_rf("rm -rf -- -weird-name") == ["-weird-name"]
+
+
+# --- not-a-match cases: no modal -------------------------------------------
+
+def test_no_r_returns_none():
+ assert hook.detect_rm_rf("rm -f file") is None
+
+
+def test_no_f_returns_none():
+ assert hook.detect_rm_rf("rm -r dir") is None
+
+
+def test_not_rm_returns_none():
+ assert hook.detect_rm_rf("rmdir foo") is None
+
+
+def test_plain_rm_returns_none():
+ assert hook.detect_rm_rf("rm file.txt") is None
+
+
+# --- dangerous path banner still fires on parsed targets -------------------
+
+def test_home_var_target_flags_dangerous():
+ detection = hook.detect_destructive('rm -rf "$HOME/x"')
+ assert detection is not None
+ kind, ctx = detection
+ assert kind == "rm -rf"
+ assert "_banner" in ctx
+ assert "$HOME/x" in ctx["_banner"]
+
+
+def test_root_path_flags_dangerous():
+ detection = hook.detect_destructive("rm -rf /etc/foo")
+ assert detection is not None
+ _, ctx = detection
+ assert "_banner" in ctx
+
+
+def test_safe_relative_target_no_banner():
+ detection = hook.detect_destructive("rm -rf build/cache")
+ assert detection is not None
+ _, ctx = detection
+ assert "_banner" not in ctx
+
+
+# --- fail-toward-asking on ambiguity ---------------------------------------
+
+def test_compound_command_returns_sentinel():
+ # `ls && rm -rf foo` — the naive parser missed this; now we ask.
+ assert hook.detect_rm_rf("ls && rm -rf foo") == [SENTINEL]
+
+
+def test_pipeline_returns_sentinel():
+ assert hook.detect_rm_rf("find . -type d | xargs rm -rf") == [SENTINEL]
+
+
+def test_semicolon_returns_sentinel():
+ assert hook.detect_rm_rf("cd /tmp; rm -rf junk") == [SENTINEL]
+
+
+def test_command_substitution_returns_sentinel():
+ assert hook.detect_rm_rf("rm -rf $(echo target)") == [SENTINEL]
+
+
+def test_backtick_substitution_returns_sentinel():
+ assert hook.detect_rm_rf("rm -rf `echo target`") == [SENTINEL]
+
+
+def test_redirect_returns_sentinel():
+ assert hook.detect_rm_rf("rm -rf foo > /dev/null") == [SENTINEL]
+
+
+def test_unbalanced_quotes_returns_sentinel():
+ # shlex.split raises ValueError → ask anyway rather than silently pass.
+ assert hook.detect_rm_rf('rm -rf "unterminated') == [SENTINEL]
+
+
+def test_compound_without_rm_rf_returns_none():
+ # Compound construct but no dangerous rm — should not fire.
+ assert hook.detect_rm_rf("ls && echo done") is None
+
+
+def test_compound_with_rm_but_no_force_returns_none():
+ # `&&` present but the rm has no -f, so nothing to flag.
+ assert hook.detect_rm_rf("ls && rm -r dir") is None
+
+
+def test_sentinel_fires_modal_via_detect_destructive():
+ detection = hook.detect_destructive("ls && rm -rf foo")
+ assert detection is not None
+ kind, ctx = detection
+ assert kind == "rm -rf"
+ assert ctx["targets"] == [SENTINEL]