Repository: lirantal/lockfile-lint
Pull request #26 (2019-11-22 17:00:55)

feat(emptyhostname): allow empty hostnames in lockfiles
# Description

BREAKING CHANGE: lockfile-lint-api internal method API has changed its function signature
to allow receiving a value, and then an options object in a second argument.

<!--- 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

Relevant issues:
- https://github.com/lirantal/lockfile-lint/issues/23
- https://github.com/lirantal/lockfile-lint/issues/25

<!--- 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

Due to how `npm install <github repo>` works with implicitly referencing a github hostname without mentioning it, which causes a poor developer experience for users of lockfile-lint

<!--- Why is this change required? What problem does it solve? -->

## How Has This Been Tested?

<!--- 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! -->

- [x] I have updated the documentation (if required).
- [x] I have read the **CONTRIBUTING** document.
- [x] I have added tests to cover my changes.
- [x] 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/26?src=pr&el=h1) Report
> Merging [#26](https://codecov.io/gh/lirantal/lockfile-lint/pull/26?src=pr&el=desc) into [master](https://codecov.io/gh/lirantal/lockfile-lint/commit/abc71fa09a39ef2975648ac540eb4c57534ee463?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

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

```diff
@@           Coverage Diff           @@
##           master      #26   +/-   ##
=======================================
  Coverage   97.22%   97.22%           
=======================================
  Files          11       11           
  Lines         180      180           
  Branches       25       25           
=======================================
  Hits          175      175           
  Misses          5        5
```


| [Impacted Files](https://codecov.io/gh/lirantal/lockfile-lint/pull/26?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [packages/lockfile-lint/src/main.js](https://codecov.io/gh/lirantal/lockfile-lint/pull/26/diff?src=pr&el=tree#diff-cGFja2FnZXMvbG9ja2ZpbGUtbGludC9zcmMvbWFpbi5qcw==) | `95.65% <ø> (ø)` | :arrow_up: |
| [packages/lockfile-lint/src/cli.js](https://codecov.io/gh/lirantal/lockfile-lint/pull/26/diff?src=pr&el=tree#diff-cGFja2FnZXMvbG9ja2ZpbGUtbGludC9zcmMvY2xpLmpz) | `0% <ø> (ø)` | :arrow_up: |
| [packages/lockfile-lint/src/validators/index.js](https://codecov.io/gh/lirantal/lockfile-lint/pull/26/diff?src=pr&el=tree#diff-cGFja2FnZXMvbG9ja2ZpbGUtbGludC9zcmMvdmFsaWRhdG9ycy9pbmRleC5qcw==) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/lirantal/lockfile-lint/pull/26?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/26?src=pr&el=footer). Last update [abc71fa...ec4d934](https://codecov.io/gh/lirantal/lockfile-lint/pull/26?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/packages/lockfile-lint/README.md b/packages/lockfile-lint/README.md
index cb20fb1..73b49de 100644
--- a/packages/lockfile-lint/README.md
+++ b/packages/lockfile-lint/README.md
@@ -61,7 +61,6 @@ lockfile-lint --path yarn.lock --allowed-hosts yarn github.com --validate-https
 - `--allowed-hosts` explicitly set to match github.com as a host and specifies `yarn` as the alias for yarn's official mirror host
 - `--allowed-schemes` overrides `validate-https` and so it explicitly allows both `https:` and `git+https:` for the github URL
 
-
 # CLI command options
 
 | command line argument        | description                                                                                                                                                                                                                                                                                | implemented    |
@@ -70,7 +69,8 @@ lockfile-lint --path yarn.lock --allowed-hosts yarn github.com --validate-https
 | `--type`, `-t`               | lockfile type, options are `npm` or `yarn`                                                                                                                                                                                                                                                 | ✅             |
 | `--validate-https`, `-s`     | validates the use of HTTPS as protocol schema for all resources in the lockfile                                                                                                                                                                                                            | ✅             |
 | `--allowed-hosts`, `-a`      | validates a whitelist of allowed hosts to be used for all resources in the lockfile. Supported short-hands aliases are `npm`, `yarn`, and `verdaccio` which will match URLs `https://registry.npmjs.org`, `https://registry.yarnpkg.com` and `https://registry.verdaccio.org` respectively | ✅             |
-| `--allowed-schemes`, `-o`    | allowed [URI schemes](https://tools.ietf.org/html/rfc2396#section-3.1) such as "https:", "http", "git+ssh:", or "git+https:"                                                                                                                                                                                                                                                                                        | ✅             |
+| `--allowed-schemes`, `-o`    | allowed [URI schemes](https://tools.ietf.org/html/rfc2396#section-3.1) such as "https:", "http", "git+ssh:", or "git+https:"                                                                                                                                                               | ✅             |
+| `--empty-hostname`, `-e`     | allow empty hostnames, or set to false if you wish for a stricter policy                                                                                                                                                                                                                   | ✅             |
 | `--validate-checksum`, `-c`  | check that all resources include a checksum                                                                                                                                                                                                                                                | ❌ PRs welcome |
 | `--validate-integrity`, `-i` | check that all resources include an integrity field                                                                                                                                                                                                                                        | ❌ PRs welcome |
 
diff --git a/packages/lockfile-lint/bin/lockfile-lint.js b/packages/lockfile-lint/bin/lockfile-lint.js
index dc76377..aa6abf7 100755
--- a/packages/lockfile-lint/bin/lockfile-lint.js
+++ b/packages/lockfile-lint/bin/lockfile-lint.js
@@ -18,7 +18,10 @@ for (const [commandArgument, commandValue] of Object.entries(cli)) {
     const validatorItem = supportedValidators.get(commandArgument)
     validators.push({
       name: validatorItem,
-      options: commandValue
+      values: commandValue,
+      options: {
+        emptyHostname: cli['empty-hostname']
+      }
     })
   }
 }
diff --git a/packages/lockfile-lint/src/cli.js b/packages/lockfile-lint/src/cli.js
index e4ff3b9..24feadb 100644
--- a/packages/lockfile-lint/src/cli.js
+++ b/packages/lockfile-lint/src/cli.js
@@ -24,6 +24,12 @@ const argv = yargs
       type: 'boolean',
       describe: 'validates the use of HTTPS as protocol schema for all resources'
     },
+    e: {
+      alias: 'empty-hostname',
+      type: 'boolean',
+      default: true,
+      describe: 'allows empty hostnames, or set to false if you wish for a stricter policy'
+    },
     a: {
       alias: ['allowed-hosts'],
       type: 'array',
diff --git a/packages/lockfile-lint/src/main.js b/packages/lockfile-lint/src/main.js
index 8cce8f0..d0a99a0 100644
--- a/packages/lockfile-lint/src/main.js
+++ b/packages/lockfile-lint/src/main.js
@@ -35,6 +35,7 @@ function runValidators ({type, path, validators} = {}) {
     let validationResult = validatorFunction({
       path,
       type,
+      validatorValues: validator.values,
       validatorOptions: validator.options
     })
 
diff --git a/packages/lockfile-lint/src/validators/index.js b/packages/lockfile-lint/src/validators/index.js
index 56cfc8d..f81e485 100644
--- a/packages/lockfile-lint/src/validators/index.js
+++ b/packages/lockfile-lint/src/validators/index.js
@@ -9,9 +9,9 @@ module.exports = {
   ValidateSchemeManager
 }
 
-function ValidateSchemeManager ({path, type, validatorOptions}) {
+function ValidateSchemeManager ({path, type, validatorValues, validatorOptions}) {
   debug('validate-scheme-manager')(
-    `invoked with validator options: ${JSON.stringify(validatorOptions)}`
+    `invoked with validator options: ${JSON.stringify(validatorValues)}`
   )
 
   const options = {
@@ -23,12 +23,12 @@ function ValidateSchemeManager ({path, type, validatorOptions}) {
   const lockfile = parser.parseSync()
   const validator = new ValidateScheme({packages: lockfile.object})
 
-  return validator.validate(validatorOptions)
+  return validator.validate(validatorValues)
 }
 
-function ValidateHostManager ({path, type, validatorOptions}) {
+function ValidateHostManager ({path, type, validatorValues, validatorOptions}) {
   debug('validate-host-manager')(
-    `invoked with validator options: ${JSON.stringify(validatorOptions)}`
+    `invoked with validator options: ${JSON.stringify(validatorValues)}`
   )
 
   const options = {
@@ -40,12 +40,12 @@ function ValidateHostManager ({path, type, validatorOptions}) {
   const lockfile = parser.parseSync()
   const validator = new ValidateHost({packages: lockfile.object})
 
-  return validator.validate(validatorOptions)
+  return validator.validate(validatorValues, validatorOptions)
 }
 
-function ValidateHttpsManager ({path, type, validatorOptions}) {
+function ValidateHttpsManager ({path, type, validatorValues, validatorOptions}) {
   debug('validate-host-manager')(
-    `invoked with validator options: ${JSON.stringify(validatorOptions)}`
+    `invoked with validator options: ${JSON.stringify(validatorValues)}`
   )
 
   const options = {


Test patch:
diff --git a/packages/lockfile-lint/__tests__/fixtures/package-lock-empty-hostname.json b/packages/lockfile-lint/__tests__/fixtures/package-lock-empty-hostname.json
new file mode 100644
index 0000000..93a437c
--- /dev/null
+++ b/packages/lockfile-lint/__tests__/fixtures/package-lock-empty-hostname.json
@@ -0,0 +1,47 @@
+{
+  "name": "asdada",
+  "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==",
+      "requires": {
+        "ms": "^2.1.1"
+      }
+    },
+    "metalsmith-permalinks": {
+      "version": "github:XhmikosR/metalsmith-permalinks#432843d5823a292b2e47397ba46fd761d03eb9d3",
+      "from": "github:XhmikosR/metalsmith-permalinks#master",
+      "requires": {
+        "debug": "^4.1.1",
+        "moment": "^2.24.0",
+        "slugify": "^1.3.5",
+        "substitute": "https://github.com/segment-boneyard/substitute/archive/0.1.0.tar.gz"
+      },
+      "dependencies": {
+        "substitute": {
+          "version": "https://github.com/segment-boneyard/substitute/archive/0.1.0.tar.gz",
+          "integrity": "sha512-2ccHCDdgHt0ZrXAFM/7G/3zEwMhsUfOKfwzSAv2vqO2JQcPpNAlp10e8F5uMa6zpGFYRrdy7BGLvBZsthHrLag=="
+        }
+      }
+    },
+    "moment": {
+      "version": "2.24.0",
+      "resolved": "https://registry.npmjs.org/moment/-/moment-2.24.0.tgz",
+      "integrity": "sha512-bV7f+6l2QigeBBZSM/6yTNq4P2fNpSWj/0e7jQcy87A8e7o2nAfP/34/2ky5Vw4B9S446EtIhodAzkFCcR4dQg=="
+    },
+    "ms": {
+      "version": "2.1.2",
+      "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz",
+      "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w=="
+    },
+    "slugify": {
+      "version": "1.3.6",
+      "resolved": "https://registry.npmjs.org/slugify/-/slugify-1.3.6.tgz",
+      "integrity": "sha512-wA9XS475ZmGNlEnYYLPReSfuz/c3VQsEMoU43mi6OnKMCdbnFXd4/Yg7J0lBv8jkPolacMpOrWEaoYxuE1+hoQ=="
+    }
+  }
+}
diff --git a/packages/lockfile-lint/__tests__/main.test.js b/packages/lockfile-lint/__tests__/main.test.js
index dc34c6f..643e4c4 100644
--- a/packages/lockfile-lint/__tests__/main.test.js
+++ b/packages/lockfile-lint/__tests__/main.test.js
@@ -83,7 +83,7 @@ describe('Main CLI logic', () => {
       const validators = [
         {
           name: 'validateHosts',
-          options: ['npm']
+          values: ['npm']
         }
       ]
 
@@ -97,13 +97,38 @@ describe('Main CLI logic', () => {
       expect(result.validatorCount).toEqual(1)
       expect(result.validatorSuccesses).toEqual(0)
     })
+
+    test('a failing validator should throw an error if an empty host is not allowed', () => {
+      const lockfilePath = `${__dirname}/fixtures/package-lock-empty-hostname.json`
+      const lockfileType = 'npm'
+      const validators = [
+        {
+          name: 'validateHosts',
+          values: ['npm'],
+          options: {
+            emptyHostname: false
+          }
+        }
+      ]
+
+      const result = main.runValidators({
+        path: lockfilePath,
+        type: lockfileType,
+        validators
+      })
+
+      expect(result.validatorFailures).toEqual(2)
+      expect(result.validatorCount).toEqual(1)
+      expect(result.validatorSuccesses).toEqual(0)
+    })
+
     test('a successful validator should return proper validation object', () => {
       const lockfilePath = `${__dirname}/fixtures/yarn-only-https.lock`
       const lockfileType = 'yarn'
       const validators = [
         {
           name: 'validateHosts',
-          options: ['yarn']
+          values: ['yarn']
         }
       ]
 
@@ -126,7 +151,7 @@ describe('Main CLI logic', () => {
       const validators = [
         {
           name: 'validateSchemes',
-          options: ['https']
+          values: ['https']
         }
       ]
 
@@ -147,7 +172,7 @@ describe('Main CLI logic', () => {
       const validators = [
         {
           name: 'validateSchemes',
-          options: ['https:', 'git+https:']
+          values: ['https:', 'git+https:']
         }
       ]
 
@@ -168,7 +193,7 @@ describe('Main CLI logic', () => {
       const validators = [
         {
           name: 'validateSchemes',
-          options: ['https']
+          values: ['https']
         }
       ]
 
@@ -189,7 +214,7 @@ describe('Main CLI logic', () => {
       const validators = [
         {
           name: 'validateSchemes',
-          options: ['https:', 'git+https:']
+          values: ['https:', 'git+https:']
         }
       ]