Error 500 attempting to merge PR as admin

[guillep2k]

• Gitea version (or commit ref): 2ecf98e

Description

When I attempt to merge a PR that doesn't have enough approvals, the [Merge] button shows up in red and the message "As an administrator, you may still merge this pull request" is displayed. However, when I push the [Merge] button, a 500 Server Error page shows up with the following error:

git push: remote: Gitea: Not allowed to push to protected branch master and pr #68 is not ready to be merged: not allowed to merge [reason: Does not have enough approvals]        
To /home/giteatest/gitea-repositories/user2/miomiomio.git
 ! [remote rejected] base -> master (pre-receive hook declined)
error: failed to push some refs to '/home/giteatest/gitea-repositories/user2/miomiomio.git'

Stack trace:

2020/01/12 20:36:05 ...ters/private/hook.go:145:HookPreReceive() [W] Forbidden: User 39 is not allowed push to protected branch master in 82473
8599184:user2/miomiomio and pr #1 is not ready to be merged: not allowed to merge [reason: Does not have enough approvals]
2020/01/12 20:36:05 routers/repo/pull.go:712:MergePullRequest() [E] Merge: git push: remote: Gitea: Not allowed to push to protected branch mas
ter and pr #68 is not ready to be merged: not allowed to merge [reason: Does not have enough approvals]
    To /home/giteatest/gitea-repositories/user2/miomiomio.git
     ! [remote rejected] base -> master (pre-receive hook declined)
    error: failed to push some refs to '/home/giteatest/gitea-repositories/user2/miomiomio.git'

    /home/guillep2k/src/code.gitea.io/gitea/routers/repo/pull.go:712 (0x1471838)
        MergePullRequest: ctx.ServerError("Merge", err)
    /home/guillep2k/go/src/reflect/value.go:460 (0x49e935)
        Value.call: call(frametype, fn, args, uint32(frametype.size), uint32(retOffset))
    /home/guillep2k/go/src/reflect/value.go:321 (0x49e0f3)
        Value.Call: return v.call("Call", in)
    /home/v/src/code.gitea.io/gitea/vendor/gitea.com/macaron/inject/inject.go:177 (0x9bc749)
        (*injector).callInvoke: return reflect.ValueOf(f).Call(in), nil
    (etc)

User 39 is the owner of the repository and the user I'm logged in for the merge operation.

[zeripath]

gitea/routers/private/hook.go

Lines 143 to 144 in 80bfd51

	if err := pull_service.CheckPRReadyToMerge(pr); err != nil {
	if models.IsErrNotAllowedToMerge(err) {

The specific override would have to go here or the line below.

[zeripath]

@davidsvantesson you did not understand that the hook always runs.

gitea/routers/private/hook.go

Lines 101 to 102 in 80bfd51

	if !canPush && opts.ProtectedBranchID > 0 {
	// Manual merge

This is not true. In fact if you have the protectedbranchid set it's not a manual merge!!

[davidsvantesson]

Ohh. Is there any way to know if it was done from the UI?

[davidsvantesson]

The previous code didn't even check if status checks was ok, so it didn't prevent admins (or anyone) to merge.

@zeripath So we never allow manual merges, unless user has push rights to branch? (I.e. a user with merge rights has to use the UI or API, not trying to manual merge and push). It is a bit hard to follow the code when it makes internal requests 😕 😊

In that case this code might even be a bit double as the check is done before merge is done. Anyhow it should be an easy fix to just check if user is admin.