Skip to content

feat(inventory): audit rulesets — the protection layer nothing checked (backend#1681) - #212

Merged
LukasWodka merged 5 commits into
developfrom
feat/1681-rulesets-property
Aug 11, 2026
Merged

feat(inventory): audit rulesets — the protection layer nothing checked (backend#1681)#212
LukasWodka merged 5 commits into
developfrom
feat/1681-rulesets-property

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

What

Adds a ruleset_policy (two kinds) and a per-repo rulesets cell, and makes caller-drift.py assert them.

Why

GitHub runs two protection systems. read_protection() reads the second one — but only to satisfy the seven classic-expressible properties. So the guard can tell that a branch is protected; it can never tell that a ruleset exists, what it permits, or who may bypass it. Grepping all 26 org workflows for a ruleset assertion returns nothing.

Two things rest entirely on that unaudited layer, and both were drifting when measured on 2026-08-11:

  • allowed_merge_methods: ["merge"] on every promotion branch — not expressible in classic protection at all, and the only thing stopping a promotion PR being squash-merged, which collapses the merge-commit ancestry promote-repo.sh's squash guard reads and jams the next prepare. start-training carried no ruleset whatsoever — and its classic protection was fully compliant, so the existing guard reported it conformant.
  • The v* tag trust root.release-python — four engineers — held always bypass on backend, data-ingestors and tracebloc-py-package, two of which publish on the tag (PyPI, GHCR). Nothing could see it; it was found by hand.

Design

  • Reads /rulesets/{id} per ruleset, not the cheaper endpoints. Load-bearing: /rules/branches/{b} — the one read_protection() already calls — omits bypass_actorsentirely (verified against the live payload). Asserting an allowlist from it would assert nothing and report a pass.
  • Classifies on target + rules, never on name.client's tag ruleset is named R8 trust root — protect v* release tags; its five peers use Protect v* release tags (supply-chain trust root). A name-keyed check reports client as missing its trust root.
  • Bypass is an exact allowlist, not a floor — an unexpected actor is the finding. That's what turns a future release-python-style grant into a build failure rather than something an audit stumbles onto.
  • Also asserts enforcement: active (an evaluate ruleset looks protective and enforces nothing) and the required rule types.
  • No divergent state, deliberately: a ruleset that permits a squash merge, or carries an extra bypass actor, is drift — not a documented variant — because that is exactly what this property exists to catch.

Test plan / evidence

checkresult
selftest97 → 111 pass, 0 fail
live run, all 20 repos, real inventory"No drift. Every repo read, every entry matched."
mutation — tag bypass allowlist pointed at a team nobody holds6 findings, one per tag-ruleset repo, each naming the unexpected and the missing actor

New cases assert that a missing ruleset, a squash-permitting one, an evaluate-only one, an extra bypass actor and a stale exemption are each findings; that a differently-named tag ruleset still matches; and that an unreadable read is UNREADABLE, never a silent pass.

One of those tests caught a fail-open I had just written. I added an early return skipping the API read when every cell was exempt — it saves a call and looks harmless, but it makes "this repo has no rulesets" unfalsifiable, and a fully-exempt repo is precisely where an unnoticed ruleset would sit forever. Removed, with the reasoning recorded in the code so it isn't re-added as an optimisation.

Also: the report line now names the ruleset kinds — it previously understated its own coverage.

Sibling of #209 (required_checks). Parent epic: backend#1680.


Note

Medium Risk
Changes org-wide CI governance and live GitHub API behavior for every inventoried repo; misconfiguration or API outages surface as failed conformance runs, but runtime app code is untouched.

Overview
Extends the repo conformance contract so GitHub rulesets are audited alongside classic branch protection—controls that classic APIs and the existing guard could not fully see (merge method on promotion PRs, v* tag bypass allowlists).

repo-inventory.yml adds fleet-wide ruleset_policy for two kinds: promotion merge-commit-only (allowed_merge_methods: [merge] on staging/prod) and v* tag trust root (creation/update/deletion rules with an exact bypass allowlist). Every repo gets a rulesets cell (required or exempt with a written reason).

caller-drift.py loads and validates that policy, fetches each repo’s rulesets via listing plus per-id detail (so bypass actors are real), classifies rulesets by target + rule shape (not name), and flags missing rulesets, non-active enforcement, wrong merge methods, ref/role coverage gaps, extra bypass actors, and stale exemptions. Ruleset API failures use a separate unreadable bucket so they do not abort the run or masquerade as caller/protection failures.

Selftests cover the main drift shapes and schema rules; fully exempt repos still trigger a ruleset read so “no rulesets” stays falsifiable.

Reviewed by Cursor Bugbot for commit aae5659. Bugbot is set up for automated code reviews on this repo. Configure here.

…d (backend#1681)
GitHub runs two protection systems. `read_protection()` reads the second one, but
only to satisfy the seven classic-expressible properties -- so the guard can tell
that a branch is protected, never that a ruleset EXISTS, what it permits, or who
may bypass it. Grepping the org's 26 workflows for a ruleset assertion returns
nothing.
Two things rest entirely on that unaudited layer, and both were drifting:
* `allowed_merge_methods: ["merge"]` on every promotion branch -- not
expressible in classic protection AT ALL, and the only thing stopping a
promotion PR being squash-merged, which collapses the merge-commit ancestry
promote-repo.sh's squash guard reads and jams the next prepare.
start-training carried NO ruleset whatsoever. Its classic protection was
fully compliant, so the existing guard reported it conformant.
* the `v*` tag trust root. `release-python` -- four engineers -- held `always`
bypass on backend, data-ingestors and tracebloc-py-package, two of which
publish ON the tag (PyPI, GHCR). Nothing could see it; it was found by hand.
This adds a `ruleset_policy` of two kinds and a per-repo `rulesets` cell:
* READS `/rulesets/{id}` per ruleset, not the cheaper endpoints. Load-bearing:
`/rules/branches/{b}` -- the one read_protection() already calls -- omits
`bypass_actors` ENTIRELY, so asserting an allowlist from it would assert
nothing and report a pass.
* CLASSIFIES on target + rules, NEVER on name. client's tag ruleset is named
"R8 trust root - protect v* release tags" while its five peers use
"Protect v* release tags (supply-chain trust root)"; a name-keyed check
reports client as missing its trust root.
* BYPASS IS AN EXACT ALLOWLIST, not a floor -- an unexpected actor is the
finding, which is what turns a future release-python-style grant into a
build failure instead of something an audit has to stumble on.
* also asserts enforcement==active (an `evaluate` ruleset looks protective and
enforces nothing) and the required rule types.
No `divergent` state here, deliberately: a ruleset that exists but permits a
squash merge, or carries an extra bypass actor, is drift -- not a documented
variant -- because that is precisely what the property exists to catch.
Verified, not asserted:
* selftest 97 -> 111 pass, 0 fail. The new cases assert that a MISSING
ruleset, a squash-PERMITTING one, an `evaluate`-only one, an EXTRA bypass
actor and a STALE exemption are each findings, that a differently-named tag
ruleset still matches, and that an unreadable read is UNREADABLE rather than
a silent pass.
* One of those tests caught a fail-open I had just written: an early return
skipping the read when every cell was `exempt`. It saves an API call and
makes "this repo has no rulesets" unfalsifiable -- and a fully-exempt repo
is exactly where an unnoticed ruleset would sit forever. Removed, with the
reasoning recorded so it is not re-added as an optimisation.
* LIVE against all 20 repos: "No drift. Every repo read, every entry matched."
* MUTATION: the same run with the tag bypass allowlist pointed at a team
nobody holds produces 6 findings -- one per tag-ruleset repo, each naming
the unexpected and the missing actor. Green means something because red is
reachable.
The report line now names the ruleset kinds too; it previously understated its
own coverage.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodkaLukasWodka self-assigned this Aug 11, 2026
Comment threadscripts/caller-drift.py
…tection one (.github#212)
Two review findings on this PR:
* `quality / ruff` failed E702 (two statements on one line) in a selftest I added.
Fixed; `ruff --isolated --select E4,E7,E9,F scripts/` now passes, which is the
org gate's exact selection.
* Bugbot (Medium): `evaluate_rulesets` appends its read failures into
`protection_unreadable`, but the summary and the `::error::` line both said
"branch-protection read(s) FAILED ... protection state UNKNOWN". A
rulesets-only outage therefore named the wrong control plane and claimed
protection was unknown when it had been read successfully. That is exactly the
honesty this file exists to enforce, so: wording is now "protection/ruleset",
and the bucket carries a comment saying what it holds and why the two share it
(both mean "the caller/copy audit for this repo SUCCEEDED, a different control
plane did not").
Selftest re-run: 111 pass, 0 fail.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

Both findings fixed in a098aad.

quality / ruff (E702) — two statements on one line in a selftest I added. Fixed; ruff --isolated --select E4,E7,E9,F scripts/ now reports All checks passed! locally, using the org gate's exact selection.

Bugbot (Medium) — ruleset failures mislabeled as protection. Correct and worth fixing properly: evaluate_rulesets shares protection_unreadable, but the summary and ::error:: line both claimed "branch-protection read(s) FAILED … protection state UNKNOWN". A rulesets-only outage would have named the wrong control plane and asserted protection was unknown after it had been read successfully — precisely the kind of dishonest reporting this guard exists to prevent.

Wording is now "protection/ruleset", and the bucket carries a comment explaining what it holds and why the two legitimately share it: both mean "this repo's caller/copy audit SUCCEEDED, a different control plane did not", which is the distinction the separate list exists to preserve. Selftest re-run: 111 pass, 0 fail.

Comment threadscripts/caller-drift.py
…cludes_parents=false
Bugbot (.github#212):
- read_rulesets listed rulesets WITHOUT includes_parents=false, so org/enterprise
rulesets folded in and their ids 404'd on the per-ruleset get-by-id, failing the
whole audit. Scope the listing to this repo's own rulesets.
- ruleset read-failures were funnelled into protection_unreadable, so a rulesets-only
outage reported 'protection state UNKNOWN' (and vice versa). Give rulesets their own
unreadable bucket and name each layer separately in the summary + ::error::.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

Comment threadscripts/tests/caller-drift-selftest.py Outdated
@saadqbal

Copy link
Copy Markdown
Collaborator

There is one bugbot issue and checks are failing. Once they are fixed, i will approve

…bot #212)
The listing stubs matched endswith('/rulesets'); read_rulesets now appends the
query string, so strip it before the suffix check. Restores the conforming /
missing / weakened ruleset selftest cases.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 8a5a1c4. Configure here.

saadqbal
saadqbal previously approved these changes Aug 11, 2026
…-property
# Conflicts:
#	scripts/tests/caller-drift-selftest.py
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

@saadqbal merged develop to clear the conflict — please re-approve.

The only conflict was in scripts/tests/caller-drift-selftest.py: an additive clash where this branch appended the rulesets audit test block and develop appended the source-reusable enumeration block at the same spot. Resolved by keeping both (rulesets block first, then source-reusables), two blank lines between them — neither test was dropped. repo-inventory.yml and caller-drift.py auto-merged cleanly.

CI on the merge: selftest ✓ (both blocks run green), ruff ✓, shellcheck/gitleaks/house-rules ✓. Nothing else changed since your approval.

@LukasWodka
LukasWodka merged commit 7716f00 into developAug 11, 2026
11 checks passed
@LukasWodka
LukasWodka deleted the feat/1681-rulesets-property branch August 11, 2026 13:58
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.

2 participants

@LukasWodka@saadqbal