Deal with security vulnerabilities reported by yarn audit

[wincent]

We have a few open tickets for security problems reported by yarn audit (or npm audit). I am going to consolidate all the information in this issue and close these others:

• Cannot update packages caused by vulnerabilities for newly created theme #70 ("Cannot update packages caused by vulnerabilities for newly created theme")
• Upgrade default dependencies #97 ("Upgrade default dependencies")
• Liferay Theme Dependencies uses a version of Lodash that is vulnerable to CVE-2018-3721 #141 ("Liferay Theme Dependencies uses a version of Lodash that is vulnerable to CVE-2018-3721")
• For a 7.2 theme freshly created with liferay-js-themes-toolkit github warns "of potential security vulnerabilities in your dependencies." #436 ("For a 7.2 theme freshly created with liferay-js-themes-toolkit github warns of potential security vulnerabilities in your dependencies.")
• Vulnerable dependencies #492 ("Vulnerable dependencies")

Related but not closing because it is a very large undertaking in itself: (edit: we did it in v10)

• Upgrade to Gulp v4 #148 ("Upgrade to Gulp v4")

Current status (14 March 2019):

• When we started our efforts several weeks ago to reduce the number of vulnerabilities in our dependencies yarn audit was reporting 204 vulnerabilities.
• In our in-progress work preparing the v9 release (ie. the current "master" branch), we have updated many dependencies and are now down to "77 vulnerabilities found - Severity: 26 Low | 19 Moderate | 32 High".
• On the v8 release branch ("v8.x") where we are preparing the next v8 release, we have also updated quite a few dependencies and are now down to "111 vulnerabilities found - Severity: 42 Low | 33 Moderate | 36 High".
• Risk assessment is that the remaining vulnerabilities (and even the original set of vulnerabilities) pose no appreciable run-time risk to Liferay instances running in production, because the dependencies are all dev-time dependencies (ie. used by developers running commands such as gulp watch on their local machines); the generated themes/themelets/layouts — which are what actually get exposed over the network — have tiny dependency footprints and yarn audit --groups dependencies is clean for them.
• The support of old versions in the v8 branch of the toolkit (for example, use of dependencies such as convert-bootstrap-2-to-3, which was last updated 2 years ago) means that, realistically, there are some dependencies that we cannot update. On the v9 branch, however, we have a narrower range of functionality to support, and so we will be able "fix forward" security issues.
• It would be desirable to continue upgrading dependencies in order to avoid the scary warnings, but the cost may be high (especially for the Gulp v4 upgrade because of its extensive use of hooks, as we don't yet know if keeping those working in the upgrade will be difficult — Gulp v3 is responsible for most if not all of the remaining vulnerabilities in v9 of the toolkit).

As such, our position is that addressing the remaining vulnerabilities is important but not urgent. It is unlikely that the reported vulnerability counts will go down from the number I quoted above in the next v8 or v9 releases. (They may, in fact, go up, because new vulnerabilities are routinely reported in old dependencies.) Moving forward, our aim will be to reduce the vulnerability count in subsequent v9 releases as close as practically possible to zero, with a clear prioritization of production/run-time vulnerabilities, with dev-time vulnerabilities being treated with less urgency.

To see current dependencies vulnerabilities, excluding devDependencies, as noted here:

Sadly seems that npm audit --only=prod doesn't ignore devDependencies for the purposes of reporting, only for updating.

The theme has no non-dev dependencies, so I (incorrectly) expected the output to have been clear. Doing the same check with yarn shows this that we are indeed clear in production:

yarn # creates lockfile
yarn audit --groups dependencies

[wincent]

Status update, as summarized here:

And that's all for this branch. Final report is here. Overall, this PR drops us from:

45 vulnerabilities found - Packages audited: 538562
Severity: 18 Low | 11 Moderate | 16 High

to:

30 vulnerabilities found - Packages audited: 529199
Severity: 11 Low | 7 Moderate | 12 High

The only remaining deps with problems are all gulp-related:

• gulp
• gulp-replace-task
• gulp-storage
• gulp-load-plugins

Sadly, gulp-replace-task and gulp-storage were last updated 4 years ago, gulp-load-plugins was last updated 2 years ago. We're already using the latest version of each of those so our only real option there is to find or create replacements for them.

The other big one is gulp itself, and we have the #148 to track upgrading from v3 to v4.

That's on the v9 ("master") branch. On the 8.x branch #216 dropped had the effects summarized here:

Overall this PR takes us from:

111 vulnerabilities found - Packages audited: 144859
Severity: 42 Low | 33 Moderate | 36 High

to:

92 vulnerabilities found - Packages audited: 135399
Severity: 33 Low | 27 Moderate | 32 High

[wincent]

To keep all related discussion to this topic in one place, copying content from @andreldm in #492 here:

Hello guys, in Analytics Cloud we have a 7.1 theme that declares the following dependencies:

"devDependencies": {
	"gulp": "^3.8.10",
	"liferay-theme-deps-7.1": "8.1.2",
	"liferay-theme-tasks": "8.1.2"
}

All works fine, but in a security audit it was brought to our attention that some transitive dependencies are vulnerable. The affected packages are cli, lodash, minimatch, minimist and handlebars, they are dependencies of liferay-theme-tasks and gulp3.

We tried to update to 8.2.0 but the same vulnerabilities are reported by npm audit.

So, what can we do? Wait for a 8.x.x release with updated dependencies? What about gulp3?

And @jbalsas' reply:

Hey @andreldm, we've had quite a bit of a back and forth with those reports.

These vulnerabilities are build-time only. You're not exposing any of those through a webserver and are not exploitable in the traditional sense. There should be no needed action on your behalf.

In general, security vulnerabilities in dependencies are the ones that can be exploited and thus definitely require fixing.

We know credibility is just as important when it comes to security, so it'd be good if these were fixed. However, the amount of work they require of us is just too big to take on lightly. We'll eventually get rid of some of these, but they're not our priority right now.

I'll leave this open for now so other can chime in.

Feel free to add additional information and context here in case you feel there's something we're overlooking or that would up the importance of this issue.

[wincent]

Created #504 to do a periodic audit, but will leave this one open as the "overview issue" to explain our overall stance on audits.

[wincent]

Not going to transfer this issue to the new repo because we're handling dependency updates globally over there.