chore: readability improvements

Co-authored-by: adrobuta <alexandra.drobut@snyk.io>
Co-authored-by: neil     <neil.lowrie@snyk.io>

lib/analyzer/applications/node-modules-utils.ts

@@ -1,86 +1,129 @@
 import * as Debug from "debug";
-import * as fs from "fs";
+import { mkdir, mkdtemp, rm, stat, writeFile } from "fs/promises";
 import * as path from "path";
-import * as tmp from "tmp";
+
 import { FilePathToContent, FilesByDir } from "./types";
 const debug = Debug("snyk");
 
 export { persistAppNodeModules, cleanupAppNodeModules, groupFilesByDirectory };
 
+interface ScanPaths {
+  tempDir: string;
+  tempApplicationPath: string;
+  manifestPath?: string;
+}
+
+async function createTempAppDir(appParentDir: string): Promise<string[]> {
+  const tmpDir = await mkdtemp("snyk");
+
+  const appRootDir = appParentDir.includes("node_modules")
+    ? appParentDir.substring(0, appParentDir.indexOf("node_modules"))
+    : appParentDir;
+
+  const tempAppRootDirPath = path.join(tmpDir, appRootDir);
+
+  await mkdir(tempAppRootDirPath, { recursive: true });
+
+  return [tmpDir, tempAppRootDirPath];
+}
+
+const manifestName: string = "package.json";
+
+async function fileExists(path: string): Promise<boolean> {
+  return await stat(path)
+    .then(() => true)
+    .catch(() => false);
+}
+
+async function createAppSyntheticManifest(
+  tempRootManifestDir: string,
+): Promise<void> {
+  const tempRootManifestPath = path.join(tempRootManifestDir, manifestName);
+  debug(`Creating an empty synthetic manifest file: ${tempRootManifestPath}`);
+  await writeFile(tempRootManifestPath, "{}", "utf-8");
+}
+
+async function copyAppModulesManifestFiles(
+  appDirs: string[],
+  tempAppRootDirPath: string,
+  fileNamesGroupedByDirectory: FilesByDir,
+  filePathToContent: FilePathToContent,
+) {
+  for (const dependencyPath of appDirs) {
+    const filesInDirectory = fileNamesGroupedByDirectory[dependencyPath];
+    if (filesInDirectory.length === 0) {
+      continue;
+    }
+
+    const manifestPath = path.join(dependencyPath, "package.json");
+    const manifestContent = filePathToContent[manifestPath];
+
+    await createFile(
+      path.join(tempAppRootDirPath, manifestPath),
+      manifestContent,
+    );
+  }
+}
+
 async function persistAppNodeModules(
   filePathToContent: FilePathToContent,
   fileNamesGroupedByDirectory: FilesByDir,
-): Promise<string[]> {
-  const directoryTree = Object.keys(fileNamesGroupedByDirectory).sort();
-  if (directoryTree.length === 0) {
+): Promise<ScanPaths> {
+  const appDirs = Object.keys(fileNamesGroupedByDirectory);
+  let tmpDir: string = "";
+  let tempAppRootDirPath: string = "";
+
+  if (appDirs.length === 0) {
     debug(`Empty application directory tree.`);
-    return ["", ""];
-  }
 
-  const tempAppRootPath = tmp.dirSync().name;
-  const appRootDirNameLength = directoryTree[0].indexOf("node_modules");
-  const appRootDir =
-    appRootDirNameLength === -1
-      ? directoryTree[0]
-      : directoryTree[0].substring(0, appRootDirNameLength);
-  const appRootDirPath = path.join(tempAppRootPath, appRootDir);
+    return {
+      tempDir: tmpDir,
+      tempApplicationPath: tempAppRootDirPath,
+    };
+  }
 
   try {
-    fs.mkdirSync(appRootDirPath, { recursive: true });
-  } catch (error) {
-    debug(
-      `Failed to create the temporary root directory of the application: ${error.message}`,
+    [tmpDir, tempAppRootDirPath] = await createTempAppDir(appDirs.sort()[0]);
+
+    await copyAppModulesManifestFiles(
+      appDirs,
+      tmpDir,
+      fileNamesGroupedByDirectory,
+      filePathToContent,
     );
-    return ["", ""];
-  }
-  const appRootPackageJsonPath = path.join(appRootDirPath, "package.json");
 
-  for (const directoryPath of directoryTree) {
-    const filesInDirectory = fileNamesGroupedByDirectory[directoryPath];
-    if (filesInDirectory.length < 1) {
-      continue;
-    }
-    const moduleJsonFilePath = path.join(directoryPath, filesInDirectory[0]);
-    const moduleJsonFileContent = filePathToContent[moduleJsonFilePath];
-    const tempModuleJsonFilePath = path.join(
-      tempAppRootPath,
-      moduleJsonFilePath,
+    const result: ScanPaths = {
+      tempDir: tmpDir,
+      tempApplicationPath: tempAppRootDirPath,
+      manifestPath: path.join(
+        tempAppRootDirPath.substring(tmpDir.length),
+        manifestName,
+      ),
+    };
+
+    const manifestFileExists = await fileExists(
+      path.join(tempAppRootDirPath, manifestName),
     );
-    try {
-      await createFile(tempModuleJsonFilePath, moduleJsonFileContent);
-    } catch (error) {
-      debug(`Failed to create the temporary manifest file: ${error.message}`);
-      continue;
-    }
-  }
 
-  try {
-    // Check if the package.json exists in the app root dir,
-    // if it doesn't, create an empty "package.json" required by resolve-deps
-    fs.statSync(appRootPackageJsonPath);
+    if (!manifestFileExists) {
+      await createAppSyntheticManifest(tempAppRootDirPath);
+      delete result.manifestPath;
+    }
+    return result;
   } catch (error) {
     debug(
-      `Creating an empty ${appRootPackageJsonPath} required by resolveDeps`,
+      `Failed to copy the application manifest files locally: ${error.message}`,
     );
-    fs.writeFileSync(appRootPackageJsonPath, "{}", "utf-8");
+    return {
+      tempDir: tmpDir,
+      tempApplicationPath: tempAppRootDirPath,
+    };
   }
-
-  return [tempAppRootPath, appRootDirPath];
 }
 
-// Function to create file with content asynchronously
 async function createFile(filePath, fileContent) {
-  try {
-    const fileDir = path.dirname(filePath);
-    // Ensure directory existence before writing the file
-    fs.mkdirSync(fileDir, { recursive: true });
-
-    const fileContentJson = JSON.parse(fileContent);
-    // Write content to the file
-    fs.writeFileSync(filePath, JSON.stringify(fileContentJson), "utf-8");
-  } catch (error) {
-    debug(`Error while creating ${filePath} : ${error.message}`);
-  }
+  await mkdir(path.dirname(filePath), { recursive: true });
+  await writeFile(filePath, fileContent, "utf-8");
 }
 
 function groupFilesByDirectory(
@@ -100,7 +143,7 @@ function groupFilesByDirectory(
 
 async function cleanupAppNodeModules(appRootDir: string) {
   try {
-    fs.rmSync(appRootDir, { recursive: true, force: true });
+    rm(appRootDir, { recursive: true });
   } catch (error) {
     debug(`Error while removing ${appRootDir} : ${error.message}`);
   }

lib/analyzer/applications/node.ts

@@ -46,64 +46,61 @@ export async function nodeFilesToScannedProjects(
     fileNamesGroupedByDirectory,
   );
 
-  if (manifestFilePairs.length === 0) {
-    debug(
-      "No manifest lock file pairs found, computing the depGraph from node_modules directory",
-    );
-    return depGraphFromNodeModules(
-      filePathToContent,
-      fileNamesGroupedByDirectory,
-    );
-  } else {
-    return depGraphFromManifestFiles(filePathToContent, manifestFilePairs);
-  }
-  return [];
+  return manifestFilePairs.length === 0
+    ? depGraphFromNodeModules(filePathToContent, fileNamesGroupedByDirectory)
+    : depGraphFromManifestFiles(filePathToContent, manifestFilePairs);
 }
 
 async function depGraphFromNodeModules(
   filePathToContent: FilePathToContent,
   fileNamesGroupedByDirectory: FilesByDir,
 ): Promise<AppDepsScanResultWithoutTarget[]> {
-  const scanResults: AppDepsScanResultWithoutTarget[] = [];
+  const { tempDir, tempApplicationPath, manifestPath } =
+    await persistAppNodeModules(filePathToContent, fileNamesGroupedByDirectory);
 
-  const [appRootPath, appRootDir] = await persistAppNodeModules(
-    filePathToContent,
-    fileNamesGroupedByDirectory,
-  );
-
-  if (appRootPath === "" || appRootDir === "") {
-    return scanResults;
+  if (!tempApplicationPath) {
+    return [];
   }
 
+  const scanResults: AppDepsScanResultWithoutTarget[] = [];
+
   try {
-    const depRes: lockFileParser.PkgTree = await resolveDeps(appRootDir, {
-      dev: false,
-      noFromArrays: true,
-    });
+    const pkgTree: lockFileParser.PkgTree = await resolveDeps(
+      tempApplicationPath,
+      {
+        dev: false,
+        noFromArrays: true,
+      },
+    );
 
-    const depGraph = await legacy.depTreeToGraph(depRes, "npm");
-    const depGraphFact: DepGraphFact = {
-      type: "depGraph",
-      data: depGraph,
-    };
-    const testedFilesFact: TestedFilesFact = {
-      type: "testedFiles",
-      data: Object.keys(filePathToContent),
-    };
+    const depGraph = await legacy.depTreeToGraph(
+      pkgTree,
+      pkgTree.type || "npm",
+    );
     scanResults.push({
-      facts: [depGraphFact, testedFilesFact],
+      facts: [
+        {
+          type: "depGraph",
+          data: depGraph,
+        },
+        {
+          type: "testedFiles",
+          data: Object.keys(filePathToContent),
+        },
+      ],
       identity: {
-        type: depGraph.pkgManager ? depGraph.pkgManager.name : "npm",
-        targetFile: "package.json",
+        type: depGraph.pkgManager.name,
+        targetFile: manifestPath,
       },
     });
   } catch (error) {
     debug(
       `An error occurred while analysing node_modules dir: ${error.message}`,
     );
+  } finally {
+    await cleanupAppNodeModules(tempDir);
   }
 
-  await cleanupAppNodeModules(appRootPath);
   return scanResults;
 }
 

test/system/application-scans/node.spec.ts

@@ -68,7 +68,7 @@ describe("node application scans", () => {
     expect(pluginResultExcludeAppVulnsTrueBoolean.scanResults).toHaveLength(1);
   });
 
-  it("npm7 depgraph is generated from node modules npm manifest files ", async () => {
+  it("ScanResult contains a npm7 depGraph generated from node modules manifest files", async () => {
     const imageWithManifestFiles = getFixture(
       "npm/npm-without-lockfiles/npm7-with-package-lock-file.tar",
     );
@@ -100,17 +100,17 @@ describe("node application scans", () => {
     expect(depGraphNpmFromManifestFiles.pkgManager.name).toEqual("npm");
     expect(depGraphNpmFromManifestFiles.rootPkg.name).toEqual("goof");
     expect(depGraphNpmFromManifestFiles.rootPkg.version).toBe("1.0.1");
-    expect(depGraphNpmFromManifestFiles.getPkgs().length).toEqual(538); // approximate to the number reported by snyk test --dev
+    expect(depGraphNpmFromManifestFiles.getPkgs().length).toEqual(65); // approximate to the number reported by snyk test --dev
     expect(depGraphNpmFromNodeModules.pkgManager.name).toEqual("npm");
     expect(depGraphNpmFromNodeModules.rootPkg.name).toEqual("goof");
     expect(depGraphNpmFromNodeModules.rootPkg.version).toBe("1.0.1");
     // dev dependencies are reported
-    expect(depGraphNpmFromNodeModules.getPkgs().length).toEqual(526);
+    expect(depGraphNpmFromNodeModules.getPkgs().length).toEqual(65);
   });
 
-  it("npm7 depGraph is generated when the image does not contain package.json or package-lock.json for the application", async () => {
+  it("ScanResult contains a npm7 depGraph when package.json | package-lock.json is missing from app", async () => {
     const imageWithNodeModules = getFixture(
-      "npm/npm-without-lockfiles/npm7-without-package-and-lock-file.tar",
+      "npm/npm-without-lockfiles/npm7-with-node-modules-only.tar",
     );
     const imageWithoutLockFile = getFixture(
       "npm/npm-without-lockfiles/npm7-without-package-lock-file.tar",
@@ -140,16 +140,16 @@ describe("node application scans", () => {
     expect(depGraphNpmFromWithoutLockFiles.pkgManager.name).toEqual("npm");
     expect(depGraphNpmFromWithoutLockFiles.rootPkg.name).toEqual("goof");
     expect(depGraphNpmFromWithoutLockFiles.rootPkg.version).toBe("1.0.1");
-    expect(depGraphNpmFromWithoutLockFiles.getPkgs().length).toEqual(526);
+    expect(depGraphNpmFromWithoutLockFiles.getPkgs().length).toEqual(65);
     expect(depGraphNpmFromNodeModules.pkgManager.name).toEqual("npm");
     // when both package.json and package-lock.json is missing root package is the name of the application dir
     // and the version for the root package remains undefined and the dev dependencies are reported
     expect(depGraphNpmFromNodeModules.rootPkg.name).toEqual("goof");
     expect(depGraphNpmFromNodeModules.rootPkg.version).toBe(undefined);
-    expect(depGraphNpmFromNodeModules.getPkgs().length).toEqual(1030);
+    expect(depGraphNpmFromNodeModules.getPkgs().length).toEqual(65);
   });
 
-  it("yarn depgraph is generated from node modules npm manifest files ", async () => {
+  it("Scan result contains a yarn depgraph generated from node modules manifest files", async () => {
     const imageWithManifestFiles = getFixture(
       "npm/npm-without-lockfiles/yarn-with-lock-file.tar",
     );
@@ -181,17 +181,17 @@ describe("node application scans", () => {
     expect(depGraphNpmFromManifestFiles.pkgManager.name).toEqual("yarn");
     expect(depGraphNpmFromManifestFiles.rootPkg.name).toEqual("goof");
     expect(depGraphNpmFromManifestFiles.rootPkg.version).toBe("1.0.1");
-    expect(depGraphNpmFromManifestFiles.getPkgs().length).toEqual(601); // approximate to the number reported by snyk test --dev
+    expect(depGraphNpmFromManifestFiles.getPkgs().length).toEqual(65); // approximate to the number reported by snyk test --dev
     expect(depGraphNpmFromNodeModules.pkgManager.name).toEqual("npm");
     expect(depGraphNpmFromNodeModules.rootPkg.name).toEqual("goof");
     expect(depGraphNpmFromNodeModules.rootPkg.version).toBe("1.0.1");
     // dev dependencies are reported
-    expect(depGraphNpmFromNodeModules.getPkgs().length).toEqual(528);
+    expect(depGraphNpmFromNodeModules.getPkgs().length).toEqual(65);
   });
 
-  it("yarn depGraph is generated when the image does not contain package.json or package-lock.json for the application", async () => {
+  it("ScanResult contains a yarn depGraph package.json | package-lock.json is missing from the app", async () => {
     const imageWithNodeModules = getFixture(
-      "npm/npm-without-lockfiles/yarn-without-package-and-lock-file.tar",
+      "npm/npm-without-lockfiles/yarn-with-node-modules-only.tar",
     );
     const imageWithoutLockFile = getFixture(
       "npm/npm-without-lockfiles/yarn-without-lock-file.tar",
@@ -221,16 +221,16 @@ describe("node application scans", () => {
     expect(depGraphNpmFromWithoutLockFiles.pkgManager.name).toEqual("npm");
     expect(depGraphNpmFromWithoutLockFiles.rootPkg.name).toEqual("goof");
     expect(depGraphNpmFromWithoutLockFiles.rootPkg.version).toBe("1.0.1");
-    expect(depGraphNpmFromWithoutLockFiles.getPkgs().length).toEqual(528);
+    expect(depGraphNpmFromWithoutLockFiles.getPkgs().length).toEqual(65);
     expect(depGraphNpmFromNodeModules.pkgManager.name).toEqual("npm");
     // when both package.json and package-lock.json is missing root package is the name of the application dir
     // and the version for the root package remains undefined and the dev dependencies are reported
     expect(depGraphNpmFromNodeModules.rootPkg.name).toEqual("goof");
     expect(depGraphNpmFromNodeModules.rootPkg.version).toBe(undefined);
-    expect(depGraphNpmFromNodeModules.getPkgs().length).toEqual(683);
+    expect(depGraphNpmFromNodeModules.getPkgs().length).toEqual(65);
   });
 
-  it("resolveDeps should return a depGraph constructed from node_modules when the app doest not contain package.json ", async () => {
+  it("resolveDeps should return a depGraph constructed from node_modules when the application dir doesn't contain the package.json file", async () => {
     const fixturePath = getFixture("/npm/npm-without-lockfiles/home/app/");
     const expectedDepgraphJson = getObjFromFixture(
       "npm/npm-without-lockfiles/resolveDepsResultEmptyPackage.json",