ci: migrate to OIDC trusted publishing and align workflows with module template - #421
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| "packageManager": "yarn@4.16.0+sha256.ba05224324578801b9cc98170d64aa50b9a36733b440fb0942306da3fbbdc7d1", | ||
| "engines": { | ||
| "node": "^18.18 || ^20.14 || >=22" | ||
| "node": "^20 || ^22 || >=24" |
There was a problem hiding this comment.
This is a breaking change. You should create a separate PR for this, and bump to ^22 || ^24 || >=26 matching the module template.
There was a problem hiding this comment.
Done — split out into #422 (chore!: drop support for Node.js 18 and 20), which bumps engines.node to ^22 || ^24 || >=26 to match the module template and adds a **BREAKING** changelog entry. That PR is merged, and this one has been rebased on top of main, so the engines change no longer appears here.
| @@ -1 +1 @@ | |||
| v18 | |||
There was a problem hiding this comment.
We use lts/* in the module template now. You can do that as part of the separate Node.js bump PR though.
There was a problem hiding this comment.
Done in #422 — .nvmrc is now lts/*. Rebased here, so .nvmrc is no longer part of this PR.
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - name: Enable Corepack | ||
| run: corepack enable |
There was a problem hiding this comment.
Might be a good time to align these more with the module template too. We use action-checkout-and-setup which removes the need for manual checkout, corepack, setup Node.js, install dependencies.
There was a problem hiding this comment.
Good call — action-checkout-and-setup would remove the manual checkout / corepack enable / setup-node / install boilerplate across these workflows (and eliminate the corepack enable steps I added here). To keep this PR focused on the OIDC migration and avoid a broad workflow refactor, I have kept the minimal changes for now. Happy to align the workflows with the module template (action-checkout-and-setup) — would you prefer I fold that into this PR, or track it as a separate follow-up?
There was a problem hiding this comment.
Fine to make this one big "align with module template" PR. You should mostly be able to just 1:1 copy the workflows (and other required files) from the template.
## What is the current state of things and why does it need to change? `@metamask/eth-sig-util` currently supports Node.js 18 and 20 (`engines.node: ^18.18 || ^20.14 || >=22`). Node.js 18 is end-of-life and 20 is entering maintenance, and the current MetaMask module template has moved its baseline to Node.js 22. This bump is also a prerequisite for migrating npm publishing to OIDC trusted publishing via `MetaMask/action-npm-publish@v6`, which requires Node.js >= 22.14.0 (see #421). Splitting it out here keeps that breaking, consumer-facing change isolated with its own changelog entry. ## What is the solution your changes offer and how does it work? Bump the minimum supported Node.js version to 22, aligning with the module template: - `engines.node`: `^18.18 || ^20.14 || >=22` → `^22 || ^24 || >=26` - `.nvmrc`: `v18` → `lts/*` - CI test matrix: `18.x` / `20.x` / `22.x` → `22.x` / `24.x` / `26.x` - README: Node 18 → 22 This PR intentionally does **not** touch the package manager (`yarn@3.2.2`) or the publishing workflow; those are handled in the OIDC migration PR (#421), which will rebase on top of this once merged. ## Are there any smaller changes to this PR that you would like to highlight? - This is a **breaking change** for consumers on Node.js 18 or 20, hence the `!` in the commit and the `**BREAKING**` changelog entry. ## References - Required by the OIDC trusted-publishing migration: #421 - Matches the [module template](https://github.com/MetaMask/metamask-module-template) Node.js baseline (`^22 || ^24 || >=26`, `.nvmrc` `lts/*`) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Policy and CI-only changes with no runtime code paths modified; the main impact is consumers still on Node 18 or 20 must upgrade before installing future releases. > > **Overview** > **Breaking change:** the package no longer supports Node.js 18 or 20. The minimum is **Node.js 22**, reflected in `package.json` `engines.node` (`^22 || ^24 || >=26`), contributor docs, and a **BREAKING** `[Unreleased]` changelog entry. > > CI **prepare**, **build**, and **test** matrices now run on **22.x / 24.x / 26.x** instead of 18.x / 20.x / 22.x. Local dev defaults move from `.nvmrc` `v18` to **`lts/*`**. README setup instructions now ask for Node **22**. > > No library source or signing behavior changes—only supported Node versions and how they are validated in CI. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit d43ef9c. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
Adopt OIDC trusted publishing via MetaMask/action-npm-publish@v6, aligning with the module-template reference and sibling MetaMask packages. - Upgrade MetaMask/action-npm-publish v1 -> v6 in the dry-run and publish jobs. - Add id-token: write permission to the publish-npm job so an OIDC token can be minted; NPM_TOKEN is now only needed for the very first publish and can be deleted from the npm-publish environment afterward. - v6 requires Yarn >= 4.16.0 and Node >= 22.14.0 and no longer supports npm, so bump the toolchain: yarn 3.2.2 -> 4.16.0 (via Corepack, using the packageManager integrity hash), .nvmrc v18 -> v22, engines.node to ^20 || ^22 || >=24, and drop Node 18 from the CI test matrix. - Switch Yarn resolution to Corepack: remove yarnPath and the committed .yarn/releases binary, and add 'corepack enable' to every workflow that runs Yarn. - Regenerate yarn.lock in the Yarn 4 format (metadata v6 -> v10).
action-npm-publish@v6 runs `yarn pack`/`yarn npm publish` against the node-modules linker, which requires `node_modules/.yarn-state.yml`. The publish jobs previously restored only `./dist`, so `yarn pack` failed with "Couldn't find the node_modules state file" even with SKIP_PREPACK set. Cache and restore `./node_modules` alongside `./dist` (keyed on the release SHA) in the build, dry-run, and publish jobs so the state file is present.
0e6c60c to
1917a08
Compare
Restructure the GitHub Actions workflows to match the module template, replacing the manual checkout / corepack / setup-node / install boilerplate with MetaMask/action-checkout-and-setup@v3 and adopting the reusable workflow pattern. - Add main.yml orchestrator that runs check-workflows + build-lint-test on push/PR, and gates the release (is-release -> publish-release caller with id-token: write). - Replace build-test.yml with reusable build-lint-test.yml (workflow_call, action-checkout-and-setup@v3, Node 22/24/26 matrix). - Rewrite publish-release.yml as a reusable workflow using artifacts (upload/download-artifact) instead of actions/cache, action-npm-publish@v6, action-publish-release@v3, and split docs/GitHub-release jobs. - Update create-release-pr.yml (action-create-release-pr@v5) and publish-docs.yml / publish-rc-docs.yml to action-checkout-and-setup@v3. - Add .github/actionlint.yml and .github/actionlint-matcher.json. Adaptations to eth-sig-util (vs a literal template copy): keep the existing standalone security-code-scanner.yml, keep GITHUB_TOKEN for docs deploys, use auto-changelog for changelog validation, and omit the Slack integration (the repo has no SLACK_WEBHOOK_URL secret).
auto-changelog@3.1.0 bundles an old yargs that uses CommonJS `require` in a way that throws "require is not defined in ES module scope" on Node.js 26 (which the CI lint job now runs on after the module-template alignment). Bumping to ^6.2.1 (the version the module template uses) restores `yarn auto-changelog validate` on the 22/24/26 matrix.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning MetaMask internal reviewing guidelines:
|
The actionlint.yml config existed only to suppress a warning caused by the empty-string option in create-release-pr's `type: choice` release-type input, and actionlint-matcher.json only added inline PR annotations. Neither is used by the sibling library repos. Revert create-release-pr.yml's release-type to a plain string input (as before this migration), which removes the empty-string option and therefore the need for the actionlint config. Drop both files and the associated `::add-matcher::` line.
OGPoyraz
changed the title
OGPoyraz
changed the title
| if ! git diff --exit-code; then | ||
| echo "Working tree dirty at end of job" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
The module template adds a compatibility-test job to build-lint-test.yml which ensures that minor and patch bumps to dependencies don't accidentally cause breaking changes (ensuring that if the package is installed in another project it doesn't have issues). Does it make sense to add this while we are at it?
| fi | |
| fi | |
| compatibility-test: | |
| name: Compatibility test | |
| needs: prepare | |
| runs-on: ubuntu-latest | |
| strategy: | |
| matrix: | |
| node-version: [22.x, 24.x, 26.x] | |
| steps: | |
| - name: Checkout and setup environment | |
| uses: MetaMask/action-checkout-and-setup@v3 | |
| with: | |
| is-high-risk-environment: false | |
| persist-credentials: false | |
| node-version: ${{ matrix.node-version }} | |
| - name: Install dependencies via Yarn | |
| run: rm yarn.lock && YARN_ENABLE_IMMUTABLE_INSTALLS=false yarn | |
| - run: yarn test | |
| - name: Restore lockfile | |
| run: git restore yarn.lock | |
| - name: Require clean working directory | |
| shell: bash | |
| run: | | |
| if ! git diff --exit-code; then | |
| echo "Working tree dirty at end of job" | |
| exit 1 | |
| fi |
There was a problem hiding this comment.
It looks like there have been other changes to create-release-pr.yml made since this repo was created, is it worth overwriting the file completely using https://github.com/MetaMask/metamask-module-template/blob/main/.github/workflows/create-release-pr.yml?
There was a problem hiding this comment.
It looks like main.yml in the module template has more changes, notably it passes SLACK_WEBHOOK_URL to publish-release so that @MetaMask/npm-publishers will be notified when there is a new release of eth-sig-util. Is it worth copying this file completely? https://github.com/MetaMask/metamask-module-template/blob/main/.github/workflows/main.yml
There was a problem hiding this comment.
Done, copied main.yml from the template.
| workflow_call: | ||
| secrets: | ||
| NPM_TOKEN: | ||
| required: false |
There was a problem hiding this comment.
In module template this file accepts a SLACK_WEBHOOK_URL and then uses it in the publish-npm-dry-run job, thoughts on adding that here?
| required: false | |
| required: false | |
| SLACK_WEBHOOK_URL: | |
| required: true |
| uses: MetaMask/action-npm-publish@v1 | ||
| uses: MetaMask/action-npm-publish@v6 | ||
| with: | ||
| dry-run: true |
There was a problem hiding this comment.
Thoughts on adding?
| dry-run: true | |
| dry-run: true | |
| slack-webhook-url: ${{ secrets.SLACK_WEBHOOK_URL }} | |
| subteam: S042S7RE4AE # @metamask-npm-publishers |
Address review feedback by pulling in the module-template pieces that were initially adapted or omitted: - Overwrite main.yml and create-release-pr.yml 1:1 with the template (re-adds the analyse-code security scan job, the actionlint problem matcher, the type:choice release-type input, and the oxfmt changelog formatter). - Restore Slack notifications: publish-release.yml now requires SLACK_WEBHOOK_URL and the dry-run job pings @metamask-npm-publishers. - Add the compatibility-test job to build-lint-test.yml (installs with a floating lockfile to catch breaking transitive dependency bumps). - Remove the standalone security-code-scanner.yml, now covered by the analyse-code job in main.yml. - Restore .github/actionlint.yml and .github/actionlint-matcher.json. Kept GITHUB_TOKEN for docs deploys (no PUBLISH_DOCS_TOKEN dependency).
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
zizmor found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
The analyse-code job (added with the module-template alignment) runs zizmor, whose blanket policy requires every action to be pinned to a full commit hash. Our workflows pin actions/* and MetaMask/* to version tags (as the module template does), which triggered 22 unpinned-uses errors. Add .github/zizmor.yml from the module template, which sets a ref-pin policy for actions/* and MetaMask/* (third-party actions like peaceiris are still required to be hash-pinned, and already are).
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2463920. Configure here.
The module template sets `formatter: 'oxfmt'` because it depends on oxfmt, but eth-sig-util uses prettier and has no oxfmt dependency. `auto-changelog` loads the formatter at runtime, so create-release-pr would fail to format CHANGELOG.md with a missing formatter. Omit the `formatter` input so action-create-release-pr@v5 uses its default (`prettier`), which the repo already has.
## Explanation The `9.0.0` release ([#420](#420)) was tagged but **never published to npm** — the team decided to migrate publishing to OIDC trusted publishing ([#421](#421)) before cutting the release. This PR un-releases `9.0.0` so it can be re-cut cleanly on top of the OIDC migration. ## Changes - **`package.json`**: `9.0.0` → `8.2.0`. - **`CHANGELOG.md`**: - Remove the `## [9.0.0]` section heading and fold its entries back under `## [Unreleased]`: - **BREAKING**: Reject ambiguous `bool` values in `signTypedData` ([#419](#419)) - Add additional validation to EIP7702 methods ([#410](#410)) - Reset the `[Unreleased]` compare link to `v8.2.0...HEAD` and drop the `[9.0.0]` link. The Node.js 18/20 drop entry ([#422](#422)), which was added after `9.0.0` was cut, is preserved under `[Unreleased]`. ## Follow-ups (not in this PR) - The existing `v9.0.0` **git tag** (`646b724`) will be deleted separately before the new `release/9.0.0` PR (npm was never published, so nothing to unpublish). - After this merges, a fresh `release/9.0.0` PR will be created via the standard release workflow. ## References - Reverts the release portion of #420 - Depends on OIDC migration #421 (merged)
Switch the gh-pages deploy from the static PUBLISH_DOCS_TOKEN secret to
the org's OIDC token-exchange service, matching the pattern already
used by MetaMask/snaps (publish-github-pages.yml).
Each job requests a short-lived contents:write token via
MetaMask/github-tools/.github/actions/get-token@v1, which exchanges
the job's signed OIDC identity token against ${{ vars.TOKEN_EXCHANGE_URL
}} (an org variable already inherited by this repo). That token is fed
into peaceiris/actions-gh-pages as personal_token, replacing the
static PUBLISH_DOCS_TOKEN secret used in the previous commit.
Benefits over the static-token approach:
- No long-lived secret stored in the github-pages environment.
- Token is minted per-run and expires in minutes.
- Nothing to rotate or accidentally leave stale (this is what caused
the original outage: PUBLISH_DOCS_TOKEN silently going unused for
months after #421 switched to GITHUB_TOKEN).
Changes:
- publish-docs.yml: add id-token: write permission, add "Get access
token" step using MetaMask/github-tools/.github/actions/get-token@v1,
use the exchanged token as personal_token instead of
secrets.PUBLISH_DOCS_TOKEN. Drop the now-unused PUBLISH_DOCS_TOKEN
secret declaration.
- publish-main-docs.yml / publish-rc-docs.yml: update caller
permissions to contents: read + id-token: write; drop the
PUBLISH_DOCS_TOKEN secret forwarding (no longer needed).
- publish-release.yml: same permission update for both gh-pages jobs;
drop the PUBLISH_DOCS_TOKEN secret requirement and forwarding.
- main.yml: drop the now-unused PUBLISH_DOCS_TOKEN forward to
publish-release.yml.
4 participants
What is the current state of things and why does it need to change?
@metamask/eth-sig-util's CI/publish setup had drifted from the metamask-module-template:MetaMask/action-npm-publish@v1(token-based) with noid-token: write, so it could not use OIDC. Core Platform is enforcing trusted publishing via OIDC and staged publishing, and non-staged publishes will be rejected.checkout/corepack enable/setup-node/yarn installboilerplate instead of the sharedaction-checkout-and-setupaction and the reusablemain.ymlorchestrator pattern.packageManager: yarn@3.2.2was below whataction-npm-publish@v6requires.What is the solution your changes offer and how does it work?
Aligns the GitHub Actions setup with the module template and adopts OIDC trusted publishing:
action-npm-publish@v1→@v6(dry-run + publish), addid-token: write, and makeNPM_TOKENoptional (only needed for the first publish, then deleted from thenpm-publishenvironment).packageManagertoyarn@4.16.0(integrity hash), removeyarnPathand the committed.yarn/releasesbinary.main.ymlorchestrator (runscheck-workflows+build-lint-teston push/PR, gates the release viais-release→publish-release); convertbuild-lint-test.ymlandpublish-release.ymlto reusableworkflow_callworkflows usingMetaMask/action-checkout-and-setup@v3and artifact-based build/publish; updatecreate-release-pr.yml(action-create-release-pr@v5),publish-docs.yml, andpublish-rc-docs.yml.@metamask/auto-changelog^3.1.0→^6.2.1soauto-changelog validateworks on the Node 22/24/26 CI matrix.yarn.lockin the Yarn 4 format. No dependency versions changed beyondauto-changelog.Intentional adaptations (vs a literal template copy)
security-code-scanner.yml(not folded intomain.yml).GITHUB_TOKENfor docs deploys (noPUBLISH_DOCS_TOKENdependency).SLACK_WEBHOOK_URLsecret).auto-changelogfor changelog validation (nolint:changelogscript).release-typeinput (notype: choice), which avoids needing anactionlint.ymlconfig file.Once merged, trusted publishing must be configured for
@metamask/eth-sig-utilon the NPM side (via @metamask-npm-publishers) before the next release.References
Note
High Risk
Changes npm publish flow (OIDC, artifacts, job ordering) and release gating on main, so a misconfiguration could block or mishandle releases.
Overview
Replaces the monolithic Build, Lint, and Test workflow with a
main.ymlorchestrator on push/PR that runs actionlint (new matcher + config, including an ignore for intentional emptyrelease-type), a reusable security scan (replacing the standalone scanner workflow), and aworkflow_callbuild/lint/test pipeline usingMetaMask/action-checkout-and-setup@v3. Build and lint now run on Node 26.x only (tests still matrix 22/24/26), and a new compatibility-test job reinstalls deps without the lockfile before restoring it.Release publishing is no longer triggered directly from
publish-release.ymlonmainpushes;main.ymlgatesis-release→publish-release.yml(now callable). That workflow builds once, uploads artifacts, runs npm dry-run + publish viaaction-npm-publish@v6withid-token: write, optionalNPM_TOKEN, then docs deploys and a separateaction-publish-release@v3GitHub release step.create-release-prmoves to the shared checkout action andaction-create-release-pr@v5, withrelease-typeas a choice (including empty). Docs publishing addsgithub-pagesenvironment, shared checkout/setup, and bumps peaceiris/actions-gh-pages. Adds.github/zizmor.ymlfor workflow lint policy tweaks.Reviewed by Cursor Bugbot for commit 8b88ff0. Bugbot is set up for automated code reviews on this repo. Configure here.