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

fix: eliminate the serialize-javascript vulnerability warning #5829

Closed
wants to merge 1 commit into from

Conversation

sodatea
Copy link
Member

@sodatea sodatea commented Aug 27, 2020

By switching to a fork of copy-webpack-plugin v5, so that we can
circumvent the issue in a non-semver-breaking way.

Fixes #5782

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Underlying tools
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

Other information:

By switching to a fork of copy-webpack-plugin v5, so that we can
circumvent the issue in a non-semver-breaking way.

Fixes vuejs#5782
serialize-javascript "^4.0.0"
webpack-log "^2.0.0"

copy-webpack-plugin@^5.0.2:
Copy link

Choose a reason for hiding this comment

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

Thanks for the pr.
Do you know why there is a second copy-webpack-plugin here?

Copy link
Member Author

Choose a reason for hiding this comment

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

It comes from vuepress, which is used to deploy the documentation.

@@ -46,7 +46,7 @@
"cli-highlight": "^2.1.4",
"clipboardy": "^2.3.0",
"cliui": "^6.0.0",
"copy-webpack-plugin": "^5.1.1",
"copy-webpack-plugin-v5": "^5.1.2",
Copy link

Choose a reason for hiding this comment

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

Maybe an idea to make this a fixed version, because of security.
But I don't know.

Copy link

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

I strongly do not recommend do it

@yashha
Copy link

yashha commented Aug 27, 2020

So maybe it would be better to do the Update and Migration?
https://github.com/webpack-contrib/copy-webpack-plugin/blob/master/CHANGELOG.md#-breaking-changes

@peteruithoven
Copy link

I strongly do not recommend do it

Why?
You're afraid that doing this quick fix will mean people won't work on properly updating / migrating?

@alexander-akait
Copy link

@peteruithoven We can do it in copy-webpack-plugin, it is not hard, I just ask why it is hard to update, but if it is require more time I can do patch release right now

@sodatea
Copy link
Member Author

sodatea commented Aug 27, 2020

@evilebottnawi it's hard to update because Vue CLI allows users to tap into the copy-webpack-plugin configuration (I don't know how many would do this but there will surely be some). So updating it would break their apps.

I do plan to update it in the next major, but that would take another 2 to 3 months before being stable.
So the fork is used as a temporary fix to prevent the audit warning from scaring newcomers away.

It would certainly be better if a release can be made in the upstream package to address this issue.
I'll close this PR then.

@alexander-akait
Copy link

@sodatea let's do security release

@alexander-akait
Copy link

Done https://github.com/webpack-contrib/copy-webpack-plugin/releases/tag/v5.1.2

@sodatea
Copy link
Member Author

sodatea commented Aug 27, 2020

@evilebottnawi Thanks!

@sodatea sodatea closed this Aug 27, 2020
@yashha
Copy link

yashha commented Aug 27, 2020

Thanks! :)

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

Successfully merging this pull request may close these issues.

High vulnerability in dependencies -> copy webpack plugin -> serialize js
4 participants