Fix dependabot alert (Express.js Open Redirect in malformed URLs)

[natelevinson10]

Will look into the cause / finding a solution for this dependabot security flag

[natelevinson10]

It appears that the package causing issues here (express@4.18.2) must be updated to version @4.19.2. However a dependency of the extension, web-ext, relies on a webpack-dev-server that currently does not support this new version of express.

Manually updating web-ext, webpack-dev-server, and other dependent packages didn't work as, and neither did adding the updated versions directly into the package.json file. Npm audit forces web-ext to @7.2.0 from @7.10.0, which is presumably the closest version that supports express@4.19.2, but this breaks the extension.

It seems like the other option is to just wait for an update that supports this new express version or look into the work-around in the security description "The fix for this involves pre-parsing the url string with either require('node:url').parse or new URL. These are steps you can take on your own before passing the user input string to res.location or res.redirect."

[SebastianZimmeck]

Thank you, @natelevinson10!

but this breaks the extension

Do you know what breaks? We may need to change our code. As @danielgoldelman said, we keep getting these issues because we are carrying around a good amount of tech debt.

[SebastianZimmeck]

Also, web-ext, webpack-dev-server, etc., all of that should be related to the build process and not to the extension code itself, right? If so, that seems fairly separable. I remember two years back or so Webpack introduced some changes that were not backwards compatible. The bottom line is that we need to make sure that our build process is current.

[natelevinson10]

Yes web-ext, webpack-dev-server, etc. are all related to the build process. It looks like a case of needing a dependency to allow support for express@4.19.2. And I shouldn't have said it "breaks the extension", it just doesn't resolve the security vulnerability.

[SebastianZimmeck]

Do you know what we need web-ext for? It seems pretty unimportant. Can we remove it with some minor effort? What, if any, is the error when we do so?

[natelevinson10]

Extension compiled fine upon removing web-ext. Will look into how this impacts a possible solution for the vulnerability

[SebastianZimmeck]

Excellent! If after testing you are reasonably sure that nothing breaks, maybe just remove web-ext, push to the main branch, and see if any tests are failing. Maybe, the Dependabot alert goes away if it no longer detects web-ext. Otherwise, we can close the alert manually.

[natelevinson10]

Removing web-ext successfully removed all vulnerabilities - I tested out the dev build and everything seems to be working fine. Going to make a PR and someone else should do a sanity check and make sure everything works properly before merging.

[SebastianZimmeck]

Maybe, @JoeChampeau, if you have time to take a brief look.

[JoeChampeau]

This issue can probably be closed, since we determined the vulnerability wasn't a problem

[SebastianZimmeck]

Sounds good. This issue relates to this alert