diff --git a/lib/crates/fabro-automation/src/lib.rs b/lib/crates/fabro-automation/src/lib.rs index 4dd253cbc..f5f453100 100644 --- a/lib/crates/fabro-automation/src/lib.rs +++ b/lib/crates/fabro-automation/src/lib.rs @@ -7,6 +7,7 @@ pub use error::{AutomationStoreError, AutomationValidationError}; pub use id::{AutomationId, AutomationRevision, AutomationRevisionParseError, AutomationTriggerId}; pub use model::{ ApiTrigger, Automation, AutomationDraft, AutomationReplace, AutomationTarget, - AutomationTrigger, ScheduleTrigger, parse_schedule_expression, + AutomationTrigger, GitHubRepositorySlug, ScheduleTrigger, parse_github_repository_slug, + parse_schedule_expression, }; pub use store::AutomationStore; diff --git a/lib/crates/fabro-automation/src/model.rs b/lib/crates/fabro-automation/src/model.rs index b110f69c8..f00be4fdf 100644 --- a/lib/crates/fabro-automation/src/model.rs +++ b/lib/crates/fabro-automation/src/model.rs @@ -137,6 +137,24 @@ pub struct AutomationTarget { pub workflow: String, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct GitHubRepositorySlug { + owner: String, + repo: String, +} + +impl GitHubRepositorySlug { + #[must_use] + pub fn owner(&self) -> &str { + &self.owner + } + + #[must_use] + pub fn repo(&self) -> &str { + &self.repo + } +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "type", rename_all = "snake_case", deny_unknown_fields)] pub enum AutomationTrigger { @@ -273,7 +291,9 @@ fn validate_fields(value: &AutomationReplace) -> Result<(), AutomationValidation validate_triggers(&value.triggers) } -fn validate_repository_slug(value: &str) -> Result<(), AutomationValidationError> { +pub fn parse_github_repository_slug( + value: &str, +) -> Result { let Some((owner, repo)) = value.split_once('/') else { return Err(AutomationValidationError::InvalidRepositorySlug { value: value.to_string(), @@ -284,7 +304,14 @@ fn validate_repository_slug(value: &str) -> Result<(), AutomationValidationError value: value.to_string(), }); } - Ok(()) + Ok(GitHubRepositorySlug { + owner: owner.to_string(), + repo: repo.to_string(), + }) +} + +fn validate_repository_slug(value: &str) -> Result<(), AutomationValidationError> { + parse_github_repository_slug(value).map(|_| ()) } fn valid_github_owner(value: &str) -> bool { @@ -515,6 +542,15 @@ enabled = true assert_eq!(trigger_ids, vec!["nightly"]); } + #[test] + fn repository_slug_parser_returns_validated_parts() { + let slug = crate::parse_github_repository_slug("owner/.github").unwrap(); + + assert_eq!(slug.owner(), "owner"); + assert_eq!(slug.repo(), ".github"); + assert!(crate::parse_github_repository_slug("not/github/slug").is_err()); + } + #[test] fn validation_rejects_invalid_inputs() { let cases = [ diff --git a/lib/crates/fabro-server/src/automation_materializer.rs b/lib/crates/fabro-server/src/automation_materializer.rs index 52eb7ea68..4658543e6 100644 --- a/lib/crates/fabro-server/src/automation_materializer.rs +++ b/lib/crates/fabro-server/src/automation_materializer.rs @@ -1,26 +1,20 @@ -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use std::sync::Arc; -use std::time::Duration; use anyhow::Context as _; use async_trait::async_trait; -use base64::Engine as _; -use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; use fabro_api::types::RunManifest; -use fabro_automation::{AutomationId, AutomationTarget}; +use fabro_automation::{AutomationId, AutomationTarget, GitHubRepositorySlug}; use fabro_config::{EnvironmentLayer, MergeMap}; use fabro_manifest::ManifestBuildInput; -use fabro_store::KeyedMutex; use fabro_types::{DirtyStatus, GitContext, PreRunPushOutcome, RunId}; use fabro_util::error::collect_chain; -use tokio::process::Command; -use tokio::{fs, task, time}; +use tokio::{fs, task}; -const GIT_CLONE_TIMEOUT: Duration = Duration::from_mins(2); -const GIT_FETCH_TIMEOUT: Duration = Duration::from_mins(1); -const GIT_WORKTREE_ADD_TIMEOUT: Duration = Duration::from_secs(30); -const GIT_WORKTREE_PRUNE_TIMEOUT: Duration = Duration::from_secs(10); -const GIT_REV_PARSE_TIMEOUT: Duration = Duration::from_secs(10); +use crate::git_checkout::{ + GitCheckoutError, GitRepoCache, WorktreePrepareInput, github_metadata_url, + parse_github_repository_slug, resolve_git_auth_config, +}; #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct AutomationRunMaterializeInput { @@ -38,23 +32,32 @@ pub(crate) struct AutomationRunMaterialized { } #[derive(thiserror::Error, Debug, Clone, PartialEq, Eq)] -pub(crate) enum AutomationRunMaterializeError { - #[error("invalid automation target: {0}")] +pub(crate) enum RunMaterializeError { + #[error("invalid repository target: {0}")] InvalidTarget(String), - #[error("failed to clone automation repository: {0}")] + #[error("failed to clone repository: {0}")] CloneFailed(String), - #[error("failed to resolve automation workflow: {0}")] + #[error("failed to resolve workflow: {0}")] WorkflowNotFound(String), #[error("failed to build run manifest: {0}")] Manifest(String), } +impl From for RunMaterializeError { + fn from(value: GitCheckoutError) -> Self { + match value { + GitCheckoutError::InvalidTarget(message) => Self::InvalidTarget(message), + GitCheckoutError::CloneFailed(message) => Self::CloneFailed(message), + } + } +} + #[async_trait] pub(crate) trait AutomationRunMaterializer: Send + Sync { async fn materialize( &self, input: AutomationRunMaterializeInput, - ) -> Result; + ) -> Result; } #[derive(Clone)] @@ -84,44 +87,15 @@ impl ProductionAutomationRunMaterializer { } } -/// Persistent on-disk cache of bare GitHub clones, one per `(owner, repo)`. -/// -/// Materializing an automation run only needs to read the workflow + its -/// supporting files out of the repo at a given ref. Cloning fresh on every -/// click costs 5–15s on the request thread. With this cache, the first -/// materialize for a repo pays the clone, and every subsequent one pays only -/// the delta `git fetch` plus a cheap `git worktree add` into the per-call -/// scratch dir. -#[derive(Debug)] -pub(crate) struct GitRepoCache { - cache_root: PathBuf, - locks: KeyedMutex<(String, String)>, -} - -impl GitRepoCache { - pub(crate) fn new(cache_root: PathBuf) -> Self { - Self { - cache_root, - locks: KeyedMutex::new(), - } - } - - fn bare_dir(&self, repo: &GithubRepository) -> PathBuf { - self.cache_root - .join(&repo.owner) - .join(format!("{}.git", repo.name)) - } -} - #[async_trait] impl AutomationRunMaterializer for ProductionAutomationRunMaterializer { async fn materialize( &self, input: AutomationRunMaterializeInput, - ) -> Result { + ) -> Result { let repo = parse_github_repository_slug(&input.target.repository)?; fs::create_dir_all(&input.temp_root).await.map_err(|err| { - AutomationRunMaterializeError::CloneFailed(format!( + RunMaterializeError::CloneFailed(format!( "failed to create temp root {}: {err}", input.temp_root.display() )) @@ -134,13 +108,12 @@ impl AutomationRunMaterializer for ProductionAutomationRunMaterializer { )) .tempdir_in(&input.temp_root) .map_err(|err| { - AutomationRunMaterializeError::CloneFailed(format!( + RunMaterializeError::CloneFailed(format!( "failed to create per-run temp directory under {}: {err}", input.temp_root.display() )) })?; let checkout_dir = temp_dir.path().join("repo"); - let clone_url = github_clone_url(&repo); let auth = resolve_git_auth_config( self.github_credentials.as_ref(), &repo, @@ -148,15 +121,12 @@ impl AutomationRunMaterializer for ProductionAutomationRunMaterializer { self.http_client.clone(), ) .await - .map_err(|err| { - AutomationRunMaterializeError::CloneFailed(render_error_chain(err.as_ref())) - })?; + .map_err(|err| RunMaterializeError::CloneFailed(render_error_chain(err.as_ref())))?; let checked_out_sha = self .repo_cache .prepare_worktree(WorktreePrepareInput { repo: &repo, - clone_url: &clone_url, ref_selector: &input.target.ref_selector, auth: auth.as_ref(), worktree_dir: &checkout_dir, @@ -164,448 +134,62 @@ impl AutomationRunMaterializer for ProductionAutomationRunMaterializer { .await?; let manifest_input = ManifestFromCheckoutInput { - input, + workflow: input.target.workflow, + run_id: input.run_id, + user_settings_path: input.user_settings_path, checkout_dir, - repo, - checked_out_sha: Some(checked_out_sha), + git_context: ManifestGitContextInput { + repo, + ref_selector: input.target.ref_selector, + checked_out_sha, + }, environment_defaults: self.environment_defaults.clone(), }; task::spawn_blocking(move || build_manifest_from_checkout(manifest_input)) .await .map_err(|err| { - AutomationRunMaterializeError::Manifest(format!( - "manifest build task failed: {err}" - )) + RunMaterializeError::Manifest(format!("manifest build task failed: {err}")) })? } } -pub(crate) struct WorktreePrepareInput<'a> { - pub repo: &'a GithubRepository, - pub clone_url: &'a str, - pub ref_selector: &'a str, - pub auth: Option<&'a GitAuthConfig>, - pub worktree_dir: &'a Path, -} - -impl GitRepoCache { - /// Prepare a worktree containing the requested ref of `repo` at - /// `worktree_dir`. Returns the resolved commit SHA. - /// - /// First call for a repo: a `--bare --depth 1` clone is created at - /// `//.git`. Subsequent calls reuse the bare clone - /// and only `git fetch --depth 1` the requested ref. In both cases a - /// short-lived worktree is added at `worktree_dir`; the caller owns its - /// lifetime (typically a `TempDir`). Stale worktree admin entries from - /// crashed prior calls are pruned at the start of each call so they do - /// not accumulate. - pub(crate) async fn prepare_worktree( - &self, - args: WorktreePrepareInput<'_>, - ) -> Result { - let _guard = self - .locks - .lock((args.repo.owner.clone(), args.repo.name.clone())) - .await; - let bare_dir = self.bare_dir(args.repo); - - match self.try_prepare_worktree(&bare_dir, &args).await { - Ok(sha) => Ok(sha), - Err(first_err) if bare_clone_may_be_corrupt(&bare_dir).await => { - // Best-effort wipe and one retry. Corruption is rare; auth - // and network failures don't trip this branch because - // `bare_clone_may_be_corrupt` only returns true when the - // bare repo's `HEAD` file is missing or empty. - let _ = fs::remove_dir_all(&bare_dir).await; - self.try_prepare_worktree(&bare_dir, &args) - .await - .map_err(|_| first_err) - } - Err(err) => Err(err), - } - } - - async fn try_prepare_worktree( - &self, - bare_dir: &Path, - args: &WorktreePrepareInput<'_>, - ) -> Result { - let bare_exists = fs::try_exists(&bare_dir.join("HEAD")) - .await - .unwrap_or(false); - if bare_exists { - // Drop any worktree admin entries whose working trees were - // deleted by previous `TempDir` cleanup. Cheap and idempotent. - let _ = run_git_plan(build_worktree_prune_plan(bare_dir)).await; - } else { - if let Some(parent) = bare_dir.parent() { - fs::create_dir_all(parent).await.map_err(|err| { - AutomationRunMaterializeError::CloneFailed(format!( - "failed to create cache dir {}: {err}", - parent.display() - )) - })?; - } - run_git_plan(build_bare_clone_plan(args.clone_url, bare_dir, args.auth)).await?; - } - - run_git_plan(build_bare_fetch_plan( - bare_dir, - args.clone_url, - args.ref_selector, - args.auth, - )) - .await?; - - let checked_out_sha = run_git_plan(build_rev_parse_fetch_head_plan(bare_dir)) - .await - .map(|stdout| String::from_utf8_lossy(&stdout).trim().to_string())?; - - run_git_plan(build_worktree_add_plan(bare_dir, args.worktree_dir)).await?; - - Ok(checked_out_sha) - } -} - -async fn bare_clone_may_be_corrupt(bare_dir: &Path) -> bool { - match fs::metadata(&bare_dir.join("HEAD")).await { - Ok(meta) => meta.len() == 0, - Err(_) => bare_dir.exists(), - } -} - -#[derive(Debug, Clone, PartialEq, Eq)] -pub(crate) struct GithubRepository { - owner: String, - name: String, -} - -fn parse_github_repository_slug( - value: &str, -) -> Result { - let Some((owner, repo)) = value.split_once('/') else { - return Err(AutomationRunMaterializeError::InvalidTarget(format!( - "repository must be a GitHub owner/repo slug: {value}" - ))); - }; - if repo.contains('/') || !valid_github_owner(owner) || !valid_github_repo(repo) { - return Err(AutomationRunMaterializeError::InvalidTarget(format!( - "repository must be a GitHub owner/repo slug: {value}" - ))); - } - Ok(GithubRepository { - owner: owner.to_string(), - name: repo.to_string(), - }) -} - -fn valid_github_owner(value: &str) -> bool { - if value.is_empty() || value.len() > 39 { - return false; - } - let bytes = value.as_bytes(); - let first = bytes[0]; - let last = bytes[bytes.len() - 1]; - (first.is_ascii_alphanumeric() && last.is_ascii_alphanumeric()) - && bytes - .iter() - .all(|byte| byte.is_ascii_alphanumeric() || *byte == b'-') -} - -fn valid_github_repo(value: &str) -> bool { - !value.is_empty() - && value.len() <= 100 - && value != "." - && value != ".." - && !value.starts_with('.') - && value - .bytes() - .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'.' | b'_' | b'-')) -} - -fn github_clone_url(repo: &GithubRepository) -> String { - format!("https://github.com/{}/{}.git", repo.owner, repo.name) -} - -fn github_metadata_url(repo: &GithubRepository) -> String { - format!("https://github.com/{}/{}", repo.owner, repo.name) -} - -#[derive(Clone, Debug, PartialEq, Eq)] -pub(crate) struct GitAuthConfig { - extraheader: Option, - sensitive_values: Vec, -} - -impl GitAuthConfig { - fn new(username: Option, password: Option) -> Self { - let Some(password) = password.filter(|value| !value.is_empty()) else { - return Self { - extraheader: None, - sensitive_values: Vec::new(), - }; - }; - let username = username - .filter(|value| !value.is_empty()) - .unwrap_or_else(|| "x-access-token".to_string()); - let encoded_credentials = BASE64_STANDARD.encode(format!("{username}:{password}")); - let extraheader = basic_auth_header_from_encoded(&encoded_credentials); - Self { - sensitive_values: vec![password, encoded_credentials, extraheader.clone()], - extraheader: Some(extraheader), - } - } - - fn git_env(&self, clone_url: &str) -> Vec<(String, String)> { - let Some(extraheader) = self.extraheader.as_ref() else { - return Vec::new(); - }; - vec![ - ("GIT_CONFIG_COUNT".to_string(), "1".to_string()), - ( - "GIT_CONFIG_KEY_0".to_string(), - format!("http.{clone_url}.extraheader"), - ), - ("GIT_CONFIG_VALUE_0".to_string(), extraheader.clone()), - ] - } - - fn sensitive_values(&self) -> &[String] { - &self.sensitive_values - } -} - -async fn resolve_git_auth_config( - credentials: Option<&fabro_github::GitHubCredentials>, - repo: &GithubRepository, - github_api_base_url: &str, - http_client: Option, -) -> anyhow::Result> { - let Some(credentials) = credentials else { - return Ok(None); - }; - let context = match http_client { - Some(client) => { - fabro_github::GitHubContext::with_http_client(credentials, github_api_base_url, client) - } - None => fabro_github::GitHubContext::new(credentials, github_api_base_url), - }; - let (username, password) = - fabro_github::resolve_clone_credentials(&context, &repo.owner, &repo.name).await?; - Ok(Some(GitAuthConfig::new(username, password))) -} - -fn basic_auth_header(username: &str, password: &str) -> String { - basic_auth_header_from_encoded(&BASE64_STANDARD.encode(format!("{username}:{password}"))) -} - -fn basic_auth_header_from_encoded(encoded_credentials: &str) -> String { - format!("AUTHORIZATION: basic {encoded_credentials}") -} - -#[derive(Debug, Clone, PartialEq, Eq)] -pub(crate) struct GitCommandPlan { - program: String, - args: Vec, - env: Vec<(String, String)>, - current_dir: Option, - timeout: Duration, - sensitive_values: Vec, -} - -impl GitCommandPlan { - fn new(args: impl IntoIterator>, timeout: Duration) -> Self { - Self { - program: "git".to_string(), - args: args.into_iter().map(Into::into).collect(), - env: vec![("GIT_TERMINAL_PROMPT".to_string(), "0".to_string())], - current_dir: None, - timeout, - sensitive_values: Vec::new(), - } - } - - fn current_dir(mut self, current_dir: impl Into) -> Self { - self.current_dir = Some(current_dir.into()); - self - } - - fn with_auth(mut self, clone_url: &str, auth: Option<&GitAuthConfig>) -> Self { - if let Some(auth) = auth { - self.env.extend(auth.git_env(clone_url)); - self.sensitive_values - .extend(auth.sensitive_values().iter().cloned()); - } - self - } - - fn env_value(&self, name: &str) -> Option<&str> { - self.env - .iter() - .find(|(key, _)| key == name) - .map(|(_, value)| value.as_str()) - } -} - -fn build_bare_clone_plan( - clone_url: &str, - bare_dir: &Path, - auth: Option<&GitAuthConfig>, -) -> GitCommandPlan { - GitCommandPlan::new( - [ - "clone".to_string(), - "--bare".to_string(), - "--depth".to_string(), - "1".to_string(), - clone_url.to_string(), - bare_dir.display().to_string(), - ], - GIT_CLONE_TIMEOUT, - ) - .with_auth(clone_url, auth) -} - -fn build_bare_fetch_plan( - bare_dir: &Path, - clone_url: &str, - ref_selector: &str, - auth: Option<&GitAuthConfig>, -) -> GitCommandPlan { - GitCommandPlan::new( - [ - "fetch".to_string(), - "--depth".to_string(), - "1".to_string(), - "origin".to_string(), - "--".to_string(), - ref_selector.to_string(), - ], - GIT_FETCH_TIMEOUT, - ) - .current_dir(bare_dir) - .with_auth(clone_url, auth) -} - -fn build_worktree_add_plan(bare_dir: &Path, worktree_dir: &Path) -> GitCommandPlan { - GitCommandPlan::new( - [ - "worktree".to_string(), - "add".to_string(), - "--detach".to_string(), - "--force".to_string(), - worktree_dir.display().to_string(), - "FETCH_HEAD".to_string(), - ], - GIT_WORKTREE_ADD_TIMEOUT, - ) - .current_dir(bare_dir) -} - -fn build_worktree_prune_plan(bare_dir: &Path) -> GitCommandPlan { - GitCommandPlan::new(["worktree", "prune"], GIT_WORKTREE_PRUNE_TIMEOUT).current_dir(bare_dir) -} - -fn build_rev_parse_fetch_head_plan(bare_dir: &Path) -> GitCommandPlan { - GitCommandPlan::new(["rev-parse", "FETCH_HEAD"], GIT_REV_PARSE_TIMEOUT).current_dir(bare_dir) -} - -async fn run_git_plan(plan: GitCommandPlan) -> Result, AutomationRunMaterializeError> { - let mut command = Command::new(&plan.program); - command.args(&plan.args); - command.envs(plan.env.iter().map(|(key, value)| (key, value))); - if let Some(current_dir) = plan.current_dir.as_ref() { - command.current_dir(current_dir); - } - command.kill_on_drop(true); - - let output = time::timeout(plan.timeout, command.output()) - .await - .map_err(|_| { - AutomationRunMaterializeError::CloneFailed(format!( - "{} timed out after {}s", - safe_command_label(&plan), - plan.timeout.as_secs() - )) - })? - .map_err(|err| { - AutomationRunMaterializeError::CloneFailed(format!( - "failed to run {}: {err}", - safe_command_label(&plan) - )) - })?; - - if output.status.success() { - return Ok(output.stdout); - } - - let stdout = String::from_utf8_lossy(&output.stdout); - let stderr = String::from_utf8_lossy(&output.stderr); - let mut message = format!( - "{} exited with status {}", - safe_command_label(&plan), - output.status - ); - if !stderr.trim().is_empty() { - message.push_str(": "); - message.push_str(stderr.trim()); - } else if !stdout.trim().is_empty() { - message.push_str(": "); - message.push_str(stdout.trim()); - } - Err(AutomationRunMaterializeError::CloneFailed( - redact_git_output(&message, &plan.sensitive_values), - )) -} - -fn safe_command_label(plan: &GitCommandPlan) -> String { - if plan.args.is_empty() { - plan.program.clone() - } else { - format!("{} {}", plan.program, plan.args.join(" ")) - } -} - -fn redact_git_output(text: &str, sensitive_values: &[String]) -> String { - let mut redacted = fabro_redact::redact_string(text); - for value in sensitive_values - .iter() - .map(String::as_str) - .filter(|value| !value.is_empty()) - { - redacted = redacted.replace(value, "REDACTED"); - } - redacted -} - fn render_error_chain(error: &(dyn std::error::Error + 'static)) -> String { collect_chain(error).join(": ") } #[derive(Debug)] pub(crate) struct ManifestFromCheckoutInput { - input: AutomationRunMaterializeInput, + workflow: String, + run_id: RunId, + user_settings_path: PathBuf, checkout_dir: PathBuf, - repo: GithubRepository, - checked_out_sha: Option, + git_context: ManifestGitContextInput, environment_defaults: MergeMap, } +#[derive(Debug)] +pub(crate) struct ManifestGitContextInput { + repo: GitHubRepositorySlug, + ref_selector: String, + checked_out_sha: String, +} + fn build_manifest_from_checkout( args: ManifestFromCheckoutInput, -) -> Result { +) -> Result { let ManifestFromCheckoutInput { - input, + workflow, + run_id, + user_settings_path, checkout_dir, - repo, - checked_out_sha, + git_context, environment_defaults, } = args; let built = fabro_manifest::build_run_manifest(ManifestBuildInput { - workflow: input.target.workflow.as_str().into(), + workflow: workflow.into(), cwd: checkout_dir, - run_id: Some(input.run_id), - user_settings_path: Some(input.user_settings_path), + run_id: Some(run_id), + user_settings_path: Some(user_settings_path), environment_defaults, ..ManifestBuildInput::default() }) @@ -613,35 +197,30 @@ fn build_manifest_from_checkout( let mut manifest = built.manifest; manifest.git = Some(GitContext { - origin_url: github_metadata_url(&repo), - branch: input.target.ref_selector, - sha: checked_out_sha, + origin_url: github_metadata_url(&git_context.repo), + branch: git_context.ref_selector, + sha: Some(git_context.checked_out_sha), dirty: DirtyStatus::Clean, push_outcome: PreRunPushOutcome::NotAttempted, }); let submitted_manifest_bytes = serde_json::to_vec(&manifest) - .with_context(|| { - format!( - "failed to serialize materialized manifest for automation {}", - input.automation_id.as_str() - ) - }) - .map_err(|err| AutomationRunMaterializeError::Manifest(err.to_string()))?; + .context("failed to serialize materialized run manifest") + .map_err(|err| RunMaterializeError::Manifest(err.to_string()))?; Ok(AutomationRunMaterialized { manifest, submitted_manifest_bytes, }) } -fn manifest_build_error(error: &anyhow::Error) -> AutomationRunMaterializeError { +fn manifest_build_error(error: &anyhow::Error) -> RunMaterializeError { if error.chain().any(|source| { source .downcast_ref::() .is_some_and(|err| matches!(err, fabro_config::Error::WorkflowNotFound(_))) }) { - AutomationRunMaterializeError::WorkflowNotFound(render_error_chain(error.as_ref())) + RunMaterializeError::WorkflowNotFound(render_error_chain(error.as_ref())) } else { - AutomationRunMaterializeError::Manifest(render_error_chain(error.as_ref())) + RunMaterializeError::Manifest(render_error_chain(error.as_ref())) } } @@ -654,7 +233,7 @@ pub struct TestAutomationRunMaterializer { #[cfg(any(test, feature = "test-support"))] struct TestAutomationRunMaterializerState { captured_inputs: Vec, - response: Result, + response: Result, } #[cfg(any(test, feature = "test-support"))] @@ -667,12 +246,10 @@ impl TestAutomationRunMaterializer { } pub fn fail_invalid_target(message: impl Into) -> Self { - Self::new(Err(AutomationRunMaterializeError::InvalidTarget( - message.into(), - ))) + Self::new(Err(RunMaterializeError::InvalidTarget(message.into()))) } - fn new(response: Result) -> Self { + fn new(response: Result) -> Self { Self { inner: std::sync::Arc::new(std::sync::Mutex::new(TestAutomationRunMaterializerState { captured_inputs: Vec::new(), @@ -700,7 +277,7 @@ impl AutomationRunMaterializer for TestAutomationRunMaterializer { async fn materialize( &self, input: AutomationRunMaterializeInput, - ) -> Result { + ) -> Result { let mut guard = self .inner .lock() @@ -719,22 +296,12 @@ mod tests { use std::collections::HashMap; use std::fs; - use std::path::Path; - use fabro_automation::{AutomationId, AutomationTarget}; use fabro_types::{DirtyStatus, PreRunPushOutcome, RunId}; use tempfile::TempDir; use super::*; - fn target(repository: &str, ref_selector: &str, workflow: &str) -> AutomationTarget { - AutomationTarget { - repository: repository.to_string(), - ref_selector: ref_selector.to_string(), - workflow: workflow.to_string(), - } - } - fn test_environment_defaults() -> MergeMap { MergeMap::from(HashMap::from([("default".to_string(), EnvironmentLayer { provider: Some("local".to_string()), @@ -743,152 +310,7 @@ mod tests { } #[test] - fn target_repository_urls_are_github_metadata_urls_without_credentials() { - let repo = parse_github_repository_slug("fabro-sh/fabro").expect("slug should parse"); - - assert_eq!(repo.owner, "fabro-sh"); - assert_eq!(repo.name, "fabro"); - assert_eq!( - github_clone_url(&repo), - "https://github.com/fabro-sh/fabro.git" - ); - assert_eq!( - github_metadata_url(&repo), - "https://github.com/fabro-sh/fabro" - ); - assert!(!github_clone_url(&repo).contains('@')); - } - - #[test] - fn target_repository_validation_rejects_non_github_owner_repo_shapes() { - for value in [ - "fabro-sh", - "https://github.com/fabro-sh/fabro", - "fabro-sh/fabro/extra", - "-owner/repo", - "owner/.git", - ] { - let error = parse_github_repository_slug(value).expect_err("invalid slug should fail"); - assert!( - error.to_string().contains("invalid automation target"), - "unexpected error for {value}: {error}" - ); - } - } - - #[test] - fn bare_cache_command_plans_use_argv_prompt_disable_and_timeouts() { - let repo = parse_github_repository_slug("fabro-sh/fabro").unwrap(); - let clone_url = github_clone_url(&repo); - let temp = TempDir::new().unwrap(); - let bare_dir = temp.path().join("fabro-sh/fabro.git"); - let worktree_dir = temp.path().join("worktree/repo"); - - let clone = build_bare_clone_plan(&clone_url, &bare_dir, None); - assert_eq!(clone.program, "git"); - assert_eq!(clone.args, vec![ - "clone", - "--bare", - "--depth", - "1", - "https://github.com/fabro-sh/fabro.git", - bare_dir.to_str().unwrap(), - ]); - assert_eq!(clone.timeout, Duration::from_mins(2)); - assert_eq!(clone.env_value("GIT_TERMINAL_PROMPT"), Some("0")); - - let fetch = build_bare_fetch_plan(&bare_dir, &clone_url, "feature/materialize", None); - assert_eq!(fetch.args, vec![ - "fetch", - "--depth", - "1", - "origin", - "--", - "feature/materialize", - ]); - assert_eq!(fetch.current_dir.as_deref(), Some(bare_dir.as_path())); - assert_eq!(fetch.timeout, Duration::from_mins(1)); - assert_eq!(fetch.env_value("GIT_TERMINAL_PROMPT"), Some("0")); - - let worktree = build_worktree_add_plan(&bare_dir, &worktree_dir); - assert_eq!(worktree.args, vec![ - "worktree", - "add", - "--detach", - "--force", - worktree_dir.to_str().unwrap(), - "FETCH_HEAD", - ]); - assert_eq!(worktree.current_dir.as_deref(), Some(bare_dir.as_path())); - assert_eq!(worktree.timeout, Duration::from_secs(30)); - assert_eq!(worktree.env_value("GIT_TERMINAL_PROMPT"), Some("0")); - - let prune = build_worktree_prune_plan(&bare_dir); - assert_eq!(prune.args, vec!["worktree", "prune"]); - assert_eq!(prune.current_dir.as_deref(), Some(bare_dir.as_path())); - assert_eq!(prune.timeout, Duration::from_secs(10)); - - let rev_parse = build_rev_parse_fetch_head_plan(&bare_dir); - assert_eq!(rev_parse.args, vec!["rev-parse", "FETCH_HEAD"]); - assert_eq!(rev_parse.current_dir.as_deref(), Some(bare_dir.as_path())); - assert_eq!(rev_parse.timeout, Duration::from_secs(10)); - } - - #[test] - fn credential_redaction_removes_tokens_and_basic_auth_headers() { - let secret = "ghu_materializer_secret"; - let basic = basic_auth_header("x-access-token", secret); - let message = format!( - "fatal: could not read Username for https://github.com/fabro-sh/fabro.git; token={secret}; header={basic}" - ); - - let redacted = redact_git_output(&message, &[secret.to_string(), basic.clone()]); - - assert!(!redacted.contains(secret), "token leaked: {redacted}"); - assert!( - !redacted.contains(&basic), - "basic header leaked: {redacted}" - ); - let encoded_secret = BASE64_STANDARD.encode(format!("x-access-token:{secret}")); - assert!( - !redacted.contains(&encoded_secret), - "encoded credential leaked: {redacted}" - ); - assert!( - redacted.contains("REDACTED"), - "expected redaction marker: {redacted}" - ); - } - - #[test] - fn credential_config_env_keeps_clone_url_uncredentialed() { - let repo = parse_github_repository_slug("fabro-sh/fabro").unwrap(); - let clone_url = github_clone_url(&repo); - let auth = GitAuthConfig::new( - Some("x-access-token".to_string()), - Some("ghu_secret".to_string()), - ); - let plan = build_bare_clone_plan(&clone_url, Path::new("/tmp/fabro-checkout"), Some(&auth)); - - assert!( - plan.args - .iter() - .any(|arg| arg == "https://github.com/fabro-sh/fabro.git") - ); - assert!(plan.args.iter().all(|arg| !arg.contains("ghu_secret"))); - assert_eq!(plan.env_value("GIT_CONFIG_COUNT"), Some("1")); - assert_eq!( - plan.env_value("GIT_CONFIG_KEY_0"), - Some("http.https://github.com/fabro-sh/fabro.git.extraheader") - ); - assert!( - plan.env_value("GIT_CONFIG_VALUE_0") - .is_some_and(|value| value.starts_with("AUTHORIZATION: basic ")) - ); - } - - #[test] - fn workflow_path_resolution_builds_manifest_from_checkout_directory() { + fn manifest_builder_uses_checkout_for_workflow_and_separate_git_context() { let temp = TempDir::new().unwrap(); let checkout = temp.path().join("checkout"); let workflow_dir = checkout.join(".fabro/workflows/demo"); @@ -907,20 +329,19 @@ mod tests { let user_settings_path = temp.path().join("settings.toml"); fs::write(&user_settings_path, "_version = 1\n").unwrap(); let run_id = RunId::new(); - let repo = parse_github_repository_slug("fabro-sh/fabro").unwrap(); + let repo = parse_github_repository_slug("workspace-org/app").unwrap(); let sha = "0123456789abcdef0123456789abcdef01234567".to_string(); let materialized = build_manifest_from_checkout(ManifestFromCheckoutInput { - input: AutomationRunMaterializeInput { - automation_id: AutomationId::new("nightly").unwrap(), - target: target("fabro-sh/fabro", "main", "demo"), - run_id, - user_settings_path: user_settings_path.clone(), - temp_root: temp.path().to_path_buf(), - }, + workflow: "demo".to_string(), + run_id, + user_settings_path: user_settings_path.clone(), checkout_dir: checkout.clone(), - repo, - checked_out_sha: Some(sha.clone()), + git_context: ManifestGitContextInput { + repo, + ref_selector: "release".to_string(), + checked_out_sha: sha.clone(), + }, environment_defaults: test_environment_defaults(), }) .expect("manifest should build from checkout"); @@ -946,8 +367,8 @@ mod tests { .git .as_ref() .expect("git context should be set"); - assert_eq!(git.origin_url, "https://github.com/fabro-sh/fabro"); - assert_eq!(git.branch, "main"); + assert_eq!(git.origin_url, "https://github.com/workspace-org/app"); + assert_eq!(git.branch, "release"); assert_eq!(git.sha.as_deref(), Some(sha.as_str())); assert_eq!(git.dirty, DirtyStatus::Clean); assert_eq!(git.push_outcome, PreRunPushOutcome::NotAttempted); @@ -959,180 +380,4 @@ mod tests { serde_json::to_value(&materialized.manifest).unwrap() ); } - - fn seed_upstream(upstream: &Path) -> String { - std::process::Command::new("git") - .args([ - "init", - "--bare", - "--initial-branch=main", - upstream.to_str().unwrap(), - ]) - .status() - .expect("git init --bare"); - let work = upstream.parent().unwrap().join("seed"); - fs::create_dir_all(&work).unwrap(); - std::process::Command::new("git") - .args(["init", "--initial-branch=main", work.to_str().unwrap()]) - .status() - .expect("git init seed"); - let configure = |key: &str, value: &str| { - std::process::Command::new("git") - .args(["-C", work.to_str().unwrap(), "config", key, value]) - .status() - .expect("git config seed"); - }; - configure("user.email", "test@fabro.sh"); - configure("user.name", "Fabro Test"); - configure("commit.gpgsign", "false"); - fs::write(work.join("README.md"), "seed\n").unwrap(); - std::process::Command::new("git") - .args(["-C", work.to_str().unwrap(), "add", "."]) - .status() - .expect("git add seed"); - std::process::Command::new("git") - .args(["-C", work.to_str().unwrap(), "commit", "-m", "seed"]) - .status() - .expect("git commit seed"); - std::process::Command::new("git") - .args([ - "-C", - work.to_str().unwrap(), - "push", - upstream.to_str().unwrap(), - "main", - ]) - .status() - .expect("git push seed"); - let output = std::process::Command::new("git") - .args(["-C", work.to_str().unwrap(), "rev-parse", "HEAD"]) - .output() - .expect("git rev-parse seed"); - String::from_utf8(output.stdout).unwrap().trim().to_string() - } - - fn objects_signature(dir: &Path) -> Vec<(String, u64)> { - let mut files = Vec::new(); - let mut stack = vec![dir.join("objects")]; - while let Some(path) = stack.pop() { - let Ok(iter) = fs::read_dir(&path) else { - continue; - }; - for entry in iter.flatten() { - let entry_path = entry.path(); - let Ok(meta) = entry.metadata() else { continue }; - if meta.is_dir() { - stack.push(entry_path); - } else if meta.is_file() { - let rel = entry_path - .strip_prefix(dir.join("objects")) - .unwrap() - .to_string_lossy() - .into_owned(); - files.push((rel, meta.len())); - } - } - } - files.sort(); - files - } - - #[tokio::test] - async fn bare_clone_reused_across_calls() { - let temp = TempDir::new().unwrap(); - let upstream = temp.path().join("upstream.git"); - let expected_sha = seed_upstream(&upstream); - let cache = GitRepoCache::new(temp.path().join("cache")); - let repo = GithubRepository { - owner: "fabro-sh".to_string(), - name: "fabro".to_string(), - }; - let upstream_url = upstream.to_str().unwrap().to_string(); - - let worktree_a = temp.path().join("wt-a"); - let sha_a = cache - .prepare_worktree(WorktreePrepareInput { - repo: &repo, - clone_url: &upstream_url, - ref_selector: "main", - auth: None, - worktree_dir: &worktree_a, - }) - .await - .expect("first prepare_worktree"); - assert_eq!(sha_a, expected_sha); - let bare_dir = cache.bare_dir(&repo); - assert!(bare_dir.join("HEAD").exists(), "bare clone should exist"); - let signature_before = objects_signature(&bare_dir); - assert!( - !signature_before.is_empty(), - "expected object files after clone" - ); - - let worktree_b = temp.path().join("wt-b"); - let sha_b = cache - .prepare_worktree(WorktreePrepareInput { - repo: &repo, - clone_url: &upstream_url, - ref_selector: "main", - auth: None, - worktree_dir: &worktree_b, - }) - .await - .expect("second prepare_worktree"); - assert_eq!(sha_b, expected_sha); - let signature_after = objects_signature(&bare_dir); - assert_eq!( - signature_before, signature_after, - "second call should reuse the bare clone, not re-clone", - ); - } - - #[tokio::test] - async fn bare_clone_recovers_from_corruption() { - let temp = TempDir::new().unwrap(); - let upstream = temp.path().join("upstream.git"); - let expected_sha = seed_upstream(&upstream); - let cache = GitRepoCache::new(temp.path().join("cache")); - let repo = GithubRepository { - owner: "fabro-sh".to_string(), - name: "fabro".to_string(), - }; - let upstream_url = upstream.to_str().unwrap().to_string(); - - let worktree_a = temp.path().join("wt-a"); - cache - .prepare_worktree(WorktreePrepareInput { - repo: &repo, - clone_url: &upstream_url, - ref_selector: "main", - auth: None, - worktree_dir: &worktree_a, - }) - .await - .expect("first prepare_worktree"); - - // Simulate corruption by truncating HEAD. - let bare_dir = cache.bare_dir(&repo); - fs::write(bare_dir.join("HEAD"), "").unwrap(); - - let worktree_b = temp.path().join("wt-b"); - let sha = cache - .prepare_worktree(WorktreePrepareInput { - repo: &repo, - clone_url: &upstream_url, - ref_selector: "main", - auth: None, - worktree_dir: &worktree_b, - }) - .await - .expect("prepare_worktree should recover from corruption"); - assert_eq!(sha, expected_sha); - assert!(bare_dir.join("HEAD").exists()); - assert!( - !fs::read_to_string(bare_dir.join("HEAD")) - .unwrap() - .is_empty() - ); - } } diff --git a/lib/crates/fabro-server/src/git_checkout.rs b/lib/crates/fabro-server/src/git_checkout.rs new file mode 100644 index 000000000..8abe77a62 --- /dev/null +++ b/lib/crates/fabro-server/src/git_checkout.rs @@ -0,0 +1,768 @@ +use std::path::{Path, PathBuf}; +use std::time::Duration; + +use base64::Engine as _; +use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; +use fabro_automation::GitHubRepositorySlug; +use fabro_store::KeyedMutex; +use tokio::process::Command; +use tokio::{fs, time}; + +const GIT_CLONE_TIMEOUT: Duration = Duration::from_mins(2); +const GIT_FETCH_TIMEOUT: Duration = Duration::from_mins(1); +const GIT_WORKTREE_ADD_TIMEOUT: Duration = Duration::from_secs(30); +const GIT_WORKTREE_PRUNE_TIMEOUT: Duration = Duration::from_secs(10); +const GIT_REV_PARSE_TIMEOUT: Duration = Duration::from_secs(10); + +/// Error returned while preparing a checkout from a git source. +#[derive(thiserror::Error, Debug, Clone, PartialEq, Eq)] +pub(crate) enum GitCheckoutError { + #[error("invalid repository target: {0}")] + InvalidTarget(String), + #[error("failed to clone repository: {0}")] + CloneFailed(String), +} + +/// Persistent on-disk cache of bare GitHub clones, one per `(owner, repo)`. +/// +/// Materializing an automation run only needs to read the workflow + its +/// supporting files out of the repo at a given ref. Cloning fresh on every +/// click costs 5-15s on the request thread. With this cache, the first +/// materialize for a repo pays the clone, and every subsequent one pays only +/// the delta `git fetch` plus a cheap `git worktree add` into the per-call +/// scratch dir. +#[derive(Debug)] +pub(crate) struct GitRepoCache { + cache_root: PathBuf, + locks: KeyedMutex<(String, String)>, +} + +impl GitRepoCache { + pub(crate) fn new(cache_root: PathBuf) -> Self { + Self { + cache_root, + locks: KeyedMutex::new(), + } + } + + fn bare_dir(&self, repo: &GitHubRepositorySlug) -> PathBuf { + self.cache_root + .join(repo.owner()) + .join(format!("{}.git", repo.repo())) + } + + /// Prepare a worktree containing the requested ref of `repo` at + /// `worktree_dir`. Returns the resolved commit SHA. + /// + /// First call for a repo: a `--bare --depth 1` clone is created at + /// `//.git`. Subsequent calls reuse the bare clone + /// and only `git fetch --depth 1` the requested ref. In both cases a + /// short-lived worktree is added at `worktree_dir`; the caller owns its + /// lifetime (typically a `TempDir`). Stale worktree admin entries from + /// crashed prior calls are pruned at the start of each call so they do + /// not accumulate. + pub(crate) async fn prepare_worktree( + &self, + args: WorktreePrepareInput<'_>, + ) -> Result { + let clone_url = github_clone_url(args.repo); + self.prepare_worktree_with_clone_url(args, &clone_url).await + } + + async fn prepare_worktree_with_clone_url( + &self, + args: WorktreePrepareInput<'_>, + clone_url: &str, + ) -> Result { + let _guard = self + .locks + .lock((args.repo.owner().to_string(), args.repo.repo().to_string())) + .await; + let bare_dir = self.bare_dir(args.repo); + + match self.try_prepare_worktree(&bare_dir, &args, clone_url).await { + Ok(sha) => Ok(sha), + Err(first_err) if bare_clone_may_be_corrupt(&bare_dir).await => { + // Best-effort wipe and one retry. Corruption is rare; auth + // and network failures don't trip this branch because + // `bare_clone_may_be_corrupt` only returns true when the + // bare repo's `HEAD` file is missing or empty. + let _ = fs::remove_dir_all(&bare_dir).await; + self.try_prepare_worktree(&bare_dir, &args, clone_url) + .await + .map_err(|_| first_err) + } + Err(err) => Err(err), + } + } + + async fn try_prepare_worktree( + &self, + bare_dir: &Path, + args: &WorktreePrepareInput<'_>, + clone_url: &str, + ) -> Result { + let bare_exists = fs::try_exists(&bare_dir.join("HEAD")) + .await + .unwrap_or(false); + if bare_exists { + // Drop any worktree admin entries whose working trees were + // deleted by previous `TempDir` cleanup. Cheap and idempotent. + let _ = run_git_plan(build_worktree_prune_plan(bare_dir)).await; + } else { + if let Some(parent) = bare_dir.parent() { + fs::create_dir_all(parent).await.map_err(|err| { + GitCheckoutError::CloneFailed(format!( + "failed to create cache dir {}: {err}", + parent.display() + )) + })?; + } + run_git_plan(build_bare_clone_plan(clone_url, bare_dir, args.auth)).await?; + } + + run_git_plan(build_bare_fetch_plan( + bare_dir, + clone_url, + args.ref_selector, + args.auth, + )) + .await?; + + let checked_out_sha = run_git_plan(build_rev_parse_fetch_head_plan(bare_dir)) + .await + .map(|stdout| String::from_utf8_lossy(&stdout).trim().to_string())?; + + run_git_plan(build_worktree_add_plan(bare_dir, args.worktree_dir)).await?; + + Ok(checked_out_sha) + } +} + +pub(crate) struct WorktreePrepareInput<'a> { + pub repo: &'a GitHubRepositorySlug, + pub ref_selector: &'a str, + pub auth: Option<&'a GitAuthConfig>, + pub worktree_dir: &'a Path, +} + +async fn bare_clone_may_be_corrupt(bare_dir: &Path) -> bool { + match fs::metadata(&bare_dir.join("HEAD")).await { + Ok(meta) => meta.len() == 0, + Err(_) => bare_dir.exists(), + } +} + +pub(crate) fn parse_github_repository_slug( + value: &str, +) -> Result { + fabro_automation::parse_github_repository_slug(value) + .map_err(|err| GitCheckoutError::InvalidTarget(err.to_string())) +} + +fn github_clone_url(repo: &GitHubRepositorySlug) -> String { + format!("https://github.com/{}/{}.git", repo.owner(), repo.repo()) +} + +pub(crate) fn github_metadata_url(repo: &GitHubRepositorySlug) -> String { + format!("https://github.com/{}/{}", repo.owner(), repo.repo()) +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct GitAuthConfig { + extraheader: Option, + sensitive_values: Vec, +} + +impl GitAuthConfig { + fn new(username: Option, password: Option) -> Self { + let Some(password) = password.filter(|value| !value.is_empty()) else { + return Self { + extraheader: None, + sensitive_values: Vec::new(), + }; + }; + let username = username + .filter(|value| !value.is_empty()) + .unwrap_or_else(|| "x-access-token".to_string()); + let encoded_credentials = BASE64_STANDARD.encode(format!("{username}:{password}")); + let extraheader = basic_auth_header_from_encoded(&encoded_credentials); + Self { + sensitive_values: vec![password, encoded_credentials, extraheader.clone()], + extraheader: Some(extraheader), + } + } + + fn git_env(&self, clone_url: &str) -> Vec<(String, String)> { + let Some(extraheader) = self.extraheader.as_ref() else { + return Vec::new(); + }; + vec![ + ("GIT_CONFIG_COUNT".to_string(), "1".to_string()), + ( + "GIT_CONFIG_KEY_0".to_string(), + format!("http.{clone_url}.extraheader"), + ), + ("GIT_CONFIG_VALUE_0".to_string(), extraheader.clone()), + ] + } + + fn sensitive_values(&self) -> &[String] { + &self.sensitive_values + } +} + +pub(crate) async fn resolve_git_auth_config( + credentials: Option<&fabro_github::GitHubCredentials>, + repo: &GitHubRepositorySlug, + github_api_base_url: &str, + http_client: Option, +) -> anyhow::Result> { + let Some(credentials) = credentials else { + return Ok(None); + }; + let context = match http_client { + Some(client) => { + fabro_github::GitHubContext::with_http_client(credentials, github_api_base_url, client) + } + None => fabro_github::GitHubContext::new(credentials, github_api_base_url), + }; + let (username, password) = + fabro_github::resolve_clone_credentials(&context, repo.owner(), repo.repo()).await?; + Ok(Some(GitAuthConfig::new(username, password))) +} + +#[cfg(test)] +fn basic_auth_header(username: &str, password: &str) -> String { + basic_auth_header_from_encoded(&BASE64_STANDARD.encode(format!("{username}:{password}"))) +} + +fn basic_auth_header_from_encoded(encoded_credentials: &str) -> String { + format!("AUTHORIZATION: basic {encoded_credentials}") +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct GitCommandPlan { + program: String, + args: Vec, + env: Vec<(String, String)>, + current_dir: Option, + timeout: Duration, + sensitive_values: Vec, +} + +impl GitCommandPlan { + fn new(args: impl IntoIterator>, timeout: Duration) -> Self { + Self { + program: "git".to_string(), + args: args.into_iter().map(Into::into).collect(), + env: vec![("GIT_TERMINAL_PROMPT".to_string(), "0".to_string())], + current_dir: None, + timeout, + sensitive_values: Vec::new(), + } + } + + fn current_dir(mut self, current_dir: impl Into) -> Self { + self.current_dir = Some(current_dir.into()); + self + } + + fn with_auth(mut self, clone_url: &str, auth: Option<&GitAuthConfig>) -> Self { + if let Some(auth) = auth { + self.env.extend(auth.git_env(clone_url)); + self.sensitive_values + .extend(auth.sensitive_values().iter().cloned()); + } + self + } + + #[cfg(test)] + fn env_value(&self, name: &str) -> Option<&str> { + self.env + .iter() + .find(|(key, _)| key == name) + .map(|(_, value)| value.as_str()) + } +} + +fn build_bare_clone_plan( + clone_url: &str, + bare_dir: &Path, + auth: Option<&GitAuthConfig>, +) -> GitCommandPlan { + GitCommandPlan::new( + [ + "clone".to_string(), + "--bare".to_string(), + "--depth".to_string(), + "1".to_string(), + clone_url.to_string(), + bare_dir.display().to_string(), + ], + GIT_CLONE_TIMEOUT, + ) + .with_auth(clone_url, auth) +} + +fn build_bare_fetch_plan( + bare_dir: &Path, + clone_url: &str, + ref_selector: &str, + auth: Option<&GitAuthConfig>, +) -> GitCommandPlan { + GitCommandPlan::new( + [ + "fetch".to_string(), + "--depth".to_string(), + "1".to_string(), + "origin".to_string(), + "--".to_string(), + ref_selector.to_string(), + ], + GIT_FETCH_TIMEOUT, + ) + .current_dir(bare_dir) + .with_auth(clone_url, auth) +} + +fn build_worktree_add_plan(bare_dir: &Path, worktree_dir: &Path) -> GitCommandPlan { + GitCommandPlan::new( + [ + "worktree".to_string(), + "add".to_string(), + "--detach".to_string(), + "--force".to_string(), + worktree_dir.display().to_string(), + "FETCH_HEAD".to_string(), + ], + GIT_WORKTREE_ADD_TIMEOUT, + ) + .current_dir(bare_dir) +} + +fn build_worktree_prune_plan(bare_dir: &Path) -> GitCommandPlan { + GitCommandPlan::new(["worktree", "prune"], GIT_WORKTREE_PRUNE_TIMEOUT).current_dir(bare_dir) +} + +fn build_rev_parse_fetch_head_plan(bare_dir: &Path) -> GitCommandPlan { + GitCommandPlan::new(["rev-parse", "FETCH_HEAD"], GIT_REV_PARSE_TIMEOUT).current_dir(bare_dir) +} + +async fn run_git_plan(plan: GitCommandPlan) -> Result, GitCheckoutError> { + let mut command = Command::new(&plan.program); + command.args(&plan.args); + command.envs(plan.env.iter().map(|(key, value)| (key, value))); + if let Some(current_dir) = plan.current_dir.as_ref() { + command.current_dir(current_dir); + } + command.kill_on_drop(true); + + let output = time::timeout(plan.timeout, command.output()) + .await + .map_err(|_| { + GitCheckoutError::CloneFailed(format!( + "{} timed out after {}s", + safe_command_label(&plan), + plan.timeout.as_secs() + )) + })? + .map_err(|err| { + GitCheckoutError::CloneFailed(format!( + "failed to run {}: {err}", + safe_command_label(&plan) + )) + })?; + + if output.status.success() { + return Ok(output.stdout); + } + + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + let mut message = format!( + "{} exited with status {}", + safe_command_label(&plan), + output.status + ); + if !stderr.trim().is_empty() { + message.push_str(": "); + message.push_str(stderr.trim()); + } else if !stdout.trim().is_empty() { + message.push_str(": "); + message.push_str(stdout.trim()); + } + Err(GitCheckoutError::CloneFailed(redact_git_output( + &message, + &plan.sensitive_values, + ))) +} + +fn safe_command_label(plan: &GitCommandPlan) -> String { + if plan.args.is_empty() { + plan.program.clone() + } else { + format!("{} {}", plan.program, plan.args.join(" ")) + } +} + +fn redact_git_output(text: &str, sensitive_values: &[String]) -> String { + let mut redacted = fabro_redact::redact_string(text); + for value in sensitive_values + .iter() + .map(String::as_str) + .filter(|value| !value.is_empty()) + { + redacted = redacted.replace(value, "REDACTED"); + } + redacted +} + +#[cfg(test)] +mod tests { + #![expect( + clippy::disallowed_methods, + reason = "Git checkout unit tests build local git repositories and inspect temp files synchronously." + )] + + use std::fs; + use std::path::Path; + + use tempfile::TempDir; + + use super::*; + + #[test] + fn target_repository_urls_are_github_metadata_urls_without_credentials() { + let repo = parse_github_repository_slug("fabro-sh/fabro").expect("slug should parse"); + + assert_eq!(repo.owner(), "fabro-sh"); + assert_eq!(repo.repo(), "fabro"); + assert_eq!( + github_clone_url(&repo), + "https://github.com/fabro-sh/fabro.git" + ); + assert_eq!( + github_metadata_url(&repo), + "https://github.com/fabro-sh/fabro" + ); + assert!(!github_clone_url(&repo).contains('@')); + } + + #[test] + fn target_repository_validation_rejects_non_github_owner_repo_shapes() { + for value in [ + "fabro-sh", + "https://github.com/fabro-sh/fabro", + "fabro-sh/fabro/extra", + "-owner/repo", + ] { + let error = parse_github_repository_slug(value).expect_err("invalid slug should fail"); + assert!( + error.to_string().contains("invalid repository target"), + "unexpected error for {value}: {error}" + ); + } + } + + #[test] + fn target_repository_validation_matches_automation_validation() { + let repo = parse_github_repository_slug("owner/.github").expect("slug should parse"); + + assert_eq!(repo.owner(), "owner"); + assert_eq!(repo.repo(), ".github"); + assert_eq!( + github_metadata_url(&repo), + "https://github.com/owner/.github" + ); + } + + #[test] + fn bare_cache_command_plans_use_argv_prompt_disable_and_timeouts() { + let repo = parse_github_repository_slug("fabro-sh/fabro").unwrap(); + let clone_url = github_clone_url(&repo); + let temp = TempDir::new().unwrap(); + let bare_dir = temp.path().join("fabro-sh/fabro.git"); + let worktree_dir = temp.path().join("worktree/repo"); + + let clone = build_bare_clone_plan(&clone_url, &bare_dir, None); + assert_eq!(clone.program, "git"); + assert_eq!(clone.args, vec![ + "clone", + "--bare", + "--depth", + "1", + "https://github.com/fabro-sh/fabro.git", + bare_dir.to_str().unwrap(), + ]); + assert_eq!(clone.timeout, Duration::from_mins(2)); + assert_eq!(clone.env_value("GIT_TERMINAL_PROMPT"), Some("0")); + + let fetch = build_bare_fetch_plan(&bare_dir, &clone_url, "feature/materialize", None); + assert_eq!(fetch.args, vec![ + "fetch", + "--depth", + "1", + "origin", + "--", + "feature/materialize", + ]); + assert_eq!(fetch.current_dir.as_deref(), Some(bare_dir.as_path())); + assert_eq!(fetch.timeout, Duration::from_mins(1)); + assert_eq!(fetch.env_value("GIT_TERMINAL_PROMPT"), Some("0")); + + let worktree = build_worktree_add_plan(&bare_dir, &worktree_dir); + assert_eq!(worktree.args, vec![ + "worktree", + "add", + "--detach", + "--force", + worktree_dir.to_str().unwrap(), + "FETCH_HEAD", + ]); + assert_eq!(worktree.current_dir.as_deref(), Some(bare_dir.as_path())); + assert_eq!(worktree.timeout, Duration::from_secs(30)); + assert_eq!(worktree.env_value("GIT_TERMINAL_PROMPT"), Some("0")); + + let prune = build_worktree_prune_plan(&bare_dir); + assert_eq!(prune.args, vec!["worktree", "prune"]); + assert_eq!(prune.current_dir.as_deref(), Some(bare_dir.as_path())); + assert_eq!(prune.timeout, Duration::from_secs(10)); + + let rev_parse = build_rev_parse_fetch_head_plan(&bare_dir); + assert_eq!(rev_parse.args, vec!["rev-parse", "FETCH_HEAD"]); + assert_eq!(rev_parse.current_dir.as_deref(), Some(bare_dir.as_path())); + assert_eq!(rev_parse.timeout, Duration::from_secs(10)); + } + + #[test] + fn credential_redaction_removes_tokens_and_basic_auth_headers() { + let secret = "ghu_materializer_secret"; + let basic = basic_auth_header("x-access-token", secret); + let message = format!( + "fatal: could not read Username for https://github.com/fabro-sh/fabro.git; token={secret}; header={basic}" + ); + + let redacted = redact_git_output(&message, &[secret.to_string(), basic.clone()]); + + assert!(!redacted.contains(secret), "token leaked: {redacted}"); + assert!( + !redacted.contains(&basic), + "basic header leaked: {redacted}" + ); + let encoded_secret = BASE64_STANDARD.encode(format!("x-access-token:{secret}")); + assert!( + !redacted.contains(&encoded_secret), + "encoded credential leaked: {redacted}" + ); + assert!( + redacted.contains("REDACTED"), + "expected redaction marker: {redacted}" + ); + } + + #[test] + fn credential_config_env_keeps_clone_url_uncredentialed() { + let repo = parse_github_repository_slug("fabro-sh/fabro").unwrap(); + let clone_url = github_clone_url(&repo); + let auth = GitAuthConfig::new( + Some("x-access-token".to_string()), + Some("ghu_secret".to_string()), + ); + let plan = build_bare_clone_plan(&clone_url, Path::new("/tmp/fabro-checkout"), Some(&auth)); + + assert!( + plan.args + .iter() + .any(|arg| arg == "https://github.com/fabro-sh/fabro.git") + ); + assert!(plan.args.iter().all(|arg| !arg.contains("ghu_secret"))); + assert_eq!(plan.env_value("GIT_CONFIG_COUNT"), Some("1")); + assert_eq!( + plan.env_value("GIT_CONFIG_KEY_0"), + Some("http.https://github.com/fabro-sh/fabro.git.extraheader") + ); + assert!( + plan.env_value("GIT_CONFIG_VALUE_0") + .is_some_and(|value| value.starts_with("AUTHORIZATION: basic ")) + ); + } + + fn seed_upstream(upstream: &Path) -> String { + std::process::Command::new("git") + .args([ + "init", + "--bare", + "--initial-branch=main", + upstream.to_str().unwrap(), + ]) + .status() + .expect("git init --bare"); + let work = upstream.parent().unwrap().join("seed"); + fs::create_dir_all(&work).unwrap(); + std::process::Command::new("git") + .args(["init", "--initial-branch=main", work.to_str().unwrap()]) + .status() + .expect("git init seed"); + let configure = |key: &str, value: &str| { + std::process::Command::new("git") + .args(["-C", work.to_str().unwrap(), "config", key, value]) + .status() + .expect("git config seed"); + }; + configure("user.email", "test@fabro.sh"); + configure("user.name", "Fabro Test"); + configure("commit.gpgsign", "false"); + fs::write(work.join("README.md"), "seed\n").unwrap(); + std::process::Command::new("git") + .args(["-C", work.to_str().unwrap(), "add", "."]) + .status() + .expect("git add seed"); + std::process::Command::new("git") + .args(["-C", work.to_str().unwrap(), "commit", "-m", "seed"]) + .status() + .expect("git commit seed"); + std::process::Command::new("git") + .args([ + "-C", + work.to_str().unwrap(), + "push", + upstream.to_str().unwrap(), + "main", + ]) + .status() + .expect("git push seed"); + let output = std::process::Command::new("git") + .args(["-C", work.to_str().unwrap(), "rev-parse", "HEAD"]) + .output() + .expect("git rev-parse seed"); + String::from_utf8(output.stdout).unwrap().trim().to_string() + } + + fn objects_signature(dir: &Path) -> Vec<(String, u64)> { + let mut files = Vec::new(); + let mut stack = vec![dir.join("objects")]; + while let Some(path) = stack.pop() { + let Ok(iter) = fs::read_dir(&path) else { + continue; + }; + for entry in iter.flatten() { + let entry_path = entry.path(); + let Ok(meta) = entry.metadata() else { continue }; + if meta.is_dir() { + stack.push(entry_path); + } else if meta.is_file() { + let rel = entry_path + .strip_prefix(dir.join("objects")) + .unwrap() + .to_string_lossy() + .into_owned(); + files.push((rel, meta.len())); + } + } + } + files.sort(); + files + } + + #[tokio::test] + async fn bare_clone_reused_across_calls() { + let temp = TempDir::new().unwrap(); + let upstream = temp.path().join("upstream.git"); + let expected_sha = seed_upstream(&upstream); + let cache = GitRepoCache::new(temp.path().join("cache")); + let repo = parse_github_repository_slug("fabro-sh/fabro").unwrap(); + let upstream_url = upstream.to_str().unwrap().to_string(); + + let worktree_a = temp.path().join("wt-a"); + let sha_a = cache + .prepare_worktree_with_clone_url( + WorktreePrepareInput { + repo: &repo, + ref_selector: "main", + auth: None, + worktree_dir: &worktree_a, + }, + &upstream_url, + ) + .await + .expect("first prepare_worktree"); + assert_eq!(sha_a, expected_sha); + let bare_dir = cache.bare_dir(&repo); + assert!(bare_dir.join("HEAD").exists(), "bare clone should exist"); + let signature_before = objects_signature(&bare_dir); + assert!( + !signature_before.is_empty(), + "expected object files after clone" + ); + + let worktree_b = temp.path().join("wt-b"); + let sha_b = cache + .prepare_worktree_with_clone_url( + WorktreePrepareInput { + repo: &repo, + ref_selector: "main", + auth: None, + worktree_dir: &worktree_b, + }, + &upstream_url, + ) + .await + .expect("second prepare_worktree"); + assert_eq!(sha_b, expected_sha); + let signature_after = objects_signature(&bare_dir); + assert_eq!( + signature_before, signature_after, + "second call should reuse the bare clone, not re-clone", + ); + } + + #[tokio::test] + async fn bare_clone_recovers_from_corruption() { + let temp = TempDir::new().unwrap(); + let upstream = temp.path().join("upstream.git"); + let expected_sha = seed_upstream(&upstream); + let cache = GitRepoCache::new(temp.path().join("cache")); + let repo = parse_github_repository_slug("fabro-sh/fabro").unwrap(); + let upstream_url = upstream.to_str().unwrap().to_string(); + + let worktree_a = temp.path().join("wt-a"); + cache + .prepare_worktree_with_clone_url( + WorktreePrepareInput { + repo: &repo, + ref_selector: "main", + auth: None, + worktree_dir: &worktree_a, + }, + &upstream_url, + ) + .await + .expect("first prepare_worktree"); + + // Simulate corruption by truncating HEAD. + let bare_dir = cache.bare_dir(&repo); + fs::write(bare_dir.join("HEAD"), "").unwrap(); + + let worktree_b = temp.path().join("wt-b"); + let sha = cache + .prepare_worktree_with_clone_url( + WorktreePrepareInput { + repo: &repo, + ref_selector: "main", + auth: None, + worktree_dir: &worktree_b, + }, + &upstream_url, + ) + .await + .expect("prepare_worktree should recover from corruption"); + assert_eq!(sha, expected_sha); + assert!(bare_dir.join("HEAD").exists()); + assert!( + !fs::read_to_string(bare_dir.join("HEAD")) + .unwrap() + .is_empty() + ); + } +} diff --git a/lib/crates/fabro-server/src/lib.rs b/lib/crates/fabro-server/src/lib.rs index 187f42ab0..42c6fad15 100644 --- a/lib/crates/fabro-server/src/lib.rs +++ b/lib/crates/fabro-server/src/lib.rs @@ -25,6 +25,7 @@ pub mod csp; mod demo; pub mod diagnostics; pub mod error; +mod git_checkout; pub mod github_webhooks; pub mod install; mod interp; diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 2bc7cb946..9078b3615 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -138,11 +138,12 @@ use ulid::Ulid; use crate::auth::{self, GithubEndpoints, auth_translation_middleware, demo_routing_middleware}; use crate::automation_materializer::{ - AutomationRunMaterializeError, AutomationRunMaterializeInput, AutomationRunMaterialized, - AutomationRunMaterializer, GitRepoCache, ProductionAutomationRunMaterializer, + AutomationRunMaterializeInput, AutomationRunMaterialized, AutomationRunMaterializer, + ProductionAutomationRunMaterializer, RunMaterializeError, }; use crate::canonical_origin::{canonical_origin_from_effective_web_url, effective_web_url}; use crate::error::ApiError; +use crate::git_checkout::GitRepoCache; use crate::github_webhooks::{ WEBHOOK_ROUTE, WEBHOOK_SECRET_ENV, parse_event_metadata, verify_signature, }; @@ -1118,7 +1119,7 @@ impl AppState { pub(crate) async fn materialize_automation_run( &self, input: AutomationRunMaterializeInput, - ) -> Result { + ) -> Result { #[cfg(any(test, feature = "test-support"))] if let Some(materializer) = self.automation_materializer_override.as_ref() { return materializer.materialize(input).await;