-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
misc(snyk): only keep vuln data for detectable libs #6919
Conversation
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.
LGTM
@@ -28,6 +33,21 @@ writeFileSync(filename, output, 'utf8'); | |||
*/ | |||
function cleanAndFormat(vulnString) { | |||
const snapshot = /** @type {!SnykDB} */ (JSON.parse(vulnString)); | |||
// Hack to deal with non-node-friendly code. | |||
eval(libDetectorSource.replace(/var /g, 'global.')); |
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.
I'm so curious what they were doing and how this works... :)
https://github.com/johnmichel/Library-Detector-for-Chrome/blob/e1fb487c5599bf3b6bbea98a25c60035b5e02938/library/libraries.js#L1-L2
how's about somethin' like?
const librariesDefinition = eval(`
(() => {
${libDetectorSource}
return d41d8cd98f00b204e9800998ecf8427e_LibraryDetectorTests;
})()
`)
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.
love it.
@paulirish we just reopened a pr with pretty much all the filtered vulns (#6961). Looks like this won't be an issue this time, due to filtering. Are you interested in the full list at all? is |
@aviadatsnyk we're not really interested in keeping the full list in our source at this time since we have no way of surfacing most of it. We are working on some other efforts around bundle analysis that should enable us to use the complete list, so maybe once that's completed we can revisit? |
discussing this here: #6961 |
Previous to #6888 there were only THREE libs in the vuln snapshot that we couldn't detect with js-lib-detector. But now there's lots.
thx @patrickhulce for calling this out: #6888 (review)
this updates the cleanup script to exclude vuln data for libs we can't detect.