Skip to content

fix(security): restore secret scanning — drop unused actions: read from reusable - #528

Merged
hyperpolymath merged 3 commits into
mainfrom
worktree-fix-secret-scanner-perms
Jul 27, 2026
Merged

fix(security): restore secret scanning — drop unused actions: read from reusable#528
hyperpolymath merged 3 commits into
mainfrom
worktree-fix-secret-scanner-perms

Conversation

@hyperpolymath

Copy link
Copy Markdown
Owner

Restores standards own secret scanning, which has not executed since 2026-07-26.

The outage

Every recent Secret Scanner run on main is startup_failure — zero steps, no log, no check run at all. The repo secret scanning was silently dead, while a live write-scoped credential sits in the tree.

Root cause

0ced540 ("chore: estate-wide security compliance", 2026-07-26) added actions: read to the workflow-levelpermissions: block of secret-scanner-reusable.yml.

No job needs it — the only action used is actions/checkout (×3); there are no artifacts, no cache, no gh api calls, no workflow-run queries. Since #500 replaced gitleaks-action with a pinned checksum-verified binary, pull-requests: write is unnecessary too.

rust-secrets and shell-secrets had no job-level block, so they inherited that unused grant. A called workflow cannot exceed its caller job-level grant, so the correctly least-privileged caller — granting contents: read alone — was rejected at startup.

A sweep named "security compliance" disabled the security scanner.

Fix — all three layers

  1. Workflow-level block reduced to contents: read, restoring parity with the known-good pinned versions (c65436ee, d135b05).
  2. All three jobs declare permissions: contents: read explicitly instead of inheriting. Inheritance is what let an unused grant leak into two jobs unnoticed; explicit blocks mean a future widening cannot silently re-break callers.
  3. The caller-example comment block is corrected. It still instructed wrappers to grant pull-requests: write + actions: read — guidance that was correct only before ci(secret-scanner): make gitleaks gate for real, not continue-on-error #500, and which propagates to every wrapper minted from it. It now documents the minimal correct shape plus the two rules that make this failure mode invisible.

Also removes the caller own unused actions: read, and the incorrect claim that moving to a local path meant "this class of breakage cannot recur" — it recurred by a different route.

Prevents a latent mass outage

326 estate wrappers call this reusable; 156 grant contents: read only. They are green today solely because they pin older, clean SHAs (181 on c65436ee, 83 on d135b05).

Re-pinning them to current main — as the planned pin-refresh sweep would do — would have broken all 156 at once. This must land before any such sweep.

Verification

  • yq parse OK; effective grants confirmed contents: read at workflow level and on each of the three jobs.
  • actionlint clean on the caller. On the reusable the only finding is a pre-existing, intentional SC2053 (glob matching for .shell-secrets-ignore) — count identical before and after (1 → 1).
  • Sampled five contents-only estate callers on their current pins (anamnesis, boj-server, echidnabot, conflow, …): all green, confirming they are insulated rather than already broken.

Draft pending the real proof: the check that matters is this PR own Secret Scanner run going green. A rejected run produces no check run, so absence of red is not evidence — verify with gh run list --json conclusion. This exact failure mode has already survived one "fix".

Refs #500. Fixes the startup_failure introduced by 0ced540.

🤖 Generated with Claude Code

hyperpolymathand others added 2 commits July 27, 2026 13:41
…from reusable
`standards`' own Secret Scanner has been `startup_failure` on every run since
2026-07-26: zero steps, no log, no check run. The repo's secret scanning was
silently not executing — while a live write-scoped credential sits in the tree.
Root cause
----------
`0ced540` ("chore: estate-wide security compliance", 2026-07-26) added
`actions: read` to the *workflow-level* `permissions:` block of
secret-scanner-reusable.yml. No job needs it: the only action used is
`actions/checkout` (x3), and there are no artifacts, no cache, no `gh api`
calls and no workflow-run queries. Since #500 replaced `gitleaks-action` with
a pinned checksum-verified binary, `pull-requests: write` is unnecessary too.
`rust-secrets` and `shell-secrets` had no job-level block, so they inherited
that unused grant. A called workflow cannot exceed its caller's *job-level*
grant, so the correctly least-privileged caller — which grants `contents: read`
alone — was rejected at startup. A sweep named "security compliance" disabled
the security scanner.
Fix (all three layers, not just the symptom)
--------------------------------------------
1. Workflow-level block reduced to `contents: read`, restoring parity with the
known-good pinned versions (c65436e, d135b05).
2. All three jobs now declare `permissions: contents: read` explicitly instead
of inheriting. Inheritance is what let an unused grant leak into two jobs
unnoticed; explicit blocks mean a future widening cannot silently re-break
callers.
3. The caller-example comment block — which still instructed wrappers to grant
`pull-requests: write` + `actions: read`, guidance correct only before #500
— now documents the minimal correct shape, plus the two rules that make this
failure mode invisible: a job-level block REPLACES rather than merges with
the workflow-level one, and a rejected run produces no check run at all
(`gh pr checks` shows nothing, annotations 404).
Also removes the caller's own unused `actions: read` and the incorrect claim
that moving to a local path meant "this class of breakage cannot recur" — it
recurred, by a different route, which is why the history is now recorded there.
Prevents a latent mass outage
-----------------------------
326 estate wrappers call this reusable; 156 grant `contents: read` only. They
are green today solely because they pin older, clean SHAs (181 on c65436e, 83
on d135b05). Re-pinning them to current `main` — as the planned pin-refresh
sweep would have done — would have broken all 156 at once. This fix must land
before any such sweep.
Verification
------------
- `yq` parse OK; effective grants confirmed `contents: read` at workflow level
and on each of the three jobs.
- `actionlint` clean on the caller; on the reusable the only finding is a
pre-existing, intentional SC2053 (glob matching for `.shell-secrets-ignore`),
count identical before and after (1 -> 1).
- Sampled five `contents`-only estate callers on their current pins: all green,
confirming they are insulated rather than already broken.
Refs #500. Fixes the startup_failure introduced by 0ced540.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…y .adoc
The permissions fix in the previous commit made the scanner run again. It then
reported "no leaks found" on a repo containing a live write-scoped GitLab PAT.
This commit fixes the reason.
Root cause
----------
gitleaks' DEFAULT global allowlist excludes paths matching
`(jpg|gif|doc|docx|zip|xls|pdf|bin|svg|socket)$`. The `doc` alternative is an
unanchored substring match on the filename, so it swallows `.adoc` and
`.asciidoc` as collateral — AsciiDoc is a plain-text format and was never the
intended target of a binary-file exclusion.
This estate writes documentation in AsciiDoc by default, so the blind spot
covers ~40,000 files estate-wide (1,620 in standards alone). Proven with
identical content across four extensions:
.txt -> DETECTED .md -> DETECTED
.adoc -> MISSED .asciidoc -> MISSED
That is why 40 occurrences of a live `glpat-` token in
`rhodium-pipeline/docs/QUICKSTART.adoc` were never flagged.
Why not just fix the config
---------------------------
The exclusion cannot be switched off from a config that sets
`[extend] useDefault = true`. Verified: neither `[allowlist] paths = []` nor
adding an explicit custom rule re-enables scanning, because the file is dropped
at path level before any rule executes. Setting `useDefault = false` would
forfeit all ~150 default rules.
So this adds a step that scans a throwaway mirror in which each AsciiDoc file
is copied under a `.txt` suffix — same binary, same rules, same repo config,
only the filename differs, which is the sole thing the allowlist keys on.
Findings are mapped back to real paths before reporting; the mirror never
appears in output. The caller's own `.gitleaks.toml` is passed through when
present, so repo-specific allowlists apply to docs exactly as to code.
Warn-first, with a real date
----------------------------
Uses the same 2026-08-21 cutoff as the docs and package-policy gates
(standards#513), for the same reason: this makes ~40,000 previously unscanned
files visible at once, and every hit is pre-existing debt rather than a
regression in the PR in front of it. It never prints a pass while findings
exist, always lists them, and the date flips itself with no further edit.
Seams `ENFORCE_ADOC_SCAN_FROM` / `ADOC_TODAY` make both branches testable.
Red-teamed in both directions (all measured, not asserted)
----------------------------------------------------------
leak in .adoc, pre-cutoff -> exit 0 + "NOT YET ENFORCED" + findings listed
leak in .adoc, post-cutoff -> exit 1 + ::error::
clean .adoc, post-cutoff -> exit 0 (no false positive)
no .adoc files present -> exit 0
path containing spaces -> exit 1, correct real path reported
malformed date -> exit 1, refuses to run
The clean-docs case initially FAILED with exit 1: `grep` exits 1 on no match,
which under `set -e` + `pipefail` aborted the step, so every repo with clean
AsciiDoc would have been failed by this gate. Caught by red-teaming the passing
case rather than only the failing one; `pipefail` is now disabled around the
count with a comment recording why.
actionlint: no new findings (1 pre-existing SC2053 before and after).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@hyperpolymath

Copy link
Copy Markdown
OwnerAuthor

Second, deeper fault found — and fixed in 49877ee

The permissions fix (e3f982d) worked: the scanner ran green, all three jobs, real steps. It then reported no leaks found on a repo containing a live write-scoped GitLab PAT. So I kept going.

gitleaks cannot see any .adoc file

gitleaks default global allowlist excludes paths matching (jpg|gif|doc|docx|zip|xls|pdf|bin|svg|socket)$. That doc alternative is an unanchored substring match on the filename, so it swallows .adoc and .asciidoc as collateral. AsciiDoc is plain text and was never the intended target of a binary-file exclusion.

Proven with identical content across four extensions:

extensionresult
.txtDETECTED
.mdDETECTED
.adocMISSED
.asciidocMISSED

This estate writes docs in AsciiDoc by default~40,000 files estate-wide, 1,620 in standards alone, none of which has ever been secret-scanned. That is precisely why 40 occurrences of a live glpat- token in rhodium-pipeline/docs/QUICKSTART.adoc were never flagged.

Why the config cannot fix it

The exclusion cannot be switched off while [extend] useDefault = true. Verified: neither [allowlist] paths = [] nor an explicit custom rule re-enables scanning — the file is dropped at path level, before any rule runs. Setting useDefault = false would forfeit all ~150 default rules.

So the new step scans a throwaway mirror in which each AsciiDoc file is copied under a .txt suffix: same binary, same rules, same repo config — only the filename differs, which is the sole thing the allowlist keys on. Findings are mapped back to real paths before reporting.

Warn-first, same cutoff as #513

This makes ~40,000 previously unscanned files visible at once, and every hit is pre-existing debt, not a regression from the PR in front of it. So it uses the established 2026-08-21 cutoff. It never prints a pass while findings exist, always lists them, and the date flips itself.

Red-teamed both directions

caseresult
leak in .adoc, pre-cutoffexit 0 + NOT YET ENFORCED + findings listed
leak in .adoc, post-cutoffexit 1
clean .adoc, post-cutoffexit 0 — no false positive
no .adoc presentexit 0
path containing spacesexit 1, correct real path
malformed dateexit 1, refuses to run

The clean-docs case initially FAILED (exit 1).grep exits 1 on no match, which under set -e + pipefail aborted the step — so every repo with clean AsciiDoc would have been failed by this gate. Caught only because I red-teamed the passing case, not just the failing one. Fixed, with the reason recorded in-line.

actionlint: no new findings (1 pre-existing SC2053 before and after).


This does not un-leak the token. The credential in QUICKSTART.adoc is still live and still public — it must be revoked at gitlab.com, then purged from history. This PR only ensures the next one gets caught.

@hyperpolymath
hyperpolymath marked this pull request as ready for review July 27, 2026 12:56
@hyperpolymath
hyperpolymath merged commit 20f33bd into mainJul 27, 2026
19 of 20 checks passed
@hyperpolymath
hyperpolymath deleted the worktree-fix-secret-scanner-perms branch July 27, 2026 13:04
@sonarqubecloud

Copy link
Copy Markdown

hyperpolymath added a commit that referenced this pull request Jul 27, 2026
Removes the dead credential from the working tree. **Owner revoked the
token 2026-07-27.**
## What was there
Committed **2026-02-22** ("Auto-commit: Sync changes"), **40
occurrences** in one file, on **public** `main` for five months.
Replaced with `glpat-<REVOKED-TOKEN-REDACTED>` rather than deleting the
surrounding documentation, which is otherwise legitimate.
| check | before | after |
|---|---|---|
| live-token occurrences in file | 40 | **0** |
| whole-tree grep for a live PAT | 1 file | **no matches** |
| AsciiDoc mirror scan findings | 72 | **0** |
| files changed | — | 1 |
## ⚠ History is deliberately NOT rewritten — please read
A rewrite was authorised. I measured it before running it, and the cost
now outweighs the benefit:
- The estate carries **2,021 pinned references** to **32 distinct
`standards` commit SHAs** (`uses:
hyperpolymath/standards/.github/workflows/*.yml@<sha>`).
- Those SHAs are **real commits in this history** — verified, e.g.
`c65436ee` (181 wrappers) and `d135b05` (83 wrappers).
- **A rewrite orphans every one of them.** Unreachable commits are
GC-able, and once collected every pinned caller fails to resolve its
`uses:` — precisely the estate-wide `startup_failure` class this repo
has spent the week eliminating.
- **The token is already revoked**, so a rewrite now buys hygiene, not
security.
A dead credential sitting in history is the cheaper problem than 2,021
broken workflow references.
If you still want the rewrite, it should be **sequenced with a
pin-refresh sweep** across all 2,021 references rather than done
standalone. A **full backup bundle of all refs was taken** before this
was evaluated and is retained.
## Note
`standards` already carries **zero** Nix artefacts (no
`flake.nix`/`flake.lock`/`default.nix`/`shell.nix`) — it is already
Guix-only, with `guix.scm` plus 8 manifests. Nothing to retire here.
Refs #528 — this is the leak that scanner could not see until the
AsciiDoc blind spot was closed.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Signed-off-by: Jonathan D.A. Jewell <6759885+hyperpolymath@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for freeto 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

@hyperpolymath