Skip to content

Fix CodeQL issues for checked-in yarn binaries #1017

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Mar 21, 2025

Conversation

jonthysell
Copy link
Contributor

@jonthysell jonthysell commented Mar 19, 2025

Description

This PR removes unnecessary .yarn folders from the repo and prevents them from being checked-in / scanned by CodeQL in the future.

Why

Having yarn binaries checked-in is a security risk and CodeQL complains about issues in the scripts (which we can't fix, or even see, as they're obfuscated as binary files).

Removing the .yarn folder for older samples is not a problem, but newer samples which use Yarn 3, such as NativeModuleSample/cpp-lib, need the Yarn 3 binaries checked-in.

Unfortunately there's no way to get around this. Yarn refuses to acknowledge this anti-pattern and security issue (see yarnpkg/yarn#7741) and the "solution" to require users to install Yarn 3 via corepack does not work in GitHub CI (see actions/setup-node#480). I've tried every workaround combination in these two issues and none work.

Closes #1016

Screenshots

N/A

Microsoft Reviewers: Open in CodeFlow

@jonthysell jonthysell requested a review from a team as a code owner March 19, 2025 22:24
@jonthysell jonthysell requested a review from a team as a code owner March 19, 2025 22:52
@jonthysell jonthysell changed the title Remove checked-in yarn binaries Fix CodeQL issues for checked-in yarn binaries Mar 20, 2025
@Saadnajmi
Copy link
Contributor

Just an FYI, the Node.JS Foundation voted to remove corral from the executable from Node 24 or 25 onwards

@jonthysell
Copy link
Contributor Author

Just an FYI, the Node.JS Foundation voted to remove corral from the executable from Node 24 or 25 onwards

Do you mean corepack? That's fine, I only tried enabling it in this PR as a workaround to removing the checked-in yarn 3 cjs files. But since I couldn't get it to work in CI (it worked fine locally) the only option left is to just tell CodeQL to ignore it.

@jonthysell jonthysell enabled auto-merge (squash) March 21, 2025 18:45
@jonthysell jonthysell merged commit dcc238a into microsoft:main Mar 21, 2025
29 checks passed
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.

Resolve CodeQL alerts for checked-in .yarn paths in some samples
3 participants