From 391289564c955058fa4fb685fda7d6032e9b74f5 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 25 Apr 2026 04:47:25 +0100 Subject: [PATCH] fix(media): validate managed reply media aliases --- src/agents/sandbox-paths.test.ts | 43 ++++++++++++++++++- src/agents/sandbox-paths.ts | 40 ++++++++--------- .../reply/reply-media-paths.test.ts | 37 ++++++++++++++++ src/auto-reply/reply/reply-media-paths.ts | 7 +-- 4 files changed, 101 insertions(+), 26 deletions(-) diff --git a/src/agents/sandbox-paths.test.ts b/src/agents/sandbox-paths.test.ts index 0579a1980f1..f8402398860 100644 --- a/src/agents/sandbox-paths.test.ts +++ b/src/agents/sandbox-paths.test.ts @@ -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(run: (sandboxDir: string) => Promise) { 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({ diff --git a/src/agents/sandbox-paths.ts b/src/agents/sandbox-paths.ts index 0a3c49445a2..441a06a7fa7 100644 --- a/src/agents/sandbox-paths.ts +++ b/src/agents/sandbox-paths.ts @@ -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 { + 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 { - 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; diff --git a/src/auto-reply/reply/reply-media-paths.test.ts b/src/auto-reply/reply/reply-media-paths.test.ts index 8ea9c0b9966..600225990f3 100644 --- a/src/auto-reply/reply/reply-media-paths.test.ts +++ b/src/auto-reply/reply/reply-media-paths.test.ts @@ -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"), diff --git a/src/auto-reply/reply/reply-media-paths.ts b/src/auto-reply/reply/reply-media-paths.ts index 13a0d23e1d8..c458577f9b5 100644 --- a/src/auto-reply/reply/reply-media-paths.ts +++ b/src/auto-reply/reply/reply-media-paths.ts @@ -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) {