Skip to content

feat(ui): image/PDF upload + media rendering in chat; multimodal docs (#255) - #538

Open
initializ-mk wants to merge 2 commits into
mainfrom
feat/multimodal-ui
Open

initializ-mk wants to merge 2 commits into
mainfrom
feat/multimodal-ui

Conversation

@initializ-mk

Copy link
Copy Markdown
Contributor

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 — file parts always existed; this is the UI + docs surface.

forge-ui

  • Backend (chat.go, types.go): ChatRequest gains Attachments []ChatAttachment (name, mimeType, base64 data). buildChatParts projects the text + one file part per attachment onto the outbound A2A message; the base64 passes straight through (it's exactly what the agent's FileContent.Bytes decodes from). The "message required" guard is relaxed to "message or attachment".
  • Frontend (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 (MediaPart component).
  • CSS: media + attachment-chip styling.

Docs

  • New 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

buildChatParts projection (text / text+image / attachment-only) and the relaxed message-or-attachment guard. go build + go test ./... (forge-ui) green; no go.sum churn.

Notes

  • The accept attribute + validation enforce types/size before upload, so the user gets instant feedback instead of a server 4xx.
  • Inline bytes only (matches the runtime — URI-fetch is a tracked follow-up).
  • Frontend rendering is htm/preact following the existing MessageBubble patterns; not unit-tested (no browser harness) — the backend projection is.

…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 initializ-mk left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 buildChatParts is a gated pass-through — attachments route through the agent's sendSubscribe, so the runtime checkInboundMedia (type/size/count) is the real enforcement; client-side validation is UX-only (correctly framed).
  • Doc branch uses download (neutralizes data:text/html navigation); name/alt/download are 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.

Comment thread forge-ui/static/app.js Outdated
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/')) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
@initializ-mk

Copy link
Copy Markdown
Contributor Author

Fixed the SVG XSS in 7106ce4.

MediaPart's inline branch is now gated to the raster types the server accepts — ACCEPTED_IMAGE_TYPES (png/jpeg/gif/webp) — instead of mt.startsWith('image/'). So image/svg+xml (and any other image/*) falls through to the download chip, where download forces a save rather than a top-level data: navigation that would execute the SVG's scripts.

Your reachability analysis was the key point: uploads can't be SVG (the inbound IsImageMIME gate rejects it), but MediaPart also renders agent-reply message.files, which have no such allowlist — so a custom agent/tool returning an image/svg+xml part was the live vector. Gating in MediaPart closes it regardless of source (defense in depth), not relying on browsers blocking top-level data: nav. Added a comment documenting exactly why only raster types get the clickable link.

Raster data: links (png/jpeg/gif/webp) stay clickable — opened top-level they're images, not script. The document branch was already safe via download.

Go build/tests still green (JS is static, not compiled).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant