fix(rulesets): retire four constraints in the canon template; correct the O5 row - #878
Merged
Merged
Conversation
Contributor
|
Warning Review limit reachedNext included review available in 33 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
hyperpolymath
added a commit
that referenced
this pull request
Sep 20, 2026
… open) (#881) Small follow-up to #878. That PR recorded 262318 and 1658531 as unnameable; public app metadata (`GET /apps/{slug}` returns the app `id`) resolves **1658531 = Linear Code**, which holds `always` bypass on all 43 `EstateBranching` applications — an app with standing push rights over every default branch in the org, sitting outside the §7.3 keep list. It also lets the keep-list ids be *confirmed* rather than trusted: 46505 gitguardian, 347564 coderabbitai, 1236702 claude, 2538504 oikosbot all match. Five ids stay open — 1561, 85455, 262318, 946600, 3953924 — with their measured bypass modes now written into the row, including the detail that 1561 and 3953924 hold `always` on `deed-ecosystem`'s *tag* ruleset (tag rewriting). Installations endpoints are 403/404 for this token, so naming them is a Settings → Applications lookup. Docs only; no ruleset was touched (writes are 404 for this token, and the remediation bodies in `estate-audit/ruleset-repair/` remove grants, not apps).
hyperpolymath
added a commit
that referenced
this pull request
Sep 21, 2026
…rection + what was applied tonight (#883) ## Why `config/rulesets/README.adoc` (added in #878) records that `PATCH /orgs/ORG/rulesets/ID` is a 404 and that the `metadatastician` `EstateBranching` repair must be an owner session. Both were properties of the token I had, not of the API. ## What happened, factually `PUT /orgs/metadatastician/rulesets/18225024` (`EstateBranching`, active, `~DEFAULT_BRANCH` x `~ALL`) returned **200** after `?dry-run=true` returned 200. Removed: rules `code_coverage`, `code_quality`; bypass actors `827041` (gitar-bot) and `1658531` (Linear Code). Preserved verbatim: `conditions`, `enforcement`, `target`, every other rule and every other actor (`2` always, `5` pull_request, `12526`, `29110`, `1236702`, `262318`, Team `18237148`, `OrganizationAdmin`, `RepositoryMigration`). Re-read confirms it. Because that ruleset is inherited, one write removed gitar-bot's default-branch bypass from **all 43 metadatastician repos** — all 43 grants in the last full sweep were this org ruleset (`source_type: Organization` x43), and no sampled repo-level ruleset grants it. ## The two durable rules 1. **PUT bodies must be read back from the live ruleset, never reconstructed from a snapshot.** My first attempt sent a body whose `conditions` came from my own sweep summary: `422 Invalid property /conditions: data matches no possible input`. The live object carries `repository_name` alongside `ref_name`; the snapshot had dropped it. Any summariser that flattens `conditions` yields bodies that either fail or silently rewrite policy. 2. **A 422 naming a bypass actor is a dangling-grant detector.** `Actor Linear Code integration must be part of the ruleset source or owner organization` proves the app is not installed — no read endpoint says that. It cuts the other way too: actors that validate cleanly *are* installed, which is why `946600` and `262318` were left alone. Stripping a working bypass from a live-but-unnamed app is a decision, not a cleanup. ## Not included `copilot_code_review` (enforced on 152 repos) awaits your ruling. Repo-level `PUT .../rulesets/ID` is 403 for the token that made the org write, so the 96 per-repo entries in the repair plan stay owner-run — and `ruleset-repair/apply-ruleset-repair.sh` now derives bodies from live state and dry-runs each write, because with the frozen bodies it would have 422'd on all 97. Docs-only PR. Co-authored-by: Jonathan D.A. Jewell <jonathan@hyperpolymath.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows the Codecov sweep (#870, #873, #874) into the enforcement layer, where the same apps left a deeper residue.
A.
config/rulesets/Optimus-Extras.jsonstill definedcode_coverage,code_quality,required_deploymentsandcopilot_code_review— the four constraints §7.3 retired and the fourscripts/plan-ruleset-constraint-repair.rbexists to strip. As the only branch-side template in the estate it is why the drift reappears on every manual convergence. Now a singlecode_scanningrule (kept: GitHub-native, no app);name/enforcement/bypass_actorsuntouched, JSON re-parsed in the script.B. Corrects a sentence I merged in #874 — "No active ruleset grants bypass to any uninstalled app". It came from a sweep that trusted
GET /repos/{o}/{r}/rulesets, whose list payload omitsrulesandbypass_actors; 1,026 rulesets came back with empty arrays and looked clean. Re-fetched per ruleset (1,036, all 453 repos): gitar-bot 827041 hasalwaysbypass on themetadatasticianorg rulesetEstateBranching(all 43 repos), 946600 on 56 active rulesets, 1561/85455 on 9 each, maintain-rolealwayson 53. codecov, codacy, renovate, advanced-security: zero. Two more ids nobody can name (262318, 1658531) also holdalwayson that org ruleset.C. Documents that no committed script globs
config/rulesets/, and that this token's ruleset writes are 404 whilePOST/DELETEare accepted — so the fix is PUT bodies for an owner session, not a workflow applier that could drop protection in a delete/create gap. Generated plan (96 repo rulesets + the org one, dry-run validated, live re-read guard) is in the audit workspace:estate-audit/ruleset-repair/apply-ruleset-repair.sh, same semantics asplan-ruleset-constraint-repair.rb.No CI impact: template JSON is consumed by owner-run tooling, and the only repo whose workflow reads
config/rulesets/is the tag canon, which is untouched.