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 f5e025917f)
This commit is contained in:
Gusted 2024-10-24 18:40:14 +02:00 committed by forgejo-backport-action
parent 96f0c76648
commit 1f62fe8ae0
3 changed files with 35 additions and 0 deletions

View file

@ -272,6 +272,17 @@ func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLi
// GetAffectedFiles returns the affected files between two commits // GetAffectedFiles returns the affected files between two commits
func GetAffectedFiles(repo *Repository, oldCommitID, newCommitID string, env []string) ([]string, error) { 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() stdoutReader, stdoutWriter, err := os.Pipe()
if err != nil { if err != nil {
log.Error("Unable to create os.Pipe for %s", repo.Path) log.Error("Unable to create os.Pipe for %s", repo.Path)

View file

@ -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`, `:SSHLog() [E] ssh: Not allowed to push to protected branch protected. HookPreReceive(last) failed: internal API error response, status=403`,
// TestGit/HTTP/BranchProtectMerge // TestGit/HTTP/BranchProtectMerge
`:SSHLog() [E] ssh: branch protected is protected from force push. HookPreReceive(last) failed: internal API error response, status=403`, `: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 // TestGit/HTTP/MergeFork/CreatePRAndMerge
`:DeleteBranchPost() [E] DeleteBranch: GetBranch: branch does not exist [repo_id: 1099 name: user2:master]`, // sqlite `: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 "s/web/repo/branch.go:108:DeleteBranchPost() [E] DeleteBranch: GetBranch: branch does not exist [repo_id: 10000 name: user2:master]", // mysql

View file

@ -369,6 +369,28 @@ func doBranchProtect(baseCtx *APITestContext, dstPath string) func(t *testing.T)
ctx := NewAPITestContext(t, baseCtx.Username, baseCtx.Reponame, auth_model.AccessTokenScopeWriteRepository) 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("FailToPushToProtectedBranch", func(t *testing.T) {
t.Run("ProtectProtectedBranch", doProtectBranch(ctx, "protected")) t.Run("ProtectProtectedBranch", doProtectBranch(ctx, "protected"))
t.Run("Create modified-protected-branch", doGitCheckoutBranch(dstPath, "-b", "modified-protected-branch", "protected")) t.Run("Create modified-protected-branch", doGitCheckoutBranch(dstPath, "-b", "modified-protected-branch", "protected"))