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: remove webpack-inject-plugin dependency #464

Merged

Conversation

ar10642
Copy link

@ar10642 ar10642 commented Aug 11, 2023

An attempt to replace the dependency on webpack-inject-plugin with the Webpack BannerPlugin instead.

All tests pass, it seems to work in my local project that uses it. Apologies if I've missed something here or not done something I was supposed to. I am doing this because my company has a requirement for libraries to not have critical security issues.

As mentioned in #454

@kissifrot
Copy link

Wish it could be merged 🙏

@mdriessen
Copy link

mdriessen commented Nov 7, 2023

@tobias-93 This issue also showed up in our security scans. Can this fix be merged?

critical │ Prototype pollution in webpack loader-utils
Package │ loader-utils
Patched in │ >=1.4.1
Dependency of │ fos-router
Path │ fos-router > webpack-inject-plugin > loader-utils
More info │ https://www.npmjs.com/advisories/1094088

@tobias-93
Copy link
Collaborator

Hi @ar10642, sorry for the delayed response. What did you change in router.js and router.min.js? If it is just line endings then please revert, so the scope of this change is as clean as possible. Then I can merge this, I cannot test since I'm not using this method in my projects but if other developers see this work it's good to me. Thanks!

@tobias-93 tobias-93 mentioned this pull request Nov 30, 2023
@tacman
Copy link
Contributor

tacman commented Nov 30, 2023

On a related note, with AssetMapper I've been using the npm version of fos-router.

bin/console importmap:require fos-routing

@tobias-93 tobias-93 force-pushed the remove-webpack-inject-vulnerability branch from 21de161 to 6d8bbb9 Compare December 12, 2023 13:11
@tobias-93 tobias-93 merged commit 23b8ec9 into FriendsOfSymfony:master Dec 12, 2023
10 checks passed
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.

5 participants