fix(annotate): render embedded local HTML documents instead of loading the app inside every embed - #1561
Merged
Conversation
A srcdoc document has no URL of its own, so every relative URL in it resolves against the PARENT page — the Plannotator server — and an embedded sibling (<iframe src="prototype.html">) hit the catch-all and rendered the editor app inside the embed. A serve-time attribute rewrite cannot fix that: the src is often assigned by script at runtime from data-src, and carries a query string. rewriteHtmlAssetReferences gains a baseHref option that installs a <base href> first in <head>, so RESOLUTION lands in the document's own token'd directory whatever writes the URL and whenever. resolveHtmlAssetRoute is the one place both runtimes decide what an /api/html-assets request means: .html/.htm now serve as real documents under the 2MB annotate cap with a CSP sandbox that grants scripting but never allow-same-origin, framed and document requests get a small HTML error page naming the missing file, and the traversal guard is unchanged. The share/inline path takes inertBase instead, so a portable export's embeds render empty rather than resolving onto the host's own catch-all.
rewriteHtml installs the root-relative <base href> (a srcdoc resolves its own base against the parent's URL, so the port need not be known), the asset route serves .html siblings with the sandbox CSP + nosniff, and the annotate catch-all refuses a Sec-Fetch-Dest of iframe/frame/embed/object with a 404 document instead of handing a nested frame the editor app.
Same shared decision (resolveHtmlAssetRoute), Node transport: the asset handler now takes the request so it can read Sec-Fetch-Dest, HTML responses carry the sandbox CSP and nosniff, and the SPA fallback answers a framed request with the 404 document.
The bridge is never injected into a nested frame, so a click inside an embed lands in another document. While pinpoint is armed (srcdoc sessions only, never live-app) frames become pointer-transparent, so the click pins the <iframe> itself; Interact hands the embed back for native use.
… wrapper Pointer-transparent frames make hit-testing pass THROUGH the embed to the container painted behind it, so the pin would name the wrapper div. A point inside a frame's own rect now resolves to that frame, bounded to the frames inside the element already resolved.
backnotprop
added a commit
that referenced
this pull request
Sep 18, 2026
…ts that iframe the session get the app (#1565) * fix(annotate): scope the framed 404 to paths that name a file #1561's catch-all guard fired on every framed request, including the app document at `/`. The VS Code extension frames the session URL, so an annotate session opened from the editor rendered "404 Not found" instead of the app. One shared predicate (pathNamesEmbeddedDocument / isFramedEmbeddedDocumentRequest in packages/shared/html-assets.ts, vendored to Pi) now decides it on the SHAPE OF THE PATH, and the Pi mirror uses it instead of its inline copy. * test(annotate): pin the framed catch-all matrix in both runtimes * docs: state the exact framed-404 rule for the annotate catch-all
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
1 participant
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.
Closes the "every embed in my report is a second Plannotator" report (#1554-class).
Root cause
A srcdoc document has no URL of its own — its base URL is the parent page's, which is the Plannotator server — so
<iframe src="prototype-slash.html">inside an annotated HTML file resolved tohttp://localhost:<port>/prototype-slash.html, the catch-all answered with the editor's own HTML, and every embed rendered a second Plannotator. Cookie-less, as the owner noticed, because a nested browsing context inherits the parent iframe's sandbox flags and therefore runs at an opaque origin — which is the same reason nothing leaked.This is #1532's bug in a place #1532's fix cannot reach. Links could be intercepted on click; an embed's
srcis often written by script at runtime (<iframe data-src="…" loading="lazy">plus a loader, the shape the owner's file uses), and one of them carries?step=result. No serve-time attribute rewrite ever sees that URL. Neither would a bridge-side MutationObserver see a URL the loader computes; a service worker is impossible in a sandboxed opaque-origin frame.Approach: change resolution, not markup
rewriteHtmlAssetReferencesgains abaseHrefoption that installs<base href="/api/html-assets/<token>/">first in<head>. Every relative URL the document produces then lands in its own directory — static attribute, runtime assignment,fetch('./data.json'),new URL(x, document.baseURI)— whatever writes it and whenever. Root-relative on purpose: a srcdoc resolves its own<base href>against the parent's URL, so the port need not be known. An author's own<base href>is re-anchored when relative, left untouched when absolute or root-relative (they pinned an origin deliberately)./api/html-assetsthen serves.html/.htmas real documents, so an embedded page's own relative assets, nested embeds, and../back to the token root all resolve against ITS position — which incidentally removes the documented../limitation for embedded documents (it stands for linked ones, which mint their own token). Query strings and fragments survive untouched; the route only ever readsurl.pathname.Side effects checked rather than assumed: in-page
#fragmentlinks are alreadypreventDefaulted and scrolled locally by the #1532 bridge handler, so the base cannot make them navigate; form submission is already blocked by the sandbox (allow-formswas never granted);history.pushStatealready throws at an opaque origin with or without a base.<embed>,<object data>and<frame>come free — the base does not care which element reads the URL.One shared decision,
resolveHtmlAssetRouteinpackages/shared/html-assets.ts(vendored to Pi), so the Bun route and the Pi mirror cannot drift on which extensions are documents, which failures render as HTML, or which headers a document response carries.Security
sandbox="allow-scripts"fetch('/api/plan')fails as cross-origin — measured in the browser before and after this change.Content-Security-Policy: sandbox allow-scriptson every HTML asset responseallow-same-origin, so/api/feedback,/api/approveand the PTY websocket stay unreachable.X-Content-Type-Options: nosniff..refused bynormalizeHtmlAssetRoutePath;isWithinDirectory's realpath symlink check. Nothing new is readable — same directory, more extensions. HTML documents take the 2MBMAX_ANNOTATABLE_FILE_BYTEScap, not the 50MB asset cap.Sec-Fetch-Dest: iframe|frame|embed|object→ a small plain 404 document naming the missing file, never the app, never JSON. Belt and braces: the<base href>is what actually fixes embeds; this catches anything that still arrives.Pinpoint on an embed (decided and documented)
The bridge is never injected into a nested frame, so an embed is one pinpointable element from the outer page's perspective. While pinpoint is armed, frames become
pointer-events: none(body[data-plannotator-frame-inert], srcdoc sessions only — live-app is untouched, its nested frames belong to the user's app), which is the only way the click reaches the outer document at all. Hit-testing would then pass through to the container painted behind, sopreferInertFrameAtresolves a point inside a frame's own rect back to the frame — otherwise the reviewer's comment would name the wrapper<div>. Interact (Esc, the header pen,Mod+Shift+A) hands the embed back for native use, which is also the only state a link inside an embed can be followed from.Portable export / share links — honest
Embeds do not travel.
inlineHtmlLocalAssetsinlines assets asdata:URLs, but a sibling document is a second page with its own relative assets, so it is not inlined. Instead a document that contains any frame gets<base href="about:blank">, so its embeds render empty rather than resolving onto the share portal's own catch-all — which would have reproduced exactly this bug on share.plannotator.ai.No regressions
#1532 link navigation (relative link opens in place, frame stays
about:srcdoc, header Back present), HTML Refresh,/api/share-html, the live-app surface (excluded by construction: no base is installed on the proxy path, and the frame-inert rule is!LIVE), and markdown surfaces are all unchanged.Tests
packages/shared/html-assets.test.ts— +13: base first in head, no-head insertion, relative author base re-anchored, absolute/root-relative left alone, byte-identical without the option,inertBaseonly when frames exist;resolveHtmlAssetRoute(document + CSP + 2MB cap, traversal refused both spellings, framed vs JSON error shape, unsupported types still 415, non-asset paths ignored); error-document escaping;isFramedFetchDest.packages/server/annotate-html-assets.test.ts— +7: sibling served with the sandbox CSP + nosniff and a query string, a subdirectory document reaching../assets inside the root, traversal refused without leaking, framed miss → HTML 404 naming the file, non-framed miss → JSON, >2MB → 413,framedDocumentNotFound, share HTML gets the inert base.apps/pi-extension/server/serverAnnotate-embeds.test.ts— new, 4: the Node transport over the same shared decision (headers on the wire, framed miss → 404 document not the app shell, ordinary navigation still gets the app, traversal refused).packages/ui/components/html-viewer/srcdoc.test.ts— +2: armed pinpoint sets/clearsdata-plannotator-frame-inert(plus a string-level guard that the CSS rule ships, since happy-dom honors neitherpointer-eventsnor:is()); an armed click inside an embed's box pins the frame, not the container behind it.bun run typecheckclean ·bun test packages/shared packages/server packages/core apps/hook apps/pi-extension→ 2647 pass, 1 skip, 0 fail ·DOM_TESTS=1 bun test --isolate packages/ui/components/html-viewer packages/ui/utils→ 1007 pass, 1 skip, 0 fail ·bun run --cwd apps/guides-show build:viewer && check:manifestin sync.Headless proof on the owner's file
plannotator annotate REPORT.html, driven with Playwright/Chromium. Before: all five embeds report titlePlannotator, served from/prototype-slash.htmletc. by the catch-all. After: all five render their real documents — "Smart slash commands: clickable prototype", "Format result: apply now, or preview first", "Running state: three places to show a command at work", "View mode: where the three actions live", "Find decisions: what comes back" — none contains the app,?step=resultis observed inside the fifth, every response istext/html+sandbox allow-scripts+nosniff, a fetch from inside an embed to/api/planis blocked withwindow.origin === "null", an armed click pins the<iframe>itself, a paragraph pin saves a durable anchor and restores after a reload, and the #1532 link path still opens in place with the header Back control.Sign-off page (before/after screenshots, the full pass list, the honest limits):
/private/tmp/claude-501/-Users-ramos-plannotator-plannotator/34d56d5a-33a8-4878-92b2-3f4318214ae2/scratchpad/html-embeds/index.html