#6946 Run hooks on merge/edit and cope with protected branches (#6961)

* Fix #6946 by checking PullRequest ID on pushing

* Ensure we have the owner name, the pr attributes and the the issue

* Fix TestSearchRepo by waiting till indexing is done

* Update integrations/repo_search_test.go

* changes as per @mrsdizzie

* missing comma

* Spelling mistake

* Fix full pushing environment

cmd/hook.go

@@ -65,6 +65,7 @@ func runHookPreReceive(c *cli.Context) error {
 	username := os.Getenv(models.EnvRepoUsername)
 	reponame := os.Getenv(models.EnvRepoName)
 	userID, _ := strconv.ParseInt(os.Getenv(models.EnvPusherID), 10, 64)
+	prID, _ := strconv.ParseInt(os.Getenv(models.ProtectedBranchPRID), 10, 64)
 
 	buf := bytes.NewBuffer(nil)
 	scanner := bufio.NewScanner(os.Stdin)
@@ -95,6 +96,7 @@ func runHookPreReceive(c *cli.Context) error {
 				UserID:                          userID,
 				GitAlternativeObjectDirectories: os.Getenv(private.GitAlternativeObjectDirectories),
 				GitObjectDirectory:              os.Getenv(private.GitObjectDirectory),
+				ProtectedBranchID:               prID,
 			})
 			switch statusCode {
 			case http.StatusInternalServerError:

cmd/serv.go

@@ -191,6 +191,7 @@ func runServ(c *cli.Context) error {
 	os.Setenv(models.EnvPusherName, results.UserName)
 	os.Setenv(models.EnvPusherID, strconv.FormatInt(results.UserID, 10))
 	os.Setenv(models.ProtectedBranchRepoID, strconv.FormatInt(results.RepoID, 10))
+	os.Setenv(models.ProtectedBranchPRID, fmt.Sprintf("%d", 0))
 
 	//LFS token authentication
 	if verb == lfsAuthenticateVerb {
@@ -239,8 +240,6 @@ func runServ(c *cli.Context) error {
 		gitcmd = exec.Command(verb, repoPath)
 	}
 
-	os.Setenv(models.ProtectedBranchRepoID, fmt.Sprintf("%d", results.RepoID))
-
 	gitcmd.Dir = setting.RepoRootPath
 	gitcmd.Stdout = os.Stdout
 	gitcmd.Stdin = os.Stdin

models/branches.go

@@ -19,6 +19,8 @@ import (
 const (
 	// ProtectedBranchRepoID protected Repo ID
 	ProtectedBranchRepoID = "GITEA_REPO_ID"
+	// ProtectedBranchPRID protected Repo PR ID
+	ProtectedBranchPRID = "GITEA_PR_ID"
 )
 
 // ProtectedBranch struct

models/helper_environment.go

@@ -12,26 +12,34 @@ import (
 
 // PushingEnvironment returns an os environment to allow hooks to work on push
 func PushingEnvironment(doer *User, repo *Repository) []string {
+	return FullPushingEnvironment(doer, doer, repo, 0)
+}
+
+// FullPushingEnvironment returns an os environment to allow hooks to work on push
+func FullPushingEnvironment(author, committer *User, repo *Repository, prID int64) []string {
 	isWiki := "false"
 	if strings.HasSuffix(repo.Name, ".wiki") {
 		isWiki = "true"
 	}
 
-	sig := doer.NewGitSig()
+	authorSig := author.NewGitSig()
+	committerSig := committer.NewGitSig()
 
 	// We should add "SSH_ORIGINAL_COMMAND=gitea-internal",
 	// once we have hook and pushing infrastructure working correctly
 	return append(os.Environ(),
-		"GIT_AUTHOR_NAME="+sig.Name,
+		"GIT_AUTHOR_NAME="+authorSig.Name,
-		"GIT_AUTHOR_EMAIL="+sig.Email,
+		"GIT_AUTHOR_EMAIL="+authorSig.Email,
-		"GIT_COMMITTER_NAME="+sig.Name,
+		"GIT_COMMITTER_NAME="+committerSig.Name,
-		"GIT_COMMITTER_EMAIL="+sig.Email,
+		"GIT_COMMITTER_EMAIL="+committerSig.Email,
 		EnvRepoName+"="+repo.Name,
 		EnvRepoUsername+"="+repo.MustOwnerName(),
 		EnvRepoIsWiki+"="+isWiki,
-		EnvPusherName+"="+doer.Name,
+		EnvPusherName+"="+committer.Name,
-		EnvPusherID+"="+fmt.Sprintf("%d", doer.ID),
+		EnvPusherID+"="+fmt.Sprintf("%d", committer.ID),
 		ProtectedBranchRepoID+"="+fmt.Sprintf("%d", repo.ID),
+		ProtectedBranchPRID+"="+fmt.Sprintf("%d", prID),
+		"SSH_ORIGINAL_COMMAND=gitea-internal",
 	)
 
 }

models/pull.go

@@ -876,7 +876,7 @@ func (pr *PullRequest) UpdatePatch() (err error) {
 	if err = pr.GetHeadRepo(); err != nil {
 		return fmt.Errorf("GetHeadRepo: %v", err)
 	} else if pr.HeadRepo == nil {
-		log.Trace("PullRequest[%d].UpdatePatch: ignored cruppted data", pr.ID)
+		log.Trace("PullRequest[%d].UpdatePatch: ignored corrupted data", pr.ID)
 		return nil
 	}
 

modules/private/hook.go

@@ -29,11 +29,12 @@ type HookOptions struct {
 	UserName                        string
 	GitObjectDirectory              string
 	GitAlternativeObjectDirectories string
+	ProtectedBranchID               int64
 }
 
 // HookPreReceive check whether the provided commits are allowed
 func HookPreReceive(ownerName, repoName string, opts HookOptions) (int, string) {
-	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/pre-receive/%s/%s?old=%s&new=%s&ref=%s&userID=%d&gitObjectDirectory=%s&gitAlternativeObjectDirectories=%s",
+	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/pre-receive/%s/%s?old=%s&new=%s&ref=%s&userID=%d&gitObjectDirectory=%s&gitAlternativeObjectDirectories=%s&prID=%d",
 		url.PathEscape(ownerName),
 		url.PathEscape(repoName),
 		url.QueryEscape(opts.OldCommitID),
@@ -42,6 +43,7 @@ func HookPreReceive(ownerName, repoName string, opts HookOptions) (int, string)
 		opts.UserID,
 		url.QueryEscape(opts.GitObjectDirectory),
 		url.QueryEscape(opts.GitAlternativeObjectDirectories),
+		opts.ProtectedBranchID,
 	)
 
 	resp, err := newInternalRequest(reqURL, "GET").Response()

modules/pull/merge.go

@@ -231,7 +231,17 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor
 		}
 	}
 
-	env := models.PushingEnvironment(doer, pr.BaseRepo)
+	headUser, err := models.GetUserByName(pr.HeadUserName)
+	if err != nil {
+		if !models.IsErrUserNotExist(err) {
+			log.Error("Can't find user: %s for head repository - %v", pr.HeadUserName, err)
+			return err
+		}
+		log.Error("Can't find user: %s for head repository - defaulting to doer: %s - %v", pr.HeadUserName, doer.Name, err)
+		headUser = doer
+	}
+
+	env := models.FullPushingEnvironment(headUser, doer, pr.BaseRepo, pr.ID)
 
 	// Push back to upstream.
 	if err := git.NewCommand("push", "origin", pr.BaseBranch).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, nil, &errbuf); err != nil {

routers/private/hook.go

@@ -31,6 +31,7 @@ func HookPreReceive(ctx *macaron.Context) {
 	userID := ctx.QueryInt64("userID")
 	gitObjectDirectory := ctx.QueryTrim("gitObjectDirectory")
 	gitAlternativeObjectDirectories := ctx.QueryTrim("gitAlternativeObjectDirectories")
+	prID := ctx.QueryInt64("prID")
 
 	branchName := strings.TrimPrefix(refFullName, git.BranchPrefix)
 	repo, err := models.GetRepositoryByOwnerAndName(ownerName, repoName)
@@ -85,7 +86,24 @@ func HookPreReceive(ctx *macaron.Context) {
 			}
 		}
 
-		if !protectBranch.CanUserPush(userID) {
+		canPush := protectBranch.CanUserPush(userID)
+		if !canPush && prID > 0 {
+			pr, err := models.GetPullRequestByID(prID)
+			if err != nil {
+				log.Error("Unable to get PullRequest %d Error: %v", prID, err)
+				ctx.JSON(http.StatusInternalServerError, map[string]interface{}{
+					"err": fmt.Sprintf("Unable to get PullRequest %d Error: %v", prID, err),
+				})
+				return
+			}
+			if !protectBranch.HasEnoughApprovals(pr) {
+				log.Warn("Forbidden: User %d cannot push to protected branch: %s in %-v and pr #%d does not have enough approvals", userID, branchName, repo, pr.Index)
+				ctx.JSON(http.StatusForbidden, map[string]interface{}{
+					"err": fmt.Sprintf("protected branch %s can not be pushed to and pr #%d does not have enough approvals", branchName, prID),
+				})
+				return
+			}
+		} else if !canPush {
 			log.Warn("Forbidden: User %d cannot push to protected branch: %s in %-v", userID, branchName, repo)
 			ctx.JSON(http.StatusForbidden, map[string]interface{}{
 				"err": fmt.Sprintf("protected branch %s can not be pushed to", branchName),