Skip to:

  1. Skip to Jira Navigation
  2. Skip to Side Navigation
  3. Skip to Main Content

TF - Prevent pushing patches to tf.org directly bypassing gerrit

Description

Deliverables

  • Every patch should be reviewed by Gerrit

  • Configs in the repo need to change

 

Background:

Colleague has accidentally pushed a patch to TF-M master branch directly.

By “directly” I mean, there is no patch in gerrit review and the change was merged in master already:

https://git.trustedfirmware.org/TF-M/trusted-firmware-m.git/commit/?id=ae8965e14264973890a9a03c061264c750703d06

 

The story was I was trying to use the source control feature in VS Code, but it didn’t prompt me anything such as which remote to push.

  

Looks like this is command used:

 

03_dev_spm_code_refine_tfm_spm_get_msg_buffer_from_conn_handle is my local branch.

Looks like something dangerous, the “-f” option was not even used.

Out of Scope (Optional)

  • List items you are sure of

Risks and Assumptions (Optional)

  • Risk - Mitigation

  • Assumptions

Acceptance Criteria Example and Notes (Optional)

Criteria

Status

Closeout Notes/Links

description of acceptance criteria...

(/)(x)

notes

eg. CI Report: http://cireport.linaro.org

notes

eg. Testing Reports: http://verification.linaro.org

notes

 

 

 

Legend:

Done, Not Done, Doesn't apply (note the reason)

Environment

None

Engineering Progress Update

None

Activity

Glen Valante 
April 14, 2021 at 10:14 PM

Updated TFC Board - April with issues resolved.

Kelley Spoon 
March 18, 2021 at 6:48 PM

Hi Karl,

That ability is granted by the "Create Reference" permission, which hasn't been altered.

What "Push Merge Commit" allows is for a user to push a commit to a branch without having to go through the review process.

Let's assume there's a project with a Contributors group that has the following permissions:

Create Reference "refs/heads/*" ALLOW
Push Merge Commit "refs/heads/experimental" ALLOW

If I have a local copy of the repository that contains work in a new commit, then:

With this setup, users can create a new branch, but as with master (with the exception of "experimental") all changes will need to go through gerrit review. In the case of the "testing" branch above:

Karl Zhang 
March 18, 2021 at 2:44 PM

Hi Kelley,

 

A concern from peers, is this change will prevent us from creating the new feature branches by ourselves?

 

Thanks.

Kelley Spoon 
March 17, 2021 at 5:57 AM

Hi Karl,

Yes, the first solution is active now.

To answer the second question: effectively, yes. The "Push Merge Commit" is a seperate permission from "Code Review Label" (which allows a group to +2 a review), but the instances where "Push Merge Commit" was set were all using the same group. There are currently and were previously no instances where "Push Merge Commit" was open to the public or groups other than "*-approvers".

Karl Zhang 
March 17, 2021 at 2:08 AM
(edited)

Hi Kelley,

 

Thanks for the action, is that mean pick up the first solution active now? I am discussing with members internally that would like to comment here afterward.

I assume only people who granted +2 permission able to push in previous with "Push Merge Commit" added, right?

 

Delivered

Details

Assignee

Reporter

Labels

Upstream

Shirt Size

Small (< 1 day)

Time tracking

3h logged

Priority

Parent

Checklist

Sentry

Created March 11, 2021 at 9:51 AM
Updated April 14, 2021 at 10:14 PM
Resolved March 16, 2021 at 4:01 PM