mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-22 18:35:21 -06:00
fix(ui,gateway): single attachment admission funnel + WS-frame-budget clamp on advertised ceiling (#123977)
* fix(ui,gateway): make attachment admission one funnel and clamp advertised ceiling to the WS frame budget Three residual gaps from the #123654 size-limit fix shared one root cause: attachment admission policy was scattered instead of owned. - hello-ok advertised the decoded config ceiling (20MB default, higher with mediaMaxMb) without accounting for base64 4/3 expansion against the 25MiB WS frame cap, so a 19.6-20MB attachment passed the client guard and the encoded chat.send frame still hard-dropped the connection (1009) for every pane. The policy owner now clamps the advertised maxBytes to what one frame can carry. - Large-text paste, data-URL image paste, and browser-annotation handoff constructed attachments without any size check, bypassing the guard that only lived inline in appendAttachmentFiles. All intake paths now share one admission funnel (chat-attachment-admission.ts). - Zero-byte files rendered a normal chip, then the payload assembler silently dropped them on send; the funnel rejects them at intake with a named toast. * refactor(ui): drop unused exported type from attachment admission module
This commit is contained in:
committed by
GitHub
parent
e26bae5b2a
commit
8739432d4c
@@ -46,14 +46,27 @@ describe("resolveChatAttachmentMaxBytes", () => {
|
||||
});
|
||||
});
|
||||
|
||||
// Frame budget mirrored from the policy module: base64 expands 4/3 and the
|
||||
// JSON envelope needs slack, so the advertised ceiling must fit one WS frame.
|
||||
const MAX_ADVERTISED_BYTES = Math.floor(((25 * MB - 256 * 1024) * 3) / 4);
|
||||
|
||||
describe("resolveChatAttachmentPolicy", () => {
|
||||
it("advertises the configured ceiling with the image hydration cap applied", () => {
|
||||
expect(resolveChatAttachmentPolicy(cfgWithMediaMaxMb(20))).toEqual({
|
||||
maxBytes: 20 * MB,
|
||||
expect(resolveChatAttachmentPolicy(cfgWithMediaMaxMb(10))).toEqual({
|
||||
maxBytes: 10 * MB,
|
||||
maxImageBytes: MAX_IMAGE_BYTES,
|
||||
});
|
||||
});
|
||||
|
||||
it("clamps the advertised ceiling to what one WS frame can carry as base64", () => {
|
||||
// The 20MB default and any raised mediaMaxMb both exceed the frame budget:
|
||||
// advertising them would let the client encode a frame the server
|
||||
// hard-drops with 1009.
|
||||
expect(resolveChatAttachmentPolicy({} as OpenClawConfig).maxBytes).toBe(MAX_ADVERTISED_BYTES);
|
||||
expect(resolveChatAttachmentPolicy(cfgWithMediaMaxMb(50)).maxBytes).toBe(MAX_ADVERTISED_BYTES);
|
||||
expect(MAX_ADVERTISED_BYTES).toBeLessThan(20 * MB);
|
||||
});
|
||||
|
||||
it("clamps maxImageBytes to the configured ceiling when it is the smaller limit", () => {
|
||||
expect(resolveChatAttachmentPolicy(cfgWithMediaMaxMb(1))).toEqual({
|
||||
maxBytes: MB,
|
||||
|
||||
@@ -3,9 +3,19 @@
|
||||
// does not pull the media probe/store graph in just to read two numbers.
|
||||
import { MAX_IMAGE_BYTES } from "@openclaw/media-core/constants";
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import { MAX_PAYLOAD_BYTES } from "./server-constants.js";
|
||||
|
||||
const DEFAULT_CHAT_ATTACHMENT_MAX_MB = 20;
|
||||
|
||||
// A chat.send frame carries attachments as base64 (4/3 expansion) plus the
|
||||
// JSON envelope and message text. Advertising more than one WS frame can carry
|
||||
// lets the client encode a payload the server hard-drops with 1009 for every
|
||||
// pane — the exact failure the hello-ok policy exists to prevent.
|
||||
const WS_FRAME_ENVELOPE_SLACK_BYTES = 256 * 1024;
|
||||
const MAX_ADVERTISED_ATTACHMENT_BYTES = Math.floor(
|
||||
((MAX_PAYLOAD_BYTES - WS_FRAME_ENVELOPE_SLACK_BYTES) * 3) / 4,
|
||||
);
|
||||
|
||||
/** Default decoded-size ceiling when `agents.defaults.mediaMaxMb` is unset or invalid. */
|
||||
export const DEFAULT_CHAT_ATTACHMENT_MAX_BYTES = DEFAULT_CHAT_ATTACHMENT_MAX_MB * 1024 * 1024;
|
||||
|
||||
@@ -37,6 +47,6 @@ type ChatAttachmentPolicy = {
|
||||
* cannot be stated once per connection.
|
||||
*/
|
||||
export function resolveChatAttachmentPolicy(cfg: OpenClawConfig): ChatAttachmentPolicy {
|
||||
const maxBytes = resolveChatAttachmentMaxBytes(cfg);
|
||||
const maxBytes = Math.min(resolveChatAttachmentMaxBytes(cfg), MAX_ADVERTISED_ATTACHMENT_BYTES);
|
||||
return { maxBytes, maxImageBytes: Math.min(maxBytes, MAX_IMAGE_BYTES) };
|
||||
}
|
||||
|
||||
@@ -40,7 +40,11 @@ export function receiveBrowserAnnotation(
|
||||
(event as BrowserAnnotationEvent).rejection = "limit";
|
||||
return false;
|
||||
}
|
||||
const attachment = chatAttachmentFromDataUrl(detail.dataUrl, detail.fileName || "annotation");
|
||||
const attachment = chatAttachmentFromDataUrl(
|
||||
detail.dataUrl,
|
||||
detail.fileName || "annotation",
|
||||
state.hello?.policy?.attachments,
|
||||
);
|
||||
if (!attachment) {
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,39 @@
|
||||
// Single admission funnel for every composer attachment intake path (file
|
||||
// input, drop, image paste, large-text paste, data-URL paste, annotation
|
||||
// handoff). Enforces the hello-advertised decoded-size ceilings before
|
||||
// encoding — an oversized base64 frame would exceed the gateway's WS payload
|
||||
// cap and hard-drop the whole connection (1009) for every pane — and rejects
|
||||
// zero-byte files, which the payload assembler would otherwise drop silently
|
||||
// after send.
|
||||
import { t } from "../../../i18n/index.ts";
|
||||
import { showToast } from "../../../lib/toast.ts";
|
||||
|
||||
function skippedFilesToast(messageKey: string, skipped: readonly File[]): void {
|
||||
if (skipped.length === 0) {
|
||||
return;
|
||||
}
|
||||
showToast({
|
||||
message: t(messageKey, {
|
||||
names: skipped
|
||||
.slice(0, 3)
|
||||
.map((file) => file.name)
|
||||
.join(", "),
|
||||
more: skipped.length > 3 ? ` +${skipped.length - 3}` : "",
|
||||
}),
|
||||
});
|
||||
}
|
||||
|
||||
export function admitAttachmentFiles(
|
||||
candidates: readonly File[],
|
||||
limits: { maxBytes: number; maxImageBytes: number } | undefined,
|
||||
): File[] {
|
||||
const fileLimit = (file: File) =>
|
||||
file.type.startsWith("image/") ? limits?.maxImageBytes : limits?.maxBytes;
|
||||
const empty = candidates.filter((file) => file.size === 0);
|
||||
const oversized = candidates.filter(
|
||||
(file) => file.size > 0 && limits !== undefined && file.size > (fileLimit(file) ?? Infinity),
|
||||
);
|
||||
skippedFilesToast("chat.attachments.readFailed", empty);
|
||||
skippedFilesToast("chat.attachments.tooLarge", oversized);
|
||||
return candidates.filter((file) => !empty.includes(file) && !oversized.includes(file));
|
||||
}
|
||||
@@ -121,6 +121,68 @@ describe("chat attachment read failures", () => {
|
||||
expect(onAttachmentsChange).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects a zero-byte file instead of silently dropping it after send", async () => {
|
||||
const onAttachmentsChange = vi.fn();
|
||||
handleChatAttachmentPaste(
|
||||
pasteEventWithFiles([new File([], "empty.png", { type: "image/png" })]),
|
||||
{ attachments: [], onAttachmentsChange },
|
||||
);
|
||||
await toastHost.updateComplete;
|
||||
await vi.waitFor(() => {
|
||||
expect(toastHost.querySelector(".app-toast__message")?.textContent).toContain("empty.png");
|
||||
});
|
||||
expect(onAttachmentsChange).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("blocks a large text paste that exceeds the non-image ceiling", async () => {
|
||||
const onAttachmentsChange = vi.fn();
|
||||
const text = "x".repeat(2048);
|
||||
handleChatAttachmentPaste(
|
||||
{
|
||||
preventDefault: () => {},
|
||||
clipboardData: {
|
||||
items: [],
|
||||
getData: (type: string) => (type === "text/plain" ? text : ""),
|
||||
},
|
||||
} as unknown as ClipboardEvent,
|
||||
{
|
||||
attachmentLimits: { maxBytes: 1024, maxImageBytes: 1024 },
|
||||
attachments: [],
|
||||
onAttachmentsChange,
|
||||
},
|
||||
);
|
||||
await toastHost.updateComplete;
|
||||
await vi.waitFor(() => {
|
||||
expect(toastHost.querySelector(".app-toast__message")?.textContent).toContain("pasted-text");
|
||||
});
|
||||
expect(onAttachmentsChange).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("blocks a pasted data-URL image that exceeds the image ceiling", async () => {
|
||||
const onAttachmentsChange = vi.fn();
|
||||
const bigBase64 = btoa("p".repeat(64));
|
||||
handleChatAttachmentPaste(
|
||||
{
|
||||
preventDefault: () => {},
|
||||
clipboardData: {
|
||||
items: [],
|
||||
getData: (type: string) =>
|
||||
type === "text/plain" ? `data:image/png;base64,${bigBase64}` : "",
|
||||
},
|
||||
} as unknown as ClipboardEvent,
|
||||
{
|
||||
attachmentLimits: { maxBytes: 1024, maxImageBytes: 16 },
|
||||
attachments: [],
|
||||
onAttachmentsChange,
|
||||
},
|
||||
);
|
||||
await toastHost.updateComplete;
|
||||
await vi.waitFor(() => {
|
||||
expect(toastHost.querySelector(".app-toast__message")?.textContent).toContain("pasted-image");
|
||||
});
|
||||
expect(onAttachmentsChange).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not toast when every read succeeds", async () => {
|
||||
const onAttachmentsChange = vi.fn();
|
||||
handleChatAttachmentPaste(
|
||||
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
registerChatAttachmentPayload,
|
||||
releaseChatAttachmentPayload,
|
||||
} from "../attachment-payload-store.ts";
|
||||
import { admitAttachmentFiles } from "./chat-attachment-admission.ts";
|
||||
|
||||
const CHAT_ATTACHMENT_ACCEPT =
|
||||
"image/*,audio/*,video/*,application/pdf,text/*,.csv,.json,.md,.txt,.zip," +
|
||||
@@ -138,10 +139,7 @@ function encodeTextAsDataUrl(text: string): string {
|
||||
return `data:${LARGE_PASTE_TEXT_MIME_TYPE};base64,${btoa(chunks.join(""))}`;
|
||||
}
|
||||
|
||||
function createLargePastedTextAttachment(text: string): ChatAttachment {
|
||||
const file = new File([text], `${LARGE_PASTE_TEXT_FILE_PREFIX}${Date.now()}.txt`, {
|
||||
type: LARGE_PASTE_TEXT_MIME_TYPE,
|
||||
});
|
||||
function createLargePastedTextAttachment(text: string, file: File): ChatAttachment {
|
||||
const attachment = chatAttachmentFromFile(file, encodeTextAsDataUrl(text));
|
||||
largePastedTextAttachments.add(attachment);
|
||||
const preview = compactPastedTextPreview(text);
|
||||
@@ -210,7 +208,14 @@ function handleLargeTextPaste(e: ClipboardEvent, props: ChatAttachmentControlsPr
|
||||
return false;
|
||||
}
|
||||
e.preventDefault();
|
||||
const attachment = createLargePastedTextAttachment(text);
|
||||
const file = new File([text], `${LARGE_PASTE_TEXT_FILE_PREFIX}${Date.now()}.txt`, {
|
||||
type: LARGE_PASTE_TEXT_MIME_TYPE,
|
||||
});
|
||||
if (admitAttachmentFiles([file], props.attachmentLimits).length === 0) {
|
||||
// The rejection toast named the file; the clipboard still holds the text.
|
||||
return true;
|
||||
}
|
||||
const attachment = createLargePastedTextAttachment(text, file);
|
||||
props.onAttachmentsChange([...currentAttachments(props), attachment]);
|
||||
return true;
|
||||
}
|
||||
@@ -249,10 +254,14 @@ function dataImageClipboardFile(
|
||||
export function chatAttachmentFromDataUrl(
|
||||
dataUrl: string,
|
||||
fileName: string,
|
||||
limits?: ChatAttachmentControlsProps["attachmentLimits"],
|
||||
): ChatAttachment | null {
|
||||
const baseName = fileName.replace(/\.[a-z0-9]+$/i, "") || "image";
|
||||
const parsed = dataImageClipboardFile(dataUrl, baseName);
|
||||
return parsed ? chatAttachmentFromFile(parsed.file, parsed.dataUrl) : null;
|
||||
if (!parsed || admitAttachmentFiles([parsed.file], limits).length === 0) {
|
||||
return null;
|
||||
}
|
||||
return chatAttachmentFromFile(parsed.file, parsed.dataUrl);
|
||||
}
|
||||
|
||||
function readAttachmentFile(
|
||||
@@ -301,27 +310,7 @@ async function appendAttachmentFiles(
|
||||
if (!props.onAttachmentsChange || candidates.length === 0) {
|
||||
return;
|
||||
}
|
||||
// Enforce the hello-advertised decoded-size ceilings up front: an oversized
|
||||
// base64 frame would exceed the gateway's WS payload cap and hard-drop the
|
||||
// whole connection (1009) for every pane, so it must never start encoding.
|
||||
const limits = props.attachmentLimits;
|
||||
const fileLimit = (file: File) =>
|
||||
file.type.startsWith("image/") ? limits?.maxImageBytes : limits?.maxBytes;
|
||||
const oversized = limits
|
||||
? candidates.filter((file) => file.size > (fileLimit(file) ?? Infinity))
|
||||
: [];
|
||||
if (oversized.length > 0) {
|
||||
showToast({
|
||||
message: t("chat.attachments.tooLarge", {
|
||||
names: oversized
|
||||
.slice(0, 3)
|
||||
.map((file) => file.name)
|
||||
.join(", "),
|
||||
more: oversized.length > 3 ? ` +${oversized.length - 3}` : "",
|
||||
}),
|
||||
});
|
||||
}
|
||||
const files = limits ? candidates.filter((file) => !oversized.includes(file)) : [...candidates];
|
||||
const files = admitAttachmentFiles(candidates, props.attachmentLimits);
|
||||
if (files.length === 0) {
|
||||
return;
|
||||
}
|
||||
@@ -378,6 +367,9 @@ export function handleChatAttachmentPaste(e: ClipboardEvent, props: ChatAttachme
|
||||
return;
|
||||
}
|
||||
e.preventDefault();
|
||||
if (admitAttachmentFiles([pasted.file], props.attachmentLimits).length === 0) {
|
||||
return;
|
||||
}
|
||||
props.onAttachmentsChange([
|
||||
...currentAttachments(props),
|
||||
chatAttachmentFromFile(pasted.file, pasted.dataUrl),
|
||||
|
||||
Reference in New Issue
Block a user