feat: add reusable publish-preview workflow#223
Conversation
Extract the preview build preparation logic into a reusable composite GitHub Action so other repos can use the same logic. The jq filter is inlined and the source scope is parameterized.
|
@cryptodev-2s What are your thoughts on supporting the entire preview build workflow rather than just the "generate preview builds" step? Recently, we've added support for preview builds to other polyrepos, but currently we have to copy around the workflows. It would be nice to consolidate all of the steps into this repo. And we could make the action support both monorepos and polyrepos. What do you think? |
Yes make sense, let me do this. |
Add a `workflow_call` workflow that handles the full preview build flow (fork check, comment reaction, build, publish) for both monorepos and polyrepos. Consumer repos only need a ~13-line thin wrapper with the `issue_comment` trigger. Also add a centralized `generate-preview-build-message.sh` script that auto-detects mono vs polyrepo and produces the appropriate PR comment.
| !./packages/*/node_modules/ | ||
| !./packages/*/src/ | ||
| !./packages/*/tests/ | ||
| !./packages/**/*.test.* |
There was a problem hiding this comment.
Monorepo artifact paths hardcode packages/ directory structure
Medium Severity
The monorepo upload glob ./packages/*/ and the sanitization find packages both hardcode the packages/ directory. However, prepare-preview-builds.sh dynamically discovers workspaces via yarn workspaces list, which works for any directory layout. Monorepos with workspaces outside packages/ (e.g., libs/, modules/) would have their manifests prepared but the built output wouldn't be uploaded or security-validated before publishing.
Additional Locations (1)
There was a problem hiding this comment.
All MetaMask monorepos use packages/, this matches the existing core workflow convention. If a repo ever uses a different layout, we can add a workspace-glob input then.
There was a problem hiding this comment.
We could generate this glob automatically by looping through the workspaces field in package.json. But yes I agree that probably don't need to worry about this for v1.
| !./packages/*/node_modules/ | ||
| !./packages/*/src/ | ||
| !./packages/*/tests/ | ||
| !./packages/**/*.test.* |
There was a problem hiding this comment.
We could generate this glob automatically by looping through the workspaces field in package.json. But yes I agree that probably don't need to worry about this for v1.
| if [[ "$IS_MONOREPO" == "true" ]]; then | ||
| mapfile -t manifests < <(find packages -name package.json -not -path '*/node_modules/*') | ||
| else | ||
| manifests=(package.json) |
There was a problem hiding this comment.
Monorepo sanitization skips root package.json manifest
Medium Severity
In monorepo mode, the sanitization step only scans packages/ for package.json files but the root package.json is also included in the uploaded artifacts (line 142). A malicious PR could inject lifecycle scripts (e.g., postinstall) into the root package.json, which would execute during the yarn install --no-immutable step in the publish job. While the NPM token isn't in the environment at that point, the GITHUB_TOKEN with pull-requests: write permission is accessible to the running process.
There was a problem hiding this comment.
allow-scripts already takes care of this, so I think we are okay?
There was a problem hiding this comment.
Yes not a real concern
mcmire
left a comment
There was a problem hiding this comment.
A few things I noticed, but they are non-blocking. LGTM.
| SOURCE_SCOPE: ${{ inputs.source-scope }} | ||
| IS_MONOREPO: ${{ inputs.is-monorepo }} | ||
| run: | | ||
| prepare_manifest() { |
There was a problem hiding this comment.
Some of the scripts in this workflow could probably be made easier to read/maintain by using actions/github-script.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| path: | | ||
| . | ||
| !./node_modules/ | ||
| !./.git/ |
There was a problem hiding this comment.
Polyrepo artifact includes config files bypassing registry check
Medium Severity
The polyrepo artifact uploads the entire working tree (minus node_modules and .git), which includes .yarnrc.yml. The sanitization step only validates publishConfig.registry in package.json files. A malicious PR could set npmRegistryServer or npmScopes in .yarnrc.yml to redirect yarn npm publish to an attacker-controlled server, exfiltrating the PUBLISH_PREVIEW_NPM_TOKEN. The monorepo path is not affected since its artifact only includes specific paths.
Additional Locations (1)
There was a problem hiding this comment.
Well, we need .yarnrc.yml to run yarn. I guess this is true though. Do we need to perform validation on .yarnrc.yml too? This is getting a bit out of hand 🫤
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| name: Publish preview | ||
| needs: build-preview | ||
| permissions: | ||
| pull-requests: write |
There was a problem hiding this comment.
Missing contents: read permission breaks checkout step
High Severity
The publish-preview job sets permissions: pull-requests: write without including contents: read. When explicit permissions are set, all unspecified scopes default to none. The MetaMask/action-checkout-and-setup@v2 action performs a git checkout internally, which requires contents: read. Comparing with create-release-pr.yml in this same repo, which correctly specifies both contents: write and pull-requests: write when using the same action, confirms this is an oversight. This will cause the checkout to fail, at minimum for private repos.
Additional Locations (1)
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| path: | | ||
| . | ||
| !./node_modules/ | ||
| !./.git/ |
There was a problem hiding this comment.
Polyrepo artifacts include unsanitized .yarnrc.yml config
Medium Severity
The polyrepo artifact upload includes the entire workspace (excluding only node_modules/ and .git/), which means .yarnrc.yml from the PR branch is included. The sanitize step only validates package.json files for unexpected registries but does not inspect .yarnrc.yml. A malicious PR could add npmScopes.metamask-previews.npmRegistryServer pointing to an attacker-controlled URL, causing yarn npm publish to send the YARN_NPM_AUTH_TOKEN to that server. YARN_NPM_REGISTRY_SERVER only overrides the global npmRegistryServer, not scoped registries.


Add a reusable
workflow_callworkflow so consumer repos can replace their copy-pastedpublish-preview.ymlwith a thin wrapper.New files
.github/workflows/publish-preview.yml— Reusable workflow with 4 jobs: fork check, comment reaction, build (no token), publish (with token). Supports both monorepos and polyrepos viais-monorepoinput..github/scripts/generate-preview-build-message.sh— Generates the PR comment (collapsible package list for monorepos, single install command for polyrepos)..github/actions/prepare-preview-builds/— Composite action for monorepo manifest preparation (from prior commit).Consumer example
Polyrepo working example cryptodev-2s/core#3
Monorepo working example cryptodev-2s/create-release-branch#3
Note
Medium Risk
Introduces a new GitHub Actions workflow that publishes to NPM using a secret token; mistakes could leak credentials or publish incorrect packages, though the workflow includes manifest sanitization and registry validation to mitigate this.
Overview
Adds a reusable
workflow_callworkflow (.github/workflows/publish-preview.yml) that builds PR-based preview NPM packages and posts the published versions back to the PR.The workflow supports monorepos and polyrepos, rewrites package names/versions to a preview scope with a commit-SHA suffix, uploads build artifacts, then publishes from a separate job gated by an optional GitHub environment.
It also adds safety checks for non-fork PRs and sanitizes downloaded
package.jsonfiles before publishing by stripping pack/publish/prepare lifecycle scripts and rejecting non-registry.npmjs.orgregistries.Written by Cursor Bugbot for commit 4c4c413. This will update automatically on new commits. Configure here.