fix(security): updating vulnerable packages #1802
Conversation
|
These build errors with the Selenium driver 😕 |
|
oh my god, this is terrible :( |
|
@lirantal did you go trough helmet's changes ? You probably need to update at least hsts options from milliseconds to seconds. Moment.js duration might be useful: moment.duration(6, 'months').asSeconds(); |
|
you're right, I haven't reviewed it carefully rushed quickly to resolve the vulnerable dependencies first to have a good baseline of packages. I also remembered they also changed the middleware names like contentSecurityPolicy to csp and such. |
|
I updated Helmet stuff for Trustroots, you might be able to just copypaste from there: https://github.com/Trustroots/trustroots/blob/master/config/lib/express.js#L197-L449 |
|
@simison you guys also use milliseconds representation for the SIX_MONTHS period so you need to adjust it, or did I miss something? |
|
@lirantal correct, I noticed that too and that's how I looked up that Thanks! |
…o seconds represntation
|
np, great comment, thanks for checking in on us! |
config/lib/express.js
Outdated
| @@ -141,7 +141,7 @@ module.exports.initModulesConfiguration = function (app, db) { | |||
| */ | |||
| module.exports.initHelmetHeaders = function (app) { | |||
| // Use helmet to secure Express headers | |||
| var SIX_MONTHS = 15778476000; | |||
| var SIX_MONTHS = 15778476; | |||
There was a problem hiding this comment.
Would it make sense to just use 15778476 directly in hsts settings object since this variable isn't used anywhere else?
There was a problem hiding this comment.
Also it'll be useful for anyone modifying this to have a short comment mentioning it's seconds.
There was a problem hiding this comment.
nah because then it's just magic number, very unreadable.
There was a problem hiding this comment.
yeah I wasn't sure if to add that comment or not because someone who would change it will probably be aware of helmet
There was a problem hiding this comment.
FWIW, I prefer having a note that it's in seconds here. Even those that understand that Helmet expects seconds, it may be easy to forget or be overlooked by others coming in.
To me it looks less like a "magic" number when I see a comment about seconds.
|
LGTM. Love seeing those green check marks! |
|
LGTM |
fix(security): updating vulnerable packages (meanjs#1802)
* fix(security): updating vulnerable packages * fix(helmet): updating to support latest changes, transition from ms to seconds represntation * comment on SIX_MONTHS variable
No description provided.