diff options
author | zeripath <art27@cantab.net> | 2020-01-15 08:32:57 +0000 |
---|---|---|
committer | Antoine GIRARD <sapk@users.noreply.github.com> | 2020-01-15 09:32:57 +0100 |
commit | 66ee9b87f9aaabef836ec72bfaf8032b359b29c1 (patch) | |
tree | b6d134fb5ccc83c4b7ddad6a0eb6206496cc8b76 /modules | |
parent | 6b1fa1235904947187266789b204f19bc03872be (diff) | |
download | gitea-66ee9b87f9aaabef836ec72bfaf8032b359b29c1.tar.gz gitea-66ee9b87f9aaabef836ec72bfaf8032b359b29c1.zip |
Add require signed commit for protected branch (#9708)
* Add require signed commit for protected branch
* Fix fmt
* Make editor show if they will be signed
* bugfix
* Add basic merge check and better information for CRUD
* linting comment
* Add descriptors to merge signing
* Slight refactor
* Slight improvement to appearances
* Handle Merge API
* manage CRUD API
* Move error to error.go
* Remove fix to delete.go
* prep for merge
* need to tolerate \r\n in message
* check protected branch before trying to load it
* Apply suggestions from code review
Co-Authored-By: guillep2k <18600385+guillep2k@users.noreply.github.com>
* fix commit-reader
Co-authored-by: guillep2k <18600385+guillep2k@users.noreply.github.com>
Diffstat (limited to 'modules')
-rw-r--r-- | modules/auth/repo_form.go | 1 | ||||
-rw-r--r-- | modules/context/repo.go | 51 | ||||
-rw-r--r-- | modules/git/command.go | 8 | ||||
-rw-r--r-- | modules/git/commit.go | 10 | ||||
-rw-r--r-- | modules/git/commit_reader.go | 108 | ||||
-rw-r--r-- | modules/repofiles/delete.go | 23 | ||||
-rw-r--r-- | modules/repofiles/temp_repo.go | 5 | ||||
-rw-r--r-- | modules/repofiles/update.go | 23 | ||||
-rw-r--r-- | modules/repository/init.go | 2 |
9 files changed, 212 insertions, 19 deletions
diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index 0086419b65..a5071de47e 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -173,6 +173,7 @@ type ProtectBranchForm struct { ApprovalsWhitelistTeams string BlockOnRejectedReviews bool DismissStaleApprovals bool + RequireSignedCommits bool } // Validate validates the fields diff --git a/modules/context/repo.go b/modules/context/repo.go index 86c7df2b05..66700a6937 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -74,14 +74,57 @@ func RepoMustNotBeArchived() macaron.Handler { } } +// CanCommitToBranchResults represents the results of CanCommitToBranch +type CanCommitToBranchResults struct { + CanCommitToBranch bool + EditorEnabled bool + UserCanPush bool + RequireSigned bool + WillSign bool + SigningKey string + WontSignReason string +} + // CanCommitToBranch returns true if repository is editable and user has proper access level // and branch is not protected for push -func (r *Repository) CanCommitToBranch(doer *models.User) (bool, error) { - protectedBranch, err := r.Repository.IsProtectedBranchForPush(r.BranchName, doer) +func (r *Repository) CanCommitToBranch(doer *models.User) (CanCommitToBranchResults, error) { + protectedBranch, err := models.GetProtectedBranchBy(r.Repository.ID, r.BranchName) + if err != nil { - return false, err + return CanCommitToBranchResults{}, err } - return r.CanEnableEditor() && !protectedBranch, nil + userCanPush := true + requireSigned := false + if protectedBranch != nil { + userCanPush = protectedBranch.CanUserPush(doer.ID) + requireSigned = protectedBranch.RequireSignedCommits + } + + sign, keyID, err := r.Repository.SignCRUDAction(doer, r.Repository.RepoPath(), git.BranchPrefix+r.BranchName) + + canCommit := r.CanEnableEditor() && userCanPush + if requireSigned { + canCommit = canCommit && sign + } + wontSignReason := "" + if err != nil { + if models.IsErrWontSign(err) { + wontSignReason = string(err.(*models.ErrWontSign).Reason) + err = nil + } else { + wontSignReason = "error" + } + } + + return CanCommitToBranchResults{ + CanCommitToBranch: canCommit, + EditorEnabled: r.CanEnableEditor(), + UserCanPush: userCanPush, + RequireSigned: requireSigned, + WillSign: sign, + SigningKey: keyID, + WontSignReason: wontSignReason, + }, err } // CanUseTimetracker returns whether or not a user can use the timetracker. diff --git a/modules/git/command.go b/modules/git/command.go index 33143dbd75..3cb676c8da 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -97,7 +97,7 @@ func (c *Command) RunInDirTimeoutEnvFullPipeline(env []string, timeout time.Dura // RunInDirTimeoutEnvFullPipelineFunc executes the command in given directory with given timeout, // it pipes stdout and stderr to given io.Writer and passes in an io.Reader as stdin. Between cmd.Start and cmd.Wait the passed in function is run. -func (c *Command) RunInDirTimeoutEnvFullPipelineFunc(env []string, timeout time.Duration, dir string, stdout, stderr io.Writer, stdin io.Reader, fn func(context.Context, context.CancelFunc)) error { +func (c *Command) RunInDirTimeoutEnvFullPipelineFunc(env []string, timeout time.Duration, dir string, stdout, stderr io.Writer, stdin io.Reader, fn func(context.Context, context.CancelFunc) error) error { if timeout == -1 { timeout = DefaultCommandExecutionTimeout @@ -135,7 +135,11 @@ func (c *Command) RunInDirTimeoutEnvFullPipelineFunc(env []string, timeout time. defer process.GetManager().Remove(pid) if fn != nil { - fn(ctx, cancel) + err := fn(ctx, cancel) + if err != nil { + cancel() + return err + } } if err := cmd.Wait(); err != nil && ctx.Err() != context.DeadlineExceeded { diff --git a/modules/git/commit.go b/modules/git/commit.go index dfb7adcd1a..9646d56063 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -33,7 +33,7 @@ type Commit struct { CommitMessage string Signature *CommitGPGSignature - parents []SHA1 // SHA1 strings + Parents []SHA1 // SHA1 strings submoduleCache *ObjectCache } @@ -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, } } @@ -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. @@ -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) { diff --git a/modules/git/commit_reader.go b/modules/git/commit_reader.go new file mode 100644 index 0000000000..06d8f426d7 --- /dev/null +++ b/modules/git/commit_reader.go @@ -0,0 +1,108 @@ +// 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 git + +import ( + "bufio" + "bytes" + "io" + "strings" + + "gopkg.in/src-d/go-git.v4/plumbing" +) + +// CommitFromReader will generate a Commit from a provided reader +// We will need this to interpret commits from cat-file +func CommitFromReader(gitRepo *Repository, sha plumbing.Hash, reader io.Reader) (*Commit, error) { + commit := &Commit{ + ID: sha, + } + + payloadSB := new(strings.Builder) + signatureSB := new(strings.Builder) + messageSB := new(strings.Builder) + message := false + pgpsig := false + + scanner := bufio.NewScanner(reader) + // Split by '\n' but include the '\n' + scanner.Split(func(data []byte, atEOF bool) (advance int, token []byte, err error) { + if atEOF && len(data) == 0 { + return 0, nil, nil + } + if i := bytes.IndexByte(data, '\n'); i >= 0 { + // We have a full newline-terminated line. + return i + 1, data[0 : i+1], nil + } + // If we're at EOF, we have a final, non-terminated line. Return it. + if atEOF { + return len(data), data, nil + } + // Request more data. + return 0, nil, nil + }) + + for scanner.Scan() { + line := scanner.Bytes() + if pgpsig { + if len(line) > 0 && line[0] == ' ' { + _, _ = signatureSB.Write(line[1:]) + continue + } else { + pgpsig = false + } + } + + if !message { + // This is probably not correct but is copied from go-gits interpretation... + trimmed := bytes.TrimSpace(line) + if len(trimmed) == 0 { + message = true + _, _ = payloadSB.Write(line) + continue + } + + split := bytes.SplitN(trimmed, []byte{' '}, 2) + var data []byte + if len(split) > 1 { + data = split[1] + } + + switch string(split[0]) { + case "tree": + commit.Tree = *NewTree(gitRepo, plumbing.NewHash(string(data))) + _, _ = payloadSB.Write(line) + case "parent": + commit.Parents = append(commit.Parents, plumbing.NewHash(string(data))) + _, _ = payloadSB.Write(line) + case "author": + commit.Author = &Signature{} + commit.Author.Decode(data) + _, _ = payloadSB.Write(line) + case "committer": + commit.Committer = &Signature{} + commit.Committer.Decode(data) + _, _ = payloadSB.Write(line) + case "gpgsig": + _, _ = signatureSB.Write(data) + _ = signatureSB.WriteByte('\n') + pgpsig = true + } + } else { + _, _ = messageSB.Write(line) + } + } + commit.CommitMessage = messageSB.String() + _, _ = payloadSB.WriteString(commit.CommitMessage) + commit.Signature = &CommitGPGSignature{ + Signature: signatureSB.String(), + Payload: payloadSB.String(), + } + if len(commit.Signature.Signature) == 0 { + commit.Signature = nil + } + + return commit, scanner.Err() +} diff --git a/modules/repofiles/delete.go b/modules/repofiles/delete.go index c91f597f9a..c1689b0be0 100644 --- a/modules/repofiles/delete.go +++ b/modules/repofiles/delete.go @@ -55,9 +55,26 @@ func DeleteRepoFile(repo *models.Repository, doer *models.User, opts *DeleteRepo BranchName: opts.NewBranch, } } - } else if protected, _ := repo.IsProtectedBranchForPush(opts.OldBranch, doer); protected { - return nil, models.ErrUserCannotCommit{ - UserName: doer.LowerName, + } else { + protectedBranch, err := repo.GetBranchProtection(opts.OldBranch) + if err != nil { + return nil, err + } + if protectedBranch != nil && !protectedBranch.CanUserPush(doer.ID) { + return nil, models.ErrUserCannotCommit{ + UserName: doer.LowerName, + } + } + if protectedBranch != nil && protectedBranch.RequireSignedCommits { + _, _, err := repo.SignCRUDAction(doer, repo.RepoPath(), opts.OldBranch) + if err != nil { + if !models.IsErrWontSign(err) { + return nil, err + } + return nil, models.ErrUserCannotCommit{ + UserName: doer.LowerName, + } + } } } diff --git a/modules/repofiles/temp_repo.go b/modules/repofiles/temp_repo.go index f9ea4ba155..a1cc37e8c6 100644 --- a/modules/repofiles/temp_repo.go +++ b/modules/repofiles/temp_repo.go @@ -219,7 +219,7 @@ func (t *TemporaryUploadRepository) CommitTreeWithDate(author, committer *models // Determine if we should sign if version.Compare(binVersion, "1.7.9", ">=") { - sign, keyID := t.repo.SignCRUDAction(author, t.basePath, "HEAD") + sign, keyID, _ := t.repo.SignCRUDAction(author, t.basePath, "HEAD") if sign { args = append(args, "-S"+keyID) } else if version.Compare(binVersion, "2.0.0", ">=") { @@ -268,7 +268,7 @@ func (t *TemporaryUploadRepository) DiffIndex() (*gitdiff.Diff, error) { var finalErr error if err := git.NewCommand("diff-index", "--cached", "-p", "HEAD"). - RunInDirTimeoutEnvFullPipelineFunc(nil, 30*time.Second, t.basePath, stdoutWriter, stderr, nil, func(ctx context.Context, cancel context.CancelFunc) { + RunInDirTimeoutEnvFullPipelineFunc(nil, 30*time.Second, t.basePath, stdoutWriter, stderr, nil, func(ctx context.Context, cancel context.CancelFunc) error { _ = stdoutWriter.Close() diff, finalErr = gitdiff.ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, stdoutReader) if finalErr != nil { @@ -276,6 +276,7 @@ func (t *TemporaryUploadRepository) DiffIndex() (*gitdiff.Diff, error) { cancel() } _ = stdoutReader.Close() + return finalErr }); err != nil { if finalErr != nil { log.Error("Unable to ParsePatch in temporary repo %s (%s). Error: %v", t.repo.FullName(), t.basePath, finalErr) diff --git a/modules/repofiles/update.go b/modules/repofiles/update.go index e22a2062a0..430a83093d 100644 --- a/modules/repofiles/update.go +++ b/modules/repofiles/update.go @@ -151,8 +151,27 @@ func CreateOrUpdateRepoFile(repo *models.Repository, doer *models.User, opts *Up if err != nil && !git.IsErrBranchNotExist(err) { return nil, err } - } else if protected, _ := repo.IsProtectedBranchForPush(opts.OldBranch, doer); protected { - return nil, models.ErrUserCannotCommit{UserName: doer.LowerName} + } else { + protectedBranch, err := repo.GetBranchProtection(opts.OldBranch) + if err != nil { + return nil, err + } + if protectedBranch != nil && !protectedBranch.CanUserPush(doer.ID) { + return nil, models.ErrUserCannotCommit{ + UserName: doer.LowerName, + } + } + if protectedBranch != nil && protectedBranch.RequireSignedCommits { + _, _, err := repo.SignCRUDAction(doer, repo.RepoPath(), opts.OldBranch) + if err != nil { + if !models.IsErrWontSign(err) { + return nil, err + } + return nil, models.ErrUserCannotCommit{ + UserName: doer.LowerName, + } + } + } } // If FromTreePath is not set, set it to the opts.TreePath diff --git a/modules/repository/init.go b/modules/repository/init.go index a65b335174..9d0beb1138 100644 --- a/modules/repository/init.go +++ b/modules/repository/init.go @@ -130,7 +130,7 @@ func initRepoCommit(tmpPath string, repo *models.Repository, u *models.User) (er } if version.Compare(binVersion, "1.7.9", ">=") { - sign, keyID := models.SignInitialCommit(tmpPath, u) + sign, keyID, _ := models.SignInitialCommit(tmpPath, u) if sign { args = append(args, "-S"+keyID) } else if version.Compare(binVersion, "2.0.0", ">=") { |