From 3563650bdb273045c55400ee9520a0f78fc90e76 Mon Sep 17 00:00:00 2001 From: zeripath Date: Mon, 1 Jul 2019 02:18:13 +0100 Subject: [PATCH] #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 | 2 ++ cmd/serv.go | 3 +-- models/branches.go | 2 ++ models/helper_environment.go | 22 +++++++++++++++------- models/pull.go | 2 +- modules/private/hook.go | 4 +++- modules/pull/merge.go | 12 +++++++++++- routers/private/hook.go | 20 +++++++++++++++++++- 8 files changed, 54 insertions(+), 13 deletions(-) diff --git a/cmd/hook.go b/cmd/hook.go index b3e900afee4d..ca876f02a315 100644 --- a/cmd/hook.go +++ b/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: diff --git a/cmd/serv.go b/cmd/serv.go index 2ea89757dba8..32dd8cbd3e6f 100644 --- a/cmd/serv.go +++ b/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 diff --git a/models/branches.go b/models/branches.go index df3b69aa2188..fa4215d6a03a 100644 --- a/models/branches.go +++ b/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 diff --git a/models/helper_environment.go b/models/helper_environment.go index 199eb6062d52..0460fc3df50c 100644 --- a/models/helper_environment.go +++ b/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_EMAIL="+sig.Email, - "GIT_COMMITTER_NAME="+sig.Name, - "GIT_COMMITTER_EMAIL="+sig.Email, + "GIT_AUTHOR_NAME="+authorSig.Name, + "GIT_AUTHOR_EMAIL="+authorSig.Email, + "GIT_COMMITTER_NAME="+committerSig.Name, + "GIT_COMMITTER_EMAIL="+committerSig.Email, EnvRepoName+"="+repo.Name, EnvRepoUsername+"="+repo.MustOwnerName(), EnvRepoIsWiki+"="+isWiki, - EnvPusherName+"="+doer.Name, - EnvPusherID+"="+fmt.Sprintf("%d", doer.ID), + EnvPusherName+"="+committer.Name, + EnvPusherID+"="+fmt.Sprintf("%d", committer.ID), ProtectedBranchRepoID+"="+fmt.Sprintf("%d", repo.ID), + ProtectedBranchPRID+"="+fmt.Sprintf("%d", prID), + "SSH_ORIGINAL_COMMAND=gitea-internal", ) } diff --git a/models/pull.go b/models/pull.go index eac36235bb5f..6ee2ec555d98 100644 --- a/models/pull.go +++ b/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 } diff --git a/modules/private/hook.go b/modules/private/hook.go index 7e2a475d4b8a..caa38195559f 100644 --- a/modules/private/hook.go +++ b/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() diff --git a/modules/pull/merge.go b/modules/pull/merge.go index e12ede81d630..5685336a2bb9 100644 --- a/modules/pull/merge.go +++ b/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 { diff --git a/routers/private/hook.go b/routers/private/hook.go index 3da5e38edb50..1071c57bc075 100644 --- a/routers/private/hook.go +++ b/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),