scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-09-15 23:32:46 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

Reject escaping workflow references before resolving them

Browse source 
The pre-resolution containment check in the version bundler was a
no-op: ManifestPath::from_absolute happily returns a `..`-prefixed path
for locations outside the package root, so an escaping
stack.child_workflow reference reached WorkflowLocation resolution,
which probes and parses config files on the host before the real
containment check in read_package_file ran. The request still failed,
but the TOML parser's diagnostic quoted the host file.

Check that the normalized reference stays under the package root before
resolving it, and extend the supplied-workflow test to plant malformed
host files that any parser would quote.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Scott Werner 2026-09-11 15:22:04 -06:00
parent bafdd880f5
commit 111e737d53
2 changed files with 38 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

29
lib/components/fabro-manifest/src/supplied_workflow.rs
View file

@ -174,6 +174,17 @@ mod tests {
)
.unwrap();
std::fs::write(parent.path().join("child.fabro"), "digraph Host {}").unwrap();
// Malformed on purpose: a parser that reaches this file would quote it.
std::fs::write(
parent.path().join("secret.toml"),
"HOST_SECRET = [unterminated",
)
.unwrap();
std::fs::write(
parent.path().join("workflow.toml"),
"HOST_SECRET = [unterminated",
)
.unwrap();
for (index, input) in [
supplied("workflow.fabro", &[
("workflow.fabro", r#"digraph W { p [prompt="@prompt.md"] }"#),
@ -203,6 +214,14 @@ mod tests {
"workflow.fabro",
r#"digraph W { p [stack.child_workflow="sub/../../child.fabro"] }"#,
)]),
supplied("workflow.fabro", &[(
"workflow.fabro",
r#"digraph W { p [stack.child_workflow="../secret.toml"] }"#,
)]),
supplied("workflow.fabro", &[(
"workflow.fabro",
r#"digraph W { p [stack.child_workflow="../graph.fabro"] }"#,
)]),
supplied("workflow.fabro", &[(
"workflow.fabro",
r#"digraph W { p [stack.child_workflow="missing"] }"#,
@ -221,9 +240,15 @@ mod tests {
{
let staging = tempfile::tempdir_in(parent.path()).unwrap();
let path = staging.path().to_owned();
let error = collect_with_staging(&input, staging)
.err()
.unwrap_or_else(|| panic!("accepted invalid fixture {index}"));
let rendered = format!("{error:#}");
// Escaping references must fail before any host file is opened,
// so no host diagnostic (parse error, exists-vs-missing) leaks.
assert!(
collect_with_staging(&input, staging).is_err(),
"accepted invalid fixture {index}"
!rendered.contains("HOST_SECRET") && !rendered.contains("secret.toml:"),
"fixture {index} read a host file: {rendered}"
);
assert!(!path.exists());
}

13
lib/components/fabro-manifest/src/workflow_bundler.rs
View file

@ -160,8 +160,17 @@ impl<'a> WorkflowBundler<'a> {
workflow.to_path_buf()
};
let location = if self.workflow_version_projection {
// Check containment before location resolution can read a config.
manifest_path_from_absolute(&normalized, self.package_root)?;
// Location resolution probes and parses config files, so reject
// references that leave the package root before it can touch a
// host file. `ManifestPath::from_absolute` accepts `..`-prefixed
// results and is not a containment check.
if !normalized.starts_with(self.package_root) {
bail!(
"workflow reference `{}` escapes source root `{}`",
workflow.display(),
self.package_root.display()
);
}
WorkflowLocation::from_exact_path(&normalized, self.package_root)?
} else {
WorkflowLocation::resolve(&normalized, resolve_from)?