Marketplace fetch: offer the token only to the marketplace host (final review M1)
The credential callback answered every credential request. gix follows a redirect of the initial handshake and asks for credentials for the redirect target, so the token could be sent to another host. The callback now answers only when the request's scheme, host and port match the marketplace URL (gix's own URL normalisation, host compared case-insensitively); anything else gets no credential. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -150,6 +150,27 @@ fn open_or_init(repo_path: &Path) -> Result<gix::Repository, FetchError> {
|
||||
}
|
||||
}
|
||||
|
||||
/// `(scheme, host[:port])` of a credential request, lowercased, with a
|
||||
/// default port dropped (gix's own normalisation). None if it names no host.
|
||||
fn credential_origin(ctx: &gix::credentials::protocol::Context) -> Option<(String, String)> {
|
||||
let mut ctx = ctx.clone();
|
||||
ctx.destructure_url_in_place(false).ok()?;
|
||||
let protocol = ctx.protocol?.to_ascii_lowercase();
|
||||
let host = ctx.host?.to_ascii_lowercase();
|
||||
(!host.is_empty()).then_some((protocol, host))
|
||||
}
|
||||
|
||||
/// True when a credential request is for the marketplace's own scheme, host
|
||||
/// and port. gix follows redirects of the initial handshake, and the token
|
||||
/// must never be offered to a host it was redirected to (final review M1).
|
||||
pub(crate) fn credential_matches(ctx: &gix::credentials::protocol::Context, url: &str) -> bool {
|
||||
let wanted = gix::credentials::protocol::Context::from_url(url, Default::default());
|
||||
match (credential_origin(ctx), credential_origin(&wanted)) {
|
||||
(Some(asked), Some(wanted)) => asked == wanted,
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
/// Init the bare repo if missing, fetch `branch` (or the remote's default
|
||||
/// branch) into [`HEAD_REF`], and return the head commit hex.
|
||||
pub fn fetch(
|
||||
@@ -174,11 +195,14 @@ pub fn fetch(
|
||||
.map_err(|e| FetchError::Other(format!("Invalid repository URL: {}", e)))?
|
||||
.with_refspecs([refspec.as_str()], gix::remote::Direction::Fetch)
|
||||
.map_err(|e| FetchError::Other(format!("Invalid refspec: {}", e)))?;
|
||||
let own_url = url.to_string();
|
||||
let connection = remote
|
||||
.connect(gix::remote::Direction::Fetch)
|
||||
.map_err(classify)?
|
||||
.with_credentials(move |action| match (action, &cred) {
|
||||
(gix::credentials::helper::Action::Get(ctx), Some(c)) => {
|
||||
(gix::credentials::helper::Action::Get(ctx), Some(c))
|
||||
if credential_matches(&ctx, &own_url) =>
|
||||
{
|
||||
Ok(Some(gix::credentials::protocol::Outcome {
|
||||
identity: gix::sec::identity::Account {
|
||||
username: c.username.clone(),
|
||||
@@ -424,6 +448,48 @@ mod tests {
|
||||
assert!(shown.contains("<redacted>"));
|
||||
}
|
||||
|
||||
/// Final review M1: the token goes only to the marketplace's own scheme,
|
||||
/// host and port — never to a host the handshake was redirected to.
|
||||
#[test]
|
||||
fn credentials_are_offered_only_to_the_marketplace_host() {
|
||||
use gix::credentials::protocol::Context;
|
||||
let url = "https://git.example.com/org/repo.git";
|
||||
let ctx = |u: &str| Context::from_url(u, Default::default());
|
||||
|
||||
assert!(credential_matches(&ctx(url), url));
|
||||
assert!(credential_matches(
|
||||
&ctx("https://git.example.com/other/path.git"),
|
||||
url
|
||||
));
|
||||
assert!(credential_matches(
|
||||
&ctx("https://GIT.example.com/org/repo.git"),
|
||||
url
|
||||
));
|
||||
assert!(credential_matches(
|
||||
&ctx("https://git.example.com:443/org/repo.git"),
|
||||
url
|
||||
));
|
||||
for other in [
|
||||
"https://evil.example.net/org/repo.git",
|
||||
"https://git.example.com.evil.net/org/repo.git",
|
||||
"https://git.example.com:8443/org/repo.git",
|
||||
"http://git.example.com/org/repo.git",
|
||||
] {
|
||||
assert!(!credential_matches(&ctx(other), url), "{other}");
|
||||
}
|
||||
let with_port = "https://git.example.com:8443/org/repo.git";
|
||||
assert!(credential_matches(&ctx(with_port), with_port));
|
||||
assert!(!credential_matches(&ctx(url), with_port));
|
||||
// A request that names no host gets nothing.
|
||||
assert!(!credential_matches(&Context::default(), url));
|
||||
let host_only = Context {
|
||||
protocol: Some("https".into()),
|
||||
host: Some("git.example.com".into()),
|
||||
..Default::default()
|
||||
};
|
||||
assert!(credential_matches(&host_only, url));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn refuses_unsafe_branch_names() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
|
||||
Reference in New Issue
Block a user