Fixed security vulnerabilities indicated by "npm audit" and "snyk test"

[marcelobern]

Fixed security vulnerabilities indicated by "npm audit" and "snyk test".

It might be worth releasing this or a variation of it as a standalone 4.2.3 version as snyk test against 4.2.2 produces the following result:

✗ Medium severity vulnerability found on node-forge@0.7.1
- desc: Regular Expression Denial of Service (ReDoS)
- info: https://snyk.io/vuln/npm:node-forge:20180226
- from: botbits@1.0.0 > alexa-app@4.2.2 > alexa-verifier-middleware@1.0.1 > alexa-verifier@1.0.0 > node-forge@0.7.1
Your dependencies are out of date, otherwise you would be using a newer node-forge than node-forge@0.7.1.
Try deleting node_modules, reinstalling and running `snyk test` again.
If the problem persists, one of your dependencies may be bundling outdated modules.

[lazerwalker]

This looks good to me, other than adding a Changelog entry and figuring out why the build is broken on legacy versions of node. Looks like the dependency resolver is unhappy:

npm ERR! peerinvalid The package chai@3.5.0 does not satisfy its siblings' peerDependencies requirements!
npm ERR! peerinvalid Peer chai-as-promised@5.3.0 wants chai@>= 2.1.2 < 4
npm ERR! peerinvalid Peer chai-string@1.5.0 wants chai@^4.1.2
npm ERR! peerinvalid Peer sinon-chai@2.14.0 wants chai@>=1.9.2 <5

Our version of chai-as-promised looks ancient (the latest release is 7.1.1), I wonder if bumping the version there will clear things up.

[kobim]

@lazerwalker it was fixed in #376, this PR just needs to be synced with the master.

[marcelobern]

@kobim done. Please let me know if you need anything else from me.

BTW, I checked into the Travis CI failures and looks like it is related to a mocha test using arrow functions, which seems like were only supported later on - nodejs 4 and later .

Cheers!

[kobim]

The drop of old node versions is problematic and should be discussed broadly (cc @lazerwalker @dblock @matt-kruse).

As far for the audit log, most of the security issues are related to devDependencies which aren't installed (nor used) in production environments.

I don't think I will merge these changes at the moment, at least until we decide to drop older node versions (and in the way make use of the newer ES5/6 syntax.)

[lazerwalker]

Yep, agree it's fine to not merge these in for now.

I also agree it is worth discussing whether dropping support for old versions of node is valuable or not. That node 3 isn't even listed on the node release schedule, and that Mocha has decided to drop support for it, are signals that there may not be many negative repercussions to dropping support for legacy node versions < 4.

Someone using a version of node that's so old that the node organization doesn't even offer LTS plans seems unlikely to care about upgrading our library. I poked on npm, but couldn't find analytics about what version of node people installing alexa-app via npm tend to be using — anyone know if that data exists?

If that's true, that still isn't a vote in favor of actively dropping support. I agree that merely being able to update some of our devDependencies isn't a particularly strong reason to go ahead. Are there any other benefits other than getting to use arrow functions and some other newer syntax in the codebase itself?

[lazerwalker]

(Sorry, maybe this discussion should be in a different issue / thread?)

[marcelobern]

Please feel free to close this PR without merge if you decide to proceed with support for early node versions, as in a few weeks/months this security review would be obsolete anyway ;-)