Skip to content
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

Scan for vulnerable JS Libraries #2372

Merged
merged 62 commits into from
Sep 29, 2017
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
b6551af
Feat: Vulnerable JS libraries audit/gatherer
tkadlec May 26, 2017
59f4ee9
Feat: Adding JS Vulnerability check to default config
tkadlec May 26, 2017
24b8461
Merge branch 'master' into js-vulnerabilities
tkadlec May 26, 2017
86ac495
Add identification to snyk request so Snyk can better monitor if some…
tkadlec May 26, 2017
9ebc72b
Chore: Adjusting description to be more clear
tkadlec May 26, 2017
ac9cc34
Fix: body constructed using object
tkadlec May 26, 2017
f0fd224
Cleaner vuln filtering
tkadlec May 26, 2017
56cad31
clean audit return object
tkadlec May 26, 2017
26af245
Pass full list of libraries detected in extendedInfo
tkadlec May 26, 2017
5d63089
Wording tweak for audit
tkadlec May 31, 2017
0c700b2
Removing npmMapper as npm info is now in library detector
tkadlec May 31, 2017
0a23ee2
Feat: Vulnerable libs now look at a local snapshot of the Snyk DB ins…
tkadlec Jun 15, 2017
c398a92
Adding local snapshot of Snyk JSON for testing
tkadlec Jun 26, 2017
77c34f8
removing unnecessary console.log
tkadlec Jun 26, 2017
118417b
Merge branch 'master' into js-vulnerabilities
tkadlec Jun 26, 2017
d7fdc16
Pull library detector from npm and update audit text
tkadlec Jun 26, 2017
a7038dc
Chore: fixing typo
tkadlec Jun 26, 2017
26b59be
Merge pull request #1 from GoogleChrome/master
tkadlec Jun 26, 2017
f27b75e
Merge branch 'master' into js-vulnerabilities
tkadlec Jun 26, 2017
ce6bd00
Fix: Bringing full Snyk URL back
tkadlec Jun 26, 2017
3bd16ef
Chore: Linting error fixes
tkadlec Jun 26, 2017
c36914f
Merge remote-tracking branch 'upstream/master'
tkadlec Jul 6, 2017
01d30ef
Cleanup based on feedback
tkadlec Jul 6, 2017
b0ac61a
Merge remote-tracking branch 'upstream/master' into js-vulnerabilities
tkadlec Jul 6, 2017
ea8be78
Chore: bump js-library-detector version:
tkadlec Aug 18, 2017
de78540
Merge pull request #2 from GoogleChrome/master
tkadlec Aug 24, 2017
c17d5fc
Merge branch 'master' of https://github.com/tkadlec/lighthouse
tkadlec Aug 24, 2017
f27ce59
Merge branch 'master' into js-vulnerabilities
tkadlec Aug 24, 2017
e55ac62
Adding failureDescription to vulnerable JS audit
tkadlec Aug 24, 2017
168aa50
Prettier formatting of JSVuln audit
tkadlec Aug 24, 2017
24f708d
Fixing exception when no JS libs are found, writing tests
tkadlec Aug 24, 2017
cd17774
Vulns grouped by library, with tests
tkadlec Aug 25, 2017
f24dc9d
Script for updating snyk snapshot
tkadlec Aug 25, 2017
eb53e66
better tests
tkadlec Aug 28, 2017
3fb21f8
Merge remote-tracking branch 'upstream/master' into js-vulnerabilities
tkadlec Aug 30, 2017
678b63b
Adding new fancy links to vuln table results
tkadlec Aug 30, 2017
eb81ef2
Merge branch 'js-vulnerabilities' of https://github.com/tkadlec/light…
tkadlec Aug 30, 2017
ae7947b
One additional test
tkadlec Aug 30, 2017
b44728c
Linting fixes. whoops.
tkadlec Aug 30, 2017
09b61a2
Minor change for string formatting, dict for severity
tkadlec Aug 31, 2017
ee1f494
Change comment on severity map dict
tkadlec Aug 31, 2017
4578cd7
Early exit of audit if no libs found
tkadlec Aug 31, 2017
0adf078
New, shorter licenses
tkadlec Aug 31, 2017
a080f7c
Adding local snapshot of Snyk JSON for testing
tkadlec Jun 26, 2017
2fcff67
removing unnecessary console.log
tkadlec Jun 26, 2017
a85d697
Merge branch 'master' into js-vulnerabilities
tkadlec Sep 19, 2017
1c1db60
Moving Snyk logic to audit, not gatherer
tkadlec Sep 22, 2017
21c0a2c
Merge remote-tracking branch 'upstream/master'
tkadlec Sep 22, 2017
d57b81a
Merge branch 'master' into js-vulnerabilities
tkadlec Sep 22, 2017
a8edaf5
Adding dangling commas
tkadlec Sep 22, 2017
c07833e
yarn.lock for js-lib-detector
paulirish Sep 22, 2017
d1a3756
rename gather
paulirish Sep 22, 2017
cf3d743
rename gather file
paulirish Sep 22, 2017
250dbea
Merge remote-tracking branch 'upstream/master'
tkadlec Sep 29, 2017
90f40f9
Merge branch 'master' into js-vulnerabilities
tkadlec Sep 29, 2017
507588d
Merge branch 'js-vulnerabilities' of https://github.com/tkadlec/light…
tkadlec Sep 29, 2017
abb3246
New line at end of snapshot shell script
tkadlec Sep 29, 2017
b3e182e
rename mostSevere to highestSeverity
tkadlec Sep 29, 2017
5e45f33
More defined param comments, numericSeverity assigned earlier
tkadlec Sep 29, 2017
ab8ff76
renaming and moving snyk snapshot
tkadlec Sep 29, 2017
c0569e5
end to end test
tkadlec Sep 29, 2017
88ff656
add last JSDoc types
brendankenny Sep 29, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 86 additions & 0 deletions lighthouse-core/audits/dobetterweb/no-vulnerable-libraries.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/**
* @license
* Copyright 2017 Google Inc. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have short license headers now... copy one from one of the other files?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All set.

*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* @fileoverview Audits a page to make sure there are no JS libraries with
* known vulnerabilities being used.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mention the db is provided by snyk.io and checked in locally as third-party/snyk-snapshot.json.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GTG

*/

'use strict';

const Audit = require('../audit');
const Formatter = require('../../report/formatter');

class NoVulnerableLibrariesAudit extends Audit {

/**
* @return {!AuditMeta}
*/
static get meta() {
return {
category: 'Security',
name: 'no-vulnerable-libraries',
description: 'Avoids using any front-end JavaScript libraries'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove "using any"

+ ' with known security vulnerabilities',
helpText: 'Sites should take care to ensure that they are not using any' +
' front-end JavaScript libraries that contain known security vulnerabilities.',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This repeats the description. Is there more we can say? Also throw in a [Learn more]() link to somewhere relevant?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the link, it looks like most (all?) other audits point to a page on Google Developers. Does it make sense to put something similar to https://developers.google.com/web/tools/lighthouse/audits/contrast-ratio together for this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. @kaycebasques can work on that once we have the audit :)

Copy link
Collaborator

@wardpeet wardpeet May 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it has to be a developers.google.com blog post. Maybe it could be a snyk page that has some information about this? As long as it's explaining why we do it.

FYI: i'm not against adding it to the audit references :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The typical pipeline is to link to some external doc while we put together an official doc on developers.google.com. The goal is to just provide some sort of further guidance for peeps that fail the test, so they're not like "wtf I'm failing this test and I have no idea what it means or how to fix it"

requiredArtifacts: ['JSVulnerableLibraries']
};
}

/**
* @param {!Artifacts} artifacts
* @return {!AuditResult}
*/
static audit(artifacts) {
const vulns = artifacts.JSVulnerableLibraries.vulnerabilities;
const libraries = artifacts.JSVulnerableLibraries.libraries;

const finalVulns = vulns.map(record => ({
severity: record.severity,
library: record.name + '@' + record.version,
url: 'https://snyk.io/vuln/' + record.id
}));

let displayValue = '';
if (vulns.length > 1) {
displayValue = `${vulns.length} vulnerabilities detected.`;
} else if (vulns.length === 1) {
displayValue = `${vulns.length} vulnerability was detected.`;
}

const headings = [
{key: 'url', itemType: 'url', text: 'Details'},
{key: 'library', itemType: 'text', text: 'Library'},
{key: 'severity', itemType: 'text', text: 'Severity'}
];
const details = Audit.makeV2TableDetails(headings, finalVulns);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We just removed the "V2":

static makeTableDetails(headings, results) {


return {
rawValue: vulns.length === 0,
displayValue,
extendedInfo: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extendedInfo is legacy in LH 2.0. What do you think about just returning the new detail object. Our JSON results are getting out of control :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wouldn't say it's "legacy", but its there only for folks consuming more details in the LHR, usually via the lighthouse module. not there for presentational purposes.

I think this sorta return would make sense:

extendedInfo: {
  value: {
    vulnerabilities: finalVulns
  }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, I think. :)

So the way I was using it, apparently it was just unnecessary as I was also passing details. @rviscomi suggested including a full list of detected libs here as well, so I'm thinking:

extendedInfo: {
  value: {
    js_libs: libraries,
    vulnerabilities: finalVulns
  }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All set. See 26af245

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@paulirish anything that's useful is also being surfaced in details.items. If module consumers are pulling lower level details themselves, they're probably having a heck of time figuring out which of these to use/trust:

screen shot 2017-05-26 at 8 41 23 pm

It's time to nip bloat in the bud. #2276

js_libs: libraries,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jsLibs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GTG

vulnerabilities: finalVulns
},
details,
};
}

}

module.exports = NoVulnerableLibrariesAudit;
7 changes: 7 additions & 0 deletions lighthouse-core/config/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ module.exports = {
"dobetterweb/response-compression",
"dobetterweb/tags-blocking-first-paint",
"dobetterweb/websql",
"dobetterweb/js-vulnerable-libraries",
]
},
{
Expand Down Expand Up @@ -132,6 +133,7 @@ module.exports = {
"dobetterweb/no-websql",
"dobetterweb/notification-on-start",
"dobetterweb/script-blocking-first-paint",
"dobetterweb/no-vulnerable-libraries",
"dobetterweb/uses-http2",
"dobetterweb/uses-passive-event-listeners"
],
Expand Down Expand Up @@ -329,6 +331,10 @@ module.exports = {
"manifest-short-name-length": {
"expectedValue": true,
"weight": 1
},
"no-vulnerable-libraries": {
"expectedValue": true,
"weight": 1
}
}
}]
Expand Down Expand Up @@ -749,6 +755,7 @@ module.exports = {
{"id": "no-document-write", "weight": 1},
{"id": "external-anchors-use-rel-noopener", "weight": 1},
{"id": "geolocation-on-start", "weight": 1},
{"id": "no-vulnerable-libraries", "weight": 1},
{"id": "notification-on-start", "weight": 1},
{"id": "deprecations", "weight": 1},
{"id": "manifest-short-name-length", "weight": 1},
Expand Down
Loading