diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d0d511..9f0d978 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,40 @@ included. copy-paste surface in `examples/`. Nothing below requires a re-pin; the `examples/` changes require a re-copy. +- **The drift detector caught its first new control, and the cause was partly this repo.** + On 2026-08-02 the scheduled sweep went red: `30 … 29 allowlisted; 1 new` + ([run 30739082151](https://github.com/hseshadr/ci/actions/runs/30739082151)); the day + before it read `29 … 0 new`. The new one was `aml-filter/ci.yml/secret-scan`, from + [aml-filter#89](https://github.com/hseshadr/aml-filter/pull/89) — a PR closing a real hole + (gitleaks ran only in a weekly sweep, so a secret could merge and sit in public history + for up to seven days) that closed it by **inlining `gitleaks/gitleaks-action`, the exact + control `secret-scan.yml` publishes, at the identical pinned SHA**. Part habit, but two + causes were ours and are fixed here: **`examples/aml-filter/ci.yml` carried no + secret-scan job**, so the worked example for the very file being edited had nothing to + copy (it does now); and **nothing warned that adopting a reusable workflow renames its + check run** to ` / `, which silently breaks a required status + check named after the old inline job — a cost paid by the adopter and invisible to + whoever published the brick. It now has its own README section. A **third** cause turned + up while converging: this repo tags `ci-vX.Y.Z` while third-party actions tag `vN`, so a + consumer that lints its pinned-`uses:` comments with `^v\d` **rejects a correct + `hseshadr/ci` pin** — that is what reddened + [aml-filter#93](https://github.com/hseshadr/aml-filter/pull/93) on its first run, on + aml-filter's own supply-chain test. Also documented, with the tightening fix (key the + expected scheme off the ref; do not relax the regex). The consumer is converging rather + than being exempted; the allowlist entry is a pointer to that open PR and is marked for + deletion when it lands. +- **An allowlist entry covers one control, not the file it lives in** + (`tests/consumer-drift-cases.sh`). Already true, now pinned — the 08-02 finding depended + on it. `aml-filter/ci.yml/frontend-gate` had been allowlisted since 07-26; had the key + been read at file granularity, that older entry would have swallowed the new secret-scan + control and reported a clean run on the day it mattered most. Proven by mutation: + widening `allowlist_index` to match on `/` makes the detector report + `2 allowlisted; 0 new` on a fixture holding one known and one brand-new control, and the + new cases go red. +- **The allowlist header no longer claims "nothing here is new drift."** That sentence was + true for seven days. One entry now *is* new drift, the header says so, and it records the + 30 → 29 → 30 reconciliation (07-26 included one classifier false positive, 07-31 deleted + it, 08-02 added one real control) so adjacent counts stop reading as contradictory. - **A production Ed25519 signing seed could survive on the runner** (`examples/aml-filter/deploy.yml`). The example decoded the seed to `/tmp` and `shred`ed it on the **last line of the same `run:` block** — after a bundle-verification step whose diff --git a/README.md b/README.md index 5b2feb7..d35bb1e 100644 --- a/README.md +++ b/README.md @@ -131,6 +131,54 @@ secrets. Ready-to-copy callers for all seven consumer repos live in full commit SHA, never a moving `@ci-vN` tag; see [Version pinning](#version-pinning-full-commit-shas) for why. +### Adopting a reusable workflow renames its check run + +**Read this before converging a repo that has branch protection.** GitHub names a +reusable workflow's check run ` / `, not after the +caller job alone. So replacing an inline job called `gitleaks` with + +```yaml + gitleaks: + name: Secret scan + uses: hseshadr/ci/.github/workflows/secret-scan.yml@ # ci-v3.0.0 +``` + +produces a check named **`Secret scan / gitleaks`**. The old `gitleaks` context stops +reporting entirely. If it was a *required* status check, every PR then blocks on a context +that can never arrive — the repo looks broken and the obvious fix looks like "revert the +adoption". This repo's own dogfood job shows the effect: its check run is +`Secret scan (own brick) / gitleaks`. + +Update branch protection in the same move: + +```bash +gh api repos/hseshadr//branches/main/protection/required_status_checks \ + --jq '.checks' # see the current contexts first +``` + +This is a real cost of adoption and it is worth naming plainly, because it is paid by the +person converging and invisible to the person who published the brick. It is one reason a +hand-rolled copy keeps winning: inlining never renames anything. + +### Our releases are `ci-vX.Y.Z`, and that can trip a consumer's own pin guard + +Third-party actions tag `vN`; this repo tags `ci-vN.N.N`, so the trailing comment on a +first-party pin reads `# ci-v3.0.0`, not `# v3.0.0`. A consumer that lints its own pinned +`uses:` comments with a `^v\d` regex will **reject a correct `hseshadr/ci` pin** — and the +only way to satisfy that regex is to write a comment naming a tag that does not exist. + +This is not hypothetical: it is what turned [aml-filter#93](https://github.com/hseshadr/aml-filter/pull/93) +red on its first run, on the repo's own supply-chain test. The fix belongs in the guard, +and it should *tighten*, not loosen — key the expected scheme off the ref, so neither +naming convention is accepted for the other: + +```ts +const expected = target.startsWith("hseshadr/ci/") ? /^ci-v\d/ : /^v\d/; +``` + +If you maintain a consumer with a pin-comment guard, expect to make this edit as part of +adopting anything from here. + ### Before → after (a real consumer) edge-proc's hand-rolled `ci.yml` + `security-audit.yml` was **~89 lines** of the same @@ -523,12 +571,58 @@ carried their own Cloudflare Pages deploy while a reusable one sat here, and one five copies drifted into a fork-PR deploy hole. The bug was in the copy, not in the shared workflow — and nothing was comparing the two. -Today's count: **29 hand-rolled controls across 7 repositories** (almamesh 6, aml-filter 5, +Today's count: **30 hand-rolled controls across 7 repositories** (almamesh 6, aml-filter 6, edge-reco 5, assay 4, edge-proc 3, edgeproc-core 3, privacy-core 3). They are listed individually in `tests/consumer-drift-allowlist.txt`, which is a **convergence backlog, not an exemption list**: every entry requires a written reason, deleting one is free, and *new* drift with no entry fails the build. +#### It caught one, and the cause was partly this repo + +On 2026-08-02 the scheduled sweep went red: +[`30 … 29 allowlisted; 1 new`](https://github.com/hseshadr/ci/actions/runs/30739082151). +The 08-01 run had read `29 … 0 new`, so the window is a day. + +The new control was `aml-filter/ci.yml/secret-scan`. It arrived in +[aml-filter#89](https://github.com/hseshadr/aml-filter/pull/89), a PR that closed a genuine +hole — aml-filter's gitleaks scan ran only in its weekly `security-audit.yml` sweep and +never on a pull request, so a secret could merge and sit in public history for up to seven +days. It closed that hole by inlining `gitleaks/gitleaks-action`: **the exact control this +repo publishes as `secret-scan.yml`, at the identical pinned action SHA.** A security fix +shipped by hand-rolling the shared brick. + +The brick was a genuine drop-in — `secret-scan.yml` takes no required inputs, and +`examples/aml-filter/security-audit.yml` already showed aml-filter calling it. So part of +the cause is habit. But two parts were ours, and both are fixed above: + +1. **`examples/aml-filter/ci.yml` had no secret-scan job.** The worked example this repo + publishes for the exact file being edited offered nothing to copy. +2. **Nothing warned that adopting renames the check run.** aml-filter's branch protection + requires a context named literally `gitleaks`; adopting makes it `Secret scan / gitleaks` + and blocks merges until protection is updated. That cost is invisible until you try it. + It now has [its own section](#adopting-a-reusable-workflow-renames-its-check-run). +3. **Our `ci-vX.Y.Z` release scheme fails a consumer pin-comment guard expecting `^v\d`.** + Found the hard way: it reddened the converging PR on its first run, on aml-filter's own + supply-chain test. Also [documented](#our-releases-are-ci-vxyz-and-that-can-trip-a-consumers-own-pin-guard). + +Each one is small. Together they are three separate taxes on doing the right thing, and +none of them is charged to the person who inlines the action instead. + +A shared brick that only fits repos already shaped like it loses to hand-rolling forever, +so "the consumer should have known" is not an acceptable stopping point. The consumer is +converging to `secret-scan.yml` rather than being granted an exemption; the allowlist entry +is a pointer to that open PR and is marked for deletion when it lands. + +**On the Dagger question:** a 2026-07-31 decision not to adopt dagger.io set a disconfirming +test — *attempt the convergence sweep, and if new hand-rolled controls reappear within 60 +days, the model is the problem, not the backlog.* This episode is **not** that test firing. +Its precondition was never met: no drafted caller had been adopted, so nothing could +"reappear" after converging, and one new control cannot tell "the model is wrong" apart +from "nobody has run the sweep yet". The narrower true statement is that drift accrued while +convergence had not started. One new control out of thirty, from a PR fixing a real security +gap, is a single data point and does not re-open Dagger on its own — it is recorded so the +next one lands on a record instead of a blank page. + Two failure modes that used to read as success are now failures: a sweep that inspected **zero** repositories exits `2` rather than reporting a clean bill of health, and a scheduled run whose API token is missing **fails** instead of exiting 0 with a notice. (A @@ -576,7 +670,7 @@ everything to resolve. When the repo is public this is automatic. This is the **target** mapping — what each repo should call once migrated — not current adoption. Today only 6 call-sites exist, all on the publish path (see [Status](#hseshadrci--one-home-for-the-portfolios-cicd)), and the gap between this -table and reality is measured: **29 hand-rolled controls across these 7 repos** +table and reality is measured: **30 hand-rolled controls across these 7 repos** (see [Consumer drift](#consumer-drift-what-is-still-hand-rolled)). Adopted cells are in **bold**; everything else is still the target. @@ -664,9 +758,12 @@ An honest self-assessment against a publish-readiness checklist: broken references that actionlint and zizmor passed. See [Guards that run in CI](#guards-that-run-in-ci). - **The gap to full adoption is measured, not guessed** — ⚠️ **6** call-sites across 4 - repos today, all on the publish path, against **29** hand-rolled controls still standing - across 7 repos. Every one of the 29 is itemized with a reason in - `tests/consumer-drift-allowlist.txt`, and new drift fails the build. + repos today, all on the publish path, against **30** hand-rolled controls still standing + across 7 repos. Every one of the 30 is itemized with a reason in + `tests/consumer-drift-allowlist.txt`, and new drift fails the build — which it did, on + 2026-08-02, catching one it had never seen before + ([details](#it-caught-one-and-the-cause-was-partly-this-repo)). The gap is also **growing + slightly faster than it is closing**: 29 on 07-31, 30 on 08-02, zero converged in between. - **Live-validated end-to-end** — ✅ **for the publish path** (2026-07-22): privacy-core [run 29886074787](https://github.com/hseshadr/privacy-core/actions/runs/29886074787) (npm `v0.2.1` through cross-repo `ts-publish.yml`) and assay diff --git a/examples/aml-filter/ci.yml b/examples/aml-filter/ci.yml index 03b16a0..c122ad9 100644 --- a/examples/aml-filter/ci.yml +++ b/examples/aml-filter/ci.yml @@ -1,8 +1,16 @@ -# aml-filter — .github/workflows/ci.yml (frontend-only; STAYS bespoke) -# The single frontend job interleaves a model-weights cache (from a GitHub release), -# a Playwright cache, and 3 e2e lanes — all stateful `uses:` steps a reusable -# workflow can't own. It still re-uses every bit of setup/caching from hseshadr/ci; -# only the `pnpm run gate` line is repo-specific. +# aml-filter — .github/workflows/ci.yml +# +# Two jobs: a shared secret scan, and one irreducibly bespoke frontend job. +# +# The frontend job interleaves a model-weights cache (from a GitHub release), a +# Playwright cache, and 3 e2e lanes — all stateful `uses:` steps a reusable +# workflow can't own, so it STAYS bespoke. It still re-uses every bit of +# setup/caching from hseshadr/ci; only the `pnpm run gate` line is repo-specific. +# +# The gitleaks job is NOT bespoke and this example used to omit it, which is how +# aml-filter came to hand-roll one: on 2026-08-01 it added a PR-triggered secret +# scan by inlining `gitleaks/gitleaks-action`, and the reader who went looking for +# the converged shape of THIS file found no secret-scan job to copy. It is here now. name: CI on: push: @@ -11,6 +19,22 @@ on: permissions: contents: read jobs: + # ADOPTING THIS RENAMES THE CHECK RUN. A caller job that `uses:` a reusable + # workflow reports as " / " — here + # "Secret scan / gitleaks", not "gitleaks". If a required status check is named + # after the old inline job, update branch protection in the same move or merges + # will block on a context that can no longer report. See the README section + # "Adopting a reusable workflow renames its check run". + gitleaks: + name: Secret scan + # The reusable workflow lists a PR's commits over the API, which needs + # pull-requests: read. A caller job may raise a scope the read-only + # workflow-level default withholds. + permissions: + contents: read + pull-requests: read + uses: hseshadr/ci/.github/workflows/secret-scan.yml@2a575cd193e2e1fc093ccd26821020538e2547b7 # ci-v3.0.0 + frontend: runs-on: ubuntu-latest timeout-minutes: 30 diff --git a/tests/consumer-drift-allowlist.txt b/tests/consumer-drift-allowlist.txt index 82a6af5..43bc9fc 100644 --- a/tests/consumer-drift-allowlist.txt +++ b/tests/consumer-drift-allowlist.txt @@ -16,8 +16,7 @@ # entry that no longer drifts as stale and warns — it never fails. # # Populated 2026-07-26 from the first live sweep: 30 hand-rolled controls across -# all 7 consumers. Nothing here is new drift; new drift is what this file exists -# to make fail. +# all 7 consumers. # # Corrected 2026-07-31 to 29. One entry (almamesh/deploy.yml/secret-scan) was a # CLASSIFIER FALSE POSITIVE — the word "gitleaks" appears there only inside a @@ -25,6 +24,19 @@ # reasons asserted something untrue and were rewritten; a reason nobody can trust # is worse than no allowlist, because it is what people read before deciding not # to converge. Every reason below has been checked against the live repo. +# +# 30 again on 2026-08-02, and this time the extra one IS new drift: +# aml-filter/ci.yml/secret-scan. Until then this header said "nothing here is new +# drift", which was true for exactly seven days and is the kind of sentence that +# quietly stops being true. It is not claimed any more — read each entry instead. +# The count went 30 (07-26, one of them bogus) -> 29 (07-31, bogus one deleted) +# -> 30 (08-02, one genuinely new). Only the last step is real drift. +# +# A NEW ENTRY IS A CONFESSION, NOT A DECISION. It records that a control got +# hand-rolled and says what is being done about it. If you are adding one, the +# reason has to survive a stranger asking "why didn't you just adopt the brick?" — +# and if the honest answer is that the brick did not fit, then the finding is +# about hseshadr/ci and the fix belongs there, not in this file. # --- pages-deploy (4) -------------------------------------------------------- # The control this detector was built for. Each of these builds a dist dir and @@ -35,12 +47,19 @@ edge-reco/deploy.yml/pages-deploy|Converged caller already drafted at examples/e almamesh/deploy.yml/pages-deploy|Bespoke: builds the Pyodide runtime + models and re-verifies deployed source identity after upload. No examples/almamesh/deploy.yml exists yet — converge after aml-filter and edge-reco land. aml-filter/publish-watchlist.yml/pages-deploy|Not a site deploy: signs a watchlist bundle with a decoded key, then pushes it to the same Pages project. Needs a signing-aware pre-deploy hook before it can call the reusable workflow. -# --- secret-scan (7) --------------------------------------------------------- +# --- secret-scan (8) --------------------------------------------------------- # gitleaks runs as a step inside an existing job rather than as a job calling # secret-scan.yml. Converging means splitting a job out — mechanical, but it # touches every consumer, so it is batched deliberately rather than forgotten. almamesh/test.yml/secret-scan|Uses gitleaks/gitleaks-action directly instead of calling secret-scan.yml. aml-filter/security-audit.yml/secret-scan|Uses gitleaks/gitleaks-action directly; examples/aml-filter/security-audit.yml already shows the converged caller. +# THE ONLY ENTRY HERE THAT IS NOT FROM THE ORIGINAL 2026-07-26 SWEEP, and the one +# most likely to be read as this file doing the thing it says it never does. +# Added 2026-08-02 as a POINTER TO AN OPEN PR, not as an exemption: the fix is +# already written and cannot be merged from the same change that records it. +# Delete this line when that PR lands — the detector reports a stale entry as a +# warning, never a failure, so deleting it early costs nothing. +aml-filter/ci.yml/secret-scan|NEW on 2026-08-02, and being converged, not exempted — see aml-filter#93. Origin: aml-filter#89 (2026-08-01) closed a real hole (gitleaks ran only in the weekly security-audit.yml sweep, never on a PR, so a secret could merge and sit in public history for up to seven days) and closed it by inlining gitleaks/gitleaks-action at the same pinned SHA secret-scan.yml already uses. Three causes on OUR side, all addressed: examples/aml-filter/ci.yml carried no secret-scan job, so the worked example for the exact file being edited had nothing to copy; nothing warned that adopting renames the check run to "Secret scan / gitleaks", which breaks a required status check named "gitleaks"; and our ci-vX.Y.Z release scheme fails a consumer pin-comment guard that expects ^v\d, which is what actually reddened aml-filter#93 on its first run. DELETE THIS ENTRY when aml-filter#93 merges. assay/ci.yml/secret-scan|Uses gitleaks/gitleaks-action directly; assay has no converged examples/assay/ci.yml yet. edge-proc/ci.yml/secret-scan|Uses gitleaks/gitleaks-action directly; examples/edge-proc/ci.yml already shows the converged caller. edge-reco/ci.yml/secret-scan|Uses gitleaks/gitleaks-action directly; examples/edge-reco/ci.yml already shows the converged caller. diff --git a/tests/consumer-drift-cases.sh b/tests/consumer-drift-cases.sh index c83fb10..4310665 100755 --- a/tests/consumer-drift-cases.sh +++ b/tests/consumer-drift-cases.sh @@ -78,6 +78,20 @@ expect_output_contains() { report "$description: report never mentioned '$needle'" } +# One report ROW must carry both needles. Column-width-independent on purpose: +# asserting the padded string would make a cosmetic table change read as a +# behaviour change, and would let "control X somewhere, verdict Y somewhere else" +# pass as "control X has verdict Y". +expect_row() { + local control="$1" verdict="$2" description="$3" + shift 3 + local output + output="$("$@" 2>&1)" + awk -v c="$control" -v v="$verdict" \ + 'index($0, c) && index($0, v) { hit = 1 } END { exit !hit }' <<< "$output" || + report "$description: no row shows '$control' with '$verdict'" +} + # --- fixtures --------------------------------------------------------------- write_fixture drifty/deploy.yml < "$reasonless_allowlist" +# Covers ONE of the two controls in halfknown/ci.yml. The other must still fail. +partial_allowlist="$fixtures/partial-allowlist.txt" +printf 'halfknown/ci.yml/dependency-audit|known, scheduled for convergence\n' \ + > "$partial_allowlist" + expect_exit 1 "new drift fails the run" \ "$detector" --local "$fixtures" --consumers "drifty" --allowlist "$empty_allowlist" expect_exit 0 "allowlisted drift passes the run" \ @@ -411,6 +451,18 @@ expect_output_contains "inspected 0 of 1" "a sweep that inspected nothing says s expect_exit 2 "an allowlist entry with no reason is a tool error" \ "$detector" --local "$fixtures" --consumers "drifty" --allowlist "$reasonless_allowlist" +# The 2026-08-02 regression, pinned in both polarities: an entry for one control +# in a file must not suppress a DIFFERENT control in that same file, and must +# still suppress its own. +expect_exit 1 "an allowlisted control does not cover a second control in the same file" \ + "$detector" --local "$fixtures" --consumers "halfknown" --allowlist "$partial_allowlist" +expect_row "secret-scan" "DRIFT (NEW)" \ + "the un-allowlisted control in a partly-allowlisted file is named as NEW" \ + "$detector" --local "$fixtures" --consumers "halfknown" --allowlist "$partial_allowlist" +expect_row "dependency-audit" "DRIFT (allowlisted)" \ + "the allowlisted control in that same file is still suppressed" \ + "$detector" --local "$fixtures" --consumers "halfknown" --allowlist "$partial_allowlist" + # The checked-in allowlist has to survive the same parser, mandatory reasons and # all — a malformed one would otherwise only surface on the day it is consulted. expect_exit 0 "the checked-in allowlist parses" \