Skip to content

Add temp flag to prevent inserting unapproved content

Gary Holtz requested to merge 859-gmh-add-temp-unmergeable-status into master

What does this MR do and why?

Fix for Approved merge request can introduce extra unve... (#383776 - closed)

To prevent a merge form happening within the 10 second window where approvals are being reset, this MR will add a "temporarily unapproved" flag to Redis with the project/MR id as a key.

They key is removed after:

  • 15 seconds (to be safe)
  • All the approvals are actually deleted (merge_request.approvals.delete_all is run)

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

From Approved merge request can introduce extra unve... (#383776 - closed):

  1. Identify/create a project with "when a commit is added, remove all existing approvals" box checked in project settings
  2. Make a legit merge request
  3. Wait until the merge request is reviewed and approved by one of the eligible approvers.
  4. Push another commit to the MR source branch
  5. Refresh the MR webpage, observe that the new commit has been displayed in the UI (The "Activity" section will display something like "XXX added 2 commits 5 seconds ago").
  6. Within a few seconds, the existing approval from step 3 continues to be presented, and the merge button is still available to users with enough permission to merge. If one of them click "merge", the MR will be merged successfully. If no one click merge, after about 5-10 seconds, the approval from step 3 will be removed.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to https://gitlab.com/gitlab-org/security/gitlab/-/issues/859 Related to #383776 (closed)

Edited by Gary Holtz

Merge request reports