Only return exit code 1 when known vulnerability is detected

[sdann]

With the NPM audit service having intermittent issues for a week, I think audit-ci should only exit with 1 if a vulnerability is present. It currently exists with 1 due to the NOAUDIT error received from the NPM registry.

Right now my CI builds are failing intermittently and I have to manually disable runs of audit-ci on all my repos. I'd prefer the behavior that environmental issues are ignored and only verified vulnerabilities cause a pipeline error.

Perhaps make this configurable?

[quinnturner]

Thanks for outlining the intermittent issues with ENOAUDIT.

A retry mechanism was introduced in v1.7.0, (hopefully) mitigating this issue (obviously not removing it). Are you getting failures on the current version?

Would definitely be open to making this configurable.

[sdann]

Yes, I'm using 1.7.0. I'm not sure what the server side issues are, but audit-ci seems to work the first time or fail all 5 retries.

[clement-escolano]

@sdann Are you using NPM or yarn as the projet package manager?

I noticed that using yarn the retry mechanism seems to work (I often see RETRY-RETRY in the logs). However, I don't see the RETRY-RETRY log when my NPM audit fails randomly. Maybe the retry mechanism does not work with NPM error message?

[sdann]

I'm using NPM. The few times that audit-ci failed during the NPM outage the retry mechanism seemed to run. My output was:

RETRY-RETRY
RETRY-RETRY
RETRY-RETRY
RETRY-RETRY
RETRY-RETRY

[quinnturner]

Regarding the ENOAUDIT issue, the reliability of Yarn is independent of the reliability with NPM because Yarn and NPM use different registries. I am open to having a new option --pass-enoaudit or equivalent, but the idea that a CI would pass on a non-audit is alarming.

It is concerning that NPM can even fail that many tries in a row. I cannot think of a reason why the calls from audit-ci would be the reason, but I am not sure.

One intermittent fix is to use the --package-manager yarn option even if you're using NPM. Yarn only uses package.json for auditing and I believe the local filesystem (don't quote me on that, but I know that it doesn't use yarn.lock).

[sdann]

Hit this again today, even though NPM said it was fixed:

> node ./node_modules/audit-ci/bin/audit-ci --config .audit-ci.json

RETRY-RETRY
RETRY-RETRY
RETRY-RETRY
RETRY-RETRY
RETRY-RETRY
Invocation of npm audit failed:
npm ERR! code ENOAUDIT
npm ERR! audit Your configured registry (https://registry.npmjs.org/) does not support audit requests.
npm ERR! A complete log of this run can be found in:
npm ERR!     /root/.npm/_logs/2019-06-03T19_57_25_829Z-debug.log
Exiting...

I'll see if we can use --package-manager yarn.

IMO skipping a few audits on a non-vulnerability failure, is more acceptable than breaking the deployment pipeline due to a 3rd party service issue. We do 10s of deployments per day. Breaking that is more impactful than the small chance a new vulnerability was introduced and not detected for a few deployments.

[quinnturner]

PR: #88, @sdann, you're welcome to review!

[quinnturner]

Hi, I am hoping #89 addresses this issue for NPM in v2.0.1 just released (thanks @clement-escolano!) Please let me know if you continue to have this issue. If v2.0.1 is not fixed, #88 will come in.

[sdann]

Thanks! I've pulled down 2.0.1 and deployed it in our CI pipeline.

[sdann]

Unfortunately the fix in 2.0.1 isn't working. Just hit this today:

> node ./node_modules/.bin/audit-ci --config .audit-ci.json --pass-enoaudit

RETRY-RETRY
RETRY-RETRY
RETRY-RETRY
RETRY-RETRY
RETRY-RETRY
Invocation of npm audit failed:
npm ERR! code ENOAUDIT
npm ERR! audit Your configured registry (https://registry.npmjs.org/) does not support audit requests.
npm ERR! A complete log of this run can be found in:
npm ERR!     /root/.npm/_logs/2019-06-07T22_44_27_844Z-debug.log
Exiting...

From the debug logs:

...
12 verbose node v10.13.0
13 verbose npm  v6.4.1
14 error code ENOAUDIT
15 error audit Your configured registry (https://registry.npmjs.org/) does not support audit requests.
16 verbose exit [ 1, true ]

[quinnturner]

Thank you for attaching the log! In this case, your error would not have been addressed by v2.0.1 since v2.0.1 addressed issues regarding the error message that has ... requests, ... rather than ... requests.

At this point, I am entirely stumped by this error. Does it happen every time, or just occasionally? It looks like we will have to merge the --pass-enoaudit option 🤕

[sdann]

It only happens occasionally but then it happens repeatedly for a short period of time ~10 minutes. All deployments running within that window hit it.

[sdann]

Rough stats:

• We're doing about 10 deployments a day, roughly between 9AM - 5PM.
• We're hitting ENOAUDIT about once a week. So about 1 time for every 50 deployments during business hours.
• The last time it happened (June 11th) 2 deployments 15 minutes apart both hit it.

All failures seem to be ... requests.

[quinnturner]

I have merged the --pass-enoaudit flag, to be released in 2.1.0 #95

[sdann]

Thank you!

[quinnturner]

I am closing this, will pin this issue for others to see. Thanks :) Will re-open if more issues come up.

[sdann]

Just in time for a long npm audit outage today. Thanks @quinnturner

I got to immediately test 2.1.0:

> ./scripts/test:audit

RETRY-RETRY
RETRY-RETRY
RETRY-RETRY
RETRY-RETRY
RETRY-RETRY
ACTION RECOMMENDED: An audit could not performed due to 5 audits that resulted in ENOAUDIT. Perform an audit manually and verify that no significant vulnerabilities exist before merging.
Passed npm security audit.

[sdann]

[quinnturner]

Woohoo 🎉 well, not good that NPM is down, but at least it's gracefully handled. Awesome, thanks @sdann!