outmoded / hapi-contrib

Discussion forum for project contributors
Other
79 stars 26 forks source link

Security CI #63

Closed Marsup closed 5 years ago

Marsup commented 8 years ago

I was wondering if we needed some kind of security checks in our CIs, like nsp or snyk. I'm not so worried on our direct dependencies because it's often watched closely, but more on the indirect ones which we might not monitor that much. Any opinions on that matter ?

nlf commented 8 years ago

i'm pretty involved with nsp (as in, i'm doing a portion of the development), so i'm not too worried about adding it to CI. in the past i've notified eran directly when things are bad, that's why we ended up taking over the qs module.

Marsup commented 8 years ago

So you're saying you'd be in favor of automating that kind of check ?

nlf commented 8 years ago

i certainly don't see any harm in it

DavidTPate commented 8 years ago

We run it in all of our CI pieces and it's definitely handy at finding known vulnerabilities. One issue that we've run into is that sometimes we have to just ignore certain bulletins because the libraries aren't fixed yet and won't accept our PRs. But definitely useful for identifying issues with public dependencies.

Marsup commented 8 years ago

So how do we go about it now ? Integrate into lab ? Each project adds it into its npm scripts ?

AdriVanHoudt commented 8 years ago

adding it to lab would be the easiest no?

cjihrig commented 8 years ago

Please no to adding it to lab. Lab already has way too much crap included. Seems best to add to the npm scripts in package.json.

vdeturckheim commented 8 years ago

I agree with @cjihrig, actually, I'm not sure it would be smart to fail a test set if a security issue is raised: Sometimes, no fix is available at the time of execution.

devinivy commented 8 years ago

For a build to pass, does it have to pass this CI check? Unlike unit tests (usually), whether or not the security check passes could depend on when the test is run and what vulnerabilities are known at that time. It makes for a strange check to require to pass. I'd be more interested to see that issues are created to deal with these security issues as they arise.

The question remains– when are these checks run? I think Travis should run the security checks, but it also needs to be separate from per-build CI.

Marsup commented 8 years ago

So something like greenkeeper for security. @nlf is it in the roadmap of nsp ?

DavidTPate commented 8 years ago

I don't know about greenkeeper like functionality, but for nsp you can run nsp check --warn-only to not fail the command when there is an issue.

cjihrig commented 8 years ago

The security check should only be done at npm publish time. prepublish would be good, although it also runs on local npm install. Maybe on preversion if you use that as part of your workflow.