From 6e84322e960a535ea14eee732d59c69f1a2ecbf0 Mon Sep 17 00:00:00 2001 From: ghe Date: Tue, 23 Mar 2021 18:57:46 +0000 Subject: [PATCH 1/6] feat: fix individual file with provenance --- .../extract-version-provenance.ts | 3 ++- .../python/handlers/pip-requirements/index.ts | 17 ++++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/extract-version-provenance.ts b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/extract-version-provenance.ts index b75f98d8ee..f623979553 100644 --- a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/extract-version-provenance.ts +++ b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/extract-version-provenance.ts @@ -20,7 +20,8 @@ export async function extractProvenance( fileName: string, provenance: PythonProvenance = {}, ): Promise { - const requirementsTxt = await workspace.readFile(path.join(dir, fileName)); + const requirementsFileName = path.join(dir, fileName); + const requirementsTxt = await workspace.readFile(requirementsFileName); provenance = { ...provenance, [fileName]: parseRequirementsFile(requirementsTxt), diff --git a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/index.ts b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/index.ts index 2b457a64e7..e12c2cacf7 100644 --- a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/index.ts +++ b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/index.ts @@ -1,4 +1,5 @@ import * as debugLib from 'debug'; +import * as pathLib from 'path'; import { EntityToFix, @@ -11,7 +12,7 @@ import { MissingRemediationDataError } from '../../../../lib/errors/missing-reme import { MissingFileNameError } from '../../../../lib/errors/missing-file-name'; import { partitionByFixable } from './is-supported'; import { NoFixesCouldBeAppliedError } from '../../../../lib/errors/no-fixes-applied'; -import { parseRequirementsFile } from './update-dependencies/requirements-file-parser'; +import { extractProvenance } from './extract-version-provenance'; const debug = debugLib('snyk-fix:python:requirements.txt'); @@ -53,17 +54,19 @@ export async function fixIndividualRequirementsTxt( if (!fileName) { throw new MissingFileNameError(); } - const requirementsTxt = await entity.workspace.readFile(fileName); - const requirementsData = parseRequirementsFile(requirementsTxt); - + const { dir, base } = pathLib.parse(fileName); + const versionProvenance = await extractProvenance( + entity.workspace, + dir, + base, + ); // TODO: allow handlers per fix type (later also strategies or combine with strategies) const { updatedManifest, changes } = updateDependencies( - requirementsData, + versionProvenance[base], remediationData.pin, ); - // TODO: do this with the changes now that we only return new - if (updatedManifest === requirementsTxt) { + if (!changes.length) { debug('Manifest has not changed!'); throw new NoFixesCouldBeAppliedError(); } From bc44f9a3cd461df6cd9a2e3efa5c9ce7e244043c Mon Sep 17 00:00:00 2001 From: ghe Date: Wed, 24 Mar 2021 17:35:22 +0000 Subject: [PATCH 2/6] refactor: fix individual reqs manifest --- .../extract-version-provenance.ts | 2 +- .../python/handlers/pip-requirements/index.ts | 74 ++++++++++++------- 2 files changed, 50 insertions(+), 26 deletions(-) diff --git a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/extract-version-provenance.ts b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/extract-version-provenance.ts index f623979553..2b6a61241f 100644 --- a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/extract-version-provenance.ts +++ b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/extract-version-provenance.ts @@ -8,7 +8,7 @@ import { import { Workspace } from '../../../../types'; import { containsRequireDirective } from './contains-require-directive'; -interface PythonProvenance { +export interface PythonProvenance { [fileName: string]: ParsedRequirements; } diff --git a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/index.ts b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/index.ts index e12c2cacf7..aa78a2cdd8 100644 --- a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/index.ts +++ b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/index.ts @@ -3,8 +3,10 @@ import * as pathLib from 'path'; import { EntityToFix, + FixChangesSummary, FixOptions, - WithFixChangesApplied, + RemediationChanges, + Workspace, } from '../../../../types'; import { PluginFixResponse } from '../../../types'; import { updateDependencies } from './update-dependencies'; @@ -12,7 +14,10 @@ import { MissingRemediationDataError } from '../../../../lib/errors/missing-reme import { MissingFileNameError } from '../../../../lib/errors/missing-file-name'; import { partitionByFixable } from './is-supported'; import { NoFixesCouldBeAppliedError } from '../../../../lib/errors/no-fixes-applied'; -import { extractProvenance } from './extract-version-provenance'; +import { + extractProvenance, + PythonProvenance, +} from './extract-version-provenance'; const debug = debugLib('snyk-fix:python:requirements.txt'); @@ -32,8 +37,18 @@ export async function pipRequirementsTxt( for (const entity of fixable) { try { - const fixedEntity = await fixIndividualRequirementsTxt(entity, options); - handlerResult.succeeded.push(fixedEntity); + const { remediation, targetFile, workspace } = getRequiredData(entity); + const { dir, base } = pathLib.parse(targetFile); + const provenance = await extractProvenance(workspace, dir, base); + const changes = await fixIndividualRequirementsTxt( + workspace, + dir, + base, + remediation, + provenance, + options, + ); + handlerResult.succeeded.push({ original: entity, changes }); } catch (e) { handlerResult.failed.push({ original: entity, error: e }); } @@ -41,29 +56,41 @@ export async function pipRequirementsTxt( return handlerResult; } -// TODO: optionally verify the deps install -export async function fixIndividualRequirementsTxt( +export function getRequiredData( entity: EntityToFix, - options: FixOptions, -): Promise> { - const fileName = entity.scanResult.identity.targetFile; - const remediationData = entity.testResult.remediation; - if (!remediationData) { +): { + remediation: RemediationChanges; + targetFile: string; + workspace: Workspace; +} { + const { remediation } = entity.testResult; + if (!remediation) { throw new MissingRemediationDataError(); } - if (!fileName) { + const { targetFile } = entity.scanResult.identity; + if (!targetFile) { throw new MissingFileNameError(); } - const { dir, base } = pathLib.parse(fileName); - const versionProvenance = await extractProvenance( - entity.workspace, - dir, - base, - ); + const { workspace } = entity; + if (!workspace) { + throw new NoFixesCouldBeAppliedError(); + } + return { targetFile, remediation, workspace }; +} + +// TODO: optionally verify the deps install +export async function fixIndividualRequirementsTxt( + workspace: Workspace, + dir: string, + fileName: string, + remediation: RemediationChanges, + provenance: PythonProvenance, + options: FixOptions, +): Promise { // TODO: allow handlers per fix type (later also strategies or combine with strategies) const { updatedManifest, changes } = updateDependencies( - versionProvenance[base], - remediationData.pin, + provenance[fileName], + remediation.pin, ); if (!changes.length) { @@ -72,13 +99,10 @@ export async function fixIndividualRequirementsTxt( } if (!options.dryRun) { debug('Writing changes to file'); - await entity.workspace.writeFile(fileName, updatedManifest); + await workspace.writeFile(pathLib.join(dir, fileName), updatedManifest); } else { debug('Skipping writing changes to file in --dry-run mode'); } - return { - original: entity, - changes, - }; + return changes; } From 66ca77a8af26e4eac5f9809aa467b0ad91113fc0 Mon Sep 17 00:00:00 2001 From: ghe Date: Thu, 25 Mar 2021 11:33:44 +0000 Subject: [PATCH 3/6] feat: do not skip files with -r directive --- .../handlers/pip-requirements/is-supported.ts | 8 +++++--- .../update-dependencies.spec.ts | 20 +++++++++++++------ .../{is-supported.ts => is-supported.spec.ts} | 4 ++-- 3 files changed, 21 insertions(+), 11 deletions(-) rename packages/snyk-fix/test/unit/plugins/python/{is-supported.ts => is-supported.spec.ts} (92%) diff --git a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/is-supported.ts b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/is-supported.ts index 25ce4b9cff..35f14f2913 100644 --- a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/is-supported.ts +++ b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/is-supported.ts @@ -45,11 +45,13 @@ export async function isSupported( }; } - const { containsRequire } = await containsRequireDirective(requirementsTxt); - if (containsRequire) { + const { containsRequire, matches } = await containsRequireDirective( + requirementsTxt, + ); + if (containsRequire && matches.some((m) => m.includes('c'))) { return { supported: false, - reason: `Requirements with ${chalk.bold('-r')} or ${chalk.bold( + reason: `Requirements with ${chalk.bold( '-c', )} directive are not yet supported`, }; diff --git a/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/update-dependencies.spec.ts b/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/update-dependencies.spec.ts index 04fe080ad0..a637ca42f8 100644 --- a/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/update-dependencies.spec.ts +++ b/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/update-dependencies.spec.ts @@ -12,7 +12,8 @@ describe('fix *req*.txt / *.txt Python projects', () => { filesToDelete.map((f) => fs.unlinkSync(f)); }); const workspacesPath = pathLib.resolve(__dirname, 'workspaces'); - it('skips projects with a -r option', async () => { + + it('fixes project with a -r option', async () => { // Arrange const targetFile = 'with-require/dev.txt'; @@ -72,15 +73,22 @@ describe('fix *req*.txt / *.txt Python projects', () => { results: { python: { failed: [], - skipped: [ + skipped: [], + succeeded: [ { original: entityToFix, - userMessage: expect.stringContaining( - 'directive are not yet supported', - ), + changes: [ + { + success: true, + userMessage: 'Pinned transitive from 1.0.0 to 1.1.1', + }, + { + success: true, + userMessage: 'Upgraded Django from 1.6.1 to 2.0.1', + }, + ], }, ], - succeeded: [], }, }, }); diff --git a/packages/snyk-fix/test/unit/plugins/python/is-supported.ts b/packages/snyk-fix/test/unit/plugins/python/is-supported.spec.ts similarity index 92% rename from packages/snyk-fix/test/unit/plugins/python/is-supported.ts rename to packages/snyk-fix/test/unit/plugins/python/is-supported.spec.ts index fbb2210090..24cc508b40 100644 --- a/packages/snyk-fix/test/unit/plugins/python/is-supported.ts +++ b/packages/snyk-fix/test/unit/plugins/python/is-supported.spec.ts @@ -13,14 +13,14 @@ describe('isSupported', () => { const res = await isSupported(entity); expect(res.supported).toBeFalsy(); }); - it('with -r directive in the manifest not supported', async () => { + it('with -r directive in the manifest is supported', async () => { const entity = generateEntityToFix( 'pip', 'requirements.txt', '-r prod.txt\nDjango==1.6.1', ); const res = await isSupported(entity); - expect(res.supported).toBeFalsy(); + expect(res.supported).toBeTruthy(); }); it('with -c directive in the manifest not supported', async () => { const entity = generateEntityToFix( From f94c558f573b3600004d87deaa33d021aed8a3c0 Mon Sep 17 00:00:00 2001 From: ghe Date: Thu, 25 Mar 2021 14:50:43 +0000 Subject: [PATCH 4/6] feat: include pins optionally --- .../update-dependencies/index.ts | 15 +++++--- .../update-dependencies.spec.ts | 36 +++++++++++++++++++ 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/update-dependencies/index.ts b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/update-dependencies/index.ts index 383d90d6af..9edda80154 100644 --- a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/update-dependencies/index.ts +++ b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/update-dependencies/index.ts @@ -19,6 +19,7 @@ const debug = debugLib('snyk-fix:python:update-dependencies'); export function updateDependencies( parsedRequirementsData: ParsedRequirements, updates: DependencyPins, + directUpgradesOnly = false, ): { updatedManifest: string; changes: FixChangesSummary[] } { const { requirements, @@ -38,11 +39,15 @@ export function updateDependencies( ); debug('Finished generating upgrades to apply'); - const { pinnedRequirements, changes: pinChanges } = generatePins( - requirements, - updates, - ); - debug('Finished generating pins to apply'); + let pinnedRequirements: string[] = []; + let pinChanges: FixChangesSummary[] = []; + if (!directUpgradesOnly) { + ({ pinnedRequirements, changes: pinChanges } = generatePins( + requirements, + updates, + )); + debug('Finished generating pins to apply'); + } let updatedManifest = [ ...applyUpgrades(requirements, updatedRequirements), diff --git a/packages/snyk-fix/test/unit/plugins/python/handlers/update-dependencies/update-dependencies.spec.ts b/packages/snyk-fix/test/unit/plugins/python/handlers/update-dependencies/update-dependencies.spec.ts index 2b65831213..133a9455d2 100644 --- a/packages/snyk-fix/test/unit/plugins/python/handlers/update-dependencies/update-dependencies.spec.ts +++ b/packages/snyk-fix/test/unit/plugins/python/handlers/update-dependencies/update-dependencies.spec.ts @@ -306,4 +306,40 @@ describe('remediation', () => { ); } }); + it('skips pins if asked', () => { + const upgrades = { + 'django@1.6.1': { + upgradeTo: 'django@2.0.1', + vulns: [], + upgrades: [], + isTransitive: false, + }, + 'transitive@1.0.0': { + upgradeTo: 'transitive@1.1.1', + vulns: [], + upgrades: [], + isTransitive: true, + }, + }; + + const manifestContents = 'Django==1.6.1'; + + const expectedManifest = + 'Django==2.0.1\ntransitive>=1.1.1 # not directly required, pinned by Snyk to avoid a vulnerability'; + const directUpgradesOnly = false; + const requirements = parseRequirementsFile(manifestContents); + const result = updateDependencies( + requirements, + upgrades, + directUpgradesOnly, + ); + expect(result.changes.map((c) => c.userMessage).sort()).toEqual( + [ + 'Pinned transitive from 1.0.0 to 1.1.1', + 'Upgraded Django from 1.6.1 to 2.0.1', + ].sort(), + ); + // Note no extra newline was added to the expected manifest + expect(result.updatedManifest).toEqual(expectedManifest); + }); }); From 0384020f20fc71f1ba06a2b3d0a12f499e21a65f Mon Sep 17 00:00:00 2001 From: ghe Date: Fri, 26 Mar 2021 13:42:46 +0000 Subject: [PATCH 5/6] feat: basic pip fix -r support --- .../src/lib/errors/invalid-workspace.ts | 10 ++ .../python/handlers/pip-requirements/index.ts | 117 ++++++++++++--- .../update-dependencies/generate-pins.ts | 10 +- .../update-dependencies/generate-upgrades.ts | 17 ++- .../update-dependencies/index.ts | 27 ++-- .../update-dependencies.spec.ts.snap | 13 ++ .../update-dependencies.spec.ts | 139 ++++++++++++------ .../workspaces/pip-app/requirements.txt | 1 + packages/snyk-fix/test/unit/fix.spec.ts | 2 +- 9 files changed, 258 insertions(+), 78 deletions(-) create mode 100644 packages/snyk-fix/src/lib/errors/invalid-workspace.ts diff --git a/packages/snyk-fix/src/lib/errors/invalid-workspace.ts b/packages/snyk-fix/src/lib/errors/invalid-workspace.ts new file mode 100644 index 0000000000..6150766d05 --- /dev/null +++ b/packages/snyk-fix/src/lib/errors/invalid-workspace.ts @@ -0,0 +1,10 @@ +import { CustomError, ERROR_CODES } from './custom-error'; + +export class MissingFileNameError extends CustomError { + public constructor() { + super( + 'Filename is missing from test result. Please contact support@snyk.io.', + ERROR_CODES.MissingFileName, + ); + } +} diff --git a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/index.ts b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/index.ts index aa78a2cdd8..d57d45acdd 100644 --- a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/index.ts +++ b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/index.ts @@ -2,6 +2,7 @@ import * as debugLib from 'debug'; import * as pathLib from 'path'; import { + DependencyPins, EntityToFix, FixChangesSummary, FixOptions, @@ -18,6 +19,11 @@ import { extractProvenance, PythonProvenance, } from './extract-version-provenance'; +import { + ParsedRequirements, + parseRequirementsFile, + Requirement, +} from './update-dependencies/requirements-file-parser'; const debug = debugLib('snyk-fix:python:requirements.txt'); @@ -37,17 +43,18 @@ export async function pipRequirementsTxt( for (const entity of fixable) { try { - const { remediation, targetFile, workspace } = getRequiredData(entity); - const { dir, base } = pathLib.parse(targetFile); - const provenance = await extractProvenance(workspace, dir, base); - const changes = await fixIndividualRequirementsTxt( - workspace, - dir, - base, - remediation, - provenance, + const { changes } = await applyAllFixes( + entity, + // dir, + // base, + // remediation, + // provenance, options, ); + if (!changes.length) { + debug('Manifest has not changed!'); + throw new NoFixesCouldBeAppliedError(); + } handlerResult.succeeded.push({ original: entity, changes }); } catch (e) { handlerResult.failed.push({ original: entity, error: e }); @@ -82,27 +89,95 @@ export function getRequiredData( export async function fixIndividualRequirementsTxt( workspace: Workspace, dir: string, + entryFileName: string, fileName: string, remediation: RemediationChanges, - provenance: PythonProvenance, + parsedRequirements: ParsedRequirements, options: FixOptions, -): Promise { - // TODO: allow handlers per fix type (later also strategies or combine with strategies) - const { updatedManifest, changes } = updateDependencies( - provenance[fileName], + directUpgradesOnly: boolean, +): Promise<{ changes: FixChangesSummary[]; appliedRemediation: string[] }> { + const fullFilePath = pathLib.join(dir, fileName); + const { updatedManifest, changes, appliedRemediation } = updateDependencies( + parsedRequirements, remediation.pin, + directUpgradesOnly, + pathLib.join(dir, entryFileName) !== fullFilePath ? fileName : undefined, ); - - if (!changes.length) { - debug('Manifest has not changed!'); - throw new NoFixesCouldBeAppliedError(); - } - if (!options.dryRun) { + if (!options.dryRun && changes.length > 0) { debug('Writing changes to file'); await workspace.writeFile(pathLib.join(dir, fileName), updatedManifest); } else { debug('Skipping writing changes to file in --dry-run mode'); } - return changes; + return { changes, appliedRemediation }; +} + +export async function applyAllFixes( + entity: EntityToFix, + options: FixOptions, +): Promise<{ changes: FixChangesSummary[] }> { + const { remediation, targetFile: entryFileName, workspace } = getRequiredData( + entity, + ); + const { dir, base } = pathLib.parse(entryFileName); + const provenance = await extractProvenance(workspace, dir, base); + const upgradeChanges: FixChangesSummary[] = []; + const appliedUpgradeRemediation: string[] = []; + for (const fileName of Object.keys(provenance)) { + const skipApplyingPins = true; + const { changes, appliedRemediation } = await fixIndividualRequirementsTxt( + workspace, + dir, + base, + fileName, + remediation, + provenance[fileName], + options, + skipApplyingPins, + ); + appliedUpgradeRemediation.push(...appliedRemediation); + // what if we saw the file before and already fixed it? + upgradeChanges.push(...changes); + } + // now do left overs as pins + add tests + const requirementsTxt = await workspace.readFile(entryFileName); + + const toPin: RemediationChanges = filterOutAppliedUpgrades( + remediation, + appliedUpgradeRemediation, + ); + const directUpgradesOnly = false; + const { changes: pinnedChanges } = await fixIndividualRequirementsTxt( + workspace, + dir, + base, + base, + toPin, + parseRequirementsFile(requirementsTxt), + options, + directUpgradesOnly, + ); + + return { changes: [...upgradeChanges, ...pinnedChanges] }; +} + +function filterOutAppliedUpgrades( + remediation: RemediationChanges, + appliedRemediation: string[], +): RemediationChanges { + const pinRemediation: RemediationChanges = { + ...remediation, + pin: {}, // delete the pin remediation so we can add only not applied + }; + const pins = remediation.pin; + const lowerCasedAppliedRemediation = appliedRemediation.map((i) => + i.toLowerCase(), + ); + for (const pkgAtVersion of Object.keys(pins)) { + if (!lowerCasedAppliedRemediation.includes(pkgAtVersion.toLowerCase())) { + pinRemediation.pin[pkgAtVersion] = pins[pkgAtVersion]; + } + } + return pinRemediation; } diff --git a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/update-dependencies/generate-pins.ts b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/update-dependencies/generate-pins.ts index 779c50a2d8..dc43ee4b2a 100644 --- a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/update-dependencies/generate-pins.ts +++ b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/update-dependencies/generate-pins.ts @@ -6,7 +6,11 @@ import { Requirement } from './requirements-file-parser'; export function generatePins( requirements: Requirement[], updates: DependencyPins, -): { pinnedRequirements: string[]; changes: FixChangesSummary[] } { +): { + pinnedRequirements: string[]; + changes: FixChangesSummary[]; + appliedRemediation: string[]; +} { // Lowercase the upgrades object. This might be overly defensive, given that // we control this input internally, but its a low cost guard rail. Outputs a // mapping of upgrade to -> from, instead of the nested upgradeTo object. @@ -20,8 +24,10 @@ export function generatePins( return { pinnedRequirements: [], changes: [], + appliedRemediation: [], }; } + const appliedRemediation: string[] = []; const changes: FixChangesSummary[] = []; const pinnedRequirements = Object.keys(lowerCasedPins) .map((pkgNameAtVersion) => { @@ -32,6 +38,7 @@ export function generatePins( success: true, userMessage: `Pinned ${pkgName} from ${version} to ${newVersion}`, }); + appliedRemediation.push(pkgNameAtVersion); return `${newRequirement} # not directly required, pinned by Snyk to avoid a vulnerability`; }) .filter(isDefined); @@ -39,5 +46,6 @@ export function generatePins( return { pinnedRequirements, changes, + appliedRemediation, }; } diff --git a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/update-dependencies/generate-upgrades.ts b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/update-dependencies/generate-upgrades.ts index fe37307dc7..62a5169bbc 100644 --- a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/update-dependencies/generate-upgrades.ts +++ b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/update-dependencies/generate-upgrades.ts @@ -6,7 +6,12 @@ import { UpgradedRequirements } from './types'; export function generateUpgrades( requirements: Requirement[], updates: DependencyPins, -): { updatedRequirements: UpgradedRequirements; changes: FixChangesSummary[] } { + referenceFileInChanges?: string, +): { + updatedRequirements: UpgradedRequirements; + changes: FixChangesSummary[]; + appliedRemediation: string[]; +} { // Lowercase the upgrades object. This might be overly defensive, given that // we control this input internally, but its a low cost guard rail. Outputs a // mapping of upgrade to -> from, instead of the nested upgradeTo object. @@ -19,9 +24,11 @@ export function generateUpgrades( return { updatedRequirements: {}, changes: [], + appliedRemediation: [], }; } + const appliedRemediation: string[] = []; const changes: FixChangesSummary[] = []; const updatedRequirements = {}; requirements.map( @@ -58,16 +65,22 @@ export function generateUpgrades( const updatedRequirement = `${originalName}${versionComparator}${newVersion}`; changes.push({ success: true, - userMessage: `Upgraded ${originalName} from ${version} to ${newVersion}`, + userMessage: `Upgraded ${originalName} from ${version} to ${newVersion}${ + referenceFileInChanges + ? ` (upgraded in ${referenceFileInChanges})` + : '' + }`, }); updatedRequirements[originalText] = `${updatedRequirement}${ extras ? extras : '' }`; + appliedRemediation.push(upgrade); }, ); return { updatedRequirements, changes, + appliedRemediation, }; } diff --git a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/update-dependencies/index.ts b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/update-dependencies/index.ts index 9edda80154..5b0fdda2c7 100644 --- a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/update-dependencies/index.ts +++ b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/update-dependencies/index.ts @@ -20,7 +20,12 @@ export function updateDependencies( parsedRequirementsData: ParsedRequirements, updates: DependencyPins, directUpgradesOnly = false, -): { updatedManifest: string; changes: FixChangesSummary[] } { + referenceFileInChanges?: string, +): { + updatedManifest: string; + changes: FixChangesSummary[]; + appliedRemediation: string[]; +} { const { requirements, endsWithNewLine: shouldEndWithNewLine, @@ -33,19 +38,22 @@ export function updateDependencies( } debug('Finished parsing manifest'); - const { updatedRequirements, changes: upgradedChanges } = generateUpgrades( - requirements, - updates, - ); + const { + updatedRequirements, + changes: upgradedChanges, + appliedRemediation, + } = generateUpgrades(requirements, updates, referenceFileInChanges); debug('Finished generating upgrades to apply'); let pinnedRequirements: string[] = []; let pinChanges: FixChangesSummary[] = []; + let appliedPinsRemediation: string[] = []; if (!directUpgradesOnly) { - ({ pinnedRequirements, changes: pinChanges } = generatePins( - requirements, - updates, - )); + ({ + pinnedRequirements, + changes: pinChanges, + appliedRemediation: appliedPinsRemediation, + } = generatePins(requirements, updates)); debug('Finished generating pins to apply'); } @@ -64,5 +72,6 @@ export function updateDependencies( return { updatedManifest, changes: [...pinChanges, ...upgradedChanges], + appliedRemediation: [...appliedPinsRemediation, ...appliedRemediation], }; } diff --git a/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/__snapshots__/update-dependencies.spec.ts.snap b/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/__snapshots__/update-dependencies.spec.ts.snap index 37ceccdf94..980322383a 100644 --- a/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/__snapshots__/update-dependencies.spec.ts.snap +++ b/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/__snapshots__/update-dependencies.spec.ts.snap @@ -1,5 +1,18 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`fix *req*.txt / *.txt Python projects fixes multiple files that are included via -r 1`] = ` +Array [ + Object { + "success": true, + "userMessage": "Upgraded Django from 1.6.1 to 2.0.1", + }, + Object { + "success": true, + "userMessage": "Upgraded Jinja2 from 2.7.2 to 2.7.3 (upgraded in base2.txt)", + }, +] +`; + exports[`fix *req*.txt / *.txt Python projects retains python markers 1`] = ` "amqp==2.4.2 apscheduler==3.6.0 diff --git a/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/update-dependencies.spec.ts b/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/update-dependencies.spec.ts index a637ca42f8..bb3d000608 100644 --- a/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/update-dependencies.spec.ts +++ b/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/update-dependencies.spec.ts @@ -42,10 +42,7 @@ describe('fix *req*.txt / *.txt Python projects', () => { const entityToFix = { workspace: { readFile: async (path: string) => { - return fs.readFileSync( - pathLib.resolve(workspacesPath, path), - 'utf-8', - ); + return readFileHelper(workspacesPath, path); }, writeFile: async (path: string, contents: string) => { const res = pathLib.parse(path); @@ -80,11 +77,11 @@ describe('fix *req*.txt / *.txt Python projects', () => { changes: [ { success: true, - userMessage: 'Pinned transitive from 1.0.0 to 1.1.1', + userMessage: 'Upgraded Django from 1.6.1 to 2.0.1', }, { success: true, - userMessage: 'Upgraded Django from 1.6.1 to 2.0.1', + userMessage: 'Pinned transitive from 1.0.0 to 1.1.1', }, ], }, @@ -126,10 +123,7 @@ describe('fix *req*.txt / *.txt Python projects', () => { const entityToFix = { workspace: { readFile: async (path: string) => { - return fs.readFileSync( - pathLib.resolve(workspacesPath, path), - 'utf-8', - ); + return readFileHelper(workspacesPath, path); }, writeFile: async (path: string, contents: string) => { const res = pathLib.parse(path); @@ -170,11 +164,11 @@ describe('fix *req*.txt / *.txt Python projects', () => { changes: [ { success: true, - userMessage: 'Pinned transitive from 1.0.0 to 1.1.1', + userMessage: 'Upgraded Django from 1.6.1 to 2.0.1', }, { success: true, - userMessage: 'Upgraded Django from 1.6.1 to 2.0.1', + userMessage: 'Pinned transitive from 1.0.0 to 1.1.1', }, ], }, @@ -217,10 +211,7 @@ describe('fix *req*.txt / *.txt Python projects', () => { const entityToFix = { workspace: { readFile: async (path: string) => { - return fs.readFileSync( - pathLib.resolve(workspacesPath, path), - 'utf-8', - ); + return readFileHelper(workspacesPath, path); }, writeFile: async (path: string, contents: string) => { const res = pathLib.parse(path); @@ -261,11 +252,11 @@ describe('fix *req*.txt / *.txt Python projects', () => { changes: [ { success: true, - userMessage: 'Pinned transitive from 1.0.0 to 1.1.1', + userMessage: 'Upgraded Django from 1.6.1 to 2.0.1', }, { success: true, - userMessage: 'Upgraded Django from 1.6.1 to 2.0.1', + userMessage: 'Pinned transitive from 1.0.0 to 1.1.1', }, ], }, @@ -308,10 +299,7 @@ describe('fix *req*.txt / *.txt Python projects', () => { const entityToFix = { workspace: { readFile: async (path: string) => { - return fs.readFileSync( - pathLib.resolve(workspacesPath, path), - 'utf-8', - ); + return readFileHelper(workspacesPath, path); }, writeFile: async (path: string, contents: string) => { const res = pathLib.parse(path); @@ -353,11 +341,11 @@ describe('fix *req*.txt / *.txt Python projects', () => { changes: [ { success: true, - userMessage: 'Pinned transitive from 1.0.0 to 1.1.1', + userMessage: 'Upgraded Django from 1.6.1 to 2.0.1', }, { success: true, - userMessage: 'Upgraded Django from 1.6.1 to 2.0.1', + userMessage: 'Pinned transitive from 1.0.0 to 1.1.1', }, ], }, @@ -395,10 +383,7 @@ describe('fix *req*.txt / *.txt Python projects', () => { const entityToFix = { workspace: { readFile: async (path: string) => { - return fs.readFileSync( - pathLib.resolve(workspacesPath, path), - 'utf-8', - ); + return readFileHelper(workspacesPath, path); }, writeFile: async (path: string, contents: string) => { const res = pathLib.parse(path); @@ -478,10 +463,7 @@ describe('fix *req*.txt / *.txt Python projects', () => { const entityToFix = { workspace: { readFile: async (path: string) => { - return fs.readFileSync( - pathLib.resolve(workspacesPath, path), - 'utf-8', - ); + return readFileHelper(workspacesPath, path); }, writeFile: async (path: string, contents: string) => { const res = pathLib.parse(path); @@ -560,10 +542,7 @@ describe('fix *req*.txt / *.txt Python projects', () => { const entityToFix = { workspace: { readFile: async (path: string) => { - return fs.readFileSync( - pathLib.resolve(workspacesPath, path), - 'utf-8', - ); + return readFileHelper(workspacesPath, path); }, writeFile: async (path: string, contents: string) => { const res = pathLib.parse(path); @@ -647,10 +626,7 @@ describe('fix *req*.txt / *.txt Python projects', () => { const entityToFix = { workspace: { readFile: async (path: string) => { - return fs.readFileSync( - pathLib.resolve(workspacesPath, path), - 'utf-8', - ); + return readFileHelper(workspacesPath, path); }, writeFile: async (path: string, contents: string) => { const res = pathLib.parse(path); @@ -732,10 +708,7 @@ describe('fix *req*.txt / *.txt Python projects', () => { const entityToFix = { workspace: { readFile: async (path: string) => { - return fs.readFileSync( - pathLib.resolve(workspacesPath, path), - 'utf-8', - ); + return readFileHelper(workspacesPath, path); }, writeFile: async (path: string, contents: string) => { const res = pathLib.parse(path); @@ -782,4 +755,82 @@ describe('fix *req*.txt / *.txt Python projects', () => { }, }); }); + it('fixes multiple files that are included via -r', async () => { + // Arrange + const targetFile = 'pip-app/requirements.txt'; + filesToDelete = [ + pathLib.resolve(workspacesPath, 'pip-app/fixed-requirements.txt'), + pathLib.resolve(workspacesPath, 'pip-app/fixed-base2.txt'), + ]; + const testResult = { + ...generateTestResult(), + remediation: { + unresolved: [], + upgrade: {}, + patch: {}, + ignore: {}, + pin: { + 'django@1.6.1': { + upgradeTo: 'django@2.0.1', + vulns: [], + isTransitive: false, + }, + 'Jinja2@2.7.2': { + upgradeTo: 'Jinja2@2.7.3', + vulns: [], + isTransitive: true, + }, + }, + }, + }; + const entityToFix = { + workspace: { + readFile: async (path: string) => { + return readFileHelper(workspacesPath, path); + }, + writeFile: async (path: string, contents: string) => { + const res = pathLib.parse(path); + const fixedPath = pathLib.resolve( + workspacesPath, + res.dir, + `fixed-${res.base}`, + ); + fs.writeFileSync(fixedPath, contents, 'utf-8'); + }, + }, + scanResult: generateScanResult('pip', targetFile), + testResult, + }; + const writeFileSpy = jest.spyOn(entityToFix.workspace, 'writeFile'); + + // Act + const result = await snykFix.fix([entityToFix], { + quiet: true, + stripAnsi: true, + }); + + // 2 files needed to have changes + expect(writeFileSpy).toHaveBeenCalledTimes(2); + expect(result.results.python.succeeded[0].original).toEqual(entityToFix); + expect(result.results.python.succeeded[0].changes).toMatchSnapshot(); + }); }); + +function readFileHelper(workspacesPath: string, path: string): string { + // because we write multiple time the file + // may be have already been updated in fixed-* name + // so try read that first + const res = pathLib.parse(path); + const fixedPath = pathLib.resolve( + workspacesPath, + res.dir, + `fixed-${res.base}`, + ); + let file; + try { + file = fs.readFileSync(fixedPath, 'utf-8'); + } catch (e) { + file = fs.readFileSync(pathLib.resolve(workspacesPath, path), 'utf-8'); + } + return file; +} diff --git a/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/workspaces/pip-app/requirements.txt b/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/workspaces/pip-app/requirements.txt index a8d089e1c8..44d5b49554 100644 --- a/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/workspaces/pip-app/requirements.txt +++ b/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/workspaces/pip-app/requirements.txt @@ -1,2 +1,3 @@ -r base.txt +-r base2.txt Django==1.6.1 diff --git a/packages/snyk-fix/test/unit/fix.spec.ts b/packages/snyk-fix/test/unit/fix.spec.ts index 6eb4df456b..b0d27fed6e 100644 --- a/packages/snyk-fix/test/unit/fix.spec.ts +++ b/packages/snyk-fix/test/unit/fix.spec.ts @@ -17,7 +17,7 @@ describe('Snyk fix', () => { }); // Assert - expect(writeFileSpy).toHaveBeenCalled(); + expect(writeFileSpy).toHaveBeenCalledTimes(1); expect(res.exceptions).toMatchSnapshot(); expect(res.results).toMatchSnapshot(); }); From b286418cc7ce4c655925bf63010f5f30c5dd481c Mon Sep 17 00:00:00 2001 From: ghe Date: Fri, 26 Mar 2021 17:25:14 +0000 Subject: [PATCH 6/6] feat: v1 support for previously fixed reqs.txt --- .../python/handlers/pip-requirements/index.ts | 122 +++++-- .../update-dependencies.spec.ts.snap | 37 ++ .../update-dependencies.spec.ts | 328 ++++++++---------- .../app-with-already-fixed/base.txt | 1 + .../core/requirements.txt | 1 + .../lib/requirements.txt | 1 + .../app-with-already-fixed/requirements.txt | 4 + 7 files changed, 283 insertions(+), 211 deletions(-) create mode 100644 packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/workspaces/app-with-already-fixed/base.txt create mode 100644 packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/workspaces/app-with-already-fixed/core/requirements.txt create mode 100644 packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/workspaces/app-with-already-fixed/lib/requirements.txt create mode 100644 packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/workspaces/app-with-already-fixed/requirements.txt diff --git a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/index.ts b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/index.ts index d57d45acdd..4467986837 100644 --- a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/index.ts +++ b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/index.ts @@ -1,8 +1,9 @@ import * as debugLib from 'debug'; import * as pathLib from 'path'; +const sortBy = require('lodash.sortby'); +const groupBy = require('lodash.groupby'); import { - DependencyPins, EntityToFix, FixChangesSummary, FixOptions, @@ -15,14 +16,10 @@ import { MissingRemediationDataError } from '../../../../lib/errors/missing-reme import { MissingFileNameError } from '../../../../lib/errors/missing-file-name'; import { partitionByFixable } from './is-supported'; import { NoFixesCouldBeAppliedError } from '../../../../lib/errors/no-fixes-applied'; -import { - extractProvenance, - PythonProvenance, -} from './extract-version-provenance'; +import { extractProvenance } from './extract-version-provenance'; import { ParsedRequirements, parseRequirementsFile, - Requirement, } from './update-dependencies/requirements-file-parser'; const debug = debugLib('snyk-fix:python:requirements.txt'); @@ -38,27 +35,23 @@ export async function pipRequirementsTxt( skipped: [], }; - const { fixable, skipped } = await partitionByFixable(entities); - handlerResult.skipped.push(...skipped); + const { fixable, skipped: notFixable } = await partitionByFixable(entities); + handlerResult.skipped.push(...notFixable); - for (const entity of fixable) { - try { - const { changes } = await applyAllFixes( - entity, - // dir, - // base, - // remediation, - // provenance, - options, - ); - if (!changes.length) { - debug('Manifest has not changed!'); - throw new NoFixesCouldBeAppliedError(); - } - handlerResult.succeeded.push({ original: entity, changes }); - } catch (e) { - handlerResult.failed.push({ original: entity, error: e }); - } + const ordered = sortByDirectory(fixable); + const fixedFilesCache: string[] = []; + for (const dir of Object.keys(ordered)) { + debug(`Fixing entities in directory ${dir}`); + const entitiesPerDirectory = ordered[dir].map((e) => e.entity); + const { failed, succeeded, skipped, fixedFiles } = await fixAll( + entitiesPerDirectory, + options, + fixedFilesCache, + ); + fixedFilesCache.push(...fixedFiles); + handlerResult.succeeded.push(...succeeded); + handlerResult.failed.push(...failed); + handlerResult.skipped.push(...skipped); } return handlerResult; } @@ -85,6 +78,42 @@ export function getRequiredData( return { targetFile, remediation, workspace }; } +async function fixAll( + entities: EntityToFix[], + options: FixOptions, + fixedCache: string[], +): Promise { + const handlerResult: PluginFixResponse = { + succeeded: [], + failed: [], + skipped: [], + }; + for (const entity of entities) { + const targetFile = entity.scanResult.identity.targetFile!; + try { + const { dir, base } = pathLib.parse(targetFile); + // parse & join again to support correct separator + if (fixedCache.includes(pathLib.join(dir, base))) { + handlerResult.succeeded.push({ + original: entity, + changes: [{ success: true, userMessage: 'Previously fixed' }], + }); + continue; + } + const { changes, fixedFiles } = await applyAllFixes(entity, options); + if (!changes.length) { + debug('Manifest has not changed!'); + throw new NoFixesCouldBeAppliedError(); + } + fixedCache.push(...fixedFiles); + handlerResult.succeeded.push({ original: entity, changes }); + } catch (e) { + debug(`Failed to fix ${targetFile}.\nERROR: ${e}`); + handlerResult.failed.push({ original: entity, error: e }); + } + } + return { ...handlerResult, fixedFiles: [] }; +} // TODO: optionally verify the deps install export async function fixIndividualRequirementsTxt( workspace: Workspace, @@ -103,7 +132,12 @@ export async function fixIndividualRequirementsTxt( directUpgradesOnly, pathLib.join(dir, entryFileName) !== fullFilePath ? fileName : undefined, ); - if (!options.dryRun && changes.length > 0) { + + if (!changes.length) { + return { changes, appliedRemediation }; + } + + if (!options.dryRun) { debug('Writing changes to file'); await workspace.writeFile(pathLib.join(dir, fileName), updatedManifest); } else { @@ -116,14 +150,16 @@ export async function fixIndividualRequirementsTxt( export async function applyAllFixes( entity: EntityToFix, options: FixOptions, -): Promise<{ changes: FixChangesSummary[] }> { +): Promise<{ changes: FixChangesSummary[]; fixedFiles: string[] }> { const { remediation, targetFile: entryFileName, workspace } = getRequiredData( entity, ); + const fixedFiles: string[] = []; const { dir, base } = pathLib.parse(entryFileName); const provenance = await extractProvenance(workspace, dir, base); const upgradeChanges: FixChangesSummary[] = []; const appliedUpgradeRemediation: string[] = []; + /* Apply all upgrades first across all files that are included */ for (const fileName of Object.keys(provenance)) { const skipApplyingPins = true; const { changes, appliedRemediation } = await fixIndividualRequirementsTxt( @@ -137,10 +173,11 @@ export async function applyAllFixes( skipApplyingPins, ); appliedUpgradeRemediation.push(...appliedRemediation); - // what if we saw the file before and already fixed it? upgradeChanges.push(...changes); + fixedFiles.push(pathLib.join(dir, fileName)); } - // now do left overs as pins + add tests + + /* Apply all left over remediation as pins in the entry targetFile */ const requirementsTxt = await workspace.readFile(entryFileName); const toPin: RemediationChanges = filterOutAppliedUpgrades( @@ -159,7 +196,7 @@ export async function applyAllFixes( directUpgradesOnly, ); - return { changes: [...upgradeChanges, ...pinnedChanges] }; + return { changes: [...upgradeChanges, ...pinnedChanges], fixedFiles }; } function filterOutAppliedUpgrades( @@ -168,7 +205,7 @@ function filterOutAppliedUpgrades( ): RemediationChanges { const pinRemediation: RemediationChanges = { ...remediation, - pin: {}, // delete the pin remediation so we can add only not applied + pin: {}, // delete the pin remediation so we can collect un-applied remediation }; const pins = remediation.pin; const lowerCasedAppliedRemediation = appliedRemediation.map((i) => @@ -181,3 +218,24 @@ function filterOutAppliedUpgrades( } return pinRemediation; } + +function sortByDirectory( + entities: EntityToFix[], +): { + [dir: string]: Array<{ + entity: EntityToFix; + dir: string; + base: string; + ext: string; + root: string; + name: string; + }>; +} { + const mapped = entities.map((e) => ({ + entity: e, + ...pathLib.parse(e.scanResult.identity.targetFile!), + })); + + const sorted = sortBy(mapped, 'dir'); + return groupBy(sorted, 'dir'); +} diff --git a/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/__snapshots__/update-dependencies.spec.ts.snap b/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/__snapshots__/update-dependencies.spec.ts.snap index 980322383a..d554e57d51 100644 --- a/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/__snapshots__/update-dependencies.spec.ts.snap +++ b/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/__snapshots__/update-dependencies.spec.ts.snap @@ -13,6 +13,43 @@ Array [ ] `; +exports[`fix *req*.txt / *.txt Python projects fixes multiple files via -r with the same name (some were already fixed) 1`] = ` +"Successful fixes: + + app-with-already-fixed/requirements.txt + ✔ Upgraded Django from 1.6.1 to 2.0.1 + ✔ Upgraded Django from 1.6.1 to 2.0.1 (upgraded in core/requirements.txt) + ✔ Upgraded Jinja2 from 2.7.2 to 2.7.3 (upgraded in lib/requirements.txt) + + app-with-already-fixed/core/requirements.txt + ✔ Previously fixed + + app-with-already-fixed/lib/requirements.txt + ✔ Previously fixed + +Summary: + + 0 items were not fixed + 3 items were successfully fixed" +`; + +exports[`fix *req*.txt / *.txt Python projects fixes multiple files via -r with the same name (some were already fixed) 2`] = ` +Array [ + Object { + "success": true, + "userMessage": "Upgraded Django from 1.6.1 to 2.0.1", + }, + Object { + "success": true, + "userMessage": "Upgraded Django from 1.6.1 to 2.0.1 (upgraded in core/requirements.txt)", + }, + Object { + "success": true, + "userMessage": "Upgraded Jinja2 from 2.7.2 to 2.7.3 (upgraded in lib/requirements.txt)", + }, +] +`; + exports[`fix *req*.txt / *.txt Python projects retains python markers 1`] = ` "amqp==2.4.2 apscheduler==3.6.0 diff --git a/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/update-dependencies.spec.ts b/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/update-dependencies.spec.ts index bb3d000608..11fec6844e 100644 --- a/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/update-dependencies.spec.ts +++ b/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/update-dependencies.spec.ts @@ -1,6 +1,7 @@ import * as fs from 'fs'; import * as pathLib from 'path'; import * as snykFix from '../../../../../src'; +import { TestResult } from '../../../../../src/types'; import { generateScanResult, generateTestResult, @@ -16,6 +17,9 @@ describe('fix *req*.txt / *.txt Python projects', () => { it('fixes project with a -r option', async () => { // Arrange const targetFile = 'with-require/dev.txt'; + filesToDelete = [ + pathLib.join(workspacesPath, 'with-require/fixed-dev.txt'), + ]; const testResult = { ...generateTestResult(), @@ -39,25 +43,11 @@ describe('fix *req*.txt / *.txt Python projects', () => { }, }; - const entityToFix = { - workspace: { - readFile: async (path: string) => { - return readFileHelper(workspacesPath, path); - }, - writeFile: async (path: string, contents: string) => { - const res = pathLib.parse(path); - const fixedPath = pathLib.resolve( - workspacesPath, - res.dir, - `fixed-${res.base}`, - ); - filesToDelete = [fixedPath]; - fs.writeFileSync(fixedPath, contents, 'utf-8'); - }, - }, - scanResult: generateScanResult('pip', targetFile), + const entityToFix = generateEntityToFix( + workspacesPath, + targetFile, testResult, - }; + ); // Act const result = await snykFix.fix([entityToFix], { @@ -97,6 +87,7 @@ describe('fix *req*.txt / *.txt Python projects', () => { workspacesPath, 'basic/fixed-prod.txt', ); + filesToDelete = [fixedFilePath]; const testResult = { ...generateTestResult(), @@ -120,25 +111,11 @@ describe('fix *req*.txt / *.txt Python projects', () => { }, }; - const entityToFix = { - workspace: { - readFile: async (path: string) => { - return readFileHelper(workspacesPath, path); - }, - writeFile: async (path: string, contents: string) => { - const res = pathLib.parse(path); - const fixedPath = pathLib.resolve( - workspacesPath, - res.dir, - `fixed-${res.base}`, - ); - filesToDelete = [fixedPath]; - fs.writeFileSync(fixedPath, contents, 'utf-8'); - }, - }, - scanResult: generateScanResult('pip', targetFile), + const entityToFix = generateEntityToFix( + workspacesPath, + targetFile, testResult, - }; + ); // Act const result = await snykFix.fix([entityToFix], { @@ -185,6 +162,7 @@ describe('fix *req*.txt / *.txt Python projects', () => { workspacesPath, 'basic-with-newline/fixed-prod.txt', ); + filesToDelete = [fixedFilePath]; const testResult = { ...generateTestResult(), @@ -208,25 +186,11 @@ describe('fix *req*.txt / *.txt Python projects', () => { }, }; - const entityToFix = { - workspace: { - readFile: async (path: string) => { - return readFileHelper(workspacesPath, path); - }, - writeFile: async (path: string, contents: string) => { - const res = pathLib.parse(path); - const fixedPath = pathLib.resolve( - workspacesPath, - res.dir, - `fixed-${res.base}`, - ); - filesToDelete = [fixedPath]; - fs.writeFileSync(fixedPath, contents, 'utf-8'); - }, - }, - scanResult: generateScanResult('pip', targetFile), + const entityToFix = generateEntityToFix( + workspacesPath, + targetFile, testResult, - }; + ); // Act const result = await snykFix.fix([entityToFix], { @@ -273,6 +237,7 @@ describe('fix *req*.txt / *.txt Python projects', () => { workspacesPath, 'with-custom-formatting/fixed-requirements.txt', ); + filesToDelete = [fixedFilePath]; const testResult = { ...generateTestResult(), @@ -296,25 +261,11 @@ describe('fix *req*.txt / *.txt Python projects', () => { }, }; - const entityToFix = { - workspace: { - readFile: async (path: string) => { - return readFileHelper(workspacesPath, path); - }, - writeFile: async (path: string, contents: string) => { - const res = pathLib.parse(path); - const fixedPath = pathLib.resolve( - workspacesPath, - res.dir, - `fixed-${res.base}`, - ); - filesToDelete = [fixedPath]; - fs.writeFileSync(fixedPath, contents, 'utf-8'); - }, - }, - scanResult: generateScanResult('pip', targetFile), + const entityToFix = generateEntityToFix( + workspacesPath, + targetFile, testResult, - }; + ); // Act const result = await snykFix.fix([entityToFix], { @@ -362,6 +313,7 @@ describe('fix *req*.txt / *.txt Python projects', () => { workspacesPath, 'lower-case-dep/fixed-req.txt', ); + filesToDelete = [fixedFilePath]; const testResult = { ...generateTestResult(), @@ -380,25 +332,11 @@ describe('fix *req*.txt / *.txt Python projects', () => { }, }; - const entityToFix = { - workspace: { - readFile: async (path: string) => { - return readFileHelper(workspacesPath, path); - }, - writeFile: async (path: string, contents: string) => { - const res = pathLib.parse(path); - const fixedPath = pathLib.resolve( - workspacesPath, - res.dir, - `fixed-${res.base}`, - ); - filesToDelete = [fixedPath]; - fs.writeFileSync(fixedPath, contents, 'utf-8'); - }, - }, - scanResult: generateScanResult('pip', targetFile), + const entityToFix = generateEntityToFix( + workspacesPath, + targetFile, testResult, - }; + ); // Act const result = await snykFix.fix([entityToFix], { @@ -441,6 +379,7 @@ describe('fix *req*.txt / *.txt Python projects', () => { workspacesPath, 'basic/fixed-prod.txt', ); + filesToDelete = [fixedFilePath]; const testResult = { ...generateTestResult(), @@ -460,25 +399,11 @@ describe('fix *req*.txt / *.txt Python projects', () => { }, }; - const entityToFix = { - workspace: { - readFile: async (path: string) => { - return readFileHelper(workspacesPath, path); - }, - writeFile: async (path: string, contents: string) => { - const res = pathLib.parse(path); - const fixedPath = pathLib.resolve( - workspacesPath, - res.dir, - `fixed-${res.base}`, - ); - filesToDelete = [fixedPath]; - fs.writeFileSync(fixedPath, contents, 'utf-8'); - }, - }, - scanResult: generateScanResult('pip', targetFile), + const entityToFix = generateEntityToFix( + workspacesPath, + targetFile, testResult, - }; + ); // Act const result = await snykFix.fix([entityToFix], { @@ -521,6 +446,7 @@ describe('fix *req*.txt / *.txt Python projects', () => { workspacesPath, 'long-versions/fixed-prod.txt', ); + filesToDelete = [fixedFilePath]; const testResult = { ...generateTestResult(), @@ -539,25 +465,11 @@ describe('fix *req*.txt / *.txt Python projects', () => { }, }; - const entityToFix = { - workspace: { - readFile: async (path: string) => { - return readFileHelper(workspacesPath, path); - }, - writeFile: async (path: string, contents: string) => { - const res = pathLib.parse(path); - const fixedPath = pathLib.resolve( - workspacesPath, - res.dir, - `fixed-${res.base}`, - ); - filesToDelete = [fixedPath]; - fs.writeFileSync(fixedPath, contents, 'utf-8'); - }, - }, - scanResult: generateScanResult('pip', targetFile), + const entityToFix = generateEntityToFix( + workspacesPath, + targetFile, testResult, - }; + ); // Act const result = await snykFix.fix([entityToFix], { @@ -600,6 +512,7 @@ describe('fix *req*.txt / *.txt Python projects', () => { workspacesPath, 'with-comparator/fixed-prod.txt', ); + filesToDelete = [fixedFilePath]; const testResult = { ...generateTestResult(), @@ -623,25 +536,11 @@ describe('fix *req*.txt / *.txt Python projects', () => { }, }; - const entityToFix = { - workspace: { - readFile: async (path: string) => { - return readFileHelper(workspacesPath, path); - }, - writeFile: async (path: string, contents: string) => { - const res = pathLib.parse(path); - const fixedPath = pathLib.resolve( - workspacesPath, - res.dir, - `fixed-${res.base}`, - ); - filesToDelete = [fixedPath]; - fs.writeFileSync(fixedPath, contents, 'utf-8'); - }, - }, - scanResult: generateScanResult('pip', targetFile), + const entityToFix = generateEntityToFix( + workspacesPath, + targetFile, testResult, - }; + ); // Act const result = await snykFix.fix([entityToFix], { @@ -688,6 +587,7 @@ describe('fix *req*.txt / *.txt Python projects', () => { 'python-markers/fixed-prod.txt', ); + filesToDelete = [fixedFilePath]; const testResult = { ...generateTestResult(), remediation: { @@ -705,25 +605,11 @@ describe('fix *req*.txt / *.txt Python projects', () => { }, }; - const entityToFix = { - workspace: { - readFile: async (path: string) => { - return readFileHelper(workspacesPath, path); - }, - writeFile: async (path: string, contents: string) => { - const res = pathLib.parse(path); - const fixedPath = pathLib.resolve( - workspacesPath, - res.dir, - `fixed-${res.base}`, - ); - filesToDelete = [fixedPath]; - fs.writeFileSync(fixedPath, contents, 'utf-8'); - }, - }, - scanResult: generateScanResult('pip', targetFile), + const entityToFix = generateEntityToFix( + workspacesPath, + targetFile, testResult, - }; + ); // Act const result = await snykFix.fix([entityToFix], { @@ -783,24 +669,11 @@ describe('fix *req*.txt / *.txt Python projects', () => { }, }, }; - const entityToFix = { - workspace: { - readFile: async (path: string) => { - return readFileHelper(workspacesPath, path); - }, - writeFile: async (path: string, contents: string) => { - const res = pathLib.parse(path); - const fixedPath = pathLib.resolve( - workspacesPath, - res.dir, - `fixed-${res.base}`, - ); - fs.writeFileSync(fixedPath, contents, 'utf-8'); - }, - }, - scanResult: generateScanResult('pip', targetFile), + const entityToFix = generateEntityToFix( + workspacesPath, + targetFile, testResult, - }; + ); const writeFileSpy = jest.spyOn(entityToFix.workspace, 'writeFile'); // Act @@ -814,6 +687,77 @@ describe('fix *req*.txt / *.txt Python projects', () => { expect(result.results.python.succeeded[0].original).toEqual(entityToFix); expect(result.results.python.succeeded[0].changes).toMatchSnapshot(); }); + it('fixes multiple files via -r with the same name (some were already fixed)', async () => { + // Arrange + const targetFile1 = 'app-with-already-fixed/requirements.txt'; + const targetFile2 = 'app-with-already-fixed/lib/requirements.txt'; + const targetFile3 = 'app-with-already-fixed/core/requirements.txt'; + + filesToDelete = [ + pathLib.resolve( + workspacesPath, + 'app-with-already-fixed/fixed-requirements.txt', + ), + pathLib.resolve( + workspacesPath, + 'app-with-already-fixed/lib/fixed-requirements.txt', + ), + pathLib.resolve( + workspacesPath, + 'app-with-already-fixed/core/fixed-requirements.txt', + ), + ]; + const testResult = { + ...generateTestResult(), + remediation: { + unresolved: [], + upgrade: {}, + patch: {}, + ignore: {}, + pin: { + 'django@1.6.1': { + upgradeTo: 'django@2.0.1', + vulns: [], + isTransitive: false, + }, + 'Jinja2@2.7.2': { + upgradeTo: 'Jinja2@2.7.3', + vulns: [], + isTransitive: true, + }, + }, + }, + }; + const entityToFix1 = generateEntityToFix( + workspacesPath, + targetFile1, + testResult, + ); + const entityToFix2 = generateEntityToFix( + workspacesPath, + targetFile2, + testResult, + ); + const entityToFix3 = generateEntityToFix( + workspacesPath, + targetFile3, + testResult, + ); + const writeFileSpy = jest.spyOn(entityToFix1.workspace, 'writeFile'); + // Act + const result = await snykFix.fix( + [entityToFix2, entityToFix3, entityToFix1], + { + quiet: true, + stripAnsi: true, + }, + ); + // 3 files needed to have changes + expect(result.fixSummary).toMatchSnapshot(); + expect(writeFileSpy).toHaveBeenCalledTimes(3); + expect(result.results.python.succeeded[0].original).toEqual(entityToFix1); + expect(result.results.python.succeeded[0].changes).toMatchSnapshot(); + }); }); function readFileHelper(workspacesPath: string, path: string): string { @@ -834,3 +778,29 @@ function readFileHelper(workspacesPath: string, path: string): string { } return file; } + +function generateEntityToFix( + workspacesPath: string, + targetFile: string, + testResult: TestResult, +): snykFix.EntityToFix { + const entityToFix = { + workspace: { + readFile: async (path: string) => { + return readFileHelper(workspacesPath, path); + }, + writeFile: async (path: string, contents: string) => { + const res = pathLib.parse(path); + const fixedPath = pathLib.resolve( + workspacesPath, + res.dir, + `fixed-${res.base}`, + ); + fs.writeFileSync(fixedPath, contents, 'utf-8'); + }, + }, + scanResult: generateScanResult('pip', targetFile), + testResult, + }; + return entityToFix; +} diff --git a/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/workspaces/app-with-already-fixed/base.txt b/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/workspaces/app-with-already-fixed/base.txt new file mode 100644 index 0000000000..aba69a5a27 --- /dev/null +++ b/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/workspaces/app-with-already-fixed/base.txt @@ -0,0 +1 @@ +click>7.0 diff --git a/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/workspaces/app-with-already-fixed/core/requirements.txt b/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/workspaces/app-with-already-fixed/core/requirements.txt new file mode 100644 index 0000000000..e5568357b9 --- /dev/null +++ b/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/workspaces/app-with-already-fixed/core/requirements.txt @@ -0,0 +1 @@ +Django==1.6.1 ; python_version > '1.0' diff --git a/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/workspaces/app-with-already-fixed/lib/requirements.txt b/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/workspaces/app-with-already-fixed/lib/requirements.txt new file mode 100644 index 0000000000..c3eeef4fb7 --- /dev/null +++ b/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/workspaces/app-with-already-fixed/lib/requirements.txt @@ -0,0 +1 @@ +Jinja2==2.7.2 diff --git a/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/workspaces/app-with-already-fixed/requirements.txt b/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/workspaces/app-with-already-fixed/requirements.txt new file mode 100644 index 0000000000..3693dd3bd5 --- /dev/null +++ b/packages/snyk-fix/test/acceptance/plugins/python/update-dependencies/workspaces/app-with-already-fixed/requirements.txt @@ -0,0 +1,4 @@ +-r core/requirements.txt +-r lib/requirements.txt +-r base.txt +Django==1.6.1