From 93880717f1cd34feaa45e74e939b7a5256288901 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 23 Mar 2026 00:28:30 -0700 Subject: [PATCH] fix(media): harden secondary local path seams --- .../pi-embedded-runner/run/images.test.ts | 18 +++++++++++- src/agents/pi-embedded-runner/run/images.ts | 9 ++++-- src/agents/sandbox-paths.test.ts | 22 +++++++++++++- src/agents/sandbox-paths.ts | 12 +++++--- .../attachments.normalize.test.ts | 29 +++++++++++++++++++ .../attachments.normalize.ts | 9 ++++-- 6 files changed, 89 insertions(+), 10 deletions(-) create mode 100644 src/media-understanding/attachments.normalize.test.ts diff --git a/src/agents/pi-embedded-runner/run/images.test.ts b/src/agents/pi-embedded-runner/run/images.test.ts index 59b3673e90f..b0dc1008ede 100644 --- a/src/agents/pi-embedded-runner/run/images.test.ts +++ b/src/agents/pi-embedded-runner/run/images.test.ts @@ -1,7 +1,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { createHostSandboxFsBridge } from "../../test-helpers/host-sandbox-fs-bridge.js"; import { createUnsafeMountedSandbox } from "../../test-helpers/unsafe-mounted-sandbox.js"; import { @@ -190,6 +190,22 @@ what is this?`); // Only 1 ref - the local path (example.com URLs are skipped) expect(ref?.resolved).toContain("ChatGPT Image Apr 21, 2025.png"); }); + + it("ignores remote-host file URLs", () => { + expectNoImageReferences("See file://attacker/share/evil.png"); + }); + + it("ignores Windows network paths from attachment-style references", () => { + const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); + + try { + expectNoImageReferences( + "[media attached: \\\\attacker\\share\\photo.png (image/png)] what is this?", + ); + } finally { + platformSpy.mockRestore(); + } + }); }); describe("modelSupportsImages", () => { diff --git a/src/agents/pi-embedded-runner/run/images.ts b/src/agents/pi-embedded-runner/run/images.ts index 3fa8b714255..686a7791e5a 100644 --- a/src/agents/pi-embedded-runner/run/images.ts +++ b/src/agents/pi-embedded-runner/run/images.ts @@ -1,6 +1,6 @@ import path from "node:path"; -import { fileURLToPath } from "node:url"; import type { ImageContent } from "@mariozechner/pi-ai"; +import { assertNoWindowsNetworkPath, safeFileURLToPath } from "../../../infra/local-file-access.js"; import { loadWebMedia } from "../../../media/web-media.js"; import { resolveUserPath } from "../../../utils.js"; import type { ImageSanitizationLimits } from "../../image-sanitization.js"; @@ -108,6 +108,11 @@ export function detectImageReferences(prompt: string): DetectedImageRef[] { if (!isImageExtension(trimmed)) { return; } + try { + assertNoWindowsNetworkPath(trimmed, "Image path"); + } catch { + return; + } seen.add(dedupeKey); const resolved = trimmed.startsWith("~") ? resolveUserPath(trimmed) : trimmed; refs.push({ raw: trimmed, type: "path", resolved }); @@ -160,7 +165,7 @@ export function detectImageReferences(prompt: string): DetectedImageRef[] { seen.add(dedupeKey); // Use fileURLToPath for proper handling (e.g., file://localhost/path) try { - const resolved = fileURLToPath(raw); + const resolved = safeFileURLToPath(raw); refs.push({ raw, type: "path", resolved }); } catch { // Skip malformed file:// URLs diff --git a/src/agents/sandbox-paths.test.ts b/src/agents/sandbox-paths.test.ts index 3deb30a0179..b462ec60253 100644 --- a/src/agents/sandbox-paths.test.ts +++ b/src/agents/sandbox-paths.test.ts @@ -2,7 +2,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { pathToFileURL } from "node:url"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js"; import { resolveSandboxedMediaSource } from "./sandbox-paths.js"; @@ -162,6 +162,11 @@ describe("resolveSandboxedMediaSource", () => { media: "file:///etc/passwd", expected: /sandbox/i, }, + { + name: "file:// URLs with remote hosts", + media: "file://attacker/share/photo.png", + expected: /remote hosts are not allowed/i, + }, { name: "invalid file:// URLs", media: "file://not a valid url\x00", @@ -277,4 +282,19 @@ describe("resolveSandboxedMediaSource", () => { }); expect(result).toBe(""); }); + + it("rejects Windows network paths before sandbox resolution", async () => { + const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); + + try { + await expect( + resolveSandboxedMediaSource({ + media: "\\\\attacker\\share\\photo.png", + sandboxRoot: "/any/path", + }), + ).rejects.toThrow(/network paths/i); + } finally { + platformSpy.mockRestore(); + } + }); }); diff --git a/src/agents/sandbox-paths.ts b/src/agents/sandbox-paths.ts index 1d46d02db63..7b8d0aa6a5e 100644 --- a/src/agents/sandbox-paths.ts +++ b/src/agents/sandbox-paths.ts @@ -1,6 +1,7 @@ import os from "node:os"; import path from "node:path"; -import { fileURLToPath, URL } from "node:url"; +import { URL } from "node:url"; +import { assertNoWindowsNetworkPath, safeFileURLToPath } from "../infra/local-file-access.js"; import { assertNoPathAliasEscape, type PathAliasPolicy } from "../infra/path-alias-guards.js"; import { isPathInside } from "../infra/path-guards.js"; import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js"; @@ -106,9 +107,11 @@ export async function resolveSandboxedMediaSource(params: { candidate = workspaceMappedFromUrl; } else { try { - candidate = fileURLToPath(candidate); - } catch { - throw new Error(`Invalid file:// URL for sandboxed media: ${raw}`); + candidate = safeFileURLToPath(candidate); + } catch (err) { + throw new Error(`Invalid file:// URL for sandboxed media: ${(err as Error).message}`, { + cause: err, + }); } } } @@ -119,6 +122,7 @@ export async function resolveSandboxedMediaSource(params: { if (containerWorkspaceMapped) { candidate = containerWorkspaceMapped; } + assertNoWindowsNetworkPath(candidate, "Sandbox media path"); const tmpMediaPath = await resolveAllowedTmpMediaPath({ candidate, sandboxRoot: params.sandboxRoot, diff --git a/src/media-understanding/attachments.normalize.test.ts b/src/media-understanding/attachments.normalize.test.ts new file mode 100644 index 00000000000..dacebccc46f --- /dev/null +++ b/src/media-understanding/attachments.normalize.test.ts @@ -0,0 +1,29 @@ +import os from "node:os"; +import path from "node:path"; +import { pathToFileURL } from "node:url"; +import { describe, expect, it, vi } from "vitest"; +import { normalizeAttachmentPath } from "./attachments.normalize.js"; + +describe("normalizeAttachmentPath", () => { + it("allows localhost file URLs", () => { + const localPath = path.join(os.tmpdir(), "photo.png"); + const fileUrl = pathToFileURL(localPath); + fileUrl.hostname = "localhost"; + + expect(normalizeAttachmentPath(fileUrl.href)).toBe(localPath); + }); + + it("rejects remote-host file URLs", () => { + expect(normalizeAttachmentPath("file://attacker/share/photo.png")).toBeUndefined(); + }); + + it("rejects Windows network paths", () => { + const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); + + try { + expect(normalizeAttachmentPath("\\\\attacker\\share\\photo.png")).toBeUndefined(); + } finally { + platformSpy.mockRestore(); + } + }); +}); diff --git a/src/media-understanding/attachments.normalize.ts b/src/media-understanding/attachments.normalize.ts index 4c248c538f9..eea1467fa7e 100644 --- a/src/media-understanding/attachments.normalize.ts +++ b/src/media-understanding/attachments.normalize.ts @@ -1,5 +1,5 @@ -import { fileURLToPath } from "node:url"; import type { MsgContext } from "../auto-reply/templating.js"; +import { assertNoWindowsNetworkPath, safeFileURLToPath } from "../infra/local-file-access.js"; import { getFileExtension, isAudioFileName, kindFromMime } from "../media/mime.js"; import type { MediaAttachment } from "./types.js"; @@ -10,11 +10,16 @@ export function normalizeAttachmentPath(raw?: string | null): string | undefined } if (value.startsWith("file://")) { try { - return fileURLToPath(value); + return safeFileURLToPath(value); } catch { return undefined; } } + try { + assertNoWindowsNetworkPath(value, "Attachment path"); + } catch { + return undefined; + } return value; }