EastCoastGreenwayAlliance / ecg-map

Interactive map and trip planner for the ECGA
https://map.greenway.org
7 stars 0 forks source link

Update webpack server #83

Open danrademacher opened 5 years ago

danrademacher commented 5 years ago

Known low severity security vulnerability detected in webpack-dev-server < 3.1.11 defined in package.json.

tomay commented 5 years ago

webpack-dev-server appears to only be used in client/. However webpack-dev-server@3.1.11 requires Node >= 6.11.5, whereas this application currently uses 6.11.1 per .nvmrc

potential solution

  1. edit .nvmrc and the root-level package.json to show node version 6.11.5
  2. nvm use and install 6.11.5
  3. yarn upgrade webpack-dev-server@3.1.11 in client/

This completes, but gives the following warnings:

warning " > react-moment-proptypes@1.4.0" has unmet peer dependency "moment@>=1.6.0".
warning " > stylelint-config-standard@16.0.0" has unmet peer dependency "stylelint@^7.8.0".
warning " > stylelint-webpack-plugin@0.7.0" has unmet peer dependency "stylelint@^7.7.0".
warning " > uglifyjs-webpack-plugin@0.4.3" has unmet peer dependency "uglify-js@^2.8.0".
warning " > webpack-dev-server@3.1.11" has incorrect peer dependency "webpack@^4.0.0".
warning "webpack-dev-server > webpack-dev-middleware@3.4.0" has incorrect peer dependency "webpack@^4.0.0".
tomay commented 5 years ago

strange, even when installing webpack-dev-server on a new project in an empty folder with yarn init then yarn add -D webpack-dev-server, I get the following warnings:

warning " > url-loader@0.6.2" has unmet peer dependency "file-loader@*".
warning "webpack-dev-server > webpack-dev-middleware@3.4.0" has incorrect peer dependency "webpack@^4.0.0".
warning " > webpack-dev-server@3.1.14" has incorrect peer dependency "webpack@^4.0.0".

So it may be the order of operations that matters here.

This works without warnings yarn init yarn add -D webpack yarn add -D webpack-dev-server

result:

  "devDependencies": {
    "webpack": "^4.28.4",
    "webpack-dev-server": "^3.1.14"
  }

Starting to think we may need to init the package.json from scratch, and reinstall everything (after first updating the Node version with nvm)

I'll want to know more about client/ vs server.js here, and how to test that this doesn't break anything, before committing this and pushing to heroku

Worth considering that this is all to fix a vulnerability in a package that is not used in production

tomay commented 5 years ago

a better solution would be a security patch retroactively applied to webpack-dev-server 2.x (the version in /client/ is 2.4.2)

clhenrick commented 5 years ago

@tomay the Node.js version increment from 6.11.1 to 6.11.5 is a minor one, so I would doubt that would break any of the build processes.

I don't think the react-moment-proptypes dependency is being used, you could try removing it and reinstalling the node_modules in client to see if anything breaks the client build, but I doubt it will.

If the app still builds locally I wouldn't worry about the other dependency warnings.

I believe that webpack-dev-server@3.x might require webpack@4.29.x based on this line. However if you upgrade Webpack to v4 you will need to update the config based on the 4.x docs which might not be trivial.

Also when I do the following in an empty directory:

yarn init
yarn add -D webpack webpack-dev-server@3.1.11

The package.json shows webpack: ^4.29.5.

Overall I don't think this security issue is worth stressing over as the webpack-dev-server dependency is only ever used to serve the site locally in a development environment, it's not used by Heroku in the production environment. It doesn't seem worthy to stress over if it's too difficult to upgrade all the webpack stuff, IMO.