fix(security): restrict MEDIA path extraction to prevent LFI (#4930)
* fix(security): restrict inbound media staging to media directory * docs: update MEDIA path guidance for security restrictions - Update agent hint to warn against absolute/~ paths - Update docs example to use https:// instead of /tmp/ --------- Co-authored-by: Evan Otero <evanotero@google.com>
This commit is contained in:
@@ -211,7 +211,7 @@ Outbound attachments from the agent: include `MEDIA:<path-or-url>` on its own li
|
|||||||
|
|
||||||
```
|
```
|
||||||
Here’s the screenshot.
|
Here’s the screenshot.
|
||||||
MEDIA:/tmp/screenshot.png
|
MEDIA:https://example.com/screenshot.png
|
||||||
```
|
```
|
||||||
|
|
||||||
OpenClaw extracts these and sends them as media alongside the text.
|
OpenClaw extracts these and sends them as media alongside the text.
|
||||||
|
|||||||
@@ -0,0 +1,79 @@
|
|||||||
|
import fs from "node:fs/promises";
|
||||||
|
import { basename, join } from "node:path";
|
||||||
|
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||||
|
import { withTempHome as withTempHomeBase } from "../../test/helpers/temp-home.js";
|
||||||
|
import type { MsgContext, TemplateContext } from "../templating.js";
|
||||||
|
|
||||||
|
const sandboxMocks = vi.hoisted(() => ({
|
||||||
|
ensureSandboxWorkspaceForSession: vi.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("../agents/sandbox.js", () => sandboxMocks);
|
||||||
|
|
||||||
|
import { ensureSandboxWorkspaceForSession } from "../agents/sandbox.js";
|
||||||
|
import { stageSandboxMedia } from "./reply/stage-sandbox-media.js";
|
||||||
|
|
||||||
|
async function withTempHome<T>(fn: (home: string) => Promise<T>): Promise<T> {
|
||||||
|
return withTempHomeBase(async (home) => await fn(home), { prefix: "openclaw-triggers-bypass-" });
|
||||||
|
}
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
vi.restoreAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("stageSandboxMedia security", () => {
|
||||||
|
it("rejects staging host files from outside the media directory", async () => {
|
||||||
|
await withTempHome(async (home) => {
|
||||||
|
// Sensitive host file outside .openclaw
|
||||||
|
const sensitiveFile = join(home, "secrets.txt");
|
||||||
|
await fs.writeFile(sensitiveFile, "SENSITIVE DATA");
|
||||||
|
|
||||||
|
const sandboxDir = join(home, "sandboxes", "session");
|
||||||
|
vi.mocked(ensureSandboxWorkspaceForSession).mockResolvedValue({
|
||||||
|
workspaceDir: sandboxDir,
|
||||||
|
containerWorkdir: "/work",
|
||||||
|
});
|
||||||
|
|
||||||
|
const ctx: MsgContext = {
|
||||||
|
Body: "hi",
|
||||||
|
From: "whatsapp:group:demo",
|
||||||
|
To: "+2000",
|
||||||
|
ChatType: "group",
|
||||||
|
Provider: "whatsapp",
|
||||||
|
MediaPath: sensitiveFile,
|
||||||
|
MediaType: "image/jpeg",
|
||||||
|
MediaUrl: sensitiveFile,
|
||||||
|
};
|
||||||
|
const sessionCtx: TemplateContext = { ...ctx };
|
||||||
|
|
||||||
|
// This should fail or skip the file
|
||||||
|
await stageSandboxMedia({
|
||||||
|
ctx,
|
||||||
|
sessionCtx,
|
||||||
|
cfg: {
|
||||||
|
agents: {
|
||||||
|
defaults: {
|
||||||
|
model: "anthropic/claude-opus-4-5",
|
||||||
|
workspace: join(home, "openclaw"),
|
||||||
|
sandbox: {
|
||||||
|
mode: "non-main",
|
||||||
|
workspaceRoot: join(home, "sandboxes"),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
channels: { whatsapp: { allowFrom: ["*"] } },
|
||||||
|
session: { store: join(home, "sessions.json") },
|
||||||
|
},
|
||||||
|
sessionKey: "agent:main:main",
|
||||||
|
workspaceDir: join(home, "openclaw"),
|
||||||
|
});
|
||||||
|
|
||||||
|
const stagedFullPath = join(sandboxDir, "media", "inbound", basename(sensitiveFile));
|
||||||
|
// Expect the file NOT to be staged
|
||||||
|
await expect(fs.stat(stagedFullPath)).rejects.toThrow();
|
||||||
|
|
||||||
|
// Context should NOT be rewritten to a sandbox path if it failed to stage
|
||||||
|
expect(ctx.MediaPath).toBe(sensitiveFile);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -248,7 +248,7 @@ export async function runPreparedReply(
|
|||||||
const prefixedBody = [threadStarterNote, prefixedBodyBase].filter(Boolean).join("\n\n");
|
const prefixedBody = [threadStarterNote, prefixedBodyBase].filter(Boolean).join("\n\n");
|
||||||
const mediaNote = buildInboundMediaNote(ctx);
|
const mediaNote = buildInboundMediaNote(ctx);
|
||||||
const mediaReplyHint = mediaNote
|
const mediaReplyHint = mediaNote
|
||||||
? "To send an image back, prefer the message tool (media/path/filePath). If you must inline, use MEDIA:/path or MEDIA:https://example.com/image.jpg (spaces ok, quote if needed). Keep caption in the text body."
|
? "To send an image back, prefer the message tool (media/path/filePath). If you must inline, use MEDIA:https://example.com/image.jpg (spaces ok, quote if needed) or a safe relative path like MEDIA:./image.jpg. Avoid absolute paths (MEDIA:/...) and ~ paths — they are blocked for security. Keep caption in the text body."
|
||||||
: undefined;
|
: undefined;
|
||||||
let prefixedCommandBody = mediaNote
|
let prefixedCommandBody = mediaNote
|
||||||
? [mediaNote, mediaReplyHint, prefixedBody ?? ""].filter(Boolean).join("\n").trim()
|
? [mediaNote, mediaReplyHint, prefixedBody ?? ""].filter(Boolean).join("\n").trim()
|
||||||
|
|||||||
@@ -2,9 +2,11 @@ import { spawn } from "node:child_process";
|
|||||||
import fs from "node:fs/promises";
|
import fs from "node:fs/promises";
|
||||||
import path from "node:path";
|
import path from "node:path";
|
||||||
import { fileURLToPath } from "node:url";
|
import { fileURLToPath } from "node:url";
|
||||||
|
import { assertSandboxPath } from "../../agents/sandbox-paths.js";
|
||||||
import { ensureSandboxWorkspaceForSession } from "../../agents/sandbox.js";
|
import { ensureSandboxWorkspaceForSession } from "../../agents/sandbox.js";
|
||||||
import type { OpenClawConfig } from "../../config/config.js";
|
import type { OpenClawConfig } from "../../config/config.js";
|
||||||
import { logVerbose } from "../../globals.js";
|
import { logVerbose } from "../../globals.js";
|
||||||
|
import { getMediaDir } from "../../media/store.js";
|
||||||
import { CONFIG_DIR } from "../../utils.js";
|
import { CONFIG_DIR } from "../../utils.js";
|
||||||
import type { MsgContext, TemplateContext } from "../templating.js";
|
import type { MsgContext, TemplateContext } from "../templating.js";
|
||||||
|
|
||||||
@@ -80,6 +82,21 @@ export async function stageSandboxMedia(params: {
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Local paths must be restricted to the media directory.
|
||||||
|
if (!ctx.MediaRemoteHost) {
|
||||||
|
const mediaDir = getMediaDir();
|
||||||
|
try {
|
||||||
|
await assertSandboxPath({
|
||||||
|
filePath: source,
|
||||||
|
cwd: mediaDir,
|
||||||
|
root: mediaDir,
|
||||||
|
});
|
||||||
|
} catch {
|
||||||
|
logVerbose(`Blocking attempt to stage media from outside media directory: ${source}`);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
const baseName = path.basename(source);
|
const baseName = path.basename(source);
|
||||||
if (!baseName) {
|
if (!baseName) {
|
||||||
continue;
|
continue;
|
||||||
|
|||||||
Reference in New Issue
Block a user