Clean-up HookPreReceive and restore functionality for pushing non-standard refs (#16705)

* Clean-up HookPreReceive and restore functionality for pushing non-standard refs

There was an inadvertent breaking change in #15629 meaning that notes refs and other
git extension refs will be automatically rejected.

Further following #14295 and #15629 the pre-recieve hook code is untenably long and
too complex.

This PR refactors the hook code and removes the incorrect forced rejection of
non-standard refs.

Fix #16688

Signed-off-by: Andrew Thornton <art27@cantab.net>

modules/web/route.go

@@ -31,6 +31,7 @@ func Wrap(handlers ...interface{}) http.HandlerFunc {
 			func(ctx *context.Context) goctx.CancelFunc,
 			func(*context.APIContext),
 			func(*context.PrivateContext),
+			func(*context.PrivateContext) goctx.CancelFunc,
 			func(http.Handler) http.Handler:
 		default:
 			panic(fmt.Sprintf("Unsupported handler type: %#v", t))
@@ -59,6 +60,15 @@ func Wrap(handlers ...interface{}) http.HandlerFunc {
 				if ctx.Written() {
 					return
 				}
+			case func(*context.PrivateContext) goctx.CancelFunc:
+				ctx := context.GetPrivateContext(req)
+				cancel := t(ctx)
+				if cancel != nil {
+					defer cancel()
+				}
+				if ctx.Written() {
+					return
+				}
 			case func(ctx *context.Context):
 				ctx := context.GetContext(req)
 				t(ctx)

routers/private/default_branch.go

@@ -0,0 +1,75 @@
+// Copyright 2021 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 private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead.
+package private
+
+import (
+	"fmt"
+	"net/http"
+
+	"code.gitea.io/gitea/models"
+	gitea_context "code.gitea.io/gitea/modules/context"
+	"code.gitea.io/gitea/modules/git"
+	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/private"
+)
+
+// ________          _____             .__   __
+// \______ \   _____/ ____\____   __ __|  |_/  |_
+//  |    |  \_/ __ \   __\\__  \ |  |  \  |\   __\
+//  |    `   \  ___/|  |   / __ \|  |  /  |_|  |
+// /_______  /\___  >__|  (____  /____/|____/__|
+//         \/     \/           \/
+// __________                             .__
+// \______   \____________    ____   ____ |  |__
+//  |    |  _/\_  __ \__  \  /    \_/ ___\|  |  \
+//  |    |   \ |  | \// __ \|   |  \  \___|   Y  \
+//  |______  / |__|  (____  /___|  /\___  >___|  /
+//         \/             \/     \/     \/     \/
+
+// SetDefaultBranch updates the default branch
+func SetDefaultBranch(ctx *gitea_context.PrivateContext) {
+	ownerName := ctx.Params(":owner")
+	repoName := ctx.Params(":repo")
+	branch := ctx.Params(":branch")
+	repo, err := models.GetRepositoryByOwnerAndName(ownerName, repoName)
+	if err != nil {
+		log.Error("Failed to get repository: %s/%s Error: %v", ownerName, repoName, err)
+		ctx.JSON(http.StatusInternalServerError, private.Response{
+			Err: fmt.Sprintf("Failed to get repository: %s/%s Error: %v", ownerName, repoName, err),
+		})
+		return
+	}
+	if repo.OwnerName == "" {
+		repo.OwnerName = ownerName
+	}
+
+	repo.DefaultBranch = branch
+	gitRepo, err := git.OpenRepository(repo.RepoPath())
+	if err != nil {
+		ctx.JSON(http.StatusInternalServerError, private.Response{
+			Err: fmt.Sprintf("Failed to get git repository: %s/%s Error: %v", ownerName, repoName, err),
+		})
+		return
+	}
+	if err := gitRepo.SetDefaultBranch(repo.DefaultBranch); err != nil {
+		if !git.IsErrUnsupportedVersion(err) {
+			gitRepo.Close()
+			ctx.JSON(http.StatusInternalServerError, private.Response{
+				Err: fmt.Sprintf("Unable to set default branch on repository: %s/%s Error: %v", ownerName, repoName, err),
+			})
+			return
+		}
+	}
+	gitRepo.Close()
+
+	if err := repo.UpdateDefaultBranch(); err != nil {
+		ctx.JSON(http.StatusInternalServerError, private.Response{
+			Err: fmt.Sprintf("Unable to set default branch on repository: %s/%s Error: %v", ownerName, repoName, err),
+		})
+		return
+	}
+	ctx.PlainText(http.StatusOK, []byte("success"))
+}

routers/private/hook_post_receive.go

@@ -0,0 +1,201 @@
+// Copyright 2021 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 private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead.
+package private
+
+import (
+	"fmt"
+	"net/http"
+	"strings"
+
+	"code.gitea.io/gitea/models"
+	gitea_context "code.gitea.io/gitea/modules/context"
+	"code.gitea.io/gitea/modules/git"
+	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/private"
+	repo_module "code.gitea.io/gitea/modules/repository"
+	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/util"
+	"code.gitea.io/gitea/modules/web"
+	repo_service "code.gitea.io/gitea/services/repository"
+)
+
+// HookPostReceive updates services and users
+func HookPostReceive(ctx *gitea_context.PrivateContext) {
+	opts := web.GetForm(ctx).(*private.HookOptions)
+
+	// We don't rely on RepoAssignment here because:
+	// a) we don't need the git repo in this function
+	// b) our update function will likely change the repository in the db so we will need to refresh it
+	// c) we don't always need the repo
+
+	ownerName := ctx.Params(":owner")
+	repoName := ctx.Params(":repo")
+
+	// defer getting the repository at this point - as we should only retrieve it if we're going to call update
+	var repo *models.Repository
+
+	updates := make([]*repo_module.PushUpdateOptions, 0, len(opts.OldCommitIDs))
+	wasEmpty := false
+
+	for i := range opts.OldCommitIDs {
+		refFullName := opts.RefFullNames[i]
+
+		// Only trigger activity updates for changes to branches or
+		// tags.  Updates to other refs (eg, refs/notes, refs/changes,
+		// or other less-standard refs spaces are ignored since there
+		// may be a very large number of them).
+		if strings.HasPrefix(refFullName, git.BranchPrefix) || strings.HasPrefix(refFullName, git.TagPrefix) {
+			if repo == nil {
+				repo = loadRepository(ctx, ownerName, repoName)
+				if ctx.Written() {
+					// Error handled in loadRepository
+					return
+				}
+				wasEmpty = repo.IsEmpty
+			}
+
+			option := repo_module.PushUpdateOptions{
+				RefFullName:  refFullName,
+				OldCommitID:  opts.OldCommitIDs[i],
+				NewCommitID:  opts.NewCommitIDs[i],
+				PusherID:     opts.UserID,
+				PusherName:   opts.UserName,
+				RepoUserName: ownerName,
+				RepoName:     repoName,
+			}
+			updates = append(updates, &option)
+			if repo.IsEmpty && option.IsBranch() && (option.BranchName() == "master" || option.BranchName() == "main") {
+				// put the master/main branch first
+				copy(updates[1:], updates)
+				updates[0] = &option
+			}
+		}
+	}
+
+	if repo != nil && len(updates) > 0 {
+		if err := repo_service.PushUpdates(updates); err != nil {
+			log.Error("Failed to Update: %s/%s Total Updates: %d", ownerName, repoName, len(updates))
+			for i, update := range updates {
+				log.Error("Failed to Update: %s/%s Update: %d/%d: Branch: %s", ownerName, repoName, i, len(updates), update.BranchName())
+			}
+			log.Error("Failed to Update: %s/%s Error: %v", ownerName, repoName, err)
+
+			ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
+				Err: fmt.Sprintf("Failed to Update: %s/%s Error: %v", ownerName, repoName, err),
+			})
+			return
+		}
+	}
+
+	// Handle Push Options
+	if len(opts.GitPushOptions) > 0 {
+		// load the repository
+		if repo == nil {
+			repo = loadRepository(ctx, ownerName, repoName)
+			if ctx.Written() {
+				// Error handled in loadRepository
+				return
+			}
+			wasEmpty = repo.IsEmpty
+		}
+
+		repo.IsPrivate = opts.GitPushOptions.Bool(private.GitPushOptionRepoPrivate, repo.IsPrivate)
+		repo.IsTemplate = opts.GitPushOptions.Bool(private.GitPushOptionRepoTemplate, repo.IsTemplate)
+		if err := models.UpdateRepositoryCols(repo, "is_private", "is_template"); err != nil {
+			log.Error("Failed to Update: %s/%s Error: %v", ownerName, repoName, err)
+			ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
+				Err: fmt.Sprintf("Failed to Update: %s/%s Error: %v", ownerName, repoName, err),
+			})
+		}
+	}
+
+	results := make([]private.HookPostReceiveBranchResult, 0, len(opts.OldCommitIDs))
+
+	// We have to reload the repo in case its state is changed above
+	repo = nil
+	var baseRepo *models.Repository
+
+	// Now handle the pull request notification trailers
+	for i := range opts.OldCommitIDs {
+		refFullName := opts.RefFullNames[i]
+		newCommitID := opts.NewCommitIDs[i]
+
+		branch := git.RefEndName(opts.RefFullNames[i])
+
+		// If we've pushed a branch (and not deleted it)
+		if newCommitID != git.EmptySHA && strings.HasPrefix(refFullName, git.BranchPrefix) {
+
+			// First ensure we have the repository loaded, we're allowed pulls requests and we can get the base repo
+			if repo == nil {
+				repo = loadRepository(ctx, ownerName, repoName)
+				if ctx.Written() {
+					return
+				}
+
+				if !repo.AllowsPulls() {
+					// We can stop there's no need to go any further
+					ctx.JSON(http.StatusOK, private.HookPostReceiveResult{
+						RepoWasEmpty: wasEmpty,
+					})
+					return
+				}
+				baseRepo = repo
+
+				if repo.IsFork {
+					if err := repo.GetBaseRepo(); err != nil {
+						log.Error("Failed to get Base Repository of Forked repository: %-v Error: %v", repo, err)
+						ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
+							Err:          fmt.Sprintf("Failed to get Base Repository of Forked repository: %-v Error: %v", repo, err),
+							RepoWasEmpty: wasEmpty,
+						})
+						return
+					}
+					baseRepo = repo.BaseRepo
+				}
+			}
+
+			// If our branch is the default branch of an unforked repo - there's no PR to create or refer to
+			if !repo.IsFork && branch == baseRepo.DefaultBranch {
+				results = append(results, private.HookPostReceiveBranchResult{})
+				continue
+			}
+
+			pr, err := models.GetUnmergedPullRequest(repo.ID, baseRepo.ID, branch, baseRepo.DefaultBranch, models.PullRequestFlowGithub)
+			if err != nil && !models.IsErrPullRequestNotExist(err) {
+				log.Error("Failed to get active PR in: %-v Branch: %s to: %-v Branch: %s Error: %v", repo, branch, baseRepo, baseRepo.DefaultBranch, err)
+				ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
+					Err: fmt.Sprintf(
+						"Failed to get active PR in: %-v Branch: %s to: %-v Branch: %s Error: %v", repo, branch, baseRepo, baseRepo.DefaultBranch, err),
+					RepoWasEmpty: wasEmpty,
+				})
+				return
+			}
+
+			if pr == nil {
+				if repo.IsFork {
+					branch = fmt.Sprintf("%s:%s", repo.OwnerName, branch)
+				}
+				results = append(results, private.HookPostReceiveBranchResult{
+					Message: setting.Git.PullRequestPushMessage && repo.AllowsPulls(),
+					Create:  true,
+					Branch:  branch,
+					URL:     fmt.Sprintf("%s/compare/%s...%s", baseRepo.HTMLURL(), util.PathEscapeSegments(baseRepo.DefaultBranch), util.PathEscapeSegments(branch)),
+				})
+			} else {
+				results = append(results, private.HookPostReceiveBranchResult{
+					Message: setting.Git.PullRequestPushMessage && repo.AllowsPulls(),
+					Create:  false,
+					Branch:  branch,
+					URL:     fmt.Sprintf("%s/pulls/%d", baseRepo.HTMLURL(), pr.Index),
+				})
+			}
+		}
+	}
+	ctx.JSON(http.StatusOK, private.HookPostReceiveResult{
+		Results:      results,
+		RepoWasEmpty: wasEmpty,
+	})
+}

routers/private/hook_proc_receive.go

@@ -0,0 +1,34 @@
+// Copyright 2021 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 private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead.
+package private
+
+import (
+	"net/http"
+
+	gitea_context "code.gitea.io/gitea/modules/context"
+	"code.gitea.io/gitea/modules/git"
+	"code.gitea.io/gitea/modules/private"
+	"code.gitea.io/gitea/modules/web"
+	"code.gitea.io/gitea/services/agit"
+)
+
+// HookProcReceive proc-receive hook - only handles agit Proc-Receive requests at present
+func HookProcReceive(ctx *gitea_context.PrivateContext) {
+	opts := web.GetForm(ctx).(*private.HookOptions)
+	if !git.SupportProcReceive {
+		ctx.Status(http.StatusNotFound)
+		return
+	}
+
+	results := agit.ProcRecive(ctx, opts)
+	if ctx.Written() {
+		return
+	}
+
+	ctx.JSON(http.StatusOK, private.HookProcReceiveResult{
+		Results: results,
+	})
+}

routers/private/hook_verification.go

@@ -0,0 +1,122 @@
+// Copyright 2021 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 private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead.
+package private
+
+import (
+	"bufio"
+	"context"
+	"fmt"
+	"io"
+	"os"
+
+	"code.gitea.io/gitea/models"
+	"code.gitea.io/gitea/modules/git"
+	"code.gitea.io/gitea/modules/log"
+)
+
+// _________                        .__  __
+// \_   ___ \  ____   _____   _____ |__|/  |_
+// /    \  \/ /  _ \ /     \ /     \|  \   __\
+// \     \___(  <_> )  Y Y  \  Y Y  \  ||  |
+//  \______  /\____/|__|_|  /__|_|  /__||__|
+//         \/             \/      \/
+// ____   ____           .__  _____.__               __  .__
+// \   \ /   /___________|__|/ ____\__| ____ _____ _/  |_|__| ____   ____
+//  \   Y   // __ \_  __ \  \   __\|  |/ ___\\__  \\   __\  |/  _ \ /    \
+//   \     /\  ___/|  | \/  ||  |  |  \  \___ / __ \|  | |  (  <_> )   |  \
+//    \___/  \___  >__|  |__||__|  |__|\___  >____  /__| |__|\____/|___|  /
+//               \/                        \/     \/                    \/
+//
+// This file contains commit verification functions for refs passed across in hooks
+
+func verifyCommits(oldCommitID, newCommitID string, repo *git.Repository, env []string) error {
+	stdoutReader, stdoutWriter, err := os.Pipe()
+	if err != nil {
+		log.Error("Unable to create os.Pipe for %s", repo.Path)
+		return err
+	}
+	defer func() {
+		_ = stdoutReader.Close()
+		_ = stdoutWriter.Close()
+	}()
+
+	// This is safe as force pushes are already forbidden
+	err = git.NewCommand("rev-list", oldCommitID+"..."+newCommitID).
+		RunInDirTimeoutEnvFullPipelineFunc(env, -1, repo.Path,
+			stdoutWriter, nil, nil,
+			func(ctx context.Context, cancel context.CancelFunc) error {
+				_ = stdoutWriter.Close()
+				err := readAndVerifyCommitsFromShaReader(stdoutReader, repo, env)
+				if err != nil {
+					log.Error("%v", err)
+					cancel()
+				}
+				_ = stdoutReader.Close()
+				return err
+			})
+	if err != nil && !isErrUnverifiedCommit(err) {
+		log.Error("Unable to check commits from %s to %s in %s: %v", oldCommitID, newCommitID, repo.Path, err)
+	}
+	return err
+}
+
+func readAndVerifyCommitsFromShaReader(input io.ReadCloser, repo *git.Repository, env []string) error {
+	scanner := bufio.NewScanner(input)
+	for scanner.Scan() {
+		line := scanner.Text()
+		err := readAndVerifyCommit(line, repo, env)
+		if err != nil {
+			log.Error("%v", err)
+			return err
+		}
+	}
+	return scanner.Err()
+}
+
+func readAndVerifyCommit(sha string, repo *git.Repository, env []string) error {
+	stdoutReader, stdoutWriter, err := os.Pipe()
+	if err != nil {
+		log.Error("Unable to create pipe for %s: %v", repo.Path, err)
+		return err
+	}
+	defer func() {
+		_ = stdoutReader.Close()
+		_ = stdoutWriter.Close()
+	}()
+	hash := git.MustIDFromString(sha)
+
+	return git.NewCommand("cat-file", "commit", sha).
+		RunInDirTimeoutEnvFullPipelineFunc(env, -1, repo.Path,
+			stdoutWriter, nil, nil,
+			func(ctx context.Context, cancel context.CancelFunc) error {
+				_ = stdoutWriter.Close()
+				commit, err := git.CommitFromReader(repo, hash, stdoutReader)
+				if err != nil {
+					return err
+				}
+				verification := models.ParseCommitWithSignature(commit)
+				if !verification.Verified {
+					cancel()
+					return &errUnverifiedCommit{
+						commit.ID.String(),
+					}
+				}
+				return nil
+			})
+}
+
+type errUnverifiedCommit struct {
+	sha string
+}
+
+func (e *errUnverifiedCommit) Error() string {
+	return fmt.Sprintf("Unverified commit: %s", e.sha)
+}
+
+func isErrUnverifiedCommit(err error) bool {
+	_, ok := err.(*errUnverifiedCommit)
+	return ok
+}

routers/private/internal.go

@@ -56,10 +56,10 @@ func Routes() *web.Route {
 	r.Post("/ssh/authorized_keys", AuthorizedPublicKeyByContent)
 	r.Post("/ssh/{id}/update/{repoid}", UpdatePublicKeyInRepo)
 	r.Post("/ssh/log", bind(private.SSHLogOption{}), SSHLog)
-	r.Post("/hook/pre-receive/{owner}/{repo}", bind(private.HookOptions{}), HookPreReceive)
+	r.Post("/hook/pre-receive/{owner}/{repo}", RepoAssignment, bind(private.HookOptions{}), HookPreReceive)
 	r.Post("/hook/post-receive/{owner}/{repo}", bind(private.HookOptions{}), HookPostReceive)
-	r.Post("/hook/proc-receive/{owner}/{repo}", bind(private.HookOptions{}), HookProcReceive)
-	r.Post("/hook/set-default-branch/{owner}/{repo}/{branch}", SetDefaultBranch)
+	r.Post("/hook/proc-receive/{owner}/{repo}", RepoAssignment, bind(private.HookOptions{}), HookProcReceive)
+	r.Post("/hook/set-default-branch/{owner}/{repo}/{branch}", RepoAssignment, SetDefaultBranch)
 	r.Get("/serv/none/{keyid}", ServNoCommand)
 	r.Get("/serv/command/{keyid}/{owner}/{repo}", ServCommand)
 	r.Post("/manager/shutdown", Shutdown)

routers/private/internal_repo.go

@@ -0,0 +1,84 @@
+// Copyright 2021 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 private includes all internal routes. The package name internal is ideal but Golang is not allowed, so we use private as package name instead.
+package private
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+
+	"code.gitea.io/gitea/models"
+	gitea_context "code.gitea.io/gitea/modules/context"
+	"code.gitea.io/gitea/modules/git"
+	"code.gitea.io/gitea/modules/log"
+)
+
+// __________
+// \______   \ ____ ______   ____
+//  |       _// __ \\____ \ /  _ \
+//  |    |   \  ___/|  |_> >  <_> )
+//  |____|_  /\___  >   __/ \____/
+//         \/     \/|__|
+//    _____                .__                                     __
+//   /  _  \   ______ _____|__| ____   ____   _____   ____   _____/  |_
+//  /  /_\  \ /  ___//  ___/  |/ ___\ /    \ /     \_/ __ \ /    \   __\
+// /    |    \\___ \ \___ \|  / /_/  >   |  \  Y Y  \  ___/|   |  \  |
+// \____|__  /____  >____  >__\___  /|___|  /__|_|  /\___  >___|  /__|
+//         \/     \/     \/  /_____/      \/      \/     \/     \/
+
+// This file contains common functions relating to setting the Repository for the
+// internal routes
+
+// RepoAssignment assigns the repository and gitrepository to the private context
+func RepoAssignment(ctx *gitea_context.PrivateContext) context.CancelFunc {
+	ownerName := ctx.Params(":owner")
+	repoName := ctx.Params(":repo")
+
+	repo := loadRepository(ctx, ownerName, repoName)
+	if ctx.Written() {
+		// Error handled in loadRepository
+		return nil
+	}
+
+	gitRepo, err := git.OpenRepository(repo.RepoPath())
+	if err != nil {
+		log.Error("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err)
+		ctx.JSON(http.StatusInternalServerError, map[string]interface{}{
+			"Err": fmt.Sprintf("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err),
+		})
+		return nil
+	}
+
+	ctx.Repo = &gitea_context.Repository{
+		Repository: repo,
+		GitRepo:    gitRepo,
+	}
+
+	// We opened it, we should close it
+	cancel := func() {
+		// If it's been set to nil then assume someone else has closed it.
+		if ctx.Repo.GitRepo != nil {
+			ctx.Repo.GitRepo.Close()
+		}
+	}
+
+	return cancel
+}
+
+func loadRepository(ctx *gitea_context.PrivateContext, ownerName, repoName string) *models.Repository {
+	repo, err := models.GetRepositoryByOwnerAndName(ownerName, repoName)
+	if err != nil {
+		log.Error("Failed to get repository: %s/%s Error: %v", ownerName, repoName, err)
+		ctx.JSON(http.StatusInternalServerError, map[string]interface{}{
+			"Err": fmt.Sprintf("Failed to get repository: %s/%s Error: %v", ownerName, repoName, err),
+		})
+		return nil
+	}
+	if repo.OwnerName == "" {
+		repo.OwnerName = ownerName
+	}
+	return repo
+}