Skip to content

Add require signed commit for protected branch #9708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 28 commits into from
Jan 15, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
ba15ed9
Add require signed commit for protected branch
zeripath Jan 11, 2020
f482e28
Fix fmt
zeripath Jan 11, 2020
43c5df5
Merge branch 'master' into protect-branch-signed-commits-only
zeripath Jan 11, 2020
f9f8557
Make editor show if they will be signed
zeripath Jan 11, 2020
27c0354
Merge branch 'protect-branch-signed-commits-only' of github.com:zerip…
zeripath Jan 11, 2020
f2a28e8
bugfix
zeripath Jan 11, 2020
9a7ec7f
Merge branch 'master' into protect-branch-signed-commits-only
zeripath Jan 11, 2020
e4d987a
Merge branch 'master' into protect-branch-signed-commits-only
zeripath Jan 12, 2020
594126a
Add basic merge check and better information for CRUD
zeripath Jan 12, 2020
dd05f6a
linting comment
zeripath Jan 12, 2020
de67865
Add descriptors to merge signing
zeripath Jan 12, 2020
97b811d
Slight refactor
zeripath Jan 12, 2020
ea21e79
Merge branch 'master' into protect-branch-signed-commits-only
zeripath Jan 12, 2020
68a8e11
Slight improvement to appearances
zeripath Jan 12, 2020
acf011a
Handle Merge API
zeripath Jan 12, 2020
24eb4a2
manage CRUD API
zeripath Jan 12, 2020
bd8d89f
Move error to error.go
zeripath Jan 13, 2020
bfb7df0
Remove fix to delete.go
zeripath Jan 13, 2020
50cf492
prep for merge
zeripath Jan 13, 2020
6793e8d
Merge branch 'master' into protect-branch-signed-commits-only
zeripath Jan 13, 2020
7a04a47
need to tolerate \r\n in message
zeripath Jan 14, 2020
161d110
check protected branch before trying to load it
zeripath Jan 14, 2020
335ec50
Apply suggestions from code review
zeripath Jan 14, 2020
5ad5a56
Merge branch 'master' into protect-branch-signed-commits-only
zeripath Jan 14, 2020
ea40bd6
fix commit-reader
zeripath Jan 14, 2020
d554a62
Merge branch 'master' into protect-branch-signed-commits-only
zeripath Jan 14, 2020
2d454cd
Merge branch 'master' into protect-branch-signed-commits-only
zeripath Jan 14, 2020
788ed2f
Merge branch 'master' into protect-branch-signed-commits-only
zeripath Jan 15, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions models/branches.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ type ProtectedBranch struct {
RequiredApprovals int64 `xorm:"NOT NULL DEFAULT 0"`
BlockOnRejectedReviews bool `xorm:"NOT NULL DEFAULT false"`
DismissStaleApprovals bool `xorm:"NOT NULL DEFAULT false"`
RequireSignedCommits bool `xorm:"NOT NULL DEFAULT false"`

CreatedUnix timeutil.TimeStamp `xorm:"created"`
UpdatedUnix timeutil.TimeStamp `xorm:"updated"`
Expand Down
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,8 @@ var migrations = []Migration{
NewMigration("Add commit id and stale to reviews", addReviewCommitAndStale),
// v119 -> v120
NewMigration("Fix migrated repositories' git service type", fixMigratedRepositoryServiceType),
// v120 -> v121
NewMigration("Add Require Signed Commits to ProtectedBranch", addRequireSignedCommits),
}

// Migrate database to current version
Expand Down
18 changes: 18 additions & 0 deletions models/migrations/v120.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright 2020 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 migrations

import (
"xorm.io/xorm"
)

func addRequireSignedCommits(x *xorm.Engine) error {

type ProtectedBranch struct {
RequireSignedCommits bool `xorm:"NOT NULL DEFAULT false"`
}

return x.Sync2(new(ProtectedBranch))
}
1 change: 1 addition & 0 deletions modules/auth/repo_form.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ type ProtectBranchForm struct {
ApprovalsWhitelistTeams string
BlockOnRejectedReviews bool
DismissStaleApprovals bool
RequireSignedCommits bool
}

// Validate validates the fields
Expand Down
10 changes: 5 additions & 5 deletions modules/git/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ type Commit struct {
CommitMessage string
Signature *CommitGPGSignature

parents []SHA1 // SHA1 strings
Parents []SHA1 // SHA1 strings
submoduleCache *ObjectCache
}

Expand Down Expand Up @@ -94,7 +94,7 @@ func convertCommit(c *object.Commit) *Commit {
Committer: &c.Committer,
Author: &c.Author,
Signature: convertPGPSignature(c),
parents: c.ParentHashes,
Parents: c.ParentHashes,
}
}

Expand All @@ -111,10 +111,10 @@ func (c *Commit) Summary() string {
// ParentID returns oid of n-th parent (0-based index).
// It returns nil if no such parent exists.
func (c *Commit) ParentID(n int) (SHA1, error) {
if n >= len(c.parents) {
if n >= len(c.Parents) {
return SHA1{}, ErrNotExist{"", ""}
}
return c.parents[n], nil
return c.Parents[n], nil
}

// Parent returns n-th parent (0-based index) of the commit.
Expand All @@ -133,7 +133,7 @@ func (c *Commit) Parent(n int) (*Commit, error) {
// ParentCount returns number of parents of the commit.
// 0 if this is the root commit, otherwise 1,2, etc.
func (c *Commit) ParentCount() int {
return len(c.parents)
return len(c.Parents)
}

func isImageFile(data []byte) (string, bool) {
Expand Down
2 changes: 2 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1416,6 +1416,8 @@ settings.protect_approvals_whitelist_users = Whitelisted reviewers:
settings.protect_approvals_whitelist_teams = Whitelisted teams for reviews:
settings.dismiss_stale_approvals = Dismiss stale approvals
settings.dismiss_stale_approvals_desc = When new commits that change the content of the pull request are pushed to the branch, old approvals will be dismissed.
settings.require_signed_commits = Require Signed Commits
settings.require_signed_commits_desc = Reject pushes to this branch if they are unsigned or unverifiable
settings.add_protected_branch = Enable protection
settings.delete_protected_branch = Disable protection
settings.update_protect_branch_success = Branch protection for branch '%s' has been updated.
Expand Down
161 changes: 161 additions & 0 deletions routers/private/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
package private

import (
"bufio"
"bytes"
"context"
"fmt"
"net/http"
"os"
Expand All @@ -18,6 +21,7 @@ import (
"code.gitea.io/gitea/modules/repofiles"
"code.gitea.io/gitea/modules/util"
pull_service "code.gitea.io/gitea/services/pull"
"gopkg.in/src-d/go-git.v4/plumbing"

"gitea.com/macaron/macaron"
)
Expand All @@ -35,6 +39,15 @@ func HookPreReceive(ctx *macaron.Context, opts private.HookOptions) {
return
}
repo.OwnerName = ownerName
gitRepo, err := git.OpenRepository(repo.RepoPath())
if err != nil {
log.Error("Unable to get git repository for: %s/%s Error: %v", ownerName, repoName, err)
ctx.JSON(http.StatusInternalServerError, map[string]interface{}{
"err": err.Error(),
})
return
}
defer gitRepo.Close()

for i := range opts.OldCommitIDs {
oldCommitID := opts.OldCommitIDs[i]
Expand Down Expand Up @@ -92,6 +105,154 @@ func HookPreReceive(ctx *macaron.Context, opts private.HookOptions) {

}
}

// Require signed commits
if protectBranch.RequireSignedCommits {
env := os.Environ()
if opts.GitAlternativeObjectDirectories != "" {
env = append(env,
private.GitAlternativeObjectDirectories+"="+opts.GitAlternativeObjectDirectories)
}
if opts.GitObjectDirectory != "" {
env = append(env,
private.GitObjectDirectory+"="+opts.GitObjectDirectory)
}
if opts.GitQuarantinePath != "" {
env = append(env,
private.GitQuarantinePath+"="+opts.GitQuarantinePath)
}

stdoutReader, stdoutWriter, err := os.Pipe()
if err != nil {
log.Error("Unable to create pipe: %v", err)
ctx.JSON(http.StatusInternalServerError, map[string]interface{}{
"err": fmt.Sprintf("Unable to create pipe: %v", err),
})
return
}
defer func() {
_ = stdoutReader.Close()
_ = stdoutWriter.Close()
}()

stderr := new(bytes.Buffer)
commits := make([]*git.Commit, 0, 10)

var finalErr error
if err := git.NewCommand("rev-list", oldCommitID+"..."+newCommitID).
RunInDirTimeoutEnvFullPipelineFunc(env, -1, repo.RepoPath(),
stdoutWriter, stderr, nil,
func(ctx context.Context, cancel context.CancelFunc) {
_ = stdoutWriter.Close()
scanner := bufio.NewScanner(stdoutReader)
for scanner.Scan() {
line := scanner.Text()
// TODO: Consider whether we really want to read these completely in to memory
var commitStr string
commitStr, finalErr = git.NewCommand("cat-file", "commit", line).RunInDirWithEnv(repo.RepoPath(), env)
if finalErr != nil {
cancel()
}
commits = append(commits, &git.Commit{
ID: plumbing.NewHash(line),
CommitMessage: commitStr,
})
}
_ = stdoutReader.Close()
}); err != nil {
if finalErr != nil {
err = finalErr
}
log.Error("Unable to check commits from %s to %s in %-v: %v", oldCommitID, newCommitID, repo, err)
ctx.JSON(http.StatusInternalServerError, map[string]interface{}{
"err": fmt.Sprintf("Unable to check commits from %s to %s: %v", oldCommitID, newCommitID, err),
})
return
}

// TODO: We should batch read a few commits in at a time: use -n and --skip
// Now parse the commitStrs
for _, commit := range commits {
payloadSB := new(strings.Builder)
signatureSB := new(strings.Builder)
messageSB := new(strings.Builder)
message := false
pgpsig := false

scanner := bufio.NewScanner(strings.NewReader(commit.CommitMessage))
for scanner.Scan() {
line := scanner.Bytes()
if pgpsig {
if len(line) > 0 && line[0] == ' ' {
line = bytes.TrimLeft(line, " ")
_, _ = signatureSB.Write(line)
_ = signatureSB.WriteByte('\n')
continue
} else {
pgpsig = false
}
}

if !message {
trimmed := bytes.TrimSpace(line)
if len(trimmed) == 0 {
message = true
_, _ = payloadSB.WriteString("\n")
continue
}

split := bytes.SplitN(trimmed, []byte{' '}, 2)

switch string(split[0]) {
case "tree":
commit.Tree = *git.NewTree(gitRepo, plumbing.NewHash(string(split[1])))
_, _ = payloadSB.Write(line)
_ = payloadSB.WriteByte('\n')
case "parent":
commit.Parents = append(commit.Parents, plumbing.NewHash(string(split[1])))
_, _ = payloadSB.Write(line)
_ = payloadSB.WriteByte('\n')
case "author":
commit.Author = &git.Signature{}
commit.Author.Decode(split[1])
_, _ = payloadSB.Write(line)
_ = payloadSB.WriteByte('\n')
case "committer":
commit.Committer = &git.Signature{}
commit.Committer.Decode(split[1])
_, _ = payloadSB.Write(line)
_ = payloadSB.WriteByte('\n')
case "gpgsig":
_, _ = signatureSB.Write(split[1])
_ = signatureSB.WriteByte('\n')
pgpsig = true
}
} else {
_, _ = messageSB.Write(line)
_ = messageSB.WriteByte('\n')
}
}
commit.CommitMessage = messageSB.String()
_, _ = payloadSB.WriteString(commit.CommitMessage)
commit.Signature = &git.CommitGPGSignature{
Signature: signatureSB.String(),
Payload: payloadSB.String(),
}
if len(commit.Signature.Signature) == 0 {
commit.Signature = nil
}

verification := models.ParseCommitWithSignature(commit)
if !verification.Verified {
log.Warn("Forbidden: Branch: %s in %-v is protected from unverified commit %s", branchName, repo, commit.ID.String())
ctx.JSON(http.StatusForbidden, map[string]interface{}{
"err": fmt.Sprintf("branch %s is protected from unverified commit %s", branchName, commit.ID.String()),
})
return
}
}
}

canPush := false
if opts.IsDeployKey {
canPush = protectBranch.CanPush && (!protectBranch.EnableWhitelist || protectBranch.WhitelistDeployKeys)
Expand Down
1 change: 1 addition & 0 deletions routers/repo/setting_protected_branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ func SettingsProtectedBranchPost(ctx *context.Context, f auth.ProtectBranchForm)
}
protectBranch.BlockOnRejectedReviews = f.BlockOnRejectedReviews
protectBranch.DismissStaleApprovals = f.DismissStaleApprovals
protectBranch.RequireSignedCommits = f.RequireSignedCommits

err = models.UpdateProtectBranch(ctx.Repo.Repository, protectBranch, models.WhitelistOptions{
UserIDs: whitelistUsers,
Expand Down
9 changes: 8 additions & 1 deletion templates/repo/settings/protected_branch.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -210,14 +210,21 @@
<label for="block_on_rejected_reviews">{{.i18n.Tr "repo.settings.block_rejected_reviews"}}</label>
<p class="help">{{.i18n.Tr "repo.settings.block_rejected_reviews_desc"}}</p>
</div>
</div>
</div>
<div class="field">
<div class="ui checkbox">
<input name="dismiss_stale_approvals" type="checkbox" {{if .Branch.DismissStaleApprovals}}checked{{end}}>
<label for="dismiss_stale_approvals">{{.i18n.Tr "repo.settings.dismiss_stale_approvals"}}</label>
<p class="help">{{.i18n.Tr "repo.settings.dismiss_stale_approvals_desc"}}</p>
</div>
</div>
<div class="field">
<div class="ui checkbox">
<input name="require_signed_commits" type="checkbox" {{if .Branch.RequireSignedCommits}}checked{{end}}>
<label for="require_signed_commits">{{.i18n.Tr "repo.settings.require_signed_commits"}}</label>
<p class="help">{{.i18n.Tr "repo.settings.require_signed_commits_desc"}}</p>
</div>
</div>

</div>

Expand Down