devour-js/devour-client

Update axios package to > 0.21.1 to solve security vulnerability

kannapples opened this issue · 14 comments

One of my repos uses devour-client as a dependency, and we received a dependabot warning about a security vulnerability for axios package versions <= 0.21.1.

image

Please upgrade the axios version to at least 0.21.2 (current is 0.21.4).

tijn commented

Hi @kajaaldias, can you make a pull request for this?

tijn commented

#232 solved this

Hi @tijn did you noticed that with the upgrade of axios you raised node.js requirement from 6.x to 13.x ? Why such a breaking change in a patch?

tijn commented

Hi @tijn did you noticed that with the upgrade of axios you raised node.js requirement from 6.x to 13.x ? Why such a breaking change in a patch?

Oh. I did not notice that! I actually had a conversation with a colleague about the impact of the next release but neither of us realised that this requirement had changed too. (But it explains our own troubles with upgrading devour in our own app).

So, basically, I messed it up. I will try to release a fix later this evening when I get home.

tijn commented

Hi @TheElter ,

This happened a month ago. Is this really the thing you're talking about, or was it #243 ?

Hi @tijn maybe I'm commenting the wrong issue, but the jump of axios is the cause of node version issue, if you want I can test the patch when you are ready (actually I'm fixing the dependency to old version on our code that runs on node 6)

tijn commented

@TheElter

  1. okay, so the updated Axios needs a newer node version. This looks totally reasonable to me; you wouldn't expect them to keep supporting an old node-js forever. Anyhow, can you point out the commit in Axios where this happened?

  2. What patch exactly do you want to test?

@tijn for point 2 depends on the goal: you want to keep node 6 compatibility or not for devour client ? :)
If YES I'm available to help you testing an eventual patch
if NO I presume that it has to be wrote somewhere here on github and/or a different version has to done for devour (to avoid getting the incompatible version with a caret dependency in package Json)

For point 1 I can look in Axios history BUT I presume that first the goal on point 2 must be clear.

tijn commented

@TheElter I don't want to keep compatibility with version 6 forever. The current LTS version is 18.12.0. 6 seems unreasonably old.

@tjin I agree with your evaluation, so if the compatibility with version 6 is forgettable looking where axios is no more compatible is unuseful. The error I got from my test was the use of import keyword that if I looked properly is supported officially from node 13 (is experimental in some previous ones).

If it was my public project a breaking change (for any reason) will have caused a MAJOR version (semver state that), I presume is something unclear due to the fact I saw other project that for the node version doesn't do that and do a MINOR version (not a patch for sure).

These are my two cents as user uf the devour-client. For my personal case means lock the version to the last one compatible and add more priority to the migration of old code that is still running on node 6 :)

tijn commented

@TheElter yeah, it's not really a breaking change because our API didn't change. We don't have any new functions to call, no different parameters, nothing deleted. But there is a big difference for users: those using node 18 will probably have a smooth upgrade while node 6-users will be stuck. And yeah, that sucks, but nevertheless, a change in dependencies is not a major version change in semver as far as I know.

Note that even if we marked this as a major version bump you'd still be doing exactly what you're doing now. You'd lock to the current version and add more priority to migrating old code on node 6, which is obviously the correct thing to do. 🤷

I see that the point on semver is effectively discussed around (this is on thread as sample semver/semver#526 ). I suggest you wrote at least a note that you bumped the minimum node version.

tijn commented

I see that the point on semver is effectively discussed around (this is on thread as sample semver/semver#526 ). I suggest you wrote at least a note that you bumped the minimum node version.

@TheElter I would've, if only I had a way to know about it before we released this version. Anyhow, please provide a PR with the note that you wished you had had. It's open source after all.

in any case, we should support only supported LTS and other versions which are supported. current minimum LTS ia 16.x right?