chore: readability improvements

Co-authored-by: adrobuta <alexandra.drobut@snyk.io> Co-authored-by: neil <neil.lowrie@snyk.io>
snyk · May 8, 2024 · 5d21fce · 5d21fce
1 parent 0b603e8
commit 5d21fce
Show file tree

Hide file tree

Showing 3 changed files with 120 additions and 100 deletions.
diff --git a/lib/analyzer/applications/node-modules-utils.ts b/lib/analyzer/applications/node-modules-utils.ts
@@ -1,86 +1,109 @@
 import * as Debug from "debug";
-import * as fs from "fs";
+import { mkdir, mkdtemp, rmdir, 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 };
 
-async function persistAppNodeModules(
-  filePathToContent: FilePathToContent,
-  fileNamesGroupedByDirectory: FilesByDir,
-): Promise<string[]> {
-  const directoryTree = Object.keys(fileNamesGroupedByDirectory).sort();
-  if (directoryTree.length === 0) {
-    debug(`Empty application directory tree.`);
-    return ["", ""];
-  }
+interface TemporaryPaths {
+  tempApplicationPath: string;
+  syntheticManifestPath?: 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);
 
-  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);
+  await mkdir(tempAppRootDirPath, { recursive: true });
 
+  return [tmpDir, tempAppRootDirPath];
+}
+
+async function createAppSyntheticManifest(
+  tempAppRootDirPath: string,
+): Promise<string | undefined> {
+  const tempRootManifestPath = path.join(tempAppRootDirPath, "package.json");
   try {
-    fs.mkdirSync(appRootDirPath, { recursive: true });
+    await stat(tempRootManifestPath);
   } catch (error) {
-    debug(
-      `Failed to create the temporary root directory of the application: ${error.message}`,
-    );
-    return ["", ""];
+    debug(`Creating an empty synthetic manifest file: ${tempRootManifestPath}`);
+    await writeFile(tempRootManifestPath, "{}", "utf-8");
+    return tempRootManifestPath;
   }
-  const appRootPackageJsonPath = path.join(appRootDirPath, "package.json");
 
-  for (const directoryPath of directoryTree) {
-    const filesInDirectory = fileNamesGroupedByDirectory[directoryPath];
-    if (filesInDirectory.length < 1) {
+  return undefined;
+}
+
+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 moduleJsonFilePath = path.join(directoryPath, filesInDirectory[0]);
-    const moduleJsonFileContent = filePathToContent[moduleJsonFilePath];
-    const tempModuleJsonFilePath = path.join(
-      tempAppRootPath,
-      moduleJsonFilePath,
+
+    const manifestPath = path.join(dependencyPath, filesInDirectory[0]);
+    const manifestContent = filePathToContent[manifestPath];
+
+    await createFile(
+      path.join(tempAppRootDirPath, manifestPath),
+      manifestContent,
     );
-    try {
-      await createFile(tempModuleJsonFilePath, moduleJsonFileContent);
-    } catch (error) {
-      debug(`Failed to create the temporary manifest file: ${error.message}`);
-      continue;
-    }
+  }
+}
+
+async function persistAppNodeModules(
+  filePathToContent: FilePathToContent,
+  fileNamesGroupedByDirectory: FilesByDir,
+): Promise<TemporaryPaths> {
+  const appDirs = Object.keys(fileNamesGroupedByDirectory);
+  if (appDirs.length === 0) {
+    debug(`Empty application directory tree.`);
+
+    return {
+      tempApplicationPath: "",
+    };
   }
 
   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);
+    const [tmpDir, tempAppRootDirPath] = await createTempAppDir(appDirs.sort()[0]);
+    await copyAppModulesManifestFiles(
+      appDirs,
+      tmpDir,
+      fileNamesGroupedByDirectory,
+      filePathToContent,
+    );
+    const tempSyntheticManifestPath = await createAppSyntheticManifest(
+      tempAppRootDirPath,
+    );
+    return {
+      tempApplicationPath: tempAppRootDirPath,
+      syntheticManifestPath: tempSyntheticManifestPath,
+    };
   } 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 {
+      tempApplicationPath: "",
+    };
   }
-
-  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 +123,7 @@ function groupFilesByDirectory(
 
 async function cleanupAppNodeModules(appRootDir: string) {
   try {
-    fs.rmSync(appRootDir, { recursive: true, force: true });
+    rmdir(appRootDir, { recursive: true });
   } catch (error) {
     debug(`Error while removing ${appRootDir} : ${error.message}`);
   }

diff --git a/lib/analyzer/applications/node.ts b/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 { tempApplicationPath, syntheticManifestPath } =
+    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: syntheticManifestPath || undefined,
       },
     });
   } catch (error) {
     debug(
       `An error occurred while analysing node_modules dir: ${error.message}`,
     );
+  } finally {
+    await cleanupAppNodeModules(tempApplicationPath);
   }
 
-  await cleanupAppNodeModules(appRootPath);
   return scanResults;
 }
 

diff --git a/test/system/application-scans/node.spec.ts b/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",
     );
@@ -108,7 +108,7 @@ describe("node application scans", () => {
     expect(depGraphNpmFromNodeModules.getPkgs().length).toEqual(526);
   });
 
-  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",
     );
@@ -149,7 +149,7 @@ describe("node application scans", () => {
     expect(depGraphNpmFromNodeModules.getPkgs().length).toEqual(1030);
   });
 
-  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",
     );
@@ -189,7 +189,7 @@ describe("node application scans", () => {
     expect(depGraphNpmFromNodeModules.getPkgs().length).toEqual(528);
   });
 
-  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",
     );
@@ -230,7 +230,7 @@ describe("node application scans", () => {
     expect(depGraphNpmFromNodeModules.getPkgs().length).toEqual(683);
   });
 
-  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",