From ecd085c52fbbf18c8e31659422629e11006ea6c5 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 12 Oct 2026 14:37:27 +0200 Subject: [PATCH 1/2] sec(scripts): replace bare .gitallowed placeholder entries with one anchored entry The 18 bare account-ID and UUID entries matched anywhere in the scanner's path:line:content line, exempting that line from every detector (a key or password beside 123456789012 scanned clean). Only tools/purchase_test.go trips the account-ID detector, so allow just that line, anchored on path and whole line. Self-test gains must-catch cases that fail on the old file. Closes #24 (part 1; DSN detector audit tracked separately) Co-Authored-By: Claude Sonnet 5.5 --- .gitallowed | 64 +++++++-------------------- CHANGELOG.md | 4 ++ scripts/test-git-secrets-allowlist.sh | 19 ++++++++ 3 files changed, 39 insertions(+), 48 deletions(-) diff --git a/.gitallowed b/.gitallowed index b764819..02edef8 100644 --- a/.gitallowed +++ b/.gitallowed @@ -1,54 +1,18 @@ -# git-secrets allowlist: regexes that should NOT trigger the AWS-secret scanner. +# git-secrets allowlist: regexes that should NOT trigger the scanner. # -# The scanner's `--register-aws` adds a 12-digit account-ID pattern that -# matches our test fixtures. The values below are obvious test placeholders -# (sequential 123456789012, all-same-digit blocks like 111111111111, and -# countdown patterns like 999888777666) — chosen specifically because they -# cannot be real customer accounts. Adding these is safer than disabling the -# AWS-account check entirely. +# Format: one regex per line, matched against the scanner's whole +# "path:line:content" output line (not just the file content), and a match +# exempts that line from EVERY detector, not only the one it was added for. +# So every entry must be anchored on the path (^path:[0-9]+:) and, where it +# names a fixed line, end in $. Never add a bare literal or keyword (#1972). # -# 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. +# The bare placeholder-digit and UUID entries that used to be here exempted +# any line containing them from every detector (#24); only one still matched a +# real scanner hit, so only that line is allowed, below. If another fixture +# trips the account-ID detector, add an entry anchored the same way. # -# 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 -210987654321 - -# All-same-digit blocks of 12 — used as table-row distinctions in fixtures. -# Listed explicitly rather than via a regex back-reference because not every -# git-secrets build supports back-refs in its allowlist regexes. -000000000000 -111111111111 -222222222222 -333333333333 -444444444444 -555555555555 -666666666666 -777777777777 -888888888888 -999999999999 - -# Group-block placeholders used in cmd/helpers_test.go variants and -# cmd/multi_service_helpers_test.go. -111222333444 -555666777888 -999888777666 -# Descending-step fixture added in #956 tests (store_postgres_pgxmock_test.go, -# handler_analytics_test.go, handler_history_test.go, handler_dashboard_test.go). -999988887777 -# Repeating-pair-block fixture used in handler_analytics_test.go, -# handler_inventory_test.go, store_postgres_pgxmock_test.go. -111122223333 - -# UUID-shaped account ID used in handler_accounts_test.go (synthetic, not a -# real subscription/account). -11111111-1111-1111-1111-111111111111 +# DO NOT add a real account ID or secret here. If one lands in the repo, +# rotate it and treat the leak seriously instead of silencing the scanner. # scripts/setup-git-secrets.sh registers its detectors as literal regex text # and three of them (Azure connection string, PostgreSQL and MySQL DSN @@ -101,3 +65,7 @@ # 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. ^[^:]+:[0-9]+:[[:space:]]*("private_key"|PrivateKey):[[:space:]]*"-----BEGIN PRIVAT[E] KEY-----\\n(MIIEvQIBADANBg)?\.\.\.(\\n)?",?$ + +# The only fixture the account-ID detector flags: a synthetic placeholder +# account ID in a struct literal. Anchored on the path and the whole line. +^tools/purchase_test\.go:[0-9]+:[[:space:]]*Account:[[:space:]]+"123456789012",$ diff --git a/CHANGELOG.md b/CHANGELOG.md index 52bb99b..f3615a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Changed +- `.gitallowed` no longer allow-lists bare placeholder account IDs and UUIDs, + which exempted any line containing them from every detector (#24). The one + fixture the account-ID detector flags is allowed by a path- and line-anchored + entry, and the allowlist self-test covers the bypass. - The purchase audit log records a new status, `unknown`, for a purchase that failed with `common.ErrOutcomeUnknown` (a lost EC2 or Redshift response: the commitment may exist). The stderr line reads `OUTCOME UNKNOWN`. Consumers diff --git a/scripts/test-git-secrets-allowlist.sh b/scripts/test-git-secrets-allowlist.sh index b698c93..afb1ca1 100755 --- a/scripts/test-git-secrets-allowlist.sh +++ b/scripts/test-git-secrets-allowlist.sh @@ -126,6 +126,19 @@ run_case "secret before an otherwise-allowed truncated PEM, same line" 1 l.go \ run_case "secret after an otherwise-allowed truncated PEM, same line" 1 m.go \ '"private_key": "'"$PEM"'\nMIIEvQIBADANBg...\n'"$KEY"'"' +# Bare placeholder digits/UUID must not exempt a line from the other detectors +# (#24): each line below carries a fixture that is caught on its own. +run_case "key beside a placeholder account ID" 1 o.txt \ + 'acct 123456789012 key '"$KEY" +run_case "password beside a placeholder account ID" 1 p.txt \ + 'account''_id = "123456789012" // pass''word = "hunter2hunter2"' +run_case "GCP key beside a placeholder UUID" 1 q.txt \ + "$AIZA 11111111-1111-1111-1111-111111111111" +# Live-detector control: a non-placeholder account ID is flagged, so the +# clean cases below are not passing because the detector is dead. +run_case "account ID assignment is still detected" 1 r.txt \ + 'account''_id="123456789013"' + # 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 @@ -166,6 +179,12 @@ run_case "PEM word fragments are not a real header" 0 k.txt \ 'this is PRIVATE data algo EC here -----BEGIN CERTIFICATE-----' +run_case "the one placeholder account ID fixture scans clean" 0 tools/purchase_test.go \ + ' Acc''ount: "123456789012",' +run_case "placeholder fixture line with a trailing key is still caught" 1 tools/purchase_test.go \ + ' Acc''ount: "123456789012", k="'"$KEY"'"' +run_case "Archera env name and test key scan clean" 0 u.go \ + 'EnvArcheraAPIKey = "ARCHERA_API_KEY"; testArcheraKey = "test-archera-key-0000"' run_staged_case ".gitallowed self-scan" 0 .gitallowed echo "$pass PASS, $fail FAIL" From f625d67bba3487a2a8752e31d4ed872999578c21 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 12 Oct 2026 14:51:00 +0200 Subject: [PATCH 2/2] test(scripts): guard .gitallowed anchoring structurally and in file mode Fail when any .gitallowed entry is not anchored ^...:[0-9]+:...$ (proven against each of the 18 removed bare entries), and exercise the purchase_test.go entry through a working-tree scan: it fails without the entry or in any other path. Co-Authored-By: Claude Sonnet 5.5 --- scripts/test-git-secrets-allowlist.sh | 53 +++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/scripts/test-git-secrets-allowlist.sh b/scripts/test-git-secrets-allowlist.sh index afb1ca1..4c6c868 100755 --- a/scripts/test-git-secrets-allowlist.sh +++ b/scripts/test-git-secrets-allowlist.sh @@ -187,5 +187,58 @@ run_case "Archera env name and test key scan clean" 0 u.go \ 'EnvArcheraAPIKey = "ARCHERA_API_KEY"; testArcheraKey = "test-archera-key-0000"' run_staged_case ".gitallowed self-scan" 0 .gitallowed +# Structural guard (#24): every entry must be anchored at both ends and pin +# the path:line: prefix of git-secrets' output, so no entry can exempt a line +# that merely contains some literal. Checked on the real .gitallowed, and +# proven to reject each bare entry this file used to carry. +allowlist_unanchored() { + grep -vE '^[[:space:]]*(#|$)' "$1" | grep -vE '^\^.*:\[0-9\]\+:.*\$$' || true +} +unanchored=$(allowlist_unanchored "$REPO_ROOT/.gitallowed") +if [ -n "$unanchored" ]; then + fail=$((fail + 1)) + echo "FAIL: .gitallowed has entries not anchored as ^...:[0-9]+:...\$: $unanchored" >&2 +else + pass=$((pass + 1)) +fi +for bare in 123456789012 210987654321 000000000000 111111111111 222222222222 \ + 333333333333 444444444444 555555555555 666666666666 777777777777 \ + 888888888888 999999999999 111222333444 555666777888 999888777666 \ + 999988887777 111122223333 11111111-1111-1111-1111-111111111111; do + { cat "$REPO_ROOT/.gitallowed"; printf '%s\n' "$bare"; } >"$tmp/mutated" + if [ -n "$(allowlist_unanchored "$tmp/mutated")" ]; then + pass=$((pass + 1)) + else + fail=$((fail + 1)) + echo "FAIL: structural guard accepts bare entry $bare" >&2 + fi +done + +# Working-tree (file-mode) scan, the path `pre-commit` takes, where the +# account-ID detector does fire. The one allowed fixture must scan clean only +# because of its anchored entry: removing the entry, or widening its path, +# must make it fail. +file_mode_scan() { + local relpath=$1 content=$2 actual=0 + mkdir -p "$(dirname "$relpath")" + printf '%s\n' "$content" >"$relpath" + git secrets --scan "$relpath" >/dev/null 2>&1 || actual=$? + rm -f "$relpath" + return "$actual" +} +FIXTURE=' Acc''ount: "123456789012",' +actual=0; file_mode_scan tools/purchase_test.go "$FIXTURE" || actual=$? +check_result "anchored account-ID fixture (file mode)" 0 "$actual" +actual=0; file_mode_scan tools/other_test.go "$FIXTURE" || actual=$? +check_result "same fixture in another path (file mode)" 1 "$actual" +cp .gitallowed "$tmp/gitallowed.real" +grep -v 'purchase_test' "$tmp/gitallowed.real" >.gitallowed +actual=0; file_mode_scan tools/purchase_test.go "$FIXTURE" || actual=$? +check_result "fixture without its entry (file mode)" 1 "$actual" +sed 's#^\^tools/purchase_test\\\.go:#^[^:]+:#' "$tmp/gitallowed.real" >.gitallowed +actual=0; file_mode_scan tools/other_test.go "$FIXTURE" || actual=$? +check_result "sensitivity: a widened entry path would allow another file (file mode)" 0 "$actual" +cp "$tmp/gitallowed.real" .gitallowed + echo "$pass PASS, $fail FAIL" [ "$fail" -eq 0 ]