Conversation
Servers can declare `icons` in server.json and the schema has supported it for some time, but the web client never rendered them, so every card looked identical apart from its text. Closes modelcontextprotocol#784. Selection, in order: - HTTPS only. The publish API already rejects other schemes, but this UI can be pointed at a staging, local or custom API base URL from its own settings, so the scheme is re-checked at render time rather than trusted. - `theme: "dark"` icons are used only when nothing else is offered, since this UI has a light background. - Scalable icons (`sizes: ["any"]`) win outright. Otherwise the smallest icon at least as wide as the rendered size is chosen, so it stays crisp without fetching a needlessly large file. Rendering notes: - `src` goes through the existing `escapeAttr`, so a quote in a URL cannot break out of the attribute. - `alt` is empty on purpose. The icon sits directly beside the server name, so announcing it again is noise for screen readers. - `loading="lazy"` keeps a grid of cards from fetching every icon at once, and `referrerpolicy="no-referrer"` avoids leaking registry URLs to icon hosts. - A broken or blocked image removes itself instead of leaving a placeholder. Verified against the first 100 servers in the production registry, which exercises every shape currently published: no `sizes` field, a single 512x512, `["any"]`, and a 256/512 pair. Also checked that http, `javascript:`, `data:` and null sources render nothing, and that a quote injected into `src` is escaped. Unit tests pass, including internal/api/handlers/v0, which is the package that embeds this file. `make validate` and `make lint` both fail identically on a clean checkout of upstream main with these changes absent, so those failures are pre-existing rather than introduced here; the lint ones are gofmt objecting to CRLF from a Windows checkout. Tests ran without `-race`, which needs cgo and a C toolchain this machine does not have.
|
Following up after three weeks, since this hasn't picked up a reviewer yet. It's still a single file at +54 lines and it closes #784. Equally happy to narrow the scope if the icon-selection rules are more than you want in the web client right now. The core of the change is just rendering Let me know either way and I'll turn it around quickly. |
|
Flagging an overlap on my own PR so a maintainer doesn't have to find it: #1208 implements this same feature and has been open since April. I missed it when I opened this one. The two overlap almost entirely, and each has one thing the other lacks:
Both enforce HTTPS at render time, escape the #1208 came first and is the better base of the two. If that's the direction you'd prefer, I'm happy to close this and send the size-selection logic as a follow-up on top of it, or as a patch to that branch if @MukundaKatta would rather take it that way. Either is fine by me, I'd just rather one of these land than both sit. |
Closes #784.
Servers can declare
iconsinserver.jsonand the schema has supported it for some time, but the web client never rendered them, so every card looks identical apart from its text. This renders them in both the "Recently Updated" cards and the main search results.Selection
validateIconalready rejects other schemes at publish, but this UI can be pointed at a staging, local or custom API base URL from its own settings, so the scheme is re-checked rather than trusted.sizes: ["any"]). Otherwise the smallest icon at least as wide as the rendered size, so it stays crisp without fetching a needlessly large file.theme: "dark"is a fallback only, used when nothing else is offered, since this UI has a light background.Rendering
srcgoes through the existingescapeAttr, so a quote in a URL cannot break out of the attribute.altis empty on purpose: the icon sits directly beside the server name, so announcing it again is noise for screen readers.loading="lazy"stops a full grid of cards fetching every icon at once.referrerpolicy="no-referrer"avoids leaking registry URLs to third-party icon hosts.SVG is rendered via
<img src>, where scripts do not execute, which is the safe path for theimage/svg+xmlcase the schema warns about.Testing
Ran the selection logic against the first 100 servers in the production registry. That covers every icon shape currently published:
sizesfield512x512512x512["any"](SVG)256x256+512x512256x25696x9696x96Also verified that
http://,javascript:,data:and null sources render nothing, that a dark-only icon set still renders, that light is preferred when both are offered, and that a quote injected intosrcis escaped rather than breaking the attribute.Unit tests pass, including
internal/api/handlers/v0, the package that embeds this file.Two pre-existing failures, noted for transparency:
make validateandmake lintboth fail identically on a clean checkout ofmainwith these changes absent. The lint ones aregofmtobjecting to CRLF from a Windows checkout. Neither is introduced here and neither is touched by this change. Tests ran without-race, which needs cgo and a C toolchain unavailable on my machine.