fix(protect): only patch when version on disk satisfies vuln #652
Conversation
src/lib/protect/apply-patch.js
Outdated
| try { | ||
| const packageJson = fs.readFileSync(path.resolve(relative, 'package.json')); | ||
| const pkg = JSON.parse(packageJson); | ||
| pkg = JSON.parse(packageJson); | ||
| debug('package at patch target location: %s@%s', pkg.name, pkg.version); | ||
| } catch (err) { | ||
| debug('Failed loading package.json of package about to be patched', err); |
There was a problem hiding this comment.
Let's return here too, if the package.json is not ok, maybe we cannot trust the dependency data?
There was a problem hiding this comment.
Also, if we don't return, pkg.version on line 32 will 💥 because pkg is undefined.
src/lib/protect/apply-patch.js
Outdated
| debug('package at patch target location: %s@%s', pkg.name, pkg.version); | ||
| } catch (err) { | ||
| debug('Failed loading package.json of package about to be patched', err); | ||
| } | ||
|
|
||
| const versionOfPackageToPatch = pkg.version; | ||
|
|
||
| const vulnerableVersions = vuln.semver.vulnerable; |
There was a problem hiding this comment.
What matters here is not the vuln's affected semver range (vuln.semver.vulnerable), but the dep version on which the vuln was detected (vuln.version) - please change
src/lib/protect/apply-patch.js
Outdated
| } | ||
|
|
||
| const versionOfPackageToPatch = pkg.version; | ||
| const patchableVersionsRange = vuln.patches.version; |
There was a problem hiding this comment.
How does the patchableVersionRange look like? I remember some issues with strings, I had to run something like semver.coalesce or something like that.
There was a problem hiding this comment.
Thanks for pointing this out! Indeed we will not assume the version ranges are pure semver notation, and will use semver.coerce.
|
🎉 This PR is included in version 1.192.6 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What does this PR do?
Updates the logic for checking whether to apply patch A on dep B to fix vuln C.
Prior to this change, our logic would sometime lead to a misdetection of the following kind:
Patch A is applicable to
pkg@1.0.0Dep B is located in node_modules as
pkg@2.0.0Vuln C was detected on
pkg@1.0.0, which was then resolved on the disk as dep B.This would cause a clash and fail when applying the patch due to version incompatibility.
The fix is a stop-gap measure that validates the version of the dep located in node_modules prior to applying a patch.