Skip to content

ci: Add top-level permissions to publish-release workflow#9026

Merged
Mrtenz merged 1 commit into
mainfrom
mrtenz/publish-release-top-level-permissions
Jun 5, 2026
Merged

ci: Add top-level permissions to publish-release workflow#9026
Mrtenz merged 1 commit into
mainfrom
mrtenz/publish-release-top-level-permissions

Conversation

@Mrtenz
Copy link
Copy Markdown
Member

@Mrtenz Mrtenz commented Jun 5, 2026

Explanation

By default, all jobs in a workflow inherit the permissions provided to it in the caller (main.yml in this case). This meant that publish-release.yml had contents: write and id-token: write in all jobs, including the dry run job. This made the dry run job think it was an actual run, and try to publish (which failed). By explicitly specifying permissions, jobs will inherit those permissions instead. We do this in the module template too.

References

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
CI-only permission scoping for GitHub Actions; no application or runtime behavior changes.

Overview
Adds workflow-level permissions: contents: read to the reusable publish-release workflow so jobs no longer inherit contents: write and id-token: write from the caller in main.yml.

Without this default, the publish-npm-dry-run job received publish-level permissions and the npm publish action treated the run as a real release instead of a dry run. Jobs that still need elevated access keep explicit overrides: publish-release sets contents: write, and publish-npm sets contents: read plus id-token: write.

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

@Mrtenz Mrtenz requested a review from a team as a code owner June 5, 2026 15:03
@Mrtenz Mrtenz temporarily deployed to default-branch June 5, 2026 15:03 — with GitHub Actions Inactive
@Mrtenz Mrtenz mentioned this pull request Jun 5, 2026
4 tasks
@Gudahtt
Copy link
Copy Markdown
Member

Gudahtt commented Jun 5, 2026

Hmm. This is a fairly subtle problem. We should have a more explicit way than this to enable dry runs.

Making the dry-run contingent on the presence of the npm token was already pretty bad IMO, this is worse.

@Mrtenz
Copy link
Copy Markdown
Member Author

Mrtenz commented Jun 5, 2026

Hmm. This is a fairly subtle problem. We should have a more explicit way than this to enable dry runs.

Making the dry-run contingent on the presence of the npm token was already pretty bad IMO, this is worse.

Yes, I agree. In hindsight we should've added an explicit dry-run option. I don't know if we can do that now without making a breaking change.

Copy link
Copy Markdown
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to know. Knowing when and where to specify permissions is always confusing to me :/

@Mrtenz Mrtenz added this pull request to the merge queue Jun 5, 2026
Merged via the queue into main with commit 397c05f Jun 5, 2026
374 checks passed
@Mrtenz Mrtenz deleted the mrtenz/publish-release-top-level-permissions branch June 5, 2026 15:14
pull Bot pushed a commit to dmrazzy/core that referenced this pull request Jun 5, 2026
## Explanation

This is the release candidate for version `1022.0.0`. Blocked by this
change:

- MetaMask#9026

## References

<!--
Are there any issues that this pull request is tied to?
Are there other links that reviewers should consult to understand these
changes better?
Are there client or consumer pull requests to adopt any breaking
changes?

For example:

* Fixes #12345
* Related to #67890
-->

## 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](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

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **High Risk**
> Major bump of transaction-controller removes constructor options/hooks
and changes messenger requirements, affecting every package that
constructs or integrates TransactionController across the monorepo.
> 
> **Overview**
> **Monorepo release `1022.0.0`**: bumps the root version from
`1021.0.0` and aligns dependents on **`@metamask/transaction-controller`
`^67.0.0`** (from `^66.0.1`) across assets, bridge, EIP-5792,
gator-permissions, phishing, shield, subscription, user-operation, and
related packages, with matching **`yarn.lock`** updates and
**Unreleased** changelog notes.
> 
> **`@metamask/transaction-controller` `67.0.0`** is cut in this release
(changelog section moved out of Unreleased). Consumers should treat
**67** as **breaking**: deprecated constructor options and several
hooks/types are removed in favor of required messenger actions
(`GasFeeController`, `KeyringController`, `NetworkController`), and
**`PendingTransactionTracker` resubmit logic is removed**.
> 
> **`@metamask/transaction-pay-controller` `23.2.0`** is also versioned
in this PR (package + changelog compare links); its listed
features/changes live in that package’s changelog entry for 23.2.0.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
a9e369c. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

3 participants