WordPress.org

Make WordPress Core

Opened 2 months ago

Closed 2 months ago

Last modified 2 months ago

#48203 closed defect (bug) (fixed)

Update/Audit npm Dependencies

Reported by: whyisjake Owned by: jorbin
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch needs-refresh
Focuses: Cc:
PR Number:

Description

When running npm audit there are several packages that have issues that can be resolved. This is a first pass at solving those issues.

Attachments (3)

48203.diff (128.9 KB) - added by whyisjake 2 months ago.
48203.2.diff (131.2 KB) - added by whyisjake 2 months ago.
48203.3.diff (130.8 KB) - added by whyisjake 2 months ago.

Download all attachments as: .zip

Change History (13)

@whyisjake
2 months ago

#1 @whyisjake
2 months ago

  • Component changed from General to Build/Test Tools

#2 @desrosj
2 months ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 5.4

Was there a reason for going with an exact version number instead of a range like ~3.1.0? Other than that, this looks good. I'm putting this in 5.4, but I don't feel strongly against committing this before 5.3 beta 3 if you have the time, @whyisjake.

For reference, before this change the issues were listed as: 26 vulnerabilities (16 low, 7 moderate, 3 high) and after the patch, the issues are listed as 3 vulnerabilities (1 low, 2 high).

Also, the results of grunt imagemin should also be committed after upgrading grunt-contrib-imagemin for consistency.

#3 @whyisjake
2 months ago

@desrosj, for the final issues, I think we need to fork the grunt-replace module. There doesn't seem to be any way to bring upstream changes as the security fixes have been open for over a year without being touched.

I could fork/publish to my person lib, but would it make sense to publish under the WordPress npm account?

@whyisjake
2 months ago

#5 @whyisjake
2 months ago

In this second patch, I have forked/updated the grunt-replace library.

=== npm audit security report ===                        
                                                                                
found 0 vulnerabilities in 979296 scanned packages

https://www.npmjs.com/package/grunt-replace-lts

#6 @netweb
2 months ago

  • Keywords needs-refresh added
  • Milestone changed from 5.4 to 5.3

@whyisjake Can you update the patch here so that it only updates grunt-contrib-imagemin, #48217 can update grunt-replace please, just so that the fork docs associated with that commit are kept separate thanks.

Aslo don't remove the ~ tilde character from the SemVer range in package.json

Use "grunt-contrib-imagemin": "~3.1.0", and not "grunt-contrib-imagemin": "3.1.0", please

Happy for this group of tickets to land in 5.3 if you've the time to commit them soonish'

p.s Don't forget to regenerate the lock between these two tickets ;)

Last edited 2 months ago by netweb (previous) (diff)

#7 @jorbin
2 months ago

In 46403:

Build/Test Tools: Fork and Update grunt-replace

The version of grunt replace that is bundled in core is using an outdated version of lodash that is bringing 2 low, 3 high, and 1 critical issue. This package is currently abandoned. There is a community forked version, but that is also harboring some similar security issues.

This switches to a fork by @whyisjake and causes no change to the build.

See #48203.
Fixes #48217.
Props whyisjake, netweb for testing.

@whyisjake
2 months ago

#8 @jorbin
2 months ago

In 46404:

Build/Test tools: Update grunt-contrib-imagemin and run it

Smaller images are good, so is not having npm vulnerabilities.

See #48203.
Fixes #48206.
Props whyisjake, netweb.

This ticket was mentioned in Slack in #core by whyisjake. View the logs.


2 months ago

#10 @jorbin
2 months ago

  • Owner set to jorbin
  • Resolution set to fixed
  • Status changed from new to closed

In 46408:

Build/Test: Bump devDependencies for WordPress 5.3

This upgrades a number of devDependencies. Some of these include changes to how the tasks are configured.

Uglify: There are deprecated options from 2.x to 4.x, see: https://github.com/gruntjs/grunt-contrib-uglify#options

Autoprefixer: Browserslist now warns when passing in the browser list, so that is put into package.json

As with most changes to uglify, this changes every minified JS file.

Fixes #48203.

#11 @jorbin
2 months ago

In 46409:

Build/Test: Update Grunt Uglify options

Follow up to [46408]

Sometimes you need to take documentation literally. And sometimes you need to take it seriously. Deprecations and replacements in uglify are a case where I took them literally and should have taken them seriously.

See #48203.

Note: See TracTickets for help on using tickets.