From fc62052f570cd3216e86e1d45580eb1d2b9291c8 Mon Sep 17 00:00:00 2001 From: "Vyctor H. Brzezowski" Date: Sat, 26 Sep 2026 12:32:02 -0300 Subject: [PATCH] fix: resend inbound attachments referenced from chat history (#158720) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #158704 ## What Problem This Solves Fixes missing attachments and literal `MEDIA:media://inbound/...` text when an assistant resends an inbound attachment using its canonical reference. ## User Impact Replies can reuse stored inbound attachments while preserving their caption and reply target. Existing file-read policy, sandbox restrictions, and supported media types still apply; no configuration change or migration is required. ## Why This Change Was Made Recognize valid inbound media references during output parsing, then resolve them through the existing media-store owner before normal reply access checks. Sharing a pure URI parser keeps filesystem code out of browser-facing parsing. ## Evidence **Real Gateway before/after:** uploaded a synthetic 3,147,780-byte PNG through authenticated `chat.send`, above the existing 2,000,000-byte offload threshold. The Gateway supplied `[media attached: media://inbound/]` to an image-capable scripted provider; the provider copied that exact reference from its request, without an injected ID. - Main `29b670557b1`: settled `chat.history` contains caption plus literal `MEDIA:media://inbound/...`, with no image block. - Candidate `59ab725eae3`: settled history contains caption plus a managed 1024×1024 PNG image block. A further webchat-mode turn produced the same result; authenticated download returned HTTP 200 `image/png` and all 3,147,780 original bytes exactly. - Both sides were checked after media publication settled. The first chat final event precedes asynchronous image materialization and is not the completion oracle. - Normal local attachment and canonical inbound reference preserve exact bytes at the reply boundary, for both directive and structured payloads. Literal `../outside.png`, encoded `%2e%2e/outside.png`, unknown ID, and other-bucket controls produce no prepared attachment, even with a real PNG immediately outside the inbound bucket. Existing sandbox-denial coverage passes. - **270 focused tests across five files passed** in 50.32 seconds command wall time: parser, media-reference, reply-media-paths, Gateway reply-media, and Gateway workspace-media suites. The directive and structured regressions exercise managed image publication and exact-byte readback. - Core production types, owning Gateway-methods and infrastructure test type projects, touched-file lint/format, and fresh builds passed. No project-wide test suite was run. **Reference exposure and limits:** Gateway/Control UI offloaded image requests expose the canonical URI directly in model text. The shared media-note projection also canonicalizes original store paths, and reply-chain metadata can carry canonical `media_path` values. The tested Telegram immediate-photo and reply-to-photo DM prompts exposed staged paths, not canonical URIs. Separate Telegram tests with a scripted actual-store reference show literal text before and a delivered photo after; they do not prove automatic URI discovery on Telegram. Structured-payload coverage is deterministic boundary proof, not a live structured-tool conversation. No visual layout changes are included. Production behavior remains owned by the existing inbound store and reply-access policy; no attachment permissions or public API were added. Co-authored-by: Ayaan Zaidi --- docs/reference/rich-output-protocol.md | 2 + src/auto-reply/reply/reply-media-paths.ts | 7 +- .../server-methods/chat-reply-media.test.ts | 57 +++++++++++++ src/media/inbound-media-uri.ts | 73 +++++++++++++++++ src/media/media-reference.ts | 82 +++---------------- src/media/parse-output.ts | 9 ++ src/media/parse.test.ts | 11 +++ 7 files changed, 168 insertions(+), 73 deletions(-) create mode 100644 src/media/inbound-media-uri.ts diff --git a/docs/reference/rich-output-protocol.md b/docs/reference/rich-output-protocol.md index 50d8c3e89b70..d6536c04e209 100644 --- a/docs/reference/rich-output-protocol.md +++ b/docs/reference/rich-output-protocol.md @@ -21,6 +21,8 @@ Remote attachments must be public `https:` URLs. `http:`, loopback, link-local, Local attachments accept absolute paths, workspace-relative paths, or home-relative `~/` paths. They still pass the agent file-read policy and media type checks before delivery. +Stored inbound attachments also accept `media://inbound/` references from conversation history. Use the reference in a structured attachment field or a standalone `MEDIA:` line. The Gateway resolves it to the stored file and applies the same file-read and sandbox checks as an explicit local path. + In Control UI chat, relative local references resolve against the session's working directory, including a selected project or worktree. They use the same authenticated media route as absolute paths; a missing file shows an attachment error instead of a literal `MEDIA:` line. Files on another execution host must first be delivered as managed attachments. diff --git a/src/auto-reply/reply/reply-media-paths.ts b/src/auto-reply/reply/reply-media-paths.ts index 579021c05d0a..fc7788758480 100644 --- a/src/auto-reply/reply/reply-media-paths.ts +++ b/src/auto-reply/reply/reply-media-paths.ts @@ -28,6 +28,7 @@ import { resolveOutboundMediaMaxBytes } from "../../media/configured-max-bytes.j import type { OutboundMediaAccess } from "../../media/load-options.js"; import { HostReadMediaTypeError, LocalMediaAccessError } from "../../media/local-media-access.js"; import { normalizeMediaReferenceForComparison } from "../../media/media-reference-comparison.js"; +import { resolveInboundMediaReference } from "../../media/media-reference.js"; import { resolveOutboundAttachmentFromUrl } from "../../media/outbound-attachment.js"; import { resolveAgentScopedOutboundMediaAccess } from "../../media/read-capability.js"; import { @@ -298,7 +299,11 @@ export function createReplyMediaSourcePreparer(params: { }; const normalizeMediaSource = async (raw: string): Promise => { - const source = raw.trim(); + const trimmed = raw.trim(); + // Canonical references from history must pass the same policy as their stored file. + const source = /^media:\/\//i.test(trimmed) + ? ((await resolveInboundMediaReference(trimmed))?.physicalPath ?? trimmed) + : trimmed; const mapping = params.workspaceMediaRoot ? resolveSandboxPathMapping( [{ hostRoot: params.workspaceDir, containerRoot: params.workspaceMediaRoot }], diff --git a/src/gateway/server-methods/chat-reply-media.test.ts b/src/gateway/server-methods/chat-reply-media.test.ts index 5067bc111040..09b3578fc3bd 100644 --- a/src/gateway/server-methods/chat-reply-media.test.ts +++ b/src/gateway/server-methods/chat-reply-media.test.ts @@ -18,6 +18,8 @@ import { } from "vitest"; import { consumePendingToolMediaIntoReply } from "../../agents/embedded-agent-subscribe.handlers.messages.replies.js"; import { setReplyPayloadMetadata } from "../../auto-reply/reply-payload.js"; +import { parseReplyDirectives } from "../../auto-reply/reply/reply-directives.js"; +import { createReplyMediaPathNormalizer } from "../../auto-reply/reply/reply-media-paths.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; import { createStructuredOutboundPayloadPlan } from "../../infra/outbound/payloads.js"; import { getAgentScopedMediaLocalRoots } from "../../media/local-roots.js"; @@ -197,6 +199,61 @@ describe("normalizeWebchatReplyMediaPathsForDisplay", () => { await expectPathMissing(path.join(stateDir, "media", "outbound")); } + it.each(["directive", "structured"])( + "publishes a canonical inbound image from a %s reply", + async (kind) => { + const { cfg } = createMediaTestContext({ allowRead: true }); + const saved = await saveMediaBuffer( + PNG_BYTES, + "image/png", + "inbound", + undefined, + "photo.png", + ); + const source = `media://inbound/${saved.id}`; + const caption = "Here it is again."; + const payload = await normalizeReplyMedia({ + cfg, + payloads: [ + kind === "directive" + ? parseReplyDirectives(`[[reply_to_current]] ${caption}\nMEDIA:${source}`) + : { text: caption, replyToCurrent: true, mediaUrls: [source] }, + ], + }); + + expect(payload?.text).toBe(caption); + expect(payload?.replyToCurrent).toBe(true); + const normalizedPath = requireString(payload?.mediaUrls?.[0], "normalized media path"); + expect(await fs.readFile(normalizedPath)).toEqual(PNG_BYTES); + const blocks = await createManagedImageBlocks({ cfg, mediaUrls: payload?.mediaUrls }); + expect(blocks).toEqual([ + expect.objectContaining({ + type: "image", + mimeType: "image/png", + sizeBytes: PNG_BYTES.length, + }), + ]); + }, + ); + + it("does not give inbound URIs broader sandbox access than their stored paths", async () => { + const { cfg, stateDir, workspaceDir } = createMediaTestContext({ allowRead: true }); + const saved = await saveMediaBuffer(PNG_BYTES, "image/png", "inbound", undefined, "photo.png"); + const normalize = createReplyMediaPathNormalizer({ + cfg, + sessionKey: TEST_SESSION_KEY, + workspaceDir, + sandboxRoot: path.join(stateDir, "sandbox"), + }); + + for (const source of [saved.path, `media://inbound/${saved.id}`]) { + const payload = await normalize({ mediaUrls: [source] }); + expect(payload.mediaUrls).toBeUndefined(); + expect(payload.text).toContain("Delivery failed"); + } + await expectOutboundMediaMissing(stateDir); + }); + it("stages Codex-home image paths before Gateway managed-image display", async () => { const { stateDir, cfg, sourcePath, payload } = await normalizeCodexHomeImage({ allowRead: true, diff --git a/src/media/inbound-media-uri.ts b/src/media/inbound-media-uri.ts new file mode 100644 index 000000000000..47da2a2bf0f5 --- /dev/null +++ b/src/media/inbound-media-uri.ts @@ -0,0 +1,73 @@ +// Pure URI parsing shared by output extraction and native media resolution. +type MediaReferenceErrorCode = "invalid-path" | "path-not-allowed"; + +/** Error raised when a media reference cannot be mapped to an allowed local media file. */ +export class MediaReferenceError extends Error { + code: MediaReferenceErrorCode; + + constructor(code: MediaReferenceErrorCode, message: string, options?: ErrorOptions) { + super(message, options); + this.code = code; + this.name = "MediaReferenceError"; + } +} + +type InboundMediaUri = { + id: string; + normalizedSource: string; +}; + +/** Strips legacy MEDIA: prefixes while preserving canonical media:// references. */ +export function normalizeMediaReferenceSource(source: string): string { + const trimmed = source.trim(); + if (/^media:\/\//i.test(trimmed)) { + return trimmed; + } + return trimmed.replace(/^\s*MEDIA\s*:\s*/i, "").trim(); +} + +/** Parses canonical inbound media-store URIs and rejects nested or cross-bucket references. */ +export function parseInboundMediaUri(source: string): InboundMediaUri | null { + const normalizedSource = normalizeMediaReferenceSource(source); + if (!/^media:\/\//i.test(normalizedSource)) { + return null; + } + + let parsed: URL; + try { + parsed = new URL(normalizedSource); + } catch (err) { + throw new MediaReferenceError("invalid-path", `Invalid media URI: ${normalizedSource}`, { + cause: err, + }); + } + + if (parsed.hostname !== "inbound") { + throw new MediaReferenceError( + "path-not-allowed", + `Unsupported media URI location: ${parsed.hostname || "(missing)"}`, + ); + } + if (parsed.username || parsed.password || parsed.search || parsed.hash) { + throw new MediaReferenceError("invalid-path", `Invalid media URI: ${normalizedSource}`); + } + + let id: string; + try { + id = decodeURIComponent(parsed.pathname.replace(/^\/+/, "")); + } catch (err) { + throw new MediaReferenceError("invalid-path", `Invalid media URI: ${normalizedSource}`, { + cause: err, + }); + } + + const invalidId = !id || id === "." || id === ".."; + if (invalidId || id.includes("/") || id.includes("\\") || id.includes("\0")) { + throw new MediaReferenceError("invalid-path", `Invalid media URI: ${normalizedSource}`); + } + + return { + id, + normalizedSource, + }; +} diff --git a/src/media/media-reference.ts b/src/media/media-reference.ts index afb21ab11f39..5f804f6b1923 100644 --- a/src/media/media-reference.ts +++ b/src/media/media-reference.ts @@ -4,20 +4,18 @@ import path from "node:path"; import { safeFileURLToPath } from "@openclaw/fs-safe/advanced"; import { hasHttpUrlPrefix } from "@openclaw/net-policy/url-protocol"; import { resolveUserPath } from "../utils.js"; +import { + MediaReferenceError, + normalizeMediaReferenceSource, + parseInboundMediaUri, +} from "./inbound-media-uri.js"; import { getMediaDir, resolveMediaBufferPath } from "./store.js"; -type MediaReferenceErrorCode = "invalid-path" | "path-not-allowed"; - -/** Error raised when a media reference cannot be mapped to an allowed local media file. */ -export class MediaReferenceError extends Error { - code: MediaReferenceErrorCode; - - constructor(code: MediaReferenceErrorCode, message: string, options?: ErrorOptions) { - super(message, options); - this.code = code; - this.name = "MediaReferenceError"; - } -} +export { + MediaReferenceError, + normalizeMediaReferenceSource, + parseInboundMediaUri, +} from "./inbound-media-uri.js"; type InboundMediaReference = { id: string; @@ -26,20 +24,6 @@ type InboundMediaReference = { sourceType: "uri" | "path"; }; -type InboundMediaUri = { - id: string; - normalizedSource: string; -}; - -/** Strips legacy MEDIA: prefixes while preserving canonical media:// references. */ -export function normalizeMediaReferenceSource(source: string): string { - const trimmed = source.trim(); - if (/^media:\/\//i.test(trimmed)) { - return trimmed; - } - return trimmed.replace(/^\s*MEDIA\s*:\s*/i, "").trim(); -} - type MediaReferenceSourceInfo = { hasScheme: boolean; hasUnsupportedScheme: boolean; @@ -114,52 +98,6 @@ async function resolvePathForContainment(candidate: string): Promise { } } -/** Parses canonical inbound media-store URIs and rejects nested or cross-bucket references. */ -export function parseInboundMediaUri(source: string): InboundMediaUri | null { - const normalizedSource = normalizeMediaReferenceSource(source); - if (!/^media:\/\//i.test(normalizedSource)) { - return null; - } - - let parsed: URL; - try { - parsed = new URL(normalizedSource); - } catch (err) { - throw new MediaReferenceError("invalid-path", `Invalid media URI: ${normalizedSource}`, { - cause: err, - }); - } - - if (parsed.hostname !== "inbound") { - throw new MediaReferenceError( - "path-not-allowed", - `Unsupported media URI location: ${parsed.hostname || "(missing)"}`, - ); - } - if (parsed.username || parsed.password || parsed.search || parsed.hash) { - throw new MediaReferenceError("invalid-path", `Invalid media URI: ${normalizedSource}`); - } - - let id: string; - try { - id = decodeURIComponent(parsed.pathname.replace(/^\/+/, "")); - } catch (err) { - throw new MediaReferenceError("invalid-path", `Invalid media URI: ${normalizedSource}`, { - cause: err, - }); - } - - const invalidId = !id || id === "." || id === ".."; - if (invalidId || id.includes("/") || id.includes("\\") || id.includes("\0")) { - throw new MediaReferenceError("invalid-path", `Invalid media URI: ${normalizedSource}`); - } - - return { - id, - normalizedSource, - }; -} - /** Converts a managed inbound path to a URI without exposing paths outside its store. */ export function buildInboundMediaUriFromPath(source: string): string | undefined { const localPath = maybeLocalPathFromSource(source.trim()); diff --git a/src/media/parse-output.ts b/src/media/parse-output.ts index 5b7b4f919da5..968cc36cb12c 100644 --- a/src/media/parse-output.ts +++ b/src/media/parse-output.ts @@ -14,6 +14,7 @@ import { expectDefined } from "@openclaw/normalization-core"; import type { MarkdownImageSpan as MarkdownImageMatch } from "../../packages/markdown-core/src/image-spans.js"; import { findCodeRegions } from "../shared/text/code-regions.js"; import { parseInlineDirectives } from "../utils/directive-tags.js"; +import { parseInboundMediaUri } from "./inbound-media-uri.js"; /** Captures legacy MEDIA: attachment directives from model/tool output. */ const MEDIA_TOKEN_RE = /\bMEDIA:\s*`?([^\n]+)`?/gi; @@ -196,6 +197,14 @@ function isValidMedia( return isAllowedRemoteMediaUrl(candidate); } + if (/^media:\/\//i.test(candidate)) { + try { + return parseInboundMediaUri(candidate) !== null; + } catch { + return false; + } + } + if (isLikelyLocalPath(candidate)) { return true; } diff --git a/src/media/parse.test.ts b/src/media/parse.test.ts index 664a50d42f0d..31fc48f09d6d 100644 --- a/src/media/parse.test.ts +++ b/src/media/parse.test.ts @@ -61,6 +61,7 @@ describe("splitMediaFromOutput", () => { ["/tmp/album.v1/photo.png copy.png", "MEDIA:/tmp/album.v1/photo.png copy.png"], ["./screenshots/image.png", "MEDIA:./screenshots/image.png"], ["media/inbound/image.png", "MEDIA:media/inbound/image.png"], + ["media://inbound/image.png", "MEDIA:media://inbound/image.png"], ["./screenshot.png", " MEDIA:./screenshot.png"], ["./screenshot.png", " MEDIA:./screenshot.png"], ["./screenshot.png", " MEDIA:./screenshot.png"], @@ -88,6 +89,16 @@ describe("splitMediaFromOutput", () => { expectAcceptedMediaPathCase(expectedPath, input); }); + it.each([ + "media://outbound/image.png", + "media://inbound/nested%2Fimage.png", + "media://inbound/%00.png", + "media://inbound/image.png?token=value", + "media://inbound/", + ])("does not extract an invalid inbound URI: %s", (source) => { + expectRejectedRemoteMediaUrlCase(`MEDIA:${source}`); + }); + it.each([",", '"', "'", "\\", ")", "}", "]", "`"])( "preserves quoted URL suffix %s while cleaning ordinary unquoted punctuation", (suffix) => {