diff --git a/docs/tools/apply-patch.md b/docs/tools/apply-patch.md index 0ecab50067dd..f790c1e70681 100644 --- a/docs/tools/apply-patch.md +++ b/docs/tools/apply-patch.md @@ -32,6 +32,7 @@ The tool accepts a single `input` string that wraps one or more file operations: - Patch paths support relative paths (from the workspace directory) and absolute paths. - `tools.exec.applyPatch.workspaceOnly` defaults to `true` (workspace-contained). Set it to `false` only if you intentionally want `apply_patch` to write/delete outside the workspace directory. +- `*** Add File:` and a non-self `*** Move to:` require the destination path to be absent. To intentionally replace a path, delete it earlier in the same patch before adding or moving the replacement. - Use `*** Move to:` within an `*** Update File:` hunk to rename files. - `*** End of File` marks an EOF-only insert when needed. - Enabled by default for every model. Set `tools.exec.applyPatch.enabled: false` diff --git a/extensions/mxc/src/fs-bridge.ts b/extensions/mxc/src/fs-bridge.ts index eed8bfa83c23..68c5673d6567 100644 --- a/extensions/mxc/src/fs-bridge.ts +++ b/extensions/mxc/src/fs-bridge.ts @@ -7,7 +7,7 @@ import { type SandboxFsStat, type SandboxResolvedPath, } from "openclaw/plugin-sdk/sandbox"; -import { isPathInside } from "openclaw/plugin-sdk/security-runtime"; +import { FsSafeError, isPathInside } from "openclaw/plugin-sdk/security-runtime"; import { resolveMxcReadOnlySkillMounts, type MxcReadOnlySkillMount, @@ -86,6 +86,33 @@ class MxcFsBridge implements SandboxFsBridge { }); } + async createFileExclusive(params: { + filePath: string; + cwd?: string; + data: Buffer | string; + encoding?: BufferEncoding; + mkdir?: boolean; + }): Promise<"created" | "exists"> { + const target = this.resolveTarget(params); + this.ensureWritable(target, "create files"); + const buffer = Buffer.isBuffer(params.data) + ? params.data + : Buffer.from(params.data, params.encoding ?? "utf8"); + try { + await ( + await fsRoot(target.mount.hostRoot) + ).create(target.mountRelativePath, buffer, { + mkdir: params.mkdir !== false, + }); + return "created"; + } catch (error) { + if (error instanceof FsSafeError && error.code === "already-exists") { + return "exists"; + } + throw error; + } + } + async mkdirp(params: { filePath: string; cwd?: string }): Promise { const target = this.resolveTarget(params); this.ensureWritable(target, "create directories"); diff --git a/extensions/mxc/test/mxc-backend.test.ts b/extensions/mxc/test/mxc-backend.test.ts index 7dacdb3b6549..3c7f63898b88 100644 --- a/extensions/mxc/test/mxc-backend.test.ts +++ b/extensions/mxc/test/mxc-backend.test.ts @@ -737,6 +737,25 @@ describeOnWindows("createMxcSandboxBackendHandle (Windows-only MXC backend tests }); expect(bridge).toBeDefined(); + const createFileExclusive = bridge?.createFileExclusive?.bind(bridge); + expect(createFileExclusive).toBeTypeOf("function"); + await expect( + createFileExclusive!({ filePath: "notes/exclusive.txt", data: "first", cwd: workdir }), + ).resolves.toBe("created"); + await expect( + createFileExclusive!({ + filePath: "notes/exclusive.txt", + data: "replacement", + cwd: workdir, + }), + ).resolves.toBe("exists"); + expect(readFileSync(path.join(workdir, "notes", "exclusive.txt"), "utf-8")).toBe("first"); + const raceOutcomes = await Promise.all([ + createFileExclusive!({ filePath: "notes/race.txt", data: "one", cwd: workdir }), + createFileExclusive!({ filePath: "notes/race.txt", data: "two", cwd: workdir }), + ]); + expect(raceOutcomes.toSorted()).toEqual(["created", "exists"]); + await bridge?.writeFile({ filePath: "notes/one.txt", data: "hello mxc", cwd: workdir }); expect(await bridge?.readFile({ filePath: "notes/one.txt", cwd: workdir })).toEqual( Buffer.from("hello mxc"), diff --git a/extensions/openshell/src/fs-bridge.ts b/extensions/openshell/src/fs-bridge.ts index 4764cc584a11..405dc3e47d56 100644 --- a/extensions/openshell/src/fs-bridge.ts +++ b/extensions/openshell/src/fs-bridge.ts @@ -117,6 +117,43 @@ class OpenShellFsBridge implements SandboxFsBridge { await this.backend.syncLocalPathToRemote(hostPath, target.containerPath); } + async createFileExclusive(params: { + filePath: string; + cwd?: string; + data: Buffer | string; + encoding?: BufferEncoding; + mkdir?: boolean; + signal?: AbortSignal; + }): Promise<"created" | "exists"> { + const target = this.resolveTarget(params); + const hostPath = this.requireHostPath(target); + this.ensureWritable(target, "create files"); + await assertLocalPathSafety({ + target, + root: target.mountHostRoot, + allowMissingLeaf: true, + allowFinalSymlinkForUnlink: false, + }); + const buffer = Buffer.isBuffer(params.data) + ? params.data + : Buffer.from(params.data, params.encoding ?? "utf8"); + const root = await fsRoot(target.mountHostRoot); + try { + await root.create(path.relative(target.mountHostRoot, hostPath), buffer, { + mkdir: params.mkdir !== false, + }); + } catch (error) { + if (error instanceof FsSafeError && error.code === "already-exists") { + return "exists"; + } + throw error; + } + // Mirror mode treats local state as canonical. Syncing may fail, but must + // never downgrade the exclusive local create to an overwriting write. + await this.backend.syncLocalPathToRemote(hostPath, target.containerPath); + return "created"; + } + async mkdirp(params: { filePath: string; cwd?: string; signal?: AbortSignal }): Promise { const target = this.resolveTarget(params); const hostPath = this.requireHostPath(target); diff --git a/extensions/openshell/src/openshell-core.test.ts b/extensions/openshell/src/openshell-core.test.ts index 0f5ec291c84e..970e2a07c81e 100644 --- a/extensions/openshell/src/openshell-core.test.ts +++ b/extensions/openshell/src/openshell-core.test.ts @@ -945,6 +945,61 @@ describe("openshell fs bridges", () => { ); }); + it("creates mirror files exclusively before syncing them", async () => { + const workspaceDir = await makeTempDir("openclaw-openshell-fs-"); + const backend = createMirrorBackendMock(); + const sandbox = createSandboxTestContext({ + overrides: { + backendId: "openshell", + workspaceDir, + agentWorkspaceDir: workspaceDir, + containerWorkdir: "/sandbox", + }, + }); + + const { createOpenShellFsBridge } = await import("./fs-bridge.js"); + const bridge = createOpenShellFsBridge({ sandbox, backend }); + const createFileExclusive = bridge.createFileExclusive?.bind(bridge); + expect(createFileExclusive).toBeTypeOf("function"); + + await expect( + createFileExclusive!({ filePath: "nested/file.txt", data: "first" }), + ).resolves.toBe("created"); + await expect( + createFileExclusive!({ filePath: "nested/file.txt", data: "replacement" }), + ).resolves.toBe("exists"); + await expect(fs.readFile(path.join(workspaceDir, "nested", "file.txt"), "utf8")).resolves.toBe( + "first", + ); + expect(backend["syncLocalPathToRemote"]).toHaveBeenCalledTimes(1); + }); + + it("keeps the canonical local exclusive create when mirror sync fails", async () => { + const workspaceDir = await makeTempDir("openclaw-openshell-fs-"); + const backend = createMirrorBackendMock(); + backend["syncLocalPathToRemote"] = vi.fn().mockRejectedValue(new Error("remote rejected")); + const sandbox = createSandboxTestContext({ + overrides: { + backendId: "openshell", + workspaceDir, + agentWorkspaceDir: workspaceDir, + containerWorkdir: "/sandbox", + }, + }); + + const { createOpenShellFsBridge } = await import("./fs-bridge.js"); + const bridge = createOpenShellFsBridge({ sandbox, backend }); + const createFileExclusive = bridge.createFileExclusive?.bind(bridge); + expect(createFileExclusive).toBeTypeOf("function"); + + await expect(createFileExclusive!({ filePath: "file.txt", data: "canonical" })).rejects.toThrow( + "remote rejected", + ); + await expect(fs.readFile(path.join(workspaceDir, "file.txt"), "utf8")).resolves.toBe( + "canonical", + ); + }); + it("creates remote mirror directories through the pinned backend operation", async () => { const workspaceDir = await makeTempDir("openclaw-openshell-fs-"); const backend = createMirrorBackendMock(); diff --git a/src/agents/agent-tools-agent-config.test.ts b/src/agents/agent-tools-agent-config.test.ts index cd905a827f06..0d689eedbae5 100644 --- a/src/agents/agent-tools-agent-config.test.ts +++ b/src/agents/agent-tools-agent-config.test.ts @@ -37,6 +37,7 @@ describe("Agent-specific tool filtering", () => { }), readFile: async () => Buffer.from(""), writeFile: async () => {}, + createFileExclusive: async () => "created", mkdirp: async () => {}, remove: async () => {}, rename: async () => {}, diff --git a/src/agents/agent-tools.create-openclaw-coding-tools.test.ts b/src/agents/agent-tools.create-openclaw-coding-tools.test.ts index fd136f871593..c1a000bf7b92 100644 --- a/src/agents/agent-tools.create-openclaw-coding-tools.test.ts +++ b/src/agents/agent-tools.create-openclaw-coding-tools.test.ts @@ -2022,8 +2022,9 @@ describe("createOpenClawCodingTools", () => { it("records ordinary write, edit, and apply_patch memory provenance from turn taint", async () => { const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-memory-write-taint-")); + const rollback = vi.fn(async () => {}); const recordWriteProvenance = vi.fn>( - async () => {}, + async () => rollback, ); registerMemoryCapability("memory-core", { flushPlanResolver: () => ({ @@ -2078,6 +2079,20 @@ describe("createOpenClawCodingTools", () => { contentAfter: "network project note\n", }), ); + await expect( + applyPatch("patch-existing-memory", { + input: [ + "*** Begin Patch", + "*** Add File: memory/project.md", + "+replacement", + "*** End Patch", + ].join("\n"), + }), + ).rejects.toThrow(/file already exists/i); + expect(rollback).toHaveBeenCalledOnce(); + await expect(fs.readFile(path.join(workspaceDir, "memory/project.md"), "utf8")).resolves.toBe( + "network project note\n", + ); } finally { await fs.rm(workspaceDir, { recursive: true, force: true }); } diff --git a/src/agents/apply-patch-file-ops.ts b/src/agents/apply-patch-file-ops.ts new file mode 100644 index 000000000000..33dbdd453cec --- /dev/null +++ b/src/agents/apply-patch-file-ops.ts @@ -0,0 +1,205 @@ +import syncFs from "node:fs"; +import fs from "node:fs/promises"; +import { openRootFile, type RootFileOpenResult } from "../infra/boundary-file-read.js"; +import { FsSafeError, root as fsRoot } from "../infra/fs-safe.js"; +import { + type MemoryWriteProvenanceObserver, + withMemoryWriteProvenance, +} from "./memory-write-provenance.js"; +import { toRelativeSandboxPath } from "./path-policy.js"; +import type { SandboxFsBridge } from "./sandbox/fs-bridge.js"; +import { decodeUtf8File } from "./utf8-file.js"; + +export type SandboxApplyPatchConfig = { + root: string; + bridge: SandboxFsBridge; +}; + +export type ApplyPatchFileOptions = { + cwd: string; + sandbox?: SandboxApplyPatchConfig; + /** Restrict patch paths to the workspace root (cwd). Default: true. Set false to opt out. */ + workspaceOnly?: boolean; + memoryWriteProvenance?: MemoryWriteProvenanceObserver; +}; + +type PatchCreateOutcome = "created" | "exists"; + +export type PatchFileOps = { + readFile: (filePath: string) => Promise; + writeFile: (filePath: string, content: string) => Promise; + createFileExclusive: (filePath: string, content: string) => Promise; + remove: (filePath: string) => Promise; + mkdirp: (dir: string) => Promise; +}; + +export async function createPatchTarget(params: { + target: { resolved: string; display: string }; + contents: string; + ops: PatchFileOps; + hint: string; +}) { + const outcome = await params.ops.createFileExclusive(params.target.resolved, params.contents); + if (outcome === "exists") { + throw new Error( + `Cannot create ${params.target.display}: the file already exists. ${params.hint}`, + ); + } +} + +export function resolvePatchFileOps(options: ApplyPatchFileOptions): PatchFileOps { + if (options.sandbox) { + const { root, bridge } = options.sandbox; + return withPatchMemoryWriteProvenance({ + observer: options.memoryWriteProvenance, + operations: { + readFile: async (filePath) => { + const buf = await bridge.readFile({ filePath, cwd: root }); + return decodeUtf8File(buf, filePath); + }, + writeFile: (filePath, content) => bridge.writeFile({ filePath, cwd: root, data: content }), + createFileExclusive: (filePath, content) => { + if (!bridge.createFileExclusive) { + throw new Error( + "Sandbox filesystem bridge does not support atomic file creation; refusing to overwrite an existing path.", + ); + } + return bridge.createFileExclusive({ filePath, cwd: root, data: content }); + }, + remove: (filePath) => bridge.remove({ filePath, cwd: root, force: false }), + mkdirp: (dir) => bridge.mkdirp({ filePath: dir, cwd: root }), + }, + }); + } + + if (options.workspaceOnly === false) { + return withPatchMemoryWriteProvenance({ + observer: options.memoryWriteProvenance, + operations: { + readFile: async (filePath) => decodeUtf8File(await fs.readFile(filePath), filePath), + writeFile: async (filePath, content) => { + await fs.writeFile(filePath, content, "utf8"); + }, + createFileExclusive: async (filePath, content) => { + try { + await fs.writeFile(filePath, content, { encoding: "utf8", flag: "wx" }); + return "created"; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "EEXIST") { + return "exists"; + } + throw error; + } + }, + remove: (filePath) => fs.rm(filePath), + mkdirp: async (dir) => { + await fs.mkdir(dir, { recursive: true }); + }, + }, + }); + } + + const rootPromise = fsRoot(options.cwd); + return withPatchMemoryWriteProvenance({ + observer: options.memoryWriteProvenance, + operations: { + readFile: async (filePath) => { + const opened = await openRootFile({ + absolutePath: filePath, + rootPath: options.cwd, + boundaryLabel: "workspace root", + }); + assertBoundaryRead(opened, filePath); + try { + return decodeUtf8File(syncFs.readFileSync(opened.fd), filePath); + } finally { + syncFs.closeSync(opened.fd); + } + }, + writeFile: async (filePath, content) => { + const relative = toRelativeSandboxPath(options.cwd, filePath); + await (await rootPromise).write(relative, content, { encoding: "utf8" }); + }, + createFileExclusive: async (filePath, content) => { + const relative = toRelativeSandboxPath(options.cwd, filePath); + try { + await (await rootPromise).create(relative, content, { encoding: "utf8" }); + return "created"; + } catch (error) { + // fs-safe opens an existing destination before its O_EXCL commit. A final + // symlink is rejected during that probe, but for create semantics it is + // still an occupied destination and must fail closed. + if ( + error instanceof FsSafeError && + (error.code === "already-exists" || error.code === "symlink") + ) { + return "exists"; + } + throw error; + } + }, + remove: async (filePath) => { + const relative = toRelativeSandboxPath(options.cwd, filePath); + await (await rootPromise).remove(relative); + }, + mkdirp: async (dir) => { + const relative = toRelativeSandboxPath(options.cwd, dir, { allowRoot: true }); + const root = await rootPromise; + if (relative === "" || relative === ".") { + await root.ensureRoot(); + return; + } + await root.mkdir(relative); + }, + }, + }); +} + +class PatchCreateExistsSignal extends Error {} + +function withPatchMemoryWriteProvenance(params: { + operations: PatchFileOps; + observer: MemoryWriteProvenanceObserver | undefined; +}): PatchFileOps { + const operations = withMemoryWriteProvenance(params.operations, params.observer); + if (!params.observer) { + return operations; + } + return { + ...operations, + createFileExclusive: async (filePath, content) => { + if (!params.observer?.classifies(filePath)) { + return params.operations.createFileExclusive(filePath, content); + } + try { + await params.observer.write({ + absolutePath: filePath, + contentBefore: "", + contentAfter: content, + commit: async () => { + if ((await params.operations.createFileExclusive(filePath, content)) === "exists") { + throw new PatchCreateExistsSignal(); + } + }, + }); + return "created"; + } catch (error) { + if (error instanceof PatchCreateExistsSignal) { + return "exists"; + } + throw error; + } + }, + }; +} + +function assertBoundaryRead( + opened: RootFileOpenResult, + targetPath: string, +): asserts opened is Extract { + if (opened.ok) { + return; + } + const reason = opened.reason === "validation" ? "unsafe path" : "path not found"; + throw new Error(`Failed boundary read for ${targetPath} (${reason})`); +} diff --git a/src/agents/apply-patch.test.ts b/src/agents/apply-patch.test.ts index 4ac8d6a1623e..36c31cbdeafc 100644 --- a/src/agents/apply-patch.test.ts +++ b/src/agents/apply-patch.test.ts @@ -40,13 +40,24 @@ function buildAddFilePatch(targetPath: string): string { *** End Patch`; } -function createMemoryPatchSandbox(initialFiles: Record = {}) { +function createMemoryPatchSandbox( + initialFiles: Record = {}, + options: { supportsExclusiveCreate?: boolean } = {}, +) { const files = new Map( Object.entries(initialFiles).map(([filePath, contents]) => [`/sandbox/${filePath}`, contents]), ); const writeFile = vi.fn(async ({ filePath, data }) => { files.set(filePath, Buffer.isBuffer(data) ? Buffer.from(data) : data); }); + const createFileExclusive = vi.fn(async ({ filePath, data }) => { + if (files.has(filePath)) { + return "exists" as const; + } + files.set(filePath, Buffer.isBuffer(data) ? Buffer.from(data) : data); + return "created" as const; + }); + const mkdirp = vi.fn(async () => {}); const bridge: SandboxFsBridge = { resolvePath: ({ filePath }) => ({ relativePath: filePath, @@ -59,6 +70,7 @@ function createMemoryPatchSandbox(initialFiles: Record : Buffer.from(contents ?? ""); }, writeFile, + ...(options.supportsExclusiveCreate === false ? {} : { createFileExclusive }), remove: async ({ filePath }) => { files.delete(filePath); }, @@ -75,12 +87,14 @@ function createMemoryPatchSandbox(initialFiles: Record ? null : { type: "file", size: Buffer.byteLength(contents), mtimeMs: 0 }; }, - mkdirp: async () => {}, + mkdirp, }; return { files, bridge, writeFile, + createFileExclusive, + mkdirp, options: { cwd: "/local/workspace", sandbox: { @@ -187,6 +201,198 @@ describe("applyPatch", () => { expect(result.summary.added).toEqual(["hello.txt"]); }); + it("rejects an add hunk that targets an existing file", async () => { + const memory = createMemoryPatchSandbox({ "notes.txt": "keep me\n" }); + const patch = `*** Begin Patch +*** Add File: notes.txt ++replacement +*** End Patch`; + + await expect(applyPatch(patch, memory.options)).rejects.toThrow( + /Cannot create notes\.txt: the file already exists/, + ); + expect(memory.files.get("/sandbox/notes.txt")).toBe("keep me\n"); + expect(memory.writeFile.mock.calls).toHaveLength(0); + }); + + it.each([ + { name: "workspace-confined host", workspaceOnly: true }, + { name: "unconfined host", workspaceOnly: false }, + ])( + "keeps existing contents in $name when an add hunk targets them", + async ({ workspaceOnly }) => { + await withWorkspaceTempDir(async (dir) => { + const target = path.join(dir, "notes.txt"); + await fs.writeFile(target, "IMPORTANT USER DATA\nsecond line\n", "utf8"); + const tool = createApplyPatchTool({ cwd: dir, workspaceOnly }); + const patch = `*** Begin Patch +*** Add File: notes.txt ++replacement +*** End Patch`; + + await expect( + tool.execute("call-add-existing", { input: patch }, undefined), + ).rejects.toThrow(/Cannot create notes\.txt: the file already exists/); + expect(await fs.readFile(target, "utf8")).toBe("IMPORTANT USER DATA\nsecond line\n"); + }); + }, + ); + + it.runIf(process.platform !== "win32")( + "refuses existing symlinks in both host modes without changing their targets", + async () => { + for (const workspaceOnly of [true, false]) { + await withWorkspaceTempDir(async (dir) => { + const target = path.join(dir, "target.txt"); + const link = path.join(dir, "notes.txt"); + await fs.writeFile(target, "keep me\n", "utf8"); + await fs.symlink("target.txt", link); + const patch = `*** Begin Patch +*** Add File: notes.txt ++replacement +*** End Patch`; + + await expect(applyPatch(patch, { cwd: dir, workspaceOnly })).rejects.toThrow( + /Cannot create notes\.txt: the file already exists/, + ); + await expect(fs.readFile(target, "utf8")).resolves.toBe("keep me\n"); + await expect(fs.readlink(link)).resolves.toBe("target.txt"); + }); + } + }, + ); + + it("refuses an add hunk when a competing writer creates the target mid-patch", async () => { + const memory = createMemoryPatchSandbox(); + memory.mkdirp.mockImplementation(async () => { + memory.files.set("/sandbox/notes.txt", "written by another writer\n"); + }); + const patch = `*** Begin Patch +*** Add File: notes.txt ++replacement +*** End Patch`; + + await expect(applyPatch(patch, memory.options)).rejects.toThrow( + /Cannot create notes\.txt: the file already exists/, + ); + expect(memory.files.get("/sandbox/notes.txt")).toBe("written by another writer\n"); + expect(memory.writeFile.mock.calls).toHaveLength(0); + }); + + it("refuses a move hunk when a competing writer creates the destination mid-patch", async () => { + const memory = createMemoryPatchSandbox({ "source.txt": "foo\nbar\n" }); + memory.mkdirp.mockImplementation(async () => { + memory.files.set("/sandbox/dest.txt", "written by another writer\n"); + }); + const patch = `*** Begin Patch +*** Update File: source.txt +*** Move to: dest.txt +@@ + foo +-bar ++baz +*** End Patch`; + + await expect(applyPatch(patch, memory.options)).rejects.toThrow( + /Cannot create dest\.txt: the file already exists/, + ); + expect(memory.files.get("/sandbox/dest.txt")).toBe("written by another writer\n"); + expect(memory.files.get("/sandbox/source.txt")).toBe("foo\nbar\n"); + }); + + it("allows an add hunk after the same path is deleted in the patch", async () => { + const memory = createMemoryPatchSandbox({ "notes.txt": "old\n" }); + const patch = `*** Begin Patch +*** Delete File: notes.txt +*** Add File: notes.txt ++new +*** End Patch`; + + const result = await applyPatch(patch, memory.options); + + expect(memory.files.get("/sandbox/notes.txt")).toBe("new\n"); + expect(result.summary.added).toEqual(["notes.txt"]); + }); + + it("rejects a move hunk that targets an existing file", async () => { + const memory = createMemoryPatchSandbox({ + "source.txt": "foo\nbar\n", + "dest.txt": "keep me\n", + }); + const patch = `*** Begin Patch +*** Update File: source.txt +*** Move to: dest.txt +@@ + foo +-bar ++baz +*** End Patch`; + + await expect(applyPatch(patch, memory.options)).rejects.toThrow( + /Cannot create dest\.txt: the file already exists/, + ); + expect(memory.files.get("/sandbox/dest.txt")).toBe("keep me\n"); + expect(memory.files.get("/sandbox/source.txt")).toBe("foo\nbar\n"); + }); + + it.each([ + { name: "workspace-confined host", workspaceOnly: true }, + { name: "unconfined host", workspaceOnly: false }, + ])( + "preserves source and destination when a move target exists in $name", + async ({ workspaceOnly }) => { + await withWorkspaceTempDir(async (dir) => { + await fs.writeFile(path.join(dir, "source.txt"), "foo\nbar\n", "utf8"); + await fs.writeFile(path.join(dir, "dest.txt"), "keep me\n", "utf8"); + const patch = `*** Begin Patch +*** Update File: source.txt +*** Move to: dest.txt +@@ + foo +-bar ++baz +*** End Patch`; + + await expect(applyPatch(patch, { cwd: dir, workspaceOnly })).rejects.toThrow( + /Cannot create dest\.txt: the file already exists/, + ); + await expect(fs.readFile(path.join(dir, "source.txt"), "utf8")).resolves.toBe("foo\nbar\n"); + await expect(fs.readFile(path.join(dir, "dest.txt"), "utf8")).resolves.toBe("keep me\n"); + }); + }, + ); + + it("fails closed on sandbox adds when atomic create is unavailable", async () => { + const memory = createMemoryPatchSandbox({}, { supportsExclusiveCreate: false }); + const patch = `*** Begin Patch +*** Add File: notes.txt ++new +*** End Patch`; + + await expect(applyPatch(patch, memory.options)).rejects.toThrow( + /does not support atomic file creation/, + ); + expect(memory.files.has("/sandbox/notes.txt")).toBe(false); + }); + + it("still permits sandbox updates when atomic create is unavailable", async () => { + const memory = createMemoryPatchSandbox( + { "source.txt": "before\n" }, + { supportsExclusiveCreate: false }, + ); + const patch = `*** Begin Patch +*** Update File: source.txt +@@ +-before ++after +*** End Patch`; + + await expect(applyPatch(patch, memory.options)).resolves.toMatchObject({ + summary: { modified: ["source.txt"] }, + }); + expect(memory.files.get("/sandbox/source.txt")).toBe("after\n"); + }); + it("updates and moves a file", async () => { const memory = createMemoryPatchSandbox({ "source.txt": "foo\nbar\n", diff --git a/src/agents/apply-patch.ts b/src/agents/apply-patch.ts index a10e9b3b261e..f76d345e11f9 100644 --- a/src/agents/apply-patch.ts +++ b/src/agents/apply-patch.ts @@ -3,28 +3,27 @@ * Parses OpenAI-style patch envelopes and applies add/update/delete/move hunks * through guarded host or sandbox filesystem operations. */ -import syncFs from "node:fs"; import fs from "node:fs/promises"; import path from "node:path"; import { Type } from "typebox"; import { createAbortError } from "../infra/abort-signal.js"; -import { openRootFile, type RootFileOpenResult } from "../infra/boundary-file-read.js"; -import { root as fsRoot } from "../infra/fs-safe.js"; import { PATH_ALIAS_POLICIES, type PathAliasPolicy } from "../infra/path-alias-guards.js"; -import { applyUpdateHunk } from "./apply-patch-update.js"; import { - type MemoryWriteProvenanceObserver, - withMemoryWriteProvenance, -} from "./memory-write-provenance.js"; -import { toRelativeSandboxPath, resolvePathFromInput } from "./path-policy.js"; + type ApplyPatchFileOptions, + createPatchTarget, + type PatchFileOps, + resolvePatchFileOps, + type SandboxApplyPatchConfig, +} from "./apply-patch-file-ops.js"; +import { applyUpdateHunk } from "./apply-patch-update.js"; +import type { MemoryWriteProvenanceObserver } from "./memory-write-provenance.js"; +import { resolvePathFromInput } from "./path-policy.js"; import type { AgentTool } from "./runtime/index.js"; import { assertSandboxPath } from "./sandbox-paths.js"; -import type { SandboxFsBridge } from "./sandbox/fs-bridge.js"; import { withFileMutationQueue, withFileMutationQueues, } from "./sessions/tools/file-mutation-queue.js"; -import { decodeUtf8File } from "./utf8-file.js"; const BEGIN_PATCH_MARKER = "*** Begin Patch"; const END_PATCH_MARKER = "*** End Patch"; @@ -87,17 +86,7 @@ function normalizeUpdateComparison(content: string): string { return `${normalized}\n`; } -type SandboxApplyPatchConfig = { - root: string; - bridge: SandboxFsBridge; -}; - -type ApplyPatchOptions = { - cwd: string; - sandbox?: SandboxApplyPatchConfig; - /** Restrict patch paths to the workspace root (cwd). Default: true. Set false to opt out. */ - workspaceOnly?: boolean; - memoryWriteProvenance?: MemoryWriteProvenanceObserver; +type ApplyPatchOptions = ApplyPatchFileOptions & { signal?: AbortSignal; }; @@ -197,7 +186,12 @@ async function applyPatch(input: string, options: ApplyPatchOptions): Promise { await assertPatchParentPath(hunk.path, options); await ensureDir(target.resolved, fileOps); - await fileOps.writeFile(target.resolved, hunk.contents); + await createPatchTarget({ + target, + contents: hunk.contents, + ops: fileOps, + hint: `Use "*** Update File: ${target.display}" to change it, or delete it earlier in the same patch.`, + }); }); recordSummary(summary, seen, "added", target.display); continue; @@ -224,20 +218,22 @@ async function applyPatch(input: string, options: ApplyPatchOptions): Promise Promise; - writeFile: (filePath: string, content: string) => Promise; - remove: (filePath: string) => Promise; - mkdirp: (dir: string) => Promise; -}; - -function resolvePatchFileOps(options: ApplyPatchOptions): PatchFileOps { - let operations: PatchFileOps; - if (options.sandbox) { - const { root, bridge } = options.sandbox; - operations = { - readFile: async (filePath) => { - const buf = await bridge.readFile({ filePath, cwd: root }); - return decodeUtf8File(buf, filePath); - }, - writeFile: (filePath, content) => bridge.writeFile({ filePath, cwd: root, data: content }), - remove: (filePath) => bridge.remove({ filePath, cwd: root, force: false }), - mkdirp: (dir) => bridge.mkdirp({ filePath: dir, cwd: root }), - }; - } else { - const workspaceOnly = options.workspaceOnly !== false; - const rootPromise = workspaceOnly ? fsRoot(options.cwd) : undefined; - operations = { - readFile: async (filePath) => { - if (!workspaceOnly) { - return decodeUtf8File(await fs.readFile(filePath), filePath); - } - const opened = await openRootFile({ - absolutePath: filePath, - rootPath: options.cwd, - boundaryLabel: "workspace root", - }); - assertBoundaryRead(opened, filePath); - try { - return decodeUtf8File(syncFs.readFileSync(opened.fd), filePath); - } finally { - syncFs.closeSync(opened.fd); - } - }, - writeFile: async (filePath, content) => { - if (!workspaceOnly) { - await fs.writeFile(filePath, content, "utf8"); - return; - } - const relative = toRelativeSandboxPath(options.cwd, filePath); - await (await rootPromise)?.write(relative, content, { encoding: "utf8" }); - }, - remove: async (filePath) => { - if (!workspaceOnly) { - await fs.rm(filePath); - return; - } - const relative = toRelativeSandboxPath(options.cwd, filePath); - await (await rootPromise)?.remove(relative); - }, - mkdirp: async (dir) => { - if (!workspaceOnly) { - await fs.mkdir(dir, { recursive: true }); - return; - } - const relative = toRelativeSandboxPath(options.cwd, dir, { allowRoot: true }); - const root = await rootPromise; - if (!root) { - return; - } - if (relative === "" || relative === ".") { - await root.ensureRoot(); - return; - } - await root.mkdir(relative); - }, - }; - } - return withMemoryWriteProvenance(operations, options.memoryWriteProvenance); -} - async function ensureDir(filePath: string, ops: PatchFileOps) { const parent = path.dirname(filePath); if (!parent || parent === ".") { @@ -477,17 +396,6 @@ async function resolvePatchPath( }; } -function assertBoundaryRead( - opened: RootFileOpenResult, - targetPath: string, -): asserts opened is Extract { - if (opened.ok) { - return; - } - const reason = opened.reason === "validation" ? "unsafe path" : "path not found"; - throw new Error(`Failed boundary read for ${targetPath} (${reason})`); -} - function toDisplayPath(resolved: string, cwd: string): string { const relative = path.relative(cwd, resolved); if (!relative || relative === "") { diff --git a/src/agents/sandbox/fs-bridge-mutation-helper.test.ts b/src/agents/sandbox/fs-bridge-mutation-helper.test.ts index 2c9c4e058e75..128f0b6aa5f7 100644 --- a/src/agents/sandbox/fs-bridge-mutation-helper.test.ts +++ b/src/agents/sandbox/fs-bridge-mutation-helper.test.ts @@ -7,6 +7,7 @@ import { describe, expect, it } from "vitest"; import { withTempDir } from "../../test-helpers/temp-dir.js"; import { buildPinnedWritePlan, + SANDBOX_CREATE_EXISTS_EXIT_CODE, SANDBOX_PINNED_MUTATION_PYTHON, } from "./fs-bridge-mutation-helper.js"; @@ -76,6 +77,63 @@ const FORCED_COPY_FAILURE_MUTATION_PYTHON = SANDBOX_PINNED_MUTATION_PYTHON.repla " raise OSError(errno.ENOSPC, 'forced copy failure')\n copy_completed = True", ); +const FORCED_CREATE_FAILURE_MUTATION_PYTHON = SANDBOX_PINNED_MUTATION_PYTHON.replace( + " # exclusive create payload is durable before publication", + " raise OSError(errno.ENOSPC, 'forced create failure')\n # exclusive create payload is durable before publication", +); + +const FORCED_CREATE_FAILURE_WITH_REPLACEMENT_MUTATION_PYTHON = + SANDBOX_PINNED_MUTATION_PYTHON.replace( + " # Publish with a native atomic no-replace rename.", + [ + " replacement_fd = os.open(basename, WRITE_FLAGS, 0o600, dir_fd=parent_fd)", + " try:", + " os.write(replacement_fd, b'replacement')", + " finally:", + " os.close(replacement_fd)", + " # Publish with a native atomic no-replace rename.", + ].join("\n"), + ); + +const FORCED_CREATE_TEMP_SUBSTITUTION_MUTATION_PYTHON = SANDBOX_PINNED_MUTATION_PYTHON.replace( + " # Publish with a native atomic no-replace rename.", + [ + " os.unlink(temp_name, dir_fd=staging_fd)", + " replacement_fd = os.open(temp_name, WRITE_FLAGS, 0o600, dir_fd=staging_fd)", + " try:", + " os.write(replacement_fd, b'replacement')", + " finally:", + " os.close(replacement_fd)", + " # Publish with a native atomic no-replace rename.", + ].join("\n"), +); + +const FORCED_MISSING_RENAMEAT2_MUTATION_PYTHON = SANDBOX_PINNED_MUTATION_PYTHON.replace( + " is_linux = sys.platform.startswith('linux')", + " is_linux = True", +).replace(" rename_fn = getattr(libc, 'renameat2', None)", " rename_fn = None"); + +const FORCED_UNSUPPORTED_RENAMEAT2_MUTATION_PYTHON = SANDBOX_PINNED_MUTATION_PYTHON.replace( + " is_linux = sys.platform.startswith('linux')", + " is_linux = True", +).replace( + " rename_fn = getattr(libc, 'renameat2', None)", + [ + " class UnsupportedRename:", + " argtypes = None", + " restype = None", + " def __call__(self, *_args):", + " ctypes.set_errno(errno.ENOSYS)", + " return -1", + " rename_fn = UnsupportedRename()", + ].join("\n"), +); + +const FORCED_STAGING_OPEN_FAILURE_MUTATION_PYTHON = SANDBOX_PINNED_MUTATION_PYTHON.replace( + " staging_fd = open_dir(candidate, dir_fd=parent_fd)", + " raise OSError(errno.EMFILE, 'forced staging open failure')\n staging_fd = open_dir(candidate, dir_fd=parent_fd)", +); + const FORCED_EXDEV_WITH_LATE_SOURCE_WRITE_MUTATION_PYTHON = FORCED_EXDEV_MUTATION_PYTHON.replace( " remove_copied_entry(src_parent_fd, src_basename, ('dir', entry_identity(src_stat), copied_children))", [ @@ -128,6 +186,156 @@ describe("sandbox pinned mutation helper", () => { }); }); + it("creates a new file through a pinned directory fd", async () => { + await withTempDir({ prefix: "openclaw-mutation-helper-" }, async (root) => { + const workspace = path.join(root, "workspace"); + await fs.mkdir(workspace, { recursive: true }); + + const result = runMutation(["create", workspace, "nested", "note.txt", "1"], "hello"); + + expect(result.status).toBe(0); + await expect(fs.readFile(path.join(workspace, "nested", "note.txt"), "utf8")).resolves.toBe( + "hello", + ); + }); + }); + + it("creates a file whose basename approaches the filesystem component limit", async () => { + await withTempDir({ prefix: "openclaw-mutation-helper-" }, async (root) => { + const workspace = path.join(root, "workspace"); + const basename = "n".repeat(240); + await fs.mkdir(workspace, { recursive: true }); + + const result = runMutation(["create", workspace, "", basename, "0"], "hello"); + + expect(result.status).toBe(0); + await expect(fs.readFile(path.join(workspace, basename), "utf8")).resolves.toBe("hello"); + }); + }); + + it("falls back when Linux libc does not export renameat2", async () => { + await withTempDir({ prefix: "openclaw-mutation-helper-" }, async (root) => { + const workspace = path.join(root, "workspace"); + const filePath = path.join(workspace, "note.txt"); + await fs.mkdir(workspace, { recursive: true }); + + const result = runMutationWithSource( + FORCED_MISSING_RENAMEAT2_MUTATION_PYTHON, + ["create", workspace, "", "note.txt", "0"], + "hello", + ); + + expect(result.status).toBe(0); + await expect(fs.readFile(filePath, "utf8")).resolves.toBe("hello"); + await expect(fs.readdir(workspace)).resolves.toStrictEqual(["note.txt"]); + }); + }); + + it("falls back when Linux renameat2 is unsupported at runtime", async () => { + await withTempDir({ prefix: "openclaw-mutation-helper-" }, async (root) => { + const workspace = path.join(root, "workspace"); + const filePath = path.join(workspace, "note.txt"); + await fs.mkdir(workspace, { recursive: true }); + + const result = runMutationWithSource( + FORCED_UNSUPPORTED_RENAMEAT2_MUTATION_PYTHON, + ["create", workspace, "", "note.txt", "0"], + "hello", + ); + + expect(result.status).toBe(0); + await expect(fs.readFile(filePath, "utf8")).resolves.toBe("hello"); + await expect(fs.readdir(workspace)).resolves.toStrictEqual(["note.txt"]); + }); + }); + + it("removes a staging directory when opening it fails", async () => { + await withTempDir({ prefix: "openclaw-mutation-helper-" }, async (root) => { + const workspace = path.join(root, "workspace"); + await fs.mkdir(workspace, { recursive: true }); + + const result = runMutationWithSource( + FORCED_STAGING_OPEN_FAILURE_MUTATION_PYTHON, + ["create", workspace, "", "note.txt", "0"], + "hello", + ); + + expect(result.status).not.toBe(0); + expect(result.stderr).toContain("forced staging open failure"); + await expect(fs.readdir(workspace)).resolves.toStrictEqual([]); + }); + }); + + it("refuses to create over an existing file and leaves it untouched", async () => { + await withTempDir({ prefix: "openclaw-mutation-helper-" }, async (root) => { + const workspace = path.join(root, "workspace"); + const filePath = path.join(workspace, "note.txt"); + await fs.mkdir(workspace, { recursive: true }); + await fs.writeFile(filePath, "keep me", "utf8"); + + const result = runMutation(["create", workspace, "", "note.txt", "0"], "replacement"); + + expect(result.status).toBe(SANDBOX_CREATE_EXISTS_EXIT_CODE); + await expect(fs.readFile(filePath, "utf8")).resolves.toBe("keep me"); + }); + }); + + it("removes private staging when writing fails before publication", async () => { + await withTempDir({ prefix: "openclaw-mutation-helper-" }, async (root) => { + const workspace = path.join(root, "workspace"); + const filePath = path.join(workspace, "note.txt"); + await fs.mkdir(workspace, { recursive: true }); + + const result = runMutationWithSource( + FORCED_CREATE_FAILURE_MUTATION_PYTHON, + ["create", workspace, "", "note.txt", "0"], + "partial", + ); + + expect(result.status).not.toBe(0); + expect(result.stderr).toContain("forced create failure"); + await expectPathMissing(filePath); + await expect(fs.readdir(workspace)).resolves.toStrictEqual([]); + }); + }); + + it("preserves a destination raced into a staged exclusive create", async () => { + await withTempDir({ prefix: "openclaw-mutation-helper-" }, async (root) => { + const workspace = path.join(root, "workspace"); + const filePath = path.join(workspace, "note.txt"); + await fs.mkdir(workspace, { recursive: true }); + + const result = runMutationWithSource( + FORCED_CREATE_FAILURE_WITH_REPLACEMENT_MUTATION_PYTHON, + ["create", workspace, "", "note.txt", "0"], + "partial", + ); + + expect(result.status).toBe(SANDBOX_CREATE_EXISTS_EXIT_CODE); + await expect(fs.readFile(filePath, "utf8")).resolves.toBe("replacement"); + await expect(fs.readdir(workspace)).resolves.toStrictEqual(["note.txt"]); + }); + }); + + it("fails when the staged exclusive-create pathname is substituted", async () => { + await withTempDir({ prefix: "openclaw-mutation-helper-" }, async (root) => { + const workspace = path.join(root, "workspace"); + const filePath = path.join(workspace, "note.txt"); + await fs.mkdir(workspace, { recursive: true }); + + const result = runMutationWithSource( + FORCED_CREATE_TEMP_SUBSTITUTION_MUTATION_PYTHON, + ["create", workspace, "", "note.txt", "0"], + "expected", + ); + + expect(result.status).not.toBe(0); + expect(result.stderr).toContain("exclusive publication source changed"); + await expect(fs.readFile(filePath, "utf8")).resolves.toBe("replacement"); + await expect(fs.readdir(workspace)).resolves.toStrictEqual(["note.txt"]); + }); + }); + it.runIf(process.platform !== "win32")( "preserves existing target file mode while writing", async () => { diff --git a/src/agents/sandbox/fs-bridge-mutation-helper.ts b/src/agents/sandbox/fs-bridge-mutation-helper.ts index 9aefcec53c48..6a717a4f34ea 100644 --- a/src/agents/sandbox/fs-bridge-mutation-helper.ts +++ b/src/agents/sandbox/fs-bridge-mutation-helper.ts @@ -4,6 +4,11 @@ * Performs symlink-resistant create/replace/delete operations inside a previously validated sandbox boundary. */ import { PATH_ALIAS_POLICIES } from "../../infra/path-alias-guards.js"; +import { + SANDBOX_CREATE_EXCLUSIVE_PYTHON, + SANDBOX_CREATE_STAGING_PYTHON, + SANDBOX_RENAME_NO_REPLACE_PYTHON, +} from "./fs-bridge-native-mutation-python.js"; import type { PathSafetyCheck, PinnedSandboxDirectoryEntry, @@ -18,7 +23,14 @@ const SANDBOX_PINNED_MUTATION_PYTHON_CANDIDATES = [ "/bin/python3", ] as const; +// Exit code the pinned helper reserves for "create target already exists" so +// callers can tell a lost exclusive-create race from a real failure. Any other +// nonzero exit stays an error. +export const SANDBOX_CREATE_EXISTS_EXIT_CODE = 17; + export const SANDBOX_PINNED_MUTATION_PYTHON = [ + `SANDBOX_CREATE_EXISTS_EXIT_CODE = ${SANDBOX_CREATE_EXISTS_EXIT_CODE}`, + "import ctypes", "import errno", "import os", "import secrets", @@ -94,6 +106,8 @@ export const SANDBOX_PINNED_MUTATION_PYTHON = [ " continue", " raise RuntimeError('failed to allocate sandbox temp directory')", "", + SANDBOX_CREATE_STAGING_PYTHON, + "", "def existing_regular_file_mode(parent_fd, basename):", " try:", " target_stat = os.lstat(basename, dir_fd=parent_fd)", @@ -111,6 +125,8 @@ export const SANDBOX_PINNED_MUTATION_PYTHON = [ " raise OSError(errno.EIO, 'failed to write copied file')", " view = view[written:]", "", + SANDBOX_RENAME_NO_REPLACE_PYTHON, + "", "def copy_regular_file(src_parent_fd, src_basename, dst_parent_fd, dst_basename):", " src_fd = os.open(src_basename, READ_FLAGS, dir_fd=src_parent_fd)", " dst_fd = None", @@ -201,6 +217,8 @@ export const SANDBOX_PINNED_MUTATION_PYTHON = [ " except FileNotFoundError:", " pass", "", + SANDBOX_CREATE_EXCLUSIVE_PYTHON, + "", "def read_file_impl(parent_fd, basename, max_bytes):", " file_fd = os.open(basename, READ_FLAGS, dir_fd=parent_fd)", " try:", @@ -255,6 +273,9 @@ export const SANDBOX_PINNED_MUTATION_PYTHON = [ " getattr(entry_stat, 'st_ctime_ns', int(entry_stat.st_ctime * 1000000000)),", " )", "", + "def inode_identity(entry_stat):", + " return (entry_stat.st_dev, entry_stat.st_ino)", + "", "def same_identity(expected, entry_stat):", " return expected == entry_identity(entry_stat)", "", @@ -425,6 +446,19 @@ export const SANDBOX_PINNED_MUTATION_PYTHON = [ " if parent_fd is not None:", " os.close(parent_fd)", " os.close(root_fd)", + "elif operation == 'create':", + " root_fd = open_dir(sys.argv[2])", + " parent_fd = None", + " try:", + " parent_fd = walk_dir(root_fd, sys.argv[3], sys.argv[5] == '1')", + " try:", + " create_exclusive(parent_fd, sys.argv[4], sys.stdin.buffer)", + " except FileExistsError:", + " sys.exit(SANDBOX_CREATE_EXISTS_EXIT_CODE)", + " finally:", + " if parent_fd is not None:", + " os.close(parent_fd)", + " os.close(root_fd)", "elif operation == 'read':", " root_fd = open_dir(sys.argv[2])", " parent_fd = None", @@ -536,6 +570,23 @@ export function buildPinnedWritePlan(params: { }); } +export function buildPinnedCreatePlan(params: { + check: PathSafetyCheck; + pinned: PinnedSandboxEntry; + mkdir: boolean; +}): SandboxFsCommandPlan { + return buildPinnedMutationPlan({ + checks: [params.check], + args: [ + "create", + params.pinned.mountRootPath, + params.pinned.relativeParentPath, + params.pinned.basename, + params.mkdir ? "1" : "0", + ], + }); +} + export function buildPinnedCopyPlan(params: { sourceCheck: PathSafetyCheck; destinationCheck: PathSafetyCheck; diff --git a/src/agents/sandbox/fs-bridge-native-mutation-python.ts b/src/agents/sandbox/fs-bridge-native-mutation-python.ts new file mode 100644 index 000000000000..ecb2ebed761f --- /dev/null +++ b/src/agents/sandbox/fs-bridge-native-mutation-python.ts @@ -0,0 +1,137 @@ +export const SANDBOX_CREATE_STAGING_PYTHON = [ + "def create_staging_dir(parent_fd):", + " # This helper guarantees descriptor-relative confinement and no-replace", + " # publication, not content integrity against same-UID peers; they can", + " # also rewrite the destination immediately after publication.", + " prefix = '.openclaw-create-'", + " for _ in range(128):", + " candidate = prefix + secrets.token_hex(6)", + " try:", + " os.mkdir(candidate, 0o700, dir_fd=parent_fd)", + " except FileExistsError:", + " continue", + " created_identity = entry_identity(os.lstat(candidate, dir_fd=parent_fd))", + " staging_fd = None", + " try:", + " staging_fd = open_dir(candidate, dir_fd=parent_fd)", + " if not same_identity(created_identity, os.fstat(staging_fd)):", + " raise OSError(errno.ESTALE, 'create staging directory changed', candidate)", + " return candidate, staging_fd", + " except Exception:", + " if staging_fd is not None:", + " os.close(staging_fd)", + " try:", + " current = os.lstat(candidate, dir_fd=parent_fd)", + " if same_identity(created_identity, current):", + " os.rmdir(candidate, dir_fd=parent_fd)", + " os.fsync(parent_fd)", + " except FileNotFoundError:", + " pass", + " raise", + " raise RuntimeError('failed to allocate sandbox create staging directory')", +].join("\n"); + +export const SANDBOX_RENAME_NO_REPLACE_PYTHON = [ + "def rename_no_replace(src_parent_fd, src_basename, dst_parent_fd, dst_basename):", + " libc = ctypes.CDLL(None, use_errno=True)", + " is_linux = sys.platform.startswith('linux')", + " if is_linux:", + " rename_fn = getattr(libc, 'renameat2', None)", + " if rename_fn is None:", + " os.link(", + " src_basename,", + " dst_basename,", + " src_dir_fd=src_parent_fd,", + " dst_dir_fd=dst_parent_fd,", + " follow_symlinks=False,", + " )", + " return", + " flags = 1 # RENAME_NOREPLACE", + " elif sys.platform == 'darwin':", + " rename_fn = getattr(libc, 'renameatx_np', None)", + " flags = 0x00000004 # RENAME_EXCL", + " else:", + " rename_fn = None", + " flags = 0", + " if rename_fn is None:", + " raise OSError(errno.ENOSYS, 'atomic no-replace rename is unavailable')", + " rename_fn.argtypes = [ctypes.c_int, ctypes.c_char_p, ctypes.c_int, ctypes.c_char_p, ctypes.c_uint]", + " rename_fn.restype = ctypes.c_int", + " result = rename_fn(", + " src_parent_fd,", + " os.fsencode(src_basename),", + " dst_parent_fd,", + " os.fsencode(dst_basename),", + " flags,", + " )", + " if result != 0:", + " error_code = ctypes.get_errno()", + " unsupported_codes = {errno.ENOSYS, errno.EINVAL, errno.ENOTSUP}", + " if hasattr(errno, 'EOPNOTSUPP'):", + " unsupported_codes.add(errno.EOPNOTSUPP)", + " if is_linux and error_code in unsupported_codes:", + " os.link(", + " src_basename,", + " dst_basename,", + " src_dir_fd=src_parent_fd,", + " dst_dir_fd=dst_parent_fd,", + " follow_symlinks=False,", + " )", + " return", + " raise OSError(error_code, os.strerror(error_code), dst_basename)", +].join("\n"); + +export const SANDBOX_CREATE_EXCLUSIVE_PYTHON = [ + "def create_exclusive(parent_fd, basename, stdin_buffer):", + " staging_fd = None", + " staging_name = None", + " temp_fd = None", + " temp_name = 'payload'", + " temp_inode = None", + " try:", + " try:", + " os.lstat(basename, dir_fd=parent_fd)", + " except FileNotFoundError:", + " pass", + " else:", + " raise FileExistsError(errno.EEXIST, os.strerror(errno.EEXIST), basename)", + " staging_name, staging_fd = create_staging_dir(parent_fd)", + " temp_fd = os.open(temp_name, WRITE_FLAGS, 0o600, dir_fd=staging_fd)", + " while True:", + " chunk = stdin_buffer.read(65536)", + " if not chunk:", + " break", + " write_all(temp_fd, chunk)", + " # exclusive create payload is durable before publication", + " os.fsync(temp_fd)", + " temp_inode = inode_identity(os.fstat(temp_fd))", + " # Publish with a native atomic no-replace rename.", + " rename_no_replace(staging_fd, temp_name, parent_fd, basename)", + " target_stat = os.lstat(basename, dir_fd=parent_fd)", + " if temp_inode != inode_identity(target_stat):", + " raise OSError(errno.ESTALE, 'exclusive publication source changed', basename)", + " os.fsync(parent_fd)", + " finally:", + " if temp_fd is not None:", + " os.close(temp_fd)", + " if staging_fd is not None:", + " # Cleanup stays relative to the private pinned directory, so a", + " # parent-path substitution cannot redirect payload deletion.", + " try:", + " os.unlink(temp_name, dir_fd=staging_fd)", + " except FileNotFoundError:", + " pass", + " staging_identity = entry_identity(os.fstat(staging_fd))", + " os.close(staging_fd)", + " staging_removed = False", + " if staging_name is not None:", + " try:", + " current_staging = os.lstat(staging_name, dir_fd=parent_fd)", + " if same_identity(staging_identity, current_staging):", + " os.rmdir(staging_name, dir_fd=parent_fd)", + " staging_removed = True", + " except FileNotFoundError:", + " pass", + " if staging_removed:", + " os.fsync(parent_fd)", +].join("\n"); diff --git a/src/agents/sandbox/fs-bridge.anchored-ops.test.ts b/src/agents/sandbox/fs-bridge.anchored-ops.test.ts index 03575556ca84..38952f85c6aa 100644 --- a/src/agents/sandbox/fs-bridge.anchored-ops.test.ts +++ b/src/agents/sandbox/fs-bridge.anchored-ops.test.ts @@ -149,6 +149,18 @@ describe("sandbox fs bridge anchored ops", () => { }); const pinnedCases = [ + { + name: "exclusive create pins canonical parent + basename", + invoke: (bridge: ReturnType) => { + const createFileExclusive = bridge.createFileExclusive?.bind(bridge); + if (!createFileExclusive) { + throw new Error("expected exclusive-create capability"); + } + return createFileExclusive({ filePath: "nested/new.txt", data: "created" }); + }, + expectedArgs: ["create", "/workspace", "nested", "new.txt", "1"], + forbiddenArgs: ["/workspace/nested/new.txt"], + }, { name: "write pins canonical parent + basename", invoke: (bridge: ReturnType) => diff --git a/src/agents/sandbox/fs-bridge.ts b/src/agents/sandbox/fs-bridge.ts index 1a8c616931a4..d8b7f42d0175 100644 --- a/src/agents/sandbox/fs-bridge.ts +++ b/src/agents/sandbox/fs-bridge.ts @@ -12,6 +12,8 @@ import type { } from "./backend-handle.types.js"; import { runDockerSandboxShellCommand } from "./docker-backend.js"; import { + buildPinnedCreatePlan, + SANDBOX_CREATE_EXISTS_EXIT_CODE, buildPinnedCopyPlan, buildPinnedMkdirpPlan, buildPinnedRemovePlan, @@ -149,6 +151,49 @@ class SandboxFsBridgeImpl implements SandboxFsBridge { }); } + async createFileExclusive(params: { + filePath: string; + cwd?: string; + data: Buffer | string; + encoding?: BufferEncoding; + mkdir?: boolean; + signal?: AbortSignal; + }): Promise<"created" | "exists"> { + const target = this.resolveResolvedPath(params); + this.ensureWriteAccess(target, "create files"); + const createCheck = { + target, + options: { action: "create files", requireWritable: true } as const, + }; + await this.pathGuard.assertPathSafety(target, createCheck.options); + const buffer = Buffer.isBuffer(params.data) + ? params.data + : Buffer.from(params.data, params.encoding ?? "utf8"); + const pinnedCreateTarget = await this.pathGuard.resolveAnchoredPinnedEntry( + target, + "create files", + ); + const result = await this.runCheckedCommand({ + ...buildPinnedCreatePlan({ + check: createCheck, + pinned: pinnedCreateTarget, + mkdir: params.mkdir !== false, + }), + allowFailure: true, + stdin: buffer, + signal: params.signal, + }); + if (result.code === SANDBOX_CREATE_EXISTS_EXIT_CODE) { + return "exists"; + } + if (result.code !== 0) { + throw new Error( + `sandbox create failed for ${target.containerPath}: ${result.stderr.toString("utf8").trim()}`, + ); + } + return "created"; + } + async mkdirp(params: { filePath: string; cwd?: string; signal?: AbortSignal }): Promise { const target = this.resolveResolvedPath(params); this.ensureWriteAccess(target, "create directories"); diff --git a/src/agents/sandbox/fs-bridge.types.ts b/src/agents/sandbox/fs-bridge.types.ts index e86280715321..d611ac3d8b2c 100644 --- a/src/agents/sandbox/fs-bridge.types.ts +++ b/src/agents/sandbox/fs-bridge.types.ts @@ -44,6 +44,19 @@ export type SandboxFsBridge = { mkdir?: boolean; signal?: AbortSignal; }): Promise; + /** + * Atomically creates a file only when no entry already exists at the path. + * Backends without this capability must omit it rather than emulate it with + * a check followed by writeFile. + */ + createFileExclusive?(params: { + filePath: string; + cwd?: string; + data: Buffer | string; + encoding?: BufferEncoding; + mkdir?: boolean; + signal?: AbortSignal; + }): Promise<"created" | "exists">; mkdirp(params: { filePath: string; cwd?: string; signal?: AbortSignal }): Promise; remove(params: { filePath: string; diff --git a/src/agents/sandbox/remote-fs-bridge.test.ts b/src/agents/sandbox/remote-fs-bridge.test.ts index eb636122d8dc..31e56f156826 100644 --- a/src/agents/sandbox/remote-fs-bridge.test.ts +++ b/src/agents/sandbox/remote-fs-bridge.test.ts @@ -100,6 +100,74 @@ function createWorkspaceReadBridge(workspaceDir: string) { } describe("remote sandbox fs bridge", () => { + it.runIf(process.platform !== "win32")( + "creates files exclusively and preserves existing entries", + async () => { + await withTempDir("openclaw-remote-fs-create-", async (stateDir) => { + const workspaceDir = path.join(stateDir, "workspace"); + await fs.mkdir(workspaceDir, { recursive: true }); + const { runtime } = createLocalRemoteRuntime({ + remoteWorkspaceDir: workspaceDir, + remoteAgentWorkspaceDir: workspaceDir, + }); + const bridge = createRemoteShellSandboxFsBridge({ + sandbox: createSandbox({ workspaceDir, agentWorkspaceDir: workspaceDir }), + runtime, + }); + const createFileExclusive = bridge.createFileExclusive?.bind(bridge); + expect(createFileExclusive).toBeTypeOf("function"); + + await expect( + createFileExclusive!({ filePath: "nested/note.txt", data: "first" }), + ).resolves.toBe("created"); + await expect( + createFileExclusive!({ filePath: "nested/note.txt", data: "replacement" }), + ).resolves.toBe("exists"); + await expect( + fs.readFile(path.join(workspaceDir, "nested", "note.txt"), "utf8"), + ).resolves.toBe("first"); + + const outcomes = await Promise.all([ + createFileExclusive!({ filePath: "race.txt", data: "one" }), + createFileExclusive!({ filePath: "race.txt", data: "two" }), + ]); + expect(outcomes.toSorted()).toEqual(["created", "exists"]); + await expect(fs.readFile(path.join(workspaceDir, "race.txt"), "utf8")).resolves.toMatch( + /^(one|two)$/u, + ); + }); + }, + ); + + it.runIf(process.platform !== "win32")( + "treats a symlink destination as existing without changing its target", + async () => { + await withTempDir("openclaw-remote-fs-create-", async (stateDir) => { + const workspaceDir = path.join(stateDir, "workspace"); + await fs.mkdir(workspaceDir, { recursive: true }); + await fs.writeFile(path.join(workspaceDir, "target.txt"), "keep", "utf8"); + await fs.symlink("target.txt", path.join(workspaceDir, "link.txt")); + const { runtime } = createLocalRemoteRuntime({ + remoteWorkspaceDir: workspaceDir, + remoteAgentWorkspaceDir: workspaceDir, + }); + const bridge = createRemoteShellSandboxFsBridge({ + sandbox: createSandbox({ workspaceDir, agentWorkspaceDir: workspaceDir }), + runtime, + }); + const createFileExclusive = bridge.createFileExclusive?.bind(bridge); + expect(createFileExclusive).toBeTypeOf("function"); + + await expect( + createFileExclusive!({ filePath: "link.txt", data: "replacement" }), + ).resolves.toBe("exists"); + await expect(fs.readFile(path.join(workspaceDir, "target.txt"), "utf8")).resolves.toBe( + "keep", + ); + }); + }, + ); + it.runIf(process.platform !== "win32")( "reads files with the pinned mutation helper", async () => { diff --git a/src/agents/sandbox/remote-fs-bridge.ts b/src/agents/sandbox/remote-fs-bridge.ts index 4ab69a854482..29253547c103 100644 --- a/src/agents/sandbox/remote-fs-bridge.ts +++ b/src/agents/sandbox/remote-fs-bridge.ts @@ -11,7 +11,10 @@ import type { SandboxBackendCommandResult, SandboxFsBridgeContext, } from "./backend-handle.types.js"; -import { SANDBOX_PINNED_MUTATION_PYTHON } from "./fs-bridge-mutation-helper.js"; +import { + SANDBOX_CREATE_EXISTS_EXIT_CODE, + SANDBOX_PINNED_MUTATION_PYTHON, +} from "./fs-bridge-mutation-helper.js"; import { createWritableRenameTargetResolver } from "./fs-bridge-rename-targets.js"; import { parseSandboxStatMtimeMs, parseSandboxStatSize } from "./fs-bridge-stat-parse.js"; import type { SandboxFsBridge, SandboxFsStat, SandboxResolvedPath } from "./fs-bridge.types.js"; @@ -191,6 +194,48 @@ class RemoteShellSandboxFsBridge implements SandboxFsBridge { }); } + async createFileExclusive(params: { + filePath: string; + cwd?: string; + data: Buffer | string; + encoding?: BufferEncoding; + mkdir?: boolean; + signal?: AbortSignal; + }): Promise<"created" | "exists"> { + const target = this.resolveTarget(params); + await this.ensureRemoteWritable(target, "create files", params.signal); + const pinned = await this.resolvePinnedParent({ + containerPath: target.containerPath, + action: "create files", + requireWritable: true, + signal: params.signal, + }); + const buffer = Buffer.isBuffer(params.data) + ? params.data + : Buffer.from(params.data, params.encoding ?? "utf8"); + const result = await this.runMutation({ + args: [ + "create", + pinned.mountRootPath, + pinned.relativeParentPath, + pinned.basename, + params.mkdir !== false ? "1" : "0", + ], + stdin: buffer, + allowFailure: true, + signal: params.signal, + }); + if (result.code === SANDBOX_CREATE_EXISTS_EXIT_CODE) { + return "exists"; + } + if (result.code !== 0) { + throw new Error( + `Sandbox create failed for ${target.containerPath}: ${result.stderr.toString("utf8").trim()}`, + ); + } + return "created"; + } + async mkdirp(params: { filePath: string; cwd?: string; signal?: AbortSignal }): Promise { const target = this.resolveTarget(params); await this.ensureRemoteWritable(target, "create directories", params.signal); diff --git a/src/agents/test-helpers/host-sandbox-fs-bridge.ts b/src/agents/test-helpers/host-sandbox-fs-bridge.ts index 61a4fed0e5b4..2547236a4f02 100644 --- a/src/agents/test-helpers/host-sandbox-fs-bridge.ts +++ b/src/agents/test-helpers/host-sandbox-fs-bridge.ts @@ -45,6 +45,25 @@ export function createSandboxFsBridgeFromResolver( const buffer = Buffer.isBuffer(data) ? data : Buffer.from(data); await fs.writeFile(target.hostPath, buffer); }, + createFileExclusive: async ({ filePath, cwd, data, mkdir = true }) => { + const target = resolvePath(filePath, cwd); + if (!target.hostPath) { + throw new Error(`Expected hostPath for ${target.containerPath}`); + } + if (mkdir) { + await fs.mkdir(path.dirname(target.hostPath), { recursive: true }); + } + const buffer = Buffer.isBuffer(data) ? data : Buffer.from(data); + try { + await fs.writeFile(target.hostPath, buffer, { flag: "wx" }); + return "created"; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "EEXIST") { + return "exists"; + } + throw error; + } + }, mkdirp: async ({ filePath, cwd }) => { const target = resolvePath(filePath, cwd); if (!target.hostPath) {