isaacs/github

Disable admins to be able to push to protected branch

huehnerlady opened this issue ยท 26 comments

Hi,

we use github on a daily basis. We are currently in the process to protect out master branches better and just allow very few people (like automated bots) to be able to push to master directly.

We found the branch protection rule Restrict who can push to matching branches.
This sounds exactly like what we want, BUT it seems to default to repository and organization administrators:
screen shot 2018-10-24 at 10 42 57

We could not find a possibility to JUST allow the bots to be able to push directly.
Is there a way to JUST allow the bots to push to the master branch without a pull request?

Create a branch rule and then disable the exclusion of admins on that rule.
image

Hi,
Thanks for your quick answer @KieranDevvs.
I had thought of that, but when I enable this setting, there is still the point in the Restrict who... setting:
screen shot 2018-10-24 at 12 13 44

Is that maybe a bug then?

Hi,
Thanks for your quick answer @KieranDevvs.
I had thought of that, but when I enable this setting, there is still the point in the Restrict who... setting:
screen shot 2018-10-24 at 12 13 44

Is that maybe a bug then?

That's an option to allow an exception to the rule, i.e you may want a certain person or team to be able to push to the branches matched by the rule. Leaving this option blank means no one can.

I tried it but we still cannot push directly in master. I know found out that IF that restriction is enabled, NOBODY can click the merge/squash/rebase button apart from these people. So it is definitely not the functionality we want.

Does anyone know how to enable JUST bots to be able to push into a branch directlz and others have to get approved PullRequests and passing status checks?

Looks like the same feature I am looking for. Being able to block push (git push) to a branch but no restrictions on pull requests merging.

Main reason, block accidental merges to master.
Secondary reason, encourage looking at the tests.

Anyone has found a solution for it?

I found a workaround. In the branch protection rule you could tick the box "include admins".
IF an admin needs to merge a branch without it being reviewed, the admin can temporarily remove that flag and do the merge. But then accidental merges to master cannot happen

I found that a bit tedious, but we merge PRs so rarely without approval that the advantage of not pushing to master by mistakes overweights the little hustle in merging without approval :)

Thanks, though not the work around I was looking for. :)

I just wish, i could have the limit on git push without the limit of pr merge(without second persons approval). That change I wouldn't need to sell in to the teams, I could just do it

yes me too :)
But this is about the fact that you cannot restrict direct push to people without opening it up to admins

What is needed is another option: Bypass branch protection rules for users then we can select github ids from a list. This is so that we only add the jenkins bot there, as unticking the include administrators option increases the risk of people pushing by accident to the branch.

What is needed is another option: Bypass branch protection rules for users then we can select github ids from a list. This is so that we only add the jenkins bot there, as unticking the include administrators option increases the risk of people pushing by accident to the branch.

well that is exactly what this option is - this option allows github users - e.g. bots - to push directly to the branch without any review. BUT it always allows admins to do something too. So for me it looks like that should be an option which you should at least be able to disable

I'm setting this up for our team too. I decided to just create a machineuser which will have a kind of super-admin role and will be the only account used to force pushes

Eh. I'm a repo admin, there is a branch protection rule and I cannot force-push. It's very inconvenient tbh :(

Does anybody find a solution for this?

I feel that this used to be possible. I had a branch rule that prevented direct pushes to a particular branch. It allows PRs to be openend and merged into that branch.

I can no longer replicate this behavior. I tried a bunch of things today and failed to come up with a solution.

Is anyone aware that something changed in GitHub in the past year or two around this?

I am using the Require signed commits checkbox instead of the Include administrators and it works like a charm. If you click the merge button, that commit is automatically signed because you are logged into GitHub, but if you push from your Git client, the commit is usually not signed.

Oh snap, Require signed commits requires all commits to be signed โ€“ makes sense, I guess :/

@UX3D-schmithuesen So that's only a solution for using the squash strategy on merging, I guess?

Yeah, that might work, but I really don't like it.

In our team, we're trying to automate as much as we can and we've been struggling for months due to this issue. We were left considering two options:

  1. Always disable Include administrators
  2. Don't automate releases

We then noticed the Github API allows you to play with the include administrators or enforce_admins flag. We wrote this tool https://github.com/benjefferies/branch-protection-bot which, if put in a pipeline before a release step and ran again after it will temporarily enable, then disable Include administrators minimising the risk of accidental commits.

+1 on needing this.

Our workflow is such that we have Jenkins push k8s manifests to Github which then are applied to kubernetes via ArgoCD (Gitops pattern). We only want allow the Jenkins Github User to commit directly to master, everyone else should go through standard PR workflow including Admins.

We could have our automation create PRs and merge into Master, but that's a huge pain. I suppose Bulldozer could help with merging the PR (https://github.com/palantir/bulldozer)

+1

This is quite a common workflow when automizing release process. We want to stop everyone to push to master without PRs but we want to let a machine user push to master directly when performing automated release. Currently, it seems it is not possible. The checkbox: Restrict who can dismiss pull request reviews only adds a button for the users in the list which they can use to dismiss the Requested changes proposed by one of the reviewers. It does not exclude the listed users from the rule.

+1

We resently moved away from Bitbucket which had that feature and now we have to think of another way for our automated releases until this is implemented...

For the record, from https://github.community/t5/Support-Protips/Best-practices-for-protected-branches/ba-p/10224:

โ›”๏ธ Don't: expect branch protection to prevent pushing commits which don't alter the commit history. If you want to disable pushing directly to the branch, use the setting Require pull request reviews before merging instead.

That's the best documentation I have found for the current behavior. And I think this issue mainly disagrees with this point.

An easy way to fix this would just be to allow 0 reviewers:

image

Or allow admins to assign themselves as reviewers. One of the two.

But as it stands, it forces there to be at least 1 reviewer, who can't be me. Thus, I can still accidentally push to protected branches if I'm not careful.

An easy way to fix this would just be to allow 0 reviewers:

image

Or allow admins to assign themselves as reviewers. One of the two.

But as it stands, it forces there to be at least 1 reviewer, who can't be me. Thus, I can still accidentally push to protected branches if I'm not careful.

indeed why not an option for admins to approve their own reviews? that would fix al my problems.

Adding the option to allow administrators and code owners to override and approve would simply solve this problem and this one https://github.community/t/do-not-require-owner-approval-if-the-pull-request-is-from-an-owner/369.

Azure DevOps has ability for years. Would be great if GitHub has it too.