From b135d4dc6d4783396466cbc2814b3a6f337eec2f Mon Sep 17 00:00:00 2001 From: Grace Date: Wed, 15 Jun 2022 10:31:52 -0700 Subject: [PATCH 1/4] @d/W-11267130@: violation messages use
instead of \n in html --- src/lib/formatter/RuleResultRecombinator.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/formatter/RuleResultRecombinator.ts b/src/lib/formatter/RuleResultRecombinator.ts index 1c87daa8c..7565d7e91 100644 --- a/src/lib/formatter/RuleResultRecombinator.ts +++ b/src/lib/formatter/RuleResultRecombinator.ts @@ -373,7 +373,7 @@ URL: ${url}`; ruleName: v.ruleName, category: v.category, url: v.url, - message: v.message, + message: v.message.replace(/\n/g, '
'), //
used for line breaks in html line: v.line, column: v.column, endLine: v.endLine || null, @@ -387,7 +387,7 @@ URL: ${url}`; ruleName: v.ruleName, category: v.category, url: v.url, - message: v.message, + message: v.message.replace(/\n/g, '
'), //
used for line breaks in html line: v.sourceLine, column: v.sourceColumn, sinkFileName: v.sinkFileName, From a4c5e19e8c04921a0919d783b046c55652022751 Mon Sep 17 00:00:00 2001 From: Grace Date: Wed, 15 Jun 2022 12:48:33 -0700 Subject: [PATCH 2/4] newline change only applies when flag used --- src/lib/DefaultRuleManager.ts | 3 ++- src/lib/formatter/RuleResultRecombinator.ts | 10 +++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/lib/DefaultRuleManager.ts b/src/lib/DefaultRuleManager.ts index ce4ad7d51..b9ee1c31e 100644 --- a/src/lib/DefaultRuleManager.ts +++ b/src/lib/DefaultRuleManager.ts @@ -14,6 +14,7 @@ import {Controller} from '../Controller'; import globby = require('globby'); import path = require('path'); import {uxEvents, EVENTS} from './ScannerEvents'; +import {CUSTOM_CONFIG} from '../Constants'; Messages.importMessagesDirectory(__dirname); const messages = Messages.loadMessages('@salesforce/sfdx-scanner', 'DefaultRuleManager'); @@ -127,7 +128,7 @@ export class DefaultRuleManager implements RuleManager { psResults.forEach(r => results = results.concat(r)); this.logger.trace(`Received rule violations: ${JSON.stringify(results)}`); this.logger.trace(`Recombining results into requested format ${runOptions.format}`); - return await RuleResultRecombinator.recombineAndReformatResults(results, runOptions.format, executedEngines); + return await RuleResultRecombinator.recombineAndReformatResults(results, runOptions.format, executedEngines, engineOptions.has(CUSTOM_CONFIG.VerboseViolations)); } catch (e) { const message: string = e instanceof Error ? e.message : e as string; throw new SfdxError(message); diff --git a/src/lib/formatter/RuleResultRecombinator.ts b/src/lib/formatter/RuleResultRecombinator.ts index 7565d7e91..4111619cb 100644 --- a/src/lib/formatter/RuleResultRecombinator.ts +++ b/src/lib/formatter/RuleResultRecombinator.ts @@ -37,7 +37,7 @@ type DfaTableRow = BaseTableRow & { export class RuleResultRecombinator { - public static async recombineAndReformatResults(results: RuleResult[], format: OUTPUT_FORMAT, executedEngines: Set): Promise { + public static async recombineAndReformatResults(results: RuleResult[], format: OUTPUT_FORMAT, executedEngines: Set, verboseViolations = false): Promise { // We need to change the results we were given into the desired final format. let formattedResults: string | {columns; rows} = null; switch (format) { @@ -45,7 +45,7 @@ export class RuleResultRecombinator { formattedResults = await this.constructCsv(results, executedEngines); break; case OUTPUT_FORMAT.HTML: - formattedResults = await this.constructHtml(results, executedEngines); + formattedResults = await this.constructHtml(results, executedEngines, verboseViolations); break; case OUTPUT_FORMAT.JSON: formattedResults = this.constructJson(results); @@ -351,7 +351,7 @@ URL: ${url}`; return JSON.stringify(results.filter(r => r.violations.length > 0)); } - private static async constructHtml(results: RuleResult[], executedEngines: Set): Promise { + private static async constructHtml(results: RuleResult[], executedEngines: Set, verboseViolations = false): Promise { // If the results were just an empty string, we can return it. if (results.length === 0) { return ''; @@ -373,7 +373,7 @@ URL: ${url}`; ruleName: v.ruleName, category: v.category, url: v.url, - message: v.message.replace(/\n/g, '
'), //
used for line breaks in html + message: verboseViolations ? v.message.replace(/\n/g, '
') : v.message, //
used for line breaks in html line: v.line, column: v.column, endLine: v.endLine || null, @@ -387,7 +387,7 @@ URL: ${url}`; ruleName: v.ruleName, category: v.category, url: v.url, - message: v.message.replace(/\n/g, '
'), //
used for line breaks in html + message: verboseViolations ? v.message.replace(/\n/g, '
') : v.message, //
used for line breaks in html line: v.sourceLine, column: v.sourceColumn, sinkFileName: v.sinkFileName, From 9b3a269af1b3dc4d62d2f3e12a69fed80d4079d6 Mon Sep 17 00:00:00 2001 From: Grace Date: Thu, 16 Jun 2022 08:54:38 -0700 Subject: [PATCH 3/4] added test for html verbose-violations message format --- src/lib/formatter/RuleResultRecombinator.ts | 4 +-- .../formatter/RuleResultRecombinator.test.ts | 25 +++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/lib/formatter/RuleResultRecombinator.ts b/src/lib/formatter/RuleResultRecombinator.ts index 4111619cb..191fe1cf1 100644 --- a/src/lib/formatter/RuleResultRecombinator.ts +++ b/src/lib/formatter/RuleResultRecombinator.ts @@ -373,7 +373,7 @@ URL: ${url}`; ruleName: v.ruleName, category: v.category, url: v.url, - message: verboseViolations ? v.message.replace(/\n/g, '
') : v.message, //
used for line breaks in html + message: verboseViolations && result.engine === 'retire-js' ? v.message.replace(/\n/g, '
') : v.message, //
used for line breaks in html line: v.line, column: v.column, endLine: v.endLine || null, @@ -387,7 +387,7 @@ URL: ${url}`; ruleName: v.ruleName, category: v.category, url: v.url, - message: verboseViolations ? v.message.replace(/\n/g, '
') : v.message, //
used for line breaks in html + message: verboseViolations && result.engine === 'retire-js' ? v.message.replace(/\n/g, '
') : v.message, //
used for line breaks in html line: v.sourceLine, column: v.sourceColumn, sinkFileName: v.sinkFileName, diff --git a/test/lib/formatter/RuleResultRecombinator.test.ts b/test/lib/formatter/RuleResultRecombinator.test.ts index 414808b5c..88122f072 100644 --- a/test/lib/formatter/RuleResultRecombinator.test.ts +++ b/test/lib/formatter/RuleResultRecombinator.test.ts @@ -288,6 +288,22 @@ const allFakeDfaRuleResultsNormalized: RuleResult[] = [ } ]; +const retireJsVerboseViolations: RuleResult[] = [ + { + engine: 'retire-js', + fileName: sampleFile1, + violations: [{ + "line": 1, + "column": 1, + "severity": 2, + "message": "jquery 3.1.0 has known vulnerabilities:\nseverity: medium; summary: jQuery before 3.4.0, as used in Drupal, Backdrop CMS, and other products, mishandles jQuery.extend(true, {}, ...) because of Object.prototype pollution; CVE: CVE-2019-11358; https://blog.jquery.com/2019/04/10/jquery-3-4-0-released/ https://nvd.nist.gov/vuln/detail/CVE-2019-11358 https://github.com/jquery/jquery/commit/753d591aea698e57d6db58c9f722cd0808619b1b\nseverity: medium; summary: Regex in its jQuery.htmlPrefilter sometimes may introduce XSS; CVE: CVE-2020-11022; https://blog.jquery.com/2020/04/10/jquery-3-5-0-released/\nseverity: medium; summary: Regex in its jQuery.htmlPrefilter sometimes may introduce XSS; CVE: CVE-2020-11023; https://blog.jquery.com/2020/04/10/jquery-3-5-0-released/", + "ruleName": "insecure-bundled-dependencies", + "category": "Insecure Dependencies", + }] + } +]; + + function isString(x: string | {columns; rows}): x is string { return typeof x === 'string'; } @@ -1268,5 +1284,14 @@ describe('RuleResultRecombinator', () => { expect(problemNumber).to.equal(6, 'Problem Number Index'); }); }); + + describe('Output Format: HTML', () => { + it ('Using --verbose-violations', async () => { + const results = (await RuleResultRecombinator.recombineAndReformatResults(retireJsVerboseViolations, OUTPUT_FORMAT.HTML, new Set(['retire-js']), true)).results as string; + const violationString = results.split("const violations = [")[1].split("];\n")[0]; + const violation: RuleViolation = JSON.parse(violationString as string); + expect(violation.message).to.equal("jquery 3.1.0 has known vulnerabilities:
severity: medium; summary: jQuery before 3.4.0, as used in Drupal, Backdrop CMS, and other products, mishandles jQuery.extend(true, {}, ...) because of Object.prototype pollution; CVE: CVE-2019-11358; https://blog.jquery.com/2019/04/10/jquery-3-4-0-released/ https://nvd.nist.gov/vuln/detail/CVE-2019-11358 https://github.com/jquery/jquery/commit/753d591aea698e57d6db58c9f722cd0808619b1b
severity: medium; summary: Regex in its jQuery.htmlPrefilter sometimes may introduce XSS; CVE: CVE-2020-11022; https://blog.jquery.com/2020/04/10/jquery-3-5-0-released/
severity: medium; summary: Regex in its jQuery.htmlPrefilter sometimes may introduce XSS; CVE: CVE-2020-11023; https://blog.jquery.com/2020/04/10/jquery-3-5-0-released/"); + }); + }); }); }); From 182afac1a555458bb2aee439595294442b5f8950 Mon Sep 17 00:00:00 2001 From: Grace Date: Thu, 16 Jun 2022 10:16:36 -0700 Subject: [PATCH 4/4] feedback from PR --- src/lib/formatter/RuleResultRecombinator.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/formatter/RuleResultRecombinator.ts b/src/lib/formatter/RuleResultRecombinator.ts index 191fe1cf1..fc1450884 100644 --- a/src/lib/formatter/RuleResultRecombinator.ts +++ b/src/lib/formatter/RuleResultRecombinator.ts @@ -1,7 +1,7 @@ import {SfdxError} from '@salesforce/core'; import * as path from 'path'; import {EngineExecutionSummary, RecombinedData, RecombinedRuleResults, RuleResult, RuleViolation} from '../../types'; -import {DfaEngineFilters} from '../../Constants'; +import {DfaEngineFilters, ENGINE} from '../../Constants'; import {OUTPUT_FORMAT} from '../RuleManager'; import * as wrap from 'word-wrap'; import {FileHandler} from '../util/FileHandler'; @@ -373,7 +373,7 @@ URL: ${url}`; ruleName: v.ruleName, category: v.category, url: v.url, - message: verboseViolations && result.engine === 'retire-js' ? v.message.replace(/\n/g, '
') : v.message, //
used for line breaks in html + message: verboseViolations && result.engine === ENGINE.RETIRE_JS ? v.message.replace(/\n/g, '
') : v.message, //
used for line breaks in html line: v.line, column: v.column, endLine: v.endLine || null, @@ -387,7 +387,7 @@ URL: ${url}`; ruleName: v.ruleName, category: v.category, url: v.url, - message: verboseViolations && result.engine === 'retire-js' ? v.message.replace(/\n/g, '
') : v.message, //
used for line breaks in html + message: v.message, line: v.sourceLine, column: v.sourceColumn, sinkFileName: v.sinkFileName,