test: speed apply-patch and exec approval hotspots

This commit is contained in:
Peter Steinberger
2026-04-18 18:33:05 +01:00
parent e45a50c828
commit 16bd427cb6
2 changed files with 119 additions and 81 deletions

View File

@@ -7,6 +7,7 @@ import {
withRealpathSymlinkRebindRace,
} from "../test-utils/symlink-rebind-race.js";
import { applyPatch } from "./apply-patch.js";
import type { SandboxFsBridge } from "./sandbox/fs-bridge.js";
async function withTempDir<T>(fn: (dir: string) => Promise<T>) {
const dir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-patch-"));
@@ -33,6 +34,49 @@ function buildAddFilePatch(targetPath: string): string {
*** End Patch`;
}
function createMemoryPatchSandbox(initialFiles: Record<string, string> = {}) {
const files = new Map<string, string>(
Object.entries(initialFiles).map(([filePath, contents]) => [`/sandbox/${filePath}`, contents]),
);
const bridge: SandboxFsBridge = {
resolvePath: ({ filePath }) => ({
relativePath: filePath,
containerPath: `/sandbox/${filePath}`,
}),
readFile: async ({ filePath }) => Buffer.from(files.get(filePath) ?? "", "utf8"),
writeFile: async ({ filePath, data }) => {
files.set(filePath, Buffer.isBuffer(data) ? data.toString("utf8") : data);
},
remove: async ({ filePath }) => {
files.delete(filePath);
},
rename: async ({ from, to }) => {
const contents = files.get(from);
if (contents !== undefined) {
files.set(to, contents);
files.delete(from);
}
},
stat: async ({ filePath }) => {
const contents = files.get(filePath);
return contents === undefined
? null
: { type: "file", size: Buffer.byteLength(contents), mtimeMs: 0 };
},
mkdirp: async () => {},
};
return {
files,
options: {
cwd: "/local/workspace",
sandbox: {
root: "/local/workspace",
bridge,
},
},
};
}
async function expectOutsideWriteRejected(params: {
dir: string;
patchTargetPath: string;
@@ -45,26 +89,23 @@ async function expectOutsideWriteRejected(params: {
describe("applyPatch", () => {
it("adds a file", async () => {
await withTempDir(async (dir) => {
const patch = `*** Begin Patch
const memory = createMemoryPatchSandbox();
const patch = `*** Begin Patch
*** Add File: hello.txt
+hello
*** End Patch`;
const result = await applyPatch(patch, { cwd: dir });
const contents = await fs.readFile(path.join(dir, "hello.txt"), "utf8");
const result = await applyPatch(patch, memory.options);
expect(contents).toBe("hello\n");
expect(result.summary.added).toEqual(["hello.txt"]);
});
expect(memory.files.get("/sandbox/hello.txt")).toBe("hello\n");
expect(result.summary.added).toEqual(["hello.txt"]);
});
it("updates and moves a file", async () => {
await withTempDir(async (dir) => {
const source = path.join(dir, "source.txt");
await fs.writeFile(source, "foo\nbar\n", "utf8");
const patch = `*** Begin Patch
const memory = createMemoryPatchSandbox({
"source.txt": "foo\nbar\n",
});
const patch = `*** Begin Patch
*** Update File: source.txt
*** Move to: dest.txt
@@
@@ -73,32 +114,27 @@ describe("applyPatch", () => {
+baz
*** End Patch`;
const result = await applyPatch(patch, { cwd: dir });
const dest = path.join(dir, "dest.txt");
const contents = await fs.readFile(dest, "utf8");
const result = await applyPatch(patch, memory.options);
expect(contents).toBe("foo\nbaz\n");
await expect(fs.stat(source)).rejects.toBeDefined();
expect(result.summary.modified).toEqual(["dest.txt"]);
});
expect(memory.files.get("/sandbox/dest.txt")).toBe("foo\nbaz\n");
expect(memory.files.has("/sandbox/source.txt")).toBe(false);
expect(result.summary.modified).toEqual(["dest.txt"]);
});
it("supports end-of-file inserts", async () => {
await withTempDir(async (dir) => {
const target = path.join(dir, "end.txt");
await fs.writeFile(target, "line1\n", "utf8");
const patch = `*** Begin Patch
const memory = createMemoryPatchSandbox({
"end.txt": "line1\n",
});
const patch = `*** Begin Patch
*** Update File: end.txt
@@
+line2
*** End of File
*** End Patch`;
await applyPatch(patch, { cwd: dir });
const contents = await fs.readFile(target, "utf8");
expect(contents).toBe("line1\nline2\n");
});
await applyPatch(patch, memory.options);
expect(memory.files.get("/sandbox/end.txt")).toBe("line1\nline2\n");
});
it("rejects path traversal outside cwd by default", async () => {
@@ -152,17 +188,17 @@ describe("applyPatch", () => {
});
it("deletes the resolved target path", async () => {
await withTempDir(async (dir) => {
const target = path.join(dir, "delete-me.txt");
await fs.writeFile(target, "x\n", "utf8");
const patch = `*** Begin Patch
const memory = createMemoryPatchSandbox({
"delete-me.txt": "x\n",
});
const patch = `*** Begin Patch
*** Delete File: delete-me.txt
*** End Patch`;
const result = await applyPatch(patch, { cwd: dir });
expect(result.summary.deleted).toEqual(["delete-me.txt"]);
await expect(fs.stat(target)).rejects.toBeDefined();
});
const result = await applyPatch(patch, memory.options);
expect(result.summary.deleted).toEqual(["delete-me.txt"]);
expect(memory.files.has("/sandbox/delete-me.txt")).toBe(false);
});
it("rejects symlink escape attempts by default", async () => {

View File

@@ -3,7 +3,7 @@ import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { clearConfigCache, clearRuntimeConfigSnapshot } from "../config/config.js";
import { clearConfigCache, clearRuntimeConfigSnapshot } from "../config/io.js";
import { sendMessage } from "../infra/outbound/message.js";
import { buildSystemRunPreparePayload } from "../test-utils/system-run-prepare-payload.js";
import { createExecTool } from "./bash-tools.exec.js";
@@ -494,66 +494,68 @@ describe("exec approvals", () => {
expect(calls).not.toContain("exec.approval.request");
});
it("uses exec-approvals ask=off to suppress gateway prompts", async () => {
await expectGatewayExecWithoutApproval({
config: {
version: 1,
defaults: { security: "full", ask: "off", askFallback: "full" },
agents: {
main: { security: "full", ask: "off", askFallback: "full" },
it("uses exec-approvals defaults to suppress gateway prompts", async () => {
const cases: Array<{
config: Record<string, unknown>;
ask?: "always" | "on-miss" | "off";
security?: "allowlist" | "full";
}> = [
{
config: {
version: 1,
defaults: { security: "full", ask: "off", askFallback: "full" },
agents: {
main: { security: "full", ask: "off", askFallback: "full" },
},
},
ask: "on-miss",
},
{
config: {
version: 1,
defaults: { security: "full", ask: "off", askFallback: "full" },
agents: {},
},
},
command: "echo ok",
ask: "on-miss",
});
});
it("inherits ask=off from exec-approvals defaults when tool ask is unset", async () => {
await expectGatewayExecWithoutApproval({
config: {
version: 1,
defaults: { security: "full", ask: "off", askFallback: "full" },
agents: {},
{
config: {
version: 1,
defaults: { security: "full", ask: "off", askFallback: "full" },
agents: {},
},
security: undefined,
},
command: "echo ok",
});
];
for (const testCase of cases) {
await expectGatewayExecWithoutApproval({
...testCase,
command: "echo ok",
});
}
});
it("inherits security=full from exec-approvals defaults when tool security is unset", async () => {
await expectGatewayExecWithoutApproval({
config: {
version: 1,
defaults: { security: "full", ask: "off", askFallback: "full" },
agents: {},
},
command: "echo ok",
security: undefined,
});
});
it("keeps ask=always prompts even when durable allow-always trust matches", async () => {
const result = await expectGatewayAskAlwaysPrompt({
it("keeps ask=always prompts for durable and static allowlist entries", async () => {
const durable = await expectGatewayAskAlwaysPrompt({
turnId: "call-gateway-durable-still-prompts",
allowlist: [{ pattern: process.execPath, source: "allow-always" }],
});
expect(result.details.status).toBe("approval-pending");
expect(result.details).toMatchObject({
expect(durable.details.status).toBe("approval-pending");
expect(durable.details).toMatchObject({
allowedDecisions: ["allow-once", "deny"],
});
expect(getResultText(result)).toContain("Reply with: /approve ");
expect(getResultText(result)).toContain("allow-once|deny");
expect(getResultText(result)).not.toContain("allow-once|allow-always|deny");
expect(getResultText(result)).toContain("Allow Always is unavailable");
});
expect(getResultText(durable)).toContain("Reply with: /approve ");
expect(getResultText(durable)).toContain("allow-once|deny");
expect(getResultText(durable)).not.toContain("allow-once|allow-always|deny");
expect(getResultText(durable)).toContain("Allow Always is unavailable");
it("keeps ask=always prompts for static allowlist entries without allow-always trust", async () => {
const result = await expectGatewayAskAlwaysPrompt({
const staticAllowlist = await expectGatewayAskAlwaysPrompt({
turnId: "call-static-allowlist-still-prompts",
allowlist: [{ pattern: process.execPath }],
});
expect(result.details.status).toBe("approval-pending");
expect(staticAllowlist.details.status).toBe("approval-pending");
});
it("reuses gateway allow-always approvals for repeated exact commands", async () => {