diff options
| author | Craig Jennings <c@cjennings.net> | 2026-05-22 15:38:58 -0500 |
|---|---|---|
| committer | Craig Jennings <c@cjennings.net> | 2026-05-22 15:38:58 -0500 |
| commit | 53d33d9cdd5c6d7fd7c4dc7315b04d225add94d6 (patch) | |
| tree | a2cfe5331eed9e22cc9880c3b75e246d74e2239e /hooks/tests/test_git_commit_confirm.py | |
| parent | efcc8e5ffdd09f538fd2d83824f4c632264ad96c (diff) | |
| download | rulesets-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_git_commit_confirm.py')
| -rw-r--r-- | hooks/tests/test_git_commit_confirm.py | 97 |
1 files changed, 97 insertions, 0 deletions
diff --git a/hooks/tests/test_git_commit_confirm.py b/hooks/tests/test_git_commit_confirm.py new file mode 100644 index 0000000..83519ad --- /dev/null +++ b/hooks/tests/test_git_commit_confirm.py @@ -0,0 +1,97 @@ +"""Tests for hooks/git-commit-confirm.py — file-backed commit messages.""" + +from conftest import load_hook + +hook = load_hook("git-commit-confirm.py") + + +# --- existing forms still parse (regression guard) ------------------------- + +def test_extract_dash_m_simple(): + msg = hook.extract_commit_message('git commit -m "fix: tidy parser"') + assert msg == "fix: tidy parser" + + +def test_extract_heredoc(): + cmd = ( + "git commit -m \"$(cat <<'EOF'\n" + "feat: add thing\n" + "\n" + "body line\n" + "EOF\n" + ")\"" + ) + msg = hook.extract_commit_message(cmd) + assert msg.startswith("feat: add thing") + assert "body line" in msg + + +def test_extract_unparseable_falls_through(): + # Bare `git commit` would drop into $EDITOR. + assert hook.extract_commit_message("git commit") == hook.UNPARSEABLE_MESSAGE + + +# --- new: -F / --file / --file= forms read the file ------------------------ + +def test_extract_dash_F_reads_file(tmp_path): + f = tmp_path / "msg.txt" + f.write_text("fix: from a file\n\nsome body\n") + assert hook.extract_commit_message(f"git commit -F {f}") == "fix: from a file\n\nsome body" + + +def test_extract_long_file_flag_reads_file(tmp_path): + f = tmp_path / "msg.txt" + f.write_text("docs: long form file flag\n") + assert hook.extract_commit_message(f"git commit --file {f}") == "docs: long form file flag" + + +def test_extract_file_equals_form_reads_file(tmp_path): + f = tmp_path / "msg.txt" + f.write_text("chore: equals form\n") + assert hook.extract_commit_message(f"git commit --file={f}") == "chore: equals form" + + +def test_extract_F_strips_quotes_around_path(tmp_path): + f = tmp_path / "my msg.txt" + f.write_text("feat: quoted path\n") + assert hook.extract_commit_message(f'git commit -F "{f}"') == "feat: quoted path" + + +# --- the audit-item bug: attribution in a file-backed message is now caught - + +def test_file_backed_attribution_is_caught(tmp_path): + f = tmp_path / "msg.txt" + f.write_text("feat: add widget\n\nCo-Authored-By: Claude <noreply@anthropic.com>\n") + msg = hook.extract_commit_message(f"git commit -F {f}") + issues = hook.collect_issues(msg, staged=["a.py"], author="Real Dev <dev@example.com>") + assert any(i.startswith("AI-attribution") for i in issues) + + +def test_inline_message_without_attribution_is_clean(tmp_path): + # Sanity: a clean file-backed message produces no attribution issue. + f = tmp_path / "msg.txt" + f.write_text("fix: handle empty input\n") + msg = hook.extract_commit_message(f"git commit -F {f}") + issues = hook.collect_issues(msg, staged=["a.py"], author="Real Dev <dev@example.com>") + assert not any(i.startswith("AI-attribution") for i in issues) + + +# --- unreadable file falls through to UNPARSEABLE (fail-safe: ask) --------- + +def test_missing_file_falls_through_to_unparseable(tmp_path): + missing = tmp_path / "nope.txt" + assert hook.extract_commit_message(f"git commit -F {missing}") == hook.UNPARSEABLE_MESSAGE + + +def test_oversized_file_falls_through_and_hook_asks(tmp_path, monkeypatch): + f = tmp_path / "big.txt" + f.write_text("x" * 5000) + # Force the read to refuse via a tiny limit (simulates oversize). + monkeypatch.setattr( + hook, "read_referenced_file", lambda p, max_bytes=10: None + ) + msg = hook.extract_commit_message(f"git commit -F {f}") + assert msg == hook.UNPARSEABLE_MESSAGE + # And the hook would ask, because UNPARSEABLE_MESSAGE is a flagged issue. + issues = hook.collect_issues(msg, staged=["a.py"], author="Dev <d@e.com>") + assert any("not parseable" in i for i in issues) |
