From 693adee5e5b9e62dfe156de4ae5fd9a6e5262427 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 3 Sep 2026 17:12:20 -0700 Subject: [PATCH 1/3] Allow nested iframe proxy documents --- .github/audit/application-security.md | 5 +++++ docs/specs/dor-browser.md | 11 ++++++----- docs/specs/dor-browser.rationale.md | 2 ++ docs/specs/security-local.md | 12 +++++------- docs/specs/security-local.rationale.md | 7 +++++++ lib/src/host/iframe-proxy-rewrite.test.ts | 6 ++++-- lib/src/host/iframe-proxy-rewrite.ts | 16 +++++++++++----- lib/src/host/iframe-proxy.test.ts | 12 ++++++------ lib/src/host/iframe-proxy.ts | 22 ++++++++++++---------- 9 files changed, 58 insertions(+), 35 deletions(-) diff --git a/.github/audit/application-security.md b/.github/audit/application-security.md index d7169564..fa2bbad5 100644 --- a/.github/audit/application-security.md +++ b/.github/audit/application-security.md @@ -105,6 +105,11 @@ Be adversarial, and go past the `FAIL IF` list. Ask specifically: - Where does untrusted input enter — relay frames, push endpoints, terminal bytes, notification text, state files read back from disk — and what happens on a malformed, oversized, or hostile value? +- Does the iframe proxy replace framing controls with exactly `'self'` plus the + fully validated app ancestor chain, and only when that chain is usable? Treat + `'self'` as the explicit same-grant nesting relaxation: confirm each grant is + one origin bound to one upstream, no wildcard or foreign source is admitted, + and the no-chain path preserves the upstream controls and injects no shim. - Does the shipped code still match what the specs and this section claim? Spec drift is a finding; say which side is wrong. The newest sections are the ones most likely to have drifted: `remote-security-model.md`'s Presence proofs, diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index d27ad771..6f88de44 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -456,7 +456,7 @@ What is rewritten, exactly: | request | `Origin` | upstream origin **only** when it is the proxy's own; else forwarded untouched (absent stays absent) | | request | `Referer` | proxy origin substituted for the upstream origin | | request | `Accept-Encoding` | deleted, so HTML comes back identity for rewriting | -| response | `X-Frame-Options`, `Content-Security-Policy`, `Content-Security-Policy-Report-Only` | dropped **whole**, never per-directive (rationale) | +| response | `X-Frame-Options`, `Content-Security-Policy`, `Content-Security-Policy-Report-Only` | replaced **whole** by `frame-ancestors 'self' ` (rationale) | | response | hop-by-hop (RFC 7230 §6.1) | dropped | | response | `Location` | upstream origin rewritten back to the proxy origin, so a redirect stays inside the proxy | | response body | `` | removed, like the header | @@ -560,10 +560,11 @@ Security boundaries: - every other user-supplied `http://` target is trusted as the user's command, at the cost of the upstream's own XSS policy inside the frame. -**Must replace the upstream's framing controls with a `frame-ancestors` naming -the embedder chain, never merely drop them**, and **the shim's `postMessage` must -target that chain's origin, never `'*'`** (rationale; `docs/specs/security-local.md` → "Loopback -Listeners"). **With no chain the proxy strips nothing and injects nothing.** +**Must replace framing controls with exactly `frame-ancestors 'self' +`.** `'self'` permits same-grant nesting; foreign +ancestors fail. **The shim's `postMessage` must target that +chain's origin, never `'*'`** (rationale; `docs/specs/security-local.md` → +"Loopback Listeners"). **With no chain it preserves headers and injects nothing.** **Must refresh a grant's idle timer for every caller except one that named itself foreign.** `isOwnOrigin` and `isForeignOrigin` are not each other's negation — an diff --git a/docs/specs/dor-browser.rationale.md b/docs/specs/dor-browser.rationale.md index dd440ca9..72402d27 100644 --- a/docs/specs/dor-browser.rationale.md +++ b/docs/specs/dor-browser.rationale.md @@ -102,4 +102,6 @@ A post-open blank-tab sweep can become such a query when a later relaunch, expli **Why a partial chain is no chain.** A `frame-ancestors` naming a subset of the real ancestors blocks Dormouse's own frame, the one embed that must always work. Failing closed to "no chain" instead leaves the caller exactly what the upstream would have served it directly. +**Why the policy also admits `'self'`.** Storybook and similar apps render same-origin documents in nested frames, so an app-only ancestor list blocks their inner document. Each proxy origin belongs to one grant and one fixed upstream; documents already executing there share same-origin authority. Admitting `'self'` deliberately lets a proxy document frame another document from that grant, including after a top-level navigation, but no foreign ancestor matches and no grant can frame another grant. + **Why the idle timer refreshes for an absent `Origin`.** "Own origin only" would expire a grant the user is still looking at, because a live frame's navigations and sub-resource loads carry no `Origin` at all. What a foreign `Origin` must not buy is keeping a closed pane's grant — and its live upstream binding — alive indefinitely by polling. diff --git a/docs/specs/security-local.md b/docs/specs/security-local.md index d9098119..2b7c7ace 100644 --- a/docs/specs/security-local.md +++ b/docs/specs/security-local.md @@ -126,17 +126,15 @@ listener because their URLs differ, and the differences are forced, not stylisti URL token is available to the browser-dev harness, which owns its page's URL, and never to the iframe proxy (rationale). -**A third question has no request-header answer at all: who is allowed to frame me?** -A response that confers something on its *embedder* must name that embedder and let the -browser enforce it, which no request header can do — so the webview supplies its own -ancestor chain, not merely its origin (the check walks the whole chain), with each -request for a proxy URL, and that chain is what the proxy's `frame-ancestors` names -(rationale). +**The replacement policy must allow exactly `'self'` plus the full validated +ancestor chain the webview supplies with each proxy URL request.** `'self'` allows +same-grant nesting; any foreign ancestor fails. No request header identifies the +embedder, and the browser checks the whole chain (rationale). - **FAIL IF** any loopback HTTP or WebSocket listener grants an unrecognized caller a privilege it could not obtain by reaching the upstream directly. Refusing the request is one way; the iframe proxy's *admits all, vouches for none, names its embedder* is another, and is not a violation (rationale). `scripts/loopback-lint.mjs` (`pnpm test`) makes the cheap half deterministic — a new loopback bind that does not reference a guard module fails the build — but only in the bind forms it knows: `.listen` positional and options-object, `@hono/node-server`'s `serve({ hostname })`, and `ws`'s `new WebSocketServer({ port | host })`, each proved load-bearing by `scripts/loopback-lint-selftest.mjs`. **Adding a server dependency means adding its bind spelling there**; a host built at runtime is invisible to a regex in any spelling. It also only sees that a file *knows* a guard exists, never that the guard is called on every request, so this bullet still has to be read. Derive the set by searching the shipped trees for `createServer`, `.listen(`, `serve(` and `WebSocketServer` rather than trusting this list. Today the set is three: the iframe proxy (`lib/src/host/iframe-proxy.ts`), the VS Code agent-browser stream relay (`vscode-ext/src/agent-browser-host.ts`), and the browser-dev bridge (`standalone/scripts/dev-agent-browser.mjs`). A Unix-domain socket or named pipe is not in scope — no browser can reach one — which is why the `dor` control channel is bounded by socket permissions instead. - **FAIL IF** the iframe proxy rewrites `Origin` to the upstream's own origin for a caller whose inbound `Origin` is not the proxy's own — in `handleRequest` **or** `handleUpgrade`. A foreign `Origin` must be forwarded untouched rather than blocked, so the upstream sees the truth and applies its own policy (rationale). - **FAIL IF** the iframe proxy stops checking that `Host` names its own grant port, on either path. Its per-grant ephemeral port and one-fixed-upstream binding are real mitigations but neither is a secret, so the `Host` check is what makes DNS rebinding fail. -- **FAIL IF** the iframe proxy **drops** an upstream's `X-Frame-Options` / CSP `frame-ancestors` without **replacing** them with a `frame-ancestors` naming the embedder chain the webview supplied, or injects its shim with any `postMessage` target but that chain's own origin. With no usable chain the proxy must strip nothing and inject nothing, which leaves the caller exactly what the upstream would have given it directly (rationale). +- **FAIL IF** the iframe proxy drops upstream `X-Frame-Options` / CSP `frame-ancestors` without replacing them with exactly `frame-ancestors 'self' `, admits another source, or targets the shim anywhere but that chain's innermost origin. With no usable chain it must preserve the headers and inject nothing (rationale). - **FAIL IF** a request bearing a *foreign* `Origin` refreshes a grant's idle timer: a grant holds a live upstream binding, and a stranger polling it keeps a closed pane's binding open. An *absent* `Origin` must keep refreshing it — that is what a live frame's own navigations and sub-resources send. - **FAIL IF** the stream relay's grant stops being single-use, TTL-bounded, and pinned to one target port, or if it begins rewriting `Origin` rather than dropping it. It needs no `Host` check while the token holds (rationale). - **FAIL IF** the browser-dev bridge drops any of its four gates — the per-run token, the loopback `Host` check, the `application/json` content-type required of every non-GET, and the exact-origin `access-control-allow-origin`. The first three live together in the gate that runs before routing, so a route that never reads a body is covered by all of them. It is dev-only and ships in nothing, but it dispatches `pty_spawn` with caller-supplied `shell`, `args`, `cwd` and `env` — arbitrary command execution on a maintainer or CI-agent machine (`docs/specs/security-ci.md` -> "Automated Maintainer (tend)"). The content-type rule is a security control, not tidiness (rationale). diff --git a/docs/specs/security-local.rationale.md b/docs/specs/security-local.rationale.md index 2fd55b56..90ef14c3 100644 --- a/docs/specs/security-local.rationale.md +++ b/docs/specs/security-local.rationale.md @@ -116,6 +116,13 @@ and reading that document's live URL and anchor hrefs back cross-origin. No requ header can tell that page apart from Dormouse's webview, which is why the replacement `frame-ancestors` has to name the embedder chain the webview supplied. +**Why same-grant framing is an accepted relaxation.** Storybook and similar apps put +same-origin documents in nested frames, which an app-only policy blocks. The extra +`'self'` source also permits a proxy page loaded top-level to frame another document +from that grant, but one grant is one origin and one fixed upstream: those documents +already share same-origin authority. A foreign page still appears in the ancestor +chain and fails the policy, and a different grant has a different origin. + **Why the listener set is derived, not trusted.** An enumeration goes stale the moment someone adds a listener — the same failure mode that once left `.vscode/` owned by nobody. diff --git a/lib/src/host/iframe-proxy-rewrite.test.ts b/lib/src/host/iframe-proxy-rewrite.test.ts index d1874633..2a2a68be 100644 --- a/lib/src/host/iframe-proxy-rewrite.test.ts +++ b/lib/src/host/iframe-proxy-rewrite.test.ts @@ -103,9 +103,11 @@ describe('normalizeEmbedderOrigins', () => { } }); - it('renders every ancestor into the policy', () => { + it("allows only 'self' and every validated app ancestor", () => { + expect(frameAncestorsCsp(['tauri://localhost'])) + .toBe("frame-ancestors 'self' tauri://localhost"); expect(frameAncestorsCsp(['vscode-webview://abc-123', 'vscode-file://vscode-app'])) - .toBe('frame-ancestors vscode-webview://abc-123 vscode-file://vscode-app'); + .toBe("frame-ancestors 'self' vscode-webview://abc-123 vscode-file://vscode-app"); }); }); diff --git a/lib/src/host/iframe-proxy-rewrite.ts b/lib/src/host/iframe-proxy-rewrite.ts index 55d0a96c..b5bec74b 100644 --- a/lib/src/host/iframe-proxy-rewrite.ts +++ b/lib/src/host/iframe-proxy-rewrite.ts @@ -28,9 +28,11 @@ export const HOP_BY_HOP_RESPONSE_HEADERS = new Set([ * alike) can tell the two embedders apart at request time. So the recognizer * cannot be a request header — it is the *embedder* that has to be named, and * `frame-ancestors` is the mechanism browsers already enforce for exactly that - * question. With it, a stranger's frame never loads; without a known embedder - * origin the upstream's own "do not embed" is forwarded untouched, which grants - * the caller nothing it could not get by reaching the upstream directly. + * question. The replacement also admits `'self'`: documents already inside one + * fixed-upstream grant may frame each other, which is required by apps such as + * Storybook whose preview is a nested iframe. A stranger's foreign ancestor + * still never matches; without a known app embedder origin the upstream's own + * "do not embed" is forwarded untouched. */ export const FRAMING_RESPONSE_HEADERS = new Set([ 'x-frame-options', 'content-security-policy', 'content-security-policy-report-only', @@ -68,9 +70,13 @@ export function normalizeEmbedderOrigins(value: unknown): string[] | null { return out; } -/** The `Content-Security-Policy` the proxy serves in place of the upstream's. */ +/** + * The `Content-Security-Policy` the proxy serves in place of the upstream's. + * `'self'` permits nested documents within this one per-grant origin; every + * external ancestor must still be in the validated app chain. + */ export function frameAncestorsCsp(embedderOrigins: string[]): string { - return `frame-ancestors ${embedderOrigins.join(' ')}`; + return `frame-ancestors 'self' ${embedderOrigins.join(' ')}`; } // The fixed, Dormouse-owned shim — like agent-browser's EDIT_SCRIPTS, never diff --git a/lib/src/host/iframe-proxy.test.ts b/lib/src/host/iframe-proxy.test.ts index de7b8679..7a5d20de 100644 --- a/lib/src/host/iframe-proxy.test.ts +++ b/lib/src/host/iframe-proxy.test.ts @@ -168,10 +168,10 @@ describe('iframe proxy — serving', () => { expect(res.status).toBe(200); expect(res.headers['x-frame-options']).toBeUndefined(); - // Replaced, not merely dropped: only this webview's own ancestor chain may - // frame what the upstream said nobody could. + // Replaced, not merely dropped: same-grant documents may nest, while every + // external ancestor still has to belong to this webview's validated chain. expect(res.headers['content-security-policy']) - .toBe('frame-ancestors vscode-webview://abc-123 vscode-file://vscode-app'); + .toBe("frame-ancestors 'self' vscode-webview://abc-123 vscode-file://vscode-app"); expect(res.body).not.toMatch(/http-equiv=["']?content-security-policy/i); expect(res.body).toContain('__dormouse'); expect(res.body).toMatch(/<\/script><\/head>/); @@ -211,7 +211,7 @@ describe('iframe proxy — serving', () => { expect(res.body).not.toContain('__dormouse'); expect(res.headers['x-frame-options']).toBeUndefined(); expect(res.headers['content-security-policy']) - .toBe('frame-ancestors vscode-webview://abc-123 vscode-file://vscode-app'); + .toBe("frame-ancestors 'self' vscode-webview://abc-123 vscode-file://vscode-app"); }); it('rewrites an upstream-origin Location redirect onto the proxy origin', async () => { @@ -311,12 +311,12 @@ describe('iframe proxy — the two privileges are the embedder’s, not the port s.end('secret'); }; - it('names only the app’s ancestor chain as allowed to frame it', async () => { + it("allows only the grant itself and the app's ancestor chain to frame it", async () => { const port = await upstream((_q, s) => DENY_HTML(s)); const res = await get(await frame(`http://127.0.0.1:${port}/`)); expect(res.headers['content-security-policy']) - .toBe('frame-ancestors vscode-webview://abc-123 vscode-file://vscode-app'); + .toBe("frame-ancestors 'self' vscode-webview://abc-123 vscode-file://vscode-app"); expect(res.headers['x-frame-options']).toBeUndefined(); expect(res.body).toContain('__dormouse'); }); diff --git a/lib/src/host/iframe-proxy.ts b/lib/src/host/iframe-proxy.ts index f78bdc06..a1bf9ff2 100644 --- a/lib/src/host/iframe-proxy.ts +++ b/lib/src/host/iframe-proxy.ts @@ -46,9 +46,10 @@ * back out. Neither is per-request (an iframe navigation carries no * `Origin`, and `Sec-Fetch-Site` says `cross-site` for our own webview and * for an attacker page alike), so both are conditioned on a - * `frame-ancestors` naming the app's own chain, which the browser enforces. - * With no chain the proxy takes nothing away and injects nothing, which - * leaves a caller exactly what the upstream would have given it directly. + * `frame-ancestors` allowing only the grant's own origin (for nested frames) + * and the app's validated chain, which the browser enforces. With no chain + * the proxy takes nothing away and injects nothing, which leaves a caller + * exactly what the upstream would have given it directly. */ import * as http from 'http'; import * as net from 'net'; @@ -248,11 +249,11 @@ function handleRequest(grant: Grant, req: http.IncomingMessage, res: http.Server } // Any http upstream is framed, loopback or remote: sanitizeResponseHeaders // replaces its frame-blocking headers (X-Frame-Options / CSP - // frame-ancestors) with one naming this webview's own ancestor chain, and - // streamHtml injects the shim. A site's "do not embed" is overridden - // because the embed is the user's own `dor iframe`, not a third party - // framing them — but only for that one embedder, so a stranger who scans - // the port and frames it is refused by their own browser. + // frame-ancestors) with one allowing same-grant nesting plus this webview's + // own ancestor chain, and streamHtml injects the shim. A site's "do not + // embed" is overridden because the embed is the user's own `dor iframe`, + // not a third party framing them — a stranger's foreign ancestor still + // fails the policy. streamHtml(grant, embedder, upstreamRes, res); }); upstreamReq.on('error', (err) => { @@ -340,8 +341,9 @@ function sanitizeResponseHeaders(grant: Grant, headers: http.IncomingHttpHeaders const lower = name.toLowerCase(); if (HOP_BY_HOP_RESPONSE_HEADERS.has(lower)) continue; // Replaced, never merely dropped: this proxy may only take the upstream's - // "do not embed" away if it puts back one that names the single embedder - // entitled to frame it (`FRAMING_RESPONSE_HEADERS`). + // "do not embed" away if it puts back one that names the exact allowed set: + // this per-grant origin plus the app's validated ancestor chain + // (`FRAMING_RESPONSE_HEADERS`). if (replaceFraming && FRAMING_RESPONSE_HEADERS.has(lower)) continue; out[name] = value; } From 87981f5a2736403b876c95741d0dd360a0a0ac2f Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 3 Sep 2026 17:24:36 -0700 Subject: [PATCH 2/3] Relay nested iframe shim messages --- .github/audit/application-security.md | 3 ++ docs/specs/dor-browser.md | 8 ++-- docs/specs/dor-browser.rationale.md | 2 + docs/specs/security-local.md | 15 +++--- docs/specs/security-local.rationale.md | 4 +- lib/src/host/iframe-proxy-rewrite.test.ts | 57 ++++++++++++++++++++++- lib/src/host/iframe-proxy-rewrite.ts | 21 ++++++--- 7 files changed, 90 insertions(+), 20 deletions(-) diff --git a/.github/audit/application-security.md b/.github/audit/application-security.md index fa2bbad5..f980411e 100644 --- a/.github/audit/application-security.md +++ b/.github/audit/application-security.md @@ -110,6 +110,9 @@ Be adversarial, and go past the `FAIL IF` list. Ask specifically: `'self'` as the explicit same-grant nesting relaxation: confirm each grant is one origin bound to one upstream, no wildcard or foreign source is admitted, and the no-chain path preserves the upstream controls and injects no shim. + Trace nested shim messages too: each hop must accept only its proxy origin and + the four registered shapes, reconstruct the message, and target only that + origin plus the validated app origin—never a wildcard or foreign origin. - Does the shipped code still match what the specs and this section claim? Spec drift is a finding; say which side is wrong. The newest sections are the ones most likely to have drifted: `remote-security-model.md`'s Presence proofs, diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index 6f88de44..9bad4d3c 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -481,8 +481,8 @@ Source of truth: `lib/src/components/wall/IframePanel.tsx`, ### Iframe Shim -**The injected shim is fixed Dormouse-owned code, never user-provided eval.** It -posts only these messages to the parent: +**Must relay only these fixed, never-user-provided shim messages through +same-origin proxy parents to the app:** - `leader`: dual-tap Meta/Shift leader chord. - `pointerdown`: genuine click inside the frame, used to select/focus the pane. @@ -562,8 +562,8 @@ Security boundaries: **Must replace framing controls with exactly `frame-ancestors 'self' `.** `'self'` permits same-grant nesting; foreign -ancestors fail. **The shim's `postMessage` must target that -chain's origin, never `'*'`** (rationale; `docs/specs/security-local.md` → +ancestors fail. **Each shim hop must target only `'self'` and that chain's +origin, never `'*'`** (rationale; `docs/specs/security-local.md` → "Loopback Listeners"). **With no chain it preserves headers and injects nothing.** **Must refresh a grant's idle timer for every caller except one that named itself diff --git a/docs/specs/dor-browser.rationale.md b/docs/specs/dor-browser.rationale.md index 72402d27..04b56f6e 100644 --- a/docs/specs/dor-browser.rationale.md +++ b/docs/specs/dor-browser.rationale.md @@ -84,6 +84,8 @@ A post-open blank-tab sweep can become such a query when a later relaunch, expli **Why the CLI's own check is not enough.** `open-window` carries a string the framed page chose, and the new-tab prompt in front of it is user consent, not a boundary — the user is agreeing to open a pane, not vetting a scheme. The same check gates `surface.iframe`, a wire protocol on the control socket rather than the CLI, so nothing upstream of it has already filtered. +**Why each shim hop has two explicit targets.** An injected document cannot tell whether its parent is the app or another document on the grant's proxy origin. It posts to both known origins; the browser delivers only the matching one. A same-origin parent relays only the four registered shapes upward, reconstructing each message, until the outer document reaches the app. No wildcard or foreign origin enters the path. + ## Iframe Focus And Rendering Notes **Why the raw fallback is sandboxed too.** Reading "raw" as the trusted path and the proxy as the one needing containment is backwards: the raw fallback is the case with *no* proxy in front of the page at all. diff --git a/docs/specs/security-local.md b/docs/specs/security-local.md index 2b7c7ace..99482315 100644 --- a/docs/specs/security-local.md +++ b/docs/specs/security-local.md @@ -64,13 +64,14 @@ fails closed when no token was injected (`docs/specs/vscode.md` -> "Webview message authentication"). **The standalone adapters have no forgeable inbox**: host events arrive over Tauri IPC, never `window.postMessage`. -**The shim the proxy injects addresses the embedder chain's innermost origin, -never `'*'`**, and with no usable chain the proxy injects nothing and strips no -framing header (`docs/specs/dor-browser.md` -> "Iframe Host Capability And CSP"). -What it grants a *caller* is [Loopback Listeners](#loopback-listeners)'s -business. - -- **FAIL IF** the injected shim targets anything but the embedder chain's innermost origin, or the proxy uses a chain it did not validate in full: `iframeShim` and `normalizeEmbedderOrigins` in `lib/src/host/iframe-proxy-rewrite.ts`, applied in `lib/src/host/iframe-proxy.ts`. Pinned by `lib/src/host/iframe-proxy-rewrite.test.ts` and `lib/src/host/iframe-proxy.test.ts`. +**Each injected shim hop must address only its proxy origin and the embedder +chain's innermost origin, never `'*'`.** Nested frames relay the four registered +messages through same-origin parents. With no usable chain the proxy injects +nothing and strips no framing header (`docs/specs/dor-browser.md` -> "Iframe +Host Capability And CSP"). What it grants a *caller* is [Loopback +Listeners](#loopback-listeners)'s business. + +- **FAIL IF** an injected shim targets anything but its proxy origin or the embedder chain's innermost origin, relays a foreign-origin or unregistered message, or the proxy uses a chain it did not validate in full: `iframeShim` and `normalizeEmbedderOrigins` in `lib/src/host/iframe-proxy-rewrite.ts`, applied in `lib/src/host/iframe-proxy.ts`. Pinned by `lib/src/host/iframe-proxy-rewrite.test.ts` and `lib/src/host/iframe-proxy.test.ts`. - **FAIL IF** a `VSCodeAdapter` host-channel listener acts on a message before `isHostMessage` (`lib/src/lib/vscode-message-token.ts`) accepts it, or the token stops being minted per serve and attached only by `WebviewChannel.post` in `vscode-ext/src/webview-messaging.ts`: `dor:controlRequest` is one of the shapes a framed page could otherwise claim. The proxy-origin listeners above are guarded by origin, not the token. Pinned by the `host message authentication` block in `lib/src/lib/platform/vscode-adapter.test.ts`. Source of truth: `isProxyOrigin` in `lib/src/lib/iframe-proxy-registry.ts`, the diff --git a/docs/specs/security-local.rationale.md b/docs/specs/security-local.rationale.md index 90ef14c3..8ecf40ba 100644 --- a/docs/specs/security-local.rationale.md +++ b/docs/specs/security-local.rationale.md @@ -46,7 +46,9 @@ downstream instead — exiting passthrough, selecting a pane, an `http:`/`https: only `browserSurfaceUrl` behind an open prompt, and a frame-URL reading that may lie. `use-wall-keyboard`'s leader channel accepts any live grant rather than one panel's, so a page in one browser pane can exit passthrough while another is -focused. +focused. The nested-frame relay preserves this boundary: it accepts only the +same proxy origin and reconstructs one of those four shapes, so unrelated +application messages and every foreign origin stop at the child frame. Why "the standalone adapters" and not "the standalone webview". The Wall's two proxy-origin `message` listeners (`use-wall-keyboard.ts`, `IframePanel.tsx`) are diff --git a/lib/src/host/iframe-proxy-rewrite.test.ts b/lib/src/host/iframe-proxy-rewrite.test.ts index 2a2a68be..776a64ec 100644 --- a/lib/src/host/iframe-proxy-rewrite.test.ts +++ b/lib/src/host/iframe-proxy-rewrite.test.ts @@ -1,3 +1,4 @@ +import { runInNewContext } from 'node:vm'; import { describe, it, expect } from 'vitest'; import { frameAncestorsCsp, @@ -11,8 +12,40 @@ import { } from './iframe-proxy-rewrite'; const APP = 'vscode-webview://abc-123'; +const PROXY = 'http://127.0.0.1:4321'; const IFRAME_SHIM = iframeShim(APP); +type ShimListener = (event: Record) => void; + +function shimFrame(parentOrigin: string, deliver: (data: unknown) => void) { + const listeners = new Map(); + const addEventListener = (type: string, listener: ShimListener) => { + const current = listeners.get(type) ?? []; + current.push(listener); + listeners.set(type, current); + }; + const parent = { + postMessage(data: unknown, target: string) { + if (target === parentOrigin) deliver(JSON.parse(JSON.stringify(data))); + }, + }; + const window = { parent, open: undefined as unknown }; + runInNewContext(IFRAME_SHIM, { + window, + location: { origin: PROXY, href: `${PROXY}/story` }, + document: { readyState: 'loading' }, + history: {}, + addEventListener, + setTimeout: () => 0, + URL, + }); + return { + emit(type: string, event: Record) { + for (const listener of listeners.get(type) ?? []) listener(event); + }, + }; +} + describe('instrumentHtml', () => { it('injects the shim before ', () => { const out = instrumentHtml('xhi', APP); @@ -64,12 +97,32 @@ describe('instrumentHtml', () => { // The shim reads the framed page's live URL and its anchor hrefs and hands them // out — reads the same-origin policy would otherwise forbid. `'*'` handed them // to whoever had framed the proxy, which the port scan makes anybody. -describe('the shim addresses the app, not the world', () => { - it('posts to the embedder origin and never to a wildcard', () => { +describe('the shim addresses the grant and app, not the world', () => { + it('posts only to the proxy and embedder origins', () => { expect(IFRAME_SHIM).toContain(`var TARGET="${APP}"`); + expect(IFRAME_SHIM).toContain('P.postMessage(m,location.origin)'); expect(IFRAME_SHIM).toContain('P.postMessage(m,TARGET)'); expect(IFRAME_SHIM).not.toContain("postMessage(m,'*')"); }); + + it('relays registered messages through same-origin nested frames', () => { + const delivered: unknown[] = []; + const outer = shimFrame(APP, (data) => delivered.push(data)); + const inner = shimFrame(PROXY, (data) => outer.emit('message', { origin: PROXY, data })); + + inner.emit('pointerdown', {}); + expect(delivered).toEqual([{ __dormouse: 'pointerdown' }]); + + outer.emit('message', { origin: PROXY, data: { __dormouse: 'unknown', secret: 'x' } }); + outer.emit('message', { origin: 'https://evil.example', data: { __dormouse: 'leader' } }); + expect(delivered).toHaveLength(1); + + outer.emit('message', { + origin: PROXY, + data: { __dormouse: 'location', url: `${PROXY}/next`, secret: 'x' }, + }); + expect(delivered[1]).toEqual({ __dormouse: 'location', url: `${PROXY}/next` }); + }); }); describe('normalizeEmbedderOrigins', () => { diff --git a/lib/src/host/iframe-proxy-rewrite.ts b/lib/src/host/iframe-proxy-rewrite.ts index b5bec74b..b4792e6a 100644 --- a/lib/src/host/iframe-proxy-rewrite.ts +++ b/lib/src/host/iframe-proxy-rewrite.ts @@ -84,7 +84,9 @@ export function frameAncestorsCsp(embedderOrigins: string[]): string { // this is why the upstream CSP is dropped whole rather than per-directive (an // inline script needs `script-src` gone as much as the frame needs // `frame-ancestors` gone). It posts four message kinds to the Wall and nothing -// else (every other keystroke flows to the tool): +// else (every other keystroke flows to the tool). A nested document relays them +// through its same-origin proxy parents until the outer document reaches the +// app: // - `leader`: the reserved dual-tap ⌘/⇧ chord (matching handle-dual-tap.ts), // so the global chord keeps working with the frame focused. // - `pointerdown`: a click landed in the frame. A cross-origin click reaches @@ -101,11 +103,18 @@ export function iframeShim(embedderOrigin: string): string { var P=window.parent; var TARGET=${JSON.stringify(embedderOrigin)}; if(!P||P===window)return; - // Addressed to the app's own origin, never '*': every message here is a - // cross-origin *read* of the framed page — its live URL, its anchor hrefs — - // that the same-origin policy would otherwise forbid, and postMessage only - // delivers when the target window's origin matches. - function post(t,d){try{var m={__dormouse:t};if(d)for(var k in d)m[k]=d[k];P.postMessage(m,TARGET);}catch(e){}} + // Address each hop to the proxy's own origin and the app origin, never '*'. + // Exactly one matches: a nested frame reaches its same-origin parent, while + // the outer frame reaches the app. Relays accept only the four fixed shapes + // below, so unrelated same-origin application messages never escape. + function send(m){try{P.postMessage(m,location.origin);}catch(e){}try{P.postMessage(m,TARGET);}catch(e){}} + function post(t,d){var m={__dormouse:t};if(d)for(var k in d)m[k]=d[k];send(m);} + addEventListener('message',function(e){ + if(e.origin!==location.origin)return; + var d=e.data,t=d&&d.__dormouse; + if(t==='leader'||t==='pointerdown')post(t); + else if((t==='location'||t==='open-window')&&typeof d.url==='string')post(t,{url:d.url}); + },true); function postLocation(){post('location',{url:String(location.href)});} function anchorHref(e){ var n=e&&e.target; From adb8ae7fb95e933d1cf772a4669a00a322e9fae7 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 3 Sep 2026 17:37:15 -0700 Subject: [PATCH 3/3] Keep nested frame locations local --- .github/audit/application-security.md | 7 ++++--- docs/specs/dor-browser.md | 17 ++++++++--------- docs/specs/dor-browser.rationale.md | 2 +- docs/specs/security-local.md | 9 +++++---- docs/specs/security-local.rationale.md | 5 +++-- lib/src/host/iframe-proxy-rewrite.test.ts | 10 ++++++++-- lib/src/host/iframe-proxy-rewrite.ts | 14 ++++++++------ 7 files changed, 37 insertions(+), 27 deletions(-) diff --git a/.github/audit/application-security.md b/.github/audit/application-security.md index f980411e..45432233 100644 --- a/.github/audit/application-security.md +++ b/.github/audit/application-security.md @@ -110,9 +110,10 @@ Be adversarial, and go past the `FAIL IF` list. Ask specifically: `'self'` as the explicit same-grant nesting relaxation: confirm each grant is one origin bound to one upstream, no wildcard or foreign source is admitted, and the no-chain path preserves the upstream controls and injects no shim. - Trace nested shim messages too: each hop must accept only its proxy origin and - the four registered shapes, reconstruct the message, and target only that - origin plus the validated app origin—never a wildcard or foreign origin. + Trace nested shim messages too: each hop must accept only its proxy origin, + reconstruct and relay only the registered pane-level shapes, never relay a + nested document's location, and target only that origin plus the validated + app origin—never a wildcard or foreign origin. - Does the shipped code still match what the specs and this section claim? Spec drift is a finding; say which side is wrong. The newest sections are the ones most likely to have drifted: `remote-security-model.md`'s Presence proofs, diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index 9bad4d3c..eaa3b00b 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -481,14 +481,13 @@ Source of truth: `lib/src/components/wall/IframePanel.tsx`, ### Iframe Shim -**Must relay only these fixed, never-user-provided shim messages through -same-origin proxy parents to the app:** +**Must send these fixed, never-user-provided shim messages to the app:** -- `leader`: dual-tap Meta/Shift leader chord. -- `pointerdown`: genuine click inside the frame, used to select/focus the pane. -- `location`: same-frame navigation after history/hash/page events, and after a - same-frame anchor click the page did not cancel. -- `open-window`: intercepted `target=_blank` anchor or `window.open` URL. +- `leader`: dual-tap Meta/Shift leader chord; relayed from nested documents. +- `pointerdown`: genuine click, used to select/focus the pane; relayed. +- `location`: outer-document history/hash/page events and uncancelled same-frame + anchor clicks; never relayed from nested documents. +- `open-window`: intercepted `target=_blank` anchor or `window.open` URL; relayed. **A URL from the frame is re-checked before it becomes a pane.** `open-window` and the control socket's `surface.iframe` both go through `browserSurfaceUrl`; @@ -562,8 +561,8 @@ Security boundaries: **Must replace framing controls with exactly `frame-ancestors 'self' `.** `'self'` permits same-grant nesting; foreign -ancestors fail. **Each shim hop must target only `'self'` and that chain's -origin, never `'*'`** (rationale; `docs/specs/security-local.md` → +ancestors fail. **Each shim hop must target only its origin and that chain's +innermost origin, never `'*'`** (rationale; `docs/specs/security-local.md` → "Loopback Listeners"). **With no chain it preserves headers and injects nothing.** **Must refresh a grant's idle timer for every caller except one that named itself diff --git a/docs/specs/dor-browser.rationale.md b/docs/specs/dor-browser.rationale.md index 04b56f6e..bf59250e 100644 --- a/docs/specs/dor-browser.rationale.md +++ b/docs/specs/dor-browser.rationale.md @@ -84,7 +84,7 @@ A post-open blank-tab sweep can become such a query when a later relaunch, expli **Why the CLI's own check is not enough.** `open-window` carries a string the framed page chose, and the new-tab prompt in front of it is user consent, not a boundary — the user is agreeing to open a pane, not vetting a scheme. The same check gates `surface.iframe`, a wire protocol on the control socket rather than the CLI, so nothing upstream of it has already filtered. -**Why each shim hop has two explicit targets.** An injected document cannot tell whether its parent is the app or another document on the grant's proxy origin. It posts to both known origins; the browser delivers only the matching one. A same-origin parent relays only the four registered shapes upward, reconstructing each message, until the outer document reaches the app. No wildcard or foreign origin enters the path. +**Why each shim hop has two explicit targets.** An injected document cannot tell whether its parent is the app or another document on the grant's proxy origin. It posts to both known origins; the browser delivers only the matching one. A same-origin parent reconstructs and relays only the three pane-level shapes upward; location is document-level, so only the outer document reports it. No wildcard or foreign origin enters the path. ## Iframe Focus And Rendering Notes diff --git a/docs/specs/security-local.md b/docs/specs/security-local.md index 99482315..3a92d7d6 100644 --- a/docs/specs/security-local.md +++ b/docs/specs/security-local.md @@ -65,13 +65,14 @@ message authentication"). **The standalone adapters have no forgeable inbox**: host events arrive over Tauri IPC, never `window.postMessage`. **Each injected shim hop must address only its proxy origin and the embedder -chain's innermost origin, never `'*'`.** Nested frames relay the four registered -messages through same-origin parents. With no usable chain the proxy injects +chain's innermost origin, never `'*'`.** Nested frames relay the three pane-level +messages through same-origin parents; their document-level locations stop there. +With no usable chain the proxy injects nothing and strips no framing header (`docs/specs/dor-browser.md` -> "Iframe Host Capability And CSP"). What it grants a *caller* is [Loopback Listeners](#loopback-listeners)'s business. -- **FAIL IF** an injected shim targets anything but its proxy origin or the embedder chain's innermost origin, relays a foreign-origin or unregistered message, or the proxy uses a chain it did not validate in full: `iframeShim` and `normalizeEmbedderOrigins` in `lib/src/host/iframe-proxy-rewrite.ts`, applied in `lib/src/host/iframe-proxy.ts`. Pinned by `lib/src/host/iframe-proxy-rewrite.test.ts` and `lib/src/host/iframe-proxy.test.ts`. +- **FAIL IF** an injected shim targets anything but its proxy origin or the embedder chain's innermost origin, relays a nested `location`, a foreign-origin message, or an unregistered message, or the proxy uses a chain it did not validate in full: `iframeShim` and `normalizeEmbedderOrigins` in `lib/src/host/iframe-proxy-rewrite.ts`, applied in `lib/src/host/iframe-proxy.ts`. Pinned by `lib/src/host/iframe-proxy-rewrite.test.ts` and `lib/src/host/iframe-proxy.test.ts`. - **FAIL IF** a `VSCodeAdapter` host-channel listener acts on a message before `isHostMessage` (`lib/src/lib/vscode-message-token.ts`) accepts it, or the token stops being minted per serve and attached only by `WebviewChannel.post` in `vscode-ext/src/webview-messaging.ts`: `dor:controlRequest` is one of the shapes a framed page could otherwise claim. The proxy-origin listeners above are guarded by origin, not the token. Pinned by the `host message authentication` block in `lib/src/lib/platform/vscode-adapter.test.ts`. Source of truth: `isProxyOrigin` in `lib/src/lib/iframe-proxy-registry.ts`, the @@ -135,7 +136,7 @@ embedder, and the browser checks the whole chain (rationale). - **FAIL IF** any loopback HTTP or WebSocket listener grants an unrecognized caller a privilege it could not obtain by reaching the upstream directly. Refusing the request is one way; the iframe proxy's *admits all, vouches for none, names its embedder* is another, and is not a violation (rationale). `scripts/loopback-lint.mjs` (`pnpm test`) makes the cheap half deterministic — a new loopback bind that does not reference a guard module fails the build — but only in the bind forms it knows: `.listen` positional and options-object, `@hono/node-server`'s `serve({ hostname })`, and `ws`'s `new WebSocketServer({ port | host })`, each proved load-bearing by `scripts/loopback-lint-selftest.mjs`. **Adding a server dependency means adding its bind spelling there**; a host built at runtime is invisible to a regex in any spelling. It also only sees that a file *knows* a guard exists, never that the guard is called on every request, so this bullet still has to be read. Derive the set by searching the shipped trees for `createServer`, `.listen(`, `serve(` and `WebSocketServer` rather than trusting this list. Today the set is three: the iframe proxy (`lib/src/host/iframe-proxy.ts`), the VS Code agent-browser stream relay (`vscode-ext/src/agent-browser-host.ts`), and the browser-dev bridge (`standalone/scripts/dev-agent-browser.mjs`). A Unix-domain socket or named pipe is not in scope — no browser can reach one — which is why the `dor` control channel is bounded by socket permissions instead. - **FAIL IF** the iframe proxy rewrites `Origin` to the upstream's own origin for a caller whose inbound `Origin` is not the proxy's own — in `handleRequest` **or** `handleUpgrade`. A foreign `Origin` must be forwarded untouched rather than blocked, so the upstream sees the truth and applies its own policy (rationale). - **FAIL IF** the iframe proxy stops checking that `Host` names its own grant port, on either path. Its per-grant ephemeral port and one-fixed-upstream binding are real mitigations but neither is a secret, so the `Host` check is what makes DNS rebinding fail. -- **FAIL IF** the iframe proxy drops upstream `X-Frame-Options` / CSP `frame-ancestors` without replacing them with exactly `frame-ancestors 'self' `, admits another source, or targets the shim anywhere but that chain's innermost origin. With no usable chain it must preserve the headers and inject nothing (rationale). +- **FAIL IF** the iframe proxy drops upstream `X-Frame-Options` / CSP `frame-ancestors` without replacing them with exactly `frame-ancestors 'self' `, admits another source, or targets the shim anywhere but its own proxy origin and that chain's innermost origin. With no usable chain it must preserve the headers and inject nothing (rationale). - **FAIL IF** a request bearing a *foreign* `Origin` refreshes a grant's idle timer: a grant holds a live upstream binding, and a stranger polling it keeps a closed pane's binding open. An *absent* `Origin` must keep refreshing it — that is what a live frame's own navigations and sub-resources send. - **FAIL IF** the stream relay's grant stops being single-use, TTL-bounded, and pinned to one target port, or if it begins rewriting `Origin` rather than dropping it. It needs no `Host` check while the token holds (rationale). - **FAIL IF** the browser-dev bridge drops any of its four gates — the per-run token, the loopback `Host` check, the `application/json` content-type required of every non-GET, and the exact-origin `access-control-allow-origin`. The first three live together in the gate that runs before routing, so a route that never reads a body is covered by all of them. It is dev-only and ships in nothing, but it dispatches `pty_spawn` with caller-supplied `shell`, `args`, `cwd` and `env` — arbitrary command execution on a maintainer or CI-agent machine (`docs/specs/security-ci.md` -> "Automated Maintainer (tend)"). The content-type rule is a security control, not tidiness (rationale). diff --git a/docs/specs/security-local.rationale.md b/docs/specs/security-local.rationale.md index 8ecf40ba..98b16baf 100644 --- a/docs/specs/security-local.rationale.md +++ b/docs/specs/security-local.rationale.md @@ -47,8 +47,9 @@ only `browserSurfaceUrl` behind an open prompt, and a frame-URL reading that may lie. `use-wall-keyboard`'s leader channel accepts any live grant rather than one panel's, so a page in one browser pane can exit passthrough while another is focused. The nested-frame relay preserves this boundary: it accepts only the -same proxy origin and reconstructs one of those four shapes, so unrelated -application messages and every foreign origin stop at the child frame. +same proxy origin and reconstructs one of the three pane-level shapes, so +document-level locations, unrelated application messages, and every foreign +origin stop at the child frame. Why "the standalone adapters" and not "the standalone webview". The Wall's two proxy-origin `message` listeners (`use-wall-keyboard.ts`, `IframePanel.tsx`) are diff --git a/lib/src/host/iframe-proxy-rewrite.test.ts b/lib/src/host/iframe-proxy-rewrite.test.ts index 776a64ec..8c6d15ea 100644 --- a/lib/src/host/iframe-proxy-rewrite.test.ts +++ b/lib/src/host/iframe-proxy-rewrite.test.ts @@ -105,7 +105,7 @@ describe('the shim addresses the grant and app, not the world', () => { expect(IFRAME_SHIM).not.toContain("postMessage(m,'*')"); }); - it('relays registered messages through same-origin nested frames', () => { + it('relays pane-level messages but not nested locations through same-origin frames', () => { const delivered: unknown[] = []; const outer = shimFrame(APP, (data) => delivered.push(data)); const inner = shimFrame(PROXY, (data) => outer.emit('message', { origin: PROXY, data })); @@ -121,7 +121,13 @@ describe('the shim addresses the grant and app, not the world', () => { origin: PROXY, data: { __dormouse: 'location', url: `${PROXY}/next`, secret: 'x' }, }); - expect(delivered[1]).toEqual({ __dormouse: 'location', url: `${PROXY}/next` }); + expect(delivered).toHaveLength(1); + + outer.emit('message', { + origin: PROXY, + data: { __dormouse: 'open-window', url: `${PROXY}/next`, secret: 'x' }, + }); + expect(delivered[1]).toEqual({ __dormouse: 'open-window', url: `${PROXY}/next` }); }); }); diff --git a/lib/src/host/iframe-proxy-rewrite.ts b/lib/src/host/iframe-proxy-rewrite.ts index b4792e6a..15617d03 100644 --- a/lib/src/host/iframe-proxy-rewrite.ts +++ b/lib/src/host/iframe-proxy-rewrite.ts @@ -84,9 +84,10 @@ export function frameAncestorsCsp(embedderOrigins: string[]): string { // this is why the upstream CSP is dropped whole rather than per-directive (an // inline script needs `script-src` gone as much as the frame needs // `frame-ancestors` gone). It posts four message kinds to the Wall and nothing -// else (every other keystroke flows to the tool). A nested document relays them -// through its same-origin proxy parents until the outer document reaches the -// app: +// else (every other keystroke flows to the tool). A nested document relays the +// three pane-level kinds through its same-origin proxy parents until the outer +// document reaches the app; its document-level location stays inside the outer +// frame: // - `leader`: the reserved dual-tap ⌘/⇧ chord (matching handle-dual-tap.ts), // so the global chord keeps working with the frame focused. // - `pointerdown`: a click landed in the frame. A cross-origin click reaches @@ -105,15 +106,16 @@ export function iframeShim(embedderOrigin: string): string { if(!P||P===window)return; // Address each hop to the proxy's own origin and the app origin, never '*'. // Exactly one matches: a nested frame reaches its same-origin parent, while - // the outer frame reaches the app. Relays accept only the four fixed shapes - // below, so unrelated same-origin application messages never escape. + // the outer frame reaches the app. Relays accept only the three pane-level + // shapes below, so unrelated same-origin application messages and a nested + // document's location never escape. function send(m){try{P.postMessage(m,location.origin);}catch(e){}try{P.postMessage(m,TARGET);}catch(e){}} function post(t,d){var m={__dormouse:t};if(d)for(var k in d)m[k]=d[k];send(m);} addEventListener('message',function(e){ if(e.origin!==location.origin)return; var d=e.data,t=d&&d.__dormouse; if(t==='leader'||t==='pointerdown')post(t); - else if((t==='location'||t==='open-window')&&typeof d.url==='string')post(t,{url:d.url}); + else if(t==='open-window'&&typeof d.url==='string')post(t,{url:d.url}); },true); function postLocation(){post('location',{url:String(location.href)});} function anchorHref(e){