feat: (experimental) shoiw pinning remediation advice for Python

snyk · Sep 4, 2019 · faac0f6 · faac0f6
1 parent 9dbbc58
commit faac0f6
Show file tree

Hide file tree

Showing 14 changed files with 1,234 additions and 68 deletions.
diff --git a/src/cli/commands/monitor.ts b/src/cli/commands/monitor.ts
@@ -11,7 +11,6 @@ import chalk from 'chalk';
 import * as pathUtil from 'path';
 import * as spinner from '../../lib/spinner';
 
-import request = require('../../lib/request');
 import * as detect from '../../lib/detect';
 import * as plugins from '../../lib/plugins';
 import {ModuleInfo} from '../../lib/module-info'; // TODO(kyegupov): fix import
@@ -28,14 +27,10 @@ import {
   UnsupportedFeatureFlagError,
 } from '../../lib/errors';
 import { legacyPlugin as pluginApi } from '@snyk/cli-interface';
+import { isFeatureFlagSupportedForOrg } from '../../lib/feature-flags';
 
 const SEPARATOR = '\n-------------------------------------------------------\n';
 
-interface OrgFeatureFlagResponse {
-  ok: boolean;
-  userMessage?: string;
-}
-
 interface GoodResult {
   ok: true;
   data: string;
@@ -292,17 +287,3 @@ function formatMonitorOutput(
       packageManager,
     })) : strOutput;
 }
-
-async function isFeatureFlagSupportedForOrg(featureFlag: string): Promise<OrgFeatureFlagResponse> {
-  const response = await request({
-    method: 'GET',
-    headers: {
-      Authorization: `token ${snyk.api}`,
-    },
-    url: `${config.API}/cli-config/feature-flags/${featureFlag}`,
-    gzip: true,
-    json: true,
-  });
-
-  return (response as any).body;
-}
diff --git a/src/cli/commands/test/formatters/remediation-based-format-issues.ts b/src/cli/commands/test/formatters/remediation-based-format-issues.ts
@@ -1,10 +1,14 @@
-import * as _ from 'lodash';
 import chalk from 'chalk';
 import * as wrap from 'wrap-ansi';
 import * as config from '../../../../lib/config';
 import { TestOptions } from '../../../../lib/types';
-import { RemediationResult, PatchRemediation,
-  DependencyUpdates, IssueData, SEVERITY, GroupedVuln } from '../../../../lib/snyk-test/legacy';
+import {
+  RemediationChanges, PatchRemediation,
+  DependencyUpdates, IssueData, SEVERITY, GroupedVuln,
+  DependencyPins,
+  UpgradeRemediation,
+  PinRemediation,
+} from '../../../../lib/snyk-test/legacy';
 import { SEVERITIES } from '../../../../lib/snyk-test/common';
 
 interface BasicVulnInfo {
@@ -16,11 +20,21 @@ interface BasicVulnInfo {
   version: string;
   fixedIn: string[];
   legalInstructions?: string;
+  paths: string[][];
+}
+
+interface TopLevelPackageUpgrade {
+  name: string;
+  version: string;
+}
+
+interface UpgradesByAffectedPackage {
+  [pkgNameAndVersion: string]: TopLevelPackageUpgrade[];
 }
 
 export function formatIssuesWithRemediation(
   vulns: GroupedVuln[],
-  remediationInfo: RemediationResult,
+  remediationInfo: RemediationChanges,
   options: TestOptions,
 ): string[] {
 
@@ -42,6 +56,7 @@ export function formatIssuesWithRemediation(
       version: vuln.version,
       fixedIn: vuln.fixedIn,
       legalInstructions: vuln.legalInstructions,
+      paths: vuln.list.map((v) => v.from),
     };
 
     basicVulnInfo[vuln.metadata.id] = vulnData;
@@ -53,7 +68,28 @@ export function formatIssuesWithRemediation(
 
   const results = [chalk.bold.white('Remediation advice')];
 
-  const upgradeTextArray = constructUpgradesText(remediationInfo.upgrade, basicVulnInfo);
+  let upgradeTextArray: string[];
+  if (remediationInfo.pin && Object.keys(remediationInfo.pin).length) {
+    const upgradesByAffected: UpgradesByAffectedPackage = {};
+    for (const topLevelPkg of Object.keys(remediationInfo.upgrade)) {
+      for (const targetPkgStr of remediationInfo.upgrade[topLevelPkg].upgrades) {
+        if (!upgradesByAffected[targetPkgStr]) {
+          upgradesByAffected[targetPkgStr] = [];
+        }
+        upgradesByAffected[targetPkgStr].push({
+          name: topLevelPkg,
+          version: remediationInfo.upgrade[topLevelPkg].upgradeTo,
+        });
+      }
+    }
+    upgradeTextArray = constructPinText(remediationInfo.pin, upgradesByAffected, basicVulnInfo);
+    const allVulnIds = new Set();
+    Object.keys(remediationInfo.pin).forEach(
+      (name) => remediationInfo.pin[name].issues.forEach((vid) => allVulnIds.add(vid)));
+    remediationInfo.unresolved = remediationInfo.unresolved.filter((issue) => !allVulnIds.has(issue.id));
+  } else {
+    upgradeTextArray = constructUpgradesText(remediationInfo.upgrade, basicVulnInfo);
+  }
   if (upgradeTextArray.length > 0) {
     results.push(upgradeTextArray.join('\n'));
   }
@@ -130,8 +166,7 @@ function constructPatchesText(
     // todo: add vulnToPatch package name
     const packageAtVersion = `${basicVulnInfo[id].name}@${basicVulnInfo[id].version}`;
     const patchedText = `\n  Patch available for ${chalk.bold.whiteBright(packageAtVersion)}\n`;
-    const thisPatchFixes =
-    formatIssue(
+    const thisPatchFixes = formatIssue(
       id,
       basicVulnInfo[id].title,
       basicVulnInfo[id].severity,
@@ -145,6 +180,40 @@ function constructPatchesText(
   return patchedTextArray;
 }
 
+function thisUpgradeFixes(vulnIds: string[], basicVulnInfo: Record<string, BasicVulnInfo>) {
+  return vulnIds
+    .sort((a, b) => getSeverityValue(basicVulnInfo[a].severity) - getSeverityValue(basicVulnInfo[b].severity))
+    .filter((id) => basicVulnInfo[id].type !== 'license')
+    .map((id) => formatIssue(
+      id,
+      basicVulnInfo[id].title,
+      basicVulnInfo[id].severity,
+      basicVulnInfo[id].isNew,
+      undefined,
+      `${basicVulnInfo[id].name}@${basicVulnInfo[id].version}`,
+      ))
+    .join('\n');
+}
+
+function processUpgrades(
+  sink: string[],
+  upgradesByDep: DependencyUpdates | DependencyPins,
+  deps: string[],
+  basicVulnInfo: Record<string, BasicVulnInfo>,
+) {
+  for (const dep of deps) {
+
+
+
+    const data = upgradesByDep[dep];
+    const upgradeDepTo = data.upgradeTo;
+    const vulnIds = (data as UpgradeRemediation).vulns || (data as PinRemediation).issues;
+    const upgradeText =
+      `\n  Upgrade ${chalk.bold.whiteBright(dep)} to ${chalk.bold.whiteBright(upgradeDepTo)} to fix\n`;
+    sink.push(upgradeText + thisUpgradeFixes(vulnIds, basicVulnInfo));
+  }
+}
+
 function constructUpgradesText(
   upgrades: DependencyUpdates,
   basicVulnInfo: {
@@ -156,26 +225,73 @@ function constructUpgradesText(
     return [];
   }
 
-  const upgradeTextArray = [chalk.bold.green('\nUpgradable Issues:')];
-  for (const upgrade of Object.keys(upgrades)) {
-    const upgradeDepTo = _.get(upgrades, [upgrade, 'upgradeTo']);
-    const vulnIds = _.get(upgrades, [upgrade, 'vulns']);
-    const upgradeText =
-      `\n  Upgrade ${chalk.bold.whiteBright(upgrade)} to ${chalk.bold.whiteBright(upgradeDepTo)} to fix\n`;
-    const thisUpgradeFixes = vulnIds
-      .sort((a, b) => getSeverityValue(basicVulnInfo[a].severity) - getSeverityValue(basicVulnInfo[b].severity))
-      .filter((id) => basicVulnInfo[id].type !== 'license')
-      .map((id) => formatIssue(
-        id,
-        basicVulnInfo[id].title,
-        basicVulnInfo[id].severity,
-        basicVulnInfo[id].isNew,
-        undefined,
-        `${basicVulnInfo[id].name}@${basicVulnInfo[id].version}`,
-      ))
-      .join('\n');
-    upgradeTextArray.push(upgradeText + thisUpgradeFixes);
+  const upgradeTextArray = [chalk.bold.green('\nIssues to fix by upgrading:')];
+  processUpgrades(upgradeTextArray, upgrades, Object.keys(upgrades), basicVulnInfo);
+  return upgradeTextArray;
+}
+
+function constructPinText(
+  pins: DependencyPins,
+  upgradesByAffected: UpgradesByAffectedPackage, // classical "remediation via top-level dep" upgrades
+  basicVulnInfo: Record<string, BasicVulnInfo>,
+): string[] {
+
+  if (!(Object.keys(pins).length)) {
+    return [];
+  }
+
+  // First, direct upgrades
+  const upgradeTextArray: string[] = [];
+
+  const upgradeables = Object.keys(pins).filter((name) => !pins[name].isTransitive);
+  if (upgradeables.length) {
+    upgradeTextArray.push(chalk.bold.green('\nIssues to fix by upgrading existing dependencies:'));
+    processUpgrades(upgradeTextArray, pins, upgradeables, basicVulnInfo);
   }
+
+  // Second, pins
+  const pinables = Object.keys(pins).filter((name) => pins[name].isTransitive);
+
+  if (pinables.length) {
+    upgradeTextArray.push(chalk.bold.green('\nIssues to fix by pinning sub-dependencies:'));
+
+    for (const pkgName of pinables) {
+      const data = pins[pkgName];
+      const vulnIds = data.issues;
+      const upgradeDepTo = data.upgradeTo;
+      const upgradeText =
+        `\n  Pin ${chalk.bold.whiteBright(pkgName)} to ${chalk.bold.whiteBright(upgradeDepTo)} to fix`;
+      upgradeTextArray.push(upgradeText);
+      upgradeTextArray.push(thisUpgradeFixes(vulnIds, basicVulnInfo));
+
+      // Transitive dependencies are not visible for the user. Therefore, it makes sense to print
+      // at least some guidance explaining where they do come from. We want to limit the number
+      // of paths, because it can be in thousands for some projects.
+      const allPaths = new Set();
+      for (const vid of vulnIds) {
+        for (const path of basicVulnInfo[vid].paths) {
+          allPaths.add(path.slice(1).join(' > '));
+        }
+      }
+      upgradeTextArray.push(allPaths.size === 1
+        ? `  (introduced by ${allPaths.keys().next().value})`
+        : `  (introduced by ${allPaths.keys().next().value} and ${allPaths.size - 1} other path(s))`);
+
+      // Finally, if we have some upgrade paths that fix the same issues, suggest them as well.
+      const topLevelUpgradesAlreadySuggested = new Set();
+      for (const vid of vulnIds) {
+        for (const topLevelPkg of upgradesByAffected[pkgName + '@' + basicVulnInfo[vid].version] || []) {
+          const setKey = `${topLevelPkg.name}\n${topLevelPkg.version}`;
+          if (!topLevelUpgradesAlreadySuggested.has(setKey)) {
+            topLevelUpgradesAlreadySuggested.add(setKey);
+            upgradeTextArray.push('  The issues above can also be fixed by upgrading top-level dependency ' +
+              `${topLevelPkg.name} to ${topLevelPkg.version}`);
+          }
+        }
+      }
+    }
+  }
+
   return upgradeTextArray;
 }
 
@@ -189,7 +305,7 @@ function constructUnfixableText(unresolved: IssueData[]) {
       ? `\n  This issue was fixed in versions: ${chalk.bold(issue.fixedIn.join(', '))}`
       : '\n  No upgrade or patch available';
     const packageNameAtVersion = chalk.bold
-    .whiteBright(`\n  ${issue.packageName}@${issue.version}\n`);
+      .whiteBright(`\n  ${issue.packageName}@${issue.version}\n`);
     unfixableIssuesTextArray
       .push(packageNameAtVersion +
       formatIssue(

diff --git a/src/lib/feature-flags.ts b/src/lib/feature-flags.ts
@@ -0,0 +1,22 @@
+import request = require('./request');
+import snyk = require('.'); // TODO(kyegupov): fix import
+import * as config from './config';
+
+interface OrgFeatureFlagResponse {
+  ok: boolean;
+  userMessage?: string;
+}
+
+export async function isFeatureFlagSupportedForOrg(featureFlag: string): Promise<OrgFeatureFlagResponse> {
+  const response = await request({
+    method: 'GET',
+    headers: {
+      Authorization: `token ${snyk.api}`,
+    },
+    url: `${config.API}/cli-config/feature-flags/${featureFlag}`,
+    gzip: true,
+    json: true,
+  });
+
+  return (response as any).body;
+}
diff --git a/src/lib/package-managers.ts b/src/lib/package-managers.ts
@@ -26,3 +26,7 @@ export const PROTECT_SUPPORTED_PACKAGE_MANAGERS: SupportedPackageManagers[]
   = ['yarn', 'npm'];
 export const GRAPH_SUPPORTED_PACKAGE_MANAGERS: SupportedPackageManagers[]
   = ['npm', 'sbt'];
+// For ecosystems with a flat set of libraries (e.g. Python, JVM), one can
+// "pin" a transitive dependency
+export const PINNING_SUPPORTED_PACKAGE_MANAGERS: SupportedPackageManagers[]
+  = ['pip'];