Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: extend detect non literal fs filename #92

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

298 changes: 277 additions & 21 deletions rules/detect-non-literal-fs-filename.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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;
}
},
};
},
Expand Down
41 changes: 41 additions & 0 deletions rules/utils/import-utils.js
Original file line number Diff line number Diff line change
@@ -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
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you include some JSDoc comments for the parameters and return types?

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))
);
});