refactor(scanner)!: improve global warning implementation (#509)

Author: fraxken

.changeset/clean-sites-find.md

@@ -0,0 +1,5 @@
+---
+"@nodesecure/scanner": major
+---
+
+Extend global warning with multi-properties objects

workspaces/scanner/src/comparePayloads.ts

@@ -11,12 +11,13 @@ import type {
   Publisher,
   Maintainer,
   Repository,
-  DependencyLinks
+  DependencyLinks,
+  GlobalWarning
 } from "./types.js";
 
 export interface PayloadComparison {
   title: string;
-  warnings: ArrayDiff<string>;
+  warnings: ArrayDiff<GlobalWarning>;
   scannerVersion: ValueComparison<string>;
   vulnerabilityStrategy: ValueComparison<string>;
   dependencies: DependenciesComparison;

workspaces/scanner/src/depWalker.ts

@@ -27,6 +27,7 @@ import { Logger, ScannerLoggerEvents } from "./class/logger.class.js";
 import type {
   Dependency,
   DependencyVersion,
+  GlobalWarning,
   Options,
   Payload
 } from "./types.js";
@@ -213,7 +214,7 @@ export async function depWalker(
 
   // We do this because it "seem" impossible to link all dependencies in the first walk.
   // Because we are dealing with package only one time it may happen sometimes.
-  const globalWarnings: string[] = [];
+  const globalWarnings: GlobalWarning[] = [];
   for (const [packageName, dependency] of dependencies) {
     const metadataIntegrities = dependency.metadata?.integrity ?? {};
 
@@ -222,15 +223,21 @@ export async function depWalker(
 
       const isEmptyPackage = dependencyVer.warnings.some((warning) => warning.kind === "empty-package");
       if (isEmptyPackage) {
-        globalWarnings.push(`${packageName}@${version} only contain a package.json file!`);
+        globalWarnings.push({
+          type: "empty-package",
+          message: `${packageName}@${version} only contain a package.json file!`
+        });
       }
 
       if (!("integrity" in dependencyVer) || dependencyVer.flags.includes("isGit")) {
         continue;
       }
 
       if (dependencyVer.integrity !== integrity) {
-        globalWarnings.push(`${packageName}@${version} manifest & tarball integrity doesn't match!`);
+        globalWarnings.push({
+          type: "integrity-mismatch",
+          message: `${packageName}@${version} manifest & tarball integrity doesn't match!`
+        });
       }
     }
     for (const version of Object.entries(dependency.versions)) {

workspaces/scanner/src/types.ts

@@ -157,13 +157,30 @@ export interface Dependency {
 
 export type Dependencies = Record<string, Dependency>;
 
+export type GlobalWarning = { message: string; } & (
+  {
+    type:
+      | "dangerous-dependency"
+      | "integrity-mismatch"
+      | "empty-package";
+    metadata?: Record<string, unknown>;
+  } |
+  {
+    type: "typo-squatting";
+    metadata: {
+      name: string;
+      similar: string[];
+    };
+  }
+);
+
 export interface Payload {
   /** Payload unique id */
   id: string;
   /** Name of the analyzed package */
   rootDependencyName: string;
   /** Global warnings list */
-  warnings: string[];
+  warnings: GlobalWarning[];
   highlighted: {
     contacts: IlluminatedContact[];
   };

workspaces/scanner/src/utils/warnings.ts

@@ -14,7 +14,7 @@ import type { Contact } from "@nodesecure/npm-types";
 // Import Internal Dependencies
 import { getDirNameFromUrl } from "./dirname.js";
 import { TopPackages } from "../class/TopPackages.class.js";
-import type { Dependency } from "../types.js";
+import type { Dependency, GlobalWarning } from "../types.js";
 
 await i18n.extendFromSystemPath(
   path.join(getDirNameFromUrl(import.meta.url), "..", "i18n")
@@ -35,7 +35,7 @@ const kDependencyWarnMessage = {
 } as const;
 
 export interface GetWarningsResult {
-  warnings: string[];
+  warnings: GlobalWarning[];
   illuminated: IlluminatedContact[];
 }
 
@@ -49,8 +49,17 @@ export async function getDependenciesWarnings(
   const topPackages = new TopPackages();
   await topPackages.loadJSON();
 
-  const warnings = vulnerableDependencyNames
-    .flatMap((name) => (dependenciesMap.has(name) ? `${kDetectedDep(name)} ${kDependencyWarnMessage[name]}` : []));
+  const warnings: GlobalWarning[] = vulnerableDependencyNames
+    .flatMap((name) => {
+      if (!dependenciesMap.has(name)) {
+        return [];
+      }
+
+      return {
+        type: "dangerous-dependency",
+        message: `${kDetectedDep(name)} ${kDependencyWarnMessage[name]}`
+      };
+    });
 
   const dependencies: Record<string, ContactExtractorPackageMetadata> = Object.create(null);
   for (const [packageName, dependency] of dependenciesMap) {
@@ -62,7 +71,14 @@ export async function getDependenciesWarnings(
         packageName,
         similarPackages.join(", ")
       );
-      warnings.push(warningMessage);
+      warnings.push({
+        type: "typo-squatting",
+        message: warningMessage,
+        metadata: {
+          name: packageName,
+          similar: similarPackages
+        }
+      });
     }
 
     dependencies[packageName] = {

workspaces/scanner/test/comparePayloads.spec.ts

@@ -28,13 +28,23 @@ it("should throw an error if compared payloads are not from the same package", (
 });
 
 it("should detect warnings diff", () => {
-  const { warnings: { added, removed }, dependencies: { compared } } = compareTo("warningChangedPayload");
-
-  assert.strictEqual(added.length, 1);
-  assert.deepStrictEqual(added[0], "encoded-literal");
-
-  assert.strictEqual(removed.length, 1);
-  assert.deepStrictEqual(removed[0], "unsafe-regex");
+  const {
+    warnings: { added, removed },
+    dependencies: { compared }
+  } = compareTo("warningChangedPayload");
+
+  assert.deepEqual(added, [
+    {
+      type: "empty-package",
+      message: "..."
+    }
+  ]);
+  assert.deepEqual(removed, [
+    {
+      type: "dangerous-dependency",
+      message: "..."
+    }
+  ]);
 
   const deepWarnings = compared.get("foo")!.versions.compared.get("2.0.0")!.warnings;
   assert.strictEqual(deepWarnings.added.length, 1);

workspaces/scanner/test/depWalker.spec.ts

@@ -139,9 +139,14 @@ test("execute depWalker on typo-squatting", async() => {
     registry: getLocalRegistryURL()
   });
 
-  assert.deepEqual(result.warnings, [
+  assert.ok(result.warnings.length > 0);
+  const warning = result.warnings[0];
+
+  assert.equal(warning.type, "typo-squatting");
+  assert.strictEqual(
+    result.warnings[0].message,
     "The package 'mecha' is similar to the following popular packages: fecha, mocha"
-  ]);
+  );
 });
 
 test("fetch payload of pacote on the npm registry", async() => {

workspaces/scanner/test/fixtures/scannerPayloads/nullAuthor.json

@@ -1,22 +1,8 @@
 {
   "id": "YtK0Cx",
   "rootDependencyName": "foo",
-  "warnings": [
-    "unsafe-import",
-    "unsafe-regex"
-  ],
-  "highlighted": {
-    "contacts": [
-      {
-        "name": "sindresorhus",
-        "email": "[email protected]"
-      },
-      {
-        "name": "jack",
-        "email": "[email protected]"
-      }
-    ]
-  },
+  "warnings": [],
+  "highlighted": {},
   "vulnerabilityStrategy": "npm",
   "scannerVersion": "1.0.0",
   "dependencies": {

workspaces/scanner/test/fixtures/scannerPayloads/payload.json

@@ -2,8 +2,10 @@
   "id": "YuK0CL",
   "rootDependencyName": "foo",
   "warnings": [
-    "unsafe-import",
-    "unsafe-regex"
+    {
+      "type": "dangerous-dependency",
+      "message": "..."
+    }
   ],
   "highlighted": {
     "contacts": [

workspaces/scanner/test/fixtures/scannerPayloads/sameIdPayload.json

@@ -1,22 +1,8 @@
 {
   "id": "YuK0CL",
   "rootDependencyName": "foo",
-  "warnings": [
-    "unsafe-import",
-    "unsafe-regex"
-  ],
-  "highlighted": {
-    "contacts": [
-      {
-        "name": "sindresorhus",
-        "email": "[email protected]"
-      },
-      {
-        "name": "jack",
-        "email": "[email protected]"
-      }
-    ]
-  },
+  "warnings": [],
+  "highlighted": {},
   "vulnerabilityStrategy": "npm",
   "scannerVersion": "1.0.0",
   "dependencies": {