Skip to content

fix(credential): trim newlines in tokens from stdin #13770

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Apr 17, 2024
Merged

fix(credential): trim newlines in tokens from stdin #13770

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a498391
show buggy behavior of not trimming newlines in new credential proces…
arlosi Apr 17, 2024
6207f93
fix(credential): trim newlines in token from stdin for credential pro…
arlosi Apr 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/cargo/ops/registry/login.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub fn registry_login(
let mut token_from_stdin = None;
let token = token_from_cmdline.or_else(|| {
if !std::io::stdin().is_terminal() {
let token = std::io::read_to_string(std::io::stdin()).unwrap_or_default();
let token = cargo_credential::read_line().unwrap_or_default();
if !token.is_empty() {
token_from_stdin = Some(token);
}
Expand Down
19 changes: 19 additions & 0 deletions tests/testsuite/credential_process.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -695,3 +695,22 @@ fn alias_builtin_warning() {
)
.run();
}

#[cargo_test]
fn login_token_from_stdin() {
// Test reading a token from stdin, ensuring newlines are trimmed.
let registry = registry::RegistryBuilder::new()
.no_configure_token()
.credential_provider(&[&build_provider("test-cred", r#"{"Ok": {"kind": "login"}}"#)])
.build();

cargo_process("login")
.replace_crates_io(registry.index_url())
.with_stdin("abcdefg\n")
.with_stderr(
r#"[UPDATING] [..]
{"v":1,"registry":{"index-url":"https://github.com/rust-lang/crates.io-index","name":"crates-io"},"kind":"login","token":"abcdefg","login-url":"[..]"}
"#,
)
.run();
}
1 change: 1 addition & 0 deletions tests/testsuite/login.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ fn empty_login_token() {
.with_stderr(
"\
[UPDATING] crates.io index
please paste the token found on [..] below
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this test passing today?

Copy link
Contributor Author

@arlosi arlosi Apr 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the read_token function, if a token is provided by stdin or by the command line, then no prompt is made.

The change causes login_options.token to be None, instead of a Some("\n"), so Cargo now prints the "please paste" message in this case.
 

cargo/credential/cargo-credential/src/lib.rs

Lines 281 to 290 in 852a316

pub fn read_token(
login_options: &LoginOptions<'_>,
registry: &RegistryInfo<'_>,
) -> Result<Secret<String>, Error> {
if let Some(token) = &login_options.token {
return Ok(token.to_owned());
}
if let Some(url) = login_options.login_url {
eprintln!("please paste the token found on {url} below");

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move that change into the commit that caused it so that each commit passes tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. I should have amended that instead of adding a commit.

[ERROR] credential provider `cargo:token` failed action `login`

Caused by:
Expand Down