Skip to content

The secret-scan check read 1 of main's 81 commits - #92

Merged
ChelseaKR merged 1 commit into
mainfrom
chore/secret-scan-history-full-walk
Sep 13, 2026
Merged

ChelseaKR merged 1 commit into
mainfrom
chore/secret-scan-history-full-walk

Conversation

@ChelseaKR

@ChelseaKR ChelseaKR commented Sep 13, 2026

Copy link
Copy Markdown
Owner

What the check actually read

secret-scan was gitleaks/gitleaks-action@e0c47f4 (v3.0.0). Read at that pinned SHA
(src/index.js, src/gitleaks.js), the action does not scan the repository — it picks a
commit range out of the event that triggered the run:

event what it runs
push, N commits gitleaks detect --log-opts=--no-merges --first-parent BASE^..HEAD
push, 1 commit gitleaks detect --log-opts=-1exactly one commit
pull_request the pull request's own commits
schedule / workflow_dispatch no --log-opts at all, i.e. the whole history

ci.yml triggers on push and pull_request and nothing else. The only two events for
which that action reads history have never fired in this repository. And every merge into
main here is a squash merge, which is a one-commit push — so on main the job ran
--log-opts=-1 and read 1 of main's 81 commits, then reported success.

A credential added in one commit and deleted in the next is invisible to that. It is
still in the history, and nothing in this repository has ever opened the history to look.

Why fetch-depth: 0 did not prevent it

It cannot. fetch-depth: 0 decides how much history actions/checkout puts on disk;
what the scanner reads is decided by how it is invoked. A checkout deep enough to scan
and an invocation that declines to scan it is exactly the state this job was in, and the
comment above the step said nothing about it. fetch-depth: 0 stays — it is the necessary
precondition — and its comment now says plainly that it is not the sufficient one.

The fix

The action is replaced by a pinned, checksum-verified gitleaks 8.30.1 binary invoked as:

/tmp/gitleaks git . --no-banner --redact --exit-code 1

No --log-opts, so it walks every commit reachable from HEAD on every event; the range no
longer depends on how the run was triggered. The release archive is checked against the
gitleaks_..._checksums.txt published with it before it is unpacked. --exit-code 1 makes
a finding fail the job; --redact keeps the secret out of the log.

pull-requests: read and the GITHUB_TOKEN handed to the action are dropped with it —
both existed only so the action could list a pull request's commits in order to scope the
scan, and nothing is scoped now. contents: read is all that is left.

The job id and display name are byte-identical. secret-scan is one of the five
required contexts in .github/rulesets/main.json, and tests/test_ruleset_evidence.py
derives that list from this workflow; a rename would empty the requirement on the day the
profile is applied.

Posture, stated plainly: main carries no ruleset and no branch protection today —
gh api repos/ChelseaKR/perimeter/rulesets returns [] and branches/main reports
"protected": false (re-verified 2026-09-13). So this check blocks nothing and never has.
It is still the only secret scan this repository has, and per ADR-0004 that is the reason
to make it able to fail, not a reason to leave it.

Negative control

Run entirely locally, in a throwaway clone of this repository with its remote removed.
Nothing below was ever pushed; the planted credential is random, generated for the run,
and never leaves the deleted clone.

baseline tree (main, 81 commits) 82f4a65c55bc2a211466b254ca5e3e8a8470ed4e
baseline HEAD d562f900af55a934ccade6665e85f173f2ad98e6
new invocation on the clean baseline PASS — the fix does not turn the check red
planted key blob 0364105399d1ee01f2c46eb828dcb0c6296cc668, present in the object store
commits introducing or removing it (git log -S) 2 — the plant and the removal
occurrences in the working tree 0 — reachable only from history
tip tree vs. baseline, with the key planted identical
OLD invocation (--log-opts=-1) PASS — green with the credential in history. This is the defect.
NEW invocation (gitleaks git .) FAIL — it found the planted credential
restored tree 82f4a65c55bc2a211466b254ca5e3e8a8470ed4e — byte-identical
restored HEAD identical; scratch branch gone; clone deleted

The guard test was negative-controlled the same way. Three separate sabotages of
ci.yml, each run with __pycache__ cleared:

sabotage result
add --log-opts=-1 to the invocation test_the_scanner_is_not_handed_a_range fails
restore gitleaks/gitleaks-action as a step test_the_event_driven_action_does_not_come_back fails
drop fetch-depth: 0 from this job only test_the_scan_checkout_still_fetches_the_history_it_walks fails
restored all 5 pass

That third one matters twice over: ci.yml holds two real fetch-depth: 0 lines and one
that appears inside the new comment, so the test slices the secret-scan job out of the
jobs: mapping and reads the file with comments stripped. Four conformance checks
elsewhere in this portfolio passed because they matched a tool name inside a comment, and
the comment added here names both the action removed and the flag forbidden.

Checks

make verify's Python half runs clean in a fresh worktree: 965 passed, 25 skipped (the
axe-core and Playwright gates skip until make node-sync / make browser-sync; CI runs
both). ruff check, ruff format --check, mypy --strict, zizmor 1.29.0 over
.github/workflows/ (online, no findings) and the repository's own semgrep invocation
(295 rules, 74 files, 0 findings) were all run locally against this branch.

Prepared with AI assistance; reviewed before submission.


Correction, appended 2026-09-13 — this check is required

The statement above that main has no branch protection and no ruleset was true when it was measured and is false now. A protect-main ruleset was created on this repository today, while this PR was being prepared, and it names this job as a required status check.

Measured just now:

gh api repos/ChelseaKR/perimeter/rulesets
  protect-main  active  created=2026-09-13T09:42:33.829-07:00  updated=2026-09-13T09:42:33.883-07:00

gh api repos/ChelseaKR/perimeter/rules/branches/main
  required_status_checks → verify, secret-scan, sast, zizmor, codeql (actions · python · javascript)

Note that GET /repos/.../branches/main/protection still answers 404 Branch not protected. That endpoint reports only classic branch protection and is blind to rulesets, which is how the original claim was reached honestly and still ended up wrong. Rulesets are the mechanism in use across this portfolio; GET /repos/.../rules/branches/main is the endpoint that sees them.

The correction raises the stakes of this PR rather than changing its content: the scan whose range this fixes is not advisory, it gates merges into main.

The original paragraph is left in place rather than edited, so the record shows what was measured and when.

`secret-scan` was `gitleaks/gitleaks-action`, which picks its scan range from
the triggering event rather than scanning the repository. Read at the pinned
SHA, it runs `gitleaks detect --log-opts=--no-merges --first-parent BASE^..HEAD`
on a push and `--log-opts=-1` -- exactly one commit -- when the push carries a
single commit. It drops `--log-opts`, and so reads the whole history, only on
`schedule` and `workflow_dispatch`; `ci.yml` fires on neither. Every merge here
is a squash merge, i.e. a one-commit push, so the job read one commit of 81 and
reported success.

`fetch-depth: 0` did not prevent that and could not: it governs what
actions/checkout puts on disk, not what the scanner is asked to read.

The step is now a pinned, checksum-verified gitleaks 8.30.1 binary invoked as
`gitleaks git . --no-banner --redact --exit-code 1`, with no `--log-opts`, so it
walks every commit reachable from HEAD on every event. `pull-requests: read` and
the action's `GITHUB_TOKEN` are dropped: both existed only so the action could
list a pull request's commits to scope the scan.

The job id and display name are unchanged, because `secret-scan` is a required
context in .github/rulesets/main.json.

tests/test_secret_scan_reads_history.py holds the invocation, reading ci.yml
with comments stripped so the comment naming the removed action cannot satisfy
it.
@ChelseaKR
ChelseaKR merged commit 874107f into main Sep 13, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant