Skip to content
This repository was archived by the owner on Aug 11, 2022. It is now read-only.

Remove unpublish #12017

Closed
wants to merge 1 commit into from
Closed

Remove unpublish #12017

wants to merge 1 commit into from

Conversation

tlrobinson
Copy link

Resolves future instances of left-pad/left-pad#4

Needs corresponding changes in server etc.

@camsaul
Copy link

camsaul commented Mar 23, 2016

This is a BASIC need for all users.

@othiym23
Copy link
Contributor

We may come up with a more defined policy for handling unpublishes in the future, but the feature itself is an essential part of supporting an ecosystem of packages where anyone can publish, and just as easily anyone can make a mistake. Whether that's including credentials, or finding after the fact an edge case that exposes a security flaw, or publishing incorrectly in the first place, there will always need to be a way for users to remove one or all versions of packages they published. This feature will not be removed from the npm CLI.

@othiym23 othiym23 closed this Mar 23, 2016
@othiym23 othiym23 removed the review label Mar 23, 2016
@camsaul
Copy link

camsaul commented Mar 23, 2016

FWIW a lot of other repos like Clojars, NuGet, CPAN, or Maven don't let you unpublish old packages to prevent exactly this sort of problem. To fix security flaws or something incorrectly published, re-publish a new version.

@mathieumg
Copy link

To fix security flaws or something incorrectly published, just re-publish a new version.

This doesn't address the problem where you accidentally bundled credentials.

I think the solution would be both to allow republishing an unpublished version ONLY if the shasum matches and to enforce publishing a breaking version if ownership changed because a package was removed. (Or even whenever ownership changes following any kind of dispute)

In this case, for instance, anyone who claims ownership of one of azer's removed packages cannot publish a version that would get caught by the caret ^. (Of course, if someone uses *, they're running after trouble already) However, anyone who claims ownership of one of azer's removed packages can republish the same versions as long as the files (and their content) published exactly match those for that version from before.

@othiym23
Copy link
Contributor

@camsaul That still leaves downstream consumers depending on the old, insecure versions of packages exposed to risk. This is a set of situations that are genuinely tricky to deal with, with many edge cases, but it's the judgment of the team at npm that unpublishing is an essential tool to have as a package developer and maintainer.

@salsakran
Copy link

@mathieumg Your example doesn't make much sense. If someone accidentally bundled credentials, then they should consider those credentials exposed and revoke them. Just un-publishing doesn't guarantee that no one saw them.

@soc
Copy link

soc commented Mar 23, 2016

This doesn't address the problem where you accidentally bundled credentials.

If you think that unpublishing would solve this issue, I hope you never work on anything security critical ...

@mathieumg
Copy link

@salsakran Yes, but these actions are orthogonal. Of course you would want to do both immediately. Another reason could be that you published a version that has major negative consequences and want to make sure no one ever installs that again. Bottom line is that there are valid use cases for unpublishing.

@tlrobinson
Copy link
Author

@mathieumg If you accidentally publish credentials you should immediately revoke those credentials. Period. If you publish something you have to assume other people have it, no matter how fast you're able to unpublish it.

@othiym23 Why do package authors get to determine what's a critical security vulnerability in my application? I do think it should be possible to mark versions as deprecated/vulnerable, but it shouldn't break people's builds (failing could be an option you opt into or out of, though).

The only way a package should be unpublished is if it contains illegal or other TOS violating content. This is how Maven and many other package repos work.

@mathieumg
Copy link

@soc Pretty blunt. Of course I didn't mean that.

@tlrobinson Yes, see my answer. I never meant to say that unpublishing would magically fix the credential leak, but it can't hurt removing it from being "out there" while the credentials are being revoked, etc. There are also other cases where it's relevant, as pointed out.

@camsaul
Copy link

camsaul commented Mar 23, 2016

@mathieumg A pretty major consequence is breaking thousands of peoples' builds, right? It seems like with unpublish, the cure is worse than the disease.

@mathieumg
Copy link

@salsakran No, you cannot republish an unpublished version. However, you can publish a new version that would be in same semver range, which makes little sense under different ownerships.

@camsaul Yes, that's a valid point. Perhaps unpublishing should only be allowed within a certain timeframe after publishing, to address any kind of gaffe. If you accidentally published credentials, you should still revoke them immediately.

@devongovett
Copy link

IMHO if you want to unpublish a package you should have to talk to npm support. Deprecation is the common case and that is already supported correctly: with a warning on install.

@peter-dolkens
Copy link

@othiym23 nothing tricky about it.

You know how many time I've had an issue because nuget users unpublished a package? ZERO

I've also been contacted by project/trademark owners for my own packages there, and handed over support willingly. You know why? Because I knew that all the packages out there using existing versions would keep on working.

I'm extremely disappointed to see you addressing such a widely publicized issue like this in the same way that you addressed long-file-names in windows "Not out problem, go away". And go away is exactly what I have done, and is why I, as a Solution Architect, recommend AGAINST using npm in any of our projects.

@dstufft
Copy link

dstufft commented Mar 23, 2016

@camsaul PyPI does not prevent unpublishing.

@othiym23
Copy link
Contributor

I'm extremely disappointed to see you addressing such a widely publicized issue like this in the same way that you addressed long-file-names in windows "Not out problem, go away".

https://github.com/npm/npm/blob/master/CHANGELOG.md#flat-flat-flat

Is a marquee, above-the-line feature in a major new release of the project telling Windows users to go away?

@jacksonrayhamilton
Copy link

I am concerned with other users having the ability to reclaim package names and potentially publish malicious code, that I then download and run, because I depended on the old version of the package.

In this case, for instance, anyone who claims ownership of one of azer's removed packages cannot publish a version that would get caught by the caret ^. (Of course, if someone uses *, they're running after trouble already)

@mathieumg: I don't believe it should ever be possible for us to download code from an owner outside our "web of trust:" That being the original authors of the packages we've installed, and the authors they trusted (for dependencies or as new maintainers).

What do you think of this idea? Bind package names to users even after unpublishing, and require users to manually relinquish names to other users.

@wmira
Copy link

wmira commented Mar 23, 2016

@othiym23 Why not revoke the ability to unpublished after some period of time? e.g. after a month then unpublished isnt possible anymore.

@medicalwei
Copy link

For security concerns, if one needs to revoke the repo, we should revoke the repo forever, not someone who can un-unpublish it.

@peter-dolkens
Copy link

@othiym23 Forgive me - I stopped paying attention after nodejs/node-v0.x-archive#6960 - I guess you finally woke up to yourselves on that. I thought I heard something about it, but after claiming "first class windows support" and then the general attitude of that thread, forgive me if I take anything you guys say with a rather large truckload of salt.

Doesn't change the fact that your current unpublish mechanism is:

  1. A security risk
  2. A dependency risk
  3. A good reason not to rely on anything built on the npm platform

@othiym23
Copy link
Contributor

Why not revoke the ability to unpublished after some period of time?

There's no upper bound on the period during which people can find valid, potentially critical reasons, to take a package down.

I'd like everyone to take a step back and realize that events like today's are very rare. In fact, this is only the third time in the two years I've been working on the npm CLI that a package with a significant number of dependents has been unpublished. The impact was severe, to an unprecedented degree, and also comes freighted with a lot of other important issues, but that doesn't invalidate the usefulness or the importance of having access to unpublishing.

On a certain level, having the ability to take your code down is as important as being able to share it in the first place. That's one of the sources of strength for an open ecosystem like npm's: developers retain control over the things that they create. Also, it's a source of scalability – if every time somebody needed to take a package down they had to contact npm support, it would be a drain on npm's support resources, and would also be ceding even more control and responsibility to npm, Inc. to manage other people's code.

This brings up a related point: those who advocate requiring that unpublish requests go through npm support should recognize that that wouldn't have prevented the problems that resulted from @azer unpublishing their packages. These packages and @azer's npm account were removed at their own request, and I think it would be ethically indefensible for npm, Inc. to put itself in the position of refusing to do so when requested merely because they disagree with a user's reasoning. @azer was warned that their actions would be inconveniencing a large number of users, and they chose to do so anyway. My position is that, as troublesome or difficult as that action turned out to be in practice, it's within their rights as the developer and publisher of that code.

Today required a lot of tough judgment calls. If you follow me on Twitter, you may have seen that I don't agree with some of them. On the whole, though, everybody at npm involved with the decision-making process was trying to balance our responsibilities as stewards of the registry with the need to keep things working for the developers who rely on the registry.

@othiym23
Copy link
Contributor

@dolkensp

I thought I heard something about it, but after claiming "first class windows support" and then the general attitude of that thread, forgive me if I take anything you guys say with a rather large truckload of salt.

My involvement with that issue was to make clear where the division in responsibilities between the npm CLI and the Node.js platform lay. At no point did I, or anyone else connected to the npm project, say or even imply anything to the effect that Windows users should go away, believe that it wasn't our problem to solve, or even be satisfied with the status quo. One of the things on my to-do list for the near future, in fact, is to stand up an AppVeyor instance so that we can get our complete test suite running (and passing) on Windows.

@chadly
Copy link

chadly commented Mar 23, 2016

I really don't understand why this is such an issue. As @camsaul mentioned above, other package managers don't allow "unpublishing". It seems to me this is the basic function of the package manager - ensuring when I request a version of a package, I always get that version.

On a certain level, having the ability to take your code down is as important as being able to share it in the first place.

Once it is published, it is out there. Just removing it from npm doesn't change that. It just breaks everyone's builds. This seems to me to be the same broken logic that would make someone think if they accidentally published security credentials, "unpublishing" is going to save them.

This is a dangerous feature that should be removed.

@peter-dolkens
Copy link

@othiym23

https://www.nuget.org/packages/UmbracoCms.Core-U4-5491/7.1.8.1/Delete

Why can’t I delete my package?
Our policy is to only permanently delete NuGet packages that really need it, such as packages that contain passwords, malicious/harmful code, etc. This policy is very similar to the policies employed by other package managers such as Ruby Gems.

Unlisting the package will prevent users from finding it by searching the gallery and it will prevent the dependency resolver from discovering the package. However, the package will still be available for download through NuGet Package Restore so that existing projects referencing the package don't become broken.

If you need the package permanently removed, click on the Contact Support link and we'll take care of it for you. PLEASE ONLY DO THIS IF THERE IS AN URGENT PROBLEM WITH THE PACKAGE. (Passwords, malicious code, etc). Even if you remove it, it's prudent to immediately reset any passwords/sensitive data you accidentally pushed instead of waiting for us to delete the package.

I like their old version better though

Unlisting the package will remove the package from being available in the NuGet. The package is still available for download as a dependency for three main reasons.

Other packages may depend on that package. Those packages might not necessarily be in this gallery. Ensures that folks using NuGet without committing packages (package restore) will not be broken. Helps ensure that important community owned packages are not mass deleted."

Man - that last line - sounds like they had foresight to think about situations like today.

@andyburke
Copy link

There are a lot of benefits to an immutable history in which packages can not be unpublished.

Let's look at the outlined reasons for needing unpublish:

including credentials

The answer to this is not unpublishing, as the credentials have been out into the world and are now compromised. The answer in this case is to update the credentials.

or finding after the fact an edge case that exposes a security flaw

The answer here is publishing a new version of the package.

A good feature addition for npm would be the ability to deprecate a given version with either a warning or an error, eg:

npm deprecate 0.0.3 error "This version contains a severe security flaw. Update to 0.0.4."

A deprecation error could break everyone's build, but it would be explained by the message, and there would be an audit trail of what happened to lead to the build breakage.

or publishing incorrectly in the first place

The answer here is either to immediately publish a 'rollback' as a new version or to just publish what you intended as soon as possible.

The benefits of having an immutable published set of code far outweigh the various arguments that have been raised so far for supporting unpublish. It seems like it would be reasonable for npm's terms of service to include the fact that once published, that code will forever be available as it was published.

@spinlock99
Copy link

Simple solution: create an immutable npmjs mirror.

@medicalwei
Copy link

What if there exists a legal reason that the package must be unpublished (DMCA takedown for example)?

I still think there's still necessity for unpublish, but not so simple by the user.

@peter-dolkens
Copy link

@othiym23 - took 18 months, but I guess you got there in the end. I hope we don't have to wait another 18 months for this issue to be addressed.

I also acknowledge that you seem to be one of the primary contributors to the fix for it, so props to you for that - here's hoping you can sort something out for this one too.

@Jesin
Copy link

Jesin commented Mar 23, 2016

@othiym23

That's one of the sources of strength for an open ecosystem like npm's: developers retain control over the things that they create.

Wasn't this principle violated when the ownership of Kik was changed without the developer's permission?

@camsaul
Copy link

camsaul commented Mar 23, 2016

I really like the approach taken by Rust's Cargo:

cargo yank

Occasions may arise where you publish a version of a crate that actually ends up being broken for one reason or another (syntax error, forgot to include a file, etc.). For situations such as this, Cargo supports a “yank” of a version of a crate.

$ cargo yank --vers 1.0.1
$ cargo yank --vers 1.0.1 --undo

A yank does not delete any code. This feature is not intended for deleting accidentally uploaded secrets, for example. If that happens, you must reset those secrets immediately.

The semantics of a yanked version are that no new dependencies can be created against that version, but all existing dependencies continue to work. One of the major goals of crates.io is to act as a permanent archive of crates that does not change over time, and allowing deletion of a version would go against this goal. Essentially a yank means that all projects with a Cargo.lock will not break, while any future Cargo.lock files generated will not list the yanked version.

@STRML
Copy link
Contributor

STRML commented Mar 23, 2016

I give a big 👍 to making npm packages immutable. In the case of a copyright takedown like with kik, the new owner would simply have to make a new major version and/or deprecate existing versions, but existing versions would continue to work.

A major step forward with this would also be in identifying packages not by name@version, but by hash similar to ied, at least for shrinkwrapped packages. One of my major concerns in this fiasco was that somebody would take over a recently abandoned package (such as auto-complete) and publish a malicious version over it, causing thousands of applications to be backdoored. To that end, I've republished stub modules and deprecated as many of these packages as I could find.

This sort of action shouldn't be necessary, and the only thing that kept this from being a larger disaster was the fast action of the Babel maintainers and NPM Inc. employees. We shouldn't have to rely on that.

@othiym23
Copy link
Contributor

This thread isn't particularly constructive any longer, so because I want to have an evening away from this, I'm going to, at least temporarily, lock this issue. I'm thinking about the points that have been made, and I'm sure that we as a team will consider them going forward, but for now:

  1. npm unpublish is not going to go away.
  2. Speaking only for myself, I believe events like today's will continue to be rare enough to make that a sensible decision. If this becomes more common, we will take another look at it.

Thanks to all for your thoughts and comments.

@npm npm locked and limited conversation to collaborators Mar 23, 2016
@othiym23
Copy link
Contributor

I've given this more consideration since then, and my conclusion is even stronger that removing npm unpublish from the CLI is not the correct resolution to the concerns raised in this discussion. The CLI is used with many more registries than just the registry hosted by npm, Inc. In addition, removing a capability from the CLI doesn't remove the need to remove that capability from the registry, because of the wide spread of versions of the CLI out in the wild.

I understand that many of you are trying to address the underlying policy issues raised over the last few days, and it's unfortunate that there isn't really a good place to discuss those on GitHub (although npm's policies do have their own repository). That said, this repository & issue tracker are for the npm command-line interface, and the npm CLI team (including me, as its lead) is not empowered to define the policy of the npm registry, much less the company's dispute-resolution or retention policies. Therefore, I'm not going to unlock this thread, for the pragmatic reason that even were I to agree with any of the points raised here, I wouldn't be in a position to act on them. Thanks for your understanding.

That said, see #12045, where I will do what I can to respond to some of the issues raised in this discussion.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.