update image-size to 2.0.2 to prevent CWE-835 #11061
Conversation
|
Hi @renebaudisch! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
✅ [V2]Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
⚡️ Lighthouse report for the deploy preview of this PR
|
slorber
added
Argos
There was a problem hiding this comment.
This doesn't upgrade image-size consistently across the monorepo.
Also:
- This Dos vuln is harmless in our case because you control inputs
- We use
^1range and v1 is also fixed: https://github.com/image-size/image-size/releases/tag/v1.2.1
However, the v2 upgrade seems to have no major breaking change affecting us (I guess? CI will tell) so I'm ok to do this upgrade if it works.
| @@ -104,7 +104,7 @@ | |||
| "eslint-plugin-react-hooks": "^4.6.0", | |||
| "eslint-plugin-regexp": "^1.15.0", | |||
| "husky": "^8.0.3", | |||
| "image-size": "^1.0.2", | |||
There was a problem hiding this comment.
This will only upgrade our monorepo, not third-party Docusaurus sites
Our mdx-loader package should also upgrade to v2 otherwise this creates a dependency duplicate that we won't even use
There was a problem hiding this comment.
I've seen the fix on 1.2.1 but when I check out image-size node_modules folder there is 1.2.0 used:
I'm pretty sure you are totally right by what you are telling me, it's just tools that seek for vulnerabilities like snyk will list you because of this exploit and this give people a bad meaning if they do not understand fully what happens.
BTW, I do not have a CLA and I don't know if my organization wants to do this again, this has been a pain doing it for AMP decades ago, but I still updated mdx-loader and pushed it as you can see.
About third-party websites, I dunno understand I guess. Wouldn't this just lead to merge this PR to the next release so it would be fixed in 3.7.1 or 3.7.2 at least, so each consumer would just need to fetch and update?
There was a problem hiding this comment.
If your site gets 1.2.0, then you can use your package manager features to upgrade the deps to latest compatible version, or fully re-generate the lockfile.
I technically can't merge a PR if the CLA is not signed
Wouldn't this just lead to merge this PR to the next release so it would be fixed in 3.7.1 or 3.7.2 at least, so each consumer would just need to fetch and update?
Users of v2.0 and v3.0 can already upgrade to image-size 1.2.1. If we merge this PR, we only prevent them from using an older version, but they can already upgrade the lib on their own.
There was a problem hiding this comment.
As you can see after upgrading the mdx-loader, our CI fails
There are probably breaking changes to handle in the code to ensure our code works with this new version.
Let me know if you need help. I can close this PR and open a new one doing the correct change, this way we can move on and you don't need to sign the CLA.
|
ok, there are also more than just "image-size", I'll close and use "overrides" |
|
Will do the upgrade in #11065 |
Image-size 1.02. is vulnerable to CWE-835.
This updates to 2.0.2.