How to lock down commits on master(pull requests too)?

We LOCK DOWN master. NO ONE but circleCI can commit to master at all so master is never broken(except flaky tests of course). The only exception is high level account in emergencies(rarely used).

We just found a security hole though → Pull requests can be sent directly and merged into master through the web interface. This is a security hole for us. I have two questions

  1. How can we lock down the web interface so it is not allowed to put stuff on master for admins accidentally(much like sudo command)? (Everyone else is locked down but admins can accidentally just merge a pull request onto master and should really have to do something like sudo superpower)

  2. How can we make it so merge pull request creates a branch of the form submit_{anyname of branch} so that circleCI picks up the pull request, merges with master if needed, runs build, runs tests, and if it all passes, circleCI pushes all this up to master so master is 99% ALWAYS working and no one is merging bad stuff in that fails tests. OR how to have a submit pull request to circleCI or other build system basically → for circleCI, I have it build all branches starting with name “submit_” and merge that into master and push to github.

No one - including automation tools - should be allowed to commit to master directly. if you need emergency changes you should temporarily introduce a hotfix branch instead in order to bypass your normal feature / dev branches.

All the protection requirements you seem to have can be met with branch protection rules. With those you can limit which branches you can’t commit to, which status checks (Circle CI runs) need to suceed before you allow merging, require reviews, and enforce all of this for admins too.

Not sure what you mean with #2, since a PR always encompasses a source branch, but can’t say much on that since I haven’t worked with Cirecle CI yet. Triggers should however work the same with this. If Circle CI is merging something on your behalf and not correctly respecting your rules that seems like a whole different problem though.

“No one - including automation tools - should be allowed to commit to master directly” - This confuses me as ‘someone’ or ‘something’ has to put the code on github master,right? If not, master branch will never change. In our case, ONLY CI is allowed to put code on github master when the build has passed. Let me repeat though “CI only puts code on master if the build has passed” ← this is important for the next few notes →

There is a merge button in the github web interface that

  • Does NOT run CI and therefore puts bad code on master creating a master branch that does not build violating our policy of ‘master branch always works’
  • We can’t seem to hook into so the button such that it uses circleCI to run the build and let circleCI put the changes on master instead of the button since button doesn’t know if circleCI passed.

We ran a test and did two pull requests →

Both passed CI but then we clicked merge pull request for each one in the GUI and then after that master branch was broken mainly because ‘clicking the merge pull request button in the GUI does not run CI’

Our goal is to never allow a straight merge without doing CI and CI tool puts the changes on master as a linear commit.

Circle CI workflow results can apparently be reported to the GitHub Repo / Pull Request, which would enable you to define required status checks as you seem to want. See the CircleCI documentation here. (It says, that it is only applicable for CircleCI cloud - not sure what you are using, but there are surely alternatives available)

Regarding the “nobody commits to master directly” comment, I meant any code changes only happen through reviewed pull requests - you should not just checkout the master branch, add you changes, and then push (even as a CI that is not best practice).

Perhaps we are in agreement? not sure

For clarity we want the process to be

  1. post pull request
  2. someone reviews and approves and clicks ‘merge pull request’
  3. CI kicks off merges in master to the pull request first and builds
  4. IF CI passes, CI will then put the results on master as a working copy

In this way, master is never broken. In any way a human puts on master, then there is a chance it is broken. Currently github allows a human to push merge pull request and it does not go through CI and is merged onto master and my use case above shows master breaking

Yeah that is the workflow I had in mind too. I just didn’t take liking to the wording “the CI pushes to master”, since even with pull request CI does NOT do that. It is just the CI job that give the GitHub Pull Request its okay, which can then proceed to be merged (either manually or automatically by github as configured). CI does not actually push anything

ohhh, ok, we are in slight disagreement. That is ‘not’ what twitter does. CI pushes so that no human can break it. In fact, we proved this morning the process you just laid out allows master to get broken. Just see these two pull requests which broke master because a human did the merge by clicking a button instead of CI doing it →

So here, the two changes work great independently but after clicking the merge button twice, master is now broken. Our current process prevents this BUT we can’t use a pull request.

We WANT a human to click the ‘merge pull request’ button which kicks off CI on a local computer to merge that pull request into LOCAL master only and run. If all is well, we want CI to then push it up to master. - This is proven to 100% keep master working instead of the 90% method you currently use. (By the way, my method is shamelessly stolen from Twitter and I honestly think they have the best practice of master always is working)