@W-10127077@: adds --verbose-violations flag and functionality

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.",

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 = {

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<void> {
@@ -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;
 	}
 

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<RuleResult[]>[] = [];
 		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<RuleResult[]> {
+	private async executeRetireJs(invocation: RetireJsInvocation, verboseViolations: boolean): Promise<RuleResult[]> {
 		return new Promise<RuleResult[]>((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':

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) {