From 1124758b6b4918e5c7d66eebdf3519d10e1459c1 Mon Sep 17 00:00:00 2001
From: tommy0103 <vast_starry_sky@qq.com>
Date: Mon, 3 Aug 2026 02:10:23 +0800
Subject: [PATCH] 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>
---
 .../src/components/SessionImage.ce.vue        | 29 ++++++---
 .../renderer/src/markdown-image-renderer.js   | 61 +++++++++++++------
 app/src/renderer/styles/detail.css            | 19 ++----
 tests/markdown-image-renderer.test.mjs        | 58 +++++++++++++++++-
 4 files changed, 125 insertions(+), 42 deletions(-)

diff --git a/app/src/renderer/src/components/SessionImage.ce.vue b/app/src/renderer/src/components/SessionImage.ce.vue
index 0fd9303..b219188 100644
--- a/app/src/renderer/src/components/SessionImage.ce.vue
+++ b/app/src/renderer/src/components/SessionImage.ce.vue
@@ -1,18 +1,25 @@
 <script setup>
-import { computed, ref } from 'vue';
+import { computed, ref, watch } from 'vue';
 import { SESSION_IMAGE_SETTLED_EVENT } from '../session-image-contract.js';
 
 defineOptions({ name: 'SessionImage' });
 
 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: '' },
   title: { type: String, default: '' },
 });
 
-const status = ref('loading');
+const status = ref(props.src ? 'loading' : 'error');
 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,
 // so the timeline already knows the row is about to change size for a reason
 // the reader did not cause.
@@ -57,6 +64,10 @@ function handleError(event) {
 </template>
 
 <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 {
   display: block;
   max-inline-size: 100%;
@@ -70,9 +81,9 @@ function handleError(event) {
   min-inline-size: 0;
   margin: 0;
   overflow: clip;
-  border: 1px solid rgba(255, 255, 255, 0.08);
+  border: 1px solid var(--hairline-strong);
   border-radius: 6px;
-  background: rgba(0, 0, 0, 0.22);
+  background: var(--session-image-backdrop, rgba(0, 0, 0, 0.22));
 }
 
 img {
@@ -80,9 +91,9 @@ img {
   inline-size: auto;
   max-inline-size: 100%;
   block-size: auto;
-  max-block-size: min(70vh, 720px);
+  max-block-size: var(--session-image-max-block, min(70vh, 720px));
   object-fit: contain;
-  color: rgba(255, 255, 255, 0.48);
+  color: var(--muted);
 }
 
 .is-loading {
@@ -95,8 +106,8 @@ figcaption {
   align-items: center;
   gap: 6px;
   padding: 10px 12px;
-  color: rgba(255, 255, 255, 0.48);
-  font: 12px/1.5 ui-monospace, 'SF Mono', Menlo, monospace;
+  color: var(--muted);
+  font: var(--text-sm, 12px)/1.5 var(--font-mono);
 }
 
 .image-label::before {
diff --git a/app/src/renderer/src/markdown-image-renderer.js b/app/src/renderer/src/markdown-image-renderer.js
index f48abdd..e0ae6e6 100644
--- a/app/src/renderer/src/markdown-image-renderer.js
+++ b/app/src/renderer/src/markdown-image-renderer.js
@@ -1,17 +1,49 @@
 import { SESSION_IMAGE_TAG } from './session-image-contract.js';
 
 const SAFE_IMAGE_PROTOCOLS = new Set(['blob:', 'file:', 'http:', 'https:']);
+const NAMED_ENTITIES = {
+  amp: '&',
+  lt: '<',
+  gt: '>',
+  quot: '"',
+  apos: "'",
+  nbsp: ' ',
+};
 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) {
-  const decoder = document.createElement('textarea');
-  decoder.innerHTML = String(value ?? '');
-  return decoder.value;
+  return String(value ?? '').replace(/&(#\d+|#x[0-9a-f]+|[a-z]+);/gi, (whole, entity) => {
+    if (entity[0] === '#') {
+      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) {
   try {
-    const url = new URL(source, document.baseURI);
+    const url = new URL(source, globalThis.document?.baseURI ?? 'file:///');
     return SAFE_IMAGE_PROTOCOLS.has(url.protocol)
       || (url.protocol === 'data:' && /^data:image\//i.test(source));
   } 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
 // image token instead. Accepting both keeps an upgrade from silently turning
 // every session image into fallback text.
@@ -40,18 +65,18 @@ export function normalizeMarkdownImageToken(hrefOrToken, title, 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) {
   const token = normalizeMarkdownImageToken(hrefOrToken, title, text);
   const source = decodeMarkedAttribute(token.href).trim();
   const alt = decodeMarkedAttribute(token.text);
   const accessibleTitle = decodeMarkedAttribute(token.title);
-  if (!source || !isSafeImageSource(source)) return imageFallback(alt);
-
-  const image = document.createElement(SESSION_IMAGE_TAG);
-  image.setAttribute('src', source);
-  image.setAttribute('alt', alt);
-  if (accessibleTitle) image.setAttribute('title', accessibleTitle);
-  return image.outerHTML;
+  const attributes = [`alt="${escapeAttribute(alt)}"`];
+  if (source && isSafeImageSource(source)) attributes.unshift(`src="${escapeAttribute(source)}"`);
+  if (accessibleTitle) attributes.push(`title="${escapeAttribute(accessibleTitle)}"`);
+  return `<${SESSION_IMAGE_TAG} ${attributes.join(' ')}></${SESSION_IMAGE_TAG}>`;
 }
 
 export function configureMarkdownImages(marked) {
diff --git a/app/src/renderer/styles/detail.css b/app/src/renderer/styles/detail.css
index f7f3c18..fcf6bd0 100644
--- a/app/src/renderer/styles/detail.css
+++ b/app/src/renderer/styles/detail.css
@@ -363,20 +363,11 @@
 .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 mark { background: var(--accent-soft); color: var(--accent-2); padding: 0 2px; border-radius: 2px; }
-.session-image-fallback {
-  display: block;
-  max-width: 100%;
-  margin: 0.7em 0;
-  padding: 10px 12px;
-  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;
-}
+
+/* Session images cap themselves against the reading column. Compact Markdown
+   renders in much smaller surfaces (subagent panes, memory rows, tool results),
+   where a viewport-sized image would swamp the block it belongs to. */
+.markdown-compact { --session-image-max-block: 240px; }
 
 .detail-section-divider {
   display: flex; align-items: center; gap: 10px;
diff --git a/tests/markdown-image-renderer.test.mjs b/tests/markdown-image-renderer.test.mjs
index cd65fe0..84c992b 100644
--- a/tests/markdown-image-renderer.test.mjs
+++ b/tests/markdown-image-renderer.test.mjs
@@ -1,6 +1,9 @@
 import assert from 'node:assert/strict';
 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
 // 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: '' },
   );
 });
+
+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>',
+  );
+});