From 3abe17f9e088d24eff4f4e3b98a6c555db334202 Mon Sep 17 00:00:00 2001 From: zeripath Date: Sun, 15 Dec 2019 11:06:31 +0000 Subject: [PATCH] Sign protected branches (#8993) * Move SignMerge to PullRequest * Add approved signing mode * As per @guillep2k comment --- custom/conf/app.ini.sample | 1 + .../doc/advanced/config-cheat-sheet.en-us.md | 3 +- docs/content/doc/advanced/signing.en-us.md | 1 + models/pull_sign.go | 121 ++++++++++++++++++ models/repo_sign.go | 98 +------------- services/pull/merge.go | 2 +- 6 files changed, 129 insertions(+), 97 deletions(-) create mode 100644 models/pull_sign.go diff --git a/custom/conf/app.ini.sample b/custom/conf/app.ini.sample index 76889484b58a6..701374d4b8520 100644 --- a/custom/conf/app.ini.sample +++ b/custom/conf/app.ini.sample @@ -110,6 +110,7 @@ WIKI = never ; Determines when to sign on merges ; - basesigned: require that the parent of commit on the base repo is signed. ; - commitssigned: require that all the commits in the head branch are signed. +; - approved: only sign when merging an approved pr to a protected branch MERGES = pubkey, twofa, basesigned, commitssigned [cors] diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index 4174a874715c8..36e56c3fed347 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -96,7 +96,8 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`. - `CRUD_ACTIONS`: **pubkey, twofa, parentsigned**: \[never, pubkey, twofa, parentsigned, always\]: Sign CRUD actions. - Options as above, with the addition of: - `parentsigned`: Only sign if the parent commit is signed. -- `MERGES`: **pubkey, twofa, basesigned, commitssigned**: \[never, pubkey, twofa, basesigned, commitssigned, always\]: Sign merges. +- `MERGES`: **pubkey, twofa, basesigned, commitssigned**: \[never, pubkey, twofa, approved, basesigned, commitssigned, always\]: Sign merges. + - `approved`: Only sign approved merges to a protected branch. - `basesigned`: Only sign if the parent commit in the base repo is signed. - `headsigned`: Only sign if the head commit in the head branch is signed. - `commitssigned`: Only sign if all the commits in the head branch to the merge point are signed. diff --git a/docs/content/doc/advanced/signing.en-us.md b/docs/content/doc/advanced/signing.en-us.md index b6c99e269e09b..72d294e7bd62a 100644 --- a/docs/content/doc/advanced/signing.en-us.md +++ b/docs/content/doc/advanced/signing.en-us.md @@ -136,6 +136,7 @@ The possible options are: * `basesigned`: Only sign if the parent commit in the base repo is signed. * `headsigned`: Only sign if the head commit in the head branch is signed. * `commitssigned`: Only sign if all the commits in the head branch to the merge point are signed. +* `approved`: Only sign approved merges to a protected branch. * `always`: Always sign Options other than `never` and `always` can be combined as a comma diff --git a/models/pull_sign.go b/models/pull_sign.go new file mode 100644 index 0000000000000..19d8907c3dbfe --- /dev/null +++ b/models/pull_sign.go @@ -0,0 +1,121 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package models + +import ( + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" +) + +// SignMerge determines if we should sign a PR merge commit to the base repository +func (pr *PullRequest) SignMerge(u *User, tmpBasePath, baseCommit, headCommit string) (bool, string) { + if err := pr.GetBaseRepo(); err != nil { + log.Error("Unable to get Base Repo for pull request") + return false, "" + } + repo := pr.BaseRepo + + signingKey := signingKey(repo.RepoPath()) + if signingKey == "" { + return false, "" + } + rules := signingModeFromStrings(setting.Repository.Signing.Merges) + + var gitRepo *git.Repository + var err error + + for _, rule := range rules { + switch rule { + case never: + return false, "" + case always: + break + case pubkey: + keys, err := ListGPGKeys(u.ID) + if err != nil || len(keys) == 0 { + return false, "" + } + case twofa: + twofa, err := GetTwoFactorByUID(u.ID) + if err != nil || twofa == nil { + return false, "" + } + case approved: + protectedBranch, err := GetProtectedBranchBy(repo.ID, pr.BaseBranch) + if err != nil || protectedBranch == nil { + return false, "" + } + if protectedBranch.GetGrantedApprovalsCount(pr) < 1 { + return false, "" + } + case baseSigned: + if gitRepo == nil { + gitRepo, err = git.OpenRepository(tmpBasePath) + if err != nil { + return false, "" + } + defer gitRepo.Close() + } + commit, err := gitRepo.GetCommit(baseCommit) + if err != nil { + return false, "" + } + verification := ParseCommitWithSignature(commit) + if !verification.Verified { + return false, "" + } + case headSigned: + if gitRepo == nil { + gitRepo, err = git.OpenRepository(tmpBasePath) + if err != nil { + return false, "" + } + defer gitRepo.Close() + } + commit, err := gitRepo.GetCommit(headCommit) + if err != nil { + return false, "" + } + verification := ParseCommitWithSignature(commit) + if !verification.Verified { + return false, "" + } + case commitsSigned: + if gitRepo == nil { + gitRepo, err = git.OpenRepository(tmpBasePath) + if err != nil { + return false, "" + } + defer gitRepo.Close() + } + commit, err := gitRepo.GetCommit(headCommit) + if err != nil { + return false, "" + } + verification := ParseCommitWithSignature(commit) + if !verification.Verified { + return false, "" + } + // need to work out merge-base + mergeBaseCommit, _, err := gitRepo.GetMergeBase("", baseCommit, headCommit) + if err != nil { + return false, "" + } + commitList, err := commit.CommitsBeforeUntil(mergeBaseCommit) + if err != nil { + return false, "" + } + for e := commitList.Front(); e != nil; e = e.Next() { + commit = e.Value.(*git.Commit) + verification := ParseCommitWithSignature(commit) + if !verification.Verified { + return false, "" + } + } + } + } + return true, signingKey +} diff --git a/models/repo_sign.go b/models/repo_sign.go index a02b027f89f89..a684efb55fc44 100644 --- a/models/repo_sign.go +++ b/models/repo_sign.go @@ -24,6 +24,7 @@ const ( baseSigned signingMode = "basesigned" headSigned signingMode = "headsigned" commitsSigned signingMode = "commitssigned" + approved signingMode = "approved" ) func signingModeFromStrings(modeStrings []string) []signingMode { @@ -45,6 +46,8 @@ func signingModeFromStrings(modeStrings []string) []signingMode { fallthrough case headSigned: fallthrough + case approved: + fallthrough case commitsSigned: returnable = append(returnable, signMode) } @@ -211,98 +214,3 @@ func (repo *Repository) SignCRUDAction(u *User, tmpBasePath, parentCommit string } return true, signingKey } - -// SignMerge determines if we should sign a merge commit to this repository -func (repo *Repository) SignMerge(u *User, tmpBasePath, baseCommit, headCommit string) (bool, string) { - rules := signingModeFromStrings(setting.Repository.Signing.Merges) - signingKey := signingKey(repo.RepoPath()) - if signingKey == "" { - return false, "" - } - var gitRepo *git.Repository - var err error - - for _, rule := range rules { - switch rule { - case never: - return false, "" - case always: - break - case pubkey: - keys, err := ListGPGKeys(u.ID) - if err != nil || len(keys) == 0 { - return false, "" - } - case twofa: - twofa, err := GetTwoFactorByUID(u.ID) - if err != nil || twofa == nil { - return false, "" - } - case baseSigned: - if gitRepo == nil { - gitRepo, err = git.OpenRepository(tmpBasePath) - if err != nil { - return false, "" - } - defer gitRepo.Close() - } - commit, err := gitRepo.GetCommit(baseCommit) - if err != nil { - return false, "" - } - verification := ParseCommitWithSignature(commit) - if !verification.Verified { - return false, "" - } - case headSigned: - if gitRepo == nil { - gitRepo, err = git.OpenRepository(tmpBasePath) - if err != nil { - return false, "" - } - defer gitRepo.Close() - } - commit, err := gitRepo.GetCommit(headCommit) - if err != nil { - return false, "" - } - verification := ParseCommitWithSignature(commit) - if !verification.Verified { - return false, "" - } - case commitsSigned: - if gitRepo == nil { - gitRepo, err = git.OpenRepository(tmpBasePath) - if err != nil { - return false, "" - } - defer gitRepo.Close() - } - commit, err := gitRepo.GetCommit(headCommit) - if err != nil { - return false, "" - } - verification := ParseCommitWithSignature(commit) - if !verification.Verified { - return false, "" - } - // need to work out merge-base - mergeBaseCommit, _, err := gitRepo.GetMergeBase("", baseCommit, headCommit) - if err != nil { - return false, "" - } - commitList, err := commit.CommitsBeforeUntil(mergeBaseCommit) - if err != nil { - return false, "" - } - for e := commitList.Front(); e != nil; e = e.Next() { - commit = e.Value.(*git.Commit) - verification := ParseCommitWithSignature(commit) - if !verification.Verified { - return false, "" - } - } - } - } - return true, signingKey -} diff --git a/services/pull/merge.go b/services/pull/merge.go index 9b75c5ffda765..8aa38bf11eab7 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -162,7 +162,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor // Determine if we should sign signArg := "" if version.Compare(binVersion, "1.7.9", ">=") { - sign, keyID := pr.BaseRepo.SignMerge(doer, tmpBasePath, "HEAD", trackingBranch) + sign, keyID := pr.SignMerge(doer, tmpBasePath, "HEAD", trackingBranch) if sign { signArg = "-S" + keyID } else if version.Compare(binVersion, "2.0.0", ">=") {