fix(media): guard local media reads + accept all path types in MEDIA directive

This commit is contained in:
buddyh
2026-02-08 23:52:11 -05:00
committed by the sun gif man
parent 029b77c85b
commit 4baa43384a
5 changed files with 170 additions and 26 deletions

View File

@@ -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;

View File

@@ -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();
});
});

View File

@@ -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;

View File

@@ -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");
});
});

View File

@@ -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,
});
}