Repository: lirantal/lockfile-lint (js)

Issue:
Packages in local filesystem always yield errors when using -s (yarn)
Thanks for releasing this very important tool! There's one small issue preventing us from adopting it:

## Expected Behavior

If you add a package from the local filesystem (e.g. `yarn add path/to/my/package`), an entry is created in `yarn.lock` corresponding to the package which has no `resolved` field. I would expect this package to be ignored by `lockfile-lint` (or at least to have a flag to ignore it), given that the code is already on the host machine, even when the user has specifically requested to validate protocol schemas (or whitelisted allowed schemes some other way).

## Current Behavior

```console
ABORTING lockfile lint process due to error exceptions 

Encountered error Invalid URL: undefined in package: "my-local-package@../my-local-package" 

TypeError [ERR_INVALID_URL]: Invalid URL: undefined
    at onParseError (internal/url.js:241:17)
    at new URL (internal/url.js:319:5)
    at ValidateHost.validate (/home/richard/.npm/_npx/31760/lib/node_modules/lockfile-lint/node_modules/lockfile-lint-api/src/validators/ValidateHost.js:30:30)
    at ValidateHostManager (/home/richard/.npm/_npx/31760/lib/node_modules/lockfile-lint/src/validators/index.js:43:20)
    at validators.forEach.validator (/home/richard/.npm/_npx/31760/lib/node_modules/lockfile-lint/src/main.js:35:28)
    at Array.forEach (<anonymous>)
    at Object.runValidators (/home/richard/.npm/_npx/31760/lib/node_modules/lockfile-lint/src/main.js:25:14)
    at Object.<anonymous> (/home/richard/.npm/_npx/31760/lib/node_modules/lockfile-lint/bin/lockfile-lint.js:31:17)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10) 

error: command failed with exit code 1 
```

## Possible Solution

Skipping host validation for entries without a `resolved` field in `ValidateHost.js` would resolve this, but I'm not sure whether it's acceptable to do so other than behind a flag.

## Steps to Reproduce (for bugs)

https://github.com/richsilv/lockfile-lint-bug-example/tree/master

## Context

We have a single, common shared library required by a variety of different services, and we find it simpler to copy this package into the local filesystem and install from there when building containers than via a private or self-hosted registry. I can't imagine we're the only people doing this (although I'd be interested to know if I was wrong!). For all other, registry-sourced packages, we would certainly want to validate the protocol.

## Your Environment

- Library Version used: 3.0.5
- Node.js version (e.g. Node.js 5.4): 10.16.2
- Operating System and version (desktop or mobile): Ubuntu 18.04


Pull request:
fix(validators): skip packages with no resolved field
Prevent URL-based validators (Host, Scheme or HTTPS) throwing errors when no `resolved` field exists for the package.

fix #42

<!--- Provide a general summary of your changes in the Title above -->

## Description
`ValidateHost`, `ValidateHttps` and `ValidateScheme` are updated to skip validation for any packages which don't have a `resolved` field in the lock-file.
<!--- 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: -->

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Related Issue
https://github.com/lirantal/lockfile-lint/issues/42
<!--- 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
Some package entries have no `resolved` field, for example those installed from the local filesystem. For these packages, protocol, scheme and host validation should be skipped completely as there is no external source to check, but the validators are currently trying to parse a non-existent URL which results in a validation failure.
<!--- Why is this change required? What problem does it solve? -->

## How Has This Been Tested?
New tests added to the `validateHost`, `validateHttps` and `validateScheme` specs to reflect the expected behaviour; tests failed before changes, and pass afterwards.
<!--- 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.
- [x] I added a picture of a cute animal cause it's fun

![Baby-Goose-Gosling-10](https://user-images.githubusercontent.com/3180526/72539118-bc9e2a80-3876-11ea-8170-8f84c42eb289.jpg)


Patch:
diff --git a/packages/lockfile-lint-api/src/validators/ValidateHost.js b/packages/lockfile-lint-api/src/validators/ValidateHost.js
index 683495a..aa1b3cb 100644
--- a/packages/lockfile-lint-api/src/validators/ValidateHost.js
+++ b/packages/lockfile-lint-api/src/validators/ValidateHost.js
@@ -25,6 +25,10 @@ module.exports = class ValidateHost {
     }
 
     for (const [packageName, packageMetadata] of Object.entries(this.packages)) {
+      if (!('resolved' in packageMetadata)) {
+        continue
+      }
+
       let packageResolvedURL = {}
       try {
         packageResolvedURL = new URL(packageMetadata.resolved)
diff --git a/packages/lockfile-lint-api/src/validators/ValidateHttps.js b/packages/lockfile-lint-api/src/validators/ValidateHttps.js
index dc64ffc..380fb64 100644
--- a/packages/lockfile-lint-api/src/validators/ValidateHttps.js
+++ b/packages/lockfile-lint-api/src/validators/ValidateHttps.js
@@ -21,6 +21,10 @@ module.exports = class ValidateHttps {
     }
 
     for (const [packageName, packageMetadata] of Object.entries(this.packages)) {
+      if (!('resolved' in packageMetadata)) {
+        continue
+      }
+
       let packageResolvedURL = {}
       try {
         packageResolvedURL = new URL(packageMetadata.resolved)
diff --git a/packages/lockfile-lint-api/src/validators/ValidateScheme.js b/packages/lockfile-lint-api/src/validators/ValidateScheme.js
index 9cab6e1..3b7464f 100644
--- a/packages/lockfile-lint-api/src/validators/ValidateScheme.js
+++ b/packages/lockfile-lint-api/src/validators/ValidateScheme.js
@@ -23,6 +23,10 @@ module.exports = class ValidateProtocol {
     }
 
     for (const [packageName, packageMetadata] of Object.entries(this.packages)) {
+      if (!('resolved' in packageMetadata)) {
+        continue
+      }
+
       let packageResolvedURL = {}
       try {
         packageResolvedURL = new URL(packageMetadata.resolved)


Test patch:
diff --git a/packages/lockfile-lint-api/__tests__/validators.host.test.js b/packages/lockfile-lint-api/__tests__/validators.host.test.js
index a8b86d1..7100188 100644
--- a/packages/lockfile-lint-api/__tests__/validators.host.test.js
+++ b/packages/lockfile-lint-api/__tests__/validators.host.test.js
@@ -195,4 +195,18 @@ describe('Validator: Host', () => {
       ]
     })
   })
+
+  it('validator should not throw if package has no `resolved` field', () => {
+    const mockedPackages = {
+      '@babel/code-frame': {
+        resolved: 'https://registry.npmjs.org/@babel/code-frame/-/code-frame-7.0.0.tgz'
+      },
+      meow: {}
+    }
+    const validator = new ValidatorHost({packages: mockedPackages})
+
+    expect(() => {
+      validator.validate(['npm'])
+    }).not.toThrow()
+  })
 })
diff --git a/packages/lockfile-lint-api/__tests__/validators.https.test.js b/packages/lockfile-lint-api/__tests__/validators.https.test.js
index c8ebbc1..f316f70 100644
--- a/packages/lockfile-lint-api/__tests__/validators.https.test.js
+++ b/packages/lockfile-lint-api/__tests__/validators.https.test.js
@@ -70,4 +70,22 @@ describe('Validator: HTTPS', () => {
 
     expect(() => validator.validate(['npm'])).toThrow(PackageError)
   })
+
+  it('validator should succeed if package has no `resolved` field', () => {
+    const mockedPackages = {
+      '@babel/code-frame': {
+        resolved: 'https://registry.npmjs.org/@babel/code-frame/-/code-frame-7.0.0.tgz'
+      },
+      meow: {},
+      '@babel/generator': {
+        resolved: 'https://registry.npmjs.org/@babel/generator/-/generator-7.4.4.tgz'
+      }
+    }
+
+    const validator = new ValidatorHTTPS({packages: mockedPackages})
+    expect(validator.validate()).toEqual({
+      type: 'success',
+      errors: []
+    })
+  })
 })
diff --git a/packages/lockfile-lint-api/__tests__/validators.scheme.test.js b/packages/lockfile-lint-api/__tests__/validators.scheme.test.js
index 7bf961c..e6c0d8e 100644
--- a/packages/lockfile-lint-api/__tests__/validators.scheme.test.js
+++ b/packages/lockfile-lint-api/__tests__/validators.scheme.test.js
@@ -80,4 +80,21 @@ describe('Validator: Protocol', () => {
 
     expect(() => validator.validate(['npm'])).toThrow(PackageError)
   })
+
+  it('validator should succeed if package has no `resolved` field', () => {
+    const mockedPackages = {
+      '@babel/code-frame': {
+        resolved: 'https://registry.npmjs.org/@babel/code-frame/-/code-frame-7.0.0.tgz'
+      },
+      meow: {},
+      '@babel/generator': {
+        resolved: 'git+ssh://registry.npmjs.org/@babel/generator/-/generator-7.4.4.tgz'
+      }
+    }
+    const validator = new ValidatorScheme({packages: mockedPackages})
+    expect(validator.validate(['https:', 'git+ssh:'])).toEqual({
+      type: 'success',
+      errors: []
+    })
+  })
 })