Support unprotected file patterns (#16395)

Fixes #16381

Note that changes to unprotected files via the web editor still cannot be pushed directly to the protected branch. I could easily add such support for edits and deletes if needed. But for adding, uploading or renaming unprotected files, it is not trivial.

* Extract & Move GetAffectedFiles to modules/git
This commit is contained in:
Jimmy Praet 2021-09-11 16:21:17 +02:00 committed by GitHub
parent eb03e819d3
commit 3d6cb25e31
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 254 additions and 126 deletions

View File

@ -365,7 +365,7 @@ func doBranchProtectPRMerge(baseCtx *APITestContext, dstPath string) func(t *tes
t.Run("PushProtectedBranch", doGitPushTestRepository(dstPath, "origin", "protected"))
ctx := NewAPITestContext(t, baseCtx.Username, baseCtx.Reponame)
t.Run("ProtectProtectedBranchNoWhitelist", doProtectBranch(ctx, "protected", ""))
t.Run("ProtectProtectedBranchNoWhitelist", doProtectBranch(ctx, "protected", "", ""))
t.Run("GenerateCommit", func(t *testing.T) {
_, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-")
assert.NoError(t, err)
@ -391,7 +391,15 @@ func doBranchProtectPRMerge(baseCtx *APITestContext, dstPath string) func(t *tes
t.Run("MergePR2", doAPIMergePullRequest(ctx, baseCtx.Username, baseCtx.Reponame, pr2.Index))
t.Run("MergePR", doAPIMergePullRequest(ctx, baseCtx.Username, baseCtx.Reponame, pr.Index))
t.Run("PullProtected", doGitPull(dstPath, "origin", "protected"))
t.Run("ProtectProtectedBranchWhitelist", doProtectBranch(ctx, "protected", baseCtx.Username))
t.Run("ProtectProtectedBranchUnprotectedFilePaths", doProtectBranch(ctx, "protected", "", "unprotected-file-*"))
t.Run("GenerateCommit", func(t *testing.T) {
_, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "unprotected-file-")
assert.NoError(t, err)
})
t.Run("PushUnprotectedFilesToProtectedBranch", doGitPushTestRepository(dstPath, "origin", "protected"))
t.Run("ProtectProtectedBranchWhitelist", doProtectBranch(ctx, "protected", baseCtx.Username, ""))
t.Run("CheckoutMaster", doGitCheckoutBranch(dstPath, "master"))
t.Run("CreateBranchForced", doGitCreateBranch(dstPath, "toforce"))
@ -406,7 +414,7 @@ func doBranchProtectPRMerge(baseCtx *APITestContext, dstPath string) func(t *tes
}
}
func doProtectBranch(ctx APITestContext, branch string, userToWhitelist string) func(t *testing.T) {
func doProtectBranch(ctx APITestContext, branch string, userToWhitelist string, unprotectedFilePatterns string) func(t *testing.T) {
// We are going to just use the owner to set the protection.
return func(t *testing.T) {
csrf := GetCSRF(t, ctx.Session, fmt.Sprintf("/%s/%s/settings/branches", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame)))
@ -416,6 +424,7 @@ func doProtectBranch(ctx APITestContext, branch string, userToWhitelist string)
req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/settings/branches/%s", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame), url.PathEscape(branch)), map[string]string{
"_csrf": csrf,
"protected": "on",
"unprotected_file_patterns": unprotectedFilePatterns,
})
ctx.Session.MakeRequest(t, req, http.StatusFound)
} else {
@ -428,6 +437,7 @@ func doProtectBranch(ctx APITestContext, branch string, userToWhitelist string)
"enable_push": "whitelist",
"enable_whitelist": "on",
"whitelist_users": strconv.FormatInt(user.ID, 10),
"unprotected_file_patterns": unprotectedFilePatterns,
})
ctx.Session.MakeRequest(t, req, http.StatusFound)
}

View File

@ -43,6 +43,7 @@ type ProtectedBranch struct {
DismissStaleApprovals bool `xorm:"NOT NULL DEFAULT false"`
RequireSignedCommits bool `xorm:"NOT NULL DEFAULT false"`
ProtectedFilePatterns string `xorm:"TEXT"`
UnprotectedFilePatterns string `xorm:"TEXT"`
CreatedUnix timeutil.TimeStamp `xorm:"created"`
UpdatedUnix timeutil.TimeStamp `xorm:"updated"`
@ -214,8 +215,17 @@ func (protectBranch *ProtectedBranch) MergeBlockedByOutdatedBranch(pr *PullReque
// GetProtectedFilePatterns parses a semicolon separated list of protected file patterns and returns a glob.Glob slice
func (protectBranch *ProtectedBranch) GetProtectedFilePatterns() []glob.Glob {
return getFilePatterns(protectBranch.ProtectedFilePatterns)
}
// GetUnprotectedFilePatterns parses a semicolon separated list of unprotected file patterns and returns a glob.Glob slice
func (protectBranch *ProtectedBranch) GetUnprotectedFilePatterns() []glob.Glob {
return getFilePatterns(protectBranch.UnprotectedFilePatterns)
}
func getFilePatterns(filePatterns string) []glob.Glob {
extarr := make([]glob.Glob, 0, 10)
for _, expr := range strings.Split(strings.ToLower(protectBranch.ProtectedFilePatterns), ";") {
for _, expr := range strings.Split(strings.ToLower(filePatterns), ";") {
expr = strings.TrimSpace(expr)
if expr != "" {
if g, err := glob.Compile(expr, '.', '/'); err != nil {
@ -260,6 +270,28 @@ func (protectBranch *ProtectedBranch) IsProtectedFile(patterns []glob.Glob, path
return r
}
// IsUnprotectedFile return if path is unprotected
func (protectBranch *ProtectedBranch) IsUnprotectedFile(patterns []glob.Glob, path string) bool {
if len(patterns) == 0 {
patterns = protectBranch.GetUnprotectedFilePatterns()
if len(patterns) == 0 {
return false
}
}
lpath := strings.ToLower(strings.TrimSpace(path))
r := false
for _, pat := range patterns {
if pat.Match(lpath) {
r = true
break
}
}
return r
}
// GetProtectedBranchBy getting protected branch by ID/Name
func GetProtectedBranchBy(repoID int64, branchName string) (*ProtectedBranch, error) {
return getProtectedBranchBy(x, repoID, branchName)

View File

@ -340,6 +340,8 @@ var migrations = []Migration{
NewMigration("RecreateIssueResourceIndexTable to have a primary key instead of an unique index", recreateIssueResourceIndexTable),
// v193 -> v194
NewMigration("Add repo id column for attachment table", addRepoIDForAttachment),
// v194 -> v195
NewMigration("Add Branch Protection Unprotected Files Column", addBranchProtectionUnprotectedFilesColumn),
}
// GetCurrentDBVersion returns the current db version

22
models/migrations/v194.go Normal file
View File

@ -0,0 +1,22 @@
// Copyright 2021 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 addBranchProtectionUnprotectedFilesColumn(x *xorm.Engine) error {
type ProtectedBranch struct {
UnprotectedFilePatterns string `xorm:"TEXT"`
}
if err := x.Sync2(new(ProtectedBranch)); err != nil {
return fmt.Errorf("Sync2: %v", err)
}
return nil
}

View File

@ -127,6 +127,7 @@ func ToBranchProtection(bp *models.ProtectedBranch) *api.BranchProtection {
DismissStaleApprovals: bp.DismissStaleApprovals,
RequireSignedCommits: bp.RequireSignedCommits,
ProtectedFilePatterns: bp.ProtectedFilePatterns,
UnprotectedFilePatterns: bp.UnprotectedFilePatterns,
Created: bp.CreatedUnix.AsTime(),
Updated: bp.UpdatedUnix.AsTime(),
}

View File

@ -10,6 +10,7 @@ import (
"context"
"fmt"
"io"
"os"
"os/exec"
"regexp"
"strconv"
@ -273,3 +274,46 @@ func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLi
oldBegin, oldNumOfLines, newBegin, newNumOfLines)
return strings.Join(newHunk, "\n"), nil
}
// GetAffectedFiles returns the affected files between two commits
func GetAffectedFiles(oldCommitID, newCommitID string, env []string, repo *Repository) ([]string, error) {
stdoutReader, stdoutWriter, err := os.Pipe()
if err != nil {
log.Error("Unable to create os.Pipe for %s", repo.Path)
return nil, err
}
defer func() {
_ = stdoutReader.Close()
_ = stdoutWriter.Close()
}()
affectedFiles := make([]string, 0, 32)
// Run `git diff --name-only` to get the names of the changed files
err = NewCommand("diff", "--name-only", oldCommitID, newCommitID).
RunInDirTimeoutEnvFullPipelineFunc(env, -1, repo.Path,
stdoutWriter, nil, nil,
func(ctx context.Context, cancel context.CancelFunc) error {
// Close the writer end of the pipe to begin processing
_ = stdoutWriter.Close()
defer func() {
// Close the reader on return to terminate the git command if necessary
_ = stdoutReader.Close()
}()
// Now scan the output from the command
scanner := bufio.NewScanner(stdoutReader)
for scanner.Scan() {
path := strings.TrimSpace(scanner.Text())
if len(path) == 0 {
continue
}
affectedFiles = append(affectedFiles, path)
}
return scanner.Err()
})
if err != nil {
log.Error("Unable to get affected files for commits from %s to %s in %s: %v", oldCommitID, newCommitID, repo.Path, err)
}
return affectedFiles, err
}

View File

@ -56,38 +56,9 @@ func DeleteRepoFile(repo *models.Repository, doer *models.User, opts *DeleteRepo
BranchName: opts.NewBranch,
}
}
} else {
protectedBranch, err := repo.GetBranchProtection(opts.OldBranch)
if err != nil {
} else if err := VerifyBranchProtection(repo, doer, opts.OldBranch, opts.TreePath); err != nil {
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,
}
}
}
}
}
// Check that the path given in opts.treeName is valid (not a git path)
treePath := CleanUploadFileName(opts.TreePath)

View File

@ -148,38 +148,9 @@ func CreateOrUpdateRepoFile(repo *models.Repository, doer *models.User, opts *Up
if err != nil && !git.IsErrBranchNotExist(err) {
return nil, err
}
} else {
protectedBranch, err := repo.GetBranchProtection(opts.OldBranch)
if err != nil {
} else if err := VerifyBranchProtection(repo, doer, opts.OldBranch, opts.TreePath); err != nil {
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,
}
}
}
}
}
// If FromTreePath is not set, set it to the opts.TreePath
if opts.TreePath != "" && opts.FromTreePath == "" {
@ -465,3 +436,43 @@ func CreateOrUpdateRepoFile(repo *models.Repository, doer *models.User, opts *Up
}
return file, nil
}
// VerifyBranchProtection verify the branch protection for modifying the given treePath on the given branch
func VerifyBranchProtection(repo *models.Repository, doer *models.User, branchName string, treePath string) error {
protectedBranch, err := repo.GetBranchProtection(branchName)
if err != nil {
return err
}
if protectedBranch != nil {
isUnprotectedFile := false
glob := protectedBranch.GetUnprotectedFilePatterns()
if len(glob) != 0 {
isUnprotectedFile = protectedBranch.IsUnprotectedFile(glob, treePath)
}
if !protectedBranch.CanUserPush(doer.ID) && !isUnprotectedFile {
return models.ErrUserCannotCommit{
UserName: doer.LowerName,
}
}
if protectedBranch.RequireSignedCommits {
_, _, _, err := repo.SignCRUDAction(doer, repo.RepoPath(), branchName)
if err != nil {
if !models.IsErrWontSign(err) {
return err
}
return models.ErrUserCannotCommit{
UserName: doer.LowerName,
}
}
}
patterns := protectedBranch.GetProtectedFilePatterns()
for _, pat := range patterns {
if pat.Match(strings.ToLower(treePath)) {
return models.ErrFilePathProtected{
Path: treePath,
}
}
}
}
return nil
}

View File

@ -44,6 +44,7 @@ type BranchProtection struct {
DismissStaleApprovals bool `json:"dismiss_stale_approvals"`
RequireSignedCommits bool `json:"require_signed_commits"`
ProtectedFilePatterns string `json:"protected_file_patterns"`
UnprotectedFilePatterns string `json:"unprotected_file_patterns"`
// swagger:strfmt date-time
Created time.Time `json:"created_at"`
// swagger:strfmt date-time
@ -73,6 +74,7 @@ type CreateBranchProtectionOption struct {
DismissStaleApprovals bool `json:"dismiss_stale_approvals"`
RequireSignedCommits bool `json:"require_signed_commits"`
ProtectedFilePatterns string `json:"protected_file_patterns"`
UnprotectedFilePatterns string `json:"unprotected_file_patterns"`
}
// EditBranchProtectionOption options for editing a branch protection
@ -97,4 +99,5 @@ type EditBranchProtectionOption struct {
DismissStaleApprovals *bool `json:"dismiss_stale_approvals"`
RequireSignedCommits *bool `json:"require_signed_commits"`
ProtectedFilePatterns *string `json:"protected_file_patterns"`
UnprotectedFilePatterns *string `json:"unprotected_file_patterns"`
}

View File

@ -1908,6 +1908,8 @@ 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 <a href="https://pkg.go.dev/github.com/gobwas/glob#Compile">github.com/gobwas/glob</a> documentation for pattern syntax. Examples: <code>.drone.yml</code>, <code>/docs/**/*.txt</code>.
settings.protect_unprotected_file_patterns = Unprotected file patterns (separated using semicolon '\;'):
settings.protect_unprotected_file_patterns_desc = Unprotected files that are allowed to be changed directly if user has write access, bypassing push restriction. Multiple patterns can be separated using semicolon ('\;'). See <a href="https://pkg.go.dev/github.com/gobwas/glob#Compile">github.com/gobwas/glob</a> documentation for pattern syntax. Examples: <code>.drone.yml</code>, <code>/docs/**/*.txt</code>.
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.

View File

@ -498,6 +498,7 @@ func CreateBranchProtection(ctx *context.APIContext) {
DismissStaleApprovals: form.DismissStaleApprovals,
RequireSignedCommits: form.RequireSignedCommits,
ProtectedFilePatterns: form.ProtectedFilePatterns,
UnprotectedFilePatterns: form.UnprotectedFilePatterns,
BlockOnOutdatedBranch: form.BlockOnOutdatedBranch,
}
@ -643,6 +644,10 @@ func EditBranchProtection(ctx *context.APIContext) {
protectBranch.ProtectedFilePatterns = *form.ProtectedFilePatterns
}
if form.UnprotectedFilePatterns != nil {
protectBranch.UnprotectedFilePatterns = *form.UnprotectedFilePatterns
}
if form.BlockOnOutdatedBranch != nil {
protectBranch.BlockOnOutdatedBranch = *form.BlockOnOutdatedBranch
}

View File

@ -343,6 +343,23 @@ func HookPreReceive(ctx *gitea_context.PrivateContext) {
return
}
// Allow commits that only touch unprotected files
globs := protectBranch.GetUnprotectedFilePatterns()
if len(globs) > 0 {
unprotectedFilesOnly, err := pull_service.CheckUnprotectedFiles(oldCommitID, newCommitID, globs, env, gitRepo)
if err != nil {
log.Error("Unable to check file protection for commits from %s to %s in %-v: %v", oldCommitID, newCommitID, repo, err)
ctx.JSON(http.StatusInternalServerError, private.Response{
Err: fmt.Sprintf("Unable to check file protection for commits from %s to %s: %v", oldCommitID, newCommitID, err),
})
return
}
if unprotectedFilesOnly {
// Commit only touches unprotected files, this is allowed
continue
}
}
// Or we're simply not able to push to this protected branch
log.Warn("Forbidden: User %d is not allowed to push to protected branch: %s in %-v", opts.UserID, branchName, repo)
ctx.JSON(http.StatusForbidden, private.Response{

View File

@ -253,6 +253,7 @@ func SettingsProtectedBranchPost(ctx *context.Context) {
protectBranch.DismissStaleApprovals = f.DismissStaleApprovals
protectBranch.RequireSignedCommits = f.RequireSignedCommits
protectBranch.ProtectedFilePatterns = f.ProtectedFilePatterns
protectBranch.UnprotectedFilePatterns = f.UnprotectedFilePatterns
protectBranch.BlockOnOutdatedBranch = f.BlockOnOutdatedBranch
err = models.UpdateProtectBranch(ctx.Repo.Repository, protectBranch, models.WhitelistOptions{

View File

@ -199,6 +199,7 @@ type ProtectBranchForm struct {
DismissStaleApprovals bool
RequireSignedCommits bool
ProtectedFilePatterns string
UnprotectedFilePatterns string
}
// Validate validates the fields

View File

@ -245,47 +245,19 @@ func checkConflicts(pr *models.PullRequest, gitRepo *git.Repository, tmpBasePath
// CheckFileProtection check file Protection
func CheckFileProtection(oldCommitID, newCommitID string, patterns []glob.Glob, limit int, env []string, repo *git.Repository) ([]string, error) {
// 1. If there are no patterns short-circuit and just return nil
if len(patterns) == 0 {
return nil, nil
}
// 2. Prep the pipe
stdoutReader, stdoutWriter, err := os.Pipe()
affectedFiles, err := git.GetAffectedFiles(oldCommitID, newCommitID, env, repo)
if err != nil {
log.Error("Unable to create os.Pipe for %s", repo.Path)
return nil, err
}
defer func() {
_ = stdoutReader.Close()
_ = stdoutWriter.Close()
}()
changedProtectedFiles := make([]string, 0, limit)
// 3. Run `git diff --name-only` to get the names of the changed files
err = git.NewCommand("diff", "--name-only", oldCommitID, newCommitID).
RunInDirTimeoutEnvFullPipelineFunc(env, -1, repo.Path,
stdoutWriter, nil, nil,
func(ctx context.Context, cancel context.CancelFunc) error {
// Close the writer end of the pipe to begin processing
_ = stdoutWriter.Close()
defer func() {
// Close the reader on return to terminate the git command if necessary
_ = stdoutReader.Close()
}()
// Now scan the output from the command
scanner := bufio.NewScanner(stdoutReader)
for scanner.Scan() {
path := strings.TrimSpace(scanner.Text())
if len(path) == 0 {
continue
}
lpath := strings.ToLower(path)
for _, affectedFile := range affectedFiles {
lpath := strings.ToLower(affectedFile)
for _, pat := range patterns {
if pat.Match(lpath) {
changedProtectedFiles = append(changedProtectedFiles, path)
changedProtectedFiles = append(changedProtectedFiles, lpath)
break
}
}
@ -293,20 +265,37 @@ func CheckFileProtection(oldCommitID, newCommitID string, patterns []glob.Glob,
break
}
}
if len(changedProtectedFiles) > 0 {
return models.ErrFilePathProtected{
err = models.ErrFilePathProtected{
Path: changedProtectedFiles[0],
}
}
return scanner.Err()
})
// 4. log real errors if there are any...
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 changedProtectedFiles, err
}
return changedProtectedFiles, err
// CheckUnprotectedFiles check if the commit only touches unprotected files
func CheckUnprotectedFiles(oldCommitID, newCommitID string, patterns []glob.Glob, env []string, repo *git.Repository) (bool, error) {
if len(patterns) == 0 {
return false, nil
}
affectedFiles, err := git.GetAffectedFiles(oldCommitID, newCommitID, env, repo)
if err != nil {
return false, err
}
for _, affectedFile := range affectedFiles {
lpath := strings.ToLower(affectedFile)
unprotected := false
for _, pat := range patterns {
if pat.Match(lpath) {
unprotected = true
break
}
}
if !unprotected {
return false, nil
}
}
return true, nil
}
// checkPullFilesProtection check if pr changed protected files and save results

View File

@ -244,6 +244,11 @@
<input name="protected_file_patterns" id="protected_file_patterns" type="text" value="{{.Branch.ProtectedFilePatterns}}">
<p class="help">{{.i18n.Tr "repo.settings.protect_protected_file_patterns_desc" | Safe}}</p>
</div>
<div class="field">
<label for="unprotected_file_patterns">{{.i18n.Tr "repo.settings.protect_unprotected_file_patterns"}}</label>
<input name="unprotected_file_patterns" id="unprotected_file_patterns" type="text" value="{{.Branch.UnprotectedFilePatterns}}">
<p class="help">{{.i18n.Tr "repo.settings.protect_unprotected_file_patterns_desc" | Safe}}</p>
</div>
</div>

View File

@ -12387,6 +12387,10 @@
},
"x-go-name": "StatusCheckContexts"
},
"unprotected_file_patterns": {
"type": "string",
"x-go-name": "UnprotectedFilePatterns"
},
"updated_at": {
"type": "string",
"format": "date-time",
@ -12834,6 +12838,10 @@
"type": "string"
},
"x-go-name": "StatusCheckContexts"
},
"unprotected_file_patterns": {
"type": "string",
"x-go-name": "UnprotectedFilePatterns"
}
},
"x-go-package": "code.gitea.io/gitea/modules/structs"
@ -13836,6 +13844,10 @@
"type": "string"
},
"x-go-name": "StatusCheckContexts"
},
"unprotected_file_patterns": {
"type": "string",
"x-go-name": "UnprotectedFilePatterns"
}
},
"x-go-package": "code.gitea.io/gitea/modules/structs"