mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-22 13:28:13 +00:00
fix: protect bootstrap files during memory flush (#38574)
Merged via squash.
Prepared head SHA: a0b9a02e2e
Co-authored-by: frankekn <4488090+frankekn@users.noreply.github.com>
Co-authored-by: frankekn <4488090+frankekn@users.noreply.github.com>
Reviewed-by: @frankekn
This commit is contained in:
@@ -870,6 +870,8 @@ export async function runEmbeddedAttempt(
|
||||
agentDir,
|
||||
workspaceDir: effectiveWorkspace,
|
||||
config: params.config,
|
||||
trigger: params.trigger,
|
||||
memoryFlushWritePath: params.memoryFlushWritePath,
|
||||
abortSignal: runAbortController.signal,
|
||||
modelProvider: params.model.provider,
|
||||
modelId: params.modelId,
|
||||
|
||||
@@ -29,6 +29,8 @@ export type RunEmbeddedPiAgentParams = {
|
||||
agentAccountId?: string;
|
||||
/** What initiated this agent run: "user", "heartbeat", "cron", or "memory". */
|
||||
trigger?: string;
|
||||
/** Relative workspace path that memory-triggered writes are allowed to append to. */
|
||||
memoryFlushWritePath?: string;
|
||||
/** Delivery target (e.g. telegram:group:123:topic:456) for topic/thread routing. */
|
||||
messageTo?: string;
|
||||
/** Thread/topic identifier for routing replies to the originating thread. */
|
||||
|
||||
@@ -4,6 +4,7 @@ import { fileURLToPath } from "node:url";
|
||||
import type { AgentToolResult } from "@mariozechner/pi-agent-core";
|
||||
import { createEditTool, createReadTool, createWriteTool } from "@mariozechner/pi-coding-agent";
|
||||
import {
|
||||
appendFileWithinRoot,
|
||||
SafeOpenError,
|
||||
openFileWithinRoot,
|
||||
readFileWithinRoot,
|
||||
@@ -406,6 +407,161 @@ function mapContainerPathToWorkspaceRoot(params: {
|
||||
return path.resolve(params.root, ...relative.split("/").filter(Boolean));
|
||||
}
|
||||
|
||||
export function resolveToolPathAgainstWorkspaceRoot(params: {
|
||||
filePath: string;
|
||||
root: string;
|
||||
containerWorkdir?: string;
|
||||
}): string {
|
||||
const mapped = mapContainerPathToWorkspaceRoot(params);
|
||||
const candidate = mapped.startsWith("@") ? mapped.slice(1) : mapped;
|
||||
return path.isAbsolute(candidate)
|
||||
? path.resolve(candidate)
|
||||
: path.resolve(params.root, candidate || ".");
|
||||
}
|
||||
|
||||
type MemoryFlushAppendOnlyWriteOptions = {
|
||||
root: string;
|
||||
relativePath: string;
|
||||
containerWorkdir?: string;
|
||||
sandbox?: {
|
||||
root: string;
|
||||
bridge: SandboxFsBridge;
|
||||
};
|
||||
};
|
||||
|
||||
async function readOptionalUtf8File(params: {
|
||||
absolutePath: string;
|
||||
relativePath: string;
|
||||
sandbox?: MemoryFlushAppendOnlyWriteOptions["sandbox"];
|
||||
signal?: AbortSignal;
|
||||
}): Promise<string> {
|
||||
try {
|
||||
if (params.sandbox) {
|
||||
const stat = await params.sandbox.bridge.stat({
|
||||
filePath: params.relativePath,
|
||||
cwd: params.sandbox.root,
|
||||
signal: params.signal,
|
||||
});
|
||||
if (!stat) {
|
||||
return "";
|
||||
}
|
||||
const buffer = await params.sandbox.bridge.readFile({
|
||||
filePath: params.relativePath,
|
||||
cwd: params.sandbox.root,
|
||||
signal: params.signal,
|
||||
});
|
||||
return buffer.toString("utf-8");
|
||||
}
|
||||
return await fs.readFile(params.absolutePath, "utf-8");
|
||||
} catch (error) {
|
||||
if ((error as NodeJS.ErrnoException | undefined)?.code === "ENOENT") {
|
||||
return "";
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
async function appendMemoryFlushContent(params: {
|
||||
absolutePath: string;
|
||||
root: string;
|
||||
relativePath: string;
|
||||
content: string;
|
||||
sandbox?: MemoryFlushAppendOnlyWriteOptions["sandbox"];
|
||||
signal?: AbortSignal;
|
||||
}) {
|
||||
if (!params.sandbox) {
|
||||
await appendFileWithinRoot({
|
||||
rootDir: params.root,
|
||||
relativePath: params.relativePath,
|
||||
data: params.content,
|
||||
mkdir: true,
|
||||
prependNewlineIfNeeded: true,
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const existing = await readOptionalUtf8File({
|
||||
absolutePath: params.absolutePath,
|
||||
relativePath: params.relativePath,
|
||||
sandbox: params.sandbox,
|
||||
signal: params.signal,
|
||||
});
|
||||
const separator =
|
||||
existing.length > 0 && !existing.endsWith("\n") && !params.content.startsWith("\n") ? "\n" : "";
|
||||
const next = `${existing}${separator}${params.content}`;
|
||||
if (params.sandbox) {
|
||||
const parent = path.posix.dirname(params.relativePath);
|
||||
if (parent && parent !== ".") {
|
||||
await params.sandbox.bridge.mkdirp({
|
||||
filePath: parent,
|
||||
cwd: params.sandbox.root,
|
||||
signal: params.signal,
|
||||
});
|
||||
}
|
||||
await params.sandbox.bridge.writeFile({
|
||||
filePath: params.relativePath,
|
||||
cwd: params.sandbox.root,
|
||||
data: next,
|
||||
mkdir: true,
|
||||
signal: params.signal,
|
||||
});
|
||||
return;
|
||||
}
|
||||
await fs.mkdir(path.dirname(params.absolutePath), { recursive: true });
|
||||
await fs.writeFile(params.absolutePath, next, "utf-8");
|
||||
}
|
||||
|
||||
export function wrapToolMemoryFlushAppendOnlyWrite(
|
||||
tool: AnyAgentTool,
|
||||
options: MemoryFlushAppendOnlyWriteOptions,
|
||||
): AnyAgentTool {
|
||||
const allowedAbsolutePath = path.resolve(options.root, options.relativePath);
|
||||
return {
|
||||
...tool,
|
||||
description: `${tool.description} During memory flush, this tool may only append to ${options.relativePath}.`,
|
||||
execute: async (toolCallId, args, signal, onUpdate) => {
|
||||
const normalized = normalizeToolParams(args);
|
||||
const record =
|
||||
normalized ??
|
||||
(args && typeof args === "object" ? (args as Record<string, unknown>) : undefined);
|
||||
assertRequiredParams(record, CLAUDE_PARAM_GROUPS.write, tool.name);
|
||||
const filePath =
|
||||
typeof record?.path === "string" && record.path.trim() ? record.path : undefined;
|
||||
const content = typeof record?.content === "string" ? record.content : undefined;
|
||||
if (!filePath || content === undefined) {
|
||||
return tool.execute(toolCallId, normalized ?? args, signal, onUpdate);
|
||||
}
|
||||
|
||||
const resolvedPath = resolveToolPathAgainstWorkspaceRoot({
|
||||
filePath,
|
||||
root: options.root,
|
||||
containerWorkdir: options.containerWorkdir,
|
||||
});
|
||||
if (resolvedPath !== allowedAbsolutePath) {
|
||||
throw new Error(
|
||||
`Memory flush writes are restricted to ${options.relativePath}; use that path only.`,
|
||||
);
|
||||
}
|
||||
|
||||
await appendMemoryFlushContent({
|
||||
absolutePath: allowedAbsolutePath,
|
||||
root: options.root,
|
||||
relativePath: options.relativePath,
|
||||
content,
|
||||
sandbox: options.sandbox,
|
||||
signal,
|
||||
});
|
||||
return {
|
||||
content: [{ type: "text", text: `Appended content to ${options.relativePath}.` }],
|
||||
details: {
|
||||
path: options.relativePath,
|
||||
appendOnly: true,
|
||||
},
|
||||
};
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
export function wrapToolWorkspaceRootGuardWithOptions(
|
||||
tool: AnyAgentTool,
|
||||
root: string,
|
||||
|
||||
@@ -36,6 +36,7 @@ import {
|
||||
createSandboxedWriteTool,
|
||||
normalizeToolParams,
|
||||
patchToolSchemaForClaudeCompatibility,
|
||||
wrapToolMemoryFlushAppendOnlyWrite,
|
||||
wrapToolWorkspaceRootGuard,
|
||||
wrapToolWorkspaceRootGuardWithOptions,
|
||||
wrapToolParamNormalization,
|
||||
@@ -67,6 +68,7 @@ const TOOL_DENY_BY_MESSAGE_PROVIDER: Readonly<Record<string, readonly string[]>>
|
||||
voice: ["tts"],
|
||||
};
|
||||
const TOOL_DENY_FOR_XAI_PROVIDERS = new Set(["web_search"]);
|
||||
const MEMORY_FLUSH_ALLOWED_TOOL_NAMES = new Set(["read", "write"]);
|
||||
|
||||
function normalizeMessageProvider(messageProvider?: string): string | undefined {
|
||||
const normalized = messageProvider?.trim().toLowerCase();
|
||||
@@ -207,6 +209,10 @@ export function createOpenClawCodingTools(options?: {
|
||||
sessionId?: string;
|
||||
/** Stable run identifier for this agent invocation. */
|
||||
runId?: string;
|
||||
/** What initiated this run (for trigger-specific tool restrictions). */
|
||||
trigger?: string;
|
||||
/** Relative workspace path that memory-triggered writes may append to. */
|
||||
memoryFlushWritePath?: string;
|
||||
agentDir?: string;
|
||||
workspaceDir?: string;
|
||||
config?: OpenClawConfig;
|
||||
@@ -258,6 +264,11 @@ export function createOpenClawCodingTools(options?: {
|
||||
}): AnyAgentTool[] {
|
||||
const execToolName = "exec";
|
||||
const sandbox = options?.sandbox?.enabled ? options.sandbox : undefined;
|
||||
const isMemoryFlushRun = options?.trigger === "memory";
|
||||
if (isMemoryFlushRun && !options?.memoryFlushWritePath) {
|
||||
throw new Error("memoryFlushWritePath required for memory-triggered tool runs");
|
||||
}
|
||||
const memoryFlushWritePath = isMemoryFlushRun ? options.memoryFlushWritePath : undefined;
|
||||
const {
|
||||
agentId,
|
||||
globalPolicy,
|
||||
@@ -322,7 +333,7 @@ export function createOpenClawCodingTools(options?: {
|
||||
const execConfig = resolveExecConfig({ cfg: options?.config, agentId });
|
||||
const fsConfig = resolveToolFsConfig({ cfg: options?.config, agentId });
|
||||
const fsPolicy = createToolFsPolicy({
|
||||
workspaceOnly: fsConfig.workspaceOnly,
|
||||
workspaceOnly: isMemoryFlushRun || fsConfig.workspaceOnly,
|
||||
});
|
||||
const sandboxRoot = sandbox?.workspaceDir;
|
||||
const sandboxFsBridge = sandbox?.fsBridge;
|
||||
@@ -515,7 +526,32 @@ export function createOpenClawCodingTools(options?: {
|
||||
sessionId: options?.sessionId,
|
||||
}),
|
||||
];
|
||||
const toolsForMessageProvider = applyMessageProviderToolPolicy(tools, options?.messageProvider);
|
||||
const toolsForMemoryFlush =
|
||||
isMemoryFlushRun && memoryFlushWritePath
|
||||
? tools.flatMap((tool) => {
|
||||
if (!MEMORY_FLUSH_ALLOWED_TOOL_NAMES.has(tool.name)) {
|
||||
return [];
|
||||
}
|
||||
if (tool.name === "write") {
|
||||
return [
|
||||
wrapToolMemoryFlushAppendOnlyWrite(tool, {
|
||||
root: sandboxRoot ?? workspaceRoot,
|
||||
relativePath: memoryFlushWritePath,
|
||||
containerWorkdir: sandbox?.containerWorkdir,
|
||||
sandbox:
|
||||
sandboxRoot && sandboxFsBridge
|
||||
? { root: sandboxRoot, bridge: sandboxFsBridge }
|
||||
: undefined,
|
||||
}),
|
||||
];
|
||||
}
|
||||
return [tool];
|
||||
})
|
||||
: tools;
|
||||
const toolsForMessageProvider = applyMessageProviderToolPolicy(
|
||||
toolsForMemoryFlush,
|
||||
options?.messageProvider,
|
||||
);
|
||||
const toolsForModelProvider = applyModelProviderToolPolicy(toolsForMessageProvider, {
|
||||
modelProvider: options?.modelProvider,
|
||||
modelId: options?.modelId,
|
||||
|
||||
@@ -1,7 +1,13 @@
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
vi.mock("@mariozechner/pi-ai/oauth", () => ({
|
||||
getOAuthApiKey: () => undefined,
|
||||
getOAuthProviders: () => [],
|
||||
}));
|
||||
|
||||
import { createOpenClawCodingTools } from "./pi-tools.js";
|
||||
|
||||
describe("FS tools with workspaceOnly=false", () => {
|
||||
@@ -181,4 +187,50 @@ describe("FS tools with workspaceOnly=false", () => {
|
||||
}),
|
||||
).rejects.toThrow(/Path escapes (workspace|sandbox) root/);
|
||||
});
|
||||
|
||||
it("restricts memory-triggered writes to append-only canonical memory files", async () => {
|
||||
const allowedRelativePath = "memory/2026-03-07.md";
|
||||
const allowedAbsolutePath = path.join(workspaceDir, allowedRelativePath);
|
||||
await fs.mkdir(path.dirname(allowedAbsolutePath), { recursive: true });
|
||||
await fs.writeFile(allowedAbsolutePath, "seed");
|
||||
|
||||
const tools = createOpenClawCodingTools({
|
||||
workspaceDir,
|
||||
trigger: "memory",
|
||||
memoryFlushWritePath: allowedRelativePath,
|
||||
config: {
|
||||
tools: {
|
||||
exec: {
|
||||
applyPatch: {
|
||||
enabled: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
modelProvider: "openai",
|
||||
modelId: "gpt-5",
|
||||
});
|
||||
|
||||
const writeTool = tools.find((tool) => tool.name === "write");
|
||||
expect(writeTool).toBeDefined();
|
||||
expect(tools.map((tool) => tool.name).toSorted()).toEqual(["read", "write"]);
|
||||
|
||||
await expect(
|
||||
writeTool!.execute("test-call-memory-deny", {
|
||||
path: outsideFile,
|
||||
content: "should not write here",
|
||||
}),
|
||||
).rejects.toThrow(/Memory flush writes are restricted to memory\/2026-03-07\.md/);
|
||||
|
||||
const result = await writeTool!.execute("test-call-memory-append", {
|
||||
path: allowedRelativePath,
|
||||
content: "new note",
|
||||
});
|
||||
expect(hasToolError(result)).toBe(false);
|
||||
expect(result.content).toContainEqual({
|
||||
type: "text",
|
||||
text: "Appended content to memory/2026-03-07.md.",
|
||||
});
|
||||
await expect(fs.readFile(allowedAbsolutePath, "utf-8")).resolves.toBe("seed\nnew note");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user