Skip to content

Commit 7c32cd8

Browse files
ota-meshiota-meshi
committed
fix(detect-non-literal-fs-filename, detect-child-process, detect-non-literal-regexp, detect-non-literal-require): false positives for static expressions
1 parent c54e618 commit 7c32cd8

12 files changed

+586
-44
lines changed

‎rules/detect-child-process.js‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
'use strict';
77

8+
const { isStaticExpression } = require('../utils/is-static-expression');
9+
810
//------------------------------------------------------------------------------
911
// Rule Definition
1012
//------------------------------------------------------------------------------
@@ -64,7 +66,15 @@ module.exports = {
6466
},
6567
MemberExpression: function (node) {
6668
if (node.property.name === 'exec' && childProcessIdentifiers.has(node.object)) {
67-
if (node.parent && node.parent.arguments && node.parent.arguments.length && node.parent.arguments[0].type !== 'Literal') {
69+
if (
70+
node.parent &&
71+
node.parent.arguments &&
72+
node.parent.arguments.length &&
73+
!isStaticExpression({
74+
node: node.parent.arguments[0],
75+
scope: context.getScope(),
76+
})
77+
) {
6878
return context.report({ node: node, message: 'Found child_process.exec() with non Literal first argument' });
6979
}
7080
}

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

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -10,21 +10,7 @@ const funcNames = Object.keys(fsMetaData);
1010
const fsPackageNames = ['fs', 'node:fs', 'fs/promises', 'node:fs/promises', 'fs-extra'];
1111

1212
const { getImportAccessPath } = require('../utils/import-utils');
13-
14-
//------------------------------------------------------------------------------
15-
// Utils
16-
//------------------------------------------------------------------------------
17-
18-
function getIndices(node, argMeta) {
19-
return (argMeta || []).filter((argIndex) => node.arguments[argIndex].type !== 'Literal');
20-
}
21-
22-
function generateReport({ context, node, packageName, methodName, indices }) {
23-
if (!indices || indices.length === 0) {
24-
return;
25-
}
26-
context.report({ node, message: `Found ${methodName} from package "${packageName}" with non literal argument at index ${indices.join(',')}` });
27-
}
13+
const { isStaticExpression, isStaticExpressionFast } = require('../utils/is-static-expression');
2814

2915
//------------------------------------------------------------------------------
3016
// Rule Definition
@@ -44,7 +30,7 @@ module.exports = {
4430
return {
4531
CallExpression: function (node) {
4632
// don't check require. If all arguments are Literals, it's surely safe!
47-
if ((node.callee.type === 'Identifier' && node.callee.name === 'require') || node.arguments.every((argument) => argument.type === 'Literal')) {
33+
if ((node.callee.type === 'Identifier' && node.callee.name === 'require') || node.arguments.every((argument) => isStaticExpressionFast(argument))) {
4834
return;
4935
}
5036

@@ -87,15 +73,23 @@ module.exports = {
8773
}
8874
const packageName = pathInfo.packageName;
8975

90-
const indices = getIndices(node, fsMetaData[fnName]);
91-
92-
generateReport({
93-
context,
94-
node,
95-
packageName,
96-
methodName: fnName,
97-
indices,
98-
});
76+
const indices = [];
77+
for (const index of fsMetaData[fnName] || []) {
78+
if (index >= node.arguments.length) {
79+
continue;
80+
}
81+
const argument = node.arguments[index];
82+
if (isStaticExpression({ node: argument, scope: context.getScope() })) {
83+
continue;
84+
}
85+
indices.push(index);
86+
}
87+
if (indices.length) {
88+
context.report({
89+
node,
90+
message: `Found ${fnName} from package "${packageName}" with non literal argument at index ${indices.join(',')}`,
91+
});
92+
}
9993
},
10094
};
10195
},

‎rules/detect-non-literal-regexp.js‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
'use strict';
77

8+
const { isStaticExpression } = require('../utils/is-static-expression');
9+
810
//------------------------------------------------------------------------------
911
// Rule Definition
1012
//------------------------------------------------------------------------------
@@ -24,7 +26,14 @@ module.exports = {
2426
NewExpression: function (node) {
2527
if (node.callee.name === 'RegExp') {
2628
const args = node.arguments;
27-
if (args && args.length > 0 && args[0].type !== 'Literal') {
29+
if (
30+
args &&
31+
args.length > 0 &&
32+
!isStaticExpression({
33+
node: args[0],
34+
scope: context.getScope(),
35+
})
36+
) {
2837
return context.report({ node: node, message: 'Found non-literal argument to RegExp Constructor' });
2938
}
3039
}

‎rules/detect-non-literal-require.js‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
'use strict';
77

8+
const { isStaticExpression } = require('../utils/is-static-expression');
9+
810
//------------------------------------------------------------------------------
911
// Rule Definition
1012
//------------------------------------------------------------------------------
@@ -25,8 +27,12 @@ module.exports = {
2527
if (node.callee.name === 'require') {
2628
const args = node.arguments;
2729
if (
28-
(args && args.length > 0 && args[0].type === 'TemplateLiteral' && args[0].expressions.length > 0) ||
29-
(args[0].type !== 'TemplateLiteral' && args[0].type !== 'Literal')
30+
args &&
31+
args.length > 0 &&
32+
!isStaticExpression({
33+
node: args[0],
34+
scope: context.getScope(),
35+
})
3036
) {
3137
return context.report({ node: node, message: 'Found non-literal argument in require' });
3238
}

‎test/detect-child-process.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ tester.run(ruleName, rule, {
1919
code: "var { spawn } = require('child_process'); spawn(str);",
2020
parserOptions: { ecmaVersion: 6 },
2121
},
22+
`
23+
var child_process = require('child_process');
24+
var FOO = 'ls';
25+
child_process.exec(FOO);`,
2226
],
2327
invalid: [
2428
{

‎test/detect-non-literal-fs-filename.js‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
const RuleTester = require('eslint').RuleTester;
44
const tester = new RuleTester({
55
parserOptions: {
6-
ecmaVersion: 6,
6+
ecmaVersion: 13,
77
sourceType: 'module',
88
},
99
});
@@ -24,6 +24,29 @@ tester.run(ruleName, require(`../rules/${ruleName}`), {
2424
code: `var something = require('fs').readFile, readFile = require('foo').readFile;
2525
readFile(c);`,
2626
},
27+
{
28+
code: `
29+
import { promises as fsp } from 'fs';
30+
import fs from 'fs';
31+
import path from 'path';
32+
33+
const index = await fsp.readFile(path.resolve(__dirname, './index.html'), 'utf-8');
34+
const key = fs.readFileSync(path.join(__dirname, './ssl.key'));
35+
await fsp.writeFile(path.resolve(__dirname, './sitemap.xml'), sitemap);`,
36+
globals: {
37+
__dirname: 'readonly',
38+
},
39+
},
40+
{
41+
code: `
42+
import fs from 'fs';
43+
import path from 'path';
44+
const dirname = path.dirname(__filename)
45+
const key = fs.readFileSync(path.resolve(dirname, './index.html'));`,
46+
globals: {
47+
__filename: 'readonly',
48+
},
49+
},
2750
],
2851
invalid: [
2952
/// requires
@@ -141,5 +164,15 @@ tester.run(ruleName, require(`../rules/${ruleName}`), {
141164
code: "var fs = require('fs');\nfunction foo () {\nvar { readFile: something } = fs.promises;\nsomething(filename);\n}",
142165
errors: [{ message: 'Found readFile from package "fs" with non literal argument at index 0' }],
143166
},
167+
{
168+
code: `
169+
import fs from 'fs';
170+
import path from 'path';
171+
const key = fs.readFileSync(path.resolve(__dirname, foo));`,
172+
globals: {
173+
__filename: 'readonly',
174+
},
175+
errors: [{ message: 'Found readFileSync from package "fs" with non literal argument at index 0' }],
176+
},
144177
],
145178
});

‎test/detect-non-literal-regexp.js‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,14 @@ const ruleName = 'detect-non-literal-regexp';
77
const invalid = "var a = new RegExp(c, 'i')";
88

99
tester.run(ruleName, require(`../rules/${ruleName}`), {
10-
valid: [{ code: "var a = new RegExp('ab+c', 'i')" }],
10+
valid: [
11+
{ code: "var a = new RegExp('ab+c', 'i')" },
12+
{
13+
code: `
14+
var source = 'ab+c'
15+
var a = new RegExp(source, 'i')`,
16+
},
17+
],
1118
invalid: [
1219
{
1320
code: invalid,

‎test/detect-non-literal-require.js‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,15 @@ const tester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });
77
const ruleName = 'detect-non-literal-require';
88

99
tester.run(ruleName, require(`../rules/${ruleName}`), {
10-
valid: [{ code: "var a = require('b')" }, { code: 'var a = require(`b`)' }],
10+
valid: [
11+
{ code: "var a = require('b')" },
12+
{ code: 'var a = require(`b`)' },
13+
{
14+
code: `
15+
const d = 'debounce'
16+
var a = require(\`lodash/\${d}\`)`,
17+
},
18+
],
1119
invalid: [
1220
{
1321
code: 'var a = require(c)',

0 commit comments

Comments
 (0)