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

Crates can easily be published to crates.io accidentally when using alternative registries #6123

Closed
cswindle opened this issue Oct 3, 2018 · 16 comments · Fixed by #6135
Closed

Comments

@cswindle
Copy link
Contributor

cswindle commented Oct 3, 2018

Currently when cargo new is run it requires that users remember to set the registry that the crate can be published to, if that is not done it can lead to closed-source software being uploaded to crates.io accidentally.

For enterprises to be able to safely use alternative registries I think there should be a mechanism which allows blocking publishing by default.

A couple of options I have thought of are:

  • set “publish = []” when using cargo new for everyone
  • if an alternative registry is configured set “publish = []”
  • set a default registry in config and set publish = []” on cargo new.

The third of these has the advantage that it could also be used in place of having —registry on every command run.

@cswindle
Copy link
Contributor Author

cswindle commented Oct 3, 2018

@sfackler & @withoutboats, as you are both also interested in alternative registries, is this something you have put any thought into, or have a view?

@sfackler
Copy link
Member

sfackler commented Oct 3, 2018

Yeah, it is something I've wondered about. In practice any accidental publishes to crates.io for us at least will fail since we don't set the license field for internal crates, which crates.io requires.

It does seem like a more robust solution would be to be able to set the default publish registry in the manifest though.

@alexcrichton
Copy link
Member

I like the idea of global configuration in .cargo/config, we may want to start out conservatively by specifically configuring cargo new to generate a project with publish = [], but we could eventually expand that to default registries for subcommands too!

@cswindle
Copy link
Contributor Author

cswindle commented Oct 3, 2018

I just realised that the registry was missing in option 3 from publish (due to not escaping it), I presume that is what you were agreeing with.

@alexcrichton
Copy link
Member

Indeed yeah! For now I'm thinking we can start off with something like:

[cargo-new]
default-publish = []

and we could expand from there

@cswindle
Copy link
Contributor Author

cswindle commented Oct 4, 2018

After having a think about this overnight, I think I would prefer to add --registry as an option to new (and init) which adds the registry to publish, then in command_prelude.rs/registry() add a check for the registry, if none was provided, go to the default from config (which can also then pick up from an environment variable for free). This would then mean that all commands which take --registry would work with the default.

I worry that adding default-publish in cargo-new would then introduce multiple commands which perform the same role (assuming that in time a default registry config option is added) and then we will need to document what order they are checked.

@alexcrichton
Copy link
Member

Makes sense to me!

@cswindle
Copy link
Contributor Author

cswindle commented Oct 4, 2018

Any preference on the field name in the config file for the default registry? Happy for it to be something like this?:

[registry]
default = "crates.io"

Or for an alternative registry we could have a new field to specify that it is the default (and only allow one to be set as default).

[registry.alternative]
index = ...
default = true

Personally I think that it would be better with the first option, but happy with either (or would welcome an alternative suggestion).

For when a default is specified and someone wants to use crates.io, does the user just provide --registry crates.io to commands?

@alexcrichton
Copy link
Member

Either way's fine by me, i wouldn't have much of an opinion

@withoutboats
Copy link
Contributor

I've read the discussion but I'm not completely sure what's being proposed. I think what's being proposed is adding a .cargo/config file when projects are created with cargo new which specifies the default registry for this project. That sounds fine, but I'd prefer it only create that file if you pass --registry, and the behavior of cargo new without --registry doesn't change.

@cswindle
Copy link
Contributor Author

cswindle commented Oct 5, 2018

No, it is setting the publish field in Cargo.toml based on if you pass --registry. There is also the option of adding a default registry in cargo/config files (in all the normal scopes) to allow setting where you want the default registry to be (thus not requiring passing --registry all the time if you default is an alternative registry).

@withoutboats
Copy link
Contributor

No, it is setting the publish field in Cargo.toml based on if you pass --registry.

That sounds fine to me also.

@jmaargh
Copy link

jmaargh commented Oct 6, 2018

Perhaps this belongs in a separate issue, but wouldn't it be sensible to have publish = [] or publish = false (or something else that will prevent publishing) by default?

My guess is that most people who intend to publish (on crates.io or elsewhere) will have several things they want to configure before doing so (like setting a license file), so adding another configuration tweak isn't a problem. Meanwhile, it should almost completely remove accidental publishings, like this.

@cswindle
Copy link
Contributor Author

cswindle commented Oct 6, 2018

I agree that defaulting to disabled would make more sense to me, but I think that may be a more controversial change and would be better as a separate issue.

@alexcrichton
Copy link
Member

Yes disabling publishing by default is something that shouldn't happen here, a change like that would want its own dedicated thread.

@jmaargh
Copy link

jmaargh commented Oct 7, 2018

@alexcrichton @cswindle No problem: #6153

bors added a commit that referenced this issue Oct 8, 2018
Add support for a default registry for cargo commands

This adds two things:
  - --registry support for new and init
 - adds default registry configuration to cargo

The main reason for these changes is to reduce the risk of closed-source software ending up accidentally on crates.io.

This fixes #6123.
@bors bors closed this as completed in #6135 Oct 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants