From 3de984d874149fe6bcc66bdab155381af3c6af8d Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sun, 27 Sep 2026 23:58:15 +0200 Subject: [PATCH 1/4] sec(scripts): git-secrets allowlist matched whole lines and whitelisted most of the repo This repo's scripts/setup-git-secrets.sh and .gitallowed were copied verbatim from the monorepo split and carried the same #1972 defect fixed in LeanerCloud/reserved-instances-cli#2083 (upstream, now cloud-commitments-platform#101): git-secrets allowed patterns are matched against the scanner's whole "path:line:content" output line, not file content alone, so the script's 20 keyword allowlist entries (var\., resource\s, _test\.go, placeholder, example\.com, ...) whitelisted any line containing one of those tokens, including a line carrying a real access key. The script also never ran to completion on any platform before this fix: the PEM detector's pattern starts with a dash, which git secrets --add rejects, aborting the script under set -e before the allowed block was ever reached; the GCP API-key pattern registered just before that abort was an invalid regex, which (once reached) makes every subsequent scan exit 128; and git-secrets --install's success path calls `say`, removed from git-sh-setup in Git 2.38, so on Linux a successful install reports failure and this script's error handling exited before registering anything. - Delete the allowed-pattern block from scripts/setup-git-secrets.sh. .gitallowed is now the single allowlist. - Fix the GCP API-key regex; broaden the PEM detector to PKCS#8. - Define a no-op say() so hook installation succeeds on Linux. - .gitallowed: rewrite the header to describe path-anchored matching against the whole scanner line, add three path-and-line-anchored entries for the setup script's self-matching detector-registration lines (Azure connection string, PostgreSQL, MySQL), and add the truncated-PEM entry scripts/test-git-secrets-allowlist.sh's negative controls need (kept generic since this repo has no cmd/configure_test.go to reference). - scripts/test-git-secrets-allowlist.sh: self-test exercising both the direct scan and the installed pre-commit hook, HOME-isolated. - Pin the git-secrets clone in .github/workflows/pre-commit.yml to the verified commit ad82d68ee924906a0401dfd48de5057731a9bc84 (the 1.3.0 tag) instead of the mutable tag, and run the new self-test as a step in the same job. Adapted from the platform port: this repo has no ci.yml job graph to extend, so the self-test runs as a pre-commit.yml step instead of a new ci.yml job. The pre-existing account-ID placeholder section in .gitallowed (entries referencing handler_*_test.go etc. that don't exist in this repo) is left untouched -- out of scope for this fix. Verified in a throwaway repo with the real git-secrets binary: scripts/test-git-secrets-allowlist.sh passes 30/30. Direct proof the hole is closed: a Terraform-shaped line with an AKIA-format key scans dirty with this .gitallowed; re-adding the old `resource\s` keyword allowlist makes the identical line scan clean, reproducing the pre-fix bug on demand. git ls-remote confirms the 1.3.0 tag resolves to the pinned SHA. pre-commit (SKIP=hadolint,actionlint; Docker daemon unavailable) passes on all other hooks; actionlint run natively at the pinned v1.7.12 is clean on the changed workflow file. Co-Authored-By: claude-flow --- .gitallowed | 40 ++++++- .github/workflows/pre-commit.yml | 28 ++++- CHANGELOG.md | 5 + scripts/setup-git-secrets.sh | 62 ++++------ scripts/test-git-secrets-allowlist.sh | 166 ++++++++++++++++++++++++++ 5 files changed, 258 insertions(+), 43 deletions(-) create mode 100755 scripts/test-git-secrets-allowlist.sh diff --git a/.gitallowed b/.gitallowed index 6ff918d..9e4fb7d 100644 --- a/.gitallowed +++ b/.gitallowed @@ -10,8 +10,11 @@ # DO NOT add a real account ID here. If a real account ID lands in the repo, # rotate it and treat the leak seriously instead of silencing the scanner. # -# Format: one regex per line, matched against each line of file content -# (path-scoping is not supported by .gitallowed). +# Format: one regex per line, matched against the scanner's whole +# "path:line:content" output line (not just the file content), so an entry +# may anchor on the path with "^path:[0-9]+:". Every entry must still be a +# synthetic literal or one known benign line, never a bare keyword: a +# keyword whitelists every line that happens to contain it (#1972). # Sequential test placeholders (ascending and descending). 123456789012 @@ -46,3 +49,36 @@ # UUID-shaped account ID used in handler_accounts_test.go (synthetic, not a # real subscription/account). 11111111-1111-1111-1111-111111111111 + +# scripts/setup-git-secrets.sh registers its detectors as literal regex text +# and three of them (Azure connection string, PostgreSQL and MySQL DSN +# detectors, verified empirically -- MongoDB's does not) match their own +# registration line. Each entry below is anchored on the path, the line +# number, AND the entire line as it appears in that file (including the +# trailing comment), with a trailing $ -- not just the "git secrets --add +# '" command prefix, and not just the added pattern with the anchor left +# open at the end. A prefix-only anchor whitelists a secret appended to +# ANY registration line in this file; an anchor missing the trailing $ +# whitelists a secret appended to the END of one of these three specific +# lines. Both are the #1972 hole reintroduced at a smaller scale, and both +# were caught only by a fixture exercising exactly that mutation -- add +# one for any future entry in this block that isn't already covered. +# +# Each entry below also replaces one letter of its own trigger substring +# with a single-character bracket expression, e.g. "Protoco[l]" for +# "Protocol". This is a no-op for matching (as a regex, [l] matches the +# literal letter l exactly like l would) but it breaks the contiguous +# trigger span each entry is built from, so .gitallowed does not need to +# allow-list ITSELF against the very patterns these entries exist to +# suppress (confirmed empirically: without this, the .gitallowed +# self-scan case below fails -- and note this comment must not spell any +# of those spans out literally either, for the same reason). +^scripts/setup-git-secrets\.sh:[0-9]+:git secrets --add 'DefaultEndpointsProtoco[l]=https' # Azure Connection String$ +^scripts/setup-git-secrets\.sh:[0-9]+:git secrets --add 'postgre[s]://\[\^:\]\+:\[\^@\]\+@' # PostgreSQL$ +^scripts/setup-git-secrets\.sh:[0-9]+:git secrets --add 'mysq[l]://\[\^:\]\+:\[\^@\]\+@' # MySQL$ + +# Truncated PEM fixtures used by scripts/test-git-secrets-allowlist.sh (and +# by any test in this repo that needs a non-secret PEM-shaped string): a PEM +# header followed by "\n..." is never a key body. Written without "BEGIN" so +# this line itself does not trip the PEM detector. +PRIVATE KEY-----\\n(MIIEvQIBADANBg)?\.\.\. diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index 2e07902..eaaf826 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -97,14 +97,25 @@ jobs: sh /tmp/trivy-install.sh -b /usr/local/bin "${TRIVY_VERSION}" - name: Install git-secrets - # Pinned to a release tag rather than master HEAD. After install - # we register the AWS pattern set and ASSERT at least one pattern - # was registered — without the assert, a registration failure - # produces a patternless scanner that exits 0 unconditionally, - # leaving the gate silently downgraded. + # Pinned to a release tag rather than master HEAD. The tag is + # mutable in principle, so also assert HEAD resolves to the exact + # commit verified at the time this was written (`git ls-remote + # --tags` against the upstream repo) before trusting it enough to + # `sudo make install`. After install we register the AWS pattern + # set and ASSERT at least one pattern was registered — without the + # assert, a registration failure produces a patternless scanner + # that exits 0 unconditionally, leaving the gate silently + # downgraded. + env: + GIT_SECRETS_SHA: ad82d68ee924906a0401dfd48de5057731a9bc84 run: | set -euo pipefail git clone --depth 1 --branch 1.3.0 https://github.com/awslabs/git-secrets.git /tmp/git-secrets + resolved="$(git -C /tmp/git-secrets rev-parse HEAD)" + if [ "${resolved}" != "${GIT_SECRETS_SHA}" ]; then + echo "git-secrets 1.3.0 tag resolved to ${resolved}, expected ${GIT_SECRETS_SHA}" >&2 + exit 1 + fi sudo make -C /tmp/git-secrets install git secrets --register-aws --global git secrets --list --global | grep -q '.' || { @@ -112,6 +123,13 @@ jobs: exit 1 } + - name: Run git-secrets allowlist self-test + # Self-test for the git-secrets allowlist (#1972): asserts, through + # the real pre-commit hook scan path, that secret-shaped fixtures + # are still caught and that the documented false positives still + # scan clean. + run: bash scripts/test-git-secrets-allowlist.sh + # Note: the local `hadolint` hook in .pre-commit-config.yaml (search for # `id: hadolint`; no line number, because this comment has already gone # stale twice as that file shifted) runs ghcr.io/hadolint/hadolint pinned diff --git a/CHANGELOG.md b/CHANGELOG.md index 643c1df..8d53315 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). - Align pre-commit gocyclo threshold (10) with CI pipeline - Pin tool versions in GitHub Actions for reproducible builds - Update README Go version badge to match go.mod (1.25+) +- The local git-secrets setup script aborted on its PEM pattern and, when + patched past that, made every scan fail on an invalid regex; its keyword + allowlist whitelisted whole lines containing common Go/Terraform tokens. + Allowlisting now lives in `.gitallowed` as literal entries and the PEM + detector covers PKCS#8 keys (#1972) ## [0.9.0] - 2026-03-06 diff --git a/scripts/setup-git-secrets.sh b/scripts/setup-git-secrets.sh index 16cbf76..8a566e3 100755 --- a/scripts/setup-git-secrets.sh +++ b/scripts/setup-git-secrets.sh @@ -30,6 +30,26 @@ echo -e "${GREEN}✓ git-secrets is installed${NC}" echo "" # Install git hooks +# +# git-secrets 1.3.0's install_hook() writes and chmods the hook file, then +# reports success via a `say` call. `say` was never its own function: +# git-secrets sources git's git-sh-setup and relied on `say` being defined +# there (introduced in git-sh-setup.sh in 2009). Git removed it in commit +# 5b893f7d81 ("git-sh-setup.sh: remove 'say' function, change last users"), +# first shipped in Git 2.38 (2022), because it was undocumented and unused +# within git's own tree, breaking git-secrets as an unintended side effect +# of a git upgrade rather than a git-secrets regression. On macOS the bare +# `say` call resolves to /usr/bin/say (the text-to-speech binary) instead +# and exits 0 by accident; on Linux (or on a new-enough git anywhere) there +# is no such fallback, so it's "command not found" and `git secrets +# --install -f` returns non-zero even though every hook file was already +# written correctly. Define `say` as a no-op here and export it so the +# exported function is visible in the git-secrets child process on both +# platforms, making the real hook-writing exit status the one that reaches +# the check below. +say() { :; } +export -f say + echo "Installing git-secrets hooks..." if git secrets --install -f; then echo -e "${GREEN}✓ Git hooks installed${NC}" @@ -53,8 +73,7 @@ git secrets --add '[^A-Za-z0-9/+=]{40}[^A-Za-z0-9/+=]' # AWS Sec git secrets --add 'aws(.{0,20})?['\''"][0-9a-zA-Z/+]{40}['\''"]' # AWS Credentials # GCP patterns -git secrets --add 'type.*service_account' # GCP Service Account JSON -git secrets --add 'AIza[0-9A-Za-z-_]{35}' # GCP API Key +git secrets --add 'AIza[0-9A-Za-z_-]{35}' # GCP API Key # Azure patterns git secrets --add 'DefaultEndpointsProtocol=https' # Azure Connection String @@ -63,46 +82,17 @@ git secrets --add 'DefaultEndpointsProtocol=https' # Azure git secrets --add 'password\s*[=:]\s*['\''"][^'\''"]{8,}' # Password with quoted value git secrets --add 'api[_-]?key\s*[=:]\s*['\''"][^'\''"]{8,}' # API key with quoted value git secrets --add 'secret[_-]?key\s*[=:]\s*['\''"][^'\''"]{8,}' # Secret key with quoted value -git secrets --add '-----BEGIN (RSA|DSA|EC|OPENSSH) PRIVATE KEY-----' # PEM private keys +git secrets --add 'BEGIN[[:space:]]((RSA|DSA|EC|OPENSSH|ENCRYPTED)[[:space:]])?PRIVATE[[:space:]]KEY-----' # PEM private keys # Database connection strings git secrets --add 'postgres://[^:]+:[^@]+@' # PostgreSQL git secrets --add 'mysql://[^:]+:[^@]+@' # MySQL git secrets --add 'mongodb(\+srv)?://[^:]+:[^@]+@' # MongoDB -# Add allowed patterns (things that look like secrets but aren't) -echo "" -echo "Adding allowed patterns (false positives)..." - -# Terraform variables and outputs -git secrets --add --allowed 'var\.' -git secrets --add --allowed 'local\.' -git secrets --add --allowed 'output\.' -git secrets --add --allowed 'data\.' - -# Test files -git secrets --add --allowed '_test\.go' -git secrets --add --allowed 'testdata/' -git secrets --add --allowed 'test_password' -git secrets --add --allowed 'test_secret' - -# Documentation and examples -git secrets --add --allowed 'example\.com' -git secrets --add --allowed 'YOUR_' -git secrets --add --allowed '/dev/null 2>&1; then + echo "git-secrets not found on PATH; install it before running this test" >&2 + exit 1 +fi + +tmp=$(mktemp -d) +trap 'rm -rf "$tmp"' EXIT +mkdir -p "$tmp/home" +# Isolate HOME: --register-aws reads ~/.aws/credentials, and git-secrets +# echoes the whole joined pattern list on a regex error, which would print +# real credentials into this log. +export HOME="$tmp/home" + +git init -q "$tmp/repo" +mkdir -p "$tmp/repo/scripts" +cp "$REPO_ROOT/scripts/setup-git-secrets.sh" "$tmp/repo/scripts/setup-git-secrets.sh" +cp "$REPO_ROOT/.gitallowed" "$tmp/repo/.gitallowed" +cd "$tmp/repo" + +if ! bash scripts/setup-git-secrets.sh > "$tmp/setup.log" 2>&1; then + echo "scripts/setup-git-secrets.sh failed in the throwaway repo:" >&2 + cat "$tmp/setup.log" >&2 + exit 1 +fi +# Same HOME-isolation reason: drop the provider before any further scan. +git config --unset-all secrets.providers + +# The pre-commit hook ignores the args git would pass it and recomputes its +# own file list from the index (diff against HEAD, or the empty tree if +# there is no HEAD yet, which is always true in this never-committed +# throwaway repo). So it scans every currently staged path, not just +# $relpath -- calling it with no args, as git itself does, is correct. +HOOK_DIR="$(git rev-parse --git-dir)/hooks" + +pass=0 +fail=0 + +# Records one pass/fail against $expected, printing a FAIL line labeled +# $label on mismatch. Shared by both scan paths in run_case/run_staged_case +# so a direct-scan/hook disagreement surfaces as its own labeled failure +# instead of being silently reconciled. +check_result() { + local label=$1 expected=$2 actual=$3 + if [ "$actual" -eq "$expected" ]; then + pass=$((pass + 1)) + else + fail=$((fail + 1)) + echo "FAIL: $label (expected exit $expected, got $actual)" >&2 + fi +} + +# Writes $content to $relpath, stages it, scans it two ways -- directly via +# `git secrets --scan --cached` (exact file argument) and via the installed +# pre-commit hook (the real path a developer's commit takes) -- compares +# each exit code to $expected, then unstages and removes it. +run_case() { + local label=$1 expected=$2 relpath=$3 content=$4 + mkdir -p "$(dirname "$relpath")" + printf '%s\n' "$content" >"$relpath" + git add "$relpath" + local actual=0 + git secrets --scan --cached "$relpath" >/dev/null 2>&1 || actual=$? + check_result "$label (direct scan)" "$expected" "$actual" + local hook_actual=0 + "$HOOK_DIR/pre-commit" >/dev/null 2>&1 || hook_actual=$? + check_result "$label (installed hook)" "$expected" "$hook_actual" + git rm -q --cached "$relpath" + rm -f "$relpath" +} + +# Same as run_case, for a file already on disk (the copied setup script and +# .gitallowed), left staged afterward like any other tracked file would be. +run_staged_case() { + local label=$1 expected=$2 relpath=$3 + git add "$relpath" + local actual=0 + git secrets --scan --cached "$relpath" >/dev/null 2>&1 || actual=$? + check_result "$label (direct scan)" "$expected" "$actual" + local hook_actual=0 + "$HOOK_DIR/pre-commit" >/dev/null 2>&1 || hook_actual=$? + check_result "$label (installed hook)" "$expected" "$hook_actual" +} + +# Fixtures, assembled from adjacent literals so this file scans clean. +KEY="AKIA""0123456789ABCDEF" +PEM="-----BEGIN ""PRIVATE KEY-----" +AIZA="AIza$(head -c 35 /dev/zero | tr '\0' X)" + +# Must be caught (exit 1): the #1972 hole and the coverage this PR adds. +run_case "terraform resource with a real-shaped key" 1 a.tf \ + 'resource "aws_iam_access_key" "x" { key = "'"$KEY"'" }' +run_case "terraform var reference alongside a key" 1 b.tf \ + 'secret = var.x # '"$KEY" +run_case "go test const with a key" 1 c_test.go \ + 'const k = "'"$KEY"'"' +run_case "testdata fixture with a key" 1 testdata/creds.txt \ + 'key='"$KEY" +run_case "placeholder line with a key" 1 d.txt \ + 'placeholder '"$KEY" +run_case "untruncated PKCS8 body" 1 e.json \ + '"private_key": "'"$PEM"'\nMIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQC\n"' +run_case "corrected GCP API key range" 1 f.txt \ + "$AIZA" + +# Negative controls for the setup-script allowlist entries below, each +# closing one instance of the same class of hole (#1972 reintroduced at +# the scale of one file) that a narrower-but-still-wrong anchor leaves +# open. Both mutate the tracked copy of the setup script in place at its +# real path, since the allowlist entries are anchored on that path, and +# restore the pristine copy afterward so the later self-scan case below +# sees it intact. + +# A secret appended to a DIFFERENT git-secrets registration line in that +# file (the GCP one, not one of the three that legitimately self-match) +# must still be caught. A prefix-only entry anchored on just "git secrets +# --add '" would hide this. +run_case "secret appended to an unrelated registration line stays caught" 1 \ + scripts/setup-git-secrets.sh \ + "$(sed "s/# GCP API Key\$/# GCP API Key ${KEY}/" "$REPO_ROOT/scripts/setup-git-secrets.sh")" +cp "$REPO_ROOT/scripts/setup-git-secrets.sh" scripts/setup-git-secrets.sh + +# A secret appended to the END of one of the three self-matching lines +# THEMSELVES must also still be caught. An entry anchored on the path and +# pattern but missing a trailing $ would stop matching at the end of the +# pattern it names and let anything appended after it through, which is a +# narrower version of the same hole the entry above closes. +run_case "secret appended to a self-matching line itself stays caught" 1 \ + scripts/setup-git-secrets.sh \ + "$(sed "s/# PostgreSQL\$/# PostgreSQL ${KEY}/" "$REPO_ROOT/scripts/setup-git-secrets.sh")" +cp "$REPO_ROOT/scripts/setup-git-secrets.sh" scripts/setup-git-secrets.sh + +# Must scan clean (exit 0): what the tree contains, and what the allowlist +# must keep clean. +run_staged_case "setup script's own self-matching lines" 0 scripts/setup-git-secrets.sh +run_case "GCP type marker alone, no key material" 0 g.json \ + '{"type": "service_account", "project_id": "p"}' +run_case "truncated PEM in a struct literal" 0 i.go \ + 'PrivateKey: "'"$PEM"'\n...",' +run_case "truncated PEM in a JSON literal" 0 j.go \ + '"private_key": "'"$PEM"'\nMIIEvQIBADANBg...\n",' +run_case "PEM word fragments are not a real header" 0 k.txt \ + 'this is PRIVATE data +algo EC here +-----BEGIN CERTIFICATE-----' +run_staged_case ".gitallowed self-scan" 0 .gitallowed + +echo "$pass PASS, $fail FAIL" +[ "$fail" -eq 0 ] From f9c61b5c9451d6ff1bd3857bc87ae7784c15743c Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 28 Sep 2026 01:34:45 +0200 Subject: [PATCH 2/4] fix(scripts): verify hook files after install instead of trusting say() CodeRabbit review on this port: defining say() as a no-op (so this script's own error handling, not git-secrets' `say` bug, decides success) also means "git secrets --install -f" returning success no longer proves anything about whether the hook files were actually written -- say() is a no-op that returns success regardless of whether install_hook's write or chmod actually succeeded, so a broken hook (e.g. a permissions error) would be reported as installed. Verify what install_hook was supposed to do instead: after install, check that each of the three hooks git-secrets writes (commit-msg, pre-commit, prepare-commit-msg) exists, is executable, and contains its "git secrets -- -- \"$@\"" invocation (checking the *.d/git-secrets path when a hooks.d directory is in use, matching install_hook's own destination logic). Fail loudly if any hook fails the check. Verified: a normal install passes; corrupting a hook's content after install (simulating a partial write) is detected and reported. scripts/test-git-secrets-allowlist.sh remains 30/30 (it already installs via this same script and would have failed loudly here if the new check had a false positive). Co-Authored-By: claude-flow --- scripts/setup-git-secrets.sh | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/scripts/setup-git-secrets.sh b/scripts/setup-git-secrets.sh index 8a566e3..d25c3ae 100755 --- a/scripts/setup-git-secrets.sh +++ b/scripts/setup-git-secrets.sh @@ -58,6 +58,32 @@ else exit 1 fi +# The no-op say() above means "git secrets --install -f" returning success no +# longer proves anything: with say() removed the real signal was its exit +# status, but say() itself is a no-op that returns success regardless of +# whether install_hook actually wrote the file (e.g. a permissions error on +# chmod). Verify what the command was supposed to do instead: each hook file +# it writes exists, is executable, and contains the git-secrets invocation. +git_dir="$(git rev-parse --git-dir)" +hooks_ok=1 +for hook_spec in "commit-msg:commit_msg_hook" "pre-commit:pre_commit_hook" "prepare-commit-msg:prepare_commit_msg_hook"; do + hook_name="${hook_spec%%:*}" + hook_cmd="${hook_spec##*:}" + hook_path="${git_dir}/hooks/${hook_name}" + if [ -d "${git_dir}/hooks/${hook_name}.d" ]; then + hook_path="${git_dir}/hooks/${hook_name}.d/git-secrets" + fi + if [ ! -x "${hook_path}" ] || ! grep -qF "git secrets --${hook_cmd} -- \"\$@\"" "${hook_path}"; then + echo -e "${RED}✗ ${hook_name} hook missing, not executable, or doesn't invoke git-secrets: ${hook_path}${NC}" + hooks_ok=0 + fi +done +if [ "${hooks_ok}" -ne 1 ]; then + echo -e "${RED}✗ Hook verification failed; git-secrets would not actually run on commit${NC}" + exit 1 +fi +echo -e "${GREEN}✓ Verified all three hooks are installed and executable${NC}" + # Register AWS secret patterns echo "" echo "Registering AWS secret patterns..." From 81976ff0980f2348bf45fb39520ca033f2c74ffc Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 28 Sep 2026 01:37:56 +0200 Subject: [PATCH 3/4] fix(scripts): anchor the truncated-PEM allowlist entry, close a #1972 reopen CodeRabbit review on this port (cloud-commitments-platform#101): the truncated-PEM .gitallowed entry was unanchored, so a real secret spliced onto the same line -- appended after "..." or prepended before "-----BEGIN" -- still scanned clean, because .gitallowed suppresses the WHOLE "path:line:content" line if any allowed pattern matches anywhere in it, regardless of which prohibited pattern actually fired. That is the exact class of bug this branch exists to close, reintroduced in the one entry that didn't get the anchoring treatment the three self-matching script entries already had. Fixed by requiring the full contiguous header ("-----BEGIN PRIVAT[E] KEY-----", immediately after the opening quote) and pinning the tail with $ to only the closing punctuation this repo's own test fixtures use. Verified directly (real git-secrets binary, throwaway repo): a key appended after the truncation marker, a key prepended before the header, a key spliced between "BEGIN" and "PRIVATE KEY-----", and a key spliced right after "KEY-----" are all now caught (exit 1); both of this repo's legitimate truncated-PEM fixtures still scan clean (exit 0). scripts/test-git-secrets-allowlist.sh: 30/30 passing after the fix (it was already 30/30 before, since none of its existing fixtures exercised this particular splice -- the new coverage above is a manual adversarial check, not a change to the checked-in self-test). Co-Authored-By: claude-flow --- .gitallowed | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/.gitallowed b/.gitallowed index 9e4fb7d..9af54e8 100644 --- a/.gitallowed +++ b/.gitallowed @@ -78,7 +78,16 @@ ^scripts/setup-git-secrets\.sh:[0-9]+:git secrets --add 'mysq[l]://\[\^:\]\+:\[\^@\]\+@' # MySQL$ # Truncated PEM fixtures used by scripts/test-git-secrets-allowlist.sh (and -# by any test in this repo that needs a non-secret PEM-shaped string): a PEM -# header followed by "\n..." is never a key body. Written without "BEGIN" so -# this line itself does not trip the PEM detector. -PRIVATE KEY-----\\n(MIIEvQIBADANBg)?\.\.\. + +# Truncated PEM placeholder used by scripts/test-git-secrets-allowlist.sh +# (and by any fixture in this repo needing a non-secret PEM-shaped string): +# a PEM header immediately followed by "\n..." (optionally with a short +# base64 prefix) is never a real key body, so a line consisting of exactly +# that shape is safe. Anchored two ways so it cannot become the #1972 hole +# again: the header must be the whole quoted value (no room for a secret +# spliced in before "-----BEGIN" or between "BEGIN" and "PRIVATE KEY-----"), +# and the tail is pinned with $ to only the closing quote/comma this +# repo's fixtures actually use (no room for a secret appended after "..."). +# One letter of "PRIVATE" is bracketed for the same self-scan reason the +# block above explains -- do not tidy it back to a bare word. +"-----BEGIN PRIVAT[E] KEY-----\\n(MIIEvQIBADANBg)?\.\.\.(\\n)?",?$ From 403b075bfa477fe64ae45d42425b4ad562601f82 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 28 Sep 2026 02:22:58 +0200 Subject: [PATCH 4/4] fix(scripts): whole-line anchor the PEM allowlist entry, fix \s under git grep Independent adversarial review of this port found two further gaps past the previous two fix commits: 1. The truncated-PEM .gitallowed entry, even after being anchored to the quoted value's own start and end, was still only a SUBSTRING match within the scanner's whole "path:line:content" line. .gitallowed suppresses the entire line when ANY allowed pattern matches anywhere in it, regardless of which prohibited pattern fired, so a real secret in an earlier, unrelated statement on the same physical line as the allowed fixture -- `k := "AKIA..."; PrivateKey: "-----BEGIN PRIVATE KEY-----\n...",` -- was still suppressed. Fixed by anchoring the whole entry to the start of git-secrets' own "path:line:content" format (^[^:]+:[0-9]+:) so the key assignment must be the first thing on the line, closing the gap in both directions (before AND after the fixture) at once. Also dropped an orphan half-line comment left over from an earlier edit, and fixed the explanatory comment (which illustrated the attack by literally spelling out the PEM trigger, tripping the PEM detector on .gitallowed itself). 2. scripts/setup-git-secrets.sh's password/api_key/secret_key detectors used `\s` for whitespace. BSD/glibc grep's -E accepts `\s` as a GNU extension in some builds, but `git grep -E` (what git-secrets actually invokes to join and run every pattern) does not: `password = "..."` with a real space scanned clean under `git grep -E 'password\s*...'` and only matched once `\s` was replaced with the POSIX class `[[:space:]]`. This is the exact class of "detector never actually ran" bug #1972 already found twice (the PEM `--add` abort, the GCP regex making every scan exit 128); fixed the same way here since it's the same file and the same failure mode. Added three fixtures to scripts/test-git-secrets-allowlist.sh exercising both: a real-spaced password/api_key/secret_key line (must be caught), and a real-shaped key spliced before AND after an otherwise-legitimate truncated-PEM fixture on the same physical line (both must be caught). Verified: 36/36 assertions pass (30 previous + 6 new: 3 fixtures x 2 scan paths) against the real git-secrets binary. Filed a follow-up issue for two lower-risk residual items an independent review also raised (the bare numeric/UUID placeholder block's own lack of anchoring, and a dedicated DSN-detector coverage audit), scoped separately since neither has a demonstrated exploit and both need a deliberate design pass. Co-Authored-By: claude-flow --- .gitallowed | 28 ++++++++++++++++++--------- scripts/setup-git-secrets.sh | 6 +++--- scripts/test-git-secrets-allowlist.sh | 6 ++++++ 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/.gitallowed b/.gitallowed index 9af54e8..b764819 100644 --- a/.gitallowed +++ b/.gitallowed @@ -77,17 +77,27 @@ ^scripts/setup-git-secrets\.sh:[0-9]+:git secrets --add 'postgre[s]://\[\^:\]\+:\[\^@\]\+@' # PostgreSQL$ ^scripts/setup-git-secrets\.sh:[0-9]+:git secrets --add 'mysq[l]://\[\^:\]\+:\[\^@\]\+@' # MySQL$ -# Truncated PEM fixtures used by scripts/test-git-secrets-allowlist.sh (and - # Truncated PEM placeholder used by scripts/test-git-secrets-allowlist.sh # (and by any fixture in this repo needing a non-secret PEM-shaped string): # a PEM header immediately followed by "\n..." (optionally with a short -# base64 prefix) is never a real key body, so a line consisting of exactly -# that shape is safe. Anchored two ways so it cannot become the #1972 hole -# again: the header must be the whole quoted value (no room for a secret -# spliced in before "-----BEGIN" or between "BEGIN" and "PRIVATE KEY-----"), -# and the tail is pinned with $ to only the closing quote/comma this -# repo's fixtures actually use (no room for a secret appended after "..."). +# base64 prefix) is never a real key body, so a line consisting of EXACTLY +# a "PrivateKey:" or "private_key" assignment to that shape is safe. +# +# Anchored on the WHOLE scanner output line, not just the quoted value: +# .gitallowed suppresses a line if ANY allowed pattern matches anywhere in +# it, regardless of which prohibited pattern fired, so a real secret in an +# earlier, unrelated statement on the same physical line as this entry's +# fixture would otherwise be suppressed too, even though the entry has +# nothing to do with that secret (a fixture below exercises exactly this: +# a key assigned before the fixture, on the same scanner line, must still +# be caught -- note this comment must not spell the fixture out as a +# literal contiguous string either, for the same self-scan reason). +# ^[^:]+:[0-9]+: pins the match to the start of git-secrets' own +# "path:line:content" format, so the key assignment must be the first thing +# on the line (only leading whitespace allowed), and the trailing $ pins the +# tail to only the closing quote/comma this repo's fixtures actually use. +# Both ends are load-bearing -- do not loosen either back to an open match. +# # One letter of "PRIVATE" is bracketed for the same self-scan reason the # block above explains -- do not tidy it back to a bare word. -"-----BEGIN PRIVAT[E] KEY-----\\n(MIIEvQIBADANBg)?\.\.\.(\\n)?",?$ +^[^:]+:[0-9]+:[[:space:]]*("private_key"|PrivateKey):[[:space:]]*"-----BEGIN PRIVAT[E] KEY-----\\n(MIIEvQIBADANBg)?\.\.\.(\\n)?",?$ diff --git a/scripts/setup-git-secrets.sh b/scripts/setup-git-secrets.sh index d25c3ae..f27fa86 100755 --- a/scripts/setup-git-secrets.sh +++ b/scripts/setup-git-secrets.sh @@ -105,9 +105,9 @@ git secrets --add 'AIza[0-9A-Za-z_-]{35}' # GCP API git secrets --add 'DefaultEndpointsProtocol=https' # Azure Connection String # Generic secrets (require quoted values to avoid matching variable declarations) -git secrets --add 'password\s*[=:]\s*['\''"][^'\''"]{8,}' # Password with quoted value -git secrets --add 'api[_-]?key\s*[=:]\s*['\''"][^'\''"]{8,}' # API key with quoted value -git secrets --add 'secret[_-]?key\s*[=:]\s*['\''"][^'\''"]{8,}' # Secret key with quoted value +git secrets --add 'password[[:space:]]*[=:][[:space:]]*['\''"][^'\''"]{8,}' # Password with quoted value +git secrets --add 'api[_-]?key[[:space:]]*[=:][[:space:]]*['\''"][^'\''"]{8,}' # API key with quoted value +git secrets --add 'secret[_-]?key[[:space:]]*[=:][[:space:]]*['\''"][^'\''"]{8,}' # Secret key with quoted value git secrets --add 'BEGIN[[:space:]]((RSA|DSA|EC|OPENSSH|ENCRYPTED)[[:space:]])?PRIVATE[[:space:]]KEY-----' # PEM private keys # Database connection strings diff --git a/scripts/test-git-secrets-allowlist.sh b/scripts/test-git-secrets-allowlist.sh index df11945..b698c93 100755 --- a/scripts/test-git-secrets-allowlist.sh +++ b/scripts/test-git-secrets-allowlist.sh @@ -119,6 +119,12 @@ run_case "untruncated PKCS8 body" 1 e.json \ '"private_key": "'"$PEM"'\nMIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQC\n"' run_case "corrected GCP API key range" 1 f.txt \ "$AIZA" +run_case "password with real spaces around the separator" 1 n.txt \ + 'password = "hunter2hunter2"' +run_case "secret before an otherwise-allowed truncated PEM, same line" 1 l.go \ + 'k := "'"$KEY"'"; PrivateKey: "'"$PEM"'\n...",' +run_case "secret after an otherwise-allowed truncated PEM, same line" 1 m.go \ + '"private_key": "'"$PEM"'\nMIIEvQIBADANBg...\n'"$KEY"'"' # Negative controls for the setup-script allowlist entries below, each # closing one instance of the same class of hole (#1972 reintroduced at