refactor(renderer): give session images one unavailable state, off the DOM

A source the Markdown renderer refuses now goes through the same element
with no src rather than a separate light-DOM span, so a blocked source and
a source that fails to load are one piece of UI instead of two. That drops
the span's stylesheet rule and the only reason the renderer built elements
just to serialise them.

Decoding what marked escaped no longer runs untrusted markup through a
detached element's innerHTML; the entities marked actually emits are
decoded in one pass, and attributes are escaped on the way out. With the
DOM dependency gone the renderer is directly unit-testable, so the escaping
and the protocol allowlist now have coverage that does not need Electron.

The element's shadow styles also stop hard-coding values that already exist
as tokens -- custom properties cross the shadow boundary, so --muted and
--hairline-strong are now the single source of truth -- and the height cap
becomes --session-image-max-block, which compact Markdown surfaces (subagent
panes, memory rows, tool results) lower from a viewport fraction to 240px.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
tommy0103
2026-08-03 02:10:50 +08:00
co-authored by Claude Opus 5
parent a9e6d1bab8
commit 1124758b6b
4 changed files with 125 additions and 42 deletions
@@ -1,18 +1,25 @@
<script setup> <script setup>
import { computed, ref } from 'vue'; import { computed, ref, watch } from 'vue';
import { SESSION_IMAGE_SETTLED_EVENT } from '../session-image-contract.js'; import { SESSION_IMAGE_SETTLED_EVENT } from '../session-image-contract.js';
defineOptions({ name: 'SessionImage' }); defineOptions({ name: 'SessionImage' });
const props = defineProps({ const props = defineProps({
src: { type: String, required: true }, // Absent when the Markdown renderer refused the source. The element still
// renders, in its error state, so a blocked source and a source that fails
// to load are one piece of UI rather than two.
src: { type: String, default: '' },
alt: { type: String, default: '' }, alt: { type: String, default: '' },
title: { type: String, default: '' }, title: { type: String, default: '' },
}); });
const status = ref('loading'); const status = ref(props.src ? 'loading' : 'error');
const accessibleLabel = computed(() => props.alt || props.title || 'Session image'); const accessibleLabel = computed(() => props.alt || props.title || 'Session image');
watch(() => props.src, source => {
status.value = source ? 'loading' : 'error';
});
// Announce synchronously, before the resize observation this growth triggers, // Announce synchronously, before the resize observation this growth triggers,
// so the timeline already knows the row is about to change size for a reason // so the timeline already knows the row is about to change size for a reason
// the reader did not cause. // the reader did not cause.
@@ -57,6 +64,10 @@ function handleError(event) {
</template> </template>
<style> <style>
/* Custom properties cross the shadow boundary, so the app's tokens are the
single source of truth for these colours. --session-image-max-block lets a
host context (a compact Markdown block, say) cap the image lower than the
session timeline does. */
:host { :host {
display: block; display: block;
max-inline-size: 100%; max-inline-size: 100%;
@@ -70,9 +81,9 @@ function handleError(event) {
min-inline-size: 0; min-inline-size: 0;
margin: 0; margin: 0;
overflow: clip; overflow: clip;
border: 1px solid rgba(255, 255, 255, 0.08); border: 1px solid var(--hairline-strong);
border-radius: 6px; border-radius: 6px;
background: rgba(0, 0, 0, 0.22); background: var(--session-image-backdrop, rgba(0, 0, 0, 0.22));
} }
img { img {
@@ -80,9 +91,9 @@ img {
inline-size: auto; inline-size: auto;
max-inline-size: 100%; max-inline-size: 100%;
block-size: auto; block-size: auto;
max-block-size: min(70vh, 720px); max-block-size: var(--session-image-max-block, min(70vh, 720px));
object-fit: contain; object-fit: contain;
color: rgba(255, 255, 255, 0.48); color: var(--muted);
} }
.is-loading { .is-loading {
@@ -95,8 +106,8 @@ figcaption {
align-items: center; align-items: center;
gap: 6px; gap: 6px;
padding: 10px 12px; padding: 10px 12px;
color: rgba(255, 255, 255, 0.48); color: var(--muted);
font: 12px/1.5 ui-monospace, 'SF Mono', Menlo, monospace; font: var(--text-sm, 12px)/1.5 var(--font-mono);
} }
.image-label::before { .image-label::before {
+43 -18
View File
@@ -1,17 +1,49 @@
import { SESSION_IMAGE_TAG } from './session-image-contract.js'; import { SESSION_IMAGE_TAG } from './session-image-contract.js';
const SAFE_IMAGE_PROTOCOLS = new Set(['blob:', 'file:', 'http:', 'https:']); const SAFE_IMAGE_PROTOCOLS = new Set(['blob:', 'file:', 'http:', 'https:']);
const NAMED_ENTITIES = {
amp: '&',
lt: '<',
gt: '>',
quot: '"',
apos: "'",
nbsp: ' ',
};
let configuredMarked = null; let configuredMarked = null;
// marked escapes the alt text and title it hands to the renderer, and the
// attributes are re-escaped on the way back out, so they have to be decoded
// once in between. Doing that through a detached element's innerHTML would
// mean parsing untrusted markup, so decode the entities marked actually emits
// in a single pass instead -- one pass, so `&amp;lt;` stays `&lt;`.
function decodeMarkedAttribute(value) { function decodeMarkedAttribute(value) {
const decoder = document.createElement('textarea'); return String(value ?? '').replace(/&(#\d+|#x[0-9a-f]+|[a-z]+);/gi, (whole, entity) => {
decoder.innerHTML = String(value ?? ''); if (entity[0] === '#') {
return decoder.value; const codePoint = entity[1] === 'x' || entity[1] === 'X'
? Number.parseInt(entity.slice(2), 16)
: Number.parseInt(entity.slice(1), 10);
if (!Number.isInteger(codePoint) || codePoint < 0 || codePoint > 0x10ffff) return whole;
try {
return String.fromCodePoint(codePoint);
} catch {
return whole;
}
}
return NAMED_ENTITIES[entity.toLowerCase()] ?? whole;
});
}
function escapeAttribute(value) {
return String(value ?? '')
.replace(/&/g, '&amp;')
.replace(/"/g, '&quot;')
.replace(/</g, '&lt;')
.replace(/>/g, '&gt;');
} }
function isSafeImageSource(source) { function isSafeImageSource(source) {
try { try {
const url = new URL(source, document.baseURI); const url = new URL(source, globalThis.document?.baseURI ?? 'file:///');
return SAFE_IMAGE_PROTOCOLS.has(url.protocol) return SAFE_IMAGE_PROTOCOLS.has(url.protocol)
|| (url.protocol === 'data:' && /^data:image\//i.test(source)); || (url.protocol === 'data:' && /^data:image\//i.test(source));
} catch { } catch {
@@ -19,13 +51,6 @@ function isSafeImageSource(source) {
} }
} }
function imageFallback(alt) {
const fallback = document.createElement('span');
fallback.className = 'session-image-fallback';
fallback.textContent = alt || 'Image unavailable';
return fallback.outerHTML;
}
// marked <= 14 calls renderer.image(href, title, text); marked >= 15 passes the // marked <= 14 calls renderer.image(href, title, text); marked >= 15 passes the
// image token instead. Accepting both keeps an upgrade from silently turning // image token instead. Accepting both keeps an upgrade from silently turning
// every session image into fallback text. // every session image into fallback text.
@@ -40,18 +65,18 @@ export function normalizeMarkdownImageToken(hrefOrToken, title, text) {
return { href: hrefOrToken ?? '', title: title ?? '', text: text ?? '' }; return { href: hrefOrToken ?? '', title: title ?? '', text: text ?? '' };
} }
// A source the app will not load is rendered through the same element with no
// src, so a blocked image and an image that fails to load look the same to the
// reader instead of being two different pieces of UI.
export function renderSessionMarkdownImage(hrefOrToken, title, text) { export function renderSessionMarkdownImage(hrefOrToken, title, text) {
const token = normalizeMarkdownImageToken(hrefOrToken, title, text); const token = normalizeMarkdownImageToken(hrefOrToken, title, text);
const source = decodeMarkedAttribute(token.href).trim(); const source = decodeMarkedAttribute(token.href).trim();
const alt = decodeMarkedAttribute(token.text); const alt = decodeMarkedAttribute(token.text);
const accessibleTitle = decodeMarkedAttribute(token.title); const accessibleTitle = decodeMarkedAttribute(token.title);
if (!source || !isSafeImageSource(source)) return imageFallback(alt); const attributes = [`alt="${escapeAttribute(alt)}"`];
if (source && isSafeImageSource(source)) attributes.unshift(`src="${escapeAttribute(source)}"`);
const image = document.createElement(SESSION_IMAGE_TAG); if (accessibleTitle) attributes.push(`title="${escapeAttribute(accessibleTitle)}"`);
image.setAttribute('src', source); return `<${SESSION_IMAGE_TAG} ${attributes.join(' ')}></${SESSION_IMAGE_TAG}>`;
image.setAttribute('alt', alt);
if (accessibleTitle) image.setAttribute('title', accessibleTitle);
return image.outerHTML;
} }
export function configureMarkdownImages(marked) { export function configureMarkdownImages(marked) {
+5 -14
View File
@@ -363,20 +363,11 @@
.markdown-msg th, .markdown-msg td { border: 1px solid var(--hairline); padding: 5px 9px; text-align: left; } .markdown-msg th, .markdown-msg td { border: 1px solid var(--hairline); padding: 5px 9px; text-align: left; }
.markdown-msg th { background: rgba(255,255,255,0.04); font-weight: 600; } .markdown-msg th { background: rgba(255,255,255,0.04); font-weight: 600; }
.markdown-msg mark { background: var(--accent-soft); color: var(--accent-2); padding: 0 2px; border-radius: 2px; } .markdown-msg mark { background: var(--accent-soft); color: var(--accent-2); padding: 0 2px; border-radius: 2px; }
.session-image-fallback {
display: block; /* Session images cap themselves against the reading column. Compact Markdown
max-width: 100%; renders in much smaller surfaces (subagent panes, memory rows, tool results),
margin: 0.7em 0; where a viewport-sized image would swamp the block it belongs to. */
padding: 10px 12px; .markdown-compact { --session-image-max-block: 240px; }
overflow: hidden;
border: 1px solid var(--hairline-strong);
border-radius: 6px;
color: var(--muted);
background: rgba(0,0,0,0.22);
font: 12px/1.5 var(--font-mono);
text-overflow: ellipsis;
white-space: nowrap;
}
.detail-section-divider { .detail-section-divider {
display: flex; align-items: center; gap: 10px; display: flex; align-items: center; gap: 10px;
+57 -1
View File
@@ -1,6 +1,9 @@
import assert from 'node:assert/strict'; import assert from 'node:assert/strict';
import test from 'node:test'; import test from 'node:test';
import { normalizeMarkdownImageToken } from '../app/src/renderer/src/markdown-image-renderer.js'; import {
normalizeMarkdownImageToken,
renderSessionMarkdownImage,
} from '../app/src/renderer/src/markdown-image-renderer.js';
// marked <= 14 calls renderer.image(href, title, text); marked >= 15 passes the // marked <= 14 calls renderer.image(href, title, text); marked >= 15 passes the
// token. Getting this wrong degrades silently: every session image turns into // token. Getting this wrong degrades silently: every session image turns into
@@ -34,3 +37,56 @@ test('fills in the fields marked leaves null', () => {
{ href: 'http://example.test/a.png', title: '', text: '' }, { href: 'http://example.test/a.png', title: '', text: '' },
); );
}); });
test('renders an allowed source as the session image element', () => {
assert.equal(
renderSessionMarkdownImage('http://example.test/a.png', '', 'Alt text'),
'<obelisk-session-image src="http://example.test/a.png" alt="Alt text"></obelisk-session-image>',
);
assert.equal(
renderSessionMarkdownImage('data:image/png;base64,AAAA', '', ''),
'<obelisk-session-image src="data:image/png;base64,AAAA" alt=""></obelisk-session-image>',
);
});
test('carries the title through when marked supplies one', () => {
assert.equal(
renderSessionMarkdownImage('file:///shots/a.png', 'A title', 'Alt'),
'<obelisk-session-image src="file:///shots/a.png" alt="Alt" title="A title"></obelisk-session-image>',
);
});
test('drops a source the app will not load, keeping one unavailable state', () => {
for (const href of ['javascript:alert(1)', 'data:text/html,<b>x</b>', 'ftp://example.test/a.png', '']) {
assert.equal(
renderSessionMarkdownImage(href, '', 'Alt text'),
'<obelisk-session-image alt="Alt text"></obelisk-session-image>',
`expected ${href || '(empty)'} to render without a src`,
);
}
});
test('decodes what marked escaped exactly once, then re-escapes it', () => {
assert.equal(
renderSessionMarkdownImage('http://example.test/a.png?x=1&amp;y=2', '', '&quot;quoted&quot; &amp; &#39;single&#39;'),
'<obelisk-session-image src="http://example.test/a.png?x=1&amp;y=2"'
+ ' alt="&quot;quoted&quot; &amp; \'single\'"></obelisk-session-image>',
);
// A single decoding pass, so text that was literally `&lt;` in the source
// does not decay into a real angle bracket.
assert.equal(
renderSessionMarkdownImage('http://example.test/a.png', '', '&amp;lt;script&amp;gt;'),
'<obelisk-session-image src="http://example.test/a.png"'
+ ' alt="&amp;lt;script&amp;gt;"></obelisk-session-image>',
);
});
test('never lets alt text break out of the attribute', () => {
const html = renderSessionMarkdownImage('http://example.test/a.png', '', '"><img src=x onerror=alert(1)>');
assert.equal(html.includes('onerror=alert(1)>'), false);
assert.equal(
html,
'<obelisk-session-image src="http://example.test/a.png"'
+ ' alt="&quot;&gt;&lt;img src=x onerror=alert(1)&gt;"></obelisk-session-image>',
);
});