Skip to content
This repository has been archived by the owner on May 30, 2024. It is now read-only.

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

Closed
wants to merge 1 commit into from

Conversation

mmcgahan
Copy link

@mmcgahan mmcgahan commented May 3, 2019

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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.

LaunchDarklyCI pushed a commit that referenced this pull request Oct 23, 2019
@eli-darkly
Copy link
Contributor

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

@eli-darkly eli-darkly closed this Oct 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants