feat(ui): image/PDF upload + media rendering in chat; multimodal docs (#255) - #538
initializ-mk wants to merge 2 commits into
Conversation
…al docs (#255) Wire the forge dashboard chat to the multimodal A2A I/O already supported by the runtime (no A2A schema change — file parts always existed). forge-ui: - ChatRequest gains Attachments []ChatAttachment (name, mimeType, base64 data); buildChatParts projects the text + a `file` part per attachment onto the outbound A2A message. The attachment data is standard-base64 — exactly what the agent's FileContent.Bytes decodes from — so it passes straight through. The "message required" guard is relaxed to "message or attachment". - app.js: a 📎 attach button on the composer with client-side validation matching the server gate — accepted types (PNG/JPEG/GIF/WebP, PDF), per-file size caps (image 5 MiB, PDF 24 MiB effective), and per-message counts (20 images / 5 docs). Pending attachments show as removable chips. Received media renders in the bubble: images inline, documents as download links — for both the user's message and the agent's reply (MediaPart component). - style.css: media + attachment-chip styling. docs: new reference/multimodal-io.md (A2A shapes, input/output examples, capability matrix + limits, persistence, UI); web-dashboard.md chat rows; README index link. Tests: buildChatParts projection (text/image/attachment-only) + the relaxed message-or-attachment guard.
initializ-mk
left a comment
There was a problem hiding this comment.
Self-review — verified against source ✅ one requested change (XSS)
UI multimodal surface; posting as a COMMENT (own PR) with one XSS fix.
Verified good:
- Backend
buildChatPartsis a gated pass-through — attachments route through the agent'ssendSubscribe, so the runtimecheckInboundMedia(type/size/count) is the real enforcement; client-side validation is UX-only (correctly framed). - Doc branch uses
download(neutralizesdata:text/htmlnavigation);name/alt/downloadare htm-escaped (no attribute-injection); projection tests genuine.
Requesting one change (LOW/MEDIUM XSS): allowlist the inline-image mime — inline below. The image branch renders any image/* as a clickable top-level data: link; an agent-returned image/svg+xml would execute script on click where the browser permits top-level data: navigation.
| return html`<div class="chat-file"><span class="chat-file-icon">\u{1F4CE}</span>${name || mt || 'file'}</div>`; | ||
| } | ||
| const src = `data:${mt || 'application/octet-stream'};base64,${b64}`; | ||
| if (mt.startsWith('image/')) { |
There was a problem hiding this comment.
Requesting change (LOW/MEDIUM XSS): allowlist the inline-image mime; don't make SVG a navigable data: link. The <img src=data:…> is safe even for SVG (img context), but the wrapping <a href="data:image/svg+xml;base64,…" target="_blank"> is a top-level navigation — a data:image/svg+xml opened as a top-level document runs its scripts.\n\nReachability: the inbound gate rejects SVG (IsImageMIME = png/jpeg/gif/webp), so user uploads / Phase-5 output can't be SVG — but MediaPart also renders agent-reply file parts (message.files) with no mime allowlist, so a custom agent/tool returning an image/svg+xml part is rendered as a clickable SVG link → click → script. Modern Chrome/Firefox block top-level data: nav (mitigating), but relying on that is fragile.\n\nFix: gate the inline branch to the raster types the server accepts — if (['image/png','image/jpeg','image/gif','image/webp'].includes(mt)) — and let image/svg+xml (and any other image/*) fall through to the download chip, where download neutralizes it. (I confirmed the vector is SVG-specific: a crafted data:image/png,<script> is served as image/png and not executed; the doc branch is already safe via download.)
review) MediaPart wrapped every image/* in a top-level `data:` link (target=_blank). A data:image/svg+xml opened as a top-level document executes its scripts, so an agent-reply file part with mimeType image/svg+xml was a clickable XSS vector (message.files isn't gated by the inbound checkInboundMedia allowlist the way uploads are). Gate the inline <img> + link branch to the raster types the server accepts (ACCEPTED_IMAGE_TYPES: png/jpeg/gif/webp); svg and any other image/* fall through to the download chip, where `download` neutralizes it.
|
Fixed the SVG XSS in
Your reachability analysis was the key point: uploads can't be SVG (the inbound Raster Go build/tests still green (JS is static, not compiled). |
Wires the forge dashboard chat to the multimodal A2A I/O the runtime already supports (Phases 2–5), and adds a reference doc. No A2A schema change —
fileparts always existed; this is the UI + docs surface.forge-ui
chat.go,types.go):ChatRequestgainsAttachments []ChatAttachment(name,mimeType, base64data).buildChatPartsprojects the text + onefilepart per attachment onto the outbound A2A message; the base64 passes straight through (it's exactly what the agent'sFileContent.Bytesdecodes from). The "message required" guard is relaxed to "message or attachment".app.js): a 📎 attach button on the composer with client-side validation mirroring the server gate — accepted types (PNG/JPEG/GIF/WebP, PDF), per-file size caps (image 5 MiB, PDF 24 MiB effective), per-message counts (20 images / 5 docs), with inline error messages. Pending attachments show as removable chips. Received media renders in the bubble — images inline, documents as download links — for both the user's message and the agent's reply (MediaPartcomponent).Docs
docs/reference/multimodal-io.md— A2A shapes, input/output JSON examples (JSON-RPC + REST), capability matrix, ingest limits, persistence, and the UI. Linked from README +web-dashboard.md.Tests
buildChatPartsprojection (text / text+image / attachment-only) and the relaxed message-or-attachment guard.go build+go test ./...(forge-ui) green; nogo.sumchurn.Notes
acceptattribute + validation enforce types/size before upload, so the user gets instant feedback instead of a server 4xx.bytesonly (matches the runtime — URI-fetch is a tracked follow-up).MessageBubblepatterns; not unit-tested (no browser harness) — the backend projection is.