security: validate reported html against a ledger of what the server sent - #288
Draft
cpsievert wants to merge 5 commits into
Draft
security: validate reported html against a ledger of what the server sent#288cpsievert wants to merge 5 commits into
cpsievert wants to merge 5 commits into
Conversation
The browser's `<id>_messages` report was carried into saved history and bookmark state verbatim, dependency objects included, and replayed with `send_chat_action(html_deps =)` on restore -- which reaches `Shiny.renderDependenciesAsync()` and injects the dependency's script/head content into the page. Because a server bookmark is shareable via its `_state_id_` URL, an attacker could forge a `_messages` value in their own session, let `chat_restore()` mint the bookmark, and share the URL to load attacker-chosen scripts into the recipient's session. `send_chat_action()` now records every dependency it sends, keyed by name+version, and the `_messages` input handler treats the client's report as nothing more than a set of identities: each reported dependency is replaced by the server's own copy of it, and anything the server never sent is dropped. Sanitizing at the input boundary covers the history store as well as bookmark state. Hostile `html` segment content is a separate, unaddressed vector with the same reach; tracked separately.
A reported content_type: 'html' segment now has to match something the server actually sent this session; a miss degrades that one segment to markdown, where the client escapes shinychat's raw-HTML element names. Closes the last reach of a forged _messages report: bookmark state is shareable via its _state_id_ URL, and RawHTML assigns html content straight to innerHTML.
…sent Replaces the prefix-hash registry with a ledger that mirrors the client's chunk merging. The old version recorded a hash for every prefix of a streaming html run, because it couldn't predict where the client would close a segment. It didn't have to: buildMessagesSnapshot() drops any message still streaming, so the browser only ever reports the *settled* segment. Recording that one string -- merged the same way the client merges it -- is the whole job, and it drops the O(n^2) prefix hashing along with the per-chat run state. Also ports the boundary to Python, which had the same exposure: its messages_input_value() took content_type and htmlDeps from the client verbatim into StoredMessage and on into the history store.
2 tasks
is_trusted_html() and dep_key() both assumed their input matched the client's normal shape (a str, a dict), but both receive values straight from a client-controlled JSON report -- a forged segment or htmlDeps entry of the wrong type would raise out of hash_content()/dep.get() instead of just failing the trust check, turning a would-be no-op into an unhandled exception in the request path. Caught by Copilot's review on #288.
cpsievert
marked this pull request as draft
August 7, 2026 20:55
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.
Summary
input$<id>_messages/input[f"{id}_messages"]), and that report is currently trusted forcontent_type: "html"segments andhtmlDepsobjects. That report is persisted verbatim into the already-shipped chat-history store, and is the input a future bookmark-restore feature would persist into shareable server-bookmark state. A forged report is a stored-script vector against whoever later opens that history or bookmark — the raw-HTML sinks it reaches (innerHTML, and the tool cards'icon/footer/valueattributes viadangerouslySetInnerHTML) don't distinguish content the server actually sent from content the client merely claims it sent.session$input$<id>_messagesis ever read by anything downstream — so history saves, and any future consumer of the same input, get the protection automatically. Untrusted html degrades to inert markdown; untrusted dependencies are dropped and trusted ones are swapped for the server's own copy._html_trust.py), which had the identical gap inmessages_input_value().Test plan
devtools::test()(R) — 684 passing, including an adversarial pair that forges a_messagesreport, sanitizes it, and asserts the replay primitive shared by history-switch and bookmark-restore never re-emits"html"or a dependency the server didn't senduv run pytest(py) — 372 passing, including an end-to-end case that drives a realChat.append_message(HTML(...))through the actual send path and feeds the wire content back through the input handler the way the browser wouldpyright,ruff check— clean