Close the inert-pattern hole in the credential scan
A count proves the scan READ a file. It says nothing about whether the pattern can match anything — and "now it reports a number" and "now it only reports a number" print the identical line. The woo-ai-manager session proved this the hard way: it planted an AWS key, watched its file count rise from 37 to 38, and the scan passed green. Its prefix list had no AKIA entry at all. This tree's list did have AKIA, but the proof behind that claim was a grep of the regex in isolation, not a run of ci/static.sh — a stub harness that proves the pattern compiles, not that the check fires. Re-done properly: every shape the pattern claims to catch is now planted in a tracked file and run through the whole script. That exercise found a real hole. A prefix list matches the shape of a VALUE, so it is blind to any credential with no vendor marker: an api_key assigned twenty opaque characters walked straight past it. Added a second, name-anchored rule that fires only where the code itself calls the value a secret, covering both the array/assignment form and define(). It is not an entropy heuristic — the file's existing refusal of those still stands, because they fire on minified assets and hashes and a check that cries wolf gets commented out. Placeholders are excluded, or the rule would go red on the one thing a settings screen is guaranteed to contain. Proven from both sides, through ci/static.sh rather than in isolation: 9 shapes planted and all 9 flagged (AKIA, PEM, sk-ant-, sk-, ghp_, gho_, github_pat_, array form, define form), and 4 lookalikes correctly ignored (a bare hash, an unfilled placeholder, a masked value, an empty/short value). 13/13. The one false positive measured was this change's own explanation: the first version of the comment spelled the example out in full and the rule flagged it. Fixed by writing it in prose. Deliberately no bypass marker — an unexercised escape hatch is its own hollow check; add one when a real documentation case needs it. Note for the sibling plugins: an untracked fixture is invisible here, since the scan reads git ls-files. A probe planted without `git add -N` passes clean and looks like the scan working. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
a19859c09c
commit
82d6cc7d7d
+39
-4
@@ -165,9 +165,32 @@ ci_step "no secrets in tracked files"
|
||||
# unpushed. It has to be rotated, and the window between the push and noticing
|
||||
# is the whole problem.
|
||||
#
|
||||
# Deliberately narrow. Broad entropy heuristics fire on minified assets, hashes
|
||||
# and base64 images, and a check that cries wolf gets commented out — which is
|
||||
# worse than not having it. These are prefixes that mean one thing only.
|
||||
# Two rules, at two different confidence levels.
|
||||
#
|
||||
# The first is content-anchored: prefixes that mean one thing only. Broad
|
||||
# ENTROPY heuristics are still refused here — they fire on minified assets,
|
||||
# hashes and base64 images, and a check that cries wolf gets commented out,
|
||||
# which is worse than not having it.
|
||||
#
|
||||
# The second is name-anchored, and closes a hole the first cannot see. A prefix
|
||||
# list matches the shape of a VALUE, so it is blind to any credential that does
|
||||
# not carry a vendor's marker: an api_key assigned twenty opaque characters
|
||||
# walks straight past it. This is not an entropy heuristic — it fires only
|
||||
# where the code itself names the value a secret, which is why it does not
|
||||
# touch a minified asset. Obvious placeholders are excluded so a settings
|
||||
# default does not cry wolf.
|
||||
#
|
||||
# ⚠️ It DOES fire on a documentation example, and there is no bypass marker on
|
||||
# purpose — an unexercised escape hatch is its own hollow check. The first
|
||||
# version of this comment spelled the example out in full and the rule flagged
|
||||
# its own explanation, which is the one false positive measured so far. Write
|
||||
# such an example in prose, or break the literal up.
|
||||
#
|
||||
# 🔴 A count cannot tell you a pattern is not INERT. Proven the hard way in a
|
||||
# sibling plugin: it planted an AWS key, the file count rose, and the scan
|
||||
# passed green — that prefix was simply missing from its list. So every shape
|
||||
# claimed below is planted and run through this whole script, not grepped in
|
||||
# isolation, and a lookalike control is checked not to fire.
|
||||
#
|
||||
# 🔴 And it says how many files it read. A pushed key is compromised even after
|
||||
# the commit is deleted, so this scan silently examining nothing is not a weak
|
||||
@@ -178,14 +201,26 @@ ci_step "no secrets in tracked files"
|
||||
# partially wrong, so the only way it misreports is by coming out empty, and
|
||||
# any higher number would just be a guess about the plugin's size. The count on
|
||||
# the success line is what catches a list that shrank without emptying.
|
||||
secret_value_pat='(sk-ant-[A-Za-z0-9_-]{16,}|sk-[A-Za-z0-9]{32,}|ghp_[A-Za-z0-9]{30,}|gho_[A-Za-z0-9]{30,}|github_pat_[A-Za-z0-9_]{50,}|AKIA[0-9A-Z]{16}|-----BEGIN [A-Z ]*PRIVATE KEY-----)'
|
||||
|
||||
# The code calls it a secret and then hands it twenty-odd opaque characters.
|
||||
secret_name_pat="['\"](api[_-]?key|secret|password|passwd|token|auth[_-]?token|private[_-]?key)['\"][[:space:]]*(=>|=|:)[[:space:]]*['\"][A-Za-z0-9/+_-]{20,}['\"]|define\([[:space:]]*['\"][A-Z0-9_]*(SECRET|PASSWORD|TOKEN|API_KEY)['\"][[:space:]]*,[[:space:]]*['\"][A-Za-z0-9/+_-]{20,}['\"]"
|
||||
|
||||
# A default nobody has filled in yet is not a leak. Without this the check goes
|
||||
# red on the one thing a settings screen is guaranteed to contain.
|
||||
placeholder_pat='REPLACE|YOUR[_-]|EXAMPLE|CHANGE[_-]?ME|TODO|FIXME|PLACEHOLDER|xxxxx|XXXXX|\*\*\*\*'
|
||||
|
||||
before=$fails
|
||||
secrets_scanned=0
|
||||
while IFS= read -r -d '' f; do
|
||||
[ -f "$f" ] || continue
|
||||
secrets_scanned=$((secrets_scanned + 1))
|
||||
if LC_ALL=C grep -nE '(sk-ant-[A-Za-z0-9_-]{16,}|sk-[A-Za-z0-9]{32,}|ghp_[A-Za-z0-9]{30,}|gho_[A-Za-z0-9]{30,}|github_pat_[A-Za-z0-9_]{50,}|AKIA[0-9A-Z]{16}|-----BEGIN [A-Z ]*PRIVATE KEY-----)' "$f"; then
|
||||
if LC_ALL=C grep -nEI "$secret_value_pat" "$f"; then
|
||||
note_fail "$f looks like it contains a credential. It cannot be unpushed — rotate it."
|
||||
fi
|
||||
if LC_ALL=C grep -nEI "$secret_name_pat" "$f" | LC_ALL=C grep -vE "$placeholder_pat"; then
|
||||
note_fail "$f assigns a long opaque value to something the code calls a secret. If it is one, it cannot be unpushed — rotate it."
|
||||
fi
|
||||
done < <(git ls-files -z)
|
||||
if [ "$secrets_scanned" -lt 1 ]; then
|
||||
note_fail "The credential scan examined no files at all. git tracks nothing here, so this check proved nothing."
|
||||
|
||||
Reference in New Issue
Block a user