Fix code quality issues found in simplify review

Extract `embed_token_in_url` and `resolve_authenticated_url` helpers
into arc-github to eliminate duplicated credential resolution + URL
authentication logic across ExeSandbox, DaytonaSandbox, and engine.rs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-03-09 12:29:01 -04:00
parent 3d21b0cd40
commit 5f33e01f10
4 changed files with 51 additions and 65 deletions

View file

@ -225,20 +225,10 @@ impl ExeSandbox {
/// Resolve an authenticated clone URL from the clean URL and github_app credentials.
async fn resolve_clone_url(&self, url: &str) -> Result<String, String> {
let creds = match &self.github_app {
Some(c) => c,
None => return Ok(url.to_string()),
};
let (owner, repo) = match arc_github::parse_github_owner_repo(url) {
Ok(pair) => pair,
Err(_) => return Ok(url.to_string()),
};
let (_username, password) =
arc_github::resolve_clone_credentials(creds, &owner, &repo).await?;
match password {
Some(token) => {
Ok(url.replacen("https://", &format!("https://x-access-token:{token}@"), 1))
}
match &self.github_app {
Some(creds) => arc_github::resolve_authenticated_url(creds, url)
.await
.or_else(|_| Ok(url.to_string())),
None => Ok(url.to_string()),
}
}
@ -831,24 +821,17 @@ impl Sandbox for ExeSandbox {
None => return Ok(()),
};
let (owner, repo) = arc_github::parse_github_owner_repo(origin_url)
.map_err(|e| format!("Failed to parse origin URL for credential refresh: {e}"))?;
let (_username, password) = arc_github::resolve_clone_credentials(creds, &owner, &repo)
let auth_url = arc_github::resolve_authenticated_url(creds, origin_url)
.await
.map_err(|e| format!("Failed to refresh GitHub App token: {e}"))?;
if let Some(token) = password {
let auth_url =
origin_url.replacen("https://", &format!("https://x-access-token:{token}@"), 1);
let cmd = format!(
"git -c maintenance.auto=0 remote set-url origin {}",
shell_quote(&auth_url)
);
self.exec_command(&cmd, 10_000, None, None, None)
.await
.map_err(|e| format!("Failed to set refreshed push credentials: {e}"))?;
}
let cmd = format!(
"git -c maintenance.auto=0 remote set-url origin {}",
shell_quote(&auth_url)
);
self.exec_command(&cmd, 10_000, None, None, None)
.await
.map_err(|e| format!("Failed to set refreshed push credentials: {e}"))?;
Ok(())
}

View file

@ -331,6 +331,31 @@ pub async fn resolve_clone_credentials(
Ok((Some("x-access-token".to_string()), Some(token)))
}
/// Embed a token into an HTTPS URL for authenticated git operations.
///
/// Converts `https://github.com/owner/repo` to
/// `https://x-access-token:<token>@github.com/owner/repo`.
pub fn embed_token_in_url(url: &str, token: &str) -> String {
url.replacen("https://", &format!("https://x-access-token:{token}@"), 1)
}
/// Resolve an authenticated HTTPS URL for a GitHub repository.
///
/// Parses owner/repo from the URL, obtains a fresh installation access token,
/// and returns the URL with embedded credentials. Returns the original URL
/// unchanged if it's not a GitHub URL.
pub async fn resolve_authenticated_url(
creds: &GitHubAppCredentials,
url: &str,
) -> Result<String, String> {
let (owner, repo) = parse_github_owner_repo(url)?;
let (_username, password) = resolve_clone_credentials(creds, &owner, &repo).await?;
match password {
Some(token) => Ok(embed_token_in_url(url, &token)),
None => Ok(url.to_string()),
}
}
#[cfg(test)]
mod tests {
use super::*;

View file

@ -784,24 +784,17 @@ impl Sandbox for DaytonaSandbox {
None => return Ok(()),
};
let (owner, repo) = arc_github::parse_github_owner_repo(origin_url)
.map_err(|e| format!("Failed to parse origin URL for credential refresh: {e}"))?;
let (_username, password) = arc_github::resolve_clone_credentials(creds, &owner, &repo)
let auth_url = arc_github::resolve_authenticated_url(creds, origin_url)
.await
.map_err(|e| format!("Failed to refresh GitHub App token: {e}"))?;
if let Some(token) = password {
let auth_url =
origin_url.replacen("https://", &format!("https://x-access-token:{token}@"), 1);
let cmd = format!(
"git -c maintenance.auto=0 remote set-url origin {}",
shell_quote(&auth_url),
);
self.exec_command(&cmd, 10_000, None, None, None)
.await
.map_err(|e| format!("Failed to set refreshed push credentials: {e}"))?;
}
let cmd = format!(
"git -c maintenance.auto=0 remote set-url origin {}",
shell_quote(&auth_url),
);
self.exec_command(&cmd, 10_000, None, None, None)
.await
.map_err(|e| format!("Failed to set refreshed push credentials: {e}"))?;
Ok(())
}

View file

@ -698,28 +698,13 @@ async fn git_push_meta_host(
let https_url = arc_github::ssh_url_to_https(&origin_url);
let push_url = match &github_app {
Some(creds) => {
let (owner, repo) = match arc_github::parse_github_owner_repo(&https_url) {
Ok(pair) => pair,
Err(e) => {
tracing::warn!(error = %e, "Cannot parse GitHub URL for metadata push");
return;
}
};
match arc_github::resolve_clone_credentials(creds, &owner, &repo).await {
Ok((_, Some(token))) => {
https_url.replacen("https://", &format!("https://x-access-token:{token}@"), 1)
}
Ok(_) => {
tracing::warn!("No token returned for metadata push");
return;
}
Err(e) => {
tracing::warn!(error = %e, "Failed to get token for metadata push");
return;
}
Some(creds) => match arc_github::resolve_authenticated_url(creds, &https_url).await {
Ok(url) => url,
Err(e) => {
tracing::warn!(error = %e, "Failed to get token for metadata push");
return;
}
}
},
None => {
tracing::warn!("No GitHub App credentials for metadata push");
return;