Skip to content

Commit

Permalink
Reduce json audit output
Browse files Browse the repository at this point in the history
- Print advisories object to reduce redundancy in json output

Solves yarnpkg#7404
  • Loading branch information
kohakukun committed Aug 25, 2019
1 parent 1a57117 commit 9a8164b
Show file tree
Hide file tree
Showing 9 changed files with 141 additions and 75 deletions.
18 changes: 9 additions & 9 deletions __tests__/commands/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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']});
});
});

Expand Down Expand Up @@ -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']});
});
});

Expand Down
47 changes: 47 additions & 0 deletions __tests__/reporters/__snapshots__/console-reporter.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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": "",
Expand Down
4 changes: 2 additions & 2 deletions __tests__/reporters/__snapshots__/json-reporter.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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\\"}}}",
}
`;

Expand Down
47 changes: 47 additions & 0 deletions __tests__/reporters/console-reporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
15 changes: 4 additions & 11 deletions __tests__/reporters/json-reporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -175,7 +168,7 @@ test('JSONReporter.auditAdvisory', async () => {
},
url: 'https://nodesecurity.io/advisories/118',
},
);
});
}),
).toMatchSnapshot();
});
Expand Down
48 changes: 11 additions & 37 deletions src/cli/commands/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/reporters/base-reporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
Expand Down
Loading

0 comments on commit 9a8164b

Please sign in to comment.