diff --git a/docs/specs/dor-tool.md b/docs/specs/dor-tool.md index 841419af4..43a78e3d2 100644 --- a/docs/specs/dor-tool.md +++ b/docs/specs/dor-tool.md @@ -238,6 +238,7 @@ Source of truth: `runFilePicker` in `dor/src/commands/open-picker.ts`; `listOpen - **Must resolve a Tool's document-link request through ordinary open dispatch and trust checks**, relative to the current canonical document's directory. The request chooses the current Surface or another Surface; it never requests a preview or a fresh instance. Existing matching Tools are focused through ordinary reuse without changing either Surface's history. - **Must keep Back/Forward history on the navigating Surface**, covering every file/folder handler, including user Tools. Traversal reopens the recorded canonical target with its selected handler and lookup directory in that Surface even when another Surface shows the target; a new in-place visit after Back drops the forward branch. Pin a preview on a committed in-place file visit. - **Must obtain Save / Discard / Cancel before replacing a dirty document**, using [Closing unsaved Tools](#closing-unsaved-tools). A same-document anchor or reuse of another Surface leaves the draft alone. Never cache drafts or undo state in history. +- **Must ask in the external-link dialog before a document link opens a page that runs its own script**, Back/Forward included (`docs/specs/security-local.md` -> "Terminal output"); a cancel is a navigation failure, and a same-document anchor never asks. - **Must validate the destination before interrupting the current Tool**, and keep the current location/history on lookup failure or cancelled consent. Refuse overlapping navigation from one Surface and stale requests whose source run changed. When the interrupt or the new run fails, keep or restore the source's run, never typing over user input. Report navigation failures on the source, without opening the preview error viewer. - **Must retain history through minimize, renderer changes, and live Workspace transfer, but never in durable saves**. Only the current document restarts after cold restore. Ending designation, Break, or closing discards history. Refuse Workspace transfer during navigation or pending reading-position restoration. - **Must release pending reading-position restoration on timeout**, even when the destination never serves a page or never answers; renderer rebinding must not renew the deadline. diff --git a/docs/specs/dor-tools-builtin.md b/docs/specs/dor-tools-builtin.md index b700083af..62525a1e6 100644 --- a/docs/specs/dor-tools-builtin.md +++ b/docs/specs/dor-tools-builtin.md @@ -57,7 +57,7 @@ Source of truth: `dor-tools-builtin/viewer/markdown.tsx`; `safeCreateDOM` in `do ### Relative links -- **Must follow a relative link on plain click**, and offer Open in another surface and Edit link on its context menu. Cmd/Ctrl-click chooses another Surface. Dispatch follows `docs/specs/dor-tool.md` → Document navigation. (rationale) +- **Must follow a relative link on plain click**, and offer Open in another surface and Edit link on its context menu. Cmd/Ctrl-click chooses another Surface. Dispatch, and any confirmation the host asks first, follows `docs/specs/dor-tool.md` → Document navigation. (rationale) - **Must open a local PNG or JPEG image's file the same way on Edit or double-click**, with a link's context menu whose edit item opens it. - **Must let an enclosing link own double-click and context-menu behavior.** - **Must treat only a scheme-less relative reference as a document link**, resolved from the opened canonical file's directory with parent references allowed and its query ignored; HTTP(S)/mail links keep their existing handling. diff --git a/docs/specs/security-local.md b/docs/specs/security-local.md index a2f796ffa..eac365411 100644 --- a/docs/specs/security-local.md +++ b/docs/specs/security-local.md @@ -10,6 +10,7 @@ | Guarantee | Feature | Pinned by | | --- | --- | --- | | **Terminal output cannot write your clipboard or steal focus.** File access requires a user action or the gated OSC 367 `open` of a Tool run Dormouse launched. OSC 52 only offers a copy format; links require confirmation, naming any app scheme, or an allowed local preview; deceptive links and dangerous Windows handlers have no open action. | | `lib/src/lib/terminal-protocol.test.ts`, `lib/src/lib/mouse-selection.test.ts`, `lib/src/lib/copy-editor.test.ts`, `lib/src/lib/external-links.test.ts`, `lib/src/lib/terminal-link-activation.test.ts`, `lib/src/components/ExternalLinkModalHost.test.tsx`, `lib/src/lib/tool-launch-latch.test.ts`, `lib/src/components/wall/preview-slot.test.tsx` | +| **A link in a document you are viewing cannot open a page that runs its own scripts without asking.** Dormouse's own dialog shows the file's full path first; Markdown, text, and images still open on a click. | | `lib/src/components/wall/preview-slot.test.tsx`, `lib/src/components/wall/tool-navigation.test.ts`, `lib/src/components/ExternalLinkModalHost.test.tsx` | The attacker is any program writing to a PTY. @@ -19,6 +20,8 @@ Output reaches the screen, Session state, and bounded terminal reports — rende **An `OSC 8` hyperlink opens only after a confirmation dialog**, except a local `file:` link whose display text names its target, which previews through the user's `open` rules (`docs/specs/dor-tool.md` -> "Terminal links"); any scheme but `http`, `https`, `mailto`, and `file` gets a dialog naming it as another app, and a target whose display text names a different host gets **no open action at all** (`docs/specs/mouse-and-clipboard.md` -> "OSC 8 hyperlinks"). File opens use `docs/specs/dor-tool.md` -> "Opening local files". +A Tool's document link names its own target, anywhere on disk under any link text (`docs/specs/dor-tool.md` -> "Document navigation"). + Notification text: `docs/specs/alert.md` -> "Text And Security". **Agent recovery is the one path from terminal output to typed input**: at shutdown each host interrupts every live PTY and scans its later output for an agent's resume hint, which the next cold restore types unconfirmed (`docs/compatible-agents.md` -> "Capture", "Cold restore"). It types a rebuilt invocation, never sliced output; what a terminal still chooses is `docs/specs/security.md` -> "What is not defended". @@ -34,6 +37,7 @@ Notification text: `docs/specs/alert.md` -> "Text And Security". - **FAIL IF** `S130` a host launches an external URL without revalidating it through `normalizeExternalUri` in `lib/src/lib/external-links.ts`, consent notwithstanding — VS Code's in the extension host (`vscode-ext/src/message-router.ts`), standalone's in `standalone/src/tauri-adapter.ts` and `openUrl` in `standalone/src/updater.ts` — or that function accepts a scheme in `PSEUDO_SCHEMES` or `REFUSED_APP_SCHEMES`, or the latter drops `ms-msdt`, `search-ms`, `ms-officecmd`, or `ms-appinstaller` (rationale). Pinned by `vscode-ext/test/message-router.test.ts` and `standalone/src/tauri-adapter.test.ts`. - **FAIL IF** `S131` an unconfirmed `OSC 8` activation reaches the preview path when its display text is not a whole-component suffix of its decoded target, or the host opens a `file:` URL whose host is not empty, `localhost`, or this machine: `localFileLinkPreviewPath` in `lib/src/lib/external-links.ts`, `activateTerminalLink` in `lib/src/lib/terminal-link-activation.ts`, `resolveLocalToolTarget` in `lib/src/host/tool-input.ts`. Pinned by `lib/src/lib/external-links.test.ts`, `lib/src/lib/terminal-link-activation.test.ts`, and `lib/src/host/tool-input.test.ts`. - **FAIL IF** `S132` an OSC 367 `open` reaches `surface.tool` from replay, from a Session that is not a Tool, from output outside the run the host launched in that PTY (`docs/specs/dor-tool.md` → OSC 367), or naming anything but an absolute path free of controls; if anything output reports, a shell-integration command line included, can begin that run; if a write but the host's own command launch can carry `launch`; or if a control-socket request can set its `oscOpen` flag, or its failure reaches the error viewer's argv with a control character. Read `parseToolOpen` in `dor-tools-lib/src/osc.ts`; `ToolLaunchLatch` in `lib/src/lib/tool-launch-latch.ts` and its one user, `createOwnerPtyStream` in `lib/src/host/owner-pty.ts`; the `launch` writes in `typeToolCommand` in `lib/src/components/wall/use-dor-control.ts`, `typeCommandWhenPromptReady` in `lib/src/lib/terminal-lifecycle.ts`, and `watchHelper` in `lib/src/lib/helper-terminal.ts` and each host's arm (`onPtyLaunch` in `lib/src/host/remote/sidecar-entry.ts`, `vscode-ext/src/message-router.ts`); `dispatchToolOpens` in `lib/src/lib/tool-open-requests.ts` (reached from `applyLiveToolEvents`, never from `parseReplay` in `lib/src/lib/platform/replay-parse.ts`); the `oscOpen` argument of `dispatchDorControlRequest` in `lib/src/lib/platform/dor-control-dispatch.ts`; and the `oscOpen` gate in `lib/src/components/wall/use-dor-control.ts`. Pinned by `lib/src/lib/tool-launch-latch.test.ts` and `lib/src/components/wall/preview-slot.test.tsx`. +- **FAIL IF** `S269` a document-link `open` — one carrying a disposition, Back/Forward through a Surface's history included — runs or focuses a target `builtin:file` would serve as a page running that file's own script, under any handler but `builtin:code` or `builtin:folder`, before the user confirms its full resolved path in host UI the Tool's page cannot reach; or a cancelled, replaced, or aborted confirmation lets it proceed; or `runsOwnScript` misses a format the viewer CSP lets run script. Read `documentLinkNeedsConfirmation` in `lib/src/components/wall/tool-navigation.ts`, `runsOwnScript` in `dor-tools-builtin/src/file-viewer-format.ts`, its call in the `surface.tool` handler of `lib/src/components/wall/use-dor-control.ts`, and `confirmDocumentLinkOpen` in `lib/src/lib/external-link-confirmation.ts`. Pinned by `lib/src/components/wall/preview-slot.test.tsx`, `lib/src/components/wall/tool-navigation.test.ts`, and `lib/src/components/ExternalLinkModalHost.test.tsx`. - **FAIL IF** `S133` cold restore types a command `normalizeResumeCommand` in `lib/src/lib/resume-patterns.ts` did not rebuild at the typing boundary (`restoreTerminal` in `lib/src/lib/terminal-lifecycle.ts`), its grammar admits more than the invocations `docs/compatible-agents.md` -> "Detection" names — another argument, whitespace, or a shell metacharacter — or capture scans output from before its first interrupt (`captureAgentRecovery` in `lib/src/host/recovery-capture.ts`). Pinned by `lib/src/lib/coding-agents.test.ts`, `lib/src/lib/terminal-registry.alert.test.ts`, and `lib/src/host/recovery-capture.test.ts`. ## Browser panes @@ -165,7 +169,7 @@ The attacker is another local account reading disk; what the remote stack leaves No standalone log call (`docs/specs/standalone.md` -> "Logging") carries PTY bytes, but the log records the `dor` socket path (rationale). - **FAIL IF** `S162` `write_file_atomically` in `standalone/src-tauri/src/lib.rs` stops restricting the directory and the file it writes to the owning user on **every** platform `restrict_to_owner` has an arm for — `0700`/`0600` on unix, and on Windows a DACL protected from inheritance carrying exactly one ACE for the current user — or the mode stops reaching the temp file *before* any bytes are written, or **any** of its callers stops going through it. Enumerate them from the file rather than from this line: every writer under the state root is one, the legacy-transcript scrub and `arrivals.json` included. The one exception is the macOS hang sample `/usr/bin/sample` writes into `hangs/`; it goes through `write_owner_only_with`, which restricts the directory before the child runs and the file after. Pinned by the Rust tests in the same file (rationale). -- **FAIL IF** `S163` a standalone log handle opens other than through `open_log` in `standalone/src-tauri/src/lib.rs`, or `open_log` stops leaving the log owner-only on **every** platform — unix: `0600`, an existing file tightened, a symlink refused; Windows: `restrict_to_owner`'s DACL — or the Linux default is not a per-user directory whenever `XDG_STATE_HOME` or `HOME` is absolute. Pinned by the Rust tests in the same file (rationale). +- **FAIL IF** `S163` a standalone log handle, read or write, opens other than through `open_log` in `standalone/src-tauri/src/lib.rs`, or `open_log` stops leaving the log owner-only on **every** platform — unix: `0600`, an existing file tightened, a symlink or anything but a regular file refused; Windows: `restrict_to_owner`'s DACL — or the Linux default is not a per-user directory whenever `XDG_STATE_HOME` or `HOME` is absolute. Pinned by the Rust tests in the same file (rationale). Source of truth: `SESSION_STATE_KEY` in `vscode-ext/src/session-state.ts`, `ensureToken` in `vscode-ext/src/peer-link.ts`, `default_log_location` in `standalone/src-tauri/src/lib.rs`, `createManagedVoiceHost` in `lib/src/host/managed-voice-host.ts`. @@ -190,7 +194,7 @@ The attacker is a file planted in a folder the user opens: Windows looks a bare - **FAIL IF** `S165` a host, `dor`, or a built-in Tool spawns a program by bare name on Windows: a system program goes by its path under `%SystemRoot%` (`cmd.exe` by `ComSpec`), a user-installed one such as git by the file `docs/specs/dor-cli.md` -> "Spawning External Binaries" resolves on `PATH`, or not at all. Search `standalone/sidecar/`, `lib/src/host/`, `vscode-ext/src/`, `dor/src/`, `dor-lib-common/src/`, and `dor-tools-builtin/src/`. Pinned by `standalone/sidecar/pty-core.test.js`, `standalone/sidecar/clipboard-ops.test.js`, and `lib/src/host/git-upstream.test.ts`. - **FAIL IF** `S166` the VS Code pty host forks in any cwd but the extension's own directory. Pinned by `vscode-ext/test/pty-manager.test.ts`. - **FAIL IF** `S167` git run in a repository the user has not trusted can run a program that repository's config names: every git spawn in the directories above must lead with `UNTRUSTED_REPO_GIT_ARGS` and run only a subcommand its comment says it covers. Pinned by `lib/src/host/git-cli.test.ts`, `dor/test/open-picker.test.mjs`, and `dor-tools-builtin/test/folder-viewer.test.mjs`, each planting a `core.fsmonitor`. -- **FAIL IF** `S168` a host spawns a browser `binaryPath` the gate did not accept, or for Playwright loads `playwright-core` beside one, or `dor` runs such a pinned executable; the gate accepts, beyond the host's own override value by exact match, a UNC or device path, a relative path but the bare executable name, a `..` segment, a control character, or a file name other than the provider's executable; or the webview sends or stores one it refuses. The path crosses from the webview realm, so a same-user writer is no reason to rate a break below WARNING. Read `isAllowedBrowserBinary` in `dor-lib-common/src/browser-providers.ts`, `runWithBinaryFallback` in `lib/src/host/agent-browser-host.ts`, `findPlaywrightInstall` in `lib/src/host/playwright-install.ts`, the pin in `dor/src/commands/browser-cli.ts`, and the callers of `isAllowedBinaryFor` in `lib/src/lib/agent-browser-binary.ts`. Pinned by `lib/src/lib/agent-browser-binary.test.ts`, `lib/src/components/Wall.test.tsx`, and `dor/test/playwright.test.mjs`. +- **FAIL IF** `S168` a host spawns a browser `binaryPath` the gate did not accept, or for Playwright loads `playwright-core` beside one, or `dor` runs such a pinned executable; the gate accepts, beyond the host's own override value by exact match, a UNC or device path, a relative path but the bare executable name, a `..` segment, a C0 or C1 control character or DEL, or a file name other than the provider's executable; or the webview sends or stores one it refuses. The path crosses from the webview realm, so a same-user writer is no reason to rate a break below WARNING. Read `isAllowedBrowserBinary` in `dor-lib-common/src/browser-providers.ts`, `runWithBinaryFallback` in `lib/src/host/agent-browser-host.ts`, `findPlaywrightInstall` in `lib/src/host/playwright-install.ts`, the pin in `dor/src/commands/browser-cli.ts`, and the callers of `isAllowedBinaryFor` in `lib/src/lib/agent-browser-binary.ts`. Pinned by `lib/src/lib/agent-browser-binary.test.ts`, `lib/src/components/Wall.test.tsx`, and `dor/test/playwright.test.mjs`. - **FAIL IF** `S169` the Playwright host loads `@playwright/cli` or `playwright-core` from outside the launcher's own installation: a directory above it, a shim script path that climbs out of the shim's directory, or a `playwright-core` neither nested in the CLI package nor beside it in its `node_modules`, though Node's lookup reaches further. Read `findPlaywrightInstall` in `lib/src/host/playwright-install.ts`. Pinned by `lib/src/host/playwright-install.test.ts`. Source of truth: `windowsSystemPath` in `standalone/sidecar/windows-system.js`; `resolveBinaryPath` in `dor-lib-common/src/resolve-binary.ts`; `ensureChild` in `vscode-ext/src/pty-manager.ts`; `UNTRUSTED_REPO_GIT_ARGS` in `dor-lib-common/src/git.ts`. diff --git a/docs/specs/security-local.rationale.md b/docs/specs/security-local.rationale.md index dcfbf8663..5de457e0b 100644 --- a/docs/specs/security-local.rationale.md +++ b/docs/specs/security-local.rationale.md @@ -82,7 +82,7 @@ Why session snapshots earn the strongest protection on disk. They are `Persisted Why the peer-link token is listed here. It is a `randomUUID()` in the VS Code extension's global storage, and its own comment says it is the only thing between another local process and this installation's terminals — a local credential at rest that the remote spec's credentials table does not cover, because nothing about it is remote. Both of its controls are unix-only: `peerDirIsSafe()` returns true immediately on `win32`, and Node's `mode: 0o600` there touches only the read-only attribute, so unlike `burrow_state_dir` no DACL work is done for it. -Why the standalone log is guarded at all. The socket path arrives via the sidecar's stderr, which Rust appends verbatim. Linux used to fall back to `env::temp_dir()`, which with `TMPDIR` unset is `/tmp` at `1777`, and still does without a usable `HOME`: another account could pre-plant `dormouse.log` as a symlink, which the truncating open followed, or as a world-writable file of its own, which then collected the log. Kernel `fs.protected_symlinks` / `fs.protected_regular` blunt both only where a distribution enables them (2026-10 audit, #1027). macOS's `$TMPDIR` is the per-user `/var/folders/.../T` at `0700` (measured 2026-09). The open is hardened as well as the directory moved because `$DORMOUSE_LOG_FILE` can point anywhere. +Why the standalone log is guarded at all. The socket path arrives via the sidecar's stderr, which Rust appends verbatim. Linux used to fall back to `env::temp_dir()`, which with `TMPDIR` unset is `/tmp` at `1777`, and still does without a usable `HOME`: another account could pre-plant `dormouse.log` as a symlink, which the truncating open followed, or as a world-writable file of its own, which then collected the log. Kernel `fs.protected_symlinks` / `fs.protected_regular` blunt both only where a distribution enables them (2026-10 audit, #1027). macOS's `$TMPDIR` is the per-user `/var/folders/.../T` at `0700` (measured 2026-09). The open is hardened as well as the directory moved because `$DORMOUSE_LOG_FILE` can point anywhere. The read takes the same open because the webview asks for the log's tail to attach to an update report: a symlink at the log's name would hand it another file's tail, and a FIFO would hang the read. ## Local-file viewer diff --git a/docs/specs/security.md b/docs/specs/security.md index 432eddc54..f6c623f95 100644 --- a/docs/specs/security.md +++ b/docs/specs/security.md @@ -44,8 +44,8 @@ Source of truth: root scripts in `package.json`; native jobs in `.github/workflo - **A process running as you.** `dor`, its socket, and every file mode bound other local accounts, never a program already running under your own account; an agent holding `dor` has exactly the power of the person at the keyboard, and the Tool trust prompt is no boundary against it ([The dor control socket](./security-local.md#the-dor-control-socket)). - **The Windows `dor` pipe carries no ACL of ours.** A named pipe has no directory to harden, so an unguessable name and the token handshake are the whole of it ([The dor control socket](./security-local.md#the-dor-control-socket)). -- **A local HTML or SVG document you open can send its contents off the machine.** The file viewer's content policy confines what the page loads, not where its scripts navigate ([Local-file viewer](./security-local.md#local-file-viewer)). -- **A mermaid sanitizer bypass in a Markdown document you open.** Its diagrams reach the editor page through mermaid's strict sanitizer, not the editor's allowlist, so a bypass would run script holding the editor's save and image routes ([Local-file viewer](./security-local.md#local-file-viewer)). +- **A local HTML or SVG document you open, or confirm from a document link, can send its contents off the machine.** The file viewer's content policy confines what the page loads, not where its scripts navigate ([Local-file viewer](./security-local.md#local-file-viewer)). +- **A mermaid sanitizer bypass in a Markdown document you open.** Its diagrams reach the editor page through mermaid's strict sanitizer, not the editor's allowlist, so a bypass would run script holding the editor's save, image, and document-open routes; an open of a page that runs its own scripts still waits on Dormouse's own dialog ([Local-file viewer](./security-local.md#local-file-viewer), [Terminal output](./security-local.md#terminal-output)). - **A Tool trust grant on an upstream URL trusts the URL a checkout claims.** Any directory whose own `.git/config` claims an already-granted upstream shares that grant; a folder-only grant is not shared ([Dor Tool configuration](./security-local.md#dor-tool-configuration)). - **Output already flowing into a Tool's terminal at launch** can, between Dormouse typing the command and the shell reporting it, make that run's OSC 367 `open` requests ([Terminal output](./security-local.md#terminal-output)). - **A terminal can choose which of your own agent sessions resumes.** Output a pane prints as Dormouse quits can name any session of a supported agent, which the next restore resumes in that pane's directory without asking; it can type nothing else ([Terminal output](./security-local.md#terminal-output)). diff --git a/docs/specs/terminal-escapes.md b/docs/specs/terminal-escapes.md index e8cbb51d4..95f292d5a 100644 --- a/docs/specs/terminal-escapes.md +++ b/docs/specs/terminal-escapes.md @@ -78,7 +78,7 @@ Replay (`pty:replay`) is the raw stream requiring re-parse: **the webview runs a | `OSC 777 ; notify ; ; <body> ST` | rxvt/WezTerm notification | [alert.md](alert.md#terminal-reports) | | `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 ; state ; <json> ST` | Tool-reported unsaved state | [dor-tool.md](dor-tool.md#unsaved-changes) | -| `OSC 367 ; open ; <json> ST` | A Tool run's request to open a local path as `dor open`, preview or not; live output of the run the host launched only | [dor-tool.md](dor-tool.md#osc-367) | +| `OSC 367 ; open ; <json> ST` | A Tool run's request to open a local path as `dor open`, preview or not, or with a disposition and optional fragment as a document-link navigation; live output of the run the host launched only | [dor-tool.md](dor-tool.md#osc-367), [document navigation](dor-tool.md#document-navigation) | | `OSC 367 ; dehydrate ; <json> ST` | A stopping Tool's restore payload; live output only | [dor-tool.md](dor-tool.md#reaping) | | `OSC 367 ; <any other verb> ST` | 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) | diff --git a/dor-tools-builtin/src/file-viewer-format.ts b/dor-tools-builtin/src/file-viewer-format.ts index c6c8cebc7..21da08071 100644 --- a/dor-tools-builtin/src/file-viewer-format.ts +++ b/dor-tools-builtin/src/file-viewer-format.ts @@ -79,6 +79,14 @@ export function builtinHandler(by: 'tool' | 'kind' | 'argv', value: unknown): Bu return BUILTIN_HANDLERS.find(handler => handler[by] === value); } +/** Whether `builtin:file` serves this format as a page that runs the file's + * own script: HTML, and SVG, which it serves as its own document under the + * viewer CSP's inline scripts. Everything else it shows inertly or in its own + * shipped editor. */ +export function runsOwnScript(format: FileFormat): boolean { + return !format.text && /^(text\/html|image\/svg\+xml)\b/.test(format.mime); +} + /** The built-in a folder or a file opens with when no rule matches. */ export function defaultBuiltin(folder: boolean): BuiltinHandler { return BUILTIN_HANDLERS.find(handler => handler.opens === (folder ? 'folder' : 'file'))!; diff --git a/lib/src/components/ExternalLinkModal.tsx b/lib/src/components/ExternalLinkModal.tsx index 16c908764..ae208db93 100644 --- a/lib/src/components/ExternalLinkModal.tsx +++ b/lib/src/components/ExternalLinkModal.tsx @@ -18,6 +18,8 @@ export interface ExternalLinkModalRequest { displayText: string; verdict: DisplayMatchVerdict; decision: ExternalUriDecision; + /** Why this open is asked about, below the target. */ + note?: string; } // "Open ___" button label suffix: the scheme noun sits next to the cursor, so @@ -107,6 +109,9 @@ export function ExternalLinkModal({ {displayUri} </ModalReviewBlock> + {request.note && openableDecision && !isDeceptive && ( + <p className="mt-3 text-xs text-muted">{request.note}</p> + )} {openableDecision?.otherApp && !isDeceptive && ( <p className="mt-3 text-xs text-muted"> Dormouse hands this link to whichever app is registered for{' '} diff --git a/lib/src/components/ExternalLinkModalHost.test.tsx b/lib/src/components/ExternalLinkModalHost.test.tsx index 99a8f7809..84c9f1300 100644 --- a/lib/src/components/ExternalLinkModalHost.test.tsx +++ b/lib/src/components/ExternalLinkModalHost.test.tsx @@ -1,10 +1,10 @@ /** @vitest-environment jsdom */ import { act } from 'react'; import { createRoot, type Root } from 'react-dom/client'; -import { afterEach, beforeEach, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { ExternalLinkModalHost } from './ExternalLinkModalHost'; import type { DorControlResult } from 'dor/protocol'; -import { clearExternalLinkConfirmation, getExternalLinkConfirmationSnapshot, requestExternalLinkConfirmation } from '../lib/external-link-confirmation'; +import { clearExternalLinkConfirmation, confirmDocumentLinkOpen, getExternalLinkConfirmationSnapshot, requestExternalLinkConfirmation } from '../lib/external-link-confirmation'; const mocks = vi.hoisted(() => ({ open: vi.fn(), toolControl: vi.fn(), confirm: () => {} })); vi.mock('../lib/platform', () => ({ getPlatform: () => ({ openExternal: mocks.open, toolControl: mocks.toolControl }) })); @@ -201,3 +201,40 @@ it('explains a file link with no originating terminal instead of passing it to t expect(mocks.open).not.toHaveBeenCalled(); expect(requests).toHaveLength(0); }); + +describe('a document link\'s confirmation', () => { + const path = '/elsewhere/page.html'; + const ask = (signal?: AbortSignal) => { + let answer!: Promise<boolean>; + act(() => { answer = confirmDocumentLinkOpen(path, signal); }); + return answer; + }; + + it('shows the full path and answers yes only on an explicit open, opening nothing itself', async () => { + const answer = ask(); + expect(document.body.textContent).toContain(path); + expect(document.activeElement?.textContent).toBe('Cancel'); + act(() => button('Open file').click()); + expect(await answer).toBe(true); + expect(getExternalLinkConfirmationSnapshot()).toBeNull(); + expect(mocks.open).not.toHaveBeenCalled(); + expect(requests).toHaveLength(0); + }); + + it('answers no on cancel, on another link replacing it, and on abort', async () => { + const cancelled = ask(); + act(() => button('Cancel').click()); + expect(await cancelled).toBe(false); + + const replaced = ask(); + act(() => requestExternalLinkConfirmation('https://example.com/')); + expect(await replaced).toBe(false); + act(() => clearExternalLinkConfirmation()); + + const controller = new AbortController(); + const aborted = ask(controller.signal); + act(() => controller.abort()); + expect(await aborted).toBe(false); + expect(getExternalLinkConfirmationSnapshot()).toBeNull(); + }); +}); diff --git a/lib/src/components/ExternalLinkModalHost.tsx b/lib/src/components/ExternalLinkModalHost.tsx index 8b3b42875..7512035c6 100644 --- a/lib/src/components/ExternalLinkModalHost.tsx +++ b/lib/src/components/ExternalLinkModalHost.tsx @@ -19,7 +19,18 @@ export function ExternalLinkModalHost() { ); useDialogKeyboardOwner(pending !== null); // Each click owns its lookup, selection and launch result, even for the same URL. - return pending ? <PendingLinkDialog key={pending.id} request={pending} /> : null; + if (!pending) return null; + return pending.answer ? <PendingAnswerDialog key={pending.id} request={pending} /> : <PendingLinkDialog key={pending.id} request={pending} />; +} + +/** A question another flow awaits: confirming only answers it. */ +function PendingAnswerDialog({ request }: { request: PendingExternalLink }) { + const confirm = () => { + if (getExternalLinkConfirmationSnapshot() !== request) return; + request.answer!(true); + clearExternalLinkConfirmation(); + }; + return <ExternalLinkModal request={request} onCancel={clearExternalLinkConfirmation} onConfirm={confirm} />; } function PendingLinkDialog({ request }: { request: PendingExternalLink }) { diff --git a/lib/src/components/wall/preview-slot.test.tsx b/lib/src/components/wall/preview-slot.test.tsx index e6007b91a..2ae929f28 100644 --- a/lib/src/components/wall/preview-slot.test.tsx +++ b/lib/src/components/wall/preview-slot.test.tsx @@ -1328,6 +1328,61 @@ describe('Tool document navigation', () => { }); +describe('a document link to a page that runs its own script', () => { + /** The Tool in `surface:slot` follows a document link, as its OSC 367 `open`. */ + const follow = (path: string) => act(async () => applyLiveToolEvents('surface:slot', + [{ kind: 'toolOpen', open: { path, preview: false, disposition: 'current' } }])); + const dialogButton = (label: string) => [...document.body.querySelectorAll('button')].find(button => button.textContent === label)!; + /** Wait for the slot to show `target`. */ + async function untilTarget(target: string): Promise<void> { + for (let tries = 0; tries < 100; tries++) { + if ((await paramsOf('surface:slot'))?.toolTarget === target) return; + await act(async () => { await new Promise(resolve => setTimeout(resolve, 20)); }); + } + expect((await paramsOf('surface:slot'))?.toolTarget).toBe(target); + } + + it('waits for the user to confirm its full path in host UI, and a cancel runs nothing', async () => { + await mountSlot(); + await follow('/elsewhere/page.html'); + await waitUntil(() => getExternalLinkConfirmationSnapshot() !== null); + expect(getExternalLinkConfirmationSnapshot()?.uri).toBe('/elsewhere/page.html'); + expect(document.body.textContent).toContain('/elsewhere/page.html'); + await flush(); + expect(typed['surface:slot']).toEqual([]); + expect((await paramsOf('surface:slot'))?.toolTarget).toBe('/repo/a.md'); + + act(() => dialogButton('Cancel').click()); + await flush(); + expect(typed['surface:slot']).toEqual([]); + expect((await paramsOf('surface:slot'))?.toolTarget).toBe('/repo/a.md'); + + await follow('/elsewhere/page.html'); + await waitUntil(() => getExternalLinkConfirmationSnapshot() !== null); + act(() => dialogButton('Open file').click()); + await untilTarget('/elsewhere/page.html'); + }); + + it('opens an inert target on the click alone, and asks again on Forward to the page', async () => { + await mountSlot(); + await follow('/elsewhere/notes.md'); + await untilTarget('/elsewhere/notes.md'); + expect(getExternalLinkConfirmationSnapshot()).toBeNull(); + await follow('/elsewhere/diagram.svg'); + await waitUntil(() => getExternalLinkConfirmationSnapshot() !== null); + act(() => dialogButton('Open file').click()); + await untilTarget('/elsewhere/diagram.svg'); + await answer(await request({ history: 'back', disposition: 'current' }, 'surface:slot')); + expect(getExternalLinkConfirmationSnapshot()).toBeNull(); + const forward = await request({ history: 'forward', disposition: 'current' }, 'surface:slot'); + await waitUntil(() => getExternalLinkConfirmationSnapshot()?.uri === '/elsewhere/diagram.svg'); + act(() => clearExternalLinkConfirmation()); + await waitUntil(() => forward.mock.calls.length > 0); + expect(forward.mock.calls[0][0]).toMatchObject({ ok: false, error: 'Navigation cancelled.' }); + expect((await paramsOf('surface:slot'))?.toolTarget).toBe('/elsewhere/notes.md'); + }); +}); + it('document navigation Cancel keeps the draft and history; Discard permits replacement', async () => { await mountSlot(); recordToolDirty('surface:slot', true); diff --git a/lib/src/components/wall/tool-navigation.test.ts b/lib/src/components/wall/tool-navigation.test.ts index 1b5cb1f19..51bea540d 100644 --- a/lib/src/components/wall/tool-navigation.test.ts +++ b/lib/src/components/wall/tool-navigation.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { commitToolHistory, toolHistory } from './tool-navigation'; +import { commitToolHistory, documentLinkNeedsConfirmation, toolHistory } from './tool-navigation'; import { directoryOf } from '../../lib/tool-open-requests'; const location = (target: string, top = 0) => ({ target, cwd: '/repo', handler: 'builtin:file', position: { top } }); @@ -28,3 +28,19 @@ describe('Tool document history', () => { expect(directoryOf('C:\\repo\\a.md')).toBe('C:\\repo'); }); }); + +describe('which document links wait for confirmation', () => { + it('asks before a page that runs its own script, under any handler that would render it', () => { + for (const target of ['/x/page.html', '/x/PAGE.HTM', '/x/diagram.svg', 'C:\\x\\page.html']) { + expect(documentLinkNeedsConfirmation('builtin', 'file', target), target).toBe(true); + expect(documentLinkNeedsConfirmation('user', 'viewer', target), target).toBe(true); + } + }); + it('opens inert targets, and active ones shown as source or as a folder, on the click alone', () => { + for (const target of ['/x/notes.md', '/x/a.txt', '/x/photo.png', '/x/anim.gif', '/x/clip.mp4', '/x/script.js', '/x/feed.xml', '/x/doc.pdf', '/x/README']) { + expect(documentLinkNeedsConfirmation('builtin', 'file', target), target).toBe(false); + } + expect(documentLinkNeedsConfirmation('builtin', 'code', '/x/page.html')).toBe(false); + expect(documentLinkNeedsConfirmation('builtin', 'folder', '/x/site.html')).toBe(false); + }); +}); diff --git a/lib/src/components/wall/tool-navigation.ts b/lib/src/components/wall/tool-navigation.ts index a1bc87c2e..059c69fc3 100644 --- a/lib/src/components/wall/tool-navigation.ts +++ b/lib/src/components/wall/tool-navigation.ts @@ -1,7 +1,7 @@ import { readToolViewPosition, type ToolViewPosition } from 'dor-tools-lib/protocol'; import { SURFACE_CONTROL_METHODS } from 'dor/protocol'; import { parseRenderMode } from 'dor-lib-common/browser-providers'; -import { builtinHandler } from 'dor-tools-builtin/file-viewer-format'; +import { builtinHandler, fileViewerFormat, runsOwnScript } from 'dor-tools-builtin/file-viewer-format'; import { getPlatform } from '../../lib/platform'; import { dispatchDorControlRequest } from '../../lib/platform/dor-control-dispatch'; import { toolEditorView } from '../../lib/tool-editor'; @@ -34,6 +34,21 @@ export function toolHandlerName(scope: unknown, name: unknown): string | undefin return scope === 'builtin' ? builtinHandler('kind', name)?.tool : name; } +/** + * Whether a document-link open of `target`, resolved to the handler `scope` and + * `name` name, waits for the user to confirm it in host UI: the target is a + * format whose built-in preview runs the file's own script, and the handler is + * not a built-in that shows it inertly. A user or repository Tool given such a + * file is assumed to render it too. A link may climb with `..` to anywhere on + * disk under any link text, so the page cannot be the one that asks. + */ +export function documentLinkNeedsConfirmation(scope: unknown, name: unknown, target: string): boolean { + const format = fileViewerFormat(target); + if (!format || !runsOwnScript(format)) return false; + const handler = toolHandlerName(scope, name); + return handler !== 'builtin:code' && handler !== 'builtin:folder'; +} + export function toolLocation(params: Record<string, unknown>, cwd: string): ToolLocation | null { if (typeof params.toolTarget !== 'string') return null; return { target: params.toolTarget, handler: toolHandlerName(params.toolScope, params.toolName), cwd }; diff --git a/lib/src/components/wall/use-dor-control.ts b/lib/src/components/wall/use-dor-control.ts index 92858fccb..f692f4dcf 100644 --- a/lib/src/components/wall/use-dor-control.ts +++ b/lib/src/components/wall/use-dor-control.ts @@ -75,8 +75,9 @@ import { commitPreviewTransition, endPreviewTransition } from '../../lib/preview import { retireToolRun } from './use-tool-serving'; import { validToolFragment } from 'dor-tools-lib/osc'; import { confirmToolEditorsClose } from '../../lib/tool-editor'; +import { confirmDocumentLinkOpen } from '../../lib/external-link-confirmation'; import { directoryOf } from '../../lib/tool-open-requests'; -import { commitToolHistory, hasToolView, toolHandlerName, toolHistory, toolLocation, toolNavigationPending, toolView } from './tool-navigation'; +import { commitToolHistory, documentLinkNeedsConfirmation, hasToolView, toolHandlerName, toolHistory, toolLocation, toolNavigationPending, toolView } from './tool-navigation'; import { listenerUrlsByPort } from './port-url'; import { becomeToolMeta, dorDirectionForEdge, toolLeafMeta, type LathWallEngine } from './lath-wall-engine'; @@ -1451,6 +1452,24 @@ export function useDorControl({ toolRun = argv!; } + // A document link's target is the Tool's to name: a Markdown link can + // climb with `..` to anywhere on disk under any link text. One whose + // preview would run that file's own script therefore opens only once + // the user confirms its full path here, in host UI the page cannot + // reach. An anchor in the document already shown loads nothing new. + // Back/Forward asks again: re-asking costs one click, and no rule then + // depends on how a history entry got there. + if (documentLink && openTarget !== undefined + && !(sourceParams!.toolTarget === openTarget && sourceParams!.toolName === toolName && sourceParams!.toolScope === toolScope) + && documentLinkNeedsConfirmation(toolScope, toolName, openTarget)) { + const confirmed = await confirmDocumentLinkOpen(openTarget, detail.signal); + if (unavailable()) return; + if (!confirmed) { + detail.respond({ ok: false, error: 'Navigation cancelled.' }); + return; + } + } + const identity: ToolIdentityParams = { command, ...(typeof toolRun === 'string' ? {} : { toolArgv: [...toolRun] }), diff --git a/lib/src/lib/external-link-confirmation.ts b/lib/src/lib/external-link-confirmation.ts index d8fab5301..1938274e1 100644 --- a/lib/src/lib/external-link-confirmation.ts +++ b/lib/src/lib/external-link-confirmation.ts @@ -17,6 +17,11 @@ export interface PendingExternalLink { displayText: string; verdict: DisplayMatchVerdict; decision: ExternalUriDecision; + /** What the dialog adds below the target. */ + note?: string; + /** Set when another flow awaits the decision: the dialog only answers, and + * that flow opens. Called once; a cancel, replacement, or abort answers false. */ + answer?: (confirmed: boolean) => void; } let nextId = 0; @@ -24,20 +29,64 @@ let pendingExternalLink: PendingExternalLink | null = null; const listeners = new Set<() => void>(); export function requestExternalLinkConfirmation(uri: string, displayText: string = '', source?: ExternalLinkSource): void { - pendingExternalLink = { + replacePending({ id: ++nextId, source, uri, displayText, verdict: classifyDisplayMatch(uri, displayText), decision: inspectExternalUri(uri), + }); +} + +/** What the dialog shows for a document link to the local file `path`. */ +export function documentLinkRequest(path: string): Omit<PendingExternalLink, 'id' | 'answer'> { + return { + uri: path, + displayText: '', + verdict: 'match', + decision: { status: 'openable', rawUri: path, uri: path, scheme: 'file', displayUri: path, otherApp: false }, + note: 'A link in the document you are viewing opens this file, which runs its own scripts and can read files beside it. Open it only if you expected this file.', }; - emitExternalLinkConfirmationChange(); +} + +/** + * Ask, in the same dialog, before a Tool's document link opens the local file + * `path`, shown in full; resolves whether the user confirmed. The caller does + * the open. Aborting `signal` withdraws the question. + */ +export function confirmDocumentLinkOpen(path: string, signal?: AbortSignal): Promise<boolean> { + return new Promise(resolve => { + if (signal?.aborted) return resolve(false); + let answered = false; + const request: PendingExternalLink = { + id: ++nextId, + ...documentLinkRequest(path), + answer: confirmed => { + if (answered) return; + answered = true; + signal?.removeEventListener('abort', withdraw); + resolve(confirmed); + }, + }; + const withdraw = () => { + if (pendingExternalLink === request) clearExternalLinkConfirmation(); + else request.answer!(false); + }; + signal?.addEventListener('abort', withdraw, { once: true }); + replacePending(request); + }); } export function clearExternalLinkConfirmation(): void { if (!pendingExternalLink) return; - pendingExternalLink = null; + replacePending(null); +} + +function replacePending(next: PendingExternalLink | null): void { + const previous = pendingExternalLink; + pendingExternalLink = next; + previous?.answer?.(false); emitExternalLinkConfirmationChange(); } diff --git a/lib/src/stories/ExternalLinkModal.stories.tsx b/lib/src/stories/ExternalLinkModal.stories.tsx index f20373c7f..d79a07845 100644 --- a/lib/src/stories/ExternalLinkModal.stories.tsx +++ b/lib/src/stories/ExternalLinkModal.stories.tsx @@ -2,8 +2,9 @@ import { useState, type ComponentProps } from 'react'; import type { Meta, StoryObj } from '@storybook/react'; import { ExternalLinkModal } from '../components/ExternalLinkModal'; import { classifyDisplayMatch, inspectExternalUri } from '../lib/external-links'; +import { documentLinkRequest } from '../lib/external-link-confirmation'; -function ExternalLinkModalStory({ uri, displayText, ...dialog }: { uri: string; displayText: string } & Pick<ComponentProps<typeof ExternalLinkModal>, 'handlers' | 'error' | 'busy'>) { +function ExternalLinkModalStory({ uri, displayText, documentLink = false, ...dialog }: { uri: string; displayText: string; documentLink?: boolean } & Pick<ComponentProps<typeof ExternalLinkModal>, 'handlers' | 'error' | 'busy'>) { const [selected, setSelected] = useState(0); return ( <div className="relative h-[360px] w-[680px] overflow-hidden rounded bg-app-bg font-mono text-terminal-fg"> @@ -15,7 +16,7 @@ function ExternalLinkModalStory({ uri, displayText, ...dialog }: { uri: string; {...dialog} selected={selected} onSelect={setSelected} - request={{ + request={documentLink ? documentLinkRequest(uri) : { uri, displayText, verdict: classifyDisplayMatch(uri, displayText), @@ -131,3 +132,13 @@ export const FileOpenFailed: Story = { error: 'no such file or folder: file:///Users/dev/project/image.png', }, }; + +// A Markdown link whose target runs its own scripts: the Tool's page named it, +// so the host asks with the full resolved path before it opens. +export const DocumentLinkToActivePage: Story = { + args: { + uri: '/Users/dev/Downloads/report/index.html', + displayText: '', + documentLink: true, + }, +}; diff --git a/scripts/security-rule-ids.json b/scripts/security-rule-ids.json index 2ab8934c3..18985fdaf 100644 --- a/scripts/security-rule-ids.json +++ b/scripts/security-rule-ids.json @@ -1,5 +1,5 @@ { - "next": 269, + "next": 270, "retired": [ 265 ] diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index a2667e3b5..f5e2578a8 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -8,8 +8,8 @@ "docs/specs/deploy.md": 2200, "docs/specs/dor-browser.md": 5800, "docs/specs/dor-cli.md": 5150, - "docs/specs/dor-tool.md": 6500, - "docs/specs/dor-tools-builtin.md": 1650, + "docs/specs/dor-tool.md": 6550, + "docs/specs/dor-tools-builtin.md": 1700, "docs/specs/dor-tools-lib.md": 350, "docs/specs/glossary.md": 2950, "docs/specs/hosted.md": 4400, @@ -27,10 +27,10 @@ "docs/specs/security-audit.md": 4700, "docs/specs/security-ci.md": 3350, "docs/specs/security-hosted.md": 3000, - "docs/specs/security-local.md": 5750, + "docs/specs/security-local.md": 5900, "docs/specs/security-remote.md": 6800, "docs/specs/security-supply-chain.md": 1650, - "docs/specs/security.md": 2450, + "docs/specs/security.md": 2500, "docs/specs/shortcuts.md": 1150, "docs/specs/standalone.md": 6450, "docs/specs/terminal-context.md": 950, diff --git a/standalone/src-tauri/src/lib.rs b/standalone/src-tauri/src/lib.rs index a5fafd510..38fe9da12 100644 --- a/standalone/src-tauri/src/lib.rs +++ b/standalone/src-tauri/src/lib.rs @@ -1071,20 +1071,35 @@ fn prepare_log_dir(path: &Path, own_dir: bool) -> bool { builder.create(dir).is_ok() && restrict_to_owner(dir, 0o700).is_ok() } -/// Open a log handle owner-only. On unix it never follows a symlink at `path`, -/// and it refuses a file another account owns, which it cannot `fchmod`. +/// Open a log handle owner-only, for reading or writing. On unix it never +/// follows a symlink at `path`, and it refuses a file another account owns, +/// which it cannot `fchmod`. Anything but a regular file is refused before its +/// mode is touched. fn open_log(path: &Path, options: &mut OpenOptions) -> std::io::Result<File> { + let regular_only = |file: File| { + if file.metadata()?.is_file() { + Ok(file) + } else { + Err(std::io::Error::other(format!("{} is not a regular file", path.display()))) + } + }; #[cfg(unix)] { use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; - let file = options.mode(0o600).custom_flags(libc::O_NOFOLLOW).open(path)?; + // O_NONBLOCK keeps a FIFO at `path` from hanging the open; it has no + // effect on the regular file that is all this ever returns. + let file = options + .mode(0o600) + .custom_flags(libc::O_NOFOLLOW | libc::O_NONBLOCK) + .open(path)?; + let file = regular_only(file)?; // An existing file keeps its mode through `open`; this tightens it. file.set_permissions(std::fs::Permissions::from_mode(0o600))?; Ok(file) } #[cfg(not(unix))] { - let file = options.open(path)?; + let file = regular_only(options.open(path)?)?; restrict_to_owner(path, 0o600).map_err(std::io::Error::other)?; Ok(file) } @@ -6477,6 +6492,38 @@ mod tests { assert_eq!(fs::read(&victim).unwrap(), b"keep"); } + /// The webview's log read goes through the same open: a planted symlink + /// is refused rather than read back, and so is anything but a regular + /// file, a FIFO without hanging. + #[cfg(unix)] + #[test] + fn log_read_refuses_a_symlink_or_non_regular_file() { + let root = TempDir::new("log-read-symlink"); + let victim = root.path().join("victim"); + fs::write(&victim, b"secret").unwrap(); + let path = root.path().join("dormouse.log"); + std::os::unix::fs::symlink(&victim, &path).unwrap(); + assert!(super::read_log_tail(&path, 10_000).is_err()); + + let fifo = root.path().join("fifo.log"); + let c_path = std::ffi::CString::new(fifo.as_os_str().as_encoded_bytes()).unwrap(); + assert_eq!(unsafe { libc::mkfifo(c_path.as_ptr(), 0o600) }, 0); + assert!(super::read_log_tail(&fifo, 10_000).is_err()); + + // A directory opens for reading on unix; it is refused before its + // mode is tightened. + use std::os::unix::fs::PermissionsExt; + let dir = root.path().join("dir.log"); + fs::create_dir(&dir).unwrap(); + fs::set_permissions(&dir, fs::Permissions::from_mode(0o755)).unwrap(); + assert!(super::open_log(&dir, fs::OpenOptions::new().read(true)).is_err()); + assert_eq!(fs::metadata(&dir).unwrap().permissions().mode() & 0o777, 0o755); + + let real = root.path().join("real.log"); + fs::write(&real, b"line\n").unwrap(); + assert_eq!(super::read_log_tail(&real, 10_000).unwrap(), "line\n"); + } + #[cfg(unix)] #[test] fn session_write_tightens_directory_and_existing_temp_file() {