mirror of
https://github.com/openclaw/openclaw.git
synced 2026-09-28 05:54:09 +08:00
fix: resend inbound attachments referenced from chat history (#158720)
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/<actual-id>]` 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 <hi@obviy.us>
This commit is contained in:
co-authored by
Ayaan Zaidi
parent
a4498e9425
commit
fc62052f57
@@ -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/<id>` 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.
|
||||
|
||||
<Warning>
|
||||
|
||||
@@ -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<PreparedReplyMediaSource> => {
|
||||
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 }],
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
}
|
||||
@@ -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<string> {
|
||||
}
|
||||
}
|
||||
|
||||
/** 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());
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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) => {
|
||||
|
||||
Reference in New Issue
Block a user