fix: harden image metadata fallback (#83579)

steipete · web-flow · commit 9fa8b868917f · 2026-05-18T12:35:26.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -40,6 +40,7 @@ Docs: https://docs.openclaw.ai
 
 ### Fixes
 
+- Media: prevent image metadata probing from invoking external decoder delegates on unrecognized image bytes, and stop fallback chaining after real processing errors.
 - Media: install Sharp with the root package and fall back to sips, Windows native imaging, ImageMagick, GraphicsMagick, or ffmpeg for image resizing/conversion when Sharp is unavailable. Fixes #83401. Thanks @scotthuang.
 - Telegram: deliver generated media completions back into forum topics by preserving topic IDs across requester-agent handoff. (#83556) Thanks @fuller-stack-dev.
 - Gateway: defer update-check startup until after readiness so package update checks no longer block sidecar-ready startup, while preserving update broadcasts and shutdown cleanup. (#83520) Thanks @samzong.
diff --git a/src/media/image-ops.security.test.ts b/src/media/image-ops.security.test.ts
@@ -0,0 +1,79 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { createPngBufferWithDimensions } from "./test-helpers.js";
+
+const { loadBundledPluginPublicArtifactModuleSyncMock, resolveSystemBinMock, runExecMock } =
+  vi.hoisted(() => ({
+    loadBundledPluginPublicArtifactModuleSyncMock: vi.fn(),
+    resolveSystemBinMock: vi.fn(),
+    runExecMock: vi.fn(),
+  }));
+
+vi.mock("../plugins/public-surface-loader.js", () => ({
+  loadBundledPluginPublicArtifactModuleSync: loadBundledPluginPublicArtifactModuleSyncMock,
+}));
+
+vi.mock("../infra/resolve-system-bin.js", () => ({
+  resolveSystemBin: resolveSystemBinMock,
+}));
+
+vi.mock("../process/exec.js", () => ({
+  runExec: runExecMock,
+}));
+
+import { getImageMetadata, resizeToJpeg } from "./image-ops.js";
+
+describe("image ops external backend security", () => {
+  const previousBackend = process.env.OPENCLAW_IMAGE_BACKEND;
+
+  afterEach(() => {
+    if (previousBackend === undefined) {
+      delete process.env.OPENCLAW_IMAGE_BACKEND;
+    } else {
+      process.env.OPENCLAW_IMAGE_BACKEND = previousBackend;
+    }
+    loadBundledPluginPublicArtifactModuleSyncMock.mockReset();
+    resolveSystemBinMock.mockReset();
+    runExecMock.mockReset();
+  });
+
+  it("does not use external metadata tools for unrecognized image bytes", async () => {
+    process.env.OPENCLAW_IMAGE_BACKEND = "imagemagick";
+    resolveSystemBinMock.mockReturnValue("/usr/bin/magick");
+
+    const svgWithExternalReference = Buffer.from(
+      '<svg xmlns="http://www.w3.org/2000/svg" width="1" height="1"><image href="http://127.0.0.1:9/probe.png" width="1" height="1"/></svg>',
+    );
+
+    await expect(getImageMetadata(svgWithExternalReference)).resolves.toBeNull();
+
+    expect(runExecMock).not.toHaveBeenCalled();
+    expect(loadBundledPluginPublicArtifactModuleSyncMock).not.toHaveBeenCalled();
+  });
+
+  it("stops backend fallback after a real processing error", async () => {
+    delete process.env.OPENCLAW_IMAGE_BACKEND;
+    resolveSystemBinMock.mockReturnValue("/usr/bin/magick");
+    loadBundledPluginPublicArtifactModuleSyncMock.mockReturnValue({
+      createMediaAttachmentImageOps: () => ({
+        getImageMetadata: vi.fn(),
+        normalizeExifOrientation: vi.fn(),
+        resizeToJpeg: vi.fn(async () => {
+          throw new Error("corrupt image payload");
+        }),
+        convertHeicToJpeg: vi.fn(),
+        hasAlphaChannel: vi.fn(),
+        resizeToPng: vi.fn(),
+      }),
+    });
+
+    await expect(
+      resizeToJpeg({
+        buffer: createPngBufferWithDimensions({ width: 1, height: 1 }),
+        maxSide: 1,
+        quality: 80,
+      }),
+    ).rejects.toThrow(/corrupt image payload/);
+
+    expect(runExecMock).not.toHaveBeenCalled();
+  });
+});
diff --git a/src/media/image-ops.tempdir.test.ts b/src/media/image-ops.tempdir.test.ts
@@ -2,7 +2,10 @@ import fs from "node:fs/promises";
 import path from "node:path";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js";
-import { getImageMetadata } from "./image-ops.js";
+import { resizeToJpeg } from "./image-ops.js";
+
+const PNG_1X1_BASE64 =
+  "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+/p9sAAAAASUVORK5CYII=";
 
 describe("image-ops temp dir", () => {
   let createdTempDir = "";
@@ -26,7 +29,11 @@ describe("image-ops temp dir", () => {
     async () => {
       const secureRoot = await fs.realpath(resolvePreferredOpenClawTmpDir());
 
-      await getImageMetadata(Buffer.from("image"));
+      await resizeToJpeg({
+        buffer: Buffer.from(PNG_1X1_BASE64, "base64"),
+        maxSide: 1,
+        quality: 80,
+      });
 
       expect(fs.mkdtemp).toHaveBeenCalledTimes(1);
       const [mkdtempCall] = vi.mocked(fs.mkdtemp).mock.calls;
diff --git a/src/media/image-ops.ts b/src/media/image-ops.ts
@@ -61,9 +61,7 @@ type ExternalImageTool =
 export const IMAGE_REDUCE_QUALITY_STEPS = [85, 75, 65, 55, 45, 35] as const;
 export const MAX_IMAGE_INPUT_PIXELS = 25_000_000;
 const IMAGE_PROCESS_TIMEOUT_MS = 20_000;
-const IMAGE_METADATA_TIMEOUT_MS = 10_000;
 const IMAGE_TOOL_MAX_BUFFER = 1024 * 1024;
-const IMAGE_METADATA_MAX_BUFFER = 512 * 1024;
 const MEDIA_UNDERSTANDING_CORE_PLUGIN_ID = "media-understanding-core";
 const MEDIA_UNDERSTANDING_CORE_IMAGE_OPS_ARTIFACT = "image-ops.js";
 
@@ -207,6 +205,7 @@ function isImageBackendUnavailableCause(error: unknown): boolean {
     detail.includes('cannot find package "sharp"') ||
     detail.includes("cannot find module 'sharp'") ||
     detail.includes('cannot find module "sharp"') ||
+    detail.includes("support for this compression format has not been built in") ||
     detail.includes("is not available") ||
     detail.includes("command not found") ||
     detail.includes("enoent")
@@ -223,12 +222,11 @@ async function runWithImageBackends<T>(
       return await fn(backend);
     } catch (error) {
       errors.push(error);
+      if (!isImageBackendUnavailableCause(error)) {
+        throw error;
+      }
     }
   }
-  const processingError = errors.find((error) => !isImageBackendUnavailableCause(error));
-  if (processingError) {
-    throw processingError;
-  }
   throw createImageProcessorUnavailableError(operation, errors);
 }
 
@@ -768,18 +766,6 @@ async function runPowerShellImageScript(
   });
 }
 
-const WINDOWS_NATIVE_METADATA_SCRIPT = `
-param([string]$InputPath)
-$ErrorActionPreference = 'Stop'
-Add-Type -AssemblyName System.Drawing
-$image = [System.Drawing.Image]::FromFile($InputPath)
-try {
-  [Console]::Out.WriteLine(('{0} {1}' -f $image.Width, $image.Height))
-} finally {
-  $image.Dispose()
-}
-`;
-
 const WINDOWS_NATIVE_RESIZE_SCRIPT = `
 param(
   [string]$InputPath,
@@ -858,22 +844,6 @@ try {
 }
 `;
 
-async function windowsNativeMetadataFromBuffer(buffer: Buffer): Promise<ImageMetadata | null> {
-  return await withImageTemp(async (workspace) => {
-    const input = await workspace.write("in.img", buffer);
-    const { stdout } = await runPowerShellImageScript(
-      "metadata.ps1",
-      WINDOWS_NATIVE_METADATA_SCRIPT,
-      [input],
-    );
-    const [widthRaw, heightRaw] = stdout.trim().split(/\s+/, 2);
-    return buildImageMetadata(
-      Number.parseInt(widthRaw ?? "", 10),
-      Number.parseInt(heightRaw ?? "", 10),
-    );
-  });
-}
-
 async function windowsNativeResize(
   params: ResizeToJpegParams | ResizeToPngParams,
   format: "jpeg" | "png",
@@ -904,51 +874,6 @@ async function runConvertTool(
   });
 }
 
-async function metadataFromIdentifyTool(
-  tool: Extract<ExternalImageTool, { flavor: "magick" | "convert" | "gm" }>,
-  buffer: Buffer,
-): Promise<ImageMetadata | null> {
-  return await withImageTemp(async (workspace) => {
-    const input = await workspace.write("in.img", buffer);
-    const command =
-      tool.flavor === "convert"
-        ? resolveSystemBin("identify", { trust: "standard" })
-        : tool.command;
-    if (!command) {
-      return null;
-    }
-    const args = tool.flavor === "magick" ? ["identify"] : tool.flavor === "gm" ? ["identify"] : [];
-    const { stdout } = await runExec(command, [...args, "-format", "%w %h", input], {
-      timeoutMs: IMAGE_METADATA_TIMEOUT_MS,
-      maxBuffer: IMAGE_METADATA_MAX_BUFFER,
-    });
-    const [widthRaw, heightRaw] = stdout.trim().split(/\s+/, 2);
-    const width = Number.parseInt(widthRaw ?? "", 10);
-    const height = Number.parseInt(heightRaw ?? "", 10);
-    return buildImageMetadata(width, height);
-  });
-}
-
-async function externalMetadataFromBuffer(
-  backend: Exclude<ImageBackend, "sharp">,
-  buffer: Buffer,
-): Promise<ImageMetadata | null> {
-  const tool = resolveImageTool(backend);
-  if (!tool) {
-    throw new Error(`Image backend ${backend} is not available`);
-  }
-  if (tool.flavor === "sips") {
-    return await sipsMetadataFromBuffer(buffer);
-  }
-  if (tool.flavor === "powershell") {
-    return await windowsNativeMetadataFromBuffer(buffer);
-  }
-  if (tool.flavor === "ffmpeg") {
-    return null;
-  }
-  return await metadataFromIdentifyTool(tool, buffer);
-}
-
 function buildResizeGeometry(maxSide: number, withoutEnlargement?: boolean): string {
   const side = clampInteger(maxSide, 1, Number.MAX_SAFE_INTEGER);
   return `${side}x${side}${withoutEnlargement === false ? "" : ">"}`;
@@ -1114,34 +1039,6 @@ async function externalResizeToPng(
   });
 }
 
-async function sipsMetadataFromBuffer(buffer: Buffer): Promise<ImageMetadata | null> {
-  return await withImageTemp(async (workspace) => {
-    const input = await workspace.write("in.img", buffer);
-    const { stdout } = await runExec(
-      "/usr/bin/sips",
-      ["-g", "pixelWidth", "-g", "pixelHeight", input],
-      {
-        timeoutMs: IMAGE_METADATA_TIMEOUT_MS,
-        maxBuffer: IMAGE_METADATA_MAX_BUFFER,
-      },
-    );
-    const w = stdout.match(/pixelWidth:\s*([0-9]+)/);
-    const h = stdout.match(/pixelHeight:\s*([0-9]+)/);
-    if (!w?.[1] || !h?.[1]) {
-      return null;
-    }
-    const width = Number.parseInt(w[1], 10);
-    const height = Number.parseInt(h[1], 10);
-    if (!Number.isFinite(width) || !Number.isFinite(height)) {
-      return null;
-    }
-    if (width <= 0 || height <= 0) {
-      return null;
-    }
-    return { width, height };
-  });
-}
-
 async function sipsResizeToJpeg(params: {
   buffer: Buffer;
   maxSide: number;
@@ -1193,13 +1090,15 @@ export async function getImageMetadata(buffer: Buffer): Promise<ImageMetadata |
     }
   }
 
-  return await runWithImageBackends("metadata", async (backend) => {
-    const meta =
-      backend === "sharp"
-        ? await (await loadMediaAttachmentImageOps()).getImageMetadata(buffer)
-        : await externalMetadataFromBuffer(backend, buffer);
+  const preference = getImageBackendPreference();
+  if (preference !== "auto" && preference !== "sharp") {
+    return null;
+  }
+
+  return await (async () => {
+    const meta = await (await loadMediaAttachmentImageOps()).getImageMetadata(buffer);
     return meta ? validateImagePixelLimit(meta) : null;
-  }).catch(() => null);
+  })().catch(() => null);
 }
 
 /**
