Skip to content

Commit 3abe17f

Browse files
authored
Sign protected branches (#8993)
* Move SignMerge to PullRequest * Add approved signing mode * As per @guillep2k comment
1 parent e3c3b33 commit 3abe17f

File tree

6 files changed

+129
-97
lines changed

6 files changed

+129
-97
lines changed

custom/conf/app.ini.sample

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ WIKI = never
110110
; Determines when to sign on merges
111111
; - basesigned: require that the parent of commit on the base repo is signed.
112112
; - commitssigned: require that all the commits in the head branch are signed.
113+
; - approved: only sign when merging an approved pr to a protected branch
113114
MERGES = pubkey, twofa, basesigned, commitssigned
114115

115116
[cors]

docs/content/doc/advanced/config-cheat-sheet.en-us.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,8 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`.
9696
- `CRUD_ACTIONS`: **pubkey, twofa, parentsigned**: \[never, pubkey, twofa, parentsigned, always\]: Sign CRUD actions.
9797
- Options as above, with the addition of:
9898
- `parentsigned`: Only sign if the parent commit is signed.
99-
- `MERGES`: **pubkey, twofa, basesigned, commitssigned**: \[never, pubkey, twofa, basesigned, commitssigned, always\]: Sign merges.
99+
- `MERGES`: **pubkey, twofa, basesigned, commitssigned**: \[never, pubkey, twofa, approved, basesigned, commitssigned, always\]: Sign merges.
100+
- `approved`: Only sign approved merges to a protected branch.
100101
- `basesigned`: Only sign if the parent commit in the base repo is signed.
101102
- `headsigned`: Only sign if the head commit in the head branch is signed.
102103
- `commitssigned`: Only sign if all the commits in the head branch to the merge point are signed.

docs/content/doc/advanced/signing.en-us.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ The possible options are:
136136
* `basesigned`: Only sign if the parent commit in the base repo is signed.
137137
* `headsigned`: Only sign if the head commit in the head branch is signed.
138138
* `commitssigned`: Only sign if all the commits in the head branch to the merge point are signed.
139+
* `approved`: Only sign approved merges to a protected branch.
139140
* `always`: Always sign
140141

141142
Options other than `never` and `always` can be combined as a comma

models/pull_sign.go

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
// Copyright 2019 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package models
6+
7+
import (
8+
"code.gitea.io/gitea/modules/git"
9+
"code.gitea.io/gitea/modules/log"
10+
"code.gitea.io/gitea/modules/setting"
11+
)
12+
13+
// SignMerge determines if we should sign a PR merge commit to the base repository
14+
func (pr *PullRequest) SignMerge(u *User, tmpBasePath, baseCommit, headCommit string) (bool, string) {
15+
if err := pr.GetBaseRepo(); err != nil {
16+
log.Error("Unable to get Base Repo for pull request")
17+
return false, ""
18+
}
19+
repo := pr.BaseRepo
20+
21+
signingKey := signingKey(repo.RepoPath())
22+
if signingKey == "" {
23+
return false, ""
24+
}
25+
rules := signingModeFromStrings(setting.Repository.Signing.Merges)
26+
27+
var gitRepo *git.Repository
28+
var err error
29+
30+
for _, rule := range rules {
31+
switch rule {
32+
case never:
33+
return false, ""
34+
case always:
35+
break
36+
case pubkey:
37+
keys, err := ListGPGKeys(u.ID)
38+
if err != nil || len(keys) == 0 {
39+
return false, ""
40+
}
41+
case twofa:
42+
twofa, err := GetTwoFactorByUID(u.ID)
43+
if err != nil || twofa == nil {
44+
return false, ""
45+
}
46+
case approved:
47+
protectedBranch, err := GetProtectedBranchBy(repo.ID, pr.BaseBranch)
48+
if err != nil || protectedBranch == nil {
49+
return false, ""
50+
}
51+
if protectedBranch.GetGrantedApprovalsCount(pr) < 1 {
52+
return false, ""
53+
}
54+
case baseSigned:
55+
if gitRepo == nil {
56+
gitRepo, err = git.OpenRepository(tmpBasePath)
57+
if err != nil {
58+
return false, ""
59+
}
60+
defer gitRepo.Close()
61+
}
62+
commit, err := gitRepo.GetCommit(baseCommit)
63+
if err != nil {
64+
return false, ""
65+
}
66+
verification := ParseCommitWithSignature(commit)
67+
if !verification.Verified {
68+
return false, ""
69+
}
70+
case headSigned:
71+
if gitRepo == nil {
72+
gitRepo, err = git.OpenRepository(tmpBasePath)
73+
if err != nil {
74+
return false, ""
75+
}
76+
defer gitRepo.Close()
77+
}
78+
commit, err := gitRepo.GetCommit(headCommit)
79+
if err != nil {
80+
return false, ""
81+
}
82+
verification := ParseCommitWithSignature(commit)
83+
if !verification.Verified {
84+
return false, ""
85+
}
86+
case commitsSigned:
87+
if gitRepo == nil {
88+
gitRepo, err = git.OpenRepository(tmpBasePath)
89+
if err != nil {
90+
return false, ""
91+
}
92+
defer gitRepo.Close()
93+
}
94+
commit, err := gitRepo.GetCommit(headCommit)
95+
if err != nil {
96+
return false, ""
97+
}
98+
verification := ParseCommitWithSignature(commit)
99+
if !verification.Verified {
100+
return false, ""
101+
}
102+
// need to work out merge-base
103+
mergeBaseCommit, _, err := gitRepo.GetMergeBase("", baseCommit, headCommit)
104+
if err != nil {
105+
return false, ""
106+
}
107+
commitList, err := commit.CommitsBeforeUntil(mergeBaseCommit)
108+
if err != nil {
109+
return false, ""
110+
}
111+
for e := commitList.Front(); e != nil; e = e.Next() {
112+
commit = e.Value.(*git.Commit)
113+
verification := ParseCommitWithSignature(commit)
114+
if !verification.Verified {
115+
return false, ""
116+
}
117+
}
118+
}
119+
}
120+
return true, signingKey
121+
}

models/repo_sign.go

Lines changed: 3 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ const (
2424
baseSigned signingMode = "basesigned"
2525
headSigned signingMode = "headsigned"
2626
commitsSigned signingMode = "commitssigned"
27+
approved signingMode = "approved"
2728
)
2829

2930
func signingModeFromStrings(modeStrings []string) []signingMode {
@@ -45,6 +46,8 @@ func signingModeFromStrings(modeStrings []string) []signingMode {
4546
fallthrough
4647
case headSigned:
4748
fallthrough
49+
case approved:
50+
fallthrough
4851
case commitsSigned:
4952
returnable = append(returnable, signMode)
5053
}
@@ -211,98 +214,3 @@ func (repo *Repository) SignCRUDAction(u *User, tmpBasePath, parentCommit string
211214
}
212215
return true, signingKey
213216
}
214-
215-
// SignMerge determines if we should sign a merge commit to this repository
216-
func (repo *Repository) SignMerge(u *User, tmpBasePath, baseCommit, headCommit string) (bool, string) {
217-
rules := signingModeFromStrings(setting.Repository.Signing.Merges)
218-
signingKey := signingKey(repo.RepoPath())
219-
if signingKey == "" {
220-
return false, ""
221-
}
222-
var gitRepo *git.Repository
223-
var err error
224-
225-
for _, rule := range rules {
226-
switch rule {
227-
case never:
228-
return false, ""
229-
case always:
230-
break
231-
case pubkey:
232-
keys, err := ListGPGKeys(u.ID)
233-
if err != nil || len(keys) == 0 {
234-
return false, ""
235-
}
236-
case twofa:
237-
twofa, err := GetTwoFactorByUID(u.ID)
238-
if err != nil || twofa == nil {
239-
return false, ""
240-
}
241-
case baseSigned:
242-
if gitRepo == nil {
243-
gitRepo, err = git.OpenRepository(tmpBasePath)
244-
if err != nil {
245-
return false, ""
246-
}
247-
defer gitRepo.Close()
248-
}
249-
commit, err := gitRepo.GetCommit(baseCommit)
250-
if err != nil {
251-
return false, ""
252-
}
253-
verification := ParseCommitWithSignature(commit)
254-
if !verification.Verified {
255-
return false, ""
256-
}
257-
case headSigned:
258-
if gitRepo == nil {
259-
gitRepo, err = git.OpenRepository(tmpBasePath)
260-
if err != nil {
261-
return false, ""
262-
}
263-
defer gitRepo.Close()
264-
}
265-
commit, err := gitRepo.GetCommit(headCommit)
266-
if err != nil {
267-
return false, ""
268-
}
269-
verification := ParseCommitWithSignature(commit)
270-
if !verification.Verified {
271-
return false, ""
272-
}
273-
case commitsSigned:
274-
if gitRepo == nil {
275-
gitRepo, err = git.OpenRepository(tmpBasePath)
276-
if err != nil {
277-
return false, ""
278-
}
279-
defer gitRepo.Close()
280-
}
281-
commit, err := gitRepo.GetCommit(headCommit)
282-
if err != nil {
283-
return false, ""
284-
}
285-
verification := ParseCommitWithSignature(commit)
286-
if !verification.Verified {
287-
return false, ""
288-
}
289-
// need to work out merge-base
290-
mergeBaseCommit, _, err := gitRepo.GetMergeBase("", baseCommit, headCommit)
291-
if err != nil {
292-
return false, ""
293-
}
294-
commitList, err := commit.CommitsBeforeUntil(mergeBaseCommit)
295-
if err != nil {
296-
return false, ""
297-
}
298-
for e := commitList.Front(); e != nil; e = e.Next() {
299-
commit = e.Value.(*git.Commit)
300-
verification := ParseCommitWithSignature(commit)
301-
if !verification.Verified {
302-
return false, ""
303-
}
304-
}
305-
}
306-
}
307-
return true, signingKey
308-
}

services/pull/merge.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor
162162
// Determine if we should sign
163163
signArg := ""
164164
if version.Compare(binVersion, "1.7.9", ">=") {
165-
sign, keyID := pr.BaseRepo.SignMerge(doer, tmpBasePath, "HEAD", trackingBranch)
165+
sign, keyID := pr.SignMerge(doer, tmpBasePath, "HEAD", trackingBranch)
166166
if sign {
167167
signArg = "-S" + keyID
168168
} else if version.Compare(binVersion, "2.0.0", ">=") {

0 commit comments

Comments
 (0)