From 5c2bd571c4eb5c7943f5e021a5ba079c86dadf15 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Tue, 8 Sep 2026 16:13:18 +0200 Subject: [PATCH] ci: own the setup, changelog, and merge-queue steps instead of calling out for them Pinning our own references was necessary but not sufficient. The policy resolves actions transitively, and the MetaMask actions we call reach mutable tags one layer down: action-checkout-and-setup -> cache/restore@v6, checkout@v7, setup-node@v6, action-retry-command@v1, cache/save@v6 github-tools/check-changelog -> checkout@v6 x2, setup-node@v6 github-tools/check-skip-merge-queue -> github-script@v9 x2 All three are on their latest releases, so no upgrade reaches them, and they live in an organization we no longer belong to. A pin one layer deep is no pin at all, so the only way to make the chain true is to hold it. `action-checkout-and-setup` becomes an explicit `actions/checkout` plus a local `setup-environment` action -- Node, Corepack, Yarn's download cache, install. Checkout cannot live inside it: `uses: ./...` resolves against the workspace, so the repository has to be on disk before the runner can find the file. The local action is narrower than the one it replaces. It drops the node_modules cache and the lookup-only fast path, keeping only Yarn's download cache, because those turn on a cache key that has to account for the native rebuilds `postinstall` runs. `yarn install` runs every job now, which costs time and buys the guarantee that what is on disk matches the lockfile. `check-changelog` cloned github-tools to run a script over the diff. The local job asks the same question directly: every package with a CHANGELOG that this pull request touches must have that changelog in the diff too. It compares against the merge base rather than the base tip, so commits landing on main afterwards are not read as this branch's. `no-changelog` still opts out. `check-skip-merge-queue` is ported as-is, its two scripts unchanged, onto a pinned `actions/github-script`. The release path still calls `action-npm-publish` and `action-is-release`, which have the same defect. Both are gated on `push`, so they block releases rather than pull requests, and neither is part of the `all-jobs-complete` gate. Publishing is where a mistake is expensive and OIDC permissions were only just fixed in #1041, so they are better handled upstream than rewritten here. Co-Authored-By: Claude Opus 5 (1M context) --- .../actions/check-skip-merge-queue/action.yml | 119 ++++++++++++++++++ .github/actions/setup-environment/action.yml | 65 ++++++++++ .github/workflows/changelog-check.yml | 60 +++++++-- .github/workflows/lint-build-test.yml | 50 ++++---- .github/workflows/main.yml | 9 +- .github/workflows/publish-release.yml | 26 ++-- 6 files changed, 287 insertions(+), 42 deletions(-) create mode 100644 .github/actions/check-skip-merge-queue/action.yml create mode 100644 .github/actions/setup-environment/action.yml diff --git a/.github/actions/check-skip-merge-queue/action.yml b/.github/actions/check-skip-merge-queue/action.yml new file mode 100644 index 000000000..7b505b50e --- /dev/null +++ b/.github/actions/check-skip-merge-queue/action.yml @@ -0,0 +1,119 @@ +name: Check skip merge queue +description: >- + Report whether the pull request in a merge group is first in the queue and + already up to date with the base branch, in which case the checks about to run + would test a commit nothing has changed under. + + Only the report is produced here. Nothing can make the merge queue skip an + entry from inside a workflow, so callers gate their own jobs on the output. + +# Ported from MetaMask/github-tools/.github/actions/check-skip-merge-queue, +# which calls actions/github-script by mutable tag. See the note in +# ../checkout-and-setup/action.yml for why that cannot be pinned from outside. + +inputs: + head-ref: + description: The merge group's head ref, which carries the pull request number. + required: true + default: ${{ github.event.merge_group.head_ref }} + base-ref: + description: The branch the pull request is merging into. + required: true + default: main + github-token: + description: The token used to read the pull request and compare branches. + required: true + default: ${{ github.token }} + +outputs: + up-to-date: + description: >- + Whether the pull request is first in the queue and up to date with the + base branch. `false` whenever that cannot be established, so a failure + here costs a redundant run rather than an untested merge. + value: ${{ steps.up-to-date.outputs.up-to-date || 'false' }} + +runs: + using: composite + steps: + # `continue-on-error` on both steps: this is an optimization, and the safe + # answer is always available. A throw here leaves `up-to-date` empty, which + # the output above reads as `false`. + - name: Find the pull request behind the merge group + continue-on-error: true + id: pr-details + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + env: + HEAD_REF: ${{ inputs.head-ref }} + with: + github-token: ${{ inputs.github-token }} + script: | + const { HEAD_REF } = process.env; + const match = HEAD_REF.match(/\/pr-([0-9]+)-/u); + if (!match) { + return core.setFailed(`Could not extract pull request number from head ref: "${HEAD_REF}".`); + } + + const number = parseInt(match[1], 10); + const result = await github.graphql(` + query($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + headRefName + mergeQueueEntry { position } + } + } + } + `, { + owner: context.repo.owner, + name: context.repo.repo, + number, + }); + + if (!result.repository.pullRequest) { + return core.setFailed(`Pull request #${number} not found in repository "${context.repo.owner}/${context.repo.repo}".`); + } + + const position = result.repository.pullRequest.mergeQueueEntry?.position; + if (!position) { + return core.setFailed(`Pull request #${number} is not in the merge queue.`); + } + + core.setOutput('pr-number', number); + core.setOutput('pr-branch', result.repository.pullRequest.headRefName); + core.setOutput('merge-queue-position', position); + + - name: Check whether it is up to date with the base branch + continue-on-error: true + id: up-to-date + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + env: + BASE_REF: ${{ inputs.base-ref }} + PR_BRANCH: ${{ steps.pr-details.outputs.pr-branch }} + MERGE_QUEUE_POSITION: ${{ steps.pr-details.outputs.merge-queue-position }} + with: + github-token: ${{ inputs.github-token }} + script: | + const { BASE_REF, PR_BRANCH, MERGE_QUEUE_POSITION } = process.env; + + // Only the first entry can be up to date with the base branch by + // definition; anything behind it is waiting on merges above it. + if (parseInt(MERGE_QUEUE_POSITION, 10) !== 1) { + core.info(`Pull request is not first in the merge queue (position: ${MERGE_QUEUE_POSITION}).`); + core.setOutput('up-to-date', 'false'); + return; + } + + const comparison = await github.rest.repos.compareCommitsWithBasehead({ + owner: context.repo.owner, + repo: context.repo.repo, + basehead: `${BASE_REF}...${PR_BRANCH}`, + }); + + if (comparison.data.status === 'identical' || comparison.data.status === 'ahead') { + core.info(`Pull request branch "${PR_BRANCH}" is up-to-date with base branch "${BASE_REF}".`); + core.setOutput('up-to-date', 'true'); + } else { + core.info(`Pull request branch "${PR_BRANCH}" is not up-to-date with base branch "${BASE_REF}".`); + core.setOutput('up-to-date', 'false'); + } diff --git a/.github/actions/setup-environment/action.yml b/.github/actions/setup-environment/action.yml new file mode 100644 index 000000000..9fe1d67d5 --- /dev/null +++ b/.github/actions/setup-environment/action.yml @@ -0,0 +1,65 @@ +name: Set up environment +description: Set up Node.js and Yarn and install dependencies, in a repository already checked out. + +# Replaces the setup half of MetaMask/action-checkout-and-setup. That action is +# pinned by SHA where we call it, but calls actions/checkout, actions/setup-node +# and actions/cache by mutable tag internally, which this organization refuses -- +# rightly, since a pin one layer deep is no pin at all. Keeping the steps here is +# the only way to pin the whole chain, and the chain is short. +# +# Checkout is not part of this action and cannot be: `uses: ./...` resolves +# against the workspace, so the repository has to be on disk before the runner +# can find this file. Callers check out first, with their own pinned step. +# +# Deliberately narrower than the action it replaces: no node_modules cache and +# no lookup-only fast path, only Yarn's download cache. Those are worth real +# minutes on a monorepo this size, but they turn on a cache key that has to +# account for the native rebuilds `postinstall` runs, and getting that subtly +# wrong yields a job that passes against stale binaries. `yarn install` runs +# every time here, so what is on disk always matches the lockfile. + +inputs: + node-version: + description: The Node.js version to use. Defaults to the one in `.nvmrc`. + required: false + default: '' + cache: + description: >- + Whether to restore and save Yarn's download cache. Pass `false` from jobs + holding publish credentials, where a poisoned entry buys an attacker more + than the cache saves us. + required: false + default: 'true' + +runs: + using: composite + steps: + - name: Set up Node.js + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: ${{ inputs.node-version }} + node-version-file: ${{ inputs.node-version == '' && '.nvmrc' || '' }} + + - name: Enable Corepack + shell: bash + run: corepack enable + + # `enableGlobalCache: false` puts Yarn's downloads in `.yarn/cache`, which is + # gitignored, so without this every job refetches the whole dependency tree. + # Restoring a partial cache is still worth it, hence `restore-keys`: a + # lockfile change invalidates the key but leaves most entries reusable. + - name: Restore Yarn's download cache + if: inputs.cache == 'true' + uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 + with: + path: .yarn/cache + key: yarn-${{ runner.os }}-${{ inputs.node-version }}-${{ hashFiles('yarn.lock') }} + restore-keys: | + yarn-${{ runner.os }}-${{ inputs.node-version }}- + yarn-${{ runner.os }}- + + # The LavaMoat plugin in `.yarnrc.yml` runs the scripts `lavamoat.allowScripts` + # permits, so there is no separate `yarn allow-scripts` step to run. + - name: Install dependencies + shell: bash + run: yarn install --immutable diff --git a/.github/workflows/changelog-check.yml b/.github/workflows/changelog-check.yml index 930dd3b60..88cc67c4c 100644 --- a/.github/workflows/changelog-check.yml +++ b/.github/workflows/changelog-check.yml @@ -8,12 +8,58 @@ jobs: check-changelog: name: Check changelog runs-on: ubuntu-latest + # `no-changelog` is the escape hatch for a pull request that genuinely + # changes no published behaviour. + if: ${{ !contains(github.event.pull_request.labels.*.name, 'no-changelog') }} steps: - - name: Check changelog - uses: MetaMask/github-tools/.github/actions/check-changelog@d0f155aad5d7b5403d29a094c2f752238bf43237 # v1.18.2 + - name: Check out the repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - base-branch: ${{ github.event.pull_request.base.ref }} - head-ref: ${{ github.head_ref }} - labels: ${{ toJSON(github.event.pull_request.labels) }} - pr-number: ${{ github.event.pull_request.number }} - repo: ${{ github.repository }} + # The whole comparison is against the merge base, so the history back + # to it has to be here. + fetch-depth: 0 + + - name: Require a changelog entry for every package this changes + shell: bash + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + set -euo pipefail + + # Against the merge base rather than the base tip, so that commits + # landing on the base branch after this pull request opened are not + # read as its own changes. + merge_base="$(git merge-base "$BASE_SHA" "$HEAD_SHA")" + changed="$(git diff --name-only "$merge_base" "$HEAD_SHA")" + + missing=() + for package_dir in packages/*/; do + package="${package_dir%/}" + changelog="${package}/CHANGELOG.md" + + # A package with no changelog is private and releases nothing, so + # there is no entry for it to be missing. This is the same set + # `yarn changelog:validate` covers, which runs `--no-private`. + [[ -f "$changelog" ]] || continue + + # Changes to the changelog itself do not oblige a changelog entry. + if ! grep -qE "^${package}/" <<< "$changed" \ + || [[ "$(grep -E "^${package}/" <<< "$changed")" == "$changelog" ]]; then + continue + fi + + grep -qxF "$changelog" <<< "$changed" || missing+=("$package") + done + + if [[ ${#missing[@]} -gt 0 ]]; then + echo "::error::These packages changed without a changelog entry:" + printf ' %s\n' "${missing[@]}" + echo + echo "Add an entry under '## [Unreleased]' in each package's CHANGELOG.md," + echo "or apply the 'no-changelog' label if this genuinely changes no" + echo "published behaviour." + exit 1 + fi + + echo "Every changed package has a changelog entry." diff --git a/.github/workflows/lint-build-test.yml b/.github/workflows/lint-build-test.yml index 4d2a83f93..a75af652a 100644 --- a/.github/workflows/lint-build-test.yml +++ b/.github/workflows/lint-build-test.yml @@ -11,11 +11,11 @@ jobs: matrix: node-version: [22.x, 24.x] steps: - - name: Checkout and setup environment - uses: MetaMask/action-checkout-and-setup@0543b5929698c71e3ccc6ed24eac87825669b5de # v3.5.0 + - name: Check out the repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Set up environment + uses: ./.github/actions/setup-environment with: - cache-node-modules: true - is-high-risk-environment: false node-version: ${{ matrix.node-version }} lint: @@ -26,10 +26,11 @@ jobs: matrix: node-version: [24.x] steps: - - name: Checkout and setup environment - uses: MetaMask/action-checkout-and-setup@0543b5929698c71e3ccc6ed24eac87825669b5de # v3.5.0 + - name: Check out the repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Set up environment + uses: ./.github/actions/setup-environment with: - is-high-risk-environment: false node-version: ${{ matrix.node-version }} - run: yarn lint - name: Require clean working directory @@ -48,10 +49,11 @@ jobs: matrix: node-version: [24.x] steps: - - name: Checkout and setup environment - uses: MetaMask/action-checkout-and-setup@0543b5929698c71e3ccc6ed24eac87825669b5de # v3.5.0 + - name: Check out the repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Set up environment + uses: ./.github/actions/setup-environment with: - is-high-risk-environment: false node-version: ${{ matrix.node-version }} - run: yarn changelog:validate - name: Require clean working directory @@ -70,10 +72,11 @@ jobs: matrix: node-version: [24.x] steps: - - name: Checkout and setup environment - uses: MetaMask/action-checkout-and-setup@0543b5929698c71e3ccc6ed24eac87825669b5de # v3.5.0 + - name: Check out the repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Set up environment + uses: ./.github/actions/setup-environment with: - is-high-risk-environment: false node-version: ${{ matrix.node-version }} - run: yarn build - name: Require clean working directory @@ -92,10 +95,11 @@ jobs: matrix: node-version: [22.x, 24.x] steps: - - name: Checkout and setup environment - uses: MetaMask/action-checkout-and-setup@0543b5929698c71e3ccc6ed24eac87825669b5de # v3.5.0 + - name: Check out the repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Set up environment + uses: ./.github/actions/setup-environment with: - is-high-risk-environment: false node-version: ${{ matrix.node-version }} - uses: ./.github/actions/playwright-install - run: yarn build @@ -123,10 +127,11 @@ jobs: matrix: node-version: [22.x, 24.x] steps: - - name: Checkout and setup environment - uses: MetaMask/action-checkout-and-setup@0543b5929698c71e3ccc6ed24eac87825669b5de # v3.5.0 + - name: Check out the repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Set up environment + uses: ./.github/actions/setup-environment with: - is-high-risk-environment: false node-version: ${{ matrix.node-version }} - run: yarn build - run: yarn test:integration @@ -165,10 +170,11 @@ jobs: - package: '@ocap/evm-wallet-experiment' directory: evm-wallet-experiment steps: - - name: Checkout and setup environment - uses: MetaMask/action-checkout-and-setup@0543b5929698c71e3ccc6ed24eac87825669b5de # v3.5.0 + - name: Check out the repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Set up environment + uses: ./.github/actions/setup-environment with: - is-high-risk-environment: false node-version: ${{ matrix.node-version }} - uses: ./.github/actions/playwright-install - run: VITE_DB_FOLDER=e2e yarn build diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 6d311621f..6ae0250ee 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -24,7 +24,7 @@ jobs: - name: Check pull request merge queue status id: check-skip-merge-queue if: github.event_name == 'merge_group' - uses: MetaMask/github-tools/.github/actions/check-skip-merge-queue@d0f155aad5d7b5403d29a094c2f752238bf43237 # v1.18.2 + uses: ./.github/actions/check-skip-merge-queue detect-changes: name: Detect changes @@ -149,10 +149,11 @@ jobs: matrix: node-version: [24.x] steps: - - name: Checkout and setup environment - uses: MetaMask/action-checkout-and-setup@0543b5929698c71e3ccc6ed24eac87825669b5de # v3.5.0 + - name: Check out the repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Set up environment + uses: ./.github/actions/setup-environment with: - is-high-risk-environment: false node-version: ${{ matrix.node-version }} - run: yarn lint - name: Require clean working directory diff --git a/.github/workflows/publish-release.yml b/.github/workflows/publish-release.yml index 44ec99da2..55635ae12 100644 --- a/.github/workflows/publish-release.yml +++ b/.github/workflows/publish-release.yml @@ -12,10 +12,12 @@ jobs: contents: write runs-on: ubuntu-latest steps: - - name: Checkout and setup environment - uses: MetaMask/action-checkout-and-setup@0543b5929698c71e3ccc6ed24eac87825669b5de # v3.5.0 + - name: Check out the repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Set up environment + uses: ./.github/actions/setup-environment with: - is-high-risk-environment: true + cache: false - uses: MetaMask/action-publish-release@f01f1be110d60fb07d86c880ce3d6bdb353524d3 # v3.3.1 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -40,11 +42,14 @@ jobs: runs-on: ubuntu-latest needs: publish-release steps: - - name: Checkout and setup environment - uses: MetaMask/action-checkout-and-setup@0543b5929698c71e3ccc6ed24eac87825669b5de # v3.5.0 + - name: Check out the repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - is-high-risk-environment: true ref: ${{ github.sha }} + - name: Set up environment + uses: ./.github/actions/setup-environment + with: + cache: false - name: Restore build artifacts uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: @@ -63,11 +68,14 @@ jobs: runs-on: ubuntu-latest needs: publish-npm-dry-run steps: - - name: Checkout and setup environment - uses: MetaMask/action-checkout-and-setup@0543b5929698c71e3ccc6ed24eac87825669b5de # v3.5.0 + - name: Check out the repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - is-high-risk-environment: true ref: ${{ github.sha }} + - name: Set up environment + uses: ./.github/actions/setup-environment + with: + cache: false - name: Restore build artifacts uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: