Uh oh!
There was an error while loading. Please reload this page.
fix(workflows): close pull_request_target fork-checkout hole; bump actions to latest - #2355
Conversation
…bump actions to latest metadata-governance.yml checked out the PR submitter's fork branch (github.event.pull_request.head.ref) under pull_request_target while holding a token scoped issues:write/pull-requests:write/repository-projects:write, then executed scripts from that checkout. A PR could edit those scripts to run arbitrary code with that token. Per GitHub's securely-using-pull_request_target guidance, pull_request_target must only ever execute trusted code from the base repository's default branch. Also brings all pinned action versions up to their current latest majors (checkout v4->v7, setup-node v3/v4->v7, upload-artifact v3/v4->v7, download-artifact v4->v8, github-script v7->v9, create-github-app-token v2->v3, labeler v5->v7, tj-actions/changed-files v46->v47, add-to-project v1.0.2->v2.0.0), verified against each project's release notes for breaking API changes before bumping.
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (5)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note
|
🚫 This PR description is missing required template content. Missing required section(s): Linked issues, Changelog, Global DoD checklist Please update the PR body using one of the repository PR templates:
Empty placeholders, unchecked checklist boxes, and stub issue references do not count. |
Merge Queue Status
This pull request spent 9 seconds in the queue, including 1 second running CI. Required conditions to merge |
Uh oh!
There was an error while loading. Please reload this page.
Milestone Allocation |
…tch repo-wide action version stragglers (#2356) Adds explicit least-privilege `permissions:` to the 6 workflows that had none at all (workflow- or job-level): changelog.yml, flaky-test-detection.yml, linting.yml, testing.yml (contents: read only), pr-template-validation.yml (permissions: {} — no checkout, no API writes), and meta-agent-validation.yml (contents: read + pull-requests: write, for its PR comment steps). Scoped by reading each job's actual steps rather than assuming. The other 12 workflows flagged earlier already had correctly-scoped job-level permissions blocks (just not at workflow level), so were left untouched. Also fixes the previous action-version-bump pass (#2355), which only covered .github/workflows/ and missed the top-level workflows/, agents/*/skills/, and skills/ directories — 10 more files still pinned to actions/github-script@v7 (the specific staleness the CI annotation on #2351 flagged after Node 20 runner deprecation). Repo-wide grep now confirms every action reference resolves to its actual current release, verified via the GitHub API.
…lop (#2362) * fix(workflows): remove fork-head checkout under pull_request_target in metadata-governance.yml metadata-governance.yml checked out the PR submitter's fork branch (github.event.pull_request.head.ref) under pull_request_target while holding a token scoped issues:write/pull-requests:write/repository- projects:write, then executed scripts from that checkout. A PR could edit those scripts to run arbitrary code with that token. Per GitHub's securely-using-pull_request_target guidance, pull_request_target must only ever execute trusted code from the base repository's default branch. This is the same vulnerability already fixed on release/v1.0.0 (PR #2355), ported here because for pull_request_target-triggered workflows GitHub resolves the workflow *definition* from the repository's actual default branch (develop), not from whatever branch the PR targets — so the release/v1.0.0 fix never applied to real contributor PRs against develop, where this workflow actually runs. * fix(ci): pin Node 24 LTS repo-wide; bump stale actions; fix remaining CommonJS/ESM breakage Ports the same three fixes already applied and verified on release/v1.0.0 (PRs #2358, #2359, #2360) to develop — the repository's actual default branch, where pull_request_target-triggered workflows resolve their *definition* from, regardless of what branch a PR targets. The release/v1.0.0 fixes never reached these code paths. 1. Node version drift: .nvmrc said 22, package.json engines said >=18, workflows hardcoded a mix of "18"/"20"/"22"/"lts/*" for setup-node (including "20", deprecated on GitHub's hosted runners). Standardised on 24 (current Active LTS per nodejs.org) across .nvmrc, package.json engines.node, and every workflow. Verified no node-version matrix strategies exist, so a blanket normalise was safe. 2. Stale action versions: bumped actions/checkout, setup-node, upload-artifact, download-artifact, github-script, create-github-app-token, tj-actions/changed-files, and add-to-project to their current latest majors — verified via the GitHub API against each project's actual releases, and checked release notes for breaking changes before bumping. Includes two vendored skill workflow files under agents/*/skills/ that carried the same stale pins. 3. CommonJS scripts crashing under this repo's "type": "module": found via the same dependency-graph-walk methodology used on release/v1.0.0, but starting fresh from develop's actual entrypoint set — which differs from release/v1.0.0's (some files already converted to real ESM here, e.g. issues.agent.js/planner.agent.js; others broken here that weren't there, e.g. the Meta Agent skills and scripts/agents/includes/label-sync.js). Renamed 32 files to .cjs and fixed every reference — literal .js paths, extensionless require() calls (this repo's .jest.config.cjs excludes 'cjs' from moduleFileExtensions, so Jest doesn't auto-resolve it either), and requires embedded inside actions/github-script blocks in YAML, which a naive `node scripts/....js`-only entrypoint scan misses entirely. Re-ran the full closure and a repo-wide reference scan afterward: zero stale references remain.
Summary
Targets #2351 (release/v1.0.0).
metadata-governance.ymlchecked out the PR submitter's fork branch (github.event.pull_request.head.ref) underpull_request_targetwhile holding a token scopedissues:write/pull-requests:write/repository-projects:write, then executed scripts from that checkout. A PR could edit those scripts to run with that token — the classic "pwn request" pattern. Per GitHub's securely-using-pull_request_target guidance, removed therepository:/ref:override so the checkout always resolves to the trusted base branch, matching the safe pattern already used correctly inopenspec-progress-phase.ymlelsewhere in this same release.checkoutv4→v7,setup-nodev3/v4→v7,upload-artifactv3/v4→v7,download-artifactv4→v8,github-scriptv7→v9,create-github-app-tokenv2→v3,labelerv5→v7,tj-actions/changed-filesv46→v47,add-to-projectv1.0.2→v2.0.0. Checked each project's release notes first — all are Node-runtime/feature bumps, no breaking API changes for how these workflows use them.checkout/create-github-app-tokenpinned by SHA with version comments — already resolve to current),deploy-pages/upload-pages-artifact/codecov-action/slack-github-action/lychee-action/mergify-ci(already latest major),titoportas/update-project-fields@v0.1.0(unmaintained since 2022, nothing newer exists).Test plan
metadata-governance.yml's issue-type-sync path (native type sync viaLS_APP_PRIVATE_KEY) still fires correctly onissuesevents — untouched by this change but worth re-verifying end to end