mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-29 17:25:14 +00:00
fix(media): resolve inbound media refs consistently
Summary:
- Resolve inbound media references through the shared media-reference path before workspace-relative handling.
- Reuse the same sandbox rewrite for Pi native images and sandbox media bridge paths.
- Add regression coverage for managed inbound images, sandbox-staged media references, and invalid media IDs.
- Fix current lint by using non-mutating cpuprofile sorting.
Verification:
- node scripts/run-vitest.mjs src/media/media-reference.test.ts src/agents/sandbox-media-paths.test.ts src/agents/pi-embedded-runner/run/images.test.ts src/agents/tools/image-tool.test.ts src/media/web-media.test.ts src/agents/tools/pdf-tool.test.ts src/agents/tools/image-generate-tool.test.ts src/agents/tools/video-generate-tool.test.ts src/agents/tools/music-generate-tool.test.ts
- node scripts/run-oxlint-shards.mjs --threads=8
- git diff --check
- /Users/steipete/Projects/agent-skills/skills/autoreview/scripts/autoreview --mode branch --base origin/main
- GitHub CI rollup passed for eceea707a7
Fixes #87024.
Supersedes #87055; thanks @TurboTheTurtle for the report and initial fix direction.
Co-authored-by: Andy Ye <35905412+TurboTheTurtle@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
b74984dd50
commit
6290ed52ff
@@ -353,6 +353,68 @@ describe("modelSupportsImages", () => {
|
||||
});
|
||||
|
||||
describe("loadImageFromRef", () => {
|
||||
it("hydrates managed inbound media URIs before workspace path resolution", async () => {
|
||||
const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-native-image-uri-"));
|
||||
const workspaceDir = path.join(stateDir, "workspace-agent");
|
||||
const inboundDir = path.join(stateDir, "media", "inbound");
|
||||
const mediaId = "telegram-photo.png";
|
||||
await fs.mkdir(workspaceDir, { recursive: true });
|
||||
await fs.mkdir(inboundDir, { recursive: true });
|
||||
await fs.writeFile(path.join(inboundDir, mediaId), Buffer.from(TINY_PNG_BASE64, "base64"));
|
||||
vi.stubEnv("OPENCLAW_STATE_DIR", stateDir);
|
||||
|
||||
try {
|
||||
const image = await loadImageFromRef(
|
||||
{
|
||||
raw: `media://inbound/${mediaId}`,
|
||||
type: "media-uri",
|
||||
resolved: `media://inbound/${mediaId}`,
|
||||
},
|
||||
workspaceDir,
|
||||
{ workspaceOnly: true },
|
||||
);
|
||||
|
||||
expect(image?.type).toBe("image");
|
||||
expect(image?.mimeType).toBe("image/png");
|
||||
expect(image?.data).toBe(TINY_PNG_BASE64);
|
||||
} finally {
|
||||
vi.unstubAllEnvs();
|
||||
await fs.rm(stateDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("hydrates sandbox-staged inbound media URIs", async () => {
|
||||
const sandboxRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-native-image-sbx-uri-"));
|
||||
const inboundDir = path.join(sandboxRoot, "media", "inbound");
|
||||
const mediaId = "telegram-photo.png";
|
||||
await fs.mkdir(inboundDir, { recursive: true });
|
||||
await fs.writeFile(path.join(inboundDir, mediaId), Buffer.from(TINY_PNG_BASE64, "base64"));
|
||||
|
||||
try {
|
||||
const image = await loadImageFromRef(
|
||||
{
|
||||
raw: `media://inbound/${mediaId}`,
|
||||
type: "media-uri",
|
||||
resolved: `media://inbound/${mediaId}`,
|
||||
},
|
||||
sandboxRoot,
|
||||
{
|
||||
workspaceOnly: true,
|
||||
sandbox: {
|
||||
root: sandboxRoot,
|
||||
bridge: createHostSandboxFsBridge(sandboxRoot),
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
expect(image?.type).toBe("image");
|
||||
expect(image?.mimeType).toBe("image/png");
|
||||
expect(image?.data).toBe(TINY_PNG_BASE64);
|
||||
} finally {
|
||||
await fs.rm(sandboxRoot, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("allows sandbox-validated host paths outside default media roots", async () => {
|
||||
const homeDir = os.homedir();
|
||||
await fs.mkdir(homeDir, { recursive: true });
|
||||
|
||||
@@ -2,6 +2,7 @@ import path from "node:path";
|
||||
import type { ImageContent } from "@earendil-works/pi-ai";
|
||||
import { formatErrorMessage } from "../../../infra/errors.js";
|
||||
import { assertNoWindowsNetworkPath, safeFileURLToPath } from "../../../infra/local-file-access.js";
|
||||
import { resolveMediaReferenceLocalPath } from "../../../media/media-reference.js";
|
||||
import type { PromptImageOrderEntry } from "../../../media/prompt-image-order.js";
|
||||
import { loadWebMedia } from "../../../media/web-media.js";
|
||||
import { normalizeLowercaseStringOrEmpty } from "../../../shared/string-coerce.js";
|
||||
@@ -436,6 +437,10 @@ export async function loadImageFromRef(
|
||||
try {
|
||||
let targetPath = ref.resolved;
|
||||
|
||||
if (!options?.sandbox) {
|
||||
targetPath = await resolveMediaReferenceLocalPath(targetPath);
|
||||
}
|
||||
|
||||
// Resolve paths relative to sandbox or workspace as needed
|
||||
if (options?.sandbox) {
|
||||
try {
|
||||
@@ -446,6 +451,7 @@ export async function loadImageFromRef(
|
||||
workspaceOnly: options.workspaceOnly,
|
||||
},
|
||||
mediaPath: targetPath,
|
||||
inboundFallbackDir: "media/inbound",
|
||||
});
|
||||
targetPath = resolved.resolved;
|
||||
} catch (err) {
|
||||
|
||||
@@ -42,4 +42,32 @@ describe("createSandboxBridgeReadFile", () => {
|
||||
expect(resolved).toEqual({ resolved: "/sandbox/image.png" });
|
||||
expect(stat).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rewrites inbound media URIs before direct sandbox resolution", async () => {
|
||||
const resolvePath = vi.fn(({ filePath }: { filePath: string }) => ({
|
||||
hostPath: `/tmp/sandbox-root/${filePath}`,
|
||||
relativePath: filePath,
|
||||
containerPath: `/sandbox/${filePath}`,
|
||||
}));
|
||||
|
||||
const resolved = await resolveSandboxedBridgeMediaPath({
|
||||
sandbox: {
|
||||
root: "/tmp/sandbox-root",
|
||||
bridge: {
|
||||
resolvePath,
|
||||
} as unknown as SandboxFsBridge,
|
||||
},
|
||||
mediaPath: "media://inbound/photo.png",
|
||||
inboundFallbackDir: "media/inbound",
|
||||
});
|
||||
|
||||
expect(resolvePath).toHaveBeenCalledWith({
|
||||
filePath: "media/inbound/photo.png",
|
||||
cwd: "/tmp/sandbox-root",
|
||||
});
|
||||
expect(resolved).toEqual({
|
||||
resolved: "/tmp/sandbox-root/media/inbound/photo.png",
|
||||
rewrittenFrom: "media://inbound/photo.png",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import path from "node:path";
|
||||
import { resolveMediaReferenceSandboxPath } from "../media/media-reference.js";
|
||||
import { assertSandboxPath } from "./sandbox-paths.js";
|
||||
import type { SandboxFsBridge } from "./sandbox/fs-bridge.js";
|
||||
|
||||
@@ -25,7 +26,11 @@ export async function resolveSandboxedBridgeMediaPath(params: {
|
||||
}): Promise<{ resolved: string; rewrittenFrom?: string }> {
|
||||
const normalizeFileUrl = (rawPath: string) =>
|
||||
rawPath.startsWith("file://") ? rawPath.slice("file://".length) : rawPath;
|
||||
const filePath = normalizeFileUrl(params.mediaPath);
|
||||
const mediaPathInfo = params.inboundFallbackDir
|
||||
? resolveMediaReferenceSandboxPath(params.mediaPath, params.inboundFallbackDir)
|
||||
: { resolved: params.mediaPath };
|
||||
const filePath = normalizeFileUrl(mediaPathInfo.resolved);
|
||||
const rewrittenFrom = mediaPathInfo.rewrittenFrom;
|
||||
const enforceWorkspaceBoundary = async (hostPath: string) => {
|
||||
if (!params.sandbox.workspaceOnly) {
|
||||
return;
|
||||
@@ -47,7 +52,10 @@ export async function resolveSandboxedBridgeMediaPath(params: {
|
||||
if (resolved.hostPath) {
|
||||
await enforceWorkspaceBoundary(resolved.hostPath);
|
||||
}
|
||||
return { resolved: resolved.hostPath ?? resolved.containerPath };
|
||||
return {
|
||||
resolved: resolved.hostPath ?? resolved.containerPath,
|
||||
...(rewrittenFrom ? { rewrittenFrom } : {}),
|
||||
};
|
||||
} catch (err) {
|
||||
const fallbackDir = params.inboundFallbackDir?.trim();
|
||||
if (!fallbackDir) {
|
||||
|
||||
@@ -2201,13 +2201,16 @@ describe("image tool managed inbound media", () => {
|
||||
}
|
||||
|
||||
it("resolves media://inbound refs", async () => {
|
||||
await withManagedInboundPng(async ({ mediaId }) => {
|
||||
await withManagedInboundPng(async ({ stateDir, mediaId }) => {
|
||||
installImageUnderstandingProviderStubs();
|
||||
const fetch = stubMinimaxOkFetch();
|
||||
const workspaceDir = path.join(stateDir, "workspace-agent");
|
||||
await fs.mkdir(workspaceDir, { recursive: true });
|
||||
await withTempAgentDir(async (agentDir) => {
|
||||
const tool = createRequiredImageTool({
|
||||
config: createMinimaxImageConfig(),
|
||||
agentDir,
|
||||
workspaceDir,
|
||||
fsPolicy: { workspaceOnly: true },
|
||||
});
|
||||
|
||||
|
||||
@@ -854,7 +854,7 @@ export function createImageTool(options?: {
|
||||
// shared image registry here, so fail gracefully instead of attempting to
|
||||
// `fs.readFile("image:0")` and producing a noisy ENOENT.
|
||||
const refInfo = classifyMediaReferenceSource(normalizedRef);
|
||||
const { isDataUrl, isFileUrl, isHttpUrl } = refInfo;
|
||||
const { isDataUrl, isFileUrl, isHttpUrl, isMediaStoreUrl } = refInfo;
|
||||
if (refInfo.hasUnsupportedScheme) {
|
||||
return {
|
||||
content: [
|
||||
@@ -888,6 +888,7 @@ export function createImageTool(options?: {
|
||||
!isDataUrl &&
|
||||
!isFileUrl &&
|
||||
!isHttpUrl &&
|
||||
!isMediaStoreUrl &&
|
||||
!refInfo.looksLikeWindowsDrivePath &&
|
||||
!isAbsolute(normalizedRef) &&
|
||||
options?.workspaceDir
|
||||
|
||||
@@ -6,8 +6,10 @@ import {
|
||||
classifyMediaReferenceSource,
|
||||
MediaReferenceError,
|
||||
normalizeMediaReferenceSource,
|
||||
parseInboundMediaUri,
|
||||
resolveInboundMediaReference,
|
||||
resolveMediaReferenceLocalPath,
|
||||
resolveMediaReferenceSandboxPath,
|
||||
} from "./media-reference.js";
|
||||
|
||||
async function expectMediaReferenceError(
|
||||
@@ -104,6 +106,20 @@ describe("media reference helpers", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("parses inbound media URIs for sandbox-relative staging", () => {
|
||||
expect(parseInboundMediaUri("MEDIA: media://inbound/photo.png")).toStrictEqual({
|
||||
id: "photo.png",
|
||||
normalizedSource: "media://inbound/photo.png",
|
||||
});
|
||||
expect(resolveMediaReferenceSandboxPath("media://inbound/photo.png")).toStrictEqual({
|
||||
resolved: "media/inbound/photo.png",
|
||||
rewrittenFrom: "media://inbound/photo.png",
|
||||
});
|
||||
expect(resolveMediaReferenceSandboxPath("MEDIA: ./out.png")).toStrictEqual({
|
||||
resolved: "./out.png",
|
||||
});
|
||||
});
|
||||
|
||||
it("maps canonical inbound media URIs to local paths for direct file readers", async () => {
|
||||
const stateDir = resolveStateDir();
|
||||
const id = `ref-local-path-${Date.now()}-${Math.random().toString(36).slice(2)}.png`;
|
||||
@@ -175,6 +191,10 @@ describe("media reference helpers", () => {
|
||||
() => resolveInboundMediaReference("media://inbound/%00.png"),
|
||||
"invalid-path",
|
||||
);
|
||||
expect(() => parseInboundMediaUri("media://inbound/nested%2Fa.png")).toThrow(
|
||||
MediaReferenceError,
|
||||
);
|
||||
expect(() => parseInboundMediaUri("media://inbound/%00.png")).toThrow(MediaReferenceError);
|
||||
});
|
||||
|
||||
it("rejects symlinked inbound media files", async () => {
|
||||
|
||||
@@ -23,6 +23,11 @@ type InboundMediaReference = {
|
||||
sourceType: "uri" | "path";
|
||||
};
|
||||
|
||||
type InboundMediaUri = {
|
||||
id: string;
|
||||
normalizedSource: string;
|
||||
};
|
||||
|
||||
export function normalizeMediaReferenceSource(source: string): string {
|
||||
const trimmed = source.trim();
|
||||
if (/^media:\/\//i.test(trimmed)) {
|
||||
@@ -104,9 +109,8 @@ async function resolvePathForContainment(candidate: string): Promise<string> {
|
||||
}
|
||||
}
|
||||
|
||||
async function resolveInboundMediaUri(
|
||||
normalizedSource: string,
|
||||
): Promise<InboundMediaReference | null> {
|
||||
export function parseInboundMediaUri(source: string): InboundMediaUri | null {
|
||||
const normalizedSource = normalizeMediaReferenceSource(source);
|
||||
if (!/^media:\/\//i.test(normalizedSource)) {
|
||||
return null;
|
||||
}
|
||||
@@ -136,18 +140,45 @@ async function resolveInboundMediaUri(
|
||||
});
|
||||
}
|
||||
|
||||
if (!id || id.includes("/") || id.includes("\\")) {
|
||||
if (!id || id.includes("/") || id.includes("\\") || id.includes("\0")) {
|
||||
throw new MediaReferenceError("invalid-path", `Invalid media URI: ${normalizedSource}`);
|
||||
}
|
||||
|
||||
return {
|
||||
id,
|
||||
normalizedSource,
|
||||
physicalPath: await resolveInboundMediaPath(id, normalizedSource),
|
||||
};
|
||||
}
|
||||
|
||||
async function resolveInboundMediaUri(
|
||||
normalizedSource: string,
|
||||
): Promise<InboundMediaReference | null> {
|
||||
const uri = parseInboundMediaUri(normalizedSource);
|
||||
if (!uri) {
|
||||
return null;
|
||||
}
|
||||
return {
|
||||
...uri,
|
||||
physicalPath: await resolveInboundMediaPath(uri.id, uri.normalizedSource),
|
||||
sourceType: "uri",
|
||||
};
|
||||
}
|
||||
|
||||
export function resolveMediaReferenceSandboxPath(
|
||||
source: string,
|
||||
inboundDir = "media/inbound",
|
||||
): { resolved: string; rewrittenFrom?: string } {
|
||||
const normalizedSource = normalizeMediaReferenceSource(source);
|
||||
const uri = parseInboundMediaUri(normalizedSource);
|
||||
if (!uri) {
|
||||
return { resolved: normalizedSource };
|
||||
}
|
||||
return {
|
||||
resolved: path.posix.join(inboundDir.replace(/\\/g, "/"), uri.id),
|
||||
rewrittenFrom: uri.normalizedSource,
|
||||
};
|
||||
}
|
||||
|
||||
export async function resolveInboundMediaReference(
|
||||
source: string,
|
||||
): Promise<InboundMediaReference | null> {
|
||||
|
||||
Reference in New Issue
Block a user