Skip to content

fix: harden generate-changeset workflow#1069

Merged
iJackWilson merged 3 commits into
mainfrom
jw/harden-workflow
May 12, 2026
Merged

fix: harden generate-changeset workflow#1069
iJackWilson merged 3 commits into
mainfrom
jw/harden-workflow

Conversation

@iJackWilson
Copy link
Copy Markdown
Contributor

@iJackWilson iJackWilson commented May 12, 2026

Description

The generate-changeset.yml workflow uses pull_request_target, which runs with access to repo secrets and write permissions. Previously, it checked out the PR head ref before running the analysis script, meaning untrusted fork code was on disk (and in the working directory) when Node.js executed. This is the classic "PwnRequest" pattern - an attacker could fork the repo, open a PR, and have their code execute with the Ecospark GitHub App credentials.

This PR restructures the workflow so that no PR code is on disk when any code executes.

  • The analysis script now uses the GitHub API exclusively to read PR branch contents (git tree, package.json blobs,
    changeset file existence) instead of filesystem reads from a checkout
  • The PR head checkout is deferred to after the script completes, and only occurs when a commit is needed
  • No code (Node, npm, build scripts) runs after the PR checkout - only git add/commit/push shell commands
  • Uses head.sha (immutable) instead of head.ref (mutable) for checkout to prevent TOCTOU races

What changed

.github/workflows/generate-changeset.yml

  • Removed PR head checkout before script execution
  • Script runs with only base branch code on disk (sparse-checkout: .github/scripts)
  • PR checkout moved to a conditional step after analysis, gated on script output
  • Git commit/push logic moved from the Node script to dedicated workflow steps

.github/scripts/generate-changeset.mjs

  • getWorkspacePackages() - reads the git tree and package.json blobs via GitHub API instead of
    readdirSync/readFileSync
  • getExistingChangeset() - checks changeset file existence via contents API instead of existsSync
  • Removed all git operations (git config, git commit, git push, removeChangeset())
  • Outputs action (skip/remove/write) and changeset_content via $GITHUB_OUTPUT

What to review

Testing

Performed a dry run against three open PRs (#1063, #1065, #1068) which produced identical results.

Further testing would require a merge to main because pull_request_target runs the base branch's workflow.


Note

Medium Risk
Changes touch a pull_request_target workflow that runs with write permissions and secrets; while the intent is to reduce attack surface, mistakes could still enable unintended writes or secret exposure. Behavior changes around when/how changesets are created/removed could also impact release automation if outputs or API queries are wrong.

Overview
Hardens the generate-changeset pull_request_target workflow by running the Node analysis only from base-branch scripts and deferring any PR-head checkout until after the analysis decides a changeset should be written/removed.

Refactors .github/scripts/generate-changeset.mjs to avoid local git/filesystem operations: it now reads PR state (changed files, package discovery via git tree/blobs, existing changeset content) via the GitHub API and emits workflow outputs (action, changeset_file, changeset_content) for downstream commit/push steps, using delimiter-based $GITHUB_OUTPUT writing to prevent output injection.

Reviewed by Cursor Bugbot for commit 1ac0ef3. Bugbot is set up for automated code reviews on this repo. Configure here.

@iJackWilson iJackWilson requested a review from a team as a code owner May 12, 2026 08:15
@iJackWilson iJackWilson self-assigned this May 12, 2026
@claude

This comment was marked as outdated.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 12, 2026

📦 Bundle Stats — @sanity/cli

Compared against main (af84de16)

@sanity/cli

Metric Value vs main (af84de1)
Internal (raw) 2.1 KB -
Internal (gzip) 799 B -
Bundled (raw) 10.97 MB -
Bundled (gzip) 2.06 MB -
Import time 831ms -1ms, -0.1%

bin:sanity

Metric Value vs main (af84de1)
Internal (raw) 975 B -
Internal (gzip) 460 B -
Bundled (raw) 9.84 MB -
Bundled (gzip) 1.77 MB -
Import time 1.99s +6ms, +0.3%

🗺️ View treemap · Artifacts

Details
  • Import time regressions over 10% are flagged with ⚠️
  • Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.

📦 Bundle Stats — @sanity/cli-core

Compared against main (af84de16)

Metric Value vs main (af84de1)
Internal (raw) 95.5 KB -
Internal (gzip) 22.5 KB -
Bundled (raw) 21.60 MB -
Bundled (gzip) 3.42 MB -
Import time 788ms -2ms, -0.2%

🗺️ View treemap · Artifacts

Details
  • Import time regressions over 10% are flagged with ⚠️
  • Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.

📦 Bundle Stats — create-sanity

Compared against main (af84de16)

Metric Value vs main (af84de1)
Internal (raw) 976 B -
Internal (gzip) 507 B -
Bundled (raw) 50.7 KB -
Bundled (gzip) 12.6 KB -
Import time ❌ ChildProcess denied: node -
Details
  • Import time regressions over 10% are flagged with ⚠️
  • Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.

cursor[bot]

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

Comment thread .github/workflows/generate-changeset.yml Outdated
gu-stav
gu-stav previously approved these changes May 12, 2026
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 1 potential issue.

Fix All in Cursor

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

Reviewed by Cursor Bugbot for commit 1ac0ef3. Configure here.

git add "$CHANGESET_FILE"
if ! git diff --cached --quiet; then
git commit -m "chore: update auto-generated changeset for PR #${{ github.event.pull_request.number }}"
git push --force-with-lease origin "HEAD:${PR_HEAD_REF}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

--force-with-lease ineffective with SHA-based detached HEAD checkout

Medium Severity

The checkout now uses head.sha (line 55) which creates a detached HEAD with no remote-tracking branches. --force-with-lease relies on comparing the remote ref against a local remote-tracking branch. Without one, --force-with-lease silently degrades — either rejecting all pushes or behaving like --force depending on git version — losing the concurrency safety it's meant to provide. The old workflow checked out by head.ref, which established proper tracking. Consider specifying an explicit lease value like --force-with-lease=${PR_HEAD_REF}:${PR_HEAD_SHA} so the expected remote state is known.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 1ac0ef3. Configure here.

@iJackWilson iJackWilson merged commit c8f9c06 into main May 12, 2026
51 of 52 checks passed
@iJackWilson iJackWilson deleted the jw/harden-workflow branch May 12, 2026 09:10
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.

2 participants