Skip to content

fix: require a bot author to repair a pull request - #10334

Merged
cryptodev-2s merged 1 commit into
mainfrom
fix/repair-gate-require-bot-author
Sep 21, 2026
Merged

cryptodev-2s merged 1 commit into
mainfrom
fix/repair-gate-require-bot-author

Conversation

@cryptodev-2s

@cryptodev-2s cryptodev-2s commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Explanation

The branch prefix alone was too loose. actions/checkout here passes ref: with no repository:, so it always checks out from MetaMask/core. A fork pull request whose head branch happens to be named after an existing renovate/ branch would pass the gate, mint a write token through the exchange, and push to that branch. No attacker code runs, since the tree comes from this repo, but an outsider should not be able to trigger it at all.

Requiring metamask-ci[bot] as the author closes it, and nobody can impersonate a bot login. Verified against the real payloads: Renovate pull requests are user.login = metamask-ci[bot], type = Bot, and Dependabot ones are dependabot[bot].

Raised by Mrtenz on #10324.

References

Follows #10324. Part of WPC-1161.

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Note

Low Risk
One-line workflow gate change with no app runtime impact; wrong bot matching would only stop repairs on Renovate PRs.

Overview
Tightens who can run the dependency-upgrade repair workflow so it no longer keys off a renovate/ branch name alone.

The job’s if condition now runs for Dependabot (dependabot[bot]) as before, or for Renovate only when the PR author is metamask-ci[bot] and the head ref still starts with renovate/. That blocks fork (or other) PRs that reused a renovate/… branch name from triggering OIDC token exchange and pushes to the head branch, while keeping legitimate Renovate automation unchanged.

Reviewed by Cursor Bugbot for commit 4923adb. Bugbot is set up for automated code reviews on this repo. Configure here.

@cryptodev-2s
cryptodev-2s requested a review from a team as a code owner September 21, 2026 19:53
@cryptodev-2s cryptodev-2s self-assigned this Sep 21, 2026
@cryptodev-2s
cryptodev-2s requested a review from Mrtenz September 21, 2026 19:53
@cryptodev-2s
cryptodev-2s deployed to default-branch September 21, 2026 19:53 — with GitHub Actions Active
@cryptodev-2s
cryptodev-2s added this pull request to the merge queue Sep 21, 2026
Merged via the queue into main with commit aeb401e Sep 21, 2026
342 checks passed
@cryptodev-2s
cryptodev-2s deleted the fix/repair-gate-require-bot-author branch September 21, 2026 20:00
@cryptodev-2s cryptodev-2s mentioned this pull request Sep 22, 2026
4 tasks
pull Bot pushed a commit to Reality2byte/core that referenced this pull request Sep 23, 2026
## Explanation

Renovate pull requests opened today carry an artifact failure, MetaMask#10369
for example:

```
error This project's package.json defines "packageManager": "yarn@4.17.1".
However the current global version of Yarn is 1.22.22.
Corepack must currently be enabled by running corepack enable in your terminal.
```

Renovate fell back to the Yarn 1 in its own image instead of resolving
Yarn from `packageManager`, so the lockfile is not updated properly.

This is a known upstream bug, not our config. `generateLockFile` for
Yarn chooses between Corepack and the Yarn 1 fallback using
`managerData.hasPackageManager`, and that flag gets lost on some update
shapes. Open since November 2023 as
[renovate#25853](renovatebot/renovate#25853),
with an unmerged fix in
[renovate#45153](renovatebot/renovate#45153).
One of our five failures, `fast-xml-parser`, is in the root
`resolutions` block, which is exactly the case that PR describes.

What changed today is how often we hit it. `renovatebot/github-action`
defaults `renovate-version` to the floating `44` tag, so pinning the
action by SHA does nothing for the Renovate version inside it. From our
own run logs:

| Run | Renovate | Branches with PRs | Artifact failures |
| --- | --- | --- | --- |
| 21 Sep 18:16 | 44.106.0 | 6 | 0 |
| 22 Sep 15:31 | 44.107.0 | 11 | 5 |

`renovate/execa-10.x` is the controlled case: it produced a correct
lockfile on 44.106.0 and failed on 44.107.0.

So this pins `renovate-version` to 44.106.0. Treat it as a stopgap
rather than a root cause fix. Nothing in the 44.107.0 changelog touches
Yarn or Corepack, its six commits are all Helm, mise, docs and CI, so
the trigger is more likely the rebuilt base image or cache state than
Renovate source. If the failures come back on 44.106.0 we will know the
pin was the wrong lever and the real answer is waiting on
renovate#45153.

Deliberately no `# renovate:` annotation on the pin: nothing in CI
exercises a Renovate run, so an automatic bump would put us back here
without warning.

The already open pull requests need their rebase checkbox ticked
afterwards to pick up a good lockfile.

## References

Follows MetaMask#10334. Part of WPC-1161.

## Checklist

- [ ] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [ ] I've communicated my changes to consumers by [updating changelogs
for packages I've
changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md)
- [ ] I've introduced [breaking
changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md)
in this PR and have prepared draft pull requests for clients and
consumer packages to resolve them

This branch was successfully deployed

1 active deployment
default-branch 4923adb9 Deployed Sep 21, 2026 by cryptodev-2s via Determine whether this PR is a release PR #4346
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.

2 participants