feat: extend detect non literal fs filename (#92)

* feat: rewritten detect-non-literal-fs-filename rule

* detects multiple cases of possible imports of fs methods
* many tests added to cover that cases

* refactor: extract searching for ImportDeclaration and VariableDeclaration in utils

* chore: added JSdoc, inlined fsPackagesNames, renamed sink terminology

* chore: changed report params to object

* chore: rename 'sink', remove '?.' for node 12

Co-authored-by: Bastian Gebhardt <Bastian [email protected]>

Author: BuZZ-T

package-lock.json

@@ -5,12 +5,210 @@
 
 'use strict';
 
+const fsMetaData = require('./data/fsFunctionData.json');
+const funcNames = Object.keys(fsMetaData);
+const fsPackageNames = ['fs', 'node:fs', 'fs/promises', 'node:fs/promises', 'fs-extra'];
+
+const { getImportDeclaration, getVariableDeclaration } = require('./utils/import-utils');
+
 //------------------------------------------------------------------------------
-// Rule Definition
+// Utils
 //------------------------------------------------------------------------------
 
-const fsMetaData = require('./data/fsFunctionData.json');
-const funcNames = Object.keys(fsMetaData);
+function getIndices(node, argMeta) {
+  return (argMeta || []).filter((argIndex) => node.arguments[argIndex].type !== 'Literal');
+}
+
+function generateReport({ context, node, packageName, methodName, indices }) {
+  if (!indices || indices.length === 0) {
+    return null;
+  }
+  return context.report({ node, message: `Found ${methodName} from package "${packageName}" with non literal argument at index ${indices.join(',')}` });
+}
+
+/**
+ * Detects:
+ * | var something = require('fs').readFile;
+ * | something(a);
+ */
+function detectOnRequireWithProperty({ context, methodName, node, program }) {
+  const declaration = getVariableDeclaration({
+    condition: (declaration) => declaration.init.parent.id.name === methodName,
+    hasObject: true,
+    methodName,
+    packageNames: fsPackageNames,
+    program,
+  });
+
+  if (!declaration) {
+    return null;
+  }
+
+  // we found the require for our method!
+  const fsFunction = declaration.init.property.name;
+  const packageName = declaration.init.object.arguments[0].value;
+  const fnName = declaration.init.property.name;
+
+  const indices = getIndices(node, fsMetaData[fsFunction]);
+
+  return generateReport({
+    context,
+    node,
+    packageName,
+    methodName: fnName,
+    indices,
+  });
+}
+
+/**
+ * Detects:
+ * | var something = require('fs');
+ * | something.readFile(c);
+ */
+function detectOnMethodCall({ context, node, program, methodName }) {
+  const declaration = getVariableDeclaration({
+    packageNames: fsPackageNames,
+    hasObject: false,
+    program,
+  });
+
+  if (!declaration) {
+    return null;
+  }
+
+  const indices = getIndices(node.parent, fsMetaData[methodName]);
+
+  return generateReport({
+    context,
+    node,
+    packageName: declaration.init.arguments[0].value,
+    methodName,
+    indices,
+  });
+}
+
+/**
+ * Detects:
+ * | var { readFile: something } = require('fs')
+ * | readFile(filename)
+ */
+function detectOnDestructuredRequire({ context, methodName, node, program }) {
+  const declaration = getVariableDeclaration({
+    condition: (declaration) => declaration && declaration.id && declaration.id.properties && declaration.id.properties.some((p) => p.value.name === methodName),
+    hasObject: false,
+    methodName,
+    packageNames: fsPackageNames,
+    program,
+  });
+
+  if (!declaration) {
+    return null;
+  }
+
+  const realMethodName = declaration.id.properties.find((p) => p.value.name === methodName).key.name;
+
+  const meta = fsMetaData[realMethodName];
+  const indices = getIndices(node, meta);
+
+  return generateReport({
+    context,
+    node,
+    packageName: declaration.init.arguments[0].value,
+    methodName: realMethodName,
+    indices,
+  });
+}
+
+/**
+ * Detects:
+ * | import { readFile as something } from 'fs';
+ * | something(filename);
+ */
+function detectOnDestructuredImport({ context, methodName, node, program }) {
+  const importDeclaration = getImportDeclaration({ methodName, packageNames: fsPackageNames, program });
+
+  const specifier = importDeclaration && importDeclaration.specifiers && importDeclaration.specifiers.find((s) => !!funcNames.includes(s.imported.name));
+
+  if (!specifier) {
+    return null;
+  }
+
+  const fnName = specifier.imported.name;
+  const meta = fsMetaData[fnName];
+  const indices = getIndices(node, meta);
+
+  return generateReport({
+    context,
+    node,
+    packageName: specifier.parent.source.value,
+    methodName: fnName,
+    indices,
+  });
+}
+
+/**
+ * Detects:
+ * | import * as something from 'fs';
+ * | something.readFile(c);
+ */
+function detectOnDefaultImport({ context, methodName, node, objectName, program }) {
+  if (!funcNames.includes(methodName)) {
+    return null;
+  }
+
+  const importDeclaration = getImportDeclaration({ methodName: objectName, packageNames: fsPackageNames, program });
+
+  if (!importDeclaration) {
+    return null;
+  }
+
+  const meta = fsMetaData[methodName];
+  const indices = getIndices(node.parent, meta);
+
+  return generateReport({
+    context,
+    node,
+    packageName: importDeclaration.source.value,
+    methodName,
+    indices,
+  });
+}
+
+/**
+ * Detects:
+ * | var something = require('fs').promises;
+ * | something.readFile(filename)
+ */
+function detectOnPromiseProperty({ context, methodName, node, objectName, program }) {
+  const declaration = program.body
+    .filter((entry) => entry.type === 'VariableDeclaration')
+    .flatMap((entry) => entry.declarations)
+    .find(
+      (declaration) =>
+        declaration.id.name === objectName &&
+        declaration.init.type === 'MemberExpression' &&
+        // package name is fs / fs-extra
+        fsPackageNames.includes(declaration.init.object.arguments[0].value)
+    );
+
+  if (!declaration) {
+    return null;
+  }
+  const meta = fsMetaData[methodName];
+  const indices = getIndices(node.parent, meta);
+
+  return generateReport({
+    context,
+    node,
+    packageName: declaration.init.object.arguments[0].value,
+    methodName,
+    indices,
+  });
+}
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
 
 module.exports = {
   meta: {
@@ -24,30 +222,88 @@ module.exports = {
   },
   create: function (context) {
     return {
+      CallExpression: function (node) {
+        // readFile/open/... (but might be renamed)
+        const localMethodName = node.callee.name;
+
+        // don't check require. If all arguments are Literals, it's surely safe!
+        if (!localMethodName || localMethodName === 'require' || node.arguments.every((argument) => argument.type === 'Literal')) {
+          return;
+        }
+
+        // this only works, when imports are on top level!
+        const program = context.getAncestors()[0];
+
+        const requireReport = detectOnRequireWithProperty({
+          context,
+          methodName: localMethodName,
+          node,
+          program,
+        });
+        if (requireReport) {
+          return requireReport;
+        }
+
+        const destructuredRequireReport = detectOnDestructuredRequire({
+          context,
+          methodName: localMethodName,
+          node,
+          program,
+        });
+        if (destructuredRequireReport) {
+          return destructuredRequireReport;
+        }
+
+        const importReport = detectOnDestructuredImport({
+          context,
+          methodName: localMethodName,
+          node,
+          program,
+        });
+        if (importReport) {
+          return importReport;
+        }
+      },
       MemberExpression: function (node) {
-        const result = [];
-        if (funcNames.indexOf(node.property.name) !== -1) {
-          const meta = fsMetaData[node.property.name];
-          const args = node.parent.arguments;
-          meta.forEach((i) => {
-            if (args && args.length > i) {
-              if (args[i].type !== 'Literal') {
-                result.push(i);
-              }
-            }
-          });
+        const realMethodName = node.property.name; // readFile/open/... (not renamed)
+        const localObjectName = node.object.name; // fs/node:fs/... (but might be renamed)
+
+        // this only works, when imports are on top level!
+        const program = context.getAncestors()[0];
+
+        const methodCallReport = detectOnMethodCall({
+          context,
+          methodName: realMethodName,
+          node,
+          program,
+        });
+        if (methodCallReport) {
+          return methodCallReport;
         }
 
-        if (result.length > 0) {
-          return context.report({ node: node, message: `Found fs.${node.property.name} with non literal argument at index ${result.join(',')}` });
+        const defaultImportReport = detectOnDefaultImport({
+          program,
+          objectName: localObjectName,
+          methodName: realMethodName,
+          context,
+          node,
+        });
+
+        if (defaultImportReport) {
+          return defaultImportReport;
         }
 
-        /*
-              if (node.parent && node.parent.arguments && node.parent.arguments[index].value) {
-                  return context.report(node, 'found Buffer.' + node.property.name + ' with noAssert flag set true');
+        const promisePropertyReport = detectOnPromiseProperty({
+          context,
+          methodName: realMethodName,
+          node,
+          objectName: localObjectName,
+          program,
+        });
 
-              }
-              */
+        if (promisePropertyReport) {
+          return promisePropertyReport;
+        }
       },
     };
   },

rules/detect-non-literal-fs-filename.js

@@ -0,0 +1,41 @@
+/**
+ * Returns the ImportDeclaration for the import of the methodName from one of the packageNames
+ * import { methodName as a } from 'packageName';
+ *
+ * @param {Object} param0
+ * @param {string} param0.methodName
+ * @param {string[]} param0.packageNames
+ * @param {Object} param0.program The AST program object
+ * @returns The ImportDeclaration for the import of the methodName from one of the packageNames
+ */
+module.exports.getImportDeclaration = ({ methodName, packageNames, program }) =>
+  program.body.find((entry) => entry.type === 'ImportDeclaration' && packageNames.includes(entry.source.value) && entry.specifiers.some((s) => s.local.name === methodName));
+
+/**
+ * Returns the VariableDeclaration for a require based import
+ *
+ * @param {Object} param0
+ * @param {Function} param0.condition Optional function to check additional conditions on the resulting VariableDeclaration
+ * @param {boolean} param0.hasObject Whether the information is received by declaration.init or declaration.init.object
+ * @param {string[]} param0.packageNames The interesting packages the method is imported from
+ * @param {Object} param0.program The AST program object
+ * @returns
+ */
+module.exports.getVariableDeclaration = ({ condition, hasObject, packageNames, program }) =>
+  program.body
+    // a node import is a variable declaration
+    .filter((entry) => entry.type === 'VariableDeclaration')
+    // one var/let/const may contain multiple declarations, separated by comma, after the "=" sign
+    .flatMap((d) => d.declarations)
+    .find((d) => {
+      const init = hasObject ? d.init.object : d.init;
+
+      return (
+        init &&
+        init.callee &&
+        init.callee.name === 'require' &&
+        init.arguments[0].type === 'Literal' &&
+        packageNames.includes(init.arguments[0].value) &&
+        (!condition || condition(d))
+      );
+    });