From bbd910ed1b4f52ee66a5cdd8d11f856598161bef Mon Sep 17 00:00:00 2001 From: Lauris BH Date: Fri, 27 Mar 2020 00:26:34 +0200 Subject: [PATCH] Allow to set protected file patterns that can not be changed under no conditions (#10806) Co-Authored-By: zeripath --- models/branches.go | 19 +++++ models/error.go | 19 +++++ models/migrations/migrations.go | 2 + models/migrations/v132.go | 22 ++++++ modules/auth/repo_form.go | 1 + modules/convert/convert.go | 1 + modules/repofiles/delete.go | 32 ++++++--- modules/repofiles/update.go | 32 ++++++--- modules/structs/repo_branch.go | 3 + options/locale/locale_en-US.ini | 2 + routers/api/v1/repo/branch.go | 5 ++ routers/private/hook.go | 69 ++++++++++++++++++- routers/repo/setting_protected_branch.go | 1 + templates/repo/settings/protected_branch.tmpl | 5 ++ templates/swagger/v1_json.tmpl | 12 ++++ 15 files changed, 202 insertions(+), 23 deletions(-) create mode 100644 models/migrations/v132.go diff --git a/models/branches.go b/models/branches.go index 75f5c0a3a..d488fc9fc 100644 --- a/models/branches.go +++ b/models/branches.go @@ -7,6 +7,7 @@ package models import ( "context" "fmt" + "strings" "time" "code.gitea.io/gitea/modules/base" @@ -15,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" + "github.com/gobwas/glob" "github.com/unknwon/com" ) @@ -47,6 +49,7 @@ type ProtectedBranch struct { BlockOnRejectedReviews bool `xorm:"NOT NULL DEFAULT false"` DismissStaleApprovals bool `xorm:"NOT NULL DEFAULT false"` RequireSignedCommits bool `xorm:"NOT NULL DEFAULT false"` + ProtectedFilePatterns string `xorm:"TEXT"` CreatedUnix timeutil.TimeStamp `xorm:"created"` UpdatedUnix timeutil.TimeStamp `xorm:"updated"` @@ -190,6 +193,22 @@ func (protectBranch *ProtectedBranch) MergeBlockedByRejectedReview(pr *PullReque return rejectExist } +// GetProtectedFilePatterns parses a semicolon separated list of protected file patterns and returns a glob.Glob slice +func (protectBranch *ProtectedBranch) GetProtectedFilePatterns() []glob.Glob { + extarr := make([]glob.Glob, 0, 10) + for _, expr := range strings.Split(strings.ToLower(protectBranch.ProtectedFilePatterns), ";") { + expr = strings.TrimSpace(expr) + if expr != "" { + if g, err := glob.Compile(expr, '.', '/'); err != nil { + log.Info("Invalid glob expresion '%s' (skipped): %v", expr, err) + } else { + extarr = append(extarr, g) + } + } + } + return extarr +} + // GetProtectedBranchByRepoID getting protected branch by repo ID func GetProtectedBranchByRepoID(repoID int64) ([]*ProtectedBranch, error) { protectedBranches := make([]*ProtectedBranch, 0) diff --git a/models/error.go b/models/error.go index 09fb4ebca..19a1229ad 100644 --- a/models/error.go +++ b/models/error.go @@ -916,6 +916,25 @@ func (err ErrFilePathInvalid) Error() string { return fmt.Sprintf("path is invalid [path: %s]", err.Path) } +// ErrFilePathProtected represents a "FilePathProtected" kind of error. +type ErrFilePathProtected struct { + Message string + Path string +} + +// IsErrFilePathProtected checks if an error is an ErrFilePathProtected. +func IsErrFilePathProtected(err error) bool { + _, ok := err.(ErrFilePathProtected) + return ok +} + +func (err ErrFilePathProtected) Error() string { + if err.Message != "" { + return err.Message + } + return fmt.Sprintf("path is protected and can not be changed [path: %s]", err.Path) +} + // ErrUserDoesNotHaveAccessToRepo represets an error where the user doesn't has access to a given repo. type ErrUserDoesNotHaveAccessToRepo struct { UserID int64 diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 2badb7278..c554121e8 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -196,6 +196,8 @@ var migrations = []Migration{ NewMigration("Expand webhooks for more granularity", expandWebhooks), // v131 -> v132 NewMigration("Add IsSystemWebhook column to webhooks table", addSystemWebhookColumn), + // v132 -> v133 + NewMigration("Add Branch Protection Protected Files Column", addBranchProtectionProtectedFilesColumn), } // Migrate database to current version diff --git a/models/migrations/v132.go b/models/migrations/v132.go new file mode 100644 index 000000000..3f7b1c970 --- /dev/null +++ b/models/migrations/v132.go @@ -0,0 +1,22 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "fmt" + + "xorm.io/xorm" +) + +func addBranchProtectionProtectedFilesColumn(x *xorm.Engine) error { + type ProtectedBranch struct { + ProtectedFilePatterns string `xorm:"TEXT"` + } + + if err := x.Sync2(new(ProtectedBranch)); err != nil { + return fmt.Errorf("Sync2: %v", err) + } + return nil +} diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index 84ab35f64..123090dbb 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -175,6 +175,7 @@ type ProtectBranchForm struct { BlockOnRejectedReviews bool DismissStaleApprovals bool RequireSignedCommits bool + ProtectedFilePatterns string } // Validate validates the fields diff --git a/modules/convert/convert.go b/modules/convert/convert.go index d75a13053..e11a599fd 100644 --- a/modules/convert/convert.go +++ b/modules/convert/convert.go @@ -120,6 +120,7 @@ func ToBranchProtection(bp *models.ProtectedBranch) *api.BranchProtection { BlockOnRejectedReviews: bp.BlockOnRejectedReviews, DismissStaleApprovals: bp.DismissStaleApprovals, RequireSignedCommits: bp.RequireSignedCommits, + ProtectedFilePatterns: bp.ProtectedFilePatterns, Created: bp.CreatedUnix.AsTime(), Updated: bp.UpdatedUnix.AsTime(), } diff --git a/modules/repofiles/delete.go b/modules/repofiles/delete.go index c1689b0be..2ffc75e7c 100644 --- a/modules/repofiles/delete.go +++ b/modules/repofiles/delete.go @@ -60,21 +60,31 @@ func DeleteRepoFile(repo *models.Repository, doer *models.User, opts *DeleteRepo if err != nil { return nil, err } - if protectedBranch != nil && !protectedBranch.CanUserPush(doer.ID) { - return nil, models.ErrUserCannotCommit{ - UserName: doer.LowerName, - } - } - if protectedBranch != nil && protectedBranch.RequireSignedCommits { - _, _, err := repo.SignCRUDAction(doer, repo.RepoPath(), opts.OldBranch) - if err != nil { - if !models.IsErrWontSign(err) { - return nil, err - } + if protectedBranch != nil { + if !protectedBranch.CanUserPush(doer.ID) { return nil, models.ErrUserCannotCommit{ UserName: doer.LowerName, } } + if protectedBranch.RequireSignedCommits { + _, _, err := repo.SignCRUDAction(doer, repo.RepoPath(), opts.OldBranch) + if err != nil { + if !models.IsErrWontSign(err) { + return nil, err + } + return nil, models.ErrUserCannotCommit{ + UserName: doer.LowerName, + } + } + } + patterns := protectedBranch.GetProtectedFilePatterns() + for _, pat := range patterns { + if pat.Match(strings.ToLower(opts.TreePath)) { + return nil, models.ErrFilePathProtected{ + Path: opts.TreePath, + } + } + } } } diff --git a/modules/repofiles/update.go b/modules/repofiles/update.go index e2aafb567..86f53d4a1 100644 --- a/modules/repofiles/update.go +++ b/modules/repofiles/update.go @@ -156,21 +156,31 @@ func CreateOrUpdateRepoFile(repo *models.Repository, doer *models.User, opts *Up if err != nil { return nil, err } - if protectedBranch != nil && !protectedBranch.CanUserPush(doer.ID) { - return nil, models.ErrUserCannotCommit{ - UserName: doer.LowerName, - } - } - if protectedBranch != nil && protectedBranch.RequireSignedCommits { - _, _, err := repo.SignCRUDAction(doer, repo.RepoPath(), opts.OldBranch) - if err != nil { - if !models.IsErrWontSign(err) { - return nil, err - } + if protectedBranch != nil { + if !protectedBranch.CanUserPush(doer.ID) { return nil, models.ErrUserCannotCommit{ UserName: doer.LowerName, } } + if protectedBranch.RequireSignedCommits { + _, _, err := repo.SignCRUDAction(doer, repo.RepoPath(), opts.OldBranch) + if err != nil { + if !models.IsErrWontSign(err) { + return nil, err + } + return nil, models.ErrUserCannotCommit{ + UserName: doer.LowerName, + } + } + } + patterns := protectedBranch.GetProtectedFilePatterns() + for _, pat := range patterns { + if pat.Match(strings.ToLower(opts.TreePath)) { + return nil, models.ErrFilePathProtected{ + Path: opts.TreePath, + } + } + } } } diff --git a/modules/structs/repo_branch.go b/modules/structs/repo_branch.go index f8c929054..886018c85 100644 --- a/modules/structs/repo_branch.go +++ b/modules/structs/repo_branch.go @@ -41,6 +41,7 @@ type BranchProtection struct { BlockOnRejectedReviews bool `json:"block_on_rejected_reviews"` DismissStaleApprovals bool `json:"dismiss_stale_approvals"` RequireSignedCommits bool `json:"require_signed_commits"` + ProtectedFilePatterns string `json:"protected_file_patterns"` // swagger:strfmt date-time Created time.Time `json:"created_at"` // swagger:strfmt date-time @@ -67,6 +68,7 @@ type CreateBranchProtectionOption struct { BlockOnRejectedReviews bool `json:"block_on_rejected_reviews"` DismissStaleApprovals bool `json:"dismiss_stale_approvals"` RequireSignedCommits bool `json:"require_signed_commits"` + ProtectedFilePatterns string `json:"protected_file_patterns"` } // EditBranchProtectionOption options for editing a branch protection @@ -88,4 +90,5 @@ type EditBranchProtectionOption struct { BlockOnRejectedReviews *bool `json:"block_on_rejected_reviews"` DismissStaleApprovals *bool `json:"dismiss_stale_approvals"` RequireSignedCommits *bool `json:"require_signed_commits"` + ProtectedFilePatterns *string `json:"protected_file_patterns"` } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 483970e03..092f64651 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1488,6 +1488,8 @@ settings.dismiss_stale_approvals = Dismiss stale approvals settings.dismiss_stale_approvals_desc = When new commits that change the content of the pull request are pushed to the branch, old approvals will be dismissed. settings.require_signed_commits = Require Signed Commits settings.require_signed_commits_desc = Reject pushes to this branch if they are unsigned or unverifiable +settings.protect_protected_file_patterns = Protected file patterns (separated using semicolon ';'): +settings.protect_protected_file_patterns_desc = Protected files that are not allowed to be changed directly even if user has rights to add, edit or delete files in this branch. Multiple patterns can be separated using semicolon (';'). See github.com/gobwas/glob documentation for pattern syntax. Examples: .drone.yml, /docs/**/*.txt. settings.add_protected_branch = Enable protection settings.delete_protected_branch = Disable protection settings.update_protect_branch_success = Branch protection for branch '%s' has been updated. diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 0d4501cd7..56a9b5ec2 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -339,6 +339,7 @@ func CreateBranchProtection(ctx *context.APIContext, form api.CreateBranchProtec BlockOnRejectedReviews: form.BlockOnRejectedReviews, DismissStaleApprovals: form.DismissStaleApprovals, RequireSignedCommits: form.RequireSignedCommits, + ProtectedFilePatterns: form.ProtectedFilePatterns, } err = models.UpdateProtectBranch(ctx.Repo.Repository, protectBranch, models.WhitelistOptions{ @@ -470,6 +471,10 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection protectBranch.RequireSignedCommits = *form.RequireSignedCommits } + if form.ProtectedFilePatterns != nil { + protectBranch.ProtectedFilePatterns = *form.ProtectedFilePatterns + } + var whitelistUsers []int64 if form.PushWhitelistUsernames != nil { whitelistUsers, err = models.GetUserIDsByNames(form.PushWhitelistUsernames, false) diff --git a/routers/private/hook.go b/routers/private/hook.go index 38b37fa7b..846d9b607 100644 --- a/routers/private/hook.go +++ b/routers/private/hook.go @@ -22,9 +22,10 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" pull_service "code.gitea.io/gitea/services/pull" - "github.com/go-git/go-git/v5/plumbing" "gitea.com/macaron/macaron" + "github.com/go-git/go-git/v5/plumbing" + "github.com/gobwas/glob" ) func verifyCommits(oldCommitID, newCommitID string, repo *git.Repository, env []string) error { @@ -57,6 +58,52 @@ func verifyCommits(oldCommitID, newCommitID string, repo *git.Repository, env [] return err } +func checkFileProtection(oldCommitID, newCommitID string, patterns []glob.Glob, repo *git.Repository, env []string) error { + + stdoutReader, stdoutWriter, err := os.Pipe() + if err != nil { + log.Error("Unable to create os.Pipe for %s", repo.Path) + return err + } + defer func() { + _ = stdoutReader.Close() + _ = stdoutWriter.Close() + }() + + err = git.NewCommand("diff", "--name-only", oldCommitID+"..."+newCommitID). + RunInDirTimeoutEnvFullPipelineFunc(env, -1, repo.Path, + stdoutWriter, nil, nil, + func(ctx context.Context, cancel context.CancelFunc) error { + _ = stdoutWriter.Close() + + scanner := bufio.NewScanner(stdoutReader) + for scanner.Scan() { + path := strings.TrimSpace(scanner.Text()) + if len(path) == 0 { + continue + } + lpath := strings.ToLower(path) + for _, pat := range patterns { + if pat.Match(lpath) { + cancel() + return models.ErrFilePathProtected{ + Path: path, + } + } + } + } + if err := scanner.Err(); err != nil { + return err + } + _ = stdoutReader.Close() + return err + }) + if err != nil && !models.IsErrFilePathProtected(err) { + log.Error("Unable to check file protection for commits from %s to %s in %s: %v", oldCommitID, newCommitID, repo.Path, err) + } + return err +} + func readAndVerifyCommitsFromShaReader(input io.ReadCloser, repo *git.Repository, env []string) error { scanner := bufio.NewScanner(input) for scanner.Scan() { @@ -216,6 +263,26 @@ func HookPreReceive(ctx *macaron.Context, opts private.HookOptions) { } } + globs := protectBranch.GetProtectedFilePatterns() + if len(globs) > 0 { + err := checkFileProtection(oldCommitID, newCommitID, globs, gitRepo, env) + if err != nil { + if !models.IsErrFilePathProtected(err) { + log.Error("Unable to check file protection for commits from %s to %s in %-v: %v", oldCommitID, newCommitID, repo, err) + ctx.JSON(http.StatusInternalServerError, map[string]interface{}{ + "err": fmt.Sprintf("Unable to check file protection for commits from %s to %s: %v", oldCommitID, newCommitID, err), + }) + return + } + protectedFilePath := err.(models.ErrFilePathProtected).Path + log.Warn("Forbidden: Branch: %s in %-v is protected from changing file %s", branchName, repo, protectedFilePath) + ctx.JSON(http.StatusForbidden, map[string]interface{}{ + "err": fmt.Sprintf("branch %s is protected from changing file %s", branchName, protectedFilePath), + }) + return + } + } + canPush := false if opts.IsDeployKey { canPush = protectBranch.CanPush && (!protectBranch.EnableWhitelist || protectBranch.WhitelistDeployKeys) diff --git a/routers/repo/setting_protected_branch.go b/routers/repo/setting_protected_branch.go index e8902ed8a..af49aefce 100644 --- a/routers/repo/setting_protected_branch.go +++ b/routers/repo/setting_protected_branch.go @@ -247,6 +247,7 @@ func SettingsProtectedBranchPost(ctx *context.Context, f auth.ProtectBranchForm) protectBranch.BlockOnRejectedReviews = f.BlockOnRejectedReviews protectBranch.DismissStaleApprovals = f.DismissStaleApprovals protectBranch.RequireSignedCommits = f.RequireSignedCommits + protectBranch.ProtectedFilePatterns = f.ProtectedFilePatterns err = models.UpdateProtectBranch(ctx.Repo.Repository, protectBranch, models.WhitelistOptions{ UserIDs: whitelistUsers, diff --git a/templates/repo/settings/protected_branch.tmpl b/templates/repo/settings/protected_branch.tmpl index 86045f433..f46e84d3b 100644 --- a/templates/repo/settings/protected_branch.tmpl +++ b/templates/repo/settings/protected_branch.tmpl @@ -225,6 +225,11 @@

{{.i18n.Tr "repo.settings.require_signed_commits_desc"}}

+
+ + +

{{.i18n.Tr "repo.settings.protect_protected_file_patterns_desc" | Safe}}

+
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 2da0af95b..c564f8739 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -9818,6 +9818,10 @@ }, "x-go-name": "MergeWhitelistUsernames" }, + "protected_file_patterns": { + "type": "string", + "x-go-name": "ProtectedFilePatterns" + }, "push_whitelist_deploy_keys": { "type": "boolean", "x-go-name": "PushWhitelistDeployKeys" @@ -10129,6 +10133,10 @@ }, "x-go-name": "MergeWhitelistUsernames" }, + "protected_file_patterns": { + "type": "string", + "x-go-name": "ProtectedFilePatterns" + }, "push_whitelist_deploy_keys": { "type": "boolean", "x-go-name": "PushWhitelistDeployKeys" @@ -10933,6 +10941,10 @@ }, "x-go-name": "MergeWhitelistUsernames" }, + "protected_file_patterns": { + "type": "string", + "x-go-name": "ProtectedFilePatterns" + }, "push_whitelist_deploy_keys": { "type": "boolean", "x-go-name": "PushWhitelistDeployKeys"