Issue with owner/repo packages

[XhmikosR]

So, I was trying a package saved as owner/repo: https://github.com/nodejs/nodejs.org/commit/654e9915c0d1b8c7db92e0c2e39b6836b10e73fe

This doesn't pass with allowed host github, npm, but shouldn't it pass?

[XhmikosR]

...and if I specify the full GH URL then I hit the git+https error which is invalid since that's how npm saves the packages.

https://github.com/nodejs/nodejs.org/pull/2671/checks?check_run_id=305868263#step:9:30

[lirantal]

hey @XhmikosR, you are referring specifically to this package the way it is set in package.json:

    "metalsmith-permalinks": "github:segmentio/metalsmith-permalinks#master",

and it's corresponding lockfile entry:

    "metalsmith-permalinks": {
      "version": "github:segmentio/metalsmith-permalinks#432843d5823a292b2e47397ba46fd761d03eb9d3",
      "from": "github:segmentio/metalsmith-permalinks#master",

If so, it looks like the syntax here is just github: instead of the fully qualified domain name such as github.com

[lirantal]

Further on, a few things to note:

• --allowed-hosts takes an actual hostname, so something like --allowed-hosts "github.com" or it can take an alias. The supported aliases right now are: verdaccio, npm and yarn which resolve to the hosts for these mirrors.

Using your package-lock.json as an example I get these issues:

The first problem, related to metalsmith-permalinks package is that since only github is used then it is being identified as the protocol, not a hostname which is why --allowed-hosts github doesn't work.

To resolve the issue with the metalsmith package you'd have to both allow the github protocol but also support the empty hostname, and so the following should pass for that lockfile:

lockfile-lint --path package-lock.json --allowed-hosts yarn npm github.com "" --allowed-schemes "github:" "https:"

Sadly, that's not ideal because we're allowing an empty hostname there. It's not very obvious, but also since that hostname is empty so it's not in any malicious party's control and so not that bad either.

@XhmikosR Let me know if this works for you.

--

On a related note, does npm know how to handle a lockfile with just the github entry such as "github:segmentio/metalsmith-permalinks#master" ? I don't think I've seen this syntax before.

[XhmikosR]

Hey, @lirantal.

npm i XhmikosR/metalsmith-permalinks#master results in:

"version": "github:XhmikosR/metalsmith-permalinks#432843d5823a292b2e47397ba46fd761d03eb9d3",
"from": "github:XhmikosR/metalsmith-permalinks#master",

And npm i https://github.com/XhmikosR/metalsmith-permalinks.git#master results in:

      "version": "git+https://github.com/XhmikosR/metalsmith-permalinks.git#432843d5823a292b2e47397ba46fd761d03eb9d3",
      "from": "git+https://github.com/XhmikosR/metalsmith-permalinks.git#master",

So, yeah, npm does know how to handle this and it's totally valid. Allowing an empty host, I'm not sure it makes a lot of sense, though?

git+https should pass IMO since it's a valid npm protocol. Also, I'm not sure if there are more cases. Maybe git+ssh.

[lirantal]

to be honest, the problem I have is with npm allowing something like this:

npm i XhmikosR/metalsmith-permalinks#master

which then becomes

"version": "github:XhmikosR/metalsmith-permalinks#432843d5823a292b2e47397ba46fd761d03eb9d3",

because:

1. it implicitly translates that into github. how does it know I didn't intend to refer to a repository on bitbucket?
2. github:XhmikosR/metalsmith-permalinks#432843d5823a292b2e47397ba46fd761d03eb9d3 is simply not a valid WHATWG URL which is causing issues when parsing it (see examples here https://url.spec.whatwg.org/#example-url-parsing)

[lirantal]

At least with yarn, if you add this package the lockfile properly refers to a standardized URL:

[XhmikosR]

Because github is the default for npm. The same way it works if you do "repository": "lirantal/lockfile-lint" in package.json.

As for parsing it, yeah, it would require replacing the github part with https. Also, do note that there might be more cases like this. Maybe git+ssh.

[lirantal]

I understand it is the default, just expressing my personal view of not liking this implicit approach :)

As for supporting other schemes, as long as you provide the scheme it should work just fine, regardless to what it actually is. So for example, if you have git+ssh type of resources then you should do this:

--allowed-schemes "git+ssh:"

[lirantal]

@XhmikosR wanted to let you know that this is now fixed in latest versions of lockfile-lint, try this:

npx lockfile-lint -p package-lock.json -t npm -a npm github.com -o "https:" "github:"

let me know if you have any questions or further issues but I'm confident this should solve the usage problem for you.

[XhmikosR]

Hey, @lirantal, sorry for the late reply :)

So I just tried this on my branch and it still fails:

> nodejs.org@ test:lint:lockfile C:\Users\xmr\Desktop\nodejs.org
> lockfile-lint --allowed-hosts npm github.com --allowed-schemes "https:" "git+https:" --empty-hostname false --validate-https --type npm --path package-lock.json

detected invalid protocol for package: metalsmith-permalinks@git+https://github.com/segmentio/metalsmith-permalinks.git#432843d5823a292b2e47397ba46fd761d03eb9d3
    expected: https:
    actual: git+https:

error: command failed with exit code 1

https://github.com/nodejs/nodejs.org/pull/2671/commits/1fc4e2567d5cc25fcc0911103ec3baf37254c50e#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R21

[lirantal]

you don't actually need --validate-https

try again and let me know?

[lirantal]

@XhmikosR forgot to mention you for a ping about ☝️

[XhmikosR]

Hey, @lirantal. Indeed without --validate-https it works. I feel something isn't clear, though. Maybe there should be an error in these cases?

[lirantal]

@XhmikosR I agree. Can you open a new issue for us to discuss this and some solutions we can think of?

[XhmikosR]

Done, see #63