fix: address review feedback on !include and folder workflow
CI / check (pull_request) Failing after 1m37s
CI / check (pull_request) Failing after 1m37s
- 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
This commit is contained in:
@@ -57,4 +57,28 @@ describe("!include tag", () => {
|
|||||||
const result = parse(yaml, { customTags: [createIncludeTag(tmpDir)] });
|
const result = parse(yaml, { customTags: [createIncludeTag(tmpDir)] });
|
||||||
expect(result.note).toBe("Hello world");
|
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" });
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1,23 +1,35 @@
|
|||||||
import { readFileSync } from "node:fs";
|
import { readFileSync } from "node:fs";
|
||||||
import { extname, resolve } from "node:path";
|
import { dirname, extname, resolve } from "node:path";
|
||||||
import { parse as parseYaml } from "yaml";
|
import { parse as parseYaml } from "yaml";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Create a YAML customTags entry for !include that resolves file paths
|
* Create a YAML customTags entry for !include that resolves file paths
|
||||||
* relative to the given base directory.
|
* 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) {
|
export function createIncludeTag(baseDir: string) {
|
||||||
|
const resolvedBase = resolve(baseDir);
|
||||||
return {
|
return {
|
||||||
tag: "!include",
|
tag: "!include",
|
||||||
resolve(str: string) {
|
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 content = readFileSync(filePath, "utf8");
|
||||||
const ext = extname(filePath).toLowerCase();
|
const ext = extname(filePath).toLowerCase();
|
||||||
if (ext === ".json") {
|
if (ext === ".json") {
|
||||||
return JSON.parse(content);
|
return JSON.parse(content);
|
||||||
}
|
}
|
||||||
if (ext === ".yaml" || ext === ".yml") {
|
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;
|
return content;
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -21,13 +21,10 @@ export type ProjectWorkflowEntry = {
|
|||||||
};
|
};
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Scan `<projectRoot>/.workflows/*.yaml` (non-recursive) and return discovered entries.
|
* Scan a single directory for workflow entries (flat YAML files + folder/index.yaml).
|
||||||
* Returns an empty array if the directory does not exist.
|
* Returns discovered entries. Returns empty array if directory does not exist.
|
||||||
*/
|
*/
|
||||||
export async function discoverProjectWorkflows(
|
async function scanWorkflowDir(dir: string): Promise<ProjectWorkflowEntry[]> {
|
||||||
projectRoot: string,
|
|
||||||
): Promise<ProjectWorkflowEntry[]> {
|
|
||||||
const dir = join(projectRoot, ".workflows");
|
|
||||||
let dirents: Dirent[];
|
let dirents: Dirent[];
|
||||||
try {
|
try {
|
||||||
dirents = await readdir(dir, { withFileTypes: true });
|
dirents = await readdir(dir, { withFileTypes: true });
|
||||||
@@ -60,6 +57,26 @@ export async function discoverProjectWorkflows(
|
|||||||
return result;
|
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`). */
|
/** Default filesystem root for uwf data (`~/.uncaged/workflow`). */
|
||||||
export function getDefaultStorageRoot(): string {
|
export function getDefaultStorageRoot(): string {
|
||||||
return join(homedir(), ".uncaged", "workflow");
|
return join(homedir(), ".uncaged", "workflow");
|
||||||
|
|||||||
Reference in New Issue
Block a user