-
-
Notifications
You must be signed in to change notification settings - Fork 300
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
ci: lint yarn lockfiles to analyze and detect security issues #6424
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## unstable #6424 +/- ##
============================================
+ Coverage 61.72% 61.76% +0.03%
============================================
Files 553 553
Lines 57856 57856
Branches 1829 1829
============================================
+ Hits 35711 35732 +21
+ Misses 22108 22089 -19
+ Partials 37 35 -2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, although it doesn't seem to check a lot of things?
Might make sense to also consider npm audit
.
Unrelated, but I noticed your PR removes some whitespaces. Might make sense to automate this too. (yaml lint
?)
This will detect a malicious packages being injected into your lockfile which is the main concern, it does not verify integrity as you would have to pull the package and cross check the hash but this is already done by yarn itself when installing. There are a few more checks we could enable but those don't seem that useful.
I think there is a yarn equivalent for it too. The problem with audit is that you have to differentiate between dev vs. prod dependencies and you have to run it continuously (cron job or similar) and not on a code change. Definitely something we could improve but a lot might already be covered by Github security alerts and Dependabot PRs bumping vulnerable packages.
If it is cheap to run it is probably good idea to add that to the CI. |
Performance Report✔️ no performance regression detected Full benchmark results
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
🎉 This PR is included in v1.16.0 🎉 |
Motivation
Lockfile changes are hard to review and we should make sure we don't introduce a malicious dependency either accidentally or from an external contribution. Don't trust, verify.
Description
Add CI job to lint yarn lockfiles to analyze and detect security issues
It only takes 4 seconds to execute the lint task
Example of detected issue