vultr/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-03-29 19:01:44 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: close sandbox media root bypass for mediaUrl/fileUrl aliases (#54034)

Browse Source 
* fix: close sandbox media root bypass for mediaUrl/fileUrl aliases

* Address Greptile feedback

* Fix windows test case failure
This commit is contained in:
Devin Robison
2026-03-24 16:28:53 -07:00
committed by GitHub
parent 907b5254f6
commit 1d7cb6fc03
5 changed files with 282 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

81
src/infra/outbound/message-action-params.test.ts
View File

@@ -185,6 +185,87 @@ describe("message action media helpers", () => {
}
});
maybeIt("normalizes mediaUrl and fileUrl sandbox media params", async () => {
const sandboxRoot = await fs.mkdtemp(path.join(os.tmpdir(), "msg-params-alias-"));
try {
const args: Record<string, unknown> = {
mediaUrl: " file:///workspace/assets/photo.png ",
fileUrl: "/workspace/docs/report.pdf",
};
await normalizeSandboxMediaParams({
args,
mediaPolicy: {
mode: "sandbox",
sandboxRoot: ` ${sandboxRoot} `,
},
});
expect(args).toMatchObject({
mediaUrl: path.join(sandboxRoot, "assets", "photo.png"),
fileUrl: path.join(sandboxRoot, "docs", "report.pdf"),
});
} finally {
await fs.rm(sandboxRoot, { recursive: true, force: true });
}
});
maybeIt(
"keeps remote HTTP mediaUrl and fileUrl aliases unchanged under sandbox normalization",
async () => {
const sandboxRoot = await fs.mkdtemp(path.join(os.tmpdir(), "msg-params-remote-alias-"));
try {
const args: Record<string, unknown> = {
mediaUrl: "https://example.com/assets/photo.png?sig=1",
fileUrl: "https://example.com/docs/report.pdf?sig=2",
};
await normalizeSandboxMediaParams({
args,
mediaPolicy: {
mode: "sandbox",
sandboxRoot,
},
});
expect(args).toMatchObject({
mediaUrl: "https://example.com/assets/photo.png?sig=1",
fileUrl: "https://example.com/docs/report.pdf?sig=2",
});
} finally {
await fs.rm(sandboxRoot, { recursive: true, force: true });
}
},
);
it("uses mediaUrl and fileUrl aliases when inferring attachment filenames", async () => {
const mediaArgs: Record<string, unknown> = {
mediaUrl: "https://example.com/pic.png",
};
await hydrateAttachmentParamsForAction({
cfg,
channel: "slack",
args: mediaArgs,
action: "sendAttachment",
dryRun: true,
mediaPolicy: { mode: "host" },
});
expect(mediaArgs.filename).toBe("pic.png");
const fileArgs: Record<string, unknown> = {
fileUrl: "https://example.com/docs/report.pdf",
};
await hydrateAttachmentParamsForAction({
cfg,
channel: "slack",
args: fileArgs,
action: "sendAttachment",
dryRun: true,
mediaPolicy: { mode: "host" },
});
expect(fileArgs.filename).toBe("report.pdf");
});
it("falls back to extension-based attachment names for remote-host file URLs", async () => {
const args: Record<string, unknown> = {
media: "file://attacker/share/photo.png",

32
src/infra/outbound/message-action-params.ts
View File

@@ -10,6 +10,27 @@ import { readBooleanParam as readBooleanParamShared } from "../../plugin-sdk/boo
export const readBooleanParam = readBooleanParamShared;
const SANDBOX_MEDIA_PARAM_KEYS = ["media", "path", "filePath", "mediaUrl", "fileUrl"] as const;
function readMediaParam(
args: Record<string, unknown>,
key: (typeof SANDBOX_MEDIA_PARAM_KEYS)[number],
): string | undefined {
return readStringParam(args, key, { trim: false });
}
function readAttachmentMediaHint(args: Record<string, unknown>): string | undefined {
return readMediaParam(args, "media") ?? readMediaParam(args, "mediaUrl");
}
function readAttachmentFileHint(args: Record<string, unknown>): string | undefined {
return (
readMediaParam(args, "path") ??
readMediaParam(args, "filePath") ??
readMediaParam(args, "fileUrl")
);
}
function resolveAttachmentMaxBytes(params: {
cfg: OpenClawConfig;
channel: ChannelId;
@@ -190,9 +211,8 @@ export async function normalizeSandboxMediaParams(params: {
}): Promise<void> {
const sandboxRoot =
params.mediaPolicy.mode === "sandbox" ? params.mediaPolicy.sandboxRoot.trim() : undefined;
const mediaKeys: Array<"media" | "path" | "filePath"> = ["media", "path", "filePath"];
for (const key of mediaKeys) {
const raw = readStringParam(params.args, key, { trim: false });
for (const key of SANDBOX_MEDIA_PARAM_KEYS) {
const raw = readMediaParam(params.args, key);
if (!raw) {
continue;
}
@@ -242,10 +262,8 @@ async function hydrateAttachmentActionPayload(params: {
allowMessageCaptionFallback?: boolean;
mediaPolicy: AttachmentMediaPolicy;
}): Promise<void> {
const mediaHint = readStringParam(params.args, "media", { trim: false });
const fileHint =
readStringParam(params.args, "path", { trim: false }) ??
readStringParam(params.args, "filePath", { trim: false });
const mediaHint = readAttachmentMediaHint(params.args);
const fileHint = readAttachmentFileHint(params.args);
const contentTypeParam =
readStringParam(params.args, "contentType") ?? readStringParam(params.args, "mimeType");

171
src/infra/outbound/message-action-runner.media.test.ts
View File

@@ -56,6 +56,7 @@ const runDrySend = (params: {
async function expectSandboxMediaRewrite(params: {
sandboxDir: string;
media?: string;
mediaField?: "media" | "mediaUrl" | "fileUrl";
message?: string;
expectedRelativePath: string;
}) {
@@ -64,7 +65,11 @@ async function expectSandboxMediaRewrite(params: {
actionParams: {
channel: "slack",
target: "#C12345678",
...(params.media ? { media: params.media } : {}),
...(params.media
? {
[params.mediaField ?? "media"]: params.media,
}
: {}),
...(params.message ? { message: params.message } : {}),
},
sandboxRoot: params.sandboxDir,
@@ -196,6 +201,7 @@ describe("runMessageAction media behavior", () => {
async function expectRejectsLocalAbsolutePathWithoutSandbox(params: {
action: "sendAttachment" | "setGroupIcon";
target: string;
mediaField?: "media" | "mediaUrl" | "fileUrl";
message?: string;
tempPrefix: string;
}) {
@@ -209,7 +215,7 @@ describe("runMessageAction media behavior", () => {
const actionParams: Record<string, unknown> = {
channel: "bluebubbles",
target: params.target,
media: outsidePath,
[params.mediaField ?? "media"]: outsidePath,
};
if (params.message) {
actionParams.message = params.message;
@@ -270,6 +276,24 @@ describe("runMessageAction media behavior", () => {
message: "caption",
expectedPath: path.join("data", "pic.png"),
},
{
name: "sendAttachment mediaUrl rewrite",
action: "sendAttachment" as const,
target: "+15551234567",
mediaField: "mediaUrl" as const,
media: "./data/pic.png",
message: "caption",
expectedPath: path.join("data", "pic.png"),
},
{
name: "sendAttachment fileUrl rewrite",
action: "sendAttachment" as const,
target: "+15551234567",
mediaField: "fileUrl" as const,
media: "/workspace/files/report.pdf",
message: "caption",
expectedPath: path.join("files", "report.pdf"),
},
{
name: "setGroupIcon rewrite",
action: "setGroupIcon" as const,
@@ -286,7 +310,7 @@ describe("runMessageAction media behavior", () => {
params: {
channel: "bluebubbles",
target: testCase.target,
media: testCase.media,
[testCase.mediaField ?? "media"]: testCase.media,
...(testCase.message ? { message: testCase.message } : {}),
},
sandboxRoot: sandboxDir,
@@ -309,6 +333,20 @@ describe("runMessageAction media behavior", () => {
message: "caption",
tempPrefix: "msg-attachment-",
},
{
action: "sendAttachment" as const,
target: "+15551234567",
mediaField: "mediaUrl" as const,
message: "caption",
tempPrefix: "msg-attachment-media-url-",
},
{
action: "sendAttachment" as const,
target: "+15551234567",
mediaField: "fileUrl" as const,
message: "caption",
tempPrefix: "msg-attachment-file-url-",
},
{
action: "setGroupIcon" as const,
target: "group:123",
@@ -337,25 +375,43 @@ describe("runMessageAction media behavior", () => {
setActivePluginRegistry(createTestRegistry([]));
});
it.each(["/etc/passwd", "file:///etc/passwd"])(
"rejects out-of-sandbox media reference: %s",
async (media) => {
await withSandbox(async (sandboxDir) => {
await expect(
runDrySend({
cfg: slackConfig,
actionParams: {
channel: "slack",
target: "#C12345678",
media,
message: "",
},
sandboxRoot: sandboxDir,
}),
).rejects.toThrow(/sandbox/i);
});
it.each([
{
name: "media absolute path",
mediaField: "media" as const,
media: "/etc/passwd",
},
);
{
name: "mediaUrl absolute path",
mediaField: "mediaUrl" as const,
media: "/etc/passwd",
},
{
name: "mediaUrl file URL",
mediaField: "mediaUrl" as const,
media: "file:///etc/passwd",
},
{
name: "fileUrl file URL",
mediaField: "fileUrl" as const,
media: "file:///etc/passwd",
},
])("rejects out-of-sandbox media reference: $name", async ({ mediaField, media }) => {
await withSandbox(async (sandboxDir) => {
await expect(
runDrySend({
cfg: slackConfig,
actionParams: {
channel: "slack",
target: "#C12345678",
[mediaField]: media,
message: "",
},
sandboxRoot: sandboxDir,
}),
).rejects.toThrow(/sandbox/i);
});
});
it("rejects data URLs in media params", async () => {
await expect(
@@ -379,6 +435,20 @@ describe("runMessageAction media behavior", () => {
message: "",
expectedRelativePath: path.join("data", "file.txt"),
},
{
name: "relative mediaUrl path",
mediaField: "mediaUrl" as const,
media: "./data/file.txt",
message: "",
expectedRelativePath: path.join("data", "file.txt"),
},
{
name: "/workspace fileUrl path",
mediaField: "fileUrl" as const,
media: "/workspace/data/file.txt",
message: "",
expectedRelativePath: path.join("data", "file.txt"),
},
{
name: "/workspace media path",
media: "/workspace/data/file.txt",
@@ -390,11 +460,12 @@ describe("runMessageAction media behavior", () => {
message: "Hello\nMEDIA: ./data/note.ogg",
expectedRelativePath: path.join("data", "note.ogg"),
},
]) {
] as const) {
await withSandbox(async (sandboxDir) => {
await expectSandboxMediaRewrite({
sandboxDir,
media: testCase.media,
mediaField: testCase.mediaField,
message: testCase.message,
expectedRelativePath: testCase.expectedRelativePath,
});
@@ -402,6 +473,62 @@ describe("runMessageAction media behavior", () => {
}
});
it("prefers media over mediaUrl when both aliases are present", async () => {
await withSandbox(async (sandboxDir) => {
const result = await runDrySend({
cfg: slackConfig,
actionParams: {
channel: "slack",
target: "#C12345678",
media: "./data/primary.txt",
mediaUrl: "./data/secondary.txt",
message: "",
},
sandboxRoot: sandboxDir,
});
expect(result.kind).toBe("send");
if (result.kind !== "send") {
throw new Error("expected send result");
}
expect(result.sendResult?.mediaUrl).toBe(path.join(sandboxDir, "data", "primary.txt"));
});
});
it.each([
{
name: "mediaUrl",
mediaField: "mediaUrl" as const,
},
{
name: "fileUrl",
mediaField: "fileUrl" as const,
},
])(
"keeps remote HTTP $name aliases unchanged under sandbox validation",
async ({ mediaField }) => {
await withSandbox(async (sandboxDir) => {
const remoteUrl = "https://example.com/files/report.pdf?sig=1";
const result = await runDrySend({
cfg: slackConfig,
actionParams: {
channel: "slack",
target: "#C12345678",
[mediaField]: remoteUrl,
message: "",
},
sandboxRoot: sandboxDir,
});
expect(result.kind).toBe("send");
if (result.kind !== "send") {
throw new Error("expected send result");
}
expect(result.sendResult?.mediaUrl).toBe(remoteUrl);
});
},
);
it("allows media paths under preferred OpenClaw tmp root", async () => {
const tmpRoot = resolvePreferredOpenClawTmpDir();
await fs.mkdir(tmpRoot, { recursive: true });

7
src/infra/outbound/message-action-runner.plugin-dispatch.test.ts
View File

@@ -1,3 +1,4 @@
import path from "node:path";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { jsonResult } from "../../agents/tools/common.js";
import type { ChannelPlugin } from "../../channels/plugins/types.js";
@@ -58,6 +59,7 @@ describe("runMessageAction plugin dispatch", () => {
afterEach(() => {
setActivePluginRegistry(createTestRegistry([]));
vi.clearAllMocks();
vi.unstubAllEnvs();
});
it("dispatches messageId/chatId-based Feishu actions through the shared runner", async () => {
@@ -114,6 +116,10 @@ describe("runMessageAction plugin dispatch", () => {
});
it("routes execution context ids into plugin handleAction", async () => {
const stateDir = path.join("/tmp", "openclaw-plugin-dispatch-media-roots");
const expectedWorkspaceRoot = path.resolve(stateDir, "workspace-alpha");
vi.stubEnv("OPENCLAW_STATE_DIR", stateDir);
await runMessageAction({
cfg: {
channels: {
@@ -148,6 +154,7 @@ describe("runMessageAction plugin dispatch", () => {
sessionKey: "agent:alpha:main",
sessionId: "session-123",
agentId: "alpha",
mediaLocalRoots: expect.arrayContaining([expectedWorkspaceRoot]),
toolContext: expect.objectContaining({
currentChannelId: "chat:oc_123",
currentThreadTs: "thread-456",

22
src/infra/outbound/message-action-runner.ts
View File

@@ -258,6 +258,7 @@ type ResolvedActionContext = {
cfg: OpenClawConfig;
params: Record<string, unknown>;
channel: ChannelId;
mediaLocalRoots: readonly string[];
accountId?: string | null;
dryRun: boolean;
gateway?: MessageActionRunnerGateway;
@@ -382,8 +383,10 @@ async function handleSendAction(ctx: ResolvedActionContext): Promise<MessageActi
// Support media, path, and filePath parameters for attachments
const mediaHint =
readStringParam(params, "media", { trim: false }) ??
readStringParam(params, "mediaUrl", { trim: false }) ??
readStringParam(params, "path", { trim: false }) ??
readStringParam(params, "filePath", { trim: false });
readStringParam(params, "filePath", { trim: false }) ??
readStringParam(params, "fileUrl", { trim: false });
const hasButtons = Array.isArray(params.buttons) && params.buttons.length > 0;
const hasCard = params.card != null && typeof params.card === "object";
const hasComponents = params.components != null && typeof params.components === "object";
@@ -620,7 +623,18 @@ async function handlePollAction(ctx: ResolvedActionContext): Promise<MessageActi
}
async function handlePluginAction(ctx: ResolvedActionContext): Promise<MessageActionRunResult> {
const { cfg, params, channel, accountId, dryRun, gateway, input, abortSignal, agentId } = ctx;
const {
cfg,
params,
channel,
mediaLocalRoots,
accountId,
dryRun,
gateway,
input,
abortSignal,
agentId,
} = ctx;
throwIfAborted(abortSignal);
const action = input.action as Exclude<ChannelMessageActionName, "send" | "poll" | "broadcast">;
if (dryRun) {
@@ -644,6 +658,7 @@ async function handlePluginAction(ctx: ResolvedActionContext): Promise<MessageAc
action,
cfg,
params,
mediaLocalRoots,
accountId: accountId ?? undefined,
requesterSenderId: input.requesterSenderId ?? undefined,
sessionKey: input.sessionKey,
@@ -753,6 +768,7 @@ export async function runMessageAction(
cfg,
params,
channel,
mediaLocalRoots,
accountId,
dryRun,
gateway,
@@ -768,6 +784,7 @@ export async function runMessageAction(
cfg,
params,
channel,
mediaLocalRoots,
accountId,
dryRun,
gateway,
@@ -780,6 +797,7 @@ export async function runMessageAction(
cfg,
params,
channel,
mediaLocalRoots,
accountId,
dryRun,
gateway,