ci: enforce yarn integrity#18018
Conversation
|
I will open a few example malicious PRs to test if the updated workflow can detect modified yarn binary. |
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/61773 |
1931e2a to
80f5b68
Compare
|
Wouldn't it be easier to just use |
|
commit: |
Good idea. I will explore a bit.
@arcanis I did some research and before actions/setup-node#531 is resolved, removing the checked-in binary will bring new hassles because we may have to manually install For the time being I think we will settle down on the integrity check, which only runs on ubuntu-latest with node.js latest, before |
There was a problem hiding this comment.
Pull request overview
This PR adds a CI safeguard to ensure the Yarn binary committed in the repo hasn’t been tampered with, by comparing the local .yarn/releases artifact hash against the integrity hash recorded in package.json’s packageManager field.
Changes:
- Add
scripts/ensure-yarn-version-integrity.tsto compute and compare the Yarn binary’s sha512. - Update
package.jsonpackageManagerto include the+sha512.<hash>integrity suffix. - Add a new GitHub Actions workflow that runs the integrity check on relevant PR changes.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
scripts/ensure-yarn-version-integrity.ts |
Implements the sha512 comparison between the committed Yarn binary and the expected integrity hash. |
package.json |
Records Yarn version + integrity hash in the packageManager field. |
.github/workflows/package-manager.yml |
Adds a PR-scoped workflow to execute the integrity verification script. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pull_request: | ||
| paths: | ||
| # Ensure packageManager field contains the correct yarn version and integrity hash. | ||
| - "package.json" | ||
| - ".yarn/releases/*" |
| const yarnPath = `./.yarn/releases/yarn-${yarnVersion}.cjs`; | ||
|
|
||
| const actualHash = createHash("sha512") | ||
| .update(fs.readFileSync(yarnPath)) | ||
| .digest("hex"); |
| const match = packageManagerSpecRegex.exec(readPackageManagerSpec()); | ||
| if (!match) { | ||
| throw new Error( | ||
| "Invalid packageManager format in package.json, expected 'yarn@<version>+sha512.<hash>'. Use 'corepack prepare yarn@<version>' to get the correct hash." |
| cp.execSync(`corepack use yarn@${yarnVersion}`); | ||
|
|
||
| // Read the package manager spec again to get the expected hash, since corepack might have updated it | ||
| const match2 = packageManagerSpecRegex.exec(readPackageManagerSpec())!; |
78521ba to
57aea21
Compare
The checkout is always shallow so it is impossible to compare with base ref.
57aea21 to
834b8e9
Compare
Fixes #1, Fixes #2In this PR we enforce the integrity check for the yarn binary.
Motivation
The yarn binary is a minified
.cjswhich is difficult to review. However, an attacker could modify the yarn binary and easily perform a supply chain attack, e.g. inject payloads to every published package or try to steal the publish credential. Becauseyarnis minified, the attacker could open a seemingly safe PR, e.g. upgrade Yarn to 5.6.7 for CVE-1234-56789 and trick maintainers to merge it. In the past Babel has temporarily modified the yarn binary as well, though it is for a good reason at that time.The
corepack usecommand will download yarn binary and add a sha512 signature to thepackageManagerfield. However, it does not enforce that the local yarn binary matches the hash. Since an integral global yarn binary will still invoke an affected yarn binary in the local repo,corepackdoes not provide the integrity assurance of the package manager.Approach
In this PR, we read the yarn version from the
packageManagerfield, runcorepack enableand then manually compare the sha512sum of the local binary to the result set bycorepack use yarn@<version>, which will download yarn from the official release channel and update the hash.To minimize the CI overhead, the check will only run if it is a PR and there are changes in
.yarn/releases.Context
There is an upstream issue proposing to add hash in
yarn version set: yarnpkg/berry#6657.Demo
Here are three example PRs and the workflow fails the first two and passes the last one as expected:
Modified yarn binary without updating hash: JLHwung#15
Modified yarn binary as well as the hash: JLHwung#16
Normal yarn update: JLHwung#17
/cc @arcanis Is it possible to let a global yarn binary (installed by corepack) to copy itself to the local repo without first invoking the local yarn binary?