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

Upgrade yargs to fix vulnerability issue #2855

Closed
wants to merge 2 commits into from
Closed

Upgrade yargs to fix vulnerability issue #2855

wants to merge 2 commits into from

Conversation

jjloneman
Copy link

  • This is a bugfix
  • This is a feature
  • This is a code refactor
  • This is a test update
  • This is a docs update
  • This is a metadata update

For Bugs and Features; did you add new tests?

Yes. Added test in cli.test.js to demonstrate that updated yargs module properly displays usage statement.

Additionally, the existing tests which pass command-line options to the webpack-dev-server cli in cli.test.js pass which prove that the updated yargs module functions as expected.

Motivation / Use-Case

Fix prototype pollution vulnerability issue in yargs.

See #2854.

Breaking Changes

No breaking changes.

Additional Info

N/A

@alexander-akait
Copy link
Member

Breaking change and we can't update it

@codecov
Copy link

codecov bot commented Nov 23, 2020

Codecov Report

Merging #2855 (fb07201) into master (4ab1f21) will decrease coverage by 0.22%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2855      +/-   ##
==========================================
- Coverage   93.77%   93.54%   -0.23%     
==========================================
  Files          34       34              
  Lines        1333     1333              
  Branches      381      381              
==========================================
- Hits         1250     1247       -3     
- Misses         81       84       +3     
  Partials        2        2              
Impacted Files Coverage Δ
lib/utils/updateCompiler.js 97.14% <0.00%> (-2.86%) ⬇️
lib/Server.js 96.36% <0.00%> (-0.43%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4ab1f21...fb07201. Read the comment docs.

@@ -2,7 +2,9 @@
"name": "webpack-dev-server",
"version": "3.11.0",
"description": "Serves a webpack app. Updates the browser on changes.",
"bin": "bin/webpack-dev-server.js",
"bin": {
"webpack-dev-server": "bin/webpack-dev-server.js"
Copy link
Author

Choose a reason for hiding this comment

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

Just realized that npm install automatically changed this line for me. Running webpack-dev-server as a binary still functions the same (no breaking changes - https://docs.npmjs.com/cli/v6/configuring-npm/package-json#bin).

I can revert this line if desired.

@jjloneman
Copy link
Author

jjloneman commented Nov 23, 2020

Breaking change and we can't update it

@evilebottnawi Is there a reason why it can't be updated?

@alexander-akait
Copy link
Member

Because

"node": ">= 6.11.5"

@jjloneman
Copy link
Author

@evilebottnawi Ah, makes sense. I'll open an issue in yargs to see if they can make a patch fix for y18n v4 for node
6 then, then I'll update my PR with the fixed compatible version if they update it.

@alexander-akait
Copy link
Member

Thanks for the PR, fixed in master, today we will release webpack-dev-server@4-beta.0 (I will do stable release on the next week), anyway it is always should be very stable (we just move some options in other places)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants