Skip to content

Allow branch protection updates via API with `block_branch_modification`

Dominic Bauer requested to merge 442421-protected-branch-cannot-be-updated-through-api-with-block_branch_modification into master

What does this MR do and why?

Fixes a regression introduced by a security fix (#435500 (closed)). Maintainers could rename a protected branch's name, even with an MR approval policy in place that sets block_branch_modification.

The security fix rejects all updates to a protected branch if params[:name] is present. This works except for the Update a protected branch REST API endpoint, since it has a name path parameter (PATCH /projects/:id/protected_branches/:name) and is thus not functional in combination with block_branch_modification.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

How to set up and validate locally

  • Create a new project and note its ID

  • In a Rails console, note the last branch protection's ID from ProtectedBranch.last.id

  • Navigate to Security > Policies and create the following Merge request approval policy:

type: approval_policy
name: Block branches
enabled: true
rules:
  - type: any_merge_request
    branch_type: protected
    commits: any
actions:
  - type: require_approval
    approvals_required: 1
    role_approvers: [owner]
approval_settings:
  block_branch_modification: true
curl --request PATCH --header "PRIVATE-TOKEN: $GITLAB_TOKEN" "http://gdk.test:3000/api/v4/projects/$PROJECT_ID/protected_branches/main?allow_force_push=true&name=foobar"
  • Verify that the protected branch cannot be renamed, e.g. by going through Projects::ProtectedBranchesController#update:
curl 'http://gdk.test:3000/root/restless-brook-6894/-/protected_branches/172' \
  -X 'PATCH' \
  -H 'Accept: application/json, text/plain, */*' \
  -H 'Content-Type: application/json' \
  -H 'Cookie: $COOKIE' \
  -H 'X-CSRF-Token: $CSRF_TOKEN' \
  --data-raw '{"protected_branch" {"allow_force_push":true,"name":"foobar"}}'

Related to #442421 (closed)

Edited by Dominic Bauer

Merge request reports