Fix protected branch files detection on pre_receive hook

modules/git/commit_test.go

@@ -4,6 +4,8 @@
 package git
 
 import (
+	"context"
+	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -345,3 +347,18 @@ func TestGetCommitFileStatusMerges(t *testing.T) {
 	assert.Equal(t, commitFileStatus.Removed, expected.Removed)
 	assert.Equal(t, commitFileStatus.Modified, expected.Modified)
 }
+
+func Test_GetCommitBranchStart(t *testing.T) {
+	bareRepo1Path := filepath.Join(testReposDir, "repo1_bare")
+	repo, err := OpenRepository(context.Background(), bareRepo1Path)
+	assert.NoError(t, err)
+	defer repo.Close()
+	commit, err := repo.GetBranchCommit("branch1")
+	assert.NoError(t, err)
+	assert.EqualValues(t, "2839944139e0de9737a044f78b0e4b40d989a9e3", commit.ID.String())
+
+	startCommitID, err := repo.GetCommitBranchStart(os.Environ(), "branch1", commit.ID.String())
+	assert.NoError(t, err)
+	assert.NotEmpty(t, startCommitID)
+	assert.EqualValues(t, "9c9aef8dd84e02bc7ec12641deb4c930a7c30185", startCommitID)
+}

modules/git/diff.go

@@ -271,7 +271,17 @@ func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLi
 }
 
 // GetAffectedFiles returns the affected files between two commits
-func GetAffectedFiles(repo *Repository, oldCommitID, newCommitID string, env []string) ([]string, error) {
+func GetAffectedFiles(repo *Repository, branchName, oldCommitID, newCommitID string, env []string) ([]string, error) {
+	if oldCommitID == emptySha1ObjectID.String() || oldCommitID == emptySha256ObjectID.String() {
+		startCommitID, err := repo.GetCommitBranchStart(env, branchName, newCommitID)
+		if err != nil {
+			return nil, err
+		}
+		if startCommitID == "" {
+			return nil, fmt.Errorf("cannot find the start commit of %s", newCommitID)
+		}
+		oldCommitID = startCommitID
+	}
 	stdoutReader, stdoutWriter, err := os.Pipe()
 	if err != nil {
 		log.Error("Unable to create os.Pipe for %s", repo.Path)

modules/git/repo_commit.go

@@ -7,6 +7,7 @@ package git
 import (
 	"bytes"
 	"io"
+	"os"
 	"strconv"
 	"strings"
 
@@ -414,7 +415,7 @@ func (repo *Repository) commitsBefore(id ObjectID, limit int) ([]*Commit, error)
 
 	commits := make([]*Commit, 0, len(formattedLog))
 	for _, commit := range formattedLog {
-		branches, err := repo.getBranches(commit, 2)
+		branches, err := repo.getBranches(os.Environ(), commit.ID.String(), 2)
 		if err != nil {
 			return nil, err
 		}
@@ -437,12 +438,15 @@ func (repo *Repository) getCommitsBeforeLimit(id ObjectID, num int) ([]*Commit,
 	return repo.commitsBefore(id, num)
 }
 
-func (repo *Repository) getBranches(commit *Commit, limit int) ([]string, error) {
+func (repo *Repository) getBranches(env []string, commitID string, limit int) ([]string, error) {
 	if DefaultFeatures().CheckVersionAtLeast("2.7.0") {
 		stdout, _, err := NewCommand(repo.Ctx, "for-each-ref", "--format=%(refname:strip=2)").
 			AddOptionFormat("--count=%d", limit).
-			AddOptionValues("--contains", commit.ID.String(), BranchPrefix).
-			RunStdString(&RunOpts{Dir: repo.Path})
+			AddOptionValues("--contains", commitID, BranchPrefix).
+			RunStdString(&RunOpts{
+				Dir: repo.Path,
+				Env: env,
+			})
 		if err != nil {
 			return nil, err
 		}
@@ -451,7 +455,10 @@ func (repo *Repository) getBranches(commit *Commit, limit int) ([]string, error)
 		return branches, nil
 	}
 
-	stdout, _, err := NewCommand(repo.Ctx, "branch").AddOptionValues("--contains", commit.ID.String()).RunStdString(&RunOpts{Dir: repo.Path})
+	stdout, _, err := NewCommand(repo.Ctx, "branch").AddOptionValues("--contains", commitID).RunStdString(&RunOpts{
+		Dir: repo.Path,
+		Env: env,
+	})
 	if err != nil {
 		return nil, err
 	}
@@ -513,3 +520,35 @@ func (repo *Repository) AddLastCommitCache(cacheKey, fullName, sha string) error
 	}
 	return nil
 }
+
+func (repo *Repository) GetCommitBranchStart(env []string, branch, endCommitID string) (string, error) {
+	cmd := NewCommand(repo.Ctx, "log", prettyLogFormat)
+	cmd.AddDynamicArguments(endCommitID)
+
+	stdout, _, runErr := cmd.RunStdBytes(&RunOpts{
+		Dir: repo.Path,
+		Env: env,
+	})
+	if runErr != nil {
+		return "", runErr
+	}
+
+	parts := bytes.Split(bytes.TrimSpace(stdout), []byte{'\n'})
+
+	var startCommitID string
+	for _, commitID := range parts {
+		branches, err := repo.getBranches(env, string(commitID), 2)
+		if err != nil {
+			return "", err
+		}
+		for _, b := range branches {
+			if b != branch {
+				return startCommitID, nil
+			}
+		}
+
+		startCommitID = string(commitID)
+	}
+
+	return "", nil
+}

modules/git/repo_commit_test.go

@@ -4,6 +4,7 @@
 package git
 
 import (
+	"os"
 	"path/filepath"
 	"testing"
 
@@ -31,7 +32,7 @@ func TestRepository_GetCommitBranches(t *testing.T) {
 	for _, testCase := range testCases {
 		commit, err := bareRepo1.GetCommit(testCase.CommitID)
 		assert.NoError(t, err)
-		branches, err := bareRepo1.getBranches(commit, 2)
+		branches, err := bareRepo1.getBranches(os.Environ(), commit.ID.String(), 2)
 		assert.NoError(t, err)
 		assert.Equal(t, testCase.ExpectedBranches, branches)
 	}

routers/private/hook_pre_receive.go

@@ -235,7 +235,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r
 
 	globs := protectBranch.GetProtectedFilePatterns()
 	if len(globs) > 0 {
-		_, err := pull_service.CheckFileProtection(gitRepo, oldCommitID, newCommitID, globs, 1, ctx.env)
+		_, err := pull_service.CheckFileProtection(gitRepo, branchName, oldCommitID, newCommitID, globs, 1, ctx.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)
@@ -293,7 +293,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r
 			// Allow commits that only touch unprotected files
 			globs := protectBranch.GetUnprotectedFilePatterns()
 			if len(globs) > 0 {
-				unprotectedFilesOnly, err := pull_service.CheckUnprotectedFiles(gitRepo, oldCommitID, newCommitID, globs, ctx.env)
+				unprotectedFilesOnly, err := pull_service.CheckUnprotectedFiles(gitRepo, branchName, oldCommitID, newCommitID, globs, ctx.env)
 				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{

services/pull/patch.go

@@ -503,11 +503,11 @@ func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo *
 }
 
 // CheckFileProtection check file Protection
-func CheckFileProtection(repo *git.Repository, oldCommitID, newCommitID string, patterns []glob.Glob, limit int, env []string) ([]string, error) {
+func CheckFileProtection(repo *git.Repository, branchName, oldCommitID, newCommitID string, patterns []glob.Glob, limit int, env []string) ([]string, error) {
 	if len(patterns) == 0 {
 		return nil, nil
 	}
-	affectedFiles, err := git.GetAffectedFiles(repo, oldCommitID, newCommitID, env)
+	affectedFiles, err := git.GetAffectedFiles(repo, branchName, oldCommitID, newCommitID, env)
 	if err != nil {
 		return nil, err
 	}
@@ -533,11 +533,11 @@ func CheckFileProtection(repo *git.Repository, oldCommitID, newCommitID string,
 }
 
 // CheckUnprotectedFiles check if the commit only touches unprotected files
-func CheckUnprotectedFiles(repo *git.Repository, oldCommitID, newCommitID string, patterns []glob.Glob, env []string) (bool, error) {
+func CheckUnprotectedFiles(repo *git.Repository, branchName, oldCommitID, newCommitID string, patterns []glob.Glob, env []string) (bool, error) {
 	if len(patterns) == 0 {
 		return false, nil
 	}
-	affectedFiles, err := git.GetAffectedFiles(repo, oldCommitID, newCommitID, env)
+	affectedFiles, err := git.GetAffectedFiles(repo, branchName, oldCommitID, newCommitID, env)
 	if err != nil {
 		return false, err
 	}
@@ -574,7 +574,7 @@ func checkPullFilesProtection(ctx context.Context, pr *issues_model.PullRequest,
 		return nil
 	}
 
-	pr.ChangedProtectedFiles, err = CheckFileProtection(gitRepo, pr.MergeBase, "tracking", pb.GetProtectedFilePatterns(), 10, os.Environ())
+	pr.ChangedProtectedFiles, err = CheckFileProtection(gitRepo, pr.HeadBranch, pr.MergeBase, "tracking", pb.GetProtectedFilePatterns(), 10, os.Environ())
 	if err != nil && !models.IsErrFilePathProtected(err) {
 		return err
 	}