mirror of
https://github.com/openclaw/openclaw.git
synced 2026-02-19 18:39:20 -05:00
fix(media): guard local media reads + accept all path types in MEDIA directive
This commit is contained in:
@@ -443,7 +443,8 @@ async function hydrateSetGroupIconParams(params: {
|
||||
channel: params.channel,
|
||||
accountId: params.accountId,
|
||||
});
|
||||
const media = await loadWebMedia(mediaSource, maxBytes);
|
||||
// localRoots: "any" — media paths are already validated by normalizeSandboxMediaList above.
|
||||
const media = await loadWebMedia(mediaSource, maxBytes, { localRoots: "any" });
|
||||
params.args.buffer = media.buffer.toString("base64");
|
||||
if (!contentTypeParam && media.contentType) {
|
||||
params.args.contentType = media.contentType;
|
||||
@@ -507,7 +508,8 @@ async function hydrateSendAttachmentParams(params: {
|
||||
channel: params.channel,
|
||||
accountId: params.accountId,
|
||||
});
|
||||
const media = await loadWebMedia(mediaSource, maxBytes);
|
||||
// localRoots: "any" — media paths are already validated by normalizeSandboxMediaList above.
|
||||
const media = await loadWebMedia(mediaSource, maxBytes, { localRoots: "any" });
|
||||
params.args.buffer = media.buffer.toString("base64");
|
||||
if (!contentTypeParam && media.contentType) {
|
||||
params.args.contentType = media.contentType;
|
||||
|
||||
@@ -8,28 +8,28 @@ describe("splitMediaFromOutput", () => {
|
||||
expect(result.text).toBe("Hello world");
|
||||
});
|
||||
|
||||
it("rejects absolute media paths to prevent LFI", () => {
|
||||
it("accepts absolute media paths", () => {
|
||||
const result = splitMediaFromOutput("MEDIA:/Users/pete/My File.png");
|
||||
expect(result.mediaUrls).toBeUndefined();
|
||||
expect(result.text).toBe("MEDIA:/Users/pete/My File.png");
|
||||
expect(result.mediaUrls).toEqual(["/Users/pete/My File.png"]);
|
||||
expect(result.text).toBe("");
|
||||
});
|
||||
|
||||
it("rejects quoted absolute media paths to prevent LFI", () => {
|
||||
it("accepts quoted absolute media paths", () => {
|
||||
const result = splitMediaFromOutput('MEDIA:"/Users/pete/My File.png"');
|
||||
expect(result.mediaUrls).toBeUndefined();
|
||||
expect(result.text).toBe('MEDIA:"/Users/pete/My File.png"');
|
||||
expect(result.mediaUrls).toEqual(["/Users/pete/My File.png"]);
|
||||
expect(result.text).toBe("");
|
||||
});
|
||||
|
||||
it("rejects tilde media paths to prevent LFI", () => {
|
||||
it("accepts tilde media paths", () => {
|
||||
const result = splitMediaFromOutput("MEDIA:~/Pictures/My File.png");
|
||||
expect(result.mediaUrls).toBeUndefined();
|
||||
expect(result.text).toBe("MEDIA:~/Pictures/My File.png");
|
||||
expect(result.mediaUrls).toEqual(["~/Pictures/My File.png"]);
|
||||
expect(result.text).toBe("");
|
||||
});
|
||||
|
||||
it("rejects directory traversal media paths to prevent LFI", () => {
|
||||
it("accepts traversal-like media paths (validated at load time)", () => {
|
||||
const result = splitMediaFromOutput("MEDIA:../../etc/passwd");
|
||||
expect(result.mediaUrls).toBeUndefined();
|
||||
expect(result.text).toBe("MEDIA:../../etc/passwd");
|
||||
expect(result.mediaUrls).toEqual(["../../etc/passwd"]);
|
||||
expect(result.text).toBe("");
|
||||
});
|
||||
|
||||
it("captures safe relative media paths", () => {
|
||||
@@ -38,6 +38,12 @@ describe("splitMediaFromOutput", () => {
|
||||
expect(result.text).toBe("");
|
||||
});
|
||||
|
||||
it("accepts sandbox-relative media paths", () => {
|
||||
const result = splitMediaFromOutput("MEDIA:media/inbound/image.png");
|
||||
expect(result.mediaUrls).toEqual(["media/inbound/image.png"]);
|
||||
expect(result.text).toBe("");
|
||||
});
|
||||
|
||||
it("keeps audio_as_voice detection stable across calls", () => {
|
||||
const input = "Hello [[audio_as_voice]]";
|
||||
const first = splitMediaFromOutput(input);
|
||||
@@ -58,4 +64,27 @@ describe("splitMediaFromOutput", () => {
|
||||
expect(result.mediaUrls).toEqual(["./screenshot.png"]);
|
||||
expect(result.text).toBe("");
|
||||
});
|
||||
|
||||
it("accepts Windows-style paths", () => {
|
||||
const result = splitMediaFromOutput("MEDIA:C:\\Users\\pete\\Pictures\\snap.png");
|
||||
expect(result.mediaUrls).toEqual(["C:\\Users\\pete\\Pictures\\snap.png"]);
|
||||
expect(result.text).toBe("");
|
||||
});
|
||||
|
||||
it("accepts TTS temp file paths", () => {
|
||||
const result = splitMediaFromOutput("MEDIA:/tmp/tts-fAJy8C/voice-1770246885083.opus");
|
||||
expect(result.mediaUrls).toEqual(["/tmp/tts-fAJy8C/voice-1770246885083.opus"]);
|
||||
expect(result.text).toBe("");
|
||||
});
|
||||
|
||||
it("accepts bare filenames with extensions", () => {
|
||||
const result = splitMediaFromOutput("MEDIA:image.png");
|
||||
expect(result.mediaUrls).toEqual(["image.png"]);
|
||||
expect(result.text).toBe("");
|
||||
});
|
||||
|
||||
it("rejects bare words without file extensions", () => {
|
||||
const result = splitMediaFromOutput("MEDIA:screenshot");
|
||||
expect(result.mediaUrls).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -14,7 +14,29 @@ function cleanCandidate(raw: string) {
|
||||
return raw.replace(/^[`"'[{(]+/, "").replace(/[`"'\\})\],]+$/, "");
|
||||
}
|
||||
|
||||
function isValidMedia(candidate: string, opts?: { allowSpaces?: boolean }) {
|
||||
const WINDOWS_DRIVE_RE = /^[a-zA-Z]:[\\/]/;
|
||||
const SCHEME_RE = /^[a-zA-Z][a-zA-Z0-9+.-]*:/;
|
||||
const HAS_FILE_EXT = /\.\w{1,10}$/;
|
||||
|
||||
// Recognize local file path patterns. Security validation is deferred to the
|
||||
// load layer (loadWebMedia / resolveSandboxedMediaSource) which has the context
|
||||
// needed to enforce sandbox roots and allowed directories.
|
||||
function isLikelyLocalPath(candidate: string): boolean {
|
||||
return (
|
||||
candidate.startsWith("/") ||
|
||||
candidate.startsWith("./") ||
|
||||
candidate.startsWith("../") ||
|
||||
candidate.startsWith("~") ||
|
||||
WINDOWS_DRIVE_RE.test(candidate) ||
|
||||
candidate.startsWith("\\\\") ||
|
||||
(!SCHEME_RE.test(candidate) && (candidate.includes("/") || candidate.includes("\\")))
|
||||
);
|
||||
}
|
||||
|
||||
function isValidMedia(
|
||||
candidate: string,
|
||||
opts?: { allowSpaces?: boolean; allowBareFilename?: boolean },
|
||||
) {
|
||||
if (!candidate) {
|
||||
return false;
|
||||
}
|
||||
@@ -28,8 +50,17 @@ function isValidMedia(candidate: string, opts?: { allowSpaces?: boolean }) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Local paths: only allow safe relative paths starting with ./ that do not traverse upwards.
|
||||
return candidate.startsWith("./") && !candidate.includes("..");
|
||||
if (isLikelyLocalPath(candidate)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Accept bare filenames (e.g. "image.png") only when the caller opts in.
|
||||
// This avoids treating space-split path fragments as separate media items.
|
||||
if (opts?.allowBareFilename && !SCHEME_RE.test(candidate) && HAS_FILE_EXT.test(candidate)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
function unwrapQuoted(value: string): string | undefined {
|
||||
@@ -128,11 +159,7 @@ export function splitMediaFromOutput(raw: string): {
|
||||
|
||||
const trimmedPayload = payloadValue.trim();
|
||||
const looksLikeLocalPath =
|
||||
trimmedPayload.startsWith("/") ||
|
||||
trimmedPayload.startsWith("./") ||
|
||||
trimmedPayload.startsWith("../") ||
|
||||
trimmedPayload.startsWith("~") ||
|
||||
trimmedPayload.startsWith("file://");
|
||||
isLikelyLocalPath(trimmedPayload) || trimmedPayload.startsWith("file://");
|
||||
if (
|
||||
!unwrapped &&
|
||||
validCount === 1 &&
|
||||
@@ -152,7 +179,7 @@ export function splitMediaFromOutput(raw: string): {
|
||||
|
||||
if (!hasValidMedia) {
|
||||
const fallback = normalizeMediaSource(cleanCandidate(payloadValue));
|
||||
if (isValidMedia(fallback, { allowSpaces: true })) {
|
||||
if (isValidMedia(fallback, { allowSpaces: true, allowBareFilename: true })) {
|
||||
media.push(fallback);
|
||||
hasValidMedia = true;
|
||||
foundMediaToken = true;
|
||||
|
||||
@@ -292,3 +292,43 @@ describe("web media loading", () => {
|
||||
expect(result.buffer.length).toBeLessThanOrEqual(cap);
|
||||
});
|
||||
});
|
||||
|
||||
describe("local media root guard", () => {
|
||||
it("rejects local paths outside allowed roots", async () => {
|
||||
const pngBuffer = await sharp({
|
||||
create: { width: 10, height: 10, channels: 3, background: "#00ff00" },
|
||||
})
|
||||
.png()
|
||||
.toBuffer();
|
||||
const file = await writeTempFile(pngBuffer, ".png");
|
||||
|
||||
// Explicit roots that don't contain the temp file.
|
||||
await expect(
|
||||
loadWebMedia(file, 1024 * 1024, { localRoots: ["/nonexistent-root"] }),
|
||||
).rejects.toThrow(/not under an allowed directory/i);
|
||||
});
|
||||
|
||||
it("allows local paths under an explicit root", async () => {
|
||||
const pngBuffer = await sharp({
|
||||
create: { width: 10, height: 10, channels: 3, background: "#00ff00" },
|
||||
})
|
||||
.png()
|
||||
.toBuffer();
|
||||
const file = await writeTempFile(pngBuffer, ".png");
|
||||
|
||||
const result = await loadWebMedia(file, 1024 * 1024, { localRoots: [os.tmpdir()] });
|
||||
expect(result.kind).toBe("image");
|
||||
});
|
||||
|
||||
it("allows any path when localRoots is 'any'", async () => {
|
||||
const pngBuffer = await sharp({
|
||||
create: { width: 10, height: 10, channels: 3, background: "#00ff00" },
|
||||
})
|
||||
.png()
|
||||
.toBuffer();
|
||||
const file = await writeTempFile(pngBuffer, ".png");
|
||||
|
||||
const result = await loadWebMedia(file, 1024 * 1024, { localRoots: "any" });
|
||||
expect(result.kind).toBe("image");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import type { SsrFPolicy } from "../infra/net/ssrf.js";
|
||||
@@ -25,8 +26,48 @@ type WebMediaOptions = {
|
||||
maxBytes?: number;
|
||||
optimizeImages?: boolean;
|
||||
ssrfPolicy?: SsrFPolicy;
|
||||
/** Allowed root directories for local path reads. "any" skips the check (caller already validated). */
|
||||
localRoots?: string[] | "any";
|
||||
};
|
||||
|
||||
function getDefaultLocalRoots(): string[] {
|
||||
const home = os.homedir();
|
||||
return [
|
||||
os.tmpdir(),
|
||||
path.join(home, ".openclaw", "media"),
|
||||
path.join(home, ".openclaw", "agents"),
|
||||
];
|
||||
}
|
||||
|
||||
async function assertLocalMediaAllowed(
|
||||
mediaPath: string,
|
||||
localRoots: string[] | "any" | undefined,
|
||||
): Promise<void> {
|
||||
if (localRoots === "any") {
|
||||
return;
|
||||
}
|
||||
const roots = localRoots ?? getDefaultLocalRoots();
|
||||
// Resolve symlinks so a symlink under /tmp pointing to /etc/passwd is caught.
|
||||
let resolved: string;
|
||||
try {
|
||||
resolved = await fs.realpath(mediaPath);
|
||||
} catch {
|
||||
resolved = path.resolve(mediaPath);
|
||||
}
|
||||
for (const root of roots) {
|
||||
let resolvedRoot: string;
|
||||
try {
|
||||
resolvedRoot = await fs.realpath(root);
|
||||
} catch {
|
||||
resolvedRoot = path.resolve(root);
|
||||
}
|
||||
if (resolved === resolvedRoot || resolved.startsWith(resolvedRoot + path.sep)) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
throw new Error(`Local media path is not under an allowed directory: ${mediaPath}`);
|
||||
}
|
||||
|
||||
const HEIC_MIME_RE = /^image\/hei[cf]$/i;
|
||||
const HEIC_EXT_RE = /\.(heic|heif)$/i;
|
||||
const MB = 1024 * 1024;
|
||||
@@ -124,7 +165,7 @@ async function loadWebMediaInternal(
|
||||
mediaUrl: string,
|
||||
options: WebMediaOptions = {},
|
||||
): Promise<WebMediaResult> {
|
||||
const { maxBytes, optimizeImages = true, ssrfPolicy } = options;
|
||||
const { maxBytes, optimizeImages = true, ssrfPolicy, localRoots } = options;
|
||||
// Use fileURLToPath for proper handling of file:// URLs (handles file://localhost/path, etc.)
|
||||
if (mediaUrl.startsWith("file://")) {
|
||||
try {
|
||||
@@ -222,6 +263,9 @@ async function loadWebMediaInternal(
|
||||
mediaUrl = resolveUserPath(mediaUrl);
|
||||
}
|
||||
|
||||
// Guard local reads against allowed directory roots to prevent file exfiltration.
|
||||
await assertLocalMediaAllowed(mediaUrl, localRoots);
|
||||
|
||||
// Local path
|
||||
const data = await fs.readFile(mediaUrl);
|
||||
const mime = await detectMime({ buffer: data, filePath: mediaUrl });
|
||||
@@ -244,24 +288,26 @@ async function loadWebMediaInternal(
|
||||
export async function loadWebMedia(
|
||||
mediaUrl: string,
|
||||
maxBytes?: number,
|
||||
options?: { ssrfPolicy?: SsrFPolicy },
|
||||
options?: { ssrfPolicy?: SsrFPolicy; localRoots?: string[] | "any" },
|
||||
): Promise<WebMediaResult> {
|
||||
return await loadWebMediaInternal(mediaUrl, {
|
||||
maxBytes,
|
||||
optimizeImages: true,
|
||||
ssrfPolicy: options?.ssrfPolicy,
|
||||
localRoots: options?.localRoots,
|
||||
});
|
||||
}
|
||||
|
||||
export async function loadWebMediaRaw(
|
||||
mediaUrl: string,
|
||||
maxBytes?: number,
|
||||
options?: { ssrfPolicy?: SsrFPolicy },
|
||||
options?: { ssrfPolicy?: SsrFPolicy; localRoots?: string[] | "any" },
|
||||
): Promise<WebMediaResult> {
|
||||
return await loadWebMediaInternal(mediaUrl, {
|
||||
maxBytes,
|
||||
optimizeImages: false,
|
||||
ssrfPolicy: options?.ssrfPolicy,
|
||||
localRoots: options?.localRoots,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user