Use gitRepo as parameter instead of repopath when invoking sign functions (#36162)

Co-authored-by: Giteabot <teabot@gitea.io>

routers/web/repo/issue_view.go

@@ -495,7 +495,7 @@ func preparePullViewSigning(ctx *context.Context, issue *issues_model.Issue) {
 	pull := issue.PullRequest
 	ctx.Data["WillSign"] = false
 	if ctx.Doer != nil {
-		sign, key, _, err := asymkey_service.SignMerge(ctx, pull, ctx.Doer, pull.BaseRepo.RepoPath(), pull.BaseBranch, pull.GetGitHeadRefName())
+		sign, key, _, err := asymkey_service.SignMerge(ctx, pull, ctx.Doer, ctx.Repo.GitRepo, pull.BaseBranch, pull.GetGitHeadRefName())
 		ctx.Data["WillSign"] = sign
 		ctx.Data["SigningKeyMergeDisplay"] = asymkey_model.GetDisplaySigningKey(key)
 		if err != nil {

services/asymkey/sign.go

@@ -169,7 +169,7 @@ Loop:
 }
 
 // SignWikiCommit determines if we should sign the commits to this repository wiki
-func SignWikiCommit(ctx context.Context, repo *repo_model.Repository, u *user_model.User) (bool, *git.SigningKey, *git.Signature, error) {
+func SignWikiCommit(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, u *user_model.User) (bool, *git.SigningKey, *git.Signature, error) {
 	rules := signingModeFromStrings(setting.Repository.Signing.Wiki)
 	signingKey, sig := gitrepo.GetSigningKey(ctx)
 	if signingKey == nil {
@@ -200,11 +200,6 @@ Loop:
 				return false, nil, nil, &ErrWontSign{twofa}
 			}
 		case parentSigned:
-			gitRepo, err := gitrepo.OpenRepository(ctx, repo.WikiStorageRepo())
-			if err != nil {
-				return false, nil, nil, err
-			}
-			defer gitRepo.Close()
 			commit, err := gitRepo.GetCommit("HEAD")
 			if err != nil {
 				return false, nil, nil, err
@@ -222,7 +217,7 @@ Loop:
 }
 
 // SignCRUDAction determines if we should sign a CRUD commit to this repository
-func SignCRUDAction(ctx context.Context, u *user_model.User, tmpBasePath, parentCommit string) (bool, *git.SigningKey, *git.Signature, error) {
+func SignCRUDAction(ctx context.Context, u *user_model.User, gitRepo *git.Repository, parentCommit string) (bool, *git.SigningKey, *git.Signature, error) {
 	rules := signingModeFromStrings(setting.Repository.Signing.CRUDActions)
 	signingKey, sig := git.GetSigningKey(ctx)
 	if signingKey == nil {
@@ -253,11 +248,6 @@ Loop:
 				return false, nil, nil, &ErrWontSign{twofa}
 			}
 		case parentSigned:
-			gitRepo, err := git.OpenRepository(ctx, tmpBasePath)
-			if err != nil {
-				return false, nil, nil, err
-			}
-			defer gitRepo.Close()
 			isEmpty, err := gitRepo.IsEmpty()
 			if err != nil {
 				return false, nil, nil, err
@@ -281,7 +271,7 @@ Loop:
 }
 
 // SignMerge determines if we should sign a PR merge commit to the base repository
-func SignMerge(ctx context.Context, pr *issues_model.PullRequest, u *user_model.User, tmpBasePath, baseCommit, headCommit string) (bool, *git.SigningKey, *git.Signature, error) {
+func SignMerge(ctx context.Context, pr *issues_model.PullRequest, u *user_model.User, gitRepo *git.Repository, baseCommit, headCommit string) (bool, *git.SigningKey, *git.Signature, error) {
 	if err := pr.LoadBaseRepo(ctx); err != nil {
 		log.Error("Unable to get Base Repo for pull request")
 		return false, nil, nil, err
@@ -294,9 +284,6 @@ func SignMerge(ctx context.Context, pr *issues_model.PullRequest, u *user_model.
 	}
 	rules := signingModeFromStrings(setting.Repository.Signing.Merges)
 
-	var gitRepo *git.Repository
-	var err error
-
 Loop:
 	for _, rule := range rules {
 		switch rule {
@@ -332,13 +319,6 @@ Loop:
 				return false, nil, nil, &ErrWontSign{approved}
 			}
 		case baseSigned:
-			if gitRepo == nil {
-				gitRepo, err = git.OpenRepository(ctx, tmpBasePath)
-				if err != nil {
-					return false, nil, nil, err
-				}
-				defer gitRepo.Close()
-			}
 			commit, err := gitRepo.GetCommit(baseCommit)
 			if err != nil {
 				return false, nil, nil, err
@@ -348,13 +328,6 @@ Loop:
 				return false, nil, nil, &ErrWontSign{baseSigned}
 			}
 		case headSigned:
-			if gitRepo == nil {
-				gitRepo, err = git.OpenRepository(ctx, tmpBasePath)
-				if err != nil {
-					return false, nil, nil, err
-				}
-				defer gitRepo.Close()
-			}
 			commit, err := gitRepo.GetCommit(headCommit)
 			if err != nil {
 				return false, nil, nil, err
@@ -364,13 +337,6 @@ Loop:
 				return false, nil, nil, &ErrWontSign{headSigned}
 			}
 		case commitsSigned:
-			if gitRepo == nil {
-				gitRepo, err = git.OpenRepository(ctx, tmpBasePath)
-				if err != nil {
-					return false, nil, nil, err
-				}
-				defer gitRepo.Close()
-			}
 			commit, err := gitRepo.GetCommit(headCommit)
 			if err != nil {
 				return false, nil, nil, err

services/context/repo.go

@@ -140,7 +140,13 @@ func PrepareCommitFormOptions(ctx *Context, doer *user_model.User, targetRepo *r
 		protectionRequireSigned = protectedBranch.RequireSignedCommits
 	}
 
-	willSign, signKey, _, err := asymkey_service.SignCRUDAction(ctx, doer, targetRepo.RepoPath(), refName.String())
+	targetGitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, targetRepo)
+	if err != nil {
+		return nil, err
+	}
+	defer closer.Close()
+
+	willSign, signKey, _, err := asymkey_service.SignCRUDAction(ctx, doer, targetGitRepo, refName.String())
 	wontSignReason := ""
 	if asymkey_service.IsErrWontSign(err) {
 		wontSignReason = string(err.(*asymkey_service.ErrWontSign).Reason)

services/pull/check.go

@@ -232,7 +232,13 @@ func isSignedIfRequired(ctx context.Context, pr *issues_model.PullRequest, doer
 		return true, nil
 	}
 
-	sign, _, _, err := asymkey_service.SignMerge(ctx, pr, doer, pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitHeadRefName())
+	gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
+	if err != nil {
+		return false, err
+	}
+	defer closer.Close()
+
+	sign, _, _, err := asymkey_service.SignMerge(ctx, pr, doer, gitRepo, pr.BaseBranch, pr.GetGitHeadRefName())
 
 	return sign, err
 }

services/pull/merge_prepare.go

@@ -84,7 +84,7 @@ func createTemporaryRepoForMerge(ctx context.Context, pr *issues_model.PullReque
 		if err != nil {
 			defer cancel()
 			log.Error("failed to get sha of head branch in %-v: show-ref[%s] --hash refs/heads/tracking: %v", mergeCtx.pr, mergeCtx.tmpBasePath, err)
-			return nil, nil, fmt.Errorf("unable to get sha of head branch in %v %w", pr, err)
+			return nil, nil, fmt.Errorf("unable to get sha of head branch in pr[%d]: %w", pr.ID, err)
 		}
 		if strings.TrimSpace(trackingCommitID) != expectedHeadCommitID {
 			defer cancel()
@@ -105,8 +105,15 @@ func createTemporaryRepoForMerge(ctx context.Context, pr *issues_model.PullReque
 	mergeCtx.sig = doer.NewGitSig()
 	mergeCtx.committer = mergeCtx.sig
 
+	gitRepo, err := git.OpenRepository(ctx, mergeCtx.tmpBasePath)
+	if err != nil {
+		defer cancel()
+		return nil, nil, fmt.Errorf("failed to open temp git repo for pr[%d]: %w", mergeCtx.pr.ID, err)
+	}
+	defer gitRepo.Close()
+
 	// Determine if we should sign
-	sign, key, signer, _ := asymkey_service.SignMerge(ctx, mergeCtx.pr, mergeCtx.doer, mergeCtx.tmpBasePath, "HEAD", trackingBranch)
+	sign, key, signer, _ := asymkey_service.SignMerge(ctx, mergeCtx.pr, mergeCtx.doer, gitRepo, "HEAD", trackingBranch)
 	if sign {
 		mergeCtx.signKey = key
 		if pr.BaseRepo.GetTrustModel() == repo_model.CommitterTrustModel || pr.BaseRepo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel {

services/repository/files/cherry_pick.go

@@ -12,6 +12,7 @@ import (
 	repo_model "code.gitea.io/gitea/models/repo"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/git"
+	"code.gitea.io/gitea/modules/gitrepo"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/services/pull"
@@ -35,7 +36,13 @@ func (err ErrCommitIDDoesNotMatch) Error() string {
 
 // CherryPick cherry-picks or reverts a commit to the given repository
 func CherryPick(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, revert bool, opts *ApplyDiffPatchOptions) (*structs.FileResponse, error) {
-	if err := opts.Validate(ctx, repo, doer); err != nil {
+	gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, repo)
+	if err != nil {
+		return nil, err
+	}
+	defer closer.Close()
+
+	if err := opts.Validate(ctx, repo, gitRepo, doer); err != nil {
 		return nil, err
 	}
 	message := strings.TrimSpace(opts.Message)

services/repository/files/patch.go

@@ -13,6 +13,7 @@ import (
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/git/gitcmd"
+	"code.gitea.io/gitea/modules/gitrepo"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/util"
@@ -52,7 +53,7 @@ type ApplyDiffPatchOptions struct {
 }
 
 // Validate validates the provided options
-func (opts *ApplyDiffPatchOptions) Validate(ctx context.Context, repo *repo_model.Repository, doer *user_model.User) error {
+func (opts *ApplyDiffPatchOptions) Validate(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, doer *user_model.User) error {
 	// If no branch name is set, assume master
 	if opts.OldBranch == "" {
 		opts.OldBranch = repo.DefaultBranch
@@ -95,7 +96,7 @@ func (opts *ApplyDiffPatchOptions) Validate(ctx context.Context, repo *repo_mode
 			}
 		}
 		if protectedBranch != nil && protectedBranch.RequireSignedCommits {
-			_, _, _, err := asymkey_service.SignCRUDAction(ctx, doer, repo.RepoPath(), opts.OldBranch)
+			_, _, _, err := asymkey_service.SignCRUDAction(ctx, doer, gitRepo, opts.OldBranch)
 			if err != nil {
 				if !asymkey_service.IsErrWontSign(err) {
 					return err
@@ -116,7 +117,13 @@ func ApplyDiffPatch(ctx context.Context, repo *repo_model.Repository, doer *user
 		return nil, err
 	}
 
-	if err := opts.Validate(ctx, repo, doer); err != nil {
+	gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, repo)
+	if err != nil {
+		return nil, err
+	}
+	defer closer.Close()
+
+	if err := opts.Validate(ctx, repo, gitRepo, doer); err != nil {
 		return nil, err
 	}
 

services/repository/files/temp_repo.go

@@ -303,7 +303,7 @@ func (t *TemporaryUploadRepository) CommitTree(ctx context.Context, opts *Commit
 	var key *git.SigningKey
 	var signer *git.Signature
 	if opts.ParentCommitID != "" {
-		sign, key, signer, _ = asymkey_service.SignCRUDAction(ctx, opts.DoerUser, t.basePath, opts.ParentCommitID)
+		sign, key, signer, _ = asymkey_service.SignCRUDAction(ctx, opts.DoerUser, t.gitRepo, opts.ParentCommitID)
 	} else {
 		sign, key, signer, _ = asymkey_service.SignInitialCommit(ctx, opts.DoerUser)
 	}

services/repository/files/update.go

@@ -167,7 +167,7 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use
 				}
 			}
 		}
-	} else if err := VerifyBranchProtection(ctx, repo, doer, opts.OldBranch, treePaths); err != nil {
+	} else if err := VerifyBranchProtection(ctx, repo, gitRepo, doer, opts.OldBranch, treePaths); err != nil {
 		return nil, err
 	}
 
@@ -659,7 +659,7 @@ func writeRepoObjectForRename(ctx context.Context, t *TemporaryUploadRepository,
 }
 
 // VerifyBranchProtection verify the branch protection for modifying the given treePath on the given branch
-func VerifyBranchProtection(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, branchName string, treePaths []string) error {
+func VerifyBranchProtection(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, doer *user_model.User, branchName string, treePaths []string) error {
 	protectedBranch, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repo.ID, branchName)
 	if err != nil {
 		return err
@@ -686,7 +686,7 @@ func VerifyBranchProtection(ctx context.Context, repo *repo_model.Repository, do
 			}
 		}
 		if protectedBranch.RequireSignedCommits {
-			_, _, _, err := asymkey_service.SignCRUDAction(ctx, doer, repo.RepoPath(), branchName)
+			_, _, _, err := asymkey_service.SignCRUDAction(ctx, doer, gitRepo, branchName)
 			if err != nil {
 				if !asymkey_service.IsErrWontSign(err) {
 					return err

services/wiki/wiki.go

@@ -193,7 +193,13 @@ func updateWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model
 
 	committer := doer.NewGitSig()
 
-	sign, signingKey, signer, _ := asymkey_service.SignWikiCommit(ctx, repo, doer)
+	originalGitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, repo.WikiStorageRepo())
+	if err != nil {
+		return fmt.Errorf("unable to open wiki repository: %w", err)
+	}
+	defer closer.Close()
+
+	sign, signingKey, signer, _ := asymkey_service.SignWikiCommit(ctx, repo, originalGitRepo, doer)
 	if sign {
 		commitTreeOpts.Key = signingKey
 		if repo.GetTrustModel() == repo_model.CommitterTrustModel || repo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel {
@@ -212,7 +218,7 @@ func updateWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model
 		return err
 	}
 
-	if err := gitrepo.PushFromLocal(gitRepo.Ctx, basePath, repo.WikiStorageRepo(), git.PushOptions{
+	if err := gitrepo.PushFromLocal(ctx, basePath, repo.WikiStorageRepo(), git.PushOptions{
 		Branch: fmt.Sprintf("%s:%s%s", commitHash.String(), git.BranchPrefix, repo.DefaultWikiBranch),
 		Env: repo_module.FullPushingEnvironment(
 			doer,
@@ -315,7 +321,13 @@ func DeleteWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model
 
 	committer := doer.NewGitSig()
 
-	sign, signingKey, signer, _ := asymkey_service.SignWikiCommit(ctx, repo, doer)
+	originalGitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, repo.WikiStorageRepo())
+	if err != nil {
+		return fmt.Errorf("unable to open wiki repository: %w", err)
+	}
+	defer closer.Close()
+
+	sign, signingKey, signer, _ := asymkey_service.SignWikiCommit(ctx, repo, originalGitRepo, doer)
 	if sign {
 		commitTreeOpts.Key = signingKey
 		if repo.GetTrustModel() == repo_model.CommitterTrustModel || repo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel {