Skip to content

Commit

Permalink
fix: add error for unsupported credential provider version
Browse files Browse the repository at this point in the history
  • Loading branch information
arlosi committed Aug 29, 2023
1 parent 87a14ed commit 24e4214
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 19 deletions.
8 changes: 5 additions & 3 deletions credential/cargo-credential/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,11 @@ pub enum CacheControl {
Unknown,
}

/// Credential process JSON protocol version. Incrementing
/// this version will prevent new credential providers
/// from working with older versions of Cargo.
/// Credential process JSON protocol version. If the protocol needs to make
/// a breaking change, a new protocol version should be defined (`PROTOCOL_VERSION_2`).
/// This library should offer support for both protocols if possible, by signaling
/// in the `CredentialHello` message. Cargo will then choose which protocol to use,
/// or it will error if there are no common protocol versions available.
pub const PROTOCOL_VERSION_1: u32 = 1;
pub trait Credential {
/// Retrieves a token for the given registry.
Expand Down
67 changes: 51 additions & 16 deletions src/cargo/util/credential/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
use std::{
io::{BufRead, BufReader, Write},
path::PathBuf,
process::{Command, Stdio},
process::{Child, Command, Stdio},
};

use anyhow::Context;
use cargo_credential::{
Action, Credential, CredentialHello, CredentialRequest, CredentialResponse, RegistryInfo,
Action, Credential, CredentialHello, CredentialRequest, CredentialResponse, Error, RegistryInfo,
};

pub struct CredentialProcessCredential {
Expand All @@ -22,31 +22,38 @@ impl<'a> CredentialProcessCredential {
path: PathBuf::from(path),
}
}
}

impl<'a> Credential for CredentialProcessCredential {
fn perform(
fn run(
&self,
registry: &RegistryInfo<'_>,
child: &mut Child,
action: &Action<'_>,
registry: &RegistryInfo<'_>,
args: &[&str],
) -> Result<CredentialResponse, cargo_credential::Error> {
let mut cmd = Command::new(&self.path);
cmd.stdout(Stdio::piped());
cmd.stdin(Stdio::piped());
cmd.arg("--cargo-plugin");
tracing::debug!("credential-process: {cmd:?}");
let mut child = cmd.spawn().context("failed to spawn credential process")?;
) -> Result<Result<CredentialResponse, Error>, Error> {
let mut output_from_child = BufReader::new(child.stdout.take().unwrap());
let mut input_to_child = child.stdin.take().unwrap();
let mut buffer = String::new();

// Read the CredentialHello
output_from_child
.read_line(&mut buffer)
.context("failed to read hello from credential provider")?;
let credential_hello: CredentialHello =
serde_json::from_str(&buffer).context("failed to deserialize hello")?;
tracing::debug!("credential-process > {credential_hello:?}");
if !credential_hello
.v
.contains(&cargo_credential::PROTOCOL_VERSION_1)
{
return Err(format!(
"credential provider supports protocol versions {:?}, while Cargo supports {:?}",
credential_hello.v,
[cargo_credential::PROTOCOL_VERSION_1]
)
.into());
}

// Send the Credential Request
let req = CredentialRequest {
v: cargo_credential::PROTOCOL_VERSION_1,
action: action.clone(),
Expand All @@ -56,14 +63,17 @@ impl<'a> Credential for CredentialProcessCredential {
let request = serde_json::to_string(&req).context("failed to serialize request")?;
tracing::debug!("credential-process < {req:?}");
writeln!(input_to_child, "{request}").context("failed to write to credential provider")?;

buffer.clear();
output_from_child
.read_line(&mut buffer)
.context("failed to read response from credential provider")?;
let response: Result<CredentialResponse, cargo_credential::Error> =

// Read the Credential Response
let response: Result<CredentialResponse, Error> =
serde_json::from_str(&buffer).context("failed to deserialize response")?;
tracing::debug!("credential-process > {response:?}");

// Tell the credential process we're done by closing stdin. It should exit cleanly.
drop(input_to_child);
let status = child.wait().context("credential process never started")?;
if !status.success() {
Expand All @@ -75,6 +85,31 @@ impl<'a> Credential for CredentialProcessCredential {
.into());
}
tracing::trace!("credential process exited successfully");
response
Ok(response)
}
}

impl<'a> Credential for CredentialProcessCredential {
fn perform(
&self,
registry: &RegistryInfo<'_>,
action: &Action<'_>,
args: &[&str],
) -> Result<CredentialResponse, Error> {
let mut cmd = Command::new(&self.path);
cmd.stdout(Stdio::piped());
cmd.stdin(Stdio::piped());
cmd.arg("--cargo-plugin");
tracing::debug!("credential-process: {cmd:?}");
let mut child = cmd.spawn().context("failed to spawn credential process")?;
match self.run(&mut child, action, registry, args) {
Err(e) => {
// Since running the credential process itself failed, ensure the
// process is stopped.
let _ = child.kill();
Err(e)
}
Ok(response) => response,
}
}
}
40 changes: 40 additions & 0 deletions tests/testsuite/credential_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -656,3 +656,43 @@ CARGO_REGISTRY_INDEX_URL=Some([..])
)
.run();
}

#[cargo_test]
fn unsupported_version() {
let cred_proj = project()
.at("new-vers")
.file("Cargo.toml", &basic_manifest("new-vers", "1.0.0"))
.file(
"src/main.rs",
&r####"
fn main() {
println!(r#"{{"v":[998, 999]}}"#);
assert_eq!(std::env::args().skip(1).next().unwrap(), "--cargo-plugin");
let mut buffer = String::new();
std::io::stdin().read_line(&mut buffer).unwrap();
panic!("child process should have been killed before getting here");
} "####,
)
.build();
cred_proj.cargo("build").run();
let provider = toml_bin(&cred_proj, "new-vers");

let registry = registry::RegistryBuilder::new()
.no_configure_token()
.credential_provider(&[&provider])
.build();

cargo_process("login -Z credential-process abcdefg")
.masquerade_as_nightly_cargo(&["credential-process"])
.replace_crates_io(registry.index_url())
.with_status(101)
.with_stderr(
r#"[UPDATING] [..]
[ERROR] credential provider `[..]new-vers` failed action `login`
Caused by:
credential provider supports protocol versions [998, 999], while Cargo supports [1]
"#,
)
.run();
}

0 comments on commit 24e4214

Please sign in to comment.