Prevent changing the target branch of a merged MR

Kai Armstrongrequested to merge
fix/ignore-target-branch-change-on-merged-mr into master

What does this MR do and why?

After a merge request is merged, its target branch can still be changed through the REST API (PUT /projects/:id/merge_requests/:merge_request_iid with target_branch) and through the GraphQL mergeRequestUpdate mutation (targetBranch argument). Once changed, the MR page reports that the changes were merged into the new target branch, even though nothing was ever merged there.

The UI has blocked this since 2020 (#223235 (closed), fixed in 4baf46d7), but that fix only hid the branch selector in the edit form. It added no server-side check, so both APIs remained able to change the target branch of a merged MR.

Both APIs go through MergeRequests::UpdateService. Its general_fallback method only ignored target_branch when the MR was closed or merged, and the source fork no longer existed. This MR changes that condition so general_fallback ignores target_branch whenever the MR is merged, regardless of whether the source fork still exists. Closed (not merged) MRs are unaffected and can still be retargeted, matching the current UI behavior.

Real-world impact: a customer saw a bot merge one branch into another, then retarget the merged MR to master. The MR page then claimed a merge into a protected branch that never happened. The customer nearly opened a security incident over it, and the incorrect record is also an audit concern. See the comment at #21458 (comment 3894724319).

API compatibility, worth reviewer input: this MR ignores the target_branch parameter silently rather than rejecting the request. The endpoint still accepts target_branch, still returns 200, with the same response shape as before. Returning a 4xx would change the response status code, which the REST API styleguide (https://docs.gitlab.com/development/api_styleguide/#what-is-a-breaking-change) lists as a breaking change. Silently ignoring the parameter also matches how the service already treats source_branch, and how it already treated target_branch in the existing merged-fork case. The downside is that callers get no feedback that their request had no effect. I'd like reviewers' opinion on whether this tradeoff is the right one.

The API documentation for target_branch on "Update a merge request" now states: "Ignored if the merge request is merged."

References

Resolves #21458 (closed)

Related:

Screenshots or screen recordings

Not applicable. This is a backend-only change with no UI impact.

How to set up and validate locally

  1. In GDK, create a merge request from a feature branch and merge it into main (or master).

  2. Run the following, replacing the token, host, project ID, MR IID, and target branch as needed:

    curl --request PUT --header "PRIVATE-TOKEN: <token>" \
  "http://gdk.test:3000/api/v4/projects/<id>/merge_requests/<iid>" \
  --data "target_branch=<other-branch>"

  3. On master (before this fix): the response and the MR page both show the new target branch, even though nothing was merged there.

  4. On this branch (after this fix): target_branch in the response is unchanged, and the MR page still shows the original target branch.

MR acceptance checklist

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

🤖 Generated with Claude Code

Merge request reports