From 52ff7b2ef2778a6e6b46d2ffc54bc8cda91bbf83 Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Sat, 29 Aug 2026 09:10:39 -0400 Subject: [PATCH] Add local RunIntent producer support --- .../fabro-server/src/manifest_validation.rs | 210 +++++- lib/apps/fabro-server/src/run_intent.rs | 135 +++- lib/apps/fabro-server/src/run_manifest.rs | 5 +- lib/components/fabro-manifest/src/lib.rs | 242 ++++++- .../src/local_workflow_package.rs | 609 ++++++++++++++++++ .../fabro-manifest/src/workflow_bundler.rs | 85 ++- .../src/workflow_version_collector.rs | 52 +- 7 files changed, 1288 insertions(+), 50 deletions(-) create mode 100644 lib/components/fabro-manifest/src/local_workflow_package.rs diff --git a/lib/apps/fabro-server/src/manifest_validation.rs b/lib/apps/fabro-server/src/manifest_validation.rs index acc5e05e1..28e9928ae 100644 --- a/lib/apps/fabro-server/src/manifest_validation.rs +++ b/lib/apps/fabro-server/src/manifest_validation.rs @@ -1,11 +1,14 @@ use std::collections::HashMap; +use std::path::PathBuf; -use anyhow::Result; +use anyhow::{Result, anyhow}; use fabro_api::types; -use fabro_config::RunLayer; +use fabro_config::{RunLayer, WorkflowSettingsBuilder}; +use fabro_manifest::CollectedWorkflowClosure; +use fabro_workflow::operations::{ValidateInput, WorkflowInput, validate}; use fabro_workflow::pipeline::TEMPLATE_UNDEFINED_VARIABLE_RULE; -use crate::run_manifest; +use crate::{run_intent, run_manifest}; /// Validate a manifest without a model catalog. /// @@ -28,6 +31,54 @@ pub fn validate_manifest( Ok(run_manifest::validate_response(&prepared, &validated)) } +/// Validate an already collected local workflow before any version upload. +/// +/// The supplied run layer is complete, including any already resolved inline +/// goal. Validation uses only seeded environment defaults, immutable workflow +/// settings, and explicit inputs; it performs no store, HTTP, user-settings, +/// project-settings, or model-catalog operation. Undefined template variables +/// are promoted to errors before the response is returned. +pub fn validate_collected_workflow( + closure: &CollectedWorkflowClosure, + run_overrides: Option<&RunLayer>, + input_overrides: &HashMap, +) -> Result { + let lowered = run_intent::lower_collected_workflow_closure(closure)?; + let workflow = lowered + .workflow_bundle + .workflow(&lowered.entrypoint) + .cloned() + .ok_or_else(|| anyhow!("lowered root workflow is missing from its bundle"))?; + let mut builder = WorkflowSettingsBuilder::new() + .server_manifest_defaults( + RunLayer::default(), + fabro_environment::seeded_catalog_layer(), + ) + .server_mcp_catalog(HashMap::new()); + if let Some(run) = run_overrides { + builder = builder.run_overrides(run.clone()); + } + if let Some(layer) = lowered.workflow_layer { + builder = builder.workflow_layer(layer); + } + let mut settings = builder.build().map_err(anyhow::Error::new)?; + settings.run.inputs.extend(input_overrides.clone()); + let validated = validate(ValidateInput { + workflow: WorkflowInput::Bundled(workflow), + settings, + vars: HashMap::new(), + cwd: PathBuf::from("/workspace"), + custom_transforms: Vec::new(), + }) + .map_err(anyhow::Error::new)?; + let mut response = types::ValidateResponse { + ok: !validated.has_errors(), + workflow: run_manifest::workflow_summary(&validated, lowered.entrypoint.as_path()), + }; + promote_template_undefined_variables_to_errors(&mut response); + Ok(response) +} + pub fn promote_template_undefined_variables_to_errors(response: &mut types::ValidateResponse) { let mut promoted = false; for diagnostic in &mut response.workflow.diagnostics { @@ -40,3 +91,156 @@ pub fn promote_template_undefined_variables_to_errors(response: &mut types::Vali response.ok = false; } } + +#[cfg(test)] +mod tests { + #![expect( + clippy::disallowed_methods, + reason = "collected-validation tests write isolated workflow fixtures synchronously" + )] + + use std::fs; + use std::path::{Path, PathBuf}; + + use fabro_config::RunGoalLayer; + use fabro_types::settings::InterpString; + + use super::*; + + fn write(root: &Path, path: &str, content: &str) { + let path = root.join(path); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, content).unwrap(); + } + + fn write_complete_fixture(root: &Path) -> PathBuf { + write( + root, + "root/workflow.toml", + r#"_version = 1 +[workflow] +graph = "workflow.fabro" +[run.goal] +file = "goal.md" +[run.environment.image] +dockerfile = { path = "Dockerfile" } +"#, + ); + write( + root, + "root/workflow.fabro", + r#"digraph Root { + start [shape=Mdiamond] + imported [import="imports/shared.fabro"] + task [prompt="@prompts/task.md", model="future-provider/future-model"] + child [stack.child_workflow="../child/workflow.fabro"] + exit [shape=Msquare] + start -> imported -> task -> child -> exit + }"#, + ); + write( + root, + "root/imports/shared.fabro", + "digraph Shared { start [shape=Mdiamond] shared [prompt=\"shared\"] exit \ + [shape=Msquare] start -> shared -> exit }", + ); + write( + root, + "root/prompts/task.md", + "Hello {{ inputs.owner }}. {% include \"detail.md\" %}", + ); + write(root, "root/prompts/detail.md", "detail"); + write(root, "root/goal.md", "workflow goal"); + write(root, "root/Dockerfile", "FROM alpine\n"); + write( + root, + "child/workflow.fabro", + "digraph Child { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", + ); + root.join("root/workflow.toml") + } + + fn run_overrides(goal: &str) -> RunLayer { + RunLayer { + goal: Some(RunGoalLayer::Inline(InterpString::parse(goal))), + ..RunLayer::default() + } + } + + #[test] + fn collected_validation_matches_legacy_response_for_equivalent_inputs() { + let temp = tempfile::tempdir().unwrap(); + let workflow = write_complete_fixture(temp.path()); + let run = run_overrides("inline goal"); + let inputs = HashMap::from([("owner".to_string(), toml::Value::String("Ada".to_string()))]); + let package = fabro_manifest::resolve_local_workflow_package( + &workflow, + temp.path(), + Some(temp.path()), + ) + .unwrap(); + let manifest = fabro_manifest::build_run_manifest(fabro_manifest::ManifestBuildInput { + workflow, + cwd: temp.path().to_path_buf(), + run_overrides: Some(run.clone()), + input_overrides: inputs.clone(), + args: Some(types::ManifestArgs { + input: vec!["owner=Ada".to_string()], + ..types::ManifestArgs::default() + }), + environment_defaults: fabro_environment::seeded_catalog_layer(), + ..fabro_manifest::ManifestBuildInput::default() + }) + .unwrap(); + + let mut legacy = validate_manifest(&RunLayer::default(), &manifest.manifest).unwrap(); + promote_template_undefined_variables_to_errors(&mut legacy); + let collected = + validate_collected_workflow(package.closure(), Some(&run), &inputs).unwrap(); + + assert_eq!( + serde_json::to_value(collected).unwrap(), + serde_json::to_value(legacy).unwrap(), + ); + } + + #[test] + fn collected_validation_promotes_undefined_inputs_and_accepts_explicit_values() { + let temp = tempfile::tempdir().unwrap(); + let workflow = write_complete_fixture(temp.path()); + let package = fabro_manifest::resolve_local_workflow_package( + &workflow, + temp.path(), + Some(temp.path()), + ) + .unwrap(); + + let missing = + validate_collected_workflow(package.closure(), None, &HashMap::new()).unwrap(); + assert!(!missing.ok); + assert!(missing.workflow.diagnostics.iter().any(|diagnostic| { + diagnostic.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE + && diagnostic.severity == types::WorkflowDiagnosticSeverity::Error + })); + + let present = validate_collected_workflow( + package.closure(), + Some(&run_overrides("resolved inline goal")), + &HashMap::from([("owner".to_string(), toml::Value::String("Ada".to_string()))]), + ) + .unwrap(); + assert!( + present + .workflow + .diagnostics + .iter() + .all(|diagnostic| { diagnostic.rule != TEMPLATE_UNDEFINED_VARIABLE_RULE }) + ); + assert!(present.ok); + assert_eq!(present.workflow.goal, "resolved inline goal"); + assert!(present.workflow.diagnostics.iter().all(|diagnostic| { + !diagnostic.message.contains("future-provider") + && !diagnostic.message.contains("future-model") + })); + } +} diff --git a/lib/apps/fabro-server/src/run_intent.rs b/lib/apps/fabro-server/src/run_intent.rs index 425392159..a872019b5 100644 --- a/lib/apps/fabro-server/src/run_intent.rs +++ b/lib/apps/fabro-server/src/run_intent.rs @@ -4,10 +4,11 @@ use std::path::{Component, Path, PathBuf}; use fabro_config::parse::SettingsSource; use fabro_config::{EnvironmentLayer, RunEnvironmentLayer, RunGoalLayer, SettingsLayer}; use fabro_environment::{EnvironmentId, EnvironmentValidationError}; +use fabro_manifest::CollectedWorkflowClosure; use fabro_types::settings::InterpString; use fabro_types::{ GitContext, ManifestPath, RunTarget, SandboxProviderKind, TargetValidationError, WorkflowPath, - WorkflowVersionId, + WorkflowVersion, WorkflowVersionId, }; use fabro_workflow::git; use fabro_workflow::workflow_bundle::{BundledWorkflow, ParsedWorkflowConfig, WorkflowBundle}; @@ -183,8 +184,69 @@ pub(crate) enum WorkflowClosureLoweringError { }, } -pub(crate) fn lower_workflow_closure( - closure: &LoadedWorkflowVersionClosure, +trait WorkflowClosureView { + fn root_id(&self) -> WorkflowVersionId; + fn root(&self) -> &WorkflowVersion; + fn validated_root(&self) -> &ValidatedWorkflowVersion; + fn get(&self, id: &WorkflowVersionId) -> Option<&WorkflowVersion>; +} + +impl WorkflowClosureView for LoadedWorkflowVersionClosure { + fn root_id(&self) -> WorkflowVersionId { + self.root_id() + } + + fn root(&self) -> &WorkflowVersion { + self.root() + } + + fn validated_root(&self) -> &ValidatedWorkflowVersion { + self.validated_root() + } + + fn get(&self, id: &WorkflowVersionId) -> Option<&WorkflowVersion> { + self.get(id) + } +} + +struct CollectedWorkflowClosureView<'a> { + root_id: WorkflowVersionId, + versions: HashMap, +} + +impl<'a> CollectedWorkflowClosureView<'a> { + fn new(closure: &'a CollectedWorkflowClosure) -> Result { + let root_id = closure.root_id(); + let versions = closure.versions().collect::>(); + if !versions.contains_key(&root_id) { + return Err(WorkflowClosureLoweringError::MissingVersion { id: root_id }); + } + Ok(Self { root_id, versions }) + } +} + +impl WorkflowClosureView for CollectedWorkflowClosureView<'_> { + fn root_id(&self) -> WorkflowVersionId { + self.root_id + } + + fn root(&self) -> &WorkflowVersion { + self.validated_root().version() + } + + fn validated_root(&self) -> &ValidatedWorkflowVersion { + self.versions + .get(&self.root_id) + .expect("collected closure view construction verifies its root") + } + + fn get(&self, id: &WorkflowVersionId) -> Option<&WorkflowVersion> { + self.versions.get(id).map(|version| version.version()) + } +} + +fn lower_workflow_closure_view( + closure: &impl WorkflowClosureView, ) -> Result { let entrypoint = manifest_path(closure.root().entrypoint(), closure.root().entrypoint())?; let mut mounts = HashMap::new(); @@ -227,6 +289,19 @@ pub(crate) fn lower_workflow_closure( }) } +pub(crate) fn lower_workflow_closure( + closure: &LoadedWorkflowVersionClosure, +) -> Result { + lower_workflow_closure_view(closure) +} + +pub(crate) fn lower_collected_workflow_closure( + closure: &CollectedWorkflowClosure, +) -> Result { + let view = CollectedWorkflowClosureView::new(closure)?; + lower_workflow_closure_view(&view) +} + pub(crate) fn pin_workflow_environment_authority(layer: &mut SettingsLayer, environment_id: &str) { // Both blocks destructure without `..` so adding a field to either layer // type forces a compile-time decision here: server-owned facts are @@ -262,7 +337,7 @@ pub(crate) fn pin_workflow_environment_authority(layer: &mut SettingsLayer, envi } fn mount_version( - closure: &LoadedWorkflowVersionClosure, + closure: &impl WorkflowClosureView, id: WorkflowVersionId, mounted_entrypoint: ManifestPath, mounts: &mut HashMap, @@ -460,6 +535,58 @@ mod tests { .unwrap() } + #[tokio::test] + async fn collected_and_stored_closures_lower_through_the_same_path() { + let temp = tempfile::tempdir().unwrap(); + let root = temp.path().join("root"); + let child = temp.path().join("child"); + std::fs::create_dir_all(&root).unwrap(); + std::fs::create_dir_all(&child).unwrap(); + fs::write( + root.join("workflow.toml"), + "_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n[run.goal]\nfile = \"goal.md\"\n", + ) + .await + .unwrap(); + fs::write( + root.join("workflow.fabro"), + "digraph Root { child [stack.child_workflow=\"../child/workflow.fabro\"] }", + ) + .await + .unwrap(); + fs::write(root.join("goal.md"), "Ship it").await.unwrap(); + fs::write(child.join("workflow.fabro"), "digraph Child {}") + .await + .unwrap(); + + let collected = + fabro_manifest::collect_workflow_versions(&root.join("workflow.toml"), temp.path()) + .unwrap(); + let (database, _) = crate::test_support::test_store_bundle(); + let store = WorkflowVersionStore::new(database.blobs()); + for (_, version) in collected.versions() { + store.put(version).await.unwrap(); + } + let stored = store + .get_closure(&collected.root_id()) + .await + .unwrap() + .unwrap(); + + let from_collected = lower_collected_workflow_closure(&collected).unwrap(); + let from_stored = lower_workflow_closure(&stored).unwrap(); + + assert_eq!(from_collected.entrypoint, from_stored.entrypoint); + assert_eq!( + serde_json::to_value(from_collected.workflow_bundle.workflows()).unwrap(), + serde_json::to_value(from_stored.workflow_bundle.workflows()).unwrap(), + ); + assert_eq!( + format!("{:?}", from_collected.workflow_layer), + format!("{:?}", from_stored.workflow_layer), + ); + } + #[tokio::test] async fn prepares_a_canonical_folder_target_without_git_projection() { let dir = tempfile::tempdir().unwrap(); diff --git a/lib/apps/fabro-server/src/run_manifest.rs b/lib/apps/fabro-server/src/run_manifest.rs index 809dd34df..ef7056fb1 100644 --- a/lib/apps/fabro-server/src/run_manifest.rs +++ b/lib/apps/fabro-server/src/run_manifest.rs @@ -1563,7 +1563,10 @@ fn preflight_response( } } -fn workflow_summary(validated: &Validated, target_path: &Path) -> types::PreflightWorkflowSummary { +pub(crate) fn workflow_summary( + validated: &Validated, + target_path: &Path, +) -> types::PreflightWorkflowSummary { types::PreflightWorkflowSummary { diagnostics: diagnostics_to_api(validated.diagnostics()), edges: i64::try_from(validated.graph().edges.len()) diff --git a/lib/components/fabro-manifest/src/lib.rs b/lib/components/fabro-manifest/src/lib.rs index b4612df28..3352a91cf 100644 --- a/lib/components/fabro-manifest/src/lib.rs +++ b/lib/components/fabro-manifest/src/lib.rs @@ -3,6 +3,7 @@ reason = "CLI manifest builder: sync file I/O building install manifests" )] +mod local_workflow_package; mod workflow_bundler; mod workflow_version_collector; @@ -24,11 +25,16 @@ use fabro_template::validate_static_reference; use fabro_types::graph::ReferenceKind; use fabro_types::settings::interp::InterpString; use fabro_types::settings::run::{ApprovalMode, ResolvedGoalSource, ResolvedRunGoal, RunMode}; -use fabro_types::{DirtyStatus, GitContext, ManifestPath, WorkflowSettings}; +use fabro_types::{ + DirtyStatus, GitContext, GitHubRepositorySlug, GitRunTarget, ManifestPath, WorkflowSettings, +}; use fabro_workflow::git::{ GitSyncStatus, branch_needs_push, head_sha, push_branch_noninteractive, sync_status, }; +pub use crate::local_workflow_package::{ + LocalWorkflowPackageError, ResolvedLocalWorkflowPackage, resolve_local_workflow_package, +}; use crate::workflow_bundler::WorkflowBundler; pub use crate::workflow_version_collector::{ CollectedWorkflowClosure, WorkflowVersionCollectError, collect_workflow_versions, @@ -210,7 +216,8 @@ pub fn build_run_manifest(input: ManifestBuildInput) -> Result { )?; let configured_repo_origin_url = configured_repo_origin_url(&workflow_settings); - let git = build_git_context(&working_directory, configured_repo_origin_url.as_deref()); + let git = observe_git_run_target(&working_directory, configured_repo_origin_url.as_deref()) + .map(GitRunTargetObservation::into_legacy_git_context); let args = input.args.filter(|args| !manifest_args_is_empty(args)); Ok(BuiltManifest { @@ -300,11 +307,57 @@ fn resolved_goal_to_manifest(resolved: ResolvedRunGoal) -> types::ManifestGoal { } } -fn build_git_context( +/// Facts observed from one usable attached local Git checkout. +/// +/// The optional target is absent when the checkout's effective origin is not +/// a canonical GitHub repository. Its SHA is present only when local tracking +/// state or the existing safe best-effort push proves that exact commit is +/// remotely available. The legacy projection retains the historical local +/// SHA and normalized-origin behavior independently. +#[derive(Clone, Debug)] +pub struct GitRunTargetObservation { + run_target: Option, + legacy_git_context: GitContext, +} + +impl GitRunTargetObservation { + #[must_use] + pub fn run_target(&self) -> Option<&GitRunTarget> { + self.run_target.as_ref() + } + + #[must_use] + pub fn into_run_target(self) -> Option { + self.run_target + } + + #[must_use] + pub fn dirty(&self) -> DirtyStatus { + self.legacy_git_context.dirty + } + + #[must_use] + pub fn legacy_git_context(&self) -> &GitContext { + &self.legacy_git_context + } + + #[must_use] + pub fn into_legacy_git_context(self) -> GitContext { + self.legacy_git_context + } +} + +/// Observe Git facts without choosing an environment or a non-Git target. +/// +/// Outer `None` means `repo_path` is not a usable attached checkout. A +/// returned observation with no target means Git facts were available but the +/// effective origin was not a canonical GitHub repository. +#[must_use] +pub fn observe_git_run_target( repo_path: &Path, configured_repo_origin_url: Option<&str>, -) -> Option { - let (origin_url, branch) = detect_manifest_repo_info(repo_path)?; +) -> Option { + let (origin_url, push_origin_url, branch) = detect_manifest_repo_info(repo_path)?; let sha = head_sha(repo_path).ok(); let dirty = match sync_status(repo_path, "origin", Some(&branch)) { GitSyncStatus::Dirty => DirtyStatus::Dirty, @@ -320,17 +373,36 @@ fn build_git_context( .filter(|url| !url.is_empty()) }) .unwrap_or_default(); - push_manifest_branch_best_effort( + let remotely_available = push_manifest_branch_best_effort( repo_path, &branch, - origin_url.as_deref(), + push_origin_url.as_deref(), configured_repo_origin_url, ); - Some(GitContext { - origin_url: repo_origin_url, - branch, + let run_target = github_run_target( + &repo_origin_url, + &branch, + remotely_available.then(|| sha.clone()).flatten(), + ); + Some(GitRunTargetObservation { + run_target, + legacy_git_context: GitContext { + origin_url: repo_origin_url, + branch, + sha, + dirty, + }, + }) +} + +fn github_run_target(origin_url: &str, branch: &str, sha: Option) -> Option { + let (owner, repository) = fabro_github::parse_github_owner_repo(origin_url).ok()?; + let slug = GitHubRepositorySlug::try_new(&format!("{owner}/{repository}"))?; + Some(GitRunTarget { + repo: slug.to_string(), + branch: branch.to_owned(), + tag: None, sha, - dirty, }) } @@ -353,14 +425,30 @@ fn configured_repo_origin_url(settings: &WorkflowSettings) -> Option { (!normalized.is_empty()).then_some(normalized) } -fn detect_manifest_repo_info(repo_path: &Path) -> Option<(Option, String)> { +fn detect_manifest_repo_info(repo_path: &Path) -> Option<(Option, Option, String)> { let repo = git2::Repository::discover(repo_path).ok()?; - let branch = repo.head().ok()?.shorthand().map(ToOwned::to_owned)?; + if repo.is_bare() { + return None; + } + let head = repo.head().ok()?; + if !head.is_branch() { + return None; + } + let branch = head.shorthand().map(ToOwned::to_owned)?; let origin_url = repo .find_remote("origin") .ok() .and_then(|remote| remote.url().map(ToOwned::to_owned)); - Some((origin_url, branch)) + // Keep the legacy observed URL above, but compare configured repository + // identity against the remote's raw config bytes. This lets a repository- + // local `url.*.insteadOf` redirect the existing push safely without making + // the rewrite target look like a different repository. + let push_origin_url = repo + .config() + .ok() + .and_then(|config| config.get_string("remote.origin.url").ok()) + .or_else(|| origin_url.clone()); + Some((origin_url, push_origin_url, branch)) } /// Best-effort push of the local branch so clone-based execution can see @@ -372,9 +460,9 @@ fn push_manifest_branch_best_effort( branch: &str, origin_url: Option<&str>, configured_repo_origin_url: Option<&str>, -) { +) -> bool { let Some(origin_url) = origin_url else { - return; + return false; }; if let Some(repo_origin_url) = configured_repo_origin_url @@ -383,15 +471,15 @@ fn push_manifest_branch_best_effort( { let remote = fabro_github::normalize_repo_origin_url(origin_url); if remote != repo_origin_url { - return; + return false; } } if !branch_needs_push(repo_path, "origin", branch) { - return; + return true; } - let _ = push_branch_noninteractive(repo_path, "origin", branch); + push_branch_noninteractive(repo_path, "origin", branch).is_ok() } /// Resolve a workflow reference and reject it when neither its config nor @@ -1627,6 +1715,122 @@ exit 1 assert_eq!(std::fs::read_to_string(helper_log).unwrap(), "0\n"); } + #[test] + fn observes_synced_github_branch_as_an_exact_target() { + let temp = tempfile::tempdir().unwrap(); + let workspace = temp.path().join("workspace"); + std::fs::create_dir_all(&workspace).unwrap(); + init_git_repo(&workspace, "feature", "https://github.com/acme/widgets.git"); + mark_origin_branch_synced(&workspace, "feature"); + + let observation = observe_git_run_target(&workspace, None).unwrap(); + let target = observation.run_target().unwrap(); + let legacy = observation.legacy_git_context(); + + assert_eq!(target.repo, "acme/widgets"); + assert_eq!(target.branch, "feature"); + assert_eq!(target.tag, None); + assert_eq!(target.sha, legacy.sha); + assert_eq!(observation.dirty(), DirtyStatus::Clean); + assert_eq!(legacy.origin_url, "https://github.com/acme/widgets"); + } + + #[test] + fn observes_exact_sha_only_after_a_successful_noninteractive_push() { + let temp = tempfile::tempdir().unwrap(); + let workspace = temp.path().join("workspace"); + std::fs::create_dir_all(&workspace).unwrap(); + let bare_origin = init_bare_origin(temp.path()); + init_git_repo(&workspace, "feature", "https://github.com/acme/widgets"); + let local_url = format!("file://{}", bare_origin.display()); + run_git(&workspace, &[ + "config", + &format!("url.{local_url}.insteadOf"), + "https://github.com/acme/widgets", + ]); + + let observation = + observe_git_run_target(&workspace, Some("https://github.com/acme/widgets")).unwrap(); + let target = observation.run_target().unwrap(); + + assert_eq!(target.sha, observation.legacy_git_context().sha); + assert_eq!(bare_remote_branch_sha(&bare_origin, "feature"), target.sha,); + } + + #[test] + fn failed_push_and_origin_mismatch_produce_branch_only_targets() { + let temp = tempfile::tempdir().unwrap(); + let failed_workspace = temp.path().join("failed"); + std::fs::create_dir_all(&failed_workspace).unwrap(); + init_git_repo( + &failed_workspace, + "feature", + "https://github.com/acme/widgets", + ); + let missing_url = format!("file://{}/missing.git", temp.path().display()); + run_git(&failed_workspace, &[ + "config", + &format!("url.{missing_url}.insteadOf"), + "https://github.com/acme/widgets", + ]); + + let failed = + observe_git_run_target(&failed_workspace, Some("https://github.com/acme/widgets")) + .unwrap(); + assert_eq!(failed.run_target().unwrap().sha, None); + assert!(failed.legacy_git_context().sha.is_some()); + assert_eq!(failed.dirty(), DirtyStatus::Clean); + + let mismatched_workspace = temp.path().join("mismatched"); + std::fs::create_dir_all(&mismatched_workspace).unwrap(); + let bare_origin = init_bare_origin(&temp.path().join("other")); + init_git_repo( + &mismatched_workspace, + "feature", + bare_origin.to_str().unwrap(), + ); + + let mismatched = observe_git_run_target( + &mismatched_workspace, + Some("https://github.com/acme/configured"), + ) + .unwrap(); + let target = mismatched.run_target().unwrap(); + assert_eq!(target.repo, "acme/configured"); + assert_eq!(target.sha, None); + assert_eq!(bare_remote_branch_sha(&bare_origin, "feature"), None); + } + + #[test] + fn git_observation_distinguishes_dirty_unsupported_and_unusable_checkouts() { + let temp = tempfile::tempdir().unwrap(); + let workspace = temp.path().join("workspace"); + std::fs::create_dir_all(&workspace).unwrap(); + let bare_origin = init_bare_origin(temp.path()); + init_git_repo(&workspace, "feature", bare_origin.to_str().unwrap()); + std::fs::write(workspace.join("dirty.txt"), "dirty").unwrap(); + + let unsupported = observe_git_run_target(&workspace, None).unwrap(); + assert!(unsupported.run_target().is_none()); + assert_eq!(unsupported.dirty(), DirtyStatus::Dirty); + assert_eq!( + unsupported.legacy_git_context().origin_url, + fabro_github::normalize_repo_origin_url(&bare_origin.to_string_lossy()), + ); + + let not_repo = temp.path().join("not-repo"); + std::fs::create_dir_all(¬_repo).unwrap(); + assert!(observe_git_run_target(¬_repo, None).is_none()); + + let unborn = temp.path().join("unborn"); + std::fs::create_dir_all(&unborn).unwrap(); + run_git(&unborn, &["init", "--quiet"]); + assert!(observe_git_run_target(&unborn, None).is_none()); + + run_git(&workspace, &["checkout", "--detach", "--quiet"]); + assert!(observe_git_run_target(&workspace, None).is_none()); + } + fn init_git_repo(path: &Path, branch: &str, origin_url: &str) { run_git(path, &[ "-c", diff --git a/lib/components/fabro-manifest/src/local_workflow_package.rs b/lib/components/fabro-manifest/src/local_workflow_package.rs new file mode 100644 index 000000000..36d47c095 --- /dev/null +++ b/lib/components/fabro-manifest/src/local_workflow_package.rs @@ -0,0 +1,609 @@ +#![expect( + clippy::result_large_err, + reason = "the public error contract preserves concrete config and collection source errors" +)] + +use std::path::{Path, PathBuf}; + +use fabro_config::project::{WorkflowLocation, discover_project_config}; +use thiserror::Error; + +use crate::workflow_version_collector::collect_workflow_versions_at_location; +use crate::{CollectedWorkflowClosure, WorkflowVersionCollectError}; + +/// One local workflow resolved to a canonical package root and collected +/// exactly once. +#[derive(Debug)] +pub struct ResolvedLocalWorkflowPackage { + workflow_location: WorkflowLocation, + source_root: PathBuf, + closure: CollectedWorkflowClosure, +} + +/// Failure while resolving and collecting one local workflow package. +#[derive(Debug, Error)] +pub enum LocalWorkflowPackageError { + #[error("failed to resolve local workflow `{workflow}`")] + Resolve { + workflow: PathBuf, + #[source] + source: fabro_config::Error, + }, + #[error("failed to inspect a Git worktree for local workflow `{workflow}`")] + Repository { + workflow: PathBuf, + #[source] + source: git2::Error, + }, + #[error("failed to canonicalize local workflow package path `{path}`")] + Canonicalize { + path: PathBuf, + #[source] + source: std::io::Error, + }, + #[error("local workflow package path `{path}` escapes source root `{source_root}`")] + EscapesSourceRoot { + path: PathBuf, + source_root: PathBuf, + }, + #[error("failed to collect local workflow package `{workflow}`")] + Collect { + workflow: PathBuf, + #[source] + source: WorkflowVersionCollectError, + }, +} + +impl ResolvedLocalWorkflowPackage { + #[must_use] + pub fn workflow_location(&self) -> &WorkflowLocation { + &self.workflow_location + } + + #[must_use] + pub fn source_root(&self) -> &Path { + &self.source_root + } + + #[must_use] + pub fn closure(&self) -> &CollectedWorkflowClosure { + &self.closure + } + + #[must_use] + pub fn into_closure(self) -> CollectedWorkflowClosure { + self.closure + } +} + +/// Resolve producer-readable workflow bytes under one stable local source +/// root, then collect one immutable closure. +/// +/// Named workflows prefer the current checkout's `.fabro/workflows` tree, +/// preserve marked-project discovery, and use `user_workflows_root` only when +/// it is explicitly supplied. Explicit paths use their containing Git +/// worktree, the supplied user root, or their own containing directory, in +/// that order. No ambient home or process-current-directory state is read. +pub fn resolve_local_workflow_package( + workflow: &Path, + cwd: &Path, + user_workflows_root: Option<&Path>, +) -> Result { + let (location, source_root) = if is_workflow_name(workflow) { + resolve_named_workflow(workflow, cwd, user_workflows_root)? + } else { + resolve_explicit_workflow(workflow, cwd, user_workflows_root)? + }; + let source_root = canonicalize(&source_root)?; + let workflow_location = canonicalize_location(location)?; + ensure_location_is_within_root(&workflow_location, &source_root)?; + let closure = collect_workflow_versions_at_location(&workflow_location, &source_root, workflow) + .map_err(|source| LocalWorkflowPackageError::Collect { + workflow: workflow.to_path_buf(), + source, + })?; + + Ok(ResolvedLocalWorkflowPackage { + workflow_location, + source_root, + closure, + }) +} + +fn is_workflow_name(workflow: &Path) -> bool { + workflow.extension().is_none() && workflow.components().count() == 1 +} + +fn resolve_named_workflow( + workflow: &Path, + cwd: &Path, + user_workflows_root: Option<&Path>, +) -> Result<(WorkflowLocation, PathBuf), LocalWorkflowPackageError> { + let current_root = match worktree_root(cwd, workflow)? { + Some(root) => root, + None => canonicalize(cwd)?, + }; + let project_candidate = current_root + .join(".fabro/workflows") + .join(workflow) + .join("workflow.toml"); + if project_candidate.is_file() { + return resolve_at(workflow, &project_candidate, current_root); + } + + let marked_project = + discover_project_config(cwd).map_err(|source| LocalWorkflowPackageError::Resolve { + workflow: workflow.to_path_buf(), + source, + })?; + if let Some(config) = marked_project { + let fabro_root = config + .parent() + .expect("a discovered project config has a parent"); + let candidate = fabro_root + .join("workflows") + .join(workflow) + .join("workflow.toml"); + if candidate.is_file() { + let project_root = fabro_root + .parent() + .expect("the .fabro directory has a project parent"); + let source_root = match worktree_root(&candidate, workflow)? { + Some(root) => root, + None => canonicalize(project_root)?, + }; + return resolve_at(workflow, &candidate, source_root); + } + } + + if let Some(user_root) = user_workflows_root { + let candidate = user_root.join(workflow).join("workflow.toml"); + if candidate.is_file() { + return resolve_at(workflow, &candidate, canonicalize(user_root)?); + } + } + + Err(LocalWorkflowPackageError::Resolve { + workflow: workflow.to_path_buf(), + source: fabro_config::Error::WorkflowNotFound(workflow.display().to_string()), + }) +} + +fn resolve_explicit_workflow( + workflow: &Path, + cwd: &Path, + user_workflows_root: Option<&Path>, +) -> Result<(WorkflowLocation, PathBuf), LocalWorkflowPackageError> { + let selected = if workflow.extension().is_none() { + let path = if workflow.is_absolute() { + workflow.to_path_buf() + } else { + cwd.join(workflow) + }; + if path.is_dir() { + path.join("workflow.toml") + } else { + return Err(LocalWorkflowPackageError::Resolve { + workflow: workflow.to_path_buf(), + source: fabro_config::Error::WorkflowNotFound(workflow.display().to_string()), + }); + } + } else { + workflow.to_path_buf() + }; + let location = resolve_location(workflow, &selected, cwd)?; + if let Some(root) = worktree_root(&location.graph, workflow)? { + return Ok((location, root)); + } + + if let Some(user_root) = user_workflows_root.filter(|root| root.exists()) { + let canonical_user_root = canonicalize(user_root)?; + let canonical_graph = canonicalize(&location.graph)?; + if canonical_graph.starts_with(&canonical_user_root) { + return Ok((location, canonical_user_root)); + } + } + + let source_root = location.dir.clone(); + Ok((location, source_root)) +} + +fn resolve_at( + workflow: &Path, + selected: &Path, + source_root: PathBuf, +) -> Result<(WorkflowLocation, PathBuf), LocalWorkflowPackageError> { + let selected = if selected.is_absolute() { + selected.to_path_buf() + } else { + canonicalize(selected)? + }; + let resolve_from = selected.parent().unwrap_or_else(|| Path::new(".")); + resolve_location(workflow, &selected, resolve_from).map(|location| (location, source_root)) +} + +fn resolve_location( + workflow: &Path, + selected: &Path, + cwd: &Path, +) -> Result { + let location = WorkflowLocation::resolve(selected, cwd).map_err(|source| { + LocalWorkflowPackageError::Resolve { + workflow: workflow.to_path_buf(), + source, + } + })?; + if location.toml.is_none() && !location.graph.is_file() { + return Err(LocalWorkflowPackageError::Resolve { + workflow: workflow.to_path_buf(), + source: fabro_config::Error::WorkflowNotFound(location.graph.display().to_string()), + }); + } + Ok(location) +} + +fn worktree_root( + path: &Path, + workflow: &Path, +) -> Result, LocalWorkflowPackageError> { + let discover_from = if path.is_dir() { + path + } else { + path.parent().unwrap_or(path) + }; + let repository = match git2::Repository::discover(discover_from) { + Ok(repository) => repository, + Err(source) if source.code() == git2::ErrorCode::NotFound => return Ok(None), + Err(source) => { + return Err(LocalWorkflowPackageError::Repository { + workflow: workflow.to_path_buf(), + source, + }); + } + }; + if repository.is_bare() { + return Ok(None); + } + Ok(repository.workdir().map(Path::to_path_buf)) +} + +fn canonicalize(path: &Path) -> Result { + path.canonicalize() + .map_err(|source| LocalWorkflowPackageError::Canonicalize { + path: path.to_path_buf(), + source, + }) +} + +fn canonicalize_location( + location: WorkflowLocation, +) -> Result { + let graph = canonicalize(&location.graph)?; + let toml = location.toml.as_deref().map(canonicalize).transpose()?; + let dir = graph + .parent() + .expect("a canonical workflow graph has a parent") + .to_path_buf(); + Ok(WorkflowLocation { + dir, + graph, + toml, + slug: location.slug, + }) +} + +fn ensure_location_is_within_root( + location: &WorkflowLocation, + source_root: &Path, +) -> Result<(), LocalWorkflowPackageError> { + for path in std::iter::once(&location.graph).chain(location.toml.iter()) { + if !path.starts_with(source_root) { + return Err(LocalWorkflowPackageError::EscapesSourceRoot { + path: path.clone(), + source_root: source_root.to_path_buf(), + }); + } + } + Ok(()) +} + +#[cfg(test)] +mod tests { + #![expect( + clippy::disallowed_methods, + reason = "local-package tests build isolated filesystem and Git fixtures synchronously" + )] + + use std::fs; + use std::path::{Path, PathBuf}; + + use crate::{LocalWorkflowPackageError, resolve_local_workflow_package}; + + fn write(root: &Path, path: &str, content: &str) { + let path = root.join(path); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, content).unwrap(); + } + + fn write_workflow(root: &Path, directory: &str, graph: &str) -> PathBuf { + write( + root, + &format!("{directory}/workflow.toml"), + "_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n", + ); + write(root, &format!("{directory}/workflow.fabro"), graph); + root.join(directory).join("workflow.toml") + } + + fn init_repo(path: &Path) { + git2::Repository::init(path).unwrap(); + } + + fn canonical_versions( + package: &crate::ResolvedLocalWorkflowPackage, + ) -> Vec<(fabro_types::WorkflowVersionId, Vec)> { + package + .closure() + .versions() + .map(|(id, version)| (id, version.version().canonical_bytes().unwrap())) + .collect() + } + + fn error_chain(error: &dyn std::error::Error) -> String { + let mut messages = vec![error.to_string()]; + let mut source = error.source(); + while let Some(error) = source { + messages.push(error.to_string()); + source = error.source(); + } + messages.join(": ") + } + + #[test] + fn named_markerless_project_workflow_precedes_explicit_user_root() { + let temp = tempfile::tempdir().unwrap(); + let project = temp.path().join("project"); + let user = temp.path().join("user-workflows"); + fs::create_dir_all(&project).unwrap(); + init_repo(&project); + write_workflow(&project, ".fabro/workflows/hello", "digraph Project {}"); + write_workflow(&user, "hello", "digraph User {}"); + + let package = + resolve_local_workflow_package(Path::new("hello"), &project, Some(&user)).unwrap(); + + assert_eq!(package.source_root(), project.canonicalize().unwrap()); + assert_eq!( + package.workflow_location().graph, + project + .join(".fabro/workflows/hello/workflow.fabro") + .canonicalize() + .unwrap(), + ); + assert_eq!( + package + .closure() + .versions() + .last() + .unwrap() + .1 + .version() + .entrypoint() + .as_str(), + ".fabro/workflows/hello/workflow.fabro", + ); + } + + #[test] + fn named_user_workflow_requires_and_uses_the_explicit_root() { + let temp = tempfile::tempdir().unwrap(); + let cwd = temp.path().join("cwd"); + let user = temp.path().join("user-workflows"); + fs::create_dir_all(&cwd).unwrap(); + write_workflow(&user, "hello", "digraph User {}"); + + let package = + resolve_local_workflow_package(Path::new("hello"), &cwd, Some(&user)).unwrap(); + assert_eq!(package.source_root(), user.canonicalize().unwrap()); + assert_eq!( + package + .closure() + .versions() + .last() + .unwrap() + .1 + .version() + .entrypoint() + .as_str(), + "hello/workflow.fabro", + ); + + let error = resolve_local_workflow_package(Path::new("hello"), &cwd, None).unwrap_err(); + assert!(matches!(error, LocalWorkflowPackageError::Resolve { .. })); + } + + #[test] + fn explicit_workflow_uses_its_own_checkout_and_has_location_independent_bytes() { + let temp = tempfile::tempdir().unwrap(); + let caller = temp.path().join("caller"); + let first = temp.path().join("first"); + let second = temp.path().join("second"); + for root in [&caller, &first, &second] { + fs::create_dir_all(root).unwrap(); + init_repo(root); + } + let first_workflow = write_workflow( + &first, + "flows/demo", + "digraph Demo { task [prompt=\"@prompt.md\"] }", + ); + write(&first, "flows/demo/prompt.md", "hello"); + let second_workflow = write_workflow( + &second, + "flows/demo", + "digraph Demo { task [prompt=\"@prompt.md\"] }", + ); + write(&second, "flows/demo/prompt.md", "hello"); + + let first_package = resolve_local_workflow_package(&first_workflow, &caller, None).unwrap(); + let second_package = + resolve_local_workflow_package(&second_workflow, &caller, None).unwrap(); + + assert_eq!(first_package.source_root(), first.canonicalize().unwrap()); + assert_eq!(second_package.source_root(), second.canonicalize().unwrap()); + assert_eq!( + canonical_versions(&first_package), + canonical_versions(&second_package) + ); + } + + #[test] + fn workflow_in_a_checkout_allows_parent_segments_that_stay_inside_the_root() { + let temp = tempfile::tempdir().unwrap(); + let package_root = temp.path().join("package"); + fs::create_dir_all(&package_root).unwrap(); + init_repo(&package_root); + let workflow = write_workflow( + &package_root, + "flows", + "digraph Demo { task [prompt=\"@../shared.md\"] }", + ); + write(&package_root, "shared.md", "shared"); + + let package = resolve_local_workflow_package(&workflow, temp.path(), None).unwrap(); + + assert_eq!(package.source_root(), package_root.canonicalize().unwrap()); + assert!( + package + .closure() + .versions() + .last() + .unwrap() + .1 + .version() + .files() + .keys() + .any(|path| path.as_str() == "shared.md"), + ); + } + + #[test] + fn loose_workflow_uses_its_canonical_containing_directory() { + let temp = tempfile::tempdir().unwrap(); + let workflow = write_workflow( + temp.path(), + "loose", + "digraph Demo { task [prompt=\"@prompt.md\"] }", + ); + write(temp.path(), "loose/prompt.md", "hello"); + + let package = resolve_local_workflow_package(&workflow, temp.path(), None).unwrap(); + + assert_eq!( + package.source_root(), + temp.path().join("loose").canonicalize().unwrap(), + ); + let root = package.closure().versions().last().unwrap().1.version(); + assert!(root.files().keys().any(|path| path.as_str() == "prompt.md")); + } + + #[cfg(unix)] + #[test] + fn rejects_direct_and_template_symlinks_that_escape_a_loose_package() { + use std::os::unix::fs::symlink; + + let temp = tempfile::tempdir().unwrap(); + let outside = temp.path().join("outside"); + fs::create_dir_all(&outside).unwrap(); + write(&outside, "secret.md", "secret"); + + let direct = write_workflow( + temp.path(), + "direct", + "digraph Demo { task [prompt=\"@secret.md\"] }", + ); + symlink( + outside.join("secret.md"), + temp.path().join("direct/secret.md"), + ) + .unwrap(); + let direct_error = resolve_local_workflow_package(&direct, temp.path(), None).unwrap_err(); + let direct_chain = error_chain(&direct_error); + assert!(direct_chain.contains("secret.md"), "{direct_chain}"); + assert!(direct_chain.contains("direct"), "{direct_chain}"); + + let template = write_workflow( + temp.path(), + "template", + "digraph Demo { task [prompt=\"@prompt.md\"] }", + ); + write( + temp.path(), + "template/prompt.md", + "{% include \"secret.md\" %}", + ); + symlink( + outside.join("secret.md"), + temp.path().join("template/secret.md"), + ) + .unwrap(); + let template_error = + resolve_local_workflow_package(&template, temp.path(), None).unwrap_err(); + let template_chain = error_chain(&template_error); + assert!( + template_chain.contains("escapes template root"), + "{template_chain}" + ); + } + + #[cfg(unix)] + #[test] + fn rejects_a_selected_workflow_symlink_outside_its_checkout() { + use std::os::unix::fs::symlink; + + let temp = tempfile::tempdir().unwrap(); + let project = temp.path().join("project"); + let outside = temp.path().join("outside"); + fs::create_dir_all(project.join(".fabro/workflows/hello")).unwrap(); + fs::create_dir_all(&outside).unwrap(); + init_repo(&project); + write( + &project, + ".fabro/workflows/hello/workflow.toml", + "_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n", + ); + write(&outside, "workflow.fabro", "digraph Outside {}"); + symlink( + outside.join("workflow.fabro"), + project.join(".fabro/workflows/hello/workflow.fabro"), + ) + .unwrap(); + + let error = resolve_local_workflow_package(Path::new("hello"), &project, None).unwrap_err(); + + assert!(matches!( + error, + LocalWorkflowPackageError::EscapesSourceRoot { .. } + )); + } + + #[test] + fn malformed_and_missing_workflows_preserve_config_sources() { + let temp = tempfile::tempdir().unwrap(); + let malformed = temp.path().join("malformed/workflow.toml"); + write(temp.path(), "malformed/workflow.toml", "not valid = ["); + + let malformed_error = + resolve_local_workflow_package(&malformed, temp.path(), None).unwrap_err(); + assert!(matches!( + malformed_error, + LocalWorkflowPackageError::Resolve { .. } + )); + assert!(std::error::Error::source(&malformed_error).is_some()); + + let missing = + resolve_local_workflow_package(Path::new("missing"), temp.path(), None).unwrap_err(); + assert!(matches!(missing, LocalWorkflowPackageError::Resolve { .. })); + assert!(std::error::Error::source(&missing).is_some()); + } +} diff --git a/lib/components/fabro-manifest/src/workflow_bundler.rs b/lib/components/fabro-manifest/src/workflow_bundler.rs index bbf7454be..2bdd4ac97 100644 --- a/lib/components/fabro-manifest/src/workflow_bundler.rs +++ b/lib/components/fabro-manifest/src/workflow_bundler.rs @@ -16,11 +16,12 @@ use fabro_template::{ }; use fabro_types::ManifestPath; use fabro_types::graph::ReferenceKind; +use thiserror::Error; use crate::{manifest_path_from_absolute, normalize_absolute_path}; pub(super) struct WorkflowBundler<'a> { - cwd: &'a Path, + package_root: &'a Path, inputs: &'a HashMap, template_store: FilesystemTemplateStore, workflows: HashMap, @@ -39,11 +40,11 @@ pub(super) struct CollectedWorkflowSource { } impl<'a> WorkflowBundler<'a> { - pub(super) fn new(cwd: &'a Path, inputs: &'a HashMap) -> Self { + pub(super) fn new(package_root: &'a Path, inputs: &'a HashMap) -> Self { Self { - cwd, + package_root, inputs, - template_store: FilesystemTemplateStore::new(cwd), + template_store: FilesystemTemplateStore::new(package_root), workflows: HashMap::new(), visited_workflows: HashSet::new(), workflow_version_projection: false, @@ -55,7 +56,7 @@ impl<'a> WorkflowBundler<'a> { workflow: &Path, project_config: Option<(&ManifestPath, &str)>, ) -> Result> { - let root_key = self.collect_workflow_entry(workflow, self.cwd)?; + let root_key = self.collect_workflow_entry(workflow, self.package_root)?; if let Some((config_path, source)) = project_config { let mut root = self @@ -89,19 +90,18 @@ impl<'a> WorkflowBundler<'a> { /// Collects the workflow at `location` and returns its manifest key. fn collect_workflow_location(&mut self, location: &WorkflowLocation) -> Result { - let dot_path = manifest_path_from_absolute(&location.graph, self.cwd)?; + let dot_path = manifest_path_from_absolute(&location.graph, self.package_root)?; let dot_key = dot_path.to_string(); if !self.visited_workflows.insert(dot_key.clone()) { return Ok(dot_key); } - let source = std::fs::read_to_string(&location.graph) - .with_context(|| format!("Failed to read {}", location.graph.display()))?; + let source = self.read_package_file(&location.graph)?; let config = if let Some(workflow_toml_path) = location.toml.as_ref() { Some(types::ManifestWorkflowConfig { - path: manifest_path_from_absolute(workflow_toml_path, self.cwd)?.to_string(), - source: std::fs::read_to_string(workflow_toml_path) - .with_context(|| format!("Failed to read {}", workflow_toml_path.display()))?, + path: manifest_path_from_absolute(workflow_toml_path, self.package_root)? + .to_string(), + source: self.read_package_file(workflow_toml_path)?, }) } else { None @@ -253,10 +253,7 @@ impl<'a> WorkflowBundler<'a> { for imported in imports { if visited_imports.insert(imported.path.to_string()) { - let imported_source = std::fs::read_to_string(&imported.absolute_path) - .with_context(|| { - format!("Failed to read {}", imported.absolute_path.display()) - })?; + let imported_source = self.read_package_file(&imported.absolute_path)?; let imported_scan = WorkflowScanInput { absolute_dot_path: imported.absolute_path, dot_path: imported.path, @@ -286,8 +283,7 @@ impl<'a> WorkflowBundler<'a> { bundled: &BundledFile, workflow_template_root: &ManifestPath, ) -> Result<()> { - let source = std::fs::read_to_string(&bundled.absolute_path) - .with_context(|| format!("Failed to read {}", bundled.absolute_path.display()))?; + let source = self.read_package_file(&bundled.absolute_path)?; let template_root = template_root_for_bundled_file(&bundled.path, workflow_template_root)?; self.collect_template_include_files( files, @@ -375,7 +371,7 @@ impl<'a> WorkflowBundler<'a> { let layer = source .parse::() .context("Failed to parse run config TOML")?; - let absolute_config_path = self.cwd.join(config_path.as_path()); + let absolute_config_path = self.package_root.join(config_path.as_path()); let base_dir = absolute_config_path .parent() .unwrap_or_else(|| Path::new(".")); @@ -460,11 +456,10 @@ impl<'a> WorkflowBundler<'a> { let absolute_path = normalize_absolute_path(base_dir, reference) .ok_or_else(|| anyhow!("unsupported manifest reference: {reference}"))?; - let path = manifest_path_from_absolute(&absolute_path, self.cwd)?; + let path = manifest_path_from_absolute(&absolute_path, self.package_root)?; let key = path.to_string(); if !files.contains_key(&key) { - let content = std::fs::read_to_string(&absolute_path) - .with_context(|| format!("Failed to read {}", absolute_path.display()))?; + let content = self.read_package_file(&absolute_path)?; files.insert(key.clone(), types::ManifestFileEntry { content, ref_: types::ManifestFileRef { @@ -480,6 +475,54 @@ impl<'a> WorkflowBundler<'a> { path, }) } + + fn read_package_file(&self, path: &Path) -> Result { + if !self.workflow_version_projection { + return std::fs::read_to_string(path) + .with_context(|| format!("Failed to read {}", path.display())); + } + let canonical = + path.canonicalize() + .map_err(|source| PackageFileReadError::Canonicalize { + path: path.to_path_buf(), + source, + })?; + if !canonical.starts_with(self.package_root) { + return Err(PackageFileReadError::EscapesSourceRoot { + path: path.to_path_buf(), + source_root: self.package_root.to_path_buf(), + } + .into()); + } + std::fs::read_to_string(&canonical).map_err(|source| { + PackageFileReadError::Read { + path: canonical, + source, + } + .into() + }) + } +} + +#[derive(Debug, Error)] +enum PackageFileReadError { + #[error("failed to canonicalize workflow package file `{path}`")] + Canonicalize { + path: PathBuf, + #[source] + source: std::io::Error, + }, + #[error("workflow package file `{path}` escapes source root `{source_root}`")] + EscapesSourceRoot { + path: PathBuf, + source_root: PathBuf, + }, + #[error("failed to read workflow package file `{path}`")] + Read { + path: PathBuf, + #[source] + source: std::io::Error, + }, } struct WorkflowScanInput { diff --git a/lib/components/fabro-manifest/src/workflow_version_collector.rs b/lib/components/fabro-manifest/src/workflow_version_collector.rs index 1a01473ce..bb8535a3f 100644 --- a/lib/components/fabro-manifest/src/workflow_version_collector.rs +++ b/lib/components/fabro-manifest/src/workflow_version_collector.rs @@ -2,6 +2,7 @@ use std::collections::{BTreeMap, HashMap, HashSet}; use std::path::{Path, PathBuf}; use fabro_api::types; +use fabro_config::project::WorkflowLocation; use fabro_types::{ WorkflowPath, WorkflowPathParseError, WorkflowVersion, WorkflowVersionId, WorkflowVersionShapeError, @@ -85,9 +86,56 @@ pub fn collect_workflow_versions( let location = crate::resolve_existing_workflow_location(&repository_workflow, checkout_root) .map_err(|source| location_error(workflow, source))?; + let package_root = + checkout_root + .canonicalize() + .map_err(|source| WorkflowVersionCollectError::Collect { + path: workflow.to_path_buf(), + source: anyhow::Error::new(source).context(format!( + "failed to canonicalize workflow package root {}", + checkout_root.display() + )), + })?; + let location = canonicalize_location(location, workflow)?; + collect_workflow_versions_at_location(&location, &package_root, workflow) +} + +fn canonicalize_location( + location: WorkflowLocation, + workflow: &Path, +) -> Result { + let canonicalize = |path: &Path| { + path.canonicalize() + .map_err(|source| WorkflowVersionCollectError::Collect { + path: workflow.to_path_buf(), + source: anyhow::Error::new(source).context(format!( + "failed to canonicalize workflow path {}", + path.display() + )), + }) + }; + let graph = canonicalize(&location.graph)?; + let toml = location.toml.as_deref().map(canonicalize).transpose()?; + let dir = graph + .parent() + .expect("a canonical workflow graph has a parent") + .to_path_buf(); + Ok(WorkflowLocation { + dir, + graph, + toml, + slug: location.slug, + }) +} + +pub(super) fn collect_workflow_versions_at_location( + location: &WorkflowLocation, + package_root: &Path, + workflow: &Path, +) -> Result { let inputs = HashMap::new(); - let collected = WorkflowBundler::new(checkout_root, &inputs) - .collect_versions(&location) + let collected = WorkflowBundler::new(package_root, &inputs) + .collect_versions(location) .map_err(|source| WorkflowVersionCollectError::Collect { path: workflow.to_path_buf(), source,