From 111e737d5376e78b7e3dc033bb82c164f328cf8a Mon Sep 17 00:00:00 2001
From: Scott Werner <stwerner@vt.edu>
Date: Fri, 11 Sep 2026 15:22:04 -0600
Subject: [PATCH] Reject escaping workflow references before resolving them

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>
---
 .../fabro-manifest/src/supplied_workflow.rs   | 29 +++++++++++++++++--
 .../fabro-manifest/src/workflow_bundler.rs    | 13 +++++++--
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/lib/components/fabro-manifest/src/supplied_workflow.rs b/lib/components/fabro-manifest/src/supplied_workflow.rs
index 3d0d7ab5b..9f247294c 100644
--- a/lib/components/fabro-manifest/src/supplied_workflow.rs
+++ b/lib/components/fabro-manifest/src/supplied_workflow.rs
@@ -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());
         }
diff --git a/lib/components/fabro-manifest/src/workflow_bundler.rs b/lib/components/fabro-manifest/src/workflow_bundler.rs
index 6e256d6a3..9eb0add4a 100644
--- a/lib/components/fabro-manifest/src/workflow_bundler.rs
+++ b/lib/components/fabro-manifest/src/workflow_bundler.rs
@@ -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)?