From 9a8164b6d4be2913bd00962eb8e21fba9dd1fd6c Mon Sep 17 00:00:00 2001 From: kohakukun Date: Sun, 25 Aug 2019 19:45:20 +0200 Subject: [PATCH] Reduce json audit output - Print advisories object to reduce redundancy in json output Solves https://github.com/yarnpkg/yarn/issues/7404 --- __tests__/commands/audit.js | 18 +++---- .../__snapshots__/console-reporter.js.snap | 47 ++++++++++++++++++ .../__snapshots__/json-reporter.js.snap | 4 +- __tests__/reporters/console-reporter.js | 47 ++++++++++++++++++ __tests__/reporters/json-reporter.js | 15 ++---- src/cli/commands/audit.js | 48 +++++-------------- src/reporters/base-reporter.js | 2 +- src/reporters/console/console-reporter.js | 29 ++++++----- src/reporters/json-reporter.js | 6 +-- 9 files changed, 141 insertions(+), 75 deletions(-) diff --git a/__tests__/commands/audit.js b/__tests__/commands/audit.js index 931d1639e3..1be51033c4 100644 --- a/__tests__/commands/audit.js +++ b/__tests__/commands/audit.js @@ -27,7 +27,7 @@ const setupMockRequestManager = function(config) { const setupMockReporter = function(reporter) { // $FlowFixMe - reporter.auditAdvisory = jest.fn(); + reporter.auditAdvisories = jest.fn(); // $FlowFixMe reporter.auditAction = jest.fn(); // $FlowFixMe @@ -214,24 +214,24 @@ test('audit groups only devDependencies omits dependencies from requires', () => }); }); -test('calls reporter auditAdvisory when using --level high flag', () => { +test('calls reporter auditAdvisories when using --level high flag', () => { return runAudit([], {level: 'high'}, 'single-vulnerable-dep-installed', (config, reporter) => { const apiResponse = getAuditResponse(config); - expect(reporter.auditAdvisory).toBeCalledWith(apiResponse.actions[0].resolves[0], apiResponse.advisories['118']); + expect(reporter.auditAdvisories).toBeCalledWith({'118': apiResponse.advisories['118']}); }); }); -test(`doesn't call reporter auditAdvisory when using --level critical flag`, () => { +test(`doesn't call reporter auditAdvisories when using --level critical flag`, () => { return runAudit([], {level: 'critical'}, 'single-vulnerable-dep-installed', (config, reporter) => { getAuditResponse(config); - expect(reporter.auditAdvisory).not.toHaveBeenCalled(); + expect(reporter.auditAdvisories).not.toHaveBeenCalled(); }); }); -test('calls reporter auditAdvisory with correct data', () => { +test('calls reporter auditAdvisories with correct data', () => { return runAudit([], {}, 'single-vulnerable-dep-installed', (config, reporter) => { const apiResponse = getAuditResponse(config); - expect(reporter.auditAdvisory).toBeCalledWith(apiResponse.actions[0].resolves[0], apiResponse.advisories['118']); + expect(reporter.auditAdvisories).toBeCalledWith({'118': apiResponse.advisories['118']}); }); }); @@ -281,10 +281,10 @@ test.concurrent('sends correct dependency map to audit api for private package.' }); }); -test('calls reporter auditAdvisory with correct data for private package', () => { +test('calls reporter auditAdvisories with correct data for private package', () => { return runAudit([], {}, 'single-vulnerable-dep-installed', (config, reporter) => { const apiResponse = getAuditResponse(config); - expect(reporter.auditAdvisory).toBeCalledWith(apiResponse.actions[0].resolves[0], apiResponse.advisories['118']); + expect(reporter.auditAdvisories).toBeCalledWith({'118': apiResponse.advisories['118']}); }); }); diff --git a/__tests__/reporters/__snapshots__/console-reporter.js.snap b/__tests__/reporters/__snapshots__/console-reporter.js.snap index 05efa5069c..82a7573be3 100644 --- a/__tests__/reporters/__snapshots__/console-reporter.js.snap +++ b/__tests__/reporters/__snapshots__/console-reporter.js.snap @@ -7,6 +7,53 @@ Object { } `; +exports[`ConsoleReporter.auditAdvisories 1`] = ` +Object { + "stderr": "", + "stdout": "┌───────────────┬──────────────────────────────────────────────────────────────┐ +│ high │ Regular Expression Denial of Service │ +├───────────────┼──────────────────────────────────────────────────────────────┤ +│ Package │ minimatch │ +├───────────────┼──────────────────────────────────────────────────────────────┤ +│ Patched in │ >=3.0.2 │ +├───────────────┼──────────────────────────────────────────────────────────────┤ +│ Dependency of │ gulp │ +├───────────────┼──────────────────────────────────────────────────────────────┤ +│ Path │ gulp > vinyl-fs > glob-stream > minimatch │ +├───────────────┼──────────────────────────────────────────────────────────────┤ +│ More info │ https://www.npmjs.com/advisories/118 │ +└───────────────┴──────────────────────────────────────────────────────────────┘ +┌───────────────┬──────────────────────────────────────────────────────────────┐ +│ high │ Regular Expression Denial of Service │ +├───────────────┼──────────────────────────────────────────────────────────────┤ +│ Package │ minimatch │ +├───────────────┼──────────────────────────────────────────────────────────────┤ +│ Patched in │ >=3.0.2 │ +├───────────────┼──────────────────────────────────────────────────────────────┤ +│ Dependency of │ jest │ +├───────────────┼──────────────────────────────────────────────────────────────┤ +│ Path │ jest > jest-cli > jest-config > jest-environment-jsdom > │ +│ │ jest-util > jest-message-util > micromatch > braces │ +├───────────────┼──────────────────────────────────────────────────────────────┤ +│ More info │ https://www.npmjs.com/advisories/118 │ +└───────────────┴──────────────────────────────────────────────────────────────┘ +┌───────────────┬──────────────────────────────────────────────────────────────┐ +│ high │ Regular Expression Denial of Service │ +├───────────────┼──────────────────────────────────────────────────────────────┤ +│ Package │ minimatch │ +├───────────────┼──────────────────────────────────────────────────────────────┤ +│ Patched in │ >=3.0.2 │ +├───────────────┼──────────────────────────────────────────────────────────────┤ +│ Dependency of │ jest │ +├───────────────┼──────────────────────────────────────────────────────────────┤ +│ Path │ jest > jest-cli > jest-environment-jsdom > jest-util > │ +│ │ jest-message-util > micromatch > braces │ +├───────────────┼──────────────────────────────────────────────────────────────┤ +│ More info │ https://www.npmjs.com/advisories/118 │ +└───────────────┴──────────────────────────────────────────────────────────────┘", +} +`; + exports[`ConsoleReporter.auditSummary 1`] = ` Object { "stderr": "", diff --git a/__tests__/reporters/__snapshots__/json-reporter.js.snap b/__tests__/reporters/__snapshots__/json-reporter.js.snap index 9606dd900a..c579ed8946 100644 --- a/__tests__/reporters/__snapshots__/json-reporter.js.snap +++ b/__tests__/reporters/__snapshots__/json-reporter.js.snap @@ -24,10 +24,10 @@ Object { } `; -exports[`JSONReporter.auditAdvisory 1`] = ` +exports[`JSONReporter.auditAdvisories 1`] = ` Object { "stderr": "", - "stdout": "{\\"type\\":\\"auditAdvisory\\",\\"data\\":{\\"resolution\\":{\\"id\\":118,\\"path\\":\\"gulp>vinyl-fs>glob-stream>minimatch\\",\\"dev\\":false,\\"optional\\":false,\\"bundled\\":false},\\"advisory\\":{\\"findings\\":[{\\"bundled\\":false,\\"optional\\":false,\\"dev\\":false,\\"paths\\":[],\\"version\\":\\"\\"}],\\"id\\":118,\\"created\\":\\"2016-05-25T16:37:20.000Z\\",\\"updated\\":\\"2018-03-01T21:58:01.072Z\\",\\"deleted\\":null,\\"title\\":\\"Regular Expression Denial of Service\\",\\"found_by\\":{\\"name\\":\\"Nick Starke\\"},\\"reported_by\\":{\\"name\\":\\"Nick Starke\\"},\\"module_name\\":\\"minimatch\\",\\"cves\\":[\\"CVE-2016-10540\\"],\\"vulnerable_versions\\":\\"<=3.0.1\\",\\"patched_versions\\":\\">=3.0.2\\",\\"overview\\":\\"\\",\\"recommendation\\":\\"Update to version 3.0.2 or later.\\",\\"references\\":\\"\\",\\"access\\":\\"public\\",\\"severity\\":\\"high\\",\\"cwe\\":\\"CWE-400\\",\\"metadata\\":{\\"module_type\\":\\"Multi.Library\\",\\"exploitability\\":4,\\"affected_components\\":\\"\\"},\\"url\\":\\"https://nodesecurity.io/advisories/118\\"}}}", + "stdout": "{\\"type\\":\\"auditAdvisories\\",\\"data\\":{\\"118\\":{\\"findings\\":[{\\"bundled\\":false,\\"optional\\":false,\\"dev\\":false,\\"paths\\":[],\\"version\\":\\"\\"}],\\"id\\":118,\\"created\\":\\"2016-05-25T16:37:20.000Z\\",\\"updated\\":\\"2018-03-01T21:58:01.072Z\\",\\"deleted\\":null,\\"title\\":\\"Regular Expression Denial of Service\\",\\"found_by\\":{\\"name\\":\\"Nick Starke\\"},\\"reported_by\\":{\\"name\\":\\"Nick Starke\\"},\\"module_name\\":\\"minimatch\\",\\"cves\\":[\\"CVE-2016-10540\\"],\\"vulnerable_versions\\":\\"<=3.0.1\\",\\"patched_versions\\":\\">=3.0.2\\",\\"overview\\":\\"\\",\\"recommendation\\":\\"Update to version 3.0.2 or later.\\",\\"references\\":\\"\\",\\"access\\":\\"public\\",\\"severity\\":\\"high\\",\\"cwe\\":\\"CWE-400\\",\\"metadata\\":{\\"module_type\\":\\"Multi.Library\\",\\"exploitability\\":4,\\"affected_components\\":\\"\\"},\\"url\\":\\"https://nodesecurity.io/advisories/118\\"}}}", } `; diff --git a/__tests__/reporters/console-reporter.js b/__tests__/reporters/console-reporter.js index 2e52373825..e8d6e96ebb 100644 --- a/__tests__/reporters/console-reporter.js +++ b/__tests__/reporters/console-reporter.js @@ -326,3 +326,50 @@ test('ConsoleReporter.auditSummary', async () => { }), ).toMatchSnapshot(); }); + +test('ConsoleReporter.auditAdvisories', async () => { + expect( + await getConsoleBuff(r => { + r.auditAdvisories({ + '118': { + findings: [ + { + bundled: false, + optional: false, + dev: false, + paths: [ + 'gulp>vinyl-fs>glob-stream>minimatch', + 'jest>jest-cli>jest-config>jest-environment-jsdom>jest-util>jest-message-util>micromatch>braces', + 'jest>jest-cli>jest-environment-jsdom>jest-util>jest-message-util>micromatch>braces', + ], + version: '', + }, + ], + id: 118, + created: '2016-05-25T16:37:20.000Z', + updated: '2018-03-01T21:58:01.072Z', + deleted: null, + title: 'Regular Expression Denial of Service', + found_by: {name: 'Nick Starke'}, + reported_by: {name: 'Nick Starke'}, + module_name: 'minimatch', + cves: ['CVE-2016-10540'], + vulnerable_versions: '<=3.0.1', + patched_versions: '>=3.0.2', + overview: '', + recommendation: 'Update to version 3.0.2 or later.', + references: '', + access: 'public', + severity: 'high', + cwe: 'CWE-400', + metadata: { + module_type: 'Multi.Library', + exploitability: 4, + affected_components: '', + }, + url: 'https://nodesecurity.io/advisories/118', + }, + }); + }), + ).toMatchSnapshot(); +}); diff --git a/__tests__/reporters/json-reporter.js b/__tests__/reporters/json-reporter.js index f21b210f20..bc5a3387cb 100644 --- a/__tests__/reporters/json-reporter.js +++ b/__tests__/reporters/json-reporter.js @@ -130,18 +130,11 @@ test('JSONReporter.auditAction', async () => { ).toMatchSnapshot(); }); -test('JSONReporter.auditAdvisory', async () => { +test('JSONReporter.auditAdvisories', async () => { expect( await getJSONBuff(r => { - r.auditAdvisory( - { - id: 118, - path: 'gulp>vinyl-fs>glob-stream>minimatch', - dev: false, - optional: false, - bundled: false, - }, - { + r.auditAdvisories({ + '118': { findings: [ { bundled: false, @@ -175,7 +168,7 @@ test('JSONReporter.auditAdvisory', async () => { }, url: 'https://nodesecurity.io/advisories/118', }, - ); + }); }), ).toMatchSnapshot(); }); diff --git a/src/cli/commands/audit.js b/src/cli/commands/audit.js index 7f2d92ebf7..fcd0938da9 100644 --- a/src/cli/commands/audit.js +++ b/src/cli/commands/audit.js @@ -307,45 +307,19 @@ export default class Audit { const startLoggingAt: number = Math.max(0, this.severityLevels.indexOf(this.options.level)); - const reportAdvisory = (resolution: AuditResolution) => { - const advisory = this.auditData.advisories[resolution.id.toString()]; + const advisoriesIds = Object.keys(this.auditData.advisories); + if (advisoriesIds.length !== 0) { + const filteredAdvisories = advisoriesIds.reduce((acc, advisoryKey) => { + const advisory = this.auditData.advisories[advisoryKey]; + if (this.severityLevels.indexOf(advisory.severity) >= startLoggingAt) { + acc[advisoryKey] = advisory; + } + return acc; + }, {}); - if (this.severityLevels.indexOf(advisory.severity) >= startLoggingAt) { - this.reporter.auditAdvisory(resolution, advisory); + if (Object.keys(filteredAdvisories).length > 0) { + this.reporter.auditAdvisories(filteredAdvisories); } - }; - - if (Object.keys(this.auditData.advisories).length !== 0) { - // let printedManualReviewHeader = false; - - this.auditData.actions.forEach(action => { - action.resolves.forEach(reportAdvisory); - - /* The following block has been temporarily removed - * because the actions returned by npm are not valid for yarn. - * Removing this action reporting until we can come up with a way - * to correctly resolve issues. - */ - // if (action.action === 'update' || action.action === 'install') { - // // these advisories can be resolved automatically by running a yarn command - // const recommendation: AuditActionRecommendation = { - // cmd: `yarn upgrade ${action.module}@${action.target}`, - // isBreaking: action.isMajor, - // action, - // }; - // this.reporter.auditAction(recommendation); - // action.resolves.forEach(reportAdvisory); - // } - - // if (action.action === 'review') { - // // these advisories cannot be resolved automatically and require manual review - // if (!printedManualReviewHeader) { - // this.reporter.auditManualReview(); - // } - // printedManualReviewHeader = true; - // action.resolves.forEach(reportAdvisory); - // } - }); } this.summary(); diff --git a/src/reporters/base-reporter.js b/src/reporters/base-reporter.js index 44653602b0..414b0614b5 100644 --- a/src/reporters/base-reporter.js +++ b/src/reporters/base-reporter.js @@ -235,7 +235,7 @@ export default class BaseReporter { auditManualReview() {} // security audit advisory - auditAdvisory(resolution: AuditResolution, auditAdvisory: AuditAdvisory) {} + auditAdvisories(advisories: {[string]: AuditAdvisory}) {} // summary for security audit report auditSummary(auditMetadata: AuditMetadata) {} diff --git a/src/reporters/console/console-reporter.js b/src/reporters/console/console-reporter.js index 0e1aa38afd..d8ba07eaf9 100644 --- a/src/reporters/console/console-reporter.js +++ b/src/reporters/console/console-reporter.js @@ -11,7 +11,7 @@ import type { PromptOptions, } from '../types.js'; import type {FormatKeys} from '../format.js'; -import type {AuditMetadata, AuditActionRecommendation, AuditAdvisory, AuditResolution} from '../../cli/commands/audit'; +import type {AuditMetadata, AuditActionRecommendation, AuditAdvisory} from '../../cli/commands/audit'; import BaseReporter from '../base-reporter.js'; import Progress from './progress-bar.js'; @@ -547,12 +547,12 @@ export default class ConsoleReporter extends BaseReporter { this._log(table.toString()); } - auditAdvisory(resolution: AuditResolution, auditAdvisory: AuditAdvisory) { + auditAdvisories(advisories: {[string]: AuditAdvisory}) { function colorSeverity(severity: string, message: ?string): string { return auditSeverityColors[severity](message || severity); } - function makeAdvisoryTableRow(patchedIn: ?string): Array { + function makeAdvisoryTableRow(path: string, auditAdvisory: AuditAdvisory, patchedIn: ?string): Array { const patchRows = []; if (patchedIn) { @@ -563,8 +563,8 @@ export default class ConsoleReporter extends BaseReporter { {[chalk.bold(colorSeverity(auditAdvisory.severity))]: chalk.bold(auditAdvisory.title)}, {Package: auditAdvisory.module_name}, ...patchRows, - {'Dependency of': `${resolution.path.split('>')[0]} ${resolution.dev ? '[dev]' : ''}`}, - {Path: resolution.path.split('>').join(' > ')}, + {'Dependency of': `${path.split('>')[0]} ${auditAdvisory.dev ? '[dev]' : ''}`}, + {Path: path.split('>').join(' > ')}, {'More info': `https://www.npmjs.com/advisories/${auditAdvisory.id}`}, ]; } @@ -573,12 +573,17 @@ export default class ConsoleReporter extends BaseReporter { colWidths: AUDIT_COL_WIDTHS, wordWrap: true, }; - const table = new Table(tableOptions); - const patchedIn = - auditAdvisory.patched_versions.replace(' ', '') === '<0.0.0' - ? 'No patch available' - : auditAdvisory.patched_versions; - table.push(...makeAdvisoryTableRow(patchedIn)); - this._log(table.toString()); + Object.keys(advisories).forEach(advisoryKey => { + const auditAdvisory = advisories[advisoryKey]; + const patchedIn = + auditAdvisory.patched_versions.replace(' ', '') === '<0.0.0' + ? 'No patch available' + : auditAdvisory.patched_versions; + for (const path of auditAdvisory.findings[0].paths) { + const table = new Table(tableOptions); + table.push(...makeAdvisoryTableRow(path, auditAdvisory, patchedIn)); + this._log(table.toString()); + } + }); } } diff --git a/src/reporters/json-reporter.js b/src/reporters/json-reporter.js index ecb7285910..d2506c93cc 100644 --- a/src/reporters/json-reporter.js +++ b/src/reporters/json-reporter.js @@ -1,7 +1,7 @@ /* @flow */ import type {ReporterSpinnerSet, Trees, ReporterSpinner} from './types.js'; -import type {AuditMetadata, AuditActionRecommendation, AuditAdvisory, AuditResolution} from '../cli/commands/audit'; +import type {AuditMetadata, AuditActionRecommendation, AuditAdvisory} from '../cli/commands/audit'; import BaseReporter from './base-reporter.js'; export default class JSONReporter extends BaseReporter { @@ -166,8 +166,8 @@ export default class JSONReporter extends BaseReporter { this._dump('auditAction', recommendation); } - auditAdvisory(resolution: AuditResolution, auditAdvisory: AuditAdvisory) { - this._dump('auditAdvisory', {resolution, advisory: auditAdvisory}); + auditAdvisories(advisories: {[string]: AuditAdvisory}) { + this._dump('auditAdvisories', advisories); } auditSummary(auditMetadata: AuditMetadata) {