diff --git a/services/pull/merge.go b/services/pull/merge.go index 4925302797..1137296cae 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -436,20 +436,24 @@ func doMergeAndPush(ctx context.Context, pr *issues_model.PullRequest, doer *use func commitAndSignNoAuthor(ctx *mergeContext, message string) error { cmdCommit := gitcmd.NewCommand("commit").AddOptionFormat("--message=%s", message) - if ctx.signKey == nil { - cmdCommit.AddArguments("--no-gpg-sign") - } else { - if ctx.signKey.Format != "" { - cmdCommit.AddConfig("gpg.format", ctx.signKey.Format) - } - cmdCommit.AddOptionFormat("-S%s", ctx.signKey.KeyID) - } + addCommitSigningOptions(cmdCommit, ctx.signKey) if err := ctx.PrepareGitCmd(cmdCommit).RunWithStderr(ctx); err != nil { return fmt.Errorf("git commit %v: %w\n%s", ctx.pr, err, ctx.outbuf.String()) } return nil } +func addCommitSigningOptions(cmd *gitcmd.Command, signKey *git.SigningKey) { + if signKey == nil { + cmd.AddArguments("--no-gpg-sign") + return + } + if signKey.Format != "" { + cmd.AddConfig("gpg.format", signKey.Format) + } + cmd.AddOptionFormat("--gpg-sign=%s", signKey.KeyID) +} + // ErrMergeConflicts represents an error if merging fails with a conflict type ErrMergeConflicts struct { Style repo_model.MergeStyle diff --git a/services/pull/merge_prepare.go b/services/pull/merge_prepare.go index 1131a23d85..c23bfdc7f6 100644 --- a/services/pull/merge_prepare.go +++ b/services/pull/merge_prepare.go @@ -260,7 +260,9 @@ func rebaseTrackingOnToBase(ctx *mergeContext, mergeStyle repo_model.MergeStyle) ctx.outbuf.Reset() // Rebase before merging - if err := ctx.PrepareGitCmd(gitcmd.NewCommand("rebase").AddDynamicArguments(tmpRepoBaseBranch)). + cmdRebase := gitcmd.NewCommand("rebase").AddDynamicArguments(tmpRepoBaseBranch) + addCommitSigningOptions(cmdRebase, ctx.signKey) + if err := ctx.PrepareGitCmd(cmdRebase). RunWithStderr(ctx); err != nil { // Rebase will leave a REBASE_HEAD file in .git if there is a conflict if _, statErr := os.Stat(filepath.Join(ctx.tmpBasePath, ".git", "REBASE_HEAD")); statErr == nil { diff --git a/services/pull/merge_rebase.go b/services/pull/merge_rebase.go index 9dbe67a6c6..0224f7f1bd 100644 --- a/services/pull/merge_rebase.go +++ b/services/pull/merge_rebase.go @@ -74,10 +74,10 @@ func doMergeRebaseFastForward(ctx *mergeContext) error { } if newMessage != "" { - if err := gitcmd.NewCommand("commit", "--amend"). - AddOptionFormat("--message=%s", newMessage). - WithDir(ctx.tmpBasePath). - Run(ctx); err != nil { + cmdCommit := gitcmd.NewCommand("commit", "--amend"). + AddOptionFormat("--message=%s", newMessage) + addCommitSigningOptions(cmdCommit, ctx.signKey) + if err := cmdCommit.WithDir(ctx.tmpBasePath).Run(ctx); err != nil { log.Error("Unable to amend commit message: %v", err) return err } diff --git a/services/pull/merge_squash.go b/services/pull/merge_squash.go index 6c101c8e89..0f716f4d77 100644 --- a/services/pull/merge_squash.go +++ b/services/pull/merge_squash.go @@ -73,14 +73,7 @@ func doMergeStyleSquash(ctx *mergeContext, message string) error { AddOptionFormat("--author='%s <%s>'", sig.Name, sig.Email). AddOptionFormat("--message=%s", message). AddArguments("--allow-empty") - if ctx.signKey == nil { - cmdCommit.AddArguments("--no-gpg-sign") - } else { - if ctx.signKey.Format != "" { - cmdCommit.AddConfig("gpg.format", ctx.signKey.Format) - } - cmdCommit.AddOptionFormat("-S%s", ctx.signKey.KeyID) - } + addCommitSigningOptions(cmdCommit, ctx.signKey) if err := ctx.PrepareGitCmd(cmdCommit).RunWithStderr(ctx); err != nil { log.Error("git commit %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), err.Stderr()) return fmt.Errorf("git commit [%s:%s -> %s:%s]: %w\n%s\n%s", ctx.pr.HeadRepo.FullName(), ctx.pr.HeadBranch, ctx.pr.BaseRepo.FullName(), ctx.pr.BaseBranch, err, ctx.outbuf.String(), err.Stderr()) diff --git a/tests/integration/gpg_ssh_git_test.go b/tests/integration/gpg_ssh_git_test.go index 56f9f87783..a95ff77dab 100644 --- a/tests/integration/gpg_ssh_git_test.go +++ b/tests/integration/gpg_ssh_git_test.go @@ -9,6 +9,7 @@ import ( "encoding/base64" "encoding/pem" "fmt" + "net/http" "net/url" "os" "testing" @@ -255,6 +256,52 @@ func testGitSigning(t *testing.T) { assert.True(t, branch.Commit.Verification.Verified) })) }) + + setting.Repository.Signing.Merges = []string{"commitssigned"} + setting.Repository.Signing.CRUDActions = []string{"always"} + t.Run("UpdateRebaseSigned", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + testCtx := NewAPITestContext(t, username, "update-rebase-signed", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) + t.Run("CreateRepository", doAPICreateRepository(testCtx, false)) + + var repoID int64 + t.Run("GetRepository", doAPIGetRepository(testCtx, func(t *testing.T, repo api.Repository) { + repoID = repo.ID + })) + enableRepoAllowUpdateWithRebase(t, repoID, true) + + t.Run("CreateFeatureCommit", crudActionCreateFile( + t, testCtx, user, "master", "feature", "signed-feature.txt")) + pr, err := doAPICreatePullRequest(testCtx, testCtx.Username, testCtx.Reponame, "master", "feature")(t) + require.NoError(t, err) + + content := base64.StdEncoding.EncodeToString([]byte("update base")) + t.Run("UpdateBase", doAPICreateFile(testCtx, "signed-base.txt", &api.CreateFileOptions{ + FileOptions: api.FileOptions{ + BranchName: "master", + Message: "update base", + Author: api.Identity{ + Name: user.FullName, + Email: user.Email, + }, + Committer: api.Identity{ + Name: user.FullName, + Email: user.Email, + }, + }, + ContentBase64: content, + })) + + req := NewRequestf(t, "POST", "/api/v1/repos/%s/%s/pulls/%d/update?style=rebase", testCtx.Username, testCtx.Reponame, pr.Index). + AddTokenAuth(testCtx.Token) + testCtx.Session.MakeRequest(t, req, http.StatusOK) + + t.Run("CheckFeatureBranchSigned", doAPIGetBranch(testCtx, "feature", func(t *testing.T, branch api.Branch) { + require.NotNil(t, branch.Commit) + require.NotNil(t, branch.Commit.Verification) + assert.True(t, branch.Commit.Verification.Verified) + })) + }) }) }