From 19c1746cefca9bc76537637ec99634eb4cf66a92 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 22 Aug 2022 21:45:14 +0800 Subject: [PATCH 001/236] Support for -c CLI config overrides in `gix config`. (#450) It's special as it creates its own repository. --- git-repository/src/config/snapshot.rs | 8 +++--- gitoxide-core/src/repository/config.rs | 5 +++- src/plumbing/main.rs | 34 +++++++++++++++----------- 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/git-repository/src/config/snapshot.rs b/git-repository/src/config/snapshot.rs index 0dbbc83d409..0f1358c54d7 100644 --- a/git-repository/src/config/snapshot.rs +++ b/git-repository/src/config/snapshot.rs @@ -83,7 +83,7 @@ impl<'repo> Snapshot<'repo> { /// pub mod apply_cli_overrides { - use crate::bstr::{BString, ByteSlice}; + use crate::bstr::{BStr, BString, ByteSlice}; use crate::config::SnapshotMut; use std::convert::TryFrom; @@ -105,13 +105,13 @@ pub mod apply_cli_overrides { impl SnapshotMut<'_> { /// Apply configuration values of the form `core.abbrev=5` or `remote.origin.url = foo` or `core.bool-implicit-true` /// to the repository configuration, marked with [source CLI][git_config::Source::Cli]. - pub fn apply_cli_overrides( + pub fn apply_cli_overrides<'a>( &mut self, - values: impl IntoIterator>, + values: impl IntoIterator>, ) -> Result<(), Error> { let mut file = git_config::File::new(git_config::file::Metadata::from(git_config::Source::Cli)); for key_value in values { - let key_value = key_value.into(); + let key_value = key_value.as_ref(); let mut tokens = key_value.splitn(2, |b| *b == b'=').map(|v| v.trim()); let key = tokens.next().expect("always one value").as_bstr(); let value = tokens.next(); diff --git a/gitoxide-core/src/repository/config.rs b/gitoxide-core/src/repository/config.rs index a179e2c1db2..8dc4680ee25 100644 --- a/gitoxide-core/src/repository/config.rs +++ b/gitoxide-core/src/repository/config.rs @@ -1,18 +1,21 @@ use anyhow::{bail, Result}; use git_repository as git; +use git_repository::bstr::BString; use crate::OutputFormat; pub fn list( repo: git::Repository, filters: Vec, + overrides: Vec, format: OutputFormat, mut out: impl std::io::Write, ) -> Result<()> { if format != OutputFormat::Human { bail!("Only human output format is supported at the moment"); } - let repo = git::open_opts(repo.git_dir(), repo.open_options().clone().lossy_config(false))?; + let mut repo = git::open_opts(repo.git_dir(), repo.open_options().clone().lossy_config(false))?; + repo.config_snapshot_mut().apply_cli_overrides(overrides.into_iter())?; let config = repo.config_snapshot(); let config = config.plumbing(); if let Some(frontmatter) = config.frontmatter() { diff --git a/src/plumbing/main.rs b/src/plumbing/main.rs index b3d51c10f4c..225469c8479 100644 --- a/src/plumbing/main.rs +++ b/src/plumbing/main.rs @@ -62,20 +62,24 @@ pub fn main() -> Result<()> { Strict, Lenient, } - let repository = move |mode: Mode| -> Result { - let mut mapping: git::sec::trust::Mapping = Default::default(); - let toggle = matches!(mode, Mode::Strict); - mapping.full = mapping.full.strict_config(toggle); - mapping.reduced = mapping.reduced.strict_config(toggle); - let mut repo = git::ThreadSafeRepository::discover_opts(repository, Default::default(), mapping) - .map(git::Repository::from) - .map(|r| r.apply_environment())?; - if !config.is_empty() { - repo.config_snapshot_mut() - .apply_cli_overrides(config) - .context("Unable to parse command-line configuration")?; + + let repository = { + let config = config.clone(); + move |mode: Mode| -> Result { + let mut mapping: git::sec::trust::Mapping = Default::default(); + let toggle = matches!(mode, Mode::Strict); + mapping.full = mapping.full.strict_config(toggle); + mapping.reduced = mapping.reduced.strict_config(toggle); + let mut repo = git::ThreadSafeRepository::discover_opts(repository, Default::default(), mapping) + .map(git::Repository::from) + .map(|r| r.apply_environment())?; + if !config.is_empty() { + repo.config_snapshot_mut() + .apply_cli_overrides(config.iter()) + .context("Unable to parse command-line configuration")?; + } + Ok(repo) } - Ok(repo) }; let progress; @@ -142,7 +146,9 @@ pub fn main() -> Result<()> { progress, progress_keep_open, None, - move |_progress, out, _err| core::repository::config::list(repository(Mode::Lenient)?, filter, format, out), + move |_progress, out, _err| { + core::repository::config::list(repository(Mode::Lenient)?, filter, config, format, out) + }, ) .map(|_| ()), Subcommands::Free(subcommands) => match subcommands { From 96a265cc67ea787ed28adde2c5d0a07babf64c9e Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 23 Aug 2022 15:45:02 +0800 Subject: [PATCH 002/236] feat!: generalize extension schemes. (#450) Previously this was hard-coded to `radicle`, now it's just an extension scheme along with a statically known string. This means we have to explicitly support new formats which should be fine. --- git-url/src/lib.rs | 5 ++--- git-url/src/parse.rs | 2 +- git-url/tests/parse/mod.rs | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/git-url/src/lib.rs b/git-url/src/lib.rs index 56a7cf0d2d8..63b9f9c6ca1 100644 --- a/git-url/src/lib.rs +++ b/git-url/src/lib.rs @@ -36,8 +36,7 @@ pub enum Scheme { Ssh, Http, Https, - // TODO: replace this with custom formats, maybe, get an idea how to do that. - Radicle, + Ext(&'static str), } impl Scheme { @@ -50,7 +49,7 @@ impl Scheme { Ssh => "ssh", Http => "http", Https => "https", - Radicle => "rad", + Ext(name) => name, } } } diff --git a/git-url/src/parse.rs b/git-url/src/parse.rs index 5f0454adf23..5ecb3172155 100644 --- a/git-url/src/parse.rs +++ b/git-url/src/parse.rs @@ -35,7 +35,7 @@ fn str_to_protocol(s: &str) -> Result { "git" => Scheme::Git, "http" => Scheme::Http, "https" => Scheme::Https, - "rad" => Scheme::Radicle, + "rad" => Scheme::Ext("rad"), _ => return Err(Error::UnsupportedProtocol { protocol: s.into() }), }) } diff --git a/git-url/tests/parse/mod.rs b/git-url/tests/parse/mod.rs index 47e01de55c0..d785fb49553 100644 --- a/git-url/tests/parse/mod.rs +++ b/git-url/tests/parse/mod.rs @@ -45,7 +45,7 @@ mod radicle { assert_url_roundtrip( "rad://hynkuwzskprmswzeo4qdtku7grdrs4ffj3g9tjdxomgmjzhtzpqf81@hwd1yregyf1dudqwkx85x5ps3qsrqw3ihxpx3ieopq6ukuuq597p6m8161c.git", url( - Scheme::Radicle, + Scheme::Ext("rad"), "hynkuwzskprmswzeo4qdtku7grdrs4ffj3g9tjdxomgmjzhtzpqf81", "hwd1yregyf1dudqwkx85x5ps3qsrqw3ihxpx3ieopq6ukuuq597p6m8161c.git", None, From bb838430ecbaa18921ef60af04ff684809572ce2 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 23 Aug 2022 15:47:00 +0800 Subject: [PATCH 003/236] adjust to changes in `git-url` (#450) --- cargo-smart-release/src/changelog/write.rs | 2 +- git-transport/src/client/blocking_io/connect.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cargo-smart-release/src/changelog/write.rs b/cargo-smart-release/src/changelog/write.rs index be3a014c045..47e5b250347 100644 --- a/cargo-smart-release/src/changelog/write.rs +++ b/cargo-smart-release/src/changelog/write.rs @@ -68,7 +68,7 @@ impl RepositoryUrl { .inner .user() .map(|user| format!("https://github.com{}/{}", user, self.cleaned_path())), - Scheme::Radicle | Scheme::File => None, + Scheme::Ext(_) | Scheme::File => None, }, None | Some(_) => None, } diff --git a/git-transport/src/client/blocking_io/connect.rs b/git-transport/src/client/blocking_io/connect.rs index 13522d77476..0d135ccdd83 100644 --- a/git-transport/src/client/blocking_io/connect.rs +++ b/git-transport/src/client/blocking_io/connect.rs @@ -21,7 +21,7 @@ pub(crate) mod function { { let mut url = url.try_into().map_err(git_url::parse::Error::from)?; Ok(match url.scheme { - git_url::Scheme::Radicle => return Err(Error::UnsupportedScheme(url.scheme)), + git_url::Scheme::Ext(_) => return Err(Error::UnsupportedScheme(url.scheme)), git_url::Scheme::File => { if url.user().is_some() || url.host().is_some() || url.port.is_some() { return Err(Error::UnsupportedUrlTokens { From cd59965f46398adc8f2ac29d2369b0f5f4e0c0a1 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 23 Aug 2022 16:11:48 +0800 Subject: [PATCH 004/236] tests for `protocol.allow` (#450) --- .../tests/fixtures/make_remote_repos.sh | 16 ++++++++ git-repository/tests/remote/connect.rs | 38 +++++++++++++++++++ git-repository/tests/remote/list_refs.rs | 18 +++++---- git-repository/tests/remote/mod.rs | 1 + 4 files changed, 66 insertions(+), 7 deletions(-) create mode 100644 git-repository/tests/remote/connect.rs diff --git a/git-repository/tests/fixtures/make_remote_repos.sh b/git-repository/tests/fixtures/make_remote_repos.sh index 4716a0a4325..97301a681ea 100644 --- a/git-repository/tests/fixtures/make_remote_repos.sh +++ b/git-repository/tests/fixtures/make_remote_repos.sh @@ -171,3 +171,19 @@ EOF git remote get-url origin --push } > baseline.git ) + +git clone --shared base protocol_denied +(cd protocol_denied + git config protocol.allow never +) + +git clone --shared base protocol_file_denied +(cd protocol_file_denied + git config protocol.file.allow never +) + +git clone --shared base protocol_file_user +(cd protocol_file_user + git config protocol.file.allow user +) + diff --git a/git-repository/tests/remote/connect.rs b/git-repository/tests/remote/connect.rs new file mode 100644 index 00000000000..9d8a60ea7a8 --- /dev/null +++ b/git-repository/tests/remote/connect.rs @@ -0,0 +1,38 @@ +#[cfg(feature = "blocking-network-client")] +mod blocking_io { + mod protocol_allow { + use crate::remote; + use git_features::progress; + use git_repository::remote::Direction::Fetch; + use serial_test::serial; + + #[test] + #[ignore] + fn deny() { + for name in ["protocol_denied", "protocol_file_denied"] { + let repo = remote::repo(name); + let remote = repo.find_remote("origin").unwrap(); + assert_eq!( + remote + .connect(Fetch, progress::Discard) + .err() + .map(|err| err.to_string()) + .unwrap(), + "a nice error message or maybe match against it" + ); + } + } + + #[test] + #[ignore] + #[serial] + fn user() { + for (env_value, should_allow) in [(None, true), (Some("0"), false), (Some("1"), true)] { + let _env = env_value.map(|value| git_testtools::Env::new().set("GIT_PROTOCOL_FROM_USER", value)); + let repo = remote::repo("protocol_file_user"); + let remote = repo.find_remote("origin").unwrap(); + assert_eq!(remote.connect(Fetch, progress::Discard).is_ok(), should_allow); + } + } + } +} diff --git a/git-repository/tests/remote/list_refs.rs b/git-repository/tests/remote/list_refs.rs index ccdeb0baa24..9cabdc7c990 100644 --- a/git-repository/tests/remote/list_refs.rs +++ b/git-repository/tests/remote/list_refs.rs @@ -6,7 +6,7 @@ mod blocking_io { use git_repository::remote::Direction::Fetch; #[test] - fn all() { + fn all() -> crate::Result { for version in [ None, Some(git::protocol::transport::Protocol::V2), @@ -14,14 +14,18 @@ mod blocking_io { ] { let mut repo = remote::repo("clone"); if let Some(version) = version { - repo.config_snapshot_mut() - .set_raw_value("protocol", None, "version", (version as u8).to_string().as_str()) - .unwrap(); + repo.config_snapshot_mut().set_raw_value( + "protocol", + None, + "version", + (version as u8).to_string().as_str(), + )?; } - let remote = repo.find_remote("origin").unwrap(); - let connection = remote.connect(Fetch, progress::Discard).unwrap(); - let refs = connection.list_refs().unwrap(); + let remote = repo.find_remote("origin")?; + let connection = remote.connect(Fetch, progress::Discard)?; + let refs = connection.list_refs()?; assert_eq!(refs.len(), 14, "it gets all remote refs, independently of the refspec."); } + Ok(()) } } diff --git a/git-repository/tests/remote/mod.rs b/git-repository/tests/remote/mod.rs index 64aee6a0262..6da4b305330 100644 --- a/git-repository/tests/remote/mod.rs +++ b/git-repository/tests/remote/mod.rs @@ -11,4 +11,5 @@ pub(crate) fn cow_str(s: &str) -> Cow { Cow::Borrowed(s) } +mod connect; mod list_refs; From 3f708f346f021662fa15867b5dbc2d42ccfc6fc8 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 23 Aug 2022 16:41:09 +0800 Subject: [PATCH 005/236] sketch for lazy initialization of information to deal with allowing schemes (#450) --- git-repository/src/config/cache.rs | 6 +++ git-repository/src/config/mod.rs | 2 + git-repository/src/remote/connect.rs | 12 ++++- git-repository/src/remote/url/mod.rs | 4 ++ .../src/remote/{url.rs => url/rewrite.rs} | 2 +- git-repository/src/remote/url/scheme.rs | 54 +++++++++++++++++++ git-repository/tests/remote/list_refs.rs | 1 + git-repository/tests/repository/mod.rs | 2 +- 8 files changed, 79 insertions(+), 4 deletions(-) create mode 100644 git-repository/src/remote/url/mod.rs rename git-repository/src/remote/{url.rs => url/rewrite.rs} (99%) create mode 100644 git-repository/src/remote/url/scheme.rs diff --git a/git-repository/src/config/cache.rs b/git-repository/src/config/cache.rs index b3653557cd3..d9a8ef5185f 100644 --- a/git-repository/src/config/cache.rs +++ b/git-repository/src/config/cache.rs @@ -224,6 +224,7 @@ impl Cache { home_env, personas: Default::default(), url_rewrite: Default::default(), + url_scheme: Default::default(), git_prefix, }) } @@ -271,6 +272,11 @@ impl Cache { self.url_rewrite .get_or_init(|| remote::url::Rewrite::from_config(&self.resolved, self.filter_config_section)) } + + pub(crate) fn url_scheme(&self) -> &remote::url::Scheme { + self.url_scheme + .get_or_init(|| remote::url::Scheme::from_config(&self.resolved, &self.git_prefix)) + } } pub(crate) fn interpolate_context<'a>( diff --git a/git-repository/src/config/mod.rs b/git-repository/src/config/mod.rs index c880b2019ba..ee5c59810f6 100644 --- a/git-repository/src/config/mod.rs +++ b/git-repository/src/config/mod.rs @@ -72,6 +72,8 @@ pub(crate) struct Cache { pub personas: OnceCell, /// A lazily loaded rewrite list for remote urls pub url_rewrite: OnceCell, + /// A lazily loaded mapping to know which url schemes to allow + pub url_scheme: OnceCell, /// The config section filter from the options used to initialize this instance. Keep these in sync! filter_config_section: fn(&git_config::file::Metadata) -> bool, /// The object kind to pick if a prefix is ambiguous. diff --git a/git-repository/src/remote/connect.rs b/git-repository/src/remote/connect.rs index a79fef1bb8f..c2f3a792acf 100644 --- a/git-repository/src/remote/connect.rs +++ b/git-repository/src/remote/connect.rs @@ -10,6 +10,8 @@ mod error { #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { + #[error("Protocol {scheme:?} of url {url:?} is denied per configuration")] + ProtocolDenied { url: BString, scheme: git_url::Scheme }, #[error(transparent)] Connect(#[from] git_protocol::transport::client::connect::Error), #[error("The {} url was missing - don't know where to establish a connection to", direction.as_str())] @@ -69,7 +71,7 @@ impl<'repo> Remote<'repo> { Ok(url) } - let protocol = self + let version = self .repo .config .resolved @@ -89,7 +91,13 @@ impl<'repo> Remote<'repo> { })?; let url = self.url(direction).ok_or(Error::MissingUrl { direction })?.to_owned(); - let transport = git_protocol::transport::connect(sanitize(url)?, protocol).await?; + if !self.repo.config.url_scheme().allow(url.scheme) { + return Err(Error::ProtocolDenied { + url: url.to_bstring(), + scheme: url.scheme, + }); + } + let transport = git_protocol::transport::connect(sanitize(url)?, version).await?; Ok(self.to_connection_with_transport(transport, progress)) } } diff --git a/git-repository/src/remote/url/mod.rs b/git-repository/src/remote/url/mod.rs new file mode 100644 index 00000000000..9dc799009ed --- /dev/null +++ b/git-repository/src/remote/url/mod.rs @@ -0,0 +1,4 @@ +mod rewrite; +mod scheme; +pub(crate) use rewrite::Rewrite; +pub(crate) use scheme::Scheme; diff --git a/git-repository/src/remote/url.rs b/git-repository/src/remote/url/rewrite.rs similarity index 99% rename from git-repository/src/remote/url.rs rename to git-repository/src/remote/url/rewrite.rs index f39ceda803d..bdc9988d097 100644 --- a/git-repository/src/remote/url.rs +++ b/git-repository/src/remote/url/rewrite.rs @@ -3,7 +3,7 @@ use crate::remote::Direction; use git_features::threading::OwnShared; #[derive(Debug, Clone)] -pub(crate) struct Replace { +struct Replace { find: BString, with: OwnShared, } diff --git a/git-repository/src/remote/url/scheme.rs b/git-repository/src/remote/url/scheme.rs new file mode 100644 index 00000000000..178642beb99 --- /dev/null +++ b/git-repository/src/remote/url/scheme.rs @@ -0,0 +1,54 @@ +#![allow(dead_code, unused_variables)] +use crate::permission; +use std::collections::BTreeMap; + +#[derive(Debug, Clone, Copy)] +enum Allow { + Always, + Never, + User, +} + +impl Allow { + pub fn to_bool(&self, user_allowed: Option) -> bool { + match self { + Allow::Always => true, + Allow::Never => false, + Allow::User => user_allowed.unwrap_or(true), + } + } +} + +#[derive(Debug, Clone)] +pub struct Scheme { + /// `None`, env-var is unset or wasn't queried, otherwise true if `GIT_PROTOCOL_FROM_USER` is `1`. + user_allowed: Option, + /// The general allow value from `protocol.allow`. + allow: Option, + /// Per scheme allow information + allow_per_scheme: BTreeMap, +} + +/// Init +impl Scheme { + pub fn from_config(config: &git_config::File<'static>, git_prefix: &permission::env_var::Resource) -> Self { + todo!() + } +} + +/// Access +impl Scheme { + pub fn allow(&self, scheme: git_url::Scheme) -> bool { + self.allow_per_scheme.get(&scheme).or(self.allow.as_ref()).map_or_else( + || { + use git_url::Scheme::*; + match scheme { + File | Git | Ssh | Http | Https => true, + Ext(_) => false, + // TODO: figure out what 'ext' really entails, and what 'other' protocols are which aren't representable for us yet + } + }, + |allow| allow.to_bool(self.user_allowed), + ) + } +} diff --git a/git-repository/tests/remote/list_refs.rs b/git-repository/tests/remote/list_refs.rs index 9cabdc7c990..5041bdc97c6 100644 --- a/git-repository/tests/remote/list_refs.rs +++ b/git-repository/tests/remote/list_refs.rs @@ -6,6 +6,7 @@ mod blocking_io { use git_repository::remote::Direction::Fetch; #[test] + #[ignore] fn all() -> crate::Result { for version in [ None, diff --git a/git-repository/tests/repository/mod.rs b/git-repository/tests/repository/mod.rs index 5c7168febe3..62202bb01f8 100644 --- a/git-repository/tests/repository/mod.rs +++ b/git-repository/tests/repository/mod.rs @@ -10,7 +10,7 @@ mod worktree; #[test] fn size_in_memory() { - let expected = [744, 760]; + let expected = [744, 800]; let actual_size = std::mem::size_of::(); assert!( expected.contains(&actual_size), From 2bce6a3f04be3ea9bcb731d05f7651357810c186 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 23 Aug 2022 16:55:00 +0800 Subject: [PATCH 006/236] support for failing configuration, for now without 'lenient' config support (#450) No leniency because it's hard to decide what to do, allowing connections is probably not it, so better fail with the correct error message. --- git-repository/src/config/cache.rs | 6 +++-- git-repository/src/config/mod.rs | 2 +- git-repository/src/remote/connect.rs | 4 +++- git-repository/src/remote/url/mod.rs | 5 +++-- .../url/{scheme.rs => scheme_permission.rs} | 22 +++++++++++++++---- 5 files changed, 29 insertions(+), 10 deletions(-) rename git-repository/src/remote/url/{scheme.rs => scheme_permission.rs} (69%) diff --git a/git-repository/src/config/cache.rs b/git-repository/src/config/cache.rs index d9a8ef5185f..060fd307749 100644 --- a/git-repository/src/config/cache.rs +++ b/git-repository/src/config/cache.rs @@ -273,9 +273,11 @@ impl Cache { .get_or_init(|| remote::url::Rewrite::from_config(&self.resolved, self.filter_config_section)) } - pub(crate) fn url_scheme(&self) -> &remote::url::Scheme { + pub(crate) fn url_scheme( + &self, + ) -> Result<&remote::url::SchemePermission, remote::url::scheme_permission::init::Error> { self.url_scheme - .get_or_init(|| remote::url::Scheme::from_config(&self.resolved, &self.git_prefix)) + .get_or_try_init(|| remote::url::SchemePermission::from_config(&self.resolved, &self.git_prefix)) } } diff --git a/git-repository/src/config/mod.rs b/git-repository/src/config/mod.rs index ee5c59810f6..1cb489e5d94 100644 --- a/git-repository/src/config/mod.rs +++ b/git-repository/src/config/mod.rs @@ -73,7 +73,7 @@ pub(crate) struct Cache { /// A lazily loaded rewrite list for remote urls pub url_rewrite: OnceCell, /// A lazily loaded mapping to know which url schemes to allow - pub url_scheme: OnceCell, + pub url_scheme: OnceCell, /// The config section filter from the options used to initialize this instance. Keep these in sync! filter_config_section: fn(&git_config::file::Metadata) -> bool, /// The object kind to pick if a prefix is ambiguous. diff --git a/git-repository/src/remote/connect.rs b/git-repository/src/remote/connect.rs index c2f3a792acf..36078f77f2c 100644 --- a/git-repository/src/remote/connect.rs +++ b/git-repository/src/remote/connect.rs @@ -10,6 +10,8 @@ mod error { #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { + #[error(transparent)] + SchemePermission(#[from] remote::url::scheme_permission::init::Error), #[error("Protocol {scheme:?} of url {url:?} is denied per configuration")] ProtocolDenied { url: BString, scheme: git_url::Scheme }, #[error(transparent)] @@ -91,7 +93,7 @@ impl<'repo> Remote<'repo> { })?; let url = self.url(direction).ok_or(Error::MissingUrl { direction })?.to_owned(); - if !self.repo.config.url_scheme().allow(url.scheme) { + if !self.repo.config.url_scheme()?.allow(url.scheme) { return Err(Error::ProtocolDenied { url: url.to_bstring(), scheme: url.scheme, diff --git a/git-repository/src/remote/url/mod.rs b/git-repository/src/remote/url/mod.rs index 9dc799009ed..792fcf4d522 100644 --- a/git-repository/src/remote/url/mod.rs +++ b/git-repository/src/remote/url/mod.rs @@ -1,4 +1,5 @@ mod rewrite; -mod scheme; +/// +pub mod scheme_permission; pub(crate) use rewrite::Rewrite; -pub(crate) use scheme::Scheme; +pub(crate) use scheme_permission::SchemePermission; diff --git a/git-repository/src/remote/url/scheme.rs b/git-repository/src/remote/url/scheme_permission.rs similarity index 69% rename from git-repository/src/remote/url/scheme.rs rename to git-repository/src/remote/url/scheme_permission.rs index 178642beb99..4d02292462c 100644 --- a/git-repository/src/remote/url/scheme.rs +++ b/git-repository/src/remote/url/scheme_permission.rs @@ -2,6 +2,17 @@ use crate::permission; use std::collections::BTreeMap; +/// +pub mod init { + use crate::bstr::BString; + + #[derive(Debug, thiserror::Error)] + pub enum Error { + #[error("{value:?} must be allow|deny|user in configuration key protocol{0}.allow", scheme.as_ref().map(|s| format!(".{}", s)).unwrap_or_default())] + InvalidConfiguration { scheme: Option, value: BString }, + } +} + #[derive(Debug, Clone, Copy)] enum Allow { Always, @@ -20,7 +31,7 @@ impl Allow { } #[derive(Debug, Clone)] -pub struct Scheme { +pub(crate) struct SchemePermission { /// `None`, env-var is unset or wasn't queried, otherwise true if `GIT_PROTOCOL_FROM_USER` is `1`. user_allowed: Option, /// The general allow value from `protocol.allow`. @@ -30,14 +41,17 @@ pub struct Scheme { } /// Init -impl Scheme { - pub fn from_config(config: &git_config::File<'static>, git_prefix: &permission::env_var::Resource) -> Self { +impl SchemePermission { + pub fn from_config( + config: &git_config::File<'static>, + git_prefix: &permission::env_var::Resource, + ) -> Result { todo!() } } /// Access -impl Scheme { +impl SchemePermission { pub fn allow(&self, scheme: git_url::Scheme) -> bool { self.allow_per_scheme.get(&scheme).or(self.allow.as_ref()).map_or_else( || { From d40f6e1f34eb3f4664caec36727bf0aa3a396a33 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 23 Aug 2022 17:22:37 +0800 Subject: [PATCH 007/236] feat: `Scheme::try_from(&str)` (#450) --- git-url/src/lib.rs | 40 +++------------------------------ git-url/src/parse.rs | 12 +++------- git-url/src/scheme.rs | 51 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 46 deletions(-) create mode 100644 git-url/src/scheme.rs diff --git a/git-url/src/lib.rs b/git-url/src/lib.rs index 63b9f9c6ca1..338e67ffe10 100644 --- a/git-url/src/lib.rs +++ b/git-url/src/lib.rs @@ -8,11 +8,8 @@ #![deny(rust_2018_idioms, missing_docs)] #![forbid(unsafe_code)] +use std::convert::TryFrom; use std::path::PathBuf; -use std::{ - convert::TryFrom, - fmt::{self}, -}; use bstr::{BStr, BString}; @@ -26,39 +23,8 @@ pub mod expand_path; #[doc(inline)] pub use expand_path::expand_path; -/// A scheme for use in a [`Url`] -#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)] -#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] -#[allow(missing_docs)] -pub enum Scheme { - File, - Git, - Ssh, - Http, - Https, - Ext(&'static str), -} - -impl Scheme { - /// Return ourselves parseable name. - pub fn as_str(&self) -> &'static str { - use Scheme::*; - match self { - File => "file", - Git => "git", - Ssh => "ssh", - Http => "http", - Https => "https", - Ext(name) => name, - } - } -} - -impl fmt::Display for Scheme { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(self.as_str()) - } -} +mod scheme; +pub use scheme::Scheme; /// A URL with support for specialized git related capabilities. /// diff --git a/git-url/src/parse.rs b/git-url/src/parse.rs index 5ecb3172155..264a98b0ccc 100644 --- a/git-url/src/parse.rs +++ b/git-url/src/parse.rs @@ -1,5 +1,5 @@ use std::borrow::Cow; -use std::convert::Infallible; +use std::convert::{Infallible, TryFrom}; use bstr::{BStr, ByteSlice}; @@ -29,14 +29,8 @@ impl From for Error { } fn str_to_protocol(s: &str) -> Result { - Ok(match s { - "ssh" => Scheme::Ssh, - "file" => Scheme::File, - "git" => Scheme::Git, - "http" => Scheme::Http, - "https" => Scheme::Https, - "rad" => Scheme::Ext("rad"), - _ => return Err(Error::UnsupportedProtocol { protocol: s.into() }), + Scheme::try_from(s).map_err(|invalid| Error::UnsupportedProtocol { + protocol: invalid.into(), }) } diff --git a/git-url/src/scheme.rs b/git-url/src/scheme.rs new file mode 100644 index 00000000000..0f9dfc40e81 --- /dev/null +++ b/git-url/src/scheme.rs @@ -0,0 +1,51 @@ +use std::convert::TryFrom; + +/// A scheme for use in a [`Url`] +#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)] +#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] +#[allow(missing_docs)] +pub enum Scheme { + File, + Git, + Ssh, + Http, + Https, + Ext(&'static str), +} + +impl<'a> TryFrom<&'a str> for Scheme { + type Error = &'a str; + + fn try_from(value: &'a str) -> Result { + Ok(match value { + "ssh" => Scheme::Ssh, + "file" => Scheme::File, + "git" => Scheme::Git, + "http" => Scheme::Http, + "https" => Scheme::Https, + "rad" => Scheme::Ext("rad"), + unknown => return Err(unknown), + }) + } +} + +impl Scheme { + /// Return ourselves parseable name. + pub fn as_str(&self) -> &'static str { + use Scheme::*; + match self { + File => "file", + Git => "git", + Ssh => "ssh", + Http => "http", + Https => "https", + Ext(name) => name, + } + } +} + +impl std::fmt::Display for Scheme { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} From 3ec28f04622f7a2a99a47cde1376ff3088595e4d Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 23 Aug 2022 17:22:50 +0800 Subject: [PATCH 008/236] The first succeeding test for allowing schemes (without 'user' setting) (#450) --- git-repository/src/config/cache.rs | 5 +- .../src/remote/url/scheme_permission.rs | 64 ++++++++++++++++++- git-repository/tests/remote/connect.rs | 19 +++--- git-repository/tests/remote/list_refs.rs | 1 - 4 files changed, 74 insertions(+), 15 deletions(-) diff --git a/git-repository/src/config/cache.rs b/git-repository/src/config/cache.rs index 060fd307749..b85c759a0cc 100644 --- a/git-repository/src/config/cache.rs +++ b/git-repository/src/config/cache.rs @@ -276,8 +276,9 @@ impl Cache { pub(crate) fn url_scheme( &self, ) -> Result<&remote::url::SchemePermission, remote::url::scheme_permission::init::Error> { - self.url_scheme - .get_or_try_init(|| remote::url::SchemePermission::from_config(&self.resolved, &self.git_prefix)) + self.url_scheme.get_or_try_init(|| { + remote::url::SchemePermission::from_config(&self.resolved, &self.git_prefix, self.filter_config_section) + }) } } diff --git a/git-repository/src/remote/url/scheme_permission.rs b/git-repository/src/remote/url/scheme_permission.rs index 4d02292462c..17f100344e4 100644 --- a/git-repository/src/remote/url/scheme_permission.rs +++ b/git-repository/src/remote/url/scheme_permission.rs @@ -1,6 +1,10 @@ #![allow(dead_code, unused_variables)] + +use crate::bstr::{BStr, BString, ByteSlice}; use crate::permission; +use std::borrow::Cow; use std::collections::BTreeMap; +use std::convert::TryFrom; /// pub mod init { @@ -9,7 +13,10 @@ pub mod init { #[derive(Debug, thiserror::Error)] pub enum Error { #[error("{value:?} must be allow|deny|user in configuration key protocol{0}.allow", scheme.as_ref().map(|s| format!(".{}", s)).unwrap_or_default())] - InvalidConfiguration { scheme: Option, value: BString }, + InvalidConfiguration { + scheme: Option<&'static str>, + value: BString, + }, } } @@ -30,6 +37,19 @@ impl Allow { } } +impl<'a> TryFrom> for Allow { + type Error = BString; + + fn try_from(v: Cow<'a, BStr>) -> Result { + Ok(match v.as_ref().as_ref() { + b"never" => Allow::Never, + b"always" => Allow::Always, + b"user" => Allow::User, + unknown => return Err(unknown.into()), + }) + } +} + #[derive(Debug, Clone)] pub(crate) struct SchemePermission { /// `None`, env-var is unset or wasn't queried, otherwise true if `GIT_PROTOCOL_FROM_USER` is `1`. @@ -45,8 +65,48 @@ impl SchemePermission { pub fn from_config( config: &git_config::File<'static>, git_prefix: &permission::env_var::Resource, + mut filter: fn(&git_config::file::Metadata) -> bool, ) -> Result { - todo!() + let allow: Option = config + .string_filter("protocol", None, "allow", &mut filter) + .map(Allow::try_from) + .transpose() + .map_err(|invalid| init::Error::InvalidConfiguration { + value: invalid, + scheme: None, + })?; + let allow_per_scheme = match config.sections_by_name_and_filter("protocol", &mut filter) { + Some(it) => { + let mut map = BTreeMap::default(); + for (section, scheme) in it.filter_map(|section| { + section.header().subsection_name().and_then(|scheme| { + scheme + .to_str() + .ok() + .and_then(|scheme| git_url::Scheme::try_from(scheme).ok().map(|scheme| (section, scheme))) + }) + }) { + if let Some(value) = section + .value("allow") + .map(Allow::try_from) + .transpose() + .map_err(|invalid| init::Error::InvalidConfiguration { + scheme: Some(scheme.as_str()), + value: invalid, + })? + { + map.insert(scheme, value); + } + } + map + } + None => Default::default(), + }; + Ok(SchemePermission { + allow, + allow_per_scheme, + user_allowed: None, + }) } } diff --git a/git-repository/tests/remote/connect.rs b/git-repository/tests/remote/connect.rs index 9d8a60ea7a8..4309d999a3b 100644 --- a/git-repository/tests/remote/connect.rs +++ b/git-repository/tests/remote/connect.rs @@ -3,29 +3,28 @@ mod blocking_io { mod protocol_allow { use crate::remote; use git_features::progress; + use git_repository as git; use git_repository::remote::Direction::Fetch; use serial_test::serial; #[test] - #[ignore] fn deny() { for name in ["protocol_denied", "protocol_file_denied"] { let repo = remote::repo(name); let remote = repo.find_remote("origin").unwrap(); - assert_eq!( - remote - .connect(Fetch, progress::Discard) - .err() - .map(|err| err.to_string()) - .unwrap(), - "a nice error message or maybe match against it" - ); + assert!(matches!( + remote.connect(Fetch, progress::Discard).err(), + Some(git::remote::connect::Error::ProtocolDenied { + url: _, + scheme: git::url::Scheme::File + }) + )); } } #[test] - #[ignore] #[serial] + #[ignore] fn user() { for (env_value, should_allow) in [(None, true), (Some("0"), false), (Some("1"), true)] { let _env = env_value.map(|value| git_testtools::Env::new().set("GIT_PROTOCOL_FROM_USER", value)); diff --git a/git-repository/tests/remote/list_refs.rs b/git-repository/tests/remote/list_refs.rs index 5041bdc97c6..9cabdc7c990 100644 --- a/git-repository/tests/remote/list_refs.rs +++ b/git-repository/tests/remote/list_refs.rs @@ -6,7 +6,6 @@ mod blocking_io { use git_repository::remote::Direction::Fetch; #[test] - #[ignore] fn all() -> crate::Result { for version in [ None, From c5bd45251ae0f47975e9fe77f0b9a9051e319d5c Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 23 Aug 2022 17:23:39 +0800 Subject: [PATCH 009/236] thanks clippy --- git-repository/src/config/cache.rs | 2 ++ git-repository/src/config/mod.rs | 1 + git-repository/src/config/snapshot.rs | 5 +---- git-repository/src/remote/url/mod.rs | 2 ++ git-repository/src/remote/url/scheme_permission.rs | 2 +- 5 files changed, 7 insertions(+), 5 deletions(-) diff --git a/git-repository/src/config/cache.rs b/git-repository/src/config/cache.rs index b85c759a0cc..36b1ec9179c 100644 --- a/git-repository/src/config/cache.rs +++ b/git-repository/src/config/cache.rs @@ -224,6 +224,7 @@ impl Cache { home_env, personas: Default::default(), url_rewrite: Default::default(), + #[cfg(any(feature = "blocking-network-client", feature = "async-network-client-async-std"))] url_scheme: Default::default(), git_prefix, }) @@ -273,6 +274,7 @@ impl Cache { .get_or_init(|| remote::url::Rewrite::from_config(&self.resolved, self.filter_config_section)) } + #[cfg(any(feature = "blocking-network-client", feature = "async-network-client-async-std"))] pub(crate) fn url_scheme( &self, ) -> Result<&remote::url::SchemePermission, remote::url::scheme_permission::init::Error> { diff --git a/git-repository/src/config/mod.rs b/git-repository/src/config/mod.rs index 1cb489e5d94..e653cfa7882 100644 --- a/git-repository/src/config/mod.rs +++ b/git-repository/src/config/mod.rs @@ -73,6 +73,7 @@ pub(crate) struct Cache { /// A lazily loaded rewrite list for remote urls pub url_rewrite: OnceCell, /// A lazily loaded mapping to know which url schemes to allow + #[cfg(any(feature = "blocking-network-client", feature = "async-network-client-async-std"))] pub url_scheme: OnceCell, /// The config section filter from the options used to initialize this instance. Keep these in sync! filter_config_section: fn(&git_config::file::Metadata) -> bool, diff --git a/git-repository/src/config/snapshot.rs b/git-repository/src/config/snapshot.rs index 0f1358c54d7..9d405d0ef72 100644 --- a/git-repository/src/config/snapshot.rs +++ b/git-repository/src/config/snapshot.rs @@ -105,10 +105,7 @@ pub mod apply_cli_overrides { impl SnapshotMut<'_> { /// Apply configuration values of the form `core.abbrev=5` or `remote.origin.url = foo` or `core.bool-implicit-true` /// to the repository configuration, marked with [source CLI][git_config::Source::Cli]. - pub fn apply_cli_overrides<'a>( - &mut self, - values: impl IntoIterator>, - ) -> Result<(), Error> { + pub fn apply_cli_overrides(&mut self, values: impl IntoIterator>) -> Result<(), Error> { let mut file = git_config::File::new(git_config::file::Metadata::from(git_config::Source::Cli)); for key_value in values { let key_value = key_value.as_ref(); diff --git a/git-repository/src/remote/url/mod.rs b/git-repository/src/remote/url/mod.rs index 792fcf4d522..66ec6c66094 100644 --- a/git-repository/src/remote/url/mod.rs +++ b/git-repository/src/remote/url/mod.rs @@ -1,5 +1,7 @@ mod rewrite; /// +#[cfg(any(feature = "blocking-network-client", feature = "async-network-client-async-std"))] pub mod scheme_permission; pub(crate) use rewrite::Rewrite; +#[cfg(any(feature = "blocking-network-client", feature = "async-network-client-async-std"))] pub(crate) use scheme_permission::SchemePermission; diff --git a/git-repository/src/remote/url/scheme_permission.rs b/git-repository/src/remote/url/scheme_permission.rs index 17f100344e4..24f156bab68 100644 --- a/git-repository/src/remote/url/scheme_permission.rs +++ b/git-repository/src/remote/url/scheme_permission.rs @@ -28,7 +28,7 @@ enum Allow { } impl Allow { - pub fn to_bool(&self, user_allowed: Option) -> bool { + pub fn to_bool(self, user_allowed: Option) -> bool { match self { Allow::Always => true, Allow::Never => false, From 2da6c862e184ac37d59147e9cf809017b65db966 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 23 Aug 2022 17:29:05 +0800 Subject: [PATCH 010/236] fix!: make Scheme work with serde, removing `Copy` in the process. e#450) This wasn't supposed to happen but a requirement to get `serde` support back. --- git-url/src/scheme.rs | 10 +++++----- git-url/tests/parse/mod.rs | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/git-url/src/scheme.rs b/git-url/src/scheme.rs index 0f9dfc40e81..9a0fe8278e1 100644 --- a/git-url/src/scheme.rs +++ b/git-url/src/scheme.rs @@ -1,7 +1,7 @@ use std::convert::TryFrom; /// A scheme for use in a [`Url`] -#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)] +#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] #[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] #[allow(missing_docs)] pub enum Scheme { @@ -10,7 +10,7 @@ pub enum Scheme { Ssh, Http, Https, - Ext(&'static str), + Ext(String), } impl<'a> TryFrom<&'a str> for Scheme { @@ -23,7 +23,7 @@ impl<'a> TryFrom<&'a str> for Scheme { "git" => Scheme::Git, "http" => Scheme::Http, "https" => Scheme::Https, - "rad" => Scheme::Ext("rad"), + "rad" => Scheme::Ext("rad".into()), unknown => return Err(unknown), }) } @@ -31,7 +31,7 @@ impl<'a> TryFrom<&'a str> for Scheme { impl Scheme { /// Return ourselves parseable name. - pub fn as_str(&self) -> &'static str { + pub fn as_str(&self) -> &str { use Scheme::*; match self { File => "file", @@ -39,7 +39,7 @@ impl Scheme { Ssh => "ssh", Http => "http", Https => "https", - Ext(name) => name, + Ext(name) => name.as_str(), } } } diff --git a/git-url/tests/parse/mod.rs b/git-url/tests/parse/mod.rs index d785fb49553..5d7db1e80b7 100644 --- a/git-url/tests/parse/mod.rs +++ b/git-url/tests/parse/mod.rs @@ -45,7 +45,7 @@ mod radicle { assert_url_roundtrip( "rad://hynkuwzskprmswzeo4qdtku7grdrs4ffj3g9tjdxomgmjzhtzpqf81@hwd1yregyf1dudqwkx85x5ps3qsrqw3ihxpx3ieopq6ukuuq597p6m8161c.git", url( - Scheme::Ext("rad"), + Scheme::Ext("rad".into()), "hynkuwzskprmswzeo4qdtku7grdrs4ffj3g9tjdxomgmjzhtzpqf81", "hwd1yregyf1dudqwkx85x5ps3qsrqw3ihxpx3ieopq6ukuuq597p6m8161c.git", None, From 6f89659bed6d44de2149c36ce188d816e11f5a64 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 23 Aug 2022 17:31:19 +0800 Subject: [PATCH 011/236] deal with changes in `git-url` (#450) --- git-repository/src/remote/connect.rs | 4 ++-- git-repository/src/remote/url/scheme_permission.rs | 9 +++------ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/git-repository/src/remote/connect.rs b/git-repository/src/remote/connect.rs index 36078f77f2c..1460b0b4619 100644 --- a/git-repository/src/remote/connect.rs +++ b/git-repository/src/remote/connect.rs @@ -93,10 +93,10 @@ impl<'repo> Remote<'repo> { })?; let url = self.url(direction).ok_or(Error::MissingUrl { direction })?.to_owned(); - if !self.repo.config.url_scheme()?.allow(url.scheme) { + if !self.repo.config.url_scheme()?.allow(&url.scheme) { return Err(Error::ProtocolDenied { url: url.to_bstring(), - scheme: url.scheme, + scheme: url.scheme.clone(), }); } let transport = git_protocol::transport::connect(sanitize(url)?, version).await?; diff --git a/git-repository/src/remote/url/scheme_permission.rs b/git-repository/src/remote/url/scheme_permission.rs index 24f156bab68..ca29da7397e 100644 --- a/git-repository/src/remote/url/scheme_permission.rs +++ b/git-repository/src/remote/url/scheme_permission.rs @@ -13,10 +13,7 @@ pub mod init { #[derive(Debug, thiserror::Error)] pub enum Error { #[error("{value:?} must be allow|deny|user in configuration key protocol{0}.allow", scheme.as_ref().map(|s| format!(".{}", s)).unwrap_or_default())] - InvalidConfiguration { - scheme: Option<&'static str>, - value: BString, - }, + InvalidConfiguration { scheme: Option, value: BString }, } } @@ -91,7 +88,7 @@ impl SchemePermission { .map(Allow::try_from) .transpose() .map_err(|invalid| init::Error::InvalidConfiguration { - scheme: Some(scheme.as_str()), + scheme: Some(scheme.as_str().into()), value: invalid, })? { @@ -112,7 +109,7 @@ impl SchemePermission { /// Access impl SchemePermission { - pub fn allow(&self, scheme: git_url::Scheme) -> bool { + pub fn allow(&self, scheme: &git_url::Scheme) -> bool { self.allow_per_scheme.get(&scheme).or(self.allow.as_ref()).map_or_else( || { use git_url::Scheme::*; From dc74fbd9a58e1d424713fc5f2442cedcc09c1200 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 23 Aug 2022 17:31:50 +0800 Subject: [PATCH 012/236] thanks clippy --- git-repository/src/remote/connect.rs | 4 +++- git-repository/src/remote/url/scheme_permission.rs | 2 +- git-repository/tests/repository/mod.rs | 2 +- git-url/src/scheme.rs | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/git-repository/src/remote/connect.rs b/git-repository/src/remote/connect.rs index 1460b0b4619..c5c0481c8cf 100644 --- a/git-repository/src/remote/connect.rs +++ b/git-repository/src/remote/connect.rs @@ -2,6 +2,7 @@ use crate::remote::Connection; use crate::{Progress, Remote}; use git_protocol::transport::client::Transport; +#[cfg(any(feature = "blocking-network-client", feature = "async-network-client-async-std"))] mod error { use crate::bstr::BString; use crate::remote; @@ -24,6 +25,7 @@ mod error { FileUrl(#[from] git_discover::is_git::Error), } } +#[cfg(any(feature = "blocking-network-client", feature = "async-network-client-async-std"))] pub use error::Error; /// Establishing connections to remote hosts @@ -96,7 +98,7 @@ impl<'repo> Remote<'repo> { if !self.repo.config.url_scheme()?.allow(&url.scheme) { return Err(Error::ProtocolDenied { url: url.to_bstring(), - scheme: url.scheme.clone(), + scheme: url.scheme, }); } let transport = git_protocol::transport::connect(sanitize(url)?, version).await?; diff --git a/git-repository/src/remote/url/scheme_permission.rs b/git-repository/src/remote/url/scheme_permission.rs index ca29da7397e..89d939594ff 100644 --- a/git-repository/src/remote/url/scheme_permission.rs +++ b/git-repository/src/remote/url/scheme_permission.rs @@ -110,7 +110,7 @@ impl SchemePermission { /// Access impl SchemePermission { pub fn allow(&self, scheme: &git_url::Scheme) -> bool { - self.allow_per_scheme.get(&scheme).or(self.allow.as_ref()).map_or_else( + self.allow_per_scheme.get(scheme).or(self.allow.as_ref()).map_or_else( || { use git_url::Scheme::*; match scheme { diff --git a/git-repository/tests/repository/mod.rs b/git-repository/tests/repository/mod.rs index 62202bb01f8..06ae361398e 100644 --- a/git-repository/tests/repository/mod.rs +++ b/git-repository/tests/repository/mod.rs @@ -10,7 +10,7 @@ mod worktree; #[test] fn size_in_memory() { - let expected = [744, 800]; + let expected = [760, 800]; let actual_size = std::mem::size_of::(); assert!( expected.contains(&actual_size), diff --git a/git-url/src/scheme.rs b/git-url/src/scheme.rs index 9a0fe8278e1..5e158d23e1e 100644 --- a/git-url/src/scheme.rs +++ b/git-url/src/scheme.rs @@ -1,6 +1,6 @@ use std::convert::TryFrom; -/// A scheme for use in a [`Url`] +/// A scheme for use in a [`Url`][crate::Url]. #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] #[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] #[allow(missing_docs)] From f8a51888d6f55efeb96bba13fdfc23a53781b4ba Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 23 Aug 2022 20:00:22 +0800 Subject: [PATCH 013/236] Respect `permissions.allow` (#450) --- .../src/remote/url/scheme_permission.rs | 13 ++++++++-- git-repository/tests/remote/connect.rs | 24 +++++++++++++++---- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/git-repository/src/remote/url/scheme_permission.rs b/git-repository/src/remote/url/scheme_permission.rs index 89d939594ff..6f9d4c31c89 100644 --- a/git-repository/src/remote/url/scheme_permission.rs +++ b/git-repository/src/remote/url/scheme_permission.rs @@ -17,7 +17,7 @@ pub mod init { } } -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, Eq, PartialEq)] enum Allow { Always, Never, @@ -72,6 +72,8 @@ impl SchemePermission { value: invalid, scheme: None, })?; + + let mut saw_user = allow.map_or(false, |allow| allow == Allow::User); let allow_per_scheme = match config.sections_by_name_and_filter("protocol", &mut filter) { Some(it) => { let mut map = BTreeMap::default(); @@ -92,6 +94,7 @@ impl SchemePermission { value: invalid, })? { + saw_user |= value == Allow::User; map.insert(scheme, value); } } @@ -99,10 +102,16 @@ impl SchemePermission { } None => Default::default(), }; + + let user_allowed = saw_user.then(|| { + std::env::var_os("GIT_PROTOCOL_FROM_USER") + .and_then(|val| git_prefix.check(val).ok().flatten()) + .map_or(true, |val| val == "1") + }); Ok(SchemePermission { allow, allow_per_scheme, - user_allowed: None, + user_allowed, }) } } diff --git a/git-repository/tests/remote/connect.rs b/git-repository/tests/remote/connect.rs index 4309d999a3b..dc4e5d6591e 100644 --- a/git-repository/tests/remote/connect.rs +++ b/git-repository/tests/remote/connect.rs @@ -24,14 +24,28 @@ mod blocking_io { #[test] #[serial] - #[ignore] - fn user() { + fn user() -> crate::Result { for (env_value, should_allow) in [(None, true), (Some("0"), false), (Some("1"), true)] { let _env = env_value.map(|value| git_testtools::Env::new().set("GIT_PROTOCOL_FROM_USER", value)); - let repo = remote::repo("protocol_file_user"); - let remote = repo.find_remote("origin").unwrap(); - assert_eq!(remote.connect(Fetch, progress::Discard).is_ok(), should_allow); + let repo = git::open_opts( + remote::repo("protocol_file_user").git_dir(), + git::open::Options::isolated().permissions(git::Permissions { + env: git::permissions::Environment { + git_prefix: git_sec::Access::resource(git_sec::Permission::Allow), + ..git::permissions::Environment::all() + }, + ..git::Permissions::isolated() + }), + )?; + let remote = repo.find_remote("origin")?; + assert_eq!( + remote.connect(Fetch, progress::Discard).is_ok(), + should_allow, + "Value = {:?}", + env_value + ); } + Ok(()) } } } From 0f18ff511909afe03c5d0d01e60c97bbd5c94ff1 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 07:57:25 +0800 Subject: [PATCH 014/236] update README to reflect new `gix remote refs` command (#450) --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 9f740c4eaf1..fc04989f93d 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,8 @@ Please see _'Development Status'_ for a listing of all crates and their capabili * [x] **explain** - show what would be done while parsing a revision specification like `HEAD~1` * [x] **resolve** - show which objects a revspec resolves to, similar to `git rev-parse` but faster and with much better error handling * [x] **previous-branches** - list all previously checked out branches, powered by the ref-log. + * **remote** + * [x] **refs** - list all references available on the remote based on the current remote configuration. * **free** - no git repository necessary * **pack** * [x] [verify](https://asciinema.org/a/352942) @@ -74,8 +76,6 @@ Please see _'Development Status'_ for a listing of all crates and their capabili * [x] detailed information about the TREE extension * [ ] …other extensions details aren't implemented yet * [x] **checkout-exclusive** - a predecessor of `git worktree`, providing flexible options to evaluate checkout performance from an index and/or an object database. - * **remote** - * [ref-list](https://asciinema.org/a/359320) - list all (or given) references from a remote at the given URL [skim]: https://github.com/lotabout/skim [git-hours]: https://github.com/kimmobrunfeldt/git-hours/blob/8aaeee237cb9d9028e7a2592a25ad8468b1f45e4/index.js#L114-L143 From a395308fdc01b5449a851b1dcb6c3e97a205a5d0 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 10:31:55 +0800 Subject: [PATCH 015/236] refactor (#450) --- Cargo.lock | 1 + git-credentials/Cargo.toml | 3 ++ git-credentials/src/helper.rs | 73 ---------------------------- git-credentials/tests/credentials.rs | 3 ++ git-credentials/tests/helper/mod.rs | 64 ++++++++++++++++++++++++ 5 files changed, 71 insertions(+), 73 deletions(-) create mode 100644 git-credentials/tests/credentials.rs create mode 100644 git-credentials/tests/helper/mod.rs diff --git a/Cargo.lock b/Cargo.lock index eda80b85ad7..d6ab8a7a377 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1168,6 +1168,7 @@ dependencies = [ "bstr", "document-features", "git-sec", + "git-testtools", "quick-error", "serde", ] diff --git a/git-credentials/Cargo.toml b/git-credentials/Cargo.toml index 00e18316b7c..82c3e9bf882 100644 --- a/git-credentials/Cargo.toml +++ b/git-credentials/Cargo.toml @@ -22,6 +22,9 @@ bstr = { version = "0.2.13", default-features = false, features = ["std"]} document-features = { version = "0.2.1", optional = true } +[dev-dependencies] +git-testtools = { path = "../tests/tools"} + [package.metadata.docs.rs] all-features = true features = ["document-features"] diff --git a/git-credentials/src/helper.rs b/git-credentials/src/helper.rs index 6b4cdc0742e..b8f77d4d3bd 100644 --- a/git-credentials/src/helper.rs +++ b/git-credentials/src/helper.rs @@ -167,76 +167,3 @@ pub fn decode_message(mut input: impl io::Read) -> io::Result>>() } - -#[cfg(test)] -mod tests { - use super::*; - type Result = std::result::Result<(), Box>; - - mod encode_message { - use bstr::ByteSlice; - - use super::*; - - #[test] - fn from_url() -> super::Result { - let mut out = Vec::new(); - encode_message("https://github.com/byron/gitoxide".into(), &mut out)?; - assert_eq!(out.as_bstr(), b"url=https://github.com/byron/gitoxide\n\n".as_bstr()); - Ok(()) - } - - mod invalid { - use std::io; - - use super::*; - - #[test] - fn contains_null() { - assert_eq!( - encode_message("https://foo\u{0}".into(), Vec::new()) - .err() - .map(|e| e.kind()), - Some(io::ErrorKind::Other) - ); - } - #[test] - fn contains_newline() { - assert_eq!( - encode_message("https://foo\n".into(), Vec::new()) - .err() - .map(|e| e.kind()), - Some(io::ErrorKind::Other) - ); - } - } - } - - mod decode_message { - use super::*; - - #[test] - fn typical_response() -> super::Result { - assert_eq!( - decode_message( - "protocol=https -host=example.com -username=bob -password=secr3t\n\n -this=is-skipped-past-empty-line" - .as_bytes() - )?, - vec![ - ("protocol", "https"), - ("host", "example.com"), - ("username", "bob"), - ("password", "secr3t") - ] - .iter() - .map(|(k, v)| (k.to_string(), v.to_string())) - .collect::>() - ); - Ok(()) - } - } -} diff --git a/git-credentials/tests/credentials.rs b/git-credentials/tests/credentials.rs new file mode 100644 index 00000000000..94eb82de319 --- /dev/null +++ b/git-credentials/tests/credentials.rs @@ -0,0 +1,3 @@ +pub use git_testtools::Result; + +mod helper; diff --git a/git-credentials/tests/helper/mod.rs b/git-credentials/tests/helper/mod.rs new file mode 100644 index 00000000000..a17bd629f8f --- /dev/null +++ b/git-credentials/tests/helper/mod.rs @@ -0,0 +1,64 @@ +mod encode_message { + use bstr::ByteSlice; + use git_credentials::helper::encode_message; + + #[test] + fn from_url() -> crate::Result { + let mut out = Vec::new(); + encode_message("https://github.com/byron/gitoxide".into(), &mut out)?; + assert_eq!(out.as_bstr(), b"url=https://github.com/byron/gitoxide\n\n".as_bstr()); + Ok(()) + } + + mod invalid { + use git_credentials::helper::encode_message; + use std::io; + + #[test] + fn contains_null() { + assert_eq!( + encode_message("https://foo\u{0}".into(), Vec::new()) + .err() + .map(|e| e.kind()), + Some(io::ErrorKind::Other) + ); + } + #[test] + fn contains_newline() { + assert_eq!( + encode_message("https://foo\n".into(), Vec::new()) + .err() + .map(|e| e.kind()), + Some(io::ErrorKind::Other) + ); + } + } +} + +mod decode_message { + use git_credentials::helper::decode_message; + + #[test] + fn typical_response() -> crate::Result { + assert_eq!( + decode_message( + "protocol=https +host=example.com +username=bob +password=secr3t\n\n +this=is-skipped-past-empty-line" + .as_bytes() + )?, + vec![ + ("protocol", "https"), + ("host", "example.com"), + ("username", "bob"), + ("password", "secr3t") + ] + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect::>() + ); + Ok(()) + } +} From 4c39521a47419bb4b0f0edbe51aa509fb4e2a7f1 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 10:35:21 +0800 Subject: [PATCH 016/236] =?UTF-8?q?rename!:=20`helper::(encode|decode)=5Fm?= =?UTF-8?q?essage(=E2=80=A6)`=20to=20`::message::(encode|decode)(=E2=80=A6?= =?UTF-8?q?)`=20(#450)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- git-credentials/src/helper.rs | 73 ++++++++++--------- git-credentials/tests/helper/mod.rs | 104 ++++++++++++++-------------- 2 files changed, 92 insertions(+), 85 deletions(-) diff --git a/git-credentials/src/helper.rs b/git-credentials/src/helper.rs index b8f77d4d3bd..06d9530013d 100644 --- a/git-credentials/src/helper.rs +++ b/git-credentials/src/helper.rs @@ -97,7 +97,7 @@ pub fn action(action: Action<'_>) -> Result { let mut stdin = child.stdin.take().expect("stdin to be configured"); match action { - Action::Fill(url) => encode_message(url, stdin)?, + Action::Fill(url) => message::encode(url, stdin)?, Action::Approve(last) | Action::Reject(last) => { stdin.write_all(&last)?; stdin.write_all(&[b'\n'])? @@ -112,7 +112,7 @@ pub fn action(action: Action<'_>) -> Result { if stdout.is_empty() { Ok(None) } else { - let kvs = decode_message(stdout.as_slice())?; + let kvs = message::decode(stdout.as_slice())?; let find = |name: &str| { kvs.iter() .find(|(k, _)| k == name) @@ -131,39 +131,44 @@ pub fn action(action: Action<'_>) -> Result { } } -/// Encode `url` to `out` for consumption by a `git credentials` helper program. -pub fn encode_message(url: &BStr, mut out: impl io::Write) -> io::Result<()> { - validate(url)?; - writeln!(out, "url={}\n", url) -} +/// +pub mod message { + use bstr::BStr; -fn validate(url: &BStr) -> io::Result<()> { - if url.contains(&0) || url.contains(&b'\n') { - return Err(io::Error::new( - io::ErrorKind::Other, - "token to encode must not contain newlines or null bytes", - )); + /// Encode `url` to `out` for consumption by a `git credentials` helper program. + pub fn encode(url: &BStr, mut out: impl std::io::Write) -> std::io::Result<()> { + validate(url)?; + writeln!(out, "url={}\n", url) } - Ok(()) -} -/// Decode all lines in `input` as key-value pairs produced by a `git credentials` helper program. -pub fn decode_message(mut input: impl io::Read) -> io::Result> { - let mut buf = String::new(); - input.read_to_string(&mut buf)?; - buf.lines() - .take_while(|l| !l.is_empty()) - .map(|l| { - let mut iter = l.splitn(2, '=').map(|s| s.to_owned()); - match (iter.next(), iter.next()) { - (Some(key), Some(value)) => validate(key.as_str().into()) - .and_then(|_| validate(value.as_str().into())) - .map(|_| (key, value)), - _ => Err(io::Error::new( - io::ErrorKind::Other, - "Invalid format, expecting key=value", - )), - } - }) - .collect::>>() + fn validate(url: &BStr) -> std::io::Result<()> { + if url.contains(&0) || url.contains(&b'\n') { + return Err(std::io::Error::new( + std::io::ErrorKind::Other, + "token to encode must not contain newlines or null bytes", + )); + } + Ok(()) + } + + /// Decode all lines in `input` as key-value pairs produced by a `git credentials` helper program. + pub fn decode(mut input: impl std::io::Read) -> std::io::Result> { + let mut buf = String::new(); + input.read_to_string(&mut buf)?; + buf.lines() + .take_while(|l| !l.is_empty()) + .map(|l| { + let mut iter = l.splitn(2, '=').map(|s| s.to_owned()); + match (iter.next(), iter.next()) { + (Some(key), Some(value)) => validate(key.as_str().into()) + .and_then(|_| validate(value.as_str().into())) + .map(|_| (key, value)), + _ => Err(std::io::Error::new( + std::io::ErrorKind::Other, + "Invalid format, expecting key=value", + )), + } + }) + .collect::>>() + } } diff --git a/git-credentials/tests/helper/mod.rs b/git-credentials/tests/helper/mod.rs index a17bd629f8f..57ce3ff1c19 100644 --- a/git-credentials/tests/helper/mod.rs +++ b/git-credentials/tests/helper/mod.rs @@ -1,64 +1,66 @@ -mod encode_message { - use bstr::ByteSlice; - use git_credentials::helper::encode_message; - - #[test] - fn from_url() -> crate::Result { - let mut out = Vec::new(); - encode_message("https://github.com/byron/gitoxide".into(), &mut out)?; - assert_eq!(out.as_bstr(), b"url=https://github.com/byron/gitoxide\n\n".as_bstr()); - Ok(()) - } - - mod invalid { - use git_credentials::helper::encode_message; - use std::io; +mod message { + mod encode { + use bstr::ByteSlice; + use git_credentials::helper::message; #[test] - fn contains_null() { - assert_eq!( - encode_message("https://foo\u{0}".into(), Vec::new()) - .err() - .map(|e| e.kind()), - Some(io::ErrorKind::Other) - ); + fn from_url() -> crate::Result { + let mut out = Vec::new(); + message::encode("https://github.com/byron/gitoxide".into(), &mut out)?; + assert_eq!(out.as_bstr(), b"url=https://github.com/byron/gitoxide\n\n".as_bstr()); + Ok(()) } - #[test] - fn contains_newline() { - assert_eq!( - encode_message("https://foo\n".into(), Vec::new()) - .err() - .map(|e| e.kind()), - Some(io::ErrorKind::Other) - ); + + mod invalid { + use git_credentials::helper::message; + use std::io; + + #[test] + fn contains_null() { + assert_eq!( + message::encode("https://foo\u{0}".into(), Vec::new()) + .err() + .map(|e| e.kind()), + Some(io::ErrorKind::Other) + ); + } + #[test] + fn contains_newline() { + assert_eq!( + message::encode("https://foo\n".into(), Vec::new()) + .err() + .map(|e| e.kind()), + Some(io::ErrorKind::Other) + ); + } } } -} -mod decode_message { - use git_credentials::helper::decode_message; + mod decode { + use git_credentials::helper::message; - #[test] - fn typical_response() -> crate::Result { - assert_eq!( - decode_message( - "protocol=https + #[test] + fn typical_response() -> crate::Result { + assert_eq!( + message::decode( + "protocol=https host=example.com username=bob password=secr3t\n\n this=is-skipped-past-empty-line" - .as_bytes() - )?, - vec![ - ("protocol", "https"), - ("host", "example.com"), - ("username", "bob"), - ("password", "secr3t") - ] - .iter() - .map(|(k, v)| (k.to_string(), v.to_string())) - .collect::>() - ); - Ok(()) + .as_bytes() + )?, + vec![ + ("protocol", "https"), + ("host", "example.com"), + ("username", "bob"), + ("password", "secr3t") + ] + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect::>() + ); + Ok(()) + } } } From 081934ca4452e550cf2663026905bce67253af81 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 10:38:24 +0800 Subject: [PATCH 017/236] change!: hide `helper::action()` in favor of single path via `helper()` (#450) --- git-credentials/src/helper.rs | 110 +++++++++++++++++----------------- git-credentials/src/lib.rs | 2 +- 2 files changed, 56 insertions(+), 56 deletions(-) diff --git a/git-credentials/src/helper.rs b/git-credentials/src/helper.rs index 06d9530013d..dde0d62f799 100644 --- a/git-credentials/src/helper.rs +++ b/git-credentials/src/helper.rs @@ -1,14 +1,8 @@ use bstr::{BStr, BString}; -use std::{ - io::{self, Write}, - process::{Command, Stdio}, -}; +use std::io; use quick_error::quick_error; -/// The result used in [`action()`]. -pub type Result = std::result::Result, Error>; - quick_error! { /// The error used in the [credentials helper][action()]. #[derive(Debug)] @@ -77,57 +71,63 @@ pub struct Outcome { pub next: NextAction, } -// TODO(sec): reimplement helper execution so it won't use the `git credential` anymore to allow enforcing our own security model. -// Currently we support more flexible configuration than downright not working at all. -/// Call the `git` credentials helper program performing the given `action`. -/// -/// Usually the first call is performed with [`Action::Fill`] to obtain an identity, which subsequently can be used. -/// On successful usage, use [`NextAction::approve()`], otherwise [`NextAction::reject()`]. -pub fn action(action: Action<'_>) -> Result { - let mut cmd = Command::new(cfg!(windows).then(|| "git.exe").unwrap_or("git")); - cmd.arg("credential") - .arg(action.as_str()) - .stdin(Stdio::piped()) - .stdout(if action.is_fill() { - Stdio::piped() - } else { - Stdio::null() - }); - let mut child = cmd.spawn()?; - let mut stdin = child.stdin.take().expect("stdin to be configured"); +pub(crate) mod function { + use crate::helper::{message, Action, Error, NextAction, Outcome}; + use std::io::Write; + use std::process::{Command, Stdio}; - match action { - Action::Fill(url) => message::encode(url, stdin)?, - Action::Approve(last) | Action::Reject(last) => { - stdin.write_all(&last)?; - stdin.write_all(&[b'\n'])? + // TODO(sec): reimplement helper execution so it won't use the `git credential` anymore to allow enforcing our own security model. + // Currently we support more flexible configuration than downright not working at all. + /// Call the `git` credentials helper program performing the given `action`. + /// + /// Usually the first call is performed with [`Action::Fill`] to obtain an identity, which subsequently can be used. + /// On successful usage, use [`NextAction::approve()`], otherwise [`NextAction::reject()`]. + pub fn helper(action: Action<'_>) -> std::result::Result, Error> { + let mut cmd = Command::new(cfg!(windows).then(|| "git.exe").unwrap_or("git")); + cmd.arg("credential") + .arg(action.as_str()) + .stdin(Stdio::piped()) + .stdout(if action.is_fill() { + Stdio::piped() + } else { + Stdio::null() + }); + let mut child = cmd.spawn()?; + let mut stdin = child.stdin.take().expect("stdin to be configured"); + + match action { + Action::Fill(url) => message::encode(url, stdin)?, + Action::Approve(last) | Action::Reject(last) => { + stdin.write_all(&last)?; + stdin.write_all(&[b'\n'])? + } } - } - let output = child.wait_with_output()?; - if !output.status.success() { - return Err(Error::CredentialsHelperFailed(output.status.code())); - } - let stdout = output.stdout; - if stdout.is_empty() { - Ok(None) - } else { - let kvs = message::decode(stdout.as_slice())?; - let find = |name: &str| { - kvs.iter() - .find(|(k, _)| k == name) - .ok_or_else(|| Error::KeyNotFound(name.into())) - .map(|(_, n)| n.to_owned()) - }; - Ok(Some(Outcome { - identity: git_sec::identity::Account { - username: find("username")?, - password: find("password")?, - }, - next: NextAction { - previous_output: stdout.into(), - }, - })) + let output = child.wait_with_output()?; + if !output.status.success() { + return Err(Error::CredentialsHelperFailed(output.status.code())); + } + let stdout = output.stdout; + if stdout.is_empty() { + Ok(None) + } else { + let kvs = message::decode(stdout.as_slice())?; + let find = |name: &str| { + kvs.iter() + .find(|(k, _)| k == name) + .ok_or_else(|| Error::KeyNotFound(name.into())) + .map(|(_, n)| n.to_owned()) + }; + Ok(Some(Outcome { + identity: git_sec::identity::Account { + username: find("username")?, + password: find("password")?, + }, + next: NextAction { + previous_output: stdout.into(), + }, + })) + } } } diff --git a/git-credentials/src/lib.rs b/git-credentials/src/lib.rs index 4e543392a88..cef084007c0 100644 --- a/git-credentials/src/lib.rs +++ b/git-credentials/src/lib.rs @@ -11,4 +11,4 @@ /// pub mod helper; -pub use helper::action as helper; +pub use helper::function::helper; From 4c1a1a28558c4f8d084b8046afd5d87a11fd25b7 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 10:42:20 +0800 Subject: [PATCH 018/236] change!: use `thiserror` instead of `quickerror` (#450) --- Cargo.lock | 2 +- git-credentials/Cargo.toml | 2 +- git-credentials/src/helper.rs | 31 +++++++++++++------------------ 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d6ab8a7a377..5e5848eceaa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1169,8 +1169,8 @@ dependencies = [ "document-features", "git-sec", "git-testtools", - "quick-error", "serde", + "thiserror", ] [[package]] diff --git a/git-credentials/Cargo.toml b/git-credentials/Cargo.toml index 82c3e9bf882..b1cbd58d151 100644 --- a/git-credentials/Cargo.toml +++ b/git-credentials/Cargo.toml @@ -16,7 +16,7 @@ serde1 = ["serde", "bstr/serde1", "git-sec/serde1"] [dependencies] git-sec = { version = "^0.3.1", path = "../git-sec" } -quick-error = "2.0.0" +thiserror = "1.0.32" serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] } bstr = { version = "0.2.13", default-features = false, features = ["std"]} diff --git a/git-credentials/src/helper.rs b/git-credentials/src/helper.rs index dde0d62f799..df834234bc1 100644 --- a/git-credentials/src/helper.rs +++ b/git-credentials/src/helper.rs @@ -1,26 +1,19 @@ use bstr::{BStr, BString}; -use std::io; -use quick_error::quick_error; - -quick_error! { +mod error { /// The error used in the [credentials helper][action()]. - #[derive(Debug)] + #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { - Io(err: io::Error) { - display("An IO error occurred while communicating to the credentials helper") - from() - source(err) - } - KeyNotFound(name: String) { - display("Could not find '{}' in output of git credentials helper", name) - } - CredentialsHelperFailed(code: Option) { - display("Credentials helper program failed with status code {:?}", code) - } + #[error("An IO error occurred while communicating to the credentials helper")] + Io(#[from] std::io::Error), + #[error("Could not find {name:?} in output of credentials helper")] + KeyNotFound { name: String }, + #[error("Credentials helper program failed with status code {code:?}")] + CredentialsHelperFailed { code: Option }, } } +pub use error::Error; /// The action to perform by the credentials [`action()`]. #[derive(Clone, Debug)] @@ -105,7 +98,9 @@ pub(crate) mod function { let output = child.wait_with_output()?; if !output.status.success() { - return Err(Error::CredentialsHelperFailed(output.status.code())); + return Err(Error::CredentialsHelperFailed { + code: output.status.code(), + }); } let stdout = output.stdout; if stdout.is_empty() { @@ -115,7 +110,7 @@ pub(crate) mod function { let find = |name: &str| { kvs.iter() .find(|(k, _)| k == name) - .ok_or_else(|| Error::KeyNotFound(name.into())) + .ok_or_else(|| Error::KeyNotFound { name: name.into() }) .map(|(_, n)| n.to_owned()) }; Ok(Some(Outcome { From de92fce44496b050e5697aab6d6d1ea98a5954dc Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 10:46:47 +0800 Subject: [PATCH 019/236] re-add `Result` type (#450) It's actually used and useful. --- git-credentials/src/helper.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/git-credentials/src/helper.rs b/git-credentials/src/helper.rs index df834234bc1..e654dbfd0e4 100644 --- a/git-credentials/src/helper.rs +++ b/git-credentials/src/helper.rs @@ -14,6 +14,8 @@ mod error { } } pub use error::Error; +/// The Result type used in [`helper()`][crate::helper()]. +pub type Result = std::result::Result, Error>; /// The action to perform by the credentials [`action()`]. #[derive(Clone, Debug)] @@ -65,7 +67,7 @@ pub struct Outcome { } pub(crate) mod function { - use crate::helper::{message, Action, Error, NextAction, Outcome}; + use crate::helper::{message, Action, Error, NextAction, Outcome, Result}; use std::io::Write; use std::process::{Command, Stdio}; @@ -75,7 +77,7 @@ pub(crate) mod function { /// /// Usually the first call is performed with [`Action::Fill`] to obtain an identity, which subsequently can be used. /// On successful usage, use [`NextAction::approve()`], otherwise [`NextAction::reject()`]. - pub fn helper(action: Action<'_>) -> std::result::Result, Error> { + pub fn helper(action: Action<'_>) -> Result { let mut cmd = Command::new(cfg!(windows).then(|| "git.exe").unwrap_or("git")); cmd.arg("credential") .arg(action.as_str()) From db46b60d8f9b4341cf215da6e2cd74bf554fe4b8 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 10:48:57 +0800 Subject: [PATCH 020/236] fix docs (#450) --- git-credentials/src/helper.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/git-credentials/src/helper.rs b/git-credentials/src/helper.rs index e654dbfd0e4..8ba44cc6741 100644 --- a/git-credentials/src/helper.rs +++ b/git-credentials/src/helper.rs @@ -1,7 +1,7 @@ use bstr::{BStr, BString}; mod error { - /// The error used in the [credentials helper][action()]. + /// The error used in the [credentials helper][crate::helper()]. #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { @@ -17,7 +17,7 @@ pub use error::Error; /// The Result type used in [`helper()`][crate::helper()]. pub type Result = std::result::Result, Error>; -/// The action to perform by the credentials [`action()`]. +/// The action to perform by the credentials [helper][`crate::helper()`]. #[derive(Clone, Debug)] pub enum Action<'a> { /// Provide credentials using the given repository URL (as &str) as context. @@ -58,11 +58,11 @@ impl NextAction { } } -/// The outcome of [`action()`]. +/// The outcome of the credentials [`helper`][crate::helper()]. pub struct Outcome { /// The obtained identity. pub identity: git_sec::identity::Account, - /// A handle to the action to perform next using another call to [`action()`]. + /// A handle to the action to perform next in another call to [`helper()`][crate::helper()]. pub next: NextAction, } From 167b5215326ff2f39e89f2130ff575f4ef6c02d6 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 13:46:47 +0800 Subject: [PATCH 021/236] refactor (#450) align usage of stdin and stdout to factor it elsewhere. --- git-credentials/src/helper.rs | 57 +++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/git-credentials/src/helper.rs b/git-credentials/src/helper.rs index 8ba44cc6741..671b11f2831 100644 --- a/git-credentials/src/helper.rs +++ b/git-credentials/src/helper.rs @@ -68,7 +68,7 @@ pub struct Outcome { pub(crate) mod function { use crate::helper::{message, Action, Error, NextAction, Outcome, Result}; - use std::io::Write; + use std::io::{Read, Write}; use std::process::{Command, Stdio}; // TODO(sec): reimplement helper execution so it won't use the `git credential` anymore to allow enforcing our own security model. @@ -89,6 +89,7 @@ pub(crate) mod function { }); let mut child = cmd.spawn()?; let mut stdin = child.stdin.take().expect("stdin to be configured"); + let stdout = child.stdout.take(); match action { Action::Fill(url) => message::encode(url, stdin)?, @@ -97,33 +98,37 @@ pub(crate) mod function { stdin.write_all(&[b'\n'])? } } + let stdout = stdout + .map(|mut stdout| { + let mut buf = Vec::new(); + stdout.read_to_end(&mut buf).map(|_| buf) + }) + .transpose()?; - let output = child.wait_with_output()?; - if !output.status.success() { - return Err(Error::CredentialsHelperFailed { - code: output.status.code(), - }); + let status = child.wait()?; + if !status.success() { + return Err(Error::CredentialsHelperFailed { code: status.code() }); } - let stdout = output.stdout; - if stdout.is_empty() { - Ok(None) - } else { - let kvs = message::decode(stdout.as_slice())?; - let find = |name: &str| { - kvs.iter() - .find(|(k, _)| k == name) - .ok_or_else(|| Error::KeyNotFound { name: name.into() }) - .map(|(_, n)| n.to_owned()) - }; - Ok(Some(Outcome { - identity: git_sec::identity::Account { - username: find("username")?, - password: find("password")?, - }, - next: NextAction { - previous_output: stdout.into(), - }, - })) + match stdout { + None => Ok(None), + Some(stdout) => { + let kvs = message::decode(stdout.as_slice())?; + let find = |name: &str| { + kvs.iter() + .find(|(k, _)| k == name) + .ok_or_else(|| Error::KeyNotFound { name: name.into() }) + .map(|(_, n)| n.to_owned()) + }; + Ok(Some(Outcome { + identity: git_sec::identity::Account { + username: find("username")?, + password: find("password")?, + }, + next: NextAction { + previous_output: stdout.into(), + }, + })) + } } } } From 486ef98b792cc57412a4a90d2cf28586a06d7041 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 14:27:29 +0800 Subject: [PATCH 022/236] prepare for more additional implementations of helpers (#450) --- git-credentials/src/helper.rs | 51 +++++++++++++++++--------- git-credentials/src/lib.rs | 67 +++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 16 deletions(-) diff --git a/git-credentials/src/helper.rs b/git-credentials/src/helper.rs index 671b11f2831..ccf9f512de9 100644 --- a/git-credentials/src/helper.rs +++ b/git-credentials/src/helper.rs @@ -1,5 +1,11 @@ use bstr::{BStr, BString}; +/// The kind of helper program to use. +pub enum Kind { + /// The built-in git-credential helper program, part of any git distribution. + GitCredential, +} + mod error { /// The error used in the [credentials helper][crate::helper()]. #[derive(Debug, thiserror::Error)] @@ -14,6 +20,7 @@ mod error { } } pub use error::Error; + /// The Result type used in [`helper()`][crate::helper()]. pub type Result = std::result::Result, Error>; @@ -29,13 +36,19 @@ pub enum Action<'a> { } impl<'a> Action<'a> { - fn is_fill(&self) -> bool { + /// Returns true if this action expects output from the helper. + pub fn expects_output(&self) -> bool { matches!(self, Action::Fill(_)) } - fn as_str(&self) -> &str { + /// The name of the argument to describe this action. If `is_custom` is true, the target program is + /// a custom credentials helper, not a built-in one. + pub fn as_helper_arg(&self, is_custom: bool) -> &str { match self { - Action::Approve(_) => "approve", + Action::Fill(_) if is_custom => "get", Action::Fill(_) => "fill", + Action::Approve(_) if is_custom => "store", + Action::Approve(_) => "approve", + Action::Reject(_) if is_custom => "erase", Action::Reject(_) => "reject", } } @@ -68,11 +81,22 @@ pub struct Outcome { pub(crate) mod function { use crate::helper::{message, Action, Error, NextAction, Outcome, Result}; - use std::io::{Read, Write}; + use std::io::Read; use std::process::{Command, Stdio}; - // TODO(sec): reimplement helper execution so it won't use the `git credential` anymore to allow enforcing our own security model. - // Currently we support more flexible configuration than downright not working at all. + impl Action<'_> { + /// Send ourselves to the given `write` which is expected to be credentials-helper compatible + pub fn send(&self, mut write: impl std::io::Write) -> std::io::Result<()> { + match self { + Action::Fill(url) => message::encode(url, write), + Action::Approve(last) | Action::Reject(last) => { + write.write_all(&last)?; + write.write_all(&[b'\n']) + } + } + } + } + /// Call the `git` credentials helper program performing the given `action`. /// /// Usually the first call is performed with [`Action::Fill`] to obtain an identity, which subsequently can be used. @@ -80,9 +104,9 @@ pub(crate) mod function { pub fn helper(action: Action<'_>) -> Result { let mut cmd = Command::new(cfg!(windows).then(|| "git.exe").unwrap_or("git")); cmd.arg("credential") - .arg(action.as_str()) + .arg(action.as_helper_arg(false)) .stdin(Stdio::piped()) - .stdout(if action.is_fill() { + .stdout(if action.expects_output() { Stdio::piped() } else { Stdio::null() @@ -91,24 +115,19 @@ pub(crate) mod function { let mut stdin = child.stdin.take().expect("stdin to be configured"); let stdout = child.stdout.take(); - match action { - Action::Fill(url) => message::encode(url, stdin)?, - Action::Approve(last) | Action::Reject(last) => { - stdin.write_all(&last)?; - stdin.write_all(&[b'\n'])? - } - } + action.send(&mut stdin)?; + let stdout = stdout .map(|mut stdout| { let mut buf = Vec::new(); stdout.read_to_end(&mut buf).map(|_| buf) }) .transpose()?; - let status = child.wait()?; if !status.success() { return Err(Error::CredentialsHelperFailed { code: status.code() }); } + match stdout { None => Ok(None), Some(stdout) => { diff --git a/git-credentials/src/lib.rs b/git-credentials/src/lib.rs index cef084007c0..b3d8c7c943e 100644 --- a/git-credentials/src/lib.rs +++ b/git-credentials/src/lib.rs @@ -9,6 +9,73 @@ #![deny(missing_docs, rust_2018_idioms)] #![forbid(unsafe_code)] +/// A utility trait to launch a credentials helper, as well as stop them gracefully. +pub trait Helper { + /// A way to send data to the helper. + type Send: std::io::Write; + /// A way to receive data from the helper. + type Receive: std::io::Read; + + /// Start the helper and provide handles to send and receive from it. + fn start(&mut self, action: &helper::Action<'_>) -> std::io::Result<(Self::Send, Option)>; + /// Stop the helper and provide a way to determine it's successful. + fn finish(self) -> std::io::Result<()>; +} + +/// A program implementing a credentials helper. +pub struct Program { + /// The kind of helper program. + pub kind: helper::Kind, + child: Option, +} + +mod program { + use crate::{helper, Helper, Program}; + use std::process::{Command, Stdio}; + + impl Helper for Program { + type Send = std::process::ChildStdin; + type Receive = std::process::ChildStdout; + + fn start(&mut self, action: &helper::Action<'_>) -> std::io::Result<(Self::Send, Option)> { + assert!(self.child.is_none(), "BUG: cannot call this twice"); + let (mut cmd, is_custom) = match self.kind { + helper::Kind::GitCredential => { + let mut cmd = Command::new(cfg!(windows).then(|| "git.exe").unwrap_or("git")); + cmd.arg("credential"); + (cmd, false) + } + }; + cmd.arg(action.as_helper_arg(is_custom)) + .stdin(Stdio::piped()) + .stdout(if action.expects_output() { + Stdio::piped() + } else { + Stdio::null() + }); + let mut child = cmd.spawn()?; + let stdin = child.stdin.take().expect("stdin to be configured"); + let stdout = child.stdout.take(); + + self.child = Some(child); + Ok((stdin, stdout)) + } + + fn finish(self) -> std::io::Result<()> { + self.child.expect("start() called").wait().and_then(|status| { + if status.success() { + Ok(()) + } else { + Err(std::io::Error::new( + std::io::ErrorKind::Other, + format!("Credentials helper program failed with status code {:?}", status.code()), + )) + } + }) + } + } +} + /// pub mod helper; pub use helper::function::helper; From af27d20909d14f2737fbad5edd9a6c9d86c93e24 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 14:29:40 +0800 Subject: [PATCH 023/236] refactor (#450) --- git-credentials/src/lib.rs | 49 ++-------------------------------- git-credentials/src/program.rs | 44 ++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 47 deletions(-) create mode 100644 git-credentials/src/program.rs diff --git a/git-credentials/src/lib.rs b/git-credentials/src/lib.rs index b3d8c7c943e..60064996065 100644 --- a/git-credentials/src/lib.rs +++ b/git-credentials/src/lib.rs @@ -22,59 +22,14 @@ pub trait Helper { fn finish(self) -> std::io::Result<()>; } -/// A program implementing a credentials helper. +/// A program/executable implementing the credential helper protocol. pub struct Program { /// The kind of helper program. pub kind: helper::Kind, child: Option, } -mod program { - use crate::{helper, Helper, Program}; - use std::process::{Command, Stdio}; - - impl Helper for Program { - type Send = std::process::ChildStdin; - type Receive = std::process::ChildStdout; - - fn start(&mut self, action: &helper::Action<'_>) -> std::io::Result<(Self::Send, Option)> { - assert!(self.child.is_none(), "BUG: cannot call this twice"); - let (mut cmd, is_custom) = match self.kind { - helper::Kind::GitCredential => { - let mut cmd = Command::new(cfg!(windows).then(|| "git.exe").unwrap_or("git")); - cmd.arg("credential"); - (cmd, false) - } - }; - cmd.arg(action.as_helper_arg(is_custom)) - .stdin(Stdio::piped()) - .stdout(if action.expects_output() { - Stdio::piped() - } else { - Stdio::null() - }); - let mut child = cmd.spawn()?; - let stdin = child.stdin.take().expect("stdin to be configured"); - let stdout = child.stdout.take(); - - self.child = Some(child); - Ok((stdin, stdout)) - } - - fn finish(self) -> std::io::Result<()> { - self.child.expect("start() called").wait().and_then(|status| { - if status.success() { - Ok(()) - } else { - Err(std::io::Error::new( - std::io::ErrorKind::Other, - format!("Credentials helper program failed with status code {:?}", status.code()), - )) - } - }) - } - } -} +mod program; /// pub mod helper; diff --git a/git-credentials/src/program.rs b/git-credentials/src/program.rs new file mode 100644 index 00000000000..e240152a1dd --- /dev/null +++ b/git-credentials/src/program.rs @@ -0,0 +1,44 @@ +use crate::{helper, Helper, Program}; +use std::process::{Command, Stdio}; + +impl Helper for Program { + type Send = std::process::ChildStdin; + type Receive = std::process::ChildStdout; + + fn start(&mut self, action: &helper::Action<'_>) -> std::io::Result<(Self::Send, Option)> { + assert!(self.child.is_none(), "BUG: cannot call this twice"); + let (mut cmd, is_custom) = match self.kind { + helper::Kind::GitCredential => { + let mut cmd = Command::new(cfg!(windows).then(|| "git.exe").unwrap_or("git")); + cmd.arg("credential"); + (cmd, false) + } + }; + cmd.arg(action.as_helper_arg(is_custom)) + .stdin(Stdio::piped()) + .stdout(if action.expects_output() { + Stdio::piped() + } else { + Stdio::null() + }); + let mut child = cmd.spawn()?; + let stdin = child.stdin.take().expect("stdin to be configured"); + let stdout = child.stdout.take(); + + self.child = Some(child); + Ok((stdin, stdout)) + } + + fn finish(self) -> std::io::Result<()> { + self.child.expect("start() called").wait().and_then(|status| { + if status.success() { + Ok(()) + } else { + Err(std::io::Error::new( + std::io::ErrorKind::Other, + format!("Credentials helper program failed with status code {:?}", status.code()), + )) + } + }) + } +} From 64bc2ec666dacba486bd1de2fbd95f97f2efc7a5 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 14:37:20 +0800 Subject: [PATCH 024/236] feat: `helper::invoke(helper, action, context)` function that allows for more flexible helper invocation (#450) --- git-credentials/src/helper.rs | 59 ++++++++++++++++++++++++++++++++--- 1 file changed, 55 insertions(+), 4 deletions(-) diff --git a/git-credentials/src/helper.rs b/git-credentials/src/helper.rs index ccf9f512de9..19cf12cca6f 100644 --- a/git-credentials/src/helper.rs +++ b/git-credentials/src/helper.rs @@ -6,6 +6,10 @@ pub enum Kind { GitCredential, } +/// Additional context to be passed to the credentials helper. +// TODO: fill in what's needed per configuration +pub struct Context; + mod error { /// The error used in the [credentials helper][crate::helper()]. #[derive(Debug, thiserror::Error)] @@ -15,8 +19,8 @@ mod error { Io(#[from] std::io::Error), #[error("Could not find {name:?} in output of credentials helper")] KeyNotFound { name: String }, - #[error("Credentials helper program failed with status code {code:?}")] - CredentialsHelperFailed { code: Option }, + #[error(transparent)] + CredentialsHelperFailed { source: std::io::Error }, } } pub use error::Error; @@ -80,7 +84,7 @@ pub struct Outcome { } pub(crate) mod function { - use crate::helper::{message, Action, Error, NextAction, Outcome, Result}; + use crate::helper::{message, Action, Context, Error, NextAction, Outcome, Result}; use std::io::Read; use std::process::{Command, Stdio}; @@ -125,7 +129,9 @@ pub(crate) mod function { .transpose()?; let status = child.wait()?; if !status.success() { - return Err(Error::CredentialsHelperFailed { code: status.code() }); + return Err(Error::CredentialsHelperFailed { + source: std::io::Error::new(std::io::ErrorKind::Other, "credentials helper failed"), + }); } match stdout { @@ -150,7 +156,52 @@ pub(crate) mod function { } } } + + /// Invoke the given `helper` with `action` in `context`. + /// + /// Usually the first call is performed with [`Action::Fill`] to obtain an identity, which subsequently can be used. + /// On successful usage, use [`NextAction::approve()`], otherwise [`NextAction::reject()`]. + pub fn invoke(mut helper: impl crate::Helper, action: Action<'_>, _context: Context) -> Result { + let (stdin, stdout) = helper.start(&action)?; + action.send(stdin)?; + let stdout = stdout + .map(|mut stdout| { + let mut buf = Vec::new(); + stdout.read_to_end(&mut buf).map(|_| buf) + }) + .transpose()?; + helper.finish().map_err(|err| { + if err.kind() == std::io::ErrorKind::Other { + Error::CredentialsHelperFailed { source: err } + } else { + err.into() + } + })?; + + match stdout { + None => Ok(None), + Some(stdout) => { + let kvs = message::decode(stdout.as_slice())?; + let find = |name: &str| { + kvs.iter() + .find(|(k, _)| k == name) + .ok_or_else(|| Error::KeyNotFound { name: name.into() }) + .map(|(_, n)| n.to_owned()) + }; + Ok(Some(Outcome { + identity: git_sec::identity::Account { + username: find("username")?, + password: find("password")?, + }, + next: NextAction { + previous_output: stdout.into(), + }, + })) + } + } + } } +pub use function::invoke; /// pub mod message { From f2a2c5ebb7d7428460fe22e9b84dec242a992302 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 14:43:57 +0800 Subject: [PATCH 025/236] express `helper()` in terms of `helper::invoke()` (#450) --- git-credentials/src/helper.rs | 63 +++++----------------------------- git-credentials/src/program.rs | 7 ++++ 2 files changed, 16 insertions(+), 54 deletions(-) diff --git a/git-credentials/src/helper.rs b/git-credentials/src/helper.rs index 19cf12cca6f..28b34fc4bd7 100644 --- a/git-credentials/src/helper.rs +++ b/git-credentials/src/helper.rs @@ -8,6 +8,7 @@ pub enum Kind { /// Additional context to be passed to the credentials helper. // TODO: fill in what's needed per configuration +#[derive(Debug, Default)] pub struct Context; mod error { @@ -84,9 +85,8 @@ pub struct Outcome { } pub(crate) mod function { - use crate::helper::{message, Action, Context, Error, NextAction, Outcome, Result}; + use crate::helper::{message, Action, Context, Error, Kind, NextAction, Outcome, Result}; use std::io::Read; - use std::process::{Command, Stdio}; impl Action<'_> { /// Send ourselves to the given `write` which is expected to be credentials-helper compatible @@ -101,60 +101,15 @@ pub(crate) mod function { } } - /// Call the `git` credentials helper program performing the given `action`. + /// Call the `git` credentials helper program performing the given `action`, without any context from git configuration. /// - /// Usually the first call is performed with [`Action::Fill`] to obtain an identity, which subsequently can be used. - /// On successful usage, use [`NextAction::approve()`], otherwise [`NextAction::reject()`]. + /// See [`invoke()`] for a more flexible implementation. pub fn helper(action: Action<'_>) -> Result { - let mut cmd = Command::new(cfg!(windows).then(|| "git.exe").unwrap_or("git")); - cmd.arg("credential") - .arg(action.as_helper_arg(false)) - .stdin(Stdio::piped()) - .stdout(if action.expects_output() { - Stdio::piped() - } else { - Stdio::null() - }); - let mut child = cmd.spawn()?; - let mut stdin = child.stdin.take().expect("stdin to be configured"); - let stdout = child.stdout.take(); - - action.send(&mut stdin)?; - - let stdout = stdout - .map(|mut stdout| { - let mut buf = Vec::new(); - stdout.read_to_end(&mut buf).map(|_| buf) - }) - .transpose()?; - let status = child.wait()?; - if !status.success() { - return Err(Error::CredentialsHelperFailed { - source: std::io::Error::new(std::io::ErrorKind::Other, "credentials helper failed"), - }); - } - - match stdout { - None => Ok(None), - Some(stdout) => { - let kvs = message::decode(stdout.as_slice())?; - let find = |name: &str| { - kvs.iter() - .find(|(k, _)| k == name) - .ok_or_else(|| Error::KeyNotFound { name: name.into() }) - .map(|(_, n)| n.to_owned()) - }; - Ok(Some(Outcome { - identity: git_sec::identity::Account { - username: find("username")?, - password: find("password")?, - }, - next: NextAction { - previous_output: stdout.into(), - }, - })) - } - } + invoke( + crate::Program::from_kind(Kind::GitCredential), + action, + Context::default(), + ) } /// Invoke the given `helper` with `action` in `context`. diff --git a/git-credentials/src/program.rs b/git-credentials/src/program.rs index e240152a1dd..7e7dbd0757c 100644 --- a/git-credentials/src/program.rs +++ b/git-credentials/src/program.rs @@ -1,6 +1,13 @@ use crate::{helper, Helper, Program}; use std::process::{Command, Stdio}; +impl Program { + /// Create a new program of the given `kind`. + pub fn from_kind(kind: helper::Kind) -> Self { + Program { kind, child: None } + } +} + impl Helper for Program { type Send = std::process::ChildStdin; type Receive = std::process::ChildStdout; From 077ab57425e9becba646c4651653aae256b8a1c9 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 14:48:56 +0800 Subject: [PATCH 026/236] refactor (#450) avoid using actual credentials helper where none is required --- git-protocol/tests/fetch/mod.rs | 4 ++++ git-protocol/tests/fetch/v1.rs | 8 ++++---- git-protocol/tests/fetch/v2.rs | 10 +++++----- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/git-protocol/tests/fetch/mod.rs b/git-protocol/tests/fetch/mod.rs index e8366fb2cca..0433b9f2e01 100644 --- a/git-protocol/tests/fetch/mod.rs +++ b/git-protocol/tests/fetch/mod.rs @@ -11,6 +11,10 @@ type Cursor = std::io::Cursor>; #[cfg(feature = "async-client")] type Cursor = futures_lite::io::Cursor>; +fn helper_unused(_action: git_credentials::helper::Action<'_>) -> git_credentials::helper::Result { + panic!("Call to credentials helper is unexpected") +} + #[derive(Default)] pub struct CloneDelegate { pack_bytes: usize, diff --git a/git-protocol/tests/fetch/v1.rs b/git-protocol/tests/fetch/v1.rs index 2eb9cf1be9c..9088b9d3c99 100644 --- a/git-protocol/tests/fetch/v1.rs +++ b/git-protocol/tests/fetch/v1.rs @@ -3,7 +3,7 @@ use git_features::progress; use git_protocol::{fetch, FetchConnection}; use git_transport::Protocol; -use crate::fetch::{oid, transport, CloneDelegate, LsRemoteDelegate}; +use crate::fetch::{helper_unused, oid, transport, CloneDelegate, LsRemoteDelegate}; #[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] async fn clone() -> crate::Result { @@ -17,7 +17,7 @@ async fn clone() -> crate::Result { git_transport::client::git::ConnectMode::Daemon, ), &mut dlg, - git_protocol::credentials::helper, + helper_unused, progress::Discard, FetchConnection::TerminateOnSuccessfulCompletion, ) @@ -39,7 +39,7 @@ async fn ls_remote() -> crate::Result { git_protocol::fetch( &mut transport, &mut delegate, - git_protocol::credentials::helper, + helper_unused, progress::Discard, FetchConnection::AllowReuse, ) @@ -80,7 +80,7 @@ async fn ls_remote_handshake_failure_due_to_downgrade() -> crate::Result { git_transport::client::git::ConnectMode::Process, ), delegate, - git_protocol::credentials::helper, + helper_unused, progress::Discard, FetchConnection::AllowReuse, ) diff --git a/git-protocol/tests/fetch/v2.rs b/git-protocol/tests/fetch/v2.rs index 3f6d9c6eb50..d4bf585447a 100644 --- a/git-protocol/tests/fetch/v2.rs +++ b/git-protocol/tests/fetch/v2.rs @@ -3,7 +3,7 @@ use git_features::progress; use git_protocol::{fetch, FetchConnection}; use git_transport::Protocol; -use crate::fetch::{oid, transport, CloneDelegate, CloneRefInWantDelegate, LsRemoteDelegate}; +use crate::fetch::{helper_unused, oid, transport, CloneDelegate, CloneRefInWantDelegate, LsRemoteDelegate}; #[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] async fn clone_abort_prep() -> crate::Result { @@ -21,7 +21,7 @@ async fn clone_abort_prep() -> crate::Result { let err = git_protocol::fetch( &mut transport, &mut dlg, - git_protocol::credentials::helper, + helper_unused, progress::Discard, FetchConnection::TerminateOnSuccessfulCompletion, ) @@ -65,7 +65,7 @@ async fn ls_remote() -> crate::Result { git_protocol::fetch( &mut transport, &mut delegate, - git_protocol::credentials::helper, + helper_unused, progress::Discard, FetchConnection::AllowReuse, ) @@ -118,7 +118,7 @@ async fn ls_remote_abort_in_prep_ls_refs() -> crate::Result { let err = git_protocol::fetch( &mut transport, &mut delegate, - git_protocol::credentials::helper, + helper_unused, progress::Discard, FetchConnection::AllowReuse, ) @@ -157,7 +157,7 @@ async fn ref_in_want() -> crate::Result { git_protocol::fetch( &mut transport, &mut delegate, - git_protocol::credentials::helper, + helper_unused, progress::Discard, FetchConnection::TerminateOnSuccessfulCompletion, ) From 01efe88cff52e75ba0b76ecc27a41994ee908d2c Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 14:51:53 +0800 Subject: [PATCH 027/236] thanks clippy --- git-credentials/src/helper.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-credentials/src/helper.rs b/git-credentials/src/helper.rs index 28b34fc4bd7..6edeb4aac60 100644 --- a/git-credentials/src/helper.rs +++ b/git-credentials/src/helper.rs @@ -94,7 +94,7 @@ pub(crate) mod function { match self { Action::Fill(url) => message::encode(url, write), Action::Approve(last) | Action::Reject(last) => { - write.write_all(&last)?; + write.write_all(last)?; write.write_all(&[b'\n']) } } From 03bf747292af7792bc175c4f06939b1e02f7234c Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 14:57:32 +0800 Subject: [PATCH 028/236] refactor (#450) --- git-credentials/src/lib.rs | 9 +++-- git-credentials/src/program.rs | 69 +++++++++++++++++++--------------- 2 files changed, 43 insertions(+), 35 deletions(-) diff --git a/git-credentials/src/lib.rs b/git-credentials/src/lib.rs index 60064996065..f44b9fd5059 100644 --- a/git-credentials/src/lib.rs +++ b/git-credentials/src/lib.rs @@ -23,10 +23,11 @@ pub trait Helper { } /// A program/executable implementing the credential helper protocol. -pub struct Program { - /// The kind of helper program. - pub kind: helper::Kind, - child: Option, +pub enum Program { + /// The kind of program, ready for launch + Ready(helper::Kind), + /// The process is running. + Started(std::process::Child), } mod program; diff --git a/git-credentials/src/program.rs b/git-credentials/src/program.rs index 7e7dbd0757c..d636abaf1d7 100644 --- a/git-credentials/src/program.rs +++ b/git-credentials/src/program.rs @@ -4,7 +4,7 @@ use std::process::{Command, Stdio}; impl Program { /// Create a new program of the given `kind`. pub fn from_kind(kind: helper::Kind) -> Self { - Program { kind, child: None } + Program::Ready(kind) } } @@ -13,39 +13,46 @@ impl Helper for Program { type Receive = std::process::ChildStdout; fn start(&mut self, action: &helper::Action<'_>) -> std::io::Result<(Self::Send, Option)> { - assert!(self.child.is_none(), "BUG: cannot call this twice"); - let (mut cmd, is_custom) = match self.kind { - helper::Kind::GitCredential => { - let mut cmd = Command::new(cfg!(windows).then(|| "git.exe").unwrap_or("git")); - cmd.arg("credential"); - (cmd, false) - } - }; - cmd.arg(action.as_helper_arg(is_custom)) - .stdin(Stdio::piped()) - .stdout(if action.expects_output() { - Stdio::piped() - } else { - Stdio::null() - }); - let mut child = cmd.spawn()?; - let stdin = child.stdin.take().expect("stdin to be configured"); - let stdout = child.stdout.take(); + match self { + Program::Ready(kind) => { + let (mut cmd, is_custom) = match kind { + helper::Kind::GitCredential => { + let mut cmd = Command::new(cfg!(windows).then(|| "git.exe").unwrap_or("git")); + cmd.arg("credential"); + (cmd, false) + } + }; + cmd.arg(action.as_helper_arg(is_custom)) + .stdin(Stdio::piped()) + .stdout(if action.expects_output() { + Stdio::piped() + } else { + Stdio::null() + }); + let mut child = cmd.spawn()?; + let stdin = child.stdin.take().expect("stdin to be configured"); + let stdout = child.stdout.take(); - self.child = Some(child); - Ok((stdin, stdout)) + *self = Program::Started(child); + Ok((stdin, stdout)) + } + Program::Started(_) => panic!("BUG: must not call `start()` twice"), + } } fn finish(self) -> std::io::Result<()> { - self.child.expect("start() called").wait().and_then(|status| { - if status.success() { - Ok(()) - } else { - Err(std::io::Error::new( - std::io::ErrorKind::Other, - format!("Credentials helper program failed with status code {:?}", status.code()), - )) - } - }) + match self { + Program::Started(mut child) => child.wait().and_then(|status| { + if status.success() { + Ok(()) + } else { + Err(std::io::Error::new( + std::io::ErrorKind::Other, + format!("Credentials helper program failed with status code {:?}", status.code()), + )) + } + }), + Program::Ready(_) => panic!("Call `start()` before calling finish()"), + } } } From 71f651930e6fd53e3c3f9e82dfd95828e4981d92 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 15:29:05 +0800 Subject: [PATCH 029/236] change!: move `helper::invoke()` related types into `helper::invoke` module. (#450) Also allow to pass arbitrary bytes (more or less) as context by not forcing it all into a string. Values can now be everything, which helps with passing paths or other values. --- git-credentials/src/helper.rs | 201 --------------------------- git-credentials/src/helper/invoke.rs | 93 +++++++++++++ git-credentials/src/helper/mod.rs | 105 ++++++++++++++ git-credentials/src/lib.rs | 12 +- git-credentials/tests/helper/mod.rs | 2 +- 5 files changed, 210 insertions(+), 203 deletions(-) delete mode 100644 git-credentials/src/helper.rs create mode 100644 git-credentials/src/helper/invoke.rs create mode 100644 git-credentials/src/helper/mod.rs diff --git a/git-credentials/src/helper.rs b/git-credentials/src/helper.rs deleted file mode 100644 index 6edeb4aac60..00000000000 --- a/git-credentials/src/helper.rs +++ /dev/null @@ -1,201 +0,0 @@ -use bstr::{BStr, BString}; - -/// The kind of helper program to use. -pub enum Kind { - /// The built-in git-credential helper program, part of any git distribution. - GitCredential, -} - -/// Additional context to be passed to the credentials helper. -// TODO: fill in what's needed per configuration -#[derive(Debug, Default)] -pub struct Context; - -mod error { - /// The error used in the [credentials helper][crate::helper()]. - #[derive(Debug, thiserror::Error)] - #[allow(missing_docs)] - pub enum Error { - #[error("An IO error occurred while communicating to the credentials helper")] - Io(#[from] std::io::Error), - #[error("Could not find {name:?} in output of credentials helper")] - KeyNotFound { name: String }, - #[error(transparent)] - CredentialsHelperFailed { source: std::io::Error }, - } -} -pub use error::Error; - -/// The Result type used in [`helper()`][crate::helper()]. -pub type Result = std::result::Result, Error>; - -/// The action to perform by the credentials [helper][`crate::helper()`]. -#[derive(Clone, Debug)] -pub enum Action<'a> { - /// Provide credentials using the given repository URL (as &str) as context. - Fill(&'a BStr), - /// Approve the credentials as identified by the previous input provided as `BString`. - Approve(BString), - /// Reject the credentials as identified by the previous input provided as `BString`. - Reject(BString), -} - -impl<'a> Action<'a> { - /// Returns true if this action expects output from the helper. - pub fn expects_output(&self) -> bool { - matches!(self, Action::Fill(_)) - } - /// The name of the argument to describe this action. If `is_custom` is true, the target program is - /// a custom credentials helper, not a built-in one. - pub fn as_helper_arg(&self, is_custom: bool) -> &str { - match self { - Action::Fill(_) if is_custom => "get", - Action::Fill(_) => "fill", - Action::Approve(_) if is_custom => "store", - Action::Approve(_) => "approve", - Action::Reject(_) if is_custom => "erase", - Action::Reject(_) => "reject", - } - } -} - -/// A handle to [approve][NextAction::approve()] or [reject][NextAction::reject()] the outcome of the initial action. -#[derive(Clone, Debug)] -pub struct NextAction { - previous_output: BString, -} - -impl NextAction { - /// Approve the result of the previous [Action]. - pub fn approve(self) -> Action<'static> { - Action::Approve(self.previous_output) - } - /// Reject the result of the previous [Action]. - pub fn reject(self) -> Action<'static> { - Action::Reject(self.previous_output) - } -} - -/// The outcome of the credentials [`helper`][crate::helper()]. -pub struct Outcome { - /// The obtained identity. - pub identity: git_sec::identity::Account, - /// A handle to the action to perform next in another call to [`helper()`][crate::helper()]. - pub next: NextAction, -} - -pub(crate) mod function { - use crate::helper::{message, Action, Context, Error, Kind, NextAction, Outcome, Result}; - use std::io::Read; - - impl Action<'_> { - /// Send ourselves to the given `write` which is expected to be credentials-helper compatible - pub fn send(&self, mut write: impl std::io::Write) -> std::io::Result<()> { - match self { - Action::Fill(url) => message::encode(url, write), - Action::Approve(last) | Action::Reject(last) => { - write.write_all(last)?; - write.write_all(&[b'\n']) - } - } - } - } - - /// Call the `git` credentials helper program performing the given `action`, without any context from git configuration. - /// - /// See [`invoke()`] for a more flexible implementation. - pub fn helper(action: Action<'_>) -> Result { - invoke( - crate::Program::from_kind(Kind::GitCredential), - action, - Context::default(), - ) - } - - /// Invoke the given `helper` with `action` in `context`. - /// - /// Usually the first call is performed with [`Action::Fill`] to obtain an identity, which subsequently can be used. - /// On successful usage, use [`NextAction::approve()`], otherwise [`NextAction::reject()`]. - pub fn invoke(mut helper: impl crate::Helper, action: Action<'_>, _context: Context) -> Result { - let (stdin, stdout) = helper.start(&action)?; - action.send(stdin)?; - let stdout = stdout - .map(|mut stdout| { - let mut buf = Vec::new(); - stdout.read_to_end(&mut buf).map(|_| buf) - }) - .transpose()?; - helper.finish().map_err(|err| { - if err.kind() == std::io::ErrorKind::Other { - Error::CredentialsHelperFailed { source: err } - } else { - err.into() - } - })?; - - match stdout { - None => Ok(None), - Some(stdout) => { - let kvs = message::decode(stdout.as_slice())?; - let find = |name: &str| { - kvs.iter() - .find(|(k, _)| k == name) - .ok_or_else(|| Error::KeyNotFound { name: name.into() }) - .map(|(_, n)| n.to_owned()) - }; - Ok(Some(Outcome { - identity: git_sec::identity::Account { - username: find("username")?, - password: find("password")?, - }, - next: NextAction { - previous_output: stdout.into(), - }, - })) - } - } - } -} -pub use function::invoke; - -/// -pub mod message { - use bstr::BStr; - - /// Encode `url` to `out` for consumption by a `git credentials` helper program. - pub fn encode(url: &BStr, mut out: impl std::io::Write) -> std::io::Result<()> { - validate(url)?; - writeln!(out, "url={}\n", url) - } - - fn validate(url: &BStr) -> std::io::Result<()> { - if url.contains(&0) || url.contains(&b'\n') { - return Err(std::io::Error::new( - std::io::ErrorKind::Other, - "token to encode must not contain newlines or null bytes", - )); - } - Ok(()) - } - - /// Decode all lines in `input` as key-value pairs produced by a `git credentials` helper program. - pub fn decode(mut input: impl std::io::Read) -> std::io::Result> { - let mut buf = String::new(); - input.read_to_string(&mut buf)?; - buf.lines() - .take_while(|l| !l.is_empty()) - .map(|l| { - let mut iter = l.splitn(2, '=').map(|s| s.to_owned()); - match (iter.next(), iter.next()) { - (Some(key), Some(value)) => validate(key.as_str().into()) - .and_then(|_| validate(value.as_str().into())) - .map(|_| (key, value)), - _ => Err(std::io::Error::new( - std::io::ErrorKind::Other, - "Invalid format, expecting key=value", - )), - } - }) - .collect::>>() - } -} diff --git a/git-credentials/src/helper/invoke.rs b/git-credentials/src/helper/invoke.rs new file mode 100644 index 00000000000..d7ca78cf067 --- /dev/null +++ b/git-credentials/src/helper/invoke.rs @@ -0,0 +1,93 @@ +use crate::helper::NextAction; + +/// The outcome of the credentials [`helper`][crate::helper()]. +pub struct Outcome { + /// The obtained identity. + pub identity: git_sec::identity::Account, + /// A handle to the action to perform next in another call to [`helper()`][crate::helper()]. + pub next: NextAction, +} + +/// The Result type used in [`helper()`][crate::helper()]. +pub type Result = std::result::Result, Error>; + +/// The error used in the [credentials helper][crate::helper::invoke()]. +#[derive(Debug, thiserror::Error)] +#[allow(missing_docs)] +pub enum Error { + #[error("Cannot handle usernames or passwords in illformed UTF-8 encoding")] + IllformedUtf8InUsernameOrPassword, + #[error("An IO error occurred while communicating to the credentials helper")] + Io(#[from] std::io::Error), + #[error("Could not find {name:?} in output of credentials helper")] + KeyNotFound { name: String }, + #[error(transparent)] + CredentialsHelperFailed { source: std::io::Error }, +} + +pub(crate) mod function { + use crate::helper::{invoke::Error, invoke::Outcome, invoke::Result, message, Action, Context, NextAction}; + use bstr::ByteVec; + use std::io::Read; + + impl Action<'_> { + /// Send ourselves to the given `write` which is expected to be credentials-helper compatible + pub fn send(&self, mut write: impl std::io::Write) -> std::io::Result<()> { + match self { + Action::Fill(url) => message::encode(url, write), + Action::Approve(last) | Action::Reject(last) => { + write.write_all(last)?; + write.write_all(&[b'\n']) + } + } + } + } + + /// Invoke the given `helper` with `action` in `context`. + /// + /// Usually the first call is performed with [`Action::Fill`] to obtain an identity, which subsequently can be used. + /// On successful usage, use [`NextAction::approve()`], otherwise [`NextAction::reject()`]. + pub fn invoke(mut helper: impl crate::Helper, action: Action<'_>, _context: Context) -> Result { + let (stdin, stdout) = helper.start(&action)?; + action.send(stdin)?; + let stdout = stdout + .map(|mut stdout| { + let mut buf = Vec::new(); + stdout.read_to_end(&mut buf).map(|_| buf) + }) + .transpose()?; + helper.finish().map_err(|err| { + if err.kind() == std::io::ErrorKind::Other { + Error::CredentialsHelperFailed { source: err } + } else { + err.into() + } + })?; + + match stdout { + None => Ok(None), + Some(stdout) => { + let kvs = message::decode(stdout.as_slice())?; + let find = |name: &str| { + kvs.iter() + .find(|(k, _)| k == name) + .ok_or_else(|| Error::KeyNotFound { name: name.into() }) + .map(|(_, n)| n.to_vec()) + }; + Ok(Some(Outcome { + identity: git_sec::identity::Account { + username: find("username")? + .into_string() + .map_err(|_| Error::IllformedUtf8InUsernameOrPassword)?, + password: find("password")? + .into_string() + .map_err(|_| Error::IllformedUtf8InUsernameOrPassword)?, + }, + next: NextAction { + previous_output: stdout.into(), + }, + })) + } + } + } +} diff --git a/git-credentials/src/helper/mod.rs b/git-credentials/src/helper/mod.rs new file mode 100644 index 00000000000..ae853190cc4 --- /dev/null +++ b/git-credentials/src/helper/mod.rs @@ -0,0 +1,105 @@ +use bstr::{BStr, BString}; + +/// The kind of helper program to use. +pub enum Kind { + /// The built-in git-credential helper program, part of any git distribution. + GitCredential, +} + +/// Additional context to be passed to the credentials helper. +// TODO: fill in what's needed per configuration +#[derive(Debug, Default)] +pub struct Context; + +/// The action to perform by the credentials [helper][`crate::helper()`]. +#[derive(Clone, Debug)] +pub enum Action<'a> { + /// Provide credentials using the given repository URL (as &str) as context. + Fill(&'a BStr), + /// Approve the credentials as identified by the previous input provided as `BString`. + Approve(BString), + /// Reject the credentials as identified by the previous input provided as `BString`. + Reject(BString), +} + +impl<'a> Action<'a> { + /// Returns true if this action expects output from the helper. + pub fn expects_output(&self) -> bool { + matches!(self, Action::Fill(_)) + } + /// The name of the argument to describe this action. If `is_custom` is true, the target program is + /// a custom credentials helper, not a built-in one. + pub fn as_helper_arg(&self, is_custom: bool) -> &str { + match self { + Action::Fill(_) if is_custom => "get", + Action::Fill(_) => "fill", + Action::Approve(_) if is_custom => "store", + Action::Approve(_) => "approve", + Action::Reject(_) if is_custom => "erase", + Action::Reject(_) => "reject", + } + } +} + +/// A handle to [approve][NextAction::approve()] or [reject][NextAction::reject()] the outcome of the initial action. +#[derive(Clone, Debug)] +pub struct NextAction { + previous_output: BString, +} + +impl NextAction { + /// Approve the result of the previous [Action]. + pub fn approve(self) -> Action<'static> { + Action::Approve(self.previous_output) + } + /// Reject the result of the previous [Action]. + pub fn reject(self) -> Action<'static> { + Action::Reject(self.previous_output) + } +} + +/// +pub mod invoke; +pub use invoke::function::invoke; + +/// +pub mod message { + use bstr::{BStr, BString, ByteSlice}; + + /// Encode `url` to `out` for consumption by a `git credentials` helper program. + pub fn encode(url: &BStr, mut out: impl std::io::Write) -> std::io::Result<()> { + validate(url)?; + writeln!(out, "url={}\n", url) + } + + fn validate(url: &BStr) -> std::io::Result<()> { + if url.contains(&0) || url.contains(&b'\n') { + return Err(std::io::Error::new( + std::io::ErrorKind::Other, + "token to encode must not contain newlines or null bytes", + )); + } + Ok(()) + } + + /// Decode all lines in `input` as key-value pairs produced by a `git credentials` helper program. + pub fn decode(mut input: impl std::io::Read) -> std::io::Result> { + let mut buf = Vec::::with_capacity(512); + input.read_to_end(&mut buf)?; + buf.lines() + .take_while(|line| !line.is_empty()) + .map(|line| { + let mut it = line.splitn(2, |b| *b == b'='); + match (it.next().and_then(|k| k.to_str().ok()), it.next().map(|v| v.as_bstr())) { + (Some(key), Some(value)) => validate(key.into()) + .and_then(|_| validate(value.into())) + .map(|_| (key.into(), value.into())), + _ => Err(std::io::Error::new( + std::io::ErrorKind::Other, + format!("Invalid format, expecting key=value, got {:?}", line.as_bstr()), + )), + } + }) + .collect::>>() + } +} diff --git a/git-credentials/src/lib.rs b/git-credentials/src/lib.rs index f44b9fd5059..adb2486d405 100644 --- a/git-credentials/src/lib.rs +++ b/git-credentials/src/lib.rs @@ -34,4 +34,14 @@ mod program; /// pub mod helper; -pub use helper::function::helper; + +/// Call the `git` credentials helper program performing the given `action`, without any context from git configuration. +/// +/// See [`invoke()`] for a more flexible implementation. +pub fn helper(action: helper::Action<'_>) -> helper::invoke::Result { + helper::invoke( + Program::from_kind(helper::Kind::GitCredential), + action, + helper::Context::default(), + ) +} diff --git a/git-credentials/tests/helper/mod.rs b/git-credentials/tests/helper/mod.rs index 57ce3ff1c19..632a74f6483 100644 --- a/git-credentials/tests/helper/mod.rs +++ b/git-credentials/tests/helper/mod.rs @@ -57,7 +57,7 @@ this=is-skipped-past-empty-line" ("password", "secr3t") ] .iter() - .map(|(k, v)| (k.to_string(), v.to_string())) + .map(|(k, v)| (k.to_string(), (*v).into())) .collect::>() ); Ok(()) From 40cc1dc09bf8639b53e0ef70e78dcd56518d92ea Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 15:30:42 +0800 Subject: [PATCH 030/236] adjust to changes in `git-credentials` (#450) --- git-protocol/src/fetch/handshake.rs | 6 +++--- git-protocol/src/fetch_fn.rs | 2 +- git-protocol/tests/fetch/mod.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/git-protocol/src/fetch/handshake.rs b/git-protocol/src/fetch/handshake.rs index c105f812126..2f8ca4fc2d0 100644 --- a/git-protocol/src/fetch/handshake.rs +++ b/git-protocol/src/fetch/handshake.rs @@ -21,7 +21,7 @@ mod error { #[allow(missing_docs)] pub enum Error { #[error(transparent)] - Credentials(#[from] credentials::helper::Error), + Credentials(#[from] credentials::helper::invoke::Error), #[error(transparent)] Transport(#[from] client::Error), #[error("The transport didn't accept the advertised server version {actual_version:?} and closed the connection client side")] @@ -54,7 +54,7 @@ pub(crate) mod function { progress: &mut impl Progress, ) -> Result where - AuthFn: FnMut(credentials::helper::Action<'_>) -> credentials::helper::Result, + AuthFn: FnMut(credentials::helper::Action<'_>) -> credentials::helper::invoke::Result, T: client::Transport, { let (server_protocol_version, refs, capabilities) = { @@ -79,7 +79,7 @@ pub(crate) mod function { drop(result); // needed to workaround this: https://github.com/rust-lang/rust/issues/76149 let url = transport.to_url(); progress.set_name("authentication"); - let credentials::helper::Outcome { identity, next } = + let credentials::helper::invoke::Outcome { identity, next } = authenticate(credentials::helper::Action::Fill(url.as_str().into()))? .expect("FILL provides an identity"); transport.set_identity(identity)?; diff --git a/git-protocol/src/fetch_fn.rs b/git-protocol/src/fetch_fn.rs index 3558fad142f..9df3db13201 100644 --- a/git-protocol/src/fetch_fn.rs +++ b/git-protocol/src/fetch_fn.rs @@ -53,7 +53,7 @@ pub async fn fetch( fetch_mode: FetchConnection, ) -> Result<(), Error> where - F: FnMut(credentials::helper::Action<'_>) -> credentials::helper::Result, + F: FnMut(credentials::helper::Action<'_>) -> credentials::helper::invoke::Result, D: Delegate, T: client::Transport, { diff --git a/git-protocol/tests/fetch/mod.rs b/git-protocol/tests/fetch/mod.rs index 0433b9f2e01..909ed627e95 100644 --- a/git-protocol/tests/fetch/mod.rs +++ b/git-protocol/tests/fetch/mod.rs @@ -11,7 +11,7 @@ type Cursor = std::io::Cursor>; #[cfg(feature = "async-client")] type Cursor = futures_lite::io::Cursor>; -fn helper_unused(_action: git_credentials::helper::Action<'_>) -> git_credentials::helper::Result { +fn helper_unused(_action: git_credentials::helper::Action<'_>) -> git_credentials::helper::invoke::Result { panic!("Call to credentials helper is unexpected") } From 0cb1ed4600c614169118b2a94fed83699989a6be Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 15:35:29 +0800 Subject: [PATCH 031/236] flesh out `helper::Context` as it will soon be used. (#450) --- git-credentials/src/helper/mod.rs | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/git-credentials/src/helper/mod.rs b/git-credentials/src/helper/mod.rs index ae853190cc4..a3c5836a29f 100644 --- a/git-credentials/src/helper/mod.rs +++ b/git-credentials/src/helper/mod.rs @@ -8,17 +8,33 @@ pub enum Kind { /// Additional context to be passed to the credentials helper. // TODO: fill in what's needed per configuration -#[derive(Debug, Default)] -pub struct Context; +#[derive(Debug, Default, Clone)] +pub struct Context { + /// The protocol over which the credential will be used (e.g., https). + pub protocol: Option, + /// The remote hostname for a network credential. This includes the port number if one was specified (e.g., "example.com:8088"). + pub host: Option, + /// The path with which the credential will be used. E.g., for accessing a remote https repository, this will be the repository’s path on the server. + /// It can also be a path on the file system. + pub path: Option, + /// The credential’s username, if we already have one (e.g., from a URL, the configuration, the user, or from a previously run helper). + pub username: Option, + /// The credential’s password, if we are asking it to be stored. + pub password: Option, + /// When this special attribute is read by git credential, the value is parsed as a URL and treated as if its constituent + /// parts were read (e.g., url=https://example.com would behave as if + /// protocol=https and host=example.com had been provided). This can help callers avoid parsing URLs themselves. + pub url: Option, +} /// The action to perform by the credentials [helper][`crate::helper()`]. #[derive(Clone, Debug)] pub enum Action<'a> { - /// Provide credentials using the given repository URL (as &str) as context. + /// Provide credentials using the given repository URL (as &str) as context and pre-parsed url information as seen in [`Context`]. Fill(&'a BStr), - /// Approve the credentials as identified by the previous input provided as `BString`. + /// Approve the credentials as identified by the previous input provided as `BString`, containing information from [`Context`]. Approve(BString), - /// Reject the credentials as identified by the previous input provided as `BString`. + /// Reject the credentials as identified by the previous input provided as `BString`. containing information from [`Context`]. Reject(BString), } From 280e4a3f69699e11428decc6858711b35ae8249e Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 17:13:15 +0800 Subject: [PATCH 032/236] basic round-tripping of fully fleshed-out context(#450) --- git-credentials/src/helper/context.rs | 137 ++++++++++++++++++++++++++ git-credentials/src/helper/mod.rs | 22 +---- git-credentials/tests/helper/mod.rs | 27 +++++ 3 files changed, 166 insertions(+), 20 deletions(-) create mode 100644 git-credentials/src/helper/context.rs diff --git a/git-credentials/src/helper/context.rs b/git-credentials/src/helper/context.rs new file mode 100644 index 00000000000..ce2e8eec46b --- /dev/null +++ b/git-credentials/src/helper/context.rs @@ -0,0 +1,137 @@ +use bstr::BString; + +/// Additional context to be passed to the credentials helper. +// TODO: fill in what's needed per configuration +#[derive(Debug, Default, Clone, Eq, PartialEq)] +pub struct Context { + /// The protocol over which the credential will be used (e.g., https). + pub protocol: Option, + /// The remote hostname for a network credential. This includes the port number if one was specified (e.g., "example.com:8088"). + pub host: Option, + /// The path with which the credential will be used. E.g., for accessing a remote https repository, this will be the repository’s path on the server. + /// It can also be a path on the file system. + pub path: Option, + /// The credential’s username, if we already have one (e.g., from a URL, the configuration, the user, or from a previously run helper). + pub username: Option, + /// The credential’s password, if we are asking it to be stored. + pub password: Option, + /// When this special attribute is read by git credential, the value is parsed as a URL and treated as if its constituent + /// parts were read (e.g., url=https://example.com would behave as if + /// protocol=https and host=example.com had been provided). This can help callers avoid parsing URLs themselves. + pub url: Option, +} + +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error("{key:?}={value:?} must not contain null bytes or newlines neither in key nor in value.")] + Encoding { key: String, value: BString }, +} + +mod serde { + use crate::helper::context::Error; + use bstr::BStr; + + mod write { + use crate::helper::context::serde::validate; + use crate::helper::Context; + + impl Context { + /// Write ourselves to `out` such that [`from_bytes()`][Self::from_bytes()] can decode it losslessly. + pub fn write_to(&self, mut out: impl std::io::Write) -> std::io::Result<()> { + for (key, value) in [("url", &self.url), ("path", &self.path)] { + if let Some(value) = value { + validate(key, value.as_slice().into()) + .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err))?; + out.write_all(key.as_bytes())?; + out.write_all(b"=")?; + out.write_all(&value)?; + out.write_all(b"\n")?; + } + } + for (key, value) in [ + ("protocol", &self.protocol), + ("host", &self.host), + ("username", &self.username), + ("password", &self.password), + ] { + if let Some(value) = value { + validate(key, value.as_str().into()) + .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err))?; + out.write_all(key.as_bytes())?; + out.write_all(b"=")?; + out.write_all(value.as_bytes())?; + out.write_all(b"\n")?; + } + } + Ok(()) + } + } + } + + /// + pub mod decode { + use crate::helper::context; + use crate::helper::context::serde::validate; + use crate::helper::Context; + use bstr::{BString, ByteSlice}; + + /// The error returned by [`from_bytes()`][Context::from_bytes()]. + #[derive(Debug, thiserror::Error)] + pub enum Error { + #[error("Illformed UTF-8 in value of key {key:?}: {value:?}")] + IllformedUtf8InValue { key: String, value: BString }, + #[error(transparent)] + Encoding(#[from] context::Error), + #[error("Invalid format in line {line:?}, expecting key=value")] + Syntax { line: BString }, + } + + impl Context { + /// Decode ourselves from `input` which is the format written by [`write_to()`][Self::write_to()]. + pub fn from_bytes(input: &[u8]) -> Result { + let mut ctx = Context::default(); + for res in input.lines().take_while(|line| !line.is_empty()).map(|line| { + let mut it = line.splitn(2, |b| *b == b'='); + match (it.next().and_then(|k| k.to_str().ok()), it.next().map(|v| v.as_bstr())) { + (Some(key), Some(value)) => validate(key, value) + .map(|_| (key, value.to_owned())) + .map_err(Into::into), + _ => Err(Error::Syntax { line: line.into() }), + } + }) { + let (key, value) = res?; + match key { + "protocol" | "host" | "username" | "password" => { + if !value.is_utf8() { + return Err(Error::IllformedUtf8InValue { key: key.into(), value }); + } + let value = value.to_string(); + *match key { + "protocol" => &mut ctx.protocol, + "host" => &mut ctx.host, + "username" => &mut ctx.username, + "password" => &mut ctx.password, + _ => unreachable!("checked field names in match above"), + } = Some(value); + } + "url" => ctx.url = Some(value), + "path" => ctx.path = Some(value), + _ => {} + } + } + Ok(ctx) + } + } + } + + fn validate(key: &str, value: &BStr) -> Result<(), Error> { + if key.contains('\0') || key.contains('\n') || value.contains(&0) || value.contains(&b'\n') { + return Err(Error::Encoding { + key: key.to_owned(), + value: value.to_owned(), + }); + } + Ok(()) + } +} +pub use self::serde::decode; diff --git a/git-credentials/src/helper/mod.rs b/git-credentials/src/helper/mod.rs index a3c5836a29f..6e43eb1d952 100644 --- a/git-credentials/src/helper/mod.rs +++ b/git-credentials/src/helper/mod.rs @@ -6,26 +6,8 @@ pub enum Kind { GitCredential, } -/// Additional context to be passed to the credentials helper. -// TODO: fill in what's needed per configuration -#[derive(Debug, Default, Clone)] -pub struct Context { - /// The protocol over which the credential will be used (e.g., https). - pub protocol: Option, - /// The remote hostname for a network credential. This includes the port number if one was specified (e.g., "example.com:8088"). - pub host: Option, - /// The path with which the credential will be used. E.g., for accessing a remote https repository, this will be the repository’s path on the server. - /// It can also be a path on the file system. - pub path: Option, - /// The credential’s username, if we already have one (e.g., from a URL, the configuration, the user, or from a previously run helper). - pub username: Option, - /// The credential’s password, if we are asking it to be stored. - pub password: Option, - /// When this special attribute is read by git credential, the value is parsed as a URL and treated as if its constituent - /// parts were read (e.g., url=https://example.com would behave as if - /// protocol=https and host=example.com had been provided). This can help callers avoid parsing URLs themselves. - pub url: Option, -} +mod context; +pub use context::Context; /// The action to perform by the credentials [helper][`crate::helper()`]. #[derive(Clone, Debug)] diff --git a/git-credentials/tests/helper/mod.rs b/git-credentials/tests/helper/mod.rs index 632a74f6483..c047b12647f 100644 --- a/git-credentials/tests/helper/mod.rs +++ b/git-credentials/tests/helper/mod.rs @@ -1,3 +1,30 @@ +mod context { + use git_credentials::helper::Context; + + #[test] + fn encode_decode_roundtrip() { + for ctx in [ + Context { + protocol: Some("https".into()), + host: Some("github.com".into()), + path: Some("byron/gitoxide".into()), + username: Some("user".into()), + password: Some("pass".into()), + url: Some("https://github.com/byron/gitoxide".into()), + }, + Context::default(), + Context { + url: Some("/path/to/repo".into()), + ..Context::default() + }, + ] { + let mut buf = Vec::::new(); + ctx.write_to(&mut buf).unwrap(); + let actual = Context::from_bytes(&buf).unwrap(); + assert_eq!(actual, ctx, "ctx should encode itself losslessly"); + } + } +} mod message { mod encode { use bstr::ByteSlice; From 20dde9eb93ecfb56e72bc5d59caacf31328a55e4 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 17:47:10 +0800 Subject: [PATCH 033/236] test context value validation (#450) --- git-credentials/src/helper/context.rs | 24 +++--------------------- git-credentials/src/helper/mod.rs | 24 ++++++++++++++++++++++-- git-credentials/tests/helper/mod.rs | 22 ++++++++++++++++++++++ 3 files changed, 47 insertions(+), 23 deletions(-) diff --git a/git-credentials/src/helper/context.rs b/git-credentials/src/helper/context.rs index ce2e8eec46b..1e260c9c75e 100644 --- a/git-credentials/src/helper/context.rs +++ b/git-credentials/src/helper/context.rs @@ -1,27 +1,8 @@ use bstr::BString; -/// Additional context to be passed to the credentials helper. -// TODO: fill in what's needed per configuration -#[derive(Debug, Default, Clone, Eq, PartialEq)] -pub struct Context { - /// The protocol over which the credential will be used (e.g., https). - pub protocol: Option, - /// The remote hostname for a network credential. This includes the port number if one was specified (e.g., "example.com:8088"). - pub host: Option, - /// The path with which the credential will be used. E.g., for accessing a remote https repository, this will be the repository’s path on the server. - /// It can also be a path on the file system. - pub path: Option, - /// The credential’s username, if we already have one (e.g., from a URL, the configuration, the user, or from a previously run helper). - pub username: Option, - /// The credential’s password, if we are asking it to be stored. - pub password: Option, - /// When this special attribute is read by git credential, the value is parsed as a URL and treated as if its constituent - /// parts were read (e.g., url=https://example.com would behave as if - /// protocol=https and host=example.com had been provided). This can help callers avoid parsing URLs themselves. - pub url: Option, -} - +/// Indicates key or values contain errors that can't be encoded. #[derive(Debug, thiserror::Error)] +#[allow(missing_docs)] pub enum Error { #[error("{key:?}={value:?} must not contain null bytes or newlines neither in key nor in value.")] Encoding { key: String, value: BString }, @@ -77,6 +58,7 @@ mod serde { /// The error returned by [`from_bytes()`][Context::from_bytes()]. #[derive(Debug, thiserror::Error)] + #[allow(missing_docs)] pub enum Error { #[error("Illformed UTF-8 in value of key {key:?}: {value:?}")] IllformedUtf8InValue { key: String, value: BString }, diff --git a/git-credentials/src/helper/mod.rs b/git-credentials/src/helper/mod.rs index 6e43eb1d952..3ada5508e1c 100644 --- a/git-credentials/src/helper/mod.rs +++ b/git-credentials/src/helper/mod.rs @@ -6,8 +6,28 @@ pub enum Kind { GitCredential, } -mod context; -pub use context::Context; +/// Additional context to be passed to the credentials helper. +// TODO: fill in what's needed per configuration +#[derive(Debug, Default, Clone, Eq, PartialEq)] +pub struct Context { + /// The protocol over which the credential will be used (e.g., https). + pub protocol: Option, + /// The remote hostname for a network credential. This includes the port number if one was specified (e.g., "example.com:8088"). + pub host: Option, + /// The path with which the credential will be used. E.g., for accessing a remote https repository, this will be the repository’s path on the server. + /// It can also be a path on the file system. + pub path: Option, + /// The credential’s username, if we already have one (e.g., from a URL, the configuration, the user, or from a previously run helper). + pub username: Option, + /// The credential’s password, if we are asking it to be stored. + pub password: Option, + /// When this special attribute is read by git credential, the value is parsed as a URL and treated as if its constituent + /// parts were read (e.g., url=https://example.com would behave as if + /// protocol=https and host=example.com had been provided). This can help callers avoid parsing URLs themselves. + pub url: Option, +} +/// +pub mod context; /// The action to perform by the credentials [helper][`crate::helper()`]. #[derive(Clone, Debug)] diff --git a/git-credentials/tests/helper/mod.rs b/git-credentials/tests/helper/mod.rs index c047b12647f..55fabd5f18d 100644 --- a/git-credentials/tests/helper/mod.rs +++ b/git-credentials/tests/helper/mod.rs @@ -24,6 +24,28 @@ mod context { assert_eq!(actual, ctx, "ctx should encode itself losslessly"); } } + + #[test] + fn null_bytes_when_decoding() { + let err = Context::from_bytes(b"url=https://foo\0").unwrap_err(); + assert!(matches!( + err, + git_credentials::helper::context::decode::Error::Encoding(_) + )); + } + + #[test] + fn null_bytes_and_newlines_are_invalid_during_encoding() { + for input in [&b"https://foo\0"[..], b"https://foo\n"] { + let ctx = Context { + url: Some(input.into()), + ..Default::default() + }; + let mut buf = Vec::::new(); + let err = ctx.write_to(&mut buf).unwrap_err(); + assert_eq!(err.kind(), std::io::ErrorKind::Other); + } + } } mod message { mod encode { From 0e76efe035a48f9d042096342ac79804f1eeebdc Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 19:23:05 +0800 Subject: [PATCH 034/236] remaining decode tests (#450) --- git-credentials/tests/helper/mod.rs | 74 ++++++++++++++++++++++------- 1 file changed, 57 insertions(+), 17 deletions(-) diff --git a/git-credentials/tests/helper/mod.rs b/git-credentials/tests/helper/mod.rs index 55fabd5f18d..b07bb55b255 100644 --- a/git-credentials/tests/helper/mod.rs +++ b/git-credentials/tests/helper/mod.rs @@ -25,28 +25,68 @@ mod context { } } - #[test] - fn null_bytes_when_decoding() { - let err = Context::from_bytes(b"url=https://foo\0").unwrap_err(); - assert!(matches!( - err, - git_credentials::helper::context::decode::Error::Encoding(_) - )); + mod write_to { + use git_credentials::helper::Context; + + #[test] + fn null_bytes_and_newlines_are_invalid() { + for input in [&b"https://foo\0"[..], b"https://foo\n"] { + let ctx = Context { + url: Some(input.into()), + ..Default::default() + }; + let mut buf = Vec::::new(); + let err = ctx.write_to(&mut buf).unwrap_err(); + assert_eq!(err.kind(), std::io::ErrorKind::Other); + } + } } - #[test] - fn null_bytes_and_newlines_are_invalid_during_encoding() { - for input in [&b"https://foo\0"[..], b"https://foo\n"] { - let ctx = Context { - url: Some(input.into()), - ..Default::default() - }; - let mut buf = Vec::::new(); - let err = ctx.write_to(&mut buf).unwrap_err(); - assert_eq!(err.kind(), std::io::ErrorKind::Other); + mod from_bytes { + use git_credentials::helper::Context; + + #[test] + fn empty_newlines_cause_skipping_remaining_input() { + let input = b"protocol=https +host=example.com\n +password=secr3t +username=bob"; + assert_eq!( + Context::from_bytes(input).unwrap(), + Context { + protocol: Some("https".into()), + host: Some("example.com".into()), + ..Default::default() + } + ) + } + + #[test] + fn unknown_field_names_are_skipped() { + let input = b"protocol=https +unknown=value +username=bob"; + assert_eq!( + Context::from_bytes(input).unwrap(), + Context { + protocol: Some("https".into()), + username: Some("bob".into()), + ..Default::default() + } + ) + } + + #[test] + fn null_bytes_when_decoding() { + let err = Context::from_bytes(b"url=https://foo\0").unwrap_err(); + assert!(matches!( + err, + git_credentials::helper::context::decode::Error::Encoding(_) + )); } } } + mod message { mod encode { use bstr::ByteSlice; From 9c6f024f838d866645937a67cd67dffb8be17259 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 19:40:50 +0800 Subject: [PATCH 035/236] change!: Use `helper::Context` in `helper::Action::Fill()` (#450) That way additional information, like from configuration, can be passed as well. --- git-credentials/src/helper/invoke.rs | 34 +++++--------- git-credentials/src/helper/mod.rs | 56 +++-------------------- git-credentials/src/lib.rs | 10 ++--- git-credentials/src/program.rs | 2 +- git-credentials/tests/helper/mod.rs | 67 ---------------------------- 5 files changed, 22 insertions(+), 147 deletions(-) diff --git a/git-credentials/src/helper/invoke.rs b/git-credentials/src/helper/invoke.rs index d7ca78cf067..1759ba3ccaf 100644 --- a/git-credentials/src/helper/invoke.rs +++ b/git-credentials/src/helper/invoke.rs @@ -15,26 +15,25 @@ pub type Result = std::result::Result, Error>; #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { - #[error("Cannot handle usernames or passwords in illformed UTF-8 encoding")] - IllformedUtf8InUsernameOrPassword, + #[error(transparent)] + Context(#[from] crate::helper::context::decode::Error), #[error("An IO error occurred while communicating to the credentials helper")] Io(#[from] std::io::Error), #[error("Could not find {name:?} in output of credentials helper")] - KeyNotFound { name: String }, + KeyNotFound { name: &'static str }, #[error(transparent)] CredentialsHelperFailed { source: std::io::Error }, } pub(crate) mod function { - use crate::helper::{invoke::Error, invoke::Outcome, invoke::Result, message, Action, Context, NextAction}; - use bstr::ByteVec; + use crate::helper::{invoke::Error, invoke::Outcome, invoke::Result, Action, Context, NextAction}; use std::io::Read; - impl Action<'_> { + impl Action { /// Send ourselves to the given `write` which is expected to be credentials-helper compatible pub fn send(&self, mut write: impl std::io::Write) -> std::io::Result<()> { match self { - Action::Fill(url) => message::encode(url, write), + Action::Fill(ctx) => ctx.write_to(write), Action::Approve(last) | Action::Reject(last) => { write.write_all(last)?; write.write_all(&[b'\n']) @@ -47,7 +46,7 @@ pub(crate) mod function { /// /// Usually the first call is performed with [`Action::Fill`] to obtain an identity, which subsequently can be used. /// On successful usage, use [`NextAction::approve()`], otherwise [`NextAction::reject()`]. - pub fn invoke(mut helper: impl crate::Helper, action: Action<'_>, _context: Context) -> Result { + pub fn invoke(mut helper: impl crate::Helper, action: Action) -> Result { let (stdin, stdout) = helper.start(&action)?; action.send(stdin)?; let stdout = stdout @@ -67,22 +66,11 @@ pub(crate) mod function { match stdout { None => Ok(None), Some(stdout) => { - let kvs = message::decode(stdout.as_slice())?; - let find = |name: &str| { - kvs.iter() - .find(|(k, _)| k == name) - .ok_or_else(|| Error::KeyNotFound { name: name.into() }) - .map(|(_, n)| n.to_vec()) - }; + let ctx = Context::from_bytes(stdout.as_slice())?; + let username = ctx.username.ok_or_else(|| Error::KeyNotFound { name: "username" })?; + let password = ctx.password.ok_or_else(|| Error::KeyNotFound { name: "password" })?; Ok(Some(Outcome { - identity: git_sec::identity::Account { - username: find("username")? - .into_string() - .map_err(|_| Error::IllformedUtf8InUsernameOrPassword)?, - password: find("password")? - .into_string() - .map_err(|_| Error::IllformedUtf8InUsernameOrPassword)?, - }, + identity: git_sec::identity::Account { username, password }, next: NextAction { previous_output: stdout.into(), }, diff --git a/git-credentials/src/helper/mod.rs b/git-credentials/src/helper/mod.rs index 3ada5508e1c..9e9298821a1 100644 --- a/git-credentials/src/helper/mod.rs +++ b/git-credentials/src/helper/mod.rs @@ -1,4 +1,4 @@ -use bstr::{BStr, BString}; +use bstr::BString; /// The kind of helper program to use. pub enum Kind { @@ -31,16 +31,16 @@ pub mod context; /// The action to perform by the credentials [helper][`crate::helper()`]. #[derive(Clone, Debug)] -pub enum Action<'a> { - /// Provide credentials using the given repository URL (as &str) as context and pre-parsed url information as seen in [`Context`]. - Fill(&'a BStr), +pub enum Action { + /// Provide credentials using the given repository context, which must include the repository url. + Fill(Context), /// Approve the credentials as identified by the previous input provided as `BString`, containing information from [`Context`]. Approve(BString), /// Reject the credentials as identified by the previous input provided as `BString`. containing information from [`Context`]. Reject(BString), } -impl<'a> Action<'a> { +impl Action { /// Returns true if this action expects output from the helper. pub fn expects_output(&self) -> bool { matches!(self, Action::Fill(_)) @@ -67,11 +67,11 @@ pub struct NextAction { impl NextAction { /// Approve the result of the previous [Action]. - pub fn approve(self) -> Action<'static> { + pub fn approve(self) -> Action { Action::Approve(self.previous_output) } /// Reject the result of the previous [Action]. - pub fn reject(self) -> Action<'static> { + pub fn reject(self) -> Action { Action::Reject(self.previous_output) } } @@ -79,45 +79,3 @@ impl NextAction { /// pub mod invoke; pub use invoke::function::invoke; - -/// -pub mod message { - use bstr::{BStr, BString, ByteSlice}; - - /// Encode `url` to `out` for consumption by a `git credentials` helper program. - pub fn encode(url: &BStr, mut out: impl std::io::Write) -> std::io::Result<()> { - validate(url)?; - writeln!(out, "url={}\n", url) - } - - fn validate(url: &BStr) -> std::io::Result<()> { - if url.contains(&0) || url.contains(&b'\n') { - return Err(std::io::Error::new( - std::io::ErrorKind::Other, - "token to encode must not contain newlines or null bytes", - )); - } - Ok(()) - } - - /// Decode all lines in `input` as key-value pairs produced by a `git credentials` helper program. - pub fn decode(mut input: impl std::io::Read) -> std::io::Result> { - let mut buf = Vec::::with_capacity(512); - input.read_to_end(&mut buf)?; - buf.lines() - .take_while(|line| !line.is_empty()) - .map(|line| { - let mut it = line.splitn(2, |b| *b == b'='); - match (it.next().and_then(|k| k.to_str().ok()), it.next().map(|v| v.as_bstr())) { - (Some(key), Some(value)) => validate(key.into()) - .and_then(|_| validate(value.into())) - .map(|_| (key.into(), value.into())), - _ => Err(std::io::Error::new( - std::io::ErrorKind::Other, - format!("Invalid format, expecting key=value, got {:?}", line.as_bstr()), - )), - } - }) - .collect::>>() - } -} diff --git a/git-credentials/src/lib.rs b/git-credentials/src/lib.rs index adb2486d405..d7a1eddec35 100644 --- a/git-credentials/src/lib.rs +++ b/git-credentials/src/lib.rs @@ -17,7 +17,7 @@ pub trait Helper { type Receive: std::io::Read; /// Start the helper and provide handles to send and receive from it. - fn start(&mut self, action: &helper::Action<'_>) -> std::io::Result<(Self::Send, Option)>; + fn start(&mut self, action: &helper::Action) -> std::io::Result<(Self::Send, Option)>; /// Stop the helper and provide a way to determine it's successful. fn finish(self) -> std::io::Result<()>; } @@ -38,10 +38,6 @@ pub mod helper; /// Call the `git` credentials helper program performing the given `action`, without any context from git configuration. /// /// See [`invoke()`] for a more flexible implementation. -pub fn helper(action: helper::Action<'_>) -> helper::invoke::Result { - helper::invoke( - Program::from_kind(helper::Kind::GitCredential), - action, - helper::Context::default(), - ) +pub fn helper(action: helper::Action) -> helper::invoke::Result { + helper::invoke(Program::from_kind(helper::Kind::GitCredential), action) } diff --git a/git-credentials/src/program.rs b/git-credentials/src/program.rs index d636abaf1d7..d5c51947d1c 100644 --- a/git-credentials/src/program.rs +++ b/git-credentials/src/program.rs @@ -12,7 +12,7 @@ impl Helper for Program { type Send = std::process::ChildStdin; type Receive = std::process::ChildStdout; - fn start(&mut self, action: &helper::Action<'_>) -> std::io::Result<(Self::Send, Option)> { + fn start(&mut self, action: &helper::Action) -> std::io::Result<(Self::Send, Option)> { match self { Program::Ready(kind) => { let (mut cmd, is_custom) = match kind { diff --git a/git-credentials/tests/helper/mod.rs b/git-credentials/tests/helper/mod.rs index b07bb55b255..d32cb0fc370 100644 --- a/git-credentials/tests/helper/mod.rs +++ b/git-credentials/tests/helper/mod.rs @@ -86,70 +86,3 @@ username=bob"; } } } - -mod message { - mod encode { - use bstr::ByteSlice; - use git_credentials::helper::message; - - #[test] - fn from_url() -> crate::Result { - let mut out = Vec::new(); - message::encode("https://github.com/byron/gitoxide".into(), &mut out)?; - assert_eq!(out.as_bstr(), b"url=https://github.com/byron/gitoxide\n\n".as_bstr()); - Ok(()) - } - - mod invalid { - use git_credentials::helper::message; - use std::io; - - #[test] - fn contains_null() { - assert_eq!( - message::encode("https://foo\u{0}".into(), Vec::new()) - .err() - .map(|e| e.kind()), - Some(io::ErrorKind::Other) - ); - } - #[test] - fn contains_newline() { - assert_eq!( - message::encode("https://foo\n".into(), Vec::new()) - .err() - .map(|e| e.kind()), - Some(io::ErrorKind::Other) - ); - } - } - } - - mod decode { - use git_credentials::helper::message; - - #[test] - fn typical_response() -> crate::Result { - assert_eq!( - message::decode( - "protocol=https -host=example.com -username=bob -password=secr3t\n\n -this=is-skipped-past-empty-line" - .as_bytes() - )?, - vec![ - ("protocol", "https"), - ("host", "example.com"), - ("username", "bob"), - ("password", "secr3t") - ] - .iter() - .map(|(k, v)| (k.to_string(), (*v).into())) - .collect::>() - ); - Ok(()) - } - } -} From 2073b583dc2bd83b800584edda6592bb71a01538 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 19:43:07 +0800 Subject: [PATCH 036/236] change!: rename `helper::Action` variants to 'Get', 'Store', 'Erase' (#450) It's more obvious what it does and is more typical for what credentials helpers do. --- git-credentials/src/helper/invoke.rs | 4 ++-- git-credentials/src/helper/mod.rs | 24 ++++++++++++------------ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/git-credentials/src/helper/invoke.rs b/git-credentials/src/helper/invoke.rs index 1759ba3ccaf..52e5747b2b0 100644 --- a/git-credentials/src/helper/invoke.rs +++ b/git-credentials/src/helper/invoke.rs @@ -33,8 +33,8 @@ pub(crate) mod function { /// Send ourselves to the given `write` which is expected to be credentials-helper compatible pub fn send(&self, mut write: impl std::io::Write) -> std::io::Result<()> { match self { - Action::Fill(ctx) => ctx.write_to(write), - Action::Approve(last) | Action::Reject(last) => { + Action::Get(ctx) => ctx.write_to(write), + Action::Store(last) | Action::Erase(last) => { write.write_all(last)?; write.write_all(&[b'\n']) } diff --git a/git-credentials/src/helper/mod.rs b/git-credentials/src/helper/mod.rs index 9e9298821a1..f9613886f24 100644 --- a/git-credentials/src/helper/mod.rs +++ b/git-credentials/src/helper/mod.rs @@ -33,28 +33,28 @@ pub mod context; #[derive(Clone, Debug)] pub enum Action { /// Provide credentials using the given repository context, which must include the repository url. - Fill(Context), + Get(Context), /// Approve the credentials as identified by the previous input provided as `BString`, containing information from [`Context`]. - Approve(BString), + Store(BString), /// Reject the credentials as identified by the previous input provided as `BString`. containing information from [`Context`]. - Reject(BString), + Erase(BString), } impl Action { /// Returns true if this action expects output from the helper. pub fn expects_output(&self) -> bool { - matches!(self, Action::Fill(_)) + matches!(self, Action::Get(_)) } /// The name of the argument to describe this action. If `is_custom` is true, the target program is /// a custom credentials helper, not a built-in one. pub fn as_helper_arg(&self, is_custom: bool) -> &str { match self { - Action::Fill(_) if is_custom => "get", - Action::Fill(_) => "fill", - Action::Approve(_) if is_custom => "store", - Action::Approve(_) => "approve", - Action::Reject(_) if is_custom => "erase", - Action::Reject(_) => "reject", + Action::Get(_) if is_custom => "get", + Action::Get(_) => "fill", + Action::Store(_) if is_custom => "store", + Action::Store(_) => "approve", + Action::Erase(_) if is_custom => "erase", + Action::Erase(_) => "reject", } } } @@ -68,11 +68,11 @@ pub struct NextAction { impl NextAction { /// Approve the result of the previous [Action]. pub fn approve(self) -> Action { - Action::Approve(self.previous_output) + Action::Store(self.previous_output) } /// Reject the result of the previous [Action]. pub fn reject(self) -> Action { - Action::Reject(self.previous_output) + Action::Erase(self.previous_output) } } From a253d30093122e37b5560ff86a7888f8062c7014 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 19:47:40 +0800 Subject: [PATCH 037/236] =?UTF-8?q?feat:=20add=20`helper::Action::get=5Ffo?= =?UTF-8?q?r=5Furl(=E2=80=A6)`=20(#450)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- git-credentials/src/helper/mod.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/git-credentials/src/helper/mod.rs b/git-credentials/src/helper/mod.rs index f9613886f24..60870c141d9 100644 --- a/git-credentials/src/helper/mod.rs +++ b/git-credentials/src/helper/mod.rs @@ -40,6 +40,18 @@ pub enum Action { Erase(BString), } +/// Initialization +impl Action { + /// Create a `Get` action with context containing the given URL + pub fn get_for_url(url: impl Into) -> Action { + Action::Get(Context { + url: Some(url.into()), + ..Default::default() + }) + } +} + +/// Access impl Action { /// Returns true if this action expects output from the helper. pub fn expects_output(&self) -> bool { From fe1a06da4f935c565505e6f004ce927ecb466998 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 19:48:03 +0800 Subject: [PATCH 038/236] adjust to changes in `git-credentials` (#450) --- git-protocol/src/fetch/handshake.rs | 4 ++-- git-protocol/src/fetch_fn.rs | 2 +- git-protocol/tests/fetch/mod.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/git-protocol/src/fetch/handshake.rs b/git-protocol/src/fetch/handshake.rs index 2f8ca4fc2d0..d2a67e63e77 100644 --- a/git-protocol/src/fetch/handshake.rs +++ b/git-protocol/src/fetch/handshake.rs @@ -54,7 +54,7 @@ pub(crate) mod function { progress: &mut impl Progress, ) -> Result where - AuthFn: FnMut(credentials::helper::Action<'_>) -> credentials::helper::invoke::Result, + AuthFn: FnMut(credentials::helper::Action) -> credentials::helper::invoke::Result, T: client::Transport, { let (server_protocol_version, refs, capabilities) = { @@ -80,7 +80,7 @@ pub(crate) mod function { let url = transport.to_url(); progress.set_name("authentication"); let credentials::helper::invoke::Outcome { identity, next } = - authenticate(credentials::helper::Action::Fill(url.as_str().into()))? + authenticate(credentials::helper::Action::get_for_url(url))? .expect("FILL provides an identity"); transport.set_identity(identity)?; progress.step(); diff --git a/git-protocol/src/fetch_fn.rs b/git-protocol/src/fetch_fn.rs index 9df3db13201..6e4c9762cd1 100644 --- a/git-protocol/src/fetch_fn.rs +++ b/git-protocol/src/fetch_fn.rs @@ -53,7 +53,7 @@ pub async fn fetch( fetch_mode: FetchConnection, ) -> Result<(), Error> where - F: FnMut(credentials::helper::Action<'_>) -> credentials::helper::invoke::Result, + F: FnMut(credentials::helper::Action) -> credentials::helper::invoke::Result, D: Delegate, T: client::Transport, { diff --git a/git-protocol/tests/fetch/mod.rs b/git-protocol/tests/fetch/mod.rs index 909ed627e95..4805dd948db 100644 --- a/git-protocol/tests/fetch/mod.rs +++ b/git-protocol/tests/fetch/mod.rs @@ -11,7 +11,7 @@ type Cursor = std::io::Cursor>; #[cfg(feature = "async-client")] type Cursor = futures_lite::io::Cursor>; -fn helper_unused(_action: git_credentials::helper::Action<'_>) -> git_credentials::helper::invoke::Result { +fn helper_unused(_action: git_credentials::helper::Action) -> git_credentials::helper::invoke::Result { panic!("Call to credentials helper is unexpected") } From 8317b4672c8cd38520ed90c42eaadd539ea4df66 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 19:48:38 +0800 Subject: [PATCH 039/236] thanks clippy --- git-credentials/src/helper/context.rs | 2 +- git-credentials/src/helper/invoke.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/git-credentials/src/helper/context.rs b/git-credentials/src/helper/context.rs index 1e260c9c75e..11e28a29fc0 100644 --- a/git-credentials/src/helper/context.rs +++ b/git-credentials/src/helper/context.rs @@ -25,7 +25,7 @@ mod serde { .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err))?; out.write_all(key.as_bytes())?; out.write_all(b"=")?; - out.write_all(&value)?; + out.write_all(value)?; out.write_all(b"\n")?; } } diff --git a/git-credentials/src/helper/invoke.rs b/git-credentials/src/helper/invoke.rs index 52e5747b2b0..5a3f40bbe51 100644 --- a/git-credentials/src/helper/invoke.rs +++ b/git-credentials/src/helper/invoke.rs @@ -67,8 +67,8 @@ pub(crate) mod function { None => Ok(None), Some(stdout) => { let ctx = Context::from_bytes(stdout.as_slice())?; - let username = ctx.username.ok_or_else(|| Error::KeyNotFound { name: "username" })?; - let password = ctx.password.ok_or_else(|| Error::KeyNotFound { name: "password" })?; + let username = ctx.username.ok_or(Error::KeyNotFound { name: "username" })?; + let password = ctx.password.ok_or(Error::KeyNotFound { name: "password" })?; Ok(Some(Outcome { identity: git_sec::identity::Account { username, password }, next: NextAction { From d9b4ba5a00c1c9f9c199ac218da76cb716896b75 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 19:51:04 +0800 Subject: [PATCH 040/236] fix docs (#450) --- git-credentials/src/helper/invoke.rs | 2 +- git-credentials/src/helper/mod.rs | 2 +- git-credentials/src/lib.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/git-credentials/src/helper/invoke.rs b/git-credentials/src/helper/invoke.rs index 5a3f40bbe51..9cf1d240145 100644 --- a/git-credentials/src/helper/invoke.rs +++ b/git-credentials/src/helper/invoke.rs @@ -44,7 +44,7 @@ pub(crate) mod function { /// Invoke the given `helper` with `action` in `context`. /// - /// Usually the first call is performed with [`Action::Fill`] to obtain an identity, which subsequently can be used. + /// Usually the first call is performed with [`Action::Get`] to obtain an identity, which subsequently can be used. /// On successful usage, use [`NextAction::approve()`], otherwise [`NextAction::reject()`]. pub fn invoke(mut helper: impl crate::Helper, action: Action) -> Result { let (stdin, stdout) = helper.start(&action)?; diff --git a/git-credentials/src/helper/mod.rs b/git-credentials/src/helper/mod.rs index 60870c141d9..65db79d8e2a 100644 --- a/git-credentials/src/helper/mod.rs +++ b/git-credentials/src/helper/mod.rs @@ -22,7 +22,7 @@ pub struct Context { /// The credential’s password, if we are asking it to be stored. pub password: Option, /// When this special attribute is read by git credential, the value is parsed as a URL and treated as if its constituent - /// parts were read (e.g., url=https://example.com would behave as if + /// parts were read (e.g., url= would behave as if /// protocol=https and host=example.com had been provided). This can help callers avoid parsing URLs themselves. pub url: Option, } diff --git a/git-credentials/src/lib.rs b/git-credentials/src/lib.rs index d7a1eddec35..9ee3002a666 100644 --- a/git-credentials/src/lib.rs +++ b/git-credentials/src/lib.rs @@ -37,7 +37,7 @@ pub mod helper; /// Call the `git` credentials helper program performing the given `action`, without any context from git configuration. /// -/// See [`invoke()`] for a more flexible implementation. +/// See [`invoke()`][helper::invoke()] for a more flexible implementation. pub fn helper(action: helper::Action) -> helper::invoke::Result { helper::invoke(Program::from_kind(helper::Kind::GitCredential), action) } From ddd53988a6d5da17fc65451a059bed1bfa2eb454 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 19:52:33 +0800 Subject: [PATCH 041/236] change!: rename `helper::NextAction` variants to `store` and `erase` (#450) --- git-credentials/src/helper/invoke.rs | 2 +- git-credentials/src/helper/mod.rs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/git-credentials/src/helper/invoke.rs b/git-credentials/src/helper/invoke.rs index 9cf1d240145..a77b0d83853 100644 --- a/git-credentials/src/helper/invoke.rs +++ b/git-credentials/src/helper/invoke.rs @@ -45,7 +45,7 @@ pub(crate) mod function { /// Invoke the given `helper` with `action` in `context`. /// /// Usually the first call is performed with [`Action::Get`] to obtain an identity, which subsequently can be used. - /// On successful usage, use [`NextAction::approve()`], otherwise [`NextAction::reject()`]. + /// On successful usage, use [`NextAction::store()`], otherwise [`NextAction::erase()`]. pub fn invoke(mut helper: impl crate::Helper, action: Action) -> Result { let (stdin, stdout) = helper.start(&action)?; action.send(stdin)?; diff --git a/git-credentials/src/helper/mod.rs b/git-credentials/src/helper/mod.rs index 65db79d8e2a..838a57839ec 100644 --- a/git-credentials/src/helper/mod.rs +++ b/git-credentials/src/helper/mod.rs @@ -71,19 +71,19 @@ impl Action { } } -/// A handle to [approve][NextAction::approve()] or [reject][NextAction::reject()] the outcome of the initial action. +/// A handle to [store][NextAction::store()] or [erase][NextAction::erase()] the outcome of the initial action. #[derive(Clone, Debug)] pub struct NextAction { previous_output: BString, } impl NextAction { - /// Approve the result of the previous [Action]. - pub fn approve(self) -> Action { + /// Approve the result of the previous [Action] and store for lookup. + pub fn store(self) -> Action { Action::Store(self.previous_output) } - /// Reject the result of the previous [Action]. - pub fn reject(self) -> Action { + /// Reject the result of the previous [Action] and erase it as to not be returned when being looked up. + pub fn erase(self) -> Action { Action::Erase(self.previous_output) } } From 33f1347f8c91abf4c226e66dd18e2299dd965995 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 19:52:57 +0800 Subject: [PATCH 042/236] adjust to changes in `git-credentials` (#450) --- git-protocol/src/fetch/handshake.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git-protocol/src/fetch/handshake.rs b/git-protocol/src/fetch/handshake.rs index d2a67e63e77..14ccd2d66df 100644 --- a/git-protocol/src/fetch/handshake.rs +++ b/git-protocol/src/fetch/handshake.rs @@ -87,12 +87,12 @@ pub(crate) mod function { progress.set_name("handshake (authenticated)"); match transport.handshake(Service::UploadPack, &extra_parameters).await { Ok(v) => { - authenticate(next.approve())?; + authenticate(next.store())?; Ok(v) } // Still no permission? Reject the credentials. Err(client::Error::Io { err }) if err.kind() == std::io::ErrorKind::PermissionDenied => { - authenticate(next.reject())?; + authenticate(next.erase())?; Err(client::Error::Io { err }) } // Otherwise, do nothing, as we don't know if it actually got to try the credentials. From cb9d32a3611463f983afea3b3ea875c33092207b Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 19:58:43 +0800 Subject: [PATCH 043/236] refactor (#450) In preparation for invoke tests and logic for the credentials helper itself. --- git-credentials/src/helper/invoke.rs | 7 +- git-credentials/tests/helper/context.rs | 86 ++++++++++++++++++++++++ git-credentials/tests/helper/mod.rs | 89 +------------------------ 3 files changed, 91 insertions(+), 91 deletions(-) create mode 100644 git-credentials/tests/helper/context.rs diff --git a/git-credentials/src/helper/invoke.rs b/git-credentials/src/helper/invoke.rs index a77b0d83853..a9929095340 100644 --- a/git-credentials/src/helper/invoke.rs +++ b/git-credentials/src/helper/invoke.rs @@ -4,7 +4,7 @@ use crate::helper::NextAction; pub struct Outcome { /// The obtained identity. pub identity: git_sec::identity::Account, - /// A handle to the action to perform next in another call to [`helper()`][crate::helper()]. + /// A handle to the action to perform next in another call to [`helper::invoke()`][crate::helper::invoke()]. pub next: NextAction, } @@ -44,8 +44,9 @@ pub(crate) mod function { /// Invoke the given `helper` with `action` in `context`. /// - /// Usually the first call is performed with [`Action::Get`] to obtain an identity, which subsequently can be used. - /// On successful usage, use [`NextAction::store()`], otherwise [`NextAction::erase()`]. + /// Usually the first call is performed with [`Action::Get`] to obtain `Some` identity, which subsequently can be used. + /// On successful usage, use [`NextAction::store()`], otherwise [`NextAction::erase()`], which returns `Ok(None)` as no outcome + /// is expected. pub fn invoke(mut helper: impl crate::Helper, action: Action) -> Result { let (stdin, stdout) = helper.start(&action)?; action.send(stdin)?; diff --git a/git-credentials/tests/helper/context.rs b/git-credentials/tests/helper/context.rs new file mode 100644 index 00000000000..628e90d69bf --- /dev/null +++ b/git-credentials/tests/helper/context.rs @@ -0,0 +1,86 @@ +use git_credentials::helper::Context; + +#[test] +fn encode_decode_roundtrip() { + for ctx in [ + Context { + protocol: Some("https".into()), + host: Some("github.com".into()), + path: Some("byron/gitoxide".into()), + username: Some("user".into()), + password: Some("pass".into()), + url: Some("https://github.com/byron/gitoxide".into()), + }, + Context::default(), + Context { + url: Some("/path/to/repo".into()), + ..Context::default() + }, + ] { + let mut buf = Vec::::new(); + ctx.write_to(&mut buf).unwrap(); + let actual = Context::from_bytes(&buf).unwrap(); + assert_eq!(actual, ctx, "ctx should encode itself losslessly"); + } +} + +mod write_to { + use git_credentials::helper::Context; + + #[test] + fn null_bytes_and_newlines_are_invalid() { + for input in [&b"https://foo\0"[..], b"https://foo\n"] { + let ctx = Context { + url: Some(input.into()), + ..Default::default() + }; + let mut buf = Vec::::new(); + let err = ctx.write_to(&mut buf).unwrap_err(); + assert_eq!(err.kind(), std::io::ErrorKind::Other); + } + } +} + +mod from_bytes { + use git_credentials::helper::Context; + + #[test] + fn empty_newlines_cause_skipping_remaining_input() { + let input = b"protocol=https +host=example.com\n +password=secr3t +username=bob"; + assert_eq!( + Context::from_bytes(input).unwrap(), + Context { + protocol: Some("https".into()), + host: Some("example.com".into()), + ..Default::default() + } + ) + } + + #[test] + fn unknown_field_names_are_skipped() { + let input = b"protocol=https +unknown=value +username=bob"; + assert_eq!( + Context::from_bytes(input).unwrap(), + Context { + protocol: Some("https".into()), + username: Some("bob".into()), + ..Default::default() + } + ) + } + + #[test] + fn null_bytes_when_decoding() { + let err = Context::from_bytes(b"url=https://foo\0").unwrap_err(); + assert!(matches!( + err, + git_credentials::helper::context::decode::Error::Encoding(_) + )); + } +} diff --git a/git-credentials/tests/helper/mod.rs b/git-credentials/tests/helper/mod.rs index d32cb0fc370..3d9885db80d 100644 --- a/git-credentials/tests/helper/mod.rs +++ b/git-credentials/tests/helper/mod.rs @@ -1,88 +1 @@ -mod context { - use git_credentials::helper::Context; - - #[test] - fn encode_decode_roundtrip() { - for ctx in [ - Context { - protocol: Some("https".into()), - host: Some("github.com".into()), - path: Some("byron/gitoxide".into()), - username: Some("user".into()), - password: Some("pass".into()), - url: Some("https://github.com/byron/gitoxide".into()), - }, - Context::default(), - Context { - url: Some("/path/to/repo".into()), - ..Context::default() - }, - ] { - let mut buf = Vec::::new(); - ctx.write_to(&mut buf).unwrap(); - let actual = Context::from_bytes(&buf).unwrap(); - assert_eq!(actual, ctx, "ctx should encode itself losslessly"); - } - } - - mod write_to { - use git_credentials::helper::Context; - - #[test] - fn null_bytes_and_newlines_are_invalid() { - for input in [&b"https://foo\0"[..], b"https://foo\n"] { - let ctx = Context { - url: Some(input.into()), - ..Default::default() - }; - let mut buf = Vec::::new(); - let err = ctx.write_to(&mut buf).unwrap_err(); - assert_eq!(err.kind(), std::io::ErrorKind::Other); - } - } - } - - mod from_bytes { - use git_credentials::helper::Context; - - #[test] - fn empty_newlines_cause_skipping_remaining_input() { - let input = b"protocol=https -host=example.com\n -password=secr3t -username=bob"; - assert_eq!( - Context::from_bytes(input).unwrap(), - Context { - protocol: Some("https".into()), - host: Some("example.com".into()), - ..Default::default() - } - ) - } - - #[test] - fn unknown_field_names_are_skipped() { - let input = b"protocol=https -unknown=value -username=bob"; - assert_eq!( - Context::from_bytes(input).unwrap(), - Context { - protocol: Some("https".into()), - username: Some("bob".into()), - ..Default::default() - } - ) - } - - #[test] - fn null_bytes_when_decoding() { - let err = Context::from_bytes(b"url=https://foo\0").unwrap_err(); - assert!(matches!( - err, - git_credentials::helper::context::decode::Error::Encoding(_) - )); - } - } -} +mod context; From c48eb390a2f95954f542992806d4e8667ee97981 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 20:40:49 +0800 Subject: [PATCH 044/236] sketch for helper::invoke (get) test (#450) --- Cargo.lock | 1 + git-credentials/Cargo.toml | 4 +- git-credentials/src/helper/mod.rs | 9 ++++- git-credentials/tests/helper/invoke.rs | 54 ++++++++++++++++++++++++++ git-credentials/tests/helper/mod.rs | 1 + 5 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 git-credentials/tests/helper/invoke.rs diff --git a/Cargo.lock b/Cargo.lock index 5e5848eceaa..0cdfef8cbe4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1167,6 +1167,7 @@ version = "0.4.0" dependencies = [ "bstr", "document-features", + "git-features", "git-sec", "git-testtools", "serde", diff --git a/git-credentials/Cargo.toml b/git-credentials/Cargo.toml index b1cbd58d151..1c58c40a893 100644 --- a/git-credentials/Cargo.toml +++ b/git-credentials/Cargo.toml @@ -23,7 +23,9 @@ bstr = { version = "0.2.13", default-features = false, features = ["std"]} document-features = { version = "0.2.1", optional = true } [dev-dependencies] -git-testtools = { path = "../tests/tools"} +git-testtools = { path = "../tests/tools" } +git-features = { path = "../git-features", features = ["io-pipe"] } +git-sec = { path = "../git-sec" } [package.metadata.docs.rs] all-features = true diff --git a/git-credentials/src/helper/mod.rs b/git-credentials/src/helper/mod.rs index 838a57839ec..c13fb5db4a2 100644 --- a/git-credentials/src/helper/mod.rs +++ b/git-credentials/src/helper/mod.rs @@ -1,4 +1,4 @@ -use bstr::BString; +use bstr::{BStr, BString, ByteSlice}; /// The kind of helper program to use. pub enum Kind { @@ -53,6 +53,13 @@ impl Action { /// Access impl Action { + /// Return the payload of store or erase actions. + pub fn payload(&self) -> Option<&BStr> { + match self { + Action::Get(_) => None, + Action::Store(p) | Action::Erase(p) => Some(p.as_bstr()), + } + } /// Returns true if this action expects output from the helper. pub fn expects_output(&self) -> bool { matches!(self, Action::Get(_)) diff --git a/git-credentials/tests/helper/invoke.rs b/git-credentials/tests/helper/invoke.rs new file mode 100644 index 00000000000..1be7edcab8c --- /dev/null +++ b/git-credentials/tests/helper/invoke.rs @@ -0,0 +1,54 @@ +use crate::helper::invoke::util::MockHelper; +use git_credentials::helper::Action; + +#[test] +#[ignore] +fn get() { + let mut helper = MockHelper::default(); + let outcome = + git_credentials::helper::invoke(&mut helper, Action::get_for_url("https://github.com/byron/gitoxide")) + .unwrap() + .expect("mock provides credentials"); + assert_eq!( + outcome.identity, + git_sec::identity::Account { + username: "user".into(), + password: "pass".into() + } + ); + assert_eq!( + outcome.next.store().payload().unwrap(), + "url=https://github.com/byron/gitoxide\nusername=user\npass=pass\n" + ); +} + +mod util { + use git_credentials::helper::Action; + + #[derive(Default)] + pub struct MockHelper { + handle: Option>, + } + + impl git_credentials::Helper for &mut MockHelper { + type Send = git_features::io::pipe::Writer; + type Receive = git_features::io::pipe::Reader; + + fn start(&mut self, action: &Action) -> std::io::Result<(Self::Send, Option)> { + let ((them_send, us_receive), output) = match action { + Action::Get(_) => (git_features::io::pipe::unidirectional(128), None), + Action::Erase(_) | Action::Store(_) => ( + git_features::io::pipe::unidirectional(128), + git_features::io::pipe::unidirectional(128).into(), + ), + }; + let (us_send, them_receive) = output.map(|(tx, rx)| (Some(tx), Some(rx))).unwrap_or_default(); + self.handle = std::thread::spawn(move || todo!("thread main")).into(); + Ok((them_send, them_receive)) + } + + fn finish(self) -> std::io::Result<()> { + Ok(self.handle.take().expect("started").join().unwrap()) + } + } +} diff --git a/git-credentials/tests/helper/mod.rs b/git-credentials/tests/helper/mod.rs index 3d9885db80d..a8de271eeda 100644 --- a/git-credentials/tests/helper/mod.rs +++ b/git-credentials/tests/helper/mod.rs @@ -1 +1,2 @@ mod context; +mod invoke; From 4b7d0b6d2c43cac9823885bc69510cc4bb6a3f00 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 20:44:34 +0800 Subject: [PATCH 045/236] change!: move `helper::(Next)Action` into `helper::invoke::` module (#450) These are only relevant for invoke, hence the change. --- git-credentials/src/helper/invoke.rs | 73 +++++++++++++++++++++++++- git-credentials/src/helper/mod.rs | 69 +----------------------- git-credentials/src/lib.rs | 4 +- git-credentials/src/program.rs | 2 +- git-credentials/tests/helper/invoke.rs | 14 ++--- 5 files changed, 83 insertions(+), 79 deletions(-) diff --git a/git-credentials/src/helper/invoke.rs b/git-credentials/src/helper/invoke.rs index a9929095340..29d28a983d9 100644 --- a/git-credentials/src/helper/invoke.rs +++ b/git-credentials/src/helper/invoke.rs @@ -1,4 +1,5 @@ -use crate::helper::NextAction; +use crate::helper::Context; +use bstr::{BStr, BString}; /// The outcome of the credentials [`helper`][crate::helper()]. pub struct Outcome { @@ -25,8 +26,76 @@ pub enum Error { CredentialsHelperFailed { source: std::io::Error }, } +/// The action to perform by the credentials [helper][`crate::helper()`]. +#[derive(Clone, Debug)] +pub enum Action { + /// Provide credentials using the given repository context, which must include the repository url. + Get(Context), + /// Approve the credentials as identified by the previous input provided as `BString`, containing information from [`Context`]. + Store(BString), + /// Reject the credentials as identified by the previous input provided as `BString`. containing information from [`Context`]. + Erase(BString), +} + +/// Initialization +impl Action { + /// Create a `Get` action with context containing the given URL + pub fn get_for_url(url: impl Into) -> Action { + Action::Get(Context { + url: Some(url.into()), + ..Default::default() + }) + } +} + +/// Access +impl Action { + /// Return the payload of store or erase actions. + pub fn payload(&self) -> Option<&BStr> { + use bstr::ByteSlice; + match self { + Action::Get(_) => None, + Action::Store(p) | Action::Erase(p) => Some(p.as_bstr()), + } + } + /// Returns true if this action expects output from the helper. + pub fn expects_output(&self) -> bool { + matches!(self, Action::Get(_)) + } + /// The name of the argument to describe this action. If `is_custom` is true, the target program is + /// a custom credentials helper, not a built-in one. + pub fn as_helper_arg(&self, is_custom: bool) -> &str { + match self { + Action::Get(_) if is_custom => "get", + Action::Get(_) => "fill", + Action::Store(_) if is_custom => "store", + Action::Store(_) => "approve", + Action::Erase(_) if is_custom => "erase", + Action::Erase(_) => "reject", + } + } +} + +/// A handle to [store][NextAction::store()] or [erase][NextAction::erase()] the outcome of the initial action. +#[derive(Clone, Debug)] +pub struct NextAction { + previous_output: BString, +} + +impl NextAction { + /// Approve the result of the previous [Action] and store for lookup. + pub fn store(self) -> Action { + Action::Store(self.previous_output) + } + /// Reject the result of the previous [Action] and erase it as to not be returned when being looked up. + pub fn erase(self) -> Action { + Action::Erase(self.previous_output) + } +} + pub(crate) mod function { - use crate::helper::{invoke::Error, invoke::Outcome, invoke::Result, Action, Context, NextAction}; + use crate::helper::invoke::{Action, NextAction}; + use crate::helper::{invoke::Error, invoke::Outcome, invoke::Result, Context}; use std::io::Read; impl Action { diff --git a/git-credentials/src/helper/mod.rs b/git-credentials/src/helper/mod.rs index c13fb5db4a2..6cc215a5529 100644 --- a/git-credentials/src/helper/mod.rs +++ b/git-credentials/src/helper/mod.rs @@ -1,4 +1,4 @@ -use bstr::{BStr, BString, ByteSlice}; +use bstr::BString; /// The kind of helper program to use. pub enum Kind { @@ -7,7 +7,6 @@ pub enum Kind { } /// Additional context to be passed to the credentials helper. -// TODO: fill in what's needed per configuration #[derive(Debug, Default, Clone, Eq, PartialEq)] pub struct Context { /// The protocol over which the credential will be used (e.g., https). @@ -29,72 +28,6 @@ pub struct Context { /// pub mod context; -/// The action to perform by the credentials [helper][`crate::helper()`]. -#[derive(Clone, Debug)] -pub enum Action { - /// Provide credentials using the given repository context, which must include the repository url. - Get(Context), - /// Approve the credentials as identified by the previous input provided as `BString`, containing information from [`Context`]. - Store(BString), - /// Reject the credentials as identified by the previous input provided as `BString`. containing information from [`Context`]. - Erase(BString), -} - -/// Initialization -impl Action { - /// Create a `Get` action with context containing the given URL - pub fn get_for_url(url: impl Into) -> Action { - Action::Get(Context { - url: Some(url.into()), - ..Default::default() - }) - } -} - -/// Access -impl Action { - /// Return the payload of store or erase actions. - pub fn payload(&self) -> Option<&BStr> { - match self { - Action::Get(_) => None, - Action::Store(p) | Action::Erase(p) => Some(p.as_bstr()), - } - } - /// Returns true if this action expects output from the helper. - pub fn expects_output(&self) -> bool { - matches!(self, Action::Get(_)) - } - /// The name of the argument to describe this action. If `is_custom` is true, the target program is - /// a custom credentials helper, not a built-in one. - pub fn as_helper_arg(&self, is_custom: bool) -> &str { - match self { - Action::Get(_) if is_custom => "get", - Action::Get(_) => "fill", - Action::Store(_) if is_custom => "store", - Action::Store(_) => "approve", - Action::Erase(_) if is_custom => "erase", - Action::Erase(_) => "reject", - } - } -} - -/// A handle to [store][NextAction::store()] or [erase][NextAction::erase()] the outcome of the initial action. -#[derive(Clone, Debug)] -pub struct NextAction { - previous_output: BString, -} - -impl NextAction { - /// Approve the result of the previous [Action] and store for lookup. - pub fn store(self) -> Action { - Action::Store(self.previous_output) - } - /// Reject the result of the previous [Action] and erase it as to not be returned when being looked up. - pub fn erase(self) -> Action { - Action::Erase(self.previous_output) - } -} - /// pub mod invoke; pub use invoke::function::invoke; diff --git a/git-credentials/src/lib.rs b/git-credentials/src/lib.rs index 9ee3002a666..2638bef868a 100644 --- a/git-credentials/src/lib.rs +++ b/git-credentials/src/lib.rs @@ -17,7 +17,7 @@ pub trait Helper { type Receive: std::io::Read; /// Start the helper and provide handles to send and receive from it. - fn start(&mut self, action: &helper::Action) -> std::io::Result<(Self::Send, Option)>; + fn start(&mut self, action: &helper::invoke::Action) -> std::io::Result<(Self::Send, Option)>; /// Stop the helper and provide a way to determine it's successful. fn finish(self) -> std::io::Result<()>; } @@ -38,6 +38,6 @@ pub mod helper; /// Call the `git` credentials helper program performing the given `action`, without any context from git configuration. /// /// See [`invoke()`][helper::invoke()] for a more flexible implementation. -pub fn helper(action: helper::Action) -> helper::invoke::Result { +pub fn helper(action: helper::invoke::Action) -> helper::invoke::Result { helper::invoke(Program::from_kind(helper::Kind::GitCredential), action) } diff --git a/git-credentials/src/program.rs b/git-credentials/src/program.rs index d5c51947d1c..d1141381317 100644 --- a/git-credentials/src/program.rs +++ b/git-credentials/src/program.rs @@ -12,7 +12,7 @@ impl Helper for Program { type Send = std::process::ChildStdin; type Receive = std::process::ChildStdout; - fn start(&mut self, action: &helper::Action) -> std::io::Result<(Self::Send, Option)> { + fn start(&mut self, action: &helper::invoke::Action) -> std::io::Result<(Self::Send, Option)> { match self { Program::Ready(kind) => { let (mut cmd, is_custom) = match kind { diff --git a/git-credentials/tests/helper/invoke.rs b/git-credentials/tests/helper/invoke.rs index 1be7edcab8c..a1f866f388a 100644 --- a/git-credentials/tests/helper/invoke.rs +++ b/git-credentials/tests/helper/invoke.rs @@ -1,14 +1,16 @@ use crate::helper::invoke::util::MockHelper; -use git_credentials::helper::Action; +use git_credentials::helper::invoke; #[test] #[ignore] fn get() { let mut helper = MockHelper::default(); - let outcome = - git_credentials::helper::invoke(&mut helper, Action::get_for_url("https://github.com/byron/gitoxide")) - .unwrap() - .expect("mock provides credentials"); + let outcome = git_credentials::helper::invoke( + &mut helper, + invoke::Action::get_for_url("https://github.com/byron/gitoxide"), + ) + .unwrap() + .expect("mock provides credentials"); assert_eq!( outcome.identity, git_sec::identity::Account { @@ -23,7 +25,7 @@ fn get() { } mod util { - use git_credentials::helper::Action; + use git_credentials::helper::invoke::Action; #[derive(Default)] pub struct MockHelper { From eaff67c14366f149ccca346acb46af12531a24e6 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 21:28:41 +0800 Subject: [PATCH 046/236] feat: `helper::main` to easily create credential helper implementations (#450) --- git-credentials/src/helper/invoke.rs | 5 +- git-credentials/src/helper/main.rs | 92 ++++++++++++++++++++++++++ git-credentials/src/helper/mod.rs | 3 + git-credentials/src/lib.rs | 1 + git-credentials/tests/helper/invoke.rs | 63 ++++++++++++++---- git-protocol/src/fetch/handshake.rs | 4 +- git-protocol/src/fetch_fn.rs | 2 +- git-protocol/tests/fetch/mod.rs | 2 +- 8 files changed, 154 insertions(+), 18 deletions(-) create mode 100644 git-credentials/src/helper/main.rs diff --git a/git-credentials/src/helper/invoke.rs b/git-credentials/src/helper/invoke.rs index 29d28a983d9..18410b6de04 100644 --- a/git-credentials/src/helper/invoke.rs +++ b/git-credentials/src/helper/invoke.rs @@ -118,6 +118,9 @@ pub(crate) mod function { /// is expected. pub fn invoke(mut helper: impl crate::Helper, action: Action) -> Result { let (stdin, stdout) = helper.start(&action)?; + if let (Action::Get(_), None) = (&action, &stdout) { + panic!("BUG: `Helper` impls must return an output handle to read output from if Action::Get is provided") + } action.send(stdin)?; let stdout = stdout .map(|mut stdout| { @@ -133,7 +136,7 @@ pub(crate) mod function { } })?; - match stdout { + match matches!(action, Action::Get(_)).then(|| stdout).flatten() { None => Ok(None), Some(stdout) => { let ctx = Context::from_bytes(stdout.as_slice())?; diff --git a/git-credentials/src/helper/main.rs b/git-credentials/src/helper/main.rs new file mode 100644 index 00000000000..8e8a93f7f6d --- /dev/null +++ b/git-credentials/src/helper/main.rs @@ -0,0 +1,92 @@ +#![allow(missing_docs)] +use bstr::BString; +use std::convert::TryFrom; +use std::ffi::OsString; + +#[derive(Debug, Copy, Clone)] +pub enum Action { + Get, + Store, + Erase, +} + +impl TryFrom for Action { + type Error = Error; + + fn try_from(value: OsString) -> Result { + Ok(match value.to_str() { + Some("fill") | Some("get") => Action::Get, + Some("approve") | Some("store") => Action::Store, + Some("reject") | Some("erase") => Action::Erase, + _ => return Err(Error::ActionInvalid { name: value }), + }) + } +} + +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error("Action named {name:?} is invalid, need 'get', 'store', 'erase' or 'fill', 'approve', 'reject'")] + ActionInvalid { name: OsString }, + #[error("The first argument must be the action to perform")] + ActionMissing, + #[error(transparent)] + Helper { + source: Box, + }, + #[error(transparent)] + Io(#[from] std::io::Error), + #[error(transparent)] + Context(#[from] crate::helper::context::decode::Error), + #[error("Credentials for {url:?} could not be obtained")] + CredentialsMissing { url: BString }, + #[error("The url wasn't provided in input - the git credentials protocol mandates this")] + UrlMissing, +} + +pub(crate) mod function { + use crate::helper::main::{Action, Error}; + use crate::helper::Context; + use std::convert::TryInto; + use std::ffi::OsString; + + /// Invoke a custom credentials helper which receives program `args`, with the first argument being the + /// action to perform (as opposed to the program name). + /// Then read context information from `stdin` and if the action is `Action::Get`, then write the result to `stdout`. + /// `credentials` is the API version of such call, where`Ok(Some(context))` returns credentials, and `Ok(None)` indicates + /// no credentials could be found for `url`, which is always set when called. + /// + /// Call this function from a programs `main`, passing `std::env::args_os()`, `stdin()` and `stdout` accordingly, along with + /// your own helper implementation. + pub fn main( + args: impl IntoIterator, + mut stdin: impl std::io::Read, + stdout: impl std::io::Write, + credentials: CredentialsFn, + ) -> Result<(), Error> + where + CredentialsFn: FnOnce(Action, Context) -> Result, E>, + E: std::error::Error + Send + Sync + 'static, + { + let action: Action = args.into_iter().next().ok_or(Error::ActionMissing)?.try_into()?; + let mut buf = Vec::::with_capacity(512); + stdin.read_to_end(&mut buf)?; + let ctx = Context::from_bytes(&buf)?; + if ctx.url.is_none() { + return Err(Error::UrlMissing); + } + let res = credentials(action, ctx).map_err(|err| Error::Helper { source: Box::new(err) })?; + match (action, res) { + (Action::Get, None) => { + return Err(Error::CredentialsMissing { + url: Context::from_bytes(&buf)?.url.expect("present and checked above"), + }) + } + (Action::Get, Some(ctx)) => ctx.write_to(stdout)?, + (Action::Erase | Action::Store, None) => {} + (Action::Erase | Action::Store, Some(_)) => { + panic!("BUG: credentials helper must not return context for erase or store actions") + } + } + Ok(()) + } +} diff --git a/git-credentials/src/helper/mod.rs b/git-credentials/src/helper/mod.rs index 6cc215a5529..5454f0ffb31 100644 --- a/git-credentials/src/helper/mod.rs +++ b/git-credentials/src/helper/mod.rs @@ -31,3 +31,6 @@ pub mod context; /// pub mod invoke; pub use invoke::function::invoke; + +pub mod main; +pub use main::function::main; diff --git a/git-credentials/src/lib.rs b/git-credentials/src/lib.rs index 2638bef868a..e1b91b28eb7 100644 --- a/git-credentials/src/lib.rs +++ b/git-credentials/src/lib.rs @@ -17,6 +17,7 @@ pub trait Helper { type Receive: std::io::Read; /// Start the helper and provide handles to send and receive from it. + /// If `Action::Get` is provided, it's valid to return `None` for the receive half. fn start(&mut self, action: &helper::invoke::Action) -> std::io::Result<(Self::Send, Option)>; /// Stop the helper and provide a way to determine it's successful. fn finish(self) -> std::io::Result<()>; diff --git a/git-credentials/tests/helper/invoke.rs b/git-credentials/tests/helper/invoke.rs index a1f866f388a..cb5c20b3d93 100644 --- a/git-credentials/tests/helper/invoke.rs +++ b/git-credentials/tests/helper/invoke.rs @@ -1,8 +1,8 @@ use crate::helper::invoke::util::MockHelper; -use git_credentials::helper::invoke; +use bstr::BString; +use git_credentials::helper::{invoke, Context}; #[test] -#[ignore] fn get() { let mut helper = MockHelper::default(); let outcome = git_credentials::helper::invoke( @@ -20,12 +20,34 @@ fn get() { ); assert_eq!( outcome.next.store().payload().unwrap(), - "url=https://github.com/byron/gitoxide\nusername=user\npass=pass\n" + "url=https://github.com/byron/gitoxide\nusername=user\npassword=pass\n" ); } +#[test] +fn store_and_reject() { + let mut helper = MockHelper::default(); + let ctx = Context { + url: Some("https://github.com/byron/gitoxide".into()), + ..Default::default() + }; + let ctxbuf = || -> BString { + let mut buf = Vec::::new(); + ctx.write_to(&mut buf).expect("cannot fail"); + buf.into() + }; + for action in [invoke::Action::Store(ctxbuf()), invoke::Action::Erase(ctxbuf())] { + let outcome = git_credentials::helper::invoke(&mut helper, action).unwrap(); + assert!( + outcome.is_none(), + "store and erase have no outcome, they just shouln't fail" + ); + } +} + mod util { use git_credentials::helper::invoke::Action; + use git_credentials::helper::{main, Context}; #[derive(Default)] pub struct MockHelper { @@ -37,16 +59,31 @@ mod util { type Receive = git_features::io::pipe::Reader; fn start(&mut self, action: &Action) -> std::io::Result<(Self::Send, Option)> { - let ((them_send, us_receive), output) = match action { - Action::Get(_) => (git_features::io::pipe::unidirectional(128), None), - Action::Erase(_) | Action::Store(_) => ( - git_features::io::pipe::unidirectional(128), - git_features::io::pipe::unidirectional(128).into(), - ), - }; - let (us_send, them_receive) = output.map(|(tx, rx)| (Some(tx), Some(rx))).unwrap_or_default(); - self.handle = std::thread::spawn(move || todo!("thread main")).into(); - Ok((them_send, them_receive)) + let ((them_send, us_receive), (us_send, them_receive)) = ( + git_features::io::pipe::unidirectional(128), + git_features::io::pipe::unidirectional(128), + ); + let action_name = action.as_helper_arg(true).into(); + self.handle = std::thread::spawn(move || { + git_credentials::helper::main( + Some(action_name), + us_receive, + us_send, + |action, context| -> std::io::Result<_> { + match action { + main::Action::Get => Ok(Some(Context { + username: Some("user".into()), + password: Some("pass".into()), + ..context + })), + main::Action::Store | main::Action::Erase => Ok(None), + } + }, + ) + .expect("cannot fail") + }) + .into(); + Ok((them_send, them_receive.into())) } fn finish(self) -> std::io::Result<()> { diff --git a/git-protocol/src/fetch/handshake.rs b/git-protocol/src/fetch/handshake.rs index 14ccd2d66df..97d4089633f 100644 --- a/git-protocol/src/fetch/handshake.rs +++ b/git-protocol/src/fetch/handshake.rs @@ -54,7 +54,7 @@ pub(crate) mod function { progress: &mut impl Progress, ) -> Result where - AuthFn: FnMut(credentials::helper::Action) -> credentials::helper::invoke::Result, + AuthFn: FnMut(credentials::helper::invoke::Action) -> credentials::helper::invoke::Result, T: client::Transport, { let (server_protocol_version, refs, capabilities) = { @@ -80,7 +80,7 @@ pub(crate) mod function { let url = transport.to_url(); progress.set_name("authentication"); let credentials::helper::invoke::Outcome { identity, next } = - authenticate(credentials::helper::Action::get_for_url(url))? + authenticate(credentials::helper::invoke::Action::get_for_url(url))? .expect("FILL provides an identity"); transport.set_identity(identity)?; progress.step(); diff --git a/git-protocol/src/fetch_fn.rs b/git-protocol/src/fetch_fn.rs index 6e4c9762cd1..2e38cb274ae 100644 --- a/git-protocol/src/fetch_fn.rs +++ b/git-protocol/src/fetch_fn.rs @@ -53,7 +53,7 @@ pub async fn fetch( fetch_mode: FetchConnection, ) -> Result<(), Error> where - F: FnMut(credentials::helper::Action) -> credentials::helper::invoke::Result, + F: FnMut(credentials::helper::invoke::Action) -> credentials::helper::invoke::Result, D: Delegate, T: client::Transport, { diff --git a/git-protocol/tests/fetch/mod.rs b/git-protocol/tests/fetch/mod.rs index 4805dd948db..6c13edd9d70 100644 --- a/git-protocol/tests/fetch/mod.rs +++ b/git-protocol/tests/fetch/mod.rs @@ -11,7 +11,7 @@ type Cursor = std::io::Cursor>; #[cfg(feature = "async-client")] type Cursor = futures_lite::io::Cursor>; -fn helper_unused(_action: git_credentials::helper::Action) -> git_credentials::helper::invoke::Result { +fn helper_unused(_action: git_credentials::helper::invoke::Action) -> git_credentials::helper::invoke::Result { panic!("Call to credentials helper is unexpected") } From a360594fac3102cd48aac0039efbe71693c5fa25 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 21:31:15 +0800 Subject: [PATCH 047/236] add docs (#450) --- git-credentials/src/helper/main.rs | 7 ++++++- git-credentials/src/helper/mod.rs | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/git-credentials/src/helper/main.rs b/git-credentials/src/helper/main.rs index 8e8a93f7f6d..d7833ec6c44 100644 --- a/git-credentials/src/helper/main.rs +++ b/git-credentials/src/helper/main.rs @@ -1,12 +1,15 @@ -#![allow(missing_docs)] use bstr::BString; use std::convert::TryFrom; use std::ffi::OsString; +/// The action passed to the credential helper implementation in [`main()`][crate::helper::main()]. #[derive(Debug, Copy, Clone)] pub enum Action { + /// Get credentials for a url. Get, + /// Store credentials provided in the given context. Store, + /// Erase credentials identified by the given context. Erase, } @@ -23,7 +26,9 @@ impl TryFrom for Action { } } +/// The error of [`main()`][crate::helper::main()]. #[derive(Debug, thiserror::Error)] +#[allow(missing_docs)] pub enum Error { #[error("Action named {name:?} is invalid, need 'get', 'store', 'erase' or 'fill', 'approve', 'reject'")] ActionInvalid { name: OsString }, diff --git a/git-credentials/src/helper/mod.rs b/git-credentials/src/helper/mod.rs index 5454f0ffb31..112a9c10a09 100644 --- a/git-credentials/src/helper/mod.rs +++ b/git-credentials/src/helper/mod.rs @@ -32,5 +32,6 @@ pub mod context; pub mod invoke; pub use invoke::function::invoke; +/// pub mod main; pub use main::function::main; From b1d528ae60001ae51dd89b29c26ea505eacbef45 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 21:41:01 +0800 Subject: [PATCH 048/236] feat: an example implementing a custom credential helper program (#450) --- git-credentials/examples/custom-helper.rs | 24 +++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 git-credentials/examples/custom-helper.rs diff --git a/git-credentials/examples/custom-helper.rs b/git-credentials/examples/custom-helper.rs new file mode 100644 index 00000000000..6f6d9948781 --- /dev/null +++ b/git-credentials/examples/custom-helper.rs @@ -0,0 +1,24 @@ +use git_credentials::helper; + +/// Run like this `echo url=https://example.com | cargo run --example custom-helper -- get` +pub fn main() -> Result<(), git_credentials::helper::main::Error> { + git_credentials::helper::main( + std::env::args_os().skip(1), + std::io::stdin(), + std::io::stdout(), + |action, credentials| -> std::io::Result<_> { + match action { + helper::main::Action::Get => Ok(Some(helper::Context { + username: Some("user".into()), + password: Some("pass".into()), + ..credentials + })), + helper::main::Action::Erase => Err(std::io::Error::new( + std::io::ErrorKind::Other, + "Refusing to delete credentials for demo purposes", + )), + helper::main::Action::Store => Ok(None), + } + }, + ) +} From 9b8a6d6afeab13968dea61619b1e742e93f60fab Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 24 Aug 2022 21:41:47 +0800 Subject: [PATCH 049/236] thanks clippy --- etc/check-package-size.sh | 2 +- git-credentials/tests/helper/invoke.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/etc/check-package-size.sh b/etc/check-package-size.sh index c2adf00df5b..d602e856cf9 100755 --- a/etc/check-package-size.sh +++ b/etc/check-package-size.sh @@ -46,7 +46,7 @@ echo "in root: gitoxide CLI" (enter git-note && indent cargo diet -n --package-size-limit 5KB) (enter git-sec && indent cargo diet -n --package-size-limit 15KB) (enter git-tix && indent cargo diet -n --package-size-limit 5KB) -(enter git-credentials && indent cargo diet -n --package-size-limit 10KB) +(enter git-credentials && indent cargo diet -n --package-size-limit 15KB) (enter git-object && indent cargo diet -n --package-size-limit 25KB) (enter git-commitgraph && indent cargo diet -n --package-size-limit 25KB) (enter git-pack && indent cargo diet -n --package-size-limit 115KB) diff --git a/git-credentials/tests/helper/invoke.rs b/git-credentials/tests/helper/invoke.rs index cb5c20b3d93..f8ee6e57e82 100644 --- a/git-credentials/tests/helper/invoke.rs +++ b/git-credentials/tests/helper/invoke.rs @@ -87,7 +87,8 @@ mod util { } fn finish(self) -> std::io::Result<()> { - Ok(self.handle.take().expect("started").join().unwrap()) + self.handle.take().expect("started").join().unwrap(); + Ok(()) } } } From 4d7b1ddec6ef747665edcfddbba68ed12e3970c2 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Thu, 25 Aug 2022 11:02:59 +0800 Subject: [PATCH 050/236] first test for launching the git credential helper (#450) --- git-credentials/src/helper/invoke.rs | 3 ++- git-credentials/tests/credentials.rs | 1 + git-credentials/tests/program/mod.rs | 17 +++++++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 git-credentials/tests/program/mod.rs diff --git a/git-credentials/src/helper/invoke.rs b/git-credentials/src/helper/invoke.rs index 18410b6de04..2b0f00b1843 100644 --- a/git-credentials/src/helper/invoke.rs +++ b/git-credentials/src/helper/invoke.rs @@ -2,6 +2,7 @@ use crate::helper::Context; use bstr::{BStr, BString}; /// The outcome of the credentials [`helper`][crate::helper()]. +#[derive(Debug, Clone, Eq, PartialEq)] pub struct Outcome { /// The obtained identity. pub identity: git_sec::identity::Account, @@ -77,7 +78,7 @@ impl Action { } /// A handle to [store][NextAction::store()] or [erase][NextAction::erase()] the outcome of the initial action. -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Eq, PartialEq)] pub struct NextAction { previous_output: BString, } diff --git a/git-credentials/tests/credentials.rs b/git-credentials/tests/credentials.rs index 94eb82de319..c4d2ec79dd5 100644 --- a/git-credentials/tests/credentials.rs +++ b/git-credentials/tests/credentials.rs @@ -1,3 +1,4 @@ pub use git_testtools::Result; mod helper; +mod program; diff --git a/git-credentials/tests/program/mod.rs b/git-credentials/tests/program/mod.rs new file mode 100644 index 00000000000..eca8cd6efa5 --- /dev/null +++ b/git-credentials/tests/program/mod.rs @@ -0,0 +1,17 @@ +use git_credentials::helper::{invoke, Kind}; +use git_credentials::Program; + +#[test] +fn git_credentials() { + assert!( + matches!( + git_credentials::helper::invoke( + Program::from_kind(Kind::GitCredential), + invoke::Action::get_for_url("/path/without/scheme/fails/with/error"), + ) + .unwrap_err(), + invoke::Error::CredentialsHelperFailed { .. } + ), + "this failure indicates we could launch the helper, even though it wasn't happy which is fine" + ); +} From 46e3045e04e5197560d8c786642b8f1924a577f9 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Thu, 25 Aug 2022 11:52:08 +0800 Subject: [PATCH 051/236] sketch additional credentials programs (#450) --- git-credentials/src/helper/mod.rs | 19 +++++++++++++++++-- git-credentials/src/lib.rs | 2 +- git-credentials/src/program.rs | 9 ++++++++- git-credentials/tests/program/mod.rs | 4 ++-- 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/git-credentials/src/helper/mod.rs b/git-credentials/src/helper/mod.rs index 112a9c10a09..f117d3aeb48 100644 --- a/git-credentials/src/helper/mod.rs +++ b/git-credentials/src/helper/mod.rs @@ -1,9 +1,24 @@ use bstr::BString; /// The kind of helper program to use. +#[derive(Debug, Clone, Eq, PartialEq)] pub enum Kind { - /// The built-in git-credential helper program, part of any git distribution. - GitCredential, + /// The built-in `git credential` helper program, part of any git distribution. + Builtin, + /// A custom credentials helper, as identified just by the name with optional arguments + CustomName { + /// The name like `foo` along with optional args, like `foo --arg --bar="a b"`, with arguments using `sh` shell quoting rules. + /// The program executed will be `git-credential-foo` if `name_and_args` starts with `foo`. + name_and_args: BString, + }, + /// A custom credentials helper, as identified just by the absolute path to the program and optional arguments. The program is executed through a shell. + CustomPath { + /// The absolute path to the executable, like `/path/to/exe` along with optional args, like `/path/to/exe --arg --bar="a b"`, with arguments using `sh` + /// shell quoting rules. + path_and_args: BString, + }, + /// A script to execute with `sh`. + Script(BString), } /// Additional context to be passed to the credentials helper. diff --git a/git-credentials/src/lib.rs b/git-credentials/src/lib.rs index e1b91b28eb7..dd8dbd159a1 100644 --- a/git-credentials/src/lib.rs +++ b/git-credentials/src/lib.rs @@ -40,5 +40,5 @@ pub mod helper; /// /// See [`invoke()`][helper::invoke()] for a more flexible implementation. pub fn helper(action: helper::invoke::Action) -> helper::invoke::Result { - helper::invoke(Program::from_kind(helper::Kind::GitCredential), action) + helper::invoke(Program::from_kind(helper::Kind::Builtin), action) } diff --git a/git-credentials/src/program.rs b/git-credentials/src/program.rs index d1141381317..679322c4b1f 100644 --- a/git-credentials/src/program.rs +++ b/git-credentials/src/program.rs @@ -16,11 +16,18 @@ impl Helper for Program { match self { Program::Ready(kind) => { let (mut cmd, is_custom) = match kind { - helper::Kind::GitCredential => { + helper::Kind::Builtin => { let mut cmd = Command::new(cfg!(windows).then(|| "git.exe").unwrap_or("git")); cmd.arg("credential"); (cmd, false) } + helper::Kind::Script(for_shell) + | helper::Kind::CustomName { + name_and_args: for_shell, + } + | helper::Kind::CustomPath { + path_and_args: for_shell, + } => todo!("name and args: {for_shell:?}"), }; cmd.arg(action.as_helper_arg(is_custom)) .stdin(Stdio::piped()) diff --git a/git-credentials/tests/program/mod.rs b/git-credentials/tests/program/mod.rs index eca8cd6efa5..ba5173900c7 100644 --- a/git-credentials/tests/program/mod.rs +++ b/git-credentials/tests/program/mod.rs @@ -2,11 +2,11 @@ use git_credentials::helper::{invoke, Kind}; use git_credentials::Program; #[test] -fn git_credentials() { +fn builtin() { assert!( matches!( git_credentials::helper::invoke( - Program::from_kind(Kind::GitCredential), + Program::from_kind(Kind::Builtin), invoke::Action::get_for_url("/path/without/scheme/fails/with/error"), ) .unwrap_err(), From b8c54f03fdb6060caf9c04557c0530c090e7a975 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Thu, 25 Aug 2022 11:54:00 +0800 Subject: [PATCH 052/236] rename!: `helper::Kind` to `program::Kind` (#450) --- git-credentials/src/helper/mod.rs | 21 ------------------ git-credentials/src/lib.rs | 7 +++--- git-credentials/src/program.rs | 33 +++++++++++++++++++++++----- git-credentials/tests/program/mod.rs | 4 ++-- 4 files changed, 34 insertions(+), 31 deletions(-) diff --git a/git-credentials/src/helper/mod.rs b/git-credentials/src/helper/mod.rs index f117d3aeb48..aabc40427ad 100644 --- a/git-credentials/src/helper/mod.rs +++ b/git-credentials/src/helper/mod.rs @@ -1,26 +1,5 @@ use bstr::BString; -/// The kind of helper program to use. -#[derive(Debug, Clone, Eq, PartialEq)] -pub enum Kind { - /// The built-in `git credential` helper program, part of any git distribution. - Builtin, - /// A custom credentials helper, as identified just by the name with optional arguments - CustomName { - /// The name like `foo` along with optional args, like `foo --arg --bar="a b"`, with arguments using `sh` shell quoting rules. - /// The program executed will be `git-credential-foo` if `name_and_args` starts with `foo`. - name_and_args: BString, - }, - /// A custom credentials helper, as identified just by the absolute path to the program and optional arguments. The program is executed through a shell. - CustomPath { - /// The absolute path to the executable, like `/path/to/exe` along with optional args, like `/path/to/exe --arg --bar="a b"`, with arguments using `sh` - /// shell quoting rules. - path_and_args: BString, - }, - /// A script to execute with `sh`. - Script(BString), -} - /// Additional context to be passed to the credentials helper. #[derive(Debug, Default, Clone, Eq, PartialEq)] pub struct Context { diff --git a/git-credentials/src/lib.rs b/git-credentials/src/lib.rs index dd8dbd159a1..6e202922e15 100644 --- a/git-credentials/src/lib.rs +++ b/git-credentials/src/lib.rs @@ -26,12 +26,13 @@ pub trait Helper { /// A program/executable implementing the credential helper protocol. pub enum Program { /// The kind of program, ready for launch - Ready(helper::Kind), + Ready(program::Kind), /// The process is running. Started(std::process::Child), } -mod program; +/// +pub mod program; /// pub mod helper; @@ -40,5 +41,5 @@ pub mod helper; /// /// See [`invoke()`][helper::invoke()] for a more flexible implementation. pub fn helper(action: helper::invoke::Action) -> helper::invoke::Result { - helper::invoke(Program::from_kind(helper::Kind::Builtin), action) + helper::invoke(Program::from_kind(program::Kind::Builtin), action) } diff --git a/git-credentials/src/program.rs b/git-credentials/src/program.rs index 679322c4b1f..3481d3f1ad3 100644 --- a/git-credentials/src/program.rs +++ b/git-credentials/src/program.rs @@ -1,9 +1,32 @@ use crate::{helper, Helper, Program}; + +/// The kind of helper program to use. +#[derive(Debug, Clone, Eq, PartialEq)] +pub enum Kind { + /// The built-in `git credential` helper program, part of any git distribution. + Builtin, + /// A custom credentials helper, as identified just by the name with optional arguments + CustomName { + /// The name like `foo` along with optional args, like `foo --arg --bar="a b"`, with arguments using `sh` shell quoting rules. + /// The program executed will be `git-credential-foo` if `name_and_args` starts with `foo`. + name_and_args: BString, + }, + /// A custom credentials helper, as identified just by the absolute path to the program and optional arguments. The program is executed through a shell. + CustomPath { + /// The absolute path to the executable, like `/path/to/exe` along with optional args, like `/path/to/exe --arg --bar="a b"`, with arguments using `sh` + /// shell quoting rules. + path_and_args: BString, + }, + /// A script to execute with `sh`. + Script(BString), +} + +use bstr::BString; use std::process::{Command, Stdio}; impl Program { /// Create a new program of the given `kind`. - pub fn from_kind(kind: helper::Kind) -> Self { + pub fn from_kind(kind: Kind) -> Self { Program::Ready(kind) } } @@ -16,16 +39,16 @@ impl Helper for Program { match self { Program::Ready(kind) => { let (mut cmd, is_custom) = match kind { - helper::Kind::Builtin => { + Kind::Builtin => { let mut cmd = Command::new(cfg!(windows).then(|| "git.exe").unwrap_or("git")); cmd.arg("credential"); (cmd, false) } - helper::Kind::Script(for_shell) - | helper::Kind::CustomName { + Kind::Script(for_shell) + | Kind::CustomName { name_and_args: for_shell, } - | helper::Kind::CustomPath { + | Kind::CustomPath { path_and_args: for_shell, } => todo!("name and args: {for_shell:?}"), }; diff --git a/git-credentials/tests/program/mod.rs b/git-credentials/tests/program/mod.rs index ba5173900c7..ab10f0d5fd5 100644 --- a/git-credentials/tests/program/mod.rs +++ b/git-credentials/tests/program/mod.rs @@ -1,5 +1,5 @@ -use git_credentials::helper::{invoke, Kind}; -use git_credentials::Program; +use git_credentials::helper::invoke; +use git_credentials::{program::Kind, Program}; #[test] fn builtin() { From 2b2cd0001babdc16e940fa7242c6d723fc9f789b Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Thu, 25 Aug 2022 13:00:20 +0800 Subject: [PATCH 053/236] initial version of parsing of custom helper definitions (#450) --- Cargo.lock | 1 + git-credentials/Cargo.toml | 2 + git-credentials/src/lib.rs | 1 + git-credentials/src/program.rs | 31 +++++++++++--- git-credentials/tests/helper/invoke.rs | 20 +++++++++ git-credentials/tests/program/mod.rs | 56 +++++++++++++++++++------- 6 files changed, 91 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 753210dff0e..bfdc0554eec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1168,6 +1168,7 @@ dependencies = [ "bstr", "document-features", "git-features", + "git-path", "git-sec", "git-testtools", "serde", diff --git a/git-credentials/Cargo.toml b/git-credentials/Cargo.toml index 1c58c40a893..1c09e1dc92a 100644 --- a/git-credentials/Cargo.toml +++ b/git-credentials/Cargo.toml @@ -16,6 +16,8 @@ serde1 = ["serde", "bstr/serde1", "git-sec/serde1"] [dependencies] git-sec = { version = "^0.3.1", path = "../git-sec" } +git-path = { version = "^0.4.1", path = "../git-path" } + thiserror = "1.0.32" serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] } bstr = { version = "0.2.13", default-features = false, features = ["std"]} diff --git a/git-credentials/src/lib.rs b/git-credentials/src/lib.rs index 6e202922e15..ef52cf01cbf 100644 --- a/git-credentials/src/lib.rs +++ b/git-credentials/src/lib.rs @@ -24,6 +24,7 @@ pub trait Helper { } /// A program/executable implementing the credential helper protocol. +#[derive(Debug)] pub enum Program { /// The kind of program, ready for launch Ready(program::Kind), diff --git a/git-credentials/src/program.rs b/git-credentials/src/program.rs index 3481d3f1ad3..93d13a898fa 100644 --- a/git-credentials/src/program.rs +++ b/git-credentials/src/program.rs @@ -1,4 +1,6 @@ use crate::{helper, Helper, Program}; +use bstr::{BString, ByteSlice}; +use std::process::{Command, Stdio}; /// The kind of helper program to use. #[derive(Debug, Clone, Eq, PartialEq)] @@ -18,17 +20,36 @@ pub enum Kind { path_and_args: BString, }, /// A script to execute with `sh`. - Script(BString), + CustomScript(BString), } -use bstr::BString; -use std::process::{Command, Stdio}; - impl Program { /// Create a new program of the given `kind`. pub fn from_kind(kind: Kind) -> Self { Program::Ready(kind) } + + /// Parse the given input as per the custom helper definition, supporting `!