fix(agents): map container workdir paths in workspace guard
Co-authored-by: Explorer1092 <32663226+Explorer1092@users.noreply.github.com>
This commit is contained in:
@@ -33,6 +33,7 @@ Docs: https://docs.openclaw.ai
|
||||
### Fixes
|
||||
|
||||
- Plugins/Media sandbox: propagate trusted `mediaLocalRoots` through plugin action dispatch (including Discord/Telegram action adapters) so plugin send paths enforce the same agent-scoped local-media sandbox roots as core outbound sends. (#20258, #22718)
|
||||
- Agents/Workspace guard: map sandbox container-workdir file-tool paths (for example `/workspace/...` and `file:///workspace/...`) to host workspace roots before workspace-only validation, preventing false `Path escapes sandbox root` rejections for sandbox file tools. (#9560)
|
||||
- Slack/Threading: sessions: keep parent-session forking and thread-history context active beyond first turn by removing first-turn-only gates in session init, thread-history fetch, and reply prompt context injection. (#23843, #23090) Thanks @vincentkoc and @Taskle.
|
||||
- Slack/Threading: respect `replyToMode` when Slack auto-populates top-level `thread_ts`, and ignore inline `replyToId` directive tags when `replyToMode` is `off` so thread forcing stays disabled unless explicitly configured. (#23839, #23320, #23513) Thanks @vincentkoc and @dorukardahan.
|
||||
- Slack/Extension: forward `message read` `threadId` to `readMessages` and use delivery-context `threadId` as outbound `thread_ts` fallback so extension replies/reads stay in the correct Slack thread. (#22216, #22485, #23836) Thanks @vincentkoc, @lan17 and @dorukardahan.
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import type { AgentToolResult } from "@mariozechner/pi-agent-core";
|
||||
import { createEditTool, createReadTool, createWriteTool } from "@mariozechner/pi-coding-agent";
|
||||
import { detectMime } from "../media/mime.js";
|
||||
@@ -548,6 +550,57 @@ export function wrapToolParamNormalization(
|
||||
}
|
||||
|
||||
export function wrapToolWorkspaceRootGuard(tool: AnyAgentTool, root: string): AnyAgentTool {
|
||||
return wrapToolWorkspaceRootGuardWithOptions(tool, root);
|
||||
}
|
||||
|
||||
function mapContainerPathToWorkspaceRoot(params: {
|
||||
filePath: string;
|
||||
root: string;
|
||||
containerWorkdir?: string;
|
||||
}): string {
|
||||
const containerWorkdir = params.containerWorkdir?.trim();
|
||||
if (!containerWorkdir) {
|
||||
return params.filePath;
|
||||
}
|
||||
const normalizedWorkdir = containerWorkdir.replace(/\\/g, "/").replace(/\/+$/, "");
|
||||
if (!normalizedWorkdir.startsWith("/")) {
|
||||
return params.filePath;
|
||||
}
|
||||
if (!normalizedWorkdir) {
|
||||
return params.filePath;
|
||||
}
|
||||
|
||||
let candidate = params.filePath;
|
||||
if (/^file:\/\//i.test(candidate)) {
|
||||
try {
|
||||
candidate = fileURLToPath(candidate);
|
||||
} catch {
|
||||
return params.filePath;
|
||||
}
|
||||
}
|
||||
|
||||
const normalizedCandidate = candidate.replace(/\\/g, "/");
|
||||
if (normalizedCandidate === normalizedWorkdir) {
|
||||
return path.resolve(params.root);
|
||||
}
|
||||
const prefix = `${normalizedWorkdir}/`;
|
||||
if (!normalizedCandidate.startsWith(prefix)) {
|
||||
return candidate;
|
||||
}
|
||||
const relative = normalizedCandidate.slice(prefix.length);
|
||||
if (!relative) {
|
||||
return path.resolve(params.root);
|
||||
}
|
||||
return path.resolve(params.root, ...relative.split("/").filter(Boolean));
|
||||
}
|
||||
|
||||
export function wrapToolWorkspaceRootGuardWithOptions(
|
||||
tool: AnyAgentTool,
|
||||
root: string,
|
||||
options?: {
|
||||
containerWorkdir?: string;
|
||||
},
|
||||
): AnyAgentTool {
|
||||
return {
|
||||
...tool,
|
||||
execute: async (toolCallId, args, signal, onUpdate) => {
|
||||
@@ -557,7 +610,12 @@ export function wrapToolWorkspaceRootGuard(tool: AnyAgentTool, root: string): An
|
||||
(args && typeof args === "object" ? (args as Record<string, unknown>) : undefined);
|
||||
const filePath = record?.path;
|
||||
if (typeof filePath === "string" && filePath.trim()) {
|
||||
await assertSandboxPath({ filePath, cwd: root, root });
|
||||
const sandboxPath = mapContainerPathToWorkspaceRoot({
|
||||
filePath,
|
||||
root,
|
||||
containerWorkdir: options?.containerWorkdir,
|
||||
});
|
||||
await assertSandboxPath({ filePath: sandboxPath, cwd: root, root });
|
||||
}
|
||||
return tool.execute(toolCallId, normalized ?? args, signal, onUpdate);
|
||||
},
|
||||
|
||||
76
src/agents/pi-tools.read.workspace-root-guard.test.ts
Normal file
76
src/agents/pi-tools.read.workspace-root-guard.test.ts
Normal file
@@ -0,0 +1,76 @@
|
||||
import path from "node:path";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { wrapToolWorkspaceRootGuardWithOptions } from "./pi-tools.read.js";
|
||||
import type { AnyAgentTool } from "./pi-tools.types.js";
|
||||
|
||||
const assertSandboxPath = vi.fn(async () => ({ resolved: "/tmp/root", relative: "" }));
|
||||
|
||||
vi.mock("./sandbox-paths.js", () => ({
|
||||
assertSandboxPath: (...args: unknown[]) => assertSandboxPath(...args),
|
||||
}));
|
||||
|
||||
function createToolHarness() {
|
||||
const execute = vi.fn(async () => ({
|
||||
content: [{ type: "text", text: "ok" }],
|
||||
}));
|
||||
const tool = {
|
||||
name: "read",
|
||||
description: "test tool",
|
||||
inputSchema: { type: "object", properties: {} },
|
||||
execute,
|
||||
} as unknown as AnyAgentTool;
|
||||
return { execute, tool };
|
||||
}
|
||||
|
||||
describe("wrapToolWorkspaceRootGuardWithOptions", () => {
|
||||
const root = "/tmp/root";
|
||||
|
||||
beforeEach(() => {
|
||||
assertSandboxPath.mockClear();
|
||||
});
|
||||
|
||||
it("maps container workspace paths to host workspace root", async () => {
|
||||
const { tool } = createToolHarness();
|
||||
const wrapped = wrapToolWorkspaceRootGuardWithOptions(tool, root, {
|
||||
containerWorkdir: "/workspace",
|
||||
});
|
||||
|
||||
await wrapped.execute("tc1", { path: "/workspace/docs/readme.md" });
|
||||
|
||||
expect(assertSandboxPath).toHaveBeenCalledWith({
|
||||
filePath: path.resolve(root, "docs", "readme.md"),
|
||||
cwd: root,
|
||||
root,
|
||||
});
|
||||
});
|
||||
|
||||
it("maps file:// container workspace paths to host workspace root", async () => {
|
||||
const { tool } = createToolHarness();
|
||||
const wrapped = wrapToolWorkspaceRootGuardWithOptions(tool, root, {
|
||||
containerWorkdir: "/workspace",
|
||||
});
|
||||
|
||||
await wrapped.execute("tc2", { path: "file:///workspace/docs/readme.md" });
|
||||
|
||||
expect(assertSandboxPath).toHaveBeenCalledWith({
|
||||
filePath: path.resolve(root, "docs", "readme.md"),
|
||||
cwd: root,
|
||||
root,
|
||||
});
|
||||
});
|
||||
|
||||
it("does not remap absolute paths outside the configured container workdir", async () => {
|
||||
const { tool } = createToolHarness();
|
||||
const wrapped = wrapToolWorkspaceRootGuardWithOptions(tool, root, {
|
||||
containerWorkdir: "/workspace",
|
||||
});
|
||||
|
||||
await wrapped.execute("tc3", { path: "/workspace-two/secret.txt" });
|
||||
|
||||
expect(assertSandboxPath).toHaveBeenCalledWith({
|
||||
filePath: "/workspace-two/secret.txt",
|
||||
cwd: root,
|
||||
root,
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -42,6 +42,7 @@ import {
|
||||
normalizeToolParams,
|
||||
patchToolSchemaForClaudeCompatibility,
|
||||
wrapToolWorkspaceRootGuard,
|
||||
wrapToolWorkspaceRootGuardWithOptions,
|
||||
wrapToolParamNormalization,
|
||||
} from "./pi-tools.read.js";
|
||||
import { cleanToolSchemaForGemini, normalizeToolParameters } from "./pi-tools.schema.js";
|
||||
@@ -317,7 +318,13 @@ export function createOpenClawCodingTools(options?: {
|
||||
modelContextWindowTokens: options?.modelContextWindowTokens,
|
||||
imageSanitization,
|
||||
});
|
||||
return [workspaceOnly ? wrapToolWorkspaceRootGuard(sandboxed, sandboxRoot) : sandboxed];
|
||||
return [
|
||||
workspaceOnly
|
||||
? wrapToolWorkspaceRootGuardWithOptions(sandboxed, sandboxRoot, {
|
||||
containerWorkdir: sandbox.containerWorkdir,
|
||||
})
|
||||
: sandboxed,
|
||||
];
|
||||
}
|
||||
const freshReadTool = createReadTool(workspaceRoot);
|
||||
const wrapped = createOpenClawReadTool(freshReadTool, {
|
||||
@@ -410,15 +417,21 @@ export function createOpenClawCodingTools(options?: {
|
||||
? allowWorkspaceWrites
|
||||
? [
|
||||
workspaceOnly
|
||||
? wrapToolWorkspaceRootGuard(
|
||||
? wrapToolWorkspaceRootGuardWithOptions(
|
||||
createSandboxedEditTool({ root: sandboxRoot, bridge: sandboxFsBridge! }),
|
||||
sandboxRoot,
|
||||
{
|
||||
containerWorkdir: sandbox.containerWorkdir,
|
||||
},
|
||||
)
|
||||
: createSandboxedEditTool({ root: sandboxRoot, bridge: sandboxFsBridge! }),
|
||||
workspaceOnly
|
||||
? wrapToolWorkspaceRootGuard(
|
||||
? wrapToolWorkspaceRootGuardWithOptions(
|
||||
createSandboxedWriteTool({ root: sandboxRoot, bridge: sandboxFsBridge! }),
|
||||
sandboxRoot,
|
||||
{
|
||||
containerWorkdir: sandbox.containerWorkdir,
|
||||
},
|
||||
)
|
||||
: createSandboxedWriteTool({ root: sandboxRoot, bridge: sandboxFsBridge! }),
|
||||
]
|
||||
|
||||
Reference in New Issue
Block a user