diff options
| author | Craig Jennings <c@cjennings.net> | 2026-07-24 09:02:29 -0500 |
|---|---|---|
| committer | Craig Jennings <c@cjennings.net> | 2026-07-24 09:02:29 -0500 |
| commit | 1b86dd5b738d9b0b272f0d01b8913b2ae8938e78 (patch) | |
| tree | 20c8237215339ffe0ca63131fb85088285eef296 /githooks/pre-commit | |
| parent | d9ab1074991ee3c4c680024b14d6de0475560380 (diff) | |
| download | dotemacs-1b86dd5b738d9b0b272f0d01b8913b2ae8938e78.tar.gz dotemacs-1b86dd5b738d9b0b272f0d01b8913b2ae8938e78.zip | |
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.
Diffstat (limited to 'githooks/pre-commit')
| -rwxr-xr-x | githooks/pre-commit | 22 |
1 files changed, 19 insertions, 3 deletions
diff --git a/githooks/pre-commit b/githooks/pre-commit index 2c0cf0b0..8fa489dd 100755 --- a/githooks/pre-commit +++ b/githooks/pre-commit @@ -18,8 +18,17 @@ cd "$REPO_ROOT" || exit 1 SECRET_PATTERNS_CS='(AKIA[0-9A-Z]{16}|sk-[a-zA-Z0-9_-]{20,}|-----BEGIN (RSA|DSA|EC|OPENSSH|PGP)( PRIVATE)?( KEY| KEY BLOCK)?-----)' SECRET_PATTERNS_CI='(api[_-]?key|api[_-]?secret|auth[_-]?token|secret[_-]?key|bearer[_-]?token|access[_-]?token|password)[[:space:]]*[:=][[:space:]]*["'"'"'][^"'"'"']{16,}["'"'"']' -added_lines="$(git diff --cached -U0 --diff-filter=AM \ - | grep '^+' | grep -v '^+++' || true)" +# Read the diff on its own so a git failure is distinguishable from "grep +# matched nothing". Both end in a non-zero status, but only one of them means +# there is nothing to scan; piping them together and swallowing the result with +# `|| true` made a broken git look like a clean commit. +if ! staged_diff="$(git diff --cached -U0 --diff-filter=AM)"; then + echo "pre-commit: cannot read the staged diff — refusing to skip the secret scan" >&2 + exit 1 +fi + +# The greps keep their `|| true`: exiting 1 on no match is their normal result. +added_lines="$(printf '%s\n' "$staged_diff" | grep '^+' | grep -v '^+++' || true)" cs_hits="$(printf '%s\n' "$added_lines" | grep -nE "$SECRET_PATTERNS_CS" || true)" ci_hits="$(printf '%s\n' "$added_lines" | grep -niE "$SECRET_PATTERNS_CI" || true)" @@ -37,7 +46,14 @@ if [ -n "$secret_hits" ]; then fi # --- 2. Paren check on staged .el files --- -staged_el="$(git diff --cached --name-only --diff-filter=AM | grep '\.el$' || true)" +# Same split as the secret scan above: a git failure must not read as "no .el +# files staged", which would skip the paren check silently. +if ! staged_names="$(git diff --cached --name-only --diff-filter=AM)"; then + echo "pre-commit: cannot read the staged file list — refusing to skip the paren check" >&2 + exit 1 +fi + +staged_el="$(printf '%s\n' "$staged_names" | grep '\.el$' || true)" if [ -n "$staged_el" ]; then paren_fail="" |
