Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 22 commits into from
Jul 1, 2019
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
71467f9
Fix #6946 by checking PullRequest ID on pushing
zeripath May 15, 2019
fca3a51
Ensure we have the owner name, the pr attributes and the the issue
zeripath May 15, 2019
4d68e21
Merge branch 'master' into fix-#6946-pass-in-pr-into-hooks
zeripath May 16, 2019
56d7a2f
Merge branch 'master' into fix-#6946-pass-in-pr-into-hooks
zeripath May 17, 2019
1271a3b
Merge branch 'master' into fix-#6946-pass-in-pr-into-hooks
zeripath May 17, 2019
5e1d436
Merge branch 'master' into fix-#6946-pass-in-pr-into-hooks
zeripath May 19, 2019
1f9915d
Merge branch 'master' into fix-#6946-pass-in-pr-into-hooks
zeripath May 20, 2019
dde55bd
Fix TestSearchRepo by waiting till indexing is done
zeripath May 20, 2019
f7fba98
Update integrations/repo_search_test.go
zeripath May 21, 2019
cb9b7bb
changes as per @mrsdizzie
zeripath May 21, 2019
a90466b
Merge branch 'fix-#6946-pass-in-pr-into-hooks' of github.com:zeripath…
zeripath May 21, 2019
fefc60f
Merge branch 'master' into fix-#6946-pass-in-pr-into-hooks
zeripath May 21, 2019
604117c
Merge branch 'master' into fix-#6946-pass-in-pr-into-hooks
zeripath May 23, 2019
5b63303
Merge branch 'master' into fix-#6946-pass-in-pr-into-hooks
zeripath Jun 2, 2019
10773f1
missing comma
zeripath Jun 2, 2019
46d8b2e
Spelling mistake
zeripath Jun 2, 2019
757c395
Merge branch 'master' into fix-#6946-pass-in-pr-into-hooks
zeripath Jun 3, 2019
4feeb65
Merge branch 'master' into fix-#6946-pass-in-pr-into-hooks
zeripath Jun 13, 2019
48c4bbd
Merge branch 'master' into fix-#6946-pass-in-pr-into-hooks
zeripath Jun 17, 2019
172cf28
Merge branch 'master' into fix-#6946-pass-in-pr-into-hooks
zeripath Jun 27, 2019
c015839
Fix full pushing environment
zeripath Jun 27, 2019
77e8790
Merge branch 'master' into fix-#6946-pass-in-pr-into-hooks
techknowlogick Jun 30, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cmd/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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:
Expand Down
3 changes: 1 addition & 2 deletions cmd/serv.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
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
22 changes: 15 additions & 7 deletions models/helper_environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)

}
2 changes: 1 addition & 1 deletion models/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 3 additions & 1 deletion modules/private/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we get the prID from routers but not envs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately not. Remember the hooks are called from within git.

url.PathEscape(ownerName),
url.PathEscape(repoName),
url.QueryEscape(opts.OldCommitID),
Expand All @@ -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()
Expand Down
12 changes: 11 additions & 1 deletion modules/pull/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
20 changes: 19 additions & 1 deletion routers/private/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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),
Expand Down