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

Allow specifying a credentials file other than ~/.cargo/credentials #5638

Closed
alexwlchan opened this issue Jun 19, 2018 · 15 comments
Closed

Allow specifying a credentials file other than ~/.cargo/credentials #5638

alexwlchan opened this issue Jun 19, 2018 · 15 comments
Labels
A-registries Area: registries A-registry-authentication Area: registry authentication and authorization (authn authz) C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`

Comments

@alexwlchan
Copy link

alexwlchan commented Jun 19, 2018

As the title suggests.

Use case

In Hypothesis (https://github.com/HypothesisWorks/hypothesis), we’re going to start publishing Rust packages soon. We have a continuous release process that means all our releases are done by Travis CI.

We store our credentials (crates.io, PyPI, Rubygems, …) in an encrypted ZIP file, which is unpacked by Travis at runtime. For Cargo, this means copying our credentials file from the ZIP file into ~/.cargo/credentials – but we ask Travis to cache ~/.cargo!

The Travis docs are very explicit about not putting secrets in the cache:

Cache content is available to any build on the repository, including Pull Requests, so make sure you do not put any sensitive information in the cache.

So it would be nice if we could tell cargo publish to look somewhere outside ~/.cargo for a credentials file.

I wonder if this also opens the door for #4645?

Possible interface

For consistency with --manifest-path, something like:

$ cargo publish --credentials-path secrets/cargo-credentials

Other examples

We use twine to upload our Python packages, which has a --config-file for passing a path to our PyPI credentials:

$ twine upload --help
usage: twine upload [-h] [-r REPOSITORY] [--repository-url REPOSITORY_URL]
                    [-s] [--sign-with SIGN_WITH] [-i IDENTITY] [-u USERNAME]
                    [-p PASSWORD] [-c COMMENT] [--config-file CONFIG_FILE]
                    [--skip-existing] [--cert path] [--client-cert path]
                    dist [dist ...]

optional arguments:
  --config-file CONFIG_FILE
                        The .pypirc config file to use.

(ETA: I found a couple of other tickets about the credentials file, but I couldn’t find this exact suggestion. Happy to close if it’s a duplicate I’ve missed!)

@DRMacIver
Copy link

It is worth noting (for others in the same boat if nothing else) that the current plan is to just make ~/.cargo/credentials a symlink into the place where we store the decrypted file, so that the actual encrypted contents don't ever end up in the cache. The main problem with this is doing it in a way that doesn't stomp on users existing config (fortunately we only ever run this code from within Travis so we don't have to care about it, but this makes the workaround very specific to our use case).

@jeremy-w
Copy link

Throwing in another possible workaround: Maybe CARGO_HOME=where/that/zipfile/got/unpacked/cargo cargo publish would allow just the publish step to go ahead with the secret credentials, while leaving the default $CARGO_HOME directory untouched for other operations. (But that may also just break because other parts of publishing than credential lookup need to poke at stuff in $CARGO_HOME.)

Having a flag to avoid the need for any workaround would be best.

@alexwlchan
Copy link
Author

I’m in the middle of a house move, but a quick grep for credentials makes me think this wouldn’t be a huge change. (Famous last words!)

I’d be willing to have a first attempt at this patch if people think it’s a good idea.

@Eh2406
Copy link
Contributor

Eh2406 commented Jun 26, 2018

Sorry for the slow reply, the decisive people that have final say on these things are on vacation or otherwise unavailable. My impression is that they will be back soon.

FWIIW, I like the suggestion, and stoked that Hypothesis will be using rust.

@matklad
Copy link
Member

matklad commented Jun 26, 2018

Another possible solution is to make cargo read credentials from project_dir/.cargo/credentials and not only from ~/.cargo/credentials. Not sure if this is a good solution, but, if anyone wants to implement it, the relevant code is here:

walk_tree(&self.cwd, |path| {
let mut contents = String::new();
let mut file = File::open(&path)?;
file.read_to_string(&mut contents)
.chain_err(|| format!("failed to read configuration file `{}`", path.display()))?;
let toml = cargo_toml::parse(&contents, path, self).chain_err(|| {
format!("could not parse TOML configuration in `{}`", path.display())
})?;
let value = CV::from_toml(path, toml).chain_err(|| {
format!(
"failed to load TOML configuration from `{}`",
path.display()
)
})?;
cfg.merge(value)
.chain_err(|| format!("failed to merge configuration at `{}`", path.display()))?;
Ok(())
}).chain_err(|| "could not load Cargo configuration")?;
self.load_credentials(&mut cfg)?;

@alexcrichton alexcrichton added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Jun 28, 2018
@Eh2406
Copy link
Contributor

Eh2406 commented Jul 23, 2018

@alexwlchan how is that first pass going? Anything we can do to help?

@alexwlchan
Copy link
Author

@Eh2406 I haven’t started working on it yet; I read this bit:

the decisive people that have final say on these things are on vacation or otherwise unavailable. My impression is that they will be back soon

and decided I�’d wait until I got the definitive okay, and since it never came, the emails languished in my inbox.

@Eh2406
Copy link
Contributor

Eh2406 commented Jul 24, 2018

I am sorry for that. Let me see if I can get there attenchen. @rust-lang/cargo recommendation on how to resolve this issue, are the proposed solutions exeptibal?

@alexwlchan
Copy link
Author

No worries, no rush, I just didn’t want to sink a bunch of time without knowing if I was doing the right thing. :-)

@joshtriplett
Copy link
Member

We'd definitely like to see an RFC for this, especially because any change here has security implications.

Speaking only for myself, I would roughly suggest that I'd prefer a solution that passes this configuration either by environment variable or by an include directive, rather than by an executable script, unless you absolutely need a script.

@dwijnand
Copy link
Member

Another, Travis CI only fix is to delete the file in before_cache.

We could also propose changing the default behaviour of cache: cargo so that it never stores that file?

@Eh2406
Copy link
Contributor

Eh2406 commented Jul 23, 2019

Btw this RFC may help with this.

@Eh2406
Copy link
Contributor

Eh2406 commented Sep 11, 2019

@ehuss pointed out that you can also use the env CARGO_REGISTRY_TOKEN. This can be done in Travis's ui for encrypted env. Or by having a script that reads it out of your file and sets the env for cargo.

@ehuss ehuss added the A-registries Area: registries label Dec 30, 2019
@eminence
Copy link
Contributor

Triage: The credential file is still hardcoded to be in $CARGO_HOME/credentials, but the CARGO_REGISTRY_TOKEN envvar still works, and now you can also use cargo publish --token to specify the token on the command line. I think both of these solutions would solve the original problem, and would keep credentials out of the travis CI cache

@ehuss ehuss added the A-registry-authentication Area: registry authentication and authorization (authn authz) label Dec 11, 2022
@epage
Copy link
Contributor

epage commented Oct 23, 2023

Btw this RFC may help with this.

This has now been stabilized. Between that and the other options mentioned, I'm going to assume this is resolved. If there is a reason we should re-open this, let us know!

@epage epage closed this as not planned Won't fix, can't repro, duplicate, stale Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-registries Area: registries A-registry-authentication Area: registry authentication and authorization (authn authz) C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`
Projects
None yet
Development

No branches or pull requests