fix: false positives for static expressions in detect-non-literal-fs-…

…filename, detect-child-process, detect-non-literal-regexp, and detect-non-literal-require (#109) * fix: false positives for static expressions in detect-non-literal-fs-filename, detect-child-process, detect-non-literal-regexp, and detect-non-literal-require * remove dupe test * add tests
eslint-community · Feb 2, 2023 · 56102b5 · 56102b5
1 parent 75e1e9d
commit 56102b5
Show file tree

Hide file tree

Showing 12 changed files with 622 additions and 43 deletions.
diff --git a/rules/detect-child-process.js b/rules/detect-child-process.js
@@ -6,6 +6,7 @@
 'use strict';
 
 const { getImportAccessPath } = require('../utils/import-utils');
+const { isStaticExpression } = require('../utils/is-static-expression');
 const childProcessPackageNames = ['child_process', 'node:child_process'];
 
 //------------------------------------------------------------------------------
@@ -41,7 +42,13 @@ module.exports = {
         }
 
         // Reports non-literal `exec()` calls.
-        if (!node.arguments.length || node.arguments[0].type === 'Literal') {
+        if (
+          !node.arguments.length ||
+          isStaticExpression({
+            node: node.arguments[0],
+            scope: context.getScope(),
+          })
+        ) {
           return;
         }
         const pathInfo = getImportAccessPath({

diff --git a/rules/detect-non-literal-fs-filename.js b/rules/detect-non-literal-fs-filename.js
@@ -10,21 +10,7 @@ const funcNames = Object.keys(fsMetaData);
 const fsPackageNames = ['fs', 'node:fs', 'fs/promises', 'node:fs/promises', 'fs-extra'];
 
 const { getImportAccessPath } = require('../utils/import-utils');
-
-//------------------------------------------------------------------------------
-// Utils
-//------------------------------------------------------------------------------
-
-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;
-  }
-  context.report({ node, message: `Found ${methodName} from package "${packageName}" with non literal argument at index ${indices.join(',')}` });
-}
+const { isStaticExpression } = require('../utils/is-static-expression');
 
 //------------------------------------------------------------------------------
 // Rule Definition
@@ -87,15 +73,23 @@ module.exports = {
         }
         const packageName = pathInfo.packageName;
 
-        const indices = getIndices(node, fsMetaData[fnName]);
-
-        generateReport({
-          context,
-          node,
-          packageName,
-          methodName: fnName,
-          indices,
-        });
+        const indices = [];
+        for (const index of fsMetaData[fnName] || []) {
+          if (index >= node.arguments.length) {
+            continue;
+          }
+          const argument = node.arguments[index];
+          if (isStaticExpression({ node: argument, scope: context.getScope() })) {
+            continue;
+          }
+          indices.push(index);
+        }
+        if (indices.length) {
+          context.report({
+            node,
+            message: `Found ${fnName} from package "${packageName}" with non literal argument at index ${indices.join(',')}`,
+          });
+        }
       },
     };
   },

diff --git a/rules/detect-non-literal-regexp.js b/rules/detect-non-literal-regexp.js
@@ -5,6 +5,8 @@
 
 'use strict';
 
+const { isStaticExpression } = require('../utils/is-static-expression');
+
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -24,7 +26,14 @@ module.exports = {
       NewExpression: function (node) {
         if (node.callee.name === 'RegExp') {
           const args = node.arguments;
-          if (args && args.length > 0 && args[0].type !== 'Literal') {
+          if (
+            args &&
+            args.length > 0 &&
+            !isStaticExpression({
+              node: args[0],
+              scope: context.getScope(),
+            })
+          ) {
             return context.report({ node: node, message: 'Found non-literal argument to RegExp Constructor' });
           }
         }

diff --git a/rules/detect-non-literal-require.js b/rules/detect-non-literal-require.js
@@ -5,6 +5,8 @@
 
 'use strict';
 
+const { isStaticExpression } = require('../utils/is-static-expression');
+
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -25,8 +27,12 @@ module.exports = {
         if (node.callee.name === 'require') {
           const args = node.arguments;
           if (
-            (args && args.length > 0 && args[0].type === 'TemplateLiteral' && args[0].expressions.length > 0) ||
-            (args[0].type !== 'TemplateLiteral' && args[0].type !== 'Literal')
+            args &&
+            args.length > 0 &&
+            !isStaticExpression({
+              node: args[0],
+              scope: context.getScope(),
+            })
           ) {
             return context.report({ node: node, message: 'Found non-literal argument in require' });
           }

diff --git a/test/detect-child-process.js b/test/detect-child-process.js
@@ -50,6 +50,14 @@ tester.run(ruleName, rule, {
     function fn () {
       require('child_process').spawn(str)
     }`,
+    `
+    var child_process = require('child_process');
+    var FOO = 'ls';
+    child_process.exec(FOO);`,
+    `
+    import child_process from 'child_process';
+    const FOO = 'ls';
+    child_process.exec(FOO);`,
   ],
   invalid: [
     {

diff --git a/test/detect-non-literal-fs-filename.js b/test/detect-non-literal-fs-filename.js
@@ -3,7 +3,7 @@
 const RuleTester = require('eslint').RuleTester;
 const tester = new RuleTester({
   parserOptions: {
-    ecmaVersion: 6,
+    ecmaVersion: 13,
     sourceType: 'module',
   },
 });
@@ -24,6 +24,51 @@ tester.run(ruleName, require(`../rules/${ruleName}`), {
       code: `var something = require('fs').readFile, readFile = require('foo').readFile;
              readFile(c);`,
     },
+    {
+      code: `
+            import { promises as fsp } from 'fs';
+            import fs from 'fs';
+            import path from 'path';
+            
+            const index = await fsp.readFile(path.resolve(__dirname, './index.html'), 'utf-8');
+            const key = fs.readFileSync(path.join(__dirname, './ssl.key'));
+            await fsp.writeFile(path.resolve(__dirname, './sitemap.xml'), sitemap);`,
+      globals: {
+        __dirname: 'readonly',
+      },
+    },
+    {
+      code: `
+            import fs from 'fs';
+            import path from 'path';
+            const dirname = path.dirname(__filename)
+            const key = fs.readFileSync(path.resolve(dirname, './index.html'));`,
+      globals: {
+        __filename: 'readonly',
+      },
+    },
+    {
+      code: `
+            import fs from 'fs';
+            const key = fs.readFileSync(\`\${process.cwd()}/path/to/foo.json\`);`,
+      globals: {
+        process: 'readonly',
+      },
+    },
+    `
+    import fs from 'fs';
+    import path from 'path';
+    import url from 'url';
+    const dirname = path.dirname(url.fileURLToPath(import.meta.url));
+    const html = fs.readFileSync(path.resolve(dirname, './index.html'), 'utf-8');`,
+    {
+      code: `
+      import fs from 'fs';
+      const pkg = fs.readFileSync(require.resolve('eslint/package.json'), 'utf-8');`,
+      globals: {
+        require: 'readonly',
+      },
+    },
   ],
   invalid: [
     /// requires
@@ -141,5 +186,15 @@ tester.run(ruleName, require(`../rules/${ruleName}`), {
       code: "var fs = require('fs');\nfunction foo () {\nvar { readFile: something } = fs.promises;\nsomething(filename);\n}",
       errors: [{ message: 'Found readFile from package "fs" with non literal argument at index 0' }],
     },
+    {
+      code: `
+            import fs from 'fs';
+            import path from 'path';
+            const key = fs.readFileSync(path.resolve(__dirname, foo));`,
+      globals: {
+        __filename: 'readonly',
+      },
+      errors: [{ message: 'Found readFileSync from package "fs" with non literal argument at index 0' }],
+    },
   ],
 });
diff --git a/test/detect-non-literal-regexp.js b/test/detect-non-literal-regexp.js
@@ -7,7 +7,14 @@ const ruleName = 'detect-non-literal-regexp';
 const invalid = "var a = new RegExp(c, 'i')";
 
 tester.run(ruleName, require(`../rules/${ruleName}`), {
-  valid: [{ code: "var a = new RegExp('ab+c', 'i')" }],
+  valid: [
+    { code: "var a = new RegExp('ab+c', 'i')" },
+    {
+      code: `
+            var source = 'ab+c'
+            var a = new RegExp(source, 'i')`,
+    },
+  ],
   invalid: [
     {
       code: invalid,

diff --git a/test/detect-non-literal-require.js b/test/detect-non-literal-require.js
@@ -7,7 +7,21 @@ const tester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });
 const ruleName = 'detect-non-literal-require';
 
 tester.run(ruleName, require(`../rules/${ruleName}`), {
-  valid: [{ code: "var a = require('b')" }, { code: 'var a = require(`b`)' }],
+  valid: [
+    { code: "var a = require('b')" },
+    { code: 'var a = require(`b`)' },
+    {
+      code: `
+  const d = 'debounce'
+  var a = require(\`lodash/\${d}\`)`,
+    },
+    {
+      code: "const utils = require(__dirname + '/utils');",
+      globals: {
+        __dirname: 'readonly',
+      },
+    },
+  ],
   invalid: [
     {
       code: 'var a = require(c)',