From 6b8b631e375f35b1794b8362763cc28eda81adc2 Mon Sep 17 00:00:00 2001 From: Yaowei Zheng Date: Sun, 26 Jul 2026 14:52:25 +0800 Subject: [PATCH] fix(web): use the separate-origin preview for in-app HTML rendering (#59) Co-authored-by: Claude Fable 5 --- packages/docs/content/server-api.en.md | 5 +- packages/docs/content/server-api.zh.md | 5 +- packages/server/src/services/preview-token.ts | 19 +- .../server/test/workspace-preview.test.ts | 33 +++ packages/web/e2e/chat.spec.mjs | 11 +- packages/web/e2e/files-switch.spec.mjs | 90 ++++++++ .../src/features/chat/workspace-browser.tsx | 212 +++++++++++++++--- packages/web/src/state/auth.tsx | 14 ++ 8 files changed, 340 insertions(+), 49 deletions(-) create mode 100644 packages/web/e2e/files-switch.spec.mjs diff --git a/packages/docs/content/server-api.en.md b/packages/docs/content/server-api.en.md index 0df56a3..7a4f701 100644 --- a/packages/docs/content/server-api.en.md +++ b/packages/docs/content/server-api.en.md @@ -168,11 +168,11 @@ Workspace files may be Agent-generated, so `GET /files/content` treats them as u | `preview=1` | the real type (`text/html`, `image/svg+xml`, …) | `inline` | `sandbox allow-scripts allow-popups allow-modals allow-forms`, sent only for `.html` / `.htm` / `.svg` | | `download=1` | the real type | `attachment` | — | -The filename always rides along as `filename*=UTF-8''` with percent-encoding. `preview=1` renders inside the Files panel's sandboxed iframe, and is also the fallback for "open in a new tab" when no separate preview origin is available: the document keeps its real type and does render and run, but the sandbox deliberately omits `allow-same-origin`, so it lands in an opaque origin and can reach neither this origin's cookies nor the API. That isolation is also why `localStorage`, `document.cookie` and third-party embeds do not work there. +The filename always rides along as `filename*=UTF-8''` with percent-encoding. `preview=1` is where the preview redirect falls back when no separate preview origin is available: the document keeps its real type and does render and run, but the sandbox deliberately omits `allow-same-origin`, so it lands in an opaque origin and can reach neither this origin's cookies nor the API. That isolation is also why `localStorage`, `document.cookie` and third-party embeds do not work there. ### Preview on a separate origin -"Open in a new tab" goes through `GET /files/preview-redirect?path=`, which authenticates the caller, then mints a short-lived HMAC token and 302s to a **different origin**: +Both the Files panel's rendered HTML view (an iframe) and "open in a new tab" go through `GET /files/preview-redirect?path=`, which authenticates the caller, then mints a short-lived HMAC token and 302s to a **different origin**: ```text GET /api/sessions/:sessionId/files/preview-redirect?path=index.html @@ -181,6 +181,7 @@ GET /preview// (unauthenticated; the token is th ``` - **Why a separate origin.** The page needs a real origin to have working storage, cookies and third-party embeds — but it must not be the app's origin, or Agent-written HTML would run with the session cookie. Locally the app is canonicalized onto `localhost` and previews are served from `127.0.0.1`; cookies are keyed by host and ignore port, so those are separate cookie jars while a second port would not be. Otherwise `PENGUIN_PREVIEW_ORIGIN` applies; with neither (a wildcard or non-loopback bind, or the variable unset), the redirect falls back to the same-origin sandbox above and `previewIsolated` on `GET /api/me` reports `false` so the UI can say so first. +- **In-app rendering rides the same URL.** The Files panel embeds the redirect URL in an iframe sandboxed with `allow-scripts allow-same-origin allow-forms allow-popups allow-modals allow-downloads` — `allow-same-origin` grants the preview origin's identity, not the App's, so this stays strictly tighter than the sandbox-free new tab. Without a separate preview origin the panel instead falls back to inline `srcdoc` rendering (`allow-scripts` only, plus an in-memory storage shim), where relative subresources cannot load. Note that some browsers partition or block storage inside a cross-site iframe, so a page may behave slightly differently in the panel than in the top-level tab. - **The preview host serves only `/preview/*`.** It is the same process as the app, so it answers `/api` with `401` and `302`s every other route to the canonical app host. A session cookie is therefore never set or honored on the preview host, and Agent HTML there cannot reach the API same-origin. (For a deployed `PENGUIN_PREVIEW_ORIGIN`, the reverse proxy must enforce the equivalent: route only `/preview/*` to the app on that origin.) - **Path-based, not a query parameter**, so a page's relative subresources (`app.js`, `style.css`, images) resolve against the document and load under the same token. - **The token binds the Session, the preview host and an expiry.** The host binding is load-bearing: the same process also answers on the app origin, so `/preview/...` refuses to serve there — otherwise it would be a same-origin XSS. Access is read-only and scoped to that Session's Workspace, and the path is re-resolved server-side, so `..` and symlink escapes are rejected as before. diff --git a/packages/docs/content/server-api.zh.md b/packages/docs/content/server-api.zh.md index 92a0f52..7ad2fd9 100644 --- a/packages/docs/content/server-api.zh.md +++ b/packages/docs/content/server-api.zh.md @@ -168,11 +168,11 @@ Workspace 文件可能由 Agent 生成，`GET /files/content` 一律按不可信 | `preview=1` | 真实类型（`text/html`、`image/svg+xml` 等） | `inline` | `sandbox allow-scripts allow-popups allow-modals allow-forms`，仅对 `.html` / `.htm` / `.svg` 下发 | | `download=1` | 真实类型 | `attachment` | 无 | -文件名始终以 `filename*=UTF-8''` 形式携带（百分号编码）。`preview=1` 用于 Files 面板内的沙箱 iframe 渲染，同时也是“新页面打开”在没有独立预览源时的回退：文档保留真实类型，可以正常渲染并执行脚本，但沙箱刻意不含 `allow-same-origin`，因此它落在一个不透明源里，既拿不到本源的 Cookie，也调不动 API。这份隔离也正是那里 `localStorage`、`document.cookie` 与第三方 embed 全都不可用的原因。 +文件名始终以 `filename*=UTF-8''` 形式携带（百分号编码）。`preview=1` 是预览跳转在没有独立预览源时的回退目标：文档保留真实类型，可以正常渲染并执行脚本，但沙箱刻意不含 `allow-same-origin`，因此它落在一个不透明源里，既拿不到本源的 Cookie，也调不动 API。这份隔离也正是那里 `localStorage`、`document.cookie` 与第三方 embed 全都不可用的原因。 ### 独立源预览 -“新页面打开”走 `GET /files/preview-redirect?path=`：先鉴权，再签发一枚短时效 HMAC 令牌，然后 302 跳转到**另一个源**： +Files 面板内的 HTML 渲染视图（iframe）与“新页面打开”都走 `GET /files/preview-redirect?path=`：先鉴权，再签发一枚短时效 HMAC 令牌，然后 302 跳转到**另一个源**： ```text GET /api/sessions/:sessionId/files/preview-redirect?path=index.html @@ -181,6 +181,7 @@ GET /preview//<相对路径> （不鉴权，令牌即凭证 ``` - **为什么要独立源。** 页面需要一个真实的源，才能有可用的 storage、Cookie 与第三方 embed；但它不能是应用自己的源，否则 Agent 写出来的 HTML 就带着会话 Cookie 在跑。本地把 App 固定在规范主机 `localhost`，预览用 `127.0.0.1`——Cookie 按主机划分且不区分端口，所以这两者天然是两个 Cookie jar，而只换端口做不到。其余情况用 `PENGUIN_PREVIEW_ORIGIN`；两者都没有时（通配或非回环绑定，或变量未设）回退到上面的同源沙箱，并由 `GET /api/me` 的 `previewIsolated` 返回 `false`，界面据此提前说明。 +- **面板内渲染共用同一 URL。** Files 面板把该跳转 URL 嵌入 iframe，沙箱为 `allow-scripts allow-same-origin allow-forms allow-popups allow-modals allow-downloads`——`allow-same-origin` 赋予的是预览源而非应用源的身份，因此仍严格紧于不带沙箱的新标签页。没有独立预览源时，面板回退为内联 `srcdoc` 渲染（仅 `allow-scripts`，附内存版 storage 垫片），相对子资源在那里无法加载。另注意部分浏览器会对跨站 iframe 内的 storage 做分区或屏蔽，页面在面板内的行为可能与顶层标签页略有差异。 - **预览主机只服务 `/preview/*`。** 它与 App 是同一个进程，故其 `/api` 一律 401，其余路径一律 302 回规范 App 主机——会话 Cookie 因此永远不会落在预览主机上，也不被其接受，那里的 Agent HTML 无法同源调用 API。（部署 `PENGUIN_PREVIEW_ORIGIN` 时，反向代理须做等价保证：该源上只把 `/preview/*` 路由到 App。） - **路径式而非查询参数**，页面里的相对子资源（`app.js`、`style.css`、图片）才能相对文档解析，并在同一个令牌下加载。 - **令牌绑定 Session、预览主机与过期时间。** 其中主机绑定是承重的：同一个进程也在应用源上应答，因此 `/preview/...` 在应用源上一律拒绝服务——否则那就是一个同源 XSS。权限只读、限定该 Session 的 Workspace，路径仍在服务端重新解析，`..` 与符号链接逃逸照旧拒绝。 diff --git a/packages/server/src/services/preview-token.ts b/packages/server/src/services/preview-token.ts index 3e57c3d..f0cd186 100644 --- a/packages/server/src/services/preview-token.ts +++ b/packages/server/src/services/preview-token.ts @@ -151,7 +151,8 @@ export function requestAuthority(requestUrl: string, hostHeader: string | undefi /** * Where previews for this request should be served from: the configured origin when * PENGUIN_PREVIEW_ORIGIN is set, otherwise the loopback counterpart of the host the - * caller is using. Null when neither applies — the caller degrades to the same-origin + * caller is using. Null when neither applies — or when the configured origin's host is + * the request's own host (no boundary) — and the caller degrades to the same-origin * sandbox rather than silently serving unisolated content. * * The port comes from **this server's own binding**, never from the request. Those differ @@ -169,13 +170,23 @@ export function resolvePreviewTarget( configuredOrigin: string | null, serverBind: { host: string; port: number }, ): { origin: string; host: string } | null { + const requestHost = hostOnly(requestAuthority(requestUrl, hostHeader)).toLowerCase(); if (configuredOrigin) { const url = new URL(configuredOrigin); + // A configured origin that lands on the very host this request is using is no + // boundary at all: "isolated" previews would be same-origin with the App, and the + // allow-same-origin iframe the UI mounts for them would hand Agent HTML the App's + // origin. Such a misconfiguration degrades to null — previewIsolated reports false + // and the redirect falls back to the same-origin sandbox — instead of claiming an + // isolation that does not exist. Host-only comparison on purpose: cookies ignore + // ports, so a port difference would not separate anything either. + if (url.hostname.toLowerCase() === requestHost) return null; return { origin: url.origin, host: url.hostname }; } - const raw = requestAuthority(requestUrl, hostHeader); - if (raw === "") return null; - const counterpart = loopbackCounterpart(hostOnly(raw)); + if (requestHost === "") return null; + // The loopback counterpart differs from the request host by construction, so the + // same-host guard above is only needed for the configured branch. + const counterpart = loopbackCounterpart(requestHost); if (!counterpart) return null; if (!loopbackHostRoles(serverBind.host)) return null; diff --git a/packages/server/test/workspace-preview.test.ts b/packages/server/test/workspace-preview.test.ts index 5b71b5b..b42abde 100644 --- a/packages/server/test/workspace-preview.test.ts +++ b/packages/server/test/workspace-preview.test.ts @@ -128,6 +128,39 @@ describe("preview origin derivation", () => { resolvePreviewTarget("http://192.168.1.5:7364/x", "192.168.1.5:7364", null, bind), ).toBeNull(); }); + + it("refuses a configured origin that matches the App request's host", () => { + // PENGUIN_PREVIEW_ORIGIN pointing back at the host the App is being used on is no + // boundary: "isolated" previews would be same-origin with the App (and the UI would + // mount an allow-same-origin iframe onto them). Must degrade to null so + // previewIsolated reports false and the safe same-origin sandbox engages. + expect( + resolvePreviewTarget( + "https://app.example.com/x", + "app.example.com", + "https://app.example.com", + bind, + ), + ).toBeNull(); + // Port and case differences don't rescue it: cookies ignore ports, hosts ignore case. + expect( + resolvePreviewTarget( + "https://app.example.com/x", + "app.example.com:8443", + "https://APP.example.com:9443", + bind, + ), + ).toBeNull(); + // A genuinely different host on the same registrable domain is still accepted. + expect( + resolvePreviewTarget( + "https://app.example.com/x", + "app.example.com", + "https://preview.app.example.com", + bind, + ), + ).toEqual({ origin: "https://preview.app.example.com", host: "preview.app.example.com" }); + }); }); describe("preview route", () => { diff --git a/packages/web/e2e/chat.spec.mjs b/packages/web/e2e/chat.spec.mjs index 54e0839..4e61ccc 100644 --- a/packages/web/e2e/chat.spec.mjs +++ b/packages/web/e2e/chat.spec.mjs @@ -215,9 +215,12 @@ test("chat + tool approval + stats/cost/copy + traces + files", async ({ page }) await seg.hover(); await expect(main.locator("button.bg-amber-50").first()).toBeVisible(); - // --- files: HTML preview (scripts run in sandbox, localStorage shim) + path hidden --- - // Script uses localStorage: without the shim it would throw SecurityError (opaque origin); - // with the shim it runs and appends #shim-ok. + // --- files: HTML preview (isolated separate-origin iframe, scripts + storage) + path hidden --- + // run.sh binds 127.0.0.1 and the spec browses on localhost, so previewIsolated=true and the + // in-app rendered view loads via /files/preview-redirect on the separate preview origin + // (127.0.0.1) — a real origin where localStorage works natively. The #shim-ok marker (id kept + // from the old srcDoc-plus-shim path, which now only engages when no preview origin exists) + // proves the page's script ran and storage was writable. const html = Buffer.from( "

Hello E2E