security: the scanner never ran its private-key pattern, and verify never saw dist

Three faults in one guard, the only thing standing between a credential and a
pushed commit. Found while planning Batch 17; closes #235.

1. grep was called without -e, at both call sites. The private-key pattern
   starts with dashes, so grep read it as an option, exited 2, and the beside-it
   2>/dev/null threw the complaint away. A staged private key passed the
   pre-commit hook, and had since the rule was written. Proven in a scratch
   repository before and after.

2. The 2026-08-29 pattern, committed as-is in 7415e19, flagged two comment
   lines that exist to show the shape of a credential. verify was therefore red,
   and release.sh would have died after bumping package.json, package-lock.json
   and the Dockerfile, leaving a tree that pre-push then refuses.

3. verify never scanned dist/, though SECURITY_CHECKLIST.md has listed the
   bundle scan as a release check for months. A key can reach the bundle from an
   environment variable inlined at build time without ever being committed.

What changed, beyond -e: every pattern is compiled against empty input before
the scan and an unreadable one exits 2, because silence from a broken matcher
looks exactly like a clean tree. The NAME=value pattern now also catches quoted
values, which is how a real secret is usually written down and which it has
always missed. A line that must show a credential shape carries `secrets-ok:`
and a reason, which excuses that line alone and stays visible to review and to
grep. Guard 20-secrets runs --tracked and --built dist/.

Proven by mutation, per GUARDS.md: a staged PEM header, a bare API_KEY=, a
quoted API_KEY=, an exported secret and an AWS key id each exit 1; a ${VAR}
value, a <placeholder>, an excused line and a delete-only commit each exit 0; an
unreadable pattern exits 2 with content staged and with none; a key planted in
dist/ fails guard 20-secrets alone, masked in the report.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

docs/TOOLS.md

@@ -64,6 +64,11 @@ bash scripts/check-env.sh --file .env   # what is configured, before anything re
 bash scripts/secrets.sh --tracked       # what is already committed
 ```
 
+`npm run verify` runs that scan too, as guard `20-secrets`, together with
+`--built dist/` over the bundle the build just produced. A line that has to show
+a credential shape, such as a usage example, carries `secrets-ok:` and a reason
+on the same line; that excuses the one line, visibly.
+
 Then [`architecture/GUARDS.md`](architecture/GUARDS.md) before you write a check
 of your own — how to write one that can actually fail.
 

docs/architecture/GUARDS.md

@@ -109,6 +109,16 @@ tr '\0' '\n' < /proc/$PID/environ | grep -c .   # 0 here means "could not read"
 This is the confident-absence failure one level up: the same trap as a screen
 rendering a failed query as a count of zero, applied to your own diagnosis.
 
+**The same trap inside a scanner.** A matcher that discards its errors turns
+"could not run" into "found nothing". `grep "$pattern" 2>/dev/null` answers a
+pattern it cannot read with exit 2 and no output, and a loop reading its matches
+sees an empty list. `scripts/secrets.sh` ran its private-key pattern exactly that
+way from the day the rule was written: the pattern starts with dashes, grep took
+it for an option, and the scanner reported a clean tree over a staged private
+key until 2026-09-10 (#235). Two rules follow. Pass every pattern with `-e`. And
+compile each pattern once before trusting its silence, exiting 2 when one cannot
+be read, which is what `secrets.sh` now does.
+
 ## 5. A guard that is often wrong is worse than none
 
 A check with a high false-positive rate trains everybody to skip its output,

docs/architecture/README.md

@@ -141,7 +141,7 @@ each row says what it does *here*.
 | Path | What it is |
 | --- | --- |
 | `scripts/check-env.sh` | which of the 17 Zoho / reCAPTCHA / CORS / rate-limit variables are set and plausible, before the server reads them. Exit 2 means nothing was checked |
-| `scripts/secrets.sh` | credential shapes in a staged diff, and `--tracked` for a whole-tree audit. **`--built dist/` is the one that matters here**: `VITE_RECAPTCHA_SITE_KEY` is inlined into the bundle at build time, so the repository scan cannot see what users receive. Carries this project's own shapes, and one **tightened** pattern — see below |
+| `scripts/secrets.sh` | credential shapes in a staged diff, and `--tracked` for a whole-tree audit. **`--built dist/` is the one that matters here**: `VITE_RECAPTCHA_SITE_KEY` is inlined into the bundle at build time, so the repository scan cannot see what users receive. `npm run verify` runs both, as guard `20-secrets`. Every pattern is compiled before the scan, because a pattern grep cannot read produces the same silence as a clean tree (#235). A line that must show a credential shape carries `secrets-ok:` and its reason. Carries this project's own shapes, and one **tightened** pattern, described below |
 | `scripts/verify.sh` | every check this project has, in one table. Honestly thin — there is no test suite, and it says so rather than printing a green row |
 | `scripts/doc-triggers.py` | which documents a pending change fires, read from the `Governs:` headers. Run it before committing, not after |
 | `scripts/forgejo-issue.py` | files and closes issues in the tracker convention, refusing malformed ones before they are filed |

docs/security/SECURITY.md

@@ -53,7 +53,9 @@ so: that is the one that went, and that is why.
 credentials in the tracked tree — including in the history, which a `git log -p`
 search covers and a directory listing does not. `scripts/secrets.sh --tracked`
 does the tree; `--built dist/` does the bundle, which is the artifact users
-actually receive and the one the repository scan never sees.
+actually receive and the one the repository scan never sees. `npm run verify`
+runs both, as guard `20-secrets`. Until 2026-09-10 neither could see a private
+key: the scanner never ran that pattern (#235).
 
 **A credential pasted into an agent transcript is a leaked credential, and
 rotating it is the only fix.** Deleting the message does not help and neither

docs/security/SECURITY_CHECKLIST.md

@@ -19,8 +19,7 @@ is short enough to finish rather than a document to skim.
 
 ## Before a release
 
-- [ ] `bash scripts/secrets.sh --tracked` is clean — proves nothing credential-shaped is committed
-- [ ] `npm run build && bash scripts/secrets.sh --built dist/` is clean — proves the *bundle* is clean, which the tracked scan cannot tell you
+- [ ] `npm run verify` passes guard `20-secrets`. It runs `scripts/secrets.sh --tracked`, which proves nothing credential-shaped is committed, and `--built dist/` over the bundle the build just made, which proves the *bundle* is clean, something the tracked scan cannot tell you
 - [ ] `bash scripts/check-env.sh --file .env` exits 0 — proves every variable the server reads is set and shaped right, before it reads them. **Exit 2 is not a pass**
 - [ ] `npm audit` shows no high or critical advisory in production dependencies — proves no known-exploitable code ships
 - [ ] `bash scripts/preflight.sh` is clean against **both** front doors — `queuenorth.com` by default and `PREFLIGHT_ORIGIN=https://qn.isnull.dev` for the other. Proves headers, CSP and TLS survived the deploy on two separate ingresses that can rot independently
@@ -73,3 +72,4 @@ with.
 | 2026-05-11 → 2026-06-14 | The Zoho WebToLead tokens `xnQsjsdp` and `xmIwtLD` were hardcoded in `index.html` and later `src/pages/Contact.jsx`, and reached four commits on what was then a **public** repository, before being moved to environment variables at `05b27d2`. Low severity — they are public-by-design form identifiers a browser renders anyway — and deliberately **not** rewritten out of the history, which is now private | `scripts/secrets.sh` in the `pre-commit` hook, scanning the staged diff. This is the finding that makes the hook worth having in a project with no test suite to run beside it |
 | 2026-08-18 | Nobody had ever checked whether the reCAPTCHA **secret** key had reached git. It had not — zero commits, zero tracked files — but "we would have noticed" is not a check | `secrets.sh --tracked` is now the first thing run in a fresh clone, per `docs/TOOLS.md` |
 | 2026-08-18 | The live lead database had no backup and no restore had ever been attempted, and nothing in any document said so | `scripts/backup.sh` and `scripts/restore-check.sh`, **both run against production the same day**: a verified snapshot was taken from the running container and replayed into a scratch database — 2 tables, 3 rows, under a second. `docs/OPERATIONS.md` now carries a real *Last verified restore* date. What is still missing is a schedule and an off-machine copy, tracked in `Batch 15` |
+| 2026-09-10 | `scripts/secrets.sh` had never run its private-key pattern: grep read the leading dashes as an option and `2>/dev/null` hid the error, so a staged private key passed the pre-commit hook. Its NAME=value pattern matched nothing at all until 2026-08-29, and missed quoted values until 2026-09-10. Nothing ran the bundle scan this checklist asked for | Every pattern is compiled before the scan, and an unreadable one exits 2 instead of passing; every grep takes `-e`; quoted values are caught; a documentation line is excused only by a visible `secrets-ok:` note; guard `20-secrets` now scans `dist/` as well (#235) |

scripts/check-env.sh

@@ -58,7 +58,7 @@
 #                          is the container, and the database is elsewhere
 #   NODE_ENV=Production    matches no `=== "production"` anywhere, so every
 #                          branch takes its development arm in production
-#   JWT_SECRET=devsecret   the eight characters someone typed to get the dev
+#   JWT_SECRET=devsecret   the eight characters someone typed to get the dev  (secrets-ok: example)
 #                          server up, now signing real sessions
 #   API_URL=https://api.example.com/
 #                          one trailing slash; every joined path is `//v1/...`
@@ -247,12 +247,12 @@ SPEC=(
 # API_KEY, KEYCLOAK_SECRET and MONKEY_HOST alike. The last is a false positive
 # and costs nothing: its value is described rather than shown.
 # ---------------------------------------------------------------------------
-SECRET_NAME_PATTERN='SECRET|TOKEN|PASSWORD|PASSWD|PWD|CREDENTIAL|PRIVATE|SALT|SIGNATURE|SIGNING|AUTH|KEY|DSN|COOKIE'
+SECRET_NAME_PATTERN='SECRET|TOKEN|PASSWORD|PASSWD|PWD|CREDENTIAL|PRIVATE|SALT|SIGNATURE|SIGNING|AUTH|KEY|DSN|COOKIE'  # secrets-ok: names, not a value
 
 # Values a `secret-min-length` variable must not be, compared in lower case and
 # never echoed. These are the strings typed to make the dev server start, which
 # then travel to production inside a copied .env and satisfy every length rule.
-PLACEHOLDER_SECRETS='changeme change-me change_me changethis secret mysecret supersecret password passwd hunter2 test testing example placeholder todo tbd xxx xxxx admin dev devsecret development your-secret-here your_secret_here notasecret 123456 12345678 abc123'
+PLACEHOLDER_SECRETS='changeme change-me change_me changethis secret mysecret supersecret password passwd hunter2 test testing example placeholder todo tbd xxx xxxx admin dev devsecret development your-secret-here your_secret_here notasecret 123456 12345678 abc123'  # secrets-ok: known-weak values
 
 # Redaction applies to NAMES read out of a file too, not only to values.
 #

scripts/secrets.sh

@@ -26,7 +26,7 @@
 #   bash scripts/secrets.sh                 # staged changes (use in pre-commit)
 #   bash scripts/secrets.sh --tracked       # everything tracked, for an audit
 #   bash scripts/secrets.sh --built dist/   # the artifact users receive
-#   SECRETS_PATTERN_FILE=src/lib/log.ts bash scripts/secrets.sh
+#   SECRETS_PATTERN_FILE=src/lib/log.ts bash scripts/secrets.sh   # secrets-ok: a path
 #   bash scripts/secrets.sh --allow docs/examples/
 #
 # ## --built, and why the repository is the wrong place to stop
@@ -56,7 +56,16 @@
 # in the history** after you delete it, and this will not tell you that — the
 # fix there is a rotation, not a scan. Rotate first, then clean up.
 #
-# Exit codes: 0 nothing found. 1 a candidate found. 2 nothing was scanned.
+# ## Excusing one line
+#
+# A line that has to show a credential SHAPE, such as a usage example or a list
+# of known-weak values, carries `secrets-ok:` and a reason on the same line.
+# That excuses that one line and nothing else, and it stays visible in review
+# and to grep, which an --allow path does not. There is no blanket exemption for
+# comments: a key pasted into a comment is still a leaked key.
+#
+# Exit codes: 0 nothing found. 1 a candidate found. 2 nothing was scanned, or a
+# pattern could not be read, which is the same thing.
 # ---------------------------------------------------------------------------
 
 set -uo pipefail
@@ -167,10 +176,20 @@ PATTERNS=(
   '//[^/@[:space:]:"'"'"'{},<>]+:[^/@[:space:]"'"'"'{},<>]+@'
   '[?&](token|key|secret|password|access_token|api_key)=[^&[:space:]"]+'
   '\b(Bearer|Basic)[[:space:]]+[A-Za-z0-9._~+/=-]{20,}' # an authorization header
-  # Anchored to the start of a line or an `export`, because unanchored it
-  # matched `access_token = $1` in SQL and `apiKey=` in a property list — three
-  # findings in src/ that were column names, not credentials.
-  '(^|[^A-Za-z0-9_])[A-Z0-9_]*(SECRET|TOKEN|PASSWORD|PASSWD|PASS|API_KEY)[A-Z0-9_]*=[^[:space:]"'"'"'$][^[:space:]"'"'"']{7,}'
+  # NAME=value, where NAME is upper-case and contains SECRET, TOKEN, PASS or
+  # API_KEY. Three things about it were learned the hard way:
+  #
+  # - It must not be anchored to the start of a line. Neither scan mode ever
+  #   presents one: a staged diff starts every line with "+", and --tracked
+  #   prefixes each with its file name. Anchored, it matched nothing at all, so
+  #   the name now merely must not continue an identifier.
+  # - Case matters. Lower-case `access_token = $1` in SQL and `apiKey=` in a
+  #   property list are column and field names, not credentials.
+  # - The value may be quoted. Unquoted-only, it missed API_KEY="...", which is
+  #   how a real secret is most often written down. A value starting with `$`
+  #   is a variable reference and one starting with `<` is a placeholder, and
+  #   neither is flagged.
+  '(^|[^A-Za-z0-9_])[A-Z0-9_]*(SECRET|TOKEN|PASSWORD|PASSWD|PASS|API_KEY)[A-Z0-9_]*=["'"'"']?[^[:space:]"'"'"'$<][^[:space:]"'"'"']{7,}'
   '-----BEGIN [A-Z ]*PRIVATE KEY-----'
   '\bghp_[A-Za-z0-9]{20,}'                              # GitHub
   '\bxox[baprs]-[A-Za-z0-9-]{10,}'                      # Slack
@@ -277,6 +296,19 @@ else
   done < <(git ls-files)
 fi
 
+# Every pattern must compile before its silence means anything. grep answers an
+# unreadable pattern with exit 2, and the scan below discards grep's errors, so
+# a broken pattern reads as "no credential shapes", the same silence as a clean
+# tree. The private-key pattern was exactly that for as long as it existed: it
+# starts with dashes, grep took it for an option, and nothing said so.
+for pattern in "${PATTERNS[@]}" "${NOTED_PATTERNS[@]}"; do
+  grep -qE -e "$pattern" </dev/null 2>/dev/null
+  if [ "$?" -eq 2 ]; then
+    say "grep cannot read the pattern '${pattern:0:40}', so nothing was scanned."
+    exit 2
+  fi
+done
+
 if [ -z "$CONTENT" ]; then
   # "Nothing to scan" means two different things and collapsing them makes a
   # DELETE-ONLY COMMIT IMPOSSIBLE. In staged mode CONTENT is built from ADDED
@@ -304,6 +336,9 @@ for pattern in "${PATTERNS[@]}"; do
     done
     [ -n "$skip" ] && continue
 
+    # One line excused on purpose, with its reason beside it. See the header.
+    case "$hit" in *secrets-ok:*) continue ;; esac
+
     # The match is masked, then the line is truncated. Truncation alone was not
     # enough and used to be all there was: it bounds how much of a LONG value
     # reaches the terminal and prints a short one whole, so the scanner
@@ -317,7 +352,7 @@ for pattern in "${PATTERNS[@]}"; do
     [ -n "$masked" ] || masked="[a line matching a credential pattern, unprintable]"
     printf '  %.120s…\n' "$masked"
     found=$((found + 1))
-  done < <(printf '%s\n' "$CONTENT" | grep -nEI "$pattern" 2>/dev/null | head -20)
+  done < <(printf '%s\n' "$CONTENT" | grep -nEI -e "$pattern" 2>/dev/null | head -20)
 done
 
 # The public-by-design tier. Printed, counted, and deliberately not fatal.
@@ -332,7 +367,7 @@ if [ "$MODE" = "built" ]; then
       fi
       printf '  %.120s…\n' "$hit"
       noted=$((noted + 1))
-    done < <(printf '%s\n' "$CONTENT" | grep -oEI "$pattern" 2>/dev/null | sort -u | head -20)
+    done < <(printf '%s\n' "$CONTENT" | grep -oEI -e "$pattern" 2>/dev/null | sort -u | head -20)
   done
 
   if [ "$noted" -gt 0 ]; then

scripts/verify.d/20-secrets

@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 #
-# Credentials in the tracked tree.
+# Credentials in the tracked tree, and in the bundle users receive.
 #
 # `scripts/secrets.sh` runs on the staged diff from the pre-commit hook, which
 # is the cheap moment. This is the whole-tree version, run as part of verify so
@@ -11,9 +11,25 @@
 # commits of a then-public repository for a month, and a staged-diff scan
 # installed afterwards would never have mentioned them.
 #
+# It also scans dist/, which 10-build has just produced. A key can reach the
+# bundle from an environment variable inlined at build time without ever being
+# committed, and the tracked scan cannot see that. SECURITY_CHECKLIST.md listed
+# this as a manual release check for months; nothing ran it.
+#
 # Exit 0 clean, 1 findings, 2 the scanner could not run.
 set -uo pipefail
 cd "$(git rev-parse --show-toplevel)" || exit 1
 
-[ -f scripts/secrets.sh ] || { echo "secrets: scripts/secrets.sh is missing — nothing was scanned." >&2; exit 2; }
-bash scripts/secrets.sh --tracked
+[ -f scripts/secrets.sh ] || { echo "secrets: scripts/secrets.sh is missing, so nothing was scanned." >&2; exit 2; }
+
+status=0
+bash scripts/secrets.sh --tracked || status=$?
+
+if [ ! -d dist ]; then
+  echo "secrets: dist/ is missing, so the built output was not scanned. Run 10-build first." >&2
+  exit 2
+fi
+
+bash scripts/secrets.sh --built dist/ || { rc=$?; [ "$rc" -gt "$status" ] && status=$rc; }
+
+exit "$status"