Skip to content

Commit

Permalink
Fix #6946 by checking PullRequest ID on pushing
Browse files Browse the repository at this point in the history
  • Loading branch information
zeripath committed May 15, 2019
1 parent 7dd9837 commit 71467f9
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 9 deletions.
4 changes: 4 additions & 0 deletions cmd/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ func runHookPreReceive(c *cli.Context) error {
reponame := os.Getenv(models.EnvRepoName)
userIDStr := os.Getenv(models.EnvPusherID)
repoPath := models.RepoPath(username, reponame)
prID, _ := strconv.ParseInt(os.Getenv(models.ProtectedBranchPRID), 10, 64)

buf := bytes.NewBuffer(nil)
scanner := bufio.NewScanner(os.Stdin)
Expand Down Expand Up @@ -115,6 +116,9 @@ func runHookPreReceive(c *cli.Context) error {

userID, _ := strconv.ParseInt(userIDStr, 10, 64)
canPush, err := private.CanUserPush(protectBranch.ID, userID)
if err == nil && !canPush {
canPush, err = private.HasEnoughApprovals(protectBranch.ID, prID)
}
if err != nil {
fail("Internal error", "Fail to detect user can push: %v", err)
} else if !canPush {
Expand Down
1 change: 1 addition & 0 deletions cmd/serv.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ func runServ(c *cli.Context) error {
}

os.Setenv(models.ProtectedBranchRepoID, fmt.Sprintf("%d", repo.ID))
os.Setenv(models.ProtectedBranchPRID, fmt.Sprintf("%d", 0))

gitcmd.Dir = setting.RepoRootPath
gitcmd.Stdout = os.Stdout
Expand Down
2 changes: 2 additions & 0 deletions models/branches.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 14 additions & 7 deletions models/helper_environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,31 @@ 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()

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.OwnerName,
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",
)

Expand Down
4 changes: 2 additions & 2 deletions models/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,7 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle
return ErrInvalidMergeStyle{pr.BaseRepo.ID, mergeStyle}
}

env := PushingEnvironment(doer, pr.BaseRepo)
env := FullPushingEnvironment(doer, doer, pr.BaseRepo, pr.ID)

// Push back to upstream.
if err := git.NewCommand("push", baseGitRepo.Path, pr.BaseBranch).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, nil, &errbuf); err != nil {
Expand Down Expand Up @@ -1123,7 +1123,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
}

Expand Down
30 changes: 30 additions & 0 deletions modules/private/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,33 @@ func CanUserPush(protectedBranchID, userID int64) (bool, error) {

return canPush["can_push"].(bool), nil
}

// HasEnoughApprovals returns if true if pr has enough granted approvals.
func HasEnoughApprovals(protectedBranchID, prID int64) (bool, error) {
if prID <= 0 {
return false, nil
}

// Ask for running deliver hook and test pull request tasks.
reqURL := setting.LocalURL + fmt.Sprintf("api/internal/protectedbranchpr/%d/%d", protectedBranchID, prID)
log.GitLogger.Trace("CanUserPush: %s", reqURL)

resp, err := newInternalRequest(reqURL, "GET").Response()
if err != nil {
return false, err
}

var canPush = make(map[string]interface{})
if err := json.NewDecoder(resp.Body).Decode(&canPush); err != nil {
return false, err
}

defer resp.Body.Close()

// All 2XX status codes are accepted and others will return an error
if resp.StatusCode/100 != 2 {
return false, fmt.Errorf("Failed to retrieve push user: %s", decodeJSONError(resp).Err)
}

return canPush["can_push"].(bool), nil
}
29 changes: 29 additions & 0 deletions routers/private/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,32 @@ func CanUserPush(ctx *macaron.Context) {
})
}
}

// HasEnoughApprovals return if PR has enough approvals
func HasEnoughApprovals(ctx *macaron.Context) {
pbID := ctx.ParamsInt64(":pbid")
prID := ctx.ParamsInt64(":prid")

protectBranch, err := models.GetProtectedBranchByID(pbID)
if err != nil {
ctx.JSON(500, map[string]interface{}{
"err": err.Error(),
})
return
} else if prID > 0 && protectBranch != nil {
pr, err := models.GetPullRequestByID(prID)
if err != nil {
ctx.JSON(500, map[string]interface{}{
"err": err.Error(),
})
return
}
ctx.JSON(200, map[string]interface{}{
"can_push": protectBranch.HasEnoughApprovals(pr),
})
} else {
ctx.JSON(200, map[string]interface{}{
"can_push": false,
})
}
}
1 change: 1 addition & 0 deletions routers/private/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ func RegisterRoutes(m *macaron.Macaron) {
m.Get("/repositories/:repoid/wiki/init", InitWiki)
m.Post("/push/update", PushUpdate)
m.Get("/protectedbranch/:pbid/:userid", CanUserPush)
m.Get("/protectedbranchpr/:pbid/:prid", HasEnoughApprovals)
m.Get("/repo/:owner/:repo", GetRepositoryByOwnerAndName)
m.Get("/branch/:id/*", GetProtectedBranchBy)
m.Get("/repository/:rid", GetRepository)
Expand Down

0 comments on commit 71467f9

Please sign in to comment.