From bbd6b2a51715e101c6a2ac058fa439b4c13ba03d Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 15 Sep 2026 16:44:25 -0700 Subject: [PATCH 01/15] Add a capability-scoped local-file Tool viewer --- .github/audit/application-security.md | 4 +- docs/specs/dor-tool.md | 15 +- docs/specs/dor-tool.rationale.md | 4 + docs/specs/security-local.md | 10 +- docs/specs/terminal-escapes.md | 2 +- dor/package.json | 1 + dor/src/commands/open.ts | 6 +- dor/src/dor.ts | 6 +- dor/src/file-viewer-format.ts | 20 ++ dor/src/file-viewer-loopback-guard.ts | 12 ++ dor/src/file-viewer.ts | 176 ++++++++++++++++++ dor/src/node-runtime.d.ts | 51 ----- dor/test/file-viewer.test.mjs | 135 ++++++++++++++ dor/test/snapshots/help/open.md | 6 +- dor/tsconfig.json | 1 + lib/src/components/wall/browser-surface.ts | 2 +- lib/src/components/wall/lath-wall-engine.ts | 1 + lib/src/components/wall/tool-transfer.test.ts | 3 +- lib/src/components/wall/use-dor-control.ts | 2 +- .../components/wall/use-tool-serving.test.tsx | 13 ++ lib/src/components/wall/use-tool-serving.ts | 36 ++-- lib/src/host/tool-open.test.ts | 12 ++ lib/src/host/tool-open.ts | 5 + lib/src/host/tool-registry.ts | 2 +- lib/src/lib/platform/tool-types.ts | 2 +- lib/src/lib/session-save.ts | 2 +- lib/src/lib/session-types.ts | 4 +- lib/src/lib/tool-announce.test.ts | 7 + lib/src/lib/tool-announce.ts | 9 + pnpm-lock.yaml | 3 + scripts/spec-word-budgets.json | 4 +- 31 files changed, 466 insertions(+), 90 deletions(-) create mode 100644 dor/src/file-viewer-format.ts create mode 100644 dor/src/file-viewer-loopback-guard.ts create mode 100644 dor/src/file-viewer.ts delete mode 100644 dor/src/node-runtime.d.ts create mode 100644 dor/test/file-viewer.test.mjs diff --git a/.github/audit/application-security.md b/.github/audit/application-security.md index 49ab208b8..1e5214582 100644 --- a/.github/audit/application-security.md +++ b/.github/audit/application-security.md @@ -44,7 +44,9 @@ harnesses that already exercise this are For `## Loopback Listeners`, read `lib/src/host/loopback-guard.ts` first — it states the rule — then each listener it names. Derive the set of listeners by searching the shipped trees yourself; the section's own list is a description of -today's tree, not the scope. +today's tree, not the scope. The Local-file viewer subsection adds a tokenized +file grant: read `dor/src/file-viewer.ts` and `dor/src/file-viewer-loopback-guard.ts`, +including its static asset discovery, descriptor lifetime, and every request gate. For the rest of `docs/specs/security-local.md`, read each section's owner first — `docs/specs/terminal-escapes.md`, `docs/specs/dor-browser.md`, diff --git a/docs/specs/dor-tool.md b/docs/specs/dor-tool.md index d406f1b42..20b790a40 100644 --- a/docs/specs/dor-tool.md +++ b/docs/specs/dor-tool.md @@ -95,7 +95,7 @@ Source of truth: `createToolHost` in `lib/src/host/tool-host.ts`; `FileToolTrust | Anonymous command | Uses `auto` | - **Must poll unbound Tools every 1.5 seconds while their command runs.** Reset settle memory on command exit. (rationale) -- **Must let a changed announced port override a committed conflict or browser**, but only after a matching scan. An unchanged announcement never undoes URL-bar navigation. (rationale) +- **Must let a changed announced port or path override a committed conflict or browser**, but only after a matching scan. An unchanged announcement never undoes URL-bar navigation. (rationale) - **Must stop ordinary port scans once a browser or conflict is committed.** An unannounced additional port appearing after settle is not detected. - **Must display the browser destination before awaiting agent-browser startup**, leaving the session-less renderer inert until the binding arrives. Close any browser session whose Tool disappeared or changed command during startup. - **Must retain a runtime re-key within the Tool's namespace**, following [Identity and dedupe](#identity-and-dedupe). @@ -135,14 +135,22 @@ Source of truth: `toolCommand` in `dor/src/commands/tool.ts`; `dor/test/snapshot **Must accept exactly one existing local regular file for `dor open`.** Resolve it with the `$TARGET` rules in Declaring tools. URLs (including `file:`), directories, and Surface handles fail; no native-editor fallback occurs. -**Must select the first matching entry of the user file's ordered `open` list**, whose entries contain `match` and `tool`. `--tool` explicitly selects a user Tool. Every association must name a Tool in that same user file. Never discover project configuration during this lookup; project `open` rules are ignored with a warning during explicit project-tool lookup. +**Must select the first matching entry of the user file's ordered `open` list**, whose entries contain `match` and `tool`. `--tool` explicitly selects a handler. Every association must name a Tool in that same user file or `builtin:file`. Never discover project configuration during this lookup; project `open` rules are ignored with a warning during explicit project-tool lookup. **Must match patterns without `/` against the canonical filename, and patterns with `/` against the canonical path relative to the invocation CWD.** Normalize separators to `/` and use Node's POSIX `matchesGlob` semantics, including explicit patterns for dotfiles. A miss names the user config path and suggests `--tool`. **Must pass the canonical file path as one input to the selected Tool.** Reuse follows Identity and dedupe; `$TARGET` in the key provides per-file identity. `--fresh` bypasses reuse. **Never transform a plain calling terminal through `dor open`.** Create a focus-neutral split or reveal the existing Tool; an idle match in the caller's Tool pane uses the answer/prompt handshake only for a standalone integrated `dor open` invocation. +**Must use `builtin:file` for supported files when no user rule matches.** An explicit unknown handler or malformed user configuration fails without fallback. Built-in identity is the canonical file path in its own scope, separate from user and project Tools. + +**Must run the built-in viewer as a Tool-owned `dor` process**, serving HTML, images, PDF/media, and escaped text/source previews. Markdown is source text; custom viewers may render it. Text previews and HTML/CSS dependency inspection are limited to 8 MiB per file. The grant contains at most 256 files: the opened document and statically referenced relative HTML/CSS assets within its directory tree. Root-relative, external, dynamically discovered, and unreferenced resources are unavailable. + +**Must retain the viewer's opened file descriptors until the Tool exits.** Refresh reads those files again, but atomic replacements and changes to the dependency graph require restarting the viewer. Cold restore runs the saved file command with a fresh URL capability; Workspace movement keeps the live binding. The listener's authority is `docs/specs/security-local.md` → Local-file viewer. + Source of truth: `openCommand` in `dor/src/commands/open.ts`; `resolveOpenTool` in `lib/src/host/tool-open.ts`; `parseToolFile` in `lib/src/host/tool-registry.ts`; `surface.tool` in `lib/src/components/wall/use-dor-control.ts`. Tests: `lib/src/host/tool-open.test.ts`, `dor/test/cli-output.test.mjs`, `lib/src/components/Wall.test.tsx`. +Source of truth: `fileViewerFormat` in `dor/src/file-viewer-format.ts`; `startFileViewer` / `runFileViewer` in `dor/src/file-viewer.ts`; `dor/test/file-viewer.test.mjs`. + ## Take-over **Must run a standalone `dor tool` invocation in its calling pane when every takeover condition holds.** Otherwise use the ordinary split path. Trust approval and keyed reuse take precedence. (rationale) @@ -174,6 +182,7 @@ Source of truth: `toolTakesOverCaller` / `toolRerunsInCaller` in `lib/src/compon **Must consume OSC 367 at the PTY owner's parser**, including malformed and unknown verbs, and emit no reply. `serve` is the only implemented verb. The escape registry is `docs/specs/terminal-escapes.md`. - **Must sanitize and bound the payload before retaining it.** `ToolAnnounce` and `parseToolAnnounce` own the field shapes and validation limits. +- **Must treat an optional serve `path` as a path/query on the discovered port, never as another authority.** Accept at most 2,048 characters starting with one `/`, with no backslash, ASCII whitespace/control, or DEL; invalid paths are ignored and the default is `/`. The port still must belong to the designated Session's process tree. Live binding memory includes the path; durable saves omit it. - **Must forward parsed announcements from the host to the owning renderer**, which records the latest announcement per Session. Standalone uses `terminal:protocolEvents`; VS Code uses `terminal:toolAnnounce` scoped to the owning webview. The fake adapter applies locally. - **Must reconstruct announcements from raw replay without emitting replies**, and clear the renderer record on Session disposal. Ordinary terminal announcements stay inert. - Reserved: **Must retain `name`, `dehydrate`, and `persist` as inert parsed fields**, serving the announced-name and D1/D2 items under [Future](#future). Neither `persist: never` nor a `dehydrate` verb changes current persistence. @@ -205,8 +214,6 @@ Source of truth: `PersistedToolMetadata` in `lib/src/lib/session-types.ts`; `sav **Scope: dor-tools** — remaining design, in implementation order. -- **C — local-file presentation.** The loopback file/viewer endpoint a local - file needs (the iframe proxy instruments only `http://` upstreams). - **D1 — reaping without cooperation.** Idle-threshold reap + rehydrate-from-args + `persist: "never"`: every stateless tool, no new API, no Windows question. diff --git a/docs/specs/dor-tool.rationale.md b/docs/specs/dor-tool.rationale.md index 78d4cfcb5..a12b0577e 100644 --- a/docs/specs/dor-tool.rationale.md +++ b/docs/specs/dor-tool.rationale.md @@ -36,6 +36,10 @@ A hardcoded Storybook port can disagree with the port it obtains under contentio The September 2026 integration reuses Terminal Context for the Tool's primary terminal. The auxiliary helper's automatic refresh, Reset, and Promote semantics do not describe a serving command, whose Session also owns the browser and remote terminal identity. Sharing the presentation avoids introducing a second navigation mechanism or a second shell. +## Opening local files + +Keeping the built-in viewer in the Tool's process tree reuses port discovery, kill, restart, and Workspace transfer. An OSC path carries the per-run URL capability without saving that secret in the restart command. Holding the selected file descriptors bounds what the server can read after launch; it trades automatic replacement-file refresh for a grant whose contents cannot widen through path replacement. + ## Take-over **Why the gate is conservative in the split direction.** Every condition can be read wrong in two directions, and the two costs are nowhere near equal. Declining a take-over that should have happened costs a pane the user closes — the tool still runs, in the placement `dor tool` has always used. Taking over a pane that should have split types a command into a shell that belongs to something else: an agent's session, a line with work queued behind `dor`, a directory the tool was not asked to run in. So each condition is written to fail closed, and quoting is not unpicked — a line carrying `&&` inside quotes splits rather than being parsed for whether that `&&` is real. diff --git a/docs/specs/security-local.md b/docs/specs/security-local.md index 5700d8b7b..a6d234bec 100644 --- a/docs/specs/security-local.md +++ b/docs/specs/security-local.md @@ -116,7 +116,7 @@ ancestor chain the webview supplies with each proxy URL request.** `'self'` allo 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 its `BIND_FORMS` lists, each pinned by a fixture in `scripts/loopback-lint-selftest.mjs`, which goes red on a form that has none. **Adding a server dependency means adding its bind spelling there**; a host built at runtime is invisible to a regex in any spelling. The lint sees only a guard reference, not whether every request calls it, so this bullet is still read by hand. Derive the set by searching the shipped trees for `createServer`, `.listen(`, `serve(` and `WebSocket` 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** 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 its `BIND_FORMS` lists, each pinned by a fixture in `scripts/loopback-lint-selftest.mjs`, which goes red on a form that has none. **Adding a server dependency means adding its bind spelling there**; a host built at runtime is invisible to a regex in any spelling. The lint sees only a guard reference, not whether every request calls it, so this bullet is still read by hand. Derive the set by searching the shipped trees for `createServer`, `.listen(`, `serve(` and `WebSocket` rather than trusting this list. Today the set is four: the local-file viewer (`dor/src/file-viewer.ts`), 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 forwards `Cookie` upstream or `Set-Cookie` downstream on HTTP or WebSocket handshakes, including refused upgrades. Pinned by `lib/src/host/iframe-proxy.test.ts` (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. @@ -130,6 +130,14 @@ embedder, and the browser checks the whole chain (rationale). Source of truth: the shared rule and predicates — `isLoopbackHost`, `isOwnOrigin`, `isForeignOrigin` — in `lib/src/host/loopback-guard.ts`. +### Local-file viewer + +**FAIL IF** `dor/src/file-viewer.ts` serves any request without the fresh 256-bit URL capability, its own loopback `Host`, an absent or same-listener `Origin`, and a GET/HEAD method. `allowsFileViewerRequest` in `dor/src/file-viewer-loopback-guard.ts` gates every route. Never grant CORS access to foreign origins, cache responses, or send the capability as a referrer. + +**FAIL IF** the local-file viewer exposes directory listings, arbitrary path reads, writes, or a file outside its opened-document grant. Grant construction permits only regular files, rejects symlinks escaping the canonical document directory, bounds static dependency discovery, and retains descriptors so later path replacement cannot widen the grant. Viewer content is restricted by CSP to its own origin plus inline scripts/styles and data images; escaped text previews execute no document markup. + +Source of truth: `startFileViewer` in `dor/src/file-viewer.ts`; `allowsFileViewerRequest` in `dor/src/file-viewer-loopback-guard.ts`; `dor/test/file-viewer.test.mjs`. + ## Persisted state The attacker is another local account reading disk; what the remote stack leaves diff --git a/docs/specs/terminal-escapes.md b/docs/specs/terminal-escapes.md index ef3664df5..5abac8650 100644 --- a/docs/specs/terminal-escapes.md +++ b/docs/specs/terminal-escapes.md @@ -61,7 +61,7 @@ Replay (`pty:replay`) is the one raw stream and the one legitimate re-parse: **t | `OSC 633 ; E ; [; ] ST` | VS Code command line | [terminal-state.md](terminal-state.md#supported-osc-inputs) | | `OSC 633 ; P ; Cwd= ST` | CWD (VS Code) | [terminal-state.md](terminal-state.md#supported-osc-inputs) | | `OSC 777 ; notify ; ; <body> ST` | rxvt/WezTerm notification | [alert.md](alert.md#terminal-reports) | -| `OSC 367 ; serve ; <json> ST` | Dor Tool announcement: names which bound port to frame, plus a reserved name and runtime re-key | [dor-tool.md](dor-tool.md#osc-367) | +| `OSC 367 ; serve ; <json> ST` | Dor Tool announcement: selects a bound port and optional same-origin path, plus a reserved name and runtime re-key | [dor-tool.md](dor-tool.md#osc-367) | | `OSC 367 ; <any other verb> ST` | Reserved for the staged `dehydrate` verb; consumed and ignored. | [dor-tool.md](dor-tool.md#osc-367) | | `OSC 1337 ; CurrentDir=<cwd> ST` | CWD (iTerm2 compatibility) | [terminal-state.md](terminal-state.md#supported-osc-inputs) | | `OSC 1337 ; File=...:<data> ST` / `MultipartFile=...` / `FilePart=...` / `FileEnd` | iTerm2 inline image protocol (IIP); passed through to ImageAddon. | [Inline graphics](#inline-graphics) | diff --git a/dor/package.json b/dor/package.json index 2f584b801..258245704 100644 --- a/dor/package.json +++ b/dor/package.json @@ -17,6 +17,7 @@ "test": "pnpm run build && node --test test/*.test.mjs" }, "devDependencies": { + "@types/node": "^24.13.4", "esbuild": "^0.28.0", "typescript": "^6.0.3" }, diff --git a/dor/src/commands/open.ts b/dor/src/commands/open.ts index 78617e6cf..19fbcd5e4 100644 --- a/dor/src/commands/open.ts +++ b/dor/src/commands/open.ts @@ -20,9 +20,9 @@ export const openCommand: Command = { brief: 'Open a local file with a Dor Tool.', fullDescription: `Opens one existing local file. Relative paths resolve from the caller's directory (or --cwd); symlink aliases resolve to the same file. URLs, directories, and Surface handles are not accepted. -The first matching rule in the user dormouse.yml selects a user Tool. --tool chooses a user Tool explicitly. Project associations and project Tools never participate in this lookup. The user file is $XDG_CONFIG_HOME/dormouse/dormouse.yml, or ~/.config/dormouse/dormouse.yml. +The first matching rule in the user dormouse.yml selects a user Tool or builtin:file. --tool chooses a handler explicitly. Without a matching rule, the built-in viewer opens supported HTML, text/source, image, PDF, and media files. Use --tool builtin:file to select it explicitly. Markdown is shown as source text; a user Tool can provide rendered Markdown. Project associations and project Tools never participate in this lookup. The user file is $XDG_CONFIG_HOME/dormouse/dormouse.yml, or ~/.config/dormouse/dormouse.yml. -The ordered open list contains {match, tool} entries. Patterns without a slash match the filename; patterns with a slash match the canonical file path relative to the invocation directory, using forward slashes and Node glob syntax. Dotfiles require explicit patterns. +The ordered open list contains {match, tool} entries. Patterns without a slash match the filename; patterns with a slash match the canonical file path relative to the invocation directory, using forward slashes and Node glob syntax. Dotfiles require explicit patterns. The built-in HTML viewer serves statically referenced relative assets within the document directory tree; root-relative and external resources are unavailable. Text previews are capped at 8 MiB. The selected Tool receives the canonical absolute filename as one argument. Configure prespawn_dedupe: [$TARGET] to reveal the same file on repeated opens within a Workspace. --fresh bypasses reuse. @@ -36,7 +36,7 @@ Opening creates a focus-neutral split or reveals an existing Tool, never taking surface: { kind: 'parsed', parse: stringParser, brief: 'Surface to split when creating.', optional: true, placeholder: 'id|ref' }, workspace: workspaceFlag, cwd: { kind: 'parsed', parse: stringParser, brief: 'Directory for resolving the file.', optional: true, placeholder: 'path' }, - tool: { kind: 'parsed', parse: stringParser, brief: 'Use this user-global Tool.', optional: true, placeholder: 'name' }, + tool: { kind: 'parsed', parse: stringParser, brief: 'Use a user Tool or builtin:file.', optional: true, placeholder: 'name' }, }, positional: { kind: 'tuple', parameters: [{ parse: stringParser, brief: 'Local file to open.', placeholder: 'file' }] }, }, diff --git a/dor/src/dor.ts b/dor/src/dor.ts index dfc58b746..3e7190609 100644 --- a/dor/src/dor.ts +++ b/dor/src/dor.ts @@ -1,6 +1,7 @@ #!/usr/bin/env node import { runCli } from './cli.js'; +import { runFileViewer } from './file-viewer.js'; type ProcessLike = { argv: string[]; @@ -19,7 +20,10 @@ type ProcessLike = { declare const process: ProcessLike; -runCli(process.argv.slice(2), { env: process.env, readStdin }).then( +const execution = process.argv[2] === '__view-file' && process.argv.length === 4 + ? runFileViewer(process.argv[3]).then(() => ({ stdout: '', stderr: '', exitCode: 0 })) + : runCli(process.argv.slice(2), { env: process.env, readStdin }); +execution.then( (result) => { process.stdout.write(result.stdout); process.stderr.write(result.stderr); diff --git a/dor/src/file-viewer-format.ts b/dor/src/file-viewer-format.ts new file mode 100644 index 000000000..aebec0559 --- /dev/null +++ b/dor/src/file-viewer-format.ts @@ -0,0 +1,20 @@ +/** Formats supported by the built-in local viewer. Unknown files need a user + * Tool association rather than being guessed to be text. */ +const MIME: Record<string, string> = { + html: 'text/html; charset=utf-8', htm: 'text/html; charset=utf-8', + pdf: 'application/pdf', svg: 'image/svg+xml', png: 'image/png', jpg: 'image/jpeg', jpeg: 'image/jpeg', + gif: 'image/gif', webp: 'image/webp', avif: 'image/avif', ico: 'image/x-icon', + css: 'text/css; charset=utf-8', js: 'text/javascript; charset=utf-8', mjs: 'text/javascript; charset=utf-8', + json: 'application/json', woff: 'font/woff', woff2: 'font/woff2', ttf: 'font/ttf', otf: 'font/otf', + mp4: 'video/mp4', webm: 'video/webm', mp3: 'audio/mpeg', wav: 'audio/wav', ogg: 'audio/ogg', +}; +const TEXT = new Set(['txt', 'md', 'mdx', 'log', 'csv', 'tsv', 'json', 'jsonl', 'yaml', 'yml', 'toml', 'xml', + 'css', 'js', 'mjs', 'cjs', 'ts', 'tsx', 'jsx', 'py', 'rs', 'go', 'java', 'c', 'h', 'cpp', 'sh', 'ps1', 'sql', 'ini', 'conf']); + +export function fileViewerFormat(path: string): { mime: string; text: boolean } | null { + const name = path.replace(/\\/g, '/').split('/').pop()!.toLowerCase(); + const ext = name.includes('.') ? name.split('.').pop()! : ''; + const text = TEXT.has(ext) || /^(readme|license|licence|makefile|dockerfile|\.gitignore|\.env)(\..*)?$/.test(name); + const mime = MIME[ext] ?? (text ? 'text/plain; charset=utf-8' : null); + return mime ? { mime, text } : null; +} diff --git a/dor/src/file-viewer-loopback-guard.ts b/dor/src/file-viewer-loopback-guard.ts new file mode 100644 index 000000000..85f86a941 --- /dev/null +++ b/dor/src/file-viewer-loopback-guard.ts @@ -0,0 +1,12 @@ +import type { IncomingMessage } from 'node:http'; + +/** This listener owns its URLs: a per-process 256-bit capability authorizes + * each read. Host and Origin checks also reject rebinding and foreign scripts. + * See docs/specs/security-local.md -> Local-file viewer. */ +export function allowsFileViewerRequest(req: IncomingMessage, port: number, prefix: string): boolean { + const origins = [`http://localhost:${port}`, `http://127.0.0.1:${port}`]; + return (req.method === 'GET' || req.method === 'HEAD') + && origins.includes(`http://${req.headers.host ?? ''}`) + && (!req.headers.origin || origins.includes(req.headers.origin)) + && !!req.url?.startsWith(prefix); +} diff --git a/dor/src/file-viewer.ts b/dor/src/file-viewer.ts new file mode 100644 index 000000000..9a2bc3c35 --- /dev/null +++ b/dor/src/file-viewer.ts @@ -0,0 +1,176 @@ +import { randomBytes } from 'node:crypto'; +import { constants } from 'node:fs'; +import { open, realpath, type FileHandle } from 'node:fs/promises'; +import { createServer, type ServerResponse } from 'node:http'; +import { basename, dirname, isAbsolute, relative, resolve, sep } from 'node:path'; +import { fileViewerFormat } from './file-viewer-format.js'; +import { allowsFileViewerRequest } from './file-viewer-loopback-guard.js'; + +const TEXT_LIMIT = 8 * 1024 * 1024; +const ASSET_LIMIT = 256; +type Resource = { file: FileHandle; mime: string; text: boolean; path: string }; +const escapeHtml = (s: string) => s.replace(/[&<>"']/g, c => ({ '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' })[c]!); + +async function readText(file: FileHandle): Promise<string> { + const size = (await file.stat()).size; + if (size > TEXT_LIMIT) throw new Error('text preview exceeds 8 MiB; configure a Tool for this file'); + const bytes = Buffer.alloc(size + 1); + let offset = 0; + while (offset < bytes.length) { + const { bytesRead } = await file.read(bytes, offset, bytes.length - offset, offset); + if (!bytesRead) break; + offset += bytesRead; + } + if (offset > size) throw new Error('file changed while preparing preview; open it again'); + return bytes.subarray(0, offset).toString('utf8'); +} + +/** Static local dependencies only. No directory browsing, arbitrary fetch API, + * or external URL loading. Relative CSS dependencies are followed recursively. */ +function references(text: string, html: boolean): string[] { + const refs: string[] = []; + if (html) { + for (const tag of text.matchAll(/<(?:img|script|link|source|video|audio|iframe|embed|object)\b[^>]*>/gi)) { + for (const attr of tag[0].matchAll(/\b(?:src|href|poster|data)\s*=\s*(?:"([^"]*)"|'([^']*)'|([^\s>]+))/gi)) { + refs.push(attr[1] ?? attr[2] ?? attr[3]); + } + } + } + for (const match of text.matchAll(/url\(\s*(?:"([^"]*)"|'([^']*)'|([^\s)]*))\s*\)|@import\s+["']([^"']+)["']/gi)) { + refs.push(match[1] ?? match[2] ?? match[3] ?? match[4]); + } + return refs; +} + +function finish(res: ServerResponse, status: number, message = ''): void { + res.writeHead(status, { 'Content-Type': 'text/plain; charset=utf-8' }); + res.end(message); +} + +/** One Tool process owns one file grant and its file descriptors. Restarting + * creates a fresh capability; only the file argument is persisted by Dormouse. */ +export async function startFileViewer(input: string): Promise<{ port: number; path: string; close(): Promise<void> }> { + const target = await realpath(input); + const format = fileViewerFormat(target); + if (!format) throw new Error('unsupported file format; configure a user Tool association'); + const root = dirname(target); + const prefix = `/${randomBytes(32).toString('hex')}/`; + const resources = new Map<string, Resource>(); + const paths = new Set<string>(); + const closeFiles = async () => { await Promise.all([...resources.values()].map(r => r.file.close())); }; + + async function register(path: string, required: boolean): Promise<void> { + let file: FileHandle | undefined; + try { + const canonical = await realpath(path); + const rel = relative(root, canonical); + if (isAbsolute(rel) || rel === '..' || rel.startsWith(`..${sep}`)) return; + const route = `file/${relative(root, path).split(sep).join('/')}`; + if (resources.has(route)) return; + if (resources.size >= ASSET_LIMIT) throw new Error('local preview exceeds 256 referenced files'); + const type = fileViewerFormat(canonical); + if (!type) return; + file = await open(canonical, constants.O_RDONLY | (constants.O_NOFOLLOW ?? 0) | (constants.O_NONBLOCK ?? 0)); + if (!(await file.stat()).isFile()) throw new Error('not a regular file'); + const resource = { file, ...type, path: canonical }; + resources.set(route, resource); + file = undefined; // grant owns it now + if (paths.has(canonical)) return; + paths.add(canonical); + const html = type.mime.startsWith('text/html'); + if (html || type.mime.startsWith('text/css')) { + const contents = await readText(resource.file); + for (const ref of references(contents, html)) { + if (!ref || ref.startsWith('/') || ref.startsWith('#') || /^[a-z][a-z\d+.-]*:/i.test(ref) || ref.includes('\\')) continue; + let local: string; + try { local = decodeURIComponent(ref.split(/[?#]/, 1)[0]); } catch { continue; } + const asset = resolve(dirname(path), local); + const inside = relative(root, asset); + if (isAbsolute(inside) || inside === '..' || inside.startsWith(`..${sep}`)) continue; + await register(asset, false); + } + } + } catch (error) { + if (required || (error instanceof Error && /exceeds/.test(error.message))) throw error; + // A missing/broken relative asset stays unavailable; never broaden the grant. + } finally { await file?.close(); } + } + + try { + await register(target, true); + const main = resources.get(`file/${basename(target)}`)!; + if (!main) throw new Error('not a supported regular file'); + if (format.text) await readText(main.file); // fail oversized text before announcing + let port = 0; + const server = createServer((req, res) => { + res.setHeader('Cache-Control', 'no-store'); + res.setHeader('Referrer-Policy', 'no-referrer'); + res.setHeader('X-Content-Type-Options', 'nosniff'); + res.setHeader('Content-Security-Policy', "default-src 'none'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; media-src 'self'; font-src 'self'; connect-src 'self'; frame-src 'self'; object-src 'self'; base-uri 'self'; form-action 'none'"); + if (!allowsFileViewerRequest(req, port, prefix)) { finish(res, 403); return; } + void (async () => { + let route: string; + try { route = decodeURIComponent(new URL(req.url!, 'http://localhost').pathname.slice(prefix.length)); } + catch { finish(res, 400); return; } + if (route.includes('\\') || route.split('/').some(part => part === '..' || part === '.')) { finish(res, 403); return; } + if (route === 'view' && format.text) { + const text = await readText(main.file); + const body = `<!doctype html><meta charset="utf-8"><meta name="viewport" content="width=device-width"><title>${escapeHtml(basename(target))}
${escapeHtml(text)}
`; + res.writeHead(200, { 'Content-Type': 'text/html; charset=utf-8', 'Content-Length': Buffer.byteLength(body) }); + res.end(req.method === 'HEAD' ? undefined : body); + return; + } + const resource = resources.get(route); + if (!resource) { finish(res, 404); return; } + const size = (await resource.file.stat()).size; + let start = 0; + let end = size - 1; + const range = req.headers.range; + if (range) { + const match = /^bytes=(\d*)-(\d*)$/.exec(range); + if (!match || (!match[1] && !match[2])) { res.setHeader('Content-Range', `bytes */${size}`); finish(res, 416); return; } + start = match[1] ? Number(match[1]) : Math.max(0, size - Number(match[2])); + end = match[1] && match[2] ? Math.min(Number(match[2]), end) : end; + if (!Number.isSafeInteger(start) || !Number.isSafeInteger(end) || start > end || start < 0 || start >= size) { + res.setHeader('Content-Range', `bytes */${size}`); finish(res, 416); return; + } + res.setHeader('Content-Range', `bytes ${start}-${end}/${size}`); + } + res.writeHead(range ? 206 : 200, { 'Content-Type': resource.mime, 'Content-Length': Math.max(0, end - start + 1), 'Accept-Ranges': 'bytes' }); + if (req.method === 'HEAD' || size === 0) { res.end(); return; } + // Positional reads let simultaneous range requests share a descriptor. + // A disconnected response must not destroy the grant's shared handle. + const buffer = Buffer.alloc(64 * 1024); + for (let offset = start; offset <= end && !res.destroyed;) { + const { bytesRead } = await resource.file.read(buffer, 0, Math.min(buffer.length, end - offset + 1), offset); + if (!bytesRead) { res.destroy(); return; } + offset += bytesRead; + const chunk = Buffer.from(buffer.subarray(0, bytesRead)); + if (!res.write(chunk) && !res.destroyed) await new Promise(done => { + const complete = () => { res.off('drain', complete); res.off('close', complete); done(); }; + res.once('drain', complete); + res.once('close', complete); + }); + } + res.end(); + })().catch(() => { if (res.headersSent) res.destroy(); else finish(res, 500, 'File preview unavailable'); }); + }); + await new Promise((yes, no) => { server.once('error', no); server.listen(0, '127.0.0.1', yes); }); + port = (server.address() as { port: number }).port; + let closing: Promise | undefined; + return { port, path: `${prefix}${format.text ? 'view' : `file/${encodeURIComponent(basename(target))}`}`, + close: () => closing ??= new Promise((yes, no) => { + server.close(error => { void closeFiles().then(() => error ? no(error) : yes(), no); }); + server.closeAllConnections(); + }), + }; + } catch (error) { await closeFiles(); throw error; } +} + +export async function runFileViewer(file: string): Promise { + const viewer = await startFileViewer(file); + const stop = () => { void viewer.close().then(() => { process.exitCode = 0; }); }; + process.once('SIGINT', stop); + process.once('SIGTERM', stop); + process.stdout.write(`\x1b]367;serve;${JSON.stringify({ port: viewer.port, path: viewer.path, v: 1 })}\x07`); +} diff --git a/dor/src/node-runtime.d.ts b/dor/src/node-runtime.d.ts deleted file mode 100644 index 500a86779..000000000 --- a/dor/src/node-runtime.d.ts +++ /dev/null @@ -1,51 +0,0 @@ -declare module 'node:net' { - export interface Socket { - setEncoding(encoding: string): this; - write(data: string): boolean; - destroy(): this; - on(event: 'connect', listener: () => void): this; - on(event: 'data', listener: (chunk: string) => void): this; - on(event: 'error', listener: (error: Error) => void): this; - on(event: 'end', listener: () => void): this; - } - - export function createConnection(options: { path: string }): Socket; -} - -declare module 'node:crypto' { - // Opaque stand-in for Buffer: this package ships without @types/node (see the - // hand-written shims around it), and nothing here needs more than "the thing - // digest() returns, which timingSafeEqual accepts". - export interface BinaryDigest { - readonly length: number; - } - - export interface Hash { - update(data: string): Hash; - digest(): BinaryDigest; - digest(encoding: 'hex'): string; - } - - export function createHash(algorithm: string): Hash; - export function createHmac(algorithm: string, key: string): Hash; - export function randomBytes(size: number): { toString(encoding: 'hex'): string }; - export function timingSafeEqual(a: BinaryDigest, b: BinaryDigest): boolean; -} - -declare module 'node:fs' { - export function existsSync(path: string): boolean; - export function readFileSync(path: string, encoding: 'utf8'): string; - export function writeFileSync(path: string, data: string): void; -} - -declare module 'node:path' { - export function resolve(...segments: string[]): string; -} - -declare const process: { - platform: string; - cwd(): string; -}; - -declare function setTimeout(callback: () => void, ms?: number): number; -declare function clearTimeout(timeoutId: number): void; diff --git a/dor/test/file-viewer.test.mjs b/dor/test/file-viewer.test.mjs new file mode 100644 index 000000000..65b862d51 --- /dev/null +++ b/dor/test/file-viewer.test.mjs @@ -0,0 +1,135 @@ +import assert from 'node:assert/strict'; +import { mkdtemp, mkdir, realpath, rm, symlink, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { request } from 'node:http'; +import { spawn } from 'node:child_process'; +import { once } from 'node:events'; +import { fileURLToPath } from 'node:url'; +import { afterEach, beforeEach, test } from 'node:test'; +import { startFileViewer } from '../dist/file-viewer.js'; + +let root; +const viewers = []; +beforeEach(async () => { root = await realpath(await mkdtemp(join(tmpdir(), 'dor-viewer-'))); }); +afterEach(async () => { + await Promise.all(viewers.splice(0).map(v => v.close())); + await rm(root, { recursive: true, force: true }); +}); +async function start(name, contents) { + const file = join(root, name); + await writeFile(file, contents); + const viewer = await startFileViewer(file); + viewers.push(viewer); + return viewer; +} +async function get(viewer, path = viewer.path, headers = {}, method = 'GET') { + return new Promise((resolve, reject) => { + const req = request({ host: '127.0.0.1', port: viewer.port, path, headers, method }, res => { + const chunks = []; + res.on('data', chunk => chunks.push(chunk)); + res.on('end', () => resolve({ status: res.statusCode, headers: res.headers, body: Buffer.concat(chunks).toString() })); + }); + req.on('error', reject); + req.end(); + }); +} +const asset = (viewer, path) => viewer.path.replace(/\/file\/.*$/, `/file/${path}`); + +test('renders text as escaped content and requires the per-run token on every method', async () => { + const viewer = await start('README.md', ' & hello'); + const good = await get(viewer); + assert.equal(good.status, 200); + assert.match(good.body, /<script>bad\(\)<\/script> & hello/); + assert.equal(good.headers['referrer-policy'], 'no-referrer'); + assert.equal(good.headers['cache-control'], 'no-store'); + for (const path of ['/', '/wrong/view', viewer.path.replace(/\/[a-f0-9]{64}\//, '/')]) { + assert.equal((await get(viewer, path)).status, 403); + } + assert.equal((await get(viewer, viewer.path, { Host: `evil.test:${viewer.port}` })).status, 403); + assert.equal((await get(viewer, viewer.path, { Origin: 'https://evil.test' })).status, 403); + assert.equal((await get(viewer, viewer.path, {}, 'POST')).status, 403); + assert.equal((await get(viewer, viewer.path, {}, 'HEAD')).body, ''); + const second = await startFileViewer(join(root, 'README.md')); + viewers.push(second); + assert.notEqual(second.path, viewer.path); + assert.equal((await get(second, viewer.path)).status, 403); +}); + +test('serves only the HTML document and its bounded relative dependency graph', async () => { + await mkdir(join(root, 'assets')); + await writeFile(join(root, 'assets', 'style.css'), '@import "more.css"; body { background: url(pic.svg) }'); + await writeFile(join(root, 'assets', 'more.css'), 'body { color: red }'); + await writeFile(join(root, 'assets', 'pic.svg'), ''); + await writeFile(join(root, 'unreferenced.txt'), 'private sibling'); + const viewer = await start('index.html', '

Preview

'); + assert.equal((await get(viewer)).status, 200); + for (const path of ['assets/style.css', 'assets/more.css', 'assets/pic.svg']) assert.equal((await get(viewer, asset(viewer, path))).status, 200); + assert.equal((await get(viewer, asset(viewer, 'unreferenced.txt'))).status, 404); + assert.notEqual((await get(viewer, asset(viewer, '%2e%2e/unreferenced.txt'))).status, 200); + assert.equal((await get(viewer, asset(viewer, '%E0%A4%A'))).status, 400); + assert.equal((await get(viewer, asset(viewer, 'assets/style.css'))).status, 200); // repeated streams retain the grant +}); + +test('rejects parent-directory references and symlinks escaping the document directory', { skip: process.platform === 'win32' }, async () => { + await mkdir(join(root, 'page')); + await writeFile(join(root, 'secret.txt'), 'secret'); + await symlink(join(root, 'secret.txt'), join(root, 'page', 'linked.txt')); + const viewer = await start('page/index.html', ''); + assert.equal((await get(viewer, asset(viewer, 'linked.txt'))).status, 404); + assert.notEqual((await get(viewer, asset(viewer, '../secret.txt'))).status, 200); +}); + +test('supports byte ranges and HEAD for native PDF/image presentation', async () => { + const viewer = await start('sample.pdf', '%PDF-1.7 example bytes'); + assert.equal((await get(viewer)).headers['content-type'], 'application/pdf'); + const range = await get(viewer, viewer.path, { Range: 'bytes=0-3' }); + assert.equal(range.status, 206); + assert.equal(range.body, '%PDF'); + assert.equal((await get(viewer, viewer.path, { Range: 'bytes=-5' })).body, 'bytes'); + assert.equal((await get(viewer, viewer.path, { Range: 'bytes=999-1000' })).status, 416); + assert.equal((await get(viewer, viewer.path, { Range: 'bytes=0-1,4-6' })).status, 416); + assert.equal((await get(viewer, viewer.path, {}, 'HEAD')).body, ''); +}); + +test('fails unsupported formats and oversized text before starting a viewer', async () => { + await writeFile(join(root, 'unknown.bin'), 'binary'); + await assert.rejects(startFileViewer(join(root, 'unknown.bin')), /unsupported/); + await writeFile(join(root, 'large.txt'), Buffer.alloc(8 * 1024 * 1024 + 1)); + await assert.rejects(startFileViewer(join(root, 'large.txt')), /8 MiB/); +}); + +test('bounds the asset graph and keeps a grant on the opened file after path replacement', async () => { + const viewer = await start('original.txt', 'original content'); + await rm(join(root, 'original.txt')); + await writeFile(join(root, 'original.txt'), 'replacement content'); + assert.match((await get(viewer)).body, /original content/); + const names = Array.from({ length: 256 }, (_, i) => `style${i}.css`); + await Promise.all(names.map(name => writeFile(join(root, name), ''))); + const html = join(root, 'many.html'); + await writeFile(html, names.map(name => ``).join('')); + await assert.rejects(startFileViewer(html), /256 referenced files/); +}); + +test('the bundled private entry announces its port and path, then exits on termination', { timeout: 10_000 }, async () => { + const file = join(root, 'cli.txt'); + await writeFile(file, 'cli preview'); + const child = spawn(process.execPath, [fileURLToPath(new URL('../dist/dor.js', import.meta.url)), '__view-file', file], { stdio: ['ignore', 'pipe', 'pipe'] }); + try { + let output = ''; + const announce = await new Promise((resolve, reject) => { + child.once('error', reject); + child.once('exit', code => reject(new Error(`viewer exited early: ${code}`))); + child.stdout.on('data', chunk => { + output += chunk; + const match = /\x1b\]367;serve;(\{[^\x07]*\})\x07/.exec(output); + if (match) resolve(JSON.parse(match[1])); + }); + }); + assert.equal((await get(announce)).status, 200); + const exited = once(child, 'exit'); + child.kill('SIGTERM'); + await exited; + await assert.rejects(get(announce)); + } finally { child.kill('SIGKILL'); } +}); diff --git a/dor/test/snapshots/help/open.md b/dor/test/snapshots/help/open.md index d1271c51c..c6a58519e 100644 --- a/dor/test/snapshots/help/open.md +++ b/dor/test/snapshots/help/open.md @@ -9,9 +9,9 @@ USAGE Opens one existing local file. Relative paths resolve from the caller's directory (or --cwd); symlink aliases resolve to the same file. URLs, directories, and Surface handles are not accepted. -The first matching rule in the user dormouse.yml selects a user Tool. --tool chooses a user Tool explicitly. Project associations and project Tools never participate in this lookup. The user file is $XDG_CONFIG_HOME/dormouse/dormouse.yml, or ~/.config/dormouse/dormouse.yml. +The first matching rule in the user dormouse.yml selects a user Tool or builtin:file. --tool chooses a handler explicitly. Without a matching rule, the built-in viewer opens supported HTML, text/source, image, PDF, and media files. Use --tool builtin:file to select it explicitly. Markdown is shown as source text; a user Tool can provide rendered Markdown. Project associations and project Tools never participate in this lookup. The user file is $XDG_CONFIG_HOME/dormouse/dormouse.yml, or ~/.config/dormouse/dormouse.yml. -The ordered open list contains {match, tool} entries. Patterns without a slash match the filename; patterns with a slash match the canonical file path relative to the invocation directory, using forward slashes and Node glob syntax. Dotfiles require explicit patterns. +The ordered open list contains {match, tool} entries. Patterns without a slash match the filename; patterns with a slash match the canonical file path relative to the invocation directory, using forward slashes and Node glob syntax. Dotfiles require explicit patterns. The built-in HTML viewer serves statically referenced relative assets within the document directory tree; root-relative and external resources are unavailable. Text previews are capped at 8 MiB. The selected Tool receives the canonical absolute filename as one argument. Configure prespawn_dedupe: [$TARGET] to reveal the same file on repeated opens within a Workspace. --fresh bypasses reuse. @@ -24,7 +24,7 @@ FLAGS [--surface] Surface to split when creating. [--workspace] Workspace to act in, instead of the caller's. [--cwd] Directory for resolving the file. - [--tool] Use this user-global Tool. + [--tool] Use a user Tool or builtin:file. -h --help Print help information and exit -- All subsequent inputs should be interpreted as arguments diff --git a/dor/tsconfig.json b/dor/tsconfig.json index 15395d7eb..5284c140e 100644 --- a/dor/tsconfig.json +++ b/dor/tsconfig.json @@ -2,6 +2,7 @@ "compilerOptions": { "target": "ES2022", "lib": ["ES2022"], + "types": ["node"], "module": "ES2022", "moduleResolution": "bundler", "outDir": "dist", diff --git a/lib/src/components/wall/browser-surface.ts b/lib/src/components/wall/browser-surface.ts index 8e4351869..712acee50 100644 --- a/lib/src/components/wall/browser-surface.ts +++ b/lib/src/components/wall/browser-surface.ts @@ -139,7 +139,7 @@ export function namespacedToolKey( scope?: unknown, ): string[] | null { if (!toolName || key === null) return null; - return scope === 'user' ? ['user', toolName, ...key] : [toolName, ...key]; + return scope === 'user' || scope === 'builtin' ? [scope, toolName, ...key] : [toolName, ...key]; } /** Whether params describe a plain browser surface (vs a terminal): the unified diff --git a/lib/src/components/wall/lath-wall-engine.ts b/lib/src/components/wall/lath-wall-engine.ts index 69d1985be..d1078956f 100644 --- a/lib/src/components/wall/lath-wall-engine.ts +++ b/lib/src/components/wall/lath-wall-engine.ts @@ -125,6 +125,7 @@ export function persistableLeafMeta(meta: LeafMeta): LeafMeta { showTerminal: _showTerminal, toolPortConflict: _toolPortConflict, toolAnnouncedPort: _toolAnnouncedPort, + toolAnnouncedPath: _toolAnnouncedPath, ...rest } = meta.params; return { ...meta, params: rest }; diff --git a/lib/src/components/wall/tool-transfer.test.ts b/lib/src/components/wall/tool-transfer.test.ts index 71a4301e8..79a700178 100644 --- a/lib/src/components/wall/tool-transfer.test.ts +++ b/lib/src/components/wall/tool-transfer.test.ts @@ -6,7 +6,7 @@ import type { RestoredSession } from '../../lib/session-restore'; const params = { surfaceType: 'tool', command: 'pnpm storybook', toolRender: 'ab-screencast', url: 'http://localhost:6006/edited', renderMode: 'ab-screencast', - session: 'dormouse.1.tool-one', wsPort: 9222, toolAnnouncedPort: 6006, + session: 'dormouse.1.tool-one', wsPort: 9222, toolAnnouncedPort: 6006, toolAnnouncedPath: '/token/view', }; function engine(initial = params) { @@ -27,6 +27,7 @@ describe('Tool Workspace transfer', () => { expect(plan.doors[0].params).toEqual(params); expect(durable.leafMeta.tool.params).not.toHaveProperty('url'); expect(durable.leafMeta.tool.params).not.toHaveProperty('toolAnnouncedPort'); + expect(durable.leafMeta.tool.params).not.toHaveProperty('toolAnnouncedPath'); expect(door.params).not.toHaveProperty('session'); expect(lath.getMeta('tool')!.params).toEqual(params); }); diff --git a/lib/src/components/wall/use-dor-control.ts b/lib/src/components/wall/use-dor-control.ts index e97353158..c4f9403c3 100644 --- a/lib/src/components/wall/use-dor-control.ts +++ b/lib/src/components/wall/use-dor-control.ts @@ -875,7 +875,7 @@ export function useDorControl({ } let command: string; let key: string[] | null = null; - let toolScope: 'user' | undefined; + let toolScope: 'user' | 'builtin' | undefined; const toolArgs = stringArrayParam(params.args) ?? []; let warnings: string[] = []; let render: 'iframe' | 'ab-screencast' = 'iframe'; diff --git a/lib/src/components/wall/use-tool-serving.test.tsx b/lib/src/components/wall/use-tool-serving.test.tsx index 3eb25106d..5c0e0e3a5 100644 --- a/lib/src/components/wall/use-tool-serving.test.tsx +++ b/lib/src/components/wall/use-tool-serving.test.tsx @@ -91,6 +91,19 @@ async function run(params: Record, scans: OpenPort[][]) { describe('port: announced', () => { const announced = { surfaceType: 'tool', command: 'x', toolPort: 'announced' }; + it('uses a same-origin announced path and follows path changes without resetting navigation', async () => { + const announce = { port: 6006, path: '/first/view', name: null, key: null, dehydrate: false, persist: null }; + recordToolAnnounce('tool-1', announce); + const { state } = await run(announced, [[tcp(6006)]]); + expect(state.params.url).toBe('http://localhost:6006/first/view'); + state.params.url = 'http://localhost:6006/first/edited'; + await act(async () => { await vi.advanceTimersByTimeAsync(POLL_MS); }); + expect(state.params.url).toBe('http://localhost:6006/first/edited'); + recordToolAnnounce('tool-1', { ...announce, path: '/second/view' }); + await act(async () => { await vi.advanceTimersByTimeAsync(POLL_MS); }); + expect(state.params.url).toBe('http://localhost:6006/second/view'); + }); + it('defers Workspace transfer until reopening an existing browser has settled', async () => { const { lath, state } = fakeLath({ ...announced, toolRender: 'ab-screencast', renderMode: 'ab-screencast', diff --git a/lib/src/components/wall/use-tool-serving.ts b/lib/src/components/wall/use-tool-serving.ts index 2b80d0b1d..77f3c386b 100644 --- a/lib/src/components/wall/use-tool-serving.ts +++ b/lib/src/components/wall/use-tool-serving.ts @@ -19,6 +19,7 @@ import { import { attachAgentBrowserSession } from './tool-browser-session'; import { listenerUrlsByPort } from './port-url'; import { getToolAnnounce } from '../../lib/tool-announce-store'; +import { validToolServePath } from '../../lib/tool-announce'; import { sessionForKey } from 'dor-lib-common/agent-browser'; import { markAgentBrowserSessionClosed } from './agent-browser-sessions'; import { disposeAgentBrowserSurfaceController } from './agent-browser-surface-controller'; @@ -66,10 +67,10 @@ export function useToolServing({ // A ref, not state: it drives no render, and a leaf's entry is dropped when // its command exits so a re-run settles again from scratch. const seenPorts = useRef>(new Map()); - // The announced port last applied to each leaf. A changed announcement may + // The announced port/path last applied to each leaf. A changed announcement may // re-point a live browser, but the same announcement must not keep undoing // URL-bar navigation just because params.url no longer names that port. - const appliedAnnouncedPorts = useRef>(new Map()); + const appliedAnnouncements = useRef>(new Map()); useEffect(() => { const platform = getPlatform(); @@ -84,8 +85,8 @@ export function useToolServing({ for (const id of seenPorts.current.keys()) { if (!live.has(id)) seenPorts.current.delete(id); } - for (const id of appliedAnnouncedPorts.current.keys()) { - if (!live.has(id)) appliedAnnouncedPorts.current.delete(id); + for (const id of appliedAnnouncements.current.keys()) { + if (!live.has(id)) appliedAnnouncements.current.delete(id); } for (const leaf of leaves) { @@ -118,7 +119,7 @@ export function useToolServing({ // the regression the settle window exists to prevent. if (!running) { seenPorts.current.delete(leaf.id); - appliedAnnouncedPorts.current.delete(leaf.id); + appliedAnnouncements.current.delete(leaf.id); } if ((hasUrl || hasConflict) && !running) { @@ -137,6 +138,7 @@ export function useToolServing({ lath.store.updateParams(leaf.id, { url: undefined, toolAnnouncedPort: undefined, + toolAnnouncedPath: undefined, toolPortConflict: undefined, session: undefined, wsPort: undefined, @@ -151,18 +153,20 @@ export function useToolServing({ // the pane would show the conflict for the life of the command, telling // the user to announce a port it had just announced. // An announcement outranks whatever autobind decided, framed or - // refused. Only a *changed* announced port re-points a live browser: + // refused. Only a *changed* announced port/path re-points a live browser: // treating a mismatch with params.url as a change would undo URL-bar // navigation every poll after the user left the announced origin. const announcedPort = announce?.port ?? null; - if (announcedPort === null) appliedAnnouncedPorts.current.delete(leaf.id); - if (!appliedAnnouncedPorts.current.has(leaf.id) && typeof leaf.params?.toolAnnouncedPort === 'number') { - appliedAnnouncedPorts.current.set(leaf.id, leaf.params.toolAnnouncedPort); + const announcedPath = validToolServePath(announce?.path) ? announce.path : '/'; + const announcementKey = JSON.stringify([announcedPort, announcedPath]); + if (announcedPort === null) appliedAnnouncements.current.delete(leaf.id); + if (!appliedAnnouncements.current.has(leaf.id) && typeof leaf.params?.toolAnnouncedPort === 'number') { + appliedAnnouncements.current.set(leaf.id, JSON.stringify([leaf.params.toolAnnouncedPort, leaf.params.toolAnnouncedPath ?? '/'])); } - const announcedPortChanged = announcedPort !== null - && appliedAnnouncedPorts.current.get(leaf.id) !== announcedPort; + const announcementChanged = announcedPort !== null + && appliedAnnouncements.current.get(leaf.id) !== announcementKey; if (!running) continue; - if ((hasUrl || hasConflict) && !announcedPortChanged) continue; + if ((hasUrl || hasConflict) && !announcementChanged) continue; let ports; try { @@ -180,7 +184,7 @@ export function useToolServing({ // announced port that nothing bound frames nothing. entry = entries.find((candidate) => candidate.port === announce.port); if (!entry) continue; - appliedAnnouncedPorts.current.set(leaf.id, announce.port); + appliedAnnouncements.current.set(leaf.id, announcementKey); } else if (leaf.params?.toolPort !== 'auto') { // `announced`: never guess. No announcement, no browser. continue; @@ -215,11 +219,13 @@ export function useToolServing({ // -> Instant create). `toolFace` tests the conflict before the url, so // a stale verdict would keep the conflict forward over the browser. const agentDrivable = leaf.params?.toolRender === 'ab-screencast'; + const url = new URL(announcedPath, entry.url).href; lath.store.updateParams(leaf.id, { - url: entry.url, + url, renderMode: agentDrivable ? 'ab-screencast' : 'iframe', toolPortConflict: undefined, toolAnnouncedPort: announcedPort ?? undefined, + toolAnnouncedPath: announcedPort === null ? undefined : announcedPath, // Reopening an existing browser is also an in-flight connection: // withhold its binding until open settles so a Workspace move cannot // capture the old stream while this webview still owns the launch. @@ -233,7 +239,7 @@ export function useToolServing({ // its id stable while its capabilities come and go. const session = sessionForKey(`tool.${leaf.id}`); await attachAgentBrowserSession({ - url: entry.url, + url, platform, session, surfaceId: leaf.id, diff --git a/lib/src/host/tool-open.test.ts b/lib/src/host/tool-open.test.ts index ec8458dd1..39c3d4194 100644 --- a/lib/src/host/tool-open.test.ts +++ b/lib/src/host/tool-open.test.ts @@ -64,3 +64,15 @@ it('names the user configuration in unmatched-file errors and refuses broken ass await writeFile(config, 'open:\n - {match: "*", tool: undeclared}\n'); expect(await host().handle({ op: 'open', target, cwd: root })).toMatchObject({ status: 'error', message: expect.stringContaining('defined in this user file') }); }); + +it('uses the built-in viewer only as a fallback or explicit choice', async () => { + const target = join(root, 'docs', 'README.md'); + expect(await host().handle({ op: 'open', target, cwd: root, tool: 'builtin:file' })).toMatchObject({ + status: 'ok', scope: 'builtin', run: ['dor', '__view-file', target], key: [target], port: 'announced', + }); + await writeFile(config, 'open:\n - {match: "*.md", tool: "builtin:file"}\n'); + expect(await host().handle({ op: 'open', target, cwd: root })).toMatchObject({ status: 'ok', scope: 'builtin' }); + await rm(config); + expect(await host().handle({ op: 'open', target, cwd: root })).toMatchObject({ status: 'ok', scope: 'builtin' }); + expect(await host().handle({ op: 'open', target, cwd: root, tool: 'missing' })).toMatchObject({ status: 'error' }); +}); diff --git a/lib/src/host/tool-open.ts b/lib/src/host/tool-open.ts index 0ad7b44ea..16b437dba 100644 --- a/lib/src/host/tool-open.ts +++ b/lib/src/host/tool-open.ts @@ -2,6 +2,7 @@ import { basename, posix, relative, sep } from 'node:path'; import type { ToolLookupResult } from '../lib/platform/tool-types'; import { resolveLocalToolTarget, resolveToolInput } from './tool-input'; import { readUserToolFile } from './tool-user-config'; +import { fileViewerFormat } from 'dor/file-viewer-format'; /** Dispatch is entirely user-owned. Never discover a project file here, even * when its Tool name shadows the rule's selected user Tool. */ @@ -15,6 +16,10 @@ export async function resolveOpenTool( const name = request.tool ?? file?.open.find(rule => posix.matchesGlob(rule.match.includes('/') ? relativePath : basename(target), rule.match))?.tool; const entry = name && file?.tools.get(name); + if ((!name || name === 'builtin:file') && fileViewerFormat(target)) { + return { status: 'ok', projectRoot: request.cwd, path: '', name: 'file', scope: 'builtin', + run: ['dor', '__view-file', target], key: [target], render: 'iframe', port: 'announced', warnings: [] }; + } if (!file || !entry) return { status: 'error', message: request.tool ? `no user Tool '${request.tool}' in ${path}` : `no Tool matches '${request.target}'; add an open rule to ${path}, or use dor open --tool ` }; diff --git a/lib/src/host/tool-registry.ts b/lib/src/host/tool-registry.ts index 299422979..16250ed18 100644 --- a/lib/src/host/tool-registry.ts +++ b/lib/src/host/tool-registry.ts @@ -195,7 +195,7 @@ export function parseToolFile( if (!Array.isArray(doc.open)) throw new ToolFileError(`${path}: 'open' must be an ordered list`); for (const rule of doc.open) { if (!isRecord(rule) || typeof rule.match !== 'string' || !rule.match || typeof rule.tool !== 'string' - || !tools.has(rule.tool) || Object.keys(rule).some(key => key !== 'match' && key !== 'tool')) { + || (rule.tool !== 'builtin:file' && !tools.has(rule.tool)) || Object.keys(rule).some(key => key !== 'match' && key !== 'tool')) { throw new ToolFileError(`${path}: each open rule needs a match pattern and a tool defined in this user file`); } open.push({ match: rule.match, tool: rule.tool }); diff --git a/lib/src/lib/platform/tool-types.ts b/lib/src/lib/platform/tool-types.ts index 8a85f8e71..f94c33606 100644 --- a/lib/src/lib/platform/tool-types.ts +++ b/lib/src/lib/platform/tool-types.ts @@ -34,7 +34,7 @@ export type ToolLookupResult = run: string | readonly string[]; /** Renderer for the tool's browser once it serves; 'iframe' by default. */ render: 'iframe' | 'ab-screencast'; - scope?: 'user'; + scope?: 'user' | 'builtin'; /** How to pick the port to frame absent an announcement; 'announced' by * default, meaning nothing is framed without OSC 367. */ port: 'announced' | 'auto'; diff --git a/lib/src/lib/session-save.ts b/lib/src/lib/session-save.ts index 4dda12c97..9363337ab 100644 --- a/lib/src/lib/session-save.ts +++ b/lib/src/lib/session-save.ts @@ -181,7 +181,7 @@ function toolMetadataFromParams(params: Record | undefined): Pe const key = Array.isArray(params.toolKey) && params.toolKey.every((part) => typeof part === 'string') ? params.toolKey as string[] : undefined; - return { ...(name ? { name } : {}), ...(params.toolScope === 'user' ? { scope: 'user' as const } : {}), render, port, ...(key ? { key } : {}) }; + return { ...(name ? { name } : {}), ...(params.toolScope === 'user' || params.toolScope === 'builtin' ? { scope: params.toolScope } : {}), render, port, ...(key ? { key } : {}) }; } function persistedVisiblePaneTitle(title: string): string { diff --git a/lib/src/lib/session-types.ts b/lib/src/lib/session-types.ts index 62171fc64..a68c1d2e1 100644 --- a/lib/src/lib/session-types.ts +++ b/lib/src/lib/session-types.ts @@ -16,7 +16,7 @@ export type PersistedSurfaceType = 'terminal' | 'browser' | 'tool'; * is respawned. Derived browser state (URL/session/port conflict) never enters * this projection. */ export interface PersistedToolMetadata { - scope?: 'user'; + scope?: 'user' | 'builtin'; name?: string; render: 'iframe' | 'ab-screencast'; port: 'announced' | 'auto'; @@ -169,7 +169,7 @@ function isPersistedToolMetadataShape(value: unknown): boolean { if (!isRecord(value)) return false; return ( (value.name === undefined || typeof value.name === 'string') && - (value.scope === undefined || value.scope === 'user') && + (value.scope === undefined || value.scope === 'user' || value.scope === 'builtin') && (value.render === 'iframe' || value.render === 'ab-screencast') && (value.port === 'announced' || value.port === 'auto') && (value.key === undefined || (Array.isArray(value.key) && value.key.every((part) => typeof part === 'string'))) diff --git a/lib/src/lib/tool-announce.test.ts b/lib/src/lib/tool-announce.test.ts index a1555b300..5818bd8e5 100644 --- a/lib/src/lib/tool-announce.test.ts +++ b/lib/src/lib/tool-announce.test.ts @@ -7,6 +7,13 @@ import { applyTerminalProtocolEvents } from './terminal-protocol'; const serve = (payload: unknown) => `serve;${JSON.stringify(payload)}`; describe('parseToolAnnounce', () => { + it('accepts a same-origin serve path and drops authority-changing or malformed paths', () => { + expect(parseToolAnnounce(serve({ port: 6006, path: '/token/file/a%20b.html?mode=1' }))?.path).toBe('/token/file/a%20b.html?mode=1'); + for (const path of ['https://evil.test/', '//evil.test/', '/\\evil.test/', '/a\nb', 'relative', `/${'a'.repeat(2048)}`]) { + expect(parseToolAnnounce(serve({ port: 6006, path }))?.path).toBeUndefined(); + } + }); + it('reads a full serve payload', () => { expect(parseToolAnnounce(serve({ port: 6006, name: 'Storybook', key: ['storybook', '/repo'], dehydrate: true, persist: 'never', v: 1 }))).toEqual({ port: 6006, diff --git a/lib/src/lib/tool-announce.ts b/lib/src/lib/tool-announce.ts index 39866c101..9d550fb9a 100644 --- a/lib/src/lib/tool-announce.ts +++ b/lib/src/lib/tool-announce.ts @@ -23,6 +23,8 @@ const KEY_ELEMENTS_LIMIT = 8; export type ToolAnnounce = { /** Which of the tool's ports to frame. Null when unstated. */ port: number | null; + /** Same-origin path/query for the discovered port; never an authority. */ + path?: string; /** Title candidate, feeding the existing channel in terminal-state.md. */ name: string | null; /** Re-key request. Never dedupes — a runtime re-key only re-labels its own @@ -81,6 +83,7 @@ export function parseToolAnnounce(content: string): ToolAnnounce | null { const announce: ToolAnnounce = { port: readPort(record.port), + ...(validToolServePath(record.path) ? { path: record.path } : {}), name: sanitize(record.name, NAME_LIMIT), key: readKey(record.key), dehydrate: record.dehydrate === true, @@ -90,3 +93,9 @@ export function parseToolAnnounce(content: string): ToolAnnounce | null { if (announce.port === null && announce.name === null && announce.key === null) return null; return announce; } + +/** Reject authority changes rather than trying to repair process output. */ +export function validToolServePath(value: unknown): value is string { + return typeof value === 'string' && value.length <= 2048 && value.startsWith('/') + && !value.startsWith('//') && !/[\\\u0000-\u0020\u007f]/.test(value); +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 990719cb7..8a48ae94c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -64,6 +64,9 @@ importers: specifier: workspace:* version: link:../dor-lib-common devDependencies: + '@types/node': + specifier: ^24.13.4 + version: 24.13.4 esbuild: specifier: ^0.28.0 version: 0.28.2 diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index 16f9edbdb..7215d70c8 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -7,7 +7,7 @@ "docs/specs/deploy.md": 1900, "docs/specs/dor-browser.md": 4600, "docs/specs/dor-cli.md": 6000, - "docs/specs/dor-tool.md": 3050, + "docs/specs/dor-tool.md": 3300, "docs/specs/glossary.md": 3000, "docs/specs/layout.md": 8800, "docs/specs/mobile-terminal-ui.md": 1950, @@ -19,7 +19,7 @@ "docs/specs/remote-security-model.md": 4800, "docs/specs/security-audit.md": 1750, "docs/specs/security-ci.md": 2500, - "docs/specs/security-local.md": 2900, + "docs/specs/security-local.md": 3000, "docs/specs/security-remote.md": 5850, "docs/specs/security-supply-chain.md": 1200, "docs/specs/security.md": 1900, From 2b9a880722a2951f4a7ed775a5934f4d835988c2 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 15 Sep 2026 16:55:33 -0700 Subject: [PATCH 02/15] Resolve shared CLI modules in standalone host bundles --- standalone/scripts/build-sidecar-proxy.mjs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/standalone/scripts/build-sidecar-proxy.mjs b/standalone/scripts/build-sidecar-proxy.mjs index 6e08dae0d..318c582ab 100644 --- a/standalone/scripts/build-sidecar-proxy.mjs +++ b/standalone/scripts/build-sidecar-proxy.mjs @@ -112,6 +112,9 @@ for (const { entry, out, define, assertBaked, external } of bundles) { outfile, bundle: true, platform: 'node', // node builtins (http/net/fs/child_process) stay external + // Match the frontend and VS Code host's dor/* source mapping. Host modules + // also share the CLI's pure file-format registry. + alias: { dor: path.resolve(here, '../../dor/src') }, format: 'cjs', target: 'node24', logLevel: 'warning', From 9ce15a6c958e0840400406fbaae0d79466b97b95 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 15 Sep 2026 18:01:04 -0700 Subject: [PATCH 03/15] Simplify the built-in file viewer and its lib plumbing Viewer (dor/src/file-viewer.ts): one `outsideRoot` predicate for both containment checks, a `ViewerLimitError` class instead of matching error text, an explicit descriptor hand-off in place of the `let file`/`finally` sentinel, `register` returning the resource so the main entry is not re-derived from the route formula, a stat-only 8 MiB precheck instead of reading and discarding the file, route dedupe before `realpath`, per-chunk buffers instead of alloc-plus-copy, and `node:http` / `node:fs/promises` loaded on demand so they no longer load on every `dor` invocation. CLI: `dor __view-file` dispatches inside `runCli` beside the other pre-stricli route, so `dor.ts` returns to its original shape; the `ProcessLike` and `URL` shims that `@types/node` made redundant are gone. `BUILTIN_FILE_TOOL` / `VIEW_FILE_ARGV` are exported from the pure format module and shared by `tool-open.ts`, `tool-registry.ts`, and `cli.ts`. lib: a named `ToolKeyScope` + `isToolKeyScope` replaces six hand-spelled `'user' | 'builtin'` unions; `announcementKey` builds the applied (port, path) memory in one place; the path re-validation in `use-tool-serving` documents why it is not redundant with the parser. Docs: merge the two `Source of truth:` paragraphs in Opening local files; point the viewer guard and the loopback lint header at the shared rule. Co-Authored-By: Claude Fable 5.1 --- docs/specs/dor-tool.md | 4 +- dor/src/cli.ts | 8 ++ dor/src/commands/open-target.ts | 4 - dor/src/dor.ts | 29 +------ dor/src/file-viewer-format.ts | 6 ++ dor/src/file-viewer-loopback-guard.ts | 5 +- dor/src/file-viewer.ts | 95 ++++++++++++--------- lib/src/components/wall/browser-surface.ts | 5 +- lib/src/components/wall/use-dor-control.ts | 3 +- lib/src/components/wall/use-tool-serving.ts | 20 +++-- lib/src/host/tool-open.ts | 6 +- lib/src/host/tool-registry.ts | 5 +- lib/src/lib/platform/tool-types.ts | 8 +- lib/src/lib/session-save.ts | 3 +- lib/src/lib/session-types.ts | 5 +- scripts/loopback-lint.mjs | 3 +- 16 files changed, 115 insertions(+), 94 deletions(-) diff --git a/docs/specs/dor-tool.md b/docs/specs/dor-tool.md index a5416d3e5..c72e0da7a 100644 --- a/docs/specs/dor-tool.md +++ b/docs/specs/dor-tool.md @@ -149,9 +149,7 @@ Source of truth: `toolCommand` in `dor/src/commands/tool.ts`; `dor/test/snapshot **Must retain the viewer's opened file descriptors until the Tool exits.** Refresh reads those files again, but atomic replacements and changes to the dependency graph require restarting the viewer. Cold restore runs the saved file command with a fresh URL capability; Workspace movement keeps the live binding. The listener's authority is `docs/specs/security-local.md` → Local-file viewer. -Source of truth: `openCommand` in `dor/src/commands/open.ts`; `resolveOpenTool` in `lib/src/host/tool-open.ts`; `parseToolFile` in `lib/src/host/tool-registry.ts`; `surface.tool` in `lib/src/components/wall/use-dor-control.ts`. Tests: `lib/src/host/tool-open.test.ts`, `dor/test/cli-output.test.mjs`, `lib/src/components/Wall.test.tsx`. - -Source of truth: `fileViewerFormat` in `dor/src/file-viewer-format.ts`; `startFileViewer` / `runFileViewer` in `dor/src/file-viewer.ts`; `dor/test/file-viewer.test.mjs`. +Source of truth: `openCommand` in `dor/src/commands/open.ts`; `resolveOpenTool` in `lib/src/host/tool-open.ts`; `parseToolFile` in `lib/src/host/tool-registry.ts`; `surface.tool` in `lib/src/components/wall/use-dor-control.ts`; `fileViewerFormat` in `dor/src/file-viewer-format.ts`; `startFileViewer` / `runFileViewer` in `dor/src/file-viewer.ts`. Tests: `lib/src/host/tool-open.test.ts`, `dor/test/cli-output.test.mjs`, `lib/src/components/Wall.test.tsx`, `dor/test/file-viewer.test.mjs`. ## Take-over diff --git a/dor/src/cli.ts b/dor/src/cli.ts index 1d4646578..ed15a123b 100644 --- a/dor/src/cli.ts +++ b/dor/src/cli.ts @@ -22,6 +22,8 @@ import { openCommand } from './commands/open.js'; import { versionCommand } from './commands/version.js'; import { workspaceCommand } from './commands/workspace.js'; import { errorLine, errorMessage, fail } from './commands/shared.js'; +import { VIEW_FILE_ARGV } from './file-viewer-format.js'; +import { runFileViewer } from './file-viewer.js'; import type { CliEnv, CliOptions, @@ -194,6 +196,12 @@ export async function runCli(rawArgv: string[], options: CliOptions = {}): Promi if (argv[0] === 'agent-browser' && !isAgentBrowserHelpInvocation(argv)) { return runAgentBrowserCli(argv.slice(1), options); } + // `dor __view-file ` is the built-in viewer's private entry + // (docs/specs/dor-tool.md -> Opening local files). Its server outlives this + // call; the announcement is the only output. + if (argv[0] === VIEW_FILE_ARGV && argv.length === 2) { + return { stdout: await runFileViewer(argv[1]), stderr: '', exitCode: 0 }; + } const helpTarget = getHelpTarget(argv); const [commandName, ...args] = rewriteHelpArgv(argv); diff --git a/dor/src/commands/open-target.ts b/dor/src/commands/open-target.ts index 08e240747..333002559 100644 --- a/dor/src/commands/open-target.ts +++ b/dor/src/commands/open-target.ts @@ -4,10 +4,6 @@ import { errorMessage, workspaceParam } from './shared.js'; import type { ControlClient, ParseResult } from './types.js'; -declare const URL: { - new(input: string): { href: string; protocol: string }; -}; - // A bare `:port` (optionally trailed by a path/query/hash) — localhost sugar. const BARE_PORT = /^:\d{1,5}(?:[/?#].*)?$/; // A schemeless host:port (optional path): `localhost:5173`, `box.ts.net:3000`, diff --git a/dor/src/dor.ts b/dor/src/dor.ts index 3e7190609..d0bae24cb 100644 --- a/dor/src/dor.ts +++ b/dor/src/dor.ts @@ -1,29 +1,8 @@ #!/usr/bin/env node import { runCli } from './cli.js'; -import { runFileViewer } from './file-viewer.js'; -type ProcessLike = { - argv: string[]; - env: Record; - exitCode?: number; - stdout: { write(chunk: string): void }; - stderr: { write(chunk: string): void }; - stdin: { - setEncoding?(encoding: string): void; - on(event: 'data', listener: (chunk: string) => void): void; - on(event: 'end', listener: () => void): void; - on(event: 'error', listener: (error: Error) => void): void; - resume?(): void; - }; -}; - -declare const process: ProcessLike; - -const execution = process.argv[2] === '__view-file' && process.argv.length === 4 - ? runFileViewer(process.argv[3]).then(() => ({ stdout: '', stderr: '', exitCode: 0 })) - : runCli(process.argv.slice(2), { env: process.env, readStdin }); -execution.then( +runCli(process.argv.slice(2), { env: process.env, readStdin }).then( (result) => { process.stdout.write(result.stdout); process.stderr.write(result.stderr); @@ -38,10 +17,10 @@ execution.then( function readStdin(): Promise { return new Promise((resolve, reject) => { const chunks: string[] = []; - process.stdin.setEncoding?.('utf8'); - process.stdin.on('data', (chunk) => chunks.push(chunk)); + process.stdin.setEncoding('utf8'); + process.stdin.on('data', (chunk) => chunks.push(String(chunk))); process.stdin.on('end', () => resolve(chunks.join(''))); process.stdin.on('error', reject); - process.stdin.resume?.(); + process.stdin.resume(); }); } diff --git a/dor/src/file-viewer-format.ts b/dor/src/file-viewer-format.ts index aebec0559..ac0457828 100644 --- a/dor/src/file-viewer-format.ts +++ b/dor/src/file-viewer-format.ts @@ -11,6 +11,12 @@ const MIME: Record = { const TEXT = new Set(['txt', 'md', 'mdx', 'log', 'csv', 'tsv', 'json', 'jsonl', 'yaml', 'yml', 'toml', 'xml', 'css', 'js', 'mjs', 'cjs', 'ts', 'tsx', 'jsx', 'py', 'rs', 'go', 'java', 'c', 'h', 'cpp', 'sh', 'ps1', 'sql', 'ini', 'conf']); +/** The handler name an `open` rule or `--tool` uses to select the viewer, and + * the private `dor` argv verb that runs it. The lib host's `resolveOpenTool` + * shares both through the `dor/*` alias; this module stays free of Node APIs. */ +export const BUILTIN_FILE_TOOL = 'builtin:file'; +export const VIEW_FILE_ARGV = '__view-file'; + export function fileViewerFormat(path: string): { mime: string; text: boolean } | null { const name = path.replace(/\\/g, '/').split('/').pop()!.toLowerCase(); const ext = name.includes('.') ? name.split('.').pop()! : ''; diff --git a/dor/src/file-viewer-loopback-guard.ts b/dor/src/file-viewer-loopback-guard.ts index 85f86a941..fd857ba33 100644 --- a/dor/src/file-viewer-loopback-guard.ts +++ b/dor/src/file-viewer-loopback-guard.ts @@ -2,7 +2,10 @@ import type { IncomingMessage } from 'node:http'; /** This listener owns its URLs: a per-process 256-bit capability authorizes * each read. Host and Origin checks also reject rebinding and foreign scripts. - * See docs/specs/security-local.md -> Local-file viewer. */ + * See docs/specs/security-local.md -> Local-file viewer. The Host/Origin rule is + * the one `lib/src/host/loopback-guard.ts` states for every loopback listener; + * `dor` cannot import `lib`, so it is restated here (exact-match, so a + * mixed-case `Host` is refused rather than folded). */ export function allowsFileViewerRequest(req: IncomingMessage, port: number, prefix: string): boolean { const origins = [`http://localhost:${port}`, `http://127.0.0.1:${port}`]; return (req.method === 'GET' || req.method === 'HEAD') diff --git a/dor/src/file-viewer.ts b/dor/src/file-viewer.ts index 9a2bc3c35..bd6e50070 100644 --- a/dor/src/file-viewer.ts +++ b/dor/src/file-viewer.ts @@ -1,20 +1,29 @@ import { randomBytes } from 'node:crypto'; import { constants } from 'node:fs'; -import { open, realpath, type FileHandle } from 'node:fs/promises'; -import { createServer, type ServerResponse } from 'node:http'; +import type { FileHandle } from 'node:fs/promises'; +import type { ServerResponse } from 'node:http'; import { basename, dirname, isAbsolute, relative, resolve, sep } from 'node:path'; import { fileViewerFormat } from './file-viewer-format.js'; import { allowsFileViewerRequest } from './file-viewer-loopback-guard.js'; const TEXT_LIMIT = 8 * 1024 * 1024; const ASSET_LIMIT = 256; -type Resource = { file: FileHandle; mime: string; text: boolean; path: string }; -const escapeHtml = (s: string) => s.replace(/[&<>"']/g, c => ({ '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' })[c]!); +const CHUNK = 64 * 1024; +type Resource = { file: FileHandle; mime: string }; +/** A bound on the grant itself: fatal even when reached through an optional asset. */ +class ViewerLimitError extends Error {} +const HTML_ESCAPES: Record = { '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' }; +const escapeHtml = (s: string) => s.replace(/[&<>"']/g, c => HTML_ESCAPES[c]!); + +async function textSize(file: FileHandle): Promise { + const { size } = await file.stat(); + if (size > TEXT_LIMIT) throw new ViewerLimitError('text preview exceeds 8 MiB; configure a Tool for this file'); + return size; +} async function readText(file: FileHandle): Promise { - const size = (await file.stat()).size; - if (size > TEXT_LIMIT) throw new Error('text preview exceeds 8 MiB; configure a Tool for this file'); - const bytes = Buffer.alloc(size + 1); + const size = await textSize(file); + const bytes = Buffer.allocUnsafe(size + 1); let offset = 0; while (offset < bytes.length) { const { bytesRead } = await file.read(bytes, offset, bytes.length - offset, offset); @@ -50,57 +59,59 @@ function finish(res: ServerResponse, status: number, message = ''): void { /** One Tool process owns one file grant and its file descriptors. Restarting * creates a fresh capability; only the file argument is persisted by Dormouse. */ export async function startFileViewer(input: string): Promise<{ port: number; path: string; close(): Promise }> { + // Loaded on demand: this module is bundled into every `dor` invocation, and + // these two builtins cost more to load than everything else the CLI touches. + const [{ open, realpath }, { createServer }] = await Promise.all([import('node:fs/promises'), import('node:http')]); const target = await realpath(input); const format = fileViewerFormat(target); if (!format) throw new Error('unsupported file format; configure a user Tool association'); const root = dirname(target); const prefix = `/${randomBytes(32).toString('hex')}/`; const resources = new Map(); - const paths = new Set(); + const scanned = new Set(); const closeFiles = async () => { await Promise.all([...resources.values()].map(r => r.file.close())); }; + const outsideRoot = (path: string) => { + const rel = relative(root, path); + return isAbsolute(rel) || rel === '..' || rel.startsWith(`..${sep}`); + }; - async function register(path: string, required: boolean): Promise { - let file: FileHandle | undefined; + async function register(path: string, required: boolean): Promise { try { - const canonical = await realpath(path); - const rel = relative(root, canonical); - if (isAbsolute(rel) || rel === '..' || rel.startsWith(`..${sep}`)) return; const route = `file/${relative(root, path).split(sep).join('/')}`; - if (resources.has(route)) return; - if (resources.size >= ASSET_LIMIT) throw new Error('local preview exceeds 256 referenced files'); + if (resources.has(route)) return resources.get(route); + const canonical = await realpath(path); + if (outsideRoot(canonical)) return; + if (resources.size >= ASSET_LIMIT) throw new ViewerLimitError('local preview exceeds 256 referenced files'); const type = fileViewerFormat(canonical); if (!type) return; - file = await open(canonical, constants.O_RDONLY | (constants.O_NOFOLLOW ?? 0) | (constants.O_NONBLOCK ?? 0)); - if (!(await file.stat()).isFile()) throw new Error('not a regular file'); - const resource = { file, ...type, path: canonical }; - resources.set(route, resource); - file = undefined; // grant owns it now - if (paths.has(canonical)) return; - paths.add(canonical); + const file = await open(canonical, constants.O_RDONLY | (constants.O_NOFOLLOW ?? 0) | (constants.O_NONBLOCK ?? 0)); + try { if (!(await file.stat()).isFile()) throw new Error('not a regular file'); } + catch (error) { await file.close(); throw error; } + const resource = { file, mime: type.mime }; + resources.set(route, resource); // the grant owns the descriptor from here const html = type.mime.startsWith('text/html'); - if (html || type.mime.startsWith('text/css')) { - const contents = await readText(resource.file); - for (const ref of references(contents, html)) { + if ((html || type.mime.startsWith('text/css')) && !scanned.has(canonical)) { + scanned.add(canonical); + for (const ref of references(await readText(file), html)) { if (!ref || ref.startsWith('/') || ref.startsWith('#') || /^[a-z][a-z\d+.-]*:/i.test(ref) || ref.includes('\\')) continue; let local: string; try { local = decodeURIComponent(ref.split(/[?#]/, 1)[0]); } catch { continue; } const asset = resolve(dirname(path), local); - const inside = relative(root, asset); - if (isAbsolute(inside) || inside === '..' || inside.startsWith(`..${sep}`)) continue; + if (outsideRoot(asset)) continue; await register(asset, false); } } + return resource; } catch (error) { - if (required || (error instanceof Error && /exceeds/.test(error.message))) throw error; - // A missing/broken relative asset stays unavailable; never broaden the grant. - } finally { await file?.close(); } + if (required || error instanceof ViewerLimitError) throw error; + return; // A missing/broken relative asset stays unavailable; never broaden the grant. + } } try { - await register(target, true); - const main = resources.get(`file/${basename(target)}`)!; + const main = await register(target, true); if (!main) throw new Error('not a supported regular file'); - if (format.text) await readText(main.file); // fail oversized text before announcing + if (format.text) await textSize(main.file); // fail oversized text before announcing let port = 0; const server = createServer((req, res) => { res.setHeader('Cache-Control', 'no-store'); @@ -138,15 +149,15 @@ export async function startFileViewer(input: string): Promise<{ port: number; pa } res.writeHead(range ? 206 : 200, { 'Content-Type': resource.mime, 'Content-Length': Math.max(0, end - start + 1), 'Accept-Ranges': 'bytes' }); if (req.method === 'HEAD' || size === 0) { res.end(); return; } - // Positional reads let simultaneous range requests share a descriptor. - // A disconnected response must not destroy the grant's shared handle. - const buffer = Buffer.alloc(64 * 1024); + // Positional reads let simultaneous range requests share a descriptor, + // and a disconnected response must not destroy the grant's shared handle. + // Each chunk is a fresh buffer because res.write queues it without copying. for (let offset = start; offset <= end && !res.destroyed;) { - const { bytesRead } = await resource.file.read(buffer, 0, Math.min(buffer.length, end - offset + 1), offset); + const chunk = Buffer.allocUnsafe(Math.min(CHUNK, end - offset + 1)); + const { bytesRead } = await resource.file.read(chunk, 0, chunk.length, offset); if (!bytesRead) { res.destroy(); return; } offset += bytesRead; - const chunk = Buffer.from(buffer.subarray(0, bytesRead)); - if (!res.write(chunk) && !res.destroyed) await new Promise(done => { + if (!res.write(bytesRead === chunk.length ? chunk : chunk.subarray(0, bytesRead)) && !res.destroyed) await new Promise(done => { const complete = () => { res.off('drain', complete); res.off('close', complete); done(); }; res.once('drain', complete); res.once('close', complete); @@ -167,10 +178,12 @@ export async function startFileViewer(input: string): Promise<{ port: number; pa } catch (error) { await closeFiles(); throw error; } } -export async function runFileViewer(file: string): Promise { +/** The `dor __view-file ` entry: starts the viewer, which outlives the + * call, and returns the OSC 367 announcement for the caller to print. */ +export async function runFileViewer(file: string): Promise { const viewer = await startFileViewer(file); const stop = () => { void viewer.close().then(() => { process.exitCode = 0; }); }; process.once('SIGINT', stop); process.once('SIGTERM', stop); - process.stdout.write(`\x1b]367;serve;${JSON.stringify({ port: viewer.port, path: viewer.path, v: 1 })}\x07`); + return `\x1b]367;serve;${JSON.stringify({ port: viewer.port, path: viewer.path, v: 1 })}\x07`; } diff --git a/lib/src/components/wall/browser-surface.ts b/lib/src/components/wall/browser-surface.ts index 8a71dd578..5e40bf37f 100644 --- a/lib/src/components/wall/browser-surface.ts +++ b/lib/src/components/wall/browser-surface.ts @@ -10,6 +10,7 @@ import { type RenderMode, } from './agent-browser-screen'; import type { SurfaceKind } from 'dor/commands/types'; +import { isToolKeyScope, type ToolKeyScope } from '../../lib/platform/tool-types'; type BrowserParamsLike = { surfaceType?: unknown; @@ -154,9 +155,9 @@ export function namespacedToolKey( * a project `dormouse.yml`. Project and user Tools are separate reuse scopes * (`docs/specs/dor-tool.md` -> Declaring tools), so dedupe compares this * alongside the key. */ -export function toolScopeFromParams(params: unknown): 'user' | 'builtin' | undefined { +export function toolScopeFromParams(params: unknown): ToolKeyScope | undefined { const scope = asParams(params).toolScope; - return scope === 'user' || scope === 'builtin' ? scope : undefined; + return isToolKeyScope(scope) ? scope : undefined; } /** Whether params describe a plain browser surface (vs a terminal): the unified diff --git a/lib/src/components/wall/use-dor-control.ts b/lib/src/components/wall/use-dor-control.ts index db300e357..76764eff4 100644 --- a/lib/src/components/wall/use-dor-control.ts +++ b/lib/src/components/wall/use-dor-control.ts @@ -16,6 +16,7 @@ import { hasBrowser, hasTerminal } from 'dor/commands/types'; import { MAX_AWAIT_TIMEOUT_MS } from '../../lib/alert-manager'; import { TOOLS_FLAG_KEY, isToolsEnabled } from '../../lib/feature-flags'; import type { OpenPort } from '../../lib/platform/types'; +import type { ToolKeyScope } from '../../lib/platform/tool-types'; import { buildShellCommandForKind, shellCommandKind } from 'dor/commands/shell-quote'; import { UNNAMED_PANEL_TITLE, @@ -885,7 +886,7 @@ export function useDorControl({ } let command: string; let key: string[] | null = null; - let toolScope: 'user' | 'builtin' | undefined; + let toolScope: ToolKeyScope | undefined; const toolArgs = stringArrayParam(params.args) ?? []; let warnings: string[] = []; let render: 'iframe' | 'ab-screencast' = 'iframe'; diff --git a/lib/src/components/wall/use-tool-serving.ts b/lib/src/components/wall/use-tool-serving.ts index 2f4dc7217..6498fce13 100644 --- a/lib/src/components/wall/use-tool-serving.ts +++ b/lib/src/components/wall/use-tool-serving.ts @@ -54,6 +54,10 @@ function toolLeaves(lath: LathWallEngine, doors: DooredItem[]): ToolLeaf[] { return leaves; } +/** One string per applied (port, path) announcement. A valid path starts with + * `/`, so the port cannot bleed into it; an absent path is the origin root. */ +const announcementKey = (port: number, path: unknown) => `${port}${typeof path === 'string' ? path : '/'}`; + export function useToolServing({ lath, doorsRef, @@ -67,9 +71,9 @@ export function useToolServing({ // A ref, not state: it drives no render, and a leaf's entry is dropped when // its command exits so a re-run settles again from scratch. const seenPorts = useRef>(new Map()); - // The announced port/path last applied to each leaf. A changed announcement may - // re-point a live browser, but the same announcement must not keep undoing - // URL-bar navigation just because params.url no longer names that port. + // The announcement (as `announcementKey`) last applied to each leaf. A changed + // announcement may re-point a live browser, but the same announcement must not + // keep undoing URL-bar navigation just because params.url no longer names it. const appliedAnnouncements = useRef>(new Map()); useEffect(() => { @@ -157,14 +161,16 @@ export function useToolServing({ // treating a mismatch with params.url as a change would undo URL-bar // navigation every poll after the user left the announced origin. const announcedPort = announce?.port ?? null; + // Re-checked here, not only in parseToolAnnounce: a Workspace transfer + // records the moved Session's announcement as data, and `new URL` below + // would honor `//host` as an authority. const announcedPath = validToolServePath(announce?.path) ? announce.path : '/'; - const announcementKey = JSON.stringify([announcedPort, announcedPath]); if (announcedPort === null) appliedAnnouncements.current.delete(leaf.id); if (!appliedAnnouncements.current.has(leaf.id) && typeof leaf.params?.toolAnnouncedPort === 'number') { - appliedAnnouncements.current.set(leaf.id, JSON.stringify([leaf.params.toolAnnouncedPort, leaf.params.toolAnnouncedPath ?? '/'])); + appliedAnnouncements.current.set(leaf.id, announcementKey(leaf.params.toolAnnouncedPort, leaf.params.toolAnnouncedPath)); } const announcementChanged = announcedPort !== null - && appliedAnnouncements.current.get(leaf.id) !== announcementKey; + && appliedAnnouncements.current.get(leaf.id) !== announcementKey(announcedPort, announcedPath); if (!running) continue; if ((hasUrl || hasConflict) && !announcementChanged) continue; @@ -184,7 +190,7 @@ export function useToolServing({ // announced port that nothing bound frames nothing. entry = entries.find((candidate) => candidate.port === announce.port); if (!entry) continue; - appliedAnnouncements.current.set(leaf.id, announcementKey); + appliedAnnouncements.current.set(leaf.id, announcementKey(announce.port, announcedPath)); } else if (leaf.params?.toolPort !== 'auto') { // `announced`: never guess. No announcement, no browser. continue; diff --git a/lib/src/host/tool-open.ts b/lib/src/host/tool-open.ts index 12a2c18e7..9835231d4 100644 --- a/lib/src/host/tool-open.ts +++ b/lib/src/host/tool-open.ts @@ -1,7 +1,7 @@ import { basename, relative, sep } from 'node:path'; import picomatch from 'picomatch'; import type { ToolLookupResult } from '../lib/platform/tool-types'; -import { fileViewerFormat } from 'dor/file-viewer-format'; +import { BUILTIN_FILE_TOOL, VIEW_FILE_ARGV, fileViewerFormat } from 'dor/file-viewer-format'; import { resolveLocalToolTarget } from './tool-input'; import { readUserToolFile, resolveUserTool } from './tool-user-config'; @@ -20,9 +20,9 @@ export async function resolveOpenTool( return rule.match.includes('/') ? matches(relativePath) || matches(canonicalPath) : matches(basename(target)); })?.tool; const entry = name && file?.tools.get(name); - if ((!name || name === 'builtin:file') && fileViewerFormat(target)) { + if ((!name || name === BUILTIN_FILE_TOOL) && fileViewerFormat(target)) { return { status: 'ok', projectRoot: request.cwd, path: '', name: 'file', scope: 'builtin', - run: ['dor', '__view-file', target], key: [target], render: 'iframe', port: 'announced', warnings: [] }; + run: ['dor', VIEW_FILE_ARGV, target], key: [target], render: 'iframe', port: 'announced', warnings: [] }; } if (!file || !entry) return { status: 'error', message: request.tool ? `no user Tool '${request.tool}' in ${path}` diff --git a/lib/src/host/tool-registry.ts b/lib/src/host/tool-registry.ts index 2daca09d2..df7a898b4 100644 --- a/lib/src/host/tool-registry.ts +++ b/lib/src/host/tool-registry.ts @@ -7,6 +7,7 @@ * bundle. */ import { parse as parseYaml } from 'yaml'; +import { BUILTIN_FILE_TOOL } from 'dor/file-viewer-format'; /** Where a tool file came from. `$PROJECT_ROOT` exists only for `repo`. */ export type ToolScope = 'repo' | 'user'; @@ -239,7 +240,7 @@ function parseOpenRules(node: unknown, tools: ReadonlyMap, pa if (!Array.isArray(node)) throw new ToolFileError(`${path}: 'open' must be an ordered list`); return node.map((rule: unknown) => { const entry = isRecord(rule) && typeof rule.tool === 'string' ? tools.get(rule.tool) : undefined; - const builtin = isRecord(rule) && rule.tool === 'builtin:file'; + const builtin = isRecord(rule) && rule.tool === BUILTIN_FILE_TOOL; if (!isRecord(rule) || (!builtin && !entry) || typeof rule.match !== 'string' || !rule.match || Object.keys(rule).some(key => key !== 'match' && key !== 'tool')) { throw new ToolFileError(`${path}: each open rule needs a match pattern and a tool defined in this user file`); @@ -247,7 +248,7 @@ function parseOpenRules(node: unknown, tools: ReadonlyMap, pa if (entry && typeof entry.run === 'string') { throw new ToolFileError(`${path}: open rule for '${entry.name}' needs an argument-list run to receive the file`); } - return { match: rule.match, tool: builtin ? 'builtin:file' : entry!.name }; + return { match: rule.match, tool: builtin ? BUILTIN_FILE_TOOL : entry!.name }; }); } diff --git a/lib/src/lib/platform/tool-types.ts b/lib/src/lib/platform/tool-types.ts index f94c33606..e60767616 100644 --- a/lib/src/lib/platform/tool-types.ts +++ b/lib/src/lib/platform/tool-types.ts @@ -11,6 +11,12 @@ export type ToolHostRequest = | { op: 'lookup'; name: string; cwd: string; args?: string[]; global?: boolean } | { op: 'trust'; kind: 'upstream' | 'folder'; projectRoot: string }; +/** Which authority declared a Tool, namespacing its dedupe key and persisted + * `scope`. Project Tools carry none. `docs/specs/dor-tool.md` -> Identity and + * dedupe. */ +export type ToolKeyScope = 'user' | 'builtin'; +export const isToolKeyScope = (value: unknown): value is ToolKeyScope => value === 'user' || value === 'builtin'; + /** Result of resolving a tool name. `ok` carries the rendered dedupe key: the * host owns `$PROJECT_ROOT`, so the webview never sees a template. */ export type ToolLookupResult = @@ -34,7 +40,7 @@ export type ToolLookupResult = run: string | readonly string[]; /** Renderer for the tool's browser once it serves; 'iframe' by default. */ render: 'iframe' | 'ab-screencast'; - scope?: 'user' | 'builtin'; + scope?: ToolKeyScope; /** How to pick the port to frame absent an announcement; 'announced' by * default, meaning nothing is framed without OSC 367. */ port: 'announced' | 'auto'; diff --git a/lib/src/lib/session-save.ts b/lib/src/lib/session-save.ts index d8870c4c4..4737492ef 100644 --- a/lib/src/lib/session-save.ts +++ b/lib/src/lib/session-save.ts @@ -1,5 +1,6 @@ import { normalizeAlertDeliveryOverrides, type AlertDeliveryOverrides } from './alert-delivery-model'; import type { PlatformAdapter } from './platform/types'; +import { isToolKeyScope } from './platform/tool-types'; import { browserPersistedPane, readPersistedSession, toPersistedAlertState, type PersistedDoor, type PersistedPane, type PersistedSession, type PersistedSurfaceRefs, type PersistedToolMetadata, type PersistedSurfaceType } from './session-types'; import { getActivity, getLivePersistedAlertState, getTerminalPaneState, isUntouched } from './terminal-registry'; import { UNNAMED_PANEL_TITLE } from './terminal-state'; @@ -185,7 +186,7 @@ function toolMetadataFromParams(params: Record | undefined): Pe const key = Array.isArray(params.toolKey) && params.toolKey.every((part) => typeof part === 'string') ? params.toolKey as string[] : undefined; - return { ...(name ? { name } : {}), ...(params.toolScope === 'user' || params.toolScope === 'builtin' ? { scope: params.toolScope } : {}), render, port, ...(key ? { key } : {}) }; + return { ...(name ? { name } : {}), ...(isToolKeyScope(params.toolScope) ? { scope: params.toolScope } : {}), render, port, ...(key ? { key } : {}) }; } function persistedVisiblePaneTitle(title: string): string { diff --git a/lib/src/lib/session-types.ts b/lib/src/lib/session-types.ts index 73000829d..76f3d2a64 100644 --- a/lib/src/lib/session-types.ts +++ b/lib/src/lib/session-types.ts @@ -1,5 +1,6 @@ import { normalizeAlertDeliveryOverrides, type AlertDeliveryOverrides } from './alert-delivery-model'; import { isRecord } from './is-record'; +import { isToolKeyScope, type ToolKeyScope } from './platform/tool-types'; import type { SessionStatus } from './alert-manager'; import { ACTIVITY_NOTIFICATION_SOURCES, type ActivityNotification, type TodoState } from './alert-manager'; @@ -17,7 +18,7 @@ export type PersistedSurfaceType = 'terminal' | 'browser' | 'tool'; * is respawned. Derived browser state (URL/session/port conflict) never enters * this projection. */ export interface PersistedToolMetadata { - scope?: 'user' | 'builtin'; + scope?: ToolKeyScope; name?: string; render: 'iframe' | 'ab-screencast'; port: 'announced' | 'auto'; @@ -173,7 +174,7 @@ function isPersistedToolMetadataShape(value: unknown): boolean { if (!isRecord(value)) return false; return ( (value.name === undefined || typeof value.name === 'string') && - (value.scope === undefined || value.scope === 'user' || value.scope === 'builtin') && + (value.scope === undefined || isToolKeyScope(value.scope)) && (value.render === 'iframe' || value.render === 'ab-screencast') && (value.port === 'announced' || value.port === 'auto') && (value.key === undefined || (Array.isArray(value.key) && value.key.every((part) => typeof part === 'string'))) diff --git a/scripts/loopback-lint.mjs b/scripts/loopback-lint.mjs index 85b048dfd..8bb63ba46 100644 --- a/scripts/loopback-lint.mjs +++ b/scripts/loopback-lint.mjs @@ -16,7 +16,8 @@ * bind it recognizes. Test files and this lint's own fixtures are reported * separately; every other file that binds a TCP listener to loopback must * reference one of the guard modules — `lib/src/host/loopback-guard.ts` for - * shipped code, `standalone/scripts/dev-host-guard.mjs` for the dev harness — + * shipped code, `dor/src/file-viewer-loopback-guard.ts` for the CLI, + * `standalone/scripts/dev-host-guard.mjs` for the dev harness — * or sit on ALLOWED below with a stated reason. * * `scripts/loopback-lint-selftest.mjs` proves each bind form is load-bearing by From 3166f6ec1b89576a984a6672e7fa78e5eec4c3e7 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 15 Sep 2026 17:52:46 -0700 Subject: [PATCH 04/15] Preserve file viewer CSP through the iframe proxy --- docs/specs/dor-browser.md | 9 ++- docs/specs/dor-browser.rationale.md | 2 + docs/specs/dor-tool.md | 4 +- docs/specs/security-local.md | 8 +- docs/specs/security-local.rationale.md | 4 + dor/src/file-viewer-loopback-guard.ts | 11 ++- dor/src/file-viewer.ts | 6 +- dor/test/file-viewer.test.mjs | 23 ++++++ lib/src/host/file-viewer-proxy.test.ts | 98 +++++++++++++++++++++++ lib/src/host/iframe-proxy-rewrite.test.ts | 7 ++ lib/src/host/iframe-proxy-rewrite.ts | 16 ++-- lib/src/host/iframe-proxy.test.ts | 29 +++++++ lib/src/host/iframe-proxy.ts | 18 ++++- lib/src/host/tool-registry.test.ts | 6 ++ lib/src/host/tool-registry.ts | 1 + scripts/spec-word-budgets.json | 4 +- 16 files changed, 222 insertions(+), 24 deletions(-) create mode 100644 lib/src/host/file-viewer-proxy.test.ts diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index adda68c00..e32cadee6 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -455,13 +455,16 @@ Header rewriting: | request | `Accept-Encoding` | deleted, so HTML comes back identity for rewriting | | request | `Cookie` | dropped, including WebSocket handshakes | | response | `Set-Cookie` | dropped, including successful and refused WebSocket handshakes | -| response | `X-Frame-Options`, `Content-Security-Policy`, `Content-Security-Policy-Report-Only` | with validated chain, replaced **whole** by `frame-ancestors 'self' ` (rationale) | +| response | `X-Frame-Options`, CSP headers | with validated chain, replaced by `frame-ancestors 'self' `; opted-in CSP policies remain alongside it (rationale) | +| response | `X-Dormouse-Preserve-CSP: 1` | consumed; preserves upstream CSP headers and meta policies | | 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 | +| response body | `` | removed unless the response opts into CSP preservation | **Must update this table whenever header rewriting changes.** +**Must preserve enforced and report-only CSP verbatim when the upstream response sends `X-Dormouse-Preserve-CSP: 1`.** Add the validated ancestor policy separately, for every MIME type; preserve meta policies during HTML instrumentation. Never infer this opt-in from request headers. Additional upstream restrictions may prevent framing or shim execution. (rationale) + **One dedicated `127.0.0.1:0` server per grant, with no token in the path** — the origin itself is the grant boundary (rationale). Grants have a sliding idle TTL and a hard cap; **a request refused by the `Host` check must not refresh the @@ -556,7 +559,7 @@ Security boundaries: - no user script is injected, - link-local/cloud-metadata ranges are blocked, - 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. + at the cost of the upstream's own XSS policy unless it opts into preservation. **Must replace framing controls with exactly `frame-ancestors 'self' `.** `'self'` permits same-grant nesting; foreign diff --git a/docs/specs/dor-browser.rationale.md b/docs/specs/dor-browser.rationale.md index 78107085f..07ccb6a9c 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 CSP is dropped whole rather than per-directive.** The injected shim is an inline script, so a surviving `script-src` blocks it as surely as `frame-ancestors` blocks the frame; salvaging the remaining directives would leave a frame that looks instrumented and silently is not. +The built-in local-file viewer supplies its own content boundary and permits the inline shim, so removing its CSP would expand active documents' resource access. Its response opts into preservation without new renderer or host-bridge state. The proxy adds an independent ancestor policy: CSP policies intersect, so no directive parser or partial reconstruction can accidentally weaken the upstream. An opt-in upstream with stricter framing or script restrictions keeps those restrictions even if the shim cannot run. + **Why a grant gets its own origin instead of a path token.** A dedicated origin keeps root-relative resources and client-side routers working with no body URL rewriting; a path token would have to survive every link, redirect and `fetch` the page makes. ## Iframe Shim diff --git a/docs/specs/dor-tool.md b/docs/specs/dor-tool.md index c72e0da7a..b0db893e9 100644 --- a/docs/specs/dor-tool.md +++ b/docs/specs/dor-tool.md @@ -143,9 +143,11 @@ Source of truth: `toolCommand` in `dor/src/commands/tool.ts`; `dor/test/snapshot **Must pass the canonical file path as the selected Tool's one input.** Reuse follows [Identity and dedupe](#identity-and-dedupe), `$TARGET` in the key providing per-file identity; placement follows [Take-over](#take-over). +**Must reject declared Tool names beginning with `builtin:` in either configuration scope.** Built-in handler names cannot be shadowed. + **Must use `builtin:file` for supported files when no user rule matches.** An explicit unknown handler or malformed user configuration fails without fallback. Built-in identity is the canonical file path in its own scope, separate from user and project Tools. -**Must run the built-in viewer as a Tool-owned `dor` process**, serving HTML, images, PDF/media, and escaped text/source previews. Markdown is source text; custom viewers may render it. Text previews and HTML/CSS dependency inspection are limited to 8 MiB per file. The grant contains at most 256 files: the opened document and statically referenced relative HTML/CSS assets within its directory tree. Root-relative, external, dynamically discovered, and unreferenced resources are unavailable. +**Must run the built-in viewer as a Tool-owned `dor` process**, serving HTML, images, PDF/media, and escaped text/source previews. Markdown is source text; custom viewers may render it. Text previews and HTML/CSS dependency inspection are limited to 8 MiB per file. Oversized HTML and referenced CSS still stream without dependency inspection. The grant contains at most 256 files: the opened document and statically referenced relative HTML/CSS assets within its directory tree. Never expand the grant through root-relative, external, or dynamic references; requests can read only granted paths. **Must retain the viewer's opened file descriptors until the Tool exits.** Refresh reads those files again, but atomic replacements and changes to the dependency graph require restarting the viewer. Cold restore runs the saved file command with a fresh URL capability; Workspace movement keeps the live binding. The listener's authority is `docs/specs/security-local.md` → Local-file viewer. diff --git a/docs/specs/security-local.md b/docs/specs/security-local.md index 0a5015e88..2e20a7e43 100644 --- a/docs/specs/security-local.md +++ b/docs/specs/security-local.md @@ -132,11 +132,13 @@ Source of truth: the shared rule and predicates — `isLoopbackHost`, `isOwnOrig ### Local-file viewer -**FAIL IF** `dor/src/file-viewer.ts` serves any request without the fresh 256-bit URL capability, its own loopback `Host`, an absent or same-listener `Origin`, and a GET/HEAD method. `allowsFileViewerRequest` in `dor/src/file-viewer-loopback-guard.ts` gates every route. Never grant CORS access to foreign origins, cache responses, or send the capability as a referrer. +**FAIL IF** `dor/src/file-viewer.ts` serves any request without the fresh 256-bit URL capability, its own case-insensitive loopback `Host`, an absent or same-listener `Origin`, and a GET/HEAD method. Compare capability prefixes by SHA-256 then `timingSafeEqual`, including malformed lengths. `allowsFileViewerRequest` in `dor/src/file-viewer-loopback-guard.ts` gates every route. Never grant CORS access to foreign origins, cache responses, or send the capability as a referrer. -**FAIL IF** the local-file viewer exposes directory listings, arbitrary path reads, writes, or a file outside its opened-document grant. Grant construction permits only regular files, rejects symlinks escaping the canonical document directory, bounds static dependency discovery, and retains descriptors so later path replacement cannot widen the grant. Viewer content is restricted by CSP to its own origin plus inline scripts/styles and data images; escaped text previews execute no document markup. +**FAIL IF** the local-file viewer exposes directory listings, arbitrary path reads, writes, or a file outside its opened-document grant. Grant construction permits only regular files, rejects symlinks escaping the canonical document directory, bounds static dependency discovery, and retains descriptors so later path replacement cannot widen the grant. Viewer resource loads are restricted by CSP to its own origin plus inline scripts/styles and data images, including through the iframe proxy; escaped text previews execute no document markup. The viewer opts into the proxy's upstream-policy preservation (`docs/specs/dor-browser.md` → Iframe Renderer). -Source of truth: `startFileViewer` in `dor/src/file-viewer.ts`; `allowsFileViewerRequest` in `dor/src/file-viewer-loopback-guard.ts`; `dor/test/file-viewer.test.mjs`. +**Must not describe the viewer CSP as confining active documents' navigation.** HTML/SVG scripts can navigate their frame to external URLs, including with granted contents; the resource policy is not a no-egress boundary. (rationale) + +Source of truth: `startFileViewer` in `dor/src/file-viewer.ts`; `allowsFileViewerRequest` in `dor/src/file-viewer-loopback-guard.ts`; `sanitizeResponseHeaders` in `lib/src/host/iframe-proxy.ts`. Tests: `dor/test/file-viewer.test.mjs`, `lib/src/host/file-viewer-proxy.test.ts`. ## Persisted state diff --git a/docs/specs/security-local.rationale.md b/docs/specs/security-local.rationale.md index 9fd66ea8f..c8dca10f8 100644 --- a/docs/specs/security-local.rationale.md +++ b/docs/specs/security-local.rationale.md @@ -168,3 +168,7 @@ neither sets a mode, and `restrict_to_owner` is never called on it. The socket path arrives via the sidecar's stderr, which Rust appends verbatim. What the snapshot tests cover. `restrict_to_owner_leaves_one_owner_only_ace` is Windows-only and asserts `SE_DACL_PROTECTED`, one ACE, and the SID. `session_write_tightens_directory_and_existing_temp_file` exercises the unix writer against deliberately loose modes. The failure regression injects rejection at each permission stage, verifying the old snapshot survives and no replacement bytes reach disk. The single-ACE property depends on `FILE_ALL_ACCESS` rather than `GENERIC_ALL`, which would split into two ACEs. + +## Local-file viewer + +The viewer allows inline and granted scripts for interactive local reports. CSP fetch directives constrain resource requests, but do not prevent a script assigning an external URL to its own frame. `form-action` constrains form submissions, not arbitrary navigation. Preserving the policy through the proxy repairs the resource-load boundary; it does not establish that active documents cannot send granted contents outside the machine. [CSP3 navigation checks](https://www.w3.org/TR/CSP3/) and its multiple-policy rules distinguish these mechanisms. diff --git a/dor/src/file-viewer-loopback-guard.ts b/dor/src/file-viewer-loopback-guard.ts index fd857ba33..0db2f6c81 100644 --- a/dor/src/file-viewer-loopback-guard.ts +++ b/dor/src/file-viewer-loopback-guard.ts @@ -1,15 +1,18 @@ import type { IncomingMessage } from 'node:http'; +import { createHash, timingSafeEqual } from 'node:crypto'; + +const sha256 = (value: string) => createHash('sha256').update(value).digest(); /** This listener owns its URLs: a per-process 256-bit capability authorizes * each read. Host and Origin checks also reject rebinding and foreign scripts. * See docs/specs/security-local.md -> Local-file viewer. The Host/Origin rule is * the one `lib/src/host/loopback-guard.ts` states for every loopback listener; - * `dor` cannot import `lib`, so it is restated here (exact-match, so a - * mixed-case `Host` is refused rather than folded). */ + * `dor` cannot import `lib`, so it is restated here with case-insensitive Host comparison. */ export function allowsFileViewerRequest(req: IncomingMessage, port: number, prefix: string): boolean { const origins = [`http://localhost:${port}`, `http://127.0.0.1:${port}`]; return (req.method === 'GET' || req.method === 'HEAD') - && origins.includes(`http://${req.headers.host ?? ''}`) + && origins.includes(`http://${(req.headers.host ?? '').toLowerCase()}`) && (!req.headers.origin || origins.includes(req.headers.origin)) - && !!req.url?.startsWith(prefix); + // Hash both sides so malformed lengths cannot make timingSafeEqual throw. + && timingSafeEqual(sha256(req.url?.slice(0, prefix.length) ?? ''), sha256(prefix)); } diff --git a/dor/src/file-viewer.ts b/dor/src/file-viewer.ts index bd6e50070..707d72205 100644 --- a/dor/src/file-viewer.ts +++ b/dor/src/file-viewer.ts @@ -92,7 +92,9 @@ export async function startFileViewer(input: string): Promise<{ port: number; pa const html = type.mime.startsWith('text/html'); if ((html || type.mime.startsWith('text/css')) && !scanned.has(canonical)) { scanned.add(canonical); - for (const ref of references(await readText(file), html)) { + // Inspection is optional: large or changing HTML/CSS can still stream. + const contents = await readText(file).catch(() => ''); + for (const ref of references(contents, html)) { if (!ref || ref.startsWith('/') || ref.startsWith('#') || /^[a-z][a-z\d+.-]*:/i.test(ref) || ref.includes('\\')) continue; let local: string; try { local = decodeURIComponent(ref.split(/[?#]/, 1)[0]); } catch { continue; } @@ -117,6 +119,8 @@ export async function startFileViewer(input: string): Promise<{ port: number; pa res.setHeader('Cache-Control', 'no-store'); res.setHeader('Referrer-Policy', 'no-referrer'); res.setHeader('X-Content-Type-Options', 'nosniff'); + // The iframe proxy must retain this policy on every MIME type. + res.setHeader('X-Dormouse-Preserve-CSP', '1'); res.setHeader('Content-Security-Policy', "default-src 'none'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; media-src 'self'; font-src 'self'; connect-src 'self'; frame-src 'self'; object-src 'self'; base-uri 'self'; form-action 'none'"); if (!allowsFileViewerRequest(req, port, prefix)) { finish(res, 403); return; } void (async () => { diff --git a/dor/test/file-viewer.test.mjs b/dor/test/file-viewer.test.mjs index 65b862d51..236dd8cdd 100644 --- a/dor/test/file-viewer.test.mjs +++ b/dor/test/file-viewer.test.mjs @@ -47,6 +47,11 @@ test('renders text as escaped content and requires the per-run token on every me assert.equal((await get(viewer, path)).status, 403); } assert.equal((await get(viewer, viewer.path, { Host: `evil.test:${viewer.port}` })).status, 403); + assert.equal((await get(viewer, viewer.path, { Host: `LOCALHOST:${viewer.port}` })).status, 200); + const prefix = viewer.path.split('/')[1]; + for (const token of [prefix.slice(1), prefix + '0', `${prefix[0] === '0' ? '1' : '0'}${prefix.slice(1)}`, `${prefix.slice(0, -1)}${prefix.at(-1) === '0' ? '1' : '0'}`]) { + assert.equal((await get(viewer, viewer.path.replace(prefix, token))).status, 403); + } assert.equal((await get(viewer, viewer.path, { Origin: 'https://evil.test' })).status, 403); assert.equal((await get(viewer, viewer.path, {}, 'POST')).status, 403); assert.equal((await get(viewer, viewer.path, {}, 'HEAD')).body, ''); @@ -99,6 +104,24 @@ test('fails unsupported formats and oversized text before starting a viewer', as await assert.rejects(startFileViewer(join(root, 'large.txt')), /8 MiB/); }); +test('streams oversized HTML and referenced CSS without scanning their dependencies', async () => { + const large = ' '.repeat(8 * 1024 * 1024 + 1); + await writeFile(join(root, 'hidden.svg'), ''); + const html = await start('large.html', `${large}`); + const htmlResponse = await get(html); + assert.equal(htmlResponse.status, 200); + assert.equal(htmlResponse.body.length, large.length + ''.length); + assert.equal((await get(html, asset(html, 'hidden.svg'))).status, 404); + + await writeFile(join(root, 'large.css'), `body { background: url(hidden.svg) }${large}`); + const withCss = await start('index.html', ''); + const cssResponse = await get(withCss, asset(withCss, 'large.css')); + assert.equal(cssResponse.status, 200); + assert.ok(cssResponse.body.length > 8 * 1024 * 1024); + assert.equal((await get(withCss, asset(withCss, 'hidden.svg'))).status, 404); + await assert.rejects(startFileViewer(join(root, 'large.css')), /8 MiB/); // a direct CSS text preview stays capped +}); + test('bounds the asset graph and keeps a grant on the opened file after path replacement', async () => { const viewer = await start('original.txt', 'original content'); await rm(join(root, 'original.txt')); diff --git a/lib/src/host/file-viewer-proxy.test.ts b/lib/src/host/file-viewer-proxy.test.ts new file mode 100644 index 000000000..bfe1e3636 --- /dev/null +++ b/lib/src/host/file-viewer-proxy.test.ts @@ -0,0 +1,98 @@ +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { request } from 'node:http'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, expect, it } from 'vitest'; +import { startFileViewer } from '../../../dor/src/file-viewer'; +import { createIframeProxyUrl } from './iframe-proxy'; + +const EMBEDDERS = ['vscode-webview://viewer-test', 'vscode-file://vscode-app']; +let root: string; +const viewers: Awaited>[] = []; +beforeEach(async () => { root = await mkdtemp(join(tmpdir(), 'dor-viewer-proxy-')); }); +afterEach(async () => { + await Promise.all(viewers.splice(0).map(viewer => viewer.close())); + await rm(root, { recursive: true, force: true }); +}); + +async function frame(name: string, contents: string) { + const file = join(root, name); + await writeFile(file, contents); + const viewer = await startFileViewer(file); + viewers.push(viewer); + const result = await createIframeProxyUrl(`http://127.0.0.1:${viewer.port}${viewer.path}`, { embedderOrigins: EMBEDDERS }); + if (!result.ok) throw new Error(result.detail); + return result.url; +} + +function read(url: string, method = 'GET', headers: Record = {}) { + return new Promise<{ status: number; headers: import('node:http').IncomingHttpHeaders; body: string }>((resolve, reject) => { + const req = request(url, { method, headers }, res => { + const chunks: Buffer[] = []; + res.on('data', chunk => chunks.push(chunk)); + res.on('error', reject); + res.on('end', () => resolve({ status: res.statusCode!, headers: res.headers, body: Buffer.concat(chunks).toString() })); + }); + req.on('error', reject); + req.end(); + }); +} + +function expectViewerPolicy(headers: import('node:http').IncomingHttpHeaders) { + const policy = headers['content-security-policy']; + for (const directive of ["default-src 'none'", "script-src 'self' 'unsafe-inline'", "connect-src 'self'", "base-uri 'self'", "form-action 'none'"]) { + expect(policy).toContain(directive); + } + expect(policy).toContain(`frame-ancestors 'self' ${EMBEDDERS.join(' ')}`); + expect(headers['x-dormouse-preserve-csp']).toBeUndefined(); + expect(headers['referrer-policy']).toBe('no-referrer'); +} + +it('preserves the real viewer policy and meta policy through HTML instrumentation', async () => { + await writeFile(join(root, 'style.css'), 'body { color: red }'); + await writeFile(join(root, 'app.js'), 'window.loaded = true;'); + await writeFile(join(root, 'private.txt'), 'ungranted'); + const meta = ''; + const url = await frame('index.html', `${meta}hello`); + const response = await read(url); + expect(response.status).toBe(200); + expectViewerPolicy(response.headers); + expect(response.body).toContain(meta); + expect(response.body).toContain('__dormouse'); + for (const name of ['style.css', 'app.js']) { + const asset = await read(new URL(name, url).href); + expect(asset.status).toBe(200); + expectViewerPolicy(asset.headers); + } + expect((await read(new URL('private.txt', url).href)).status).toBe(404); +}); + +it('retains the policy for an escaped text preview', async () => { + const response = await read(await frame('readme.md', '')); + expectViewerPolicy(response.headers); + expect(response.body).toContain('<script>untrusted()</script>'); + expect(response.body).toContain('__dormouse'); +}); + +it.each([ + ['image.svg', '', 'image/svg+xml'], + ['document.pdf', '%PDF-1.7 example bytes', 'application/pdf'], + ['image.png', 'image bytes', 'image/png'], + ['video.mp4', 'video bytes', 'video/mp4'], +])('retains CSP, bytes, HEAD, and ranges for %s through the proxy', async (name, bytes, mime) => { + const url = await frame(name, bytes); + const response = await read(url); + expectViewerPolicy(response.headers); + expect(response.headers['content-type']).toBe(mime); + expect(response.headers['content-length']).toBe(String(Buffer.byteLength(bytes))); + expect(response.body).toBe(bytes); + const head = await read(url, 'HEAD'); + expectViewerPolicy(head.headers); + expect(head.headers['content-length']).toBe(response.headers['content-length']); + expect(head.body).toBe(''); + const range = await read(url, 'GET', { Range: 'bytes=0-3' }); + expectViewerPolicy(range.headers); + expect(range.status).toBe(206); + expect(range.headers['content-range']).toBe(`bytes 0-3/${Buffer.byteLength(bytes)}`); + expect(range.body).toBe(bytes.slice(0, 4)); +}); diff --git a/lib/src/host/iframe-proxy-rewrite.test.ts b/lib/src/host/iframe-proxy-rewrite.test.ts index 8c6d15eaf..4920848e1 100644 --- a/lib/src/host/iframe-proxy-rewrite.test.ts +++ b/lib/src/host/iframe-proxy-rewrite.test.ts @@ -64,6 +64,13 @@ describe('instrumentHtml', () => { expect(out).not.toMatch(/http-equiv=["']?content-security-policy/i); }); + it('retains an opted-in document policy while injecting the shim', () => { + const policy = ''; + const out = instrumentHtml(`${policy}`, APP, true); + expect(out).toContain(policy); + expect(out).toContain('__dormouse'); + }); + it('forwards the leader chord and a pointerdown select signal', () => { expect(IFRAME_SHIM).toContain('__dormouse'); expect(IFRAME_SHIM).toContain("'leader'"); diff --git a/lib/src/host/iframe-proxy-rewrite.ts b/lib/src/host/iframe-proxy-rewrite.ts index 15617d03e..3a2ddf5f8 100644 --- a/lib/src/host/iframe-proxy-rewrite.ts +++ b/lib/src/host/iframe-proxy-rewrite.ts @@ -38,6 +38,10 @@ export const FRAMING_RESPONSE_HEADERS = new Set([ 'x-frame-options', 'content-security-policy', 'content-security-policy-report-only', ]); +/** Upstreams that supply their own content boundary opt into retaining it. + * This only preserves restrictions; it grants no additional authority. */ +export const PRESERVE_CSP_HEADER = 'x-dormouse-preserve-csp'; + // A serialized origin: scheme, host, optional port, nothing else. Deliberately // strict — the value ends up inside a CSP header we emit, so anything that // could carry a `;` or a space is refused rather than escaped. It covers the @@ -71,7 +75,7 @@ export function normalizeEmbedderOrigins(value: unknown): string[] | null { } /** - * The `Content-Security-Policy` the proxy serves in place of the upstream's. + * The proxy's framing policy, replacing or supplementing the upstream's CSP. * `'self'` permits nested documents within this one per-grant origin; every * external ancestor must still be in the validated app chain. */ @@ -81,7 +85,7 @@ export function frameAncestorsCsp(embedderOrigins: string[]): string { // The fixed, Dormouse-owned shim — like agent-browser's EDIT_SCRIPTS, never // user-supplied, so it is not an eval vector. Injected inline into served HTML; -// this is why the upstream CSP is dropped whole rather than per-directive (an +// this is why the default drops upstream CSP 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 the @@ -179,8 +183,8 @@ export function iframeShim(embedderOrigin: string): string { })();`; } -// Drop any in-document CSP and inject the shim before so it runs before -// the tool's own scripts. Applies to every framed http upstream, loopback or +// Drop in-document CSP unless the upstream opted into preservation, then +// inject the shim before . Applies to framed http upstreams, loopback or // remote — the trade is stated in docs/specs/dor-browser.md → "Iframe Host // Capability And CSP". The response-header CSP is replaced separately via // FRAMING_RESPONSE_HEADERS. @@ -188,8 +192,8 @@ export function iframeShim(embedderOrigin: string): string { // `embedderOrigin` is the document that frames us, and it is required: without // one there is nobody to address the shim's messages to, so the caller must not // instrument at all rather than fall back to `'*'`. -export function instrumentHtml(body: string, embedderOrigin: string): string { - const html = body.replace( +export function instrumentHtml(body: string, embedderOrigin: string, preserveCsp = false): string { + const html = preserveCsp ? body : body.replace( /]+http-equiv=["']?content-security-policy["']?[^>]*>/gi, '', ); diff --git a/lib/src/host/iframe-proxy.test.ts b/lib/src/host/iframe-proxy.test.ts index 7b13cf76b..4b6de5186 100644 --- a/lib/src/host/iframe-proxy.test.ts +++ b/lib/src/host/iframe-proxy.test.ts @@ -338,6 +338,35 @@ describe('iframe proxy — the proxy never vouches for a stranger', () => { }); describe('iframe proxy — the two privileges are the embedder’s, not the port’s', () => { + it.each([NO_LOG, NO_EMBEDDER])('preserves opted-in CSP policies without weakening them (%j)', async opts => { + const policies = ["default-src 'none'; script-src 'self' 'unsafe-inline'", "frame-ancestors 'none'; connect-src 'none'"]; + const meta = ''; + const port = await upstream((_q, s) => { + s.writeHead(200, { + 'content-type': 'text/html', + 'x-dormouse-preserve-csp': '1', + 'x-frame-options': 'DENY', + 'content-security-policy': policies, + 'content-security-policy-report-only': "default-src 'none'", + }); + s.end(`${meta}kept`); + }); + const res = await get(await frame(`http://127.0.0.1:${port}/`, opts)); + expect(res.headers['content-security-policy']).toBe(policies.join(', ') + + (opts === NO_LOG ? ", frame-ancestors 'self' vscode-webview://abc-123 vscode-file://vscode-app" : '')); + expect(res.headers['content-security-policy-report-only']).toBe("default-src 'none'"); + expect(res.headers['x-dormouse-preserve-csp']).toBeUndefined(); + expect(res.headers['x-frame-options']).toBe(opts === NO_LOG ? undefined : 'DENY'); + expect(res.body).toContain(meta); + expect(res.body.includes('__dormouse')).toBe(opts === NO_LOG); + }); + + it('does not let a request header opt an ordinary upstream into CSP preservation', async () => { + const port = await upstream((_q, s) => DENY_HTML(s)); + const res = await request(await frame(`http://127.0.0.1:${port}/`), { headers: { 'X-Dormouse-Preserve-CSP': '1' } }); + expect(res.headers['content-security-policy']).toBe("frame-ancestors 'self' vscode-webview://abc-123 vscode-file://vscode-app"); + }); + // The loopback port is not a secret, and no request header separates our // webview from an attacker page: an iframe navigation carries no `Origin`, // and `Sec-Fetch-Site` reads `cross-site` for both. So the framing-header diff --git a/lib/src/host/iframe-proxy.ts b/lib/src/host/iframe-proxy.ts index c4afb7e8d..4dde351d1 100644 --- a/lib/src/host/iframe-proxy.ts +++ b/lib/src/host/iframe-proxy.ts @@ -57,6 +57,7 @@ import type { IframeProxyResult } from '../lib/platform/iframe-proxy-types'; import { isForeignOrigin, isLoopbackHost, isOwnOrigin } from './loopback-guard'; import { FRAMING_RESPONSE_HEADERS, + PRESERVE_CSP_HEADER, HOP_BY_HOP_RESPONSE_HEADERS, errorPageHtml, frameAncestorsCsp, @@ -313,6 +314,7 @@ function streamHtml( upstreamRes: http.IncomingMessage, res: http.ServerResponse, ): void { + const preserveCsp = upstreamRes.headers[PRESERVE_CSP_HEADER] === '1'; const outHeaders = sanitizeResponseHeaders(grant, upstreamRes.headers); outHeaders['content-type'] = 'text/html; charset=utf-8'; delete outHeaders['content-length']; @@ -329,7 +331,7 @@ function streamHtml( // prefix, then hand the remainder to a raw pipe (backpressure + end). handled = true; upstreamRes.off('data', onData); - res.write(Buffer.from(instrumentHtml(text, embedderOrigin), 'latin1')); + res.write(Buffer.from(instrumentHtml(text, embedderOrigin, preserveCsp), 'latin1')); pending = Buffer.alloc(0); upstreamRes.pipe(res); }; @@ -338,7 +340,7 @@ function streamHtml( upstreamRes.on('end', () => { if (handled) return; // the pipe ends `res` // Whole document arrived before any head marker — instrument and finish. - res.end(Buffer.from(instrumentHtml(pending.toString('latin1'), embedderOrigin), 'latin1')); + res.end(Buffer.from(instrumentHtml(pending.toString('latin1'), embedderOrigin, preserveCsp), 'latin1')); }); upstreamRes.on('error', () => { if (!res.writableEnded) res.destroy(); }); } @@ -346,19 +348,27 @@ function streamHtml( function sanitizeResponseHeaders(grant: Grant, headers: http.IncomingHttpHeaders): http.OutgoingHttpHeaders { const out: http.OutgoingHttpHeaders = {}; const replaceFraming = grant.embedderOrigins !== null; + const preserveCsp = headers[PRESERVE_CSP_HEADER] === '1'; for (const [name, value] of Object.entries(headers)) { if (value === undefined) continue; const lower = name.toLowerCase(); + if (lower === PRESERVE_CSP_HEADER) continue; if (HOP_BY_HOP_RESPONSE_HEADERS.has(lower) || lower === 'set-cookie') 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 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; + if (replaceFraming && FRAMING_RESPONSE_HEADERS.has(lower) + && !(preserveCsp && lower.startsWith('content-security-policy'))) continue; out[name] = value; } if (grant.embedderOrigins !== null) { - out['content-security-policy'] = frameAncestorsCsp(grant.embedderOrigins); + const upstreamCsp = out['content-security-policy']; + // Separate policies intersect: retain every upstream directive verbatim, + // including stricter frame-ancestors, and add our embedder boundary. + out['content-security-policy'] = upstreamCsp === undefined + ? frameAncestorsCsp(grant.embedderOrigins) + : [...(Array.isArray(upstreamCsp) ? upstreamCsp : [String(upstreamCsp)]), frameAncestorsCsp(grant.embedderOrigins)]; } // Keep upstream redirects on the proxy origin so they don't bounce the frame // straight at the un-instrumented upstream. diff --git a/lib/src/host/tool-registry.test.ts b/lib/src/host/tool-registry.test.ts index 6b2d1e19c..483ef4690 100644 --- a/lib/src/host/tool-registry.test.ts +++ b/lib/src/host/tool-registry.test.ts @@ -16,6 +16,12 @@ function parse(text: string, opts = REPO) { } describe('parseToolFile', () => { + it.each([REPO, USER])('reserves builtin: tool names in $scope configuration', opts => { + for (const name of ['builtin:file', 'builtin:other']) { + expect(() => parse(`tools:\n ${name}:\n run: [viewer]\n`, opts)).toThrow(/'builtin:' prefix is reserved/); + } + }); + it('reads an entry with a key template', () => { const file = parse(` tools: diff --git a/lib/src/host/tool-registry.ts b/lib/src/host/tool-registry.ts index df7a898b4..536c0beaa 100644 --- a/lib/src/host/tool-registry.ts +++ b/lib/src/host/tool-registry.ts @@ -144,6 +144,7 @@ export function parseToolFile( for (const [name, rawEntry] of Object.entries(toolsNode)) { const where = `${path}: tools.${name}`; + if (name.startsWith('builtin:')) throw new ToolFileError(`${where}: the 'builtin:' prefix is reserved`); if (!isRecord(rawEntry)) throw new ToolFileError(`${where}: entry must be a mapping`); for (const field of Object.keys(rawEntry)) { diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index 16e931b52..92af4bf63 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -5,7 +5,7 @@ "docs/specs/alert.md": 7250, "docs/specs/auto-update.md": 1100, "docs/specs/deploy.md": 1900, - "docs/specs/dor-browser.md": 4600, + "docs/specs/dor-browser.md": 4700, "docs/specs/dor-cli.md": 6000, "docs/specs/dor-tool.md": 3350, "docs/specs/glossary.md": 3000, @@ -19,7 +19,7 @@ "docs/specs/remote-security-model.md": 4800, "docs/specs/security-audit.md": 1750, "docs/specs/security-ci.md": 2500, - "docs/specs/security-local.md": 3050, + "docs/specs/security-local.md": 3100, "docs/specs/security-remote.md": 5850, "docs/specs/security-supply-chain.md": 1200, "docs/specs/security.md": 1900, From 3fd4f6bf80e39ee5a28dd77c4ae44aa51903e377 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 15 Sep 2026 18:11:42 -0700 Subject: [PATCH 05/15] Pin built-in file reuse across Tool authority scopes --- lib/src/components/Wall.test.tsx | 50 ++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/lib/src/components/Wall.test.tsx b/lib/src/components/Wall.test.tsx index 60335ea42..2cbaa6c6f 100644 --- a/lib/src/components/Wall.test.tsx +++ b/lib/src/components/Wall.test.tsx @@ -1507,6 +1507,56 @@ describe('Wall on the Lath engine', () => { } }); + it('reuses a builtin viewer by canonical target without matching same-named project or user Tools', async () => { + setToolsEnabled(true); + vi.spyOn(terminalRegistry, 'isPaneOscDriven').mockReturnValue(true); + const target = '/repo/docs/readme.md'; + const command = `dor __view-file ${target}`; + const toolControl = vi.fn(async (request: { op: string; global?: boolean }) => ({ + status: 'ok' as const, + scope: request.op === 'open' ? 'builtin' as const : request.global ? 'user' as const : undefined, + projectRoot: '/repo', path: '/repo/dormouse.yml', name: 'file', + run: ['dor', '__view-file', target], key: [target], + render: 'iframe' as const, port: 'announced' as const, warnings: [], + })); + Object.assign(fake, { toolControl }); + const ids: string[] = []; + const requestTool = async (params: Record) => { + const respond = vi.fn(); + await act(async () => window.dispatchEvent(new CustomEvent('dormouse:control-request', { detail: { + method: SURFACE_CONTROL_METHODS.tool, surfaceId: 'pane-a', params: { cwd: '/repo', ...params }, respond, + } }))); + await waitUntil(() => respond.mock.calls.length > 0); + const response = respond.mock.calls[0][0]; + expect(response.ok).toBe(true); + return response.result as { status: string; surfaceId: string }; + }; + try { + await act(async () => root.render()); + await flush(); + for (const params of [{ name: 'file' }, { name: 'file', global: true }, { file: 'docs/readme.md' }]) { + const result = await requestTool(params); + expect(result.status).toBe('created'); + ids.push(result.surfaceId); + act(() => { + terminalRegistry.seedTerminalManualCwd(result.surfaceId, '/repo'); + reportRunning(result.surfaceId, command); + }); + } + expect(new Set(ids).size).toBe(3); + expect(leafCount()).toBe(4); + + // Host resolution gives both spellings the same canonical document key. + const reused = await requestTool({ file: './docs/../docs/readme.md' }); + expect(toolControl).toHaveBeenLastCalledWith({ op: 'open', target: './docs/../docs/readme.md', cwd: '/repo', tool: undefined }); + expect(reused).toMatchObject({ status: 'existing', surfaceId: ids[2] }); + expect(leafCount()).toBe(4); + } finally { + act(() => ids.forEach(id => terminalRegistry.removeTerminalPaneState(id))); + setToolsEnabled(false); + } + }); + it('retries failed post-grant lookup without recording permission again', async () => { setToolsEnabled(true); let calls = 0; From a79462d41467e6ee92711a20c761993ef4411107 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 15 Sep 2026 18:23:19 -0700 Subject: [PATCH 06/15] Serialize preserved CSP policies for both host type contracts --- lib/src/host/iframe-proxy.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/host/iframe-proxy.ts b/lib/src/host/iframe-proxy.ts index 4dde351d1..04c8b8d10 100644 --- a/lib/src/host/iframe-proxy.ts +++ b/lib/src/host/iframe-proxy.ts @@ -364,11 +364,11 @@ function sanitizeResponseHeaders(grant: Grant, headers: http.IncomingHttpHeaders } if (grant.embedderOrigins !== null) { const upstreamCsp = out['content-security-policy']; - // Separate policies intersect: retain every upstream directive verbatim, + // Comma-separated policies intersect: retain every upstream directive verbatim, // including stricter frame-ancestors, and add our embedder boundary. out['content-security-policy'] = upstreamCsp === undefined ? frameAncestorsCsp(grant.embedderOrigins) - : [...(Array.isArray(upstreamCsp) ? upstreamCsp : [String(upstreamCsp)]), frameAncestorsCsp(grant.embedderOrigins)]; + : [...(Array.isArray(upstreamCsp) ? upstreamCsp : [String(upstreamCsp)]), frameAncestorsCsp(grant.embedderOrigins)].join(', '); } // Keep upstream redirects on the proxy origin so they don't bounce the frame // straight at the un-instrumented upstream. From 46c9a52bba49cf749ff189b8cc8c93283de2963e Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 15 Sep 2026 18:26:32 -0700 Subject: [PATCH 07/15] Account for the Tool argv restore contract in the spec budget --- scripts/spec-word-budgets.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index c9e1e3ba9..8b471f5b3 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -7,7 +7,7 @@ "docs/specs/deploy.md": 1900, "docs/specs/dor-browser.md": 4700, "docs/specs/dor-cli.md": 6000, - "docs/specs/dor-tool.md": 3400, + "docs/specs/dor-tool.md": 3450, "docs/specs/glossary.md": 3000, "docs/specs/layout.md": 8800, "docs/specs/mobile-terminal-ui.md": 1950, From e0cc1ea5bf1be331ac8aa96b914b2b7f1a35d609 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 15 Sep 2026 19:06:03 -0700 Subject: [PATCH 08/15] Clarify file viewer errors and skip source-preview dependencies --- docs/specs/dor-tool.md | 4 ++-- docs/specs/dor-tool.rationale.md | 2 ++ dor/src/file-viewer.ts | 4 +++- dor/test/file-viewer.test.mjs | 18 ++++++++++++++++++ lib/src/host/tool-open.test.ts | 10 ++++++++++ lib/src/host/tool-open.ts | 2 ++ 6 files changed, 37 insertions(+), 3 deletions(-) diff --git a/docs/specs/dor-tool.md b/docs/specs/dor-tool.md index 4e17ac9b5..2ebbdb7e3 100644 --- a/docs/specs/dor-tool.md +++ b/docs/specs/dor-tool.md @@ -154,9 +154,9 @@ Source of truth: `toolCommand` in `dor/src/commands/tool.ts`; `dor/test/snapshot **Must reject declared Tool names beginning with `builtin:` in either configuration scope.** Built-in handler names cannot be shadowed. -**Must use `builtin:file` for supported files when no user rule matches.** An explicit unknown handler or malformed user configuration fails without fallback. Built-in identity is the canonical file path in its own scope, separate from user and project Tools. +**Must use `builtin:file` for supported files when no user rule matches.** An explicit unknown handler or malformed user configuration fails without fallback. Selecting `builtin:file` for an unsupported format reports that limitation and suggests a user Tool. Built-in identity is the canonical file path in its own scope, separate from user and project Tools. -**Must run the built-in viewer as a Tool-owned `dor` process**, serving HTML, images, PDF/media, and escaped text/source previews. Markdown is source text; custom viewers may render it. Text previews and HTML/CSS dependency inspection are limited to 8 MiB per file. Oversized HTML and referenced CSS still stream without dependency inspection. The grant contains at most 256 files: the opened document and statically referenced relative HTML/CSS assets within its directory tree. Never expand the grant through root-relative, external, or dynamic references; requests can read only granted paths. +**Must run the built-in viewer as a Tool-owned `dor` process**, serving HTML, images, PDF/media, and escaped text/source previews. Markdown is source text; custom viewers may render it. Text previews and HTML/CSS dependency inspection are limited to 8 MiB per file. Text/source previews grant only their opened file and skip dependency inspection. (rationale) Oversized HTML and referenced CSS still stream without dependency inspection. The grant contains at most 256 files: the opened document and statically referenced relative HTML/CSS assets within its directory tree; exceeding that bound fails the open without serving a partial grant. Never expand the grant through root-relative, external, or dynamic references; requests can read only granted paths. **Must retain the viewer's opened file descriptors until the Tool exits.** Refresh reads those files again, but atomic replacements and changes to the dependency graph require restarting the viewer. Cold restore runs the saved file command with a fresh URL capability; Workspace movement keeps the live binding. The listener's authority is `docs/specs/security-local.md` → Local-file viewer. diff --git a/docs/specs/dor-tool.rationale.md b/docs/specs/dor-tool.rationale.md index f6c7cb42c..c49eefadb 100644 --- a/docs/specs/dor-tool.rationale.md +++ b/docs/specs/dor-tool.rationale.md @@ -44,6 +44,8 @@ The September 2026 integration reuses Terminal Context for the Tool's primary te ## Opening local files +A CSS source preview escapes its contents, so its URLs cannot load assets. Scanning those references adds unused authority and can reject a small source file at the asset limit. CSS loaded by HTML is active, so its dependencies still enter the bounded grant. + Keeping the built-in viewer in the Tool's process tree reuses port discovery, kill, restart, and Workspace transfer. An OSC path carries the per-run URL capability without saving that secret in the restart command. Holding the selected file descriptors bounds what the server can read after launch; it trades automatic replacement-file refresh for a grant whose contents cannot widen through path replacement. diff --git a/dor/src/file-viewer.ts b/dor/src/file-viewer.ts index 707d72205..e99478169 100644 --- a/dor/src/file-viewer.ts +++ b/dor/src/file-viewer.ts @@ -65,6 +65,8 @@ export async function startFileViewer(input: string): Promise<{ port: number; pa const target = await realpath(input); const format = fileViewerFormat(target); if (!format) throw new Error('unsupported file format; configure a user Tool association'); + // A source preview escapes the document; none of its references load. + const inspectDependencies = !format.text; const root = dirname(target); const prefix = `/${randomBytes(32).toString('hex')}/`; const resources = new Map(); @@ -90,7 +92,7 @@ export async function startFileViewer(input: string): Promise<{ port: number; pa const resource = { file, mime: type.mime }; resources.set(route, resource); // the grant owns the descriptor from here const html = type.mime.startsWith('text/html'); - if ((html || type.mime.startsWith('text/css')) && !scanned.has(canonical)) { + if (inspectDependencies && (html || type.mime.startsWith('text/css')) && !scanned.has(canonical)) { scanned.add(canonical); // Inspection is optional: large or changing HTML/CSS can still stream. const contents = await readText(file).catch(() => ''); diff --git a/dor/test/file-viewer.test.mjs b/dor/test/file-viewer.test.mjs index 236dd8cdd..7778afdfa 100644 --- a/dor/test/file-viewer.test.mjs +++ b/dor/test/file-viewer.test.mjs @@ -122,6 +122,24 @@ test('streams oversized HTML and referenced CSS without scanning their dependenc await assert.rejects(startFileViewer(join(root, 'large.css')), /8 MiB/); // a direct CSS text preview stays capped }); +test('previews CSS source without granting dependencies but still bounds HTML-referenced CSS', async () => { + const names = Array.from({ length: 256 }, (_, i) => `image${i}.svg`); + await Promise.all(names.map(name => writeFile(join(root, name), ''))); + const css = names.map(name => `body { background: url("${name}") }`).join('\n'); + const viewer = await start('source.css', css); + const response = await get(viewer); + assert.equal(response.status, 200); + assert.match(response.body, /url\("image255.svg"\)/); + const prefix = viewer.path.slice(0, -'view'.length); + assert.equal((await get(viewer, `${prefix}file/image0.svg`)).status, 404); + assert.equal((await get(viewer, `${prefix}file/source.css`)).status, 200); + + // The same CSS is an active stylesheet when reached through HTML. Its + // dependencies still count against that viewer's grant and abort the open. + await writeFile(join(root, 'index.html'), ''); + await assert.rejects(startFileViewer(join(root, 'index.html')), /256 referenced files/); +}); + test('bounds the asset graph and keeps a grant on the opened file after path replacement', async () => { const viewer = await start('original.txt', 'original content'); await rm(join(root, 'original.txt')); diff --git a/lib/src/host/tool-open.test.ts b/lib/src/host/tool-open.test.ts index 8c01461cc..ac5622dc4 100644 --- a/lib/src/host/tool-open.test.ts +++ b/lib/src/host/tool-open.test.ts @@ -65,6 +65,16 @@ it('names the user configuration in unmatched-file errors', async () => { expect(await host().handle({ op: 'open', target, cwd: root })).toMatchObject({ status: 'error', message: expect.stringContaining(config) }); }); +it.each(['explicit', 'association'] as const)('explains unsupported formats when builtin:file is selected by %s', async selection => { + const target = join(root, 'unknown.binary'); + await writeFile(target, 'hi'); + if (selection === 'association') await writeConfig('open:\n - {match: "*.binary", tool: "builtin:file"}\n'); + else await rm(config); + expect(await host().handle({ op: 'open', target, cwd: root, ...(selection === 'explicit' ? { tool: 'builtin:file' } : {}) })).toEqual({ + status: 'error', message: `the built-in viewer does not support 'unknown.binary'; add an open rule to ${config} naming a user Tool`, + }); +}); + it('uses the built-in viewer only as a fallback or explicit choice', async () => { const target = join(root, 'docs', 'README.md'); expect(await host().handle({ op: 'open', target, cwd: root, tool: 'builtin:file' })).toMatchObject({ diff --git a/lib/src/host/tool-open.ts b/lib/src/host/tool-open.ts index 9835231d4..0568a55b2 100644 --- a/lib/src/host/tool-open.ts +++ b/lib/src/host/tool-open.ts @@ -24,6 +24,8 @@ export async function resolveOpenTool( return { status: 'ok', projectRoot: request.cwd, path: '', name: 'file', scope: 'builtin', run: ['dor', VIEW_FILE_ARGV, target], key: [target], render: 'iframe', port: 'announced', warnings: [] }; } + if (name === BUILTIN_FILE_TOOL) return { status: 'error', + message: `the built-in viewer does not support '${basename(target)}'; add an open rule to ${path} naming a user Tool` }; if (!file || !entry) return { status: 'error', message: request.tool ? `no user Tool '${request.tool}' in ${path}` : `no Tool matches '${request.target}'; add an open rule to ${path}, or use dor open --tool ` }; From 3ee751689c220b5bf57d0708d3319e2cec673b60 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 15 Sep 2026 19:22:44 -0700 Subject: [PATCH 09/15] Prefer known file formats over source-name fallbacks --- docs/specs/dor-tool.md | 2 ++ dor/src/file-viewer-format.ts | 5 +++-- dor/test/file-viewer.test.mjs | 21 ++++++++++++++++++--- lib/src/host/file-viewer-proxy.test.ts | 2 +- 4 files changed, 24 insertions(+), 6 deletions(-) diff --git a/docs/specs/dor-tool.md b/docs/specs/dor-tool.md index 2ea179402..6be96c0da 100644 --- a/docs/specs/dor-tool.md +++ b/docs/specs/dor-tool.md @@ -156,6 +156,8 @@ Source of truth: `toolCommand` in `dor/src/commands/tool.ts`; `dor/test/snapshot **Must use `builtin:file` for supported files when no user rule matches.** An explicit unknown handler or malformed user configuration fails without fallback. Selecting `builtin:file` for an unsupported format reports that limitation and suggests a user Tool. Built-in identity is the canonical file path in its own scope, separate from user and project Tools. +**Must prefer known extensions over filename-based text fallbacks; source extensions remain escaped previews.** + **Must run the built-in viewer as a Tool-owned `dor` process**, serving HTML, images, PDF/media, and escaped text/source previews. Markdown is source text; custom viewers may render it. Text previews and HTML/CSS dependency inspection are limited to 8 MiB per file. Text/source previews grant only their opened file and skip dependency inspection. (rationale) Oversized HTML and referenced CSS still stream without dependency inspection. The grant contains at most 256 files: the opened document and statically referenced relative HTML/CSS assets within its directory tree; exceeding that bound fails the open without serving a partial grant. Never expand the grant through root-relative, external, or dynamic references; requests can read only granted paths. **Must retain the viewer's opened file descriptors until the Tool exits.** Refresh reads those files again, but atomic replacements and changes to the dependency graph require restarting the viewer. Cold restore runs the saved file command with a fresh URL capability; Workspace movement keeps the live binding. The listener's authority is `docs/specs/security-local.md` → Local-file viewer. diff --git a/dor/src/file-viewer-format.ts b/dor/src/file-viewer-format.ts index ac0457828..c6d700a2a 100644 --- a/dor/src/file-viewer-format.ts +++ b/dor/src/file-viewer-format.ts @@ -20,7 +20,8 @@ export const VIEW_FILE_ARGV = '__view-file'; export function fileViewerFormat(path: string): { mime: string; text: boolean } | null { const name = path.replace(/\\/g, '/').split('/').pop()!.toLowerCase(); const ext = name.includes('.') ? name.split('.').pop()! : ''; - const text = TEXT.has(ext) || /^(readme|license|licence|makefile|dockerfile|\.gitignore|\.env)(\..*)?$/.test(name); - const mime = MIME[ext] ?? (text ? 'text/plain; charset=utf-8' : null); + const knownMime = Object.prototype.hasOwnProperty.call(MIME, ext) ? MIME[ext] : undefined; + const text = TEXT.has(ext) || (!knownMime && /^(readme|license|licence|makefile|dockerfile|\.gitignore|\.env)(\..*)?$/.test(name)); + const mime = knownMime ?? (text ? 'text/plain; charset=utf-8' : null); return mime ? { mime, text } : null; } diff --git a/dor/test/file-viewer.test.mjs b/dor/test/file-viewer.test.mjs index 7778afdfa..478d45b96 100644 --- a/dor/test/file-viewer.test.mjs +++ b/dor/test/file-viewer.test.mjs @@ -8,6 +8,7 @@ import { once } from 'node:events'; import { fileURLToPath } from 'node:url'; import { afterEach, beforeEach, test } from 'node:test'; import { startFileViewer } from '../dist/file-viewer.js'; +import { fileViewerFormat } from '../dist/file-viewer-format.js'; let root; const viewers = []; @@ -36,6 +37,17 @@ async function get(viewer, path = viewer.path, headers = {}, method = 'GET') { } const asset = (viewer, path) => viewer.path.replace(/\/file\/.*$/, `/file/${path}`); +test('known formats override source-name heuristics without treating prototype keys as formats', () => { + for (const [name, mime] of [['README.pdf', 'application/pdf'], ['readme.png', 'image/png'], ['LICENSE.html', 'text/html; charset=utf-8']]) { + assert.deepEqual(fileViewerFormat(name), { mime, text: false }); + } + for (const name of ['README', 'Dockerfile.dev', 'README.md', '.gitignore']) { + assert.deepEqual(fileViewerFormat(name), { mime: 'text/plain; charset=utf-8', text: true }); + } + assert.deepEqual(fileViewerFormat('README.css'), { mime: 'text/css; charset=utf-8', text: true }); + assert.equal(fileViewerFormat('file.constructor'), null); +}); + test('renders text as escaped content and requires the per-run token on every method', async () => { const viewer = await start('README.md', ' & hello'); const good = await get(viewer); @@ -67,8 +79,11 @@ test('serves only the HTML document and its bounded relative dependency graph', await writeFile(join(root, 'assets', 'more.css'), 'body { color: red }'); await writeFile(join(root, 'assets', 'pic.svg'), ''); await writeFile(join(root, 'unreferenced.txt'), 'private sibling'); - const viewer = await start('index.html', '

Preview

'); - assert.equal((await get(viewer)).status, 200); + const viewer = await start('LICENSE.html', '

Preview

'); + const response = await get(viewer); + assert.equal(response.status, 200); + assert.equal(response.headers['content-type'], 'text/html; charset=utf-8'); + assert.match(response.body, /

Preview<\/h1>/); for (const path of ['assets/style.css', 'assets/more.css', 'assets/pic.svg']) assert.equal((await get(viewer, asset(viewer, path))).status, 200); assert.equal((await get(viewer, asset(viewer, 'unreferenced.txt'))).status, 404); assert.notEqual((await get(viewer, asset(viewer, '%2e%2e/unreferenced.txt'))).status, 200); @@ -86,7 +101,7 @@ test('rejects parent-directory references and symlinks escaping the document dir }); test('supports byte ranges and HEAD for native PDF/image presentation', async () => { - const viewer = await start('sample.pdf', '%PDF-1.7 example bytes'); + const viewer = await start('README.pdf', '%PDF-1.7 example bytes'); assert.equal((await get(viewer)).headers['content-type'], 'application/pdf'); const range = await get(viewer, viewer.path, { Range: 'bytes=0-3' }); assert.equal(range.status, 206); diff --git a/lib/src/host/file-viewer-proxy.test.ts b/lib/src/host/file-viewer-proxy.test.ts index bfe1e3636..9cade49c1 100644 --- a/lib/src/host/file-viewer-proxy.test.ts +++ b/lib/src/host/file-viewer-proxy.test.ts @@ -77,7 +77,7 @@ it('retains the policy for an escaped text preview', async () => { it.each([ ['image.svg', '', 'image/svg+xml'], ['document.pdf', '%PDF-1.7 example bytes', 'application/pdf'], - ['image.png', 'image bytes', 'image/png'], + ['readme.png', 'image bytes', 'image/png'], ['video.mp4', 'video bytes', 'video/mp4'], ])('retains CSP, bytes, HEAD, and ranges for %s through the proxy', async (name, bytes, mime) => { const url = await frame(name, bytes); From cfa1690cd7df5ace5a90b6d3947378854d3909eb Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 16 Sep 2026 14:52:26 -0700 Subject: [PATCH 10/15] Expose Dor Tools toggle in experimental settings --- docs/specs/dor-tool.md | 4 +++- lib/src/components/SettingsDialog.tsx | 17 +++++++++++++++++ lib/src/components/wall/use-dor-control.ts | 4 ++-- lib/src/lib/feature-flags.ts | 2 +- 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/docs/specs/dor-tool.md b/docs/specs/dor-tool.md index a651e9249..c7aecd5c0 100644 --- a/docs/specs/dor-tool.md +++ b/docs/specs/dor-tool.md @@ -16,7 +16,9 @@ **Must gate tool creation on `isToolsEnabled`.** The flag disables new designation; existing Tools retain serving and exit cleanup. Inert announcement parsing and capability predicates remain active. Capability semantics belong to `docs/specs/glossary.md` → Panes and Surfaces; CLI reporting belongs to `docs/specs/dor-cli.md` → `dor list`. -Source of truth: `isToolsEnabled` in `lib/src/lib/feature-flags.ts`; `surface.tool` in `lib/src/components/wall/use-dor-control.ts`; `useToolServing` in `lib/src/components/wall/use-tool-serving.ts`. +**Must expose the flag in Settings → Experimental → Dor Tools.** Persist localStorage changes for subsequent launches without reloading; disabled-launch errors point here. + +Source of truth: `isToolsEnabled` in `lib/src/lib/feature-flags.ts`; `SettingsDialog` in `lib/src/components/SettingsDialog.tsx`; `surface.tool` in `lib/src/components/wall/use-dor-control.ts`; `useToolServing` in `lib/src/components/wall/use-tool-serving.ts`. ## The tool capability set diff --git a/lib/src/components/SettingsDialog.tsx b/lib/src/components/SettingsDialog.tsx index 0d36db07d..cbaa72e19 100644 --- a/lib/src/components/SettingsDialog.tsx +++ b/lib/src/components/SettingsDialog.tsx @@ -20,6 +20,7 @@ import { WatchedCommandList } from './WatchedCommandList'; import { RemoteControlSection } from './RemoteControlSection'; import { PushTestButton, SpeakTestButton } from './AlarmTestButtons'; import { getPlatform } from '../lib/platform'; +import { isToolsEnabled, setToolsEnabled } from '../lib/feature-flags'; import { hasNotepadArchive } from '../lib/notepad/archive-service'; import { getShellsSnapshot, subscribeToShells } from '../lib/shell-store'; import { @@ -92,6 +93,7 @@ export function SettingsDialog({ onClose }: { onClose: () => void }) { const watched = useSyncExternalStore(subscribeToWatchedCommands, getWatchedCommandsSnapshot); const settings = useSyncExternalStore(subscribeToAlertSettings, getAlertSettings); const shellState = useSyncExternalStore(subscribeToShells, getShellsSnapshot); + const [toolsEnabled, setToolsEnabledState] = useState(isToolsEnabled); const closeRef = useRef(null); // One union rather than a boolean per picker, so two menus can never be open // at once and Escape has a single thing to close. @@ -215,6 +217,21 @@ export function SettingsDialog({ onClose }: { onClose: () => void }) { has nowhere to go. Renders nothing on a build with no Burrow service. */} +
+

Experimental

+ { + setToolsEnabled(enabled); + setToolsEnabledState(isToolsEnabled()); + }} + /> +
+ Open local files with dor open and run tools with dor tool. Changes apply immediately. +
+
+ {/* Last: the only row here that leads somewhere instead of setting something, so it reads as the door it is. */} {showArchive ? ( diff --git a/lib/src/components/wall/use-dor-control.ts b/lib/src/components/wall/use-dor-control.ts index 6f4b41751..bfbf1ec11 100644 --- a/lib/src/components/wall/use-dor-control.ts +++ b/lib/src/components/wall/use-dor-control.ts @@ -15,7 +15,7 @@ import type { } from 'dor/commands/types'; import { hasBrowser, hasTerminal } from 'dor/commands/types'; import { MAX_AWAIT_TIMEOUT_MS } from '../../lib/alert-manager'; -import { TOOLS_FLAG_KEY, isToolsEnabled } from '../../lib/feature-flags'; +import { isToolsEnabled } from '../../lib/feature-flags'; import type { OpenPort } from '../../lib/platform/types'; import type { ToolKeyScope } from '../../lib/platform/tool-types'; import { buildShellCommandForKind, hasShellInputControls, shellCommandKind } from 'dor/commands/shell-quote'; @@ -882,7 +882,7 @@ export function useDorControl({ if (!isToolsEnabled()) { detail.respond({ ok: false, - error: `Dor Tools are off. Enable them by setting localStorage '${TOOLS_FLAG_KEY}' to 'true'.`, + error: 'Dor Tools are off. Enable them in Settings → Experimental → Dor Tools.', }); return; } diff --git a/lib/src/lib/feature-flags.ts b/lib/src/lib/feature-flags.ts index a48069104..650f35ec2 100644 --- a/lib/src/lib/feature-flags.ts +++ b/lib/src/lib/feature-flags.ts @@ -31,7 +31,7 @@ export function isToolsEnabled(): boolean { return readBoolFlag(TOOLS_FLAG_KEY); } -/** Toggle the tools flag (dev tooling / Storybook). */ +/** Toggle Dor Tools from Settings, dev tooling, or Storybook. */ export function setToolsEnabled(enabled: boolean): void { writeBoolFlag(TOOLS_FLAG_KEY, enabled); } From df7c9cbc031026ad3c29c4b6b8da938bf9555420 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 16 Sep 2026 15:24:14 -0700 Subject: [PATCH 11/15] Let dor open take over its calling terminal --- docs/specs/dor-tool.md | 4 +-- dor/src/commands/open.ts | 2 +- dor/test/snapshots/help/open.md | 2 +- lib/src/components/Wall.test.tsx | 29 +++++++++++++++---- lib/src/components/wall/tool-takeover.test.ts | 14 ++++----- lib/src/components/wall/tool-takeover.ts | 14 ++++----- 6 files changed, 38 insertions(+), 27 deletions(-) diff --git a/docs/specs/dor-tool.md b/docs/specs/dor-tool.md index c7aecd5c0..25bbe8004 100644 --- a/docs/specs/dor-tool.md +++ b/docs/specs/dor-tool.md @@ -170,11 +170,11 @@ Source of truth: `openCommand` in `dor/src/commands/open.ts`; `resolveOpenTool` ## Take-over -**Must run a standalone `dor tool` invocation in its calling pane when every takeover condition holds.** Otherwise use the ordinary split path. Trust approval and keyed reuse take precedence. (rationale) +**Must run a standalone `dor tool` or `dor open` invocation in its calling pane when every takeover condition holds.** Otherwise use the ordinary split path. Trust approval and keyed reuse take precedence. (rationale) | Condition | Required state | | --- | --- | -| Verb | `dor tool`; `dor open` never transforms a plain terminal, though a keyed match in its own Tool pane reruns there | +| Verb | `dor tool` or `dor open` | | Caller | Visible pane of the active Workspace; integrated plain terminal; not closing or dying | | Command line | OSC 633 reports the invocation alone; compound shell syntax rejects takeover | | Directory | Resolved Tool CWD equals the caller's reported CWD | diff --git a/dor/src/commands/open.ts b/dor/src/commands/open.ts index 15df6d5ac..b4c886a26 100644 --- a/dor/src/commands/open.ts +++ b/dor/src/commands/open.ts @@ -25,7 +25,7 @@ The ordered open list contains {match, tool} entries. Patterns without a slash m The selected Tool receives the canonical absolute filename as one argument. Configure prespawn_dedupe: [$TARGET] to reveal the same file on repeated opens within a Workspace. --fresh bypasses reuse. -Opening creates a focus-neutral split or reveals an existing Tool, never taking over the caller's terminal. The command prints the Surface handle; --json prints structured output.`, +Placement follows dor tool: typed alone at a prompt in a visible, integrated plain terminal in the requested directory, opening takes over that pane, preserving its terminal and scrollback. Agent/script invocations, compound lines, a pane with a helper, --minimize, --surface, or --cwd elsewhere split without taking focus. A matching Tool is reused. The command prints the Surface handle; --json prints structured output.`, }, parameters: { flags: { diff --git a/dor/test/snapshots/help/open.md b/dor/test/snapshots/help/open.md index df5292172..611ddbe8c 100644 --- a/dor/test/snapshots/help/open.md +++ b/dor/test/snapshots/help/open.md @@ -15,7 +15,7 @@ The ordered open list contains {match, tool} entries. Patterns without a slash m The selected Tool receives the canonical absolute filename as one argument. Configure prespawn_dedupe: [$TARGET] to reveal the same file on repeated opens within a Workspace. --fresh bypasses reuse. -Opening creates a focus-neutral split or reveals an existing Tool, never taking over the caller's terminal. The command prints the Surface handle; --json prints structured output. +Placement follows dor tool: typed alone at a prompt in a visible, integrated plain terminal in the requested directory, opening takes over that pane, preserving its terminal and scrollback. Agent/script invocations, compound lines, a pane with a helper, --minimize, --surface, or --cwd elsewhere split without taking focus. A matching Tool is reused. The command prints the Surface handle; --json prints structured output. FLAGS [--json] Print JSON output. diff --git a/lib/src/components/Wall.test.tsx b/lib/src/components/Wall.test.tsx index f0d72b591..904604e3c 100644 --- a/lib/src/components/Wall.test.tsx +++ b/lib/src/components/Wall.test.tsx @@ -1478,10 +1478,15 @@ describe('Wall on the Lath engine', () => { } }); - it('dispatches open through the user host and splits even when the caller could be taken over', async () => { + it.each([ + { line: 'dor open a.md', explicitSurface: false, status: 'takeover' }, + { line: 'claude', explicitSurface: false, status: 'created' }, + { line: 'dor open --surface surface:self a.md', explicitSurface: true, status: 'created' }, + ])('dispatches open through the user host with $status placement for $line', async ({ line, explicitSurface, status }) => { setToolsEnabled(true); const controller = new AbortController(); let toolId: string | undefined; + const typed: string[] = []; vi.spyOn(terminalRegistry, 'isPaneOscDriven').mockReturnValue(true); const toolControl = vi.fn(async () => ({ status: 'ok' as const, scope: 'user' as const, projectRoot: '/config', path: '/config/dormouse.yml', name: 'viewer', run: ['view', '/repo/a.md'], @@ -1490,21 +1495,32 @@ describe('Wall on the Lath engine', () => { try { await act(async () => root.render()); await flush(); + act(() => fake.spawnPty('pane-a')); + fake.setInputHandler('pane-a', data => typed.push(data)); terminalRegistry.seedTerminalManualCwd('pane-a', '/repo'); terminalRegistry.applyTerminalSemanticEvents('pane-a', [ - { type: 'commandLine', commandLine: 'dor open a.md' }, + { type: 'commandLine', commandLine: line }, { type: 'commandStart', source: 'osc633_boundaries' }, ]); const respond = vi.fn(); await act(async () => window.dispatchEvent(new CustomEvent('dormouse:control-request', { detail: { - method: SURFACE_CONTROL_METHODS.tool, surfaceId: 'pane-a', params: { file: 'a.md', cwd: '/repo' }, signal: controller.signal, respond, + method: SURFACE_CONTROL_METHODS.tool, surfaceId: 'pane-a', params: { file: 'a.md', cwd: '/repo', ...(explicitSurface ? { surface: 'pane-a' } : {}) }, signal: controller.signal, respond, } }))); await waitUntil(() => respond.mock.calls.length > 0); expect(toolControl).toHaveBeenCalledWith({ op: 'open', target: 'a.md', cwd: '/repo', tool: undefined }); - expect(respond).toHaveBeenCalledWith(expect.objectContaining({ ok: true, result: expect.objectContaining({ status: 'created' }) })); + expect(respond).toHaveBeenCalledWith(expect.objectContaining({ ok: true, result: expect.objectContaining({ status }) })); toolId = respond.mock.calls[0][0].result.surfaceId; - expect(toolId).not.toBe('pane-a'); - expect(leafCount()).toBe(2); + expect(typed).toEqual([]); + if (status === 'takeover') { + expect(toolId).toBe('pane-a'); + expect(leafCount()).toBe(1); + act(() => promptBack('pane-a')); + await waitUntil(() => typed.length > 0); + expect(typed).toEqual(['view /repo/a.md\r']); + } else { + expect(toolId).not.toBe('pane-a'); + expect(leafCount()).toBe(2); + } // This fixture stubs TerminalPane, so report the staged command's startup // explicitly before disposing the Wall and its shared launch queue wait. act(() => { @@ -1514,6 +1530,7 @@ describe('Wall on the Lath engine', () => { await act(async () => { await new Promise(resolve => setTimeout(resolve, 150)); }); } finally { await act(async () => controller.abort()); + fake.clearInputHandler('pane-a'); if (toolId) { pendingShellOpts.delete(toolId); act(() => terminalRegistry.removeTerminalPaneState(toolId!)); diff --git a/lib/src/components/wall/tool-takeover.test.ts b/lib/src/components/wall/tool-takeover.test.ts index b5545bb8b..51624688c 100644 --- a/lib/src/components/wall/tool-takeover.test.ts +++ b/lib/src/components/wall/tool-takeover.test.ts @@ -43,16 +43,16 @@ describe('isNakedToolInvocation', () => { }); }); -describe('toolTakesOverCaller', () => { +describe.each(['tool', 'open'] as const)('toolTakesOverCaller for dor %s', (verb) => { const passing: ToolTakeoverGate = { - verb: 'tool', + verb, explicitSurface: false, minimized: false, workspaceActive: true, visible: true, kind: 'terminal', oscDriven: true, - rawCommandLine: 'dor tool storybook', + rawCommandLine: verb === 'tool' ? 'dor tool storybook' : 'dor open README.md', cwdMatches: true, helperPresent: false, }; @@ -61,12 +61,6 @@ describe('toolTakesOverCaller', () => { expect(toolTakesOverCaller(passing)).toBe(true); }); - it('only permits open to rerun an existing Tool, never take over a terminal', () => { - const opening: ToolTakeoverGate = { ...passing, verb: 'open', rawCommandLine: 'dor open README.md' }; - expect(toolTakesOverCaller(opening)).toBe(false); - expect(toolRerunsInCaller({ ...opening, kind: 'tool' })).toBe(true); - }); - it('splits when any condition fails', () => { const splits: Array<[string, Partial]> = [ ['--surface named a reference', { explicitSurface: true }], @@ -78,6 +72,8 @@ describe('toolTakesOverCaller', () => { ['the caller is a browser', { kind: 'browser' }], ['the shell reports no OSC 633', { oscDriven: false }], ['the line is not naked', { rawCommandLine: 'claude' }], + ['the line is compound', { rawCommandLine: `${passing.rawCommandLine} && echo done` }], + ['the verb does not match', { rawCommandLine: verb === 'tool' ? 'dor open README.md' : 'dor tool storybook' }], ['--cwd named another directory', { cwdMatches: false }], ]; for (const [why, override] of splits) { diff --git a/lib/src/components/wall/tool-takeover.ts b/lib/src/components/wall/tool-takeover.ts index 0603287dc..027a4668a 100644 --- a/lib/src/components/wall/tool-takeover.ts +++ b/lib/src/components/wall/tool-takeover.ts @@ -1,6 +1,6 @@ /** - * The take-over gate: `dor tool` typed alone at a prompt runs the tool in that - * pane instead of splitting (`docs/specs/dor-tool.md` -> Take-over). + * The take-over gate: `dor tool` or `dor open` typed alone at a prompt runs the + * tool in that pane instead of splitting (`docs/specs/dor-tool.md` -> Take-over). * * Pure predicates over facts the host has already read, so the placement rule is * testable without a Wall: the handler in `use-dor-control.ts` gathers the @@ -16,7 +16,7 @@ const COMPOUND_SYNTAX = /[;&|<>()`\n\r]/; /** * Whether the shell reported running exactly one command and that command is - * `dor tool` — the human-intent signal, not a security boundary + * the requested `dor` verb — the human-intent signal, not a security boundary * (`docs/specs/dor-tool.md` -> Take-over). Case folds on the launcher, which is * a filename, and not on the verb, which stricli parses case-sensitively. */ @@ -29,8 +29,7 @@ export function isNakedToolInvocation(rawCommandLine: string | null | undefined, /** What the placement rule reads. Every field is already known to the handler. */ export interface ToolTakeoverGate { - /** The `dor` verb the request came from: `open` never transforms a plain - * terminal, but may re-run its own Tool pane. */ + /** The `dor` verb the request came from. */ verb: 'tool' | 'open'; /** `--surface`: an explicit placement, which take-over must not override. */ explicitSurface: boolean; @@ -66,13 +65,12 @@ function callerTypedTool(gate: ToolTakeoverGate): boolean { } /** - * Whether this `dor tool` transforms its calling pane into the tool. Every + * Whether this Tool launch transforms its calling pane into the tool. Every * condition is conservative — failing one is a split, which is never wrong * (rationale). */ export function toolTakesOverCaller(gate: ToolTakeoverGate): boolean { - return gate.verb === 'tool' - && gate.workspaceActive + return gate.workspaceActive && !gate.explicitSurface && !gate.minimized && callerStillPlaceable(gate) From f1ea63a3b5116d846be56c16a44d8b5474b83585 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 16 Sep 2026 15:32:00 -0700 Subject: [PATCH 12/15] Keep Tool faces hidden with their workspace --- docs/specs/dor-tool.md | 2 ++ lib/src/components/wall/ToolPanel.test.tsx | 23 ++++++++++++++++++++-- lib/src/components/wall/ToolPanel.tsx | 4 +++- 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/docs/specs/dor-tool.md b/docs/specs/dor-tool.md index 25bbe8004..4e40372b6 100644 --- a/docs/specs/dor-tool.md +++ b/docs/specs/dor-tool.md @@ -134,6 +134,8 @@ Source of truth: `useToolServing` in `lib/src/components/wall/use-tool-serving.t **Must show the full terminal before serving and after command exit.** A serving Tool shows its browser, and Terminal Context reveals the same primary terminal (`docs/specs/terminal-context.md` → Tool context). Keep the browser mounted behind context, and keep the hidden terminal sized with `visibility` and `inert`, never `display: none`. Pending approval mounts neither capability. +**Must hide Tools in inactive Workspaces and minimized leaves without unmounting.** + Notepad follows `docs/specs/notepad.md` → Notepad UI. Tool context follows `docs/specs/terminal-context.md` → Tool context. Source of truth: `TerminalPane` in `lib/src/components/TerminalPane.tsx`; `focusSession` in `lib/src/lib/terminal-lifecycle.ts`; `ToolPanel` in `lib/src/components/wall/ToolPanel.tsx`; `ToolPaneHeader` in `lib/src/components/wall/ToolPaneHeader.tsx`; `toolLeafMeta` / `shouldParkOnMinimize` in `lib/src/components/wall/lath-wall-engine.ts`; `closeSurface` in `lib/src/components/Wall.tsx`. Tests: `lib/src/components/wall/ToolPanel.test.tsx`, `lib/src/components/Wall.test.tsx`, `lib/src/components/TerminalPane.test.tsx`, `lib/src/lib/terminal-registry.alert.test.ts`. diff --git a/lib/src/components/wall/ToolPanel.test.tsx b/lib/src/components/wall/ToolPanel.test.tsx index c0f5f724f..05b1c2bec 100644 --- a/lib/src/components/wall/ToolPanel.test.tsx +++ b/lib/src/components/wall/ToolPanel.test.tsx @@ -4,6 +4,8 @@ import { createRoot, type Root } from 'react-dom/client'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { ToolPanel } from './ToolPanel'; +globalThis.IS_REACT_ACT_ENVIRONMENT = true; + vi.mock('./TerminalPanel', () => ({ TerminalPanel: () =>
terminal
, })); @@ -53,6 +55,23 @@ describe('ToolPanel', () => { expect(container.querySelector('[data-testid="browser"]')).not.toBeNull(); }); + it.each([ + ['terminal', booting], + ['iframe', serving], + ['agent-browser', { ...serving, renderMode: 'ab-screencast' }], + ])('hides the %s face with its Workspace or parked leaf without remounting', (_face, params) => { + show(params); + const terminal = half('terminal'); + const browser = half('browser'); + container.style.visibility = 'hidden'; + expect(getComputedStyle(terminal).visibility).toBe('hidden'); + expect(getComputedStyle(browser).visibility).toBe('hidden'); + container.style.visibility = 'visible'; + expect(half('terminal')).toBe(terminal); + expect(half('browser')).toBe(browser); + expect(getComputedStyle('url' in params ? browser : terminal).visibility).toBe('visible'); + }); + it('hides with visibility, never display', () => { // A display:none container measures zero, so the fit addon would resize the // PTY to a degenerate size and reflow the output of the command still @@ -66,14 +85,14 @@ describe('ToolPanel', () => { it('shows the terminal and hides the browser before the tool serves', () => { show(booting); - expect(half('terminal').style.visibility).toBe('visible'); + expect(getComputedStyle(half('terminal')).visibility).toBe('visible'); expect(half('browser').style.visibility).toBe('hidden'); }); it('shows the browser once serving', () => { show(serving); expect(half('terminal').style.visibility).toBe('hidden'); - expect(half('browser').style.visibility).toBe('visible'); + expect(getComputedStyle(half('browser')).visibility).toBe('visible'); }); it('parks the browser while it is hidden, so a screencast stops decoding', () => { diff --git a/lib/src/components/wall/ToolPanel.tsx b/lib/src/components/wall/ToolPanel.tsx index f9dbdea39..ef545d13a 100644 --- a/lib/src/components/wall/ToolPanel.tsx +++ b/lib/src/components/wall/ToolPanel.tsx @@ -19,7 +19,9 @@ function Half({ shown, children }: { shown: boolean; children: React.ReactNode } return (