Enable warnings/vulnerabilities ignore possibility through an .ignore file

[antoine-coulon]

Proposal

This proposal was initially kindly suggested by @RomainLanz and @targos

The main idea is to provide a way to ignore some vulnerabilities for specific dependencies and/or warnings for specific modules.

• Dependency warnings

Take for example these following reported warnings:

unsafe-stmt regenerator-runtime/runtime.js:752:4,752:52
unsafe-stmt lodash.difference/index.js:40:37,40:62
encoded-literal iconv-lite/encodings/internal.js:36,24:36,38

The goal would be to be able to specify through an .ignore file some ignore patterns:

.nsci-ignore

{
  "warnings": {
    "unsafe-stmt": ["lodash.difference", "regenerator-runtime"]
  }
}

Which would result in the following reporting outcome:

encoded-literal iconv-lite/encodings/internal.js:36,24:36,38

After that, we could even consider a lower level of granularity by specifying paths to files that should be ignored for a given warning.

Take the following warnings:

unsafe-regex negotiator/lib/encoding/encode.js:24:27,24:56
unsafe-regex negotiator/lib/encoding/decode.js:24:27,24:56
unsafe-stmt lodash.truncate/index.js:77:37,77:62

.nsci-ignore

{
  "warnings": {
    "unsafe-regex": {
      "negotiator": ["lib/encoding/*.js"]
    },
    "unsafe-stmt": {
      "lodash.truncate": ["index.js"]
    }
  }
}

This would totally ignore the three warnings above.

• Vulnerabilities

In the same spirit as for the warnings, we could provide a way to ignore vulnerabilities for specific dependencies.

Take for instance the following vulnerabilities from @npmcli/arborist, next and node-fetch:

[@npmcli/arborist] UNIX Symbolic Link (Symlink) Following in @npmcli/arborist <2.8.2
[next] Unexpected server crash in Next.js. >=12.0.0 <12.0.5
[node-fetch] node-fetch is vulnerable to Exposure of Sensitive Information to an Unauthorized Actor <2.6.7

By providing the following ignore patterns, we could get rid of the vulnerability check for any given dependencies (with any valid SemVer) in the dependency tree.

.nsci-ignore

{
  "warnings": {},
  "vulnerabilities": {
    "@npmcli/arborist": "<2.8.2",
    "next": ">=12.0.0 <12.0.5",
    "node-fetch": "*"
  }
}

The .ignore configuration above would simply ignore checks for the given SemVer of @npmcli/arborist, next and node-fetch.

Implementation

Big picture

First, it would require to enhance the Nsci.Configuration object (which is used when interpreting the @nodesecure/scanner payload) by reflecting the content of the .ignore file.

The process of ignoring should be introduced just before the Interpretation of the @nodesecure/scanner payload.

Given that the Interpretation step uses an Array of dependency warnings/vulnerabilities (provided by the Extraction step), we should:

• ignore (i.e: filter) warnings/vulnerabilities of the Array matching ignore patterns
• interpret the filtered warnings/vulnerabilities

Detailled

- Warnings

Given the following ignore patterns:

.nsci-ignore

{
  "warnings": {
    "obfuscated-code": ["famous-lib"]
  }
}

and the following warning:

EDIT:
.nsci-ignore example of js-x-ray warning

{
  "package": "famous-lib",
  "warnings": [
    {
      "kind": "obfuscated-code",
      "location": [
        [0, 0],
        [0, 0]
      ],
      "value": "trojan-source",
      "file": "obfuscated.js"
    }
  ]
}

The ignore pattern here allows us to ignore this warning by simply looking at the warning's root "package" and "kind". For RegEx patterns on files, the "file" property should be useful.

- Vulnerabilities

Given the following ignore patterns:

{
  "vulnerabilities": {
    "next": ">=12.0.0 <12.0.5"
  }
}

and the following vulnerability:

{
  "origin": "npm",
  "package": "next",
  "title": "Unexpected server crash in Next.js.",
  "url": "https://github.com/advisories/GHSA-25mp-g6fv-mqxx",
  "severity": "high",
  "vulnerableRanges": [">=12.0.0 <12.0.5"],
  "vulnerableVersions": []
}

We can filter the vulnerability by matching the SemVer defined within the .nsci-ignore against the "vulnerableRanges" SemVer Array.

[tony-go]

What I propose for this task, is to split it up into three milestones. Then, we could deliver the value piece by piece.

Milestone 1 - Basics Warnings

You'll be able to ignore:

{
  "warnings": {
    "unsafe-stmt": ["lodash.difference", "regenerator-runtime"]
  }
}

Milestone 2 - Vulns

You'll be able to ignore:

{
  "vulnerabilities": {
    "@npmcli/arborist": "<2.8.2",
    "next": ">=12.0.0 <12.0.5",
    "node-fetch": "*"
  }
}

Milestone 3 - Detailed Warnings

So instead of basic usage, you could:

{
  "warnings": {
    "unsafe-stmt": [
       {
         "package": "famous-lib",
         "files": ["obfuscated.js"]
      }
    ]
  }
}

cc @antoine-coulon for that one I'd prefer to stick to the warnings[<rule>] model.

[antoine-coulon]

Hey @tony-go 👋
That's a great idea to split this PR into these multiple milestones.
About the ignore patterns, I initially suggested to put them in a dedicated nsci-ignore, should we stick with that idea? What do you think?

cc @antoine-coulon for that one I'd prefer to stick to the warnings[<rule>] model.

I am not sure I understood this statement.
I think I did a typo in the issue, the warning described as

{
  "package": "famous-lib",
  "warnings": [
    {
      "kind": "obfuscated-code",
      "location": [
        [0, 0],
        [0, 0]
      ],
      "value": "trojan-source",
      "file": "obfuscated.js"
    }
  ]
}

comes from js-x-ray and does not correspond to the structure of an ignore pattern in the nsci-ignore. I'll edit that to make it clearer.

So about the Milestone 3, you're right it could be what you mentioned, but I'm not sure about specifying "value" and "location", as these parameters are quite implementation details and are not visible in the public API of js-x-ray. Specifying "package" and "file" for each rule is already quite advanced in my opinion:

{
  "warnings": {
    "unsafe-stmt": [
       {
         "package": "famous-lib",
-       "location": [
-          [0, 0],
-          [0, 0]
-         ],
-       "value": "trojan-source",
         "file": "obfuscated.js"
      }
    ]
  }
}

Potentially for each package we could handle an Array of files, to allow multiple ignore patterns for a specific warning and for a specific package

{
  "warnings": {
    "unsafe-stmt": [
       {
         "package": "famous-lib",
         "files": ["obfuscated.js", "lib/other-file.js"]
      }
    ]
  }
}

What do you think?

[tony-go]

@antoine-coulon I correct my comment, I'll start with that ^^