--audit-level pass-through does not behave as expected

[joebowbeer]

An observation and possibly a bug.

I tried passing --audit-level moderate in v2.2.0 as a replacement for the removed ignoreLow support (#19), but it is not working as I expected. Even though npm audit succeeded (exit: 0), check-audit failed:

$ npx check-audit --audit-level moderate
>>>> npm audit --json --audit-level moderate
>>>> exit: 0
Total of 1 actions to process
--------------------------------------------------
[low] Regular Expression Denial of Service
 - dependencies: gulp-watch>anymatch>micromatch>braces
--------------------------------------------------
 😱   Unresolved issues found!
--------------------------------------------------
$ echo $?
1

NOTE: --audit-level does not change the output, it only changes the exit code.

Repro:

mkdir foo
cd foo
npm init
npm i gulp-watch npm-audit-resolver
npx check-audit --audit-level moderate
echo $?

[naugtur]

Oh. Your note explains it all. check-audit ignores exit code from npm/yarn because even if exit code is >0 it may be ok after we apply the ignore rules.
You'd need to use a switch that limits output or implement more logic in audit-resolver to understand that exit code of 0 means check-audit should also be ok.
But then if you get 100 low issues and one high, it fails and reports all of them anyway (which is more problematic for the resolve command than check, but anyway)

Looks like It'll have to be a feature request.

[colinbjohnson]

Should this be entered in #1? I would be interested in the same functionality.

[naugtur]

It's good here. #1 is for stuff you're not sure about.

I wonder if it'd make sense to handle this transparently, but then I'd have to go through all options for both npm and yarn to be consistent and not cause confusion why some of them work and some don't.

So I'd rather go with rules you'd need to specify in the audit-resolve file.

Unless there's a general rule that applies here. But if I just passed successful exit code in this case, the content would come up in resolver for review anyway and you'd have to take action.

[Jackques]

--audit-level flag is not working in my situation. It's not clear to me if it should work or not but judging from the comments above or if this would work in a new version?

At the very least i would love something like a --ignoreLowVulnerabilities flag seeing as i get some low vulnerabilities and one moderate or above.

Could something like this be supported?

[naugtur]

again, --audit-level only changes the exit code of audit process, but doesn't change the output and output is the only thing resolver can reasonably use.

The functionality will come to the resolver.
I'm not doing it now, because I'd like to get some more context on how resolver fits into npm itself and the talks have resumed.

But some form of auto-ignoring certain level will be supported.

[naugtur]

I'm considering changing the behavior in v3 to exit 0 if oriinal command npm audit exits 0.

[naugtur]

Anybody who needs this still here? I'd like to pick it up and discuss the solution in latest context.

[joebowbeer]

I could still use this. I'd like to pass-through --production --audit-level moderate and have check-audit only apply leniency. That is, if npm audit ... doesn't fail, the checker should not contramand it.

[naugtur]

In other words - if npm audit itself wouldn't fail the build, check-audit shouldn't either, and I should skip all further work after getting exit code 0.

Wondering if it should always be the case or only if certain flags are detected.

[joebowbeer]

Always the case (independent of flags) seems logical to me.

[naugtur]

Assuming npm doesn't accidentally exit 0 in some configuration...

[naugtur]

Ok, so I implemented this.
@joebowbeer - feel freer to try it out from the commit. I'll need to fix tests before I publish it ;)

[joebowbeer]

@naugtur thanks; I'll try as soon as there is a published version

[naugtur]

Not what I hoped for but I'll take it

[joebowbeer]

I installed this commit globally and it worked as expected/desired

npm i -g git+https://github.com/naugtur/npm-audit-resolver#ba17250
npm ls -g --depth 0
.nvm/versions/node/v14.21.2/lib
├── npm@6.14.17
└── npm-audit-resolver@3.0.0-7 (git+https://github.com/naugtur/npm-audit-resolver.git#ba17250)

check-audit --audit-level high
echo $?
1

check-audit --audit-level critical
echo $?
0

[joebowbeer]

Fixed in npm-audit-resolver#ba17250

[ptc-tyourchuck]

Awesome! Any ETA on when the fix will be officially part of next? I ran into this issue using npm-audit-resolver@next (v3.0.0-7)

[naugtur]

I just need to make sure it's complete and release 3.0.0.
Down with COVID at the moment, but that might mean I get the time to do it.

[ptc-tyourchuck]

Get well soon.

[ptc-tyourchuck]

Are there still plans for "some form of auto-ignoring certain level" as you referenced in #22 (comment) ?

Using npm-audit-resolver@3.0.0-9 there is still a scenario that isn't working how I'd expect. And I think the "auto-ignoring certain level" would be the solution for this scenario. Is that correct?

Scenario:

1. install a dep with a moderate issue and a dep with a high issue (e.g. npm install cookiejar@2.1.3 json5@2.2.1 )
2. run resolve-audit skip the moderate issue and ignore the high issue

{
  "decisions": {
    "1091148|json5": {
      "decision": "ignore",
      "madeAt": 1679072208865
    }
  },
  "rules": {},
  "version": 1
}

3. run check-audit --audit-level high

Expected the check-audit would succeed, but it exits with non-zero exit code.

[naugtur]

I thought I implemented that. I'll check and fix before final release if possible. Thank you for reporting.
If you look at the recent changes there's a piece that takes exit code of nom audit and if it's zero, it won't fail check-audit.

Aaaand I just rubber-duckied the problem by writing about it.
So --audit-level works by passing it into npm. If there's a higher level vuln that's ignored, npm audit exits nonzero so the exit passthrough will not be helpful.

We'd need to implement audit level in resolver or make npm audit --json not return the issues below the level and what it's doing now seems to be just deciding the exit code.
Could you check what npm audit itself returns? I'm pretty sure that's the case.

So to sum up, I won't fix that for 3.0.0 but it's definitely doable as 3.1.0

[ptc-tyourchuck]

yes, npm audit --audit-level high (and npm audit --audit-level high --json) exit non-zero in this scenario (as expected since there is a high issue)

and as you mention, npm audit --audit-level high --json currently returns all the issues, its not filtered by the --audit-level

[naugtur]

Ok, so we'd have to implement this as a feature to do the filtering.
It could either be using the same --audit-level flag or it could be a configuration in the audit-resolve.json file.
The latter is less problematic for the future and yarn support etc.
Would it work for you if it was a configuration in the "rules" section of the audit-resolve.json?

[ptc-tyourchuck]

It could be more convenient if it were an arg (--audit-level or other arg) or an env var. Then we'd be able to add that into the shared CI/CD template as the default rather than adding it into the audit-resolve.json of every repo. That said, it's certainly pretty easy to add a new rule into all the audit-resolve.json files.
Either way, will just be glad to have the feature. Thanks :)

[ptc-tyourchuck]

Would the configuration be a true/false flag? or it'd be the level? For some reason I'd initially thought it would be a true/false flag, but probably you meant it would be the level to filter? A true/false flag in the audit-resolve.json rules works fine in our case, but putting the level in the audit-resolve.json file is less convenient.

We found it didn't scale well to always detect all levels of audit issues in our pipelines for N microservices.
So we have 2 jobs in our pipelines, audit-high and audit-low. The audit-high runs against every commit and runs when a release branch is created. The audit-low job runs on demand or in any merge request where the package-lock.json changes. We still want to handle the lows and moderates, just less urgently/agressively.

These audit-high and audit-low jobs currently use the same audit-resolve.json file and just pass in different --audit-level arg.