scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-08-28 05:27:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

feat: bundle workflow package files for remote runs

Browse source
This commit is contained in:
Bryan Helmkamp 2026-07-29 11:02:03 -04:00
parent d8434e7672
commit ea176b6ca1
No known key found for this signature in database
13 changed files with 789 additions and 61 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
Cargo.lock generated
View file

@ -2804,6 +2804,7 @@ dependencies = [
"temp-env",
"tempfile",
"toml 0.8.23",
"walkdir",
]
[[package]]

7
docs/public/api-reference/fabro-api.yaml
View file

@ -9003,7 +9003,7 @@ components:
properties:
version:
type: integer
description: Manifest schema version.
description: Manifest schema version. Version 2 adds workflow runtime files.
example: 1
run_id:
type: ["string", "null"]
@ -9288,6 +9288,11 @@ components:
type: object
additionalProperties:
$ref: "#/components/schemas/ManifestFileEntry"
runtime_files:
description: UTF-8 files from the workflow package that Fabro writes into fresh remote sandboxes before preparation.
type: object
additionalProperties:
type: string
PreflightResponse:
type: object

251
lib/apps/fabro-server/src/run_manifest.rs
View file

@ -40,7 +40,9 @@ use fabro_workflow::operations::{
};
use fabro_workflow::pipeline::Validated;
use fabro_workflow::run_materialization::materialize_run_with_ready_providers;
use fabro_workflow::workflow_bundle::{BundledWorkflow, ParsedWorkflowConfig, WorkflowBundle};
use fabro_workflow::workflow_bundle::{
self, BundledWorkflow, ParsedWorkflowConfig, WorkflowBundle,
};
use futures_util::stream::{self, StreamExt};
use tokio::process::Command;
use tokio::time;
@ -81,9 +83,17 @@ pub(crate) fn prepare_manifest_with_environment_defaults(
manifest_mcp_server_catalog: &HashMap<String, McpServerSettings>,
manifest: &types::RunManifest,
) -> Result<PreparedManifest> {
if manifest.version != 1 {
if !matches!(manifest.version, 1 | 2) {
bail!("unsupported manifest version {}", manifest.version);
}
if manifest.version == 1
&& manifest
.workflows
.values()
.any(|workflow| !workflow.runtime_files.is_empty())
{
bail!("workflow runtime files require manifest version 2");
}
let cwd = PathBuf::from(&manifest.cwd);
let target_path = ManifestPath::from_wire(&manifest.target.path)
@ -302,6 +312,9 @@ pub fn workflow_bundle_from_manifest(
) -> Result<WorkflowBundle> {
let mut bundled = HashMap::new();
let mut workflow_wire_keys = HashMap::new();
let mut runtime_destinations = HashMap::new();
let mut runtime_file_count = 0usize;
let mut runtime_file_bytes = 0usize;
for (wire_key, workflow) in workflows {
let path = ManifestPath::from_wire(wire_key)
@ -315,6 +328,35 @@ pub fn workflow_bundle_from_manifest(
workflow_wire_keys.insert(path.clone(), wire_key.clone());
let files = workflow_files_from_manifest(&workflow.files)?;
let runtime_files = workflow_runtime_files_from_manifest(&path, &workflow.runtime_files)?;
for (runtime_path, content) in &runtime_files {
runtime_file_count = runtime_file_count
.checked_add(1)
.ok_or_else(|| anyhow!("workflow runtime file count overflow"))?;
if runtime_file_count > workflow_bundle::MAX_WORKFLOW_RUNTIME_FILES {
bail!(
"workflow manifest contains more than {} runtime files",
workflow_bundle::MAX_WORKFLOW_RUNTIME_FILES
);
}
runtime_file_bytes = runtime_file_bytes
.checked_add(content.len())
.ok_or_else(|| anyhow!("workflow runtime file byte count overflow"))?;
if runtime_file_bytes > workflow_bundle::MAX_WORKFLOW_RUNTIME_BYTES {
bail!(
"workflow runtime files exceed the {} byte limit",
workflow_bundle::MAX_WORKFLOW_RUNTIME_BYTES
);
}
if let Some(previous) = runtime_destinations.get(runtime_path) {
if previous != content {
bail!("conflicting workflow runtime file contents for path: {runtime_path}");
}
} else {
runtime_destinations.insert(runtime_path.clone(), content.clone());
}
}
let config = workflow
.config
.as_ref()
@ -334,12 +376,44 @@ pub fn workflow_bundle_from_manifest(
source: workflow.source.clone(),
config,
files,
runtime_files,
});
}
Ok(WorkflowBundle::new(bundled))
}
fn workflow_runtime_files_from_manifest(
workflow_path: &ManifestPath,
files: &HashMap<String, String>,
) -> Result<HashMap<ManifestPath, String>> {
let mut bundled = HashMap::new();
let mut file_wire_keys = HashMap::new();
for (wire_key, content) in files {
if wire_key.split('/').any(|segment| segment == "..")
|| wire_key.chars().any(char::is_control)
{
bail!("invalid workflow runtime file path: {wire_key:?}");
}
let path = ManifestPath::from_wire(wire_key)
.ok_or_else(|| anyhow!("invalid workflow runtime file path: {wire_key:?}"))?;
if !workflow_bundle::is_workflow_runtime_file_path(workflow_path, &path) {
bail!("workflow runtime file path is outside the package directories: {wire_key:?}");
}
if let Some(previous) = file_wire_keys.get(&path) {
bail!(
"duplicate canonical workflow runtime file path: {path} (from wire keys \
{previous:?} and {wire_key:?})"
);
}
file_wire_keys.insert(path.clone(), wire_key.clone());
bundled.insert(path, content.clone());
}
Ok(bundled)
}
fn workflow_files_from_manifest(
files: &HashMap<String, types::ManifestFileEntry>,
) -> Result<HashMap<ManifestPath, String>> {
@ -1419,8 +1493,9 @@ mod tests {
},
version: 1,
workflows: HashMap::from([("workflow.fabro".to_string(), types::ManifestWorkflow {
config: None,
files: HashMap::new(),
config: None,
files: HashMap::new(),
runtime_files: HashMap::new(),
source:
"digraph Demo { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }"
.to_string(),
@ -1431,9 +1506,10 @@ mod tests {
fn invalid_manifest() -> types::RunManifest {
types::RunManifest {
workflows: HashMap::from([("workflow.fabro".to_string(), types::ManifestWorkflow {
config: None,
files: HashMap::new(),
source: "digraph Invalid { exit [shape=Msquare] orphan exit -> orphan }"
config: None,
files: HashMap::new(),
runtime_files: HashMap::new(),
source: "digraph Invalid { exit [shape=Msquare] orphan exit -> orphan }"
.to_string(),
})]),
..minimal_manifest()
@ -1591,10 +1667,12 @@ digraph Demo {{
fn manifest_workflow() -> types::ManifestWorkflow {
types::ManifestWorkflow {
config: None,
files: HashMap::new(),
source: "digraph Demo { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }"
.to_string(),
config: None,
files: HashMap::new(),
runtime_files: HashMap::new(),
source:
"digraph Demo { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }"
.to_string(),
}
}
@ -1609,6 +1687,147 @@ digraph Demo {{
}
}
#[test]
fn manifest_version_two_preserves_valid_workflow_runtime_files() {
let mut manifest = minimal_manifest();
manifest.version = 2;
manifest
.workflows
.get_mut("workflow.fabro")
.unwrap()
.runtime_files
.insert(
"scripts/security_review.py".to_string(),
"print('review')\n".to_string(),
);
let bundle = workflow_bundle_from_manifest(&manifest.workflows).unwrap();
let workflow = bundle
.workflow(&ManifestPath::from_wire("workflow.fabro").unwrap())
.unwrap();
assert_eq!(
workflow
.runtime_files
.get(&ManifestPath::from_wire("scripts/security_review.py").unwrap())
.map(String::as_str),
Some("print('review')\n")
);
}
#[test]
fn manifest_version_one_rejects_workflow_runtime_files() {
let mut manifest = minimal_manifest();
manifest
.workflows
.get_mut("workflow.fabro")
.unwrap()
.runtime_files
.insert(
"scripts/review.py".to_string(),
"print('review')\n".to_string(),
);
let result = prepare_manifest(
&manifest_run_defaults(Some(&default_settings_fixture())),
&manifest,
);
let Err(err) = result else {
panic!("manifest version 1 should reject workflow runtime files");
};
assert_eq!(
err.to_string(),
"workflow runtime files require manifest version 2"
);
}
#[test]
fn workflow_runtime_files_reject_parent_traversal() {
let mut workflow = manifest_workflow();
workflow
.runtime_files
.insert("../scripts/review.py".to_string(), "unsafe".to_string());
let err = workflow_bundle_from_manifest(&HashMap::from([(
"workflow.fabro".to_string(),
workflow,
)]))
.unwrap_err();
assert!(
err.to_string()
.contains("invalid workflow runtime file path")
);
}
#[test]
fn workflow_runtime_files_reject_paths_outside_package_directories() {
let mut workflow = manifest_workflow();
workflow
.runtime_files
.insert("prompts/review.md".to_string(), "unsafe".to_string());
let err = workflow_bundle_from_manifest(&HashMap::from([(
"workflow.fabro".to_string(),
workflow,
)]))
.unwrap_err();
assert!(err.to_string().contains("outside the package directories"));
}
#[test]
fn workflow_runtime_files_reject_conflicting_destinations() {
let mut first = manifest_workflow();
first
.runtime_files
.insert("scripts/review.py".to_string(), "first".to_string());
let mut second = manifest_workflow();
second
.runtime_files
.insert("scripts/review.py".to_string(), "second".to_string());
let err = workflow_bundle_from_manifest(&HashMap::from([
("first.fabro".to_string(), first),
("second.fabro".to_string(), second),
]))
.unwrap_err();
assert!(
err.to_string()
.contains("conflicting workflow runtime file contents")
);
}
#[test]
fn workflow_runtime_files_enforce_count_and_size_limits() {
let mut too_many = manifest_workflow();
for index in 0..=workflow_bundle::MAX_WORKFLOW_RUNTIME_FILES {
too_many
.runtime_files
.insert(format!("assets/file-{index}.txt"), String::new());
}
let count_err = workflow_bundle_from_manifest(&HashMap::from([(
"workflow.fabro".to_string(),
too_many,
)]))
.unwrap_err();
assert!(count_err.to_string().contains("runtime files"));
let mut too_large = manifest_workflow();
too_large.runtime_files.insert(
"assets/large.txt".to_string(),
"x".repeat(workflow_bundle::MAX_WORKFLOW_RUNTIME_BYTES + 1),
);
let size_err = workflow_bundle_from_manifest(&HashMap::from([(
"workflow.fabro".to_string(),
too_large,
)]))
.unwrap_err();
assert!(size_err.to_string().contains("runtime files exceed"));
}
fn git_context(origin_url: &str, branch: &str) -> types::GitContext {
types::GitContext {
origin_url: origin_url.to_string(),
@ -2759,14 +2978,16 @@ digraph Demo {
fn workflow_with_config(source: &str) -> BundledWorkflow {
BundledWorkflow {
path: ManifestPath::from_wire("workflow.fabro").expect("path should be valid"),
source: "digraph G {}".to_string(),
config: Some(ParsedWorkflowConfig {
path: ManifestPath::from_wire("workflow.fabro")
.expect("path should be valid"),
source: "digraph G {}".to_string(),
config: Some(ParsedWorkflowConfig {
path: ManifestPath::from_wire("workflow.toml")
.expect("config path should be valid"),
source: source.to_string(),
}),
files: std::collections::HashMap::new(),
files: std::collections::HashMap::new(),
runtime_files: std::collections::HashMap::new(),
}
}

19
lib/apps/fabro-server/src/server/handler/runs.rs
View file

@ -38,10 +38,11 @@ use tokio::fs;
use tracing::info;
use super::super::{
AppState, DeleteRunOutcome, ListResponse, RunExecutionMode, VariableError, answer_from_request,
api_question_from_pending_interview, clamp_page_limit, clamp_page_offset, default_page_limit,
delete_run_internal, load_pending_interview, managed_run, parse_run_id_path,
parse_stage_id_path, reject_if_archived, submit_pending_interview_answer, workflow_event,
AppState, DefaultBodyLimit, DeleteRunOutcome, ListResponse, RunExecutionMode, VariableError,
answer_from_request, api_question_from_pending_interview, clamp_page_limit, clamp_page_offset,
default_page_limit, delete_run_internal, load_pending_interview, managed_run,
parse_run_id_path, parse_stage_id_path, reject_if_archived, submit_pending_interview_answer,
workflow_event,
};
use crate::error::ApiError;
use crate::principal_middleware::{
@ -55,15 +56,23 @@ use crate::run_title_generation::{self, GenerateTitleInput, TitlePromptInput, Wo
#[cfg(any(test, feature = "test-support"))]
use crate::test_support as server_test_support;
const RUN_MANIFEST_BODY_LIMIT: usize = 10 * 1024 * 1024;
pub(super) fn manifest_routes() -> Router<Arc<AppState>> {
Router::new()
.route("/preflight", post(run_preflight))
.route("/validate", post(validate_run_manifest))
.layer(DefaultBodyLimit::max(RUN_MANIFEST_BODY_LIMIT))
}
pub(super) fn routes() -> Router<Arc<AppState>> {
Router::new()
.route("/runs", get(list_runs).post(create_run))
.route(
"/runs",
get(list_runs)
.post(create_run)
.layer(DefaultBodyLimit::max(RUN_MANIFEST_BODY_LIMIT)),
)
.route("/runs/resolve", get(resolve_run))
.route(
"/runs/{id}",

1
lib/components/fabro-manifest/Cargo.toml
View file

@ -23,6 +23,7 @@ fabro-types = { path = "../../foundation/fabro-types" }
fabro-workflow = { path = "../fabro-workflow" }
git2.workspace = true
toml.workspace = true
walkdir.workspace = true
[dev-dependencies]
tempfile = "3"

313
lib/components/fabro-manifest/src/lib.rs
View file

@ -7,7 +7,7 @@ use std::collections::{HashMap, HashSet};
use std::path::{Component, Path, PathBuf};
use std::sync::Arc;
use anyhow::{Context, Result, anyhow};
use anyhow::{Context, Result, anyhow, bail};
use fabro_api::types;
use fabro_config::project::{self, WorkflowLocation, discover_project_config};
use fabro_config::run::{resolve_run_goal_from_layer, resolve_run_goal_from_namespace};
@ -33,6 +33,10 @@ use fabro_workflow::git::{
use fabro_workflow::static_reference::{
AttributeScope, ReferenceKind, reference_kind_for_attribute,
};
use fabro_workflow::workflow_bundle::{
MAX_WORKFLOW_RUNTIME_BYTES, MAX_WORKFLOW_RUNTIME_FILES, WORKFLOW_PACKAGE_DIR_NAMES,
};
use walkdir::WalkDir;
#[derive(Debug, Default)]
pub struct ManifestBuildInput {
@ -136,10 +140,13 @@ pub fn build_sparse_run_overrides(input: RunOverrideInput<'_>) -> Option<RunLaye
}
struct CollectContext<'a> {
cwd: &'a Path,
inputs: HashMap<String, toml::Value>,
workflows: HashMap<String, types::ManifestWorkflow>,
visited_workflows: HashSet<String>,
cwd: &'a Path,
canonical_cwd: PathBuf,
inputs: HashMap<String, toml::Value>,
workflows: HashMap<String, types::ManifestWorkflow>,
visited_workflows: HashSet<String>,
runtime_file_count: usize,
runtime_file_bytes: usize,
}
#[derive(Clone)]
@ -198,10 +205,14 @@ pub fn build_run_manifest(input: ManifestBuildInput) -> Result<BuiltManifest> {
let target_key = target_manifest_path.to_string();
let mut context = CollectContext {
cwd: &input.cwd,
inputs: workflow_settings.run.inputs.clone(),
workflows: HashMap::new(),
visited_workflows: HashSet::new(),
cwd: &input.cwd,
canonical_cwd: std::fs::canonicalize(&input.cwd)
.with_context(|| format!("Failed to resolve {}", input.cwd.display()))?,
inputs: workflow_settings.run.inputs.clone(),
workflows: HashMap::new(),
visited_workflows: HashSet::new(),
runtime_file_count: 0,
runtime_file_bytes: 0,
};
collect_workflow_entry(&mut context, &input.workflow, &input.cwd)?;
if let Some((_, config_path, source)) = project_config_source.as_ref() {
@ -251,6 +262,16 @@ pub fn build_run_manifest(input: ManifestBuildInput) -> Result<BuiltManifest> {
let git = build_git_context(&working_directory, configured_repo_origin_url.as_deref());
let args = input.args.filter(|args| !manifest_args_is_empty(args));
let version = if context
.workflows
.values()
.any(|workflow| !workflow.runtime_files.is_empty())
{
2
} else {
1
};
Ok(BuiltManifest {
manifest: types::RunManifest {
args,
@ -265,7 +286,7 @@ pub fn build_run_manifest(input: ManifestBuildInput) -> Result<BuiltManifest> {
identifier: input.workflow.display().to_string(),
path: target_key,
},
version: 1,
version,
workflows: context.workflows,
},
target_path,
@ -306,12 +327,14 @@ fn collect_workflow_entry(
None
};
let workflow_dir = location.dir.clone();
let scan = WorkflowScanInput {
absolute_dot_path: location.graph,
dot_path,
source: source.clone(),
};
let mut files = HashMap::new();
let mut runtime_files = HashMap::new();
let mut visited_imports = HashSet::new();
if let Some(config) = config.as_ref() {
let config_path = ManifestPath::from_wire(&config.path)
@ -319,16 +342,122 @@ fn collect_workflow_entry(
collect_config_dockerfile(context.cwd, &config_path, &config.source, &mut files)?;
}
collect_workflow_files(context, &scan, &mut files, &mut visited_imports)?;
collect_workflow_runtime_files(context, &workflow_dir, &mut runtime_files)?;
context.workflows.insert(dot_key, types::ManifestWorkflow {
config,
files,
runtime_files,
source,
});
Ok(())
}
fn collect_workflow_runtime_files(
context: &mut CollectContext<'_>,
workflow_dir: &Path,
runtime_files: &mut HashMap<String, String>,
) -> Result<()> {
for package_dir_name in WORKFLOW_PACKAGE_DIR_NAMES {
let package_dir = workflow_dir.join(package_dir_name);
let metadata = match std::fs::symlink_metadata(&package_dir) {
Ok(metadata) => metadata,
Err(err) if err.kind() == std::io::ErrorKind::NotFound => continue,
Err(err) => {
return Err(err)
.with_context(|| format!("Failed to inspect {}", package_dir.display()));
}
};
if metadata.file_type().is_symlink() {
bail!(
"workflow package directory cannot be a symlink: {}",
package_dir.display()
);
}
if !metadata.is_dir() {
bail!(
"workflow package path must be a directory: {}",
package_dir.display()
);
}
for entry in WalkDir::new(&package_dir)
.follow_links(false)
.sort_by_file_name()
{
let entry = entry
.map_err(anyhow::Error::new)
.with_context(|| format!("Failed to walk {}", package_dir.display()))?;
let file_type = entry.file_type();
if file_type.is_dir() {
continue;
}
if file_type.is_symlink() {
bail!(
"workflow package files cannot be symlinks: {}",
entry.path().display()
);
}
if !file_type.is_file() {
bail!(
"workflow package entries must be regular files: {}",
entry.path().display()
);
}
let canonical_path = std::fs::canonicalize(entry.path())
.with_context(|| format!("Failed to resolve {}", entry.path().display()))?;
if !canonical_path.starts_with(&context.canonical_cwd) {
bail!(
"workflow package file must be inside the working directory: {}",
entry.path().display()
);
}
let metadata = entry
.metadata()
.map_err(anyhow::Error::new)
.with_context(|| format!("Failed to inspect {}", entry.path().display()))?;
let metadata_size = usize::try_from(metadata.len())
.context("workflow package file size does not fit in memory")?;
validate_runtime_file_limits(context, metadata_size)?;
let content = std::fs::read_to_string(entry.path())
.with_context(|| format!("Failed to read {}", entry.path().display()))?;
validate_runtime_file_limits(context, content.len())?;
let path = manifest_path_from_absolute(entry.path(), context.cwd)?;
let content_bytes = content.len();
runtime_files.insert(path.to_string(), content);
context.runtime_file_count += 1;
context.runtime_file_bytes += content_bytes;
}
}
Ok(())
}
fn validate_runtime_file_limits(context: &CollectContext<'_>, file_bytes: usize) -> Result<()> {
let file_count = context
.runtime_file_count
.checked_add(1)
.ok_or_else(|| anyhow!("workflow package file count overflow"))?;
if file_count > MAX_WORKFLOW_RUNTIME_FILES {
bail!("workflow packages contain more than {MAX_WORKFLOW_RUNTIME_FILES} runtime files");
}
let total_bytes = context
.runtime_file_bytes
.checked_add(file_bytes)
.ok_or_else(|| anyhow!("workflow package byte count overflow"))?;
if total_bytes > MAX_WORKFLOW_RUNTIME_BYTES {
bail!("workflow package files exceed the {MAX_WORKFLOW_RUNTIME_BYTES} byte limit");
}
Ok(())
}
fn collect_workflow_files(
context: &mut CollectContext<'_>,
workflow: &WorkflowScanInput,
@ -913,6 +1042,23 @@ mod tests {
)]))
}
fn write_minimal_workflow(project: &Path, name: &str) -> PathBuf {
let workflow_dir = project.join(".fabro/workflows").join(name);
std::fs::create_dir_all(&workflow_dir).unwrap();
std::fs::write(project.join(".fabro/project.toml"), "_version = 1\n").unwrap();
std::fs::write(
workflow_dir.join("workflow.toml"),
"_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\n",
)
.unwrap();
std::fs::write(
workflow_dir.join("workflow.fabro"),
"digraph Demo { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }",
)
.unwrap();
workflow_dir
}
fn assert_manifest_bundles_output_schema_file(node_attributes: &str) {
let temp = tempfile::tempdir().unwrap();
let project = temp.path();
@ -1039,6 +1185,142 @@ mod tests {
assert_manifest_bundles_output_schema_file(r#"type="command", script="echo""#);
}
#[test]
fn build_manifest_bundles_conventional_workflow_runtime_files() {
let temp = tempfile::tempdir().unwrap();
let project = temp.path();
let workflow_dir = write_minimal_workflow(project, "security-review");
std::fs::create_dir_all(workflow_dir.join("scripts/lib")).unwrap();
std::fs::create_dir_all(workflow_dir.join("references")).unwrap();
std::fs::create_dir_all(workflow_dir.join("assets/fixtures")).unwrap();
std::fs::create_dir_all(workflow_dir.join("other")).unwrap();
std::fs::write(
workflow_dir.join("scripts/security_review.py"),
"print('review')\n",
)
.unwrap();
std::fs::write(workflow_dir.join("scripts/lib/check.py"), "CHECK = True\n").unwrap();
std::fs::write(workflow_dir.join("references/policy.md"), "# Policy\n").unwrap();
std::fs::write(
workflow_dir.join("assets/fixtures/input.json"),
"{\"known\":true}\n",
)
.unwrap();
std::fs::write(workflow_dir.join("other/ignored.txt"), "ignored\n").unwrap();
let built = build_run_manifest(ManifestBuildInput {
workflow: PathBuf::from(".fabro/workflows/security-review/workflow.toml"),
cwd: project.to_path_buf(),
environment_defaults: test_environment_defaults(),
..Default::default()
})
.unwrap();
assert_eq!(built.manifest.version, 2);
let workflow = &built.manifest.workflows[".fabro/workflows/security-review/workflow.fabro"];
assert_eq!(
workflow.runtime_files,
HashMap::from([
(
".fabro/workflows/security-review/scripts/security_review.py".to_string(),
"print('review')\n".to_string(),
),
(
".fabro/workflows/security-review/scripts/lib/check.py".to_string(),
"CHECK = True\n".to_string(),
),
(
".fabro/workflows/security-review/references/policy.md".to_string(),
"# Policy\n".to_string(),
),
(
".fabro/workflows/security-review/assets/fixtures/input.json".to_string(),
"{\"known\":true}\n".to_string(),
),
])
);
assert!(
!workflow
.runtime_files
.contains_key(".fabro/workflows/security-review/other/ignored.txt")
);
}
#[test]
fn build_manifest_without_workflow_runtime_files_stays_at_version_one() {
let temp = tempfile::tempdir().unwrap();
let project = temp.path();
write_minimal_workflow(project, "demo");
let built = build_run_manifest(ManifestBuildInput {
workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"),
cwd: project.to_path_buf(),
environment_defaults: test_environment_defaults(),
..Default::default()
})
.unwrap();
assert_eq!(built.manifest.version, 1);
assert!(
built.manifest.workflows[".fabro/workflows/demo/workflow.fabro"]
.runtime_files
.is_empty()
);
}
#[test]
fn build_manifest_rejects_non_utf8_workflow_runtime_files() {
let temp = tempfile::tempdir().unwrap();
let project = temp.path();
let workflow_dir = write_minimal_workflow(project, "demo");
std::fs::create_dir_all(workflow_dir.join("assets")).unwrap();
std::fs::write(workflow_dir.join("assets/binary.dat"), [0xff, 0xfe]).unwrap();
let err = build_run_manifest(ManifestBuildInput {
workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"),
cwd: project.to_path_buf(),
environment_defaults: test_environment_defaults(),
..Default::default()
})
.unwrap_err();
assert!(err.chain().any(|cause| {
cause
.to_string()
.contains("stream did not contain valid UTF-8")
}));
}
#[cfg(unix)]
#[test]
fn build_manifest_rejects_symlinks_in_workflow_package_directories() {
use std::os::unix::fs::symlink;
let temp = tempfile::tempdir().unwrap();
let project = temp.path();
let workflow_dir = write_minimal_workflow(project, "demo");
std::fs::create_dir_all(workflow_dir.join("scripts")).unwrap();
std::fs::write(workflow_dir.join("target.py"), "print('target')\n").unwrap();
symlink(
workflow_dir.join("target.py"),
workflow_dir.join("scripts/review.py"),
)
.unwrap();
let err = build_run_manifest(ManifestBuildInput {
workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"),
cwd: project.to_path_buf(),
environment_defaults: test_environment_defaults(),
..Default::default()
})
.unwrap_err();
assert!(
err.to_string()
.contains("workflow package files cannot be symlinks")
);
}
#[test]
fn build_manifest_bundles_imports_prompts_and_children() {
let temp = tempfile::tempdir().unwrap();
@ -1047,7 +1329,7 @@ mod tests {
let child_dir = project.join(".fabro/workflows/child");
std::fs::create_dir_all(workflow_dir.join("prompts")).unwrap();
std::fs::create_dir_all(workflow_dir.join("imports")).unwrap();
std::fs::create_dir_all(&child_dir).unwrap();
std::fs::create_dir_all(child_dir.join("scripts")).unwrap();
std::fs::write(project.join(".fabro/project.toml"), "_version = 1\n").unwrap();
std::fs::write(
workflow_dir.join("workflow.toml"),
@ -1085,6 +1367,11 @@ mod tests {
r"digraph Child { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }",
)
.unwrap();
std::fs::write(
child_dir.join("scripts/child.py"),
"print('child package')\n",
)
.unwrap();
let built = build_run_manifest(ManifestBuildInput {
workflow: PathBuf::from(".fabro/workflows/demo/workflow.toml"),
@ -1123,6 +1410,10 @@ mod tests {
.workflows
.contains_key(".fabro/workflows/child/workflow.fabro")
);
assert_eq!(
built.manifest.workflows[".fabro/workflows/child/workflow.fabro"].runtime_files[".fabro/workflows/child/scripts/child.py"],
"print('child package')\n"
);
}
#[test]

9
lib/components/fabro-workflow/src/handler/manager_loop.rs
View file

@ -597,10 +597,11 @@ mod tests {
services.workflow_bundle = Some(Arc::new(WorkflowBundle::new(HashMap::from([(
ManifestPath::from_wire("children/review.fabro").unwrap(),
BundledWorkflow {
path: ManifestPath::from_wire("children/review.fabro").unwrap(),
source: child_dot_succeeds().to_string(),
config: None,
files: HashMap::new(),
path: ManifestPath::from_wire("children/review.fabro").unwrap(),
source: child_dot_succeeds().to_string(),
config: None,
files: HashMap::new(),
runtime_files: HashMap::new(),
},
)]))));

18
lib/components/fabro-workflow/src/operations/create.rs
View file

@ -1242,8 +1242,8 @@ reasoning = false
fn validate_from_bundle_resolves_nested_import_files_relative_to_imported_graph() {
let validated = validate(ValidateInput {
workflow: WorkflowInput::Bundled(BundledWorkflow {
path: ManifestPath::from_wire("workflow.fabro").unwrap(),
source: r#"digraph Test {
path: ManifestPath::from_wire("workflow.fabro").unwrap(),
source: r#"digraph Test {
graph [goal="Ship"]
start [shape=Mdiamond]
validate [import="./child/validate.fabro"]
@ -1251,8 +1251,8 @@ reasoning = false
start -> validate -> exit
}"#
.to_string(),
config: None,
files: HashMap::from([
config: None,
files: HashMap::from([
(
ManifestPath::from_wire("child/validate.fabro").unwrap(),
r#"digraph Validate {
@ -1268,6 +1268,7 @@ reasoning = false
"Lint {{ goal }}".to_string(),
),
]),
runtime_files: HashMap::new(),
}),
settings: WorkflowSettings::default(),
vars: HashMap::new(),
@ -1290,8 +1291,8 @@ reasoning = false
fn validate_from_bundle_resolves_minijinja_includes_in_prompt_and_goal_files() {
let validated = validate(ValidateInput {
workflow: WorkflowInput::Bundled(BundledWorkflow {
path: ManifestPath::from_wire("workflow.fabro").unwrap(),
source: r#"digraph Test {
path: ManifestPath::from_wire("workflow.fabro").unwrap(),
source: r#"digraph Test {
graph [goal="@goals/goal.md"]
start [shape=Mdiamond]
work [prompt="@prompts/work.md"]
@ -1299,8 +1300,8 @@ reasoning = false
start -> work -> exit
}"#
.to_string(),
config: None,
files: HashMap::from([
config: None,
files: HashMap::from([
(
ManifestPath::from_wire("goals/goal.md").unwrap(),
r#"{% include "goal.tpl.md" %}"#.to_string(),
@ -1318,6 +1319,7 @@ reasoning = false
"Bundled prompt".to_string(),
),
]),
runtime_files: HashMap::new(),
}),
settings: WorkflowSettings::default(),
vars: HashMap::new(),

18
lib/components/fabro-workflow/src/operations/start.rs
View file

@ -2422,8 +2422,8 @@ reasoning = false
(
ManifestPath::from_wire("workflow.fabro").unwrap(),
BundledWorkflow {
path: ManifestPath::from_wire("workflow.fabro").unwrap(),
source: r#"digraph Root {
path: ManifestPath::from_wire("workflow.fabro").unwrap(),
source: r#"digraph Root {
graph [goal="Bundle child"]
start [shape=Mdiamond]
manager [
@ -2436,22 +2436,24 @@ reasoning = false
start -> manager -> exit
}"#
.to_string(),
config: None,
files: HashMap::new(),
config: None,
files: HashMap::new(),
runtime_files: HashMap::new(),
},
),
(
ManifestPath::from_wire("children/review.fabro").unwrap(),
BundledWorkflow {
path: ManifestPath::from_wire("children/review.fabro").unwrap(),
source: r"digraph Review {
path: ManifestPath::from_wire("children/review.fabro").unwrap(),
source: r"digraph Review {
start [shape=Mdiamond]
exit [shape=Msquare]
start -> exit
}"
.to_string(),
config: None,
files: HashMap::new(),
config: None,
files: HashMap::new(),
runtime_files: HashMap::new(),
},
),
]));

91
lib/components/fabro-workflow/src/pipeline/initialize.rs
View file

@ -1,4 +1,4 @@
use std::collections::HashMap;
use std::collections::{BTreeMap, HashMap};
use std::path::PathBuf;
use std::sync::Arc;
use std::time::Instant;
@ -33,6 +33,7 @@ use crate::services::{
};
use crate::stage_execution::{StageExecutionSeed, StageExecutionTracker};
use crate::steering_hub::SteeringHub;
use crate::workflow_bundle::WorkflowBundle;
type BuiltSandboxEnv = (HashMap<String, String>, Option<Arc<GitHubTokenSource>>);
@ -48,6 +49,38 @@ async fn run_hooks(
runner.run(hook_context, sandbox, execution_context).await
}
async fn materialize_workflow_runtime_files(
sandbox: &dyn Sandbox,
workflow_bundle: &WorkflowBundle,
) -> Result<(), Error> {
let mut runtime_files = BTreeMap::new();
for workflow in workflow_bundle.workflows().values() {
for (path, content) in &workflow.runtime_files {
let path = path.to_string();
if let Some(previous) = runtime_files.get(&path) {
if *previous != content.as_str() {
return Err(Error::Precondition(format!(
"conflicting workflow runtime file contents for path: {path}"
)));
}
} else {
runtime_files.insert(path, content.as_str());
}
}
}
for (path, content) in runtime_files {
sandbox.write_file(&path, content).await.map_err(|err| {
Error::engine_with_source(
format!("Failed to materialize workflow runtime file {path}"),
err,
)
})?;
}
Ok(())
}
fn git_setup_intent(run_options: &RunOptions) -> GitSetupIntent {
if let Some(source) = run_options.fork_source_ref.as_ref() {
GitSetupIntent::ForkFromCheckpoint {
@ -319,7 +352,9 @@ pub async fn initialize(
options.emitter.notice(
RunNoticeLevel::Warn,
RunNoticeCode::DirtyWorktree,
"Uncommitted changes will not be included in the remote sandbox.",
"Uncommitted project changes will not be included in the remote sandbox. Files under \
workflow package scripts/, references/, and assets/ directories are bundled \
separately.",
);
}
@ -389,6 +424,12 @@ pub async fn initialize(
.map_err(|e| Error::engine_with_source("Failed to initialize sandbox", e))?;
}
if !attach_existing && !matches!(&options.sandbox, SandboxSpec::Local { .. }) {
if let Some(workflow_bundle) = options.workflow_bundle.as_deref() {
materialize_workflow_runtime_files(sandbox.as_ref(), workflow_bundle).await?;
}
}
let locations = RunLocations::for_sandbox(host_source_dir, sandbox.as_ref(), run_dir.clone());
let hook_ctx = HookContext::new(
@ -653,9 +694,12 @@ mod tests {
use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node};
use fabro_interview::AutoApproveInterviewer;
use fabro_sandbox::SandboxSpec;
use fabro_sandbox::test_support::MockSandbox;
use fabro_store::Database;
use fabro_types::settings::run::RunModelControls;
use fabro_types::{EventBody, RunEvent, RunId, WorkflowSettings, fixtures, test_support};
use fabro_types::{
EventBody, ManifestPath, RunEvent, RunId, WorkflowSettings, fixtures, test_support,
};
use fabro_vault::{SecretType, Vault};
use object_store::memory::InMemory;
use tokio::fs::{create_dir_all, write};
@ -667,6 +711,7 @@ mod tests {
use crate::pipeline::types::InitOptions;
use crate::records::RunSpec;
use crate::run_options::RunOptions;
use crate::workflow_bundle::BundledWorkflow;
fn test_run_id() -> RunId {
fixtures::RUN_1
@ -679,6 +724,46 @@ mod tests {
}
}
#[tokio::test]
async fn materialize_workflow_runtime_files_writes_each_destination_in_path_order() {
let workflow_path =
ManifestPath::from_wire(".fabro/workflows/demo/workflow.fabro").unwrap();
let workflow_bundle =
WorkflowBundle::new(HashMap::from([(workflow_path.clone(), BundledWorkflow {
path: workflow_path,
source: "digraph Demo {}".to_string(),
config: None,
files: HashMap::new(),
runtime_files: HashMap::from([
(
ManifestPath::from_wire(".fabro/workflows/demo/scripts/z-last.py").unwrap(),
"print('last')\n".to_string(),
),
(
ManifestPath::from_wire(".fabro/workflows/demo/assets/a-first.txt")
.unwrap(),
"first\n".to_string(),
),
]),
})]));
let sandbox = MockSandbox::default();
materialize_workflow_runtime_files(&sandbox, &workflow_bundle)
.await
.unwrap();
assert_eq!(*sandbox.written_files.lock().unwrap(), vec![
(
".fabro/workflows/demo/assets/a-first.txt".to_string(),
"first\n".to_string(),
),
(
".fabro/workflows/demo/scripts/z-last.py".to_string(),
"print('last')\n".to_string(),
),
]);
}
fn test_catalog() -> Arc<Catalog> {
Arc::new(Catalog::from_builtin().expect("default catalog should build"))
}

116
lib/components/fabro-workflow/src/workflow_bundle.rs
View file

@ -1,5 +1,5 @@
use std::collections::HashMap;
use std::path::PathBuf;
use std::path::{Component, Path, PathBuf};
use std::sync::Arc;
use fabro_types::ManifestPath;
@ -7,6 +7,10 @@ use serde::{Deserialize, Serialize};
use crate::file_resolver::{BundleFileResolver, FileResolver};
pub const WORKFLOW_PACKAGE_DIR_NAMES: [&str; 3] = ["scripts", "references", "assets"];
pub const MAX_WORKFLOW_RUNTIME_FILES: usize = 256;
pub const MAX_WORKFLOW_RUNTIME_BYTES: usize = 5 * 1024 * 1024;
#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct ParsedWorkflowConfig {
pub path: ManifestPath,
@ -15,10 +19,12 @@ pub struct ParsedWorkflowConfig {
#[derive(Clone, Debug, Default, Serialize, Deserialize)]
pub struct BundledWorkflow {
pub path: ManifestPath,
pub source: String,
pub config: Option<ParsedWorkflowConfig>,
pub files: HashMap<ManifestPath, String>,
pub path: ManifestPath,
pub source: String,
pub config: Option<ParsedWorkflowConfig>,
pub files: HashMap<ManifestPath, String>,
#[serde(default)]
pub runtime_files: HashMap<ManifestPath, String>,
}
impl BundledWorkflow {
@ -33,6 +39,43 @@ impl BundledWorkflow {
}
}
#[must_use]
pub fn is_workflow_runtime_file_path(
workflow_path: &ManifestPath,
candidate: &ManifestPath,
) -> bool {
if candidate
.as_path()
.components()
.any(|component| matches!(component, Component::ParentDir))
{
return false;
}
let package_dir = workflow_path.parent_or_dot();
let relative = if package_dir.as_os_str().is_empty() || package_dir == Path::new(".") {
candidate.as_path()
} else {
let Ok(relative) = candidate.as_path().strip_prefix(package_dir) else {
return false;
};
relative
};
let mut components = relative.components();
let Some(Component::Normal(package_dir_name)) = components.next() else {
return false;
};
if !WORKFLOW_PACKAGE_DIR_NAMES
.iter()
.any(|allowed| package_dir_name == *allowed)
{
return false;
}
matches!(components.next(), Some(Component::Normal(_)))
&& components.all(|component| matches!(component, Component::Normal(_)))
}
#[derive(Clone, Debug, Default, Serialize, Deserialize)]
pub struct WorkflowBundle {
workflows: HashMap<ManifestPath, BundledWorkflow>,
@ -83,3 +126,66 @@ impl RunDefinition {
WorkflowBundle::new(self.workflows.clone())
}
}
#[cfg(test)]
mod tests {
use super::*;
fn manifest_path(value: &str) -> ManifestPath {
ManifestPath::from_wire(value).expect("test path should be valid")
}
#[test]
fn workflow_runtime_file_path_accepts_reserved_package_directories() {
let workflow = manifest_path(".fabro/workflows/security-review/workflow.fabro");
for path in [
".fabro/workflows/security-review/scripts/review.py",
".fabro/workflows/security-review/references/policy.md",
".fabro/workflows/security-review/assets/fixtures/input.json",
] {
assert!(is_workflow_runtime_file_path(
&workflow,
&manifest_path(path)
));
}
}
#[test]
fn workflow_runtime_file_path_rejects_paths_outside_reserved_directories() {
let workflow = manifest_path(".fabro/workflows/security-review/workflow.fabro");
for path in [
"../secrets.txt",
".fabro/workflows/other/scripts/review.py",
".fabro/workflows/security-review/workflow.toml",
".fabro/workflows/security-review/scripts",
] {
assert!(!is_workflow_runtime_file_path(
&workflow,
&manifest_path(path)
));
}
}
#[test]
fn workflow_runtime_file_path_supports_a_workflow_at_the_workspace_root() {
assert!(is_workflow_runtime_file_path(
&manifest_path("workflow.fabro"),
&manifest_path("scripts/review.py")
));
}
#[test]
fn bundled_workflow_deserializes_definitions_without_runtime_files() {
let workflow: BundledWorkflow = serde_json::from_value(serde_json::json!({
"path": "workflow.fabro",
"source": "digraph Demo {}",
"config": null,
"files": {}
}))
.expect("existing bundled workflow should deserialize");
assert!(workflow.runtime_files.is_empty());
}
}

4
lib/packages/fabro-api-client/src/models/manifest-workflow.ts generated
View file

@ -24,4 +24,8 @@ export interface ManifestWorkflow {
'source': string;
'config'?: ManifestWorkflowConfig;
'files'?: { [key: string]: ManifestFileEntry; };
/**
* UTF-8 files from the workflow package that Fabro writes into fresh remote sandboxes before preparation.
*/
'runtime_files'?: { [key: string]: string; };
}

2
lib/packages/fabro-api-client/src/models/run-manifest.ts generated
View file

@ -37,7 +37,7 @@ import type { ManifestWorkflow } from './manifest-workflow';
*/
export interface RunManifest {
/**
* Manifest schema version.
* Manifest schema version. Version 2 adds workflow runtime files.
*/
'version': number;
/**