fix(sphere-stock-import): Security vulnerability detected in hoek #77
Conversation
src/coffee/stockimport.coffee
Outdated
| @@ -383,8 +383,7 @@ class StockImport | |||
| debug "Error on handling 409 stock update error. Details: #{JSON.stringify(err)}" | |||
| Promise.reject err | |||
| else | |||
| debug "Failed to retry the task after #{@max409Retries} attempts for stock #{JSON.stringify(entry)}" | |||
| Promise.reject err | |||
| Promise.reject new Error("Failed to retry the task after #{@max409Retries} attempts for stock #{JSON.stringify(entry)}") | |||
There was a problem hiding this comment.
I dont think this should be part of this PR
There was a problem hiding this comment.
Also please do not swallow original error objects as they contain a lot of valuable informations. See my comment here
There was a problem hiding this comment.
We need here both the original error with its stack trace as also its context/custom message regarding amount of retries.
There was a problem hiding this comment.
@butenkor @junajan so apparently library hoek had some issues which some of our dependencies depended on. Im not sure if upgrading and installing the dependencies like this is the way to solve it or if there is a better way. Do you have any ideas and thoughts?
Message below:
Known moderate severity security vulnerability detected in hoek < 4.2.1defined in package-lock.json.
--
package-lock.json update suggested: hoek ~> 4.2.1.
|
Hi @Babanila changing dependencies in package.json won't change nested dependencies. You need to do it like this: https://stackoverflow.com/questions/15806152/how-do-i-override-nested-npm-dependency-versions |
|
When you add |
| @@ -42,31 +42,37 @@ | |||
| }, | |||
There was a problem hiding this comment.
All you need to update in this file in order to get rid of hoek vulnerability is in this patch:
diff --git a/package.json b/package.json
index 83ebba7..baf4ac1 100644
--- a/package.json
+++ b/package.json
@@ -46,7 +46,7 @@
"csv": "^1.1.0",
"debug": "^2.2.0",
"optimist": "^0.6.1",
- "sphere-node-sdk": "^1.22.0",
+ "sphere-node-sdk": "^2.0.0",
"sphere-node-utils": "^0.9.1",
"tmp": "0.0.33",
"underscore": "^1.8.3",
@@ -54,7 +54,7 @@
},
"devDependencies": {
"coffee-script": "^1.12.7",
- "coveralls": "2.11.2",
+ "coveralls": "2.13.2",
"grunt": "0.4.5",
"grunt-bump": "0.0.13",
"grunt-cli": "0.1.13",
There was a problem hiding this comment.
everything else should be reverted since they are not related to the issue context
|
But IMO, we should close this PR and start a new one which fixes all 48 vulnerabilities in this library |
| @@ -45,16 +45,18 @@ | |||
| "bunyan-logentries": "^0.1.0", | |||
| "csv": "^1.1.0", | |||
| "debug": "^2.2.0", | |||
| "hoek": "^5.0.3", | |||
There was a problem hiding this comment.
No, it's not needed. As well as adding request package as a dependency.
And as I previously said it looks redundant to me to create a PR for each 48 vulnerabilities.
Summary
Security vulnerability detected in hoek
Description
Known moderate severity security vulnerability detected in hoek < 4.2.1 defined in package-lock.json.
Closes #76