feat(desktop): render kind:40009 HTML artifacts as sandboxed embeds - #1
Open
jbforge wants to merge 3 commits into
Open
feat(desktop): render kind:40009 HTML artifacts as sandboxed embeds#1jbforge wants to merge 3 commits into
jbforge wants to merge 3 commits into
Conversation
Adds an HTML artifact message kind so an agent can post a self-contained
page — a dashboard, a report, a chart — and have it render inline in the
timeline instead of arriving as a wall of markup.
The security model is capability denial, not markup rewriting:
- `<iframe sandbox="">` with no tokens. No scripts, no same-origin, no
forms, no top-level navigation. The attribute is set by the renderer
and can never be influenced by the event.
- A CSP meta prepended by the wrapper document closes passive loads,
so a remote `<img src>` cannot leak "this person opened this" to a
third-party host. `data:` images and fonts still work.
Neither layer parses the artifact HTML.
Relay-side ingest validates the kind's tags (`title`, `alt`, `height`,
content size); the renderer clamps `height` again rather than trusting
that, since old events and local-archive replays bypass the relay check.
Gated behind the off-by-default `htmlEmbeds` preview flag. Live-updating
artifacts are deliberately out of scope for v1.
Protocol notes in docs/nips/NIP-HE.md.
Co-authored-by: jbforge <270093744+jbforge@users.noreply.github.com>
Signed-off-by: jbforge <270093744+jbforge@users.noreply.github.com>
…ally Security review found the docs promising more than the frame delivers. A sandboxed frame may navigate *itself* — `sandbox=""` stops `_top`, `_blank`, and forms, but not a plain same-frame link. An artifact can wrap its whole surface in an anchor, and one click then sends the reader's IP to a remote host and lands them on a document our CSP does not govern, since the policy lived in the `srcdoc` document and does not survive navigation. The row ends up showing content fetched after the event was signed, which is not the same trust question as static spoofed markup. Scripts still never execute. The sandbox's guarantee is intact; it is the CSP layer whose reach was overstated. Corrected three claims in NIP-HE.md and added a "Known gap" section naming the real fix: a host-document `frame-src` policy, which is app-wide and so deliberately out of scope here. `<base target="_blank">` is not a fix — an artifact overrides it with `target="_self"`. Tests: the CSP had only a string assertion, which would not have caught this. Added e2e 04, which serves the beacon successfully so a pass means the CSP blocked it rather than DNS failing, and e2e 05, which pins the navigation gap so its blast radius cannot widen unnoticed. Verified in both Chromium and WebKit — WebKit because the app ships on WKWebView. Co-authored-by: jbforge <270093744+jbforge@users.noreply.github.com> Signed-off-by: jbforge <270093744+jbforge@users.noreply.github.com>
Re-review caught that the `@font-face` in e2e 04 was inert. A browser only
fetches a webfont once a rendered element applies the family, so declaring
`font-family:x` and never using it meant `font-src` was never exercised —
the test claimed three vectors and covered two.
Measured with no CSP at all: 2 requests as written (img, background-image),
3 once `body{font-family:x}` applies the family. Added that rule and noted
why it is required, since it reads like decoration.
Not a false green before this — a broken CSP still failed the assertion on
the other two vectors — but the coverage did not match the comment.
Co-authored-by: jbforge <270093744+jbforge@users.noreply.github.com>
Signed-off-by: jbforge <270093744+jbforge@users.noreply.github.com>
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.
Lets an agent post a self-contained HTML page — a dashboard, a report, a chart — and have it render inline in the timeline instead of arriving as a wall of markup.
Security model
Capability denial, not markup rewriting. Two layers, neither of which parses the artifact HTML:
<iframe sandbox="">with no tokens. No scripts, no same-origin, no forms, no top-level navigation, no popups. The attribute is set byHtmlMessage.tsxand can never be influenced by the event, so<script>never executes andonclick=never fires.<img src>orbackground-image: url(…)would otherwise leak "this person opened this artifact" to a third-party host with no script involved.default-src 'none', withdata:images and fonts still allowed so self-contained pages render fully.Sanitizing untrusted markup by rewriting it is a losing game; denying the capabilities is not.
Validation
Relay ingest validates the kind's tags (
title,alt,height) and content size. The renderer clampsheightagain rather than trusting that — events predating the check, an older relay build, and local-archive replay all bypass relay validation.Scope
Gated behind the off-by-default
htmlEmbedspreview flag (Settings → Experiments). Live-updating artifacts are deliberately not in v1; that's the next step and it needs its own security pass rather than being rushed in alongside this.Protocol notes:
docs/nips/NIP-HE.md.Tests
desktop/src/features/messages/lib/htmlArtifact.test.mjs— sandbox wrapper + height clampingdesktop/src/shared/constants/kinds.test.mjs— kind registrationdesktop/tests/e2e/html-embed.spec.ts— end-to-end render