From 6bad829ed069d50eaac9fbfa0c1df7192e9d1feb Mon Sep 17 00:00:00 2001 From: Andrew Altshuler Date: Wed, 13 May 2026 17:38:20 +0300 Subject: [PATCH] branch-protection: declarative policy + apply script (#89) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Branch protection on main, declared as code rather than as opaque GitHub UI state. Pairs with the CODEOWNERS chassis (#88): once this PR lands and an admin runs the apply script, every PR to main must satisfy code-owner review and the listed required checks. Components: - .github/branch-protection.json — the policy. Edit this to change required checks, review counts, etc. Includes a _comment field for human readers; the apply script strips it before PUT. - scripts/apply-branch-protection.sh — idempotent apply via `gh api`. Reads back current state for verification. Supports DRY_RUN=1. - docs/branch-protection.md — explains the policy, how to apply, how to change, why declared as code. - AGENTS.md topic-index row. Policy summary: - Required status checks (strict): Classify Changes, Check AGENTS.md Links, Test Workspace, Test omnigraph-server --features aws, CODEOWNERS / drift, CODEOWNERS / noedit. - Required approving reviews: 1, must be a code owner. - Dismiss stale reviews on new commits. - Required linear history (squash or rebase merges only). - No force pushes, no deletions, no admin bypasses. - Required conversation resolution. What's NOT in this PR: - Required signed commits — not yet; maintainers must enroll GPG/SSH signing first or merges will block. - Tag protection for v* tags — separate PR. - Additional required checks (cargo deny, audit, fmt, clippy, CodeQL, schema-lint MR-946) — separate PRs as each lands. - The script is NOT run by CI. Branch-protection changes are admin actions; CI-driven auto-apply would defeat the purpose. Manual invocation is the audit point. How to apply after merge: ./scripts/apply-branch-protection.sh Requires gh-CLI auth with repo-admin permissions. Co-authored-by: Claude Opus 4.7 (1M context) --- .github/branch-protection.json | 30 +++++++++++ AGENTS.md | 1 + docs/branch-protection.md | 83 ++++++++++++++++++++++++++++++ scripts/apply-branch-protection.sh | 62 ++++++++++++++++++++++ 4 files changed, 176 insertions(+) create mode 100644 .github/branch-protection.json create mode 100644 docs/branch-protection.md create mode 100755 scripts/apply-branch-protection.sh diff --git a/.github/branch-protection.json b/.github/branch-protection.json new file mode 100644 index 0000000..d472de8 --- /dev/null +++ b/.github/branch-protection.json @@ -0,0 +1,30 @@ +{ + "_comment": "Branch protection policy for main. Applied via scripts/apply-branch-protection.sh. See docs/branch-protection.md for rationale.", + "required_status_checks": { + "strict": true, + "contexts": [ + "Classify Changes", + "Check AGENTS.md Links", + "Test Workspace", + "Test omnigraph-server --features aws", + "CODEOWNERS / drift", + "CODEOWNERS / noedit" + ] + }, + "enforce_admins": true, + "required_pull_request_reviews": { + "dismissal_restrictions": {}, + "dismiss_stale_reviews": true, + "require_code_owner_reviews": true, + "required_approving_review_count": 1, + "require_last_push_approval": false + }, + "restrictions": null, + "required_linear_history": true, + "allow_force_pushes": false, + "allow_deletions": false, + "block_creations": false, + "required_conversation_resolution": true, + "lock_branch": false, + "allow_fork_syncing": false +} diff --git a/AGENTS.md b/AGENTS.md index c6bf097..10f2e41 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -94,6 +94,7 @@ Full diagram and concurrency model: [docs/architecture.md](docs/architecture.md) | Deployment (binary / container / RustFS bootstrap / auth / build variants) | [docs/deployment.md](docs/deployment.md) | | CI / release workflows | [docs/ci.md](docs/ci.md) | | Code ownership (CODEOWNERS source of truth, roles, regeneration) | [docs/codeowners.md](docs/codeowners.md) | +| Branch protection policy (declarative, applied via `scripts/apply-branch-protection.sh`) | [docs/branch-protection.md](docs/branch-protection.md) | | Constants & tunables cheat sheet | [docs/constants.md](docs/constants.md) | | Per-version release notes | [docs/releases/](docs/releases/) | diff --git a/docs/branch-protection.md b/docs/branch-protection.md new file mode 100644 index 0000000..2f878de --- /dev/null +++ b/docs/branch-protection.md @@ -0,0 +1,83 @@ +# Branch protection on `main` + +`main` is gated by a declarative branch-protection policy. The source of truth is `.github/branch-protection.json`; the apply mechanism is `scripts/apply-branch-protection.sh`. Re-running the script with a changed JSON is idempotent. + +This page explains what the policy says and how to change it. + +## Current policy + +| Setting | Value | Why | +|---|---|---| +| **Required status checks (strict)** | `Classify Changes`, `Check AGENTS.md Links`, `Test Workspace`, `Test omnigraph-server --features aws`, `CODEOWNERS / drift`, `CODEOWNERS / noedit` | Every PR must pass workspace tests, AGENTS.md link integrity, and the CODEOWNERS hygiene checks. `strict: true` requires the branch to be up-to-date with `main` before merge. | +| **Required approving reviews** | `1` | At least one reviewer. With a 2-person team, going higher would block all merges when one person is unavailable. | +| **Require code-owner reviews** | `true` | The reviewer must be a code owner per `.github/CODEOWNERS`. This is what makes the codeowners chassis enforced. | +| **Dismiss stale reviews on new commits** | `true` | A push after approval invalidates the prior review. Prevents the "approve, then sneak in unreviewed changes" pattern. | +| **Require linear history** | `true` | No merge commits — squash or rebase only. Matches recent practice. | +| **Disallow force pushes** | `true` | No history rewrites on `main`. | +| **Disallow branch deletions** | `true` | `main` cannot be deleted. | +| **Required conversation resolution** | `true` | All review comment threads must be resolved before merge. | +| **Enforce on admins** | `true` | Even repo admins go through the gates. The point is no bypasses. | +| **Required signed commits** | not yet | Not enabled. Would lock out maintainers until everyone enrolls GPG/SSH commit signing. Tracked as a follow-up. | + +## How to apply + +Run from the repo root: + +```bash +./scripts/apply-branch-protection.sh +``` + +The script reads `.github/branch-protection.json`, strips the human-readable `_comment` field (the GitHub API rejects unknown keys), and PUTs to `repos/ModernRelay/omnigraph/branches/main/protection`. + +Requires `gh` authenticated with a token that has admin permissions on the repo. + +To preview without applying: + +```bash +DRY_RUN=1 ./scripts/apply-branch-protection.sh +``` + +## How to change the policy + +1. Edit `.github/branch-protection.json`. +2. Open a PR. The JSON change goes through normal review. +3. After the PR merges, an admin runs `./scripts/apply-branch-protection.sh` to push the new policy to GitHub. + +The script is **not run automatically** by CI. Branch-protection changes are admin actions that should be applied deliberately — a CI-driven automatic apply would mean any merged PR could rewrite protection rules, which defeats the purpose. The script's existence makes the apply reproducible; the admin's manual invocation is the audit point. + +## How to read the current GitHub state + +```bash +gh api repos/ModernRelay/omnigraph/branches/main/protection +``` + +Outputs the live policy. Compare against `.github/branch-protection.json` to detect drift. + +## Why declared as code + +- **Audit trail**: `git log .github/branch-protection.json` shows every change with a reviewable diff and a merge commit. +- **Disaster recovery**: if branch protection is accidentally removed or weakened via the UI, the JSON is the canonical recovery point. +- **Consistency**: pairs with `.github/codeowners-roles.yml` (the CODEOWNERS source of truth). Repo policy lives in the repo. + +## What this gates + +After branch protection is applied, every PR targeting `main` must: + +1. Pass all listed status checks. +2. Be up-to-date with `main` (rebase or merge-from-main). +3. Have at least one approving review from a code owner for the touched paths. +4. Have all review conversations resolved. +5. Be squash- or rebase-merged (no merge commits). + +Even repo admins are subject to these rules. + +## Subsequent hardening (not in this PR) + +The branch-protection policy is the foundation. Future hardening adds: + +- **Required signed commits** (`required_signatures: true`) — once maintainers enroll GPG/SSH signing. +- **Tag protection** for `v*` tags via `repos/.../tags/protection`. +- **Required reviewers from specific teams** for high-leverage paths (e.g., `docs/invariants.md`) via CODEOWNERS tier expansion + the N-unique-approvers CI workaround. +- **More required CI checks**: `cargo deny`, `cargo audit`, `cargo fmt --check`, `cargo clippy -D warnings`, CodeQL, secret scanning, schema-lint (MR-946). + +See the hardening playbook for the full plan. diff --git a/scripts/apply-branch-protection.sh b/scripts/apply-branch-protection.sh new file mode 100755 index 0000000..910d5b6 --- /dev/null +++ b/scripts/apply-branch-protection.sh @@ -0,0 +1,62 @@ +#!/usr/bin/env bash +# Apply branch protection rules to the main branch. +# +# Requires: +# - `gh` CLI authenticated. +# - Repo-admin or org-admin permissions on ModernRelay/omnigraph. +# +# This script is idempotent: re-running applies whatever is currently +# declared in .github/branch-protection.json. The JSON file is the +# source of truth; this script is the apply mechanism. +# +# Usage: +# ./scripts/apply-branch-protection.sh # apply to main +# REPO=ModernRelay/omnigraph BRANCH=main ./scripts/... # explicit +# DRY_RUN=1 ./scripts/apply-branch-protection.sh # show what would apply + +set -euo pipefail + +REPO="${REPO:-ModernRelay/omnigraph}" +BRANCH="${BRANCH:-main}" +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +POLICY_FILE="${POLICY_FILE:-$HERE/../.github/branch-protection.json}" + +if [ ! -f "$POLICY_FILE" ]; then + echo "error: policy file not found: $POLICY_FILE" >&2 + exit 1 +fi + +# Strip the _comment field — the GitHub API rejects unknown keys. +PAYLOAD="$(mktemp)" +trap 'rm -f "$PAYLOAD"' EXIT +jq 'del(._comment)' "$POLICY_FILE" > "$PAYLOAD" + +if [ "${DRY_RUN:-0}" = "1" ]; then + echo "DRY RUN — would apply to $REPO/$BRANCH:" + echo + cat "$PAYLOAD" + exit 0 +fi + +echo "Applying branch protection to $REPO/$BRANCH from $POLICY_FILE" +gh api -X PUT "repos/$REPO/branches/$BRANCH/protection" \ + --input "$PAYLOAD" \ + -H "Accept: application/vnd.github+json" \ + > /dev/null +echo "OK" + +# Verify by reading back. +echo +echo "Current policy on $REPO/$BRANCH:" +gh api "repos/$REPO/branches/$BRANCH/protection" \ + --jq '{ + required_status_checks: .required_status_checks.contexts, + strict_status_checks: .required_status_checks.strict, + required_approvals: .required_pull_request_reviews.required_approving_review_count, + require_code_owner_reviews: .required_pull_request_reviews.require_code_owner_reviews, + enforce_admins: .enforce_admins.enabled, + linear_history: .required_linear_history.enabled, + force_pushes_allowed: .allow_force_pushes.enabled, + deletions_allowed: .allow_deletions.enabled, + conversation_resolution_required: .required_conversation_resolution.enabled + }'