fix(detect-non-literal-fs-filename, detect-child-process, detect-non-…

…literal-regexp, detect-non-literal-require): false positives for static expressions
eslint-community · Jan 12, 2023 · b2d4d0f · b2d4d0f
1 parent c54e618
commit b2d4d0f
Show file tree

Hide file tree

Showing 12 changed files with 586 additions and 44 deletions.
diff --git a/rules/detect-child-process.js b/rules/detect-child-process.js
@@ -5,6 +5,8 @@
 
 'use strict';
 
+const { isStaticExpression } = require('../utils/is-static-expression');
+
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -64,7 +66,15 @@ module.exports = {
       },
       MemberExpression: function (node) {
         if (node.property.name === 'exec' && childProcessIdentifiers.has(node.object)) {
-          if (node.parent && node.parent.arguments && node.parent.arguments.length && node.parent.arguments[0].type !== 'Literal') {
+          if (
+            node.parent &&
+            node.parent.arguments &&
+            node.parent.arguments.length &&
+            !isStaticExpression({
+              node: node.parent.arguments[0],
+              scope: context.getScope(),
+            })
+          ) {
             return context.report({ node: node, message: 'Found child_process.exec() with non Literal first argument' });
           }
         }

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, isStaticExpressionFast } = require('../utils/is-static-expression');
 
 //------------------------------------------------------------------------------
 // Rule Definition
@@ -44,7 +30,7 @@ module.exports = {
     return {
       CallExpression: function (node) {
         // don't check require. If all arguments are Literals, it's surely safe!
-        if ((node.callee.type === 'Identifier' && node.callee.name === 'require') || node.arguments.every((argument) => argument.type === 'Literal')) {
+        if ((node.callee.type === 'Identifier' && node.callee.name === 'require') || node.arguments.every((argument) => isStaticExpressionFast(argument))) {
           return;
         }
 
@@ -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
@@ -19,6 +19,10 @@ tester.run(ruleName, rule, {
       code: "var { spawn } = require('child_process'); spawn(str);",
       parserOptions: { ecmaVersion: 6 },
     },
+    `
+    var child_process = require('child_process');
+    var 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,29 @@ 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',
+      },
+    },
   ],
   invalid: [
     /// requires
@@ -141,5 +164,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,15 @@ 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}\`)`,
+    },
+  ],
   invalid: [
     {
       code: 'var a = require(c)',