From eb93981d45a7860e148f2bec65663493790ba478 Mon Sep 17 00:00:00 2001 From: Nicolas Date: Sat, 16 May 2026 16:23:42 +0200 Subject: [PATCH] feat: Add bypass allowlist for branch protection (#36514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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). image Fixes #36476 --------- Co-authored-by: silverwind Co-authored-by: Giteabot Co-authored-by: wxiaoguang Co-authored-by: Codex GPT-5.3 Co-authored-by: GPT-5.2 Co-authored-by: Cursor Co-authored-by: Claude (Opus 4.7) --- models/git/protected_branch.go | 41 +++++++++++ models/git/protected_branch_test.go | 49 +++++++++++++ models/migrations/migrations.go | 1 + models/migrations/v1_27/v333.go | 20 ++++++ models/migrations/v1_27/v333_test.go | 60 ++++++++++++++++ modules/structs/repo_branch.go | 9 +++ options/locale/locale_en-US.json | 8 ++- routers/api/v1/repo/branch.go | 68 ++++++++++++++++++- routers/private/hook_pre_receive.go | 4 +- routers/web/repo/issue_view.go | 17 +++-- routers/web/repo/pull.go | 8 ++- routers/web/repo/pull_merge_box.go | 12 ++-- routers/web/repo/pull_merge_form.go | 8 +-- routers/web/repo/setting/protected_branch.go | 16 ++++- services/convert/convert.go | 5 ++ services/forms/repo_form.go | 3 + services/pull/check.go | 30 ++++---- templates/repo/settings/protected_branch.tmpl | 43 ++++++++++++ templates/swagger/v1_json.tmpl | 54 +++++++++++++++ templates/swagger/v1_openapi3_json.tmpl | 54 +++++++++++++++ tests/integration/api_branch_test.go | 33 +++++++++ tests/integration/pull_merge_test.go | 67 ++++++++++++++++++ web_src/css/repo.css | 2 +- 23 files changed, 572 insertions(+), 40 deletions(-) create mode 100644 models/migrations/v1_27/v333.go create mode 100644 models/migrations/v1_27/v333_test.go diff --git a/models/git/protected_branch.go b/models/git/protected_branch.go index f242f94f7b..32014615e6 100644 --- a/models/git/protected_branch.go +++ b/models/git/protected_branch.go @@ -43,6 +43,9 @@ type ProtectedBranch struct { WhitelistDeployKeys bool `xorm:"NOT NULL DEFAULT false"` MergeWhitelistUserIDs []int64 `xorm:"JSON TEXT"` MergeWhitelistTeamIDs []int64 `xorm:"JSON TEXT"` + EnableBypassAllowlist bool `xorm:"NOT NULL DEFAULT false"` + BypassAllowlistUserIDs []int64 `xorm:"JSON TEXT"` + BypassAllowlistTeamIDs []int64 `xorm:"JSON TEXT"` CanForcePush bool `xorm:"NOT NULL DEFAULT false"` EnableForcePushAllowlist bool `xorm:"NOT NULL DEFAULT false"` ForcePushAllowlistUserIDs []int64 `xorm:"JSON TEXT"` @@ -204,6 +207,29 @@ func IsUserMergeWhitelisted(ctx context.Context, protectBranch *ProtectedBranch, return in } +// CanBypassBranchProtection reports whether the user can bypass branch protection checks (status checks, approvals, protected files) +// Either a repo admin (when not blocked) or a user/team on the bypass allowlist can bypass. +func CanBypassBranchProtection(ctx context.Context, protectBranch *ProtectedBranch, user *user_model.User, isRepoAdmin bool) bool { + if isRepoAdmin && !protectBranch.BlockAdminMergeOverride { + return true + } + if !protectBranch.EnableBypassAllowlist { + return false + } + if slices.Contains(protectBranch.BypassAllowlistUserIDs, user.ID) { + return true + } + if len(protectBranch.BypassAllowlistTeamIDs) == 0 { + return false + } + in, err := organization.IsUserInTeams(ctx, user.ID, protectBranch.BypassAllowlistTeamIDs) + if err != nil { + log.Error("IsUserInTeams failed: userID=%d, repoID=%d, allowlistTeamIDs=%v, err=%v", user.ID, protectBranch.RepoID, protectBranch.BypassAllowlistTeamIDs, err) + return false + } + return in +} + // IsUserOfficialReviewer check if user is official reviewer for the branch (counts towards required approvals) func IsUserOfficialReviewer(ctx context.Context, protectBranch *ProtectedBranch, user *user_model.User) (bool, error) { repo, err := repo_model.GetRepositoryByID(ctx, protectBranch.RepoID) @@ -347,6 +373,9 @@ type WhitelistOptions struct { ApprovalsUserIDs []int64 ApprovalsTeamIDs []int64 + + BypassUserIDs []int64 + BypassTeamIDs []int64 } // UpdateProtectBranch saves branch protection options of repository. @@ -387,6 +416,12 @@ func UpdateProtectBranch(ctx context.Context, repo *repo_model.Repository, prote } protectBranch.ApprovalsWhitelistUserIDs = whitelist + whitelist, err = updateUserWhitelist(ctx, repo, protectBranch.BypassAllowlistUserIDs, opts.BypassUserIDs) + if err != nil { + return err + } + protectBranch.BypassAllowlistUserIDs = whitelist + // if the repo is in an organization whitelist, err = updateTeamWhitelist(ctx, repo, protectBranch.WhitelistTeamIDs, opts.TeamIDs) if err != nil { @@ -412,6 +447,12 @@ func UpdateProtectBranch(ctx context.Context, repo *repo_model.Repository, prote } protectBranch.ApprovalsWhitelistTeamIDs = whitelist + whitelist, err = updateTeamWhitelist(ctx, repo, protectBranch.BypassAllowlistTeamIDs, opts.BypassTeamIDs) + if err != nil { + return err + } + protectBranch.BypassAllowlistTeamIDs = whitelist + // Looks like it's a new rule if protectBranch.ID == 0 { // as it's a new rule and if priority was not set, we need to calc it. diff --git a/models/git/protected_branch_test.go b/models/git/protected_branch_test.go index 3aa1d7daa8..043e24cbe8 100644 --- a/models/git/protected_branch_test.go +++ b/models/git/protected_branch_test.go @@ -9,6 +9,7 @@ import ( "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" "github.com/stretchr/testify/assert" ) @@ -153,3 +154,51 @@ func TestNewProtectBranchPriority(t *testing.T) { assert.NoError(t, err) assert.Equal(t, int64(2), savedPB2.Priority) } + +func TestCanBypassBranchProtection(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) // not in team 1 + teamMember := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + pb := &ProtectedBranch{ + EnableBypassAllowlist: true, + BypassAllowlistUserIDs: []int64{user.ID}, + } + + testBypass := func(t *testing.T, expected bool, pb *ProtectedBranch, doer *user_model.User, isAdmin bool) { + assert.Equal(t, expected, CanBypassBranchProtection(t.Context(), pb, doer, isAdmin)) + } + // User bypasses via explicit allowlist. + testBypass(t, true, pb, user, false) + + // Non-admin cannot bypass when allowlist is disabled. + pb.EnableBypassAllowlist = false + testBypass(t, false, pb, user, false) + + // Repo admin can bypass independently of allowlist when not blocked. + testBypass(t, true, pb, user, true) + + // Admin override block still allows bypass for allowlisted users. + pb.EnableBypassAllowlist = true + pb.BlockAdminMergeOverride = true + testBypass(t, true, pb, user, false) + + // admin cannot bypass without allowlist membership. + pb.BypassAllowlistUserIDs = nil + testBypass(t, false, pb, user, true) + + // admin can bypass when allowlisted. + pb.BypassAllowlistUserIDs = []int64{user.ID} + testBypass(t, true, pb, user, true) + + // User bypasses via team allowlist membership. + pb.EnableBypassAllowlist = true + pb.BlockAdminMergeOverride = false + pb.BypassAllowlistUserIDs = nil + pb.BypassAllowlistTeamIDs = []int64{1} // team 1 contains user 2 in test fixtures + testBypass(t, true, pb, teamMember, false) + + // User does not bypass when not in allowlisted teams. + testBypass(t, false, pb, user, false) +} diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index c1d247bc41..3689f448d8 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -410,6 +410,7 @@ func prepareMigrationTasks() []*migration { newMigration(331, "Add ActionRunAttempt model and related action fields", v1_27.AddActionRunAttemptModel), newMigration(332, "Add last_sync_unix to mirror", v1_27.AddLastSyncUnixToMirror), + newMigration(333, "Add bypass allowlist to branch protection", v1_27.AddBranchProtectionBypassAllowlist), } return preparedMigrations } diff --git a/models/migrations/v1_27/v333.go b/models/migrations/v1_27/v333.go new file mode 100644 index 0000000000..0ffab0a1f2 --- /dev/null +++ b/models/migrations/v1_27/v333.go @@ -0,0 +1,20 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_27 + +import "xorm.io/xorm" + +func AddBranchProtectionBypassAllowlist(x *xorm.Engine) error { + type ProtectedBranch struct { + EnableBypassAllowlist bool `xorm:"NOT NULL DEFAULT false"` + BypassAllowlistUserIDs []int64 `xorm:"JSON TEXT"` + BypassAllowlistTeamIDs []int64 `xorm:"JSON TEXT"` + } + + _, err := x.SyncWithOptions(xorm.SyncOptions{ + IgnoreConstrains: true, + IgnoreIndices: true, + }, new(ProtectedBranch)) + return err +} diff --git a/models/migrations/v1_27/v333_test.go b/models/migrations/v1_27/v333_test.go new file mode 100644 index 0000000000..768fa10261 --- /dev/null +++ b/models/migrations/v1_27/v333_test.go @@ -0,0 +1,60 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_27 + +import ( + "testing" + + "code.gitea.io/gitea/models/migrations/migrationtest" + + "github.com/stretchr/testify/require" +) + +func Test_AddBranchProtectionBypassAllowlist(t *testing.T) { + type ProtectedBranch struct { + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"INDEX"` + BranchName string `xorm:"INDEX"` + EnableBypassAllowlist bool `xorm:"NOT NULL DEFAULT false"` + BypassAllowlistUserIDs []int64 `xorm:"JSON TEXT"` + BypassAllowlistTeamIDs []int64 `xorm:"JSON TEXT"` + } + + x, deferable := migrationtest.PrepareTestEnv(t, 0, new(ProtectedBranch)) + defer deferable() + + // Test with default values + _, err := x.Insert(&ProtectedBranch{RepoID: 1, BranchName: "main"}) + require.NoError(t, err) + + // Test with populated allowlist + _, err = x.Insert(&ProtectedBranch{ + RepoID: 1, + BranchName: "develop", + EnableBypassAllowlist: true, + BypassAllowlistUserIDs: []int64{1, 2, 3}, + BypassAllowlistTeamIDs: []int64{10, 20}, + }) + require.NoError(t, err) + + require.NoError(t, AddBranchProtectionBypassAllowlist(x)) + + // Verify the default values record + var pb ProtectedBranch + has, err := x.Where("repo_id = ? AND branch_name = ?", 1, "main").Get(&pb) + require.NoError(t, err) + require.True(t, has) + require.False(t, pb.EnableBypassAllowlist) + require.Nil(t, pb.BypassAllowlistUserIDs) + require.Nil(t, pb.BypassAllowlistTeamIDs) + + // Verify the populated allowlist record + var pb2 ProtectedBranch + has, err = x.Where("repo_id = ? AND branch_name = ?", 1, "develop").Get(&pb2) + require.NoError(t, err) + require.True(t, has) + require.True(t, pb2.EnableBypassAllowlist) + require.Equal(t, []int64{1, 2, 3}, pb2.BypassAllowlistUserIDs) + require.Equal(t, []int64{10, 20}, pb2.BypassAllowlistTeamIDs) +} diff --git a/modules/structs/repo_branch.go b/modules/structs/repo_branch.go index 75f7878aa6..fc84ab364e 100644 --- a/modules/structs/repo_branch.go +++ b/modules/structs/repo_branch.go @@ -50,6 +50,9 @@ type BranchProtection struct { EnableMergeWhitelist bool `json:"enable_merge_whitelist"` MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"` MergeWhitelistTeams []string `json:"merge_whitelist_teams"` + EnableBypassAllowlist bool `json:"enable_bypass_allowlist"` + BypassAllowlistUsernames []string `json:"bypass_allowlist_usernames"` + BypassAllowlistTeams []string `json:"bypass_allowlist_teams"` EnableStatusCheck bool `json:"enable_status_check"` StatusCheckContexts []string `json:"status_check_contexts"` RequiredApprovals int64 `json:"required_approvals"` @@ -90,6 +93,9 @@ type CreateBranchProtectionOption struct { EnableMergeWhitelist bool `json:"enable_merge_whitelist"` MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"` MergeWhitelistTeams []string `json:"merge_whitelist_teams"` + EnableBypassAllowlist bool `json:"enable_bypass_allowlist"` + BypassAllowlistUsernames []string `json:"bypass_allowlist_usernames"` + BypassAllowlistTeams []string `json:"bypass_allowlist_teams"` EnableStatusCheck bool `json:"enable_status_check"` StatusCheckContexts []string `json:"status_check_contexts"` RequiredApprovals int64 `json:"required_approvals"` @@ -123,6 +129,9 @@ type EditBranchProtectionOption struct { EnableMergeWhitelist *bool `json:"enable_merge_whitelist"` MergeWhitelistUsernames []string `json:"merge_whitelist_usernames"` MergeWhitelistTeams []string `json:"merge_whitelist_teams"` + EnableBypassAllowlist *bool `json:"enable_bypass_allowlist"` + BypassAllowlistUsernames []string `json:"bypass_allowlist_usernames"` + BypassAllowlistTeams []string `json:"bypass_allowlist_teams"` EnableStatusCheck *bool `json:"enable_status_check"` StatusCheckContexts []string `json:"status_check_contexts"` RequiredApprovals *int64 `json:"required_approvals"` diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index 25fdafb550..46992c5760 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -1819,6 +1819,7 @@ "repo.pulls.required_status_check_failed": "Some required checks were not successful.", "repo.pulls.required_status_check_missing": "Some required checks are missing.", "repo.pulls.required_status_check_administrator": "As an administrator, you may still merge this pull request.", + "repo.pulls.required_status_check_bypass_allowlist": "You are allowed to bypass branch protection rules for this merge.", "repo.pulls.blocked_by_approvals": "This pull request doesn't have enough required approvals yet. %d of %d official approvals granted.", "repo.pulls.blocked_by_approvals_whitelisted": "This pull request doesn't have enough required approvals yet. %d of %d approvals granted from users or teams on the allowlist.", "repo.pulls.blocked_by_rejection": "This pull request has changes requested by an official reviewer.", @@ -2415,6 +2416,11 @@ "repo.settings.protect_merge_whitelist_committers_desc": "Allow only allowlisted users or teams to merge pull requests into this branch.", "repo.settings.protect_merge_whitelist_users": "Allowlisted users for merging:", "repo.settings.protect_merge_whitelist_teams": "Allowlisted teams for merging:", + "repo.settings.protect_bypass_allowlist": "Bypass branch protection", + "repo.settings.protect_enable_bypass_allowlist": "Allow selected users or teams to bypass branch protection", + "repo.settings.protect_enable_bypass_allowlist_desc": "Allowlisted users or teams can merge or push even when required approvals, status checks, or protected-file rules would otherwise block them.", + "repo.settings.protect_bypass_allowlist_users": "Allowlisted users for bypassing protection:", + "repo.settings.protect_bypass_allowlist_teams": "Allowlisted teams for bypassing protection:", "repo.settings.protect_check_status_contexts": "Enable Status Check", "repo.settings.protect_status_check_patterns": "Status check patterns:", "repo.settings.protect_status_check_patterns_desc": "Enter patterns to specify which status checks must pass before branches can be merged into a branch that matches this rule. Each line specifies a pattern. Patterns cannot be empty.", @@ -2456,7 +2462,7 @@ "repo.settings.block_outdated_branch": "Block merge if pull request is outdated", "repo.settings.block_outdated_branch_desc": "Merging will not be possible when head branch is behind base branch.", "repo.settings.block_admin_merge_override": "Administrators must follow branch protection rules", - "repo.settings.block_admin_merge_override_desc": "Administrators must follow branch protection rules and cannot circumvent it.", + "repo.settings.block_admin_merge_override_desc": "Administrators must follow branch protection rules and cannot circumvent it. Users or teams in the bypass allowlist can still bypass these rules if bypass allowlist is enabled.", "repo.settings.default_branch_desc": "Select a default branch for code commits.", "repo.settings.default_target_branch_desc": "Pull requests can use different default target branch if it is set in the Pull Requests section of Repository Advance Settings.", "repo.settings.merge_style_desc": "Merge Styles", diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index a9b88317d4..248bb8b5b2 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -711,7 +711,19 @@ func CreateBranchProtection(ctx *context.APIContext) { ctx.APIErrorInternal(err) return } - var whitelistTeams, forcePushAllowlistTeams, mergeWhitelistTeams, approvalsWhitelistTeams []int64 + var bypassAllowlistUsers []int64 + if form.EnableBypassAllowlist { + bypassAllowlistUsers, err = user_model.GetUserIDsByNames(ctx, form.BypassAllowlistUsernames, false) + if err != nil { + if user_model.IsErrUserNotExist(err) { + ctx.APIError(http.StatusUnprocessableEntity, err) + return + } + ctx.APIErrorInternal(err) + return + } + } + var whitelistTeams, forcePushAllowlistTeams, mergeWhitelistTeams, approvalsWhitelistTeams, bypassAllowlistTeams []int64 if repo.Owner.IsOrganization() { whitelistTeams, err = organization.GetTeamIDsByNames(ctx, repo.OwnerID, form.PushWhitelistTeams, false) if err != nil { @@ -749,6 +761,17 @@ func CreateBranchProtection(ctx *context.APIContext) { ctx.APIErrorInternal(err) return } + if form.EnableBypassAllowlist { + bypassAllowlistTeams, err = organization.GetTeamIDsByNames(ctx, repo.OwnerID, form.BypassAllowlistTeams, false) + if err != nil { + if organization.IsErrTeamNotExist(err) { + ctx.APIError(http.StatusUnprocessableEntity, err) + return + } + ctx.APIErrorInternal(err) + return + } + } } protectBranch = &git_model.ProtectedBranch{ @@ -762,6 +785,7 @@ func CreateBranchProtection(ctx *context.APIContext) { EnableForcePushAllowlist: form.EnablePush && form.EnableForcePush && form.EnableForcePushAllowlist, ForcePushAllowlistDeployKeys: form.EnablePush && form.EnableForcePush && form.EnableForcePushAllowlist && form.ForcePushAllowlistDeployKeys, EnableMergeWhitelist: form.EnableMergeWhitelist, + EnableBypassAllowlist: form.EnableBypassAllowlist, EnableStatusCheck: form.EnableStatusCheck, StatusCheckContexts: form.StatusCheckContexts, EnableApprovalsWhitelist: form.EnableApprovalsWhitelist, @@ -786,6 +810,8 @@ func CreateBranchProtection(ctx *context.APIContext) { MergeTeamIDs: mergeWhitelistTeams, ApprovalsUserIDs: approvalsWhitelistUsers, ApprovalsTeamIDs: approvalsWhitelistTeams, + BypassUserIDs: bypassAllowlistUsers, + BypassTeamIDs: bypassAllowlistTeams, }); err != nil { ctx.APIErrorInternal(err) return @@ -906,6 +932,10 @@ func EditBranchProtection(ctx *context.APIContext) { protectBranch.EnableMergeWhitelist = *form.EnableMergeWhitelist } + if form.EnableBypassAllowlist != nil { + protectBranch.EnableBypassAllowlist = *form.EnableBypassAllowlist + } + if form.EnableStatusCheck != nil { protectBranch.EnableStatusCheck = *form.EnableStatusCheck } @@ -958,7 +988,7 @@ func EditBranchProtection(ctx *context.APIContext) { protectBranch.BlockAdminMergeOverride = *form.BlockAdminMergeOverride } - var whitelistUsers, forcePushAllowlistUsers, mergeWhitelistUsers, approvalsWhitelistUsers []int64 + var whitelistUsers, forcePushAllowlistUsers, mergeWhitelistUsers, approvalsWhitelistUsers, bypassAllowlistUsers []int64 if form.PushWhitelistUsernames != nil { whitelistUsers, err = user_model.GetUserIDsByNames(ctx, form.PushWhitelistUsernames, false) if err != nil { @@ -1011,8 +1041,21 @@ func EditBranchProtection(ctx *context.APIContext) { } else { approvalsWhitelistUsers = protectBranch.ApprovalsWhitelistUserIDs } + if form.BypassAllowlistUsernames != nil { + bypassAllowlistUsers, err = user_model.GetUserIDsByNames(ctx, form.BypassAllowlistUsernames, false) + if err != nil { + if user_model.IsErrUserNotExist(err) { + ctx.APIError(http.StatusUnprocessableEntity, err) + return + } + ctx.APIErrorInternal(err) + return + } + } else { + bypassAllowlistUsers = protectBranch.BypassAllowlistUserIDs + } - var whitelistTeams, forcePushAllowlistTeams, mergeWhitelistTeams, approvalsWhitelistTeams []int64 + var whitelistTeams, forcePushAllowlistTeams, mergeWhitelistTeams, approvalsWhitelistTeams, bypassAllowlistTeams []int64 if repo.Owner.IsOrganization() { if form.PushWhitelistTeams != nil { whitelistTeams, err = organization.GetTeamIDsByNames(ctx, repo.OwnerID, form.PushWhitelistTeams, false) @@ -1066,6 +1109,23 @@ func EditBranchProtection(ctx *context.APIContext) { } else { approvalsWhitelistTeams = protectBranch.ApprovalsWhitelistTeamIDs } + if form.BypassAllowlistTeams != nil { + bypassAllowlistTeams, err = organization.GetTeamIDsByNames(ctx, repo.OwnerID, form.BypassAllowlistTeams, false) + if err != nil { + if organization.IsErrTeamNotExist(err) { + ctx.APIError(http.StatusUnprocessableEntity, err) + return + } + ctx.APIErrorInternal(err) + return + } + } else { + bypassAllowlistTeams = protectBranch.BypassAllowlistTeamIDs + } + } + if !protectBranch.EnableBypassAllowlist { + bypassAllowlistUsers = nil + bypassAllowlistTeams = nil } err = git_model.UpdateProtectBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{ @@ -1077,6 +1137,8 @@ func EditBranchProtection(ctx *context.APIContext) { MergeTeamIDs: mergeWhitelistTeams, ApprovalsUserIDs: approvalsWhitelistUsers, ApprovalsTeamIDs: approvalsWhitelistTeams, + BypassUserIDs: bypassAllowlistUsers, + BypassTeamIDs: bypassAllowlistTeams, }) if err != nil { ctx.APIErrorInternal(err) diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index 3936e223d5..deaeadb5ef 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -371,8 +371,8 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r return } - // If we're an admin for the repository we can ignore status checks, reviews and override protected files - if ctx.userPerm.IsAdmin() { + // If we can bypass branch protection we can ignore status checks, reviews and protected files + if git_model.CanBypassBranchProtection(ctx, protectBranch, ctx.user, ctx.userPerm.IsAdmin()) { return } diff --git a/routers/web/repo/issue_view.go b/routers/web/repo/issue_view.go index 1a42884ef8..93f24637da 100644 --- a/routers/web/repo/issue_view.go +++ b/routers/web/repo/issue_view.go @@ -907,7 +907,7 @@ func (prInfo *pullRequestViewInfo) prepareMergeBox(ctx *context.Context, issue * if !canWriteToHeadRepo { // maintainers maybe allowed to push to head repo even if they can't write to it canWriteToHeadRepo = pull.AllowMaintainerEdit && perm.CanWrite(unit.TypeCode) } - data.allowMerge, err = pull_service.IsUserAllowedToMerge(ctx, pull, perm, ctx.Doer) + data.hasPermToMerge, err = pull_service.IsUserAllowedToMerge(ctx, pull, perm, ctx.Doer) if err != nil { ctx.ServerError("IsUserAllowedToMerge", err) return @@ -954,16 +954,19 @@ func (prInfo *pullRequestViewInfo) prepareMergeBox(ctx *context.Context, issue * // this logic is from: // {{$notAllOverridableChecksOk := or .IsBlockedByApprovals .IsBlockedByRejection .IsBlockedByOfficialReviewRequests .IsBlockedByOutdatedBranch .IsBlockedByChangedProtectedFiles (and .EnableStatusCheck (not $requiredStatusCheckState.IsSuccess))}} // HINT: if a PR's status is not mergeable, then it is a non-overridable blocker, such logic is handled separately (see IsStatusMergeable) - data.HasOverridableBlockers = data.isBlockedByApprovals || data.isBlockedByRejection || + data.hasOverridableBlockers = data.isBlockedByApprovals || data.isBlockedByRejection || data.isBlockedByOfficialReviewRequests || data.isBlockedByOutdatedBranch || data.isBlockedByChangedProtectedFiles || data.hasStatusCheckBlocker - // this logic is from: - // {{$canMergeNow := and (or (and (not $.ProtectedBranch.BlockAdminMergeOverride) $.IsRepoAdmin) (not $notAllOverridableChecksOk)) (or (not .AllowMerge) (not .RequireSigned) .WillSign)}} - // HINT: legacy "(not .AllowMerge)" is not right (always false, does nothing), fixed here + data.canBypassProtection = isRepoAdmin + data.canBypassProtectionAsAdmin = isRepoAdmin + if ctx.IsSigned && prInfo.ProtectedBranchRule != nil { + data.canBypassProtection = git_model.CanBypassBranchProtection(ctx, prInfo.ProtectedBranchRule, ctx.Doer, isRepoAdmin) + data.canBypassProtectionAsAdmin = isRepoAdmin && !prInfo.ProtectedBranchRule.BlockAdminMergeOverride + } + // CanMergeNow means: if the doer has write permission, whether the PR can be merged now - adminCanOverrideBlockers := (prInfo.ProtectedBranchRule == nil || !prInfo.ProtectedBranchRule.BlockAdminMergeOverride) && isRepoAdmin - data.CanMergeNow = (!data.HasOverridableBlockers || adminCanOverrideBlockers) && // status checks are satisfied + data.canMergeNow = (!data.hasOverridableBlockers || data.canBypassProtection) && // status checks are satisfied (!data.requireSigned || data.willSign) // signing requirement is satisfied prInfo.prepareMergeBoxFormProps(ctx) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index bf96dbd2be..4c1a2afb75 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -276,9 +276,11 @@ type pullMergeBoxData struct { StatusCheckData *pullCommitStatusCheckData ShowStatusCheck bool - HasOverridableBlockers bool - CanMergeNow bool // PR is mergeable, either no blocker, or doer is admin and can bypass the blockers - allowMerge bool // doer has permission to merge + hasOverridableBlockers bool + canMergeNow bool // PR is mergeable, either no blocker, or doer can bypass the blockers + hasPermToMerge bool // doer has permission to merge + canBypassProtection bool + canBypassProtectionAsAdmin bool ShowUpdatePullInfo bool UpdatePrimaryAction *pullUpdateAction diff --git a/routers/web/repo/pull_merge_box.go b/routers/web/repo/pull_merge_box.go index 27bb8aef28..aae22bbfb2 100644 --- a/routers/web/repo/pull_merge_box.go +++ b/routers/web/repo/pull_merge_box.go @@ -162,18 +162,22 @@ func (prInfo *pullRequestViewInfo) prepareMergeBoxInfoItems(ctx *context.Context } } - if !data.allowMerge { + if !data.hasPermToMerge { prInfo.MergeBoxData.infoProtectionBlockers.AddInfoItem( svg.RenderHTML("octicon-info"), ctx.Locale.Tr("repo.pulls.no_merge_access"), ) } - if data.CanMergeNow { - if data.HasOverridableBlockers { + if data.canMergeNow { + if data.hasOverridableBlockers { + prompt := ctx.Locale.Tr("repo.pulls.required_status_check_bypass_allowlist") + if data.canBypassProtectionAsAdmin { + prompt = ctx.Locale.Tr("repo.pulls.required_status_check_administrator") + } prInfo.MergeBoxData.infoMergePrompts.AddInfoItem( svg.RenderHTML("octicon-dot-fill"), - ctx.Locale.Tr("repo.pulls.required_status_check_administrator"), + prompt, ) } else if pull.IsStatusMergeable() || pull.IsEmpty() { prInfo.MergeBoxData.infoMergePrompts.AddInfoItem( diff --git a/routers/web/repo/pull_merge_form.go b/routers/web/repo/pull_merge_form.go index 27619a8877..3fa3a91bc9 100644 --- a/routers/web/repo/pull_merge_form.go +++ b/routers/web/repo/pull_merge_form.go @@ -20,7 +20,7 @@ func (prInfo *pullRequestViewInfo) prepareMergeBoxFormProps(ctx *context.Context if pull.HasMerged || prInfo.issue.IsClosed { return } - if !prInfo.MergeBoxData.allowMerge { + if !prInfo.MergeBoxData.hasPermToMerge { return } @@ -76,7 +76,7 @@ func (prInfo *pullRequestViewInfo) prepareMergeBoxFormProps(ctx *context.Context defaultSquashMergeCommitMessages = pull_service.GetSquashMergeCommitMessages(ctx, pull) } - allOverridableChecksOk := !prInfo.MergeBoxData.HasOverridableBlockers + allOverridableChecksOk := !prInfo.MergeBoxData.hasOverridableBlockers mergeFormProps := map[string]any{ "baseLink": prInfo.issue.Link(), "textCancel": ctx.Locale.Tr("cancel"), @@ -88,7 +88,7 @@ func (prInfo *pullRequestViewInfo) prepareMergeBoxFormProps(ctx *context.Context "textClearMergeMessageHint": ctx.Locale.Tr("repo.pulls.clear_merge_message_hint"), "textMergeCommitId": ctx.Locale.Tr("repo.pulls.merge_commit_id"), - "canMergeNow": prInfo.MergeBoxData.CanMergeNow, + "canMergeNow": prInfo.MergeBoxData.canMergeNow, "allOverridableChecksOk": allOverridableChecksOk, "emptyCommit": pull.IsEmpty(), "pullHeadCommitID": prInfo.CompareInfo.HeadCommitID, @@ -103,7 +103,7 @@ func (prInfo *pullRequestViewInfo) prepareMergeBoxFormProps(ctx *context.Context } // if this pr can be merged now, then hide the auto merge - generalHideAutoMerge := prInfo.MergeBoxData.CanMergeNow && allOverridableChecksOk + generalHideAutoMerge := prInfo.MergeBoxData.canMergeNow && allOverridableChecksOk var mergeStyles []any if pull.IsStatusMergeable() { diff --git a/routers/web/repo/setting/protected_branch.go b/routers/web/repo/setting/protected_branch.go index a5a25e6c4e..ac09b4fab9 100644 --- a/routers/web/repo/setting/protected_branch.go +++ b/routers/web/repo/setting/protected_branch.go @@ -83,6 +83,7 @@ func SettingsProtectedBranch(c *context.Context) { c.Data["whitelist_users"] = strings.Join(base.Int64sToStrings(rule.WhitelistUserIDs), ",") c.Data["force_push_allowlist_users"] = strings.Join(base.Int64sToStrings(rule.ForcePushAllowlistUserIDs), ",") c.Data["merge_whitelist_users"] = strings.Join(base.Int64sToStrings(rule.MergeWhitelistUserIDs), ",") + c.Data["bypass_allowlist_users"] = strings.Join(base.Int64sToStrings(rule.BypassAllowlistUserIDs), ",") c.Data["approvals_whitelist_users"] = strings.Join(base.Int64sToStrings(rule.ApprovalsWhitelistUserIDs), ",") c.Data["status_check_contexts"] = strings.Join(rule.StatusCheckContexts, "\n") contexts, _ := git_model.FindRepoRecentCommitStatusContexts(c, c.Repo.Repository.ID, 7*24*time.Hour) // Find last week status check contexts @@ -98,6 +99,7 @@ func SettingsProtectedBranch(c *context.Context) { c.Data["whitelist_teams"] = strings.Join(base.Int64sToStrings(rule.WhitelistTeamIDs), ",") c.Data["force_push_allowlist_teams"] = strings.Join(base.Int64sToStrings(rule.ForcePushAllowlistTeamIDs), ",") c.Data["merge_whitelist_teams"] = strings.Join(base.Int64sToStrings(rule.MergeWhitelistTeamIDs), ",") + c.Data["bypass_allowlist_teams"] = strings.Join(base.Int64sToStrings(rule.BypassAllowlistTeamIDs), ",") c.Data["approvals_whitelist_teams"] = strings.Join(base.Int64sToStrings(rule.ApprovalsWhitelistTeamIDs), ",") } @@ -155,7 +157,7 @@ func SettingsProtectedBranchPost(ctx *context.Context) { } } - var whitelistUsers, whitelistTeams, forcePushAllowlistUsers, forcePushAllowlistTeams, mergeWhitelistUsers, mergeWhitelistTeams, approvalsWhitelistUsers, approvalsWhitelistTeams []int64 + var whitelistUsers, whitelistTeams, forcePushAllowlistUsers, forcePushAllowlistTeams, mergeWhitelistUsers, mergeWhitelistTeams, approvalsWhitelistUsers, approvalsWhitelistTeams, bypassAllowlistUsers, bypassAllowlistTeams []int64 protectBranch.RuleName = f.RuleName if f.RequiredApprovals < 0 { ctx.Flash.Error(ctx.Tr("repo.settings.protected_branch_required_approvals_min")) @@ -215,6 +217,16 @@ func SettingsProtectedBranchPost(ctx *context.Context) { } } + protectBranch.EnableBypassAllowlist = f.EnableBypassAllowlist + if f.EnableBypassAllowlist { + if strings.TrimSpace(f.BypassAllowlistUsers) != "" { + bypassAllowlistUsers, _ = base.StringsToInt64s(strings.Split(f.BypassAllowlistUsers, ",")) + } + if strings.TrimSpace(f.BypassAllowlistTeams) != "" { + bypassAllowlistTeams, _ = base.StringsToInt64s(strings.Split(f.BypassAllowlistTeams, ",")) + } + } + protectBranch.EnableStatusCheck = f.EnableStatusCheck if f.EnableStatusCheck { patterns := strings.Split(strings.ReplaceAll(f.StatusCheckContexts, "\r", "\n"), "\n") @@ -271,6 +283,8 @@ func SettingsProtectedBranchPost(ctx *context.Context) { MergeTeamIDs: mergeWhitelistTeams, ApprovalsUserIDs: approvalsWhitelistUsers, ApprovalsTeamIDs: approvalsWhitelistTeams, + BypassUserIDs: bypassAllowlistUsers, + BypassTeamIDs: bypassAllowlistTeams, }); err != nil { ctx.ServerError("CreateOrUpdateProtectedBranch", err) return diff --git a/services/convert/convert.go b/services/convert/convert.go index dae0587ec4..d73f0aafff 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -148,6 +148,7 @@ func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo forcePushAllowlistUsernames := getWhitelistEntities(readers, bp.ForcePushAllowlistUserIDs) mergeWhitelistUsernames := getWhitelistEntities(readers, bp.MergeWhitelistUserIDs) approvalsWhitelistUsernames := getWhitelistEntities(readers, bp.ApprovalsWhitelistUserIDs) + bypassAllowlistUsernames := getWhitelistEntities(readers, bp.BypassAllowlistUserIDs) teamReaders, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, repo.Owner.ID, repo.ID, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests) if err != nil { @@ -158,6 +159,7 @@ func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo forcePushAllowlistTeams := getWhitelistEntities(teamReaders, bp.ForcePushAllowlistTeamIDs) mergeWhitelistTeams := getWhitelistEntities(teamReaders, bp.MergeWhitelistTeamIDs) approvalsWhitelistTeams := getWhitelistEntities(teamReaders, bp.ApprovalsWhitelistTeamIDs) + bypassAllowlistTeams := getWhitelistEntities(teamReaders, bp.BypassAllowlistTeamIDs) branchName := "" if !git_model.IsRuleNameSpecial(bp.RuleName) { @@ -181,6 +183,9 @@ func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo EnableMergeWhitelist: bp.EnableMergeWhitelist, MergeWhitelistUsernames: mergeWhitelistUsernames, MergeWhitelistTeams: mergeWhitelistTeams, + EnableBypassAllowlist: bp.EnableBypassAllowlist, + BypassAllowlistUsernames: bypassAllowlistUsernames, + BypassAllowlistTeams: bypassAllowlistTeams, EnableStatusCheck: bp.EnableStatusCheck, StatusCheckContexts: bp.StatusCheckContexts, RequiredApprovals: bp.RequiredApprovals, diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 9c60e44fc5..0460f8f518 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -181,6 +181,9 @@ type ProtectBranchForm struct { EnableMergeWhitelist bool MergeWhitelistUsers string MergeWhitelistTeams string + EnableBypassAllowlist bool + BypassAllowlistUsers string + BypassAllowlistTeams string EnableStatusCheck bool StatusCheckContexts string RequiredApprovals int64 diff --git a/services/pull/check.go b/services/pull/check.go index 241eb633b6..f0eb8ad47c 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -139,7 +139,7 @@ const ( // - merge: both the head commits must be verified and Gitea must sign the merge commit. // - rebase, rebase-merge, squash: Gitea rewrites the commits and signs each, so only Gitea's // signing ability is checked. -func CheckPullMergeable(stdCtx context.Context, doer *user_model.User, perm *access_model.Permission, pr *issues_model.PullRequest, mergeCheckType MergeCheckType, mergeStyle repo_model.MergeStyle, adminForceMerge bool) error { +func CheckPullMergeable(stdCtx context.Context, doer *user_model.User, perm *access_model.Permission, pr *issues_model.PullRequest, mergeCheckType MergeCheckType, mergeStyle repo_model.MergeStyle, forceMerge bool) error { return db.WithTx(stdCtx, func(ctx context.Context) error { if pr.HasMerged { return ErrHasMerged @@ -176,21 +176,21 @@ func CheckPullMergeable(stdCtx context.Context, doer *user_model.User, perm *acc return ErrIsChecking } - if err := CheckPullBranchProtections(ctx, pr, false); err != nil { - if !errors.Is(err, ErrNotReadyToMerge) { - log.Error("Error whilst checking pull branch protection for %-v: %v", pr, err) - return err + if errProtection := CheckPullBranchProtections(ctx, pr, false); errProtection != nil { + if !errors.Is(errProtection, ErrNotReadyToMerge) { + log.Error("Error whilst checking pull branch protection for %-v: %v", pr, errProtection) + return errProtection } // Now the branch protection check failed, check whether the failure could be skipped (skip by setting err = nil) // * when doing Auto Merge (Scheduled Merge After Checks Succeed), skip the branch protection check if mergeCheckType == MergeCheckTypeAuto { - err = nil + errProtection = nil } - // * if admin tries to "Force Merge", they could sometimes skip the branch protection check - if adminForceMerge { + // * if the doer tries to "Force Merge", check whether it is really allowed + if forceMerge { isRepoAdmin, errForceMerge := access_model.IsUserRepoAdmin(ctx, pr.BaseRepo, doer) if errForceMerge != nil { return fmt.Errorf("IsUserRepoAdmin failed, repo: %v, doer: %v, err: %w", pr.BaseRepoID, doer.ID, errForceMerge) @@ -201,16 +201,18 @@ func CheckPullMergeable(stdCtx context.Context, doer *user_model.User, perm *acc return fmt.Errorf("GetFirstMatchProtectedBranchRule failed, repo: %v, base branch: %v, err: %w", pr.BaseRepoID, pr.BaseBranch, errForceMerge) } - // if doer is admin and the "Force Merge" is not blocked, then clear the branch protection check error - blockAdminForceMerge := protectedBranchRule != nil && protectedBranchRule.BlockAdminMergeOverride - if isRepoAdmin && !blockAdminForceMerge { - err = nil + canForceMerge := isRepoAdmin + if protectedBranchRule != nil { + canForceMerge = git_model.CanBypassBranchProtection(ctx, protectedBranchRule, doer, isRepoAdmin) + } + if canForceMerge { + errProtection = nil } } // If there is still a branch protection check error, return it - if err != nil { - return err + if errProtection != nil { + return errProtection } } diff --git a/templates/repo/settings/protected_branch.tmpl b/templates/repo/settings/protected_branch.tmpl index fc26c3b517..ae262c9500 100644 --- a/templates/repo/settings/protected_branch.tmpl +++ b/templates/repo/settings/protected_branch.tmpl @@ -305,6 +305,49 @@ {{end}} +
{{ctx.Locale.Tr "repo.settings.protect_bypass_allowlist"}}
+
+
+
+ + +

{{ctx.Locale.Tr "repo.settings.protect_enable_bypass_allowlist_desc"}}

+
+
+
+
+ +
+ +
{{ctx.Locale.Tr "search.user_kind"}}
+
+ {{range .Users}} +
+ {{ctx.AvatarUtils.Avatar . 28 "mini"}}{{template "repo/search_name" .}} +
+ {{end}} +
+
+
+ {{if .Owner.IsOrganization}} +
+ +
+ +
{{ctx.Locale.Tr "search.team_kind"}}
+
+ {{range .Teams}} +
+ {{svg "octicon-people"}} + {{.Name}} +
+ {{end}} +
+
+
+ {{end}} +
+
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index bed3a32203..312eb25fd8 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -22666,6 +22666,20 @@ "type": "string", "x-go-name": "BranchName" }, + "bypass_allowlist_teams": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "BypassAllowlistTeams" + }, + "bypass_allowlist_usernames": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "BypassAllowlistUsernames" + }, "created_at": { "type": "string", "format": "date-time", @@ -22679,6 +22693,10 @@ "type": "boolean", "x-go-name": "EnableApprovalsWhitelist" }, + "enable_bypass_allowlist": { + "type": "boolean", + "x-go-name": "EnableBypassAllowlist" + }, "enable_force_push": { "type": "boolean", "x-go-name": "EnableForcePush" @@ -23522,6 +23540,20 @@ "type": "string", "x-go-name": "BranchName" }, + "bypass_allowlist_teams": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "BypassAllowlistTeams" + }, + "bypass_allowlist_usernames": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "BypassAllowlistUsernames" + }, "dismiss_stale_approvals": { "type": "boolean", "x-go-name": "DismissStaleApprovals" @@ -23530,6 +23562,10 @@ "type": "boolean", "x-go-name": "EnableApprovalsWhitelist" }, + "enable_bypass_allowlist": { + "type": "boolean", + "x-go-name": "EnableBypassAllowlist" + }, "enable_force_push": { "type": "boolean", "x-go-name": "EnableForcePush" @@ -24929,6 +24965,20 @@ "type": "boolean", "x-go-name": "BlockOnRejectedReviews" }, + "bypass_allowlist_teams": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "BypassAllowlistTeams" + }, + "bypass_allowlist_usernames": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "BypassAllowlistUsernames" + }, "dismiss_stale_approvals": { "type": "boolean", "x-go-name": "DismissStaleApprovals" @@ -24937,6 +24987,10 @@ "type": "boolean", "x-go-name": "EnableApprovalsWhitelist" }, + "enable_bypass_allowlist": { + "type": "boolean", + "x-go-name": "EnableBypassAllowlist" + }, "enable_force_push": { "type": "boolean", "x-go-name": "EnableForcePush" diff --git a/templates/swagger/v1_openapi3_json.tmpl b/templates/swagger/v1_openapi3_json.tmpl index f96418a899..04b8bd2d62 100644 --- a/templates/swagger/v1_openapi3_json.tmpl +++ b/templates/swagger/v1_openapi3_json.tmpl @@ -2850,6 +2850,20 @@ "type": "string", "x-go-name": "BranchName" }, + "bypass_allowlist_teams": { + "items": { + "type": "string" + }, + "type": "array", + "x-go-name": "BypassAllowlistTeams" + }, + "bypass_allowlist_usernames": { + "items": { + "type": "string" + }, + "type": "array", + "x-go-name": "BypassAllowlistUsernames" + }, "created_at": { "format": "date-time", "type": "string", @@ -2863,6 +2877,10 @@ "type": "boolean", "x-go-name": "EnableApprovalsWhitelist" }, + "enable_bypass_allowlist": { + "type": "boolean", + "x-go-name": "EnableBypassAllowlist" + }, "enable_force_push": { "type": "boolean", "x-go-name": "EnableForcePush" @@ -3724,6 +3742,20 @@ "type": "string", "x-go-name": "BranchName" }, + "bypass_allowlist_teams": { + "items": { + "type": "string" + }, + "type": "array", + "x-go-name": "BypassAllowlistTeams" + }, + "bypass_allowlist_usernames": { + "items": { + "type": "string" + }, + "type": "array", + "x-go-name": "BypassAllowlistUsernames" + }, "dismiss_stale_approvals": { "type": "boolean", "x-go-name": "DismissStaleApprovals" @@ -3732,6 +3764,10 @@ "type": "boolean", "x-go-name": "EnableApprovalsWhitelist" }, + "enable_bypass_allowlist": { + "type": "boolean", + "x-go-name": "EnableBypassAllowlist" + }, "enable_force_push": { "type": "boolean", "x-go-name": "EnableForcePush" @@ -5100,6 +5136,20 @@ "type": "boolean", "x-go-name": "BlockOnRejectedReviews" }, + "bypass_allowlist_teams": { + "items": { + "type": "string" + }, + "type": "array", + "x-go-name": "BypassAllowlistTeams" + }, + "bypass_allowlist_usernames": { + "items": { + "type": "string" + }, + "type": "array", + "x-go-name": "BypassAllowlistUsernames" + }, "dismiss_stale_approvals": { "type": "boolean", "x-go-name": "DismissStaleApprovals" @@ -5108,6 +5158,10 @@ "type": "boolean", "x-go-name": "EnableApprovalsWhitelist" }, + "enable_bypass_allowlist": { + "type": "boolean", + "x-go-name": "EnableBypassAllowlist" + }, "enable_force_push": { "type": "boolean", "x-go-name": "EnableForcePush" diff --git a/tests/integration/api_branch_test.go b/tests/integration/api_branch_test.go index d67ba98f7c..d6f3f6a9a1 100644 --- a/tests/integration/api_branch_test.go +++ b/tests/integration/api_branch_test.go @@ -356,7 +356,11 @@ func testAPIRenameBranch(t *testing.T, doerName, ownerName, repoName, from, to s func TestAPIBranchProtection(t *testing.T) { defer tests.PrepareTestEnv(t)() + t.Run("Basic", testAPIBranchProtectionBasic) + t.Run("BypassAllowlistValidation", testAPIBranchProtectionBypassAllowlistValidation) +} +func testAPIBranchProtectionBasic(t *testing.T) { // Can create branch protection on branch that not exist testAPICreateBranchProtection(t, "non-existing/branch", 1, http.StatusCreated) testAPIGetBranchProtection(t, "non-existing/branch", http.StatusOK) @@ -406,6 +410,35 @@ func TestAPIBranchProtection(t *testing.T) { testAPIDeleteBranch(t, "no-such-branch", http.StatusNotFound) // non-existing branch, not exist in git or DB } +func testAPIBranchProtectionBypassAllowlistValidation(t *testing.T) { + token := getUserToken(t, "user2", auth_model.AccessTokenScopeWriteRepository) + + t.Run("IgnoreInvalidBypassUsernamesWhenDisabled", func(t *testing.T) { + ruleName := "bypass-disabled-invalid-user" + req := NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo1/branch_protections", &api.CreateBranchProtectionOption{ + RuleName: ruleName, + EnableBypassAllowlist: false, + BypassAllowlistUsernames: []string{"nonexistent-user"}, + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusCreated) + testAPIDeleteBranchProtection(t, ruleName, http.StatusNoContent) + }) + + t.Run("IgnoreInvalidBypassTeamsWhenDisabled", func(t *testing.T) { + ruleName := "bypass-disabled-invalid-team" + req := NewRequestWithJSON(t, "POST", "/api/v1/repos/org3/repo3/branch_protections", &api.CreateBranchProtectionOption{ + RuleName: ruleName, + EnableBypassAllowlist: false, + BypassAllowlistTeams: []string{"nonexistent-team"}, + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusCreated) + + deleteReq := NewRequestf(t, "DELETE", "/api/v1/repos/org3/repo3/branch_protections/%s", ruleName). + AddTokenAuth(token) + MakeRequest(t, deleteReq, http.StatusNoContent) + }) +} + func TestAPICreateBranchWithSyncBranches(t *testing.T) { defer tests.PrepareTestEnv(t)() diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index 83cf1d7605..bf046e17e6 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -20,6 +20,7 @@ import ( "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" + "code.gitea.io/gitea/models/perm" pull_model "code.gitea.io/gitea/models/pull" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" @@ -29,6 +30,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git/gitcmd" "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/queue" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" @@ -1092,6 +1094,71 @@ func TestPullNonMergeForAdminWithBranchProtection(t *testing.T) { }) } +func TestPullForceMergeForBypassAllowlistUser(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + ownerSession := loginUser(t, "user2") + ownerCtx := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository) + + bypassUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user4"}) + doAPIAddCollaborator(ownerCtx, bypassUser.Name, perm.AccessModeWrite)(t) + + bypassSession := loginUser(t, bypassUser.Name) + forkedName := "repo1-bypass-allowlist" + testRepoFork(t, bypassSession, "user2", "repo1", bypassUser.Name, forkedName, "") + defer testDeleteRepository(t, bypassSession, bypassUser.Name, forkedName) + + testEditFile(t, bypassSession, bypassUser.Name, forkedName, "master", "README.md", "Hello, World (Bypass Allowlist)\n") + resp := testPullCreate(t, bypassSession, bypassUser.Name, forkedName, false, "master", "master", "Bypass allowlist merge test pull") + pullURL := test.RedirectURL(resp) + elem := strings.Split(pullURL, "/") + assert.Equal(t, "pulls", elem[3]) + + prIndex, err := strconv.ParseInt(elem[4], 10, 64) + assert.NoError(t, err) + + pbCreateReq := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/edit", map[string]string{ + "rule_name": "master", + "enable_push": "all", + "enable_status_check": "true", + "status_check_contexts": "gitea/actions", + "block_admin_merge_override": "true", + "enable_bypass_allowlist": "on", + "bypass_allowlist_users": strconv.FormatInt(bypassUser.ID, 10), + }) + ownerSession.MakeRequest(t, pbCreateReq, http.StatusSeeOther) + defer testAPIDeleteBranchProtection(t, "master", http.StatusNoContent) + + token := getTokenForLoggedInUser(t, bypassSession, auth_model.AccessTokenScopeWriteRepository) + + resp = bypassSession.MakeRequest(t, NewRequest(t, "GET", pullURL), http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + assert.Contains(t, htmlDoc.doc.Find(".merge-section").Text(), "You are allowed to bypass branch protection rules for this merge.") + mergeFormProps, exists := htmlDoc.doc.Find("#pull-request-merge-form").Attr("data-merge-form-props") + require.True(t, exists) + var mergeForm map[string]any + require.NoError(t, json.Unmarshal([]byte(mergeFormProps), &mergeForm)) + assert.Equal(t, true, mergeForm["canMergeNow"]) + assert.Equal(t, false, mergeForm["allOverridableChecksOk"]) + + mergeReq := func(forceMerge bool) *RequestWrapper { + return NewRequestWithValues(t, "POST", fmt.Sprintf("/api/v1/repos/user2/repo1/pulls/%d/merge", prIndex), map[string]string{ + "head_commit_id": "", + "merge_when_checks_succeed": "false", + "force_merge": strconv.FormatBool(forceMerge), + "do": "rebase", + }).AddTokenAuth(token) + } + + bypassSession.MakeRequest(t, mergeReq(false), http.StatusMethodNotAllowed) + bypassSession.MakeRequest(t, mergeReq(true), http.StatusOK) + + baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) + pr, err := issues_model.GetPullRequestByIndex(t.Context(), baseRepo.ID, prIndex) + assert.NoError(t, err) + assert.True(t, pr.HasMerged) + }) +} + func TestPullSquashMergeEmpty(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { session := loginUser(t, "user1") // FIXME: don't use admin user for testing diff --git a/web_src/css/repo.css b/web_src/css/repo.css index 3eb016650f..eabbad3be7 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -1050,7 +1050,7 @@ td .commit-summary { /* if the element is for a checkbox, then it should have a padding-left to align to the checkbox's text */ .repository.settings.branches .branch-protection .ui.checkbox .help, .repository.settings.branches .branch-protection .checkbox-sub-item { - padding-left: 26px; + padding-left: 20px; } .repository.settings.branches .branch-protection .status-check-matched-mark {