Skip to content

Fix CodeQL issues for checked-in yarn binaries#1017

Merged
jonthysell merged 10 commits intomicrosoft:mainfrom
jonthysell:removedotyarn
Mar 21, 2025
Merged

Fix CodeQL issues for checked-in yarn binaries#1017
jonthysell merged 10 commits intomicrosoft:mainfrom
jonthysell:removedotyarn

Conversation

@jonthysell
Copy link
Copy Markdown
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
Copy Markdown
Contributor

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

@jonthysell
Copy link
Copy Markdown
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
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

4 participants