feat(rulesets): relocate release-channel-tags to .github as targeted org standard (#596)#609
Conversation
…org standard (#596) Make release-channel-tags a codified org standard owned by .github (per #596: standards live only in .github), and generalize it from an enumerated namespace list to a single generic rule. - standards/rulesets/release-channel-tags.json: new. Generic include refs/tags/** (protects every channel pointer + version tag on the reusable-hosting meta-repos from unauthorized update/deletion; creation stays free so autocut can still cut new version tags). Carries OrgAdmin + automation Integration 3167543 + the release-manager App 4193127 as always-bypass, so channel promotion / dev-lead autocut can move channel tags. - scripts/apply-rulesets.sh: the no-name default is now the FLEET_RULESETS allowlist (code-quality, pr-quality), NOT "every *.json" — so the targeted tag ruleset never leaks into --all / --repo fleet runs. release-channel-tags is applied only when named explicitly. - tests + README updated (17/17 bats green). Targeted apply (post-merge), to the two meta-repos only: apply-rulesets.sh --repo petry-projects/.github release-channel-tags apply-rulesets.sh --repo petry-projects/.github-private release-channel-tags Refs #596. Follow-ups: move bootstrap-new-repo.sh into .github and delete the duplicate .github-private applier + repo-local ruleset JSON. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015xAMAj4bRUEz53wYih8UFD
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThis PR adds a new release-channel-tags ruleset JSON targeting tag refs, changes scripts/apply-rulesets.sh to apply a default FLEET_RULESETS allowlist (code-quality, pr-quality) instead of all JSON files unless explicitly named, updates related documentation, and adds bats test coverage. ChangesRelease-channel-tags ruleset and fleet allowlist
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ApplyRulesetsScript
participant RulesetsDirectory
participant GitHubAPI
User->>ApplyRulesetsScript: run apply --repo (no ruleset names)
ApplyRulesetsScript->>ApplyRulesetsScript: select FLEET_RULESETS (code-quality, pr-quality)
ApplyRulesetsScript->>RulesetsDirectory: validate each *.json exists
ApplyRulesetsScript->>GitHubAPI: apply code-quality.json, pr-quality.json
User->>ApplyRulesetsScript: run apply --repo release-channel-tags (explicit name)
ApplyRulesetsScript->>RulesetsDirectory: validate release-channel-tags.json exists
ApplyRulesetsScript->>GitHubAPI: apply release-channel-tags.json
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a targeted ruleset, release-channel-tags.json, to protect release tags from unauthorized updates and deletions. To prevent this targeted ruleset from being applied fleet-wide by default, the apply-rulesets.sh script is updated to use an explicit allowlist (FLEET_RULESETS) for default runs, rather than applying all JSON files in the directory. Tests and documentation are updated accordingly. Feedback on the changes suggests declaring the global array FLEET_RULESETS as readonly to prevent accidental modification, especially since the script is sourced by the test suite.
There was a problem hiding this comment.
Pull request overview
This PR relocates the release-channel-tags repository ruleset into .github as a codified org standard and adjusts the ruleset applier so targeted rulesets are not accidentally applied fleet-wide.
Changes:
- Add
standards/rulesets/release-channel-tags.jsonas a genericrefs/tags/**tag ruleset (update/deletion only) with the intended bypass actors. - Change
scripts/apply-rulesets.shdefault behavior to apply only theFLEET_RULESETSallowlist (code-quality,pr-quality) unless names are explicitly provided. - Extend Bats tests and the rulesets README to cover the new targeted ruleset and the new default selection behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/scripts/apply-rulesets/apply-rulesets.bats | Adds coverage for the new tag ruleset JSON and for the new “fleet allowlist only” default behavior. |
| standards/rulesets/release-channel-tags.json | Introduces the codified release-channel-tags tag ruleset under .github standards. |
| standards/rulesets/README.md | Documents release-channel-tags alongside fleet rulesets and explains targeted vs default application behavior. |
| scripts/apply-rulesets.sh | Prevents unintended fleet-wide application by defaulting to an explicit allowlist when no ruleset names are provided. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
Dev-Lead — fix-bot-comment (applied)Changes committed and pushed. |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
… rulesets need Team plan)
Captures that release-channel-tags (and code/pr-quality) are organization-level
in intent — org-scoped bypass actors, org-wide protection — but replicated
per-repo by apply-rulesets.sh because native /orgs/{org}/rulesets is a Team-plan
feature and petry-projects is on Free (403 'Upgrade to GitHub Team').
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015xAMAj4bRUEz53wYih8UFD
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 776ebfa3a1e706d992c234dac217130d67c735c9
Review mode: triage-approved (single reviewer)
Summary
Relocates the release-channel-tags tag ruleset into .github/standards/rulesets/ as a codified org standard (first slice of #596) and hardens scripts/apply-rulesets.sh so the no-name default applies only the FLEET_RULESETS allowlist (code-quality, pr-quality) instead of every *.json — preventing the targeted tag ruleset from being swept fleet-wide. Docs (AGENTS.md, github-settings.md, rulesets README) updated consistently; 2 new bats tests cover the new JSON and the allowlist default (17/17 green).
Linked issue analysis
Refs #596 (open): this PR delivers the first slice — standards live only in .github — and is explicit that applier consolidation, bootstrap-new-repo.sh relocation, and deletion of the duplicate .github-private applier/ruleset are follow-ups. No closing reference is claimed, which is correct for a partial slice. The change substantively advances the issue.
Findings
- No blocking findings.
- Risk classified MEDIUM: new org-policy artifact (release-channel-tags.json) plus non-trivial selection-logic change in apply-rulesets.sh. Not HIGH: no auth/secret/credential code; the ruleset adds protection (update+deletion on refs/tags/**, creation free) and its bypass actors (OrgAdmin, Integration 3167543, release-manager App 4193127) match the already-live ruleset on .github-private per the PR body.
- Applier change is defensive and correct: names default to the readonly FLEET_RULESETS allowlist, each name validated against an existing JSON, unknown names still error. Bats tests explicitly assert release-channel-tags is NOT applied by default and IS applied when named.
- Both bot review threads resolved: gemini-code-assist readonly-array suggestion applied (readonly FLEET_RULESETS); Copilot doc-consistency finding fixed in AGENTS.md + standards/github-settings.md.
- Secret scan: run_secret_scanning MCP tool not available in this environment; gitleaks CI check passed. No secrets in the diff (IDs 3167543/4193127 are public App identifiers, not credentials).
- package-lock.json name churn (pr-595 → pr-609) is cosmetic.
CI status
All required checks green: ShellCheck, Lint, Agent Security Scan, Secret scan (gitleaks), CodeQL, SonarCloud (Quality Gate passed, 0 issues/hotspots), Apply Rulesets Tests (lint + bats), AgentShield, npm audit, dependency audit. Skipped checks are conditional ecosystem audits (pip/pnpm/cargo/govulncheck) and dependabot-automerge — expected. mergeStateStatus is BEHIND main (branch needs update before merge), which is not a review blocker.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
|
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/apply-rulesets.sh (1)
139-166: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
--allcan be combined with a targeted ruleset name, bypassing the safety net this PR adds.
FLEET_RULESETSonly guards the default (no-name) case. Nothing stops--all release-channel-tagsfrom sweeping this targeted ruleset (and its release-manager bypass grant) across every repo in the org — the exact outcome this PR set out to prevent. No test covers this combination either.🛡️ Proposed guard
local files=() if [ "${`#names`[@]}" -eq 0 ]; then names=("${FLEET_RULESETS[@]}"); fi + if [ "$all" = true ]; then + local n + for n in "${names[@]}"; do + case " ${FLEET_RULESETS[*]} " in + *" ${n} "*) ;; + *) echo "::error::'${n}' is a targeted ruleset and cannot be applied with --all; use --repo instead" >&2; return 2 ;; + esac + done + fi local n for n in "${names[@]}"; do🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/apply-rulesets.sh` around lines 139 - 166, The `apply-rulesets` flow still allows `--all` to be used with an explicit targeted ruleset name, which bypasses the new safety guard. Update the argument handling in `apply-rulesets.sh` so the `--all` path only permits the default allowlisted fleet rulesets and rejects any explicit names like `release-channel-tags` before calling `apply_repo`. Add a test covering the `--all` plus named-ruleset combination to ensure it fails with an error.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@standards/github-settings.md`:
- Around line 128-134: The “only sanctioned rulesets” statement in
github-settings.md is now ambiguous because the section also introduces
release-channel-tags as a separate codified ruleset. Update the surrounding
wording in this section to clearly scope that claim to branch-protection
rulesets only, using the existing ruleset names referenced here (`pr-quality`,
`code-quality`, and `release-channel-tags`) so it no longer reads like a
contradiction.
---
Outside diff comments:
In `@scripts/apply-rulesets.sh`:
- Around line 139-166: The `apply-rulesets` flow still allows `--all` to be used
with an explicit targeted ruleset name, which bypasses the new safety guard.
Update the argument handling in `apply-rulesets.sh` so the `--all` path only
permits the default allowlisted fleet rulesets and rejects any explicit names
like `release-channel-tags` before calling `apply_repo`. Add a test covering the
`--all` plus named-ruleset combination to ensure it fails with an error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 19df586d-24f4-481f-b6bc-fb2615bb6e5e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
AGENTS.mdscripts/apply-rulesets.shstandards/github-settings.mdstandards/rulesets/README.mdstandards/rulesets/release-channel-tags.jsontest/scripts/apply-rulesets/apply-rulesets.bats
| The codified ruleset JSONs are the source of truth for all rulesets; | ||
| `scripts/apply-rulesets.sh` (and any org automation consuming it) applies them to each repo. | ||
| As **org-wide compliance policy they are owned by `petry-projects/.github`**, and | ||
| their canonical home is `standards/rulesets/`. Do **not** author them in | ||
| their canonical home is `standards/rulesets/`. Do **not** author rulesets in | ||
| `petry-projects/.github-private` — that repo is scoped to agents/skills and their | ||
| reusable assets. The only ruleset that belongs there is `release-channel-tags` | ||
| (it protects `.github-private`'s own `pr-review/**` / `dev-lead/**` release tags). | ||
| reusable assets. All rulesets, including targeted ones like `release-channel-tags`, | ||
| are defined in `.github/standards/rulesets/`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Consider scoping the "only sanctioned rulesets" line for clarity.
The unchanged sentence above ("pr-quality and code-quality are the only sanctioned rulesets") sits in the same section that now lists release-channel-tags as a third codified ruleset (lines 155-159). It's technically about branch-protection consolidation (tags aren't in scope there), but as written it reads as contradicting the new content just below it.
✏️ Suggested clarification (outside reviewed range, at line 142)
-**`pr-quality` and `code-quality` are the only sanctioned rulesets.** Legacy
+**`pr-quality` and `code-quality` are the only sanctioned *branch-protection* rulesets.** Legacy
`protect-branches` rulesets and ad-hoc `main` rulesets are deprecated: theyAlso applies to: 155-159
🧰 Tools
🪛 LanguageTool
[uncategorized] ~134-~134: The official name of this software platform is spelled with a capital “H”.
Context: ... release-channel-tags, are defined in .github/standards/rulesets/. See the repo-boun...
(GITHUB)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@standards/github-settings.md` around lines 128 - 134, The “only sanctioned
rulesets” statement in github-settings.md is now ambiguous because the section
also introduces release-channel-tags as a separate codified ruleset. Update the
surrounding wording in this section to clearly scope that claim to
branch-protection rulesets only, using the existing ruleset names referenced
here (`pr-quality`, `code-quality`, and `release-channel-tags`) so it no longer
reads like a contradiction.
What
Realizes the first slice of #596 — standards live only in
.github— for therelease-channel-tagsruleset, and generalizes it per review.standards/rulesets/release-channel-tags.json(new) — the tag ruleset is now a codified org standard owned by.github.refs/tags/**instead of an enumerated namespace list (which drifted every time a new reusable/agent channel was added). On the reusable-hosting meta-repos every tag is a release tag, so "protect all tags" matches intent exactly.update+deletiononly — creation stays free, so dev-lead autocut can still cut newvX.Y.Ztags; only moving a channel pointer or deleting a tag needs bypass.OrganizationAdmin+ automation Integration3167543+ release-manager App4193127(allalways), so channel promotion / autocut work. (This matches the live ruleset on.github-private, which was just updated to add4193127.)scripts/apply-rulesets.sh— the no-name default is now theFLEET_RULESETSallowlist (code-quality,pr-quality), not "every*.jsonin the dir". Without this, addingrelease-channel-tags.jsonwould sweep it onto every repo via--all/--repo. It is now applied only when named explicitly.Apply plan (post-merge, targeted to the two meta-repos only)
Not in this PR (follow-ups for #596)
bootstrap-new-repo.shinto.github(+ reconcile the divergentapply-repo-settings.shcopies)..github-private/scripts/apply-rulesets.shand the repo-local.github/rulesets/release-channel-tags.json.Refs #596.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation