lirantal / lockfile-lint

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

Feat: whitelist sources based on specific properties #39

Closed lirantal closed 4 years ago

lirantal commented 4 years ago

Describe the solution you'd like In continuation to discussion in https://github.com/lirantal/lockfile-lint/issues/11 we'd like to be able to also whitelist specific URLs to resources such as files hosted on a GitHub archive.

I would prioritize to support:

  1. Specific GitHub URLs to a commit/hash
  2. Wild-card for a GitHub repo, as in: https://github.com/lirantal/lockfile-lint
bolatovumar commented 4 years ago

I would be interested in this feature. For 1. would it look something like this: npx lockfile-lint --path yarn.lock --validate-https --allowed-hosts github.com/<org-name>/<repo-name>#<hash>?

lirantal commented 4 years ago

@bolatovumar would be happy to have you submit a PR for this.

Implementation-wise, the current allowed-hosts is checking for the actual host, as in github.com is a host, where-as https://github.com/lirantal... is a URL or href value from the new URL API on Node.js. I'm wondering if we should update the current behavior of allowed-hosts then to match the URL which is a smaller change, or introduce a new --allowed-urls to handle it which may introduce some challenges (see https://github.com/lirantal/lockfile-lint/issues/11 for more context)

ixti commented 4 years ago

I think --allowed-hosts is good-enough (although in case of abc.xys/foo/bar#baz it sounds a bit erroneous, as it's not host anymore but url sort of). Perfect addition for that will be --allowed-schemas that will be ["https", "git+ssh"] by default, and which can supersede --validate-https.

bolatovumar commented 4 years ago

I'm willing to take a crack at implementing this once I have some time. The use case I need to support is the case when a certain project forks an existing package and uses the forked version which is hosted on Github. See this line for an example: https://github.com/LN-Zap/zap-desktop/blob/master/package.json#L324

lirantal commented 4 years ago

schemas and https are already taken care in terms of being supported. I'm thinking the same about allowed hosts being confused with a URL which is why I didn't want to use it for the same thing.

lirantal commented 4 years ago

@bolatovumar yep I agree. the case of specifying https://github.com/LN-Zap/bolt11#0492874ea9ced4ab49bf0f59a62368687f147247 as a whitelist is exactly the case of a URI rather than a host.

It's not "hard" to do, just that I had offloaded some validates to be independent, and now we need to combine them so that one validator doesn't reject the other.

lirantal commented 4 years ago

Addressed in https://github.com/lirantal/lockfile-lint/pull/52