Branched can be protected to prevent changes being pushed or merged except by certain people. It is common to configure master and release-* branches so that No one can push directly to the branch so that every change must occur through a merge request. However, this creates the situation where no one can create new release branches!
GitLab should make it possible to create new protected branches without having to give push permissions to someone or temporarily disable then enable the protected branch rules.
Further details
This problem exists when a pattern like release-* is used and release branches are created regularly.
One approach would be to add a Create branch setting for protected branches, but this makes no sense for non-pattern protected branch rules and adds yet another configuration.
Proposal
Allow Developers to be able to create new protected branches if the HEAD of the new protected branch is already in a protected branch. Subsequent pushes would be rejected because the branch already exists.
@DouweM@nick.thomas I hear the request to make protected branch creation configurable reasonably often because of the no one can push situation. What do you think of this alternative approach? The customer above is happy with this approach.
@jramsay What do you think about only allowing these branches to be created through the web interface? We have the same requirement for protected branch deletion. That would prevent some accidental branch creation. I would also be in favor of restricting this to maintainers or even owners.
@DouweM I think your point was more about preventing protected branch creation via Git push which could happen accidentally on the command line by any developer, right? Allowing it via the UI and API are pretty equivalent.
James Ramsay (ex-GitLab)changed title from Allow protected branches to be created by Developers if no new commits are added to Allow protected branches to be created by Developers if no new commits are added via API and interface
changed title from Allow protected branches to be created by Developers if no new commits are added to Allow protected branches to be created by Developers if no new commits are added via API and interface
Allow Developers to be able to create new protected branches if the HEAD of the new protected branch is already in a protected branch.
Since we're limiting creation of protected branches via web interface and API only, do we still need to check if the HEAD is already in a protected branch? I'm asking because I don't see a way on the web interface and API to create a branch with new commit so it seems adding an additional check is not needed right now.
Or do we want to check whether the ref is/comes from a protected branch?
@patrickbajao Good point. I think the situation where I was thinking about was someone to open a random commit in the Web IDE and create a new protected branch when they make the commit – however that wouldn't be possible unless they had push permissions anyway.
Should we require "Can Merge" instead of "Developer" permissions? Allowing developer access would make it hard to lock down a branch so "No One" can create, such as preventing creation of a branch matching auto-release/* that would usually be managed by a bot. (See: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/24969#note_143466464)
Is it dangerous that checking if the HEAD of the new protected branch is already in a protected branch allows old commits to be used for new protected branches? Should we instead require commits at the HEAD of a protected branch? Could this allow someone to maliciously creating a branch at an old commit to reveal secret CI variables? (See: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/24969#note_143479598)
Should we require "Can Merge" instead of "Developer" permissions? Allowing developer access would make it hard to lock down a branch so "No One" can create, such as preventing creation of a branch matching auto-release/* that would usually be managed by a bot.
Great point @jamedjo and thanks for the example scenario, didn't think about that. I think requiring merge access makes sense.
Is it dangerous that checking if the HEAD of the new protected branch is already in a protected branch allows old commits to be used for new protected branches? Should we instead require commits at the HEAD of a protected branch?
I agree on this as well. While it's not very likely for a regular user to do this (I think), it seems like a security hole to me (e.g. a protected variable has a different name in an old commit, so it's no longer protected) and should be prevented.
Should we require "Can Merge" instead of "Developer" permissions? Allowing developer access would make it hard to lock down a branch so "No One" can create, such as preventing creation of a branch matching auto-release/* that would usually be managed by a bot.
Thanks @jamedjo – that's a good idea. It won't cover the situation where a developer who doesn't have merge permissions or push permissions can't initiate the release process, but it makes it possible to broaden the pool of people who can do this.
Is it dangerous that checking if the HEAD of the new protected branch is already in a protected branch allows old commits to be used for new protected branches? Should we instead require commits at the HEAD of a protected branch?
I initially proposed this, but thought that it creates a timing problem where if you were waiting for one last thing to merge, something else might get merged that you didn't want before creating the branch. That'd be a real nuisance.
Also I don't think we're creating a new security issue. If I have merge permission, I can in theory (subject to approvals) merge whatever I want in to the target protected branch, and re-run pipelines on the target branch. This is based on a conversation I had with @brendan since the docs didn't explicitly describe who can run which pipeline.
Manual actions are considered to be write actions, so permissions for protected branches are used when user wants to trigger an action. In other words, in order to trigger a manual action assigned to a branch that the pipeline is running for, user needs to have ability to merge to this branch.
Though maybe we should be more explicit if that didn't make it clear. There's actually an entire Epic over overhauling our docs - &784 (closed)
Hmm yeah, good point @jramsay. I can see how that may become a nuisance.
About the security part, yeah it seems that if the user with merge permissions makes a mistake by either merging/creating a new protected branch, it can possibly happen either way.