mattermost / mattermost-redux

Redux for Mattermost
Apache License 2.0
200 stars 386 forks source link

Update to isomorphic-fetch 3.x to resolve vulnerability in transitive dependency node-fetch #1290

Closed sbley closed 3 years ago

sbley commented 3 years ago

Summary

Update to isomorphic-fetch 3.x to resolve vulnerability in transitive dependency node-fetch 1.7.3

Steps to reproduce

Run npm audit in a project that has "mattermost-redux": "5.28.1" as dependency.

Expected behavior

There are no security vulnerabilities related to mattermost-redux.

Observed behavior

A vulnerability is indicated that cannot be resolved automatically:

  Low             Denial of Service                                             

  Package         node-fetch                                                    

  Patched in      >=2.6.1 <3.0.0-beta.1|| >= 3.0.0-beta.9                       

  Dependency of   mattermost-redux                                              

  Path            mattermost-redux > isomorphic-fetch > node-fetch              

  More info       https://npmjs.com/advisories/1556                

Possible fixes

Update isomorphic-fetch to latest version.

hmhealey commented 3 years ago

Thanks for the report, @sbley! We'll be doing a round of dependency updates later this week, so we can take a look at it then. By the sound of the advisory, it shouldn't affect us since we're not limiting the size of responses here, but it's still good to know about.

sbley commented 3 years ago

@hmhealey, sounds good. Are you planning to add these updates also to current ESR version 5.25.x?

hmhealey commented 3 years ago

No, we're not planning on it.

Looking into it a bit further, while isomorphic-fetch is listed under dependencies, it's actually only used for the unit tests. The web/desktop apps use the native browser and Electron versions of fetch, so they wouldn't be affected anyway. I'll make a note to fix that when we're updating the dependencies.

sbley commented 3 years ago

Could isomorphic-fetch be a devDependency, then?

In our project we are constrained to use only ESR versions of Mattermost. On the other hand, we have strict policies to not use any vulnerable dependencies in our code.

hmhealey commented 3 years ago

Yeah, it should be under devDependencies. Even though it's under dependencies though, I don't think it's shipped with actual releases since it's only referenced by test code.

sbley commented 3 years ago

Hi @hmhealey. Have you had a chance to update the dependencies? If it helps, I could create a pull request that moves isomorphic-fetch to the devDependencies.

hmhealey commented 3 years ago

No sorry, I haven't had a chance to do that yet. We'd definitely welcome a PR to move that though 🙂

hmhealey commented 3 years ago

@sbley Thanks again for bringing this up and for the PR. At this point, it'll be included in the 5.31 release of mattermost-redux coming out in January. In the mean time, since you said you're only able to use ESR versions, do you have a fork of this that you can use? Even if we were to get this into an update to the ESR, I'm not actually sure if we cut new versions of mattermost-redux for it because it usually doesn't change in them.

sbley commented 3 years ago

Hi @hmhealey. We are planning to release (code freeze) end of January, so updating to 5.31 would be an option. Is your release date "in January" pretty reliable or could it also be delayed until February?

hmhealey commented 3 years ago

Our releases are consistently in the middle of the month around the 16th. It's sometimes moved by a few days due to weekends, but that's only a few days at most.

sbley commented 3 years ago

Hi @hmhealey, can we expect a release by the end of the week (Jan 22)?

hmhealey commented 3 years ago

The release was cut last week, so it should be available now from https://mattermost.com. It's also our next ESR release since I know you mentioned you could only use ESRs.

hmhealey commented 3 years ago

Actually, if you're looking for the mattermost-redux release, I don't know why we didn't have a release cut for it for 5.30 or 5.31. I'll look into why that's not happening. Perhaps there's something being missed in our build process.