[Feature]: Ignore devDependencies about audit-ci

[shockey]

@quinnturner, would you consider reopening this?

Our use case is that we want to have different severity cutoffs for dev and production dependencies: any production dependency vulnerability should cause a failure since users will be notified of them upon install of our module, but only high/critical dev dependency vulnerabilities should be show-stoppers.

Currently, we're doing this in another tool by doing two passes with different configurations.

[simontabor]

@quinnturner would you consider an option to set the severity level for dev dependencies? I agree with your justification about not skipping dev devs (disaster waiting to happen, a dev dep can easily affect your prod code), but I think it's a common and justified use-case to set different failure thresholds.

Something like this:

audit-ci --moderate --dev-critical

[quinnturner]

@simontabor I would consider that as an option. I do understand that there may be different requirements for severities in dev vs. prod.

I do have one major concern with the devDependency approach. I just ran yarn audit --json on a Yarn project with one vulnerable devDependency and no dependencies. It explicitly does not handle devDependencies and treated it as a regular dependency. To accommodate this, we would have to traverse the original package.json and negotiate whether a dependency is dev or not.

{
   "type":"auditAdvisory",
   "data":{
      "resolution":{
         "id":690,
         "path":"cryo",
         "dev":false,
         "bundled":false,
         "optional":false
      },
      "advisory":{
         "findings":[
            {
               "version":"0.0.6",
               "paths":[
                  "cryo"
               ]
            }
         ],
         "id":690,
         "created":"2018-08-16T19:50:35.895Z",
         "updated":"2019-06-24T23:04:26.491Z",
         "deleted":null,
         "title":"Code Injection",
         "found_by":{
            "link":"",
            "name":"Alexey Tyurin"
         },
         "reported_by":{
            "link":"",
            "name":"Alexey Tyurin"
         },
         "module_name":"cryo",
         "cves":[
            "CVE-2018-3784"
         ],
         "vulnerable_versions":">=0.0.0",
         "patched_versions":"<0.0.0",
         "overview":"All versions of `cryo` are vulnerable to code injection due to an Insecure implementation of deserialization.\n\n\n## Proof of concept\n\n```\nvar Cryo = require('cryo');\nvar frozen = '{\"root\":\"_CRYO_REF_3\",\"references\":[{\"contents\":{},\"value\":\"_CRYO_FUNCTION_function () {console.log(\\\\\"defconrussia\\\\\"); return 1111;}\"},{\"contents\":{},\"value\":\"_CRYO_FUNCTION_function () {console.log(\\\\\"defconrussia\\\\\");return 2222;}\"},{\"contents\":{\"toString\":\"_CRYO_REF_0\",\"valueOf\":\"_CRYO_REF_1\"},\"value\":\"_CRYO_OBJECT_\"},{\"contents\":{\"__proto__\":\"_CRYO_REF_2\"},\"value\":\"_CRYO_OBJECT_\"}]}'\nvar hydrated = Cryo.parse(frozen);\nconsole.log(hydrated);\n```",
         "recommendation":"No fix is currently available. Consider using an alternative module until a fix is made available.",
         "references":"- [HackerOne Report](https://hackerone.com/reports/350418)",
         "access":"public",
         "severity":"high",
         "cwe":"CWE-502",
         "metadata":{
            "module_type":"",
            "exploitability":7,
            "affected_components":""
         },
         "url":"https://npmjs.com/advisories/690"
      }
   }
}
{
   "type":"auditSummary",
   "data":{
      "vulnerabilities":{
         "info":0,
         "low":0,
         "moderate":0,
         "high":1,
         "critical":0
      },
      "dependencies":1,
      "devDependencies":0,
      "optionalDependencies":0,
      "totalDependencies":1
   }
}

Here's the package.json:

{
  "name": "audit-ci-yarn-high-vulnerability",
  "description": "Test package.json with high vulnerability",
  "devDependencies": {
    "cryo": "0.0.6"
  }
}

I will note that NPM's results are correct:

{
  "actions": [
    {
      "action": "review",
      "module": "cryo",
      "resolves": [
        {
          "id": 690,
          "path": "cryo",
          "dev": true,
          "bundled": false,
          "optional": false
        }
      ]
    }
  ],
  "advisories": {
    "690": {
      "findings": [
        {
          "version": "0.0.6",
          "paths": [
            "cryo"
          ]
        }
      ],
      "id": 690,
      "created": "2018-08-16T19:50:35.895Z",
      "updated": "2019-06-24T23:04:26.491Z",
      "deleted": null,
      "title": "Code Injection",
      "found_by": {
        "link": "",
        "name": "Alexey Tyurin"
      },
      "reported_by": {
        "link": "",
        "name": "Alexey Tyurin"
      },
      "module_name": "cryo",
      "cves": [
        "CVE-2018-3784"
      ],
      "vulnerable_versions": ">=0.0.0",
      "patched_versions": "<0.0.0",
      "overview": "All versions of `cryo` are vulnerable to code injection due to an Insecure implementation of deserialization.\n\n\n## Proof of concept\n\n```\nvar Cryo = require('cryo');\nvar frozen = '{\"root\":\"_CRYO_REF_3\",\"references\":[{\"contents\":{},\"value\":\"_CRYO_FUNCTION_function () {console.log(\\\\\"defconrussia\\\\\"); return 1111;}\"},{\"contents\":{},\"value\":\"_CRYO_FUNCTION_function () {console.log(\\\\\"defconrussia\\\\\");return 2222;}\"},{\"contents\":{\"toString\":\"_CRYO_REF_0\",\"valueOf\":\"_CRYO_REF_1\"},\"value\":\"_CRYO_OBJECT_\"},{\"contents\":{\"__proto__\":\"_CRYO_REF_2\"},\"value\":\"_CRYO_OBJECT_\"}]}'\nvar hydrated = Cryo.parse(frozen);\nconsole.log(hydrated);\n```",
      "recommendation": "No fix is currently available. Consider using an alternative module until a fix is made available.",
      "references": "- [HackerOne Report](https://hackerone.com/reports/350418)",
      "access": "public",
      "severity": "high",
      "cwe": "CWE-502",
      "metadata": {
        "module_type": "",
        "exploitability": 7,
        "affected_components": ""
      },
      "url": "https://npmjs.com/advisories/690"
    }
  },
  "muted": [],
  "metadata": {
    "vulnerabilities": {
      "info": 0,
      "low": 0,
      "moderate": 0,
      "high": 1,
      "critical": 0
    },
    "dependencies": 0,
    "devDependencies": 1,
    "optionalDependencies": 0,
    "totalDependencies": 1
  },
  "runId": "e561d7a9-89c4-4e9b-8c94-32be0598a5aa"
}

Given these results, it seems only reasonable to implement it in NPM (without writing a lot more complex and potentially brittle code... What happens when using Yarn workspaces and the dependency is using different versions at different levels of the tree and some are dev and some are prod?)

[quinnturner]

@betimer, while I understand the convenience listing all devDependencies as whitelisted, I still stand by my initial reason why it is likely not appropriate behaviour in my previous comment: #91 (comment). I believe that my previously mentioned solution is still valid. If you have a particular use-case that is not covered by that solution I will consider integrating it.

[quinnturner]

I was wondering when this question would come up! 😄

My personal belief is that this is not the desired behaviour; if {npm|yarn} audit identifies fixable vulnerabilities, the developer should address the issue. If a dev package got severely compromised such as grabbing the user's or server's SSH keys or running a crypto miner, this should be caught immediately.

Regardless, if you want this behaviour, the appropriate course of action would be to allowlist each dev dependency individually. I would suggest creating a config file to make the CLI command less verbose.

Here's an example:

audit-ci.json

{
  "high": true,
  "allowlist": ["mocha", "chai"]
}

Then for the CLI:

audit-ci --config ./audit-ci.json

[mandric]

Fair enough!

[quinnturner]

Thanks for the feature suggestion though. I will close for now but will pin this issue for future users.

[mandric]

👍

Seconding here that this would be incredibly helpful for a project I'm working on too

[philstavri]

It would be really useful to be able to target prod dependencies only, for example when building PRs and targeting all dependencies, i.e. for an overnight build.

Also worth noting it looks like the reporting of the dependencies is a bit messed up (they are all coming through as dependencies despite most being in devDependencies):

"dependencies": 60754,
"devDependencies": 0,
"optionalDependencies": 0,
"totalDependencies": 60754

[quinnturner]

@JoeNewmanMSM What specifically would be useful in this case? Is it the multiple levels of dependency vulnerability catching?

Does the current solution of specifying what vulnerabilities to ignore satisfy your needs? There are three options available.

"advisories": [12, 23, 24], <-- across all packages, ignore advisory xxx
"whitelist": ["example1", "example2"], <-- ignore all advisories for package yyy (useful for dev)
"path-whitelist": ["52|example3", "880|example4", "880|example5>example4"], <-- Ignore specific advisories for specific packages

@philstavri, I don't know what to suggest in your case other than the above solution.

[betimer]

How is this feature going? Has it been developed? @quinnturner

[quinnturner]

With #186, this highly requested feature will have a solid middle-ground solution in the upcoming v4.1.0 release. While I still stand by #91 (comment) that skipping devDependencies is strictly inferior for security than using allowlist if using it as your only vulnerability detection mechanism, I understand that there are several workflows that skipping dev dependencies makes sense. Thanks to @WhatIfWeDigDeeper for the PR!