From c5bc8860068b86e7c7d61832cd616335b54068c2 Mon Sep 17 00:00:00 2001 From: Tim Schumacher Date: Sat, 12 Oct 2024 21:54:46 +0200 Subject: [PATCH 1/9] Make admins adhere to branch protection rules This introduces a new flag `BlockAdminMergeOverride` on the branch protection rules that prevents admins/repo owners from bypassing branch protection rules and merging without approvals or failing status checks. Fixes #17131 --- models/git/protected_branch.go | 1 + modules/structs/repo_branch.go | 3 ++ options/locale/locale_en-US.ini | 2 + routers/web/repo/issue.go | 1 + routers/web/repo/setting/protected_branch.go | 1 + services/forms/repo_form.go | 1 + services/pull/check.go | 21 ++++++-- templates/repo/issue/view_content/pull.tmpl | 2 +- templates/repo/settings/protected_branch.tmpl | 7 +++ templates/swagger/v1_json.tmpl | 12 +++++ tests/integration/pull_merge_test.go | 49 +++++++++++++++++++ 11 files changed, 95 insertions(+), 5 deletions(-) diff --git a/models/git/protected_branch.go b/models/git/protected_branch.go index bde6057375e55..0033a42d4e39f 100644 --- a/models/git/protected_branch.go +++ b/models/git/protected_branch.go @@ -63,6 +63,7 @@ type ProtectedBranch struct { RequireSignedCommits bool `xorm:"NOT NULL DEFAULT false"` ProtectedFilePatterns string `xorm:"TEXT"` UnprotectedFilePatterns string `xorm:"TEXT"` + BlockAdminMergeOverride bool `xorm:"NOT NULL DEFAULT false"` CreatedUnix timeutil.TimeStamp `xorm:"created"` UpdatedUnix timeutil.TimeStamp `xorm:"updated"` diff --git a/modules/structs/repo_branch.go b/modules/structs/repo_branch.go index 0f2cf482fd5a2..12a8344e87666 100644 --- a/modules/structs/repo_branch.go +++ b/modules/structs/repo_branch.go @@ -52,6 +52,7 @@ type BranchProtection struct { RequireSignedCommits bool `json:"require_signed_commits"` ProtectedFilePatterns string `json:"protected_file_patterns"` UnprotectedFilePatterns string `json:"unprotected_file_patterns"` + BlockAdminMergeOverride bool `json:"block_admin_merge_override"` // swagger:strfmt date-time Created time.Time `json:"created_at"` // swagger:strfmt date-time @@ -90,6 +91,7 @@ type CreateBranchProtectionOption struct { RequireSignedCommits bool `json:"require_signed_commits"` ProtectedFilePatterns string `json:"protected_file_patterns"` UnprotectedFilePatterns string `json:"unprotected_file_patterns"` + BlockAdminMergeOverride bool `json:"block_admin_merge_override"` } // EditBranchProtectionOption options for editing a branch protection @@ -121,4 +123,5 @@ type EditBranchProtectionOption struct { RequireSignedCommits *bool `json:"require_signed_commits"` ProtectedFilePatterns *string `json:"protected_file_patterns"` UnprotectedFilePatterns *string `json:"unprotected_file_patterns"` + BlockAdminMergeOverride *bool `json:"block_admin_merge_override"` } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index a02d939b79eda..aefef81a0c8f0 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2461,6 +2461,8 @@ settings.block_on_official_review_requests = Block merge on official review requ settings.block_on_official_review_requests_desc = Merging will not be possible when it has official review requests, even if there are enough approvals. settings.block_outdated_branch = Block merge if pull request is outdated settings.block_outdated_branch_desc = Merging will not be possible when head branch is behind base branch. +settings.block_admin_merge_override=Administrators must follow branch protection rules +settings.block_admin_merge_override_desc=Administrators must follow branch protection rules and can not circumvent it. settings.default_branch_desc = Select a default repository branch for pull requests and code commits: settings.merge_style_desc = Merge Styles settings.default_merge_style_desc = Default Merge Style diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 507b5af9d904a..879a20b887e85 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1943,6 +1943,7 @@ func ViewIssue(ctx *context.Context) { ctx.Data["IsBlockedByChangedProtectedFiles"] = len(pull.ChangedProtectedFiles) != 0 ctx.Data["ChangedProtectedFilesNum"] = len(pull.ChangedProtectedFiles) ctx.Data["RequireApprovalsWhitelist"] = pb.EnableApprovalsWhitelist + ctx.Data["BlockAdminMergeOverride"] = pb.BlockAdminMergeOverride } ctx.Data["WillSign"] = false if ctx.Doer != nil { diff --git a/routers/web/repo/setting/protected_branch.go b/routers/web/repo/setting/protected_branch.go index 7d943fd434dfe..940a138aff38e 100644 --- a/routers/web/repo/setting/protected_branch.go +++ b/routers/web/repo/setting/protected_branch.go @@ -256,6 +256,7 @@ func SettingsProtectedBranchPost(ctx *context.Context) { protectBranch.ProtectedFilePatterns = f.ProtectedFilePatterns protectBranch.UnprotectedFilePatterns = f.UnprotectedFilePatterns protectBranch.BlockOnOutdatedBranch = f.BlockOnOutdatedBranch + protectBranch.BlockAdminMergeOverride = f.BlockAdminMergeOverride err = git_model.UpdateProtectBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{ UserIDs: whitelistUsers, diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 988e479a48138..ddd07a64cbf75 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -219,6 +219,7 @@ type ProtectBranchForm struct { RequireSignedCommits bool ProtectedFilePatterns string UnprotectedFilePatterns string + BlockAdminMergeOverride bool } // Validate validates the fields diff --git a/services/pull/check.go b/services/pull/check.go index ce212f7d83bed..688126a7d363c 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -118,13 +118,26 @@ func CheckPullMergeable(stdCtx context.Context, doer *user_model.User, perm *acc err = nil } - // * if the doer is admin, they could skip the branch protection check + // * if the doer is admin, they could sometimes skip the branch protection check if adminSkipProtectionCheck { - if isRepoAdmin, errCheckAdmin := access_model.IsUserRepoAdmin(ctx, pr.BaseRepo, doer); errCheckAdmin != nil { + pb, pbErr := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch) + if pbErr != nil { + return fmt.Errorf("LoadProtectedBranch: %v", err) + } + + isRepoAdmin, errCheckAdmin := access_model.IsUserRepoAdmin(ctx, pr.BaseRepo, doer) + + /** + * Checks are only skipable if there is no branch protection available or BlockAdminMergeOverride + * of branch protection is set to false + */ + if errCheckAdmin != nil { log.Error("Unable to check if %-v is a repo admin in %-v: %v", doer, pr.BaseRepo, errCheckAdmin) return errCheckAdmin - } else if isRepoAdmin { - err = nil // repo admin can skip the check, so clear the error + } else if isRepoAdmin && pb != nil && !pb.BlockAdminMergeOverride { + err = nil + } else if isRepoAdmin && pb == nil { + err = nil } } diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index 1ce658ed00f4d..db71dfb5c27b8 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -164,7 +164,7 @@ {{$notAllOverridableChecksOk := or .IsBlockedByApprovals .IsBlockedByRejection .IsBlockedByOfficialReviewRequests .IsBlockedByOutdatedBranch .IsBlockedByChangedProtectedFiles (and .EnableStatusCheck (not .RequiredStatusCheckState.IsSuccess))}} {{/* admin can merge without checks, writer can merge when checks succeed */}} - {{$canMergeNow := and (or $.IsRepoAdmin (not $notAllOverridableChecksOk)) (or (not .AllowMerge) (not .RequireSigned) .WillSign)}} + {{$canMergeNow := and (or (and (not $.BlockAdminMergeOverride) $.IsRepoAdmin) (not $notAllOverridableChecksOk)) (or (not .AllowMerge) (not .RequireSigned) .WillSign)}} {{/* admin and writer both can make an auto merge schedule */}} {{if $canMergeNow}} diff --git a/templates/repo/settings/protected_branch.tmpl b/templates/repo/settings/protected_branch.tmpl index 6fab4a625a528..61cc6077a15dc 100644 --- a/templates/repo/settings/protected_branch.tmpl +++ b/templates/repo/settings/protected_branch.tmpl @@ -323,6 +323,13 @@

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

+
+
+ + +

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

+
+
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index a2b75bd873999..4cbf511aacbe8 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -18771,6 +18771,10 @@ }, "x-go-name": "ApprovalsWhitelistUsernames" }, + "block_admin_merge_override": { + "type": "boolean", + "x-go-name": "BlockAdminMergeOverride" + }, "block_on_official_review_requests": { "type": "boolean", "x-go-name": "BlockOnOfficialReviewRequests" @@ -19466,6 +19470,10 @@ }, "x-go-name": "ApprovalsWhitelistUsernames" }, + "block_admin_merge_override": { + "type": "boolean", + "x-go-name": "BlockAdminMergeOverride" + }, "block_on_official_review_requests": { "type": "boolean", "x-go-name": "BlockOnOfficialReviewRequests" @@ -20685,6 +20693,10 @@ }, "x-go-name": "ApprovalsWhitelistUsernames" }, + "block_admin_merge_override": { + "type": "boolean", + "x-go-name": "BlockAdminMergeOverride" + }, "block_on_official_review_requests": { "type": "boolean", "x-go-name": "BlockOnOfficialReviewRequests" diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index c1c8a8bf4e839..a4fc2e79d0e9d 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -976,3 +976,52 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApprovalForAgitFlow(t *testing. unittest.AssertNotExistsBean(t, &pull_model.AutoMerge{PullID: pr.ID}) }) } + +func TestPullNonMergeForAdminWithBranchProtection(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + // create a pull request + session := loginUser(t, "user1") + // user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + forkedName := "repo1-1" + testRepoFork(t, session, "user2", "repo1", "user1", forkedName, "") + defer func() { + testDeleteRepository(t, session, "user1", forkedName) + }() + testEditFile(t, session, "user1", forkedName, "master", "README.md", "Hello, World (Edited)\n") + testPullCreate(t, session, "user1", forkedName, false, "master", "master", "Indexer notifier test pull") + + baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) + forkedRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user1", Name: forkedName}) + unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ + BaseRepoID: baseRepo.ID, + BaseBranch: "master", + HeadRepoID: forkedRepo.ID, + HeadBranch: "master", + }) + + // add protected branch for commit status + csrf := GetUserCSRFToken(t, session) + // Change master branch to protected + pbCreateReq := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/edit", map[string]string{ + "_csrf": csrf, + "rule_name": "master", + "enable_push": "true", + "enable_status_check": "true", + "status_check_contexts": "gitea/actions", + "block_admin_merge_override": "true", + }) + session.MakeRequest(t, pbCreateReq, http.StatusSeeOther) + + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) + + mergeReq := NewRequestWithValues(t, "POST", "/api/v1/repos/user2/repo1/pulls/6/merge", map[string]string{ + "_csrf": csrf, + "head_commit_id": "", + "merge_when_checks_succeed": "false", + "force_merge": "true", + "do": "rebase", + }).AddTokenAuth(token) + + session.MakeRequest(t, mergeReq, http.StatusMethodNotAllowed) + }) +} From ffddf437ef8d45cbcf92e9c0d7bd402e60e7096a Mon Sep 17 00:00:00 2001 From: Tim Schumacher Date: Fri, 18 Oct 2024 21:36:22 +0200 Subject: [PATCH 2/9] Add migration --- models/migrations/v1_23/v306.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 models/migrations/v1_23/v306.go diff --git a/models/migrations/v1_23/v306.go b/models/migrations/v1_23/v306.go new file mode 100644 index 0000000000000..276b438e95bcf --- /dev/null +++ b/models/migrations/v1_23/v306.go @@ -0,0 +1,13 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_23 //nolint + +import "xorm.io/xorm" + +func AddBlockAdminMergeOverrideBranchProtection(x *xorm.Engine) error { + type ProtectedBranch struct { + BlockAdminMergeOverride bool `xorm:"NOT NULL DEFAULT false"` + } + return x.Sync(new(ProtectedBranch)) +} From 70eb2f0b6c40284a3a79d039016fa160020f1332 Mon Sep 17 00:00:00 2001 From: Tim Schumacher Date: Sat, 19 Oct 2024 22:10:59 +0200 Subject: [PATCH 3/9] Make the API work --- routers/api/v1/repo/branch.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 63de4b8b6a2be..bb16858c81160 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -642,6 +642,7 @@ func CreateBranchProtection(ctx *context.APIContext) { ProtectedFilePatterns: form.ProtectedFilePatterns, UnprotectedFilePatterns: form.UnprotectedFilePatterns, BlockOnOutdatedBranch: form.BlockOnOutdatedBranch, + BlockAdminMergeOverride: form.BlockAdminMergeOverride, } err = git_model.UpdateProtectBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{ @@ -852,6 +853,10 @@ func EditBranchProtection(ctx *context.APIContext) { protectBranch.BlockOnOutdatedBranch = *form.BlockOnOutdatedBranch } + if form.BlockAdminMergeOverride != nil { + protectBranch.BlockAdminMergeOverride = *form.BlockAdminMergeOverride + } + var whitelistUsers, forcePushAllowlistUsers, mergeWhitelistUsers, approvalsWhitelistUsers []int64 if form.PushWhitelistUsernames != nil { whitelistUsers, err = user_model.GetUserIDsByNames(ctx, form.PushWhitelistUsernames, false) From 71b5edd500b282d312ca068679d7f41b934d86a4 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 20 Oct 2024 12:49:05 +0800 Subject: [PATCH 4/9] fix --- models/migrations/migrations.go | 2 ++ options/locale/locale_en-US.ini | 4 ++-- services/pull/check.go | 30 +++++++++++++----------------- 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index f99718ead2859..e2872ac6c7771 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -603,6 +603,8 @@ var migrations = []Migration{ NewMigration("Add index for release sha1", v1_23.AddIndexForReleaseSha1), // v305 -> v306 NewMigration("Add Repository Licenses", v1_23.AddRepositoryLicenses), + // v306 -> v307 + NewMigration("Add Repository Licenses", v1_23.AddBlockAdminMergeOverrideBranchProtection), } // GetCurrentDBVersion returns the current db version diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index aefef81a0c8f0..06bf57fc62cf2 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2461,8 +2461,8 @@ settings.block_on_official_review_requests = Block merge on official review requ settings.block_on_official_review_requests_desc = Merging will not be possible when it has official review requests, even if there are enough approvals. settings.block_outdated_branch = Block merge if pull request is outdated settings.block_outdated_branch_desc = Merging will not be possible when head branch is behind base branch. -settings.block_admin_merge_override=Administrators must follow branch protection rules -settings.block_admin_merge_override_desc=Administrators must follow branch protection rules and can not circumvent it. +settings.block_admin_merge_override = Administrators must follow branch protection rules +settings.block_admin_merge_override_desc = Administrators must follow branch protection rules and can not circumvent it. settings.default_branch_desc = Select a default repository branch for pull requests and code commits: settings.merge_style_desc = Merge Styles settings.default_merge_style_desc = Default Merge Style diff --git a/services/pull/check.go b/services/pull/check.go index 688126a7d363c..b40f5e071624a 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -68,7 +68,7 @@ const ( ) // CheckPullMergeable check if the pull mergeable based on all conditions (branch protection, merge options, ...) -func CheckPullMergeable(stdCtx context.Context, doer *user_model.User, perm *access_model.Permission, pr *issues_model.PullRequest, mergeCheckType MergeCheckType, adminSkipProtectionCheck bool) error { +func CheckPullMergeable(stdCtx context.Context, doer *user_model.User, perm *access_model.Permission, pr *issues_model.PullRequest, mergeCheckType MergeCheckType, adminForceMerge bool) error { return db.WithTx(stdCtx, func(ctx context.Context) error { if pr.HasMerged { return ErrHasMerged @@ -118,25 +118,21 @@ func CheckPullMergeable(stdCtx context.Context, doer *user_model.User, perm *acc err = nil } - // * if the doer is admin, they could sometimes skip the branch protection check - if adminSkipProtectionCheck { - pb, pbErr := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch) - if pbErr != nil { - return fmt.Errorf("LoadProtectedBranch: %v", err) + // * if admin tries to "Force Merge", they could sometimes skip the branch protection check + if adminForceMerge { + isRepoAdmin, errForceMerge := access_model.IsUserRepoAdmin(ctx, pr.BaseRepo, doer) + if errForceMerge != nil { + return fmt.Errorf("IsUserRepoAdmin failed, repo: %-v, doer: %-v, err: %w", pr.BaseRepo, doer, errForceMerge) } - isRepoAdmin, errCheckAdmin := access_model.IsUserRepoAdmin(ctx, pr.BaseRepo, doer) + protectedBranchRule, errForceMerge := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch) + if errForceMerge != nil { + return fmt.Errorf("GetFirstMatchProtectedBranchRule failed: repo: %-v, base branch: %v, err: %w", pr.BaseRepo, pr.BaseBranch, errForceMerge) + } - /** - * Checks are only skipable if there is no branch protection available or BlockAdminMergeOverride - * of branch protection is set to false - */ - if errCheckAdmin != nil { - log.Error("Unable to check if %-v is a repo admin in %-v: %v", doer, pr.BaseRepo, errCheckAdmin) - return errCheckAdmin - } else if isRepoAdmin && pb != nil && !pb.BlockAdminMergeOverride { - err = nil - } else if isRepoAdmin && pb == nil { + // 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 } } From ea2f87dc76517c33cf8a9580b0ffe192530ef02d Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 20 Oct 2024 12:55:32 +0800 Subject: [PATCH 5/9] fix err msg --- services/pull/check.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/pull/check.go b/services/pull/check.go index b40f5e071624a..736be4611bedd 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -122,12 +122,12 @@ func CheckPullMergeable(stdCtx context.Context, doer *user_model.User, perm *acc if adminForceMerge { isRepoAdmin, errForceMerge := access_model.IsUserRepoAdmin(ctx, pr.BaseRepo, doer) if errForceMerge != nil { - return fmt.Errorf("IsUserRepoAdmin failed, repo: %-v, doer: %-v, err: %w", pr.BaseRepo, doer, errForceMerge) + return fmt.Errorf("IsUserRepoAdmin failed, repo: %v, doer: %v, err: %w", pr.BaseRepoID, doer.ID, errForceMerge) } protectedBranchRule, errForceMerge := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch) if errForceMerge != nil { - return fmt.Errorf("GetFirstMatchProtectedBranchRule failed: repo: %-v, base branch: %v, err: %w", pr.BaseRepo, pr.BaseBranch, errForceMerge) + 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 From 78cc842f45a982beb2f97bc173bb83945f4e3547 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 20 Oct 2024 13:08:40 +0800 Subject: [PATCH 6/9] remove unnecessary template var --- routers/web/repo/issue.go | 1 - templates/repo/issue/view_content/pull.tmpl | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 879a20b887e85..507b5af9d904a 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1943,7 +1943,6 @@ func ViewIssue(ctx *context.Context) { ctx.Data["IsBlockedByChangedProtectedFiles"] = len(pull.ChangedProtectedFiles) != 0 ctx.Data["ChangedProtectedFilesNum"] = len(pull.ChangedProtectedFiles) ctx.Data["RequireApprovalsWhitelist"] = pb.EnableApprovalsWhitelist - ctx.Data["BlockAdminMergeOverride"] = pb.BlockAdminMergeOverride } ctx.Data["WillSign"] = false if ctx.Doer != nil { diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index db71dfb5c27b8..5353357e818ae 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -164,7 +164,7 @@ {{$notAllOverridableChecksOk := or .IsBlockedByApprovals .IsBlockedByRejection .IsBlockedByOfficialReviewRequests .IsBlockedByOutdatedBranch .IsBlockedByChangedProtectedFiles (and .EnableStatusCheck (not .RequiredStatusCheckState.IsSuccess))}} {{/* admin can merge without checks, writer can merge when checks succeed */}} - {{$canMergeNow := and (or (and (not $.BlockAdminMergeOverride) $.IsRepoAdmin) (not $notAllOverridableChecksOk)) (or (not .AllowMerge) (not .RequireSigned) .WillSign)}} + {{$canMergeNow := and (or (and (not $.ProtectedBranch.BlockAdminMergeOverride) $.IsRepoAdmin) (not $notAllOverridableChecksOk)) (or (not .AllowMerge) (not .RequireSigned) .WillSign)}} {{/* admin and writer both can make an auto merge schedule */}} {{if $canMergeNow}} From 71afbc00ccde43d9b08ba7d152863690cc957800 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 20 Oct 2024 13:12:51 +0800 Subject: [PATCH 7/9] clean up test code --- tests/integration/pull_merge_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index a4fc2e79d0e9d..43210e852e53b 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -981,12 +981,10 @@ func TestPullNonMergeForAdminWithBranchProtection(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { // create a pull request session := loginUser(t, "user1") - // user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) forkedName := "repo1-1" testRepoFork(t, session, "user2", "repo1", "user1", forkedName, "") - defer func() { - testDeleteRepository(t, session, "user1", forkedName) - }() + defer testDeleteRepository(t, session, "user1", forkedName) + testEditFile(t, session, "user1", forkedName, "master", "README.md", "Hello, World (Edited)\n") testPullCreate(t, session, "user1", forkedName, false, "master", "master", "Indexer notifier test pull") From 623d5da5713d8545624e4b3499a5c1f97afc8269 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 20 Oct 2024 18:25:18 +0800 Subject: [PATCH 8/9] Update models/migrations/migrations.go --- models/migrations/migrations.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index e2872ac6c7771..f0651ddbfafd3 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -604,7 +604,7 @@ var migrations = []Migration{ // v305 -> v306 NewMigration("Add Repository Licenses", v1_23.AddRepositoryLicenses), // v306 -> v307 - NewMigration("Add Repository Licenses", v1_23.AddBlockAdminMergeOverrideBranchProtection), + NewMigration("Add BlockAdminMergeOverride to ProtectedBranch", v1_23.AddBlockAdminMergeOverrideBranchProtection), } // GetCurrentDBVersion returns the current db version From 3303a10826d84042407aa30e63fcff2cb8bbd96e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 23 Oct 2024 10:51:50 +0800 Subject: [PATCH 9/9] fix missing API BlockAdminMergeOverride --- services/convert/convert.go | 1 + 1 file changed, 1 insertion(+) diff --git a/services/convert/convert.go b/services/convert/convert.go index 041d553e98cc5..8dc311dae9a60 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -185,6 +185,7 @@ func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo RequireSignedCommits: bp.RequireSignedCommits, ProtectedFilePatterns: bp.ProtectedFilePatterns, UnprotectedFilePatterns: bp.UnprotectedFilePatterns, + BlockAdminMergeOverride: bp.BlockAdminMergeOverride, Created: bp.CreatedUnix.AsTime(), Updated: bp.UpdatedUnix.AsTime(), }