Merge pull request #12 from openclaw/codex/ensure-absolute-directory

Add safe absolute directory creation helper

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Changes
 
 - Add a `durable: false` option to async atomic text and JSON writes so callers can preserve replace semantics while skipping temp-file and parent-directory fsync.
+- Add `ensureAbsoluteDirectory()` for creating trusted absolute directory paths one segment at a time while rejecting symlink and non-directory components. (#12; thanks @jesse-merhi)
 
 ### Fixes
 

docs/advanced.md

@@ -38,9 +38,19 @@ The exports group into a handful of themes. Each documented helper has its own p
 | Export | Page | Notes |
 |---|---|---|
 | `assertAbsolutePathInput` | – | Validate a caller-supplied absolute path string. |
+| `ensureAbsoluteDirectory`, `EnsureAbsoluteDirectoryOptions`, `EnsureAbsoluteDirectoryResult` | – | Create a trusted absolute directory path one segment at a time, rejecting symlink or non-directory segments. |
 | `canonicalPathFromExistingAncestor`, `findExistingAncestor` | – | Canonicalize without requiring the leaf to exist. |
 | `resolveAbsolutePathForRead`, `resolveAbsolutePathForWrite`, `ResolvedAbsolutePath`, `ResolvedWritableAbsolutePath`, `AbsolutePathSymlinkPolicy` | – | Validate an absolute path against a symlink policy before opening. |
 
+`ensureAbsoluteDirectory()` is for paths you already intend to trust as absolute
+locations, such as a configured output root. It does not enforce a root boundary;
+use `pathScope().ensureDir()` or `ensureDirectoryWithinRoot()` when the caller
+supplies a path that must stay under a root.
+
+The helper returns `{ ok: false, code, error }` for path-policy failures such as
+relative paths, symlinks, non-directories, or directory swaps during creation.
+Operational filesystem failures such as permissions or I/O errors are rethrown.
+
 ### Files and identity
 
 | Export | Page | Notes |

src/absolute-path.ts

@@ -1,6 +1,12 @@
+import type { Stats } from "node:fs";
 import fs from "node:fs/promises";
 import path from "node:path";
-import { FsSafeError } from "./errors.js";
+import {
+  assertAsyncDirectoryGuard,
+  type AsyncDirectoryGuard,
+  createAsyncDirectoryGuard,
+} from "./directory-guard.js";
+import { FsSafeError, type FsSafeErrorCode } from "./errors.js";
 
 export type AbsolutePathSymlinkPolicy = "reject" | "follow";
 
@@ -14,6 +20,192 @@ export type ResolvedWritableAbsolutePath = ResolvedAbsolutePath & {
   parentExists: boolean;
 };
 
+export type EnsureAbsoluteDirectoryOptions = {
+  scopeLabel?: string;
+  mode?: number;
+};
+
+export type EnsureAbsoluteDirectoryResult =
+  | { ok: true; path: string }
+  | { ok: false; code: FsSafeErrorCode; error: FsSafeError };
+
+type EnsureAbsoluteDirectoryFailure = Extract<EnsureAbsoluteDirectoryResult, { ok: false }>;
+type DirectoryGuardCheckResult = { ok: true } | EnsureAbsoluteDirectoryFailure;
+type DirectoryGuardCreateResult =
+  | { ok: true; guard: AsyncDirectoryGuard }
+  | EnsureAbsoluteDirectoryFailure;
+type DirectoryPrefixResult =
+  | {
+      ok: true;
+      ancestorPath: string;
+      missingSegments: string[];
+    }
+  | EnsureAbsoluteDirectoryFailure;
+
+function ensureDirectoryFailure(
+  code: FsSafeErrorCode,
+  message: string,
+  cause?: unknown,
+): EnsureAbsoluteDirectoryFailure {
+  return {
+    ok: false,
+    code,
+    error: new FsSafeError(code, message, { cause }),
+  };
+}
+
+async function assertGuardResult(
+  guard: AsyncDirectoryGuard,
+  scopeLabel: string,
+): Promise<DirectoryGuardCheckResult> {
+  try {
+    await assertAsyncDirectoryGuard(guard);
+    return { ok: true };
+  } catch (err) {
+    if (err instanceof FsSafeError) {
+      return await directoryGuardFailure(err, guard.dir, scopeLabel);
+    }
+    throw err;
+  }
+}
+
+async function createDirectoryGuardResult(
+  dir: string,
+  scopeLabel: string,
+): Promise<DirectoryGuardCreateResult> {
+  try {
+    return { ok: true, guard: await createAsyncDirectoryGuard(dir) };
+  } catch (err) {
+    if (err instanceof FsSafeError) {
+      return await directoryGuardFailure(err, dir, scopeLabel);
+    }
+    throw err;
+  }
+}
+
+function classifyDirectoryLookupError(
+  err: unknown,
+  scopeLabel: string,
+): EnsureAbsoluteDirectoryFailure | null {
+  const code = (err as NodeJS.ErrnoException).code;
+  if (code === "ENOENT") {
+    return ensureDirectoryFailure(
+      "not-found",
+      `directory path must have a real existing ancestor within ${scopeLabel}`,
+      err,
+    );
+  }
+  if (code === "ENOTDIR") {
+    return ensureDirectoryFailure(
+      "not-file",
+      `path must be a real directory within ${scopeLabel}`,
+      err,
+    );
+  }
+  return null;
+}
+
+function classifyExistingDirectorySegment(
+  stat: Stats,
+  scopeLabel: string,
+): EnsureAbsoluteDirectoryFailure | null {
+  if (stat.isSymbolicLink()) {
+    return ensureDirectoryFailure(
+      "symlink",
+      `directory path traverses a symlink within ${scopeLabel}`,
+    );
+  }
+  if (!stat.isDirectory()) {
+    return ensureDirectoryFailure("not-file", `path must be a real directory within ${scopeLabel}`);
+  }
+  return null;
+}
+
+async function directoryGuardFailure(
+  err: FsSafeError,
+  dir: string,
+  scopeLabel: string,
+): Promise<EnsureAbsoluteDirectoryFailure> {
+  if (err.code !== "not-file") {
+    return { ok: false, code: err.code, error: err };
+  }
+
+  try {
+    const stat = await fs.lstat(dir);
+    const failure = classifyExistingDirectorySegment(stat, scopeLabel);
+    if (failure) {
+      return failure;
+    }
+  } catch (lookupErr) {
+    const failure = classifyDirectoryLookupError(lookupErr, scopeLabel);
+    if (failure) {
+      return failure;
+    }
+    throw lookupErr;
+  }
+  return { ok: false, code: err.code, error: err };
+}
+
+async function resolveTrustedDirectoryPrefix(
+  targetPath: string,
+  scopeLabel: string,
+): Promise<DirectoryPrefixResult> {
+  const root = path.parse(targetPath).root;
+  let current = root;
+  let currentStat: Stats;
+  try {
+    currentStat = await fs.lstat(current);
+  } catch (err) {
+    const failure = classifyDirectoryLookupError(err, scopeLabel);
+    if (failure) {
+      return failure;
+    }
+    throw err;
+  }
+
+  const rootFailure = classifyExistingDirectorySegment(currentStat, scopeLabel);
+  if (rootFailure) {
+    return rootFailure;
+  }
+
+  // Walk forward with lstat. Looking backward for the "nearest existing
+  // ancestor" can cross an existing suffix through a symlinked parent before
+  // this helper gets a chance to reject that parent.
+  const segments = path.relative(root, targetPath).split(path.sep).filter(Boolean);
+  for (let index = 0; index < segments.length; index += 1) {
+    const segment = segments[index];
+    if (!segment) {
+      continue;
+    }
+    const next = path.join(current, segment);
+    try {
+      const nextStat = await fs.lstat(next);
+      const segmentFailure = classifyExistingDirectorySegment(nextStat, scopeLabel);
+      if (segmentFailure) {
+        return segmentFailure;
+      }
+      current = next;
+      currentStat = nextStat;
+    } catch (err) {
+      const code = (err as NodeJS.ErrnoException).code;
+      if (code === "ENOENT") {
+        return {
+          ok: true,
+          ancestorPath: current,
+          missingSegments: segments.slice(index),
+        };
+      }
+      const failure = classifyDirectoryLookupError(err, scopeLabel);
+      if (failure) {
+        return failure;
+      }
+      throw err;
+    }
+  }
+
+  return { ok: true, ancestorPath: current, missingSegments: [] };
+}
+
 export function assertAbsolutePathInput(filePath: string): string {
   if (!filePath) {
     throw new FsSafeError("invalid-path", "path is required");
@@ -37,11 +229,17 @@ async function pathExists(filePath: string): Promise<boolean> {
 }
 
 export async function findExistingAncestor(filePath: string): Promise<string | null> {
+  return (await findExistingAncestorWithStat(filePath))?.path ?? null;
+}
+
+async function findExistingAncestorWithStat(filePath: string): Promise<{
+  path: string;
+  stat: Stats;
+} | null> {
   let current = path.resolve(filePath);
   while (true) {
     try {
-      await fs.lstat(current);
-      return current;
+      return { path: current, stat: await fs.lstat(current) };
     } catch (err) {
       if ((err as NodeJS.ErrnoException).code !== "ENOENT") {
         throw err;
@@ -55,6 +253,90 @@ export async function findExistingAncestor(filePath: string): Promise<string | n
   }
 }
 
+export async function ensureAbsoluteDirectory(
+  dirPath: string,
+  options: EnsureAbsoluteDirectoryOptions = {},
+): Promise<EnsureAbsoluteDirectoryResult> {
+  const scopeLabel = options.scopeLabel ?? "directory";
+  let targetPath: string;
+  try {
+    targetPath = assertAbsolutePathInput(dirPath);
+  } catch (err) {
+    if (err instanceof FsSafeError) {
+      return { ok: false, code: err.code, error: err };
+    }
+    throw err;
+  }
+
+  const prefix = await resolveTrustedDirectoryPrefix(targetPath, scopeLabel);
+  if (!prefix.ok) {
+    return prefix;
+  }
+
+  let current = prefix.ancestorPath;
+  const initialGuard = await createDirectoryGuardResult(prefix.ancestorPath, scopeLabel);
+  if (!initialGuard.ok) {
+    return initialGuard;
+  }
+  let currentGuard: AsyncDirectoryGuard = initialGuard.guard;
+  for (const segment of prefix.missingSegments) {
+    current = path.join(current, segment);
+    while (true) {
+      const guardResult = await assertGuardResult(currentGuard, scopeLabel);
+      if (!guardResult.ok) {
+        return guardResult;
+      }
+      try {
+        const stat = await fs.lstat(current);
+        if (stat.isSymbolicLink()) {
+          return ensureDirectoryFailure(
+            "symlink",
+            `directory path traverses a symlink within ${scopeLabel}`,
+          );
+        }
+        if (!stat.isDirectory()) {
+          return ensureDirectoryFailure(
+            "not-file",
+            `path must be a real directory within ${scopeLabel}`,
+          );
+        }
+        break;
+      } catch (err) {
+        if ((err as NodeJS.ErrnoException).code !== "ENOENT") {
+          throw err;
+        }
+        const parentStillValid = await assertGuardResult(currentGuard, scopeLabel);
+        if (!parentStillValid.ok) {
+          return parentStillValid;
+        }
+        try {
+          await fs.mkdir(current, { mode: options.mode });
+        } catch (mkdirErr) {
+          if ((mkdirErr as NodeJS.ErrnoException).code === "EEXIST") {
+            continue;
+          }
+          throw mkdirErr;
+        }
+      }
+    }
+    const nextGuard = await createDirectoryGuardResult(current, scopeLabel);
+    if (!nextGuard.ok) {
+      return nextGuard;
+    }
+    const previousGuardStillValid = await assertGuardResult(currentGuard, scopeLabel);
+    if (!previousGuardStillValid.ok) {
+      return previousGuardStillValid;
+    }
+    currentGuard = nextGuard.guard;
+  }
+
+  const finalGuardResult = await assertGuardResult(currentGuard, scopeLabel);
+  if (!finalGuardResult.ok) {
+    return finalGuardResult;
+  }
+  return { ok: true, path: targetPath };
+}
+
 export async function canonicalPathFromExistingAncestor(filePath: string): Promise<string> {
   const ancestor = await findExistingAncestor(filePath);
   if (!ancestor) {

src/advanced.ts

@@ -5,10 +5,13 @@ export { createAsyncLock } from "./async-lock.js";
 export {
   assertAbsolutePathInput,
   canonicalPathFromExistingAncestor,
+  ensureAbsoluteDirectory,
   findExistingAncestor,
   resolveAbsolutePathForRead,
   resolveAbsolutePathForWrite,
   type AbsolutePathSymlinkPolicy,
+  type EnsureAbsoluteDirectoryOptions,
+  type EnsureAbsoluteDirectoryResult,
   type ResolvedAbsolutePath,
   type ResolvedWritableAbsolutePath,
 } from "./absolute-path.js";

test/absolute-directory.test.ts

@@ -0,0 +1,185 @@
+import fs from "node:fs/promises";
+import os from "node:os";
+import path from "node:path";
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { ensureAbsoluteDirectory } from "../src/absolute-path.js";
+
+const tempDirs: string[] = [];
+
+async function tempRoot(prefix: string): Promise<string> {
+  const dir = await fs.mkdtemp(path.join(os.tmpdir(), prefix));
+  tempDirs.push(dir);
+  return dir;
+}
+
+afterEach(async () => {
+  vi.restoreAllMocks();
+  await Promise.all(tempDirs.splice(0).map((dir) => fs.rm(dir, { force: true, recursive: true })));
+});
+
+describe("ensureAbsoluteDirectory", () => {
+  it("safely creates missing absolute directory parents from a real ancestor", async () => {
+    const root = await fs.realpath(await tempRoot("fs-safe-absolute-dir-"));
+    const targetDir = path.join(root, "nested", "deeper");
+
+    await expect(
+      ensureAbsoluteDirectory(targetDir, { scopeLabel: "output directory", mode: 0o700 }),
+    ).resolves.toEqual({ ok: true, path: targetDir });
+    expect((await fs.stat(targetDir)).isDirectory()).toBe(true);
+  });
+
+  it("rejects relative absolute-directory inputs", async () => {
+    await expect(
+      ensureAbsoluteDirectory(path.join("..", "..", "..", "escape"), {
+        scopeLabel: "output directory",
+      }),
+    ).resolves.toMatchObject({ ok: false, code: "invalid-path" });
+  });
+
+  it("rejects absolute directory creation when the existing target is not a directory", async () => {
+    const root = await fs.realpath(await tempRoot("fs-safe-absolute-dir-file-"));
+    const targetPath = path.join(root, "file.txt");
+    await fs.writeFile(targetPath, "file", "utf8");
+
+    await expect(
+      ensureAbsoluteDirectory(targetPath, { scopeLabel: "output directory" }),
+    ).resolves.toMatchObject({ ok: false, code: "not-file" });
+  });
+
+  it.runIf(process.platform !== "win32")(
+    "rejects absolute directory creation through symlinked existing segments",
+    async () => {
+      const root = await fs.realpath(await tempRoot("fs-safe-absolute-dir-link-"));
+      const outside = await fs.realpath(await tempRoot("fs-safe-absolute-dir-outside-"));
+      const linkDir = path.join(root, "link");
+      await fs.symlink(outside, linkDir);
+
+      await expect(
+        ensureAbsoluteDirectory(path.join(linkDir, "nested"), {
+          scopeLabel: "output directory",
+        }),
+      ).resolves.toMatchObject({ ok: false, code: "symlink" });
+      await expect(fs.readdir(outside)).resolves.toEqual([]);
+    },
+  );
+
+  it.runIf(process.platform !== "win32")(
+    "rejects symlinked parents even when the requested suffix already exists",
+    async () => {
+      const root = await fs.realpath(await tempRoot("fs-safe-absolute-dir-link-existing-"));
+      const outside = await fs.realpath(
+        await tempRoot("fs-safe-absolute-dir-link-existing-outside-"),
+      );
+      const existing = path.join(outside, "existing");
+      const linkDir = path.join(root, "link");
+      await fs.mkdir(existing);
+      await fs.symlink(outside, linkDir);
+
+      await expect(
+        ensureAbsoluteDirectory(path.join(linkDir, "existing", "new"), {
+          scopeLabel: "output directory",
+        }),
+      ).resolves.toMatchObject({ ok: false, code: "symlink" });
+      await expect(fs.stat(path.join(existing, "new"))).rejects.toMatchObject({ code: "ENOENT" });
+    },
+  );
+
+  it("returns a policy failure when an intermediate component is a file", async () => {
+    const root = await fs.realpath(await tempRoot("fs-safe-absolute-dir-file-component-"));
+    const filePath = path.join(root, "file");
+    await fs.writeFile(filePath, "file", "utf8");
+
+    await expect(
+      ensureAbsoluteDirectory(path.join(filePath, "child"), {
+        scopeLabel: "output directory",
+      }),
+    ).resolves.toMatchObject({ ok: false, code: "not-file" });
+  });
+
+  it.runIf(process.platform !== "win32")(
+    "rejects absolute directory creation when an existing parent is swapped before mkdir",
+    async () => {
+      const root = await fs.realpath(await tempRoot("fs-safe-absolute-dir-race-"));
+      const outside = await fs.realpath(await tempRoot("fs-safe-absolute-dir-race-outside-"));
+      const parentDir = path.join(root, "parent");
+      const targetDir = path.join(parentDir, "child");
+      await fs.mkdir(parentDir);
+
+      const realLstat = fs.lstat.bind(fs);
+      let swapped = false;
+      const lstatSpy = vi.spyOn(fs, "lstat").mockImplementation(async (...args) => {
+        const candidate = String(args[0]);
+        if (!swapped && candidate === targetDir) {
+          swapped = true;
+          await fs.rename(parentDir, `${parentDir}-real`);
+          await fs.symlink(outside, parentDir, "dir");
+        }
+        return await realLstat(...args);
+      });
+
+      try {
+        await expect(
+          ensureAbsoluteDirectory(targetDir, { scopeLabel: "output directory" }),
+        ).resolves.toMatchObject({ ok: false, code: "symlink" });
+      } finally {
+        lstatSpy.mockRestore();
+      }
+
+      await expect(fs.stat(path.join(outside, "child"))).rejects.toMatchObject({ code: "ENOENT" });
+    },
+  );
+
+  it.runIf(process.platform !== "win32")(
+    "rejects absolute directory creation when the existing target changes before return",
+    async () => {
+      const root = await fs.realpath(await tempRoot("fs-safe-absolute-dir-target-race-"));
+      const outside = await fs.realpath(
+        await tempRoot("fs-safe-absolute-dir-target-race-outside-"),
+      );
+      const targetDir = path.join(root, "target");
+      await fs.mkdir(targetDir);
+
+      const realRealpath = fs.realpath.bind(fs);
+      let swapped = false;
+      const realpathSpy = vi.spyOn(fs, "realpath").mockImplementation(async (...args) => {
+        const candidate = String(args[0]);
+        if (!swapped && candidate === targetDir) {
+          swapped = true;
+          const resolved = await realRealpath(...args);
+          await fs.rename(targetDir, `${targetDir}-real`);
+          await fs.symlink(outside, targetDir, "dir");
+          return resolved;
+        }
+        return await realRealpath(...args);
+      });
+
+      try {
+        await expect(
+          ensureAbsoluteDirectory(targetDir, { scopeLabel: "output directory" }),
+        ).resolves.toMatchObject({ ok: false, code: "symlink" });
+      } finally {
+        realpathSpy.mockRestore();
+      }
+    },
+  );
+
+  it("rethrows operational absolute directory creation failures", async () => {
+    const root = await fs.realpath(await tempRoot("fs-safe-absolute-dir-io-"));
+    const targetDir = path.join(root, "nested");
+    const realMkdir = fs.mkdir.bind(fs);
+    const mkdirSpy = vi.spyOn(fs, "mkdir").mockImplementation(async (...args) => {
+      if (String(args[0]) === targetDir) {
+        throw Object.assign(new Error("permission denied"), { code: "EACCES" });
+      }
+      return await realMkdir(...args);
+    });
+
+    try {
+      await expect(
+        ensureAbsoluteDirectory(targetDir, { scopeLabel: "output directory" }),
+      ).rejects.toMatchObject({ code: "EACCES" });
+    } finally {
+      mkdirSpy.mockRestore();
+    }
+  });
+});

test/coverage-gaps.test.ts

@@ -242,6 +242,7 @@ describe("absolute path helpers", () => {
       code: "symlink",
     });
   });
+
 });
 
 describe("filesystem utility helpers", () => {