node-modbus icon indicating copy to clipboard operation
node-modbus copied to clipboard

Update packages for node 14 and latest security fixes

Open iconnor opened this issue 3 years ago • 15 comments

I tried to run npm install with Node v14.17.3 and ran into this error https://github.com/serialport/node-serialport/pull/1743, so I updated node-serialport the latest, and it resolved that issue.

There were also 70 vulnerabilities (25 low, 4 moderate, 38 high, 3 critical). Mostly from lodash and some others. When I updated nyc and mocha it resolved these alerts.

image

iconnor avatar Jul 26 '21 20:07 iconnor

Can you add node.js versions 12 and 14 to the .travis.yml file? I'll be back on the desk on Thursday and open up a new branch.

stefanpoeter avatar Jul 26 '21 21:07 stefanpoeter

I've put your work into the v4.1-dev branch but currently travis-ci won't build, don't know why it is not working currently.

stefanpoeter avatar Jul 29 '21 13:07 stefanpoeter

It looks like with the new include the older versions of node break: image Node ≥ 10 pass but Node ≤ 8 fail

iconnor avatar Jul 31 '21 21:07 iconnor

Once I limit it to current releases (12 onwards): https://en.wikipedia.org/wiki/Node.js#Releases

image

The build should pass on Travis: https://travis-ci.com/github/iconnor/node-modbus/builds/234171373

image

iconnor avatar Jul 31 '21 22:07 iconnor

@stefanpoeter, what are your thoughts on limiting the tests to supported Node versions only?

iconnor avatar Aug 16 '21 03:08 iconnor

My thoughts: Since we did not define what versions are to be supported we are dependend on the currently used plattforms. Is there any way to determine statistics with what node.js versions this package is being used? Otherwise there is no way to tell if this change will bother current users.

Is there maybe another way to support plattforms older than v12?

stefanpoeter avatar Aug 16 '21 08:08 stefanpoeter

If people are running versions lower than the supported ones, they are open to vulnerabilities. It is safer to mark the 2.0.1 release as the last supported version for those under v12 and require upgrades for newer features. I added a sentence in the readme to call that out.

My concern is that Modbus is common in the power industry so encouraging maintainers of those systems to keep current and limit vulnerabilities is a good goal.

iconnor avatar Sep 13 '21 00:09 iconnor

@stefanpoeter what do you think of limiting to only supported releases and updating the readme?

iconnor avatar Sep 30 '21 23:09 iconnor

Hi @iconnor you are right, if the node.js version are not support then the jsmodbus version depending on those should not be supported either.

stefanpoeter avatar Oct 06 '21 09:10 stefanpoeter

Is this okay to merge?

iconnor avatar Oct 12 '21 19:10 iconnor

It is ok to merge. I am just wondering if it should be version 4.0.7 or 4.1.0 or does it even need to be version 5 since these are kind of breaking changes!?

stefanpoeter avatar Oct 13 '21 07:10 stefanpoeter

I just went for the closest patch just in case the jump to 5 introduced regressions of some sort.

iconnor avatar Oct 14 '21 22:10 iconnor

Maybe @alexbuczynsky has some thoughts on this?

stefanpoeter avatar Oct 15 '21 07:10 stefanpoeter

@stefanpoeter and @alexbuczynsky - I saw dependabot added three PRs and just pushed a new commit that updates npm audit that also would resolve those issues.

iconnor avatar Dec 16 '21 19:12 iconnor