Skip to content

Commit 8581c16

Browse files
authored
Fix update-changelogs workflow (MetaMask#8681)
The `update-changelogs` workflow is designed to run automatically and manually (in response to a `metamaskbot` comment on a release PR). Currently, the manual run succeeds, but the automatic run does not. We use the `default-branch` environment for commands in this workflow as we need a tightly-scoped token that to push to the PR branch. The problem is that our use of the `pull_request` event for automatic runs and our use of the `default-branch` environment conflict with each other and trigger our branch protection rules (the `default-branch` is scoped to the `main` branch, not pull request branches). To fix this, instead of `pull_request`, we use the `pull_request_target` event. This event runs against the default branch, and so it should match the same scope as the `default-branch` environment and satisfy the branch protection rules we have in place. However, as this is a dangerous event — [GitHub recommends against it](https://docs.github.com/en/enterprise-cloud@latest/actions/reference/security/secure-use#mitigating-the-risks-of-untrusted-code-checkout) — we have to be careful not to allow commands to run directly against a pull request branch. So, we modify the workflow to follow this plan: 1. Check out the PR base branch (usually `main`) 2. Set up Node, install Yarn + dependencies 3. Overlay changelogs from the PR branch 4. Create a temporary commit to clear the working index 5. Update changelogs with dependency bumps as needed 6. Make a new commit 7. Check out the pull request branch completely (overwriting the base branch) 8. Cherry-pick the new commit on to it 9. Push the pull request branch 10. Done! ## Explanation <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> ## 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 --> https://consensyssoftware.atlassian.net/browse/WPC-997 ## 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] > **Medium Risk** > Medium risk because it changes a privileged `pull_request_target` workflow that checks out refs and pushes commits to PR branches; misconfiguration could create a security or branch-protection bypass. > > **Overview** > Updates `.github/workflows/update-changelogs.yml` to trigger on `pull_request_target` (instead of `pull_request`) so the workflow runs against `main` while using the `default-branch` environment/token. > > Reworks the changelog update flow to **avoid running untrusted PR code**: check out the merge base, detach `HEAD`, overlay only `**/CHANGELOG.md` from the PR, commit those as a temporary baseline, run `yarn changelog:validate --checkDeps --fix` with `continue-on-error`, then commit any resulting changelog fixes and **cherry-pick/push** that commit onto the PR head SHA/ref. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 68a9155. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 22e494c commit 8581c16

1 file changed

Lines changed: 64 additions & 28 deletions

File tree

.github/workflows/update-changelogs.yml

Lines changed: 64 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@ on:
44
issue_comment:
55
types:
66
- created
7-
# pull_request:
8-
# branches:
9-
# - main
10-
# types:
11-
# - opened
7+
pull_request_target:
8+
branches:
9+
- main
10+
types:
11+
- opened
1212

1313
permissions:
1414
contents: write
@@ -17,7 +17,7 @@ permissions:
1717
jobs:
1818
is-fork:
1919
name: Determine whether this PR is from a fork
20-
if: github.event_name == 'pull_request' || (github.event.issue.pull_request && startsWith(github.event.comment.body, '@metamaskbot update-changelogs'))
20+
if: github.event_name == 'pull_request_target' || (github.event.issue.pull_request && startsWith(github.event.comment.body, '@metamaskbot update-changelogs'))
2121
runs-on: ubuntu-latest
2222
outputs:
2323
is-fork: ${{ steps.is-fork.outputs.is-fork }}
@@ -112,53 +112,89 @@ jobs:
112112
if: ${{ needs.is-release.outputs.is-release == 'true' }}
113113
runs-on: ubuntu-latest
114114
environment: default-branch
115-
env:
116-
PR_NUMBER: ${{ github.event.issue.number || github.event.pull_request.number }}
117115
steps:
118-
- name: Checkout repository
116+
- name: Check out the base branch
119117
uses: actions/checkout@v6
120118
with:
119+
ref: ${{ needs.is-release.outputs.merge-base }}
121120
token: ${{ secrets.UPDATE_CHANGELOG_TOKEN }}
122121

123-
- name: Checkout pull request
122+
- name: Detach HEAD (to prevent accidental pushes)
123+
run: git checkout --detach HEAD
124+
125+
- name: Set up environment
126+
uses: MetaMask/action-checkout-and-setup@v3
127+
with:
128+
is-high-risk-environment: false
129+
130+
- name: Overlay changelogs from current pull request
124131
env:
125132
PR_HEAD_SHA: ${{ needs.is-release.outputs.head-sha }}
126-
PR_BASE_REF: ${{ needs.is-release.outputs.base-ref }}
133+
PR_HEAD_REF: ${{ needs.is-release.outputs.head-ref }}
127134
run: |
135+
# These next two commands are also useful later when pushing
128136
git fetch --no-tags origin "$PR_HEAD_SHA"
129-
git fetch --no-tags origin "$PR_BASE_REF"
130-
git checkout --detach "$PR_HEAD_SHA"
137+
git fetch --no-tags origin "$PR_HEAD_REF"
138+
git checkout "$PR_HEAD_SHA" -- '**/CHANGELOG.md'
139+
shell: bash
131140

132-
- name: Setup environment
133-
uses: MetaMask/action-checkout-and-setup@v3
134-
with:
135-
is-high-risk-environment: false
141+
- name: Configure Git with name and email
142+
run: |
143+
# This is necessary to make a commit
144+
# Passing `token` to the `checkout` action does not do this for us
145+
git config user.name "github-actions[bot]"
146+
git config user.email "41898282+github-actions[bot]@users.noreply.github.com"
147+
shell: bash
148+
149+
- name: Commit changelogs from current pull request
150+
run: |
151+
git add -- '**/CHANGELOG.md'
152+
git commit -m "[Temporary] Add changelogs from current pull request"
153+
shell: bash
136154

137155
- name: Ensure required dependency bump entries exist across all changelogs
138156
id: update-changelogs
139157
env:
158+
PR_NUMBER: ${{ github.event.issue.number || github.event.pull_request.number }}
140159
MERGE_BASE: ${{ needs.is-release.outputs.merge-base }}
141-
run: yarn changelog:validate --checkDeps --fix --currentPr "$PR_NUMBER" --fromRef "$MERGE_BASE"
160+
run: |
161+
yarn changelog:validate --checkDeps --fix --currentPr "$PR_NUMBER" --fromRef "$MERGE_BASE"
162+
shell: bash
163+
# If changelogs were updated but there were other validation errors
164+
# found, we need to still create a commit below
142165
continue-on-error: true
143166

144-
- name: Commit and push updated changelogs
145-
id: push-changes
146-
env:
147-
PR_HEAD_REF: ${{ needs.is-release.outputs.head-ref }}
167+
- name: Commit updated changelogs
168+
id: commit-updated-changelogs
148169
run: |
149170
if git diff --quiet; then
150-
echo "changes-pushed=false" >> "$GITHUB_OUTPUT"
171+
# Nothing to commit; no changelogs updated
151172
exit 0
152173
fi
153174
154-
git diff --stat
155-
git config user.name "github-actions[bot]"
156-
git config user.email "41898282+github-actions[bot]@users.noreply.github.com"
157175
git add -- '**/CHANGELOG.md'
158176
git commit -m "chore: Update dependency bump changelog entries"
159-
git push origin "HEAD:$PR_HEAD_REF"
160177
161-
echo "changes-pushed=true" >> "$GITHUB_OUTPUT"
178+
new_commit_id="$(git log -1 --pretty='format:%h')"
179+
echo "new-commit-id=${new_commit_id}" >> "$GITHUB_OUTPUT"
180+
shell: bash
181+
182+
- name: Cherry-pick new commit on top of pull request branch and push it
183+
id: push-changes
184+
env:
185+
NEW_COMMIT_ID: ${{ steps.commit-updated-changelogs.outputs.new-commit-id }}
186+
PR_HEAD_SHA: ${{ needs.is-release.outputs.head-sha }}
187+
PR_HEAD_REF: ${{ needs.is-release.outputs.head-ref }}
188+
run: |
189+
if [[ -n "$NEW_COMMIT_ID" ]]; then
190+
git checkout "$PR_HEAD_SHA"
191+
git cherry-pick "$NEW_COMMIT_ID"
192+
git push origin "HEAD:$PR_HEAD_REF"
193+
echo "changes-pushed=true" >> "$GITHUB_OUTPUT"
194+
else
195+
echo "changes-pushed=false" >> "$GITHUB_OUTPUT"
196+
fi
197+
shell: bash
162198

163199
- name: Comment result
164200
if: always()

0 commit comments

Comments
 (0)