lirantal / lockfile-lint

Lint an npm or yarn lockfile to analyze and detect security issues
Apache License 2.0
781 stars 35 forks source link

Packages in local filesystem always yield errors when using -s (yarn) #42

Closed richsilv closed 4 years ago

richsilv commented 4 years ago

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

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

richsilv commented 4 years ago

Happy to submit a PR to fix this if you confirm that it's a valid issue and whether it requires a new flag.

kruczy commented 4 years ago

have the same issue with our locally installed packages

our use case could be more common as we use it in a monorepo where we do not publish any of the packages to a registry

lirantal commented 4 years ago

Greatly detailed report and reproducing steps, thank you @richsilv. I agree and confirm that by default local packages such as this should be ignored. The PR should be simple and small so if you'd like to submit it I'd gladly merge.

Thanks @kruczy for confirming also that this is a valuable fix for you too.