Custom linter to check code quality (#2753)
This commit is contained in:
Scott Nonnenberg
GitHub
parent
366401f77a
commit
ecb126e74c
13 changed files with 7467 additions and 66 deletions
29
ts/util/lint/analyze_exceptions.ts
Normal file
29
ts/util/lint/analyze_exceptions.ts
Normal file
|
|
@ -0,0 +1,29 @@
|
|||
// tslint:disable no-console
|
||||
|
||||
import { join } from 'path';
|
||||
|
||||
import { fromPairs, groupBy, map } from 'lodash';
|
||||
|
||||
import { ExceptionType } from './types';
|
||||
import { loadJSON } from './util';
|
||||
|
||||
const exceptionsPath = join(__dirname, 'exceptions.json');
|
||||
const exceptions: Array<ExceptionType> = loadJSON(exceptionsPath);
|
||||
const byRule = groupBy(exceptions, 'rule');
|
||||
|
||||
const byRuleThenByCategory = fromPairs(
|
||||
map(byRule, (list, ruleName) => {
|
||||
const byCategory = groupBy(list, 'reasonCategory');
|
||||
|
||||
return [
|
||||
ruleName,
|
||||
fromPairs(
|
||||
map(byCategory, (innerList, categoryName) => {
|
||||
return [categoryName, innerList.length];
|
||||
})
|
||||
),
|
||||
];
|
||||
})
|
||||
);
|
||||
|
||||
console.log(JSON.stringify(byRuleThenByCategory, null, ' '));
|
||||
6715
ts/util/lint/exceptions.json
Normal file
6715
ts/util/lint/exceptions.json
Normal file
File diff suppressed because it is too large
Load diff
265
ts/util/lint/linter.ts
Normal file
265
ts/util/lint/linter.ts
Normal file
|
|
@ -0,0 +1,265 @@
|
|||
// tslint:disable no-console
|
||||
|
||||
import { readFileSync } from 'fs';
|
||||
import { join, relative } from 'path';
|
||||
|
||||
// @ts-ignore
|
||||
import glob from 'glob';
|
||||
import { forEach, some, values } from 'lodash';
|
||||
|
||||
import { ExceptionType, REASONS, RuleType } from './types';
|
||||
import { ENCODING, loadJSON } from './util';
|
||||
|
||||
const ALL_REASONS = REASONS.join('|');
|
||||
const now = new Date();
|
||||
|
||||
function getExceptionKey(exception: any) {
|
||||
return `${exception.rule}-${exception.path}-${exception.lineNumber}`;
|
||||
}
|
||||
|
||||
function createLookup(list: Array<any>) {
|
||||
const lookup = Object.create(null);
|
||||
|
||||
forEach(list, exception => {
|
||||
const key = getExceptionKey(exception);
|
||||
|
||||
if (lookup[key]) {
|
||||
throw new Error(`Duplicate exception found for key ${key}`);
|
||||
}
|
||||
|
||||
lookup[key] = exception;
|
||||
});
|
||||
|
||||
return lookup;
|
||||
}
|
||||
|
||||
const rulesPath = join(__dirname, 'rules.json');
|
||||
const exceptionsPath = join(__dirname, 'exceptions.json');
|
||||
const basePath = join(__dirname, '../../..');
|
||||
|
||||
const searchPattern = join(basePath, '**/*.{js,ts,tsx}');
|
||||
|
||||
const rules: Array<RuleType> = loadJSON(rulesPath);
|
||||
const exceptions: Array<ExceptionType> = loadJSON(exceptionsPath);
|
||||
const exceptionsLookup = createLookup(exceptions);
|
||||
let scannedCount = 0;
|
||||
|
||||
const allSourceFiles = glob.sync(searchPattern, { nodir: true });
|
||||
|
||||
const results: Array<ExceptionType> = [];
|
||||
|
||||
const excludedFiles = [
|
||||
// Generated files
|
||||
'^js/components.js',
|
||||
'^js/libtextsecure.js',
|
||||
'^js/util_worker.js',
|
||||
'^libtextsecure/components.js',
|
||||
'^libtextsecure/test/test.js',
|
||||
'^test/test.js',
|
||||
|
||||
// From libsignal-protocol-javascript project
|
||||
'^js/libsignal-protocol-worker.js',
|
||||
'^libtextsecure/libsignal-protocol.js',
|
||||
|
||||
// Copied from dependency
|
||||
'^js/Mp3LameEncoder.min.js',
|
||||
|
||||
// Test files
|
||||
'^libtextsecure/test/*',
|
||||
'^test/*',
|
||||
|
||||
// Modules used only in test/development scenarios
|
||||
'^node_modules/@types/*',
|
||||
'^node_modules/ajv/*',
|
||||
'^node_modules/amdefine/*',
|
||||
'^node_modules/anymatch/*',
|
||||
'^node_modules/asn1\\.js/*',
|
||||
'^node_modules/autoprefixer/*',
|
||||
'^node_modules/babel*',
|
||||
'^node_modules/bluebird/*',
|
||||
'^node_modules/body-parser/*',
|
||||
'^node_modules/bower/*',
|
||||
'^node_modules/buble/*',
|
||||
'^node_modules/chai/*',
|
||||
'^node_modules/cli-table2/*',
|
||||
'^node_modules/codemirror/*',
|
||||
'^node_modules/coffee-script/*',
|
||||
'^node_modules/compression/*',
|
||||
'^node_modules/degenerator/*',
|
||||
'^node_modules/detect-port-alt/*',
|
||||
'^node_modules/electron-builder/*',
|
||||
'^node_modules/electron-icon-maker/*',
|
||||
'^node_modules/electron-osx-sign/*',
|
||||
'^node_modules/electron-publish/*',
|
||||
'^node_modules/escodegen/*',
|
||||
'^node_modules/eslint*',
|
||||
'^node_modules/esprima/*',
|
||||
'^node_modules/express/*',
|
||||
'^node_modules/extract-zip/*',
|
||||
'^node_modules/finalhandler/*',
|
||||
'^node_modules/fsevents/*',
|
||||
'^node_modules/globule/*',
|
||||
'^node_modules/grunt*',
|
||||
'^node_modules/handle-thing/*',
|
||||
'^node_modules/har-validator/*',
|
||||
'^node_modules/highlight\\.js/*',
|
||||
'^node_modules/hpack\\.js/*',
|
||||
'^node_modules/http-proxy-middlewar/*',
|
||||
'^node_modules/icss-utils/*',
|
||||
'^node_modules/intl-tel-input/examples/*',
|
||||
'^node_modules/istanbul*',
|
||||
'^node_modules/jimp/*',
|
||||
'^node_modules/jquery/*',
|
||||
'^node_modules/jss/*',
|
||||
'^node_modules/jss-global/*',
|
||||
'^node_modules/livereload-js/*',
|
||||
'^node_modules/lolex/*',
|
||||
'^node_modules/magic-string/*',
|
||||
'^node_modules/mocha/*',
|
||||
'^node_modules/minimatch/*',
|
||||
'^node_modules/nise/*',
|
||||
'^node_modules/node-sass-import-once/*',
|
||||
'^node_modules/node-sass/*',
|
||||
'^node_modules/nsp/*',
|
||||
'^node_modules/nyc/*',
|
||||
'^node_modules/phantomjs-prebuilt/*',
|
||||
'^node_modules/postcss*',
|
||||
'^node_modules/preserve/*',
|
||||
'^node_modules/prettier/*',
|
||||
'^node_modules/protobufjs/cli/*',
|
||||
'^node_modules/ramda/*',
|
||||
'^node_modules/react-docgen/*',
|
||||
'^node_modules/react-error-overlay/*',
|
||||
'^node_modules/react-styleguidist/*',
|
||||
'^node_modules/recast/*',
|
||||
'^node_modules/reduce-css-calc/*',
|
||||
'^node_modules/resolve/*',
|
||||
'^node_modules/sass-graph/*',
|
||||
'^node_modules/scss-tokenizer/*',
|
||||
'^node_modules/send/*',
|
||||
'^node_modules/serve-index/*',
|
||||
'^node_modules/sinon/*',
|
||||
'^node_modules/snapdragon-util/*',
|
||||
'^node_modules/snapdragon/*',
|
||||
'^node_modules/sockjs-client/*',
|
||||
'^node_modules/spectron/*',
|
||||
'^node_modules/style-loader/*',
|
||||
'^node_modules/svgo/*',
|
||||
'^node_modules/testcheck/*',
|
||||
'^node_modules/text-encoding/*',
|
||||
'^node_modules/tinycolor2/*',
|
||||
'^node_modules/to-ast/*',
|
||||
'^node_modules/trough/*',
|
||||
'^node_modules/ts-loader/*',
|
||||
'^node_modules/tslint*',
|
||||
'^node_modules/tweetnacl/*',
|
||||
'^node_modules/typescript/*',
|
||||
'^node_modules/uglify-es/*',
|
||||
'^node_modules/uglify-js/*',
|
||||
'^node_modules/use/*',
|
||||
'^node_modules/vary/*',
|
||||
'^node_modules/vm-browserify/*',
|
||||
'^node_modules/webdriverio/*',
|
||||
'^node_modules/webpack*',
|
||||
'^node_modules/xmldom/*',
|
||||
'^node_modules/xml-parse-from-string/*',
|
||||
];
|
||||
|
||||
function setupRules(allRules: Array<RuleType>) {
|
||||
forEach(allRules, (rule, index) => {
|
||||
if (!rule.name) {
|
||||
throw new Error(`Rule at index ${index} is missing a name`);
|
||||
}
|
||||
|
||||
if (!rule.expression) {
|
||||
throw new Error(`Rule '${rule.name}' is missing an expression`);
|
||||
}
|
||||
|
||||
rule.regex = new RegExp(rule.expression, 'g');
|
||||
});
|
||||
}
|
||||
|
||||
setupRules(rules);
|
||||
|
||||
forEach(allSourceFiles, file => {
|
||||
const relativePath = relative(basePath, file).replace(/\\/g, '/');
|
||||
if (
|
||||
some(excludedFiles, excluded => {
|
||||
const regex = new RegExp(excluded);
|
||||
|
||||
return regex.test(relativePath);
|
||||
})
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
scannedCount += 1;
|
||||
|
||||
const fileContents = readFileSync(file, ENCODING);
|
||||
const lines = fileContents.split('\n');
|
||||
|
||||
forEach(rules, (rule: RuleType) => {
|
||||
const excludedModules = rule.excludedModules || [];
|
||||
if (some(excludedModules, module => relativePath.startsWith(module))) {
|
||||
return;
|
||||
}
|
||||
|
||||
forEach(lines, (rawLine, lineIndex) => {
|
||||
const line = rawLine.replace(/\r/g, '');
|
||||
if (!rule.regex.test(line)) {
|
||||
return;
|
||||
}
|
||||
|
||||
const path = relativePath;
|
||||
const lineNumber = lineIndex + 1;
|
||||
|
||||
const exceptionKey = getExceptionKey({
|
||||
rule: rule.name,
|
||||
path: relativePath,
|
||||
lineNumber,
|
||||
});
|
||||
|
||||
const exception = exceptionsLookup[exceptionKey];
|
||||
if (exception && (!exception.line || exception.line === line)) {
|
||||
delete exceptionsLookup[exceptionKey];
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
results.push({
|
||||
rule: rule.name,
|
||||
path,
|
||||
line: line.length < 300 ? line : undefined,
|
||||
lineNumber,
|
||||
reasonCategory: ALL_REASONS,
|
||||
updated: now.toJSON(),
|
||||
reasonDetail: '<optional>',
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
const unusedExceptions = values(exceptionsLookup);
|
||||
|
||||
console.log(
|
||||
`${scannedCount} files scanned.`,
|
||||
`${results.length} questionable lines,`,
|
||||
`${unusedExceptions.length} unused exceptions,`,
|
||||
`${exceptions.length} total exceptions.`
|
||||
);
|
||||
|
||||
if (results.length === 0 && unusedExceptions.length === 0) {
|
||||
process.exit();
|
||||
}
|
||||
|
||||
console.log();
|
||||
console.log('Questionable lines:');
|
||||
console.log(JSON.stringify(results, null, ' '));
|
||||
|
||||
if (unusedExceptions.length) {
|
||||
console.log();
|
||||
console.log('Unused exceptions!');
|
||||
console.log(JSON.stringify(unusedExceptions, null, ' '));
|
||||
}
|
||||
|
||||
process.exit(1);
|
||||
173
ts/util/lint/rules.json
Normal file
173
ts/util/lint/rules.json
Normal file
|
|
@ -0,0 +1,173 @@
|
|||
[
|
||||
{
|
||||
"name": "eval",
|
||||
"expression": "\\beval\\(",
|
||||
"reason": "Arbitrary code execution"
|
||||
},
|
||||
{
|
||||
"name": "DOM-innerHTML",
|
||||
"expression": "\\binnerHTML\\b",
|
||||
"reason": "Potential XSS"
|
||||
},
|
||||
{
|
||||
"name": "DOM-outerHTML",
|
||||
"expression": "\\bouterHTML\\b",
|
||||
"reason": "Potential XSS"
|
||||
},
|
||||
{
|
||||
"name": "DOM-document.write(",
|
||||
"expression": "\\bdocument.write(ln)?\\(",
|
||||
"reason": "Potential XSS"
|
||||
},
|
||||
{
|
||||
"name": "jQuery-$(",
|
||||
"expression": "\\$\\(",
|
||||
"reason": "Potential XSS",
|
||||
"excludedModules": ["node_modules/prelude-ls"]
|
||||
},
|
||||
{
|
||||
"name": "jQuery-html(",
|
||||
"expression": "\\bhtml\\(",
|
||||
"reason": "Potential XSS"
|
||||
},
|
||||
{
|
||||
"name": "jQuery-append(",
|
||||
"expression": "\\bappend\\(",
|
||||
"reason": "Potential XSS",
|
||||
"excludedModules": [
|
||||
"components/bytebuffer",
|
||||
"components/protobuf",
|
||||
"node_modules/google-libphonenumber",
|
||||
"node_modules/handlebars"
|
||||
]
|
||||
},
|
||||
{
|
||||
"name": "jQuery-appendTo(",
|
||||
"expression": "\\bappendTo\\(",
|
||||
"reason": "Potential XSS"
|
||||
},
|
||||
{
|
||||
"name": "jQuery-insertAfter(",
|
||||
"expression": "\\binsertAfter\\(",
|
||||
"reason": "Potential XSS"
|
||||
},
|
||||
{
|
||||
"name": "jQuery-insertBefore(",
|
||||
"expression": "\\binsertBefore\\(",
|
||||
"reason": "Potential XSS",
|
||||
"excludedModules": ["node_modules/react-dom"]
|
||||
},
|
||||
{
|
||||
"name": "jQuery-prepend(",
|
||||
"expression": "\\bprepend\\(",
|
||||
"reason": "Potential XSS",
|
||||
"excludedModules": ["components/bytebuffer", "node_modules/handlebars"]
|
||||
},
|
||||
{
|
||||
"name": "jQuery-prependTo(",
|
||||
"expression": "\\bprependTo\\(",
|
||||
"reason": "Potential XSS"
|
||||
},
|
||||
{
|
||||
"name": "jQuery-wrap(",
|
||||
"expression": "\\bwrap\\(",
|
||||
"reason": "Potential XSS",
|
||||
"excludedModules": [
|
||||
"components/bytebuffer",
|
||||
"components/protobuf",
|
||||
"node_modules/handlebars",
|
||||
"node_modules/lodash"
|
||||
]
|
||||
},
|
||||
{
|
||||
"name": "jQuery-wrapInner(",
|
||||
"expression": "\\bwrapInner\\(",
|
||||
"reason": "Potential XSS"
|
||||
},
|
||||
{
|
||||
"name": "jQuery-wrapAll(",
|
||||
"expression": "\\bwrapAll\\(",
|
||||
"reason": "Potential XSS"
|
||||
},
|
||||
{
|
||||
"name": "jQuery-before(",
|
||||
"expression": "\\bbefore\\(",
|
||||
"reason": "Potential XSS"
|
||||
},
|
||||
{
|
||||
"name": "jQuery-after(",
|
||||
"expression": "\\bafter\\(",
|
||||
"reason": "Potential XSS"
|
||||
},
|
||||
{
|
||||
"name": "jQuery-globalEval(",
|
||||
"expression": "\\bglobalEval\\(",
|
||||
"reason": "Arbitrary code execution"
|
||||
},
|
||||
{
|
||||
"name": "jQuery-getScript(",
|
||||
"expression": "\\bgetScript\\(",
|
||||
"reason": "Arbitrary code execution"
|
||||
},
|
||||
{
|
||||
"name": "jQuery-load(",
|
||||
"expression": "\\bload\\(",
|
||||
"reason": "Arbitrary code execution"
|
||||
},
|
||||
{
|
||||
"name": "React-ref",
|
||||
"expression": "\\bref(\\s)*=\\b",
|
||||
"reason": "Potential XSS",
|
||||
"excludedModules": [
|
||||
"node_modules/react-dom",
|
||||
"node_modules/tslint-microsoft-contrib",
|
||||
"node_modules/react-error-overlay",
|
||||
"node_modules/react-styleguidist"
|
||||
]
|
||||
},
|
||||
{
|
||||
"name": "React-createRef",
|
||||
"expression": "\\bcreateRef\\(",
|
||||
"reason": "Potential XSS",
|
||||
"excludedModules": [
|
||||
"node_modules/react-dom",
|
||||
"node_modules/tslint-microsoft-contrib",
|
||||
"node_modules/react-error-overlay",
|
||||
"node_modules/react-styleguidist"
|
||||
]
|
||||
},
|
||||
{
|
||||
"name": "React-findDOMNode",
|
||||
"expression": "\\bfindDOMNode\\(",
|
||||
"reason": "Potential XSS",
|
||||
"excludedModules": [
|
||||
"node_modules/react-dom",
|
||||
"node_modules/tslint-microsoft-contrib",
|
||||
"node_modules/react-error-overlay",
|
||||
"node_modules/react-styleguidist"
|
||||
]
|
||||
},
|
||||
{
|
||||
"name": "React-dangerouslySetInnerHTML",
|
||||
"expression": "\\bdangerouslySetInnerHTML\\b",
|
||||
"reason": "Potential XSS",
|
||||
"excludedModules": [
|
||||
"node_modules/react-dom",
|
||||
"node_modules/tslint-microsoft-contrib",
|
||||
"node_modules/react-error-overlay",
|
||||
"node_modules/react-styleguidist"
|
||||
]
|
||||
},
|
||||
{
|
||||
"name": "fbjs-createNodesFromMarkup",
|
||||
"expression": "\\bcreateNodesFromMarkup\\b",
|
||||
"reason": "Potential XSS, pipes input to innerHTML",
|
||||
"excludedModules": ["node_modules/react-dom", "node_modules/fbjs"]
|
||||
},
|
||||
{
|
||||
"name": "thenify-multiArgs",
|
||||
"expression": "\\bmultiArgs\\b",
|
||||
"reason": "Potential arbitrary code execution, piped to eval",
|
||||
"excludedModules": ["node_modules/thenify"]
|
||||
}
|
||||
]
|
||||
14
ts/util/lint/sort_exceptions.ts
Normal file
14
ts/util/lint/sort_exceptions.ts
Normal file
|
|
@ -0,0 +1,14 @@
|
|||
// tslint:disable no-console
|
||||
|
||||
import { join } from 'path';
|
||||
import { writeFileSync } from 'fs';
|
||||
|
||||
import { ExceptionType } from './types';
|
||||
import { loadJSON, sortExceptions } from './util';
|
||||
|
||||
const exceptionsPath = join(__dirname, 'exceptions.json');
|
||||
const exceptions: Array<ExceptionType> = loadJSON(exceptionsPath);
|
||||
|
||||
const sorted = sortExceptions(exceptions);
|
||||
|
||||
writeFileSync(exceptionsPath, JSON.stringify(sorted, null, ' '));
|
||||
63
ts/util/lint/types.ts
Normal file
63
ts/util/lint/types.ts
Normal file
|
|
@ -0,0 +1,63 @@
|
|||
// Tool requirements:
|
||||
// - Feed it a set of regular expressions with descriptions as to what the risks are
|
||||
// - Feed it also a set of exceptions
|
||||
// - It would tell us if there were any new matches that didn't already have exceptions
|
||||
//
|
||||
// Rules:
|
||||
// {
|
||||
// "name": "rule-name",
|
||||
// "expression": "^regex-as-string$",
|
||||
// "reason": "Reason that this expression is dangerous"
|
||||
// }
|
||||
//
|
||||
// Categories of reasons - low to high risk:
|
||||
// "falseMatch"
|
||||
// "testCode"
|
||||
// "exampleCode"
|
||||
// "otherUtilityCode"
|
||||
// "regexMatchedSafeCode"
|
||||
// "notExercisedByOurApp"
|
||||
// "ruleNeeded"
|
||||
// "usageTrusted"
|
||||
//
|
||||
// Exceptions:
|
||||
// [{
|
||||
// "rule": "rule-name",
|
||||
// "path": "path/to/filename.js",
|
||||
// "lineNumber": 45,
|
||||
// "reasonCategory": "<category from list above>",
|
||||
// "updated": "2018-09-08T00:21:13.180Z",
|
||||
// "reasonDetail": "<Optional additional information about why this is okay>"
|
||||
// }]
|
||||
//
|
||||
// When the tool finds issues it outputs them in exception format to make it easy to add
|
||||
// to the exceptions.json file
|
||||
|
||||
export const REASONS = [
|
||||
'falseMatch',
|
||||
'testCode',
|
||||
'exampleCode',
|
||||
'otherUtilityCode',
|
||||
'regexMatchedSafeCode',
|
||||
'notExercisedByOurApp',
|
||||
'ruleNeeded',
|
||||
'usageTrusted',
|
||||
];
|
||||
|
||||
export type RuleType = {
|
||||
name: string;
|
||||
expression: string | null;
|
||||
reason: string;
|
||||
regex: RegExp;
|
||||
excludedModules: Array<string> | null;
|
||||
};
|
||||
|
||||
export type ExceptionType = {
|
||||
rule: string;
|
||||
path: string;
|
||||
line?: string;
|
||||
lineNumber: number;
|
||||
reasonCategory: string;
|
||||
updated: string;
|
||||
reasonDetail: string;
|
||||
};
|
||||
24
ts/util/lint/util.ts
Normal file
24
ts/util/lint/util.ts
Normal file
|
|
@ -0,0 +1,24 @@
|
|||
// tslint:disable no-console
|
||||
|
||||
import { readFileSync } from 'fs';
|
||||
|
||||
import { orderBy } from 'lodash';
|
||||
|
||||
import { ExceptionType } from './types';
|
||||
|
||||
export const ENCODING = 'utf8';
|
||||
|
||||
export function loadJSON(target: string) {
|
||||
try {
|
||||
const contents = readFileSync(target, ENCODING);
|
||||
|
||||
return JSON.parse(contents);
|
||||
} catch (error) {
|
||||
console.log(`Error loading JSON from ${target}: ${error.stack}`);
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
export function sortExceptions(exceptions: Array<ExceptionType>) {
|
||||
return orderBy(exceptions, ['path', 'lineNumber', 'rule']);
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue