fix(web): bound retained app documents by size; correct the cookie claim

Copilot review round 5 on #2100.

- The store bounded entry COUNT only, and the route admits 8Mi code units per
  document with a one-hour TTL — ~256Mi retainable on documents a server
  under test chooses the size of. Added a 64Mi retained-size budget evicted
  oldest-first alongside the count bound, with a running total kept in step
  with the map through a single `drop` seam so it cannot drift.
- The isolation claim was wrong about cookies. A separate port makes a
  separate ORIGIN, so DOM access, localStorage, sessionStorage and IndexedDB
  are isolated — but cookies are keyed by host and path, ignoring port, so an
  app served here shares the 127.0.0.1 jar with the Inspector and every other
  loopback service. Not a hole in the Inspector's own auth (the API token is
  a header plus a window global and sessionStorage, none of them cookies) and
  not closable without a distinct host this listener cannot mint, so it is
  stated plainly in both the module header and clients/web/README.md rather
  than left as a claim that is false for one of the three surfaces it names.
- Removed a shell-escaping artifact I left in the migration guide.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>

clients/web/README.md

@@ -182,6 +182,23 @@ is *for* — a real, allowlistable origin — without minting a port or a DNS na
 per app, at the cost of not being a per-app isolation boundary: two such apps
 share `localStorage`, `sessionStorage`, and cookies for that origin.
 
+**And the separate port does not isolate cookies from the rest of loopback.**
+A distinct port makes a distinct *origin*, so the origin-scoped surfaces —
+DOM/scripting access, `localStorage`, `sessionStorage`, IndexedDB — are isolated
+from the sandbox proxy and from the Inspector page. **Cookies are not
+origin-scoped**: they are keyed by host and path, ignoring port. An app served
+here therefore shares the `127.0.0.1` cookie jar with the Inspector on `6274`
+and with anything else on loopback — it can read any non-`HttpOnly` cookie set
+for that host, and set `Path=/` cookies those services will receive.
+
+This is not a hole in the Inspector's own auth: the API token travels in the
+`x-mcp-remote-auth` header and lives in a `window` global and `sessionStorage`,
+neither of which another origin can read. Closing the cookie gap properly needs
+a distinct *host*, which this listener cannot mint (the Inspector owns no DNS,
+and a second loopback address is not portable) — so it is stated rather than
+papered over. Don't run the Inspector beside a loopback service whose session
+cookie you would not hand to an app under test.
+
 **Every failure falls back rather than blanking the app.** No app-origin
 listener, a port that never bound, an older backend with no
 `POST /api/app-document` route, a network error — each renders the app the

clients/web/server/app-origin-controller.ts

@@ -27,8 +27,28 @@
  * the spec field is *for* (a real, allowlistable origin) without minting a port
  * — or a DNS name — per app. The consequence is that this origin is **not** a
  * per-app isolation boundary: two apps served here share `localStorage`,
- * `sessionStorage`, and cookies for it. They remain isolated from the sandbox
- * proxy and from the Inspector itself, which are different origins.
+ * `sessionStorage`, and cookies for it.
+ *
+ * ## What the separate port does and does not isolate
+ *
+ * A distinct port makes a distinct **origin**, so the usual origin-scoped
+ * surfaces are isolated from the sandbox proxy and from the Inspector page:
+ * DOM and scripting access, `localStorage`, `sessionStorage`, IndexedDB.
+ *
+ * **Cookies are the exception, and they are not isolated.** Cookies are scoped
+ * by host (and path), not by port — so an app served here shares the
+ * `127.0.0.1` cookie jar with the Inspector on `6274` and with every other
+ * service on loopback. It can read any non-`HttpOnly` cookie set for that host
+ * and set `Path=/` cookies those services will receive.
+ *
+ * That is a real caveat, but it is not a hole in the Inspector's own auth: the
+ * API token travels in the `x-mcp-remote-auth` header and is held in a
+ * `window` global and `sessionStorage`, both of which a different origin
+ * cannot read. Closing the cookie gap properly needs a distinct *host*, which
+ * this listener cannot mint — the Inspector owns no DNS, and a second loopback
+ * address is not portable. So it is documented rather than papered over; do
+ * not run the Inspector beside a loopback service whose session cookie you
+ * would not hand to an app under test.
  *
  * ## Why granting `allow-same-origin` here is not a regression
  *
@@ -77,6 +97,21 @@ export const DEFAULT_APP_ORIGIN_PORT = 6278;
 /** Documents kept at once. Oldest is evicted past this; see {@link publish}. */
 const MAX_DOCUMENTS = 32;
 
+/**
+ * Total retained document size, in UTF-16 code units, across every live entry.
+ *
+ * {@link MAX_DOCUMENTS} alone bounds the *count*, and the route admits up to
+ * 8Mi code units per document — so the count bound permits ~256Mi retained for
+ * a full hour, enough to OOM a memory-limited Inspector on documents a server
+ * chose the size of. Both bounds apply; whichever binds first evicts.
+ *
+ * 64Mi is roughly a full complement of ordinary widgets (a real app document is
+ * tens to hundreds of KiB) or a couple of pathological ones. An app large
+ * enough to evict its predecessors still renders — the frame fetches its `src`
+ * immediately, long before anything could push it out.
+ */
+const MAX_RETAINED_CHARS = 64 * 1024 * 1024;
+
 /**
  * How long a published document stays fetchable. An iframe fetches its `src`
  * once, so this only has to outlive the gap between publishing and the frame
@@ -192,6 +227,8 @@ export function parseDocumentId(url: string | undefined): string | null {
 
 interface StoredDocument extends AppDocument {
   expiresAt: number;
+  /** Retained size in UTF-16 code units, against {@link MAX_RETAINED_CHARS}. */
+  size: number;
 }
 
 export function createAppOriginController(
@@ -206,13 +243,25 @@ export function createAppOriginController(
 
   // Insertion-ordered, so the first key is the oldest entry.
   const documents = new Map<string, StoredDocument>();
+  // Running sum of every live entry's `size`, kept in step with the map on
+  // every insert and every delete. Tracked rather than recomputed: eviction
+  // runs per publish and summing the map there would be O(n) on a hot path.
+  let retainedChars = 0;
+
+  /** The single removal seam, so the running total cannot drift from the map. */
+  function drop(id: string): void {
+    const doc = documents.get(id);
+    if (!doc) return;
+    retainedChars -= doc.size;
+    documents.delete(id);
+  }
 
   const FRAME_ANCESTORS = frameAncestorsDirective(embedderOrigins);
 
   /** Drop everything past its TTL. Cheap: the map is bounded by MAX_DOCUMENTS. */
   function evictExpired(now: number): void {
     for (const [id, doc] of documents) {
-      if (doc.expiresAt <= now) documents.delete(id);
+      if (doc.expiresAt <= now) drop(id);
     }
   }
 
@@ -235,7 +284,7 @@ export function createAppOriginController(
           const id = req.method === "GET" ? parseDocumentId(req.url) : null;
           const doc = id ? documents.get(id) : undefined;
           if (!doc || doc.expiresAt <= Date.now()) {
-            if (id) documents.delete(id);
+            if (id) drop(id);
             res.writeHead(404, { "Content-Type": "text/plain" });
             res.end("Not Found");
             return;
@@ -293,6 +342,7 @@ export function createAppOriginController(
           server = null;
           origin = null;
           documents.clear();
+          retainedChars = 0;
           settle({ port: 0, url: "" });
         });
 
@@ -322,6 +372,7 @@ export function createAppOriginController(
 
     async close(): Promise<void> {
       documents.clear();
+      retainedChars = 0;
       if (!server) return;
       return new Promise((resolve) => {
         server!.close(() => {
@@ -340,17 +391,23 @@ export function createAppOriginController(
       if (!origin) return null;
       const now = Date.now();
       evictExpired(now);
-      // Bound the map even if nothing has expired: drop the oldest entry. An
-      // evicted document's frame has long since loaded, and the alternative is
-      // an unbounded store fed by every app the user opens in a session.
-      while (documents.size >= MAX_DOCUMENTS) {
+      const size = doc.html.length + (doc.csp?.length ?? 0);
+      // Bound the store even if nothing has expired: drop oldest-first until
+      // BOTH the count and the retained-size budget have room. An evicted
+      // document's frame has long since loaded, and the alternative is a store
+      // fed — in count and in bytes — by whatever the servers under test send.
+      while (
+        documents.size >= MAX_DOCUMENTS ||
+        (documents.size > 0 && retainedChars + size > MAX_RETAINED_CHARS)
+      ) {
         const oldest = documents.keys().next();
-        /* v8 ignore next -- `size >= MAX_DOCUMENTS` guarantees a first key. */
+        /* v8 ignore next -- both loop conditions require a non-empty map. */
         if (oldest.done) break;
-        documents.delete(oldest.value);
+        drop(oldest.value);
       }
       const id = randomBytes(16).toString("hex");
-      documents.set(id, { ...doc, expiresAt: now + DOCUMENT_TTL_MS });
+      documents.set(id, { ...doc, expiresAt: now + DOCUMENT_TTL_MS, size });
+      retainedChars += size;
       return { url: `${origin}${DOC_PATH_PREFIX}${id}` };
     },
   };

clients/web/src/test/integration/server/app-origin-controller.test.ts

@@ -238,6 +238,30 @@ describe("createAppOriginController", () => {
     expect((await fetch(first.url)).status).toBe(404);
   });
 
+  it("evicts against a retained-size budget, not just the entry count", async () => {
+    // The count bound alone permits MAX_DOCUMENTS * 8Mi retained for an hour,
+    // on documents a server under test chooses the size of. Publish a few
+    // large ones — well under the 32-entry cap — and the earliest must stop
+    // being served.
+    const c = createAppOriginController({ port: 0, host: "127.0.0.1" });
+    controller = c;
+    const { url } = await c.start();
+    // 24Mi each: three fit under no count bound, but the third pushes total
+    // retained past the 64Mi budget and the first has to go.
+    const big = "x".repeat(24 * 1024 * 1024);
+    const first = c.publish({ html: big });
+    const second = c.publish({ html: big });
+    expect(first).not.toBeNull();
+    expect(second).not.toBeNull();
+    const third = c.publish({ html: big });
+    expect(third).not.toBeNull();
+
+    expect((await fetch(first!.url)).status).toBe(404);
+    // The newest is always served — its frame is about to fetch it.
+    expect((await fetch(third!.url)).status).toBe(200);
+    expect(url).toBe(c.getOrigin());
+  });
+
   it("404s a document past its TTL", async () => {
     // Fake `Date` ONLY — the TTL is the sole thing that reads it, and faking
     // the timer queue too would deadlock the real HTTP round-trip below.

docs/v1-to-v2-migration.md

@@ -368,7 +368,7 @@ Notes:
 
 **"The Apps tab is blank in Docker / over SSH."** The MCP Apps sandbox is a second port (`6275` by default). Publish/forward it too — see [MCP Apps caveats](../clients/web/README.md#hosting-on-a-network).
 
-**"One particular App'"'"'s frame is blank, while other Apps render fine."** That App declares `_meta.ui.domain`, so it is served from the **third** port (`MCP_APP_ORIGIN_PORT`, `6278` by default) rather than from the sandbox — publish or forward that port too. Expect no console warning: the backend published the document successfully, so from its side nothing failed, and the browser'"'"'s inability to load a cross-origin URL is not observable from the page. The neighbouring symptom is different — an App that *renders* and has its requests rejected by its own backend is running on the opaque-origin fallback, which means publication itself failed (no listener, or a port that never bound) and the console **does** carry a warning naming `_meta.ui.domain`. See [MCP App dedicated origins](../clients/web/README.md#mcp-app-dedicated-origins-metauidomain).
+**"One particular App's frame is blank, while other Apps render fine."** That App declares `_meta.ui.domain`, so it is served from the **third** port (`MCP_APP_ORIGIN_PORT`, `6278` by default) rather than from the sandbox — publish or forward that port too. Expect no console warning: the backend published the document successfully, so from its side nothing failed, and the browser's inability to load a cross-origin URL is not observable from the page. The neighbouring symptom is different — an App that *renders* and has its requests rejected by its own backend is running on the opaque-origin fallback, which means publication itself failed (no listener, or a port that never bound) and the console **does** carry a warning naming `_meta.ui.domain`. See [MCP App dedicated origins](../clients/web/README.md#mcp-app-dedicated-origins-metauidomain).
 
 **"The page never loads inside a VS Code dev container."** Fixed in the release carrying #1951 — upgrade; on an older version, work around it with `HOST=127.0.0.1`. Before that fix the default `HOST=localhost` bound IPv6 loopback **only** on Linux, so anything connecting over IPv4 `127.0.0.1` was refused — which is what a dev container's port forwarder does. The terminal printed a normal banner while the browser spun forever; the failure is a **hang** rather than a connection error because the forwarder accepts on the host side before failing to connect inward. The same fix covers an `ssh -L 6274:127.0.0.1:6274` tunnel and a container healthcheck. It does **not** describe the official Docker image (it binds `0.0.0.0`, so it was never affected) or a plain `docker run -p` against a loopback bind, which is unreachable across the network namespace for an unrelated reason.