Preventing bypass of review

[Tomas Hellberg]

I need help with figuring out how to configure Gerrit. Let me try to explain what I need:

1. I need to enforce that all commits go through review

2. I want developers to be able to create branches under refs/heads/user/${username}/* (or some other namespace) so I grant them "Create Reference" on that reference

But if I grant the "Create Reference" privilege, then I also allow the developers to bypass review by doing

    [make 10 commits to local branch branch-to-bypass-review]

    git push origin branch-to-bypass-review:user/someone/my-branch

    git checkout master

    git merge branch-to-bypass-review

    git push origin master:refs/for/master

When creating new branches from the web interface, this is not a problem, since it's only possible to create a branch from something that Gerrit already knows about. But from the command line, the developers can push a branch with 10 commits to user/someone/my-branch (which will be created if the user has "Create Reference") and then push a merge commit to refs/for/master and get all 10 commits submitted without review.

Is there any way to prevent the creation of branches from the command line so that the developers don't accidentally or intentionally bypass review? Or is there some other way to achieve what I want, i.e. freedom to create branches but still enforcing review on everything?

[Alex Blewitt]

On 29 Oct 2013, at 11:54, Tomas Hellberg wrote:

> I need help with figuring out how to configure Gerrit. Let me try to explain what I need:
>
> 1. I need to enforce that all commits go through review
> 2. I want developers to be able to create branches under refs/heads/user/${username}/* (or some other namespace) so I grant them "Create Reference" on that reference

Any commit that is present under refs/heads/* is considered already reviewed, so these can be pushed/submitted to any other branch without going through a review process.

If you use a ref under something else (e.g. refs/users/*) then they are considered not reviewed and will go through the full namespace. However since Git doesn't consider things outside refs/heads/* branches you need to configure your refspec appropriately, e.g. fetchurl = +refs/user/*:refs/remotes/user/* whereupon branching etc. should work as expected.

Ideally Gerrit could have a private space under refs/heads/* which it considered as ineligible for review (e.g. refs/heasd/unreviewed/*) so that it wouldn't need to have this as a workaround - but at the moment this is not the case.

Alex

[Tomas Hellberg]

Any commit that is present under refs/heads/* is considered already reviewed

Yes, I understand this, and this is the way I want it to be. I just don't want commits to be able to land there without going through review. And any user with "Create Reference" privileges somewhere under refs/heads/* can bypass this. So if I need to ensure that everything goes through code review, I cannot have "Create Reference" on any user, am I correct?

[Luca Milanesio]

mmm ... possibly a plugin would do.

A commit validation plugin can enforce what people can do over a Git push command.

The Create Reference would then apply for the GUI whilst the plugin can actually enforce the review path over a Git push.

What do you think ?

Luca.

On 29 Oct 2013, at 12:12, Tomas Hellberg <tom...@gmail.com> wrote:

Any commit that is present under refs/heads/* is considered already reviewed

Yes, I understand this, and this is the way I want it to be. I just don't want commits to be able to land there without going through review. And any user with "Create Reference" privileges somewhere under refs/heads/* can bypass this. So if I need to ensure that everything goes through code review, I cannot have "Create Reference" on any user, am I correct?

--
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en
 
---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Tomas Hellberg]

Ideally Gerrit could have a private space under refs/heads/* which it considered as ineligible for review (e.g. refs/heasd/unreviewed/*) so that it wouldn't need to have this as a workaround - but at the moment this is not the case.

To clarify: I'm not looking for a way for developers to have a sandbox for stuff that's not reviewed. Quite the opposite. I want to make sure that everything passes review. So the desired workflow would be something like this:

1. Create a feature branch X on Gerrit based on origin/master (important here that the branch cannot be set to point to something that is not already under refs/heads/*)

2. Push to refs/for/X for review

3. At the time when it's decided to include the X branch in the next release, merge it to master and push the merge commit to refs/for/master/X

[Tomas Hellberg]

A commit validation plugin can enforce what people can do over a Git push command.

The Create Reference would then apply for the GUI whilst the plugin can actually enforce the review path over a Git push.

What do you think ?

Yes, possibly. But I just tested to create a branch from the web interface under refs/heads/* and gave the SHA-1 of something under refs/changes/* (not yet submitted) which got accepted as a new branch. So that's still a problem.

[Thomas Swindells (tswindel)]

I think what you want is the ability to restrict the create reference permission to only allow creation with a head that has already been reviewed? Both in the cases where it is pushed or created through the UI. Presumably this would just require a check that the reference is reachable from one of the heads? (Or something similar).

 

This would also be interesting to us as it is the reason why we are hesitant about allowing people to create branches.

 

Thomas

 

--

[Tomas Hellberg]

Yes, this is exactly what I want.

On Tuesday, October 29, 2013 2:21:41 PM UTC+1, Thomas Swindells wrote:

I think what you want is the ability to restrict the create reference permission to only allow creation with a head that has already been reviewed? Both in the cases where it is pushed or created through the UI. Presumably this would just require a check that the reference is reachable from one of the heads? (Or something similar).

 

This would also be interesting to us as it is the reason why we are hesitant about allowing people to create branches.

 

Thomas

 

From: repo-d...@googlegroups.com [mailto:repo-d...@googlegroups.com] On Behalf Of Tomas Hellberg
Sent: 29 October 2013 12:28
To: repo-d...@googlegroups.com
Cc: Tomas Hellberg
Subject: Re: Preventing bypass of review

 

 

A commit validation plugin can enforce what people can do over a Git push command.

The Create Reference would then apply for the GUI whilst the plugin can actually enforce the review path over a Git push.

 

What do you think ?

 

Yes, possibly. But I just tested to create a branch from the web interface under refs/heads/* and gave the SHA-1 of something under refs/changes/* (not yet submitted) which got accepted as a new branch. So that's still a problem.

--
--
To unsubscribe, email repo-discus...@googlegroups.com

[Luca Milanesio]

Worth creating a plugin or extension for it ?

Luca.

To unsubscribe, email repo-discuss...@googlegroups.com

[Thomas Swindells (tswindel)]

I’d be inclined to say it should possibly be core functionality as once a project has been setup this is normally the permissions that you want.

 

Otherwise a plugin/extension would be fantastic.

Thomas

[Saša Živkov]

I haven't tested it but this may work:

[1] BLOCK Push for refs/*

[2] ALLOW Create Reference for refs/heads/*

[3] ALLOW Push for refs/for/*

[1] should prevent pushing of new commits except for review [3].

[2] should allow creation of new branches on existing commits.

[Tomas Hellberg]

No, this does not work as [2] gives the right to create a new reference that can point to the latest in a series of 100 new commits never before seen by Gerrit (and Gerrit will not create changes for these commits as they are not pushed though refs/for/...).

[Phil Hord]

On Oct 29, 2013 7:16 AM, "Alex Blewitt" <alex.b...@gmail.com> wrote:
>
> On 29 Oct 2013, at 11:54, Tomas Hellberg wrote:
>
> > I need help with figuring out how to configure Gerrit. Let me try to explain what I need:
> >
> > 1. I need to enforce that all commits go through review
> > 2. I want developers to be able to create branches under refs/heads/user/${username}/* (or some other namespace) so I grant them "Create Reference" on that reference
>
> Any commit that is present under refs/heads/* is considered already reviewed, so these can be pushed/submitted to any other branch without going through a review process.

This sounds like a bug to me.  What if I want a 'next' branch which is reviewed by one team, and a 'master' branch which is reviewed by a different one?

Phil

P.s. sorry for the thread resurrection.

[Edwin Kempin]

2014-01-26 Phil Hord <phil...@gmail.com>

Normally both branches 'next' and 'master' would have different states, so if you review the commit for 'next' and cherry-pick it to 'master' this is no problem since after cherry-picking you have a new commit ID. If both branches are the same, review the commit for one branch, merge this branch into the other branch and review the merge commit for the other branch.

 

Phil

P.s. sorry for the thread resurrection.

--