chore: snyk integration testing

[sethbergman]

Description of changes

A continuation of #562

Issue Resolved

Fixes #496

The tests passed and this PR is ready for review.

[kylemh]

I've been emailing Snyk's help team for the past few days to figure out why this isn't working. Hopefully be able to get to the bottom of it soon. The build is passing for this branch because Travis isn't actually running anything with snyk.

Travis does currently run npm run test, so when snyk test was part of the test script in package.json we were getting build failures. The error was that snyk auth needs to be run, but that's not true for Travis... It should be working since Travis has the API key as an environment variable as detailed here.

Please remove snyk protect related stuff, and re-implement snyk test. I don't want Snyk to try to auto-correct issues. I'd like to handle them as they appear.

[sethbergman]

I don't think you'd want to run snyk test during the CI build phase. The auth token is encrypted in the travis settings, but you will likely fail the tests most of the time. The snyk vulnerability database indexes dependencies and their dependencies. So there will usually be long periods of time before the bugs are patched or updated to secure versions.

This is why I think it's better to have the snyk admin run the initial setup of the project using snyk wizard, and set it to scan daily or weekly without affecting the velocity of the team.

What I've done is make it to where snyk protect is run during the pre-commit stage - ensuring that no additional vulnerabilities are introduced in case extra dependencies are added by the contributor. What we can also do is run snyk monitor with the deploy script. This will allow snyk to watch for any new patches / updates to the currently 2 known vulnerabilities in our project, and notify us once they become available.

Another option that has worked well for me is enabling Greenkeeper. It's less cumbersome and seems to stay out of your way by automatically submitting PR's when new npm packages are updated.

I ran snyk test during pre-commit and got this output:

running fix...passed!
running format...passed!
running lint...passed!
running image-size...passed!
running snyk-test...failed!

Testing /home/seth/PROJECTS/operationcode_frontend...
✗ High severity vulnerability found on marked@0.3.12
- desc: Regular Expression Denial of Service (ReDoS)
- info: https://snyk.io/vuln/npm:marked:20171207
- from: operationcode_frontend@0.1.0 > @storybook/react@3.4.0-alpha.5 > markdown-loader@2.0.2 > marked@0.3.12


✗ High severity vulnerability found on shelljs@0.8.1
- desc: Command Injection
- info: https://snyk.io/vuln/npm:shelljs:20140723
- from: operationcode_frontend@0.1.0 > @storybook/react@3.4.0-alpha.5 > shelljs@0.8.1


✗ High severity vulnerability found on shelljs@0.8.1
- desc: Command Injection
- info: https://snyk.io/vuln/npm:shelljs:20140723
- from: operationcode_frontend@0.1.0 > @storybook/cli@3.4.0-alpha.5 > shelljs@0.8.1

Tested 1017 dependencies for known vulnerabilities, found 2 vulnerabilities, 3 vulnerable paths.

I hope this provides some clarity, snyk can be a handful if it's automated into the build process.

https://github.com/snyk/snyk/blob/master/help/help.txt

[kylemh]

@sethbergman I'd prefer the build failed when vulnerabilities are found and we can decide to ignore them from their dashboard. We had 3 vulnerabilities (as you've noted), all related to Storybook - so I ignored them "forever" because they're development dependencies.

snyk protect is less ideal because it applies patches unless custom fixes are described - I'd prefer to not have snyk apply fixes. snyk test is definitely the behavior I want.

I don't believe I need further clarification. Last week, I tied my account to the organization's repository acting as the "snyk admin" for the org, and yet running snyk test consistently fails the build because of the described need to run snyk auth. That's not what myself nor Snyk expected, and we've been emailing back and forth to figure out where I've gone wrong with integrating the organization.

[sethbergman]

There is an option to include devDependencies that is defaulted to off.
https://github.com/snyk/snyk/blob/master/help/help.txt

[sethbergman]

If you want to pass the CI build, you'll have to use Docker and install snyk globally, run snyk auth, set the environment variables for the admin, then run your tests. If you fail, you will have to manually ignore the vulnerabilities and update the .snyk policy before you're able to pass. Why not do this during pre-commit? Save yourself some time.

[kylemh]

The devDependency flag is good, but we may eventually host our Storybook as a non-devDependency, yet I would still ignore vulnerabilities as it exists as documentation and attacks are irrelevant.

I was following this blog which indicates running snyk auth locally and giving the API key to Travis as an environment variable will be fine.

I just don't understand why we'd need to run snyk auth on a build-by-build process instead of assert the organization's validity through the environment variable... We could run snyk auth process.env.SNYK_TOKEN && snyk test && ...test, but Snyk has indicated it should work without snyk auth in CI in my correspondence thus far.

Why not do this during pre-commit? Save yourself some time.

Dependencies are added so rarely that I'd argue you're losing time by making every developer run a very taxing snyk test (nearly a minute) on pre-commits instead of once on build. We're going to do it that way.

[sethbergman]

I never recommended snyk test. snyk protect was my suggestion for pre-commit, but it's still about 50% less time than snyk test, which is still too long. I know you'll find a resolution that is satisfactory. 👍

[jjhampton]

Labelling as blocked until we hear back from Snyk's help team.