security(message-tool): validate filePath/path against sandbox root (#6398)
* security(message-tool): validate filePath/path against sandbox root * style: translate Polish comments to English for consistency
This commit is contained in:
@@ -92,6 +92,7 @@ export function createOpenClawTools(options?: {
|
|||||||
currentThreadTs: options?.currentThreadTs,
|
currentThreadTs: options?.currentThreadTs,
|
||||||
replyToMode: options?.replyToMode,
|
replyToMode: options?.replyToMode,
|
||||||
hasRepliedRef: options?.hasRepliedRef,
|
hasRepliedRef: options?.hasRepliedRef,
|
||||||
|
sandboxRoot: options?.sandboxRoot,
|
||||||
}),
|
}),
|
||||||
createTtsTool({
|
createTtsTool({
|
||||||
agentChannel: options?.agentChannel,
|
agentChannel: options?.agentChannel,
|
||||||
|
|||||||
@@ -1,3 +1,6 @@
|
|||||||
|
import fs from "node:fs/promises";
|
||||||
|
import os from "node:os";
|
||||||
|
import path from "node:path";
|
||||||
import { describe, expect, it, vi } from "vitest";
|
import { describe, expect, it, vi } from "vitest";
|
||||||
import type { ChannelPlugin } from "../../channels/plugins/types.js";
|
import type { ChannelPlugin } from "../../channels/plugins/types.js";
|
||||||
import type { MessageActionRunResult } from "../../infra/outbound/message-action-runner.js";
|
import type { MessageActionRunResult } from "../../infra/outbound/message-action-runner.js";
|
||||||
@@ -161,3 +164,106 @@ describe("message tool description", () => {
|
|||||||
setActivePluginRegistry(createTestRegistry([]));
|
setActivePluginRegistry(createTestRegistry([]));
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("message tool sandbox path validation", () => {
|
||||||
|
it("rejects filePath that escapes sandbox root", async () => {
|
||||||
|
const sandboxDir = await fs.mkdtemp(path.join(os.tmpdir(), "msg-sandbox-"));
|
||||||
|
try {
|
||||||
|
const tool = createMessageTool({
|
||||||
|
config: {} as never,
|
||||||
|
sandboxRoot: sandboxDir,
|
||||||
|
});
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
tool.execute("1", {
|
||||||
|
action: "send",
|
||||||
|
target: "telegram:123",
|
||||||
|
filePath: "/etc/passwd",
|
||||||
|
message: "",
|
||||||
|
}),
|
||||||
|
).rejects.toThrow(/sandbox/i);
|
||||||
|
} finally {
|
||||||
|
await fs.rm(sandboxDir, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects path param with traversal sequence", async () => {
|
||||||
|
const sandboxDir = await fs.mkdtemp(path.join(os.tmpdir(), "msg-sandbox-"));
|
||||||
|
try {
|
||||||
|
const tool = createMessageTool({
|
||||||
|
config: {} as never,
|
||||||
|
sandboxRoot: sandboxDir,
|
||||||
|
});
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
tool.execute("1", {
|
||||||
|
action: "send",
|
||||||
|
target: "telegram:123",
|
||||||
|
path: "../../../etc/shadow",
|
||||||
|
message: "",
|
||||||
|
}),
|
||||||
|
).rejects.toThrow(/sandbox/i);
|
||||||
|
} finally {
|
||||||
|
await fs.rm(sandboxDir, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("allows filePath inside sandbox root", async () => {
|
||||||
|
mocks.runMessageAction.mockClear();
|
||||||
|
mocks.runMessageAction.mockResolvedValue({
|
||||||
|
kind: "send",
|
||||||
|
action: "send",
|
||||||
|
channel: "telegram",
|
||||||
|
to: "telegram:123",
|
||||||
|
handledBy: "plugin",
|
||||||
|
payload: {},
|
||||||
|
dryRun: true,
|
||||||
|
} satisfies MessageActionRunResult);
|
||||||
|
|
||||||
|
const sandboxDir = await fs.mkdtemp(path.join(os.tmpdir(), "msg-sandbox-"));
|
||||||
|
try {
|
||||||
|
const tool = createMessageTool({
|
||||||
|
config: {} as never,
|
||||||
|
sandboxRoot: sandboxDir,
|
||||||
|
});
|
||||||
|
|
||||||
|
await tool.execute("1", {
|
||||||
|
action: "send",
|
||||||
|
target: "telegram:123",
|
||||||
|
filePath: "./data/file.txt",
|
||||||
|
message: "",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(mocks.runMessageAction).toHaveBeenCalledTimes(1);
|
||||||
|
} finally {
|
||||||
|
await fs.rm(sandboxDir, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("skips validation when no sandboxRoot is set", async () => {
|
||||||
|
mocks.runMessageAction.mockClear();
|
||||||
|
mocks.runMessageAction.mockResolvedValue({
|
||||||
|
kind: "send",
|
||||||
|
action: "send",
|
||||||
|
channel: "telegram",
|
||||||
|
to: "telegram:123",
|
||||||
|
handledBy: "plugin",
|
||||||
|
payload: {},
|
||||||
|
dryRun: true,
|
||||||
|
} satisfies MessageActionRunResult);
|
||||||
|
|
||||||
|
const tool = createMessageTool({
|
||||||
|
config: {} as never,
|
||||||
|
});
|
||||||
|
|
||||||
|
await tool.execute("1", {
|
||||||
|
action: "send",
|
||||||
|
target: "telegram:123",
|
||||||
|
filePath: "/etc/passwd",
|
||||||
|
message: "",
|
||||||
|
});
|
||||||
|
|
||||||
|
// Without sandboxRoot the validation is skipped — unsandboxed sessions work normally.
|
||||||
|
expect(mocks.runMessageAction).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ import { normalizeAccountId } from "../../routing/session-key.js";
|
|||||||
import { normalizeMessageChannel } from "../../utils/message-channel.js";
|
import { normalizeMessageChannel } from "../../utils/message-channel.js";
|
||||||
import { resolveSessionAgentId } from "../agent-scope.js";
|
import { resolveSessionAgentId } from "../agent-scope.js";
|
||||||
import { listChannelSupportedActions } from "../channel-tools.js";
|
import { listChannelSupportedActions } from "../channel-tools.js";
|
||||||
|
import { assertSandboxPath } from "../sandbox-paths.js";
|
||||||
import { channelTargetSchema, channelTargetsSchema, stringEnum } from "../schema/typebox.js";
|
import { channelTargetSchema, channelTargetsSchema, stringEnum } from "../schema/typebox.js";
|
||||||
import { jsonResult, readNumberParam, readStringParam } from "./common.js";
|
import { jsonResult, readNumberParam, readStringParam } from "./common.js";
|
||||||
|
|
||||||
@@ -252,6 +253,7 @@ type MessageToolOptions = {
|
|||||||
currentThreadTs?: string;
|
currentThreadTs?: string;
|
||||||
replyToMode?: "off" | "first" | "all";
|
replyToMode?: "off" | "first" | "all";
|
||||||
hasRepliedRef?: { value: boolean };
|
hasRepliedRef?: { value: boolean };
|
||||||
|
sandboxRoot?: string;
|
||||||
};
|
};
|
||||||
|
|
||||||
function buildMessageToolSchema(cfg: OpenClawConfig) {
|
function buildMessageToolSchema(cfg: OpenClawConfig) {
|
||||||
@@ -362,6 +364,17 @@ export function createMessageTool(options?: MessageToolOptions): AnyAgentTool {
|
|||||||
required: true,
|
required: true,
|
||||||
}) as ChannelMessageActionName;
|
}) as ChannelMessageActionName;
|
||||||
|
|
||||||
|
// Validate file paths against sandbox root to prevent host file access.
|
||||||
|
const sandboxRoot = options?.sandboxRoot;
|
||||||
|
if (sandboxRoot) {
|
||||||
|
for (const key of ["filePath", "path"] as const) {
|
||||||
|
const raw = readStringParam(params, key, { trim: false });
|
||||||
|
if (raw) {
|
||||||
|
await assertSandboxPath({ filePath: raw, cwd: sandboxRoot, root: sandboxRoot });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
const accountId = readStringParam(params, "accountId") ?? agentAccountId;
|
const accountId = readStringParam(params, "accountId") ?? agentAccountId;
|
||||||
if (accountId) {
|
if (accountId) {
|
||||||
params.accountId = accountId;
|
params.accountId = accountId;
|
||||||
|
|||||||
Reference in New Issue
Block a user