Skip to content

Commit 3e56617

Browse files
lunnyGiteaBot
lunny
and
GiteaBot
authored
Use gitRepo as parameter instead of repopath when invoking sign functions (#36162)
Co-authored-by: Giteabot <[email protected]>
1 parent efd5dd4 commit 3e56617

File tree

10 files changed

+64
-53
lines changed

10 files changed

+64
-53
lines changed

‎routers/web/repo/issue_view.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@ func preparePullViewSigning(ctx *context.Context, issue *issues_model.Issue) {
495495
pull := issue.PullRequest
496496
ctx.Data["WillSign"] = false
497497
if ctx.Doer != nil {
498-
sign, key, _, err := asymkey_service.SignMerge(ctx, pull, ctx.Doer, pull.BaseRepo.RepoPath(), pull.BaseBranch, pull.GetGitHeadRefName())
498+
sign, key, _, err := asymkey_service.SignMerge(ctx, pull, ctx.Doer, ctx.Repo.GitRepo, pull.BaseBranch, pull.GetGitHeadRefName())
499499
ctx.Data["WillSign"] = sign
500500
ctx.Data["SigningKeyMergeDisplay"] = asymkey_model.GetDisplaySigningKey(key)
501501
if err != nil {

‎services/asymkey/sign.go‎

Lines changed: 3 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ Loop:
169169
}
170170

171171
// SignWikiCommit determines if we should sign the commits to this repository wiki
172-
func SignWikiCommit(ctx context.Context, repo *repo_model.Repository, u *user_model.User) (bool, *git.SigningKey, *git.Signature, error) {
172+
func SignWikiCommit(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, u *user_model.User) (bool, *git.SigningKey, *git.Signature, error) {
173173
rules := signingModeFromStrings(setting.Repository.Signing.Wiki)
174174
signingKey, sig := gitrepo.GetSigningKey(ctx)
175175
if signingKey == nil {
@@ -200,11 +200,6 @@ Loop:
200200
return false, nil, nil, &ErrWontSign{twofa}
201201
}
202202
case parentSigned:
203-
gitRepo, err := gitrepo.OpenRepository(ctx, repo.WikiStorageRepo())
204-
if err != nil {
205-
return false, nil, nil, err
206-
}
207-
defer gitRepo.Close()
208203
commit, err := gitRepo.GetCommit("HEAD")
209204
if err != nil {
210205
return false, nil, nil, err
@@ -222,7 +217,7 @@ Loop:
222217
}
223218

224219
// SignCRUDAction determines if we should sign a CRUD commit to this repository
225-
func SignCRUDAction(ctx context.Context, u *user_model.User, tmpBasePath, parentCommit string) (bool, *git.SigningKey, *git.Signature, error) {
220+
func SignCRUDAction(ctx context.Context, u *user_model.User, gitRepo *git.Repository, parentCommit string) (bool, *git.SigningKey, *git.Signature, error) {
226221
rules := signingModeFromStrings(setting.Repository.Signing.CRUDActions)
227222
signingKey, sig := git.GetSigningKey(ctx)
228223
if signingKey == nil {
@@ -253,11 +248,6 @@ Loop:
253248
return false, nil, nil, &ErrWontSign{twofa}
254249
}
255250
case parentSigned:
256-
gitRepo, err := git.OpenRepository(ctx, tmpBasePath)
257-
if err != nil {
258-
return false, nil, nil, err
259-
}
260-
defer gitRepo.Close()
261251
isEmpty, err := gitRepo.IsEmpty()
262252
if err != nil {
263253
return false, nil, nil, err
@@ -281,7 +271,7 @@ Loop:
281271
}
282272

283273
// SignMerge determines if we should sign a PR merge commit to the base repository
284-
func SignMerge(ctx context.Context, pr *issues_model.PullRequest, u *user_model.User, tmpBasePath, baseCommit, headCommit string) (bool, *git.SigningKey, *git.Signature, error) {
274+
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) {
285275
if err := pr.LoadBaseRepo(ctx); err != nil {
286276
log.Error("Unable to get Base Repo for pull request")
287277
return false, nil, nil, err
@@ -294,9 +284,6 @@ func SignMerge(ctx context.Context, pr *issues_model.PullRequest, u *user_model.
294284
}
295285
rules := signingModeFromStrings(setting.Repository.Signing.Merges)
296286

297-
var gitRepo *git.Repository
298-
var err error
299-
300287
Loop:
301288
for _, rule := range rules {
302289
switch rule {
@@ -332,13 +319,6 @@ Loop:
332319
return false, nil, nil, &ErrWontSign{approved}
333320
}
334321
case baseSigned:
335-
if gitRepo == nil {
336-
gitRepo, err = git.OpenRepository(ctx, tmpBasePath)
337-
if err != nil {
338-
return false, nil, nil, err
339-
}
340-
defer gitRepo.Close()
341-
}
342322
commit, err := gitRepo.GetCommit(baseCommit)
343323
if err != nil {
344324
return false, nil, nil, err
@@ -348,13 +328,6 @@ Loop:
348328
return false, nil, nil, &ErrWontSign{baseSigned}
349329
}
350330
case headSigned:
351-
if gitRepo == nil {
352-
gitRepo, err = git.OpenRepository(ctx, tmpBasePath)
353-
if err != nil {
354-
return false, nil, nil, err
355-
}
356-
defer gitRepo.Close()
357-
}
358331
commit, err := gitRepo.GetCommit(headCommit)
359332
if err != nil {
360333
return false, nil, nil, err
@@ -364,13 +337,6 @@ Loop:
364337
return false, nil, nil, &ErrWontSign{headSigned}
365338
}
366339
case commitsSigned:
367-
if gitRepo == nil {
368-
gitRepo, err = git.OpenRepository(ctx, tmpBasePath)
369-
if err != nil {
370-
return false, nil, nil, err
371-
}
372-
defer gitRepo.Close()
373-
}
374340
commit, err := gitRepo.GetCommit(headCommit)
375341
if err != nil {
376342
return false, nil, nil, err

‎services/context/repo.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,13 @@ func PrepareCommitFormOptions(ctx *Context, doer *user_model.User, targetRepo *r
140140
protectionRequireSigned = protectedBranch.RequireSignedCommits
141141
}
142142

143-
willSign, signKey, _, err := asymkey_service.SignCRUDAction(ctx, doer, targetRepo.RepoPath(), refName.String())
143+
targetGitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, targetRepo)
144+
if err != nil {
145+
return nil, err
146+
}
147+
defer closer.Close()
148+
149+
willSign, signKey, _, err := asymkey_service.SignCRUDAction(ctx, doer, targetGitRepo, refName.String())
144150
wontSignReason := ""
145151
if asymkey_service.IsErrWontSign(err) {
146152
wontSignReason = string(err.(*asymkey_service.ErrWontSign).Reason)

‎services/pull/check.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,13 @@ func isSignedIfRequired(ctx context.Context, pr *issues_model.PullRequest, doer
232232
return true, nil
233233
}
234234

235-
sign, _, _, err := asymkey_service.SignMerge(ctx, pr, doer, pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitHeadRefName())
235+
gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
236+
if err != nil {
237+
return false, err
238+
}
239+
defer closer.Close()
240+
241+
sign, _, _, err := asymkey_service.SignMerge(ctx, pr, doer, gitRepo, pr.BaseBranch, pr.GetGitHeadRefName())
236242

237243
return sign, err
238244
}

‎services/pull/merge_prepare.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func createTemporaryRepoForMerge(ctx context.Context, pr *issues_model.PullReque
8484
if err != nil {
8585
defer cancel()
8686
log.Error("failed to get sha of head branch in %-v: show-ref[%s] --hash refs/heads/tracking: %v", mergeCtx.pr, mergeCtx.tmpBasePath, err)
87-
return nil, nil, fmt.Errorf("unable to get sha of head branch in %v %w", pr, err)
87+
return nil, nil, fmt.Errorf("unable to get sha of head branch in pr[%d]: %w", pr.ID, err)
8888
}
8989
if strings.TrimSpace(trackingCommitID) != expectedHeadCommitID {
9090
defer cancel()
@@ -105,8 +105,15 @@ func createTemporaryRepoForMerge(ctx context.Context, pr *issues_model.PullReque
105105
mergeCtx.sig = doer.NewGitSig()
106106
mergeCtx.committer = mergeCtx.sig
107107

108+
gitRepo, err := git.OpenRepository(ctx, mergeCtx.tmpBasePath)
109+
if err != nil {
110+
defer cancel()
111+
return nil, nil, fmt.Errorf("failed to open temp git repo for pr[%d]: %w", mergeCtx.pr.ID, err)
112+
}
113+
defer gitRepo.Close()
114+
108115
// Determine if we should sign
109-
sign, key, signer, _ := asymkey_service.SignMerge(ctx, mergeCtx.pr, mergeCtx.doer, mergeCtx.tmpBasePath, "HEAD", trackingBranch)
116+
sign, key, signer, _ := asymkey_service.SignMerge(ctx, mergeCtx.pr, mergeCtx.doer, gitRepo, "HEAD", trackingBranch)
110117
if sign {
111118
mergeCtx.signKey = key
112119
if pr.BaseRepo.GetTrustModel() == repo_model.CommitterTrustModel || pr.BaseRepo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel {

‎services/repository/files/cherry_pick.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
repo_model "code.gitea.io/gitea/models/repo"
1313
user_model "code.gitea.io/gitea/models/user"
1414
"code.gitea.io/gitea/modules/git"
15+
"code.gitea.io/gitea/modules/gitrepo"
1516
"code.gitea.io/gitea/modules/log"
1617
"code.gitea.io/gitea/modules/structs"
1718
"code.gitea.io/gitea/services/pull"
@@ -35,7 +36,13 @@ func (err ErrCommitIDDoesNotMatch) Error() string {
3536

3637
// CherryPick cherry-picks or reverts a commit to the given repository
3738
func CherryPick(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, revert bool, opts *ApplyDiffPatchOptions) (*structs.FileResponse, error) {
38-
if err := opts.Validate(ctx, repo, doer); err != nil {
39+
gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, repo)
40+
if err != nil {
41+
return nil, err
42+
}
43+
defer closer.Close()
44+
45+
if err := opts.Validate(ctx, repo, gitRepo, doer); err != nil {
3946
return nil, err
4047
}
4148
message := strings.TrimSpace(opts.Message)

‎services/repository/files/patch.go‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
user_model "code.gitea.io/gitea/models/user"
1414
"code.gitea.io/gitea/modules/git"
1515
"code.gitea.io/gitea/modules/git/gitcmd"
16+
"code.gitea.io/gitea/modules/gitrepo"
1617
"code.gitea.io/gitea/modules/log"
1718
"code.gitea.io/gitea/modules/structs"
1819
"code.gitea.io/gitea/modules/util"
@@ -52,7 +53,7 @@ type ApplyDiffPatchOptions struct {
5253
}
5354

5455
// Validate validates the provided options
55-
func (opts *ApplyDiffPatchOptions) Validate(ctx context.Context, repo *repo_model.Repository, doer *user_model.User) error {
56+
func (opts *ApplyDiffPatchOptions) Validate(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, doer *user_model.User) error {
5657
// If no branch name is set, assume master
5758
if opts.OldBranch == "" {
5859
opts.OldBranch = repo.DefaultBranch
@@ -95,7 +96,7 @@ func (opts *ApplyDiffPatchOptions) Validate(ctx context.Context, repo *repo_mode
9596
}
9697
}
9798
if protectedBranch != nil && protectedBranch.RequireSignedCommits {
98-
_, _, _, err := asymkey_service.SignCRUDAction(ctx, doer, repo.RepoPath(), opts.OldBranch)
99+
_, _, _, err := asymkey_service.SignCRUDAction(ctx, doer, gitRepo, opts.OldBranch)
99100
if err != nil {
100101
if !asymkey_service.IsErrWontSign(err) {
101102
return err
@@ -116,7 +117,13 @@ func ApplyDiffPatch(ctx context.Context, repo *repo_model.Repository, doer *user
116117
return nil, err
117118
}
118119

119-
if err := opts.Validate(ctx, repo, doer); err != nil {
120+
gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, repo)
121+
if err != nil {
122+
return nil, err
123+
}
124+
defer closer.Close()
125+
126+
if err := opts.Validate(ctx, repo, gitRepo, doer); err != nil {
120127
return nil, err
121128
}
122129

‎services/repository/files/temp_repo.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ func (t *TemporaryUploadRepository) CommitTree(ctx context.Context, opts *Commit
303303
var key *git.SigningKey
304304
var signer *git.Signature
305305
if opts.ParentCommitID != "" {
306-
sign, key, signer, _ = asymkey_service.SignCRUDAction(ctx, opts.DoerUser, t.basePath, opts.ParentCommitID)
306+
sign, key, signer, _ = asymkey_service.SignCRUDAction(ctx, opts.DoerUser, t.gitRepo, opts.ParentCommitID)
307307
} else {
308308
sign, key, signer, _ = asymkey_service.SignInitialCommit(ctx, opts.DoerUser)
309309
}

‎services/repository/files/update.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use
167167
}
168168
}
169169
}
170-
} else if err := VerifyBranchProtection(ctx, repo, doer, opts.OldBranch, treePaths); err != nil {
170+
} else if err := VerifyBranchProtection(ctx, repo, gitRepo, doer, opts.OldBranch, treePaths); err != nil {
171171
return nil, err
172172
}
173173

@@ -659,7 +659,7 @@ func writeRepoObjectForRename(ctx context.Context, t *TemporaryUploadRepository,
659659
}
660660

661661
// VerifyBranchProtection verify the branch protection for modifying the given treePath on the given branch
662-
func VerifyBranchProtection(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, branchName string, treePaths []string) error {
662+
func VerifyBranchProtection(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, doer *user_model.User, branchName string, treePaths []string) error {
663663
protectedBranch, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repo.ID, branchName)
664664
if err != nil {
665665
return err
@@ -686,7 +686,7 @@ func VerifyBranchProtection(ctx context.Context, repo *repo_model.Repository, do
686686
}
687687
}
688688
if protectedBranch.RequireSignedCommits {
689-
_, _, _, err := asymkey_service.SignCRUDAction(ctx, doer, repo.RepoPath(), branchName)
689+
_, _, _, err := asymkey_service.SignCRUDAction(ctx, doer, gitRepo, branchName)
690690
if err != nil {
691691
if !asymkey_service.IsErrWontSign(err) {
692692
return err

‎services/wiki/wiki.go‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,13 @@ func updateWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model
193193

194194
committer := doer.NewGitSig()
195195

196-
sign, signingKey, signer, _ := asymkey_service.SignWikiCommit(ctx, repo, doer)
196+
originalGitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, repo.WikiStorageRepo())
197+
if err != nil {
198+
return fmt.Errorf("unable to open wiki repository: %w", err)
199+
}
200+
defer closer.Close()
201+
202+
sign, signingKey, signer, _ := asymkey_service.SignWikiCommit(ctx, repo, originalGitRepo, doer)
197203
if sign {
198204
commitTreeOpts.Key = signingKey
199205
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
212218
return err
213219
}
214220

215-
if err := gitrepo.PushFromLocal(gitRepo.Ctx, basePath, repo.WikiStorageRepo(), git.PushOptions{
221+
if err := gitrepo.PushFromLocal(ctx, basePath, repo.WikiStorageRepo(), git.PushOptions{
216222
Branch: fmt.Sprintf("%s:%s%s", commitHash.String(), git.BranchPrefix, repo.DefaultWikiBranch),
217223
Env: repo_module.FullPushingEnvironment(
218224
doer,
@@ -315,7 +321,13 @@ func DeleteWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model
315321

316322
committer := doer.NewGitSig()
317323

318-
sign, signingKey, signer, _ := asymkey_service.SignWikiCommit(ctx, repo, doer)
324+
originalGitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, repo.WikiStorageRepo())
325+
if err != nil {
326+
return fmt.Errorf("unable to open wiki repository: %w", err)
327+
}
328+
defer closer.Close()
329+
330+
sign, signingKey, signer, _ := asymkey_service.SignWikiCommit(ctx, repo, originalGitRepo, doer)
319331
if sign {
320332
commitTreeOpts.Key = signingKey
321333
if repo.GetTrustModel() == repo_model.CommitterTrustModel || repo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel {

0 commit comments

Comments
 (0)