Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Correctly label packages as dev-only in yarn audit #6910

Closed
wants to merge 7 commits into from
Closed

Correctly label packages as dev-only in yarn audit #6910

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
767d407
implement markDevOnlyEntries in PackageHoister so HoistManifests can …
as3richa Jan 11, 2019
068e713
implement plumbing for HoistedManifest isDevOnly in linker/hoisted tr…
as3richa Jan 11, 2019
7e598c4
distinguish dev-only from prod hoisted packages in audit
as3richa Jan 11, 2019
f6b7ee0
fix audit specs
as3richa Jan 11, 2019
fc6ee1d
spec out yarn audit correctly distinguishing dev and prod vulnerabili…
as3richa Jan 11, 2019
c09d940
spec out package hoister markDevOnlyEntries behavior
as3richa Jan 11, 2019
a61f36e
eslint . --fix
as3richa Jan 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
53 changes: 53 additions & 0 deletions __tests__/commands/audit.js
View file
Open in desktop
Expand Up @@ -68,6 +68,7 @@ test.concurrent('sends correct dependency map to audit api for single dependency
'brace-expansion': '^1.0.0',
},
dependencies: {},
dev: false,
},
'brace-expansion': {
version: '1.1.11',
Expand All @@ -77,21 +78,25 @@ test.concurrent('sends correct dependency map to audit api for single dependency
'concat-map': '0.0.1',
},
dependencies: {},
dev: false,
},
'balanced-match': {
version: '1.0.0',
integrity: 'sha1-ibTRmasr7kneFk6gK4nORi1xt2c=',
requires: {},
dependencies: {},
dev: false,
},
'concat-map': {
version: '0.0.1',
integrity: 'sha1-2Klr13/Wjfd5OnMDajug1UBdR3s=',
requires: {},
dependencies: {},
dev: false,
},
},
version: '0.0.0',
dev: false,
};

return runAudit([], {}, 'single-vulnerable-dep-installed', async config => {
Expand Down Expand Up @@ -141,8 +146,10 @@ test.concurrent('sends correct dependency map to audit api for private package.'
integrity: 'sha512-XI5MPzVNApjAyhQzphX8BkmKsKUxD4LdyK24iZeQGinBN9yTQT3bFlCBy/aVx2HrNcqQGsdot8ghrjyrvMCoEA==',
requires: {},
dependencies: {},
dev: false,
},
},
dev: false,
};

return runAudit([], {}, 'private-package', async config => {
Expand All @@ -166,6 +173,46 @@ test('calls reporter auditSummary with correct data for private package', () =>
});
});

test.concurrent('distinguishes dev and prod transitive dependencies in audit request and result', () => {
const expectedApiPost = {
name: 'foo',
version: '1.0.0',
install: [],
remove: [],
metadata: {},
requires: {
mime: '1.4.0',
hoek: '4.2.0',
},
dependencies: {
mime: {
version: '1.4.0',
integrity: 'sha512-n9ChLv77+QQEapYz8lV+rIZAW3HhAPW2CXnzb1GN5uMkuczshwvkW7XPsbzU0ZQN3sP47Er2KVkp2p3KyqZKSQ==',
requires: {},
dependencies: {},
dev: false,
},
hoek: {
version: '4.2.0',
integrity: 'sha512-v0XCLxICi9nPfYrS9RL8HbYnXi9obYAeLbSP00BmnZwCK9+Ih9WOjoZ8YoHCoav2csqn4FOz4Orldsy2dmDwmQ==',
requires: {},
dependencies: {},
dev: true,
},
},
dev: false,
};

return runAudit([], {}, 'dev-and-prod-vulnerabilities', async (config, reporter) => {
const calledWithPipe = config.requestManager.request.mock.calls[0][0].body;
const calledWith = JSON.parse(await gunzip(calledWithPipe));
expect(calledWith).toEqual(expectedApiPost);

const apiResponse = getAuditResponse(config);
expect(reporter.auditSummary).toBeCalledWith(apiResponse.metadata);
});
});

describe('returns semantic exit codes', () => {
beforeAll(() => {
// mock unrelated stuff
Expand Down Expand Up @@ -212,12 +259,14 @@ test.concurrent('sends correct dependency map to audit api for workspaces.', ()
dependencies: {
'balanced-match': {
dependencies: {},
dev: false,
integrity: 'sha1-ibTRmasr7kneFk6gK4nORi1xt2c=',
requires: {},
version: '1.0.0',
},
'brace-expansion': {
dependencies: {},
dev: false,
integrity: 'sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==',
requires: {
'balanced-match': '^1.0.0',
Expand All @@ -227,12 +276,14 @@ test.concurrent('sends correct dependency map to audit api for workspaces.', ()
},
'concat-map': {
dependencies: {},
dev: false,
integrity: 'sha1-2Klr13/Wjfd5OnMDajug1UBdR3s=',
requires: {},
version: '0.0.1',
},
minimatch: {
dependencies: {},
dev: false,
integrity: 'sha1-UjYVelHk8ATBd/s8Un/33Xjw74M=',
requires: {
'brace-expansion': '^1.0.0',
Expand All @@ -241,6 +292,7 @@ test.concurrent('sends correct dependency map to audit api for workspaces.', ()
},
prj1: {
dependencies: {},
dev: false,
integrity: '',
requires: {
minimatch: '3.0.0',
Expand All @@ -256,6 +308,7 @@ test.concurrent('sends correct dependency map to audit api for workspaces.', ()
prj1: '0.0.0',
},
version: '1.0.0',
dev: false,
};

return runAudit([], {}, 'workspace', async config => {
Expand Down
132 changes: 132 additions & 0 deletions __tests__/fixtures/audit/dev-and-prod-vulnerabilities/audit-api-response.json
View file
Open in desktop
@@ -0,0 +1,132 @@
{
"actions": [
{
"action": "install",
"module": "mime",
"target": "2.4.0",
"isMajor": true,
"resolves": [
{
"id": 535,
"path": "mime",
"dev": false,
"optional": false,
"bundled": false
}
]
},
{
"action": "install",
"module": "hoek",
"target": "6.1.2",
"isMajor": true,
"resolves": [
{
"id": 566,
"path": "hoek",
"dev": true,
"optional": false,
"bundled": false
}
]
}
],
"advisories": {
"535": {
"findings": [
{
"version": "1.4.0",
"paths": [
"mime"
],
"dev": false,
"optional": false,
"bundled": false
}
],
"id": 535,
"created": "2017-09-25T19:02:28.152Z",
"updated": "2018-04-09T00:38:22.785Z",
"deleted": null,
"title": "Regular Expression Denial of Service",
"found_by": {
"name": "Cristian-Alexandru Staicu"
},
"reported_by": {
"name": "Cristian-Alexandru Staicu"
},
"module_name": "mime",
"cves": [
"CVE-2017-16138"
],
"vulnerable_versions": "< 1.4.1 || > 2.0.0 < 2.0.3",
"patched_versions": ">= 1.4.1 < 2.0.0 || >= 2.0.3",
"overview": "Affected versions of `mime` are vulnerable to regular expression denial of service when a mime lookup is performed on untrusted user input.",
"recommendation": "Update to version 2.0.3 or later.",
"references": "[Issue #167](https://github.com/broofa/node-mime/issues/167)",
"access": "public",
"severity": "moderate",
"cwe": "CWE-400",
"metadata": {
"module_type": "Multi.Library",
"exploitability": 4,
"affected_components": ""
},
"url": "https://npmjs.com/advisories/535"
},
"566": {
"findings": [
{
"version": "4.2.0",
"paths": [
"hoek"
],
"dev": true,
"optional": false,
"bundled": false
}
],
"id": 566,
"created": "2018-04-20T21:25:58.421Z",
"updated": "2018-04-20T21:25:58.421Z",
"deleted": null,
"title": "Prototype pollution",
"found_by": {
"name": "HoLyVieR"
},
"reported_by": {
"name": "HoLyVieR"
},
"module_name": "hoek",
"cves": [],
"vulnerable_versions": "<= 4.2.0 || >= 5.0.0 < 5.0.3",
"patched_versions": "> 4.2.0 < 5.0.0 || >= 5.0.3",
"overview": "Versions of `hoek` prior to 4.2.1 and 5.0.3 are vulnerable to prototype pollution.\n\nThe `merge` function, and the `applyToDefaults` and `applyToDefaultsWithShallow` functions which leverage `merge` behind the scenes, are vulnerable to a prototype pollution attack when provided an _unvalidated_ payload created from a JSON string containing the `__proto__` property.\n\nThis can be demonstrated like so:\n\n```javascript\nvar Hoek = require('hoek');\nvar malicious_payload = '{\"__proto__\":{\"oops\":\"It works !\"}}';\n\nvar a = {};\nconsole.log(\"Before : \" + a.oops);\nHoek.merge({}, JSON.parse(malicious_payload));\nconsole.log(\"After : \" + a.oops);\n```\n\nThis type of attack can be used to overwrite existing properties causing a potential denial of service.",
"recommendation": "Update to version 4.2.1, 5.0.3 or later.",
"references": "",
"access": "public",
"severity": "moderate",
"cwe": "CWE-471",
"metadata": {
"module_type": "",
"exploitability": 5,
"affected_components": ""
},
"url": "https://npmjs.com/advisories/566"
}
},
"muted": [],
"metadata": {
"vulnerabilities": {
"info": 0,
"low": 0,
"moderate": 2,
"high": 0,
"critical": 0
},
"dependencies": 1,
"devDependencies": 1,
"optionalDependencies": 0,
"totalDependencies": 2
}
}
10 changes: 10 additions & 0 deletions __tests__/fixtures/audit/dev-and-prod-vulnerabilities/package.json
View file
Open in desktop
@@ -0,0 +1,10 @@
{
"name": "foo",
"version": "1.0.0",
"devDependencies": {
"hoek": "4.2.0"
},
"dependencies": {
"mime": "1.4.0"
}
}
13 changes: 13 additions & 0 deletions __tests__/fixtures/audit/dev-and-prod-vulnerabilities/yarn.lock
View file
Open in desktop
@@ -0,0 +1,13 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1


hoek@4.2.0:
version "4.2.0"
resolved "https://registry.yarnpkg.com/hoek/-/hoek-4.2.0.tgz#72d9d0754f7fe25ca2d01ad8f8f9a9449a89526d"
integrity sha512-v0XCLxICi9nPfYrS9RL8HbYnXi9obYAeLbSP00BmnZwCK9+Ih9WOjoZ8YoHCoav2csqn4FOz4Orldsy2dmDwmQ==

mime@1.4.0:
version "1.4.0"
resolved "https://registry.yarnpkg.com/mime/-/mime-1.4.0.tgz#69e9e0db51d44f2a3b56e48b7817d7d137f1a343"
integrity sha512-n9ChLv77+QQEapYz8lV+rIZAW3HhAPW2CXnzb1GN5uMkuczshwvkW7XPsbzU0ZQN3sP47Er2KVkp2p3KyqZKSQ==
28 changes: 28 additions & 0 deletions __tests__/package-hoister.js
View file
Open in desktop
Expand Up @@ -334,6 +334,34 @@ test('will hoist packages under subdirectories when they cannot hoist to root',
expect(result).toContainPackage('d@1.0.0', atPath('a', 'node_modules', 'd'));
});

test('correctly labels dev-only hoisted packages with markDevOnlyEntries', () => {
const {packageHoister} = createTestFixture({
'foo@1.0.0': ['bar@1.0.0'],
'bar@1.0.0': ['baz@1.0.0'],
'bar@2.0.0': ['baz@2.0.0'],
'baz@1.0.0': [],
'baz@2.0.0': [],
});

packageHoister.seed(['foo@1.0.0', 'bar@2.0.0']);
packageHoister.markDevOnlyEntries(['foo@1.0.0']);
const result = packageHoister.init();

const isDevOnly = {};

result.forEach(tuple => {
const manifest = tuple[1];
const pattern = manifest.pkg._reference.uid;
isDevOnly[pattern] = manifest.isDevOnly;
});

expect(isDevOnly['foo@1.0.0']).toBe(false);
expect(isDevOnly['bar@1.0.0']).toBe(false);
expect(isDevOnly['baz@1.0.0']).toBe(false);
expect(isDevOnly['bar@2.0.0']).toBe(true);
expect(isDevOnly['baz@2.0.0']).toBe(true);
});

describe('nohoist', () => {
test('nohoist can be turned off by disable workspaces (workspaces-experimental)', () => {
const {atPath, packageHoister, packageResolver} = createTestFixture(
Expand Down
13 changes: 12 additions & 1 deletion src/cli/commands/audit.js
View file
Open in desktop
Expand Up @@ -20,6 +20,7 @@ export type AuditNode = {
integrity: ?string,
requires: Object,
dependencies: {[string]: AuditNode},
dev: boolean,
};

export type AuditTree = AuditNode & {
Expand Down Expand Up @@ -121,6 +122,14 @@ export function hasWrapper(commander: Object, args: Array<string>): boolean {
return true;
}

export function getDevDeps(manifest: Object): Array<string> {
if (manifest.devDependencies) {
return Object.keys(manifest.devDependencies).map(key => `${key}@${manifest.devDependencies[key]}`);
} else {
return [];
}
}

export async function run(config: Config, reporter: Reporter, flags: Object, args: Array<string>): Promise<number> {
const audit = new Audit(config, reporter);
const lockfile = await Lockfile.fromDirectory(config.lockfileFolder, reporter);
Expand Down Expand Up @@ -178,6 +187,7 @@ export default class Audit {
integrity: pkg._remote ? pkg._remote.integrity || '' : '',
requires,
dependencies: {},
dev: !!node.isDevOnly,
};
if (node.children) {
this._mapHoistedNodes(auditNode.dependencies[node.name], node.children);
Expand All @@ -202,6 +212,7 @@ export default class Audit {
),
integrity: undefined,
dependencies: {},
dev: false,
};

this._mapHoistedNodes(auditTree, hoistedTrees);
Expand Down Expand Up @@ -257,7 +268,7 @@ export default class Audit {
patterns: Array<string>,
): Promise<AuditVulnerabilityCounts> {
this._insertWorkspacePackagesIntoManifest(manifest, resolver);
const hoistedTrees = await hoistedTreeBuilder(resolver, linker, patterns);
const hoistedTrees = await hoistedTreeBuilder(resolver, linker, patterns, getDevDeps(manifest));
const auditTree = this._mapHoistedTreesToAuditTree(manifest, hoistedTrees);
this.auditData = await this._fetchAudit(auditTree);
return this.auditData.metadata.vulnerabilities;
Expand Down