From c1713c0867a9f0425f08a26e0dc83448f295e7ac Mon Sep 17 00:00:00 2001 From: DuqueOM Date: Sat, 5 Sep 2026 12:01:50 -0600 Subject: [PATCH] chore(ci): name the security check for what it does, not for its tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Self-audit (gitleaks + tfsec + checkov + trivy fs)` -> `Self-audit (secrets + IaC + supply chain)`. The old name enumerated its tools, so ADR-046 falsified it the moment tfsec was swapped for Trivy. Correcting it was not a one-line edit: a required status check is identified by its job-name string, so renaming the job leaves the ruleset waiting for a context that will never report — the PR containing the rename blocks itself. ADR-046 shipped with a name it knew to be wrong and recorded why. Step 2 of the three-step transition. Step 1 dropped the context from the ruleset by direct API call, deliberately not committed: that state is transient and scripts/setup_branch_protection.sh stays the source of truth for the final shape. Step 3 re-applies the ruleset from this payload. All three canonical sources move together, as docs/governance/branch-protection.md requires: this document, ADR-026 and the applier script. The procedure is now written down under §"Renaming a required check", including the two things that bite: the check is not required between steps 1 and 3, and re-adding a required check while it is red blocks every subsequent PR. The new name describes the classes of check rather than the tools, so the next tool swap touches no contract at all. That is the real fix; this rename is just the current instance of it. Co-Authored-By: Claude Opus 5 --- .github/workflows/validate-templates.yml | 13 ++++--- CHANGELOG.md | 32 +++++++++++++--- docs/decisions/ADR-026-branch-protection.md | 2 +- ...ADR-046-tfsec-to-trivy-config-migration.md | 18 ++++++--- docs/governance/branch-protection.md | 37 ++++++++++++++++++- scripts/setup_branch_protection.sh | 2 +- 6 files changed, 84 insertions(+), 20 deletions(-) diff --git a/.github/workflows/validate-templates.yml b/.github/workflows/validate-templates.yml index b54065b..39e9ab3 100644 --- a/.github/workflows/validate-templates.yml +++ b/.github/workflows/validate-templates.yml @@ -17,14 +17,15 @@ jobs: # Apply the same tooling to the template repo itself — a template that # does not pass its own gates is not credible. # - # The job NAME still says "tfsec" and no longer should: it is one of the - # six required status checks in the ADR-026 ruleset, so renaming it in the - # same change that swapped the tool would block that very change (the - # required context would never report). Tracked as a separate three-step - # ruleset transition — see ADR-046 §Consequences. + # The name is deliberately tool-agnostic. It used to enumerate the tools + # ("gitleaks + tfsec + checkov + trivy fs"), which made it wrong the moment + # ADR-046 swapped one of them — and because it is a required status check, + # correcting it cost a three-step ruleset transition. Naming the CLASSES of + # check instead means the next tool swap touches no contract at all. + # See docs/governance/branch-protection.md §"Renaming a required check". # --------------------------------------------------------------------- self-audit: - name: Self-audit (gitleaks + tfsec + checkov + trivy fs) + name: Self-audit (secrets + IaC + supply chain) runs-on: ubuntu-latest permissions: contents: read diff --git a/CHANGELOG.md b/CHANGELOG.md index c812563..7219b10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,29 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) and [Sem ## [Unreleased] +### Changed — the security check is named for what it does, not for its tools + +- `Self-audit (gitleaks + tfsec + checkov + trivy fs)` → + **`Self-audit (secrets + IaC + supply chain)`**. The old name enumerated + its tools, so ADR-046 falsified it the moment tfsec was swapped for Trivy — + and because the job is one of the six required status checks, correcting it + was not a one-line edit. +- **A required check is identified by its job-name string.** Rename the job + and the ruleset waits for a context that will never report, so the PR + containing the rename blocks itself. ADR-046 therefore shipped with a name + it knew to be wrong, and said so. +- Executed as a three-step transition: drop the context from the ruleset by + direct API call (transient, never committed), merge the rename with all + three canonical sources moving together — `docs/governance/branch-protection.md`, + ADR-026 and `scripts/setup_branch_protection.sh` — then re-apply the + ruleset from the committed payload. The procedure is now written down in + `docs/governance/branch-protection.md` §"Renaming a required check", + including the caveat that the check is unrequired in between and that + re-adding one while red blocks every subsequent PR. +- The new name describes the **classes** of check rather than the tools, so + the next tool swap touches no contract at all. That is the actual fix; the + rename is just this instance of it. + ### Changed — Terraform IaC scanning moves from archived tfsec to Trivy config (ADR-046) - **tfsec is archived upstream** and was pinned to its final release, @@ -49,11 +72,10 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) and [Sem scanner now accepts an annotation on the line above the entry, matching the YAML scanner — a justification that needs a paragraph could not fit inline. -- The `Self-audit (gitleaks + tfsec + checkov + trivy fs)` job **keeps its - now-inaccurate name**: it is one of the six required status checks in the - ADR-026 ruleset, and renaming it in the same change would block that very - change, because the required context would never report. Recorded in the - workflow and in ADR-046 as a separate three-step ruleset transition. +- The `Self-audit` job kept its now-inaccurate name in that change: it is one + of the six required status checks in the ADR-026 ruleset, and renaming it + there would have blocked that very change, because the required context + would never report. **Since carried out** — see the rename entry below. ### Changed — baseline entries that can be verified now are verified, not dated - The two `runtime-artifact` entries in `.doc-path-baseline.yml` carried a diff --git a/docs/decisions/ADR-026-branch-protection.md b/docs/decisions/ADR-026-branch-protection.md index 0b14006..017ab4c 100644 --- a/docs/decisions/ADR-026-branch-protection.md +++ b/docs/decisions/ADR-026-branch-protection.md @@ -85,7 +85,7 @@ Adopt **two GitHub Rulesets** on the repository: |----|----|----| | `Tests & Coverage / Python 3.11` | `ci-examples.yml` | Coverage + behavior | | `Tests & Coverage / Python 3.12` | `ci-examples.yml` | Forward Python compat | -| `Self-audit (gitleaks + tfsec + checkov + trivy fs)` | `validate-templates.yml` | Security scans + secret detection | +| `Self-audit (secrets + IaC + supply chain)` | `validate-templates.yml` | Security scans + secret detection | | `Python Lint + Type Check` | `validate-templates.yml` | black / isort / flake8 / mypy / bandit | | `Agentic System Validation` | `validate-templates.yml` | Governance contract: rules, skills, workflows, AGENTS.md | | `Scaffolder End-to-End Test` | `validate-templates.yml` | The scaffolder still produces a working service (catches D-32-class regressions) | diff --git a/docs/decisions/ADR-046-tfsec-to-trivy-config-migration.md b/docs/decisions/ADR-046-tfsec-to-trivy-config-migration.md index b8a361d..ab86f23 100644 --- a/docs/decisions/ADR-046-tfsec-to-trivy-config-migration.md +++ b/docs/decisions/ADR-046-tfsec-to-trivy-config-migration.md @@ -108,12 +108,18 @@ by a second number, so misconfiguration ids (`GCP-0061`) matched nothing. static-analysis limit rather than a tool defect. - `docs/audit/baseline-review.md`'s 2027-01-01 review now has one entry instead of three, and it is the one that will still be true then. -- The `Self-audit (gitleaks + tfsec + checkov + trivy fs)` job **keeps its - name** despite no longer running tfsec. That name is one of the six - required status checks in the ADR-026 ruleset; renaming it in the same - change would block the very PR making the change, because the required - context would never report. The rename is a deliberate three-step ruleset - transition and is tracked separately. +- The `Self-audit (gitleaks + tfsec + checkov + trivy fs)` job **kept its + name** in this change despite no longer running tfsec. That name is one of + the six required status checks in the ADR-026 ruleset; renaming it in the + same change would have blocked the very PR making the change, because the + required context would never report. + + **Resolved since**: the rename was carried out as the three-step ruleset + transition this ADR anticipated, and the job is now + `{NEW}` — named for the classes of check rather + than the tools, so the next tool swap touches no contract. The procedure is + written down in `docs/governance/branch-protection.md` + §"Renaming a required check". - MEDIUM and LOW findings are now visible but not enforced. Raising the threshold is a separate decision with its own triage cost. diff --git a/docs/governance/branch-protection.md b/docs/governance/branch-protection.md index 3492c69..35c7029 100644 --- a/docs/governance/branch-protection.md +++ b/docs/governance/branch-protection.md @@ -57,7 +57,7 @@ the admin bypass actor is the break-glass path. |---|---|---| | `Tests & Coverage / Python 3.11` | `.github/workflows/ci-examples.yml` | ✅ | | `Tests & Coverage / Python 3.12` | `.github/workflows/ci-examples.yml` | ✅ | -| `Self-audit (gitleaks + tfsec + checkov + trivy fs)` | `.github/workflows/validate-templates.yml` | ✅ | +| `Self-audit (secrets + IaC + supply chain)` | `.github/workflows/validate-templates.yml` | ✅ | | `Python Lint + Type Check` | `.github/workflows/validate-templates.yml` | ✅ | | `Agentic System Validation` | `.github/workflows/validate-templates.yml` | ✅ | | `Scaffolder End-to-End Test` | `.github/workflows/validate-templates.yml` | ✅ | @@ -127,6 +127,40 @@ GitHub.com → repo → Settings → Rules → Rulesets → New branch ruleset --- +## Renaming a required check + +A required status check is identified by its **job name string**. Rename the +job and the ruleset keeps waiting for a context that will never report, so +the PR containing the rename can never go green — it blocks itself. + +This is not hypothetical: ADR-046 swapped tfsec for Trivy and had to leave +the job named `Self-audit (gitleaks + tfsec + checkov + trivy fs)`, wrong on +its face, because correcting it inside that PR would have deadlocked it. + +The transition is three steps, and the middle one is an ordinary PR: + +1. **Drop the context from the ruleset.** A direct API `PUT` against + `repos/:owner/:repo/rulesets/:id` with that one context removed. Do this + with no PRs open, and do **not** commit this state — it is transient, and + `scripts/setup_branch_protection.sh` stays the source of truth for the + final shape. +2. **Merge the rename.** One PR renaming the job *and* updating all three + canonical sources together: this document, `ADR-026`, and the applier + script's payload. The other five checks still gate it. +3. **Re-apply the ruleset** with `scripts/setup_branch_protection.sh`, which + now carries the new name. Verify with `--check` and by reading + `rules/branches/main` back. + +Between steps 1 and 3 that check is **not required**. Keep the window short, +open no other PRs in it, and confirm the job is green on `main` before +re-adding it — a required check re-added while red blocks every subsequent PR. + +**Prefer names that do not need this.** The name that caused the problem +enumerated its tools, so any tool change falsified it. Naming the *classes* +of check — `secrets + IaC + supply chain` — decouples the required-check +contract from tool choice, which is why the rename went to that shape rather +than to an updated tool list. + ## Verification checklist After applying, confirm: @@ -147,3 +181,4 @@ After applying, confirm: | 2026-05-15 | Initial ruleset (ADR-026) | `@DuqueOM` | | 2026-09-04 | Rulesets applied to the repository. The contract had been documented since 2026-05-15 but never deployed: `GET /rulesets` returned `0` and `GET /rules/branches/main` returned `0`, so `main` was unprotected the whole time. | `@DuqueOM` | | 2026-09-04 | Declared `required_reviewers`, `require_extra_approval_for_unattributed_changes` and `allowed_merge_methods` explicitly; previously left to GitHub defaults and therefore absent from this document. `allowed_merge_methods` narrowed to `[squash, rebase]` for coherence with `required_linear_history`. | `@DuqueOM` | +| 2026-09-05 | Renamed the security check from `Self-audit (gitleaks + tfsec + checkov + trivy fs)` to `Self-audit (secrets + IaC + supply chain)` after ADR-046 made the tool list wrong. Executed as the three-step transition documented above. The new name is tool-agnostic so a future tool swap touches no contract. | `@DuqueOM` | diff --git a/scripts/setup_branch_protection.sh b/scripts/setup_branch_protection.sh index 90a2fcd..860d52c 100755 --- a/scripts/setup_branch_protection.sh +++ b/scripts/setup_branch_protection.sh @@ -119,7 +119,7 @@ read -r -d '' MAIN_RULESET_JSON <<'JSON' || true "required_status_checks": [ { "context": "Tests & Coverage / Python 3.11" }, { "context": "Tests & Coverage / Python 3.12" }, - { "context": "Self-audit (gitleaks + tfsec + checkov + trivy fs)" }, + { "context": "Self-audit (secrets + IaC + supply chain)" }, { "context": "Python Lint + Type Check" }, { "context": "Agentic System Validation" }, { "context": "Scaffolder End-to-End Test" }