From 1b86dd5b738d9b0b272f0d01b8913b2ae8938e78 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Fri, 24 Jul 2026 09:02:29 -0500 Subject: fix(githooks): stop the secret scan passing when it read nothing - The scan piped git diff into grep and swallowed failures with || true. - grep exits 1 on no matches, which is normal, so the guard had to stay. - But it also hid a git failure, leaving nothing to scan and reporting clean. - Now the diff is read on its own and a git failure aborts the commit. - The staged-file list feeding the paren check had the same hole. - New bats tests drive a broken git and pin both paths. --- tests/test-pre-commit-hook.bats | 126 +++++++++++++++++++++++++++++++++++++++ tests/test-validate-el-hook.bats | 73 +++++++++++++++++++++++ 2 files changed, 199 insertions(+) create mode 100644 tests/test-pre-commit-hook.bats create mode 100644 tests/test-validate-el-hook.bats (limited to 'tests') diff --git a/tests/test-pre-commit-hook.bats b/tests/test-pre-commit-hook.bats new file mode 100644 index 00000000..413c71d0 --- /dev/null +++ b/tests/test-pre-commit-hook.bats @@ -0,0 +1,126 @@ +#!/usr/bin/env bats +# Tests for githooks/pre-commit — the secret scan and paren check. +# +# The scan reads its input through a pipeline: +# +# added_lines="$(git diff --cached ... | grep '^+' | grep -v '^+++' || true)" +# +# `grep` exits 1 when it matches nothing, which is the ordinary case, so the +# `|| true` has to stay. But with no `pipefail` it also swallows a failure of +# `git diff` itself, and an empty `added_lines` makes the scan search nothing, +# find nothing, and report clean. A gate that passes without looking is the +# failure this file exists to pin: the fail-open test drives a broken `git diff` +# and asserts the hook refuses rather than exiting 0. +# +# Each test builds a throwaway git repo in BATS_TEST_TMPDIR, so nothing touches +# the real repository or its hooks. + +setup() { + HOOK="${BATS_TEST_DIRNAME}/../githooks/pre-commit" + REPO="${BATS_TEST_TMPDIR}/repo" + mkdir -p "$REPO" + cd "$REPO" || return 1 + git init -q . + git config user.email t@example.com + git config user.name Test + # Split so the fixtures never appear as credential-shaped literals here. + AWS_TAIL="IOSFODNN7EXAMPLE" + WORD_TAIL="word" +} + +# Put a stub `git` ahead of the real one that fails for the staged-diff call +# and delegates everything else, so only the pipeline under test breaks. +break_staged_diff() { + mkdir -p "${BATS_TEST_TMPDIR}/bin" + cat > "${BATS_TEST_TMPDIR}/bin/git" <<'STUB' +#!/usr/bin/env bash +if [ "${1:-}" = "diff" ] && [ "${2:-}" = "--cached" ] && [ "${3:-}" = "-U0" ]; then + echo "simulated git failure" >&2 + exit 128 +fi +exec /usr/bin/git "$@" +STUB + chmod +x "${BATS_TEST_TMPDIR}/bin/git" + PATH="${BATS_TEST_TMPDIR}/bin:$PATH" +} + +# ------------------------------- Normal cases ------------------------------- + +@test "secret scan: blocks a staged AWS key" { + # Assembled at runtime: a literal key-shaped string in this file would trip + # the very hook under test on every commit that touches it, and this repo + # mirrors to a public remote. + printf 'aws = "%s"\n' "AKIA${AWS_TAIL}" > creds.txt + git add creds.txt + run "$HOOK" + [ "$status" -eq 1 ] + [[ "$output" == *"potential secret"* ]] +} + +@test "secret scan: blocks a staged keyword=value password" { + printf '%s = "%s"\n' "pass${WORD_TAIL}" "correcthorsebatterystaple" > conf.txt + git add conf.txt + run "$HOOK" + [ "$status" -eq 1 ] + [[ "$output" == *"potential secret"* ]] +} + +@test "secret scan: allows an ordinary staged file" { + printf 'just some prose\n' > notes.txt + git add notes.txt + run "$HOOK" + [ "$status" -eq 0 ] +} + +# ------------------------------ Boundary cases ------------------------------ + +@test "secret scan: allows a commit with nothing staged" { + run "$HOOK" + [ "$status" -eq 0 ] +} + +@test "paren check: blocks an unbalanced staged .el file" { + printf '(defun broken ()\n (message "no close"\n' > bad.el + git add bad.el + run "$HOOK" + [ "$status" -eq 1 ] + [[ "$output" == *"paren check failed"* ]] +} + +@test "paren check: allows a balanced staged .el file" { + printf '(defun fine ()\n (message "ok"))\n' > good.el + git add good.el + run "$HOOK" + [ "$status" -eq 0 ] +} + +# -------------------------------- Error cases ------------------------------- + +@test "secret scan: refuses to pass when the staged diff cannot be read" { + # The scan must not report clean after searching nothing. Without a + # pipefail-aware guard the broken diff yields an empty added_lines and the + # hook exits 0, letting a real secret through unscanned. + printf 'aws = "%s"\n' "AKIA${AWS_TAIL}" > creds.txt + git add creds.txt + break_staged_diff + run "$HOOK" + [ "$status" -ne 0 ] +} + +@test "paren check: refuses to pass when the staged file list cannot be read" { + printf '(defun broken ()\n (message "no close"\n' > bad.el + git add bad.el + mkdir -p "${BATS_TEST_TMPDIR}/bin2" + cat > "${BATS_TEST_TMPDIR}/bin2/git" <<'STUB' +#!/usr/bin/env bash +if [ "${1:-}" = "diff" ] && [ "${2:-}" = "--cached" ] && [ "${3:-}" = "--name-only" ]; then + echo "simulated git failure" >&2 + exit 128 +fi +exec /usr/bin/git "$@" +STUB + chmod +x "${BATS_TEST_TMPDIR}/bin2/git" + PATH="${BATS_TEST_TMPDIR}/bin2:$PATH" + run "$HOOK" + [ "$status" -ne 0 ] +} diff --git a/tests/test-validate-el-hook.bats b/tests/test-validate-el-hook.bats new file mode 100644 index 00000000..2dbcae79 --- /dev/null +++ b/tests/test-validate-el-hook.bats @@ -0,0 +1,73 @@ +#!/usr/bin/env bats +# Tests for .claude/hooks/validate-el.sh — specifically the auto-test cap. +# +# The hook runs the tests matching an edited file, but only when the match +# count is between 1 and MAX_AUTO_TEST_FILES. Above the cap the whole block +# was skipped with no else branch: nothing printed, exit 0, indistinguishable +# from a passing run. That is live for the three largest families here +# (calendar-sync 63 test files, music 45, ai-term 35), so every edit to those +# modules ran parens and byte-compile and zero tests, silently. +# +# The cap itself is fine — running 63 files per keystroke is not wanted. The +# defect is the silence, so these tests assert the skip announces itself and +# names what to run. +# +# Each test builds a synthetic project in BATS_TEST_TMPDIR and points +# CLAUDE_PROJECT_DIR at it, so nothing runs against the real tree. + +setup() { + HOOK="${BATS_TEST_DIRNAME}/../.claude/hooks/validate-el.sh" + PROJ="${BATS_TEST_TMPDIR}/proj" + mkdir -p "$PROJ/modules" "$PROJ/tests" + export CLAUDE_PROJECT_DIR="$PROJ" + printf '(provide (quote widget))\n' > "$PROJ/modules/widget.el" +} + +# Create N test files matching the widget stem. Each is trivially green so a +# run below the cap succeeds and the only variable is the count. +make_tests() { + local n="$1" i + for ((i = 1; i <= n; i++)); do + printf '(require (quote ert))\n(ert-deftest test-widget-%d () (should t))\n' \ + "$i" > "$PROJ/tests/test-widget-${i}.el" + done +} + +hook_input() { + printf '{"tool_input":{"file_path":"%s"}}' "$PROJ/modules/widget.el" +} + +# ------------------------------- Normal cases ------------------------------- + +@test "under the cap: runs the tests and stays quiet on success" { + make_tests 3 + run bash -c "$(printf '%q' "$HOOK") <<< '$(hook_input)'" + [ "$status" -eq 0 ] + [[ "${output,,}" != *"skipped"* ]] +} + +# ------------------------------ Boundary cases ------------------------------ + +@test "exactly at the cap: still runs the tests" { + make_tests 20 + run bash -c "$(printf '%q' "$HOOK") <<< '$(hook_input)'" + [ "$status" -eq 0 ] + [[ "${output,,}" != *"skipped"* ]] +} + +# -------------------------------- Error cases ------------------------------- + +@test "over the cap: says it skipped rather than exiting silently" { + make_tests 21 + run bash -c "$(printf '%q' "$HOOK") <<< '$(hook_input)'" + # Must not fail the edit — the cap is deliberate, the silence is not. + [ "$status" -eq 0 ] + [[ "${output,,}" == *"skipped"* ]] +} + +@test "over the cap: names the count and how to run them" { + make_tests 21 + run bash -c "$(printf '%q' "$HOOK") <<< '$(hook_input)'" + [[ "$output" == *"21"* ]] + [[ "$output" == *"make test-file"* ]] +} -- cgit v1.2.3