search

launchdarkly / node-server-sdk

LaunchDarkly Server-side SDK for Node
Other
79 stars 65 forks source link

update `semver` package and fix security issues in transitive deps #151

Closed mmcgahan closed 4 years ago

mmcgahan commented 5 years ago

I encountered a random yarn check error in a project I'm working on, and drilled it down to the semver dependency in this package that was locked to v5.5.0 (most other transitive semver dependencies were ^5.5.1).

This isn't really a problem with the package itself, but i figured I could help with dependency maintenance and upgrade it to the latest semver v6.0.0, which is officially a breaking change, but only for an API that is not being used here (semver CHANGELOG), so all tests pass.

npm audit also found a few important security vulnerabilities in transitive dependencies, so I ran npm audit fix to update package-lock.json with those updates. Again, all tests pass and this is just a background maintenance update.

eli-darkly commented 5 years ago

Sorry we didn't get to this one earlier. There have been some other package changes since then, so I'll need to resolve conflicts and verify that everything works.

eli-darkly commented 4 years ago

About npm audit, though: unfortunately it does not distinguish between runtime dependencies and devDependencies. The vulnerabilities currently being flagged are as far as I know all from devDependencies, so 1. they are not seen by application code and 2. we can safely fix them with npm audit fix --only=dev (which, since it only modifies package-lock.json, has no effect on transitive dependencies as seen by application code). If npm audit were flagging anything from the runtime dependencies, we would not want to use npm audit fix for those since it would misleadingly cause the scan to pass within our own project but wouldn't actually fix anything for applications using the SDK. So I'm currently trying to make sure that this only involves the dev tools, but the output format of npm audit does not make that easy.

eli-darkly commented 4 years ago

Actually, as far as we can tell, npm audit fix --only=dev does not (despite what the documentation says) limit its fixes to devDependencies. So we had to put together a slightly more complicated process, and found that there were indeed a couple of runtime vulnerabilities that could not be fixed that way. We'll have a patch version out shortly and we'll close this PR at that point.

eli-darkly commented 4 years ago

We made equivalent fixes in the 5.9.2 release, and improved our dependency checking process.