From f3126b57e61ecde394b41f5b2b3d1d20c9fc43f4 Mon Sep 17 00:00:00 2001 From: kristianwind Date: Thu, 17 Sep 2026 00:25:52 +0200 Subject: [PATCH] Stop the credential scan silently skipping binary files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adding the name-anchored rule in the previous commit also put `-I` on the value-pattern grep, which it did not have before. grep -I SILENTLY SKIPS a file it decides is binary, so a PEM header inside one is waved through without a word. Without -I the same file reports "binary file matches" and fails, which is the right direction to be wrong in for a check whose whole subject cannot be unpushed. Nothing went red to announce this. The tree has no binary tracked files today, so the narrowing was invisible from both sides — the clean run stayed green and no probe covered it, because every probe so far planted a text file at the repo root. It is the same class as the hole this scan already had: a check that cannot match anything and a check that matches nothing print the same line. The two rules now differ on -I deliberately, each with the reason inline. The name rule keeps it: that one is heuristic, so a chance match inside a compiled asset is noise rather than a finding. Also proven, rather than asserted, after a sibling plugin found its own scan was reading only the SHIPPED file list and therefore missing 36 of 73 tracked files — every CI script, workflow and test fixture, which is exactly where a key gets pasted to test it. This scan reads `git ls-files` and always has, but the claim had never been measured: probes in ci/, tests/ and .github/workflows/ are all caught while `dist-files.sh` lists none of them. Battery now 19, both directions, through ci/static.sh: 13 fire — AKIA, PEM, sk-ant-, sk-, ghp_, gho_, github_pat_, array form, define() form, the same key in ci/ and tests/ and .github/, and a PEM header inside a binary file 6 quiet — a bare hash, an unfilled placeholder, a masked value, an empty or short value, a comma after 'token' in ordinary code, and a define whose value is still TODO The comma control comes from the sibling plugin: it put `,` into the shared alternation and had to prove `wp_die( 'token', 'a long sentence' )` stays quiet. This tree anchors the comma case on `define(` instead, so the same string was never at risk — but it is worth a control either way. Co-Authored-By: Claude Opus 5 (1M context) --- ci/static.sh | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/ci/static.sh b/ci/static.sh index d81eda3..a026410 100755 --- a/ci/static.sh +++ b/ci/static.sh @@ -215,9 +215,18 @@ secrets_scanned=0 while IFS= read -r -d '' f; do [ -f "$f" ] || continue secrets_scanned=$((secrets_scanned + 1)) - if LC_ALL=C grep -nEI "$secret_value_pat" "$f"; then + # No -I here, deliberately, and the two rules differ on this for a reason. + # grep -I SILENTLY SKIPS a file it decides is binary, so a PEM header inside + # one would be waved through without a word. Without -I the same file reports + # "binary file matches" and fails, which is the right direction to be wrong + # in. (Narrowed by accident when the name rule was added, and caught only by + # asking what the flag actually does — the tree has no binary files today, so + # nothing went red to announce it.) + if LC_ALL=C grep -nE "$secret_value_pat" "$f"; then note_fail "$f looks like it contains a credential. It cannot be unpushed — rotate it." fi + # -I is right HERE: this rule is heuristic and name-anchored, so a chance + # match inside a compiled asset is noise, not a finding. 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