fix(agents): reject false success from delegated file mutations (#117938)

* fix(agents): verify delegated writes before reporting success

* fix(agents): verify delegated edits before success

---------

Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com>

src/agents/agent-tools.read.host-operations.test.ts

@@ -8,15 +8,19 @@ import os from "node:os";
 import path from "node:path";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { withEnvAsync } from "../test-utils/env.js";
+import type { SandboxFsBridge } from "./sandbox/fs-bridge.js";
 
 type CapturedEditOperations = {
   readFile: (absolutePath: string) => Promise<Buffer>;
   writeFile: (absolutePath: string, content: string) => Promise<void>;
+  statFile: (absolutePath: string) => Promise<unknown>;
   access: (absolutePath: string) => Promise<void>;
 };
 
 type CapturedWriteOperations = {
   mkdir: (dir: string) => Promise<void>;
+  readFile: (absolutePath: string) => Promise<Buffer | string>;
+  statFile: (absolutePath: string) => Promise<unknown>;
   writeFile: (absolutePath: string, content: string) => Promise<void>;
 };
 
@@ -50,8 +54,12 @@ vi.mock("./sessions/index.js", async () => {
   };
 });
 
-const { createHostWorkspaceEditTool, createHostWorkspaceWriteTool } =
-  await import("./agent-tools.read.js");
+const {
+  createHostWorkspaceEditTool,
+  createHostWorkspaceWriteTool,
+  createSandboxedEditTool,
+  createSandboxedWriteTool,
+} = await import("./agent-tools.read.js");
 
 const osHome = () => process.env.HOME ?? os.homedir();
 const toTildePath = (absolutePath: string) => absolutePath.replace(osHome(), "~");
@@ -228,3 +236,33 @@ describe("createHostWorkspaceEditTool host access mapping", () => {
     },
   );
 });
+
+describe("file mutation verification operations", () => {
+  it("provides readback and stat operations to host writes and edits", () => {
+    createHostWorkspaceWriteTool("/workspace", { workspaceOnly: false });
+
+    expect(readWriteOps().readFile).toBeTypeOf("function");
+    expect(readWriteOps().statFile).toBeTypeOf("function");
+
+    createHostWorkspaceEditTool("/workspace", { workspaceOnly: false });
+
+    expect(readEditOps().readFile).toBeTypeOf("function");
+    expect(readEditOps().statFile).toBeTypeOf("function");
+  });
+
+  it("provides readback and stat operations to sandbox writes and edits", () => {
+    const params = {
+      root: "/workspace",
+      bridge: {} as SandboxFsBridge,
+    };
+    createSandboxedWriteTool(params);
+
+    expect(readWriteOps().readFile).toBeTypeOf("function");
+    expect(readWriteOps().statFile).toBeTypeOf("function");
+
+    createSandboxedEditTool(params);
+
+    expect(readEditOps().readFile).toBeTypeOf("function");
+    expect(readEditOps().statFile).toBeTypeOf("function");
+  });
+});

src/agents/agent-tools.read.ts

@@ -1077,6 +1077,8 @@ function createSandboxEditOperations(params: SandboxToolParams) {
         params.bridge.readFile({ filePath: absolutePath, cwd: params.root }),
       writeFile: (absolutePath: string, content: string) =>
         params.bridge.writeFile({ filePath: absolutePath, cwd: params.root, data: content }),
+      statFile: (absolutePath: string) =>
+        params.bridge.stat({ filePath: absolutePath, cwd: params.root }),
       access: (absolutePath: string) => assertSandboxFileExists(params, absolutePath),
     } as const,
     params.memoryWriteProvenance,
@@ -1213,6 +1215,7 @@ function createHostEditOperations(
           return await fs.readFile(resolveHostPath(absolutePath));
         },
         writeFile: writeHostFile,
+        statFile: (absolutePath: string) => statHostFile(resolveHostPath(absolutePath)),
         access: async (absolutePath: string) => {
           await fs.access(resolveHostPath(absolutePath));
         },
@@ -1236,6 +1239,10 @@ function createHostEditOperations(
       },
       writeFile: (absolutePath: string, content: string) =>
         writeWorkspaceFile(root, getRoot, absolutePath, content),
+      statFile: async (absolutePath: string) => {
+        const relative = toRelativeWorkspacePath(root, absolutePath);
+        return statHostFile(path.resolve(root, relative));
+      },
       access: async (absolutePath: string) => {
         let relative: string;
         try {

src/agents/sessions/tools/edit.test.ts

@@ -34,6 +34,22 @@ describe("edit tool", () => {
     return filePath;
   }
 
+  async function statEditFile(absolutePath: string) {
+    try {
+      const stat = await fs.stat(absolutePath);
+      return {
+        type: stat.isFile() ? "file" : stat.isDirectory() ? "directory" : "other",
+        size: stat.size,
+        mtimeMs: stat.mtimeMs,
+      } as const;
+    } catch (error) {
+      if ((error as NodeJS.ErrnoException).code === "ENOENT") {
+        return null;
+      }
+      throw error;
+    }
+  }
+
   it("preserves a valid UTF-8 BOM when editing a real file", async () => {
     const filePath = await createTempFile(Buffer.from("\uFEFFheading\nprice: 5\n", "utf-8"));
     const tool = createEditTool(tmpDir);
@@ -107,6 +123,7 @@ describe("edit tool", () => {
     const operations: EditOperations = {
       access: async () => {},
       readFile: async () => Buffer.from(original),
+      statFile: async () => null,
       writeFile,
     };
     const tool = createEditTool("/remote/workspace", { operations });
@@ -167,6 +184,7 @@ describe("edit tool", () => {
         await fs.access(absolutePath);
       },
       readFile: (absolutePath) => fs.readFile(absolutePath),
+      statFile: statEditFile,
       writeFile: async (absolutePath, content) => {
         await fs.writeFile(absolutePath, content, "utf-8");
         throw new Error("Simulated post-write failure");
@@ -202,6 +220,7 @@ describe("edit tool", () => {
         await fs.access(absolutePath);
       },
       readFile: (absolutePath) => fs.readFile(absolutePath),
+      statFile: statEditFile,
       writeFile: async () => {
         throw new Error("Simulated write failure");
       },
@@ -220,6 +239,31 @@ describe("edit tool", () => {
     ).rejects.toThrow("Simulated write failure");
   });
 
+  it("rejects false success when a delegated write resolves without persisting", async () => {
+    const filePath = await createTempFile("old\n");
+    const operations: EditOperations = {
+      access: async (absolutePath) => {
+        await fs.access(absolutePath);
+      },
+      readFile: (absolutePath) => fs.readFile(absolutePath),
+      statFile: statEditFile,
+      writeFile: async () => {},
+    };
+    const tool = createEditTool(tmpDir, { operations });
+
+    await expect(
+      tool.execute(
+        "call-1",
+        {
+          path: filePath,
+          edits: [{ oldText: "old", newText: "new" }],
+        },
+        undefined,
+      ),
+    ).rejects.toThrow("Edit verification failed");
+    await expect(fs.readFile(filePath, "utf-8")).resolves.toBe("old\n");
+  });
+
   it("recovers multi-edit post-write failures", async () => {
     const filePath = await createTempFile("alpha beta gamma delta\n");
     const operations: EditOperations = {
@@ -227,6 +271,7 @@ describe("edit tool", () => {
         await fs.access(absolutePath);
       },
       readFile: (absolutePath) => fs.readFile(absolutePath),
+      statFile: statEditFile,
       writeFile: async (absolutePath, content) => {
         await fs.writeFile(absolutePath, content, "utf-8");
         throw new Error("Simulated post-write failure");
@@ -352,6 +397,7 @@ describe("edit tool", () => {
     const operations: EditOperations = {
       access: async () => {},
       readFile,
+      statFile: async () => null,
       writeFile: async () => {},
     };
     const tool = createEditToolDefinition("/workspace", { operations });
@@ -392,6 +438,7 @@ describe("edit tool", () => {
     const operations: EditOperations = {
       access: async () => {},
       readFile,
+      statFile: async () => null,
       writeFile: async () => {},
     };
     const tool = createEditToolDefinition("/workspace", { operations });
@@ -431,6 +478,7 @@ describe("edit tool", () => {
     const operations: EditOperations = {
       access: async () => {},
       readFile,
+      statFile: async () => null,
       writeFile: async () => {},
     };
     const tool = createEditToolDefinition("/workspace", { operations });
@@ -472,6 +520,7 @@ describe("edit tool", () => {
     const operations: EditOperations = {
       access: async () => {},
       readFile,
+      statFile: async () => null,
       writeFile: async () => {},
     };
     const tool = createEditToolDefinition("/workspace", { operations });
@@ -529,6 +578,7 @@ describe("edit tool", () => {
     const operations: EditOperations = {
       access: async () => {},
       readFile,
+      statFile: async () => null,
       writeFile: async () => {},
     };
     const tool = createEditToolDefinition("/workspace", { operations });
@@ -564,6 +614,7 @@ describe("edit tool", () => {
     const operations: EditOperations = {
       access: async () => {},
       readFile,
+      statFile: async () => null,
       writeFile: async () => {},
     };
     const tool = createEditToolDefinition("/workspace", { operations });
@@ -617,6 +668,7 @@ describe("edit tool", () => {
         await fs.access(absolutePath);
       },
       readFile: (absolutePath) => fs.readFile(absolutePath),
+      statFile: statEditFile,
       writeFile: async () => {
         throw new Error("No changes made to the disk because it is full");
       },

src/agents/sessions/tools/edit.ts

@@ -7,6 +7,7 @@ import { constants } from "node:fs";
 import {
   access as fsAccess,
   readFile as fsReadFile,
+  stat as fsStat,
   writeFile as fsWriteFile,
 } from "node:fs/promises";
 import { Box, Container, Spacer, Text } from "@earendil-works/pi-tui";
@@ -32,6 +33,7 @@ import {
   validateNoOpEditTargets,
 } from "./edit-diff.js";
 import { withFileMutationQueue } from "./file-mutation-queue.js";
+import { type PersistedFileStat, verifyPersistedUtf8File } from "./file-write-verification.js";
 import { resolveToCwd } from "./path-utils.js";
 import { invalidArgText, shortenPath, str } from "./render-utils.js";
 import type { EditToolDetails, EditToolInput } from "./tool-contracts.js";
@@ -98,6 +100,8 @@ export interface EditOperations {
   readFile: (absolutePath: string) => Promise<Buffer>;
   /** Write content to a file */
   writeFile: (absolutePath: string, content: string) => Promise<void>;
+  /** Stat the target before reporting success */
+  statFile: (absolutePath: string) => Promise<PersistedFileStat | null>;
   /** Check if file is readable and writable (throw if not) */
   access: (absolutePath: string) => Promise<void>;
 }
@@ -105,6 +109,26 @@ export interface EditOperations {
 const defaultEditOperations: EditOperations = {
   readFile: (path) => fsReadFile(path),
   writeFile: (path, content) => fsWriteFile(path, content, "utf-8"),
+  statFile: async (path) => {
+    try {
+      const stat = await fsStat(path);
+      return {
+        type: stat.isFile() ? "file" : stat.isDirectory() ? "directory" : "other",
+        size: stat.size,
+        mtimeMs: stat.mtimeMs,
+      } as const;
+    } catch (error) {
+      if (
+        error &&
+        typeof error === "object" &&
+        "code" in error &&
+        (error as { code?: unknown }).code === "ENOENT"
+      ) {
+        return null;
+      }
+      throw error;
+    }
+  },
   access: (path) => fsAccess(path, constants.R_OK | constants.W_OK),
 };
 
@@ -161,38 +185,6 @@ function validateEditInput(input: EditToolInput): {
   return { path: input.path, edits: input.edits };
 }
 
-function removeExactOccurrences(content: string, needle: string): string {
-  return needle.length > 0 ? content.split(needle).join("") : content;
-}
-
-function didEditLikelyApply(params: {
-  originalContent: string;
-  currentContent: string;
-  edits: Edit[];
-}): boolean {
-  if (params.edits.length === 0) {
-    return false;
-  }
-  const normalizedOriginal = normalizeToLF(params.originalContent);
-  const normalizedCurrent = normalizeToLF(params.currentContent);
-  if (normalizedOriginal === normalizedCurrent) {
-    return false;
-  }
-
-  let withoutInsertedNewText = normalizedCurrent;
-  for (const edit of params.edits) {
-    const normalizedNew = normalizeToLF(edit.newText);
-    if (normalizedNew.length > 0 && !normalizedCurrent.includes(normalizedNew)) {
-      return false;
-    }
-    withoutInsertedNewText = removeExactOccurrences(withoutInsertedNewText, normalizedNew);
-  }
-
-  return params.edits.every(
-    (edit) => !withoutInsertedNewText.includes(normalizeToLF(edit.oldText)),
-  );
-}
-
 function appendMismatchHint(error: Error, currentContent: string): Error {
   const snippet =
     currentContent.length <= EDIT_MISMATCH_HINT_LIMIT
@@ -422,6 +414,7 @@ export function createEditToolDefinition(
         }
 
         let realEdits: Edit[] = [];
+        let expectedContent: string | undefined;
 
         try {
           await ops.access(absolutePath);
@@ -462,10 +455,16 @@ export function createEditToolDefinition(
             realEdits,
             path,
           );
-          await ops.writeFile(absolutePath, bom + finalContent);
+          expectedContent = bom + finalContent;
+          await ops.writeFile(absolutePath, expectedContent);
           if (signal?.aborted) {
             throw new Error("Operation aborted");
           }
+          if (!(await verifyPersistedUtf8File(absolutePath, expectedContent, ops))) {
+            throw new Error(
+              `Edit verification failed for ${path}: the persisted regular file does not match the requested content. Inspect the target and retry.`,
+            );
+          }
 
           const diffResult = generateDiffString(baseContent, newContent);
           const patch = generateUnifiedPatch(path, baseContent, newContent);
@@ -492,11 +491,8 @@ export function createEditToolDefinition(
             .then((current) => current.toString("utf-8"))
             .catch(() => rawContent);
           if (
-            didEditLikelyApply({
-              originalContent: rawContent,
-              currentContent,
-              edits: realEdits,
-            })
+            expectedContent !== undefined &&
+            (await verifyPersistedUtf8File(absolutePath, expectedContent, ops))
           ) {
             return {
               content: [

src/agents/sessions/tools/file-write-verification.ts

@@ -0,0 +1,30 @@
+export type PersistedFileStat = {
+  type: "file" | "directory" | "other";
+  size: number;
+  mtimeMs?: number;
+};
+
+type PersistedUtf8FileOperations = {
+  readFile: (absolutePath: string) => Promise<Buffer | string>;
+  statFile: (absolutePath: string) => Promise<PersistedFileStat | null>;
+};
+
+export async function verifyPersistedUtf8File(
+  absolutePath: string,
+  content: string,
+  operations: PersistedUtf8FileOperations,
+): Promise<boolean> {
+  // Success receipts must prove the same regular-file bytes across local and delegated writes.
+  // Compare encoded bytes because UTF-8 writes normalize invalid surrogate code units.
+  const expectedContent = Buffer.from(content, "utf8");
+  const stat = await operations.statFile(absolutePath).catch(() => null);
+  if (!stat || stat.type !== "file" || stat.size !== expectedContent.byteLength) {
+    return false;
+  }
+  const readback = await operations.readFile(absolutePath).catch(() => undefined);
+  if (readback === undefined) {
+    return false;
+  }
+  const persistedContent = Buffer.isBuffer(readback) ? readback : Buffer.from(readback, "utf8");
+  return persistedContent.equals(expectedContent);
+}

src/agents/sessions/tools/write.test.ts

@@ -113,6 +113,58 @@ describe("write tool", () => {
     expect(result.content[0]?.type).toBe("text");
   });
 
+  it("rejects a delegated write that resolves without creating the file", async () => {
+    const filePath = await createTempPath("missing.txt");
+    const tool = createWriteTool(tmpDir, {
+      operations: createRecoverableOperations(async () => {}),
+    });
+
+    await expect(
+      tool.execute("call-1", { path: filePath, content: "expected\n" }, undefined),
+    ).rejects.toThrow("Write verification failed");
+    await expect(fs.stat(filePath)).rejects.toMatchObject({ code: "ENOENT" });
+  });
+
+  it("rejects a delegated write that leaves stale same-size content", async () => {
+    const filePath = await createTempPath("stale.txt");
+    await fs.writeFile(filePath, "stale\n", "utf-8");
+    const tool = createWriteTool(tmpDir, {
+      operations: createRecoverableOperations(async () => {}),
+    });
+
+    await expect(
+      tool.execute("call-1", { path: filePath, content: "fresh\n" }, undefined),
+    ).rejects.toThrow("Write verification failed");
+    await expect(fs.readFile(filePath, "utf-8")).resolves.toBe("stale\n");
+  });
+
+  it("rejects a delegated write that leaves a non-file target", async () => {
+    const filePath = await createTempPath("directory");
+    await fs.mkdir(filePath);
+    const tool = createWriteTool(tmpDir, {
+      operations: createRecoverableOperations(async () => {}),
+    });
+
+    await expect(
+      tool.execute("call-1", { path: filePath, content: "expected\n" }, undefined),
+    ).rejects.toThrow("Write verification failed");
+  });
+
+  it("verifies delegated writes by their persisted UTF-8 bytes", async () => {
+    const filePath = await createTempPath("surrogate.txt");
+    const content = "unpaired \ud800 surrogate\n";
+    const tool = createWriteTool(tmpDir, {
+      operations: createRecoverableOperations((absolutePath, requestedContent) =>
+        fs.writeFile(absolutePath, requestedContent, "utf-8"),
+      ),
+    });
+
+    await expect(
+      tool.execute("call-1", { path: filePath, content }, undefined),
+    ).resolves.toMatchObject({ details: { changed: true, created: true } });
+    await expect(fs.readFile(filePath)).resolves.toEqual(Buffer.from(content, "utf8"));
+  });
+
   it("writes file URL paths through the shared session path resolver", async () => {
     const filePath = await createTempPath("notes.md");
     const tool = createWriteTool(tmpDir);
@@ -271,13 +323,13 @@ describe("write tool", () => {
   it("reports an overwrite without a fabricated diff when the old file is too large", async () => {
     const filePath = await createTempPath("large.txt");
     await fs.writeFile(filePath, "x".repeat(1024 * 1024 + 1), "utf-8");
-    let readCalled = false;
+    let readCount = 0;
     const operations = createRecoverableOperations((absolutePath, content) =>
       fs.writeFile(absolutePath, content, "utf-8"),
     );
-    operations.readFile = async () => {
-      readCalled = true;
-      throw new Error("oversized pre-write read");
+    operations.readFile = async (absolutePath) => {
+      readCount += 1;
+      return fs.readFile(absolutePath);
     };
     const tool = createWriteTool(tmpDir, { operations });
 
@@ -287,7 +339,7 @@ describe("write tool", () => {
       undefined,
     );
 
-    expect(readCalled).toBe(false);
+    expect(readCount).toBe(1);
     expect(result.details).toEqual({ changed: true, created: false });
   });
 
@@ -306,23 +358,20 @@ describe("write tool", () => {
     expect(result.details).toEqual({ changed: true, created: false });
   });
 
-  it("does not guess creation status when the pre-write stat is unavailable", async () => {
+  it("rejects success when the post-write stat is unavailable", async () => {
     await createTempPath("unknown.txt");
     const operations: WriteOperations = {
       mkdir: (dir) => fs.mkdir(dir, { recursive: true }).then(() => {}),
       writeFile: (absolutePath, content) => fs.writeFile(absolutePath, content, "utf-8"),
+      readFile: (absolutePath) => fs.readFile(absolutePath),
       statFile: async () => {
         throw new Error("remote stat unavailable");
       },
     };
     const tool = createWriteTool(tmpDir, { operations });
 
-    const result = await tool.execute(
-      "call-1",
-      { path: "unknown.txt", content: "new\n" },
-      undefined,
-    );
-
-    expect(result.details).toEqual({ changed: true });
+    await expect(
+      tool.execute("call-1", { path: "unknown.txt", content: "new\n" }, undefined),
+    ).rejects.toThrow("Write verification failed");
   });
 });

src/agents/sessions/tools/write.ts

@@ -20,6 +20,7 @@ import { textResult } from "../../tools/common.js";
 import type { ToolDefinition, ToolRenderResultOptions } from "../extensions/types.js";
 import { generateDiffString, generateUnifiedPatch } from "./edit-diff.js";
 import { withFileMutationQueue } from "./file-mutation-queue.js";
+import { type PersistedFileStat, verifyPersistedUtf8File } from "./file-write-verification.js";
 import { resolveToCwd } from "./path-utils.js";
 import {
   invalidArgText,
@@ -74,10 +75,10 @@ export interface WriteOperations {
   writeFile: (absolutePath: string, content: string) => Promise<void>;
   /** Create directory recursively */
   mkdir: (dir: string) => Promise<void>;
-  /** Optional readback used to recover when a write succeeded but the tool aborted before returning */
-  readFile?: (absolutePath: string) => Promise<Buffer | string>;
-  /** Optional stat used to avoid reporting success for files that already matched before execution */
-  statFile?: (absolutePath: string) => Promise<WriteToolFileStat | null>;
+  /** Read persisted content before reporting success */
+  readFile: (absolutePath: string) => Promise<Buffer | string>;
+  /** Stat the target for prechecks and persisted-file verification */
+  statFile: (absolutePath: string) => Promise<PersistedFileStat | null>;
 }
 
 const defaultWriteOperations: WriteOperations = {
@@ -111,15 +112,9 @@ export interface WriteToolOptions {
   operations?: WriteOperations;
 }
 
-type WriteToolFileStat = {
-  type: "file" | "directory" | "other";
-  size: number;
-  mtimeMs?: number;
-};
-
 type WriteToolPrecheck = {
   state: "different" | "same" | "unknown";
-  beforeStat?: WriteToolFileStat | null;
+  beforeStat?: PersistedFileStat | null;
   beforeText?: string;
   readAttempted?: boolean;
 };
@@ -337,7 +332,7 @@ async function readOriginalWriteState(
   if (!ops.statFile) {
     return { state: "unknown" };
   }
-  let stat: WriteToolFileStat | null;
+  let stat: PersistedFileStat | null;
   try {
     stat = await ops.statFile(absolutePath);
   } catch (error) {
@@ -465,7 +460,7 @@ async function resolveWriteDetails(params: {
 
 async function didWriteMetadataChange(
   absolutePath: string,
-  beforeStat: WriteToolFileStat | null | undefined,
+  beforeStat: PersistedFileStat | null | undefined,
   ops: WriteOperations,
 ): Promise<boolean> {
   if (!beforeStat || !ops.statFile) {
@@ -511,16 +506,15 @@ async function recoverSuccessfulWrite(params: {
   details: WriteToolDetails;
   signal?: AbortSignal;
 }) {
-  if (!params.ops.readFile || !isWriteRecoveryCandidate(params.error, params.signal)) {
+  if (!isWriteRecoveryCandidate(params.error, params.signal)) {
     return null;
   }
-  const readback = await params.ops.readFile(params.absolutePath).catch(() => undefined);
-  const currentContent = Buffer.isBuffer(readback) ? readback.toString("utf8") : readback;
+  const verified = await verifyPersistedUtf8File(params.absolutePath, params.content, params.ops);
   const changed =
     params.precheck.state === "different" ||
     (params.precheck.state === "unknown" &&
       (await didWriteMetadataChange(params.absolutePath, params.precheck.beforeStat, params.ops)));
-  if (currentContent !== params.content || !changed) {
+  if (!verified || !changed) {
     return null;
   }
   return successfulWriteResult(params.path, params.content, params.details);
@@ -575,6 +569,11 @@ export function createWriteToolDefinition(
           if (signal?.aborted) {
             throw new Error("Operation aborted");
           }
+          if (!(await verifyPersistedUtf8File(absolutePath, content, ops))) {
+            throw new Error(
+              `Write verification failed for ${path}: the persisted regular file does not match the requested content. Inspect the target and retry.`,
+            );
+          }
           return successfulWriteResult(path, content, details);
         } catch (error: unknown) {
           const recovered = await recoverSuccessfulWrite({