fix: harden fs-safe write boundary checks

CHANGELOG.md

@@ -50,6 +50,7 @@ Docs: https://docs.openclaw.ai
 - Security/Node exec approvals: preserve shell/dispatch-wrapper argv semantics during approval hardening so approved wrapper commands (for example `env sh -c ...`) cannot drift into a different runtime command shape, and add regression coverage for both approval-plan generation and approved runtime execution paths. Thanks @tdjackey for reporting.
 - Security/ACP sandbox inheritance: enforce fail-closed runtime guardrails for `sessions_spawn` with `runtime="acp"` by rejecting ACP spawns from sandboxed requester sessions and rejecting `sandbox="require"` for ACP runtime, preventing sandbox-boundary bypass via host-side ACP initialization. (#32254) Thanks @tdjackey for reporting, and @dutifulbob for the fix.
 - Browser/Security output boundary hardening: replace check-then-rename output commits with root-bound fd-verified writes, unify install/skills canonical path-boundary checks, and add regression coverage for symlink-rebind race paths across browser output and shared fs-safe write flows. Thanks @tdjackey for reporting.
+- Security/fs-safe write hardening: make `writeFileWithinRoot` use same-directory temp writes plus atomic rename, add post-write inode/hardlink revalidation with security warnings on boundary drift, and avoid truncating existing targets when final rename fails.
 - Security/Webhook request hardening: enforce auth-before-body parsing for BlueBubbles and Google Chat webhook handlers, add strict pre-auth body/time budgets for webhook auth paths (including LINE signature verification), and add shared in-flight/request guardrails plus regression tests/lint checks to prevent reintroducing unauthenticated slow-body DoS patterns. Thanks @GCXWLP for reporting.
 - Gateway/Security hardening: tie loopback-origin dev allowance to actual local socket clients (not Host header claims), add explicit warnings/metrics when `gateway.controlUi.dangerouslyAllowHostHeaderOriginFallback` accepts websocket origins, harden safe-regex detection for quantified ambiguous alternation patterns (for example `(a|aa)+`), and bound large regex-evaluation inputs for session-filter and log-redaction paths.
 - Security/Skills archive extraction: unify tar extraction safety checks across tar.gz and tar.bz2 install flows, enforce tar compressed-size limits, and fail closed if tar.bz2 archives change between preflight and extraction to prevent bypasses of entry-type/size guardrails. Thanks @GCXWLP for reporting.

src/infra/fs-safe.test.ts

@@ -246,6 +246,60 @@ describe("fs-safe", () => {
     await expect(fs.readFile(path.join(root, "nested", "out.txt"), "utf8")).resolves.toBe("hello");
   });
 
+  it("does not truncate existing target when atomic rename fails", async () => {
+    const root = await tempDirs.make("openclaw-fs-safe-root-");
+    const targetPath = path.join(root, "nested", "out.txt");
+    await fs.mkdir(path.dirname(targetPath), { recursive: true });
+    await fs.writeFile(targetPath, "existing-content");
+    const renameSpy = vi
+      .spyOn(fs, "rename")
+      .mockRejectedValue(Object.assign(new Error("rename blocked"), { code: "EACCES" }));
+    try {
+      await expect(
+        writeFileWithinRoot({
+          rootDir: root,
+          relativePath: "nested/out.txt",
+          data: "new-content",
+        }),
+      ).rejects.toMatchObject({ code: "EACCES" });
+    } finally {
+      renameSpy.mockRestore();
+    }
+    await expect(fs.readFile(targetPath, "utf8")).resolves.toBe("existing-content");
+  });
+
+  it.runIf(process.platform !== "win32")(
+    "rejects when a hardlink appears after atomic write rename",
+    async () => {
+      const root = await tempDirs.make("openclaw-fs-safe-root-");
+      const targetPath = path.join(root, "nested", "out.txt");
+      const aliasPath = path.join(root, "nested", "alias.txt");
+      await fs.mkdir(path.dirname(targetPath), { recursive: true });
+      await fs.writeFile(targetPath, "existing-content");
+      const realRename = fs.rename.bind(fs);
+      let linked = false;
+      const renameSpy = vi.spyOn(fs, "rename").mockImplementation(async (...args) => {
+        await realRename(...args);
+        if (!linked) {
+          linked = true;
+          await fs.link(String(args[1]), aliasPath);
+        }
+      });
+      try {
+        await expect(
+          writeFileWithinRoot({
+            rootDir: root,
+            relativePath: "nested/out.txt",
+            data: "new-content",
+          }),
+        ).rejects.toMatchObject({ code: "invalid-path" });
+      } finally {
+        renameSpy.mockRestore();
+      }
+      await expect(fs.readFile(aliasPath, "utf8")).resolves.toBe("new-content");
+    },
+  );
+
   it("copies a file within root safely", async () => {
     const root = await tempDirs.make("openclaw-fs-safe-root-");
     const sourceDir = await tempDirs.make("openclaw-fs-safe-source-");

src/infra/fs-safe.ts

@@ -1,3 +1,4 @@
+import { randomUUID } from "node:crypto";
 import type { Stats } from "node:fs";
 import { constants as fsConstants } from "node:fs";
 import type { FileHandle } from "node:fs/promises";
@@ -5,6 +6,7 @@ import fs from "node:fs/promises";
 import os from "node:os";
 import path from "node:path";
 import { pipeline } from "node:stream/promises";
+import { logWarn } from "../logger.js";
 import { sameFileIdentity } from "./file-identity.js";
 import { expandHomePrefix } from "./home-dir.js";
 import { assertNoPathAliasEscape } from "./path-alias-guards.js";
@@ -287,8 +289,58 @@ export type SafeWritableOpenResult = {
   handle: FileHandle;
   createdForWrite: boolean;
   openedRealPath: string;
+  openedStat: Stats;
 };
 
+function emitWriteBoundaryWarning(reason: string) {
+  logWarn(`security: fs-safe write boundary warning (${reason})`);
+}
+
+function buildAtomicWriteTempPath(targetPath: string): string {
+  const dir = path.dirname(targetPath);
+  const base = path.basename(targetPath);
+  return path.join(dir, `.${base}.${process.pid}.${randomUUID()}.tmp`);
+}
+
+async function writeTempFileForAtomicReplace(params: {
+  tempPath: string;
+  data: string | Buffer;
+  encoding?: BufferEncoding;
+  mode: number;
+}): Promise<Stats> {
+  const tempHandle = await fs.open(params.tempPath, OPEN_WRITE_CREATE_FLAGS, params.mode);
+  try {
+    if (typeof params.data === "string") {
+      await tempHandle.writeFile(params.data, params.encoding ?? "utf8");
+    } else {
+      await tempHandle.writeFile(params.data);
+    }
+    return await tempHandle.stat();
+  } finally {
+    await tempHandle.close().catch(() => {});
+  }
+}
+
+async function verifyAtomicWriteResult(params: {
+  rootDir: string;
+  targetPath: string;
+  expectedStat: Stats;
+}): Promise<void> {
+  const rootReal = await fs.realpath(params.rootDir);
+  const rootWithSep = ensureTrailingSep(rootReal);
+  const opened = await openVerifiedLocalFile(params.targetPath, { rejectHardlinks: true });
+  try {
+    if (!sameFileIdentity(opened.stat, params.expectedStat)) {
+      throw new SafeOpenError("path-mismatch", "path changed during write");
+    }
+    if (!isPathInside(rootWithSep, opened.realPath)) {
+      throw new SafeOpenError("outside-workspace", "file is outside workspace root");
+    }
+  } finally {
+    await opened.handle.close().catch(() => {});
+  }
+}
+
 export async function resolveOpenedFileRealPathForHandle(
   handle: FileHandle,
   ioPath: string,
@@ -322,6 +374,7 @@ export async function openWritableFileWithinRoot(params: {
   relativePath: string;
   mkdir?: boolean;
   mode?: number;
+  truncateExisting?: boolean;
 }): Promise<SafeWritableOpenResult> {
   const { rootReal, rootWithSep, resolved } = await resolvePathWithinRoot(params);
   try {
@@ -416,13 +469,14 @@ export async function openWritableFileWithinRoot(params: {
 
     // Truncate only after boundary and identity checks complete. This avoids
     // irreversible side effects if a symlink target changes before validation.
-    if (!createdForWrite) {
+    if (params.truncateExisting !== false && !createdForWrite) {
       await handle.truncate(0);
     }
     return {
       handle,
       createdForWrite,
       openedRealPath: realPath,
+      openedStat: stat,
     };
   } catch (err) {
     const cleanupCreatedPath = createdForWrite && err instanceof SafeOpenError;
@@ -446,15 +500,36 @@ export async function writeFileWithinRoot(params: {
     rootDir: params.rootDir,
     relativePath: params.relativePath,
     mkdir: params.mkdir,
+    truncateExisting: false,
   });
+  const destinationPath = target.openedRealPath;
+  const targetMode = target.openedStat.mode & 0o777;
+  await target.handle.close().catch(() => {});
+  let tempPath: string | null = null;
   try {
-    if (typeof params.data === "string") {
-      await target.handle.writeFile(params.data, params.encoding ?? "utf8");
-    } else {
-      await target.handle.writeFile(params.data);
+    tempPath = buildAtomicWriteTempPath(destinationPath);
+    const writtenStat = await writeTempFileForAtomicReplace({
+      tempPath,
+      data: params.data,
+      encoding: params.encoding,
+      mode: targetMode || 0o600,
+    });
+    await fs.rename(tempPath, destinationPath);
+    tempPath = null;
+    try {
+      await verifyAtomicWriteResult({
+        rootDir: params.rootDir,
+        targetPath: destinationPath,
+        expectedStat: writtenStat,
+      });
+    } catch (err) {
+      emitWriteBoundaryWarning(`post-write verification failed: ${String(err)}`);
+      throw err;
     }
   } finally {
-    await target.handle.close().catch(() => {});
+    if (tempPath) {
+      await fs.rm(tempPath, { force: true }).catch(() => {});
+    }
   }
 }
 
@@ -477,11 +552,7 @@ export async function copyFileWithinRoot(params: {
     );
   }
 
-  let target: {
-    handle: FileHandle;
-    createdForWrite: boolean;
-    openedRealPath: string;
-  } | null = null;
+  let target: SafeWritableOpenResult | null = null;
   let sourceClosedByStream = false;
   let targetClosedByStream = false;
   try {