From 998bf7a8d46ede8fee1143828bee05e1828494f8 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 8 Sep 2026 10:03:54 +0200 Subject: [PATCH 1/9] fix(scripts): make the git-secrets GCP API-key pattern a valid regex The GCP API-key detector used the bracket range [0-9A-Za-z-_], which BSD regex (Apple git), glibc regex (git 2.43) and GNU grep 3.12 all reject as an invalid character range (the hyphen between z and _ is read as a range boundary). git-secrets joins every registered pattern into a single `git grep -E` call, so once this pattern is registered, every subsequent scan on that clone exits 128, including the pre-commit hook. The range only needs reordering so the hyphen is literal: [0-9A-Za-z_-]. Present since the script was added in c7d3c8c49. Tracked separately as #2080. Co-Authored-By: claude-flow Claude-Session: https://claude.ai/code/session_01Fu9uWjxtDFx5HDKeMRt1jC --- scripts/setup-git-secrets.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/setup-git-secrets.sh b/scripts/setup-git-secrets.sh index 16cbf763..df9d45dc 100755 --- a/scripts/setup-git-secrets.sh +++ b/scripts/setup-git-secrets.sh @@ -54,7 +54,7 @@ git secrets --add 'aws(.{0,20})?['\''"][0-9a-zA-Z/+]{40}['\''"]' # AWS Cre # 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 From 2082c506a4d9a512ee49964bb84b0dbaf45beb47 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 8 Sep 2026 10:15:13 +0200 Subject: [PATCH 2/9] sec(scripts): replace keyword git-secrets allowlist with literal .gitallowed entries git-secrets applies allowed patterns with `grep -Ev` against the scanner's whole "path:line:content" output line, not just the file content. The 20 keyword entries the setup script registered (var\., resource\s, _test\.go, placeholder, ...) therefore whitelisted every line containing that keyword anywhere in the tree, including a line carrying a real access key. Measured against the tree with the script's own detectors registered, none of the 20 entries suppressed a legitimate false positive; the real false positives were 22 lines mentioning the GCP key-file "type" marker, three lines where the script matches its own detector registrations, and three truncated PEM fixtures in a test file. The GCP marker detector (type.*service_account) is dropped rather than narrowed: the benign literal is byte-identical to the one in a real key file, so no regex can tell them apart. The real secret in a GCP service-account JSON is the private_key PEM, which is now covered by the corrected PEM detector. .gitallowed is now the single allowlist and gets three literal entries: a path-anchored entry for the script matching its own registration lines, a truncated-PEM entry for the test fixtures, and a Go format-string DSN entry (fmt.Sprintf with "%s" in the credential position) guarding a recurring benign idiom that scripts/ test-git-secrets-allowlist.sh exercises directly, even though no current file in the tree matches it literally. Its header is corrected: path-scoping is possible via a "^path:[0-9]+:" anchor, contrary to what it claimed. Closes #1972 Co-Authored-By: claude-flow Claude-Session: https://claude.ai/code/session_01Fu9uWjxtDFx5HDKeMRt1jC --- .gitallowed | 22 ++++++++++++++++++-- CHANGELOG.md | 5 +++++ scripts/setup-git-secrets.sh | 40 +++++------------------------------- 3 files changed, 30 insertions(+), 37 deletions(-) diff --git a/.gitallowed b/.gitallowed index 6ff918dd..22202d39 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,18 @@ # 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) +# match their own registration line. Anchored on the path AND the command so +# this cannot cover any file but the script. +^scripts/setup-git-secrets\.sh:[0-9]+:git secrets --add ' +# Truncated PEM fixtures in cmd/configure_test.go: 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)?\.\.\. +# Go format-string DSN (fmt.Sprintf with "%s" in the credential position, +# e.g. building a connection string from parsed config). Not currently +# present in the tree, but a recurring, benign Go idiom the DSN detectors +# must not flag; scripts/test-git-secrets-allowlist.sh exercises it. +postgres://%s:%s@ diff --git a/CHANGELOG.md b/CHANGELOG.md index 643c1df6..5466ca03 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 Terraform and test-file lines. 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 df9d45dc..20020ee2 100755 --- a/scripts/setup-git-secrets.sh +++ b/scripts/setup-git-secrets.sh @@ -53,7 +53,6 @@ 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 # Azure patterns @@ -63,46 +62,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 ' Date: Tue, 8 Sep 2026 10:22:04 +0200 Subject: [PATCH 3/9] test(scripts): self-test for the git-secrets allowlist Runs the real scripts/setup-git-secrets.sh and .gitallowed in a throwaway repo, through the pre-commit hook scan path, and asserts both directions: fixtures shaped like the #1972 hole (a real access key sitting next to a keyword the old allowlist whitelisted) still get caught, and the tree's known false positives still scan clean. HOME is isolated per case so the AWS credential provider and the developer's global git config never reach the test. Fixtures are assembled at runtime from adjacent string literals so this file's own source never contains a scannable token. Wires a new CI job that runs it, mirroring the existing scripts/test-* self-test jobs. Not added to ci-success yet, so it cannot block unrelated PRs while it beds in. Co-Authored-By: claude-flow Claude-Session: https://claude.ai/code/session_01Fu9uWjxtDFx5HDKeMRt1jC --- .github/workflows/ci.yml | 25 ++++++ scripts/test-git-secrets-allowlist.sh | 121 ++++++++++++++++++++++++++ 2 files changed, 146 insertions(+) create mode 100755 scripts/test-git-secrets-allowlist.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e1254cdb..26525c01 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -868,6 +868,31 @@ jobs: - name: Run guard script self-tests run: bash scripts/test-gcp-secret-scope.sh + # 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. Not + # part of ci-success yet, so it cannot block unrelated PRs while it beds in. + git-secrets-allowlist: + name: git-secrets allowlist self-test + runs-on: ubuntu-latest + permissions: + contents: read + + steps: + - name: Checkout code + uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 + with: + persist-credentials: false + + - name: Install git-secrets + run: | + set -euo pipefail + git clone --depth 1 --branch 1.3.0 https://github.com/awslabs/git-secrets.git /tmp/git-secrets + sudo make -C /tmp/git-secrets install + + - name: Run allowlist self-tests + run: bash scripts/test-git-secrets-allowlist.sh + # Assert that the ECR repository selector used by destroy-fargate-dev.yml and # cleanup-staging.yml picks the repository each state owns and nothing else. # The consumers force-delete what the selector prints, so both directions are diff --git a/scripts/test-git-secrets-allowlist.sh b/scripts/test-git-secrets-allowlist.sh new file mode 100755 index 00000000..750e7e53 --- /dev/null +++ b/scripts/test-git-secrets-allowlist.sh @@ -0,0 +1,121 @@ +#!/bin/bash +# Self-test for the git-secrets allowlist (#1972). Exercises the real +# scripts/setup-git-secrets.sh and .gitallowed in a throwaway repo, through +# the pre-commit hook scan path, in both directions: fixtures that must be +# caught, and fixtures that must scan clean. +# +# Fixture safety: every positive fixture below is a secret-shaped string +# that this script itself, and the repo's detect-private-key / git-secrets +# pre-commit hooks, would flag if it appeared as a contiguous literal. Each +# is assembled at runtime from adjacent string literals so this file's own +# source never contains a scannable token. Where a fixture shows \n below, +# it is the two literal characters backslash and n, as in a Go string +# literal, never an actual newline. +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" + +if ! command -v git-secrets >/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 + +pass=0 +fail=0 + +# Writes $content to $relpath, stages it, scans it through the hook path, +# compares the 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=$? + git rm -q --cached "$relpath" + rm -f "$relpath" + if [ "$actual" -eq "$expected" ]; then + pass=$((pass + 1)) + else + fail=$((fail + 1)) + echo "FAIL: $label (expected exit $expected, got $actual)" >&2 + fi +} + +# 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=$? + if [ "$actual" -eq "$expected" ]; then + pass=$((pass + 1)) + else + fail=$((fail + 1)) + echo "FAIL: $label (expected exit $expected, got $actual)" >&2 + fi +} + +# 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" + +# 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 "Go format-string DSN" 0 h.go \ + '"postgres://%s:%s@%s:%d/%s?sslmode=%s",' +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 e48971cac643acd12c08657624bd3673b6624192 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 8 Sep 2026 11:15:59 +0200 Subject: [PATCH 4/9] fix(scripts): install git-secrets hooks despite a Linux-only say bug The new "git-secrets allowlist self-test" CI job (added on this branch) fails on ubuntu-latest with: /usr/local/bin/git-secrets: line 208: say: command not found Failed to install git hooks git-secrets 1.3.0's install_hook() writes the hook file, chmods it, and only then reports success via a bare `say` call it never defines as a function anywhere in the script (confirmed against the 1.3.0 source; still true on the current release, the latest tag). On macOS that name resolves to /usr/bin/say, the text-to-speech binary, which exits 0 and makes `git secrets --install -f` look like a clean success by accident. On Linux there is no such binary, so it's "command not found" (exit 127), and that becomes the exit status of `git secrets --install -f` even though every hook file was already written and made executable correctly beforehand. Reproduced on both platforms: the commit-msg, pre-commit, and prepare-commit-msg hook files exist with correct content after the "failed" Linux install. Defines `say` as a no-op and exports it before calling `git secrets --install -f`, so the exported function is visible to git-secrets' own bash process on both platforms. `say` is used in exactly this one place upstream, so this can't mask any other diagnostic. Left `scripts/setup-git-secrets.sh`'s own error message as the accurate signal it now is: it only fires when hook installation actually fails. Co-Authored-By: claude-flow Claude-Session: https://claude.ai/code/session_01Fu9uWjxtDFx5HDKeMRt1jC --- scripts/setup-git-secrets.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/scripts/setup-git-secrets.sh b/scripts/setup-git-secrets.sh index 20020ee2..9a394d3d 100755 --- a/scripts/setup-git-secrets.sh +++ b/scripts/setup-git-secrets.sh @@ -30,6 +30,19 @@ 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 it never defines as a function. On macOS +# that resolves to /usr/bin/say (the text-to-speech binary) and exits 0 by +# accident; on Linux there is no such binary, 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}" From 38a9dc5bd6c36cf91ee1d0200c7c3b75cc830d8c Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 8 Sep 2026 12:09:04 +0200 Subject: [PATCH 5/9] fix(scripts): narrow git-secrets allowlist, pin the tag SHA, test the real hook CodeRabbit found three real problems in the git-secrets allowlist PR. The .gitallowed entry that suppressed setup-git-secrets.sh's own self-matching detector registrations was anchored on the command prefix ("git secrets --add '"), not the full added pattern. That whitelists a secret appended to ANY registration line in that file, not just the three lines that legitimately self-match (Azure, PostgreSQL and MySQL DSN; verified empirically that MongoDB's does not). That is the exact #1972 hole this allowlist exists to close, reintroduced at the scale of one file. Replaced the single prefix entry with three entries anchored on the full literal pattern, added a negative fixture proving a secret appended to a different registration line (the GCP one) still gets caught, and verified by measurement that a whole-tree scan produces byte-identical output before and after the narrowing (the only residual is the pre-existing, already-tracked #2030 AWS-secret-key false positive, unrelated to this change). Narrowing surfaced a second, self-inflicted problem: .gitallowed's own three new entries reproduce the literal trigger text they exist to suppress (e.g. "DefaultEndpointsProtocol=https", and a "postgres://...@" span the DSN detector's permissive character classes still match), so .gitallowed started failing its own self-scan. The old prefix-only entry never had this problem because it didn't spell out any trigger text. Fixed by replacing one letter of each entry's trigger substring with a single-character bracket expression (e.g. "Protoco[l]"), which is a no-op for regex matching but breaks the contiguous literal span, so the entries no longer need to allow-list themselves. The explanatory comment above them is written to avoid spelling those spans out too, for the same reason. The CI install step clones git-secrets by the "1.3.0" tag, which is mutable. Verified independently via `git ls-remote --tags` against the upstream repo that it currently resolves to ad82d68ee924906a0401dfd48de5057731a9bc84, then added a check in both the new git-secrets-allowlist job and the existing pre-commit.yml install step (which has the identical unpinned clone) that asserts the cloned HEAD matches before trusting it enough to `sudo make install`. scripts/test-git-secrets-allowlist.sh called `git secrets --scan --cached` directly, bypassing the installed pre-commit hook and its own file-selection logic entirely. Now every case also invokes the installed hook (which recomputes its file list from the index rather than accepting args, so it is called with none) and checks its exit code against the same expectation, so a hook/direct-scan disagreement surfaces as its own labeled failure instead of going unnoticed. Verified on macOS (git-secrets via homebrew) and in a Debian bookworm container (git-secrets built from the pinned SHA the same way CI does): 30 PASS, 0 FAIL on both. Co-Authored-By: claude-flow Claude-Session: https://claude.ai/code/session_01Fu9uWjxtDFx5HDKeMRt1jC --- .gitallowed | 26 +++++++++-- .github/workflows/ci.yml | 11 +++++ .github/workflows/pre-commit.yml | 21 +++++++-- scripts/test-git-secrets-allowlist.sh | 67 ++++++++++++++++++++------- 4 files changed, 98 insertions(+), 27 deletions(-) diff --git a/.gitallowed b/.gitallowed index 22202d39..e4a1feb4 100644 --- a/.gitallowed +++ b/.gitallowed @@ -50,11 +50,27 @@ # 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) -# match their own registration line. Anchored on the path AND the command so -# this cannot cover any file but the script. -^scripts/setup-git-secrets\.sh:[0-9]+:git secrets --add ' +# 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 full added pattern, not just the "git secrets --add '" +# command prefix -- a prefix-only anchor would whitelist a secret appended +# to ANY registration line in this file, which is exactly the #1972 hole +# this allowlist exists to close, reintroduced at the scale of one file. +# +# 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' +^scripts/setup-git-secrets\.sh:[0-9]+:git secrets --add 'postgre[s]://\[\^:\]\+:\[\^@\]\+@' +^scripts/setup-git-secrets\.sh:[0-9]+:git secrets --add 'mysq[l]://\[\^:\]\+:\[\^@\]\+@' # Truncated PEM fixtures in cmd/configure_test.go: a PEM header followed by # "\n..." is never a key body. Written without "BEGIN" so this line itself # does not trip the PEM detector. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 26525c01..3614c451 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -885,9 +885,20 @@ jobs: persist-credentials: false - name: Install git-secrets + # The 1.3.0 tag is mutable in principle, so clone it and then 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`. + 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 - name: Run allowlist self-tests diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index 8e91b95e..47cd55a4 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -172,14 +172,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 '.' || { diff --git a/scripts/test-git-secrets-allowlist.sh b/scripts/test-git-secrets-allowlist.sh index 750e7e53..fb835899 100755 --- a/scripts/test-git-secrets-allowlist.sh +++ b/scripts/test-git-secrets-allowlist.sh @@ -1,8 +1,9 @@ #!/bin/bash # Self-test for the git-secrets allowlist (#1972). Exercises the real -# scripts/setup-git-secrets.sh and .gitallowed in a throwaway repo, through -# the pre-commit hook scan path, in both directions: fixtures that must be -# caught, and fixtures that must scan clean. +# scripts/setup-git-secrets.sh and .gitallowed in a throwaway repo, both +# directly via `git secrets --scan --cached` and through the installed +# pre-commit hook, in both directions: fixtures that must be caught, and +# fixtures that must scan clean. # # Fixture safety: every positive fixture below is a secret-shaped string # that this script itself, and the repo's detect-private-key / git-secrets @@ -42,11 +43,34 @@ 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 -# Writes $content to $relpath, stages it, scans it through the hook path, -# compares the exit code to $expected, then unstages and removes it. +# 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")" @@ -54,14 +78,12 @@ run_case() { 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" - if [ "$actual" -eq "$expected" ]; then - pass=$((pass + 1)) - else - fail=$((fail + 1)) - echo "FAIL: $label (expected exit $expected, got $actual)" >&2 - fi } # Same as run_case, for a file already on disk (the copied setup script and @@ -71,12 +93,10 @@ run_staged_case() { git add "$relpath" local actual=0 git secrets --scan --cached "$relpath" >/dev/null 2>&1 || actual=$? - if [ "$actual" -eq "$expected" ]; then - pass=$((pass + 1)) - else - fail=$((fail + 1)) - echo "FAIL: $label (expected exit $expected, got $actual)" >&2 - fi + 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. @@ -100,6 +120,19 @@ run_case "untruncated PKCS8 body" 1 e.json \ run_case "corrected GCP API key range" 1 f.txt \ "$AIZA" +# Negative control for the setup-script allowlist entries below: 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 too -- the exact #1972 hole, reintroduced at the scale of one +# file. Mutates the tracked copy of the setup script in place at its real +# path, since the allowlist entries are anchored on that path; restores the +# pristine copy afterward so the later self-scan case below sees it intact. +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 + # 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 From 2b167f3ef6d1642f6de95eb8621c8791e548e9a9 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 8 Sep 2026 13:08:23 +0200 Subject: [PATCH 6/9] fix(scripts): close the remaining allowlist gaps and wire up ci-success Adversarial review found the "entry broader than its false positive" pattern still present twice in the entries the previous commit narrowed, plus two smaller gaps. Delete the .gitallowed entry for the Go format-string DSN idiom (postgres://%s:%s@) and its test fixture. Measured what it actually suppresses across the whole tree: only the PR's own fixture. The real idiom in the tree is "postgresql://%s:%s@..." in internal/testutil/postgres.go, which the postgres:// detector does not match anyway (the "ql" breaks the required literal). The entry guarded nothing that exists, and the fixture existed only to justify the entry. It also widened matching: a real key appended to a line shaped like "postgres://%s:%s@%s/db", "" scanned clean. If the idiom ever lands in the tree, a path-anchored entry can be added then. The three entries narrowed onto scripts/setup-git-secrets.sh's own self-matching registration lines were anchored on the path and the added pattern, but not the trailing comment, and had no trailing $. That leaves the anchor open at the end, so a key appended to the END of one of those three lines themselves (not a different line) scanned clean. Pinned the whole line, comment included, with $, for all three entries. Added a fixture that appends a key to the end of the PostgreSQL line and confirmed it fails against the pre-fix entries (exit 0, wrongly clean) and passes against the fix (exit 1, caught). Added the new git-secrets-allowlist job to ci-success's needs list. All of its sibling guard self-test jobs were already listed; this one wasn't, so a regression in it could not block anything. It has already passed on Linux on this PR, so the "not required while it beds in" rationale no longer applies. Corrected the root-cause comment on the `say` shim: `say` was never git-secrets' 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, in git's own history). 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. So this is breakage from a git upgrade, not a git-secrets regression; a git-secrets install on an older git, or on macOS where /usr/bin/say happens to answer to the same name, was never affected. The shim remains the correct fix either way. Verified on macOS (git-secrets via homebrew, shellcheck clean) and in a Debian bookworm container (git-secrets built from the pinned SHA the same way CI does, shellcheck clean): 30 PASS, 0 FAIL on both, same count as before since one case was removed and one was added. Measured the whole-tree scan before and after these changes: byte-identical output (the only residual is the pre-existing, already-tracked #2030 AWS-secret-key false positive). Co-Authored-By: claude-flow Claude-Session: https://claude.ai/code/session_01Fu9uWjxtDFx5HDKeMRt1jC --- .gitallowed | 24 ++++++++++---------- .github/workflows/ci.yml | 4 ++-- scripts/setup-git-secrets.sh | 23 ++++++++++++------- scripts/test-git-secrets-allowlist.sh | 32 ++++++++++++++++++--------- 4 files changed, 51 insertions(+), 32 deletions(-) diff --git a/.gitallowed b/.gitallowed index e4a1feb4..056c5158 100644 --- a/.gitallowed +++ b/.gitallowed @@ -54,10 +54,15 @@ # 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 full added pattern, not just the "git secrets --add '" -# command prefix -- a prefix-only anchor would whitelist a secret appended -# to ANY registration line in this file, which is exactly the #1972 hole -# this allowlist exists to close, reintroduced at the scale of one file. +# 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 @@ -68,15 +73,10 @@ # 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' -^scripts/setup-git-secrets\.sh:[0-9]+:git secrets --add 'postgre[s]://\[\^:\]\+:\[\^@\]\+@' -^scripts/setup-git-secrets\.sh:[0-9]+:git secrets --add 'mysq[l]://\[\^:\]\+:\[\^@\]\+@' +^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 in cmd/configure_test.go: 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)?\.\.\. -# Go format-string DSN (fmt.Sprintf with "%s" in the credential position, -# e.g. building a connection string from parsed config). Not currently -# present in the tree, but a recurring, benign Go idiom the DSN detectors -# must not flag; scripts/test-git-secrets-allowlist.sh exercises it. -postgres://%s:%s@ diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3614c451..c5e48117 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -870,8 +870,7 @@ jobs: # 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. Not - # part of ci-success yet, so it cannot block unrelated PRs while it beds in. + # caught and that the documented false positives still scan clean. git-secrets-allowlist: name: git-secrets allowlist self-test runs-on: ubuntu-latest @@ -1088,6 +1087,7 @@ jobs: - azure-role-parity - aws-iam-parity - gcp-secret-scope + - git-secrets-allowlist - ecr-delete-selection - rds-deletion-protection-scope - cloud-sql-delete-scope diff --git a/scripts/setup-git-secrets.sh b/scripts/setup-git-secrets.sh index 9a394d3d..8a566e36 100755 --- a/scripts/setup-git-secrets.sh +++ b/scripts/setup-git-secrets.sh @@ -32,14 +32,21 @@ 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 it never defines as a function. On macOS -# that resolves to /usr/bin/say (the text-to-speech binary) and exits 0 by -# accident; on Linux there is no such binary, 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. +# 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 diff --git a/scripts/test-git-secrets-allowlist.sh b/scripts/test-git-secrets-allowlist.sh index fb835899..df119457 100755 --- a/scripts/test-git-secrets-allowlist.sh +++ b/scripts/test-git-secrets-allowlist.sh @@ -120,26 +120,38 @@ run_case "untruncated PKCS8 body" 1 e.json \ run_case "corrected GCP API key range" 1 f.txt \ "$AIZA" -# Negative control for the setup-script allowlist entries below: 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 too -- the exact #1972 hole, reintroduced at the scale of one -# file. Mutates the tracked copy of the setup script in place at its real -# path, since the allowlist entries are anchored on that path; restores the -# pristine copy afterward so the later self-scan case below sees it intact. +# 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 "Go format-string DSN" 0 h.go \ - '"postgres://%s:%s@%s:%d/%s?sslmode=%s",' 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 \ From eae8ea2616cea5f5e1754d1ad3bf5d6b24cf2a4b Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 28 Sep 2026 01:11:40 +0200 Subject: [PATCH 7/9] 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 056c5158..435d8cc7 100644 --- a/.gitallowed +++ b/.gitallowed @@ -76,7 +76,16 @@ ^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 in cmd/configure_test.go: 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 84aa8070d2c46c3c1e1e8d2896bb5e9f44a7b8db Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 28 Sep 2026 01:34:38 +0200 Subject: [PATCH 8/9] 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 8a566e36..d25c3ae6 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 539778d2c9c6677a2ac0eb94dfe73a38ce8e8242 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 28 Sep 2026 02:22:49 +0200 Subject: [PATCH 9/9] 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 | 26 +++++++++++++++++++------- scripts/setup-git-secrets.sh | 6 +++--- scripts/test-git-secrets-allowlist.sh | 6 ++++++ 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/.gitallowed b/.gitallowed index 435d8cc7..b7648197 100644 --- a/.gitallowed +++ b/.gitallowed @@ -80,12 +80,24 @@ # 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 d25c3ae6..f27fa86a 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 df119457..b698c938 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