diff --git a/.gitallowed b/.gitallowed index 6ff918d..b764819 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,55 @@ # 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 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 +# 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. +^[^:]+:[0-9]+:[[:space:]]*("private_key"|PrivateKey):[[:space:]]*"-----BEGIN PRIVAT[E] KEY-----\\n(MIIEvQIBADANBg)?\.\.\.(\\n)?",?$ 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..f27fa86 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}" @@ -38,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..." @@ -53,56 +99,26 @@ 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 # 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 '-----BEGIN (RSA|DSA|EC|OPENSSH) PRIVATE KEY-----' # PEM private keys +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 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" +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 +# 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 ]