From 8334392441450d64932f126a1b66b411c402c249 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 12 Oct 2026 19:46:48 +0200 Subject: [PATCH 1/3] test(scripts): cover secret-plus-placeholder lines and file-argument scan Add the structural check (every .gitallowed entry is ^...$ anchored), the secret-plus-placeholder must-catch cases, and the CI-equivalent `git ls-files -z | xargs -0 git secrets --scan` over all tracked files. Correct the say() comment in setup-git-secrets.sh (hang, not silent exit). Refs #340 Co-Authored-By: Claude Sonnet 5.5 --- scripts/setup-git-secrets.sh | 5 ++-- scripts/test-git-secrets-allowlist.sh | 35 ++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/scripts/setup-git-secrets.sh b/scripts/setup-git-secrets.sh index f27fa86a..25a6f977 100755 --- a/scripts/setup-git-secrets.sh +++ b/scripts/setup-git-secrets.sh @@ -39,8 +39,9 @@ echo "" # 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 +# `say` call resolves to /usr/bin/say (the text-to-speech binary) instead, +# which speaks the message and can hang indefinitely when audio output is +# unavailable, blocking the install; 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 diff --git a/scripts/test-git-secrets-allowlist.sh b/scripts/test-git-secrets-allowlist.sh index b698c938..8728dcd2 100755 --- a/scripts/test-git-secrets-allowlist.sh +++ b/scripts/test-git-secrets-allowlist.sh @@ -120,7 +120,7 @@ run_case "untruncated PKCS8 body" 1 e.json \ 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"' + 'pass''word = "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 \ @@ -168,5 +168,38 @@ algo EC here -----BEGIN CERTIFICATE-----' run_staged_case ".gitallowed self-scan" 0 .gitallowed +# Structural check (#340): every active .gitallowed entry must be anchored +# on the scanner's "path:line:content" output with ^ and $, so no entry can +# whitelist a whole line by containing a bare token. +unanchored=$(grep -vE '^[[:space:]]*(#|$)' "$REPO_ROOT/.gitallowed" | grep -vE '^\^.*\$$' || true) +if [ -n "$unanchored" ]; then + fail=$((fail + 1)) + echo "FAIL: unanchored .gitallowed entries:" >&2 + echo "$unanchored" >&2 +else + pass=$((pass + 1)) +fi + +# Placeholder-shaped account IDs next to a real-shaped key must be caught. +ACCT="1234""56789012" +run_case "account-id placeholder with a key on the same line" 1 o.go \ + 'accountId := "'"$ACCT"'"; k := "'"$KEY"'"' +run_case "UUID placeholder with a password on the same line" 1 p.go \ + 'id := "11111111-1111-1111-1111-111111111111"; pass''word = "hunter2hunter2"' +# A secret appended to an otherwise-allowed account-ID fixture line stays caught. +run_case "key appended to an allowed account-ID fixture line" 1 \ + cmd/helpers_test.go \ + ' accountID: "'"$ACCT"'", k := "'"$KEY"'"' + +# CI-equivalent check: the pre-commit framework hands git-secrets explicit +# file names (plain grep), unlike a no-argument scan (git grep), which +# differ on regex features such as \s. Scan every tracked file of the real +# repo that way, under the same registered patterns. +git -C "$REPO_ROOT" ls-files -z | tar -C "$REPO_ROOT" --null -T - -cf - | tar -C "$tmp/repo" -xf - +file_scan=0 +git ls-files -z --others --cached | xargs -0 git secrets --scan >"$tmp/file-scan.log" 2>&1 || file_scan=$? +[ "$file_scan" -eq 0 ] || grep -E "^[^ ]+:[0-9]+:" "$tmp/file-scan.log" | cut -c1-200 >&2 +check_result "file-argument scan of all tracked files" 0 "$file_scan" + echo "$pass PASS, $fail FAIL" [ "$fail" -eq 0 ] From d5c76b6eb3df63e9408f0eeb6d3469bd31265ef7 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 12 Oct 2026 19:46:50 +0200 Subject: [PATCH 2/3] sec(scripts): drop bare placeholder entries from .gitallowed git-secrets matches allowed patterns against the whole path:line:content line, so the 18 bare account-ID/UUID entries exempted any line containing them from every detector. Replace them with path-and-line-anchored entries for the lines a real scan flags; placeholder digits are bracketed so the file does not match itself. Closes #340 Co-Authored-By: Claude Sonnet 5.5 --- .gitallowed | 64 +++++++++++++++++++++++++---------------------------- 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/.gitallowed b/.gitallowed index b7648197..ecd00d08 100644 --- a/.gitallowed +++ b/.gitallowed @@ -16,40 +16,6 @@ # 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 - # 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 @@ -101,3 +67,33 @@ # 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)?",?$ + + +# Benign lines the setup script's 40-character-run detector flags in this +# repo's tree. That detector fires on any run of 40 characters outside +# [A-Za-z0-9/+=], so it hits markdown table rules and "// ---- title ----" +# comment banners. Each entry anchors path and the ENTIRE line with a closed +# character shape, so a secret appended to or hidden in such a line is still +# scanned: the banner text is limited to short capitalised or CamelCase words +# (no quotes, no "=", digits only in a "H2:" style tag), which cannot spell an +# AKIA key or a password assignment. +^(SECURITY\.md|\.github/workflows/README\.md|providers/aws/services/AUDIT\.md):[0-9]+:\|[-:| ]+\|$ +^[^:]+\.(go|sh):[0-9]+:[[:space:]]*(//|#)[[:space:]]*((-|─)+)([[:space:]]+(HTTP|[A-Z]{1,3}|([A-Z]?[a-z]+|ID)+|[A-Z][0-9]:|[a-z]+-[a-z]+)([ /()]+(HTTP|[A-Z]{1,3}|([A-Z]?[a-z]+|ID)+|[A-Z][0-9]:|[a-z]+-[a-z]+))*\)?[[:space:]]+((-|─)+))?$ +# gofmt column alignment in struct fields and map literals (all whitespace runs). +^providers/aws/recommendations/client\.go:[0-9]+:[[:space:]]+(spRecs|spErr)[[:space:]]+(\[\]common\.Recommendation|error)$ +^providers/azure/services/compute/inventory_scope_sdk_test\.go:[0-9]+:[[:space:]]+quantity[[:space:]]+int$ +^providers/gcp/services/computeengine/client_test\.go:[0-9]+:[[:space:]]+"":[[:space:]]+"",$ + +# Synthetic 12-digit account-ID fixtures in Go tests (sequential or +# repeated-digit blocks, none a real account) that the AWS account-ID detector +# flags. Each entry pins the path, the placeholder digits and the whole line. +# One digit of every placeholder is bracketed so this file does not match the +# detector itself. +^(pkg/common/types_test|pkg/exchange/auto_test|pkg/ladder/ladder_test|providers/aws/recommendations/expiry_coverage_test|providers/aws/service_client_test)\.go:[0-9]+:[[:space:]]*\{?(Account|AccountID):[[:space:]]*"(12345678901[2]|11111111111[1]|22222222222[2]|33333333333[3])",( Service: common\.Service[A-Za-z]+\},| CommitmentType: common\.CommitmentReservedInstance,)?$ +^pkg/exchange/account_guard_test\.go:[0-9]+:[[:space:]]*c\.identity = &fakeSTS\{account: "(11111111111[1]|22222222222[2])"\}$ +^providers/aws/recommendations/expiry_coverage_test\.go:[0-9]+:[[:space:]]*recs\[1\]\.Account = "22222222222[2]"$ +^pkg/ladder/(allocate|plan|store|types)_test\.go:[0-9]+:[[:space:]]*Scope:[[:space:]]*Scope\{Provider: common\.ProviderAWS, AccountID: "12345678901[2]"\},$ +^pkg/ladder/types_test\.go:[0-9]+:[[:space:]]*\{"valid aws", Scope\{Provider: common\.ProviderAWS, AccountID: "12345678901[2]"\}, false\},$ +^providers/aws/ladder/ladder_test\.go:[0-9]+:[[:space:]]*return ladder\.Scope\{Provider: common\.ProviderAWS, AccountID: "12345678901[2]"\}$ +^providers/aws/ladder/(adapters|ladder|sp_coverage_scope)_test\.go:[0-9]+:[[:space:]]*(a, err := New\()?Config\{Region: (testRegion|"us-east-1"), AccountID: "12345678901[2]"(, HorizonDays: (30|horizonDays), LookbackDays: 30)?\},$ +^providers/aws/recommendations/parser_ri_test\.go:[0-9]+:[[:space:]]*assert\.Contains\(t, out, "account=12345678901[2]"\)$ From 99a8e9b100cbdbb6796478f340bc2b6f8ad49bf9 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 12 Oct 2026 20:28:52 +0200 Subject: [PATCH 3/3] fix(scripts): allow capitalised hyphenated word in .gitallowed banner entry GNU grep (CI) rejected the "Dry-run" banner that BSD grep accepted; reproduced locally with ggrep. Refs #340 Co-Authored-By: Claude Sonnet 5.5 --- .gitallowed | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitallowed b/.gitallowed index ecd00d08..5d9e22f6 100644 --- a/.gitallowed +++ b/.gitallowed @@ -78,7 +78,7 @@ # (no quotes, no "=", digits only in a "H2:" style tag), which cannot spell an # AKIA key or a password assignment. ^(SECURITY\.md|\.github/workflows/README\.md|providers/aws/services/AUDIT\.md):[0-9]+:\|[-:| ]+\|$ -^[^:]+\.(go|sh):[0-9]+:[[:space:]]*(//|#)[[:space:]]*((-|─)+)([[:space:]]+(HTTP|[A-Z]{1,3}|([A-Z]?[a-z]+|ID)+|[A-Z][0-9]:|[a-z]+-[a-z]+)([ /()]+(HTTP|[A-Z]{1,3}|([A-Z]?[a-z]+|ID)+|[A-Z][0-9]:|[a-z]+-[a-z]+))*\)?[[:space:]]+((-|─)+))?$ +^[^:]+\.(go|sh):[0-9]+:[[:space:]]*(//|#)[[:space:]]*((-|─)+)([[:space:]]+(HTTP|[A-Z]{1,3}|([A-Z]?[a-z]+|ID)+|[A-Z][0-9]:|[A-Za-z][a-z]*-[a-z]+)([ /()]+(HTTP|[A-Z]{1,3}|([A-Z]?[a-z]+|ID)+|[A-Z][0-9]:|[A-Za-z][a-z]*-[a-z]+))*\)?[[:space:]]+((-|─)+))?$ # gofmt column alignment in struct fields and map literals (all whitespace runs). ^providers/aws/recommendations/client\.go:[0-9]+:[[:space:]]+(spRecs|spErr)[[:space:]]+(\[\]common\.Recommendation|error)$ ^providers/azure/services/compute/inventory_scope_sdk_test\.go:[0-9]+:[[:space:]]+quantity[[:space:]]+int$