From 8a08cdda6f7b1e739c8c0f6be02588f274c47497 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 1 Sep 2023 11:14:59 +1000 Subject: [PATCH] Fix GitHub token auto discovery (#1335) * Fix GitHub token auto discovery Fixed #1333 - Rm dep `gh-token` since it is broken and we can simply run `gh auth token` in `cargo-binstall` instead. - binstalk-downloader: Make sure GitHub token is at least 40B long and other than the `_`, composes of only alphanumeric characters. - Warn on failure to read `git/credential` files - Optimize `try_from_home` to avoid heap allocation of `PathBuf` Signed-off-by: Jiahao XU * Fix typo and clippy Signed-off-by: Jiahao XU * Simplify `is_valid_gh_token` & `is_ascii_alphanumeric` impl Signed-off-by: Jiahao XU * Improve err msg in `get_inner` Signed-off-by: Jiahao XU * Improve err msg of `cargo_binstall::gh_token::get` Signed-off-by: Jiahao XU --------- Signed-off-by: Jiahao XU --- Cargo.lock | 32 ---------------- crates/bin/Cargo.toml | 1 - crates/bin/src/args.rs | 6 --- crates/bin/src/entry.rs | 13 ++++++- crates/bin/src/gh_token.rs | 38 +++++++++++++++++++ crates/bin/src/git_credentials.rs | 27 +++++++++---- crates/bin/src/lib.rs | 1 + .../binstalk-downloader/src/gh_api_client.rs | 20 +++++++--- 8 files changed, 85 insertions(+), 53 deletions(-) create mode 100644 crates/bin/src/gh_token.rs diff --git a/Cargo.lock b/Cargo.lock index d07dd3b6..b81bf971 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -527,7 +527,6 @@ dependencies = [ "dirs", "embed-resource", "file-format", - "gh-token", "home", "log", "miette", @@ -1169,18 +1168,6 @@ dependencies = [ "wasi", ] -[[package]] -name = "gh-token" -version = "0.1.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c29796559a0962994fcc5994ff97a18e68618508d3f0d8de794475a5045caf09" -dependencies = [ - "home", - "serde", - "serde_derive", - "serde_yaml", -] - [[package]] name = "gimli" version = "0.28.0" @@ -3363,19 +3350,6 @@ dependencies = [ "serde", ] -[[package]] -name = "serde_yaml" -version = "0.9.25" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a49e178e4452f45cb61d0cd8cebc1b0fafd3e41929e996cef79aa3aca91f574" -dependencies = [ - "indexmap 2.0.0", - "itoa", - "ryu", - "serde", - "unsafe-libyaml", -] - [[package]] name = "sha1" version = "0.10.5" @@ -4028,12 +4002,6 @@ version = "0.1.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c0edd1e5b14653f783770bce4a4dabb4a5108a5370a5f5d8cfe8710c361f6c8b" -[[package]] -name = "unsafe-libyaml" -version = "0.2.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f28467d3e1d3c6586d8f25fa243f544f5800fec42d97032474e17222c2b75cfa" - [[package]] name = "untrusted" version = "0.7.1" diff --git a/crates/bin/Cargo.toml b/crates/bin/Cargo.toml index 21fd059e..367fd09b 100644 --- a/crates/bin/Cargo.toml +++ b/crates/bin/Cargo.toml @@ -28,7 +28,6 @@ clap = { version = "4.3.0", features = ["derive", "env"] } compact_str = "0.7.0" dirs = "5.0.1" file-format = { version = "0.19.0", default-features = false } -gh-token = "0.1.2" home = "0.5.5" log = { version = "0.4.18", features = ["std"] } miette = "5.9.0" diff --git a/crates/bin/src/args.rs b/crates/bin/src/args.rs index 31ef8885..b42adaca 100644 --- a/crates/bin/src/args.rs +++ b/crates/bin/src/args.rs @@ -538,12 +538,6 @@ You cannot use --{option} and specify multiple packages at the same time. Do one if opts.github_token.is_none() { if let Ok(github_token) = env::var("GH_TOKEN") { opts.github_token = Some(github_token.into()); - } else if !opts.no_discover_github_token { - if let Some(github_token) = crate::git_credentials::try_from_home() { - opts.github_token = Some(github_token); - } else if let Ok(github_token) = gh_token::get() { - opts.github_token = Some(github_token.into()); - } } } diff --git a/crates/bin/src/entry.rs b/crates/bin/src/entry.rs index 8c265285..d6c7ff3c 100644 --- a/crates/bin/src/entry.rs +++ b/crates/bin/src/entry.rs @@ -33,7 +33,7 @@ use tracing::{debug, error, info, warn}; use crate::{ args::{Args, Strategy}, - install_path, + gh_token, git_credentials, install_path, ui::confirm, }; @@ -107,7 +107,16 @@ pub fn install_crates( ) .map_err(BinstallError::from)?; - let gh_api_client = GhApiClient::new(client.clone(), args.github_token); + let gh_api_client = GhApiClient::new( + client.clone(), + args.github_token.or_else(|| { + if args.no_discover_github_token { + None + } else { + git_credentials::try_from_home().or_else(gh_token::get) + } + }), + ); // Create binstall_opts let binstall_opts = Arc::new(Options { diff --git a/crates/bin/src/gh_token.rs b/crates/bin/src/gh_token.rs new file mode 100644 index 00000000..3cfb3563 --- /dev/null +++ b/crates/bin/src/gh_token.rs @@ -0,0 +1,38 @@ +use std::{io, process}; + +use compact_str::CompactString; +use tracing::warn; + +fn get_inner() -> io::Result { + let process::Output { status, stdout, .. } = process::Command::new("gh") + .args(["auth", "token"]) + .stdin(process::Stdio::null()) + .stdout(process::Stdio::piped()) + .stderr(process::Stdio::null()) + .output()?; + + if !status.success() { + return Err(io::Error::new( + io::ErrorKind::Other, + format!("process exited with `{status}`"), + )); + } + + // Use String here instead of CompactString here since + // `CompactString::from_utf8` allocates if it's longer than 24B. + let s = String::from_utf8(stdout).map_err(|_err| { + io::Error::new(io::ErrorKind::InvalidData, "Invalid output, expected utf8") + })?; + + Ok(s.trim().into()) +} + +pub(super) fn get() -> Option { + match get_inner() { + Ok(token) => Some(token), + Err(err) => { + warn!(?err, "Failed to retrieve token from `gh auth token`"); + None + } + } +} diff --git a/crates/bin/src/git_credentials.rs b/crates/bin/src/git_credentials.rs index 7accd935..613b3732 100644 --- a/crates/bin/src/git_credentials.rs +++ b/crates/bin/src/git_credentials.rs @@ -1,10 +1,8 @@ -use std::{ - env, fs, - path::{Path, PathBuf}, -}; +use std::{env, fs, path::PathBuf}; use compact_str::CompactString; use dirs::home_dir; +use tracing::warn; pub fn try_from_home() -> Option { if let Some(mut home) = home_dir() { @@ -15,7 +13,9 @@ pub fn try_from_home() -> Option { } if let Some(home) = env::var_os("XDG_CONFIG_HOME") { - let home = Path::new(&home).join("git/credentials"); + let mut home = PathBuf::from(home); + home.push("git/credentials"); + if let Some(cred) = from_file(home) { return Some(cred); } @@ -25,8 +25,7 @@ pub fn try_from_home() -> Option { } fn from_file(path: PathBuf) -> Option { - fs::read_to_string(path) - .ok()? + read_cred_file(path)? .lines() .find_map(from_line) .map(CompactString::from) @@ -41,6 +40,20 @@ fn from_line(line: &str) -> Option<&str> { Some(cred.split_once(':')?.1) } +fn read_cred_file(path: PathBuf) -> Option { + match fs::read_to_string(&path) { + Ok(s) => Some(s), + Err(err) => { + warn!( + ?err, + "Failed to read git credential file {}", + path.display() + ); + None + } + } +} + #[cfg(test)] mod test { use super::*; diff --git a/crates/bin/src/lib.rs b/crates/bin/src/lib.rs index 4475363a..1e25d408 100644 --- a/crates/bin/src/lib.rs +++ b/crates/bin/src/lib.rs @@ -3,6 +3,7 @@ mod args; mod bin_util; mod entry; +mod gh_token; mod git_credentials; mod install_path; mod logging; diff --git a/crates/binstalk-downloader/src/gh_api_client.rs b/crates/binstalk-downloader/src/gh_api_client.rs index af96863c..9c8f3f52 100644 --- a/crates/binstalk-downloader/src/gh_api_client.rs +++ b/crates/binstalk-downloader/src/gh_api_client.rs @@ -132,19 +132,29 @@ struct Inner { #[derive(Clone, Debug)] pub struct GhApiClient(Arc); -fn gh_prefixed(token: &str) -> bool { - matches!((token.get(0..2), token.get(3..4)), (Some("gh"), Some("_"))) - || token.starts_with("github_") +fn is_ascii_alphanumeric(s: &[u8]) -> bool { + s.iter().all(|byte| byte.is_ascii_alphanumeric()) +} + +fn is_valid_gh_token(token: &str) -> bool { + let token = token.as_bytes(); + + token.len() >= 40 + && ((&token[0..2] == b"gh" + && token[2].is_ascii_alphanumeric() + && token[3] == b'_' + && is_ascii_alphanumeric(&token[4..])) + || (token.starts_with(b"github_") && is_ascii_alphanumeric(&token[7..]))) } impl GhApiClient { pub fn new(client: remote::Client, auth_token: Option) -> Self { let auth_token = auth_token.and_then(|auth_token| { - if gh_prefixed(&auth_token) { + if is_valid_gh_token(&auth_token) { debug!("Using gh api token"); Some(auth_token) } else { - warn!("Invalid auth_token, expected 'gh*_' or `github_*`, fallback to unauthorized mode"); + warn!("Invalid auth_token, expected 'gh*_' or `github_*` with [A-Za-z0-9], fallback to unauthorized mode"); None } });