Skip to content

feat(merge-approved-pr): bind approval to the head commit and same repository, accept Mobile version files - #324

Draft
tommasini wants to merge 2 commits into
mainfrom
mcrm-186-bind-merge-approval-to-head
Draft

tommasini wants to merge 2 commits into
mainfrom
mcrm-186-bind-merge-approval-to-head

Conversation

@tommasini

Copy link
Copy Markdown
Contributor

Description

Part of MCRM-186 (finding 8 of the Mobile and Extension release security review).

merge-approved-pr merges a PR when a release-team member approved it. It had three gaps:

  1. The approval was not bound to the head commit. An approval of an earlier commit still counted after new commits were pushed. The action now only counts an approval whose commit_id is the current head SHA.
  2. Fork heads were accepted. The head branch check is a name regex, so a fork branch called version-bump/9.0.0 matched. The action now refuses any head that isn't in the base repository, including a deleted one.
  3. The merge was not pinned. The merge now passes sha to pulls.merge, so GitHub refuses it if the branch moves between the checks and the merge.

Mobile also needs verify-version-bump: true (Extension already sets it). Mobile can't turn it on yet: a Mobile version bump changes package.json, versionName in android/app/build.gradle and MARKETING_VERSION in ios/MetaMask.xcodeproj/project.pbxproj (for example, metamask-mobile#39045), and the check only accepted package.json.

The check now accepts those two native version lines next to package.json, under these rules:

  • package.json must still change.
  • Every changed file must be one of the three version files, and every changed line must be that file's version line.
  • Every file must move from the same old version to the same new version.

Extension's package.json-only bumps behave exactly as before.

Review listing now paginates, so a PR with more than 30 reviews is evaluated on all of them.

Testing

I ran the action's github-script bodies, taken from action.yml, against fixture data with a stubbed github/core. All 16 cases pass:

  • Real Mobile version-bump PR metamask-mobile#39045 (three files) passes.
  • Real Extension version-bump PR metamask-extension#47671 (package.json only) passes.
  • These are rejected:
    • a fork head
    • a deleted head repository
    • an approval of an older commit
    • a non-team approval
    • outstanding requested changes
    • a native-only change
    • an extra file
    • an extra line in build.gradle
    • native versions that don't match package.json
    • a non-sequential bump
    • an added (not modified) version file
  • The merge call carries the verified head SHA.

Rollout

Once this is released and v1 moves, Mobile turns on verify-version-bump: true in merge-version-bump-pr.yml (companion Mobile PR linked from MCRM-186).

…pository, accept Mobile version files

Co-authored-by: Cursor <cursoragent@cursor.com>
@tommasini tommasini self-assigned this Nov 13, 2026
Co-authored-by: Cursor <cursoragent@cursor.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant