mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-06 08:18:58 +00:00
Fix push credentials for public repos in Daytona sandbox
resolve_clone_credentials was short-circuiting for public repos, returning no token. This broke git push from the sandbox since push requires authentication regardless of repo visibility. Always generate an installation access token. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
4d9b0828c1
commit
b2f8a55b8f
2 changed files with 11 additions and 115 deletions
|
|
@ -65,54 +65,6 @@ pub fn sign_app_jwt(app_id: &str, private_key_pem: &str) -> Result<String, Strin
|
|||
Ok(jwt)
|
||||
}
|
||||
|
||||
/// Check whether a GitHub repository is public using the App JWT.
|
||||
pub async fn is_repo_public(
|
||||
client: &reqwest::Client,
|
||||
jwt: &str,
|
||||
owner: &str,
|
||||
repo: &str,
|
||||
base_url: &str,
|
||||
) -> Result<bool, String> {
|
||||
#[derive(Deserialize)]
|
||||
struct RepoResponse {
|
||||
private: bool,
|
||||
}
|
||||
|
||||
let url = format!("{base_url}/repos/{owner}/{repo}");
|
||||
let response = client
|
||||
.get(&url)
|
||||
.header("Authorization", format!("Bearer {jwt}"))
|
||||
.header("Accept", "application/vnd.github+json")
|
||||
.header("User-Agent", "arc")
|
||||
.send()
|
||||
.await
|
||||
.map_err(|e| format!("Failed to check repo visibility: {e}"))?;
|
||||
|
||||
let status = response.status();
|
||||
// 404 = repo not found (or not visible); 401/403 = app JWT can't read repos.
|
||||
// In all these cases, assume the repo is private and proceed to get an
|
||||
// installation access token, which WILL have the right permissions.
|
||||
if status == reqwest::StatusCode::NOT_FOUND
|
||||
|| status == reqwest::StatusCode::UNAUTHORIZED
|
||||
|| status == reqwest::StatusCode::FORBIDDEN
|
||||
{
|
||||
return Ok(false);
|
||||
}
|
||||
if !status.is_success() {
|
||||
let body = response.text().await.unwrap_or_default();
|
||||
return Err(format!(
|
||||
"Failed to check repo visibility (HTTP {status}): {body}"
|
||||
));
|
||||
}
|
||||
|
||||
let body: RepoResponse = response
|
||||
.json()
|
||||
.await
|
||||
.map_err(|e| format!("Failed to parse repo response: {e}"))?;
|
||||
|
||||
Ok(!body.private)
|
||||
}
|
||||
|
||||
/// Request a scoped Installation Access Token for a specific repository.
|
||||
///
|
||||
/// Uses the App JWT to find the installation for `owner/repo`, then requests
|
||||
|
|
@ -233,8 +185,9 @@ pub async fn create_installation_access_token(
|
|||
|
||||
/// Resolve git clone credentials for a GitHub repository.
|
||||
///
|
||||
/// Returns `(username, password)` for authenticated cloning, or `(None, None)`
|
||||
/// for public repositories.
|
||||
/// Returns `(username, password)` for authenticated cloning.
|
||||
/// Always generates a token regardless of repo visibility, since the token
|
||||
/// is needed for pushing from the sandbox.
|
||||
pub async fn resolve_clone_credentials(
|
||||
creds: &GitHubAppCredentials,
|
||||
owner: &str,
|
||||
|
|
@ -243,10 +196,6 @@ pub async fn resolve_clone_credentials(
|
|||
let jwt = sign_app_jwt(&creds.app_id, &creds.private_key_pem)?;
|
||||
let client = reqwest::Client::new();
|
||||
|
||||
if is_repo_public(&client, &jwt, owner, repo, GITHUB_API_BASE_URL).await? {
|
||||
return Ok((None, None));
|
||||
}
|
||||
|
||||
let token =
|
||||
create_installation_access_token(&client, &jwt, owner, repo, GITHUB_API_BASE_URL).await?;
|
||||
Ok((
|
||||
|
|
@ -364,58 +313,6 @@ mod tests {
|
|||
assert!(result.unwrap_err().contains("Invalid RSA private key"));
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// is_repo_public (mockito)
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
#[tokio::test]
|
||||
async fn is_repo_public_returns_true_for_public() {
|
||||
let mut server = mockito::Server::new_async().await;
|
||||
let mock = server
|
||||
.mock("GET", "/repos/owner/repo")
|
||||
.match_header("Authorization", "Bearer test-jwt")
|
||||
.with_status(200)
|
||||
.with_body(r#"{"private": false}"#)
|
||||
.create_async()
|
||||
.await;
|
||||
|
||||
let client = reqwest::Client::new();
|
||||
let result = is_repo_public(&client, "test-jwt", "owner", "repo", &server.url()).await;
|
||||
assert_eq!(result.unwrap(), true);
|
||||
mock.assert_async().await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn is_repo_public_returns_false_for_private() {
|
||||
let mut server = mockito::Server::new_async().await;
|
||||
let mock = server
|
||||
.mock("GET", "/repos/owner/repo")
|
||||
.with_status(200)
|
||||
.with_body(r#"{"private": true}"#)
|
||||
.create_async()
|
||||
.await;
|
||||
|
||||
let client = reqwest::Client::new();
|
||||
let result = is_repo_public(&client, "test-jwt", "owner", "repo", &server.url()).await;
|
||||
assert_eq!(result.unwrap(), false);
|
||||
mock.assert_async().await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn is_repo_public_returns_false_for_404() {
|
||||
let mut server = mockito::Server::new_async().await;
|
||||
let mock = server
|
||||
.mock("GET", "/repos/owner/repo")
|
||||
.with_status(404)
|
||||
.create_async()
|
||||
.await;
|
||||
|
||||
let client = reqwest::Client::new();
|
||||
let result = is_repo_public(&client, "test-jwt", "owner", "repo", &server.url()).await;
|
||||
assert_eq!(result.unwrap(), false);
|
||||
mock.assert_async().await;
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// create_installation_access_token — success
|
||||
// -----------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -1264,11 +1264,10 @@ async fn daytona_clone_private_repo_with_github_app_iat() {
|
|||
env.cleanup().await.unwrap();
|
||||
}
|
||||
|
||||
/// E2E: Verify that public repos are cloned without credentials even when
|
||||
/// GitHub App is configured (the `is_repo_public` optimization path).
|
||||
/// E2E: Verify that public repos still get credentials (needed for pushing).
|
||||
#[tokio::test]
|
||||
#[ignore]
|
||||
async fn daytona_clone_public_repo_no_credentials_needed() {
|
||||
async fn daytona_clone_public_repo_gets_credentials() {
|
||||
let creds = load_github_app_credentials();
|
||||
|
||||
// Directly test resolve_clone_credentials against a known public repo
|
||||
|
|
@ -1280,14 +1279,14 @@ async fn daytona_clone_public_repo_no_credentials_needed() {
|
|||
.await
|
||||
.unwrap();
|
||||
|
||||
assert!(
|
||||
username.is_none(),
|
||||
"public repo should not need credentials, got username: {:?}",
|
||||
username
|
||||
assert_eq!(
|
||||
username.as_deref(),
|
||||
Some("x-access-token"),
|
||||
"public repo should get credentials for pushing"
|
||||
);
|
||||
assert!(
|
||||
password.is_none(),
|
||||
"public repo should not need credentials, got password set"
|
||||
password.is_some(),
|
||||
"public repo should get a token for pushing"
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue