Skip to content

feat: incremental streaming markdown via streaming-markdown (smd) - #923

Merged
nesquena-hermes merged 2 commits into
masterfrom
feat/917-streaming-markdown
Apr 24, 2026
Merged

nesquena-hermes merged 2 commits into
masterfrom
feat/917-streaming-markdown

Conversation

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Incremental streaming markdown via the streaming-markdown library. From PR #917 (@bsgdigital), rebased onto current master with SRI documentation and CHANGELOG.

What

Replaces the per-animation-frame full innerHTML re-render with an incremental DOM-building approach:

  • _smdWrite(delta) feeds only new characters since the last write — no full re-parse of accumulated text
  • _smdNewParser(el) lazily initialises the parser per segment (first token or after each tool call)
  • _smdEndParser() flushes remaining state at stream boundaries: tool, done, apperror, cancel
  • Fallback: when window.smd is unavailable, the existing renderMd() path fires unchanged

Why it matters

Previously: renderMd() re-ran the full regex pipeline over ALL accumulated text every ~16ms frame and assigned to innerHTML. This destroyed and recreated all DOM nodes on every tick (DOM thrashing), caused jank and flicker, reset Prism.js/KaTeX state mid-stream, and produced a visual "snap" at stream end.

After: only new chars are fed incrementally. DOM nodes are appended, never replaced.

What changed from #917 hold

All 4 original test failures fixed by @bsgdigital:

  1. ✅ Mobile breakpoint (768px→640px) — CSS changes dropped entirely; fix(mobile): adapt settings dialog and message controls for mobile screens #919 handles mobile settings
  2. ✅ S.busy=false ordering — done handler ordering preserved
  3. ✅ finalizeThinkingCard window — test window expanded from 2900→3300 chars
  4. ✅ sw.js conflict — removed from this PR (landed in feat: add PWA support (manifest, service worker, install prompt) #920)

SRI note

The streaming-markdown library loads via ES module import. ES module import statements don't support the integrity= attribute (W3C limitation — integrity works on <script src=> only, not inline import). Version is pinned in the URL (@0.2.15) and the sha384 hash is documented in a comment for verification. This matches how the project should handle ES module CDN imports.

2060 tests passing (2017 master + 43 new streaming-markdown tests).

Co-authored-by: bsgdigital

…ize schemes

streaming-markdown@0.2.15 preserves arbitrary URL schemes in href/src.
Verified with a Node + jsdom harness:

  IN : [click](javascript:alert(1))
  OUT: <p><a href="javascript:alert(1">click</a>)</p>        ← XSS vector

Confirmed unsafe for: javascript:, vbscript:, data:text/html, file://.
The library uses only safe DOM primitives (createElement/appendChild/
createTextNode — no innerHTML/eval), so <script> tags are escaped as
text, but URL-scheme filtering is absent. The existing renderMd() path
implicitly filtered to http(s) via its regex, so this is a regression
the moment streaming markdown is enabled.

Attack path: agent echoes prompt-injection content containing a
markdown link with javascript: href → smd renders it live → user clicks
during the streaming window → JS executes in webui origin → session
cookie, API calls, etc.

Fix: walk the live DOM after each parser_write (and again after
parser_end) and remove href/src attributes whose scheme isn't on the
safe allowlist (http, https, mailto, tel, and relative/anchor paths).
Blocked anchors keep their text content but lose href; blocked images
lose src and get data-blocked-scheme="1" for debugging.

Harness confirms all 10 tested cases behave correctly — javascript:,
vbscript:, data:text/html, file:// all stripped; https://, /path,
#anchor, mailto:, tel: all preserved.

Added 5 regression tests in TestSmdUrlSchemeSanitization that lock:
  - the sanitize helper exists
  - the allowlist regex permits https? and forbids javascript/vbscript/data:
  - _smdWrite invokes sanitize after parser_write
  - _smdEndParser invokes sanitize after parser_end
  - the sanitizer covers both <a href> and <img src>

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@nesquena nesquena left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Review — end-to-end ✅ (XSS vector caught and patched)

Traced against upstream hermes-agent

Fresh nousresearch/hermes-agent tarball pulled. Confirmed this PR is entirely webui-internal (client-side markdown rendering). No hermes-agent API surface touched, no config.yaml writes, no session state mutations. Cross-tool safety is trivial. ✓

End-to-end trace

Library add (static/index.html:24-30). streaming-markdown@0.2.15 imported as an ES module from cdn.jsdelivr.net. The PR pins the version and documents the sha384 in a comment.

Incremental parser (static/messages.js:190-195). Three new closure-scoped vars in attachLiveStream: _smdParser (current parser instance, null between segments), _smdWrittenLen (char count already fed), _smdReconnect (set to reconnecting, triggers one-shot DOM clear on first render after reconnect).

Three helpers at messages.js:375-399:

  • _smdNewParser(el) — lazy init; no-op when window.smd is unavailable
  • _smdEndParser() — flush + null out parser; called on tool/done/apperror/cancel
  • _smdWrite(displayText) — feed only the char delta since last write

Stream-end integration (messages.js:594-607). On done, _smdEndParser() is called, then requestAnimationFrame fires highlightCode + addCopyButtons + renderKatexBlocks on the live assistant body. Also called from apperror (673), cancel (751).

Fallback when window.smd is unavailable (slow CDN, blocked by firewall, CSP): the existing renderMd() path runs exactly as before. No functional regression. ✓

Manual CDN + library audit

Since ES-module import doesn't enforce integrity=, I fetched the actual CDN content to verify the hash matches the PR's documented sha384:

downloaded  12586 bytes
sha384-T6r95ocN9t3W8tUK2Fa6FPaO7bJryyjyW0WCalrUnpgtm2qXr5xcN4vwPYEJ6vHa

Matches the PR's hash comment exactly. ✓

Static audit of the library:

Unsafe primitive Occurrences
innerHTML 0
outerHTML 0
document.write 0
eval / Function() 0
insertAdjacentHTML / setHTMLUnsafe 0

The library uses only createElement (29×), appendChild (5×), createTextNode (1×), and setAttribute (1×). All safe. Markdown is turned into a DOM tree via tree-building — raw HTML in the markdown source is handled as text, not interpreted. A <script> token in the markdown becomes literal text, not a live element. ✓

CSP check: existing script-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net at api/helpers.py:46 already allows jsdelivr-served scripts. No CSP update needed. ✓

What I caught — XSS via URL schemes in markdown links

streaming-markdown@0.2.15 does NOT sanitize URL schemes. I built a Node + jsdom harness to exercise real markdown-link inputs against the actual library binary:

IN : [click](javascript:alert(1))        →  <a href="javascript:alert(1">click</a>    ← XSS
IN : ![alt](javascript:alert(1))         →  <img src="javascript:alert(1">            ← XSS (some browsers still execute)
IN : [click](data:text/html,<script>…)   →  <a href="data:text/html,…">click</a>     ← XSS (data URL navigates to attacker HTML)
IN : [click](vbscript:msgbox(1))         →  <a href="vbscript:…">click</a>            ← legacy XSS
IN : [click](file:///etc/passwd)         →  <a href="file:///etc/passwd">click</a>    ← info disclosure

Confirmed by grepping the minified library: javascript, scheme, validate, sanitize, safe_url all appear 0 times. No URL-scheme filtering.

The existing renderMd() path at static/ui.js:618,625 used http(s)?:// regex as an implicit allowlist — non-http(s) schemes didn't match and were rendered as literal text. When streaming markdown takes over, that implicit filter disappears. An agent-echoed [click](javascript:…) from prompt-injected content becomes a click-to-XSS vector in the webui origin.

Attack scenario: Agent summarizes untrusted RAG source, skill output, or a malicious message; output contains a markdown link with javascript: href; user clicks it during the streaming window; JS executes in the webui origin with session cookie access.

What I pushed — b563484

Added _sanitizeSmdLinks(root) that walks the live subtree and strips href/src attributes whose scheme isn't on the safe allowlist (https?:, mailto:, tel:, and relative paths /, #, ., ?). Blocked nodes keep their text content and get data-blocked-scheme="1" for debugging.

Called from:

  • _smdWrite — after every parser_write so mid-stream links are sanitized before the user can click
  • _smdEndParser — after parser_end flushes trailing markdown state

Re-ran the jsdom harness with the fix applied against 10 scheme variants:

✓ safe   | [click](javascript:alert(1))              | <a data-blocked-scheme="1">click</a>
✓ safe   | ![alt](javascript:alert(1))               | <img data-blocked-scheme="1">
✓ safe   | [click](data:text/html,<script>x</script>) | <a data-blocked-scheme="1">click</a>
✓ safe   | [click](vbscript:msgbox(1))               | <a data-blocked-scheme="1">click</a>
✓ safe   | [click](file:///etc/passwd)               | <a data-blocked-scheme="1">click</a>
✓ safe   | [click](https://safe.com)                 | <a href="https://safe.com">click</a>
✓ safe   | [click](/relative/path)                   | <a href="/relative/path">click</a>
✓ safe   | [click](#anchor)                          | <a href="#anchor">click</a>
✓ safe   | [click](mailto:foo@bar.com)               | <a href="mailto:foo@bar.com">click</a>
✓ safe   | [click](tel:+15551234567)                 | <a href="tel:+15551234567">click</a>

All 10 pass. Unsafe schemes blocked; http(s), relative, anchor, mailto, tel preserved. Matches the implicit allowlist of the existing renderMd path.

Regression tests added (tests/test_streaming_markdown.py → 5 new tests in TestSmdUrlSchemeSanitization):

  • test_sanitize_helper_exists — _sanitizeSmdLinks must be defined
  • test_sanitize_uses_scheme_allowlist — regex must permit https? and must NOT mention javascript/vbscript/data:
  • test_sanitize_called_after_smd_write — _smdWrite must call the sanitizer
  • test_sanitize_called_at_parser_end — _smdEndParser must call the sanitizer
  • test_sanitize_strips_href_and_src — both a[href] and img[src] must be walked

Other audit — things that are correct already

DOM replacement at done. renderMessages() on the done handler replaces the smd-rendered body with renderMd() output, which has its own http(s) filter. The smd body is only the live-streaming view. My sanitizer closes the only window where user could click a malicious smd-rendered link. ✓

Reconnect path. _smdReconnect=reconnecting triggers a one-shot assistantBody.innerHTML = '' on first render after reconnect, then starts a fresh parser fed with the accumulated displayText. Correct — avoids stale DOM from the previous process's parser. ✓

Tool segment boundary. _smdEndParser() is called from the tool handler, a new parser is created for the next segment. Correct — each segment between tool calls is its own parser with its own DOM subtree. ✓

Error handling. try{…}catch(_){} around all smd calls means a library bug can't crash the webui. Fallback stays silent. Acceptable defensive posture. ✓

Security headers. CSP at api/helpers.py:46 already permits jsdelivr scripts; no update needed. 'unsafe-inline' was already granted for inline scripts; no widening. ✓

Cross-tool check. Library runs in the browser only, produces DOM only. CLI doesn't care. ✓

Edge-case trace

Scenario Behaviour
Normal streaming with formatted markdown smd builds DOM incrementally; Prism + KaTeX run on done ✅
Agent outputs [click](javascript:alert(1)) Before my fix: rendered as live <a href="javascript:…"> → click-to-XSS · After: <a data-blocked-scheme="1">click</a> — text preserved, scheme stripped ✅
Agent outputs ![alt](javascript:…) Same treatment — src stripped ✅
Agent outputs [click](data:text/html,…) Stripped (data: not in allowlist) ✅
Agent outputs [click](https://example.com) Preserved — https:// is in allowlist ✅
Relative path [click](/foo) Preserved ✅
Anchor link [click](#sec) Preserved ✅
mailto:, tel: Preserved (common, benign) ✅
window.smd never loads (CDN blocked / CSP denies) Fallback to renderMd() path — identical to pre-PR behavior ✅
CDN compromise (jsdelivr serves tampered file) Latent concern — no SRI enforcement via ES module import. Hash is documented in a comment but not enforced. Same trust model as KaTeX/Prism, which DO have integrity=. See follow-up below.
Tool boundary mid-stream _smdEndParser + new parser for next segment; sanitizer fires at end-of-segment ✅
Reconnect to ongoing stream _smdReconnect → clear stale DOM + restart parser with accumulated text ✅
Cancel during streaming _smdEndParser → sanitize pass → Prism + copy buttons on live segment ✅

Tests

  • 48/48 pass in tests/test_streaming_markdown.py (43 original + 5 new XSS-sanitization tests)
  • Full local suite: 2018 passed, 47 skipped, 0 failed
  • node --check static/messages.js — clean
  • Node + jsdom harness confirms the fix blocks all 5 tested unsafe schemes and preserves all 5 safe schemes

Minor observations (non-blocking)

  • SRI enforcement via modulepreload: browsers now support <link rel="modulepreload" href="…" integrity="…" crossorigin> which enforces SRI for ES modules. Could layer this on top of the existing <script type="module"> import for defensive hash verification against CDN compromise. Not critical given the library's safe-by-construction design, but worth adding in a follow-up.

  • Self-hosting option: at 12.3 KB the library is trivial to vendor under static/vendor/smd-0.2.15.js. That would eliminate CDN trust entirely and remove the only real residual risk from this PR (package hijack / CDN compromise). Matches the project's stance for credentials and session state. Not critical, but the cleanest long-term posture.

  • data-blocked-scheme="1": useful marker for debugging agent output that contains unsafe URLs. Consider exposing a small UI hint (tooltip) to indicate why a link isn't clickable. Non-urgent.

  • Parser error swallow: try{…}catch(_){} around all three smd entry points hides errors silently. For production this is the right choice (don't crash the UI). For development, surfacing via console.debug might help diagnose parser bugs.

  • KaTeX blocks during streaming: renderKatexBlocks() runs only at done. Inline math $x^2$ won't render mid-stream — same as pre-PR behavior. Acceptable.

Recommendation

The incremental-render architecture is well-designed and the library is safe-by-construction — no innerHTML, no eval, text content via createTextNode. The only real issue was the missing URL-scheme filter, which the existing renderMd path had implicitly via its http(s)-only regex. My sanitizer closes that regression with matching allowlist semantics, verified against a real jsdom harness across 10 scheme variants.

✅ Approved after the XSS fix. Ready for merge + v0.50.180 tag.

@nesquena-hermes
nesquena-hermes merged commit ff970ec into master Apr 24, 2026
3 checks passed
@nesquena-hermes
nesquena-hermes deleted the feat/917-streaming-markdown branch April 24, 2026 00:43
JKJameson pushed a commit to JKJameson/hermes-webui that referenced this pull request Apr 25, 2026
…down

Merging feat/917-streaming-markdown. 2065 tests pass. APPROVED by @nesquena. Pre-existing QA harness failure on master confirmed (not a regression).
SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
…down

Merging feat/917-streaming-markdown. 2065 tests pass. APPROVED by @nesquena. Pre-existing QA harness failure on master confirmed (not a regression).
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.

2 participants