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

Update packages to pass npm audit #6772

Merged
merged 4 commits into from
May 16, 2018
Merged

Update packages to pass npm audit #6772

merged 4 commits into from
May 16, 2018

Conversation

ntwb
Copy link
Member

@ntwb ntwb commented May 16, 2018

Description

npm v6 includes a new audit tool, when running npm install currently the following warning is displayed:

[!] 31 vulnerabilities found [27629 packages audited]
    Severity: 5 Low | 21 Moderate | 5 High
    Run `npm audit` for more detail

After applying the patch here in this PR the warnings are gone:

[+] no known vulnerabilities found [542 packages audited]

Notes:

codecov, webpack, and webpack-cli are version bumps

node-sass changes from v4.7.2 to v4.7.0, this is due to node-sass > v4.7.1 locking down the request library to ~2.79.0 which includes a vulnerable hoek package, see https://nodesecurity.io/advisories/566. It also uses the v4.7.0 release from GitHub rather than npmjs.com because v4.7.0 is not published to npmjs.com. This issue will not be resolved by node-sass until v5.0.0 is released in a few weeks time when as part of that release Node.js v4 support will be dropped.

• The latest release of fsevents is added as in optionalDependencies, this allows the version of hoek shipped in webpack to be overridden, it is also beneficial as Travis CI will no longer try to install fsevents which removes another warning, fsevents is a macOS only package, this change should also be made for core, see https://core.trac.wordpress.org/changeset/39368

How has this been tested?

Running npm i

Screenshots

Types of changes

Build tools.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@ntwb ntwb added the [Type] Build Tooling Issues or PRs related to build tooling label May 16, 2018
@ntwb ntwb added this to the 2.9 milestone May 16, 2018
@ntwb ntwb requested review from pento and gziolo May 16, 2018 08:30
@gziolo
Copy link
Member

gziolo commented May 16, 2018

npm audit

                   === npm audit security report ===                        

[+] no known vulnerabilities found
Packages audited: 43213 (41454 dev, 603 optional)

🎉

@gziolo
Copy link
Member

gziolo commented May 16, 2018

Why −16,565 lines were removed in the lock file? I mean, it's nice but also puzzling.

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Yes, something went wrong with the lock file. When I run npm i locally I see tons of changes.

@gziolo
Copy link
Member

gziolo commented May 16, 2018

Needs rebase after #6758 got merged 😅

@pento
Copy link
Member

pento commented May 16, 2018

It's interesting that npm audit says to upgrade node-sass to 4.9.0 to resolve the hoek issue, when that won't work. 🙃

@ntwb
Copy link
Member Author

ntwb commented May 16, 2018

I've no idea why the lock file was the way it was, b311304 is +/- ~5000 and looks much saner

@gziolo
Copy link
Member

gziolo commented May 16, 2018

Yes, all good now, I don't see any local changes after npm i 👍

@gziolo gziolo merged commit efa30aa into master May 16, 2018
@gziolo gziolo deleted the update/npm-audit branch May 16, 2018 10:20
@ntwb ntwb mentioned this pull request Jun 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants