fix(agents): tighten workspace file opens (#66636)

* fix(agents): tighten workspace file opens

* fix(agents): clarify symlink rejection tests

* fix(agents): surface unsafe identity reads

* fix(agents): use non-blocking opens for identity reads and write-mode probes

* fix(fssafe): restore symlink read identity check

* fix(worklog): append comment resolution status

* fix(fssafe): close afterOpen handle leaks

* fix(worklog): append comment resolution follow-up

* fix(worklog): drop internal user file

* fix(agents): rethrow unexpected errors in agents.files.get

* changelog: note agents.files fs-safe routing + fd-first realpath (#66636)

* fix(agents): rethrow unexpected errors in agents.files.set too

Match the narrow-SafeOpenError catch pattern that agents.files.get
(commit 633b8f92) and writeWorkspaceFileOrRespond already use, so a
real OS error (ENOSPC, EACCES, EBUSY, ...) surfaces through normal
gateway error handling instead of being masked as
'unsafe workspace file'.

* test(agents): match fsStat/fsLstat mock signatures

The mock functions are declared as
  vi.fn(async (..._args: unknown[]) => Stats | null)
so mockImplementation callbacks must accept ...unknown[], not a
narrowed (filePath: string) argument. The narrower signature
works at runtime but trips tsgo's strict type check; switch to
args[0] unpacking so the callbacks match the hoisted mock shape.

---------

Co-authored-by: Devin Robison <drobison@nvidia.com>

src/infra/fs-safe.test.ts

@@ -1,3 +1,4 @@
+import type { FileHandle } from "node:fs/promises";
 import fs from "node:fs/promises";
 import path from "node:path";
 import { afterEach, describe, expect, it, vi } from "vitest";
@@ -8,10 +9,12 @@ import {
 import { createTrackedTempDirs } from "../test-utils/tracked-temp-dirs.js";
 import * as pinnedPathHelperModule from "./fs-pinned-path-helper.js";
 import {
+  __setFsSafeTestHooksForTest,
   appendFileWithinRoot,
   copyFileWithinRoot,
   createRootScopedReadFile,
   mkdirPathWithinRoot,
+  resolveOpenedFileRealPathForHandle,
   SafeOpenError,
   openFileWithinRoot,
   readFileWithinRoot,
@@ -25,6 +28,8 @@ import {
 const tempDirs = createTrackedTempDirs();
 
 afterEach(async () => {
+  __setFsSafeTestHooksForTest(undefined);
+  vi.unstubAllEnvs();
   await tempDirs.cleanup();
 });
 
@@ -149,6 +154,32 @@ describe("fs-safe", () => {
     });
   });
 
+  it.runIf(process.platform !== "win32")(
+    "resolves opened file real paths from the fd before the current path target",
+    async () => {
+      const root = await tempDirs.make("openclaw-fs-safe-root-");
+      const outside = await tempDirs.make("openclaw-fs-safe-outside-");
+      const originalPath = path.join(root, "inside.txt");
+      const movedPath = path.join(root, "inside-moved.txt");
+      const outsidePath = path.join(outside, "outside.txt");
+      await fs.writeFile(originalPath, "inside");
+      await fs.writeFile(outsidePath, "outside");
+
+      const handle = await fs.open(originalPath, "r");
+      try {
+        await fs.rename(originalPath, movedPath);
+        await fs.symlink(outsidePath, originalPath);
+
+        const resolved = await resolveOpenedFileRealPathForHandle(handle, originalPath);
+
+        expect(resolved).toBe(movedPath);
+        await expect(handle.readFile({ encoding: "utf8" })).resolves.toBe("inside");
+      } finally {
+        await handle.close().catch(() => {});
+      }
+    },
+  );
+
   it("blocks traversal outside root", async () => {
     const root = await tempDirs.make("openclaw-fs-safe-root-");
     const outside = await tempDirs.make("openclaw-fs-safe-outside-");
@@ -221,6 +252,70 @@ describe("fs-safe", () => {
     ).rejects.toMatchObject({ code: "invalid-path" });
   });
 
+  it.runIf(process.platform !== "win32")(
+    "rejects symlink-target reads when the path target changes after open",
+    async () => {
+      const root = await tempDirs.make("openclaw-fs-safe-root-");
+      const insideA = path.join(root, "inside-a.txt");
+      const insideB = path.join(root, "inside-b.txt");
+      const link = path.join(root, "link.txt");
+      await fs.writeFile(insideA, "inside-a");
+      await fs.writeFile(insideB, "inside-b");
+      await fs.symlink(insideA, link);
+
+      __setFsSafeTestHooksForTest({
+        afterOpen: async () => {
+          await fs.rm(link);
+          await fs.symlink(insideB, link);
+        },
+      });
+
+      await expect(
+        readFileWithinRoot({
+          rootDir: root,
+          relativePath: "link.txt",
+          allowSymlinkTargetWithinRoot: true,
+        }),
+      ).rejects.toMatchObject({ code: "invalid-path" });
+    },
+  );
+
+  it("closes the opened handle when afterOpen hook throws", async () => {
+    const root = await tempDirs.make("openclaw-fs-safe-root-");
+    const filePath = path.join(root, "inside.txt");
+    await fs.writeFile(filePath, "inside");
+
+    let openedHandle: FileHandle | undefined;
+    __setFsSafeTestHooksForTest({
+      afterOpen: (_target, handle) => {
+        openedHandle = handle;
+        throw new Error("after-open boom");
+      },
+    });
+
+    await expect(
+      openFileWithinRoot({
+        rootDir: root,
+        relativePath: "inside.txt",
+      }),
+    ).rejects.toThrow("after-open boom");
+    expect(openedHandle).toBeDefined();
+    await expect(openedHandle?.readFile({ encoding: "utf8" })).rejects.toMatchObject({
+      code: "EBADF",
+    });
+  });
+
+  it("rejects setting fs-safe test hooks outside test mode", async () => {
+    vi.stubEnv("NODE_ENV", "production");
+    vi.stubEnv("VITEST", undefined);
+
+    expect(() =>
+      __setFsSafeTestHooksForTest({
+        afterPreOpenLstat: () => {},
+      }),
+    ).toThrow("__setFsSafeTestHooksForTest is only available in tests");
+  });
+
   it.runIf(process.platform !== "win32")("blocks hardlink aliases under root", async () => {
     const root = await tempDirs.make("openclaw-fs-safe-root-");
     const hardlinkPath = path.join(root, "link.txt");

src/infra/fs-safe.ts

@@ -53,11 +53,19 @@ export type SafeLocalReadResult = {
 export type FsSafeTestHooks = {
   afterPreOpenLstat?: (filePath: string) => Promise<void> | void;
   beforeOpen?: (filePath: string, flags: number) => Promise<void> | void;
+  afterOpen?: (filePath: string, handle: FileHandle) => Promise<void> | void;
 };
 
 let fsSafeTestHooks: FsSafeTestHooks | undefined;
 
+function allowFsSafeTestHooks(): boolean {
+  return process.env.NODE_ENV === "test" || process.env.VITEST === "true";
+}
+
 export function __setFsSafeTestHooksForTest(hooks?: FsSafeTestHooks): void {
+  if (hooks && !allowFsSafeTestHooks()) {
+    throw new Error("__setFsSafeTestHooksForTest is only available in tests");
+  }
   fsSafeTestHooks = hooks;
 }
 
@@ -129,6 +137,12 @@ async function openVerifiedLocalFile(
         : OPEN_READ_FLAGS;
     await fsSafeTestHooks?.beforeOpen?.(filePath, openFlags);
     handle = await fs.open(filePath, openFlags);
+    try {
+      await fsSafeTestHooks?.afterOpen?.(filePath, handle);
+    } catch (err) {
+      await handle.close().catch(() => {});
+      throw err;
+    }
   } catch (err) {
     if (isNotFoundPathError(err)) {
       throw new SafeOpenError("not-found", "file not found");
@@ -144,24 +158,30 @@ async function openVerifiedLocalFile(
   }
 
   try {
-    const [stat, pathStat] = await Promise.all([
-      handle.stat(),
-      options?.allowSymlinkTargetWithinRoot ? fs.stat(filePath) : fs.lstat(filePath),
-    ]);
-    if (!options?.allowSymlinkTargetWithinRoot && pathStat.isSymbolicLink()) {
-      throw new SafeOpenError("symlink", "symlink not allowed");
-    }
+    const stat = await handle.stat();
     if (!stat.isFile()) {
       throw new SafeOpenError("not-file", "not a file");
     }
     if (options?.rejectHardlinks && stat.nlink > 1) {
       throw new SafeOpenError("invalid-path", "hardlinked path not allowed");
     }
-    if (!sameFileIdentity(stat, pathStat)) {
-      throw new SafeOpenError("path-mismatch", "path changed during read");
+
+    if (options?.allowSymlinkTargetWithinRoot) {
+      const pathStat = await fs.stat(filePath);
+      if (!sameFileIdentity(stat, pathStat)) {
+        throw new SafeOpenError("path-mismatch", "path changed during read");
+      }
+    } else {
+      const pathStat = await fs.lstat(filePath);
+      if (pathStat.isSymbolicLink()) {
+        throw new SafeOpenError("symlink", "symlink not allowed");
+      }
+      if (!sameFileIdentity(stat, pathStat)) {
+        throw new SafeOpenError("path-mismatch", "path changed during read");
+      }
     }
 
-    const realPath = await fs.realpath(filePath);
+    const realPath = await resolveOpenedFileRealPathForHandle(handle, filePath);
     const realStat = await fs.stat(realPath);
     if (options?.rejectHardlinks && realStat.nlink > 1) {
       throw new SafeOpenError("invalid-path", "hardlinked path not allowed");
@@ -397,14 +417,6 @@ export async function resolveOpenedFileRealPathForHandle(
   handle: FileHandle,
   ioPath: string,
 ): Promise<string> {
-  try {
-    return await fs.realpath(ioPath);
-  } catch (err) {
-    if (!isNotFoundPathError(err)) {
-      throw err;
-    }
-  }
-
   const fdCandidates =
     process.platform === "linux"
       ? [`/proc/self/fd/${handle.fd}`, `/dev/fd/${handle.fd}`]
@@ -418,6 +430,14 @@ export async function resolveOpenedFileRealPathForHandle(
       // try next fd path
     }
   }
+
+  try {
+    return await fs.realpath(ioPath);
+  } catch (err) {
+    if (!isNotFoundPathError(err)) {
+      throw err;
+    }
+  }
   throw new SafeOpenError("path-mismatch", "unable to resolve opened file path");
 }
 
@@ -792,6 +812,7 @@ async function resolvePinnedWriteTargetWithinRoot(params: {
       rootDir: params.rootDir,
       relativePath: params.relativePath,
       rejectHardlinks: true,
+      nonBlockingRead: true,
     });
     try {
       mode = opened.stat.mode & 0o777;