From 2d763934335fd50a6c7f8d6b4a276c3b236c51d8 Mon Sep 17 00:00:00 2001 From: Grace Date: Fri, 3 Jun 2022 10:14:44 -0700 Subject: [PATCH] @W-10127077@: adds --verbose-violations flag to provide more verbose output for retire-js --- messages/run.js | 5 +- src/Constants.ts | 3 +- src/commands/scanner/run.ts | 11 ++++ src/lib/retire-js/RetireJsEngine.ts | 41 +++++++++++++-- test/lib/retire-js/RetireJsEngine.test.ts | 63 +++++++++++++++++++++-- 5 files changed, 112 insertions(+), 11 deletions(-) diff --git a/messages/run.js b/messages/run.js index 9ad1aa43a..b80fdd72e 100644 --- a/messages/run.js +++ b/messages/run.js @@ -29,7 +29,10 @@ module.exports = { 'eslintConfigDescription': 'location of eslintrc config to customize eslint engine', 'eslintConfigDescriptionLong': 'Location of eslintrc to customize eslint engine', 'pmdConfigDescription': 'location of PMD rule reference XML file to customize rule selection', - 'pmdConfigDescriptionLong': 'Location of PMD rule reference XML file to customize rule selection' + 'pmdConfigDescriptionLong': 'Location of PMD rule reference XML file to customize rule selection', + "verboseViolationsDescription": "retire-js violation messages include more details", + "verboseViolationsDescriptionLong": "retire-js violation messages contain details about each vulnerability (e.g. summary, CVE, urls, etc.)" + }, "validations": { "outfileFormatMismatch": "Your chosen format %s does not appear to match your output file type of %s.", diff --git a/src/Constants.ts b/src/Constants.ts index e874040d8..e9f153217 100644 --- a/src/Constants.ts +++ b/src/Constants.ts @@ -94,7 +94,8 @@ export const Services = { export enum CUSTOM_CONFIG { EslintConfig = "EslintConfig", PmdConfig = "PmdConfig", - SfgeConfig = "SfgeConfig" + SfgeConfig = "SfgeConfig", + VerboseViolations = "VerboseViolations" } export const HARDCODED_RULES = { diff --git a/src/commands/scanner/run.ts b/src/commands/scanner/run.ts index 0ebe3e51f..28c56dc88 100644 --- a/src/commands/scanner/run.ts +++ b/src/commands/scanner/run.ts @@ -104,6 +104,10 @@ export default class Run extends ScannerRunCommand { description: messages.getMessage('flags.nsDescription'), longDescription: messages.getMessage('flags.nsDescriptionLong') }), + "verbose-violations": flags.boolean({ + description: messages.getMessage('flags.verboseViolationsDescription'), + longDescription: messages.getMessage('flags.verboseViolationsDescriptionLong') + }), }; protected validateCommandFlags(): Promise { @@ -149,6 +153,13 @@ export default class Run extends ScannerRunCommand { const pmdConfig = normalize(untildify(this.flags.pmdconfig as string)); options.set(CUSTOM_CONFIG.PmdConfig, pmdConfig); } + + // Capturing verbose-violations flag value (used for RetireJS output) + if (this.flags["verbose-violations"]) { + options.set(CUSTOM_CONFIG.VerboseViolations, "true"); + } + + return options; } diff --git a/src/lib/retire-js/RetireJsEngine.ts b/src/lib/retire-js/RetireJsEngine.ts index 88b9204de..87d0875eb 100644 --- a/src/lib/retire-js/RetireJsEngine.ts +++ b/src/lib/retire-js/RetireJsEngine.ts @@ -10,6 +10,8 @@ import * as engineUtils from '../util/CommonEngineUtils'; import cspawn = require('cross-spawn'); import path = require('path'); import StreamZip = require('node-stream-zip'); +import {CUSTOM_CONFIG} from '../../Constants'; + // Unlike the other engines we use, RetireJS doesn't really have "rules" per se. So we sorta have to synthesize a @@ -49,6 +51,8 @@ export type RetireJsInvocation = { */ type RetireJsVulnerability = { severity: string; + identifiers?: { [name: string]: string }; + info?: string[]; }; type RetireJsResult = { @@ -172,7 +176,7 @@ export class RetireJsEngine extends AbstractRuleEngine { const retireJsPromises: Promise[] = []; for (const invocation of invocationArray) { - retireJsPromises.push(this.executeRetireJs(invocation)); + retireJsPromises.push(this.executeRetireJs(invocation, engineOptions.has(CUSTOM_CONFIG.VerboseViolations))); } // We can combine the results into a single array using .reduce() instead of the more verbose for-loop. @@ -199,7 +203,7 @@ export class RetireJsEngine extends AbstractRuleEngine { return invocationArray; } - private async executeRetireJs(invocation: RetireJsInvocation): Promise { + private async executeRetireJs(invocation: RetireJsInvocation, verboseViolations: boolean): Promise { return new Promise((res, rej) => { const cp = cspawn(RetireJsEngine.NODE_EXEC_PATH, invocation.args); @@ -225,7 +229,8 @@ export class RetireJsEngine extends AbstractRuleEngine { } else if (code === 13) { // If RetireJS exits with code 13, then it ran successfully, but found at least one vulnerability. // Convert the output into RuleResult objects and resolve to that. - res(this.processOutput(stdout, invocation.rule)); + res(this.processOutput(stdout, invocation.rule, verboseViolations)); + } else { // If RetireJS exits with any other code, then it means something went wrong. The error could be // contained in either stdout or stderr, so we'll send them both to a method for processing, and @@ -261,7 +266,7 @@ export class RetireJsEngine extends AbstractRuleEngine { return stderr; } - private processOutput(cmdOutput: string, ruleName: string): RuleResult[] { + private processOutput(cmdOutput: string, ruleName: string, verboseViolations:boolean): RuleResult[] { // The output should be a valid result JSON. try { const outputJson: RetireJsOutput = RetireJsEngine.convertStringToResultObj(cmdOutput); @@ -282,6 +287,9 @@ export class RetireJsEngine extends AbstractRuleEngine { // Each `result` entry generates one RuleViolation. for (const result of data.results) { + + const message: string = verboseViolations ? this.generateVerboseMessage(result) : `${result.component} v${result.version} is insecure. Please upgrade to latest version.`; + ruleResult.violations.push({ line: 1, column: 1, @@ -290,7 +298,7 @@ export class RetireJsEngine extends AbstractRuleEngine { severity: result.vulnerabilities .map(vuln => this.retireSevToScannerSev(vuln.severity)) .reduce((min, sev) => min > sev ? sev: min, 9000), - message: `${result.component} v${result.version} is insecure. Please upgrade to latest version.`, + message: message, category: 'Insecure Dependencies' }); } @@ -307,6 +315,29 @@ export class RetireJsEngine extends AbstractRuleEngine { } } + private generateVerboseMessage(result: RetireJsResult): string { + const messageLines: string[] = [`${result.component} ${result.version} has known vulnerabilities:`]; + + // Each `vulnerability` generates a new line in the RuleViolation's message + for (const vuln of result.vulnerabilities) { + + // Array of all identifiers with starting with severity and summary (if applicable) + const vulnMessageItems: string[] = []; + + for (const identifier in vuln.identifiers) { + const text = `${identifier}: ${vuln.identifiers[identifier]}`; + identifier === "summary" ? vulnMessageItems.unshift(text) : vulnMessageItems.push(text); + } + + vulnMessageItems.unshift(`severity: ${vuln.severity}`); // unshift after other identifiers so severity is first + vulnMessageItems.push(vuln.info.join(" ")); // list info elements separated by space + messageLines.push(`${vulnMessageItems.join("; ")}`) + + } + + return messageLines.join("\n"); + } + private retireSevToScannerSev(sev: string): number { switch (sev.toLowerCase()) { case 'low': diff --git a/test/lib/retire-js/RetireJsEngine.test.ts b/test/lib/retire-js/RetireJsEngine.test.ts index b61a222b3..5067a0bba 100644 --- a/test/lib/retire-js/RetireJsEngine.test.ts +++ b/test/lib/retire-js/RetireJsEngine.test.ts @@ -301,7 +301,7 @@ describe('RetireJsEngine', () => { }; // THIS IS THE ACTUAL METHOD BEING TESTED: Now we feed that fake result into the engine and see what we get back. - const results: RuleResult[] = (testEngine as any).processOutput(JSON.stringify(fakeRetireOutput), 'insecure-bundled-dependencies'); + const results: RuleResult[] = (testEngine as any).processOutput(JSON.stringify(fakeRetireOutput), 'insecure-bundled-dependencies', false); // Now we run our assertions. expect(results.length).to.equal(2, 'Should be two result objects because of the two spoofed files.'); @@ -314,6 +314,61 @@ describe('RetireJsEngine', () => { expect(results[1].violations[1].severity).to.equal(3, 'Sev should be translated to 3'); }); + it('Properly generates message for --verbose-violations', async () => { + // First, we need to seed the test engine with some fake aliases. + const firstOriginal = path.join('first', 'unimportant', 'path', 'jquery-3.1.0.js'); + const firstAlias = path.join('first', 'unimportant', 'alias', 'jquery-3.1.0.js'); + const secondOriginal = path.join('first', 'unimportant', 'path', 'angular-scenario.js'); + const secondAlias = path.join('first', 'unimportant', 'alias', 'angular-scenario.js'); + + (testEngine as any).originalFilesByAlias.set(firstAlias, firstOriginal); + (testEngine as any).originalFilesByAlias.set(secondAlias, secondOriginal); + + // Next, we want to spoof some output that looks like it came from RetireJS. + const fakeRetireResult = { + "version": "3.1.0", + "component": "jquery", + "vulnerabilities": [{ + "below": "3.4.0", + "identifiers": { + "CVE": ["CVE-2019-11358"], + "summary": "summary one", + "random": "this could be anything" + }, + "info": [ + 'https://blog.jquery.com/2019/04/10/jquery-3-4-0-released/', + 'https://nvd.nist.gov/vuln/detail/CVE-2019-11358' + ], + "severity": "medium" + }, { + "below": "3.5.0", + "identifiers": { + "summary": "summary two" + }, + "info": [ + 'https://blog.jquery.com/2020/04/10/jquery-3-5-0-released/' + ], + "severity": "medium" + }, { + "below": "3.6.0", + "identifiers": { + "CVE": ["CVE-2020-11111"], + }, + "info": [ + 'https://blog.jquery.com/2020/04/10/jquery-3-5-0-released/' + ], + "severity": "low" + }] + } + + // THIS IS THE ACTUAL METHOD BEING TESTED: Now we feed that fake result into the engine and see what we get back. + const message: string = (testEngine as any).generateVerboseMessage(fakeRetireResult, 'insecure-bundled-dependencies', true); + + // Now we run our assertions. + expect(message).to.equal("jquery 3.1.0 has known vulnerabilities:\nseverity: medium; summary: summary one; CVE: CVE-2019-11358; random: this could be anything; https://blog.jquery.com/2019/04/10/jquery-3-4-0-released/ https://nvd.nist.gov/vuln/detail/CVE-2019-11358\nseverity: medium; summary: summary two; https://blog.jquery.com/2020/04/10/jquery-3-5-0-released/\nseverity: low; CVE: CVE-2020-11111; https://blog.jquery.com/2020/04/10/jquery-3-5-0-released/", 'Verbose message should contain correct information and format'); + + }); + // Changes to the codebase make it unclear how this corner case would occur, but it's worth having the automation // so we avoid introducing any weird bugs in the future. it('Corner Case: When file has multiple aliases, results are consolidated', async () => { @@ -377,7 +432,7 @@ describe('RetireJsEngine', () => { }; // THIS IS THE ACTUAL METHOD BEING TESTED: Now we feed that fake result into the engine and see what we get back. - const results: RuleResult[] = (testEngine as any).processOutput(JSON.stringify(fakeRetireOutput), 'insecure-bundled-dependencies'); + const results: RuleResult[] = (testEngine as any).processOutput(JSON.stringify(fakeRetireOutput), 'insecure-bundled-dependencies', false); // Now we run our assertions. expect(results.length).to.equal(1, 'Should be one result object, since both aliases correspond to the same original file'); @@ -393,7 +448,7 @@ describe('RetireJsEngine', () => { const invalidJson = '{"beep": ['; try { - const results: RuleResult[] = (testEngine as any).processOutput(invalidJson, 'insecure-bundled-dependencies'); + const results: RuleResult[] = (testEngine as any).processOutput(invalidJson, 'insecure-bundled-dependencies', false); expect(true).to.equal(false, 'Exception should be thrown'); expect(results).to.equal(null, 'This assertion should never fire. It is needed to make the TS compiler stop complaining'); } catch (e) { @@ -408,7 +463,7 @@ describe('RetireJsEngine', () => { }; try { - const results: RuleResult[] = (testEngine as any).processOutput(JSON.stringify(malformedJson), 'insecure-bundled-dependencies'); + const results: RuleResult[] = (testEngine as any).processOutput(JSON.stringify(malformedJson), 'insecure-bundled-dependencies', false); expect(true).to.equal(false, 'Exception should be thrown'); expect(results).to.equal(null, 'This assertion should never fire. It is needed to make the TS compiler stop complaining'); } catch (e) {