diff --git a/rules/detect-child-process.js b/rules/detect-child-process.js index dd1f0c9..9fd86bb 100644 --- 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 index 8467258..799e43b 100644 --- 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 index 3c59461..a5f109e 100644 --- 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 index ccf71cb..31e7343 100644 --- 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 index 6af2cd2..efc45fd 100644 --- 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 index 315dc91..8d5bd30 100644 --- 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 index 04eafce..af5e054 100644 --- 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 index 2342ab5..1d15c21 100644 --- 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)', diff --git a/test/utils/is-static-expression.js b/test/utils/is-static-expression.js new file mode 100644 index 0000000..dd49c6e --- /dev/null +++ b/test/utils/is-static-expression.js @@ -0,0 +1,252 @@ +'use strict'; + +const { isStaticExpression } = require('../../utils/is-static-expression'); +const { deepStrictEqual } = require('assert'); + +const Linter = require('eslint').Linter; + +/** + * Get the return value using `isStaticExpression()`. + * Give `isStaticExpression()` the argument given to `target()` in the code as an expression. + */ +function getIsStaticExpressionResult(code) { + const linter = new Linter(); + const result = []; + linter.defineRule('test-rule', { + create(context) { + return { + 'CallExpression[callee.name = target]'(node) { + result.push( + ...node.arguments.map((expr) => + isStaticExpression({ + node: expr, + scope: context.getScope(), + }) + ) + ); + }, + }; + }, + }); + + const linterResult = linter.verify(code, { + parserOptions: { + ecmaVersion: 11, + sourceType: 'module', + }, + globals: { + __dirname: 'readonly', + __filename: 'readonly', + require: 'readonly', + }, + rules: { + 'test-rule': 'error', + }, + }); + deepStrictEqual(linterResult, []); + + return result; +} + +describe('isStaticExpression', () => { + describe('The result of isStaticExpression should be as expected.', () => { + for (const { code, result } of [ + { + code: `target('foo');`, + result: [true], + }, + { + code: `target(a);`, + result: [false], + }, + { + code: ` + const a = 'i' + target(a);`, + result: [true], + }, + { + code: ` + const a = b + target(a);`, + result: [false], + }, + { + code: ` + const a = a + target(a);`, + result: [false], + }, + { + code: ` + var a = 'foo' + var a = 'bar' + target(a);`, + result: [false], + }, + { + code: ` + var a = 'foo' + a = 'bar' + var b = 'bar' + target(a); + target(b);`, + result: [false, true], + }, + { + code: `target(\`foo\`);`, + result: [true], + }, + { + code: ` + target(\`foo\${a}\`);`, + result: [false], + }, + { + code: ` + const a = 'i' + target(\`foo\${a}\`);`, + result: [true], + }, + { + code: ` + const a = 'i' + target('foo' + 'bar'); + target(a + 'foo'); + target('foo' + a + 'bar'); + `, + result: [true, true, true], + }, + { + code: ` + const a = 'i' + target(b + 'bar'); + target('foo' + a + b); + `, + result: [false, false], + }, + { + code: ` + target(__dirname, __filename); + `, + result: [true, true], + }, + { + code: ` + function fn(__dirname) { + target(__dirname, __filename); + } + `, + result: [false, true], + }, + { + code: ` + const __filename = a + target(__dirname, __filename); + `, + result: [true, false], + }, + { + code: ` + import path from 'path'; + target(path.resolve(__dirname, './index.html')); + target(path.join(__dirname, './ssl.key')); + target(path.resolve(__dirname, './sitemap.xml')); + `, + result: [true, true, true], + }, + { + code: ` + import { posix as path } from 'path'; + target(path.resolve(__dirname, './index.html')); + `, + result: [true], + }, + { + code: ` + const path = require('path'); + target(path.resolve(__dirname, './index.html')); + `, + result: [true], + }, + { + code: ` + import path from 'unknown'; + target(path.resolve(__dirname, './index.html')); + `, + result: [false], + }, + { + code: ` + import path from 'path'; + target(path.unknown(__dirname, './index.html')); + `, + result: [false], + }, + { + code: ` + import path from 'path'; + target(path.resolve.unknown(__dirname, './index.html')); + `, + result: [false], + }, + { + code: ` + import path from 'path'; + const FOO = 'static' + target(path.resolve(__dirname, foo)); + target(path.resolve(__dirname, FOO)); + `, + result: [false, true], + }, + { + code: ` + import path from 'path'; + const FOO = 'static' + target(__dirname + path.sep + foo); + target(__dirname + path.sep + FOO); + `, + result: [false, true], + }, + { + code: ` + target(require.resolve('static')); + target(require.resolve(foo)); + `, + result: [true, false], + }, + { + code: ` + target(require); + target(require('static')); + `, + result: [false, false], + }, + { + code: ` + import url from "node:url"; + import path from "node:path"; + + const filename = url.fileURLToPath(import.meta.url); + const dirname = path.dirname(url.fileURLToPath(import.meta.url)); + + target(filename); + target(dirname); + `, + result: [true, true], + }, + { + code: ` + import url from "node:url"; + target(import.meta.url); + target(url.unknown(import.meta.url)); + `, + result: [true, false], + }, + ]) { + it(code, () => { + deepStrictEqual(getIsStaticExpressionResult(code), result); + }); + } + }); +}); diff --git a/utils/find-variable.js b/utils/find-variable.js new file mode 100644 index 0000000..b837cdb --- /dev/null +++ b/utils/find-variable.js @@ -0,0 +1,18 @@ +module.exports.findVariable = findVariable; + +/** + * Find the variable of a given name. + * @param {import("eslint").Scope.Scope} scope the scope to start finding + * @param {string} name the variable name to find. + * @returns {import("eslint").Scope.Variable | null} + */ +function findVariable(scope, name) { + while (scope != null) { + const variable = scope.set.get(name); + if (variable != null) { + return variable; + } + scope = scope.upper; + } + return null; +} diff --git a/utils/import-utils.js b/utils/import-utils.js index d812ed3..e10fb33 100644 --- a/utils/import-utils.js +++ b/utils/import-utils.js @@ -1,3 +1,5 @@ +const { findVariable } = require('./find-variable'); + module.exports.getImportAccessPath = getImportAccessPath; /** @@ -182,15 +184,3 @@ function getImportAccessPath({ node, scope, packageNames }) { return node && node.type === 'ImportDeclaration' && packageNames.includes(node.source.value); } } - -/** @returns {import("eslint").Scope.Variable | null} */ -function findVariable(scope, name) { - while (scope != null) { - const variable = scope.set.get(name); - if (variable != null) { - return variable; - } - scope = scope.upper; - } - return null; -} diff --git a/utils/is-static-expression.js b/utils/is-static-expression.js new file mode 100644 index 0000000..9ff8a96 --- /dev/null +++ b/utils/is-static-expression.js @@ -0,0 +1,219 @@ +const { findVariable } = require('./find-variable'); +const { getImportAccessPath } = require('./import-utils'); + +module.exports.isStaticExpression = isStaticExpression; + +const PATH_PACKAGE_NAMES = ['path', 'node:path', 'path/posix', 'node:path/posix']; +const URL_PACKAGE_NAMES = ['url', 'node:url']; +const PATH_CONSTRUCTION_METHOD_NAMES = new Set(['basename', 'dirname', 'extname', 'join', 'normalize', 'relative', 'resolve', 'toNamespacedPath']); +const PATH_STATIC_MEMBER_NAMES = new Set(['delimiter', 'sep']); + +/** + * @type {WeakMap} + */ +const cache = new WeakMap(); + +/** + * Checks whether the given expression node is a static or not. + * + * @param {Object} params + * @param {import("estree").Expression} params.node The node to check. + * @param {import("eslint").Scope.Scope} params.scope The scope of the given node. + * @returns {boolean} if true, the given expression node is a static. + */ +function isStaticExpression({ node, scope }) { + const tracked = new Set(); + return isStatic(node); + + /** + * @param {import("estree").Expression} node + * @returns {boolean} + */ + function isStatic(node) { + let result = cache.get(node); + if (result == null) { + result = isStaticWithoutCache(node); + cache.set(node, result); + } + return result; + } + /** + * @param {import("estree").Expression} node + * @returns {boolean} + */ + function isStaticWithoutCache(node) { + if (tracked.has(node)) { + // Guard infinite loops. + return false; + } + tracked.add(node); + if (node.type === 'Literal') { + return true; + } + if (node.type === 'TemplateLiteral') { + // A node is static if all interpolations are static. + return node.expressions.every(isStatic); + } + if (node.type === 'BinaryExpression') { + // An expression is static if both operands are static. + return isStatic(node.left) && isStatic(node.right); + } + if (node.type === 'Identifier') { + const variable = findVariable(scope, node.name); + if (variable) { + if (variable.defs.length === 0) { + if (node.name === '__dirname' || node.name === '__filename') { + // It is a global variable that can be used in CJS of Node.js. + return true; + } + } else if (variable.defs.length === 1) { + const def = variable.defs[0]; + if ( + def.type === 'Variable' && + // It has an initial value. + def.node.init && + // It does not write new values. + variable.references.every((ref) => ref.isReadOnly() || ref.identifier === def.name) + ) { + // A variable is static if its initial value is static. + return isStatic(def.node.init); + } + } + } else { + return false; + } + } + return isStaticPath(node) || isStaticFileURLToPath(node) || isStaticImportMetaUrl(node) || isStaticRequireResolve(node) || isStaticCwd(node); + } + + /** + * Checks whether the given expression is a static path construction. + * + * @param {import("estree").Expression} node The node to check. + * @returns {boolean} if true, the given expression is a static path construction. + */ + function isStaticPath(node) { + const pathInfo = getImportAccessPath({ + node: node.type === 'CallExpression' ? node.callee : node, + scope, + packageNames: PATH_PACKAGE_NAMES, + }); + if (!pathInfo) { + return false; + } + /** @type {string | undefined} */ + let name; + if (pathInfo.path.length === 1) { + // e.g. import path from 'path' + name = pathInfo.path[0]; + } else if (pathInfo.path.length === 2 && pathInfo.path[0] === 'posix') { + // e.g. import { posix as path } from 'path' + name = pathInfo.path[1]; + } + if (name == null) { + return false; + } + + if (node.type === 'CallExpression') { + if (!PATH_CONSTRUCTION_METHOD_NAMES.has(name)) { + return false; + } + return Boolean(node.arguments.length) && node.arguments.every(isStatic); + } + + return PATH_STATIC_MEMBER_NAMES.has(name); + } + + /** + * Checks whether the given expression is a static `url.fileURLToPath()`. + * + * @param {import("estree").Expression} node The node to check. + * @returns {boolean} if true, the given expression is a static `url.fileURLToPath()`. + */ + function isStaticFileURLToPath(node) { + if (node.type !== 'CallExpression') { + return false; + } + const pathInfo = getImportAccessPath({ + node: node.callee, + scope, + packageNames: URL_PACKAGE_NAMES, + }); + if (!pathInfo || pathInfo.path.length !== 1) { + return false; + } + let name = pathInfo.path[0]; + if (name !== 'fileURLToPath') { + return false; + } + return Boolean(node.arguments.length) && node.arguments.every(isStatic); + } + + /** + * Checks whether the given expression is an `import.meta.url`. + * + * @param {import("estree").Expression} node The node to check. + * @returns {boolean} if true, the given expression is an `import.meta.url`. + */ + function isStaticImportMetaUrl(node) { + return ( + node.type === 'MemberExpression' && + !node.computed && + node.property.type === 'Identifier' && + node.property.name === 'url' && + node.object.type === 'MetaProperty' && + node.object.meta.name === 'import' && + node.object.property.name === 'meta' + ); + } + + /** + * Checks whether the given expression is a static `require.resolve()`. + * + * @param {import("estree").Expression} node The node to check. + * @returns {boolean} if true, the given expression is a static `require.resolve()`. + */ + function isStaticRequireResolve(node) { + if ( + node.type !== 'CallExpression' || + node.callee.type !== 'MemberExpression' || + node.callee.computed || + node.callee.property.type !== 'Identifier' || + node.callee.property.name !== 'resolve' || + node.callee.object.type !== 'Identifier' || + node.callee.object.name !== 'require' + ) { + return false; + } + const variable = findVariable(scope, node.callee.object.name); + if (!variable || variable.defs.length !== 0) { + return false; + } + return Boolean(node.arguments.length) && node.arguments.every(isStatic); + } + + /** + * Checks whether the given expression is a static `process.cwd()`. + * + * @param {import("estree").Expression} node The node to check. + * @returns {boolean} if true, the given expression is a static `process.cwd()`. + */ + function isStaticCwd(node) { + if ( + node.type !== 'CallExpression' || + node.callee.type !== 'MemberExpression' || + node.callee.computed || + node.callee.property.type !== 'Identifier' || + node.callee.property.name !== 'cwd' || + node.callee.object.type !== 'Identifier' || + node.callee.object.name !== 'process' + ) { + return false; + } + const variable = findVariable(scope, node.callee.object.name); + if (!variable || variable.defs.length !== 0) { + return false; + } + return true; + } +}