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

RFC: add registry per package per organisation #217

Closed
wants to merge 1 commit into from

Conversation

baloran
Copy link

@baloran baloran commented Sep 2, 2020

tl;dr

We want to install some private package from github registry and some public package from npm registry with the same scope organisation.

Why

We use multiple registry, npm for public package, github for private package. We use multiple mono repos and want to host on github registry for simplicity. But we want to share with the community our public package on npm because everyone have his habits on npm registry.

@darcyclarke darcyclarke added Agenda will be discussed at the Open RFC call semver:major backwards-incompatible breaking changes Release 7.x and removed Release 7.x semver:major backwards-incompatible breaking changes labels Sep 9, 2020
@darcyclarke darcyclarke added semver:minor new backwards-compatible feature and removed Agenda will be discussed at the Open RFC call labels Sep 18, 2020
@ruyadorno
Copy link
Contributor

one concern raised by @wesleytodd in the original RRFC:

One new hazard this introduces: in older versions of npm which do not support this feature might result in users installing code from an unexpected registry and therefore opening the door to a RCE or otherwise getting the wrong code. By back porting the feature or atleast an error to npm@6 will help reduce the issue.

Also, will this support unscoped package names? I think it should.

@wesleytodd
Copy link

wesleytodd commented Oct 14, 2020

An example of the hazard:

  1. A team manages many projects published to an internal registry both scoped @company/* and un-scoped company-*
  2. Today they use npm@6 and some hacks on the registry layer to ensure company-* packages never come from the public registry (maybe like @ljharb described with a regexp match)
  3. [email protected] is released with this feature and one developer reads the release notes (someone always does)
  4. The entire team gets really excited about removing the registry hacks and updates all the projects
  5. The registry hacks are removed 🎉
  6. Oops, one Jenkins deployment isn't updated to [email protected] 💥
    6a. One engineer forgot to update npm
    6b. Someone is testing an older node version using nvm and gets npm@6 by default
    6c. Many many other scenarios where the new config is not respected...

This would result in all the company-* packages being loaded from the public registry. An attacker can (and I know for a fact this approach has worked) publish knowingly under that name and just wait for the install to phone home.

There are many more scenarios depending on configurations (I will not post publicly about ones which I have the most direct knowledge of) where this can happen, some which require even less bad luck to hit.

@isaacs
Copy link
Contributor

isaacs commented Oct 28, 2020

It seems like this entire hazard goes away if we don't support unscoped package names with this feature (or warn if we see those to point out that they're hazardous for npm v6 users).

@ljharb
Copy link
Contributor

ljharb commented Oct 28, 2020

It would still be a hazard if the scope chosen internally matched a public scope, although that’s unlikely.

@isaacs
Copy link
Contributor

isaacs commented Nov 4, 2020

It would still be a hazard if the scope chosen internally matched a public scope, although that’s unlikely.

Well, the use case in the OP here is actually a scope that is both internal (on the github packages registry) and external (on the npm public registry).

But I think maybe what you're suggesting is that it'd be a hazard if the scope chosen internally matched a public scope that you do not control.

Discussion notes

I think the plan here is to move forward with this RFC, but with the provision that it only can apply to scoped packages, in order to mitigate the hazards brought up by @wesleytodd and @ljharb.

Additionally (and what might address this in a better way, albeit it with a lot more implementation cost) we should write an RFC for a registry: dependency specifier #275

@mahnunchik
Copy link

I've faced with the same issue https://gist.github.com/azu/31530916cbce0fd2fc1f4d8f6cf0fae1 😢

isaacs added a commit that referenced this pull request Feb 6, 2021
@isaacs
Copy link
Contributor

isaacs commented Mar 2, 2021

There wasn't a way to get over the security implications that this would raise.

Addressed a different way in #314.

@isaacs isaacs closed this Mar 2, 2021
isaacs added a commit that referenced this pull request Apr 19, 2021
isaacs added a commit that referenced this pull request Apr 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:minor new backwards-compatible feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants