Skip to content

Test preview workflow#197

Closed
cryptodev-2s wants to merge 5 commits intomainfrom
test-preview-workflow
Closed

Test preview workflow#197
cryptodev-2s wants to merge 5 commits intomainfrom
test-preview-workflow

Conversation

@cryptodev-2s
Copy link
Copy Markdown
Contributor

@cryptodev-2s cryptodev-2s commented Mar 9, 2026

Note

Medium Risk
Changes the preview publish GitHub Actions workflow to delegate logic to an external reusable workflow, which can affect release/CI behavior and introduces dependency on MetaMask/github-tools (not pinned to a SHA).

Overview
Simplifies the preview publish pipeline by replacing the inline publish-preview workflow jobs (fork/PR checks, comment reaction, checkout/build/publish, and PR comment posting) with a single reusable workflow call to MetaMask/github-tools/.github/workflows/publish-preview.yml@prepare-preview-builds-action.

The workflow still triggers from @metamaskbot publish-preview issue comments, and now passes is-monorepo: false, dry-run: true, and forwards PUBLISH_PREVIEW_NPM_TOKEN as a secret.

Written by Cursor Bugbot for commit f922fe8. This will update automatically on new commits. Configure here.

@cryptodev-2s cryptodev-2s requested a review from a team as a code owner March 9, 2026 16:42
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

uses: MetaMask/github-tools/.github/workflows/publish-preview.yml@prepare-preview-builds-action
with:
is-monorepo: false
dry-run: true
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Preview publish uses dry-run mode in production

High Severity

dry-run: true is set on the reusable preview publish workflow, which will prevent any preview builds from actually being published. Given the PR title "Test preview workflow," this appears to be a testing configuration that would break preview publishing for all PRs if merged.

Fix in Cursor Fix in Web

COMMIT_SHA: ${{ steps.commit-sha.outputs.COMMIT_SHA }}
PR_NUMBER: ${{ github.event.issue.number }}
if: ${{ github.event.issue.pull_request && startsWith(github.event.comment.body, '@metamaskbot publish-preview') }}
uses: MetaMask/github-tools/.github/workflows/publish-preview.yml@prepare-preview-builds-action
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reusable workflow pinned to unstable feature branch

Medium Severity

The reusable workflow is referenced at @prepare-preview-builds-action, which looks like a feature branch on MetaMask/github-tools rather than a stable version tag or SHA. If merged, this workflow depends on a branch that could be deleted or force-pushed, breaking preview builds.

Fix in Cursor Fix in Web

is-monorepo: false
dry-run: true
secrets:
PUBLISH_PREVIEW_NPM_TOKEN: ${{ secrets.PUBLISH_PREVIEW_NPM_TOKEN }}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fork protection security check was removed entirely

Medium Severity

The old workflow explicitly checked whether the PR came from a fork (is-fork-pull-request job) and blocked publishing for fork PRs to prevent untrusted code from accessing the PUBLISH_PREVIEW_NPM_TOKEN secret. The new workflow removes this guard entirely at the caller level. If the reusable workflow doesn't implement equivalent fork protection, this could expose the npm publish token to fork PRs.

Fix in Cursor Fix in Web

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.

1 participant