Skip to content

fix #1998 #1999

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions gix-credentials/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ gix-path = { version = "^0.10.17", path = "../gix-path" }
gix-command = { version = "^0.6.0", path = "../gix-command" }
gix-config-value = { version = "^0.15.0", path = "../gix-config-value" }
gix-prompt = { version = "^0.11.0", path = "../gix-prompt" }
gix-date = { version = "^0.10.1", path = "../gix-date" }
gix-trace = { version = "^0.1.12", path = "../gix-trace" }

thiserror = "2.0.0"
Expand Down
40 changes: 31 additions & 9 deletions gix-credentials/src/helper/cascade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,30 +88,51 @@ impl Cascade {
match helper::invoke::raw(program, &action) {
Ok(None) => {}
Ok(Some(stdout)) => {
let ctx = Context::from_bytes(&stdout)?;
let Context {
protocol,
host,
path,
username,
password,
oauth_refresh_token,
password_expiry_utc,
url: ctx_url,
quit,
} = Context::from_bytes(&stdout)?;
if let Some(dst_ctx) = action.context_mut() {
if let Some(src) = ctx.path {
if let Some(src) = path {
dst_ctx.path = Some(src);
}
if let Some(src) = password_expiry_utc {
dst_ctx.password_expiry_utc = Some(src);
}
for (src, dst) in [
(ctx.protocol, &mut dst_ctx.protocol),
(ctx.host, &mut dst_ctx.host),
(ctx.username, &mut dst_ctx.username),
(ctx.password, &mut dst_ctx.password),
(protocol, &mut dst_ctx.protocol),
(host, &mut dst_ctx.host),
(username, &mut dst_ctx.username),
(password, &mut dst_ctx.password),
(oauth_refresh_token, &mut dst_ctx.oauth_refresh_token),
] {
if let Some(src) = src {
*dst = Some(src);
}
}
if let Some(src) = ctx.url {
if let Some(src) = ctx_url {
dst_ctx.url = Some(src);
url = dst_ctx.destructure_url_in_place(self.use_http_path)?.url.take();
}
if dst_ctx
.password_expiry_utc
.is_some_and(|expiry_date| expiry_date < gix_date::Time::now_utc().seconds)
{
dst_ctx.password_expiry_utc = None;
dst_ctx.clear_secrets();
}
if dst_ctx.username.is_some() && dst_ctx.password.is_some() {
break;
}
if ctx.quit.unwrap_or_default() {
dst_ctx.quit = ctx.quit;
if quit.unwrap_or_default() {
dst_ctx.quit = quit;
break;
}
}
Expand Down Expand Up @@ -152,6 +173,7 @@ impl Cascade {
action.context().map(|ctx| helper::Outcome {
username: ctx.username.clone(),
password: ctx.password.clone(),
oauth_refresh_token: ctx.oauth_refresh_token.clone(),
quit: ctx.quit.unwrap_or(false),
next: ctx.to_owned().into(),
}),
Expand Down
1 change: 1 addition & 0 deletions gix-credentials/src/helper/invoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub fn invoke(helper: &mut crate::Program, action: &Action) -> Result {
Ok(Some(Outcome {
username: ctx.username,
password: ctx.password,
oauth_refresh_token: ctx.oauth_refresh_token,
quit: ctx.quit.unwrap_or(false),
next: NextAction {
previous_output: stdout.into(),
Expand Down
10 changes: 8 additions & 2 deletions gix-credentials/src/helper/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ pub struct Outcome {
pub username: Option<String>,
/// The password to use in the identity, if set.
pub password: Option<String>,
/// An OAuth refresh token that may accompany a password. It is to be treated confidentially, just like the password.
pub oauth_refresh_token: Option<String>,
/// If set, the helper asked to stop the entire process, whether the identity is complete or not.
pub quit: bool,
/// A handle to the action to perform next in another call to [`helper::invoke()`][crate::helper::invoke()].
Expand All @@ -42,7 +44,11 @@ impl Outcome {
self.username
.take()
.zip(self.password.take())
.map(|(username, password)| gix_sec::identity::Account { username, password })
.map(|(username, password)| gix_sec::identity::Account {
username,
password,
oauth_refresh_token: self.oauth_refresh_token.take(),
})
}
}

Expand Down Expand Up @@ -131,7 +137,7 @@ impl Action {
}
}

/// A handle to [store][NextAction::store()] or [erase][NextAction::erase()] 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, Eq, PartialEq)]
pub struct NextAction {
previous_output: BString,
Expand Down
36 changes: 36 additions & 0 deletions gix-credentials/src/protocol/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,42 @@ mod access {
use crate::protocol::Context;

impl Context {
/// Clear all fields that are considered secret.
pub fn clear_secrets(&mut self) {
let Context {
protocol: _,
host: _,
path: _,
username: _,
password,
oauth_refresh_token,
password_expiry_utc: _,
url: _,
quit: _,
} = self;

*password = None;
*oauth_refresh_token = None;
}
/// Replace existing secrets with the word `<redacted>`.
pub fn redacted(mut self) -> Self {
let Context {
protocol: _,
host: _,
path: _,
username: _,
password,
oauth_refresh_token,
password_expiry_utc: _,
url: _,
quit: _,
} = &mut self;
for secret in [password, oauth_refresh_token].into_iter().flatten() {
*secret = "<redacted>".into();
}
self
}

/// Convert all relevant fields into a URL for consumption.
pub fn to_url(&self) -> Option<BString> {
use bstr::{ByteSlice, ByteVec};
Expand Down
62 changes: 49 additions & 13 deletions gix-credentials/src/protocol/context/serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,46 @@ mod write {
out.write_all(value)?;
out.write_all(b"\n")
}
for (key, value) in [("url", &self.url), ("path", &self.path)] {
let Context {
protocol,
host,
path,
username,
password,
oauth_refresh_token,
password_expiry_utc,
url,
// We only decode quit and interpret it, but won't get to pass it on as it means to stop the
// credential helper invocation chain.
quit: _,
} = self;
for (key, value) in [("url", url), ("path", path)] {
if let Some(value) = value {
validate(key, value.as_slice().into())
.map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err))?;
write_key(&mut out, key, value.as_ref()).ok();
}
}
for (key, value) in [
("protocol", &self.protocol),
("host", &self.host),
("username", &self.username),
("password", &self.password),
("protocol", protocol),
("host", host),
("username", username),
("password", password),
("oauth_refresh_token", oauth_refresh_token),
] {
if let Some(value) = value {
validate(key, value.as_str().into())
.map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err))?;
write_key(&mut out, key, value.as_bytes().as_bstr()).ok();
}
}
if let Some(value) = password_expiry_utc {
let key = "password_expiry_utc";
let value = value.to_string();
validate(key, value.as_str().into())
.map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err))?;
write_key(&mut out, key, value.as_bytes().as_bstr()).ok();
}
Ok(())
}

Expand Down Expand Up @@ -70,6 +91,17 @@ pub mod decode {
/// Decode ourselves from `input` which is the format written by [`write_to()`][Self::write_to()].
pub fn from_bytes(input: &[u8]) -> Result<Self, Error> {
let mut ctx = Context::default();
let Context {
protocol,
host,
path,
username,
password,
oauth_refresh_token,
password_expiry_utc,
url,
quit,
} = &mut ctx;
for res in input.lines().take_while(|line| !line.is_empty()).map(|line| {
let mut it = line.splitn(2, |b| *b == b'=');
match (
Expand All @@ -84,23 +116,27 @@ pub mod decode {
}) {
let (key, value) = res?;
match key {
"protocol" | "host" | "username" | "password" => {
"protocol" | "host" | "username" | "password" | "oauth_refresh_token" => {
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,
"protocol" => &mut *protocol,
"host" => host,
"username" => username,
"password" => password,
"oauth_refresh_token" => oauth_refresh_token,
_ => unreachable!("checked field names in match above"),
} = Some(value);
}
"url" => ctx.url = Some(value),
"path" => ctx.path = Some(value),
"password_expiry_utc" => {
*password_expiry_utc = value.to_str().ok().and_then(|value| value.parse().ok());
}
"url" => *url = Some(value),
"path" => *path = Some(value),
"quit" => {
ctx.quit = gix_config_value::Boolean::try_from(value.as_ref()).ok().map(Into::into);
*quit = gix_config_value::Boolean::try_from(value.as_ref()).ok().map(Into::into);
}
_ => {}
}
Expand Down
18 changes: 10 additions & 8 deletions gix-credentials/src/protocol/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ pub struct Context {
pub username: Option<String>,
/// The credential’s password, if we are asking it to be stored.
pub password: Option<String>,
/// An OAuth refresh token that may accompany a password. It is to be treated confidentially, just like the password.
pub oauth_refresh_token: Option<String>,
/// The expiry date of OAuth tokens as seconds from Unix epoch.
pub password_expiry_utc: Option<gix_date::SecondsSinceUnixEpoch>,
/// 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.
Expand All @@ -59,14 +63,10 @@ pub struct Context {
/// Convert the outcome of a helper invocation to a helper result, assuring that the identity is complete in the process.
#[allow(clippy::result_large_err)]
pub fn helper_outcome_to_result(outcome: Option<helper::Outcome>, action: helper::Action) -> Result {
fn redact(mut ctx: Context) -> Context {
if let Some(pw) = ctx.password.as_mut() {
*pw = "<redacted>".into();
}
ctx
}
match (action, outcome) {
(helper::Action::Get(ctx), None) => Err(Error::IdentityMissing { context: redact(ctx) }),
(helper::Action::Get(ctx), None) => Err(Error::IdentityMissing {
context: ctx.redacted(),
}),
(helper::Action::Get(ctx), Some(mut outcome)) => match outcome.consume_identity() {
Some(identity) => Ok(Some(Outcome {
identity,
Expand All @@ -75,7 +75,9 @@ pub fn helper_outcome_to_result(outcome: Option<helper::Outcome>, action: helper
None => Err(if outcome.quit {
Error::Quit
} else {
Error::IdentityMissing { context: redact(ctx) }
Error::IdentityMissing {
context: ctx.redacted(),
}
}),
},
(helper::Action::Store(_) | helper::Action::Erase(_), _ignore) => Ok(None),
Expand Down
7 changes: 7 additions & 0 deletions gix-credentials/tests/fixtures/expired.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/usr/bin/env bash
set -eu

test "$1" = get && \
echo username=user-expired && \
echo password=pass-expired && \
echo password_expiry_utc=1
5 changes: 5 additions & 0 deletions gix-credentials/tests/fixtures/oauth-token.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/usr/bin/env bash
set -eu

test "$1" = get && \
echo oauth_refresh_token=oauth-token
20 changes: 20 additions & 0 deletions gix-credentials/tests/helper/cascade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,25 @@ mod invoke {
assert_eq!(actual.identity, identity("user", "pass"));
}

#[test]
fn expired_credentials_are_not_returned() {
let actual = invoke_cascade(
["expired", "oauth-token", "custom-helper"],
Action::get_for_url("http://github.com"),
)
.unwrap()
.expect("credentials");

assert_eq!(
actual.identity,
Account {
oauth_refresh_token: Some("oauth-token".into()),
..identity("user-script", "pass-script")
},
"it ignored the expired password, which otherwise would have come first"
);
}

#[test]
fn bogus_password_overrides_any_helper_and_helper_overrides_username_in_url() {
let actual = Cascade::default()
Expand All @@ -144,6 +163,7 @@ mod invoke {
Account {
username: user.into(),
password: pass.into(),
oauth_refresh_token: None,
}
}

Expand Down
Loading
Loading