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

zeripath · 2019-05-15T19:19:18Z

Fix #6946 by setting and checking PullRequest ID on pushing if we're pushing from a PR.
Fixes #6003.

codecov-io · 2019-05-15T19:58:39Z

Codecov Report

Merging #6961 into master will increase coverage by 0.05%.
The diff coverage is 41.3%.

@@            Coverage Diff             @@
##           master    #6961      +/-   ##
==========================================
+ Coverage   41.24%   41.29%   +0.05%     
==========================================
  Files         466      466              
  Lines       63153    63186      +33     
==========================================
+ Hits        26048    26095      +47     
+ Misses      33700    33680      -20     
- Partials     3405     3411       +6

Impacted Files	Coverage Δ
models/branches.go	`50% <ø> (ø)`	⬆️
cmd/serv.go	`0% <0%> (ø)`	⬆️
cmd/hook.go	`0% <0%> (ø)`	⬆️
models/pull.go	`56.1% <0%> (+5.16%)`	⬆️
modules/private/hook.go	`0% <0%> (ø)`	⬆️
models/helper_environment.go	`88.46% <100%> (+2.74%)`	⬆️
modules/pull/merge.go	`38.52% <22.22%> (-0.84%)`	⬇️
routers/private/hook.go	`48.14% <26.31%> (-2.73%)`	⬇️
models/repo_indexer.go	`67.08% <0%> (-2.09%)`	⬇️
models/repo_list.go	`72.08% <0%> (-1.02%)`	⬇️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e5a4d78...77e8790. Read the comment docs.

zeripath · 2019-05-15T20:59:14Z

OK two things:

I'm getting a deadlock on merges which is interesting because I don't think that should be happening.
I'm also getting an error on the tests.

~~Right I think I'll leave this as WIP but open a second PR to stop running the push hooks on merge. ~~

These are fixed.

zeripath · 2019-05-17T16:30:59Z

OK I've discovered the issue here, TestSearchRepos is broken.

zeripath · 2019-05-20T21:40:24Z

Now working after applying #7004 to fix that broken test

integrations/repo_search_test.go

…zeripath/gitea into fix-go-gitea#6946-pass-in-pr-into-hooks

lunny · 2019-06-02T15:41:10Z

modules/private/hook.go

 }

 // 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",


Could we get the prID from routers but not envs?

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

lunny · 2019-06-26T11:41:16Z

Please resolve conflicts

lunny · 2019-06-12T06:59:34Z

models/pull.go

@@ -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)


author should be pullrequest's poster

zeripath · 2019-06-27T18:22:08Z

Conflicts resolved.

zeripath · 2019-06-27T20:15:17Z

So the next step once we have this PR is to enforce that we have to have an appropriate Gitea environment before allowing pushing to gitea repositories - this will prevent the numerous issues that we get whereby people abuse Gitea repositories by pushing outside of Gitea.

@mrsdizzie

go-gitea#6961) * Fix go-gitea#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

stealthybox · 2019-08-12T18:17:17Z

glad to see this fixed in gitea
thank you @zeripath 🌮

related gogs issue gogs/gogs#5120

Fix go-gitea#6946 by checking PullRequest ID on pushing

71467f9

zeripath force-pushed the fix-#6946-pass-in-pr-into-hooks branch from cbf008b to 71467f9 Compare May 15, 2019 19:29

lafriks added the type/bug label May 15, 2019

lafriks added this to the 1.9.0 milestone May 15, 2019

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 15, 2019

zeripath changed the title ~~Fix #6946 by setting pull request ID on pushing~~ WIP: Fix #6946 by setting pull request ID on pushing May 15, 2019

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

fca3a51

zeripath mentioned this pull request May 15, 2019

Stop running hooks on pr merge #6963

Merged

Merge branch 'master' into fix-go-gitea#6946-pass-in-pr-into-hooks

4d68e21

zeripath added 2 commits May 17, 2019 18:15

Merge branch 'master' into fix-go-gitea#6946-pass-in-pr-into-hooks

56d7a2f

Merge branch 'master' into fix-go-gitea#6946-pass-in-pr-into-hooks

1271a3b

zeripath changed the title ~~WIP: Fix #6946 by setting pull request ID on pushing~~ Fix #6946 by setting pull request ID on pushing May 17, 2019

This was referenced May 17, 2019

TestSearchRepos is broken #6977

Closed

Fix bug when merge pull request running on docker #6971

Closed

zeripath added 3 commits May 19, 2019 21:15

Merge branch 'master' into fix-go-gitea#6946-pass-in-pr-into-hooks

5e1d436

Merge branch 'master' into fix-go-gitea#6946-pass-in-pr-into-hooks

1f9915d

Fix TestSearchRepo by waiting till indexing is done

dde55bd

zeripath changed the title ~~Fix #6946 by setting pull request ID on pushing~~ #6946 Run hooks on merge/edit and cope with protected branches May 20, 2019

zeripath commented May 21, 2019

View reviewed changes

integrations/repo_search_test.go Outdated Show resolved Hide resolved

zeripath added 4 commits May 21, 2019 07:07

Update integrations/repo_search_test.go

f7fba98

changes as per @mrsdizzie

cb9b7bb

Merge branch 'fix-go-gitea#6946-pass-in-pr-into-hooks' of github.com:…

a90466b

…zeripath/gitea into fix-go-gitea#6946-pass-in-pr-into-hooks

Merge branch 'master' into fix-go-gitea#6946-pass-in-pr-into-hooks

fefc60f

zeripath mentioned this pull request May 22, 2019

Fix #6960 - LFS OID urls uses unusual content-type header #7005

Closed

zeripath added 2 commits May 23, 2019 17:23

Merge branch 'master' into fix-go-gitea#6946-pass-in-pr-into-hooks

604117c

Merge branch 'master' into fix-go-gitea#6946-pass-in-pr-into-hooks

5b63303

zeripath added 2 commits June 2, 2019 12:53

missing comma

10773f1

Spelling mistake

46d8b2e

lunny reviewed Jun 2, 2019

View reviewed changes

zeripath added 2 commits June 3, 2019 09:21

Merge branch 'master' into fix-go-gitea#6946-pass-in-pr-into-hooks

757c395

Merge branch 'master' into fix-go-gitea#6946-pass-in-pr-into-hooks

4feeb65

zeripath mentioned this pull request Jun 17, 2019

cloneLink SSH generation change #7228

Closed

Merge branch 'master' into fix-go-gitea#6946-pass-in-pr-into-hooks

48c4bbd

lunny requested changes Jun 26, 2019

View reviewed changes

Merge branch 'master' into fix-go-gitea#6946-pass-in-pr-into-hooks

172cf28

Fix full pushing environment

c015839

lunny approved these changes Jun 28, 2019

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jun 28, 2019

zeripath mentioned this pull request Jun 29, 2019

Discussion of Gitea roadmap #6998

Closed

lafriks approved these changes Jun 30, 2019

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jun 30, 2019

Merge branch 'master' into fix-go-gitea#6946-pass-in-pr-into-hooks

77e8790

lunny merged commit 3563650 into go-gitea:master Jul 1, 2019

zeripath deleted the fix-#6946-pass-in-pr-into-hooks branch July 1, 2019 06:45

go-gitea locked and limited conversation to collaborators Nov 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

zeripath commented May 15, 2019 •

edited

Loading

codecov-io commented May 15, 2019 •

edited

Loading

zeripath commented May 15, 2019 •

edited

Loading

zeripath commented May 17, 2019

zeripath commented May 20, 2019

lunny Jun 2, 2019

zeripath Jun 2, 2019

lunny commented Jun 26, 2019

lunny Jun 12, 2019

zeripath Jun 27, 2019

zeripath commented Jun 27, 2019

zeripath commented Jun 27, 2019

stealthybox commented Aug 12, 2019 •

edited

Loading

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

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

Conversation

zeripath commented May 15, 2019 • edited Loading

codecov-io commented May 15, 2019 • edited Loading

Codecov Report

zeripath commented May 15, 2019 • edited Loading

zeripath commented May 17, 2019

zeripath commented May 20, 2019

lunny Jun 2, 2019

Choose a reason for hiding this comment

zeripath Jun 2, 2019

Choose a reason for hiding this comment

lunny commented Jun 26, 2019

lunny Jun 12, 2019

Choose a reason for hiding this comment

zeripath Jun 27, 2019

Choose a reason for hiding this comment

zeripath commented Jun 27, 2019

zeripath commented Jun 27, 2019

stealthybox commented Aug 12, 2019 • edited Loading

zeripath commented May 15, 2019 •

edited

Loading

codecov-io commented May 15, 2019 •

edited

Loading

zeripath commented May 15, 2019 •

edited

Loading

stealthybox commented Aug 12, 2019 •

edited

Loading