Add bypass allowlist for branch protection by bircni · Pull Request #36514 · go-gitea/gitea

bircni · 2026-02-01T15:02:10Z

Introduce a “Bypass Protection Allowlist” on branch rules (users/teams) alongside admins, with BlockAdminMergeOverride
still respected.
- Surface the allowlist in API (create/edit options, structs) and settings UI; merge box now shows the red button +
  message for bypass-capable users.
- Apply bypass logic to merge checks and pre-receive so allowlisted users can override unmet approvals/status checks/
  protected files when force-merging.
- Add migration for new columns, locale strings, and unit tests (bypass helper; queue test tweak).

Copilot

Pull request overview

This pull request introduces a "Bypass Protection Allowlist" feature for branch protection rules, allowing specific users and teams to bypass branch protection checks (required approvals, status checks, and protected files) without requiring full administrator privileges. This addresses the need for "Merge Masters" or "Technical Leads" who need bypass capabilities while following the Principle of Least Privilege.

Changes:

Added database columns EnableBypassAllowlist, BypassAllowlistUserIDs, and BypassAllowlistTeamIDs to the ProtectedBranch model with migration v326
Implemented CanBypassBranchProtection helper function that checks both admin privileges and allowlist membership
Extended API endpoints and Swagger documentation to support bypass allowlist configuration
Updated branch protection settings UI to allow configuration of bypass users and teams
Modified pull request merge box to display appropriate messages for bypass-capable users
Applied bypass logic in pre-receive hooks and merge checks to allow allowlisted users to override unmet requirements

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
models/migrations/v1_26/v326.go	Migration adding three new columns to ProtectedBranch table for bypass allowlist feature
models/migrations/migrations.go	Registers the new migration v326
models/git/protected_branch.go	Adds bypass allowlist fields to ProtectedBranch model and implements CanBypassBranchProtection function
models/git/protected_branch_test.go	Unit tests for CanBypassBranchProtection covering various scenarios
modules/structs/repo_branch.go	Extends API structs (BranchProtection, CreateBranchProtectionOption, EditBranchProtectionOption) with bypass allowlist fields
routers/api/v1/repo/branch.go	Updates API endpoints to handle bypass allowlist users and teams during branch protection creation and editing
routers/web/repo/setting/protected_branch.go	Handles bypass allowlist data in web UI forms for branch protection settings
routers/web/repo/issue_view.go	Sets template data for bypass permissions to control merge button visibility and messages
routers/private/hook_pre_receive.go	Applies bypass logic in pre-receive hook to allow bypass users to push despite protection checks
services/pull/check.go	Uses CanBypassBranchProtection in merge checks instead of admin-only check, updates comment
services/pull/check_test.go	Removes redundant queue.Has() checks that could cause test flakiness
services/forms/repo_form.go	Adds bypass allowlist fields to ProtectBranchForm
services/convert/convert.go	Converts bypass allowlist IDs to usernames/team names for API responses
templates/repo/settings/protected_branch.tmpl	UI form for configuring bypass allowlist users and teams
templates/repo/issue/view_content/pull_merge_box.tmpl	Displays context-appropriate message when bypass user can force merge
templates/swagger/v1_json.tmpl	Swagger documentation for bypass allowlist fields in API
options/locale/locale_en-US.json	Locale strings for bypass allowlist UI elements and messages

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

services/pull/check_test.go

models/git/protected_branch_test.go

silverwind · 2026-02-13T07:15:31Z

Review

I reviewed the full diff. The overall structure follows existing patterns well (model fields, migration, API plumbing, settings UI, template). A few observations:

1. Breaking behavior change in pre-receive hook

The original hook_pre_receive.go (line 371) unconditionally allows admins through:

if ctx.userPerm.IsAdmin() {
    return
}

The new code replaces this with CanBypassBranchProtection, which checks isRepoAdmin && !protectBranch.BlockAdminMergeOverride. This means admins who have BlockAdminMergeOverride enabled on a branch rule will now also be blocked in the git push (pre-receive) path, not just the web merge path. Previously those admins could always push past status checks, reviews, and protected files via direct push even with BlockAdminMergeOverride set.

This is arguably more correct behavior (closing an inconsistency), but it is a breaking change for existing setups and should be called out explicitly in the PR description and release notes.

2. Unrelated test change in `check_test.go`

The removal of prPatchCheckerQueue.Has(...) assertions and the reorder of go prPatchCheckerQueue.Run() is a test flakiness fix unrelated to the bypass allowlist feature. It should be in a separate commit (or separate PR) to keep the changeset focused.

3. Unused template variable `IsBranchProtectionBypasser`

In issue_view.go, IsBranchProtectionBypasser is computed and set in ctx.Data but is never referenced in any template. Either it should be used (perhaps to show a different UI indicator for bypass-list users vs admins), or it should be removed.

4. No integration test for the bypass merge path

The unit test for CanBypassBranchProtection is good, but there is no integration test covering the actual merge flow—e.g., a non-admin bypass-listed user force-merging a PR with failing status checks or missing approvals. Given this touches both the web merge check (CheckPullMergeable) and the pre-receive hook, an integration test would significantly increase confidence.

5. Minor: interaction between `BlockAdminMergeOverride` and bypass list

When BlockAdminMergeOverride is true, an admin who is also on the bypass allowlist can still bypass (because CanBypassBranchProtection falls through the admin check into the allowlist check). This is probably intentional, but it may surprise users who expect BlockAdminMergeOverride to block everyone. Worth a note in the UI help text or docs.

This comment was written by Claude (claude-opus-4-6) via Claude Code.

silverwind · 2026-02-25T23:21:05Z

What do we do about point 1? Is it worth calling out a breaking change? I think not as it's probably too obscure of a feature.

bircni · 2026-02-26T06:28:43Z

I would not call it breaking but idk

silverwind · 2026-02-26T16:00:24Z

Ok, please address points 3 and 4 if you can.

bircni · 2026-02-26T17:43:52Z

both are already done - I think

silverwind · 2026-02-26T18:48:54Z

any screenshot of the added ui?

Copilot

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

routers/api/v1/repo/branch.go

models/git/protected_branch.go

models/git/protected_branch_test.go

bircni · 2026-03-03T19:03:37Z

fixed

silverwind · 2026-03-13T14:15:04Z

Fix merge conflicts please and show a screenshot of the added UI.

bircni · 2026-03-13T20:29:37Z

@silverwind Done - here a screenshot

…ch-protection-allowlist

bircni · 2026-03-16T20:21:42Z

Updated so no conflicts

bircni · 2026-03-16T20:32:43Z

Also added a test which I somehow forgot back then :-(

services/pull/check.go

wxiaoguang · 2026-04-06T03:55:31Z

models/git/protected_branch.go

+	cacheKey := fmt.Sprintf("%d:%d:%v", user.ID, protectBranch.RepoID, protectBranch.BypassAllowlistTeamIDs)
+	in, err := cache.GetWithContextCache(ctx, cachegroup.BypassAllowlist, cacheKey, func(ctx context.Context, _ string) (bool, error) {
+		return organization.IsUserInTeams(ctx, user.ID, protectBranch.BypassAllowlistTeamIDs)
+	})


Does this cache help anything? @lunny @bircni

wxiaoguang

You skipped too many useful messages?

Details

wxiaoguang · 2026-04-06T04:17:42Z

It seems the messages conflicte with each other?

Fixed

The useful message should be kept.

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 1, 2026

github-actions bot added modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files modifies/migrations labels Feb 1, 2026

bircni force-pushed the feature/bypass-branch-protection-allowlist branch from 03c404c to 36d1fbf Compare February 1, 2026 16:32

silverwind requested a review from Copilot February 12, 2026 10:51

Copilot started reviewing on behalf of silverwind February 12, 2026 10:51 View session

Copilot AI reviewed Feb 12, 2026

View reviewed changes

services/pull/check_test.go Show resolved Hide resolved

models/git/protected_branch_test.go Show resolved Hide resolved

bircni force-pushed the feature/bypass-branch-protection-allowlist branch from 36d1fbf to da1f775 Compare February 12, 2026 20:45

silverwind requested a review from Copilot February 26, 2026 18:47

Copilot AI reviewed Feb 26, 2026

View reviewed changes

Copilot started reviewing on behalf of silverwind February 26, 2026 18:59 View session

Add bypass allowlist for branch protection

b108b00

bircni force-pushed the feature/bypass-branch-protection-allowlist branch from 03d5b44 to b108b00 Compare March 13, 2026 20:29

bircni added 3 commits March 16, 2026 18:19

Merge branch 'main' into feature/bypass-branch-protection-allowlist

42e04fa

change to v328 migration

d2c9157

Merge remote-tracking branch 'upstream/main' into feature/bypass-bran…

c429020

…ch-protection-allowlist

Add a migration test test

e16164e

bircni requested review from lunny and removed request for lunny April 5, 2026 07:31

fix

96f4123

bircni force-pushed the feature/bypass-branch-protection-allowlist branch from 0831502 to 96f4123 Compare April 5, 2026 08:14

lunny approved these changes Apr 5, 2026

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 5, 2026

Merge branch 'main' into feature/bypass-branch-protection-allowlist

8a006ac

bircni added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 5, 2026

GiteaBot added 3 commits April 6, 2026 03:13

Merge branch 'main' into feature/bypass-branch-protection-allowlist

342f7e8

Merge branch 'main' into feature/bypass-branch-protection-allowlist

6c3c829

Merge branch 'main' into feature/bypass-branch-protection-allowlist

3510261

wxiaoguang marked this pull request as draft April 6, 2026 03:12

wxiaoguang reviewed Apr 6, 2026

View reviewed changes

services/pull/check.go Outdated Show resolved Hide resolved

wxiaoguang added 2 commits April 6, 2026 11:18

fix CanBypassBranchProtection

c7fb30f

refactor

56accf4

wxiaoguang reviewed Apr 6, 2026

View reviewed changes

services/pull/check.go Outdated Show resolved Hide resolved

wxiaoguang added 3 commits April 6, 2026 11:40

fix variable name

13dc7b9

fine tune

9642ce5

fix layout (due to recent checkbox changes)

0125c5a

github-actions bot added the modifies/frontend label Apr 6, 2026

wxiaoguang reviewed Apr 6, 2026

View reviewed changes

wxiaoguang added 2 commits April 6, 2026 12:08

fix logic

b3591f9

fix logic

060a5e4

wxiaoguang requested changes Apr 6, 2026

View reviewed changes

GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels Apr 6, 2026

Merge branch 'main' into feature/bypass-branch-protection-allowlist

a1d3638

wxiaoguang removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 6, 2026

Uh oh!

Conversation

bircni commented Feb 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

silverwind commented Feb 13, 2026

Review

1. Breaking behavior change in pre-receive hook

2. Unrelated test change in check_test.go

3. Unused template variable IsBranchProtectionBypasser

4. No integration test for the bypass merge path

5. Minor: interaction between BlockAdminMergeOverride and bypass list

Uh oh!

silverwind commented Feb 25, 2026

Uh oh!

bircni commented Feb 26, 2026

Uh oh!

silverwind commented Feb 26, 2026

Uh oh!

bircni commented Feb 26, 2026

Uh oh!

silverwind commented Feb 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bircni commented Mar 3, 2026

Uh oh!

silverwind commented Mar 13, 2026

Uh oh!

bircni commented Mar 13, 2026

Uh oh!

bircni commented Mar 16, 2026

Uh oh!

bircni commented Mar 16, 2026

Uh oh!

Uh oh!

Uh oh!

wxiaoguang Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

wxiaoguang left a comment

Choose a reason for hiding this comment

Uh oh!

wxiaoguang commented Apr 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

bircni commented Feb 1, 2026 •

edited

Loading

2. Unrelated test change in `check_test.go`

3. Unused template variable `IsBranchProtectionBypasser`

5. Minor: interaction between `BlockAdminMergeOverride` and bypass list