Merge pull request #9 from snyk/bug/ignore-dependencies-in-virtualenv

Bug: ignore dependencies in virtualenv

lib/index.ts

@@ -1,92 +1,21 @@
-import { DepGraph, DepGraphBuilder } from '@snyk/dep-graph';
+import { DepGraph, PkgInfo } from '@snyk/dep-graph';
 import * as manifest from './manifest-parser';
 import * as lockFile from './lock-file-parser';
 import { PoetryLockFileDependency } from './lock-file-parser';
+import * as poetryDepGraphBuilder from './poetry-dep-graph-builder';
 
 export function buildDepGraph(
   manifestFileContents: string,
   lockFileContents: string,
   includeDevDependencies = false,
 ): DepGraph {
-  const dependencyNames = manifest.getDependencyNamesFrom(
+  const dependencyNames: string[] = manifest.getDependencyNamesFrom(
     manifestFileContents,
     includeDevDependencies,
   );
-  const packageDetails = manifest.pkgInfoFrom(manifestFileContents);
-  const pkgSpecs = lockFile.packageSpecsFrom(lockFileContents);
-
-  const builder = new DepGraphBuilder({ name: 'poetry' }, packageDetails);
-  addDependenciesToGraph(
-    dependencyNames,
-    pkgSpecs,
-    builder.rootNodeId,
-    builder,
+  const pkgDetails: PkgInfo = manifest.pkgInfoFrom(manifestFileContents);
+  const pkgSpecs: PoetryLockFileDependency[] = lockFile.packageSpecsFrom(
+    lockFileContents,
   );
-  return builder.build();
-}
-
-function addDependenciesToGraph(
-  pkgNames: string[],
-  pkgSpecs: PoetryLockFileDependency[],
-  parentClientId: string,
-  builder: DepGraphBuilder,
-) {
-  for (const pkgName of pkgNames) {
-    addDependenciesFor(pkgName, pkgSpecs, parentClientId, builder);
-  }
+  return poetryDepGraphBuilder.build(pkgDetails, dependencyNames, pkgSpecs);
 }
-
-function addDependenciesFor(
-  packageName: string,
-  pkgSpecs: PoetryLockFileDependency[],
-  parentNodeId: string,
-  builder: DepGraphBuilder,
-) {
-  // Poetry will auto-resolve dependencies with hyphens to dashes, but keep transitive reference name with underscore
-  packageName = packageName.replace(/_/g, '-');
-
-  // Retrieve package info
-  const pkg = pkgLockInfoFor(packageName, pkgSpecs);
-  if (!pkg) {
-    throw new DependencyNotFound(packageName);
-  }
-
-  // Prevent circular references by skipping over packages that have already been added
-  const existingPkg = builder
-    .getPkgs()
-    .find(
-      (existingPkg) =>
-        existingPkg.name === pkg.name && existingPkg.version === pkg.version,
-    );
-  if (existingPkg) {
-    return;
-  }
-
-  // Add package info to builder
-  const pkgInfo = { name: packageName, version: pkg.version };
-  builder
-    .addPkgNode(pkgInfo, packageName)
-    .connectDep(parentNodeId, packageName);
-  addDependenciesToGraph(pkg.dependencies, pkgSpecs, packageName, builder);
-}
-
-function pkgLockInfoFor(
-  packageName: string,
-  pkgSpecs: PoetryLockFileDependency[],
-) {
-  return pkgSpecs.find((lockItem) => {
-    return lockItem.name.toLowerCase() === packageName.toLowerCase();
-  });
-}
-
-class DependencyNotFound extends Error {
-  constructor(pkgName: string) {
-    super(`Unable to find dependencies in poetry.lock for package: ${pkgName}`);
-    this.name = 'DependencyNotFound';
-  }
-}
-
-export type PoetryParsingError =
-  | manifest.ManifestFileNotValid
-  | lockFile.LockFileNotValid
-  | DependencyNotFound;

lib/poetry-dep-graph-builder.ts

@@ -0,0 +1,93 @@
+import { DepGraph, DepGraphBuilder, PkgInfo } from '@snyk/dep-graph';
+import { PoetryLockFileDependency } from './lock-file-parser';
+
+// Poetry uses the virtualenv to create an environment and this comes with these
+// packages pre-installed, therefore they won't be part of the lockfile.
+// See: https://github.com/python-poetry/poetry/issues/3075#issuecomment-703334427
+const IGNORED_DEPENDENCIES: string[] = [
+  'setuptools',
+  'distribute',
+  'pip',
+  'wheel',
+];
+
+export function build(
+  pkgDetails: PkgInfo,
+  pkgDependencyNames: string[],
+  pkgSpecs: PoetryLockFileDependency[],
+): DepGraph {
+  const builder = new DepGraphBuilder({ name: 'poetry' }, pkgDetails);
+  addDependenciesToGraph(
+    pkgDependencyNames,
+    pkgSpecs,
+    builder.rootNodeId,
+    builder,
+  );
+  return builder.build();
+}
+
+function addDependenciesToGraph(
+  pkgNames: string[],
+  pkgSpecs: PoetryLockFileDependency[],
+  parentNodeId: string,
+  builder: DepGraphBuilder,
+) {
+  for (const pkgName of pkgNames) {
+    addDependenciesForPkg(pkgName, pkgSpecs, parentNodeId, builder);
+  }
+}
+
+function addDependenciesForPkg(
+  pkgName: string,
+  pkgSpecs: PoetryLockFileDependency[],
+  parentNodeId: string,
+  builder: DepGraphBuilder,
+) {
+  if (IGNORED_DEPENDENCIES.includes(pkgName)) {
+    return;
+  }
+  // Poetry will auto-resolve dependencies with hyphens to dashes, but keep transitive reference name with underscore
+  pkgName = pkgName.replace(/_/g, '-');
+
+  const pkg = pkgLockInfoFor(pkgName, pkgSpecs);
+  if (isPkgAlreadyInGraph(pkg, builder)) {
+    return;
+  }
+
+  const pkgInfo: PkgInfo = { name: pkgName, version: pkg.version };
+  builder.addPkgNode(pkgInfo, pkgName).connectDep(parentNodeId, pkgName);
+  addDependenciesToGraph(pkg.dependencies, pkgSpecs, pkgName, builder);
+}
+
+function isPkgAlreadyInGraph(
+  pkg: PoetryLockFileDependency,
+  builder: DepGraphBuilder,
+): boolean {
+  return builder
+    .getPkgs()
+    .some(
+      (existingPkg) =>
+        existingPkg.name === pkg.name && existingPkg.version === pkg.version,
+    );
+}
+
+function pkgLockInfoFor(
+  pkgName: string,
+  pkgSpecs: PoetryLockFileDependency[],
+): PoetryLockFileDependency {
+  const pkgLockInfo = pkgSpecs.find(
+    (lockItem) => lockItem.name.toLowerCase() === pkgName.toLowerCase(),
+  );
+
+  if (!pkgLockInfo) {
+    throw new DependencyNotFound(pkgName);
+  }
+  return pkgLockInfo;
+}
+
+export class DependencyNotFound extends Error {
+  constructor(pkgName: string) {
+    super(`Unable to find dependencies in poetry.lock for package: ${pkgName}`);
+    this.name = DependencyNotFound.name;
+  }
+}

test/fixtures/index.test.ts

@@ -104,6 +104,13 @@ describe('buildDepGraph', () => {
     expect(actualGraph).toBeDefined();
     expect(actualGraph.getDepPkgs().length).toBe(2);
   });
+
+  it('on fixture with unsafe package yields graph successfully', () => {
+    // Package is in virtualenv and doesn't have an entry in poetry.lock
+    const actualGraph = depGraphForScenarioAt('scenarios/unsafe-packages');
+    expect(actualGraph).toBeDefined();
+    expect(actualGraph.getDepPkgs().length).toBe(1);
+  });
 });
 
 function depGraphForScenarioAt(

test/fixtures/scenarios/unsafe-packages/pyproject.toml

@@ -0,0 +1,13 @@
+[tool.poetry]
+name = "unsafe-packages"
+version = "0.1.0"
+description = ""
+authors = ["Daniel Trunley <[email protected]>"]
+
+[tool.poetry.dependencies]
+python = "^3.8"
+gunicorn = "^20.0.4"
+
+[build-system]
+requires = ["poetry>=0.12"]
+build-backend = "poetry.masonry.api"