Skip to content

fix(annotate): render embedded local HTML documents instead of loading the app inside every embed #1561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
backnotprop merged 6 commits into from
Sep 17, 2026
Merged

fix(annotate): render embedded local HTML documents instead of loading the app inside every embed #1561

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
61b1282
fix(annotate): anchor raw-HTML documents at their own asset directory
backnotprop Sep 17, 2026
97cdd3b
fix(annotate): serve embedded local documents from the Bun asset route
backnotprop Sep 17, 2026
d6f4218
fix(annotate): mirror embedded local documents in the Pi server
backnotprop Sep 17, 2026
171ba6a
feat(annotate): pin an embedded document as one element while armed
backnotprop Sep 17, 2026
cfbdac3
fix(annotate): an armed click inside an embed pins the frame, not its…
backnotprop Sep 17, 2026
e746145
docs: embedded local documents in raw-HTML annotate sessions
backnotprop Sep 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions AGENTS.md
View file
Open in desktop

Large diffs are not rendered by default.

125 changes: 125 additions & 0 deletions apps/pi-extension/server/serverAnnotate-embeds.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 = "<html><body>PLANNOTATOR_APP_SHELL</body></html>";

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<void>,
): Promise<void> {
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 <base href> covers.
const html =
'<!doctype html><html><head></head><body><iframe data-src="embed.html"></iframe></body></html>';
writeFileSync(pagePath, html, "utf-8");
writeFileSync(join(dir, "embed.html"), "<html><body>EMBEDDED_SIBLING</body></html>", "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(/<base href="([^"]+)"/)?.[1];
expect(base).toMatch(/^\/api\/html-assets\/[0-9a-f]{16}\/$/);
await run({ url: server.url, base: base! });
} finally {
server.stop();
}
}

test("serves a sibling document with the sandbox CSP and preserves its query string", async () => {
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);
}
});
});
});
108 changes: 70 additions & 38 deletions apps/pi-extension/server/serverAnnotate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -118,6 +123,12 @@ function parseOptionalApprovalBody(req: IncomingMessage): Promise<Record<string,
});
}

/** node:http repeats some headers; the asset route only ever wants the first. */
function firstHeader(value: string | string[] | undefined): string | null {
if (Array.isArray(value)) return value[0] ?? null;
return value ?? null;
}

function createHtmlAssetRegistry() {
const rootsByToken = new Map<string, string>();
const tokensByRoot = new Map<string, string>();
Expand All @@ -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
// <base href> against the PARENT's URL, which is this server.
{ baseHref: htmlAssetBaseHref(token) },
);
} catch {
return htmlContent;
Expand All @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 <base href>, 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);
}
Expand Down
Loading