feat: basic pip fix -r support

lili2311 · lili2311 · commit 0384020f20fc · 2021-03-29T15:49:48.000+01:00
diff --git a/packages/snyk-fix/src/lib/errors/invalid-workspace.ts 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
@@ -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<FixChangesSummary[]> {
-  // 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
@@ -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,12 +38,14 @@ 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);
 
   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
@@ -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
@@ -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
@@ -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
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
diff --git a/packages/snyk-fix/test/unit/fix.spec.ts b/packages/snyk-fix/test/unit/fix.spec.ts
