From 61b12820cbeef1edf2df74ad7dca5b0d50a78127 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Thu, 17 Sep 2026 12:49:39 -0700 Subject: [PATCH 1/6] fix(annotate): anchor raw-HTML documents at their own asset directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 (', + () => null, + { inertBase: true }, + ); + expect(withFrame).toContain(``); + const withoutFrame = rewriteHtmlAssetReferences( + "

hi

", + () => null, + { inertBase: true }, + ); + expect(withoutFrame).not.toContain(" { + const roots = (token: string) => (token === "tok" ? "/site" : undefined); + const resolve = (pathname: string, secFetchDest?: string) => + resolveHtmlAssetRoute({ pathname, secFetchDest }, roots); + + test("serves a sibling HTML file as a sandboxed document under the annotate cap", () => { + expect(resolve("/api/html-assets/tok/prototype.html")).toMatchObject({ + kind: "serve", + root: "/site", + assetPath: "prototype.html", + contentType: HTML_ASSET_DOCUMENT_CONTENT_TYPE, + document: true, + maxBytes: MAX_HTML_ASSET_DOCUMENT_BYTES, + }); + // No allow-same-origin: an embedded document must never reach this + // session's API as a same-origin caller. + expect(HTML_ASSET_DOCUMENT_CSP).not.toContain("allow-same-origin"); + }); + + test("refuses a path that climbs out of the token's directory", () => { + expect(resolve("/api/html-assets/tok/../secret.html")).toMatchObject({ kind: "error", status: 400 }); + expect(resolve("/api/html-assets/tok/%2e%2e/secret.html")).toMatchObject({ kind: "error", status: 400 }); + }); + + test("a framed request gets an HTML error document; a plain asset request keeps JSON", () => { + expect(resolve("/api/html-assets/nope/page.css", "iframe")).toMatchObject({ asDocument: true }); + expect(resolve("/api/html-assets/nope/page.css")).toMatchObject({ asDocument: false }); + // An .html path is a document however it was requested — a reviewer + // pasting the URL into a tab must not get JSON either. + expect(resolve("/api/html-assets/nope/page.html")).toMatchObject({ asDocument: true }); + }); + + test("still refuses asset types it has no content type for", () => { + expect(resolve("/api/html-assets/tok/notes.pdf")).toMatchObject({ kind: "error", status: 415 }); + }); + + test("ignores everything outside the asset prefix", () => { + expect(resolve("/api/plan")).toEqual({ kind: "not-asset-route" }); + }); +}); + +describe("buildHtmlAssetErrorDocument", () => { + test("names the missing file and escapes it", () => { + const doc = buildHtmlAssetErrorDocument(404, "Not found", ".html"); + expect(doc).toContain("<img>.html"); + expect(doc).not.toContain(""); + expect(doc.startsWith("")).toBe(true); + }); +}); + +describe("isFramedFetchDest", () => { + test("recognizes every nested-document destination and nothing else", () => { + for (const dest of ["iframe", "frame", "embed", "object", "IFRAME"]) { + expect(isFramedFetchDest(dest)).toBe(true); + } + for (const dest of ["document", "script", "image", "empty", "", null, undefined]) { + expect(isFramedFetchDest(dest)).toBe(false); + } + }); +}); diff --git a/packages/shared/html-assets.ts b/packages/shared/html-assets.ts index bd08a40c2..a03d4d8a1 100644 --- a/packages/shared/html-assets.ts +++ b/packages/shared/html-assets.ts @@ -1,8 +1,81 @@ import { posix as pathPosix } from "path"; import * as parse5 from "parse5"; +import { MAX_ANNOTATABLE_FILE_BYTES } from "@plannotator/core/annotatable"; export const HTML_ASSET_ROUTE_PREFIX = "/api/html-assets"; +/** + * Embedded local documents (`', + ); + expect(base).toMatch(/^\/api\/html-assets\/[0-9a-f]{16}\/$/); + + const response = await get(assets, `${base}embed.html?step=result`, { "sec-fetch-dest": "iframe" }); + expect(response?.status).toBe(200); + expect(response?.headers.get("content-type")).toContain("text/html"); + // Defense in depth for a reviewer who opens the asset URL in a top-level + // tab: no allow-same-origin means an opaque origin there too. + expect(response?.headers.get("content-security-policy")).toBe("sandbox allow-scripts"); + expect(response?.headers.get("content-security-policy")).not.toContain("allow-same-origin"); + expect(response?.headers.get("x-content-type-options")).toBe("nosniff"); + expect(await response?.text()).toContain("EMBEDDED_SIBLING"); + }); + + test("keeps a document in a subdirectory inside the token's root and lets it reach ../ assets", async () => { + const dir = site("nested"); + mkdirSync(join(dir, "sub")); + writeFileSync(join(dir, "sub", "deep.html"), "DEEP", "utf-8"); + writeFileSync(join(dir, "shared.css"), "body{color:red}", "utf-8"); + const { assets, base } = registerPage(dir, ''); + + expect((await get(assets, `${base}sub/deep.html`))?.status).toBe(200); + // sub/deep.html loads at /sub/deep.html, so its own ../shared.css + // resolves back inside the same token root. + expect((await get(assets, `${base}shared.css`))?.status).toBe(200); + }); + + test("refuses a document that climbs out of the annotated file's directory", async () => { + const outer = realpathSync(mkdtempSync(join(tmpdir(), "plannotator-html-embed-escape-"))); + const dir = join(outer, "site"); + mkdirSync(dir); + writeFileSync(join(outer, "secret.html"), "SECRET_OUTSIDE_CONTENT", "utf-8"); + const { assets, base } = registerPage(dir, ""); + + // Both spellings: URL parsing collapses the dot segments out of the path + // before the route sees them (which is itself a guard), and the route's own + // normalizer refuses whatever survives. Neither may reach the file. + for (const spelling of ["../secret.html", "%2e%2e/secret.html"]) { + const response = await get(assets, `${base}${spelling}`, { "sec-fetch-dest": "iframe" }); + expect(response?.status).toBeGreaterThanOrEqual(400); + expect(await response?.text()).not.toContain("SECRET_OUTSIDE_CONTENT"); + } + }); + + test("a missing embed gets a small HTML document naming the file, never JSON", async () => { + const dir = site("missing"); + const { assets, base } = registerPage(dir, ""); + + const response = await get(assets, `${base}gone.html`, { "sec-fetch-dest": "iframe" }); + expect(response?.status).toBe(404); + expect(response?.headers.get("content-type")).toContain("text/html"); + const body = await response?.text(); + expect(body).toContain("gone.html"); + expect(body?.startsWith("")).toBe(true); + expect(body).not.toContain('"error"'); + }); + + test("a non-framed asset miss keeps the JSON error shape", async () => { + const dir = site("json"); + const { assets, base } = registerPage(dir, ""); + const response = await get(assets, `${base}missing.css`); + expect(response?.headers.get("content-type")).toContain("application/json"); + }); + + test("an embedded document over the 2MB annotate cap is refused", async () => { + const dir = site("large"); + writeFileSync(join(dir, "huge.html"), "x".repeat(2 * 1024 * 1024 + 1), "utf-8"); + const { assets, base } = registerPage(dir, ""); + expect((await get(assets, `${base}huge.html`, { "sec-fetch-dest": "iframe" }))?.status).toBe(413); + }); + + // The bug this whole change is about: the catch-all rendering the editor app + // inside an annotated page's embed. + test("framedDocumentNotFound answers a framed request instead of letting the app be served", () => { + const url = new URL("http://localhost/prototype-slash.html"); + const framed = framedDocumentNotFound(new Request(String(url), { headers: { "sec-fetch-dest": "iframe" } }), url); + expect(framed?.status).toBe(404); + expect(framed?.headers.get("content-type")).toContain("text/html"); + expect(framedDocumentNotFound(new Request(String(url)), url)).toBeNull(); + expect( + framedDocumentNotFound(new Request(String(url), { headers: { "sec-fetch-dest": "document" } }), url), + ).toBeNull(); + }); + + // A portable share carries one file: an embed must render empty rather than + // resolve onto the share portal's own catch-all. + test("portable share HTML gives embeds a base nothing resolves against", () => { + const dir = site("share"); + const htmlPath = join(dir, "page.html"); + const html = ''; + writeFileSync(htmlPath, html, "utf-8"); + writeFileSync(join(dir, "embed.html"), "EMBEDDED_SIBLING", "utf-8"); + + const shared = inlineHtmlLocalAssets(html, htmlPath); + expect(shared).toContain(''); + expect(shared).not.toContain("EMBEDDED_SIBLING"); + }); +}); diff --git a/packages/server/annotate.ts b/packages/server/annotate.ts index a75a59558..24311a79d 100644 --- a/packages/server/annotate.ts +++ b/packages/server/annotate.ts @@ -53,7 +53,7 @@ import { isWSL } from "./browser"; import { handleOpenInApps, handleOpenIn } from "./open-in"; import { AI_QUERY_ENDPOINT, createAIRuntime } from "./ai-runtime"; import { isAIEndpointPath, type AIEndpoints } from "@plannotator/ai"; -import { createHtmlAssetRegistry } from "./html-assets"; +import { createHtmlAssetRegistry, framedDocumentNotFound } from "./html-assets"; import { createBunAgentTerminalBridge } from "./agent-terminal"; import { startLiveAppProxy, type LiveAppProxy } from "./live-proxy"; import { @@ -1260,6 +1260,13 @@ export async function startAnnotateServer( return handleApiNotFound(url.pathname); } + // Nested-document guard: a request the browser will render inside a + // frame must never receive the editor app. Relative embeds are + // anchored at their own directory by the asset-route , so + // anything reaching here names a file that genuinely is not there. + const framedMiss = framedDocumentNotFound(req, url); + if (framedMiss) return framedMiss; + // Serve embedded HTML for all other routes (SPA) return new Response(htmlContent, { headers: { "Content-Type": "text/html" }, diff --git a/packages/server/html-assets.ts b/packages/server/html-assets.ts index 642181db8..15d313db0 100644 --- a/packages/server/html-assets.ts +++ b/packages/server/html-assets.ts @@ -1,9 +1,14 @@ import { dirname, resolve as resolvePath } from "path"; import { + HTML_ASSET_ERROR_CSP, + HTML_ASSET_DOCUMENT_CSP, HTML_ASSET_ROUTE_PREFIX, + buildHtmlAssetErrorDocument, encodeHtmlAssetPath, - htmlAssetContentType, - normalizeHtmlAssetRoutePath, + htmlAssetBaseHref, + htmlAssetDocumentHeaders, + isFramedFetchDest, + resolveHtmlAssetRoute, rewriteHtmlAssetReferences, } from "@plannotator/shared/html-assets"; import { @@ -14,6 +19,40 @@ import { export { inlineHtmlLocalAssets }; +/** + * A failure inside the asset route. Framed and `.html` requests get a tiny + * HTML document naming the file; everything else keeps the JSON shape the + * route has always answered with. + */ +function assetError( + status: number, + message: string, + asDocument: boolean, + name?: string, +): Response { + if (!asDocument) return Response.json({ error: message }, { status }); + return new Response(buildHtmlAssetErrorDocument(status, message, name), { + status, + headers: htmlAssetDocumentHeaders(HTML_ASSET_ERROR_CSP), + }); +} + +/** + * The catch-all's guard: a request the browser will render as a nested + * document must never receive the editor app. That is the bug this whole + * change is about — Plannotator rendering inside an annotated page's embed — + * and the `` fix removes the usual way of getting here, so anything + * still arriving is a genuinely missing file and deserves to say so. + */ +export function framedDocumentNotFound(req: Request, url: URL): Response | null { + if (!isFramedFetchDest(req.headers.get("sec-fetch-dest"))) return null; + const name = url.pathname.split("/").filter(Boolean).pop(); + return new Response(buildHtmlAssetErrorDocument(404, "Not found", name), { + status: 404, + headers: htmlAssetDocumentHeaders(HTML_ASSET_ERROR_CSP), + }); +} + export function createHtmlAssetRegistry() { const rootsByToken = new Map(); const tokensByRoot = new Map(); @@ -35,6 +74,11 @@ export function createHtmlAssetRegistry() { return rewriteHtmlAssetReferences( html, (assetPath) => `${HTML_ASSET_ROUTE_PREFIX}/${token}/${encodeHtmlAssetPath(assetPath)}`, + // The base is root-relative on purpose: a srcdoc document resolves its + // own against the PARENT's URL, which is this server, so + // `/api/html-assets//` lands on the right origin without the + // rewrite needing to know the port. + { baseHref: htmlAssetBaseHref(token) }, ); } catch { return html; @@ -45,54 +89,52 @@ export function createHtmlAssetRegistry() { return inlineHtmlLocalAssets(html, htmlFilePath); } - async function handle(_req: Request, url: URL): Promise { - const prefix = `${HTML_ASSET_ROUTE_PREFIX}/`; - if (!url.pathname.startsWith(prefix)) return null; - - const rest = url.pathname.slice(prefix.length); - const slash = rest.indexOf("/"); - if (slash <= 0) { - return Response.json({ error: "Missing asset token or path" }, { status: 404 }); - } - - const token = rest.slice(0, slash); - const root = rootsByToken.get(token); - if (!root) { - return Response.json({ error: "Unknown asset root" }, { status: 404 }); - } - - const assetPath = normalizeHtmlAssetRoutePath(rest.slice(slash + 1)); - if (!assetPath) { - return Response.json({ error: "Invalid asset path" }, { status: 400 }); - } - - const contentType = htmlAssetContentType(assetPath); - if (!contentType) { - return Response.json({ error: "Unsupported asset type" }, { status: 415 }); + async function handle(req: Request, url: URL): Promise { + const decision = resolveHtmlAssetRoute( + { pathname: url.pathname, secFetchDest: req.headers.get("sec-fetch-dest") }, + (token) => rootsByToken.get(token), + ); + if (decision.kind === "not-asset-route") return null; + if (decision.kind === "error") { + return assetError(decision.status, decision.message, decision.asDocument, decision.name); } + const { root, assetPath, contentType, document, asDocument, maxBytes } = decision; const resolved = resolvePath(root, assetPath); if (!isWithinDirectory(resolved, root)) { - return Response.json({ error: "Access denied" }, { status: 403 }); + return assetError(403, "Access denied", asDocument, assetPath); } try { const file = Bun.file(resolved); if (!(await file.exists())) { - return Response.json({ error: "Asset not found" }, { status: 404 }); + return assetError(404, "Not found", asDocument, assetPath); + } + const cap = Math.min(maxBytes, MAX_HTML_ASSET_BYTES); + if (file.size > cap) { + return assetError(413, "Asset too large", asDocument, assetPath); } - if (file.size > MAX_HTML_ASSET_BYTES) { - return Response.json({ error: "Asset too large" }, { status: 413 }); + if (document) { + return new Response(file, { + headers: { + ...htmlAssetDocumentHeaders(HTML_ASSET_DOCUMENT_CSP), + // Kept for parity with the other assets: a nested document load is + // not a CORS request, but a `fetch('./page.html')` from the + // opaque-origin frame is. + "Access-Control-Allow-Origin": "*", + }, + }); } return new Response(file, { headers: { "Content-Type": contentType, "Cache-Control": "no-store", + "X-Content-Type-Options": "nosniff", "Access-Control-Allow-Origin": "*", }, }); } catch { - return Response.json({ error: "Failed to read asset" }, { status: 500 }); + return assetError(500, "Failed to read asset", asDocument, assetPath); } } From d6f421828f98a9d105f67154d4710bd7578f8680 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Thu, 17 Sep 2026 12:49:47 -0700 Subject: [PATCH 3/6] fix(annotate): mirror embedded local documents in the Pi server 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. --- .../server/serverAnnotate-embeds.test.ts | 125 ++++++++++++++++++ apps/pi-extension/server/serverAnnotate.ts | 108 +++++++++------ 2 files changed, 195 insertions(+), 38 deletions(-) create mode 100644 apps/pi-extension/server/serverAnnotate-embeds.test.ts diff --git a/apps/pi-extension/server/serverAnnotate-embeds.test.ts b/apps/pi-extension/server/serverAnnotate-embeds.test.ts new file mode 100644 index 000000000..8d8b98438 --- /dev/null +++ b/apps/pi-extension/server/serverAnnotate-embeds.test.ts @@ -0,0 +1,125 @@ +/** + * Annotate server (Pi/Node): embedded local documents + * + * Node mirror of `annotate embedded local documents` in + * packages/server/annotate-html-assets.test.ts. The decision logic is shared + * (`resolveHtmlAssetRoute` in packages/shared/html-assets.ts, vendored here), + * so what this pins is the Node transport over it: the sandbox CSP and nosniff + * reach the wire, a framed miss renders an HTML document rather than the app, + * and the traversal guard still holds. + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, realpathSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { startAnnotateServer } from "./serverAnnotate.ts"; + +const MINIMAL_HTML = "PLANNOTATOR_APP_SHELL"; + +describe("pi annotate server: embedded local documents", () => { + let savedPort: string | undefined; + let savedRemote: string | undefined; + let savedHistoryFlag: string | undefined; + + beforeEach(() => { + savedPort = process.env.PLANNOTATOR_PORT; + savedRemote = process.env.PLANNOTATOR_REMOTE; + savedHistoryFlag = process.env.PLANNOTATOR_ANNOTATE_HISTORY; + delete process.env.PLANNOTATOR_PORT; + process.env.PLANNOTATOR_REMOTE = "0"; + process.env.PLANNOTATOR_ANNOTATE_HISTORY = "0"; + }); + + afterEach(() => { + if (savedPort === undefined) delete process.env.PLANNOTATOR_PORT; + else process.env.PLANNOTATOR_PORT = savedPort; + if (savedRemote === undefined) delete process.env.PLANNOTATOR_REMOTE; + else process.env.PLANNOTATOR_REMOTE = savedRemote; + if (savedHistoryFlag === undefined) delete process.env.PLANNOTATOR_ANNOTATE_HISTORY; + else process.env.PLANNOTATOR_ANNOTATE_HISTORY = savedHistoryFlag; + }); + + // realpath: containment realpaths the root but keeps a missing target's + // lexical path, which on macOS's symlinked tmpdir would never match. + const siteDir = (label: string) => + realpathSync(mkdtempSync(join(tmpdir(), `plannotator-pi-embed-${label}-`))); + + async function withSession( + label: string, + run: (ctx: { url: string; base: string }) => Promise, + ): Promise { + const dir = siteDir(label); + const pagePath = join(dir, "page.html"); + // The src is assigned by script from data-src, the shape a serve-time + // attribute rewrite cannot reach and the covers. + const html = + ''; + writeFileSync(pagePath, html, "utf-8"); + writeFileSync(join(dir, "embed.html"), "EMBEDDED_SIBLING", "utf-8"); + + const server = await startAnnotateServer({ + markdown: "", + filePath: pagePath, + htmlContent: MINIMAL_HTML, + rawHtml: html, + renderHtml: true, + }); + try { + const plan = (await (await fetch(`${server.url}/api/plan`)).json()) as { rawHtml?: string }; + const base = plan.rawHtml?.match(/ { + await withSession("serve", async ({ url, base }) => { + const response = await fetch(`${url}${base}embed.html?step=result`, { + headers: { "sec-fetch-dest": "iframe" }, + }); + expect(response.status).toBe(200); + expect(response.headers.get("content-type")).toContain("text/html"); + expect(response.headers.get("content-security-policy")).toBe("sandbox allow-scripts"); + expect(response.headers.get("x-content-type-options")).toBe("nosniff"); + expect(await response.text()).toContain("EMBEDDED_SIBLING"); + }); + }); + + // The bug: the catch-all answering a nested-document request with the + // editor app, so every embed rendered a second Plannotator. + test("a framed request for an unknown path gets a 404 document, never the app", async () => { + await withSession("framed", async ({ url }) => { + const response = await fetch(`${url}/prototype-slash.html`, { + headers: { "sec-fetch-dest": "iframe" }, + }); + expect(response.status).toBe(404); + expect(response.headers.get("content-type")).toContain("text/html"); + const body = await response.text(); + expect(body).toContain("prototype-slash.html"); + expect(body).not.toContain("PLANNOTATOR_APP_SHELL"); + }); + }); + + test("an ordinary top-level navigation still gets the app shell", async () => { + await withSession("spa", async ({ url }) => { + const response = await fetch(`${url}/some/spa/route`, { + headers: { "sec-fetch-dest": "document" }, + }); + expect(await response.text()).toContain("PLANNOTATOR_APP_SHELL"); + }); + }); + + test("refuses an embed that climbs out of the annotated file's directory", async () => { + await withSession("escape", async ({ url, base }) => { + for (const spelling of ["../../etc/hosts.html", "%2e%2e/secret.html"]) { + const response = await fetch(`${url}${base}${spelling}`, { + headers: { "sec-fetch-dest": "iframe" }, + }); + expect(response.status).toBeGreaterThanOrEqual(400); + } + }); + }); +}); diff --git a/apps/pi-extension/server/serverAnnotate.ts b/apps/pi-extension/server/serverAnnotate.ts index fbd5d4424..dab9e78a6 100644 --- a/apps/pi-extension/server/serverAnnotate.ts +++ b/apps/pi-extension/server/serverAnnotate.ts @@ -65,10 +65,15 @@ import { getExtraMarkdownExtensions, MAX_ANNOTATABLE_FILE_BYTES, resolveUserPath import { createExternalAnnotationHandler } from "./external-annotations.ts"; import { createNodeAgentTerminalBridge } from "./agent-terminal.ts"; import { + HTML_ASSET_DOCUMENT_CSP, + HTML_ASSET_ERROR_CSP, HTML_ASSET_ROUTE_PREFIX, + buildHtmlAssetErrorDocument, encodeHtmlAssetPath, - htmlAssetContentType, - normalizeHtmlAssetRoutePath, + htmlAssetBaseHref, + htmlAssetDocumentHeaders, + isFramedFetchDest, + resolveHtmlAssetRoute, rewriteHtmlAssetReferences, } from "../generated/html-assets.ts"; import { inlineHtmlLocalAssets, isWithinDirectory, MAX_HTML_ASSET_BYTES, resolveOpenInTarget } from "../generated/html-assets-node.ts"; @@ -118,6 +123,12 @@ function parseOptionalApprovalBody(req: IncomingMessage): Promise(); const tokensByRoot = new Map(); @@ -139,6 +150,9 @@ function createHtmlAssetRegistry() { return rewriteHtmlAssetReferences( htmlContent, (assetPath) => `${HTML_ASSET_ROUTE_PREFIX}/${token}/${encodeHtmlAssetPath(assetPath)}`, + // Root-relative on purpose: a srcdoc document resolves its own + // against the PARENT's URL, which is this server. + { baseHref: htmlAssetBaseHref(token) }, ); } catch { return htmlContent; @@ -149,60 +163,70 @@ function createHtmlAssetRegistry() { return inlineHtmlLocalAssets(htmlContent, htmlFilePath); } - function handle(res: import("node:http").ServerResponse, url: URL): boolean { - const prefix = `${HTML_ASSET_ROUTE_PREFIX}/`; - if (!url.pathname.startsWith(prefix)) return false; - - const rest = url.pathname.slice(prefix.length); - const slash = rest.indexOf("/"); - if (slash <= 0) { - json(res, { error: "Missing asset token or path" }, 404); - return true; - } - - const token = rest.slice(0, slash); - const root = rootsByToken.get(token); - if (!root) { - json(res, { error: "Unknown asset root" }, 404); - return true; - } - - const assetPath = normalizeHtmlAssetRoutePath(rest.slice(slash + 1)); - if (!assetPath) { - json(res, { error: "Invalid asset path" }, 400); - return true; + function assetError( + res: import("node:http").ServerResponse, + status: number, + message: string, + asDocument: boolean, + name?: string, + ): void { + if (!asDocument) { + json(res, { error: message }, status); + return; } + res.writeHead(status, htmlAssetDocumentHeaders(HTML_ASSET_ERROR_CSP)); + res.end(buildHtmlAssetErrorDocument(status, message, name)); + } - const contentType = htmlAssetContentType(assetPath); - if (!contentType) { - json(res, { error: "Unsupported asset type" }, 415); + function handle( + req: import("node:http").IncomingMessage, + res: import("node:http").ServerResponse, + url: URL, + ): boolean { + const decision = resolveHtmlAssetRoute( + { pathname: url.pathname, secFetchDest: firstHeader(req.headers["sec-fetch-dest"]) }, + (token) => rootsByToken.get(token), + ); + if (decision.kind === "not-asset-route") return false; + if (decision.kind === "error") { + assetError(res, decision.status, decision.message, decision.asDocument, decision.name); return true; } + const { root, assetPath, contentType, document, asDocument, maxBytes } = decision; const resolved = resolvePath(root, assetPath); if (!isWithinDirectory(resolved, root)) { - json(res, { error: "Access denied" }, 403); + assetError(res, 403, "Access denied", asDocument, assetPath); return true; } try { if (!existsSync(resolved)) { - json(res, { error: "Asset not found" }, 404); + assetError(res, 404, "Not found", asDocument, assetPath); return true; } const stat = statSync(resolved); - if (stat.size > MAX_HTML_ASSET_BYTES) { - json(res, { error: "Asset too large" }, 413); + if (stat.size > Math.min(maxBytes, MAX_HTML_ASSET_BYTES)) { + assetError(res, 413, "Asset too large", asDocument, assetPath); return true; } - res.writeHead(200, { - "Content-Type": contentType, - "Cache-Control": "no-store", - "Access-Control-Allow-Origin": "*", - }); + res.writeHead( + 200, + document + ? { + ...htmlAssetDocumentHeaders(HTML_ASSET_DOCUMENT_CSP), + "Access-Control-Allow-Origin": "*", + } + : { + "Content-Type": contentType, + "Cache-Control": "no-store", + "X-Content-Type-Options": "nosniff", + "Access-Control-Allow-Origin": "*", + }, + ); res.end(readFileSync(resolved)); } catch { - json(res, { error: "Failed to read asset" }, 500); + assetError(res, 500, "Failed to read asset", asDocument, assetPath); } return true; } @@ -917,7 +941,7 @@ export async function startAnnotateServer(options: { } } else if (url.pathname === "/api/image") { handleImageRequest(res, url); - } else if (htmlAssets.handle(res, url)) { + } else if (htmlAssets.handle(req, res, url)) { return; } else if (url.pathname === "/api/upload" && req.method === "POST") { await handleUploadRequest(req, res); @@ -1140,6 +1164,14 @@ export async function startAnnotateServer(options: { await handleSaveNotesRequest(req, res); } else if (url.pathname.startsWith("/api/")) { handleApiNotFound(res, url.pathname); + } else if (isFramedFetchDest(firstHeader(req.headers["sec-fetch-dest"]))) { + // Nested-document guard: a request the browser will render inside a + // frame must never receive the editor app. Relative embeds are + // anchored at their own directory by the asset-route , so + // anything reaching here names a file that genuinely is not there. + const name = url.pathname.split("/").filter(Boolean).pop(); + res.writeHead(404, htmlAssetDocumentHeaders(HTML_ASSET_ERROR_CSP)); + res.end(buildHtmlAssetErrorDocument(404, "Not found", name)); } else { html(res, options.htmlContent); } From 171ba6ab44e4d077d4ccd9e8d7d6d043d321fe06 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Thu, 17 Sep 2026 12:49:47 -0700 Subject: [PATCH 4/6] feat(annotate): pin an embedded document as one element while armed 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 '; + postBridge({ type: "plannotator-bridge-set-input-method", method: "pinpoint" }); + postBridge({ type: "plannotator-bridge-set-annotate-mode", active: true }); + expect(document.body.hasAttribute("data-plannotator-frame-inert")).toBe(true); + + postBridge({ type: "plannotator-bridge-set-annotate-mode", active: false }); + expect(document.body.hasAttribute("data-plannotator-frame-inert")).toBe(false); + + // Re-arming, then switching input method away, also clears it. + postBridge({ type: "plannotator-bridge-set-annotate-mode", active: true }); + expect(document.body.hasAttribute("data-plannotator-frame-inert")).toBe(true); + postBridge({ type: "plannotator-bridge-set-input-method", method: "drag" }); + expect(document.body.hasAttribute("data-plannotator-frame-inert")).toBe(false); + + // String-level guard: happy-dom honors neither pointer-events nor :is(), + // so the attribute alone would pass with a typo'd rule. The rule that + // actually makes the embed pinnable must ship in the annotation CSS. + expect(ANNOTATION_HIGHLIGHT_CSS).toContain( + "body[data-plannotator-frame-inert] :is(iframe, frame, embed, object) {", + ); + postBridge({ type: "plannotator-bridge-set-input-method", method: "pinpoint" }); + document.body.replaceChildren(); + }); + test("deeply nested targets get no anchor instead of a quadratic selector walk", async () => { // Each ancestor step costs a document-wide uniqueness query against a // growing selector, so unbounded depth freezes the tab on one click From cfbdac3b6d2b93f2bfc8b30b828efeb2b9b29ffc Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Thu, 17 Sep 2026 13:11:01 -0700 Subject: [PATCH 5/6] fix(annotate): an armed click inside an embed pins the frame, not its 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. --- .../components/html-viewer/bridge-script.ts | 23 ++++++++++++ .../ui/components/html-viewer/srcdoc.test.ts | 37 +++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/packages/ui/components/html-viewer/bridge-script.ts b/packages/ui/components/html-viewer/bridge-script.ts index 0da0c49a2..850955b92 100644 --- a/packages/ui/components/html-viewer/bridge-script.ts +++ b/packages/ui/components/html-viewer/bridge-script.ts @@ -926,11 +926,34 @@ export const BRIDGE_SCRIPT = `(function() { var svgGroup = node.closest('g'); if (svgGroup) node = svgGroup; } + node = preferInertFrameAt(node, x, y); node = promoteTinyTarget(node); if (node === document.body || node === document.documentElement) return null; return node; } + // While frames are pointer-transparent (armed pinpoint, srcdoc sessions), + // hit-testing passes THROUGH an embedded document to the container painted + // behind it — so a click on an embed would pin its wrapper div. The embed is + // what the reviewer is pointing at and what the anchor must name, so a point + // inside a frame's own rect resolves to that frame. Bounded to the frames + // inside the element already resolved, so it costs nothing on ordinary pages. + var FRAME_SELECTOR = 'iframe,frame,embed,object'; + function framesArePointerInert() { + return !LIVE && annotateModeActive && currentInputMethod === 'pinpoint'; + } + function preferInertFrameAt(node, x, y) { + if (!framesArePointerInert() || !node.querySelectorAll) return node; + if (node.matches && node.matches(FRAME_SELECTOR)) return node; + var frames = node.querySelectorAll(FRAME_SELECTOR); + for (var i = 0; i < frames.length && i < 64; i++) { + var r = frames[i].getBoundingClientRect(); + if (r.width <= 0 || r.height <= 0) continue; + if (x >= r.left && x <= r.right && y >= r.top && y <= r.bottom) return frames[i]; + } + return node; + } + // Last-position reuse: a pointer that moved under 2px within 16ms resolves // to the cached element instead of re-hit-testing. The scroll reconcile // invalidates this cache — same point, different element after a scroll. diff --git a/packages/ui/components/html-viewer/srcdoc.test.ts b/packages/ui/components/html-viewer/srcdoc.test.ts index b4d2d7f6d..6c8189f0b 100644 --- a/packages/ui/components/html-viewer/srcdoc.test.ts +++ b/packages/ui/components/html-viewer/srcdoc.test.ts @@ -1091,6 +1091,43 @@ describe.if(hasDom)("bridge theme handler (DOM)", () => { document.body.replaceChildren(); }); + // Consequence of making frames pointer-transparent: hit-testing passes + // THROUGH the embed to the container painted behind it, so without the + // frame preference an armed click on an embed would pin its wrapper div — + // the reviewer's comment would name the wrong element. + test("an armed click inside an embed's box pins the frame, not the container behind it", async () => { + document.body.innerHTML = '
'; + const wrapper = document.querySelector("div.frame")!; + const frame = document.querySelector("iframe")!; + frame.getBoundingClientRect = () => rectOf(0, 0, 600, 400); + postBridge({ type: "plannotator-bridge-set-input-method", method: "pinpoint" }); + postBridge({ type: "plannotator-bridge-set-annotate-mode", active: true }); + + await new Promise((resolve) => setTimeout(resolve, 0)); + const messages: Array> = []; + const collect = (event: MessageEvent) => { + const data = bridgeMessageData(event); + if (data?.type === "plannotator-bridge-selection") messages.push(data); + }; + window.addEventListener("message", collect); + wrapper.dispatchEvent( + new MouseEvent("click", { bubbles: true, cancelable: true, clientX: 100, clientY: 100 }), + ); + await new Promise((resolve) => setTimeout(resolve, 0)); + window.removeEventListener("message", collect); + + expect(messages.length).toBe(1); + const context = messages[0]!.context as { tag?: string; path?: string } | undefined; + expect(context?.tag).toBe("iframe"); + expect(context?.path).toContain("> iframe"); + // The pin box covers the embed, not the wrapper. + expect(messages[0]!.rect).toMatchObject({ width: 600, height: 400 }); + + postBridge({ type: "plannotator-bridge-cancel-selection" }); + postBridge({ type: "plannotator-bridge-set-input-method", method: "pinpoint" }); + document.body.replaceChildren(); + }); + test("deeply nested targets get no anchor instead of a quadratic selector walk", async () => { // Each ancestor step costs a document-wide uniqueness query against a // growing selector, so unbounded depth freezes the tab on one click From e74614588a4e9ce3b28ff6f073a181568ba9fc05 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Thu, 17 Sep 2026 13:13:41 -0700 Subject: [PATCH 6/6] docs: embedded local documents in raw-HTML annotate sessions --- AGENTS.md | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 671dcdf97..da6a18e4c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -675,7 +675,7 @@ During normal plan review, an Archive sidebar tab provides the same browsing via | `/api/approve` | POST | Approve without feedback (review-gate UX, `--gate`) | | `/api/exit` | POST | Close session without feedback | | `/api/save-notes` | POST | Save to external note apps (Obsidian, Bear, Octarine) | -| `/api/html-assets//` | GET | Serve relative support assets for raw HTML annotation sessions | +| `/api/html-assets//` | GET | Serve relative support assets for raw HTML annotation sessions, and the sibling `.html`/`.htm` documents an annotated page EMBEDS (`