Style chat messages - #40
Merged
Merged
Conversation
Assistant replies reach the browser as HTML-escaped plain text, so the markdown
the model emits — **bold**, 1. lists, backticks — displays as literal
characters. This adds the renderer only; nothing calls it yet.
Two layers, and both are required. Commonmarker parses with unsafe: false,
which replaces raw HTML with a placeholder comment and empties javascript:,
data: and vbscript: URLs in the parser itself. Rails::HTML5::SafeListSanitizer
then reduces the result to `p br strong em code pre ul ol li` with no
attributes at all. Neither suffices alone: given <style>body{display:none}
</style> the sanitizer strips the tag but leaves the CSS visible as text, and
the parser is one config flag away from unsafe: true. That matters more than
usual here, because content_security_policy.rb sets script_src to :self, :https
with no strict-dynamic — any https origin is an allowed script source, so CSP
would not stop an injected <script src>. Escaping is the only defense.
The allowlist is minimal on purpose. The sanitizer keeps a stripped tag's
children, so headings, blockquotes and strikethrough flatten to plain prose
instead of leaving stray ### and ~~ behind. No <a> and no <img>: a markdown link
keeps only its label, and an image would let an LLM-chosen third-party host
receive a request on every render. Tables are disabled at the parser rather than
stripped, because stripping them jams every cell together. <pre> stays because
it is the only reliable marker for "this code is a block" — a fence inside a
list item lands as a child of <li>, so position cannot separate block from
inline code.
Four commonmarker 2.9.0 defaults are wrong here and are overridden explicitly:
the syntax highlighter (injects hardcoded hex), heading anchors, task lists
(emit <input type=checkbox>), and emoji shortcodes (a text node by sanitize
time, so the sanitizer cannot undo them).
commonmarker 2.x is a Rust extension and the image ships no cargo, but it never
needs one — 2.9.0 installs precompiled, and the lock resolves all nine of this
repo's platforms, which BUNDLE_DEPLOYMENT=1 requires.
The lock also carries brakeman 8.0.5 → 8.0.6, without which bin/brakeman
--ensure-latest refuses to run.
Wires the renderer into the message partial behind a streaming gate, ships the .msg-prose component, and tells the agent which subset to emit. Parsing happens once, on the final render. Three broadcast paths reach the partial, and each passes streaming: explicitly rather than inferring state from a billing column: mid-stream and the create append stay plain, the final replace formats. The append needs the gate even though the row is empty when it commits, because broadcast_append_later_to renders inside Turbo::Streams::ActionBroadcastJob 100-300ms later from a GlobalID reload, by which point the streaming job has written chunks. It is also the render the user sees first: every replace fired before it targets a dom_id not yet in the DOM, and Turbo drops those silently. Ungated, it would format half-written markdown on every reply. One path the gate cannot reach is a page load landing mid-reply, which formats a half-written message. It self-repairs on the next chunk and the window is a few seconds, so it is accepted rather than fixed; the deterministic fix wants a terminal-state column on messages, which cancel/retry would need anyway. .msg-prose must stay after .msg-body in the stylesheet — both are single-class selectors on the same element, so source order breaks the white-space tie. .msg-body also gains overflow-wrap: anywhere, so long paths and URLs stop overflowing the bubble on every path, formatted or not. Fenced blocks get white-space: pre and scroll horizontally instead of breaking a line mid-token. The prompt gains one bullet naming the supported subset and asking for bare URLs. It opens with "write short paragraphs", which sits in tension with the two rules that keep a tool call prose-free, so turn-taking was re-checked against the live model: the tool call still arrives with empty content, and the post-tool response is still empty. Tests cover all three broadcast paths — the two that must stay plain and the one that must format — plus the helper's three-way branch, the page-load path, and the case where a tool-call message also carries prose, which the partial can represent regardless of what the model does.
scan_ruby went red on bin/bundler-audit with 104 advisories across activestorage, concurrent-ruby, crass, faraday, json, loofah, mail, msgpack, nokogiri, rails-html-sanitizer, sqlite3 and websocket-driver. None of it is caused by the markdown work on this branch: commonmarker is flagged zero times, and main fails identically with the same 104. bundler-audit clones ruby-advisory-db fresh on a clean runner, so CI picked up today's snapshot while local caches were still on the 2026-07-29 one and reported clean. All twelve are patch-level, and the transitive Rails bumps ride along to reach activestorage >= 8.1.3.1. Every platform row in the lock is preserved, which BUNDLE_DEPLOYMENT=1 needs. Two of these — rails-html-sanitizer 1.7.0 → 1.7.1 and loofah 2.25.1 → 2.25.2 — are the sanitizer that lib/markdown.rb layers on top of, so the allowlist was re-verified against the full hostile-input set rather than trusting a green suite. Output is byte-identical: script, img/onerror, svg/onload, iframe/srcdoc, form/input, base, style tags and attributes, tab-obfuscated javascript: hrefs, markdown images and links all still reduce to nothing or to bare text.
Covers everything since the 1.2.0 entry: formatted chat replies, Sonnet 5 and Opus 5 in the picker, the wildcard preview certificate switched on in production, the model-registry breakage that made every non-default selection fail, and this week's gem advisories. Minor rather than patch, per the file's own rule — two of the entries are new functionality. The tenant-isolation write-up and the model-registry runbook are deliberately absent: both are documentation and internal tooling, and neither changes anything a user of hifumi.dev can observe.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.