From e0b611ee5028417dbbe2393a695ed2cdfe8bb453 Mon Sep 17 00:00:00 2001 From: showxu <10173746+showxu@users.noreply.github.com> Date: Tue, 6 Oct 2026 14:16:26 +0800 Subject: [PATCH] ci: audit overlapping branch protection --- .../Reference/WorkflowIntegration.md | 5 ++-- MAINTENANCE.md | 6 ++++ Scripts/check-settings.py | 16 +++++++++++ Tests/test_settings.py | 28 +++++++++++++++++++ 4 files changed, 53 insertions(+), 2 deletions(-) diff --git a/Documentation/Reference/WorkflowIntegration.md b/Documentation/Reference/WorkflowIntegration.md index ac6dad3..da5078e 100644 --- a/Documentation/Reference/WorkflowIntegration.md +++ b/Documentation/Reference/WorkflowIntegration.md @@ -48,8 +48,9 @@ record, the declared owner identity, and attribution rules. It scans added lines and paths, then invokes the caller's `Scripts/validate-version` when that file exists. An initial push checks the complete initial history. -The `allow-terms` input lists product names required by the repository, one per -line. It does not exempt private paths or plan files. `allow-bots` lists bot +The `allow-terms` input lists identifiers and product names required by the +repository, one per line. A pagination cursor or CSS cursor keyword retains +its own meaning. It does not exempt private paths or plan files. `allow-bots` lists bot logins permitted for author or committer identity; their commits must still be Verified. Keep both lists limited to the repository's actual public surface. Shared policy sources and their tests name all supported tools, so this diff --git a/MAINTENANCE.md b/MAINTENANCE.md index 48e93d0..56c72a0 100644 --- a/MAINTENANCE.md +++ b/MAINTENANCE.md @@ -39,6 +39,11 @@ Each workflow declares the permissions it needs. - `Immutable release tags` on `v*` tags, retaining the organization-admin exception declared in [VERSIONING.md](VERSIONING.md#immutable-tags). +These rulesets own branch protection. Legacy branch-protection rules are absent; +overlapping rules can otherwise retain a branch lock or a different approval +requirement. Migrate a legacy rule only after verifying the active ruleset, +including its required checks, signatures and pull-request requirements. + | Repository | Required checks | | --- | --- | | `.github` | None yet | @@ -77,6 +82,7 @@ The checker reads metadata only and never retrieves secret values. { "organization": "swift-library", "default_branch": "master", + "legacy_branch_protection": null, "merge": { "allow_squash_merge": true, "allow_merge_commit": false, diff --git a/Scripts/check-settings.py b/Scripts/check-settings.py index d829a89..5595e91 100755 --- a/Scripts/check-settings.py +++ b/Scripts/check-settings.py @@ -47,6 +47,19 @@ def collection(path, key=None): return [item for page in pages for item in (page[key] if key else page)] +def legacy_protection(path, branch): + try: + return gh(path + "/branches/" + branch + "/protection") + except subprocess.CalledProcessError as error: + try: + response = json.loads(error.output) + except (TypeError, ValueError): + raise error + if str(response.get("status")) == "404" and response.get("message") == "Branch not protected": + return None + raise + + def compare(label, actual, expected): if isinstance(expected, dict): if not isinstance(actual, dict): @@ -104,6 +117,9 @@ def check_repository(repository, settings, checks): if name not in checks: findings.append(path + ": missing required-check declaration") return findings + findings += compare(path + ".legacy_branch_protection", + legacy_protection(path, repository["default_branch"]), + settings["legacy_branch_protection"]) actual = [gh(path + "/rulesets/" + str(rule["id"])) for rule in collection(path + "/rulesets?per_page=100")] expected = expected_rulesets(settings, checks[name]) if sorted(x["name"] for x in actual) != sorted(x["name"] for x in expected): diff --git a/Tests/test_settings.py b/Tests/test_settings.py index b02cf02..4946d2a 100644 --- a/Tests/test_settings.py +++ b/Tests/test_settings.py @@ -1,7 +1,9 @@ # SPDX-License-Identifier: Apache-2.0 WITH Swift-exception from copy import deepcopy import importlib.util +import json from pathlib import Path +import subprocess import unittest from unittest.mock import patch @@ -48,6 +50,32 @@ def test_live_repository_contract_rejects_missing_checks(self): findings = settings.check_repository(repository, self.declared, {}) self.assertEqual(findings, ["repos/swift-library/swift-gyb: missing required-check declaration"]) + def test_only_explicit_unprotected_branch_response_means_absence(self): + response = {"status": "404", "message": "Branch not protected"} + failure = subprocess.CalledProcessError(1, ["gh"], output=json.dumps(response)) + with patch.object(settings, "gh", side_effect=failure): + self.assertIsNone(settings.legacy_protection("repos/example/package", "master")) + for response in ({"status": "404", "message": "Not Found"}, + {"status": "403", "message": "Resource not accessible"}): + failure = subprocess.CalledProcessError(1, ["gh"], output=json.dumps(response)) + with patch.object(settings, "gh", side_effect=failure): + with self.assertRaises(subprocess.CalledProcessError): + settings.legacy_protection("repos/example/package", "master") + + def test_overlapping_legacy_rule_is_reported_despite_matching_rulesets(self): + repository = {"name": "swift-gyb", "default_branch": "master"} + branch, tag = settings.expected_rulesets(self.declared, self.checks["swift-gyb"]) + path = "repos/swift-library/swift-gyb" + responses = {path: self.declared["merge"], + path + "/actions/permissions/workflow": self.declared["workflow_token"], + path + "/rulesets/1": branch, path + "/rulesets/2": tag} + with patch.object(settings, "gh", side_effect=lambda url: responses[url]), \ + patch.object(settings, "check_actions", return_value=[]), \ + patch.object(settings, "collection", return_value=[{"id": 1}, {"id": 2}]), \ + patch.object(settings, "legacy_protection", return_value={"lock_branch": {"enabled": True}}): + findings = settings.check_repository(repository, self.declared, self.checks) + self.assertEqual(findings, [path + ".legacy_branch_protection: differs from declaration"]) + if __name__ == "__main__": unittest.main()