diff --git a/.github/workflows/secret-scan.yml b/.github/workflows/secret-scan.yml index 5fadda02c3..caca2c20ad 100644 --- a/.github/workflows/secret-scan.yml +++ b/.github/workflows/secret-scan.yml @@ -38,6 +38,19 @@ jobs: run: | # Scan the full history of our branches (not --all, which includes # submodule git objects from _external/clawdbot). - # ~500 commits, ~15s. Catches historical leaks in our own code. + # ~2,600 commits, ~20s. Catches historical leaks in our own code. + # + # Name the commits; don't pattern-match them. `--remotes=origin/main` + # is a glob that git extends to origin/main/*, so it matched nothing. + # A push still scanned, through the checked-out branch in --branches, + # but a pull request checks out a detached merge commit with no local + # branch, and every PR scan reported "0 commits scanned" and passed. + # HEAD is that merge commit on a PR, and the pushed branch on a push. + # + # A scan of nothing still prints "no leaks found", so an empty input + # fails here instead (review checklist rule 56). + n=$(git rev-list --count HEAD origin/main origin/v1.0.x) + echo "commits to scan: $n" + [ "$n" -gt 0 ] || { echo "::error::Secret Scan has 0 commits to scan"; exit 1; } gitleaks detect --source . --config .gitleaks.toml --redact -v \ - --log-opts="--branches --remotes=origin/main --remotes=origin/v1.0.x" + --log-opts="HEAD origin/main origin/v1.0.x" diff --git a/.gitleaks.toml b/.gitleaks.toml index 80b8f7ffe9..967a926260 100644 --- a/.gitleaks.toml +++ b/.gitleaks.toml @@ -17,6 +17,11 @@ regexes = [ # (shared on the landing page, README, and Discord). Tests pin it as the # REACT_APP_COMMUNITY_INVITE_TOKEN fixture; it is not a secret. '''7b91255f18ae3c0ae3721707a6613731''', + # A fake ChatGPT JWT in agentProvisionerServiceK8s.test.js (#2213): the + # base64 of {"alg":"RS256"} followed by the literal ".jwt", no payload and no + # signature. generic-api-key matched it, and main's Secret Scan was red on + # every push from 2026-10-16. The exact literal only, so a real token still fails. + '''eyJhbGciOiJSUzI1NiJ9\.jwt''', ] paths = [ diff --git a/docs/development/review-checklist.md b/docs/development/review-checklist.md index 2f1c21fa53..2c3b182842 100644 --- a/docs/development/review-checklist.md +++ b/docs/development/review-checklist.md @@ -237,3 +237,7 @@ And then note where the fix landed. Someone hit the audience gap, felt it, and b ## Which database ran the SQL 55. **A SQL change tested only on pg-mem is not tested.** pg-mem is an in-memory imitation of Postgres. Its parser and type inference are its own, so it accepts queries real Postgres refuses, and refuses some real Postgres runs (it has no `EXISTS`, for one). A green pg-mem arm proves the query does what you meant *in pg-mem*. Any change to a query's text, parameters or casts needs an arm in the tier-1 suite (`backend/__tests__/service`, real Postgres in CI), run locally against a real server before the head is cited. A plain write is one of the cases: a guard added for an edge case runs on every row. *(Earned: #2272, 2026-10-16. The INSERT's new same-pod guard was written `$1 IN (SELECT target.pod_id …)`, with `$1` also a value in the select list. pg-mem accepted it, and all six unit arms and six mutations were green. Real Postgres refused **every** write with `42P08 inconsistent types deduced for parameter $1`: plain posts, same-pod replies and cross-pod replies alike. Merged, it would have stopped every chat write. Cofounder's security read ran it on postgres:17.10 with the production pool. The PR's own tier-1 job failed the same way, after the author had already asked for gates. Fixed with `$1::varchar`, and tier-1 arms added for a plain post, a same-pod reply, a cross-pod reply and a root named in another pod.)* + +## What a scan covered + +56. **A scan that reports nothing found has proved nothing until it reports what it covered, and a count of zero is a failure, not a pass.** "No leaks found", "0 findings" and a green tick read the same whether the scanner read 2,581 commits or none, so the result a reviewer sees cannot tell a clean input from an empty one. Read the coverage line (commits scanned, files checked, rows matched) before the result. When you own the check, make an empty input fail it. The usual shape is a scan that works on one trigger and not another: the trigger you watch is real, and it vouches for the one you don't. *(Earned: #2389, 2026-10-21. Secret Scan passed every pull request from its first day, #84 on 2026-04-03, with "0 commits scanned … no leaks found". Its `--remotes=origin/main` is a git glob for `origin/main/*` and matched nothing, and a pull request checks out a detached merge commit with no local branch for `--branches` to find. Pushes to main scanned through the checked-out branch, so main's scan was real and nobody read the pull-request one. #2213's fake-JWT fixture passed its pull request and then kept main red for five days. A one-time scan of every branch and pull-request head, 8,883 commits in #2390, found no real secret. The workflow now names its commits and fails before scanning when there are none.)*