diff --git a/lib/workers/repository/init/vulnerability.js b/lib/workers/repository/init/vulnerability.js index 648e75218d5cff5aa1f4b03090f1dd4071b41c98..fbfd6989fb8b0763df68e889d97baee2f8d00540 100644 --- a/lib/workers/repository/init/vulnerability.js +++ b/lib/workers/repository/init/vulnerability.js @@ -1,3 +1,5 @@ +const versioning = require('../../../versioning'); + module.exports = { detectVulnerabilityAlerts, }; @@ -19,44 +21,92 @@ async function detectVulnerabilityAlerts(input) { return input; } const config = { ...input }; - const alertPackageRules = alerts - .filter(alert => !alert.dismissReason) - .filter( - alert => - alert.securityVulnerability && - alert.securityVulnerability.firstPatchedVersion && - alert.securityVulnerability.package - ) - .map(alert => { - const rule = {}; + const combinedAlerts = {}; + for (const alert of alerts) { + try { + if (alert.dismissReason) { + continue; // eslint-disable-line no-continue + } const managerMapping = { - MAVEN: ['maven'], - NPM: ['npm'], - NUGET: ['nuget'], - PIP: ['pip_requirements'], - RUBYGEMS: ['bundler'], + MAVEN: 'maven', + NPM: 'npm', + NUGET: 'nuget', + PIP: 'pip_requirements', + RUBYGEMS: 'bundler', }; - const managers = + const manager = managerMapping[alert.securityVulnerability.package.ecosystem]; - if (managers) { - rule.managers = managers; + if (!combinedAlerts[manager]) { + combinedAlerts[manager] = {}; + } + const depName = alert.securityVulnerability.package.name; + if (!combinedAlerts[manager][depName]) { + combinedAlerts[manager][depName] = { + fileNames: [], + }; } - rule.packageNames = [alert.securityVulnerability.package.name]; - // Raise only for where the currentVersion is vulnerable - rule.matchCurrentVersion = `< ${ - alert.securityVulnerability.firstPatchedVersion.identifier - }`; - // Don't propose upgrades to any versions that are still vulnerable - rule.allowedVersions = `>= ${ - alert.securityVulnerability.firstPatchedVersion.identifier - }`; - rule.force = { - ...config.vulnerabilityAlerts, - vulnerabilityAlert: true, + const fileName = alert.vulnerableManifestFilename; + if (!combinedAlerts[manager][depName].fileNames.includes(fileName)) { + combinedAlerts[manager][depName].fileNames.push(fileName); + } + const firstPatchedVersion = + alert.securityVulnerability.firstPatchedVersion.identifier; + const versionSchemes = { + maven: 'maven', + npm: 'npm', + nuget: 'semver', + pip_requirements: 'pep440', + rubygems: 'ruby', }; - return rule; - }) - .filter(Boolean); + const versionScheme = versioning.get(versionSchemes[manager]); + if (versionScheme.isVersion(firstPatchedVersion)) { + if (combinedAlerts[manager][depName].firstPatchedVersion) { + if ( + versionScheme.isGreaterThan( + firstPatchedVersion, + combinedAlerts[manager][depName].firstPatchedVersion + ) + ) { + combinedAlerts[manager][ + depName + ].firstPatchedVersion = firstPatchedVersion; + } + } else { + combinedAlerts[manager][ + depName + ].firstPatchedVersion = firstPatchedVersion; + } + } else { + logger.info('Invalid firstPatchedVersion: ' + firstPatchedVersion); + } + } catch (err) { + logger.warn({ err }, 'Error parsing vulnerability alert'); + } + } + const alertPackageRules = []; + for (const [manager, dependencies] of Object.entries(combinedAlerts)) { + for (const [depName, val] of Object.entries(dependencies)) { + const matchRule = { + managers: [manager], + packageNames: [depName], + matchCurrentVersion: `< ${val.firstPatchedVersion}`, + force: { + ...config.vulnerabilityAlerts, + vulnerabilityAlert: true, + }, + }; + alertPackageRules.push(matchRule); + const allowedRule = JSON.parse(JSON.stringify(matchRule)); + delete allowedRule.matchCurrentVersion; + delete allowedRule.force; + if (manager === 'npm') { + allowedRule.allowedVersions = `^${val.firstPatchedVersion}`; + } else { + allowedRule.allowedVersions = `>= ${val.firstPatchedVersion}`; + } + alertPackageRules.push(allowedRule); + } + } config.packageRules = (config.packageRules || []).concat(alertPackageRules); return config; } diff --git a/test/workers/repository/init/__snapshots__/vulnerability.spec.js.snap b/test/workers/repository/init/__snapshots__/vulnerability.spec.js.snap index 1cc32e5f1f7a1c037c3a90b053dc9a86f811dd40..b96521c9ac4b74464b725d01416b098d18d918e0 100644 --- a/test/workers/repository/init/__snapshots__/vulnerability.spec.js.snap +++ b/test/workers/repository/init/__snapshots__/vulnerability.spec.js.snap @@ -3,7 +3,6 @@ exports[`workers/repository/init/vulnerability detectVulnerabilityAlerts() returns alerts 1`] = ` Array [ Object { - "allowedVersions": ">= 1.8.3", "force": Object { "commitMessageSuffix": "[SECURITY]", "groupName": null, @@ -15,10 +14,45 @@ Array [ "managers": Array [ "npm", ], - "matchCurrentVersion": "< 1.8.3", + "matchCurrentVersion": "< 1.8.8", "packageNames": Array [ "electron", ], }, + Object { + "allowedVersions": "^1.8.8", + "managers": Array [ + "npm", + ], + "packageNames": Array [ + "electron", + ], + }, + Object { + "force": Object { + "commitMessageSuffix": "[SECURITY]", + "groupName": null, + "masterIssueApproval": false, + "rangeStrategy": "update-lockfile", + "schedule": Array [], + "vulnerabilityAlert": true, + }, + "managers": Array [ + "pip_requirements", + ], + "matchCurrentVersion": "< 1.8.8", + "packageNames": Array [ + "ansible", + ], + }, + Object { + "allowedVersions": ">= 1.8.8", + "managers": Array [ + "pip_requirements", + ], + "packageNames": Array [ + "ansible", + ], + }, ] `; diff --git a/test/workers/repository/init/vulnerability.spec.js b/test/workers/repository/init/vulnerability.spec.js index bc0dde2981174227a5ca83acc63d490f037dfce5..5d15ca311afbc0860146e40b7242c051a24b7bc6 100644 --- a/test/workers/repository/init/vulnerability.spec.js +++ b/test/workers/repository/init/vulnerability.spec.js @@ -1,7 +1,7 @@ let config; beforeEach(() => { jest.resetAllMocks(); - config = require('../../../_fixtures/config'); + config = JSON.parse(JSON.stringify(require('../../../_fixtures/config'))); }); const { @@ -10,6 +10,10 @@ const { describe('workers/repository/init/vulnerability', () => { describe('detectVulnerabilityAlerts()', () => { + it('returns if alerts are missing', async () => { + delete config.vulnerabilityAlerts; + expect(await detectVulnerabilityAlerts(config)).toEqual(config); + }); it('returns if alerts are disabled', async () => { config.vulnerabilityAlerts.enabled = false; expect(await detectVulnerabilityAlerts(config)).toEqual(config); @@ -30,6 +34,9 @@ describe('workers/repository/init/vulnerability', () => { delete config.vulnerabilityAlerts.enabled; platform.getVulnerabilityAlerts.mockReturnValue([ {}, + { + dismissReason: 'some reason', + }, { dismissReason: null, vulnerableManifestFilename: 'package-lock.json', @@ -46,11 +53,59 @@ describe('workers/repository/init/vulnerability', () => { vulnerableVersionRange: '>= 1.8, < 1.8.3', }, }, + { + dismissReason: null, + vulnerableManifestFilename: 'package-lock.json', + vulnerableManifestPath: 'package-lock.json', + vulnerableRequirements: '= 1.8.2', + securityVulnerability: { + package: { + name: 'electron', + ecosystem: 'NPM', + }, + firstPatchedVersion: { + identifier: 'abc-1.8.8', + }, + vulnerableVersionRange: '>= 1.8, < 1.8.8', + }, + }, + { + dismissReason: null, + vulnerableManifestFilename: 'package-lock.json', + vulnerableManifestPath: 'package-lock.json', + vulnerableRequirements: '= 1.8.2', + securityVulnerability: { + package: { + name: 'electron', + ecosystem: 'NPM', + }, + firstPatchedVersion: { + identifier: '1.8.8', + }, + vulnerableVersionRange: '>= 1.8, < 1.8.8', + }, + }, + { + dismissReason: null, + vulnerableManifestFilename: 'requirements.txt', + vulnerableManifestPath: 'requirements.txt', + vulnerableRequirements: '== 1.8.2', + securityVulnerability: { + package: { + name: 'ansible', + ecosystem: 'PIP', + }, + firstPatchedVersion: { + identifier: '1.8.8', + }, + vulnerableVersionRange: '>= 1.8, < 1.8.8', + }, + }, {}, ]); const res = await detectVulnerabilityAlerts(config); expect(res.packageRules).toMatchSnapshot(); - expect(res.packageRules).toHaveLength(1); + expect(res.packageRules).toHaveLength(4); }); }); });