From 1f62fe8ae0563a09f9224fbafecac82bf04175d5 Mon Sep 17 00:00:00 2001 From: Gusted Date: Thu, 24 Oct 2024 18:40:14 +0200 Subject: [PATCH] fix: make branch protection work for new branches - If `GetAffectedFiles` is called for a push with an empty oldCommitID, then set the oldCommitID to the empty tree. This will effictively diff all the changes included in the push, which is the expected behavior for branches. - Integration test added. - Resolves #5683 - Port of gitea#31778 but implemented differently. (cherry picked from commit f5e025917f55122f9e0e748105d519e3cd53a2be) --- modules/git/diff.go | 11 +++++++++++ modules/testlogger/testlogger.go | 2 ++ tests/integration/git_test.go | 22 ++++++++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/modules/git/diff.go b/modules/git/diff.go index 10ef3d83fb..d9f3f6dda9 100644 --- a/modules/git/diff.go +++ b/modules/git/diff.go @@ -272,6 +272,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) { + objectFormat, err := repo.GetObjectFormat() + if err != nil { + return nil, err + } + + // If the oldCommitID is empty, then we must assume its a new branch, so diff + // against the empty tree. So all changes of this new branch are included. + if oldCommitID == objectFormat.EmptyObjectID().String() { + oldCommitID = objectFormat.EmptyTree().String() + } + stdoutReader, stdoutWriter, err := os.Pipe() if err != nil { log.Error("Unable to create os.Pipe for %s", repo.Path) diff --git a/modules/testlogger/testlogger.go b/modules/testlogger/testlogger.go index 95cbb86591..7897cc6b07 100644 --- a/modules/testlogger/testlogger.go +++ b/modules/testlogger/testlogger.go @@ -131,6 +131,8 @@ var ignoredErrorMessage = []string{ `:SSHLog() [E] ssh: Not allowed to push to protected branch protected. HookPreReceive(last) failed: internal API error response, status=403`, // TestGit/HTTP/BranchProtectMerge `:SSHLog() [E] ssh: branch protected is protected from force push. HookPreReceive(last) failed: internal API error response, status=403`, + // TestGit/HTTP/BranchProtect + `:SSHLog() [E] ssh: branch before-create-2 is protected from changing file protected-file-data-`, // TestGit/HTTP/MergeFork/CreatePRAndMerge `:DeleteBranchPost() [E] DeleteBranch: GetBranch: branch does not exist [repo_id: 1099 name: user2:master]`, // sqlite "s/web/repo/branch.go:108:DeleteBranchPost() [E] DeleteBranch: GetBranch: branch does not exist [repo_id: 10000 name: user2:master]", // mysql diff --git a/tests/integration/git_test.go b/tests/integration/git_test.go index 2c46fdde68..5e03d2887d 100644 --- a/tests/integration/git_test.go +++ b/tests/integration/git_test.go @@ -369,6 +369,28 @@ func doBranchProtect(baseCtx *APITestContext, dstPath string) func(t *testing.T) ctx := NewAPITestContext(t, baseCtx.Username, baseCtx.Reponame, auth_model.AccessTokenScopeWriteRepository) + t.Run("PushToNewProtectedBranch", func(t *testing.T) { + t.Run("CreateBranchProtected", doGitCreateBranch(dstPath, "before-create-1")) + t.Run("ProtectProtectedBranch", doProtectBranch(ctx, "before-create-1", parameterProtectBranch{ + "enable_push": "all", + "apply_to_admins": "on", + })) + t.Run("PushProtectedBranch", doGitPushTestRepository(dstPath, "origin", "before-create-1")) + + t.Run("GenerateCommit", func(t *testing.T) { + _, err := generateCommitWithNewData(littleSize, dstPath, "user2@example.com", "User Two", "protected-file-data-") + require.NoError(t, err) + }) + + t.Run("ProtectProtectedBranch", doProtectBranch(ctx, "before-create-2", parameterProtectBranch{ + "enable_push": "all", + "protected_file_patterns": "protected-file-data-*", + "apply_to_admins": "on", + })) + + doGitPushTestRepositoryFail(dstPath, "origin", "HEAD:before-create-2")(t) + }) + t.Run("FailToPushToProtectedBranch", func(t *testing.T) { t.Run("ProtectProtectedBranch", doProtectBranch(ctx, "protected")) t.Run("Create modified-protected-branch", doGitCheckoutBranch(dstPath, "-b", "modified-protected-branch", "protected"))