ci: enforce yarn integrity - #18018
Conversation
Sorry, something went wrong.
|
I will open a few example malicious PRs to test if the updated workflow can detect modified yarn binary. |
Sorry, something went wrong.
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/61773 |
Sorry, something went wrong.
|
Wouldn't it be easier to just use packageManager and skip the checked-in binary? |
Sorry, something went wrong.
|
commit: 834b8e9 |
Sorry, something went wrong.
Good idea. I will explore a bit. Previously I was following the recommendation from https://yarnpkg.com/getting-started/qa#which-files-should-be-gitignored, but https://yarnpkg.com/configuration/yarnrc suggested that corepack should be used in most cases. I guess we can probably remove the yarnPath and the checked-in binary. @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 corepack on some platforms (actions/setup-node#480 (comment)) when we use yarn. And this caveat will unnecessarily complicate many other workflows. 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 corepack becomes generally available in every GH action runners we use. |
Sorry, something went wrong.
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.ts to compute and compare the Yarn binary’s sha512.
- Update package.json packageManager to 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.
Sorry, something went wrong.
| 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())!; |
The checkout is always shallow so it is impossible to compare with base ref.
In this PR we enforce the integrity check for the yarn binary.
Motivation
The yarn binary is a minified .cjs which 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. Because yarn is 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 use command will download yarn binary and add a sha512 signature to the packageManager field. 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, corepack does not provide the integrity assurance of the package manager.
Approach
In this PR, we read the yarn version from the packageManager field, run corepack enable and then manually compare the sha512sum of the local binary to the result set by corepack 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?