mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 07:30:43 +00:00
fix(media): validate managed reply media aliases
This commit is contained in:
@@ -4,7 +4,7 @@ import path from "node:path";
|
||||
import { pathToFileURL } from "node:url";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js";
|
||||
import { resolveSandboxedMediaSource } from "./sandbox-paths.js";
|
||||
import { resolveAllowedManagedMediaPath, resolveSandboxedMediaSource } from "./sandbox-paths.js";
|
||||
|
||||
async function withSandboxRoot<T>(run: (sandboxDir: string) => Promise<T>) {
|
||||
const sandboxDir = await fs.mkdtemp(path.join(os.tmpdir(), "sandbox-media-"));
|
||||
@@ -137,6 +137,25 @@ describe("resolveSandboxedMediaSource", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("resolves checked managed media paths for non-sandbox callers", async () => {
|
||||
await withManagedMediaRoot(async ({ stateDir }) => {
|
||||
const media = path.join(stateDir, "media", "outbound", "reply.png");
|
||||
await fs.writeFile(media, "image", "utf8");
|
||||
|
||||
await expect(resolveAllowedManagedMediaPath(media)).resolves.toBe(media);
|
||||
});
|
||||
});
|
||||
|
||||
it("does not allow unrelated state media directories as managed media", async () => {
|
||||
await withManagedMediaRoot(async ({ stateDir }) => {
|
||||
const media = path.join(stateDir, "media", "inbound", "reply.png");
|
||||
await fs.mkdir(path.dirname(media), { recursive: true });
|
||||
await fs.writeFile(media, "image", "utf8");
|
||||
|
||||
await expect(resolveAllowedManagedMediaPath(media)).resolves.toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
// Group 2: Sandbox-relative paths (existing behavior)
|
||||
it("resolves sandbox-relative paths", async () => {
|
||||
await withSandboxRoot(async (sandboxDir) => {
|
||||
@@ -343,6 +362,28 @@ describe("resolveSandboxedMediaSource", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects checked managed media symlinks escaping the managed media root", async () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
await withManagedMediaRoot(async ({ stateDir }) => {
|
||||
const outsideDir = await fs.mkdtemp(path.join(os.tmpdir(), "managed-media-outside-"));
|
||||
const outsideFile = path.join(outsideDir, "secret.png");
|
||||
const symlinkPath = path.join(stateDir, "media", "outbound", "linked-secret.png");
|
||||
try {
|
||||
await fs.writeFile(outsideFile, "secret", "utf8");
|
||||
await fs.symlink(outsideFile, symlinkPath);
|
||||
|
||||
await expect(resolveAllowedManagedMediaPath(symlinkPath)).rejects.toThrow(
|
||||
/managed media root|symlink/i,
|
||||
);
|
||||
} finally {
|
||||
await fs.rm(symlinkPath, { force: true });
|
||||
await fs.rm(outsideDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// Group 4: Passthrough
|
||||
it("passes HTTP URLs through unchanged", async () => {
|
||||
const result = await resolveSandboxedMediaSource({
|
||||
|
||||
@@ -102,7 +102,7 @@ export function assertMediaNotDataUrl(media: string): void {
|
||||
}
|
||||
}
|
||||
|
||||
export function isAllowedManagedMediaPath(candidate: string): boolean {
|
||||
function isManagedMediaPathUnderRoot(candidate: string): boolean {
|
||||
const expanded = expandPath(candidate);
|
||||
if (!hostPathLooksAbsolute(expanded)) {
|
||||
return false;
|
||||
@@ -116,6 +116,22 @@ export function isAllowedManagedMediaPath(candidate: string): boolean {
|
||||
return MANAGED_MEDIA_SUBDIRS.has(firstSegment) || firstSegment.startsWith("tool-");
|
||||
}
|
||||
|
||||
export async function resolveAllowedManagedMediaPath(
|
||||
candidate: string,
|
||||
): Promise<string | undefined> {
|
||||
const expanded = expandPath(candidate);
|
||||
if (!isManagedMediaPathUnderRoot(expanded)) {
|
||||
return undefined;
|
||||
}
|
||||
const resolved = path.resolve(expanded);
|
||||
const managedMediaRoot = path.resolve(resolveConfigDir(), "media");
|
||||
await assertNoManagedMediaAliasEscape({
|
||||
filePath: resolved,
|
||||
managedMediaRoot,
|
||||
});
|
||||
return resolved;
|
||||
}
|
||||
|
||||
export async function resolveSandboxedMediaSource(params: {
|
||||
media: string;
|
||||
sandboxRoot: string;
|
||||
@@ -160,10 +176,7 @@ export async function resolveSandboxedMediaSource(params: {
|
||||
if (tmpMediaPath) {
|
||||
return tmpMediaPath;
|
||||
}
|
||||
const managedMediaPath = await resolveAllowedManagedMediaPath({
|
||||
candidate,
|
||||
sandboxRoot: params.sandboxRoot,
|
||||
});
|
||||
const managedMediaPath = await resolveAllowedManagedMediaPath(candidate);
|
||||
if (managedMediaPath) {
|
||||
return managedMediaPath;
|
||||
}
|
||||
@@ -175,23 +188,6 @@ export async function resolveSandboxedMediaSource(params: {
|
||||
return sandboxResult.resolved;
|
||||
}
|
||||
|
||||
async function resolveAllowedManagedMediaPath(params: {
|
||||
candidate: string;
|
||||
sandboxRoot: string;
|
||||
}): Promise<string | undefined> {
|
||||
const expanded = expandPath(params.candidate);
|
||||
if (!isAllowedManagedMediaPath(expanded)) {
|
||||
return undefined;
|
||||
}
|
||||
const resolved = path.resolve(resolveSandboxInputPath(expanded, params.sandboxRoot));
|
||||
const managedMediaRoot = path.resolve(resolveConfigDir(), "media");
|
||||
await assertNoManagedMediaAliasEscape({
|
||||
filePath: resolved,
|
||||
managedMediaRoot,
|
||||
});
|
||||
return resolved;
|
||||
}
|
||||
|
||||
async function assertNoManagedMediaAliasEscape(params: {
|
||||
filePath: string;
|
||||
managedMediaRoot: string;
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
@@ -323,6 +325,41 @@ describe("createReplyMediaPathNormalizer", () => {
|
||||
expect(resolveOutboundAttachmentFromUrl).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("drops managed outbound media symlinks escaping the shared media root without sandbox mapping", async () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-reply-media-state-"));
|
||||
const outsideDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-reply-media-outside-"));
|
||||
const outsideFile = path.join(outsideDir, "secret.png");
|
||||
const symlinkPath = path.join(stateDir, "media", "outbound", "linked-secret.png");
|
||||
try {
|
||||
await fs.mkdir(path.dirname(symlinkPath), { recursive: true });
|
||||
await fs.writeFile(outsideFile, "secret", "utf8");
|
||||
await fs.symlink(outsideFile, symlinkPath);
|
||||
vi.stubEnv("OPENCLAW_STATE_DIR", stateDir);
|
||||
const normalize = createReplyMediaPathNormalizer({
|
||||
cfg: {},
|
||||
sessionKey: "session-key",
|
||||
workspaceDir: "/tmp/agent-workspace",
|
||||
});
|
||||
|
||||
const result = await normalize({
|
||||
mediaUrls: [symlinkPath],
|
||||
});
|
||||
|
||||
expect(result).toMatchObject({
|
||||
mediaUrl: undefined,
|
||||
mediaUrls: undefined,
|
||||
});
|
||||
expect(resolveOutboundAttachmentFromUrl).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
await fs.rm(symlinkPath, { force: true });
|
||||
await fs.rm(outsideDir, { recursive: true, force: true });
|
||||
await fs.rm(stateDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("drops host-local media when shared outbound attachment policy rejects it", async () => {
|
||||
resolveOutboundAttachmentFromUrl.mockRejectedValueOnce(
|
||||
new Error("Local media path is not under an allowed directory"),
|
||||
|
||||
@@ -4,7 +4,7 @@ import { resolveSessionAgentId } from "../../agents/agent-scope.js";
|
||||
import { resolvePathFromInput, toRelativeWorkspacePath } from "../../agents/path-policy.js";
|
||||
import {
|
||||
assertMediaNotDataUrl,
|
||||
isAllowedManagedMediaPath,
|
||||
resolveAllowedManagedMediaPath,
|
||||
resolveSandboxedMediaSource,
|
||||
} from "../../agents/sandbox-paths.js";
|
||||
import { ensureSandboxWorkspaceForSession } from "../../agents/sandbox.js";
|
||||
@@ -120,8 +120,9 @@ export function createReplyMediaPathNormalizer(params: {
|
||||
if (!isLikelyLocalMediaSource(media)) {
|
||||
return media;
|
||||
}
|
||||
if (isAllowedManagedMediaPath(media)) {
|
||||
return media;
|
||||
const managedMediaPath = await resolveAllowedManagedMediaPath(media);
|
||||
if (managedMediaPath) {
|
||||
return managedMediaPath;
|
||||
}
|
||||
const cached = persistedMediaBySource.get(media);
|
||||
if (cached) {
|
||||
|
||||
Reference in New Issue
Block a user