finn/forgejo
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Supports wildcard protected branch (#20825)

Browse source 
This PR introduce glob match for protected branch name. The separator is
`/` and you can use `*` matching non-separator chars and use `**` across
separator.

It also supports input an exist or non-exist branch name as matching
condition and branch name condition has high priority than glob rule.

Should fix #2529 and #15705

screenshots

<img width="1160" alt="image"
src="https://user-images.githubusercontent.com/81045/205651179-ebb5492a-4ade-4bb4-a13c-965e8c927063.png">

Co-authored-by: zeripath <art27@cantab.net>
This commit is contained in:
Lunny Xiao 2023-01-16 16:00:22 +08:00 • committed by GitHub
parent cc1f8cbe96
commit 2782c14396
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
39 changed files with 1222 additions and 819 deletions
Download patch file Download diff file Expand all files Collapse all files

2
services/asymkey/sign.go
View file

@ -310,7 +310,7 @@ Loop:
return false, "", nil, &ErrWontSign{twofa}
}
case approved:
protectedBranch, err := git_model.GetProtectedBranchBy(ctx, repo.ID, pr.BaseBranch)
protectedBranch, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repo.ID, pr.BaseBranch)
if err != nil {
return false, "", nil, err
}

13
services/convert/convert.go
View file

@ -79,7 +79,7 @@ func ToBranch(repo *repo_model.Repository, b *git.Branch, c *git.Commit, bp *git
}
if isRepoAdmin {
branch.EffectiveBranchProtectionName = bp.BranchName
branch.EffectiveBranchProtectionName = bp.RuleName
}
if user != nil {
@ -87,7 +87,8 @@ func ToBranch(repo *repo_model.Repository, b *git.Branch, c *git.Commit, bp *git
if err != nil {
return nil, err
}
branch.UserCanPush = bp.CanUserPush(db.DefaultContext, user.ID)
bp.Repo = repo
branch.UserCanPush = bp.CanUserPush(db.DefaultContext, user)
branch.UserCanMerge = git_model.IsUserMergeWhitelisted(db.DefaultContext, bp, user.ID, permission)
}
@ -121,8 +122,14 @@ func ToBranchProtection(bp *git_model.ProtectedBranch) *api.BranchProtection {
log.Error("GetTeamNamesByID (ApprovalsWhitelistTeamIDs): %v", err)
}
branchName := ""
if !git_model.IsRuleNameSpecial(bp.RuleName) {
branchName = bp.RuleName
}
return &api.BranchProtection{
BranchName: bp.BranchName,
BranchName: branchName,
RuleName: bp.RuleName,
EnablePush: bp.CanPush,
EnablePushWhitelist: bp.EnableWhitelist,
PushWhitelistUsernames: pushWhitelistUsernames,

2
services/forms/repo_form.go
View file

@ -186,7 +186,7 @@ func (f *RepoSettingForm) Validate(req *http.Request, errs binding.Errors) bindi
// ProtectBranchForm form for changing protected branch settings
type ProtectBranchForm struct {
Protected bool
RuleName string `binding:"Required"`
EnablePush string
WhitelistUsers string
WhitelistTeams string

10
services/pull/check.go
View file

@ -14,6 +14,7 @@ import (
"code.gitea.io/gitea/models"
"code.gitea.io/gitea/models/db"
git_model "code.gitea.io/gitea/models/git"
issues_model "code.gitea.io/gitea/models/issues"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
@ -126,11 +127,12 @@ func CheckPullMergable(stdCtx context.Context, doer *user_model.User, perm *acce
// isSignedIfRequired check if merge will be signed if required
func isSignedIfRequired(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User) (bool, error) {
if err := pr.LoadProtectedBranch(ctx); err != nil {
pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch)
if err != nil {
return false, err
}
if pr.ProtectedBranch == nil || !pr.ProtectedBranch.RequireSignedCommits {
if pb == nil || !pb.RequireSignedCommits {
return true, nil
}
@ -348,8 +350,8 @@ func testPR(id int64) {
checkAndUpdateStatus(ctx, pr)
}
// CheckPrsForBaseBranch check all pulls with bseBrannch
func CheckPrsForBaseBranch(baseRepo *repo_model.Repository, baseBranchName string) error {
// CheckPRsForBaseBranch check all pulls with baseBrannch
func CheckPRsForBaseBranch(baseRepo *repo_model.Repository, baseBranchName string) error {
prs, err := issues_model.GetUnmergedPullRequestsByBaseInfo(baseRepo.ID, baseBranchName)
if err != nil {
return err

12
services/pull/commit_status.go
View file

@ -83,10 +83,11 @@ func IsCommitStatusContextSuccess(commitStatuses []*git_model.CommitStatus, requ
// IsPullCommitStatusPass returns if all required status checks PASS
func IsPullCommitStatusPass(ctx context.Context, pr *issues_model.PullRequest) (bool, error) {
if err := pr.LoadProtectedBranch(ctx); err != nil {
pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch)
if err != nil {
return false, errors.Wrap(err, "GetLatestCommitStatus")
}
if pr.ProtectedBranch == nil || !pr.ProtectedBranch.EnableStatusCheck {
if pb == nil || !pb.EnableStatusCheck {
return true, nil
}
@ -137,12 +138,13 @@ func GetPullRequestCommitStatusState(ctx context.Context, pr *issues_model.PullR
return "", errors.Wrap(err, "GetLatestCommitStatus")
}
if err := pr.LoadProtectedBranch(ctx); err != nil {
pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch)
if err != nil {
return "", errors.Wrap(err, "LoadProtectedBranch")
}
var requiredContexts []string
if pr.ProtectedBranch != nil {
requiredContexts = pr.ProtectedBranch.StatusCheckContexts
if pb != nil {
requiredContexts = pb.StatusCheckContexts
}
return MergeRequiredContextsCommitStatus(commitStatuses, requiredContexts), nil

24
services/pull/merge.go
View file

@ -760,12 +760,12 @@ func IsUserAllowedToMerge(ctx context.Context, pr *issues_model.PullRequest, p a
return false, nil
}
err := pr.LoadProtectedBranch(ctx)
pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch)
if err != nil {
return false, err
}
if (p.CanWrite(unit.TypeCode) && pr.ProtectedBranch == nil) || (pr.ProtectedBranch != nil && git_model.IsUserMergeWhitelisted(ctx, pr.ProtectedBranch, user.ID, p)) {
if (p.CanWrite(unit.TypeCode) && pb == nil) || (pb != nil && git_model.IsUserMergeWhitelisted(ctx, pb, user.ID, p)) {
return true, nil
}
@ -778,10 +778,11 @@ func CheckPullBranchProtections(ctx context.Context, pr *issues_model.PullReques
return fmt.Errorf("LoadBaseRepo: %w", err)
}
if err = pr.LoadProtectedBranch(ctx); err != nil {
return fmt.Errorf("LoadProtectedBranch: %w", err)
pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch)
if err != nil {
return fmt.Errorf("LoadProtectedBranch: %v", err)
}
if pr.ProtectedBranch == nil {
if pb == nil {
return nil
}
@ -795,23 +796,23 @@ func CheckPullBranchProtections(ctx context.Context, pr *issues_model.PullReques
}
}
if !issues_model.HasEnoughApprovals(ctx, pr.ProtectedBranch, pr) {
if !issues_model.HasEnoughApprovals(ctx, pb, pr) {
return models.ErrDisallowedToMerge{
Reason: "Does not have enough approvals",
}
}
if issues_model.MergeBlockedByRejectedReview(ctx, pr.ProtectedBranch, pr) {
if issues_model.MergeBlockedByRejectedReview(ctx, pb, pr) {
return models.ErrDisallowedToMerge{
Reason: "There are requested changes",
}
}
if issues_model.MergeBlockedByOfficialReviewRequests(ctx, pr.ProtectedBranch, pr) {
if issues_model.MergeBlockedByOfficialReviewRequests(ctx, pb, pr) {
return models.ErrDisallowedToMerge{
Reason: "There are official review requests",
}
}
if issues_model.MergeBlockedByOutdatedBranch(pr.ProtectedBranch, pr) {
if issues_model.MergeBlockedByOutdatedBranch(pb, pr) {
return models.ErrDisallowedToMerge{
Reason: "The head branch is behind the base branch",
}
@ -821,7 +822,7 @@ func CheckPullBranchProtections(ctx context.Context, pr *issues_model.PullReques
return nil
}
if pr.ProtectedBranch.MergeBlockedByProtectedFiles(pr.ChangedProtectedFiles) {
if pb.MergeBlockedByProtectedFiles(pr.ChangedProtectedFiles) {
return models.ErrDisallowedToMerge{
Reason: "Changed protected files",
}
@ -836,6 +837,9 @@ func MergedManually(pr *issues_model.PullRequest, doer *user_model.User, baseGit
defer pullWorkingPool.CheckOut(fmt.Sprint(pr.ID))
if err := db.WithTx(db.DefaultContext, func(ctx context.Context) error {
if err := pr.LoadBaseRepo(ctx); err != nil {
return err
}
prUnit, err := pr.BaseRepo.GetUnit(ctx, unit.TypePullRequests)
if err != nil {
return err

16
services/pull/patch.go
View file

@ -14,7 +14,7 @@ import (
"strings"
"code.gitea.io/gitea/models"
"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/unit"
"code.gitea.io/gitea/modules/container"
@ -106,8 +106,8 @@ func TestPatch(pr *issues_model.PullRequest) error {
}
// 3. Check for protected files changes
if err = checkPullFilesProtection(pr, gitRepo); err != nil {
return fmt.Errorf("pr.CheckPullFilesProtection(): %w", err)
if err = checkPullFilesProtection(ctx, pr, gitRepo); err != nil {
return fmt.Errorf("pr.CheckPullFilesProtection(): %v", err)
}
if len(pr.ChangedProtectedFiles) > 0 {
@ -544,23 +544,23 @@ func CheckUnprotectedFiles(repo *git.Repository, oldCommitID, newCommitID string
}
// checkPullFilesProtection check if pr changed protected files and save results
func checkPullFilesProtection(pr *issues_model.PullRequest, gitRepo *git.Repository) error {
func checkPullFilesProtection(ctx context.Context, pr *issues_model.PullRequest, gitRepo *git.Repository) error {
if pr.Status == issues_model.PullRequestStatusEmpty {
pr.ChangedProtectedFiles = nil
return nil
}
if err := pr.LoadProtectedBranch(db.DefaultContext); err != nil {
pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch)
if err != nil {
return err
}
if pr.ProtectedBranch == nil {
if pb == nil {
pr.ChangedProtectedFiles = nil
return nil
}
var err error
pr.ChangedProtectedFiles, err = CheckFileProtection(gitRepo, pr.MergeBase, "tracking", pr.ProtectedBranch.GetProtectedFilePatterns(), 10, os.Environ())
pr.ChangedProtectedFiles, err = CheckFileProtection(gitRepo, pr.MergeBase, "tracking", pb.GetProtectedFilePatterns(), 10, os.Environ())
if err != nil && !models.IsErrFilePathProtected(err) {
return err
}

21
services/pull/update.go
View file

@ -8,6 +8,7 @@ import (
"fmt"
"code.gitea.io/gitea/models"
git_model "code.gitea.io/gitea/models/git"
issues_model "code.gitea.io/gitea/models/issues"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
@ -92,20 +93,29 @@ func IsUserAllowedToUpdate(ctx context.Context, pull *issues_model.PullRequest,
return false, false, err
}
if err := pull.LoadBaseRepo(ctx); err != nil {
return false, false, err
}
pr := &issues_model.PullRequest{
HeadRepoID: pull.BaseRepoID,
HeadRepo: pull.BaseRepo,
BaseRepoID: pull.HeadRepoID,
BaseRepo: pull.HeadRepo,
HeadBranch: pull.BaseBranch,
BaseBranch: pull.HeadBranch,
}
err = pr.LoadProtectedBranch(ctx)
pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pull.BaseRepoID, pull.BaseBranch)
if err != nil {
return false, false, err
}
// can't do rebase on protected branch because need force push
if pr.ProtectedBranch == nil {
if pb == nil {
if err := pr.LoadBaseRepo(ctx); err != nil {
return false, false, err
}
prUnit, err := pr.BaseRepo.GetUnit(ctx, unit.TypePullRequests)
if err != nil {
log.Error("pr.BaseRepo.GetUnit(unit.TypePullRequests): %v", err)
@ -115,8 +125,11 @@ func IsUserAllowedToUpdate(ctx context.Context, pull *issues_model.PullRequest,
}
// Update function need push permission
if pr.ProtectedBranch != nil && !pr.ProtectedBranch.CanUserPush(ctx, user.ID) {
return false, false, nil
if pb != nil {
pb.Repo = pull.BaseRepo
if !pb.CanUserPush(ctx, user) {
return false, false, nil
}
}
baseRepoPerm, err := access_model.GetUserRepoPermission(ctx, pull.BaseRepo, user)

8
services/repository/branch.go
View file

@ -149,8 +149,7 @@ func RenameBranch(repo *repo_model.Repository, doer *user_model.User, gitRepo *g
// enmuerates all branch related errors
var (
ErrBranchIsDefault = errors.New("branch is default")
ErrBranchIsProtected = errors.New("branch is protected")
ErrBranchIsDefault = errors.New("branch is default")
)
// DeleteBranch delete branch
@ -159,13 +158,12 @@ func DeleteBranch(doer *user_model.User, repo *repo_model.Repository, gitRepo *g
return ErrBranchIsDefault
}
isProtected, err := git_model.IsProtectedBranch(db.DefaultContext, repo.ID, branchName)
isProtected, err := git_model.IsBranchProtected(db.DefaultContext, repo.ID, branchName)
if err != nil {
return err
}
if isProtected {
return ErrBranchIsProtected
return git_model.ErrBranchIsProtected
}
commit, err := gitRepo.GetBranchCommit(branchName)

11
services/repository/files/patch.go
View file

@ -66,13 +66,16 @@ func (opts *ApplyDiffPatchOptions) Validate(ctx context.Context, repo *repo_mode
return err
}
} else {
protectedBranch, err := git_model.GetProtectedBranchBy(ctx, repo.ID, opts.OldBranch)
protectedBranch, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repo.ID, opts.OldBranch)
if err != nil {
return err
}
if protectedBranch != nil && !protectedBranch.CanUserPush(ctx, doer.ID) {
return models.ErrUserCannotCommit{
UserName: doer.LowerName,
if protectedBranch != nil {
protectedBranch.Repo = repo
if !protectedBranch.CanUserPush(ctx, doer) {
return models.ErrUserCannotCommit{
UserName: doer.LowerName,
}
}
}
if protectedBranch != nil && protectedBranch.RequireSignedCommits {

5
services/repository/files/update.go
View file

@ -463,17 +463,18 @@ func CreateOrUpdateRepoFile(ctx context.Context, repo *repo_model.Repository, do
// VerifyBranchProtection verify the branch protection for modifying the given treePath on the given branch
func VerifyBranchProtection(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, branchName, treePath string) error {
protectedBranch, err := git_model.GetProtectedBranchBy(ctx, repo.ID, branchName)
protectedBranch, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repo.ID, branchName)
if err != nil {
return err
}
if protectedBranch != nil {
protectedBranch.Repo = repo
isUnprotectedFile := false
glob := protectedBranch.GetUnprotectedFilePatterns()
if len(glob) != 0 {
isUnprotectedFile = protectedBranch.IsUnprotectedFile(glob, treePath)
}
if !protectedBranch.CanUserPush(ctx, doer.ID) && !isUnprotectedFile {
if !protectedBranch.CanUserPush(ctx, doer) && !isUnprotectedFile {
return models.ErrUserCannotCommit{
UserName: doer.LowerName,
}