cargo icon indicating copy to clipboard operation
cargo copied to clipboard

cargo publish should be opt-in

Open jmaargh opened this issue 5 years ago • 24 comments

Publishing, whether to crates.io or elsewhere, should be opt-in rather by on-by-default. Although rare, people do sometimes publish accidentally. Given that the default is currently to publish to crates.io publicly, this is potentially dangerous for using cargo in a corporate closed-source environment. My guess is that even most always-intended-to-be-open-source projects would rather keep things private until they're ready for some sort of release.

~~I see several options for making this work:~~

  1. ~~Have cargo new add publish = false or publish = [] or equivalent in the default Cargo.toml. This is probably the most minor change and I can't imagine this will break any existing uses.~~

  2. ~~Change the behaviour so that publish = true in Cargo.toml is required for cargo publish to work, with an error message explaining this. This might break workflow for some existing projects, but it would be a very easy fix.~~

  3. ~~Change the behaviour of cargo publish so that if there is no publish = True in Cargo.toml it interactively asks whether you're sure. Possibly with a -y/--yes option (a la apt install) to automatically say "yes".~~

~~Personally, I'd prefer both 1 and 2. I'd be happy to make a PR if people like this idea.~~

Originally raised as an idea on #6123, but was decided it belonged as a separate issue.


Proposal as per this comment is to change the default Cargo.toml generated by cargo new to include publish = false with a comment pointing towards documentation about publishing.

A separate issue will be opened to cover breaking changes to behaviour to be targeted at an edition boundary.

jmaargh avatar Oct 07 '18 13:10 jmaargh

My initial inclination is against doing something like this, but I could be convinced.

There are a few barriers already against accidentally publishing:

  1. The cargo publish command has no other purpose, and none of our other major commands are similar, making users less likely to accidentally run cargo publish when they meant to run something else.
  2. The publication will fail if you aren't logged in, making it less likely for you to successfully publish something when you are new to Rust & haven't yet learned what cargo publish is for. That is, the first time you publish you have to actively try to by creating a crates.io account, so we avoid the problem of people running commands without understanding them.
  3. The crate created by cargo new will be rejected by crates.io because it lacks a license and description field. You have to have set both of those up before you can publish.
  4. cargo publish fails if your crate doesn't build or if your VCS repository is dirty.
  5. If you're publishing a subsequent version of a crate, cargo publish will fail if you haven't updated the version number to the new version.

My own experience of publishing is that I almost always run afoul of one of the existing checks when I go to publish - usually I haven't set a license or description field.

Often the code you don't want to publish is a project you intend to publish someday (and you may have already published a previous version of it). Its perfectly possible to set the publish bit and then run cargo publish by mistake later. That is, this only prevents an accidental publish of a completely new crate, and only once all of the other roadblocks mentioned earlier have been hurdled by the user.


More succinctly, the only interface that will prevent people from accidentally publishing is to disallow publishing entirely. I don't think accidental publishing happens often enough right now that there need to be more roadblocks than there already are.

withoutboats avatar Oct 07 '18 14:10 withoutboats

I agree that accidentally publishing is probably a niche issue, but I don't see how yours are arguments against this proposal, so much as they are arguments that in your experience it's unnecessary.

One way to think about it is this: if we're going to have a publish flag, then it should default to the "safe" state of false (you never lose anything -- except a couple of seconds -- by not publishing when you meant to). If the publish flag is so unnecessary, then it should be removed altogether. Right now, there is a flag whose only purpose is to save you from yourself that defaults to the "unsafe" option.

As for the other barriers, I think they should all remain, but none of them are as explicit as "I don't want this published". For example, you might choose to not set a license, or not log in, simply because you don't want to publish. But in a year's time, you're unlikely to remember that. The advantage of the publish flag is that the intentionality is explicit.

jmaargh avatar Oct 07 '18 14:10 jmaargh

cc @rust-lang/cargo, others likely have thoughts on this as well!

alexcrichton avatar Oct 08 '18 16:10 alexcrichton

My initial feeling is -0 (weakly opposed). I don't feel strongly about this, it just feels like a bit of an unnecessary stumbling block in a new project. I can absolutely understand the desire to make sure people don't publish accidentally, however.

joshtriplett avatar Oct 08 '18 16:10 joshtriplett

~We have the same problem, a package we would like to prevent from being accidentally published.~

~However, couldn't we do it like npm does it, providing a private flag instead?~

I think I want to have things publishable by default (less friction, expected behavior for open source projects) but would like to have the option to opt out for commercial ones.

ralfbiedert avatar Jan 21 '19 20:01 ralfbiedert

@ralfbiedert that's the current behavior via the [package] publish = false setting.

sfackler avatar Jan 21 '19 20:01 sfackler

There are actions that are reversible and actions that are irreversible.
cargo publish
is terribly easy to write, run and misunderstand.
Later is not possible to delete a published crate. That is very irreversible ! That is disproportional ! A simple easy security measure should be enought: require the name of the crate to publish:
cargo publish --crate my_crate_name
That is more complicated to write but is proportional to the gravity of the action.

Another idea is to allow 5 minutes staging time to delete the published crate on crates.io.
In that time it should not be visible to others.
And maybe a command `cargo publish --force --crate my_crate_name'
for the people that need it published pronto.

bestia-dev avatar Jan 14 '20 14:01 bestia-dev

Publishing a new crate or a new stable version of a crate is pretty rare.
And it needs a lot of preparation and thinking through.
An unintentional publish can have very grave consequences.
Because of that the command could be a little more engaging for the developer.
For example requiring this arguments: cargo publish --crate my_crate_name --vers 1.0.0. --reg crate-io This protects the developer to not publish unintentionally the wrong crate or the wrong version to the wrong registry.
Having dangerous default values can lead to secrets or private code been published publicly.
In a split second, there are robots that will make a copy of that code.
And in a few minutes, they will exploit a password or a secret token.
These are very bad consequences.
We can easily prevent that from happening.

bestia-dev avatar Jan 15 '20 10:01 bestia-dev

@LucianBuzzo The point that people might accidentally (or lazily) have secrets in code they don't intend to publish is a good point I hadn't thought of.

Personally, I find the current behaviour at odds with the usual approach to safety in Rust. Publishing code (especially to crates.io, which has no easy removal mechanism) is an "unsafe" action, therefore it should require some explicit opt-in on the part of a human.

jmaargh avatar Mar 01 '20 23:03 jmaargh

Another idea is to allow 5 minutes staging time to delete the published crate on crates.io.

That's almost implemented via rustc compilation times -- there's usually plenty of time after typing cargo publish to change your mind :-)

cargo publish --crate my_crate_name

The only time I ran cargo publish by mistake is pressing arrow up and rerunning the command from shell history -- this would not guard against that.

I tend to (weakly) agree with the opinions already expressed that the current behaviour is not too bad -- there are many "irreversible" commands one can run (e.g.rm -rf) -- given that cargo publish doesn't have any other uses, the status quo seem appropriate.

From the 3 solutions proposed by the OP, 1 seems particularly unappealing as the default should be open source, not a corporate environment (writing this as someone who mostly uses Rust in a closed source corporate env).

mcobzarenco avatar Jul 01 '20 14:07 mcobzarenco

Sorry, you didn't mention my last proposal - all args mandatory: cargo publish --crate my_crate_name --vers 1.0.0. --reg crate-io That solves all current safety/security/clumsiness issues:

  1. Cannot type 'cargo publish' by mistake and expect a guide/help for the utility. Instead it goes for the kill.
  2. Cannot inadvertently call 'cargo publish' in the wrong project.
  3. Cannot mistakenly publish the wrong version.
  4. Cannot forget to explicitly choose the crate database

I suppose developers use the bash history for all the frequent commands. Also for this.

bestia-dev avatar Jul 02 '20 06:07 bestia-dev

There are 2 extreme philosofies and a big gray area between:

  1. Most security and less comfort
  2. Most comfort and less security.

Javascript and dynamic languages are very comfortable. You dont need to think about types or lifetimes. But that is also less secure when later refactoring. The compiler will not help you to spot errors. I choose the Rust programming language exactly because it gives me all the errors, warnings and alerts in compile time. I choose to work uncomfortably to achieve more security. So I expect that philosofical standpoint also for other parts of the Rust ecosystem. Here explicitly debating about cargo publish.

bestia-dev avatar Jul 02 '20 07:07 bestia-dev

As an alternative to changing defaults, would it be more palatable to allow users to specify a behavior for new projects in their config.toml hierarchy? This is already an option for name, email, and vcs.

[cargo-new]
# Avoid publishing my hello_world projects
publish = false
email = "[email protected]"

Would it make more sense to allow specifying all defaults for cargo-new's manifest, bypassing the special-casing for name/email/vcs?

[cargo-new.default-manifest]
[cargo-new.default-manifest.package]
edition = 2018
publish = false
license = "MIT"

[cargo-new.default-manifest.badges]
maintenance = { status = "experimental" }

NTmatter avatar Jul 21 '20 08:07 NTmatter

Maybe an approach like standard-version from the Javascript ecosystem is a nice fit here. By default, running the publishing process is a dry-run. You have to set the -w,--write CLI flag to actually do the publishing.

mainrs avatar Jul 29 '20 12:07 mainrs

I think I like option 3 the most. It could show the crate name and what registry index it will be published to, and prompt for the crate name instead of asking Yes/No, to stop the people that won't check the crate name from shooting themselves in the foot.

The prompt could be disabled with a cli opt, in config.toml, and maybe be disabled by default if there's no tty.

0x6273 avatar Aug 08 '20 16:08 0x6273

So, this conversation fizzled out. Having reviewed the above discussion, my argument is that:

Priors:

  1. Publishing (to wherever, but especially to crates.io) is "unsafe" and almost certainly irreversible
  2. Publishing accidentally is rare
  3. Workflow stumbling blocks are to be avoided where possible
  4. Publishing code containing secrets is potentially catastrophic
  5. Publishing to the wrong repo is potentially catastrophic

Thus, we have a risk of something very rare but potentially also very damaging (always difficult to deal with proportionally) and also a risk that "fixing" it may introduce annoyances. My suggestion is therefore to de-risk by making the unsafe option explicitly opt-in in the simplest way possible, but also to use the error message(s) to provide useful feedback about how to publish (thus turning the stumbling block instead into a teaching moment).

Suggestion:

  1. Default the publish flag internally to false (i.e., absence of the flag is interpreted as "false", keep the flag absent in the default configuration)
    • Any existing published project is already likely to have explicitly set publish = true, for those that haven't it's a very simple one-line change
    • Nearly every instance of accidental publishing will be caught by this, because somebody has to have explicitly opted-in at some point
  2. When cargo publish fails, show a loud warning in stdout that teaches the user what's gone wrong and what they were about to do
    • Something like Warning: failed attempting to publish to [insert repository here, highlighted]
    • Followed by a checklist of required steps (like [x] logged in with cargo login, [ ] no licence specified in Cargo.toml, [ ] publish flag not set in Cargo.toml)
    • A link at the end to docs on how to properly publish a crate (e.g. https://doc.rust-lang.org/cargo/commands/cargo-publish.html)

I hope this strikes a good balance between safety and ergonomics (doesn't that sound like Rust :wink: )

jmaargh avatar Nov 02 '20 16:11 jmaargh

I have something to add here that I haven't seen discussed which has almost slipped me up several times.

The cargo release project, which is used by quite a few people, will publish your crate when cutting a new version if publish = false is not set. I use this as part of private repositories that should not be published, but I nonetheless need to maintain mono repo versioning, and adding a new project in that mono repo can inadvertently lead to publishing it accidentally. So far, I have had to be judicious to always add the --dry-run flag, but I have come close on several occasions to publishing a new crate, or at least I feel that I would have had I not looked at the dry run. This has concerned me enough that I did come looking to raise an issue precisely like this one.

vadixidav avatar Feb 25 '21 17:02 vadixidav

Disclaimer: I'm new to publishing crates (I uploaded my first crate yesterday). I am also worried about accidentally publishing something that I don't want to publish (and if I understand it right, then cargo publish even uploads (untracked) backup files or other files that could accidentally happen to be in my local repository). I currently delete my API key on crates.io (to be sure) or have to remove my local credentials to avoid accidental publishing (e.g. due to execution of a wrong command in the shell history). I would definitely like cargo to be more conservative when publishing something permanently. Adding [package] publish = false to the ~/.cargo/config.toml doesn't seem to be respected either. Ideally, I would expect such a command to always interactively ask me. I personally don't see a need to be able to automate publication (but maybe there are cases where it's necessary). Either way, it shouldn't be the default to not even ask.

Update: Publishing untracked files only seems to happen only with Mercurial, but not with Git.

JanBeh avatar Oct 31 '21 11:10 JanBeh

I almost accidentally pushed my crate for my employer to crates.io because I've misunderstood something with the configuration of registry:s

First time publishing a crate should maybe prompt some extra security. Like "Are you sure these files ..... will be uploaded to crates.io/xx/yy" ?!

Thank god I've not configured an access token to crates.io

Goffen avatar Oct 12 '23 08:10 Goffen

Shouldn't have marked this closed by #12786. We've now made package.publish opt-in if you omit package.version but most projects won't omit it unless the person knows that and explicitly does so. Whether that is sufficient for closing this, I'm unsure.

epage avatar Oct 27 '23 17:10 epage

Thanks for re-opening. I agree that #12786 (while a positive change) does not close this issue. Not least because "this is now opt-in if you do this non-default thing" is the definition of opt-out, not opt-in.

Previous commenters have pointed to other barriers to publishing (logging in, licence set, description set, etc.), but I think my main problem with all of them is that they are proxies and I don't think that Cargo should lean on implicit proxies for interaction design that can stop a footgun. I think it should explicitly design against footguns, which defaulting package.publish to false when omitted would do. Users would have to engage in a specific positive action of setting this field.

Again, happy to write a PR for this if the maintainers can be persuaded that it is the right thing to do.

jmaargh avatar Oct 29 '23 15:10 jmaargh

We already established that the command cargo publish can be catastrophic in some special circumstances (containing secrets, wrong repo, wrong version, no possibility to delete it after publish). Let's make a comparison with the rm command. If we type only rm it does not remove all the files from the system. It returns the error rm: missing operand. That is pretty secure. If you know what you are doing you know you have to explicitly say what files to remove. Same here. If you really want to publish, you know how to add the correct registry, project name and version number to the command. These arguments should be mandatory.
Publishing must be carefully considered. It is not a simple impulse. That would be much better then the publish=false opt-out that we have today.

For backward compatibility an explicit publish=true and repository=crates.io in Cargo.toml could mean you don't need additional arguments for the command cargo publish because they are already defined in Cargo.toml.
I think that the list "include" should also be mandatory for publishing. Usually you don't want to publish examples, tests and sample data:

# publish to registry crates.io. Only this files.
publish = true
registry= crates.io
include = [
    "Cargo.toml",
    "LICENSE",
    "README.md",
    "src/*"
]

bestia-dev avatar Oct 30 '23 09:10 bestia-dev

I think that the list "include" should also be mandatory for publishing. Usually you don't want to publish examples, tests and sample data:

Aren't examples and tests technically part of a package? For example, the following works:

$ cargo add bitflags && cargo check -p bitflags --example fmt

I don't know whether omitting those targets would do any harm in practice, but at least that doesn't seem to be no-op.

Anyway, I think that making publish opt-in should suffice for preventing irreversible mistakes and that mandating include field would cause too much churn to the ecosystem while its benefit is negligible for most projects.


I think the option 2. proposed in OP is the most uncontroversial change for starter, as there is a precedent for changing the default manifest, that is, cargo new --bin has become the default instead of cargo new --lib in toolchain v1.25.

I wonder if there is a motivation against this approach: in particular, does https://github.com/rust-lang/cargo/issues/6153#issuecomment-427902074 still stand?

tesaguri avatar Oct 30 '23 11:10 tesaguri

After some discussion on zulip, here's what I propose:

  1. This issue (#6153) simply narrows in scope to adding publish = false to the default Cargo.toml generated by cargo new, together with a short comment linking to documentation about publishing. This is a simple change which is a big step in the right direction, doesn't require an edition boundary as it's not a breaking change, and is unlikely to break many (if any) workflows.
  2. A new issue is created for implementing a breaking change to default behaviour on the next edition boundary. Details drawn from the current discussion and I'll leave the details to the new issue (though I do have a specific permutation of the proposals in mind).

Per the contributing guide, if this issue is marked as accepted, I'll make a PR for (1) and a new issue for (2).

jmaargh avatar Nov 03 '23 17:11 jmaargh