From 4706fcaf1455c6bd518bb3d867e0f36f0282bd74 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sat, 10 Oct 2026 00:15:57 -0400 Subject: [PATCH 1/2] fix: add a daily check that main is protected Main had no branch protection and no ruleset. Anyone with write access could push to it directly, force-push over its history, or delete it. That covers the admission ledgers and the workflows that issue and verify admissions, and nothing reported it. What changed: - scripts/check_branch_protection.py reads the ruleset rules for main and, when the token allows, its classic branch protection. It fails unless main requires a pull request and the validate-profile check from GitHub Actions, and blocks force pushes and deletion. When the token can read who may bypass those rules, it also fails if anyone can. - Profile consistency runs the check on its daily schedule and on manual dispatch, and files an issue when it fails. It doesn't run on pull requests, so it can't block unrelated changes. - main-branch-ruleset.json is a ruleset body that satisfies the check. An admin applies it with: gh api -X POST repos/OpenAdaptAI/.github/rulesets --input main-branch-ruleset.json - Tests cover the responses GitHub returns today (the check fails), classic protection, rulesets, rules split across both, and the committed ruleset body. This commit doesn't change any GitHub setting. The check stays red until an admin turns on protection for main. Ledger: B247 Co-Authored-By: Claude Opus 5.5 --- .github/workflows/profile-consistency.yml | 46 ++++ main-branch-ruleset.json | 35 +++ scripts/check_branch_protection.py | 283 ++++++++++++++++++++++ tests/test_branch_protection.py | 270 +++++++++++++++++++++ 4 files changed, 634 insertions(+) create mode 100644 main-branch-ruleset.json create mode 100644 scripts/check_branch_protection.py create mode 100644 tests/test_branch_protection.py diff --git a/.github/workflows/profile-consistency.yml b/.github/workflows/profile-consistency.yml index 53d1660..22f79ce 100644 --- a/.github/workflows/profile-consistency.yml +++ b/.github/workflows/profile-consistency.yml @@ -114,3 +114,49 @@ jobs: else gh issue create --repo "${{ github.repository }}" --title "$TITLE" --body "$BODY" fi + + check-branch-protection: + needs: reject-lifecycle-app + if: >- + (github.event_name == 'schedule' || github.event_name == 'workflow_dispatch') && + github.actor != 'openadapt-lifecycle[bot]' && + github.triggering_actor != 'openadapt-lifecycle[bot]' + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + - name: Require protection on main + # The default token reads ruleset rules. Classic branch protection is + # readable only with Administration read access, so an optional + # BRANCH_PROTECTION_READ_TOKEN secret is used when it exists. + env: + GH_TOKEN: ${{ secrets.BRANCH_PROTECTION_READ_TOKEN || github.token }} + run: python3 scripts/check_branch_protection.py --repository "$GITHUB_REPOSITORY" --branch main + + report-unprotected-branch: + needs: [reject-lifecycle-app, check-branch-protection] + if: >- + ${{ always() && needs.check-branch-protection.result == 'failure' && + github.actor != 'openadapt-lifecycle[bot]' && + github.triggering_actor != 'openadapt-lifecycle[bot]' }} + runs-on: ubuntu-latest + permissions: + issues: write + steps: + - name: File or update the branch protection failure + env: + GH_TOKEN: ${{ github.token }} + run: | + TITLE="Branch protection on main needs attention" + BODY="The scheduled branch protection check failed: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + + Main holds the admission ledgers and the workflows that issue and verify admissions. Until it requires a pull request and the validate-profile check, and blocks force pushes and deletion, anyone with write access can change them without review. Apply main-branch-ruleset.json as a repository ruleset to fix this." + EXISTING=$(gh issue list --repo "${{ github.repository }}" --state open \ + --search "in:title \"$TITLE\"" --json number --jq '.[0].number // empty') + if [ -n "$EXISTING" ]; then + gh issue comment "$EXISTING" --repo "${{ github.repository }}" --body "$BODY" + else + gh issue create --repo "${{ github.repository }}" --title "$TITLE" --body "$BODY" + fi diff --git a/main-branch-ruleset.json b/main-branch-ruleset.json new file mode 100644 index 0000000..f41f4b2 --- /dev/null +++ b/main-branch-ruleset.json @@ -0,0 +1,35 @@ +{ + "name": "OpenAdapt policy: protected main", + "target": "branch", + "enforcement": "active", + "bypass_actors": [], + "conditions": { + "ref_name": { + "include": ["refs/heads/main"], + "exclude": [] + } + }, + "rules": [ + {"type": "deletion"}, + {"type": "non_fast_forward"}, + { + "type": "pull_request", + "parameters": { + "required_approving_review_count": 0, + "dismiss_stale_reviews_on_push": false, + "require_code_owner_review": false, + "require_last_push_approval": false, + "required_review_thread_resolution": false + } + }, + { + "type": "required_status_checks", + "parameters": { + "strict_required_status_checks_policy": false, + "required_status_checks": [ + {"context": "validate-profile", "integration_id": 15368} + ] + } + } + ] +} diff --git a/scripts/check_branch_protection.py b/scripts/check_branch_protection.py new file mode 100644 index 0000000..afdd32a --- /dev/null +++ b/scripts/check_branch_protection.py @@ -0,0 +1,283 @@ +#!/usr/bin/env python3 +"""Fail when the main branch of this repository is not protected. + +This repository holds the admission ledgers, the evidence registry, and the +workflows that issue and verify admissions. Signed evidence fails verification +if someone edits it, but the workflows that define verification live on the +same branch. Without protection, anyone with write access can push to main, +force-push over its history, or delete it, with no review and no checks. + +The check requires all of these on the branch: + +1. Changes arrive through a pull request. +2. The `validate-profile` check, reported by GitHub Actions, must pass. +3. Force pushes are blocked. +4. Deleting the branch is blocked. + +It also requires that nobody, administrators included, can bypass these +rules, whenever the token can read who may bypass them. + +GitHub enforces repository rulesets and classic branch protection together, so +each requirement may come from either source: + +- Rulesets that apply to the branch. Any token that can read the repository + can read these rules. A ruleset's bypass list is readable only with + administration access; without it, the run reports bypass as not checked. +- Classic branch protection. Reading it needs a token with read access to the + repository's Administration settings. The default Actions token can't read + it, so classic protection counts only when such a token is supplied. + +To satisfy the check without an extra token, apply the committed ruleset: + + gh api -X POST repos/OpenAdaptAI/.github/rulesets \\ + --input main-branch-ruleset.json + +The requirements live in this file, not in the ruleset body, so editing the +body can't weaken the check. +""" + +from __future__ import annotations + +import argparse +import json +import os +import sys +import urllib.error +import urllib.request +from typing import Any, Callable + +API_ORIGIN = "https://api.github.com" +FETCH_TIMEOUT_SECONDS = 30 +DEFAULT_REPOSITORY = "OpenAdaptAI/.github" +DEFAULT_BRANCH = "main" + +REQUIRED_CHECK = "validate-profile" +GITHUB_ACTIONS_APP_ID = 15368 +# A sole maintainer can't approve their own pull request, so the floor matches +# the other OpenAdapt repositories: a pull request is required, approval is not. +MINIMUM_APPROVALS = 0 + +PULL_REQUEST = "pull request required" +STATUS_CHECK = f"{REQUIRED_CHECK} check from GitHub Actions required" +FORCE_PUSH = "force pushes blocked" +DELETION = "deletion blocked" +REQUIREMENTS = (PULL_REQUEST, STATUS_CHECK, FORCE_PUSH, DELETION) + +# Classic protection states other than a readable settings object. +NOT_PROTECTED = "not-protected" +UNREADABLE = "unreadable" + +Fetch = Callable[[str], tuple[int, Any]] + + +def _approvals(value: Any) -> bool: + if not isinstance(value, dict): + return False + count = value.get("required_approving_review_count") + return ( + isinstance(count, int) + and not isinstance(count, bool) + and count >= MINIMUM_APPROVALS + ) + + +def _disabled(protection: dict, key: str) -> bool: + setting = protection.get(key) + return isinstance(setting, dict) and setting.get("enabled") is False + + +def classic_requirements(protection: Any) -> set[str]: + """Return the requirements that readable classic protection satisfies.""" + if not isinstance(protection, dict): + return set() + met = set() + if _approvals(protection.get("required_pull_request_reviews")): + met.add(PULL_REQUEST) + checks = protection.get("required_status_checks") + checks = checks.get("checks") if isinstance(checks, dict) else None + if isinstance(checks, list) and { + "context": REQUIRED_CHECK, + "app_id": GITHUB_ACTIONS_APP_ID, + } in checks: + met.add(STATUS_CHECK) + if _disabled(protection, "allow_force_pushes"): + met.add(FORCE_PUSH) + if _disabled(protection, "allow_deletions"): + met.add(DELETION) + return met + + +def rule_requirement(rule: Any) -> str | None: + """Return the requirement one ruleset rule satisfies, if any.""" + if not isinstance(rule, dict): + return None + kind = rule.get("type") + parameters = rule.get("parameters") + if kind == "pull_request" and _approvals(parameters): + return PULL_REQUEST + if kind == "required_status_checks" and isinstance(parameters, dict): + checks = parameters.get("required_status_checks") + if isinstance(checks, list) and any( + isinstance(item, dict) + and item.get("context") == REQUIRED_CHECK + and item.get("integration_id") == GITHUB_ACTIONS_APP_ID + for item in checks + ): + return STATUS_CHECK + if kind == "non_fast_forward": + return FORCE_PUSH + if kind == "deletion": + return DELETION + return None + + +def evaluate( + rules: list[Any], + protection: Any, + bypass_actors: dict[int, list | None], +) -> tuple[list[str], list[str]]: + """Return (problems, notes) for the observed protection of one branch. + + `rules` are the active ruleset rules for the branch. `protection` is the + classic protection object, NOT_PROTECTED, or UNREADABLE. `bypass_actors` + maps a ruleset id to its bypass list, or to None when it was unreadable. + """ + classic = classic_requirements(protection) + enforce_admins = ( + protection.get("enforce_admins") if isinstance(protection, dict) else None + ) + classic_binds_admins = ( + isinstance(enforce_admins, dict) and enforce_admins.get("enabled") is True + ) + problems: list[str] = [] + notes: list[str] = [] + for requirement in REQUIREMENTS: + # Each source that enforces the requirement either binds + # administrators ("bound"), lets someone bypass it ("bypass"), or has a + # bypass list this token can't read ("unknown"). + outcomes = set() + if requirement in classic: + outcomes.add("bound" if classic_binds_admins else "bypass") + for rule in rules: + if rule_requirement(rule) != requirement: + continue + actors = bypass_actors.get(rule.get("ruleset_id")) + if actors is None: + outcomes.add("unknown") + else: + outcomes.add("bypass" if actors else "bound") + if not outcomes: + problems.append(f"{requirement}: not set") + elif "bound" in outcomes: + continue + elif "unknown" in outcomes: + notes.append( + f"{requirement}: set by a ruleset; whether anyone can bypass it " + "was not checked, because this token can't read the bypass list" + ) + else: + problems.append( + f"{requirement}: set, but administrators or listed actors can " + "bypass it" + ) + if problems and protection == UNREADABLE: + notes.append( + "This token can't read classic branch protection. Supply a token " + "with read access to Administration, or apply main-branch-ruleset.json " + "as a repository ruleset." + ) + return problems, notes + + +def github_get(path: str) -> tuple[int, Any]: + """GET one GitHub REST path and return (status, decoded JSON body).""" + headers = { + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + "User-Agent": "openadapt-branch-protection-check", + } + token = os.environ.get("GH_TOKEN") or os.environ.get("GITHUB_TOKEN") + if token: + headers["Authorization"] = f"Bearer {token}" + request = urllib.request.Request(API_ORIGIN + path, headers=headers) + try: + with urllib.request.urlopen(request, timeout=FETCH_TIMEOUT_SECONDS) as response: + return response.status, json.load(response) + except urllib.error.HTTPError as error: + try: + body = json.load(error) + except ValueError: + body = None + return error.code, body + + +def _ruleset_path(repository: str, rule: dict) -> str | None: + ruleset_id = rule.get("ruleset_id") + if not isinstance(ruleset_id, int) or isinstance(ruleset_id, bool): + return None + if rule.get("ruleset_source_type") == "Organization": + organization = repository.split("/", 1)[0] + return f"/orgs/{organization}/rulesets/{ruleset_id}" + return f"/repos/{repository}/rulesets/{ruleset_id}" + + +def observe(repository: str, branch: str, fetch: Fetch): + """Read the rules, classic protection, and bypass lists for a branch.""" + status, rules = fetch(f"/repos/{repository}/rules/branches/{branch}") + if status != 200 or not isinstance(rules, list): + raise RuntimeError(f"could not read the rules for {branch} (HTTP {status})") + status, body = fetch(f"/repos/{repository}/branches/{branch}/protection") + if status == 200 and isinstance(body, dict): + protection: Any = body + elif ( + status == 404 + and isinstance(body, dict) + and body.get("message") == "Branch not protected" + ): + protection = NOT_PROTECTED + else: + protection = UNREADABLE + bypass_actors: dict[int, list | None] = {} + for rule in rules: + if not isinstance(rule, dict) or rule.get("ruleset_id") in bypass_actors: + continue + path = _ruleset_path(repository, rule) + if path is None: + continue + status, detail = fetch(path) + actors = detail.get("bypass_actors") if isinstance(detail, dict) else None + bypass_actors[rule["ruleset_id"]] = ( + actors if status == 200 and isinstance(actors, list) else None + ) + return rules, protection, bypass_actors + + +def main(argv: list[str] | None = None, fetch: Fetch = github_get) -> int: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument( + "--repository", + default=os.environ.get("GITHUB_REPOSITORY") or DEFAULT_REPOSITORY, + ) + parser.add_argument("--branch", default=DEFAULT_BRANCH) + args = parser.parse_args(argv) + try: + rules, protection, bypass_actors = observe(args.repository, args.branch, fetch) + except (OSError, RuntimeError, ValueError) as error: + print(f"ERROR: {error}", file=sys.stderr) + return 2 + problems, notes = evaluate(rules, protection, bypass_actors) + branch = f"Branch {args.branch} of {args.repository}" + stream = sys.stderr if problems else sys.stdout + if problems: + print(f"{branch} is not protected as required:", file=stream) + for problem in problems: + print(f"- {problem}", file=stream) + else: + print(f"{branch} is protected as required.", file=stream) + for note in notes: + print(f"NOTE: {note}", file=stream) + return 1 if problems else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_branch_protection.py b/tests/test_branch_protection.py new file mode 100644 index 0000000..1c2ac52 --- /dev/null +++ b/tests/test_branch_protection.py @@ -0,0 +1,270 @@ +"""The scheduled check fails while main can take unreviewed or forced pushes.""" + +from __future__ import annotations + +import copy +import io +import json +import sys +import unittest +from contextlib import redirect_stderr, redirect_stdout +from pathlib import Path +from unittest import mock + +ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(ROOT / "scripts")) + +import check_branch_protection as check # noqa: E402 + +REPOSITORY = "OpenAdaptAI/.github" +RULESET_ID = 1234567 + + +def classic_protection() -> dict: + """Classic protection in the shape GitHub returns to an admin token.""" + return { + "required_status_checks": { + "strict": False, + "contexts": ["validate-profile"], + "checks": [{"context": "validate-profile", "app_id": 15368}], + }, + "required_pull_request_reviews": { + "dismiss_stale_reviews": False, + "require_code_owner_reviews": False, + "require_last_push_approval": False, + "required_approving_review_count": 0, + }, + "enforce_admins": {"enabled": True}, + "required_linear_history": {"enabled": False}, + "allow_force_pushes": {"enabled": False}, + "allow_deletions": {"enabled": False}, + } + + +def branch_rules(source_type: str = "Repository") -> list[dict]: + """Ruleset rules in the shape of GET /repos/{repo}/rules/branches/main.""" + origin = { + "ruleset_source_type": source_type, + "ruleset_source": REPOSITORY, + "ruleset_id": RULESET_ID, + } + return [ + {"type": "deletion", **origin}, + {"type": "non_fast_forward", **origin}, + { + "type": "pull_request", + "parameters": { + "required_approving_review_count": 0, + "dismiss_stale_reviews_on_push": False, + "require_code_owner_review": False, + "require_last_push_approval": False, + "required_review_thread_resolution": False, + }, + **origin, + }, + { + "type": "required_status_checks", + "parameters": { + "strict_required_status_checks_policy": False, + "required_status_checks": [ + {"context": "validate-profile", "integration_id": 15368} + ], + }, + **origin, + }, + ] + + +def evaluate(rules=(), protection=check.NOT_PROTECTED, bypass=None): + return check.evaluate(list(rules), protection, bypass or {}) + + +class TodayTests(unittest.TestCase): + """The state observed on 2026-10-09 must fail the check.""" + + def test_unprotected_main_without_rulesets_fails_every_requirement(self) -> None: + problems, _ = evaluate(rules=[], protection=check.NOT_PROTECTED) + self.assertEqual(len(problems), len(check.REQUIREMENTS)) + text = "\n".join(problems) + for requirement in check.REQUIREMENTS: + self.assertIn(requirement, text) + + def test_unreadable_classic_protection_does_not_count_as_protection(self) -> None: + problems, notes = evaluate(rules=[], protection=check.UNREADABLE) + self.assertEqual(len(problems), len(check.REQUIREMENTS)) + self.assertTrue(any("Administration" in note for note in notes), notes) + + def test_command_exits_nonzero_on_the_observed_github_responses(self) -> None: + responses = { + f"/repos/{REPOSITORY}/rules/branches/main": (200, []), + f"/repos/{REPOSITORY}/branches/main/protection": ( + 404, + {"message": "Branch not protected", "status": "404"}, + ), + } + stdout, stderr = io.StringIO(), io.StringIO() + with redirect_stdout(stdout), redirect_stderr(stderr): + code = check.main( + ["--repository", REPOSITORY, "--branch", "main"], + fetch=lambda path: responses[path], + ) + self.assertEqual(code, 1) + self.assertIn("main", stderr.getvalue()) + self.assertIn("not protected", stderr.getvalue()) + + +class ClassicProtectionTests(unittest.TestCase): + def test_complete_classic_protection_passes(self) -> None: + problems, notes = evaluate(protection=classic_protection()) + self.assertEqual(problems, []) + self.assertEqual(notes, []) + + def test_each_missing_classic_setting_fails(self) -> None: + mutations = { + "pull request": lambda p: p.pop("required_pull_request_reviews"), + "required check": lambda p: p.pop("required_status_checks"), + "check from any app": lambda p: p["required_status_checks"]["checks"][ + 0 + ].update(app_id=None), + "other check": lambda p: p["required_status_checks"].update( + checks=[{"context": "lint", "app_id": 15368}] + ), + "force push": lambda p: p["allow_force_pushes"].update(enabled=True), + "deletion": lambda p: p["allow_deletions"].update(enabled=True), + "missing force-push field": lambda p: p.pop("allow_force_pushes"), + "admins bypass": lambda p: p["enforce_admins"].update(enabled=False), + } + for label, mutate in mutations.items(): + with self.subTest(label): + protection = classic_protection() + mutate(protection) + problems, _ = evaluate(protection=protection) + self.assertNotEqual(problems, []) + + def test_approval_floor_is_enforced(self) -> None: + with mock.patch.object(check, "MINIMUM_APPROVALS", 1): + problems, _ = evaluate(protection=classic_protection()) + self.assertTrue(any("pull request" in p for p in problems), problems) + protection = classic_protection() + protection["required_pull_request_reviews"][ + "required_approving_review_count" + ] = 1 + self.assertEqual(evaluate(protection=protection)[0], []) + + +class RulesetTests(unittest.TestCase): + def test_ruleset_with_readable_empty_bypass_list_passes(self) -> None: + problems, notes = evaluate(rules=branch_rules(), bypass={RULESET_ID: []}) + self.assertEqual(problems, []) + self.assertEqual(notes, []) + + def test_ruleset_bypass_list_unreadable_passes_with_a_note(self) -> None: + problems, notes = evaluate(rules=branch_rules(), bypass={RULESET_ID: None}) + self.assertEqual(problems, []) + self.assertTrue(notes) + self.assertTrue(all("not checked" in note for note in notes), notes) + + def test_ruleset_with_a_bypass_actor_fails(self) -> None: + actor = {"actor_id": 5, "actor_type": "RepositoryRole", "bypass_mode": "always"} + problems, _ = evaluate(rules=branch_rules(), bypass={RULESET_ID: [actor]}) + self.assertEqual(len(problems), len(check.REQUIREMENTS)) + self.assertTrue(all("bypass" in p for p in problems), problems) + + def test_each_missing_ruleset_rule_fails(self) -> None: + for index, rule in enumerate(branch_rules()): + with self.subTest(rule=rule["type"]): + rules = branch_rules() + rules.pop(index) + problems, _ = evaluate(rules=rules, bypass={RULESET_ID: []}) + self.assertEqual(len(problems), 1, problems) + + def test_status_check_must_come_from_github_actions(self) -> None: + for check_entry in ( + {"context": "validate-profile"}, + {"context": "validate-profile", "integration_id": 1}, + {"context": "lint", "integration_id": 15368}, + ): + with self.subTest(check_entry=check_entry): + rules = branch_rules() + rules[3]["parameters"]["required_status_checks"] = [check_entry] + problems, _ = evaluate(rules=rules, bypass={RULESET_ID: []}) + self.assertEqual(len(problems), 1, problems) + self.assertIn("validate-profile", problems[0]) + + def test_rules_split_across_ruleset_and_classic_protection_pass(self) -> None: + protection = classic_protection() + protection.pop("required_pull_request_reviews") + protection["allow_deletions"]["enabled"] = True + rules = [r for r in branch_rules() if r["type"] in {"pull_request", "deletion"}] + problems, _ = evaluate( + rules=rules, protection=protection, bypass={RULESET_ID: []} + ) + self.assertEqual(problems, []) + + def test_command_reads_bypass_lists_by_ruleset_source(self) -> None: + for source_type, detail_path in ( + ("Repository", f"/repos/{REPOSITORY}/rulesets/{RULESET_ID}"), + ("Organization", f"/orgs/OpenAdaptAI/rulesets/{RULESET_ID}"), + ): + with self.subTest(source_type=source_type): + rules = branch_rules(source_type) + if source_type == "Organization": + for rule in rules: + rule["ruleset_source"] = "OpenAdaptAI" + responses = { + f"/repos/{REPOSITORY}/rules/branches/main": (200, rules), + f"/repos/{REPOSITORY}/branches/main/protection": ( + 403, + {"message": "Resource not accessible by integration"}, + ), + detail_path: (200, {"id": RULESET_ID, "bypass_actors": []}), + } + with redirect_stdout(io.StringIO()), redirect_stderr(io.StringIO()): + code = check.main( + ["--repository", REPOSITORY], + fetch=lambda path: responses[path], + ) + self.assertEqual(code, 0) + + +class CommittedRulesetTests(unittest.TestCase): + """Applying the committed ruleset body must satisfy the check.""" + + def test_committed_ruleset_satisfies_the_check(self) -> None: + body = json.loads((ROOT / "main-branch-ruleset.json").read_text()) + self.assertEqual(body["target"], "branch") + self.assertEqual(body["enforcement"], "active") + self.assertEqual(body["bypass_actors"], []) + self.assertEqual( + body["conditions"], + {"ref_name": {"include": ["refs/heads/main"], "exclude": []}}, + ) + origin = { + "ruleset_source_type": "Repository", + "ruleset_source": REPOSITORY, + "ruleset_id": RULESET_ID, + } + rules = [{**copy.deepcopy(rule), **origin} for rule in body["rules"]] + problems, notes = evaluate(rules=rules, bypass={RULESET_ID: []}) + self.assertEqual(problems, []) + self.assertEqual(notes, []) + + +class WorkflowTests(unittest.TestCase): + def test_profile_workflow_runs_the_check_on_schedule(self) -> None: + content = (ROOT / ".github/workflows/profile-consistency.yml").read_text() + job = content.split(" check-branch-protection:\n", 1)[1] + job = job.split("\n report-", 1)[0] + self.assertIn("github.event_name == 'schedule'", job) + self.assertIn("github.event_name == 'workflow_dispatch'", job) + self.assertIn("github.actor != 'openadapt-lifecycle[bot]'", job) + self.assertIn("python3 scripts/check_branch_protection.py", job) + self.assertIn("github.token", job) + self.assertIn("persist-credentials: false", job) + self.assertNotIn("pull_request", job) + self.assertIn(" report-unprotected-branch:\n", content) + self.assertIn("needs.check-branch-protection.result == 'failure'", content) + + +if __name__ == "__main__": + unittest.main() From 27f14fe89e941bb8d258e0dbca6f1662b299582b Mon Sep 17 00:00:00 2001 From: abrichr Date: Sat, 10 Oct 2026 00:24:56 -0400 Subject: [PATCH 2/2] fix: count pull request bypass allowances as a bypass Classic branch protection can list users, teams, or apps in bypass_pull_request_allowances. They push to main without a pull request even when enforce_admins is on, so the branch protection check treated that setup as fully bound. The check now reports the pull request requirement as bypassable when any allowance is listed, or when the allowance has a shape it doesn't recognize. Co-Authored-By: Claude Opus 5.5 --- scripts/check_branch_protection.py | 21 +++++++++++++++++- tests/test_branch_protection.py | 35 ++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/scripts/check_branch_protection.py b/scripts/check_branch_protection.py index afdd32a..12484cb 100644 --- a/scripts/check_branch_protection.py +++ b/scripts/check_branch_protection.py @@ -86,6 +86,22 @@ def _disabled(protection: dict, key: str) -> bool: return isinstance(setting, dict) and setting.get("enabled") is False +def _classic_pull_request_bypass(protection: dict) -> bool: + """Return whether classic protection lets listed actors skip the pull request. + + Users, teams, or apps in `bypass_pull_request_allowances` can push to the + branch without a pull request even when `enforce_admins` is on. An + allowance in a shape this check doesn't recognize counts as a bypass. + """ + reviews = protection.get("required_pull_request_reviews") + if not isinstance(reviews, dict) or "bypass_pull_request_allowances" not in reviews: + return False + allowances = reviews["bypass_pull_request_allowances"] + if not isinstance(allowances, dict): + return True + return any(allowances.values()) + + def classic_requirements(protection: Any) -> set[str]: """Return the requirements that readable classic protection satisfies.""" if not isinstance(protection, dict): @@ -157,7 +173,10 @@ def evaluate( # bypass list this token can't read ("unknown"). outcomes = set() if requirement in classic: - outcomes.add("bound" if classic_binds_admins else "bypass") + bound = classic_binds_admins and not ( + requirement == PULL_REQUEST and _classic_pull_request_bypass(protection) + ) + outcomes.add("bound" if bound else "bypass") for rule in rules: if rule_requirement(rule) != requirement: continue diff --git a/tests/test_branch_protection.py b/tests/test_branch_protection.py index 1c2ac52..f43bb97 100644 --- a/tests/test_branch_protection.py +++ b/tests/test_branch_protection.py @@ -133,6 +133,18 @@ def test_each_missing_classic_setting_fails(self) -> None: "deletion": lambda p: p["allow_deletions"].update(enabled=True), "missing force-push field": lambda p: p.pop("allow_force_pushes"), "admins bypass": lambda p: p["enforce_admins"].update(enabled=False), + "pull request bypass allowance": lambda p: p[ + "required_pull_request_reviews" + ].update( + bypass_pull_request_allowances={ + "users": [{"login": "maintainer", "id": 1}], + "teams": [], + "apps": [], + } + ), + "malformed pull request bypass allowance": lambda p: p[ + "required_pull_request_reviews" + ].update(bypass_pull_request_allowances=["maintainer"]), } for label, mutate in mutations.items(): with self.subTest(label): @@ -141,6 +153,29 @@ def test_each_missing_classic_setting_fails(self) -> None: problems, _ = evaluate(protection=protection) self.assertNotEqual(problems, []) + def test_pull_request_bypass_allowance_fails_only_that_requirement(self) -> None: + protection = classic_protection() + reviews = protection["required_pull_request_reviews"] + reviews["bypass_pull_request_allowances"] = { + "users": [], + "teams": [], + "apps": [{"slug": "some-app", "id": 2}], + } + problems, _ = evaluate(protection=protection) + self.assertEqual(len(problems), 1, problems) + self.assertIn(check.PULL_REQUEST, problems[0]) + self.assertIn("bypass", problems[0]) + + def test_empty_pull_request_bypass_allowances_pass(self) -> None: + protection = classic_protection() + reviews = protection["required_pull_request_reviews"] + reviews["bypass_pull_request_allowances"] = { + "users": [], + "teams": [], + "apps": [], + } + self.assertEqual(evaluate(protection=protection)[0], []) + def test_approval_floor_is_enforced(self) -> None: with mock.patch.object(check, "MINIMUM_APPROVALS", 1): problems, _ = evaluate(protection=classic_protection())