From 2019b082760b2b2840243c7dcdc124e7adc0601e Mon Sep 17 00:00:00 2001 From: jenkins Date: Mon, 17 Aug 2026 15:15:35 -0300 Subject: [PATCH] gitea: require literal branch protection rules Replace the reimplemented gobwas/glob matcher with exact-literal rule matching that fails closed on any special or malformed pattern, accepts legacy zero priorities, and rejects duplicate primary-branch rules. Co-Authored-By: Claude Fable 5 --- .../scripts/gitea_branch_protection_check.py | 111 ++++-------------- .../test_hermes_gitea_branch_coverage.py | 70 +++++++---- .../tests/test_hermes_scm_broker_policy.py | 86 ++++---------- 3 files changed, 98 insertions(+), 169 deletions(-) diff --git a/services/gitea/scripts/gitea_branch_protection_check.py b/services/gitea/scripts/gitea_branch_protection_check.py index e3f112c9..5cb9717d 100644 --- a/services/gitea/scripts/gitea_branch_protection_check.py +++ b/services/gitea/scripts/gitea_branch_protection_check.py @@ -7,7 +7,6 @@ import argparse import datetime import json import os -import re import stat from pathlib import Path @@ -35,90 +34,25 @@ def _read_bounded(path: Path) -> bytes: return value -def _glob_regex(pattern: str, position: int = 0, terminators: str = "") -> tuple[str, int]: - """Compile the gobwas/glob syntax Gitea v1.23.8 uses.""" - pieces: list[str] = [] - while position < len(pattern): - character = pattern[position] - if character in terminators: - break - if character == "\\": - position += 1 - if position >= len(pattern): - raise ValueError("trailing escape") - pieces.append(re.escape(pattern[position])) - elif character == "*": - if position + 1 < len(pattern) and pattern[position + 1] == "*": - pieces.append(".*") - position += 1 - else: - pieces.append("[^/]*") - elif character == "?": - pieces.append("[^/]") - elif character == "[": - end = pattern.find("]", position + 1) - if end < 0: - raise ValueError("unterminated range") - value = pattern[position + 1 : end] - if not value: - raise ValueError("empty range") - negate = value.startswith("!") - value = value[1:] if negate else value - if not value or "[" in value or "\\" in value: - raise ValueError("invalid range") - if "-" in value: - if value.count("-") != 1 or value.startswith("-") or value.endswith("-"): - raise ValueError("invalid range") - low, high = value.split("-") - if len(low) != 1 or len(high) != 1 or ord(high) < ord(low): - raise ValueError("invalid range") - content = re.escape(low) + "-" + re.escape(high) - else: - content = re.escape(value) - pieces.append("[" + ("^" if negate else "") + content + "]") - position = end - elif character == "{": - alternatives: list[str] = [] - position += 1 - while True: - alternative, position = _glob_regex(pattern, position, ",}") - alternatives.append(alternative) - if position >= len(pattern): - raise ValueError("unterminated alternatives") - if pattern[position] == "}": - break - position += 1 - if len(alternatives) < 2 or any(not item for item in alternatives): - raise ValueError("invalid alternatives") - pieces.append("(?:" + "|".join(alternatives) + ")") - elif character in "]}": - # A closing delimiter outside its grammar context is literal in - # gobwas/glob's lexer. - pieces.append(re.escape(character)) - else: - pieces.append(re.escape(character)) - position += 1 - return "".join(pieces), position - - def _is_plain(pattern: str) -> bool: return not any(character in SPECIAL for character in pattern) -def _matches(pattern: str, branch: str) -> bool: +def _literal_rule(pattern: str) -> str: + """Return one exact rule name or reject Gitea glob interpretation. + + Gitea 1.23.8 delegates matching to gobwas/glob. Reimplementing that + grammar and its error quoting rules here would create a second policy + engine. The reconciler instead accepts only plain ASCII literals. This + deliberately rejects braces, ranges, escapes, malformed globs, and even + special rules that appear unrelated: an operator must remove ambiguity + before this job may attest the primary-branch boundary. + """ if not pattern.isascii() or not 1 <= len(pattern) <= MAX_RULE_NAME: raise PolicyError("branch protection rule name is invalid") - if _is_plain(pattern): - return pattern.casefold() == branch.casefold() - try: - expression, position = _glob_regex(pattern) - if position != len(pattern): - raise ValueError("incomplete glob") - except (re.error, ValueError): - # Gitea quotes an invalid special pattern and matches it literally. - # A literal containing a special byte cannot equal main or master. - return False - return re.fullmatch(expression, branch) is not None + if not _is_plain(pattern): + raise PolicyError("glob branch protection rules require human review") + return pattern def _created(value: object) -> datetime.datetime: @@ -159,7 +93,7 @@ def evaluate(value: bytes, branch: str, reviewer: str) -> str: data = json.loads(value) if not isinstance(data, list) or len(data) > MAX_RULES: raise PolicyError("branch protection response is invalid") - ordered: list[tuple[int, bool, datetime.datetime, int, dict[str, object]]] = [] + ordered: list[tuple[int, datetime.datetime, int, dict[str, object]]] = [] for index, raw in enumerate(data): if not isinstance(raw, dict): raise PolicyError("branch protection entry is invalid") @@ -168,18 +102,23 @@ def evaluate(value: bytes, branch: str, reviewer: str) -> str: if ( not isinstance(priority, int) or isinstance(priority, bool) - or not 1 <= priority <= 1_000_000 + or not 0 <= priority <= 1_000_000 or not isinstance(rule_name, str) ): raise PolicyError("branch protection priority is ambiguous") created = _created(raw.get("created_at")) - if _matches(rule_name, branch): - # Gitea v1.23.8 sorts by priority, then puts plain names before - # globs, then uses creation time. The API exposes every component. - ordered.append((priority, not _is_plain(rule_name), created, index, raw)) + literal = _literal_rule(rule_name) + if literal == branch: + # With every rule constrained to a plain literal, Gitea's glob + # comparator cannot affect this result. Preserve API ordering: + # priority first (including legacy zero), creation time, then the + # stable response order for an otherwise exact tie. + ordered.append((priority, created, index, raw)) if not ordered: return "ABSENT" - effective = min(ordered)[4] + if len(ordered) != 1: + raise PolicyError("duplicate primary-branch protections are ambiguous") + effective = ordered[0][3] for key, expected in _required(reviewer).items(): if effective.get(key) != expected: raise PolicyError( diff --git a/testing/tests/test_hermes_gitea_branch_coverage.py b/testing/tests/test_hermes_gitea_branch_coverage.py index 362e8726..48421f01 100644 --- a/testing/tests/test_hermes_gitea_branch_coverage.py +++ b/testing/tests/test_hermes_gitea_branch_coverage.py @@ -83,27 +83,16 @@ def test_read_bounded_rejects_unsafe_or_changed_input( monkeypatch.setattr(module.os, "read", real_read) -@pytest.mark.parametrize( - ("pattern", "branch", "expected"), - [ - ("m?in", "main", True), - ("m[ai]in", "main", True), - ("m[!z]in", "main", True), - ("m]ain", "m]ain", True), - ("m}ain", "m}ain", True), - ("m[a-z]in", "main", True), - ("m{ain,aster}", "main", True), - ("m\\ain", "main", True), - ], -) -def test_glob_grammar_accepts_supported_constructs(pattern, branch, expected): - module = _module("branch_glob_supported") - assert module._matches(pattern, branch) is expected - - @pytest.mark.parametrize( "pattern", [ + "{main}", + "{main,master}", + "m?in", + "m[ai]in", + "m[!z]in", + "m[a-z]in", + "m\\ain", "m\\", "m[", "m[]", @@ -120,16 +109,26 @@ def test_glob_grammar_accepts_supported_constructs(pattern, branch, expected): "m{ain,aster", ], ) -def test_invalid_globs_are_literal_nonmatches(pattern): +def test_every_special_or_malformed_glob_fails_closed(pattern): module = _module("branch_glob_invalid") - assert module._matches(pattern, "main") is False + with pytest.raises(module.PolicyError, match="glob branch protection"): + module._literal_rule(pattern) @pytest.mark.parametrize("pattern", ["", "x" * 256, "máin"]) def test_rule_names_are_ascii_and_bounded(pattern): module = _module("branch_name_bounds") with pytest.raises(module.PolicyError, match="rule name"): - module._matches(pattern, "main") + module._literal_rule(pattern) + + +def test_plain_rule_matching_is_exact_and_case_sensitive(): + module = _module("branch_literal_case") + assert module._literal_rule("main") == "main" + assert module._literal_rule("MAIN") == "MAIN" + assert module.evaluate( + json.dumps([_rule(module, name="MAIN")]).encode(), "main", "bstein" + ) == "ABSENT" @pytest.mark.parametrize("value", [None, "x" * 65, "invalid", "2026-01-01T00:00:00"]) @@ -157,7 +156,7 @@ def test_evaluate_rejects_unknown_branch_and_excess_rules(): module.evaluate(json.dumps([{}] * 101).encode(), "main", "bstein") -@pytest.mark.parametrize("priority", [True, 0, 1_000_001, "1"]) +@pytest.mark.parametrize("priority", [True, -1, 1_000_001, "1"]) def test_evaluate_rejects_ambiguous_priority_types(priority): module = _module("branch_priority_bounds") rule = _rule(module, priority=priority) @@ -165,6 +164,33 @@ def test_evaluate_rejects_ambiguous_priority_types(priority): module.evaluate(json.dumps([rule]).encode(), "main", "bstein") +def test_legacy_zero_priority_is_accepted_but_duplicate_ties_fail_closed(): + module = _module("branch_priority_zero") + assert module.evaluate( + json.dumps([_rule(module, priority=0)]).encode(), "main", "bstein" + ) == "PRESENT" + + first = _rule(module, priority=0, created_at="2026-01-01T00:00:00Z") + second = _rule( + module, + priority=0, + created_at="2026-01-01T00:00:00Z", + enable_force_push=True, + ) + with pytest.raises(module.PolicyError, match="duplicate primary-branch"): + module.evaluate(json.dumps([first, second]).encode(), "main", "bstein") + + +def test_any_special_rule_blocks_absent_create_and_present_readback(): + module = _module("branch_special_readback") + for rules in ( + [_rule(module, name="feature/*")], + [_rule(module), _rule(module, name="{main}", priority=0)], + ): + with pytest.raises(module.PolicyError, match="human review"): + module.evaluate(json.dumps(rules).encode(), "main", "bstein") + + def test_main_reports_success_and_failures(tmp_path: Path, monkeypatch, capsys): module = _module("branch_main_paths") source = tmp_path / "rules.json" diff --git a/testing/tests/test_hermes_scm_broker_policy.py b/testing/tests/test_hermes_scm_broker_policy.py index 25d94fa0..163cbc91 100644 --- a/testing/tests/test_hermes_scm_broker_policy.py +++ b/testing/tests/test_hermes_scm_broker_policy.py @@ -216,83 +216,47 @@ def _protection(rule_name: str, priority: int, helper, **overrides): return value -def test_branch_protection_uses_first_effective_glob_by_priority(): +def test_branch_protection_requires_exact_literal_rules_only(): helper = _load_path( "branch_protection_test", ROOT / "services/gitea/scripts/gitea_branch_protection_check.py", ) - earlier_drift = _protection("m*", 1, helper, required_approvals=0) - later_exact = _protection("main", 2, helper) + exact = _protection("main", 2, helper) + assert ( + helper.evaluate(json.dumps([exact]).encode(), "main", "bstein") == "PRESENT" + ) + drifted = _protection("main", 2, helper, required_approvals=0) with pytest.raises(helper.PolicyError, match="effective main protection differs"): - helper.evaluate(json.dumps([later_exact, earlier_drift]).encode(), "main", "bstein") + helper.evaluate(json.dumps([drifted]).encode(), "main", "bstein") - earlier_drift["required_approvals"] = 1 - assert ( - helper.evaluate(json.dumps([later_exact, earlier_drift]).encode(), "main", "bstein") - == "PRESENT" - ) - brace_drift = _protection("m{ain,aster}", 1, helper, required_approvals=0) - with pytest.raises(helper.PolicyError, match="effective master protection differs"): - helper.evaluate(json.dumps([brace_drift]).encode(), "master", "bstein") - - upper_glob = _protection("M*", 1, helper, required_approvals=0) - assert ( - helper.evaluate(json.dumps([upper_glob, later_exact]).encode(), "main", "bstein") - == "PRESENT" - ) - - -def test_branch_protection_matches_gitea_v1238_tie_order(): - helper = _load_path( - "branch_protection_order_test", - ROOT / "services/gitea/scripts/gitea_branch_protection_check.py", - ) - older_glob = _protection( - "m*", 1, helper, created_at="2025-01-01T00:00:00Z", required_approvals=0 - ) - newer_plain = _protection( - "MAIN", 1, helper, created_at="2026-01-01T00:00:00Z" - ) - assert ( - helper.evaluate(json.dumps([older_glob, newer_plain]).encode(), "main", "bstein") - == "PRESENT" - ) - - older_glob["created_at"] = "2026-01-02T00:00:00Z" - earlier_glob = _protection( - "m{ain,aster}", - 1, - helper, - created_at="2026-01-01T00:00:00Z", - required_approvals=0, - ) - with pytest.raises(helper.PolicyError, match="effective main protection differs"): - helper.evaluate(json.dumps([older_glob, earlier_glob]).encode(), "main", "bstein") + duplicate = _protection("main", 1, helper) + with pytest.raises(helper.PolicyError, match="duplicate primary-branch"): + helper.evaluate(json.dumps([exact, duplicate]).encode(), "main", "bstein") @pytest.mark.parametrize( - ("rule_name", "branch", "expected"), + "rule_name", [ - ("release/*", "release/v1.17", True), - ("release/**/v1.17", "release/test/1/v1.17", True), - ("release/*/v1.17", "release/test/1/v1.17", False), - ("*", "release/v1.16", False), - ("**", "release/v1.16", True), - ("MAIN", "main", True), - ("M*", "main", False), - ("m{ain,aster}", "master", True), - (r"m\ain", "main", True), + "release/*", + "release/**/v1.17", + "*", + "**", + "M*", + "m{ain,aster}", + r"m\ain", + "m?in", + "m[ai]in", ], ) -def test_branch_glob_matches_gitea_v1238_source_corpus( - rule_name: str, branch: str, expected: bool -): +def test_branch_glob_rules_fail_closed_pending_human_review(rule_name: str): helper = _load_path( "branch_protection_glob_parity_test", ROOT / "services/gitea/scripts/gitea_branch_protection_check.py", ) - assert helper._matches(rule_name, branch) is expected + rules = [_protection(rule_name, 1, helper)] + with pytest.raises(helper.PolicyError, match="human review"): + helper.evaluate(json.dumps(rules).encode(), "main", "bstein") @pytest.mark.parametrize( @@ -317,5 +281,5 @@ def test_branch_protection_reports_absent_only_when_no_rule_matches(): "branch_protection_absent_test", ROOT / "services/gitea/scripts/gitea_branch_protection_check.py", ) - rules = [_protection("release/*", 1, helper)] + rules = [_protection("develop", 1, helper)] assert helper.evaluate(json.dumps(rules).encode(), "master", "bstein") == "ABSENT"