mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 12:56:01 -06:00
fix(agents): prevent silent corruption when editing non-UTF-8 files (#115088)
This commit is contained in:
committed by
GitHub
parent
7cf14c12c8
commit
b6bebfa73c
@@ -104,6 +104,79 @@ function createSandboxFsTools(params: { sandbox: UnsafeMountedSandbox; workspace
|
||||
}
|
||||
|
||||
describe("tools.fs.workspaceOnly", () => {
|
||||
it("preserves valid UTF-8 BOM bytes through real sandbox edit and patch bridges", async () => {
|
||||
await withUnsafeMountedSandboxHarness(async ({ sandboxRoot, sandbox }) => {
|
||||
const filePath = path.join(sandboxRoot, "source.txt");
|
||||
const original = Buffer.from("\uFEFFheading\nprice: 5\n", "utf8");
|
||||
const expected = Buffer.from("\uFEFFheading\nprice: 7\n", "utf8");
|
||||
const editTool = createSandboxedEditTool({
|
||||
root: sandbox.workspaceDir,
|
||||
bridge: sandbox.fsBridge!,
|
||||
});
|
||||
|
||||
await fs.writeFile(filePath, original);
|
||||
await editTool.execute("sandbox-edit-bom", {
|
||||
path: "source.txt",
|
||||
edits: [{ oldText: "price: 5", newText: "price: 7" }],
|
||||
});
|
||||
await expect(fs.readFile(filePath)).resolves.toEqual(expected);
|
||||
|
||||
await fs.writeFile(filePath, original);
|
||||
const patchTool = createApplyPatchTool({
|
||||
cwd: sandbox.workspaceDir,
|
||||
sandbox: { root: sandbox.workspaceDir, bridge: sandbox.fsBridge! },
|
||||
});
|
||||
await patchTool.execute("sandbox-patch-bom", {
|
||||
input: `*** Begin Patch
|
||||
*** Update File: source.txt
|
||||
@@
|
||||
-price: 5
|
||||
+price: 7
|
||||
*** End Patch`,
|
||||
});
|
||||
await expect(fs.readFile(filePath)).resolves.toEqual(expected);
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects invalid UTF-8 before sandbox edit or patch bridge writes", async () => {
|
||||
await withUnsafeMountedSandboxHarness(async ({ sandboxRoot, sandbox }) => {
|
||||
const filePath = path.join(sandboxRoot, "source.txt");
|
||||
const original = Buffer.concat([
|
||||
Buffer.from("heading\nprice: 5\n"),
|
||||
Buffer.from([0xff, 0xfe]),
|
||||
]);
|
||||
await fs.writeFile(filePath, original);
|
||||
const editTool = createSandboxedEditTool({
|
||||
root: sandbox.workspaceDir,
|
||||
bridge: sandbox.fsBridge!,
|
||||
});
|
||||
|
||||
await expect(
|
||||
editTool.execute("sandbox-edit-invalid-utf8", {
|
||||
path: "source.txt",
|
||||
edits: [{ oldText: "price: 5", newText: "price: 7" }],
|
||||
}),
|
||||
).rejects.toThrow(/not valid UTF-8/);
|
||||
await expect(fs.readFile(filePath)).resolves.toEqual(original);
|
||||
|
||||
const patchTool = createApplyPatchTool({
|
||||
cwd: sandbox.workspaceDir,
|
||||
sandbox: { root: sandbox.workspaceDir, bridge: sandbox.fsBridge! },
|
||||
});
|
||||
await expect(
|
||||
patchTool.execute("sandbox-patch-invalid-utf8", {
|
||||
input: `*** Begin Patch
|
||||
*** Update File: source.txt
|
||||
@@
|
||||
-price: 5
|
||||
+price: 7
|
||||
*** End Patch`,
|
||||
}),
|
||||
).rejects.toThrow(/not valid UTF-8/);
|
||||
await expect(fs.readFile(filePath)).resolves.toEqual(original);
|
||||
});
|
||||
});
|
||||
|
||||
it("defaults to allowing sandbox mounts outside the workspace root", async () => {
|
||||
await withUnsafeMountedSandboxHarness(async ({ agentRoot, sandbox }) => {
|
||||
await fs.writeFile(path.join(agentRoot, "secret.txt"), "shh", "utf8");
|
||||
|
||||
@@ -40,19 +40,24 @@ function buildAddFilePatch(targetPath: string): string {
|
||||
*** End Patch`;
|
||||
}
|
||||
|
||||
function createMemoryPatchSandbox(initialFiles: Record<string, string> = {}) {
|
||||
const files = new Map<string, string>(
|
||||
function createMemoryPatchSandbox(initialFiles: Record<string, string | Buffer> = {}) {
|
||||
const files = new Map<string, string | Buffer>(
|
||||
Object.entries(initialFiles).map(([filePath, contents]) => [`/sandbox/${filePath}`, contents]),
|
||||
);
|
||||
const writeFile = vi.fn(async ({ filePath, data }) => {
|
||||
files.set(filePath, Buffer.isBuffer(data) ? data.toString("utf8") : data);
|
||||
files.set(filePath, Buffer.isBuffer(data) ? Buffer.from(data) : data);
|
||||
});
|
||||
const bridge: SandboxFsBridge = {
|
||||
resolvePath: ({ filePath }) => ({
|
||||
relativePath: filePath,
|
||||
containerPath: `/sandbox/${filePath}`,
|
||||
}),
|
||||
readFile: async ({ filePath }) => Buffer.from(files.get(filePath) ?? "", "utf8"),
|
||||
readFile: async ({ filePath }) => {
|
||||
const contents = files.get(filePath);
|
||||
return typeof contents === "string"
|
||||
? Buffer.from(contents, "utf8")
|
||||
: Buffer.from(contents ?? "");
|
||||
},
|
||||
writeFile,
|
||||
remove: async ({ filePath }) => {
|
||||
files.delete(filePath);
|
||||
@@ -107,6 +112,68 @@ async function expectMissingPath(operation: Promise<unknown>) {
|
||||
}
|
||||
|
||||
describe("applyPatch", () => {
|
||||
const priceUpdatePatch = `*** Begin Patch
|
||||
*** Update File: source.txt
|
||||
@@
|
||||
-price: 5
|
||||
+price: 7
|
||||
*** End Patch`;
|
||||
|
||||
it.each([
|
||||
{ name: "workspace-confined host", workspaceOnly: true },
|
||||
{ name: "unconfined host", workspaceOnly: false },
|
||||
])("preserves a valid UTF-8 BOM in $name updates", async ({ workspaceOnly }) => {
|
||||
await withTempDir(async (dir) => {
|
||||
const filePath = path.join(dir, "source.txt");
|
||||
await fs.writeFile(filePath, Buffer.from("\uFEFFheading\nprice: 5\n", "utf8"));
|
||||
|
||||
await applyPatch(priceUpdatePatch, { cwd: dir, workspaceOnly });
|
||||
|
||||
await expect(fs.readFile(filePath)).resolves.toEqual(
|
||||
Buffer.from("\uFEFFheading\nprice: 7\n", "utf8"),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ name: "workspace-confined host", workspaceOnly: true },
|
||||
{ name: "unconfined host", workspaceOnly: false },
|
||||
])("rejects invalid UTF-8 in $name updates without changing bytes", async ({ workspaceOnly }) => {
|
||||
await withTempDir(async (dir) => {
|
||||
const filePath = path.join(dir, "source.txt");
|
||||
const original = Buffer.concat([
|
||||
Buffer.from("heading\nprice: 5\n"),
|
||||
Buffer.from([0xff, 0xfe]),
|
||||
]);
|
||||
await fs.writeFile(filePath, original);
|
||||
|
||||
await expect(applyPatch(priceUpdatePatch, { cwd: dir, workspaceOnly })).rejects.toThrow(
|
||||
/not valid UTF-8/,
|
||||
);
|
||||
await expect(fs.readFile(filePath)).resolves.toEqual(original);
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves a valid UTF-8 BOM in sandbox updates", async () => {
|
||||
const memory = createMemoryPatchSandbox({
|
||||
"source.txt": Buffer.from("\uFEFFheading\nprice: 5\n", "utf8"),
|
||||
});
|
||||
|
||||
await applyPatch(priceUpdatePatch, memory.options);
|
||||
|
||||
expect(memory.files.get("/sandbox/source.txt")).toBe("\uFEFFheading\nprice: 7\n");
|
||||
});
|
||||
|
||||
it("rejects invalid sandbox UTF-8 before writing or changing bytes", async () => {
|
||||
const original = Buffer.concat([Buffer.from("heading\nprice: 5\n"), Buffer.from([0xff, 0xfe])]);
|
||||
const memory = createMemoryPatchSandbox({ "source.txt": original });
|
||||
|
||||
await expect(applyPatch(priceUpdatePatch, memory.options)).rejects.toThrow(/not valid UTF-8/);
|
||||
|
||||
expect(memory.writeFile).not.toHaveBeenCalled();
|
||||
expect(memory.files.get("/sandbox/source.txt")).toEqual(original);
|
||||
});
|
||||
|
||||
it("adds a file", async () => {
|
||||
const memory = createMemoryPatchSandbox();
|
||||
const patch = `*** Begin Patch
|
||||
|
||||
@@ -16,6 +16,7 @@ import { toRelativeSandboxPath, 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 { decodeUtf8File } from "./utf8-file.js";
|
||||
|
||||
const BEGIN_PATCH_MARKER = "*** Begin Patch";
|
||||
const END_PATCH_MARKER = "*** End Patch";
|
||||
@@ -291,7 +292,7 @@ function resolvePatchFileOps(options: ApplyPatchOptions): PatchFileOps {
|
||||
return {
|
||||
readFile: async (filePath) => {
|
||||
const buf = await bridge.readFile({ filePath, cwd: root });
|
||||
return buf.toString("utf8");
|
||||
return decodeUtf8File(buf, filePath);
|
||||
},
|
||||
writeFile: (filePath, content) => bridge.writeFile({ filePath, cwd: root, data: content }),
|
||||
remove: (filePath) => bridge.remove({ filePath, cwd: root, force: false }),
|
||||
@@ -303,7 +304,7 @@ function resolvePatchFileOps(options: ApplyPatchOptions): PatchFileOps {
|
||||
return {
|
||||
readFile: async (filePath) => {
|
||||
if (!workspaceOnly) {
|
||||
return await fs.readFile(filePath, "utf8");
|
||||
return decodeUtf8File(await fs.readFile(filePath), filePath);
|
||||
}
|
||||
const opened = await openRootFile({
|
||||
absolutePath: filePath,
|
||||
@@ -312,7 +313,7 @@ function resolvePatchFileOps(options: ApplyPatchOptions): PatchFileOps {
|
||||
});
|
||||
assertBoundaryRead(opened, filePath);
|
||||
try {
|
||||
return syncFs.readFileSync(opened.fd, "utf8");
|
||||
return decodeUtf8File(syncFs.readFileSync(opened.fd), filePath);
|
||||
} finally {
|
||||
syncFs.closeSync(opened.fd);
|
||||
}
|
||||
|
||||
@@ -27,13 +27,74 @@ describe("edit tool", () => {
|
||||
}
|
||||
});
|
||||
|
||||
async function createTempFile(content: string) {
|
||||
async function createTempFile(content: string | Buffer) {
|
||||
tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-edit-tool-"));
|
||||
const filePath = path.join(tmpDir, "demo.txt");
|
||||
await fs.writeFile(filePath, content, "utf-8");
|
||||
return filePath;
|
||||
}
|
||||
|
||||
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);
|
||||
|
||||
await tool.execute(
|
||||
"call-bom",
|
||||
{
|
||||
path: filePath,
|
||||
edits: [{ oldText: "price: 5", newText: "price: 7" }],
|
||||
},
|
||||
undefined,
|
||||
);
|
||||
|
||||
await expect(fs.readFile(filePath)).resolves.toEqual(
|
||||
Buffer.from("\uFEFFheading\nprice: 7\n", "utf-8"),
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects invalid UTF-8 without changing a real file", async () => {
|
||||
const original = Buffer.concat([Buffer.from("heading\nprice: 5\n"), Buffer.from([0xff, 0xfe])]);
|
||||
const filePath = await createTempFile(original);
|
||||
const tool = createEditTool(tmpDir);
|
||||
|
||||
await expect(
|
||||
tool.execute(
|
||||
"call-invalid-utf8",
|
||||
{
|
||||
path: filePath,
|
||||
edits: [{ oldText: "price: 5", newText: "price: 7" }],
|
||||
},
|
||||
undefined,
|
||||
),
|
||||
).rejects.toThrow(/not valid UTF-8/);
|
||||
|
||||
await expect(fs.readFile(filePath)).resolves.toEqual(original);
|
||||
});
|
||||
|
||||
it("rejects invalid remote-operation UTF-8 before any write", async () => {
|
||||
const original = Buffer.concat([Buffer.from("heading\nprice: 5\n"), Buffer.from([0xff, 0xfe])]);
|
||||
const writeFile = vi.fn<EditOperations["writeFile"]>();
|
||||
const operations: EditOperations = {
|
||||
access: async () => {},
|
||||
readFile: async () => Buffer.from(original),
|
||||
writeFile,
|
||||
};
|
||||
const tool = createEditTool("/remote/workspace", { operations });
|
||||
|
||||
await expect(
|
||||
tool.execute(
|
||||
"call-remote-invalid-utf8",
|
||||
{
|
||||
path: "/remote/workspace/source.txt",
|
||||
edits: [{ oldText: "price: 5", newText: "price: 7" }],
|
||||
},
|
||||
undefined,
|
||||
),
|
||||
).rejects.toThrow(/not valid UTF-8/);
|
||||
|
||||
expect(writeFile).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("adds current file contents to exact-match mismatch errors", async () => {
|
||||
const filePath = await createTempFile("actual current content");
|
||||
const tool = createEditTool(tmpDir);
|
||||
|
||||
@@ -15,6 +15,7 @@ import { Type } from "typebox";
|
||||
import { renderDiff } from "../../modes/interactive/components/diff.js";
|
||||
import type { AgentTool } from "../../runtime/index.js";
|
||||
import { textResult } from "../../tools/common.js";
|
||||
import { decodeUtf8File } from "../../utf8-file.js";
|
||||
import type { ToolDefinition } from "../extensions/types.js";
|
||||
import {
|
||||
applyEditsToNormalizedContent,
|
||||
@@ -437,7 +438,7 @@ export function createEditToolDefinition(
|
||||
}
|
||||
|
||||
const buffer = await ops.readFile(absolutePath);
|
||||
const rawContent = buffer.toString("utf-8");
|
||||
const rawContent = decodeUtf8File(buffer, absolutePath);
|
||||
try {
|
||||
if (signal?.aborted) {
|
||||
throw new Error("Operation aborted");
|
||||
|
||||
@@ -0,0 +1,15 @@
|
||||
const strictUtf8FileDecoder = new TextDecoder("utf-8", {
|
||||
fatal: true,
|
||||
ignoreBOM: true,
|
||||
});
|
||||
|
||||
/** Reject invalid file bytes without stripping a valid leading UTF-8 BOM. */
|
||||
export function decodeUtf8File(contents: Buffer, filePath: string): string {
|
||||
try {
|
||||
return strictUtf8FileDecoder.decode(contents);
|
||||
} catch (error) {
|
||||
throw new Error(`File ${filePath} is not valid UTF-8 and cannot be edited safely.`, {
|
||||
cause: error,
|
||||
});
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user