diff --git a/models/forgejo_migrations/migrate.go b/models/forgejo_migrations/migrate.go
index c2ffda5eb7..965b748ac9 100644
--- a/models/forgejo_migrations/migrate.go
+++ b/models/forgejo_migrations/migrate.go
@@ -54,6 +54,8 @@ var migrations = []*Migration{
NewMigration("Add the `enable_repo_unit_hints` column to the `user` table", forgejo_v1_22.AddUserRepoUnitHintsSetting),
// v7 -> v8
NewMigration("Modify the `release`.`note` content to remove SSH signatures", forgejo_v1_22.RemoveSSHSignaturesFromReleaseNotes),
+ // v8 -> v9
+ NewMigration("Add the `apply_to_admins` column to the `protected_branch` table", forgejo_v1_22.AddApplyToAdminsSetting),
}
// GetCurrentDBVersion returns the current Forgejo database version.
diff --git a/models/forgejo_migrations/v1_22/v9.go b/models/forgejo_migrations/v1_22/v9.go
new file mode 100644
index 0000000000..34c2844c39
--- /dev/null
+++ b/models/forgejo_migrations/v1_22/v9.go
@@ -0,0 +1,15 @@
+// Copyright 2024 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package v1_22 //nolint
+
+import "xorm.io/xorm"
+
+func AddApplyToAdminsSetting(x *xorm.Engine) error {
+ type ProtectedBranch struct {
+ ID int64 `xorm:"pk autoincr"`
+ ApplyToAdmins bool `xorm:"NOT NULL DEFAULT false"`
+ }
+
+ return x.Sync(&ProtectedBranch{})
+}
diff --git a/models/git/protected_branch.go b/models/git/protected_branch.go
index e0ff4d1542..a8b8c81bbe 100644
--- a/models/git/protected_branch.go
+++ b/models/git/protected_branch.go
@@ -58,6 +58,7 @@ type ProtectedBranch struct {
RequireSignedCommits bool `xorm:"NOT NULL DEFAULT false"`
ProtectedFilePatterns string `xorm:"TEXT"`
UnprotectedFilePatterns string `xorm:"TEXT"`
+ ApplyToAdmins 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 e96d276b29..0b3b0bb030 100644
--- a/modules/structs/repo_branch.go
+++ b/modules/structs/repo_branch.go
@@ -47,6 +47,7 @@ type BranchProtection struct {
RequireSignedCommits bool `json:"require_signed_commits"`
ProtectedFilePatterns string `json:"protected_file_patterns"`
UnprotectedFilePatterns string `json:"unprotected_file_patterns"`
+ ApplyToAdmins bool `json:"apply_to_admins"`
// swagger:strfmt date-time
Created time.Time `json:"created_at"`
// swagger:strfmt date-time
@@ -80,6 +81,7 @@ type CreateBranchProtectionOption struct {
RequireSignedCommits bool `json:"require_signed_commits"`
ProtectedFilePatterns string `json:"protected_file_patterns"`
UnprotectedFilePatterns string `json:"unprotected_file_patterns"`
+ ApplyToAdmins bool `json:"apply_to_admins"`
}
// EditBranchProtectionOption options for editing a branch protection
@@ -106,4 +108,5 @@ type EditBranchProtectionOption struct {
RequireSignedCommits *bool `json:"require_signed_commits"`
ProtectedFilePatterns *string `json:"protected_file_patterns"`
UnprotectedFilePatterns *string `json:"unprotected_file_patterns"`
+ ApplyToAdmins *bool `json:"apply_to_admins"`
}
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index b06e864869..97dc28b795 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -2343,6 +2343,7 @@ settings.event_pull_request_review_request = Pull request review requested
settings.event_pull_request_review_request_desc = Pull request review requested or review request removed.
settings.event_pull_request_approvals = Pull request approvals
settings.event_pull_request_merge = Pull request merge
+settings.event_pull_request_enforcement = Enforcement
settings.event_package = Package
settings.event_package_desc = Package created or deleted in a repository.
settings.branch_filter = Branch filter
@@ -2457,6 +2458,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.enforce_on_admins = Enforce this rule for repository admins
+settings.enforce_on_admins_desc = Repository admins cannot bypass this rule.
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/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go
index 5e6b6a8658..c33beee0ae 100644
--- a/routers/api/v1/repo/branch.go
+++ b/routers/api/v1/repo/branch.go
@@ -621,6 +621,7 @@ func CreateBranchProtection(ctx *context.APIContext) {
ProtectedFilePatterns: form.ProtectedFilePatterns,
UnprotectedFilePatterns: form.UnprotectedFilePatterns,
BlockOnOutdatedBranch: form.BlockOnOutdatedBranch,
+ ApplyToAdmins: form.ApplyToAdmins,
}
err = git_model.UpdateProtectBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{
@@ -808,6 +809,10 @@ func EditBranchProtection(ctx *context.APIContext) {
protectBranch.BlockOnOutdatedBranch = *form.BlockOnOutdatedBranch
}
+ if form.ApplyToAdmins != nil {
+ protectBranch.ApplyToAdmins = *form.ApplyToAdmins
+ }
+
var whitelistUsers []int64
if form.PushWhitelistUsernames != nil {
whitelistUsers, err = user_model.GetUserIDsByNames(ctx, form.PushWhitelistUsernames, false)
diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go
index f45e57b9e3..0613492845 100644
--- a/routers/private/hook_pre_receive.go
+++ b/routers/private/hook_pre_receive.go
@@ -337,13 +337,9 @@ 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() {
- return
- }
-
- // Now if we're not an admin - we can't overwrite protected files so fail now
- if changedProtectedfiles {
+ // It's not allowed t overwrite protected files. Unless if the user is an
+ // admin and the protected branch rule doesn't apply to admins.
+ if changedProtectedfiles && (!ctx.user.IsAdmin || protectBranch.ApplyToAdmins) {
log.Warn("Forbidden: Branch: %s in %-v is protected from changing file %s", branchName, repo, protectedFilePath)
ctx.JSON(http.StatusForbidden, private.Response{
UserMsg: fmt.Sprintf("branch %s is protected from changing file %s", branchName, protectedFilePath),
@@ -352,8 +348,12 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r
}
// Check all status checks and reviews are ok
- if err := pull_service.CheckPullBranchProtections(ctx, pr, true); err != nil {
+ if pb, err := pull_service.CheckPullBranchProtections(ctx, pr, true); err != nil {
if models.IsErrDisallowedToMerge(err) {
+ // Allow this if the rule doesn't apply to admins and the user is an admin.
+ if ctx.user.IsAdmin && !pb.ApplyToAdmins {
+ return
+ }
log.Warn("Forbidden: User %d is not allowed push to protected branch %s in %-v and pr #%d is not ready to be merged: %s", ctx.opts.UserID, branchName, repo, pr.Index, err.Error())
ctx.JSON(http.StatusForbidden, private.Response{
UserMsg: fmt.Sprintf("Not allowed to push to protected branch %s and pr #%d is not ready to be merged: %s", branchName, ctx.opts.PullRequestID, err.Error()),
diff --git a/routers/web/repo/setting/protected_branch.go b/routers/web/repo/setting/protected_branch.go
index 7ee67e5925..25146779de 100644
--- a/routers/web/repo/setting/protected_branch.go
+++ b/routers/web/repo/setting/protected_branch.go
@@ -237,6 +237,7 @@ func SettingsProtectedBranchPost(ctx *context.Context) {
protectBranch.ProtectedFilePatterns = f.ProtectedFilePatterns
protectBranch.UnprotectedFilePatterns = f.UnprotectedFilePatterns
protectBranch.BlockOnOutdatedBranch = f.BlockOnOutdatedBranch
+ protectBranch.ApplyToAdmins = f.ApplyToAdmins
err = git_model.UpdateProtectBranch(ctx, ctx.Repo.Repository, protectBranch, git_model.WhitelistOptions{
UserIDs: whitelistUsers,
diff --git a/services/convert/convert.go b/services/convert/convert.go
index ca3ec32a40..dd2239458e 100644
--- a/services/convert/convert.go
+++ b/services/convert/convert.go
@@ -162,6 +162,7 @@ func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch) *api
RequireSignedCommits: bp.RequireSignedCommits,
ProtectedFilePatterns: bp.ProtectedFilePatterns,
UnprotectedFilePatterns: bp.UnprotectedFilePatterns,
+ ApplyToAdmins: bp.ApplyToAdmins,
Created: bp.CreatedUnix.AsTime(),
Updated: bp.UpdatedUnix.AsTime(),
}
diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go
index 0f7665804d..b5ff031f4b 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
+ ApplyToAdmins bool
}
// Validate validates the fields
diff --git a/services/pull/check.go b/services/pull/check.go
index f4dd332b14..9aab3c94f3 100644
--- a/services/pull/check.go
+++ b/services/pull/check.go
@@ -104,7 +104,7 @@ func CheckPullMergable(stdCtx context.Context, doer *user_model.User, perm *acce
return ErrIsChecking
}
- if err := CheckPullBranchProtections(ctx, pr, false); err != nil {
+ if pb, err := CheckPullBranchProtections(ctx, pr, false); err != nil {
if !models.IsErrDisallowedToMerge(err) {
log.Error("Error whilst checking pull branch protection for %-v: %v", pr, err)
return err
@@ -117,8 +117,9 @@ func CheckPullMergable(stdCtx context.Context, doer *user_model.User, perm *acce
err = nil
}
- // * if the doer is admin, they could skip the branch protection check
- if adminSkipProtectionCheck {
+ // * if the doer is admin, they could skip the branch protection check,
+ // if that's allowed by the protected branch rule.
+ if adminSkipProtectionCheck && !pb.ApplyToAdmins {
if isRepoAdmin, errCheckAdmin := access_model.IsUserRepoAdmin(ctx, pr.BaseRepo, doer); errCheckAdmin != nil {
log.Error("Unable to check if %-v is a repo admin in %-v: %v", doer, pr.BaseRepo, errCheckAdmin)
return errCheckAdmin
diff --git a/services/pull/merge.go b/services/pull/merge.go
index df8d66e2d4..7f79eca2aa 100644
--- a/services/pull/merge.go
+++ b/services/pull/merge.go
@@ -424,63 +424,64 @@ func IsUserAllowedToMerge(ctx context.Context, pr *issues_model.PullRequest, p a
return false, nil
}
-// CheckPullBranchProtections checks whether the PR is ready to be merged (reviews and status checks)
-func CheckPullBranchProtections(ctx context.Context, pr *issues_model.PullRequest, skipProtectedFilesCheck bool) (err error) {
+// CheckPullBranchProtections checks whether the PR is ready to be merged (reviews and status checks).
+// Returns the protected branch rule when `ErrDisallowedToMerge` is returned as error.
+func CheckPullBranchProtections(ctx context.Context, pr *issues_model.PullRequest, skipProtectedFilesCheck bool) (protectedBranchRule *git_model.ProtectedBranch, err error) {
if err = pr.LoadBaseRepo(ctx); err != nil {
- return fmt.Errorf("LoadBaseRepo: %w", err)
+ return nil, fmt.Errorf("LoadBaseRepo: %w", err)
}
pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch)
if err != nil {
- return fmt.Errorf("LoadProtectedBranch: %v", err)
+ return nil, fmt.Errorf("LoadProtectedBranch: %v", err)
}
if pb == nil {
- return nil
+ return nil, nil
}
isPass, err := IsPullCommitStatusPass(ctx, pr)
if err != nil {
- return err
+ return nil, err
}
if !isPass {
- return models.ErrDisallowedToMerge{
+ return pb, models.ErrDisallowedToMerge{
Reason: "Not all required status checks successful",
}
}
if !issues_model.HasEnoughApprovals(ctx, pb, pr) {
- return models.ErrDisallowedToMerge{
+ return pb, models.ErrDisallowedToMerge{
Reason: "Does not have enough approvals",
}
}
if issues_model.MergeBlockedByRejectedReview(ctx, pb, pr) {
- return models.ErrDisallowedToMerge{
+ return pb, models.ErrDisallowedToMerge{
Reason: "There are requested changes",
}
}
if issues_model.MergeBlockedByOfficialReviewRequests(ctx, pb, pr) {
- return models.ErrDisallowedToMerge{
+ return pb, models.ErrDisallowedToMerge{
Reason: "There are official review requests",
}
}
if issues_model.MergeBlockedByOutdatedBranch(pb, pr) {
- return models.ErrDisallowedToMerge{
+ return pb, models.ErrDisallowedToMerge{
Reason: "The head branch is behind the base branch",
}
}
if skipProtectedFilesCheck {
- return nil
+ return nil, nil
}
if pb.MergeBlockedByProtectedFiles(pr.ChangedProtectedFiles) {
- return models.ErrDisallowedToMerge{
+ return pb, models.ErrDisallowedToMerge{
Reason: "Changed protected files",
}
}
- return nil
+ return nil, nil
}
// MergedManually mark pr as merged manually
diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl
index 393ea1b9aa..2d657c74ac 100644
--- a/templates/repo/issue/view_content/pull.tmpl
+++ b/templates/repo/issue/view_content/pull.tmpl
@@ -158,7 +158,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 $.IsRepoAdmin (not .ProtectedBranch.ApplyToAdmins)) (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 fec4d7c8d4..a991e68e6f 100644
--- a/templates/repo/settings/protected_branch.tmpl
+++ b/templates/repo/settings/protected_branch.tmpl
@@ -260,6 +260,14 @@
<p class="help">{{ctx.Locale.Tr "repo.settings.block_outdated_branch_desc"}}</p>
</div>
</div>
+ <h5 class="ui dividing header">{{ctx.Locale.Tr "repo.settings.event_pull_request_enforcement"}}</h5>
+ <div class="field">
+ <div class="ui checkbox">
+ <input name="apply_to_admins" type="checkbox" {{if .Rule.ApplyToAdmins}}checked{{end}}>
+ <label>{{ctx.Locale.Tr "repo.settings.enforce_on_admins"}}</label>
+ <p class="help">{{ctx.Locale.Tr "repo.settings.enforce_on_admins_desc"}}</p>
+ </div>
+ </div>
<div class="divider"></div>
<div class="field">
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl
index 22ada2f023..181c564f4a 100644
--- a/templates/swagger/v1_json.tmpl
+++ b/templates/swagger/v1_json.tmpl
@@ -17759,6 +17759,10 @@
"description": "BranchProtection represents a branch protection for a repository",
"type": "object",
"properties": {
+ "apply_to_admins": {
+ "type": "boolean",
+ "x-go-name": "ApplyToAdmins"
+ },
"approvals_whitelist_teams": {
"type": "array",
"items": {
@@ -18409,6 +18413,10 @@
"description": "CreateBranchProtectionOption options for creating a branch protection",
"type": "object",
"properties": {
+ "apply_to_admins": {
+ "type": "boolean",
+ "x-go-name": "ApplyToAdmins"
+ },
"approvals_whitelist_teams": {
"type": "array",
"items": {
@@ -19580,6 +19588,10 @@
"description": "EditBranchProtectionOption options for editing a branch protection",
"type": "object",
"properties": {
+ "apply_to_admins": {
+ "type": "boolean",
+ "x-go-name": "ApplyToAdmins"
+ },
"approvals_whitelist_teams": {
"type": "array",
"items": {
diff --git a/tests/integration/proctected_branch_test.go b/tests/integration/proctected_branch_test.go
new file mode 100644
index 0000000000..9c6e5e3cae
--- /dev/null
+++ b/tests/integration/proctected_branch_test.go
@@ -0,0 +1,87 @@
+// Copyright 2024 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package integration
+
+import (
+ "net/http"
+ "net/url"
+ "strconv"
+ "strings"
+ "testing"
+
+ git_model "code.gitea.io/gitea/models/git"
+ repo_model "code.gitea.io/gitea/models/repo"
+ "code.gitea.io/gitea/models/unittest"
+ "code.gitea.io/gitea/tests"
+
+ "github.com/stretchr/testify/assert"
+)
+
+func TestProtectedBranch_AdminEnforcement(t *testing.T) {
+ onGiteaRun(t, func(t *testing.T, u *url.URL) {
+ session := loginUser(t, "user1")
+ testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
+ testEditFileToNewBranch(t, session, "user1", "repo1", "master", "add-readme", "README.md", "WIP")
+ repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: 1, Name: "repo1"})
+
+ req := NewRequestWithValues(t, "POST", "user1/repo1/compare/master...add-readme", map[string]string{
+ "_csrf": GetCSRF(t, session, "user1/repo1/compare/master...add-readme"),
+ "title": "pull request",
+ })
+ session.MakeRequest(t, req, http.StatusOK)
+
+ t.Run("No protected branch", func(t *testing.T) {
+ defer tests.PrintCurrentTest(t)()
+
+ req = NewRequest(t, "GET", "/user1/repo1/pulls/1")
+ resp := session.MakeRequest(t, req, http.StatusOK)
+ doc := NewHTMLParser(t, resp.Body)
+
+ text := strings.TrimSpace(doc.doc.Find(".merge-section").Text())
+ assert.Contains(t, text, "This pull request can be merged automatically.")
+ assert.Contains(t, text, "'canMergeNow': true")
+ })
+
+ t.Run("Without admin enforcement", func(t *testing.T) {
+ defer tests.PrintCurrentTest(t)()
+
+ req := NewRequestWithValues(t, "POST", "/user1/repo1/settings/branches/edit", map[string]string{
+ "_csrf": GetCSRF(t, session, "/user1/repo1/settings/branches/edit"),
+ "rule_name": "master",
+ "required_approvals": "1",
+ })
+ session.MakeRequest(t, req, http.StatusSeeOther)
+
+ req = NewRequest(t, "GET", "/user1/repo1/pulls/1")
+ resp := session.MakeRequest(t, req, http.StatusOK)
+ doc := NewHTMLParser(t, resp.Body)
+
+ text := strings.TrimSpace(doc.doc.Find(".merge-section").Text())
+ assert.Contains(t, text, "This pull request doesn't have enough approvals yet. 0 of 1 approvals granted.")
+ assert.Contains(t, text, "'canMergeNow': true")
+ })
+
+ t.Run("With admin enforcement", func(t *testing.T) {
+ defer tests.PrintCurrentTest(t)()
+
+ protectedBranch := unittest.AssertExistsAndLoadBean(t, &git_model.ProtectedBranch{RuleName: "master", RepoID: repo.ID})
+ req := NewRequestWithValues(t, "POST", "/user1/repo1/settings/branches/edit", map[string]string{
+ "_csrf": GetCSRF(t, session, "/user1/repo1/settings/branches/edit"),
+ "rule_name": "master",
+ "rule_id": strconv.FormatInt(protectedBranch.ID, 10),
+ "required_approvals": "1",
+ "apply_to_admins": "true",
+ })
+ session.MakeRequest(t, req, http.StatusSeeOther)
+
+ req = NewRequest(t, "GET", "/user1/repo1/pulls/1")
+ resp := session.MakeRequest(t, req, http.StatusOK)
+ doc := NewHTMLParser(t, resp.Body)
+
+ text := strings.TrimSpace(doc.doc.Find(".merge-section").Text())
+ assert.Contains(t, text, "This pull request doesn't have enough approvals yet. 0 of 1 approvals granted.")
+ assert.Contains(t, text, "'canMergeNow': false")
+ })
+ })
+}