Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] When using non-npm registry, scoped resolved URLs are being rewritten #3530

Closed
1 task done
iarna opened this issue Jul 9, 2021 · 13 comments
Closed
1 task done
Assignees
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 7.x work is associated with a specific npm 7 release

Comments

@iarna
Copy link
Contributor

iarna commented Jul 9, 2021

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

When installing a scoped modules using a private registry that returns resolved URLs that point at the public registry, npm 7 rewrites the resolved url to point at the private registry.

For example:

npm i --registry=http://localhost:8080 --cache=empty  @babel/helper-validator-identifier

results in

npm ERR! code E404
npm ERR! 404 Not Found - GET http://localhost:8080/@babel/helper-validator-identifier/-/helper-validator-identifier-7.14.5.tgz

Despite the fact that:

npm v --registry=http://localhost:8080 --cache=empty @babel/helper-validator-identifier dist.tarball

shows

https://registry.npmjs.org/@babel/helper-validator-identifier/-/helper-validator-identifier-7.14.5.tgz

This happens with and without lockfiles but only happens with scoped modules and an empty cache (as having the content hash in the cache means that network requests are skipped).

Expected Behavior

I would expect npm to not rewrite resolved urls.

If it were to rewrite resolved urls is intended, I would expect it the same for scoped and unscoped packages. (But I would much prefer that it didn't.)

Steps To Reproduce

  1. git clone https://github.com/iarna/repro-resolved-rewrite
  2. cd repro-resolved-rewrite
  3. npm it

Environment

Tested on:
OS: Debian
node: 16.4.2
npm: 7.18.1, 7.19.1

@iarna iarna added Bug thing that needs fixing Needs Triage needs review for next steps Release 7.x work is associated with a specific npm 7 release labels Jul 9, 2021
@wraithgar wraithgar added Priority 1 high priority issue and removed Needs Triage needs review for next steps labels Jul 12, 2021
@wraithgar
Copy link
Member

wraithgar commented Jul 12, 2021

@iarna does the npm versions listed here mean those are the only ones you noticed this in, or just the two that you tried and saw the problem in?

@fishcharlie
Copy link

@wraithgar My issue which was closed as a duplicate of this one, I've noticed in previous versions as well. Forget exactly when it started tho.

@wraithgar
Copy link
Member

Thanks @fishcharlie, I was pretty sure this wouldn't have been introduced recently but it's always good to clarify.

@iarna
Copy link
Contributor Author

iarna commented Jul 12, 2021

Yeah, those are the versions I've tested in. I'm not 100% sure this is a duplicate of fishcharlie's issue though? But I guess I don't know how Verdaccio works well enough to say for certain. I'm suspicious they're different because my report here only happens with scoped modules.

@wraithgar
Copy link
Member

Yeah, those are the versions I've tested in. I'm not 100% sure this is a duplicate of fishcharlie's issue though? But I guess I don't know how Verdaccio works well enough to say for certain. I'm suspicious they're different because my report here only happens with scoped modules.

We have been getting plenty of issues that seem to have verdaccio in the mix somewhere and they're all in this realm of auth not working with configured registries and based on the symptoms they really do feel like they're related. If not we can untangle that as we figure this out.

@wraithgar
Copy link
Member

wraithgar commented Jul 12, 2021

There's something going on with the configured registry and what ends up in the package-lock, what gets fetched, and what happens when you use --registry during installation of a package vs, say, ci. and this issue is the first one opened that really drills down into an actionable bug so that's why I chose to make it point on this. I think it's bigger than the specifics of either issue, but the same root cause. Really appreciate the detail and debugging you did @iarna it helped a lot.

@iarna
Copy link
Contributor Author

iarna commented Jul 15, 2021

what happens when you use --registry during installation of a package vs, say, ci.

I should note that this is actually happening for us in CI (our workaround right now is to downgrade folks to npm6). With the registry is set via npmrc or command line we get the same result.

@darcyclarke darcyclarke added this to the OSS - Sprint 35 milestone Aug 9, 2021
@fritzy
Copy link
Contributor

fritzy commented Aug 17, 2021

This is, unfortunately, expected behavior.

The decision was made in the past to treat the public registry in tarball urls as a placeholder. When another registry is specified, the hostname is replaced. A packument retrieved from the public registry (directly or via proxy) might be used in a network that is prevented from accessing the public registry (CI or locked down corporate network), and so tarball url hosts are rewritten to match the configured registry. The npm view command doesn't match, because it just shows the packument results directly.

The testing proxy is incorrectly capturing scoped tarball routes because they have a slash in the package name.

Adding the route server.get('/:scope/:package/-/:tarball', proxy) fixes the problem.

@fritzy fritzy closed this as completed Aug 17, 2021
@fritzy
Copy link
Contributor

fritzy commented Aug 17, 2021

Maybe we should add an issue to make npm view respect the registry config and rewrite the URL in the same way.

@fishcharlie
Copy link

@fritzy I strongly disagree with this decision. My issue (#3533) was marked as a duplicate of this one. There is no plausible way that someone looks at that behavior, doesn't provide a workaround, and thinks this is an acceptable behavior. It just doesn't make sense.

I totally understand the perspective that it might be retrieved from a network that is prevented from accessing the public registry. However it is just as valid a use case to assume that the other registry is only accessible from the private network and other networks need to be able to download the packages without errors (falling back to the public registry).

Maybe npm can explore having a setting or option to use the public registry instead of rewriting URLs?

I'm not sure exactly how my issue overlaps with this one. But from my perspective, this is an issue and closing it as expected behavior is not the correct answer here, as it completely dismisses & ignores perfectly valid use cases.

What additional details can I provide to appeal (and have npm reconsider) the closure of this issue?

@iarna
Copy link
Contributor Author

iarna commented Aug 26, 2021

I have to agree with @fishcharlie here, the rewriting behavior is just not in alignment with how private registries work. There is no reason to assume that tarballs on private registries are:

  1. served from the same host as the registry
  2. formatted the same way npm formats its urls

If changing registry resulted in querying the metdata of the modules and using the tarball URL found there, I'd have no complaint. But this is rewriting URLs to something NOT in the packument returned by the registry npm is talking to. IMO npm should never consider think it should know better than the packument.

If you're convinced this is working as intended, then I'll go open an RFC so that intent can change (or at least talk through in what world the npm7 only behavior is desirable).

@fishcharlie
Copy link

Following up on this. The fact that both myself and @iarna have given reasons for disputing the closure of this issue, without any response after a month is disappointing.

What additional information can I provide or actions can I take to get a better response here? @fritzy @wraithgar

@juanpicado
Copy link
Contributor

juanpicado commented Oct 11, 2021

But I guess I don't know how Verdaccio works well enough to say for certain. I'm suspicious they're different because my report here only happens with scoped modules.

@iarna just to provide some context regarding this thought. Verdaccio changes on the fly the distribute url based on configuration, so, the pakuments always points to the private registry, either is scoped or not, cached or private package. This is specially useful for users than changes domains or migrate often, thus, the storage dist url does not need to be migrated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 7.x work is associated with a specific npm 7 release
Projects
None yet
Development

No branches or pull requests

6 participants