From da1678ffef2375ea027a9b3bad6c68fa4f62d7a2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E5=B0=8F=E5=A2=A8?= <xiaomooo@shazhou.work>
Date: Sun, 31 May 2026 04:26:54 +0000
Subject: [PATCH] fix: address review feedback on !include and folder workflow

- Fix nested !include: pass customTags recursively, scoped to included file's dir
- Add path traversal guard: !include paths must resolve within base directory
- Fix discoverProjectWorkflows: scan both .workflow/ and .workflows/ (consistent with findWorkflowInDir)
- Add tests: path traversal blocking, nested !include, absolute path rejection
---
 .../src/__tests__/include-tag.test.ts         | 24 +++++++++++++++
 packages/cli-workflow/src/include.ts          | 18 ++++++++++--
 packages/cli-workflow/src/store.ts            | 29 +++++++++++++++----
 3 files changed, 62 insertions(+), 9 deletions(-)

diff --git a/packages/cli-workflow/src/__tests__/include-tag.test.ts b/packages/cli-workflow/src/__tests__/include-tag.test.ts
index 8c738ed..a061c06 100644
--- a/packages/cli-workflow/src/__tests__/include-tag.test.ts
+++ b/packages/cli-workflow/src/__tests__/include-tag.test.ts
@@ -57,4 +57,28 @@ describe("!include tag", () => {
     const result = parse(yaml, { customTags: [createIncludeTag(tmpDir)] });
     expect(result.note).toBe("Hello world");
   });
+
+  test("blocks path traversal with ../", async () => {
+    const yaml = "secret: !include ../../etc/passwd";
+    expect(() => parse(yaml, { customTags: [createIncludeTag(tmpDir)] })).toThrow(
+      /path traversal blocked/,
+    );
+  });
+
+  test("blocks absolute path traversal", async () => {
+    const yaml = "secret: !include /etc/passwd";
+    expect(() => parse(yaml, { customTags: [createIncludeTag(tmpDir)] })).toThrow(
+      /path traversal blocked/,
+    );
+  });
+
+  test("supports nested !include in yaml files", async () => {
+    const subdir = join(tmpDir, "parts");
+    await mkdir(subdir, { recursive: true });
+    await writeFile(join(subdir, "inner.md"), "nested content");
+    await writeFile(join(tmpDir, "outer.yaml"), "value: !include parts/inner.md");
+    const yaml = "config: !include outer.yaml";
+    const result = parse(yaml, { customTags: [createIncludeTag(tmpDir)] });
+    expect(result.config).toEqual({ value: "nested content" });
+  });
 });
diff --git a/packages/cli-workflow/src/include.ts b/packages/cli-workflow/src/include.ts
index 4e4014f..b34bb01 100644
--- a/packages/cli-workflow/src/include.ts
+++ b/packages/cli-workflow/src/include.ts
@@ -1,23 +1,35 @@
 import { readFileSync } from "node:fs";
-import { extname, resolve } from "node:path";
+import { dirname, extname, resolve } from "node:path";
 import { parse as parseYaml } from "yaml";
 
 /**
  * Create a YAML customTags entry for !include that resolves file paths
  * relative to the given base directory.
+ *
+ * Security: resolved paths must stay within baseDir (path traversal prevention).
+ * Nested !include in .yaml/.yml files is supported (customTags passed recursively).
  */
 export function createIncludeTag(baseDir: string) {
+  const resolvedBase = resolve(baseDir);
   return {
     tag: "!include",
     resolve(str: string) {
-      const filePath = resolve(baseDir, str);
+      const filePath = resolve(resolvedBase, str);
+      // Path traversal guard: resolved path must be inside baseDir
+      if (!filePath.startsWith(resolvedBase + "/") && filePath !== resolvedBase) {
+        throw new Error(
+          `!include path traversal blocked: "${str}" resolves outside base directory`,
+        );
+      }
       const content = readFileSync(filePath, "utf8");
       const ext = extname(filePath).toLowerCase();
       if (ext === ".json") {
         return JSON.parse(content);
       }
       if (ext === ".yaml" || ext === ".yml") {
-        return parseYaml(content);
+        // Pass customTags recursively so nested !include works,
+        // scoped to the included file's directory
+        return parseYaml(content, { customTags: [createIncludeTag(dirname(filePath))] });
       }
       return content;
     },
diff --git a/packages/cli-workflow/src/store.ts b/packages/cli-workflow/src/store.ts
index 4abb41b..7b1f847 100644
--- a/packages/cli-workflow/src/store.ts
+++ b/packages/cli-workflow/src/store.ts
@@ -21,13 +21,10 @@ export type ProjectWorkflowEntry = {
 };
 
 /**
- * Scan `<projectRoot>/.workflows/*.yaml` (non-recursive) and return discovered entries.
- * Returns an empty array if the directory does not exist.
+ * Scan a single directory for workflow entries (flat YAML files + folder/index.yaml).
+ * Returns discovered entries. Returns empty array if directory does not exist.
  */
-export async function discoverProjectWorkflows(
-  projectRoot: string,
-): Promise<ProjectWorkflowEntry[]> {
-  const dir = join(projectRoot, ".workflows");
+async function scanWorkflowDir(dir: string): Promise<ProjectWorkflowEntry[]> {
   let dirents: Dirent[];
   try {
     dirents = await readdir(dir, { withFileTypes: true });
@@ -60,6 +57,26 @@ export async function discoverProjectWorkflows(
   return result;
 }
 
+/**
+ * Scan `<projectRoot>/.workflow/` (preferred) and `.workflows/` (legacy) for workflow entries.
+ * .workflow/ takes priority: if a name is found in both, .workflow/ wins.
+ * Returns an empty array if neither directory exists.
+ */
+export async function discoverProjectWorkflows(
+  projectRoot: string,
+): Promise<ProjectWorkflowEntry[]> {
+  const primary = await scanWorkflowDir(join(projectRoot, ".workflow"));
+  const legacy = await scanWorkflowDir(join(projectRoot, ".workflows"));
+  const seen = new Set(primary.map((e) => e.name));
+  const merged = [...primary];
+  for (const entry of legacy) {
+    if (!seen.has(entry.name)) {
+      merged.push(entry);
+    }
+  }
+  return merged;
+}
+
 /** Default filesystem root for uwf data (`~/.uncaged/workflow`). */
 export function getDefaultStorageRoot(): string {
   return join(homedir(), ".uncaged", "workflow");