Skip to content

Add regex pattern support for branch protection rules - #81

Open
mistercrunch wants to merge 2 commits into
apache:mainfrom
mistercrunch:protect-branch-with-pattern
Open

mistercrunch wants to merge 2 commits into
apache:mainfrom
mistercrunch:protect-branch-with-pattern

Conversation

@mistercrunch

Copy link
Copy Markdown
Member

Managing branch protection across Apache projects has become increasingly complex as projects adopt more sophisticated branching strategies. Consider the challenges faced by large Apache projects:

Current Pain Points:

  • Release Branch Explosion: Projects like Kafka, Spark, and Tomcat maintain dozens of active release branches (release/3.1.x, release/3.2.x, release/4.0.x) that each require identical protection rules
  • Feature Branch Proliferation: With hundreds of contributors, feature branches (feature/user-auth, feature/payment-gateway) need consistent protection without manual configuration for each branch
  • Maintenance Overhead: Every new release branch requires manual .asf.yaml updates, creating bottlenecks during release cycles
  • Configuration Drift: Different protection rules accidentally applied to similar branches, creating security gaps
  • Hotfix Urgency: Critical security hotfixes (hotfix/CVE-2024-1234) need immediate protection rules without waiting for manual configuration

This implementation introduces regex pattern support for protected_branches, enabling rule-based branch protection that scales automatically with project growth.

1. Pattern-Based Rules

github:
  protected_branches:
    # Exact branches (existing behavior, unchanged)
    main:
      required_signatures: true

    # NEW: Pattern-based protection
    all-release-branches:
      pattern: "release/.*"
      required_signatures: true
      required_linear_history: true

    feature-branches:
      pattern: "feature/.*"
      required_pull_request_reviews:
        required_approving_review_count: 1

2. Smart Precedence System

  • Exact branch names always override pattern matches (backwards compatibility)
  • First matching pattern wins when multiple patterns match the same branch
  • Clear warnings show exactly what happened and why

3. Production-Grade Security

  • ReDoS Protection: 1000-character pattern limit prevents regex denial-of-service attacks
  • Validation at Parse Time: Invalid regex patterns fail immediately with helpful error messages
  • Pattern Compilation Caching: Optimized performance for repeated operations

4. Comprehensive Observability

Branch Protection Changes:

=== Branches Protected by Pattern Rules ===

release/v3.1.0 (via pattern rule "release-branches" (release/v.*)):
  - Set required signatures to True
  - Set required linear history to True

feature/user-auth (via pattern rule "feature-branches" (feature/.*)):
  - Set required approving review count to 1
  • Automatic Protection: New release branches inherit protection immediately

  • Reduced Toil: No manual .asf.yaml updates for routine branch operations

  • Consistent Security: Identical protection across all similar branches

  • Faster Releases: Remove YAML maintenance from release checklists

  • Default Secure: New branches matching patterns get protection automatically

  • Zero Gaps: No window where new branches are unprotected

  • Consistent Enforcement: Same rules applied uniformly across branch families

  • Audit Trail: Clear logging shows which rules applied to which branches

  • Backwards Compatible: All existing configurations work unchanged

  • Gradual Adoption: Projects can migrate at their own pace

  • Reduced Support Load: Fewer tickets about branch protection configuration

  • Scalable: Supports projects with hundreds of branches without performance impact

  • 24 new test cases covering all pattern functionality

  • Edge case coverage: Invalid regex, no matches, conflicts, precedence, performance

  • Integration scenarios: Real-world project configurations

  • Backwards compatibility: All existing tests pass unchanged (33 total tests pass)

  • Security hardened: ReDoS protection and input validation

  • Error handling: Graceful failure with actionable messages

  • Performance optimized: Single API query, compiled patterns

  • Observability: Detailed logging and warning system

  • asfyaml/validators.py - Added BranchPattern validator with security constraints

  • asfyaml/feature/github/__init__.py - Updated schema to support optional pattern field

  • asfyaml/feature/github/branch_protection.py - Core pattern matching and resolution logic

  • tests/github_branch_protection_patterns.py - Comprehensive test suite (24 test cases)

  • README.md - Complete documentation with examples and migration guidance

@rusackas rusackas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, the docs look great! Looking forward to leveraging this.

@sadpandajoe sadpandajoe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be helpful such that people won't try to push into our release branches

@mistercrunch

Copy link
Copy Markdown
Member Author

Yeah that's a not-so-uncommon pattern on Superset:

  • someone opens a PR against a release branch (they want to fix 5.0 for instance, don't know we always target master and cherry-pick) 🤦
  • a reviewer reviews the PR, don't notice it's not targeting master, super easy to miss 🤦
  • gets merged 🤦
  • breaks the workflow/pattern in release branch 🤦
  • release manager has to force push on a release branch 🤦

@clambertus
clambertus requested a review from Humbedooh August 30, 2025 02:59

@potiuk potiuk left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we merge it ? (after rebasing and reviewing - it's been a long time since it was touched)

It seems we have a long-standing PRs here that we could possibly speed up. Is there a way we can get some more volunteers to join to review and merge those?

Similary like it happened in https://github.com/apache/infrastructure-tooling (and with AI agents driving largely most of the creation/review process - ASF could definietly benfit ffrom more eyes and hands being allowed to review and merge the changes.

GitHub - responding to the recent wave of AI traffic - significantly sped up release processes and adding new features to GitHub - so we shoudl also allow our PMCs to make use of those features faster - specifically features that are aimed to harden security.

@Humbedooh @dave2wave @killerbees - I think we have enough willing (and capable and trusted) volunteers - myself, @ppkarwasz @raboof that could not only help to speed it up but also review and check upon each other's work (And agent submissions).

With infrastructure-actions - for example we are now - after all the volunteer time and agentic investments - down to "0 backlog" in terms of responding to the needs of the ASF PMCs. Basically what we have now - and number (and time) of volunteers workign there - and agentic help they get allowed to achieve that.

I think similar improvements can be achieved if the trusted volunteers are also allowed to revieew and merge changes here.

Of course it might not be desireable, but it seems that more gating currently means potentially more delays in getting the features in the hands of ASF PMCs.

Up to you to decide - but I think we need to speed up the review process for new GH features to be added - and you have pool of capable and trusted voiunteers who are wiling to help - if you choose so.

@ppkarwasz

Copy link
Copy Markdown
Member

I think we should close this PR and concentrate on improving the recently added GitHub Rulesets support:

  • Rulesets allow branch name patterns, which is what this PR introduces,
  • Moreover they are public (e.g. an HTML overview is available at https://github.com/<owner>/<repo>/rules).

@dave2wave

Copy link
Copy Markdown
Member

@ppkarwasz and @potiuk what would the rulesets look like to support the features offered in this PR?

@ppkarwasz

ppkarwasz commented May 3, 2026 •

Copy link
Copy Markdown
Member

The branch protection rules in the example above should map to these rulesets:

meta:
  environments:
    - github_rulesets

github:
  # Clear any legacy Branch Protection entries; they are replaced by Rulesets.
  protected_branches: ~

  rulesets:
    - name: "default-branch"
      type: branch
      branches:
        includes:
          - "~DEFAULT_BRANCH"
      required_signatures: true

    - name: "all-release-branches"
      type: branch
      branches:
        includes:
          - "release/*"
      required_signatures: true
      required_linear_history: true

    - name: "feature-branches"
      type: branch
      branches:
        includes:
          - "feature/*"
      required_pull_request_reviews:
        required_approving_review_count: 1

@mistercrunch: does this cover your use case?

@potiuk

potiuk commented May 3, 2026

Copy link
Copy Markdown
Member

I am for it. This is very good idea.

@ppkarwasz

ppkarwasz commented Aug 26, 2026 •

Copy link
Copy Markdown
Member

@Humbedooh, since rulesets cover this use case, should we close this PR?

@potiuk

potiuk commented Sep 18, 2026

Copy link
Copy Markdown
Member

Sandbox-tested this against my own sandbox repo. Two notes on getting it to run: the branch predates the --branch flag #127 added to asfyaml-run, so it needs a rebase onto current main to work with the sandbox workflow at all, and that rebase conflicts in README.md (code merges cleanly). I resolved the README conflict locally for the test branch only — the PR itself still needs a real rebase.

After rebasing: 169 tests pass and pre-commit is fully green (ruff, format, mypy, license).

Created four branches in the sandbox and applied a mix of exact and pattern rules:

Branch Rule Expected Result
main exact rule 1 review required ✅ required_approving_review_count: 1
feature/one pattern feature/.* 1 review required ✅ required_approving_review_count: 1
release/v1.2 pattern release/v\d+\.\d+ signatures required ✅ required_signatures: true
unmatched matches nothing untouched ✅ 404 Branch not protected

The run's summary output is genuinely nice to read, too — it prints which pattern matched which branch:

=== Branches Protected by Pattern Rules ===
feature/one (via pattern rule "feature-branches" (feature/.*)):
release/v1.2 (via pattern rule "release-branches" (release/v\d+\.\d+)):
=== Branches Protected by Exact Rules ===

Run: https://github.com/potiuk/infrastructure-asfyaml/actions/runs/35320418994

One thing worth considering before this lands. BranchPattern caps patterns at 1000 characters with the comment "Prevent ReDoS attacks", but length is not what makes a regex catastrophic — nesting is. A short pattern such as (a+)+$ backtracks exponentially, and re has no timeout, so a single project's .asf.yaml could hang the parser for everyone it runs for. The match input is branch names, which caps the blast radius, but committers control those. Worth either documenting the risk or constraining the pattern language (for example, translating shell-style globs rather than accepting arbitrary regex — which would also match how protected_tags and ruleset includes already express patterns, and would be more familiar to projects writing these files).

Minor: the new code uses Dict, List, Tuple, Set from typing, while the rest of the codebase uses builtin generics (dict[str, Any] in rulesets.py, pr_creation_cap.py and others). Python 3.10 is the floor here, so the builtins work.

Managing branch protection across Apache projects has become increasingly complex as projects adopt more sophisticated branching strategies. Consider the challenges faced by large Apache projects:

**Current Pain Points:**
- **Release Branch Explosion**: Projects like Kafka, Spark, and Tomcat maintain dozens of active release branches (`release/3.1.x`, `release/3.2.x`, `release/4.0.x`) that each require identical protection rules
- **Feature Branch Proliferation**: With hundreds of contributors, feature branches (`feature/user-auth`, `feature/payment-gateway`) need consistent protection without manual configuration for each branch
- **Maintenance Overhead**: Every new release branch requires manual `.asf.yaml` updates, creating bottlenecks during release cycles
- **Configuration Drift**: Different protection rules accidentally applied to similar branches, creating security gaps
- **Hotfix Urgency**: Critical security hotfixes (`hotfix/CVE-2024-1234`) need immediate protection rules without waiting for manual configuration

This implementation introduces **regex pattern support** for `protected_branches`, enabling rule-based branch protection that scales automatically with project growth.

**1. Pattern-Based Rules**
```yaml
github:
  protected_branches:
    # Exact branches (existing behavior, unchanged)
    main:
      required_signatures: true

    # NEW: Pattern-based protection
    all-release-branches:
      pattern: "release/.*"
      required_signatures: true
      required_linear_history: true

    feature-branches:
      pattern: "feature/.*"
      required_pull_request_reviews:
        required_approving_review_count: 1
```

**2. Smart Precedence System**
- **Exact branch names** always override pattern matches (backwards compatibility)
- **First matching pattern** wins when multiple patterns match the same branch
- **Clear warnings** show exactly what happened and why

**3. Production-Grade Security**
- **ReDoS Protection**: 1000-character pattern limit prevents regex denial-of-service attacks
- **Validation at Parse Time**: Invalid regex patterns fail immediately with helpful error messages
- **Pattern Compilation Caching**: Optimized performance for repeated operations

**4. Comprehensive Observability**
```
Branch Protection Changes:

=== Branches Protected by Pattern Rules ===

release/v3.1.0 (via pattern rule "release-branches" (release/v.*)):
  - Set required signatures to True
  - Set required linear history to True

feature/user-auth (via pattern rule "feature-branches" (feature/.*)):
  - Set required approving review count to 1
```

- **Automatic Protection**: New release branches inherit protection immediately
- **Reduced Toil**: No manual `.asf.yaml` updates for routine branch operations
- **Consistent Security**: Identical protection across all similar branches
- **Faster Releases**: Remove YAML maintenance from release checklists

- **Default Secure**: New branches matching patterns get protection automatically
- **Zero Gaps**: No window where new branches are unprotected
- **Consistent Enforcement**: Same rules applied uniformly across branch families
- **Audit Trail**: Clear logging shows which rules applied to which branches

- **Backwards Compatible**: All existing configurations work unchanged
- **Gradual Adoption**: Projects can migrate at their own pace
- **Reduced Support Load**: Fewer tickets about branch protection configuration
- **Scalable**: Supports projects with hundreds of branches without performance impact

- **24 new test cases** covering all pattern functionality
- **Edge case coverage**: Invalid regex, no matches, conflicts, precedence, performance
- **Integration scenarios**: Real-world project configurations
- **Backwards compatibility**: All existing tests pass unchanged (33 total tests pass)

- **Security hardened**: ReDoS protection and input validation
- **Error handling**: Graceful failure with actionable messages
- **Performance optimized**: Single API query, compiled patterns
- **Observability**: Detailed logging and warning system

- `asfyaml/validators.py` - Added `BranchPattern` validator with security constraints
- `asfyaml/feature/github/__init__.py` - Updated schema to support optional `pattern` field
- `asfyaml/feature/github/branch_protection.py` - Core pattern matching and resolution logic
- `tests/github_branch_protection_patterns.py` - Comprehensive test suite (24 test cases)
- `README.md` - Complete documentation with examples and migration guidance
- Rename loop variables from 'branch' to 'branch_name' to avoid type conflicts
- All pre-commit hooks pass when run independently
- Fixed variable shadowing issue in summary generation logic
@mistercrunch
mistercrunch force-pushed the protect-branch-with-pattern branch from 8e463d2 to 9dd627a Compare September 22, 2026 16:20
@rusackas

Copy link
Copy Markdown
Member

@potiuk touchups applied by @mistercrunch just FYI :)

@potiuk

potiuk commented Sep 22, 2026

Copy link
Copy Markdown
Member

Re-tested at 9dd627a against my own sandbox. The rebase landed — the branch now sits on current main and runs under the sandbox workflow with no patching. 184 tests pass, pre-commit fully green including mypy.

Ran the full matrix, noop then live, reading every result back from the GitHub API:

Case Config API result
exact rule main: 1 review required_approving_review_count: 1 ✅
pattern rule feature/.*: 1 review feature/one → 1 ✅
exact overrides pattern feature/override exact: 3 reviews feature/override → 3 ✅, warning printed
pattern rule release/v\d+\.\d+: signatures release/v1.2 → required_signatures: true ✅
update pattern review count 1 → 2 feature/one → 2 ✅
idempotent same config twice second run made zero writes ✅
drop a pattern release-branches deleted release/v1.2 → not protected ✅
unmanaged branch hand-protected, matches nothing protection removed ✅
no match unmatched stays unprotected ✅
explicit unset protected_branches: ~ every protection removed ✅

Warnings all fired correctly: pattern matching no branches, exact rule naming a non-existent branch, and exact-overrides-pattern.

Runs: noop · apply · idempotent · update · unset

One thing I'd want changed before this lands: the match is unanchored. resolve_protection_rules uses regex.match(), which anchors at the start only. I configured a rule with pattern: "dev" and the live run protected both dev and develop-2:

dev (via pattern rule "prefix-test" (dev)):
  - Set required linear history to True

develop-2 (via pattern rule "prefix-test" (dev)):
  - Set required linear history to True

Every example in the README ends in .*, so this never shows up there — but pattern: "main" would silently protect maintenance, and for branch protection an over-match is the direction that hurts. re.fullmatch would make feature/.* and friends behave identically while removing the surprise; if partial matching is deliberate, the README needs to say so explicitly.

Two notes from my earlier pass that are still open:

BranchPattern.MAX_PATTERN_LENGTH caps patterns at 1000 chars "to prevent ReDoS", but length is not what makes a regex catastrophic. (a+)+$ is six characters, passes validation, and backtracks exponentially — on my machine it takes 1.47s against a 26-character input, and branch names can be 255. re has no timeout, so one project's .asf.yaml can stall the parser for everyone. Either constrain the pattern language (translating shell-style globs would also match how protected_tags and ruleset includes already express patterns) or document the risk.

Minor: the new code imports Dict, List, Tuple, Set from typing while the rest of the codebase uses builtin generics (dict[str, Any] in rulesets.py, pr_creation_cap.py). 3.10 is the floor, so the builtins work here.

Two limits on what this proves: removing the protected_branches key entirely can't be exercised outside production — previous_yaml comes from the /x1/asfyaml cache, so in the sandbox that run is a no-op and the protections persist. That's pre-existing behaviour, not this PR. And I have not weighed in on whether this should land at all given the rulesets discussion above; this is a report that the mechanics work, not an argument for the approach.

@potiuk

potiuk commented Sep 22, 2026

Copy link
Copy Markdown
Member

cc: @rusackas @mistercrunch

@rusackas

Copy link
Copy Markdown
Member

Thanks for the thorough pass @potiuk. The fullmatch point seems like the one to sort out before this lands, main quietly matching maintenance is exactly the wrong direction for branch protection. @mistercrunch want to fold that in along with the other two?

@potiuk

potiuk commented Sep 23, 2026

Copy link
Copy Markdown
Member

Thanks for the thorough pass @potiuk. The fullmatch point seems like the one to sort out before this lands, main quietly matching maintenance is exactly the wrong direction for branch protection. @mistercrunch want to fold that in along with the other two?

Yep.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants