feat: apply upgrade remediation to manifests

.gitignore

@@ -8,6 +8,7 @@ npm-debug.log
 *.egg-info/
 .venvs/
 dist/
+build-with-tests/
 package-lock.json
 
 .nyc_output

lib/index.ts

@@ -21,8 +21,8 @@ type Options = api.SingleSubprojectInspectOptions & PythonInspectOptions;
 export async function inspect(
   root: string,
   targetFile: string,
-  options: Options
-) {
+  options?: Options
+): Promise<api.SinglePackageResult> {
   if (!options) {
     options = {};
   }
@@ -57,6 +57,172 @@ export async function inspect(
   return { plugin, package: pkg };
 }
 
+interface UpgradeRemediation {
+  upgradeTo: string;
+  // Other fields are of no interest
+}
+
+interface DependencyUpdates {
+  [from: string]: UpgradeRemediation;
+}
+
+interface ManifestFiles {
+  // Typically these are requirements.txt, constraints.txt and Pipfile;
+  // the plugin supports paths with subdirectories
+  [name: string]: string; // name-to-content
+}
+
+// Correction for the type; should be fixed in snyk-cli-interface
+export interface DepTreeDep {
+  name?: string; // shouldn't, but might be missing
+  version?: string; // shouldn't, but might be missing
+  dependencies?: {
+    [depName: string]: DepTreeDep;
+  };
+  labels?: {
+    [key: string]: string;
+
+    // Known keys:
+    // pruned: identical subtree already presents in the parent node.
+    //         See --prune-repeated-subdependencies flag.
+  };
+}
+
+// Applies upgrades to direct and indirect dependencies
+export async function applyRemediationToManifests(
+  root: string,
+  manifests: ManifestFiles,
+  upgrades: DependencyUpdates,
+  options: Options
+) {
+  const manifestNames = Object.keys(manifests);
+  const targetFile = manifestNames.find(
+    (fn) => path.basename(fn) === 'requirements.txt'
+  );
+  if (
+    !targetFile ||
+    !manifestNames.every(
+      (fn) =>
+        path.basename(fn) === 'requirements.txt' ||
+        path.basename(fn) === 'constraints.txt'
+    )
+  ) {
+    throw new Error(
+      'Remediation only supported for requirements.txt and constraints.txt files'
+    );
+  }
+
+  const provOptions = { ...options };
+  provOptions.args = provOptions.args || [];
+  provOptions.args.push('--only-provenance');
+
+  const topLevelDeps = (await inspect(root, targetFile, provOptions)).package;
+  applyUpgrades(manifests, upgrades, topLevelDeps);
+
+  return manifests;
+}
+
+function applyUpgrades(
+  manifests: ManifestFiles,
+  upgrades: DependencyUpdates,
+  topLevelDeps: DepTree
+) {
+  const requirementsFileName = Object.keys(manifests).find(
+    (fn) => path.basename(fn) === 'requirements.txt'
+  ) as string;
+  const constraintsFileName = Object.keys(manifests).find(
+    (fn) => path.basename(fn) === 'constraints.txt'
+  );
+
+  // Updates to requirements.txt
+  const patch: { [zeroBasedIndex: number]: string | false } = {}; // false means remove the line
+  const append: string[] = [];
+
+  const originalRequirementsLines = manifests[requirementsFileName].split('\n');
+
+  const extraMarkers = /--| \[|;/;
+
+  for (const upgradeFrom of Object.keys(upgrades)) {
+    const pkgName = upgradeFrom.split('@')[0].toLowerCase();
+    const newVersion = upgrades[upgradeFrom].upgradeTo.split('@')[1];
+    const topLevelDep = (topLevelDeps.dependencies || {})[
+      pkgName
+    ] as DepTreeDep;
+    if (topLevelDep && topLevelDep.labels && topLevelDep.labels.provenance) {
+      // Top level dependency, to be updated in a manifest
+
+      const lineNumbers = topLevelDep.labels.provenance
+        .split(':')[1]
+        .split('-')
+        .map((x) => parseInt(x));
+      // TODO(kyegupov): what if the original version spec was range, e.g. >=1.0,<2.0 ?
+      // TODO(kyegupov): prevent downgrades
+      const firstLineNo = lineNumbers[0] - 1;
+      const lastLineNo =
+        lineNumbers.length > 1 ? lineNumbers[1] - 1 : lineNumbers[0] - 1;
+      const originalRequirementString = originalRequirementsLines
+        .slice(firstLineNo, lastLineNo + 1)
+        .join('\n')
+        .replace(/\\\n/g, '');
+      const firstExtraMarkerPos = originalRequirementString.search(
+        extraMarkers
+      );
+      if (firstExtraMarkerPos > -1) {
+        // maybe we should reinstate linebreaks here?
+        patch[lineNumbers[0] - 1] =
+          pkgName +
+          '==' +
+          newVersion +
+          ' ' +
+          originalRequirementString.slice(firstExtraMarkerPos).trim();
+      } else {
+        patch[lineNumbers[0] - 1] = pkgName + '==' + newVersion;
+      }
+      if (lastLineNo > firstLineNo) {
+        for (let i = firstLineNo + 1; i <= lastLineNo; i++) {
+          patch[i - 1] = false;
+        }
+      }
+    } else {
+      // The dependency is not a top level: we are pinning a transitive using constraints file.
+      if (!constraintsFileName) {
+        append.push(
+          pkgName +
+            '>=' +
+            newVersion +
+            ' # not directly required, pinned by Snyk to avoid a vulnerability'
+        );
+      } else {
+        // TODO(kyegupov): parse constraints and replace the pre-existing one if any
+        const lines = manifests[constraintsFileName].trim().split('\n');
+        lines.push(
+          pkgName +
+            '>=' +
+            newVersion +
+            ' # pinned by Snyk to avoid a vulnerability'
+        );
+        manifests[constraintsFileName] = lines.join('\n') + '\n';
+      }
+    }
+  }
+  const lines: string[] = [];
+  originalRequirementsLines.forEach((line, i) => {
+    if (patch[i] === false) {
+      return;
+    }
+    if (patch[i]) {
+      lines.push(patch[i] as string);
+    } else {
+      lines.push(line);
+    }
+  });
+  // Drop extra trailing newlines
+  while (lines.length > 0 && !lines[lines.length - 1]) {
+    lines.pop();
+  }
+  manifests[requirementsFileName] = lines.concat(append).join('\n') + '\n';
+}
+
 function getMetaData(
   command: string,
   baseargs: string[],
@@ -153,6 +319,7 @@ async function getDependencies(
 
   dumpAllFilesInTempDir(tempDirObj.name);
   try {
+    // See ../pysrc/README.md
     const output = await subProcess.execute(
       command,
       [
@@ -173,7 +340,7 @@ async function getDependencies(
       if (error.indexOf('Required packages missing') !== -1) {
         let errMsg = error + '\nPlease run `pip install -r ' + targetFile + '`';
         if (path.basename(targetFile) === 'Pipfile') {
-          errMsg = 'Please run `pipenv update`';
+          errMsg = error + '\nPlease run `pipenv update`';
         }
         throw new Error(errMsg);
       }

package.json

@@ -9,11 +9,12 @@
   "main": "dist/index.js",
   "scripts": {
     "build": "tsc",
+    "build-tests": "tsc -p tsconfig-test.json",
     "format:check": "prettier --check '{lib,test}/**/*.{js,ts}'",
     "format": "prettier --write '{lib,test}/**/*.{js,ts}'",
     "prepare": "npm run build",
-    "test": "tap --node-arg=-r --node-arg=ts-node/register ./test/*.test.js -R spec --timeout=900",
-    "lint": "npm run format:check && eslint --cache '{lib,test}/**/*.{js,ts}'",
+    "test": "cross-env TS_NODE_PROJECT=tsconfig-test.json tap --node-arg=-r --node-arg=ts-node/register ./test/*.test.{js,ts} -R spec --timeout=900",
+    "lint": "npm run build-tests && npm run format:check && eslint --cache '{lib,test}/**/*.{js,ts}'",
     "semantic-release": "semantic-release"
   },
   "author": "snyk.io",
@@ -23,10 +24,12 @@
     "tmp": "0.0.33"
   },
   "devDependencies": {
+    "@snyk/types-tap": "^1.1.0",
     "@types/node": "^6.14.6",
     "@types/tmp": "^0.1.0",
     "@typescript-eslint/eslint-plugin": "^1.13.0",
     "@typescript-eslint/parser": "^1.13.0",
+    "cross-env": "^5.2.0",
     "eslint": "^5.16.0",
     "eslint-config-prettier": "^6.0.0",
     "prettier": "^1.18.2",

pysrc/README.md

@@ -5,7 +5,15 @@ This is the Python part of the snyk-python-plugin.
 Given a fully installed Python package with its dependencies (using a virtual environment),
 it analyzes and returns the dependency tree.
 
-The Node.js code (from the `lib` directory) only sets up the environment and launches this
-analyzer.
+The entry point is `main` in `pip_resolve.py`.
 
-The entry point is `main` in `pip_resolve.py`.
+## Implementation outline
+
+1. take pkg_resources.working_set (a list of all packages available in the current environment)
+2. convert it to a tree
+3. parse the manifest (requirements.txt/Pipfile) to find top-level deps
+4. select the parts of the tree that start from TLDs found in previous step
+5. determine actual installed versions for the packages in the tree
+6. convert that tree in DepTree format
+
+The parts 1 and 5 require access to the Python environment and thus have to be implemented in Python.

pysrc/pip_resolve.py

@@ -34,7 +34,7 @@ def create_tree_of_packages_dependencies(
         top_level_requirements,
         req_file_path,
         allow_missing=False,
-        add_provenance=False
+        only_provenance=False
     ):
     """Create packages dependencies tree
     :param dict tree: the package tree
@@ -103,16 +103,19 @@ def create_dir_as_root():
     def create_package_as_root(package, dir_as_root):
         package_as_root = {
             NAME: package.project_name.lower(),
+            # Note: _version is a private field.
             VERSION: package._obj._version,
         }
         return package_as_root
     dir_as_root = create_dir_as_root()
     for package in packages_as_dist_obj:
         package_as_root = create_package_as_root(package, dir_as_root)
-        if add_provenance:
+        if only_provenance:
             package_as_root[LABELS] = {PROVENANCE: format_provenance_label(tlr_by_key[package_as_root[NAME]].provenance)}
-        package_tree = create_children_recursive(package_as_root, key_tree, set([]))
-        dir_as_root[DEPENDENCIES][package_as_root[NAME]] = package_tree
+            dir_as_root[DEPENDENCIES][package_as_root[NAME]] = package_as_root
+        else:
+            package_tree = create_children_recursive(package_as_root, key_tree, set([]))
+            dir_as_root[DEPENDENCIES][package_as_root[NAME]] = package_tree
     return dir_as_root
 
 def satisfies_python_requirement(parsed_operator, parsed_python_version):
@@ -145,7 +148,7 @@ def matches_python_version(requirement):
         return True
     if not 'python_version' in markers_text:
         return True
-    match = PYTHON_MARKER_REGEX.match(markers_text)
+    match = PYTHON_MARKER_REGEX.search(markers_text)
     if not match:
         return False
     parsed_operator = match.group('operator')
@@ -205,7 +208,7 @@ def get_requirements_list(requirements_file_path, dev_deps=False):
 def create_dependencies_tree_by_req_file_path(requirements_file_path,
                                               allow_missing=False,
                                               dev_deps=False,
-                                              add_provenance=False):
+                                              only_provenance=False):
     # get all installed packages
     pkgs = list(pkg_resources.working_set)
 
@@ -234,7 +237,7 @@ def create_dependencies_tree_by_req_file_path(requirements_file_path,
 
     # build a tree of dependencies
     package_tree = create_tree_of_packages_dependencies(
-        dist_tree, top_level_requirements, requirements_file_path, allow_missing, add_provenance)
+        dist_tree, top_level_requirements, requirements_file_path, allow_missing, only_provenance)
     print(json.dumps(package_tree))
 
 
@@ -261,16 +264,16 @@ def main():
     parser.add_argument("--dev-deps",
         action="store_true",
         help="resolve dev dependencies")
-    parser.add_argument("--add-provenance",
+    parser.add_argument("--only-provenance",
         action="store_true",
-        help="add provenance information to top-level dependencies")
+        help="only return top level deps with provenance information")
     args = parser.parse_args()
 
     create_dependencies_tree_by_req_file_path(
         args.requirements,
         allow_missing=args.allow_missing,
         dev_deps=args.dev_deps,
-        add_provenance=args.add_provenance,
+        only_provenance=args.only_provenance,
     )
 
 if __name__ == '__main__':

test/_setup.test.ts

@@ -0,0 +1,18 @@
+import { test } from 'tap';
+import {
+  ensureVirtualenv,
+  chdirWorkspaces,
+  activateVirtualenv,
+  pipInstall,
+} from './test-utils';
+
+test('install requirements in "pip-app" venv (may take a while)', async (t) => {
+  chdirWorkspaces('pip-app');
+  ensureVirtualenv('pip-app');
+  t.teardown(activateVirtualenv('pip-app'));
+  try {
+    pipInstall();
+  } catch (error) {
+    t.bailout(error);
+  }
+});

test/fixtures/updated-manifests-with-constraints/constraints.txt

@@ -0,0 +1,2 @@
+
+transitive>=1.1.1 # pinned by Snyk to avoid a vulnerability

test/fixtures/updated-manifests-with-constraints/requirements.txt

@@ -0,0 +1,8 @@
+Jinja2==2.7.2
+django==2.0.1
+python-etcd==0.4.5
+Django-Select2==6.0.1 # this version installs with lowercase so it catches a previous bug in pip_resolve.py
+irc==16.2 # this has a cyclic dependecy (interanl jaraco.text <==> jaraco.collections)
+testtools==\
+    2.3.0 # this has a cycle (fixtures ==> testtols);
+./packages/prometheus_client-0.6.0

test/fixtures/updated-manifests-with-python-markers/requirements.txt

@@ -0,0 +1,25 @@
+amqp==2.4.2
+apscheduler==3.6.0
+asn1crypto==0.24.0
+astroid==1.6.6
+atomicwrites==1.3.0
+attrs==19.1.0
+automat==0.7.0
+backports.functools-lru-cache==1.5 ; python_version < '3.2'
+billiard==3.6.0.0
+celery==4.3.0
+certifi==2019.3.9
+cffi==1.12.3
+chardet==3.0.4
+click==7.1 ; python_version > '1.0'
+clickclick==1.2.2
+configparser==3.7.4 ; python_version == '2.7'
+connexion[swagger-ui]==2.2.0
+constantly==15.1.0
+cryptography==2.6.1
+cssselect==1.0.3
+cython==0.29.7
+enum34==1.1.6 ; python_version < '2.6'
+fastavro==0.21.21
+flask-apscheduler==1.11.0
+flask==1.0.2