lirantal / lockfile-lint

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

Use allowed url patterns in a single hostname #112

Closed eserkaraca closed 1 year ago

eserkaraca commented 2 years ago

Is your feature request related to a problem? Please describe. We have multiple npm repositories served on a single host, release, and dev on different paths. e.g.: Dev repo: https://artifactory.example.com/npm/DEV/... Release repo: https://artifactory.example.com/npm/REL/...

I want to enforce release usage.

Describe the solution you'd like Have a parameter to specify allowed URLs but with wildcard support. npx lockfile-lint --allowed-url-patterns https://artifactory.example.com/npm/REL/*/prefix

'*' means any char repeated any times.

Describe alternatives you've considered Instead of a simple glob pattern, regex can be used. But that may be unnecessarily complex as for regex you'll need to escape the URL.

lirantal commented 2 years ago

Hi @eserkaraca,

I think that's a good idea for an enhancement. I added some labels with hope that someone picks this up. If you're like to submit a PR yourself (or anyone on your team), I'd be happy to merge it and release the new feature support.

emanuelb commented 1 year ago

Another usecase for this feature is to ensure urls on github are from allowed orgs such as only the org developing the project, example in: https://github.com/BlueWallet/BlueWallet/issues/5329

lirantal commented 1 year ago

Yep, makes total sense and that's another good use-case @emanuelb.

I would say we could update --allowed-urls logic to also support patterns. WDYT?

emanuelb commented 1 year ago

Yes, both options are ok (adding another parameter that support patterns such as globbing or regex, or adding this functionality to already available --allowed-urls parameter, in case the current parameter will be used it's better to ensure the approach used wont change the functionality in a way that will make previous usages behave differently, such as if regex is used then . in domain will mean any char and thus enabling urls from another domain, etc..)

I prefer a new parameter name to support additional patterns because of the reasons mentioned above (not change previous usages in any way, less chances to introduce bugs that way for usage that dont expect any patterns)

eins78 commented 1 year ago

I found this issue after having the same need, I would propose to add a --allowed-registries option to keep it in line with how .npmrc is configured. It would validate the scheme, hostname and path-prefix of every given registry URLs. For complex setups wildcards might be desired, but I personally would rather list all the allowed registries.

eins78 commented 1 year ago

@eserkaraca It seems to me that your example use case could be supported without a glob or regex. Do you agree, or is the …*/prefix part somehow important? E.g.

npx lockfile-lint --allowed-registries https://artifactory.example.com/npm/REL

or for multiple registries

npx lockfile-lint --allowed-registries https://artifactory.example.com/npm/DEV,https://artifactory.example.com/npm/REL
lirantal commented 1 year ago

@eins78 per my above comment about using allowed-urls - I generally still think it holds to just use that and update the current capability to regexes.

However, as @emanuelb pointed out it might break existing URLs. I find --allowed-registries to be a bit confusing, so maybe we use --allowed-urls-regex to specify glob matching in URLs? (I'm not actually sure yet if "glob" or actual regex syntax but let's land on what s solution would look like first).

EDIT: @eins78 specifically for your use case above with the registries, can you explain why the existing -allowed-urls flag doesn't work for you?

baruchiro commented 1 year ago

I'm not sure if it is related, but I have an internal registry too, and I can't make it ignore my private packages:

"@cxui/cypress-util@1.0.10":
  version "1.0.10"
  resolved "https://checkmarx.jfrog.io/artifactory/api/npm/team-npm/@cxui/cypress-util/-/@cxui/cypress-util-1.0.10.tgz#3134312351eb248c1c4561d393afc6d8c23b2943"

And I get

detected resolved URL for package with a different name: @cxui/cypress-util
    expected: @cxui/cypress-util
    actual: artifactory/api/npm/team-npm/@cxui/cypress-util

Settings:

"lockfile-lint": {
  "allowed-hosts": [
    "npm",
    "yarn",
    "checkmarx.jfrog.io"
  ],
  "allowed-urls": [
    "https://checkmarx.jfrog.io/artifactory/api/npm/team-npm/@cxui/cypress-util/",
    "https://checkmarx.jfrog.io/artifactory/api/npm/team-npm/@cxui/",
    "https://checkmarx.jfrog.io/artifactory/api/npm/team-npm/@cxui",
    "https://checkmarx.jfrog.io/artifactory/api/npm/team-npm/",
    "https://checkmarx.jfrog.io/artifactory/api/npm/team-npm",
    "https://checkmarx.jfrog.io/artifactory/api/npm/",
    "https://checkmarx.jfrog.io/artifactory/api/npm",
    "https://checkmarx.jfrog.io/artifactory/api/",
    "https://checkmarx.jfrog.io/artifactory/api",
    "https://checkmarx.jfrog.io/artifactory/",
    "https://checkmarx.jfrog.io/artifactory",
    "https://checkmarx.jfrog.io/",
    "https://checkmarx.jfrog.io"
  ],
  "validate-https": true,
  "validate-package-names": true,
  "validate-integrity": true,
  "empty-hostname": false
}
lirantal commented 1 year ago

@baruchiro the problem with your setup is specifically the validate-package-names which validates the package name resolves to a matching name in the registry but it only knows to parse the open registries (npm/yarn) so the matching fails.

In this case, here are a few options we can solve this:

  1. Ignore checking package names if the registry isn't one of the officials (npm/yarn) and not report on mis-matches
  2. Allow you to specify a flag like package-name-url-prefixes where you can write https://checkmarx.jfrog.io/artifactory/api/npm/team-npm/ so that we parse everything after that to compare the package name for a match
lirantal commented 1 year ago

Fixed with the new PR to skip them. Please re-open if we want to revisit this.