Repository: lirantal/lockfile-lint
Pull request #2 (2019-07-22 12:45:29)

Feat: Support npm lockfile parsing
<!--- Provide a general summary of your changes in the Title above -->

## Description

**BREAKING CHANGE**
Add support for parsing npm package-lock.json lockfiles 

<!--- Describe your changes in detail -->

## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [x] Breaking change (fix or feature that would cause existing functionality to change)

## Related Issue

<!--- This project only accepts pull requests related to open issues -->
<!--- If suggesting a new feature or change, please discuss it in an issue first -->
<!--- If fixing a bug, there should be an issue describing it with steps to reproduce -->
<!--- Please link to the issue here: -->

## Motivation and Context

This has been a TBD for a while but we surely need to support npm's lockfile for policy and trust checking.

<!--- Why is this change required? What problem does it solve? -->

## How Has This Been Tested?

Multiple unit and integration tests.
Still room for more unit tests around the new flattening array for npm's lockfile.

<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->
<!--- Unit tests are expected for all changes. -->

## Screenshots (if appropriate):

## Checklist:

<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->

- [ ] I have updated the documentation (if required).
- [x] I have read the **CONTRIBUTING** document.
- [x] I have added tests to cover my changes.
- [ ] All new and existing tests passed.
- [ ] I added a picture of a cute animal cause it's fun


codecov-io: # [Codecov](https://codecov.io/gh/lirantal/lockfile-lint/pull/2?src=pr&el=h1) Report
> Merging [#2](https://codecov.io/gh/lirantal/lockfile-lint/pull/2?src=pr&el=desc) into [master](https://codecov.io/gh/lirantal/lockfile-lint/commit/1c96a6e03e7ddcc97d67dffed9b16d0337b691a7?src=pr&el=desc) will **decrease** coverage by `30.11%`.
> The diff coverage is `51.35%`.

[![Impacted file tree graph](https://codecov.io/gh/lirantal/lockfile-lint/pull/2/graphs/tree.svg?width=650&token=P36VJa7ksa&height=150&src=pr)](https://codecov.io/gh/lirantal/lockfile-lint/pull/2?src=pr&el=tree)

```diff
@@             Coverage Diff             @@
##           master       #2       +/-   ##
===========================================
- Coverage   95.29%   65.17%   -30.12%     
===========================================
  Files           7        5        -2     
  Lines          85      112       +27     
  Branches       12       18        +6     
===========================================
- Hits           81       73        -8     
- Misses          4       34       +30     
- Partials        0        5        +5
```


| [Impacted Files](https://codecov.io/gh/lirantal/lockfile-lint/pull/2?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [packages/lockfile-lint-api/src/ParseLockFile.js](https://codecov.io/gh/lirantal/lockfile-lint/pull/2/diff?src=pr&el=tree#diff-cGFja2FnZXMvbG9ja2ZpbGUtbGludC1hcGkvc3JjL1BhcnNlTG9ja0ZpbGUuanM=) | `0% <0%> (-100%)` | :arrow_down: |
| [packages/lockfile-lint-api/index.js](https://codecov.io/gh/lirantal/lockfile-lint/pull/2/diff?src=pr&el=tree#diff-cGFja2FnZXMvbG9ja2ZpbGUtbGludC1hcGkvaW5kZXguanM=) | `100% <100%> (ø)` | :arrow_up: |
| [packages/lockfile-lint-api/src/ParseLockfile.js](https://codecov.io/gh/lirantal/lockfile-lint/pull/2/diff?src=pr&el=tree#diff-cGFja2FnZXMvbG9ja2ZpbGUtbGludC1hcGkvc3JjL1BhcnNlTG9ja2ZpbGUuanM=) | `97.36% <97.36%> (ø)` | |
| [packages/lockfile-lint/src/cli.js](https://codecov.io/gh/lirantal/lockfile-lint/pull/2/diff?src=pr&el=tree#diff-cGFja2FnZXMvbG9ja2ZpbGUtbGludC9zcmMvY2xpLmpz) | | |
| [packages/lockfile-lint/src/main.js](https://codecov.io/gh/lirantal/lockfile-lint/pull/2/diff?src=pr&el=tree#diff-cGFja2FnZXMvbG9ja2ZpbGUtbGludC9zcmMvbWFpbi5qcw==) | | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/lirantal/lockfile-lint/pull/2?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/lirantal/lockfile-lint/pull/2?src=pr&el=footer). Last update [1c96a6e...242c1c8](https://codecov.io/gh/lirantal/lockfile-lint/pull/2?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/packages/lockfile-lint-api/index.js b/packages/lockfile-lint-api/index.js
index c50f4f3..4a86475 100644
--- a/packages/lockfile-lint-api/index.js
+++ b/packages/lockfile-lint-api/index.js
@@ -2,10 +2,10 @@
 
 const ValidateHost = require('./src/validators/ValidateHost')
 const ValidateHttps = require('./src/validators/ValidateHttps')
-const ParseLockFile = require('./src/ParseLockFile')
+const ParseLockfile = require('./src/ParseLockfile')
 
 module.exports = {
-  ParseLockFile,
+  ParseLockfile,
   ValidateHost,
   ValidateHttps
 }
diff --git a/packages/lockfile-lint-api/src/ParseLockFile.js b/packages/lockfile-lint-api/src/ParseLockFile.js
deleted file mode 100644
index 1a12642..0000000
--- a/packages/lockfile-lint-api/src/ParseLockFile.js
+++ /dev/null
@@ -1,22 +0,0 @@
-'use strict'
-
-const fs = require('fs')
-const yarnLockFileParser = require('@yarnpkg/lockfile')
-
-class ParseLockFile {
-  constructor (options) {
-    if (!options || typeof options !== 'object') {
-      throw new Error('expecting options object')
-    }
-
-    this.lockFilePath = options.lockFilePath
-  }
-
-  parseSync () {
-    // eslint-disable-next-line security/detect-non-literal-fs-filename
-    const file = fs.readFileSync(this.lockFilePath, 'utf8')
-    return yarnLockFileParser.parse(file)
-  }
-}
-
-module.exports = ParseLockFile
diff --git a/packages/lockfile-lint-api/src/ParseLockfile.js b/packages/lockfile-lint-api/src/ParseLockfile.js
new file mode 100644
index 0000000..dff8ebe
--- /dev/null
+++ b/packages/lockfile-lint-api/src/ParseLockfile.js
@@ -0,0 +1,114 @@
+/* eslint-disable security/detect-object-injection */
+'use strict'
+
+const fs = require('fs')
+const path = require('path')
+const yarnLockfileParser = require('@yarnpkg/lockfile')
+
+class ParseLockfile {
+  /**
+   * constructor
+   * @param {string} options.lockfilePath - path to the lockfile
+   * @param {string} options.lockfileType - the package manager type
+   * for lockfile
+   */
+  constructor (options) {
+    if (!options || typeof options !== 'object') {
+      throw new Error('expecting options object')
+    }
+
+    this.options = {}
+    this.options.lockfilePath = options.lockfilePath
+    this.options.lockfileType = options.lockfileType
+  }
+
+  /**
+   * Synchronously parses a lockfile
+   * @return {object} parsed file
+   */
+  parseSync () {
+    const lockfileParser = this.resolvePkgMgrForLockfile()
+    if (!lockfileParser) {
+      throw new Error('unable to find relevant lockfile parser')
+    }
+
+    // eslint-disable-next-line security/detect-non-literal-fs-filename
+    const file = fs.readFileSync(this.options.lockfilePath, 'utf8')
+    return lockfileParser.call(this, file)
+  }
+
+  resolvePkgMgrForLockfile () {
+    const lockfileResolversByPackageManager = {
+      npm: this.parseNpmLockfile,
+      npmjs: this.parseNpmLockfile,
+      yarn: this.parseYarnLockfile,
+      yarnpkg: this.parseYarnLockfile
+    }
+
+    let resolver
+    if (typeof this.options.lockfileType === 'string' && this.options.lockfileType) {
+      resolver = lockfileResolversByPackageManager[this.options.lockfileType]
+    }
+
+    if (!resolver) {
+      resolver = this.resolvePkgMgrByFilename()
+    }
+
+    return resolver
+  }
+
+  resolvePkgMgrByFilename () {
+    const lockfileResolverByFilename = {
+      'package-lock.json': this.parseNpmLockfile,
+      'yarn.lock': this.parseYarnLockfile
+    }
+
+    const pathInfo = path.parse(this.options.lockfilePath)
+    const baseFilename = pathInfo.base
+
+    return lockfileResolverByFilename[baseFilename]
+  }
+
+  parseYarnLockfile (lockfileBuffer) {
+    return yarnLockfileParser.parse(lockfileBuffer)
+  }
+
+  parseNpmLockfile (lockfileBuffer) {
+    const packageJsonParsed = JSON.parse(lockfileBuffer)
+
+    // transform original format of npm's package-json
+    // to match yarns so we have a unified format to validate
+    // against
+    const npmDepsTree = packageJsonParsed.dependencies
+    const flattenedDepTree = this._flattenNpmDepsTree(npmDepsTree)
+
+    return {
+      type: 'success',
+      object: flattenedDepTree
+    }
+  }
+
+  _flattenNpmDepsTree (npmDepsTree) {
+    let flattenedDepTree = {}
+    let flattenedNestedDepsTree = {}
+    for (const [depName, depMetadata] of Object.entries(npmDepsTree)) {
+      const depMetadataShortend = {
+        version: depMetadata.version,
+        resolved: depMetadata.resolved,
+        integrity: depMetadata.integrity,
+        requires: depMetadata.requires
+      }
+
+      flattenedDepTree[`${depName}@${depMetadata.version}`] = depMetadataShortend
+
+      const nestedDepsTree = depMetadata.dependencies
+      if (nestedDepsTree && Object.keys(nestedDepsTree).length !== 0) {
+        flattenedNestedDepsTree = this._flattenNpmDepsTree(nestedDepsTree)
+      }
+    }
+
+    return Object.assign({}, flattenedDepTree, flattenedNestedDepsTree)
+  }
+}
+
+module.exports = ParseLockfile
diff --git a/packages/lockfile-lint/src/validators/index.js b/packages/lockfile-lint/src/validators/index.js
index 0d86db2..c45e623 100644
--- a/packages/lockfile-lint/src/validators/index.js
+++ b/packages/lockfile-lint/src/validators/index.js
@@ -1,6 +1,6 @@
 'use strict'
 
-const {ValidateHost, ParseLockFile, ValidateHttps} = require('lockfile-lint-api')
+const {ValidateHost, ParseLockfile, ValidateHttps} = require('lockfile-lint-api')
 const debug = require('debug')
 
 module.exports = {
@@ -14,10 +14,11 @@ function ValidateHostManager ({path, type, validatorOptions}) {
   )
 
   const options = {
-    lockFilePath: path
+    lockfilePath: path,
+    lockfileType: type
   }
 
-  const parser = new ParseLockFile(options)
+  const parser = new ParseLockfile(options)
   const lockfile = parser.parseSync()
   const validator = new ValidateHost({packages: lockfile.object})
 
@@ -30,10 +31,11 @@ function ValidateHttpsManager ({path, type, validatorOptions}) {
   )
 
   const options = {
-    lockFilePath: path
+    lockfilePath: path,
+    lockfileType: type
   }
 
-  const parser = new ParseLockFile(options)
+  const parser = new ParseLockfile(options)
   const lockfile = parser.parseSync()
   const validator = new ValidateHttps({packages: lockfile.object})
 


Test patch:
diff --git a/packages/lockfile-lint-api/__tests__/__fixtures__/nested-package-lock.json b/packages/lockfile-lint-api/__tests__/__fixtures__/nested-package-lock.json
new file mode 100644
index 0000000..c740dc1
--- /dev/null
+++ b/packages/lockfile-lint-api/__tests__/__fixtures__/nested-package-lock.json
@@ -0,0 +1,37 @@
+{
+  "name": "abvc",
+  "version": "1.0.0",
+  "lockfileVersion": 1,
+  "requires": true,
+  "dependencies": {
+    "debug": {
+      "version": "4.1.1",
+      "resolved": "https://registry.npmjs.org/debug/-/debug-4.1.1.tgz",
+      "integrity": "sha512-pYAIzeRo8J6KPEaJ0VWOh5Pzkbw/RetuzehGM7QRRX5he4fPHx2rdKMB256ehJCkX+XRQm16eZLqLNS8RSZXZw==",
+      "dev": true,
+      "requires": {
+        "ms": "^2.1.1"
+      },
+      "dependencies": {
+        "ms": {
+          "version": "2.0.0",
+          "resolved": "https://registry.npmjs.org/ms/-/ms-2.0.0.tgz",
+          "integrity": "sha1-VgiurfwAvmwpAd9fmGF4jeDVl8g=",
+          "dependencies": {
+            "escape-html": {
+              "version": "1.0.3",
+              "resolved": "https://registry.npmjs.org/escape-html/-/escape-html-1.0.3.tgz",
+              "integrity": "sha1-Aljq5NPQwJdN4cFpGI7wBR0dGYg="
+            }
+          }
+        }
+      }
+    },
+    "ms": {
+      "version": "2.1.2",
+      "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz",
+      "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==",
+      "dev": true
+    }
+  }
+}
diff --git a/packages/lockfile-lint-api/__tests__/__fixtures__/package-lock.json b/packages/lockfile-lint-api/__tests__/__fixtures__/package-lock.json
new file mode 100644
index 0000000..4d9a9fa
--- /dev/null
+++ b/packages/lockfile-lint-api/__tests__/__fixtures__/package-lock.json
@@ -0,0 +1,23 @@
+{
+  "name": "abvc",
+  "version": "1.0.0",
+  "lockfileVersion": 1,
+  "requires": true,
+  "dependencies": {
+    "debug": {
+      "version": "4.1.1",
+      "resolved": "https://registry.npmjs.org/debug/-/debug-4.1.1.tgz",
+      "integrity": "sha512-pYAIzeRo8J6KPEaJ0VWOh5Pzkbw/RetuzehGM7QRRX5he4fPHx2rdKMB256ehJCkX+XRQm16eZLqLNS8RSZXZw==",
+      "dev": true,
+      "requires": {
+        "ms": "^2.1.1"
+      }
+    },
+    "ms": {
+      "version": "2.1.2",
+      "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz",
+      "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==",
+      "dev": true
+    }
+  }
+}
diff --git a/packages/lockfile-lint-api/__tests__/app.test.js b/packages/lockfile-lint-api/__tests__/app.test.js
index c6b468d..dc89f31 100644
--- a/packages/lockfile-lint-api/__tests__/app.test.js
+++ b/packages/lockfile-lint-api/__tests__/app.test.js
@@ -20,6 +20,6 @@ describe('Validators', () => {
 
 describe('Utils', () => {
   it('library should export ParseLockFile util', () => {
-    expect(app).toHaveProperty('ParseLockFile')
+    expect(app).toHaveProperty('ParseLockfile')
   })
 })
diff --git a/packages/lockfile-lint-api/__tests__/parseLockFile.test.js b/packages/lockfile-lint-api/__tests__/parseLockFile.test.js
index 79d3e57..6bc6f6a 100644
--- a/packages/lockfile-lint-api/__tests__/parseLockFile.test.js
+++ b/packages/lockfile-lint-api/__tests__/parseLockFile.test.js
@@ -1,36 +1,18 @@
 /* eslint-disable no-new */
 'use strict'
 
-const ParseLockFile = require('../src/ParseLockFile')
-const path = require('path')
+const ParseLockfile = require('../src/ParseLockfile')
 
-describe('ParseLockFile', () => {
+describe('ParseLockfile', () => {
   it('instantiating a parser with invalid object throws an error', () => {
     expect(() => {
-      new ParseLockFile()
+      new ParseLockfile()
     }).toThrowError('expecting options object')
   })
 
   it('instantiating a parser with string type throws an error', () => {
     expect(() => {
-      new ParseLockFile('/path/to/file')
+      new ParseLockfile('/path/to/file')
     }).toThrowError('expecting options object')
   })
-
-  it('parsing a yarn lockfile returns an object with packages', () => {
-    const mockYarnLockFilePath = path.join(__dirname, './__fixtures__/yarn.lock')
-    const options = {
-      lockFilePath: mockYarnLockFilePath
-    }
-    const parser = new ParseLockFile(options)
-    const lockfile = parser.parseSync()
-
-    expect(lockfile.type).toEqual('success')
-    expect(lockfile.object).toEqual(
-      expect.objectContaining({
-        'debug@^4.1.1': expect.any(Object),
-        'ms@^2.1.1': expect.any(Object)
-      })
-    )
-  })
 })
diff --git a/packages/lockfile-lint-api/__tests__/parseLockfile.test.js b/packages/lockfile-lint-api/__tests__/parseLockfile.test.js
new file mode 100644
index 0000000..6bc6f6a
--- /dev/null
+++ b/packages/lockfile-lint-api/__tests__/parseLockfile.test.js
@@ -0,0 +1,18 @@
+/* eslint-disable no-new */
+'use strict'
+
+const ParseLockfile = require('../src/ParseLockfile')
+
+describe('ParseLockfile', () => {
+  it('instantiating a parser with invalid object throws an error', () => {
+    expect(() => {
+      new ParseLockfile()
+    }).toThrowError('expecting options object')
+  })
+
+  it('instantiating a parser with string type throws an error', () => {
+    expect(() => {
+      new ParseLockfile('/path/to/file')
+    }).toThrowError('expecting options object')
+  })
+})
diff --git a/packages/lockfile-lint-api/__tests__/parseNpmLockfile.test.js b/packages/lockfile-lint-api/__tests__/parseNpmLockfile.test.js
new file mode 100644
index 0000000..b9e9e17
--- /dev/null
+++ b/packages/lockfile-lint-api/__tests__/parseNpmLockfile.test.js
@@ -0,0 +1,45 @@
+/* eslint-disable no-new */
+'use strict'
+
+const ParseLockfile = require('../src/ParseLockfile')
+const path = require('path')
+
+describe('ParseLockfile Npm', () => {
+  it('parsing an npm lockfile returns an object with packages', () => {
+    const mockNpmLockfilePath = path.join(__dirname, './__fixtures__/package-lock.json')
+    const options = {
+      lockfilePath: mockNpmLockfilePath,
+      lockfileType: 'npm'
+    }
+    const parser = new ParseLockfile(options)
+    const lockfile = parser.parseSync()
+
+    expect(lockfile.type).toEqual('success')
+    expect(lockfile.object).toEqual(
+      expect.objectContaining({
+        'debug@4.1.1': expect.any(Object),
+        'ms@2.1.2': expect.any(Object)
+      })
+    )
+  })
+
+  it('parsing an npm lockfile with nested deps returns an object with packages', () => {
+    const mockNpmLockfilePath = path.join(__dirname, './__fixtures__/nested-package-lock.json')
+    const options = {
+      lockfilePath: mockNpmLockfilePath,
+      lockfileType: 'npm'
+    }
+    const parser = new ParseLockfile(options)
+    const lockfile = parser.parseSync()
+
+    expect(lockfile.type).toEqual('success')
+    expect(lockfile.object).toEqual(
+      expect.objectContaining({
+        'debug@4.1.1': expect.any(Object),
+        'ms@2.1.2': expect.any(Object),
+        'ms@2.0.0': expect.any(Object),
+        'escape-html@1.0.3': expect.any(Object)
+      })
+    )
+  })
+})
diff --git a/packages/lockfile-lint-api/__tests__/parseYarnLockfile.test.js b/packages/lockfile-lint-api/__tests__/parseYarnLockfile.test.js
new file mode 100644
index 0000000..645dab6
--- /dev/null
+++ b/packages/lockfile-lint-api/__tests__/parseYarnLockfile.test.js
@@ -0,0 +1,26 @@
+/* eslint-disable no-new */
+'use strict'
+
+const ParseLockfile = require('../src/ParseLockfile')
+const path = require('path')
+
+describe('ParseLockfile Yarn', () => {
+  it('parsing a yarn lockfile returns an object with packages', () => {
+    const mockYarnLockfilePath = path.join(__dirname, './__fixtures__/yarn.lock')
+    const options = {
+      lockfilePath: mockYarnLockfilePath
+    }
+    const parser = new ParseLockfile(options)
+    const lockfile = parser.parseSync()
+
+    // console.log(lockfile)
+
+    expect(lockfile.type).toEqual('success')
+    expect(lockfile.object).toEqual(
+      expect.objectContaining({
+        'debug@^4.1.1': expect.any(Object),
+        'ms@^2.1.1': expect.any(Object)
+      })
+    )
+  })
+})