From 2c9005bd1e8cadad6f7a328352f4abfca522564c Mon Sep 17 00:00:00 2001 From: Ned Date: Thu, 1 Oct 2026 19:40:44 -0700 Subject: [PATCH 01/10] Audit host contracts and enforce private recovery and peer byte bounds --- .github/audit/application-security.md | 2 +- docs/compatible-agents.md | 6 +- docs/compatible-agents.rationale.md | 2 + docs/specs/security-local.md | 11 +-- docs/specs/security-local.rationale.md | 2 + docs/specs/security.md | 7 +- docs/specs/transport.md | 67 +++++----------- docs/specs/vscode.md | 22 ++---- lib/src/host/private-path.test-utils.ts | 37 +++++++++ lib/src/host/private-path.test.ts | 59 ++++++++++++++ lib/src/host/private-path.ts | 66 ++++++++++++++++ lib/src/host/recovery-store.test.ts | 90 +++++++++++++++++++++- lib/src/host/recovery-store.ts | 54 ++++++++++--- lib/src/host/remote/enroll-offer.ts | 11 +-- lib/src/lib/mirrored-constants.test.ts | 8 +- scripts/spec-word-budgets.json | 6 +- standalone/sidecar/pty-core.js | 14 ++-- vscode-ext/src/extension.ts | 3 +- vscode-ext/src/peer-link-protocol.ts | 5 +- vscode-ext/src/session-state.ts | 27 +++---- vscode-ext/test/helpers.ts | 4 +- vscode-ext/test/peer-link-protocol.test.ts | 25 ++++++ vscode-ext/test/peer-link.test.ts | 70 +++++++++++++---- 23 files changed, 454 insertions(+), 144 deletions(-) create mode 100644 lib/src/host/private-path.test-utils.ts create mode 100644 lib/src/host/private-path.test.ts create mode 100644 lib/src/host/private-path.ts diff --git a/.github/audit/application-security.md b/.github/audit/application-security.md index 433aa4384..340f2a67f 100644 --- a/.github/audit/application-security.md +++ b/.github/audit/application-security.md @@ -98,7 +98,7 @@ For the rest of `docs/specs/security-local.md`, read each section's owner first `docs/specs/dor-cli.md`, `docs/specs/vscode.md` -> "Webview message authentication", `docs/specs/standalone.md` -> "Persistence" — then the parser, the iframe shim, the control-socket code, and the persistence paths they point at. `## Persisted -state` covers session snapshots, written through `write_file_atomically` on standalone and through VS Code storage in the extension. +state` covers session snapshots, written through `write_file_atomically` on standalone and through VS Code storage in the extension, plus shared recovery storage. Read `lib/src/host/private-path.ts`, `lib/src/host/recovery-store.ts`, their tests, and early activation in `vscode-ext/src/extension.ts`; verify actual Windows ACLs as well as Unix modes, legacy explicit file grants, and no helper retry inside bounded capture. The attacker there is a program printing to the terminal, a page in a browser pane, or another local account, never the network. diff --git a/docs/compatible-agents.md b/docs/compatible-agents.md index fd17dbdd0..8e72c026b 100644 --- a/docs/compatible-agents.md +++ b/docs/compatible-agents.md @@ -101,12 +101,12 @@ Source of truth: `CODING_AGENTS` in `lib/src/lib/coding-agents.ts`; `detectResum ### Recovery record - **Must keep one rebuilt invocation per Surface in a host-owned, single-use record outside the persisted Session.** The renderer save path never derives or writes it. (rationale) -- **Must call `beginCapture` before capture can return early.** The first call per host process clears the previous record; subsequent calls merge, preserving captures from other Windows. (rationale) -- **Must persist every detection synchronously through `createRecoveryStore`**, using `recovery.json` in the host-selected directory, an owner-only temporary file, and atomic rename. A failed write must not throw through teardown. Without a directory the store is memory-only and logs that limitation once. +- **Must call `beginCapture` before capture can return early.** The first call per host process clears the previous record when private storage is available; subsequent calls merge, preserving captures from other Windows. (rationale) +- **Must persist every detection synchronously through `createRecoveryStore`**, using `recovery.json` in the host-selected directory, an owner-only temporary file, and atomic rename. **Must prepare the exact host-owned directory before any record bytes and tighten an existing record before claiming it**, using owner-only modes on Unix and a protected current-user-only DACL on Windows. Failed preparation preserves the previous record and permits no read, write, or unlink. Preparation occurs at startup; a cold-start claim may retry, but bounded capture never launches the permission helper. A failed write must not throw through teardown. Without a directory the store is memory-only and logs that limitation once. - **Must read and unlink the durable record on the first claim**, including on parse failure; if unlink fails, ignore it. Discard records older than 7 days after unlinking. Within the process, each container claims only its saved pane ids, and each entry is handed out once. (rationale) - **Must deliver claimed commands out of band on boot through `PlatformAdapter.getRecoveryCommands()`**; adapters whose hosts capture nothing may omit it. Only cold restore consumes these commands for execution; live resume never executes them. -Source of truth: `createRecoveryStore` in `lib/src/host/recovery-store.ts`; `PlatformAdapter` in `lib/src/lib/platform/types.ts`; pinned by `lib/src/host/recovery-store.test.ts`. +Source of truth: `createRecoveryStore` in `lib/src/host/recovery-store.ts`; `ensurePrivateDirectorySync` / `ensurePrivateFileSync` in `lib/src/host/private-path.ts`, pinned by `lib/src/host/private-path.test.ts` and `lib/src/host/recovery-store.test.ts`; `PlatformAdapter` in `lib/src/lib/platform/types.ts`. ### Cold restore diff --git a/docs/compatible-agents.rationale.md b/docs/compatible-agents.rationale.md index f246dba24..3f7a43115 100644 --- a/docs/compatible-agents.rationale.md +++ b/docs/compatible-agents.rationale.md @@ -60,6 +60,8 @@ Rows 1–2 are why a blanket second press is wrong; `Press Ctrl-C again` was abs **Why each webview claims only its own pane ids.** Two containers resolve inside one activation; a claim-everything read would let whichever resolved first delete the other's commands. Per-id claiming also means a disposed-and-re-resolved view restores without re-running the agent — its entries were already taken. +Windows Node mode bits left recovery files inheriting Everyone read access in an actual deliberately loose directory (Windows, 2026-10-01). A protected current-user-only inheritable DACL removed foreign grants from new files; legacy explicit file grants needed separate tightening. System PowerShell setup took about 303 ms in the probe, which motivated startup preparation and successful caching rather than spending the capture budget on each detection. + ## Cold restore **Why auto-run needs no confirmation prompt.** The detector rebuilds a known command and restricts the id grammar, excluding shell punctuation from the captured argument. `claude --resume ` restores the conversation, lands at an idle prompt, and makes no request until the user types. It restores *more* context than the scrollback it replaces — the resumed agent renders the real conversation, not a transcript of it — which is what made dropping persisted scrollback affordable. diff --git a/docs/specs/security-local.md b/docs/specs/security-local.md index 3d59f6b22..dde7b7b18 100644 --- a/docs/specs/security-local.md +++ b/docs/specs/security-local.md @@ -188,19 +188,14 @@ upgrade rewrites the snapshot without it, and a boot sweep deletes orphaned `*.json.tmp` files no save would ever overwrite. Snapshots older versions left behind do carry transcripts (rationale). -**Standalone writes `recovery.json` beside its sessions directory**, under the -state root, owner-only: one rebuilt agent-resume invocation per Surface, never a -buffer, unlinked as it is read (`docs/compatible-agents.md` -> "Recovery record"). +Recovery storage and single-use claiming follow `docs/compatible-agents.md` → Recovery record. **The managed-voice token is a bearer credential at rest** — `/managed-voice.json` beside the Burrow's enrollment, written by `writeJsonAtomic` (`0700`/`0600`; on Windows the owner-only DACL `burrow_state_dir` applies before the sidecar spawns), with the voice id (rationale); `docs/specs/alert.md` → "Managed voice" keeps it from any webview. **The token must go only to `hostedVoiceOrigin`'s answer**, never following a redirect (`redirect: 'error'`); a self-host build, answered `null`, sends it nowhere (`docs/specs/relay.md` -> "Relay origin"). **VS Code persists pane structure in VS Code's own storage** — `workspaceState` under `dormouse.session`, and `vscode.setState()`, a WebviewPanel's only store — so the modes there are VS Code's, not ours, and no transcript reaches either -(`docs/specs/vscode.md` -> "Serialization and restore"). Dormouse also writes -`recovery.json` under the extension's storage directory, owner-only and -temp-then-rename: one rebuilt agent-resume invocation per Surface, no buffer, -unlinked as it is read (`docs/compatible-agents.md` -> "Recovery record"). +(`docs/specs/vscode.md` -> "Serialization and restore"). **The VS Code peer-link token is a local credential at rest** — `burrow.peer-token` in the extension's global storage, written mode `0600` @@ -216,6 +211,8 @@ does. A gap, not an accepted risk. - **FAIL IF** `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, asserted by `restrict_to_owner_leaves_one_owner_only_ace` — or if **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. `session_write_tightens_directory_and_existing_temp_file` pins unix modes; `session_permission_failures_preserve_previous_snapshot_without_writing_bytes` pins both failure gates. The mode reaches the temp file *before* any bytes are written (rationale). +- **FAIL IF** recovery storage is read, written, or unlinked before owner-only permission setup succeeds, or bounded capture launches permission setup. Read `createRecoveryStore` in `lib/src/host/recovery-store.ts` and `ensurePrivateDirectorySync` / `ensurePrivateFileSync` in `lib/src/host/private-path.ts`; `lib/src/host/private-path.test.ts` and `lib/src/host/recovery-store.test.ts` pin real Windows DACLs, Unix modes, legacy file grants, failure preservation, and capture-safe caching. + Source of truth: `SESSION_STATE_KEY` in `vscode-ext/src/session-state.ts`, `ensureToken` in `vscode-ext/src/peer-link.ts`, `default_log_path` in `standalone/src-tauri/src/lib.rs`, `createManagedVoiceHost` in diff --git a/docs/specs/security-local.rationale.md b/docs/specs/security-local.rationale.md index 9d8569c99..fdb74adc0 100644 --- a/docs/specs/security-local.rationale.md +++ b/docs/specs/security-local.rationale.md @@ -173,6 +173,8 @@ true immediately on `win32`, and Node's `mode: 0o600` there touches only the read-only attribute, so unlike `remote_host_state_dir` no DACL work is done for it. +Recovery permissions were reproduced with an actual Windows DACL containing an inherited Everyone read grant (2026-10-01). Node mode `0600` did not remove it. The shared helper now protects the exact host-owned directory before writes and separately tightens explicit legacy file grants before claims; failed setup leaves the previous record untouched. + Where the standalone log is actually exposed. `env::temp_dir()` honors `TMPDIR`, which on macOS is the per-user `/var/folders/.../T` directory at `0700`, so the umask does not matter there (measured on a macOS host, 2026-09). The exposure is diff --git a/docs/specs/security.md b/docs/specs/security.md index 8ae66e8d1..113fdb8b3 100644 --- a/docs/specs/security.md +++ b/docs/specs/security.md @@ -118,10 +118,9 @@ Gaps rather than accepted risks: we intend to close them. WebSocket cookie headers are stripped, but `document.cookie` remains shared; cookie-authenticated iframe pages are unsupported ([Loopback Listeners](./security-local.md#loopback-listeners)). -- **Neither VS Code's peer-link token nor the `recovery.json` beside it carries - a Windows ACL applied by Dormouse.** Both are written owner-only by unix - mode, which Windows makes a no-op; standalone locks its state directory - instead ([Persisted state](./security-local.md#persisted-state)). +- **VS Code's peer-link token has no dedicated Windows ACL applied by Dormouse.** + Its unix mode is a no-op on Windows + ([Persisted state](./security-local.md#persisted-state)). - **The standalone log file is written at the umask** and records the `dor` socket path ([Persisted state](./security-local.md#persisted-state)). - **Revocation has no mechanism.** Revoking a lost phone is editing the Burrow's diff --git a/docs/specs/transport.md b/docs/specs/transport.md index 892b964a1..6dcec1e0d 100644 --- a/docs/specs/transport.md +++ b/docs/specs/transport.md @@ -1,65 +1,41 @@ # Transport and PTY Protocol Spec -> Adapter-agnostic protocol shared by every `PlatformAdapter`: PTY lifecycle, buffering, the webview ↔ platform message protocol, persisted-session types, and the invariants every adapter must honor. Host-specific layering lives in `docs/specs/vscode.md` and `docs/specs/standalone.md`; the phone's adapter in `docs/specs/pocket-app.md`. See `docs/specs/glossary.md` for the Process / Link state vocabulary, `docs/specs/alert.md` for `AlertManager` semantics, and `docs/specs/terminal-state.md` for the semantic events delivered over this transport. +> See `docs/specs/glossary.md` for Session / Pane / Door and Process / Link vocabulary. +> +> Adapter-agnostic protocol shared by every `PlatformAdapter`: PTY lifecycle, buffering, the webview ↔ platform message protocol, persisted-session types, and the invariants every adapter must honor. Host-specific layering lives in `docs/specs/vscode.md` and `docs/specs/standalone.md`; the phone's adapter in `docs/specs/pocket-app.md`. See `docs/specs/alert.md` for `AlertManager` semantics and `docs/specs/terminal-state.md` for semantic events. ## Adapter model -Each adapter wraps a PTY-spawning runtime and a transport channel between webview and host process. Source of truth: `PlatformAdapter` in `lib/src/lib/platform/types.ts`. +**Must expose platform capabilities through `PlatformAdapter`; unsupported optional capabilities are absent.** | Adapter | Host runtime | Transport | |---|---|---| | VS Code extension | extension host (Node.js) | `vscode.Webview.postMessage` ↔ `acquireVsCodeApi().postMessage` | | Standalone (Tauri) | sidecar process | Tauri command/event bridge | | Standalone browser-dev | sidecar + local dev HTTP bridge | fetch commands + Server-Sent Events | -| Pocket (`RemotePtyAdapter`) | the paired laptop's Host | remote protocol-v1 over the relay (`docs/specs/remote-api.md`) | +| Pocket (`RemotePtyAdapter`) | paired laptop’s Burrow | encrypted protocol-v1 over the selected Relay/direct session (`docs/specs/remote-api.md`); one-time is direct-only (`docs/specs/one-time.md`) | | Fake (tests, playground) | in-process | direct calls / event emitter | **A host that cannot do something must say so by absence, never by the UI branching on host identity.** `RemotePtyAdapter` implements only the PTY core (list/data/write/resize/exit) and no-ops or omits the rest. -Optional booleans: +**Must treat absent host-ownership capabilities as false.** Their members, defaults, and consumers are canonical comments on `PlatformAdapter`; behavior belongs to `docs/specs/theme.md` → Where the user picks a theme, `docs/specs/vscode.md` → Shell selection, and `docs/specs/remote-network.md` → Settings → Network. -| Member | Absent reads | Set by | Effect when set | -|---|---|---|---| -| `hostOwnsTheme?` | `false` | `VSCodeAdapter` → `true` | Settings hides its theme picker (`docs/specs/theme.md` → "Where the user picks a theme") | -| `hostOwnsShells?` | `false` | `VSCodeAdapter` → `true` | Settings hides its Shell row for the native QuickPick (`docs/specs/vscode.md` → "Shell selection") | -| `hostOwnsUpdates?` | `false` | `VSCodeAdapter` → `true` | Settings → Network names the Marketplace instead of an update check (`docs/specs/remote-network.md` → "Settings → Network") | +Source of truth: `PlatformAdapter` in `lib/src/lib/platform/types.ts`. ## PTY lifecycle -PTYs are managed by the platform host, not by the webview. The webview **resumes** over live PTYs (host-preserved) or **restores** from a Snapshot (cold start). - -``` -Platform host (always running while the adapter is active) -├── pty-manager (forks pty-host child process) -│ ├── pty-1 (Process: Live) -│ ├── pty-2 (Process: Live) -│ └── pty-3 (Process: Exited) -│ -├── Webview (e.g. VS Code WebviewView, a standalone window) -│ └── message-router: owns pty-1, pty-2 -│ -└── Secondary webview (a VS Code editor-tab WebviewPanel, another standalone window) - └── message-router: owns pty-3 -``` - -**Every host is multi-webview**, and **a router never takes a PTY another -owns**: ownership keeps one webview's traffic out of another's, and each host -owns the map (`docs/specs/vscode.md` → "Peer surfaces across windows", -`docs/specs/standalone.md` → Routing). +**Must keep PTYs in their platform runtime across webview hide/recreate and isolate each webview’s ownership.** Local host mechanisms belong to `docs/specs/vscode.md` → Webview hosting and `docs/specs/standalone.md` → Routing. The webview resumes over preserved PTYs or restores from a Snapshot. - **Hiding a webview does not kill its PTYs**, and becoming visible again resumes over the still-owned ones ("Reconnection protocol"). - **A naturally exited PTY may stay mounted as an exited pane**; frontend semantic state — CWD, title candidates, last command — is retained until the Session is disposed. - **Must keep explicitly killed PTYs non-resumable.** VS Code tombstones their ids (`Process: Tombstoned`) in `pty-manager.ts` so late child-process output cannot recreate a buffer; the shared `pty-core.js` drops its live record and retains no output. -- **Each host instance gets its own pty-host child process** (e.g. one per VS Code window). - **Must mark live VS Code PTYs exited and notify their owners when the child process exits unexpectedly**, retaining transcripts and already-recorded exits. **Must ignore retired-child output and exit events after replacement**; pinned by `vscode-ext/test/pty-manager.test.ts`. ### PTY buffering -VS Code's `pty-manager` keeps two buffers plus one counter per PTY. **Must cap each buffer at 1,000,000 characters**, dropping oldest chunks and truncating an oversized final chunk; pinned by `vscode-ext/test/pty-manager.test.ts`. +**Must cap each VS Code replay and scrollback buffer at 1,000,000 characters**, evicting oldest chunks and truncating an oversized final chunk. **Must clear replay on first consume and retain host-only scrollback until `kill`/`killAll`**, for repeat resume and recovery. Stream positions follow Universal invariants. -- **replayChunks** — cleared on first consume; used for resume (webview hidden then shown). -- **scrollbackChunks** — never cleared short of `kill`/`killAll`; used for repeat resumes (a re-serving router's replay buffer is already spent) and for recovery capture at teardown. Host-side only — no adapter exposes it to the renderer. -- **receivedChars** — every char ever buffered, never decremented by a trim ("A position in a pane's output is a received count", below). +Source of truth: `bufferData` / `getReplayData` in `vscode-ext/src/pty-manager.ts`, pinned by `vscode-ext/test/pty-manager.test.ts`. ### Paced input @@ -76,16 +52,11 @@ Source of truth: `pacedInputSegments` and `write` in `standalone/sidecar/pty-cor ### Reconnection protocol -``` -1. Webview becomes visible (or panel deserializes) and sends { type: 'dormouse:init' }. -2. Host answers { type: 'pty:list', ptys: [{ id, alive, exitCode, shell }] } for all owned PTYs, - then per PTY { type: 'pty:replay', id, data } and { type: 'alert:state', id, … }. -3. Webview restores terminals from replay data, including each PTY's launch-shell path, which - the rebuilt registry needs for Session-specific clipboard/drop escaping. -4. If the saved session covers those live PTYs, the frontend uses the saved Lath layout when its - leaf set matches and reattaches saved minimized doors; minimized PTYs are registered but stay - doors, not visible panes. -``` +1. The visible or deserialized webview requests initialization. +2. The host lists owned PTYs, then sends their replay and alert state. +3. The webview resumes terminals with their launch shells for Session-specific clipboard/drop escaping. +4. A saved layout is reused only when its leaves match the live visible pane set; saved minimized PTYs are registered as Doors. + **A collection finishes only on its own answer.** A `requestInit` carries the asking collector's token, and a host serving several windows echoes it on the @@ -177,7 +148,7 @@ Source of truth: the message schema in `vscode-ext/src/message-types.ts` (`Webvi **`dormouse:runWorkbenchCommand` (webview → host) is allowlisted** against `lib/src/lib/vscode-keybindings.ts` before `vscode.commands.executeCommand`; generic command execution over the webview boundary is not allowed. -**Reaching the Burrow is one optional adapter member.** `burrow?: BurrowLink` is present exactly when a PTY-owning process sits behind the webview — standalone's sidecar, VS Code's extension host — and absent on the website. Its four calls are `command`, `respond`, `notify` (argless — the directory is the only thing a peer answers), and `on`. The webview half is `lib/src/host/remote/link-client.ts`, shared by all three adapters so no host settles a command differently: command correlation, a 15 s timeout, and the rule that **an ask is always answered even when nothing matches**. Both ends compile against `lib/src/host/remote/service-protocol.ts`. **Nothing crossing this seam carries authority** (`docs/specs/remote-security-model.md`). +**Reaching the Burrow is one optional adapter member.** `burrow?: BurrowLink` is present exactly when a PTY-owning process sits behind the webview — standalone's sidecar, VS Code's extension host — and absent on the website. The webview half is `lib/src/host/remote/link-client.ts`, shared by all three adapters so no host settles a command differently: command correlation, a 15 s timeout, and the rule that **an ask is always answered even when nothing matches**. Both ends compile against `lib/src/host/remote/service-protocol.ts`; `BurrowLink` in `lib/src/lib/platform/types.ts` owns the call shapes. **Nothing crossing this seam carries authority** (`docs/specs/remote-security-model.md`). Each host maps those calls onto its own transport: @@ -197,7 +168,7 @@ Transport constraints: | --- | --- | --- | | Webview → host | `dormouse:openExternal` | Open a user-confirmed external URI from an OSC 8 hyperlink. **Hosts must revalidate**, rejecting malformed, control-character-bearing, or blocked pseudo-scheme targets (`javascript:`, `data:`, `blob:`, `about:` — `lib/src/lib/external-links.ts`). | | Webview → host | `pty:getOpenPorts` | TCP listening ports of a PTY's shell **and all of its descendant subprocesses**, resolved from the root pid, answered with `pty:openPorts`. `getOpenPortsForPid()` in `standalone/sidecar/pty-core.js` (VS Code loads it through the `lib/pty-core.cjs` shim). | -| Host → webview | `pty:openPorts` | `ports: OpenPort[]` (`{ protocol, family, address, port, pid, processName }`), de-duplicated by `(family, address, port)`, sorted by port then address. Empty when the PTY is gone or enumeration fails. | +| Host → webview | `pty:openPorts` | `OpenPort[]`, de-duplicated by `(family, address, port)`, sorted by port then address. Empty when the PTY is gone or enumeration fails. | | Host → webview | `pty:data` | PTY output after state-driving supported OSCs are parsed/stripped; `OSC 8` and ImageAddon's inline-image `OSC 1337` forms are preserved for xterm.js, routed only to the owning router. **Carries an optional `textData`** (string-control payloads removed, for the prompt heuristic), **omitted when it would equal `data`**. | | Host → webview | `terminal:semanticEvents` | Normalized CWD / prompt-command / title events the owner's parser derived, in stream order. | | Host → webview | `terminal:toolEvents` | Ordered Tool announcements, state, and command-start resets (`docs/specs/dor-tool.md` → OSC 367). | @@ -241,7 +212,7 @@ Source of truth: `ManagedVoicePort` in `lib/src/lib/platform/managed-voice-types **Surface kinds in the snapshot.** Each `PersistedPane` records a `surfaceType` (`docs/specs/glossary.md`): `'terminal'` — the default, **omitted from the row** so terminal snapshots stay byte-identical — `'browser'`, or `'tool'`, whose extra `command` and `tool` fields are `docs/specs/dor-tool.md` → Persistence and hosts. It routes restore/resume, and **a pane lacking it reads as `'terminal'`**. `restoreSession` skips terminal restoration for a browser pane rather than minting a stray PTY + xterm per browser pane id, and the resume plan keeps browser panes and minimized browser doors despite their having no live PTY, so the saved layout's leaf set still matches and is not discarded. A browser pane rebuilds from the persisted layout (visible) or `PersistedDoor.params` (minimized) — its render params (`renderMode`, `url`, agent-browser `session`) live there, not in `PersistedPane`. **Must reject a layout whose leaves differ from the visible pane set during restore or resume, and omit visible browser ids from the terminal fallback.** Browser doors retain their independent render params; pinned by `lib/src/lib/session-restore.test.ts` and `lib/src/lib/reconnect.test.ts`. -**Each mounted Workspace publishes its `PersistedSession` to a Window collector**, which orders them by the Workspace store and writes the whole Window through one debounced writer the host installs at boot. **A Workspace with neither a published nor a boot-seeded session is dropped rather than written empty**, so a snapshot taken mid-boot cannot replace a restored Workspace with a blank one. **A Workspace's save compares against its own previous record** — seeded from disk until its Wall publishes — never the Window's active one, or a dead PTY's retained cwd and alert would come from the wrong Workspace. **Reordering, renaming, or switching the active Workspace writes too**: each changes the blob with no Session changing. A `PersistedWorkspace` is a `WorkspaceId`, a `name`, `nameIsAuto`, and that Workspace's `PersistedSession`. **Always write `nameIsAuto`**; lacking it, only a `Workspace ` name is auto. The top-level snapshot is a `PersistedWindow` (its own `version: 1`) wrapping v3 sessions: the ordered `PersistedWorkspace` list plus the active `WorkspaceId`. **VS Code does not use it** — each webview persists one bare `PersistedSession`, its single Workspace, through its own per-surface state API (`docs/specs/vscode.md`). +**Each mounted Workspace publishes its `PersistedSession` to a Window collector**, which orders them by the Workspace store and writes the whole Window through one debounced writer the host installs at boot. **A Workspace with neither a published nor a boot-seeded session is dropped rather than written empty**, so a snapshot taken mid-boot cannot replace a restored Workspace with a blank one. **A Workspace's save compares against its own previous record** — seeded from disk until its Wall publishes — never the Window's active one, or a dead PTY's retained cwd and alert would come from the wrong Workspace. **Reordering, renaming, or switching the active Workspace writes too**: each changes the blob with no Session changing. **Always write `nameIsAuto`**; lacking it, only a `Workspace ` name is auto. **VS Code does not use it** — each webview persists one bare `PersistedSession`, its single Workspace, through its own per-surface state API (`docs/specs/vscode.md`). **Must publish both Workspace records in one synchronous step with the Surface move's ownership change**, unprobed and behind one Window write (`pagehide` included), fencing saves collected before or during the ownership change and retaining a departed Session's previous cwd/alert in the destination. Source of truth: `publishWorkspaceSessions` / `invalidateWorkspaceSaves` / `moveRetainedSurfaceRecord` in `lib/src/lib/window-session-aggregator.ts`; `serializeNow` / `doSave` in `lib/src/components/wall/use-session-persistence.ts`; `assemblePersistedSession` in `lib/src/lib/session-save.ts`. @@ -261,7 +232,7 @@ Source of truth: `PersistedSession` in `lib/src/lib/session-types.ts`; `surfaceR ### What is persisted -Structure only: panes (id, cwd, title, `untouched`, `surfaceType`, TODO/alert blob), doors and their Lath restore tokens, the Lath layout, and the Workspace's `dor` surface refs and delivery overrides. **Scrollback is never persisted by any writer**, and neither is the recovery command (above). +**Must persist structure only, never scrollback or recovery commands.** The accepted shapes are `PersistedSession` / `PersistedWindow` in `lib/src/lib/session-types.ts`; their behavioral contracts are in Persisted session types. ### Retiring the transcripts already on disk diff --git a/docs/specs/vscode.md b/docs/specs/vscode.md index 2bcf007f8..b66404d6d 100644 --- a/docs/specs/vscode.md +++ b/docs/specs/vscode.md @@ -20,9 +20,7 @@ Start on the side of the webview boundary involved, then follow imports: ## What's built -Two hosting modes: a `WebviewView` in the bottom panel (alongside Terminal, Problems, Output) and `WebviewPanel` editor tabs (`dormouse.open`, multiple instances). Both restore across "Developer: Reload Window". PTYs live in the extension host (`pty-manager.ts`), survive panel visibility toggling, and replay buffered output on **resume**. Scrollback is never persisted (`docs/specs/transport.md` → "Persistence policy"); `deactivate()` instead interrupts the live PTYs and records each pane's agent resume invocation for the next cold restore to auto-run (`docs/specs/layout.md` → "Agent resume on cold restore"). - -The webview is the shared `lib/` frontend, unmodified for this host (`docs/specs/layout.md`, `docs/specs/transport.md`). The only VS Code-specific pieces in `lib/`: `lib/src/lib/platform/vscode-adapter.ts` (the postMessage bridge), `lib/src/lib/vscode-message-token.ts`, `lib/src/lib/vscode-keybindings.ts`. +The shared frontend runs in the bottom-panel `WebviewView` and independent editor-tab `WebviewPanel`s. Their ownership, visibility, and restore contracts are in Webview hosting and Serialization and restore. ### Invariants (VS Code-specific) @@ -40,7 +38,7 @@ The webview is the shared `lib/` frontend, unmodified for this host (`docs/specs ### Extension manifest -**Must activate on the contributed view, restored editor panels, or an invoked contributed command.** Command activation is implicit on the supported VS Code versions ([activation events](https://code.visualstudio.com/api/references/activation-events#oncommand)). The manifest owns contributed commands, views, and title actions: ids, titles, icons, and ordering. The shipped commands are `dormouse.focus`, `dormouse.open`, `dormouse.debugTheme`, `dormouse.newTerminal`, and `dormouse.selectShell`. +**Must activate on the contributed view, restored editor panels, or an invoked contributed command.** Command activation is implicit on the supported VS Code versions ([activation events](https://code.visualstudio.com/api/references/activation-events#oncommand)). The manifest owns contributed commands, views, and title actions: ids, titles, icons, and ordering. **No `configuration`, no `keybindings`, no context key**: settings live in the in-webview Settings dialog rather than `settings.json`, chords are handled inside the webview, and nothing is `when`-gated on Dormouse state. Context keys are [Future](#context-keys). Source of truth: `vscode-ext/package.json`. @@ -140,9 +138,9 @@ needs no host-side per-panel store. #### Capturing agent recovery -**Must offer every live extension-host PTY to shared capture**, across the view and editor panels. **Must store the record under `storageUri`, falling back to `globalStorageUri`; never `workspaceState`** (rationale). If neither directory exists, skip capture. Shared behavior follows `docs/compatible-agents.md`. +**Must offer every live extension-host PTY to shared capture**, across the view and editor panels. **Must store the record under `storageUri`, falling back to `globalStorageUri`; never `workspaceState`** (rationale). If neither directory exists, skip capture. **Must prepare private recovery storage at activation; if preparation fails, bounded teardown must skip disk mutation rather than launch a permission helper.** Cold-start claims may retry. Shared record behavior follows `docs/compatible-agents.md` → Recovery record. -Source of truth: `captureAgentRecoveryCommands` / `takeRecoveryCommands` in `vscode-ext/src/session-state.ts`; `interrupt` in `vscode-ext/src/pty-manager.ts`. +Source of truth: `prepareRecoveryStorage` / `captureAgentRecoveryCommands` / `takeRecoveryCommands` in `vscode-ext/src/session-state.ts`; `interrupt` in `vscode-ext/src/pty-manager.ts`. ### Theme integration @@ -183,7 +181,7 @@ frame-src http://127.0.0.1:* http://localhost:* **That origin is a build-time constant, never a runtime value**: `vscode-ext/scripts/esbuild.mjs` bakes `DORMOUSE_RELAY_ORIGIN` into `dist/extension.js`. The default, the modes, and the build-time guards are `docs/specs/relay.md` → "Relay origin". -`unsafe-inline` for styles covers the theme CSS variables VS Code injects as inline styles on the body element. Scripts stay nonce-gated on a fresh per-render nonce of 24 CSPRNG bytes (`node:crypto` `randomBytes`) base64url-encoded to 32 characters — **never `Math.random()`**. Vite builds the webview HTML from the `lib` package; at runtime `webview-html.ts` rewrites asset URLs to webview URIs, injects the CSP meta tag, swaps Vite's nonce placeholder for the real one, and appends a nonce-gated inline script carrying the boot globals (message token, initial state, selected shell, recovery commands). +`unsafe-inline` for styles covers the theme CSS variables VS Code injects as inline styles on the body element. **Must mint a fresh per-render nonce from 24 CSPRNG bytes, never `Math.random()`**, and nonce-gate the boot globals. `getWebviewHtml` owns asset rewriting, nonce substitution, and boot serialization. **`lib/index.html` keeps `` and `` bare**: both splices match a literal and **throw when it is absent**, an attribute otherwise yielding an unpoliced document. Pinned by "refuses a document whose splice marker was edited away". @@ -204,7 +202,7 @@ Source of truth: `getWebviewHtml` in `vscode-ext/src/webview-html.ts`, `CSP_NONC **The webview's `window` is a shared inbox, so `event.data.type` cannot decide trust** — it is attacker-chosen. The extension host posts there, and so can any framed surface (`dor iframe`, agent-browser; `docs/specs/dor-browser.md`) via `parent.postMessage`, which crosses origin and sandbox boundaries by design; the CSP governs what the document may *load*, never who may *message* it. A forgery is consequential: `dor:controlRequest` becomes a `dormouse:control-request` event `use-dor-control.ts` can turn into a `writePty`, and the `pty:*` family drives what the user sees (rationale). Host-originated messages are therefore authenticated by a **per-boot message token**: -- `getWebviewHtml` mints one token per webview document — 24 CSPRNG bytes, base64url, from the same `randomSecret()` as the CSP nonce — injects it as `globalThis.__DORMOUSE_MESSAGE_TOKEN__` in the same nonce-gated inline script that seeds the other `__DORMOUSE_*` globals, and returns it alongside the HTML. +- **Must mint a fresh message token per document from 24 CSPRNG bytes**, distinct from its CSP nonce, and inject it only through the nonce-gated boot script. - **`serveWebview` is the only way to put a document on a webview**: it mints, assigns `webview.html`, and returns a `WebviewChannel` whose `post()` closes over that document's token. **Minting and serving are one step**, so a token cannot drift from its document; re-serving yields a new token and channel, and nothing keys a token by webview identity, so there is no cleanup. - **Every host → webview send goes through a channel**, making a bypass a type error rather than a convention to remember; only the two serve sites (`setupPanel`, `resolveWebviewView`) still hold a raw webview, and `attachRouter` takes a `WebviewChannel`, not a `vscode.Webview`. `DormouseViewProvider.postMessage` forwards to its stored channel, **returning `false` before the view is served or after it disposes** — the VS Code API's own undelivered signal, already handled by the `dormouse:newTerminal` retry loop and `forwardDorControlRequest`'s rejection path. - `VSCodeAdapter` captures the token **once, at construction**, and both of its `message` listeners — the main dispatcher and the per-request reply listener inside `requestResponse` — call `isHostMessage(event.data, token)` before reading anything else, `type` included. @@ -339,6 +337,8 @@ Once an answer names a `ptyId`, the broker replaces that owner-local id with a s Two UI events *are* addressed: **when a window completes the handshake the broker sends it the current `status` and `one-time` events** — each is emitted only when it changes and once as the service starts, so a window opened after the enrollment or the link would otherwise sit disarmed until reloaded. +**Must bound every peer frame in UTF-8 bytes before parsing**, complete frames and partial tails included. Oversized frames are discarded through their newline without losing adjacent valid frames. + **Socket bind errors reject startup** and are handled as an unavailable peer link; they never leave the listen promise pending or surface as an uncaught extension host error. Source of truth: `vscode-ext/src/peer-link.ts` (sockets, arbitration, `HANDSHAKE_BUDGET_MS`, and the `routes` / `routePtyIds` routing table); `vscode-ext/src/peer-link-protocol.ts` (frame shapes, framing, handshake helpers, `PEER_REPLY_BUDGET_MS`), pinned by `vscode-ext/test/peer-link-protocol.test.ts`; `askBothTiers` in `vscode-ext/src/burrow.ts`; `brokerRequest` and the `peer:*` / `burrow:command` cases in `vscode-ext/src/message-router.ts`; `lib/src/remote/burrow/remote-api.ts`. @@ -365,12 +365,6 @@ types without checking them, so `tsc` runs separately as `pnpm typecheck`, **wir into the package's `test` script** so the root `pnpm test` covers it — that wiring is what protects `deactivate()`, which has no `try`/`catch` (rationale). -The checked program spans two runtimes — `src/` is extension-host Node code but -imports webview modules from `../lib/src/` — so its config carries both DOM and -Node libs, looser than either alone, each side checked precisely by its own -project (`lib/tsconfig.app.json` for the webview). What it reliably catches is -vscode-ext's own code referring to something that no longer exists. - `pnpm dogfood:vscode` uninstalls the legacy `diffplug.mouseterm` extension before packaging and installing the current Dormouse VSIX; the VS Code window must then be reloaded. Day-to-day development uses it, since it runs against your real diff --git a/lib/src/host/private-path.test-utils.ts b/lib/src/host/private-path.test-utils.ts new file mode 100644 index 000000000..dc8a12026 --- /dev/null +++ b/lib/src/host/private-path.test-utils.ts @@ -0,0 +1,37 @@ +import { execFileSync } from 'node:child_process'; +import { join } from 'node:path'; + +export function runAclScript(script: string, target: string): string { + return execFileSync(join(process.env.SystemRoot || 'C:\\Windows', 'System32', 'WindowsPowerShell', 'v1.0', 'powershell.exe'), + ['-NoProfile', '-NonInteractive', '-Command', "$ErrorActionPreference='Stop'; $p=[Console]::In.ReadToEnd(); " + script], + { input: target, encoding: 'utf8', windowsHide: true, timeout: 5_000 }).trim(); +} + +export function seedEveryoneRead(target: string): void { + runAclScript(`$acl=Get-Acl -LiteralPath $p; + $inheritance=[System.Security.AccessControl.InheritanceFlags]::None; + if ((Get-Item -LiteralPath $p -Force).PSIsContainer) { + $inheritance=[System.Security.AccessControl.InheritanceFlags]'ContainerInherit,ObjectInherit' + } + $rule=[System.Security.AccessControl.FileSystemAccessRule]::new( + [System.Security.Principal.SecurityIdentifier]::new('S-1-1-0'), + [System.Security.AccessControl.FileSystemRights]::ReadAndExecute, + $inheritance,[System.Security.AccessControl.PropagationFlags]::None, + [System.Security.AccessControl.AccessControlType]::Allow); + $acl.AddAccessRule($rule); Set-Acl -LiteralPath $p -AclObject $acl`, target); +} + +export function readAcl(target: string): { + currentUser: string; owner: string; protected: boolean; + rules: Array<{ sid: string; rights: number; allow: boolean; inheritance: number }>; +} { + return JSON.parse(runAclScript(`$acl=Get-Acl -LiteralPath $p; + @{ currentUser=[System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value; + owner=$acl.GetOwner([System.Security.Principal.SecurityIdentifier]).Value; + protected=$acl.AreAccessRulesProtected; + rules=@($acl.GetAccessRules($true,$true,[System.Security.Principal.SecurityIdentifier]) | ForEach-Object { + @{ sid=$_.IdentityReference.Value; rights=[int]$_.FileSystemRights; + allow=$_.AccessControlType -eq [System.Security.AccessControl.AccessControlType]::Allow; + inheritance=[int]$_.InheritanceFlags } + }) } | ConvertTo-Json -Depth 4 -Compress`, target)); +} diff --git a/lib/src/host/private-path.test.ts b/lib/src/host/private-path.test.ts new file mode 100644 index 000000000..b4d47d38b --- /dev/null +++ b/lib/src/host/private-path.test.ts @@ -0,0 +1,59 @@ +import * as fs from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { ensurePrivateDirectorySync, ensurePrivateFileSync } from './private-path'; +import { readAcl, seedEveryoneRead } from './private-path.test-utils'; + +let dir: string; +beforeEach(() => { dir = fs.mkdtempSync(join(tmpdir(), 'dormouse-private-path-')); }); +afterEach(() => { fs.rmSync(dir, { recursive: true, force: true }); }); + +describe('private recovery paths', () => { + it.skipIf(process.platform === 'win32')('tightens existing directory and file modes without changing the parent', () => { + fs.chmodSync(dir, 0o755); + const nested = join(dir, 'owned'); + fs.mkdirSync(nested, { mode: 0o755 }); + ensurePrivateDirectorySync(nested); + const file = join(nested, 'recovery.json'); + fs.writeFileSync(file, 'secret', { mode: 0o644 }); + ensurePrivateFileSync(file); + expect(fs.statSync(nested).mode & 0o777).toBe(0o700); + expect(fs.statSync(file).mode & 0o777).toBe(0o600); + expect(fs.statSync(dir).mode & 0o777).toBe(0o755); + }); + + it.skipIf(process.platform !== 'win32')('replaces inherited and explicit foreign grants with the current user alone', () => { + // Quotes, $, and backticks must remain literal stdin data, not script code. + const nested = join(dir, "literal $ ' ` owned"); + fs.mkdirSync(nested); + seedEveryoneRead(nested); + const legacy = join(nested, 'legacy.json'); + fs.writeFileSync(legacy, 'secret'); + seedEveryoneRead(legacy); + expect(readAcl(legacy).rules.some(({ sid }) => sid === 'S-1-1-0')).toBe(true); + ensurePrivateDirectorySync(nested); + ensurePrivateFileSync(legacy); + const fresh = join(nested, 'fresh.tmp'); + fs.writeFileSync(fresh, 'secret', { mode: 0o600 }); + for (const [target, inheritance] of [[nested, 3], [legacy, 0], [fresh, 0]] as const) { + const acl = readAcl(target); + expect(acl.owner).toBe(acl.currentUser); + expect(acl.rules).toEqual([{ sid: acl.currentUser, rights: 0x001F01FF, allow: true, inheritance }]); + if (target !== fresh) expect(acl.protected).toBe(true); + } + }, 10_000); + + it('rejects directories masquerading as recovery files', () => { + expect(() => ensurePrivateFileSync(dir)).toThrow('plain file'); + }); + + it.skipIf(process.platform === 'win32')('rejects symlinks without tightening the linked path', () => { + const real = join(dir, 'real'); + fs.mkdirSync(real, { mode: 0o755 }); + const link = join(dir, 'link'); + fs.symlinkSync(real, link); + expect(() => ensurePrivateDirectorySync(link)).toThrow('plain directory'); + expect(fs.statSync(real).mode & 0o777).toBe(0o755); + }); +}); diff --git a/lib/src/host/private-path.ts b/lib/src/host/private-path.ts new file mode 100644 index 000000000..550761e24 --- /dev/null +++ b/lib/src/host/private-path.ts @@ -0,0 +1,66 @@ +/** Recovery's exact host-owned directory and legacy record must be private before + * any bytes are written or claimed. Unix mkdir modes do not tighten existing + * directories; Windows mode bits do not control access. Harden the directory + * once at startup so a bounded teardown does not pay for a process per record. + * Never touch ancestors. Failure prevents persistence and automatic recovery. + */ +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { execFileSync } from 'node:child_process'; + +const WINDOWS_PRIVATE_PATH = ` +$ErrorActionPreference = 'Stop' +$targetPath = [Console]::In.ReadToEnd() +$sid = [System.Security.Principal.WindowsIdentity]::GetCurrent().User +$existing = Get-Acl -LiteralPath $targetPath +if ($existing.GetOwner([System.Security.Principal.SecurityIdentifier]).Value -ne $sid.Value) { + throw 'Recovery path must belong to this user' +} +if ((Get-Item -LiteralPath $targetPath -Force).PSIsContainer) { + $acl = [System.Security.AccessControl.DirectorySecurity]::new() + $inheritance = [System.Security.AccessControl.InheritanceFlags]'ContainerInherit,ObjectInherit' +} else { + $acl = [System.Security.AccessControl.FileSecurity]::new() + $inheritance = [System.Security.AccessControl.InheritanceFlags]::None +} +$acl.SetAccessRuleProtection($true, $false) +$acl.SetOwner($sid) +$rule = [System.Security.AccessControl.FileSystemAccessRule]::new( + $sid, + [System.Security.AccessControl.FileSystemRights]0x001F01FF, + $inheritance, + [System.Security.AccessControl.PropagationFlags]::None, + [System.Security.AccessControl.AccessControlType]::Allow) +$acl.AddAccessRule($rule) +Set-Acl -LiteralPath $targetPath -AclObject $acl +`; + +function restrictToOwnerSync(target: string, directory: boolean): void { + const info = fs.lstatSync(target); + if (info.isSymbolicLink() || (directory ? !info.isDirectory() : !info.isFile())) { + throw new Error(`Recovery path must be a plain ${directory ? 'directory' : 'file'}`); + } + if (process.platform !== 'win32') { + if (process.getuid && info.uid !== process.getuid()) throw new Error('Recovery path must belong to this user'); + fs.chmodSync(target, directory ? 0o700 : 0o600); + return; + } + const powershell = path.join(process.env.SystemRoot || 'C:\\Windows', 'System32', 'WindowsPowerShell', 'v1.0', 'powershell.exe'); + execFileSync(powershell, ['-NoProfile', '-NonInteractive', '-Command', WINDOWS_PRIVATE_PATH], { + // Only literal path data crosses stdin: quotes, $, backticks and newlines + // never become PowerShell code. Use the system executable, not PATH search. + input: path.resolve(target), encoding: 'utf8', windowsHide: true, timeout: 5_000, + stdio: ['pipe', 'pipe', 'pipe'], + }); +} + +export function ensurePrivateDirectorySync(dir: string): void { + fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); + restrictToOwnerSync(dir, true); +} + +/** A legacy record may have explicit grants that directory inheritance cannot + * remove. Tighten it before reading; reject symlinks and non-files outright. */ +export function ensurePrivateFileSync(file: string): void { + restrictToOwnerSync(file, false); +} diff --git a/lib/src/host/recovery-store.test.ts b/lib/src/host/recovery-store.test.ts index 75b68a212..d190ebb6a 100644 --- a/lib/src/host/recovery-store.test.ts +++ b/lib/src/host/recovery-store.test.ts @@ -1,8 +1,15 @@ -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import * as fs from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { createRecoveryStore, RECOVERY_MAX_AGE_MS } from './recovery-store'; +import * as privatePaths from './private-path'; +import { readAcl, seedEveryoneRead } from './private-path.test-utils'; + +vi.mock('node:fs', async (importOriginal) => { + const real = await importOriginal(); + return { ...real, unlinkSync: vi.fn(real.unlinkSync) }; +}); let dir: string; const messages: string[] = []; @@ -21,6 +28,7 @@ beforeEach(() => { }); afterEach(() => { + vi.restoreAllMocks(); fs.rmSync(dir, { recursive: true, force: true }); }); @@ -49,7 +57,13 @@ describe('recovery store', () => { store.beginCapture(); store.record('a', 'claude --continue'); const mode = (path: string) => fs.statSync(path).mode & 0o777; - expect(mode(file())).toBe(0o600); + if (process.platform === 'win32') { + const acl = readAcl(file()); + expect(acl.rules).toEqual([{ sid: acl.currentUser, rights: 0x001F01FF, allow: true, inheritance: 0 }]); + } else { + expect(mode(dir)).toBe(0o700); + expect(mode(file())).toBe(0o600); + } // The temp sibling is renamed over the target, so nothing torn is left. expect(fs.readdirSync(dir)).toEqual(['recovery.json']); }); @@ -83,6 +97,44 @@ describe('recovery store', () => { }); describe('take', () => { + it('never hands out commands when removing the durable record fails', () => { + write({ createdAt: Date.now(), commands: { a: 'claude --continue' } }); + const store = createRecoveryStore(dir, { log }); + vi.mocked(fs.unlinkSync).mockImplementationOnce(() => { throw new Error('unlink refused'); }); + expect(store.take(['a'])).toEqual({}); + expect(store.take(['a'])).toEqual({}); + expect(read().commands).toEqual({ a: 'claude --continue' }); + expect(messages.some((message) => message.includes('could not clear record; ignoring it'))).toBe(true); + }); + + it.skipIf(process.platform !== 'win32')('tightens explicit legacy file grants before claiming the record', () => { + write({ createdAt: Date.now(), commands: { a: 'claude --continue' } }); + seedEveryoneRead(file()); + const store = createRecoveryStore(dir, { log }); + const original = privatePaths.ensurePrivateFileSync; + // Observe permissions immediately before the actual read and unlink. + const protect = vi.spyOn(privatePaths, 'ensurePrivateFileSync').mockImplementation((target) => { + original(target); + const acl = readAcl(target); + expect(acl.protected).toBe(true); + expect(acl.rules).toEqual([{ sid: acl.currentUser, rights: 0x001F01FF, allow: true, inheritance: 0 }]); + }); + expect(store.take(['a'])).toEqual({ a: 'claude --continue' }); + expect(protect).toHaveBeenCalledWith(file()); + protect.mockRestore(); + expect(fs.existsSync(file())).toBe(false); + }); + + it('does not read or unlink a record whose private file setup fails', () => { + write({ createdAt: Date.now(), commands: { a: 'claude --continue' } }); + const store = createRecoveryStore(dir, { log }); + const protect = vi.spyOn(privatePaths, 'ensurePrivateFileSync').mockImplementation(() => { throw new Error('ACL failed'); }); + expect(store.take(['a'])).toEqual({}); + expect(read().commands).toEqual({ a: 'claude --continue' }); + protect.mockRestore(); + expect(store.take(['a'])).toEqual({ a: 'claude --continue' }); + }); + it('unlinks on the first call and hands out each id exactly once', () => { write({ createdAt: Date.now(), commands: { a: 'claude --resume A', b: 'codex resume B' } }); const store = createRecoveryStore(dir, { log }); @@ -131,6 +183,40 @@ describe('recovery store', () => { }); }); + it('never retries a failed startup helper during capture, then recovers on a cold-start claim', () => { + write({ createdAt: Date.now(), commands: { old: 'claude --continue' } }); + const protect = vi.spyOn(privatePaths, 'ensurePrivateDirectorySync').mockImplementation(() => { throw new Error('helper timed out'); }); + const store = createRecoveryStore(dir, { log }); + expect(protect).toHaveBeenCalledTimes(1); + store.beginCapture(); + store.record('a', 'codex resume A'); + store.beginCapture(); + store.record('b', 'codex resume B'); + expect(protect).toHaveBeenCalledTimes(1); + expect(read().commands).toEqual({ old: 'claude --continue' }); + expect(fs.readdirSync(dir)).toEqual(['recovery.json']); + expect(store.take([])).toEqual({}); + expect(protect).toHaveBeenCalledTimes(2); + protect.mockRestore(); + // Only startup/claim retries expensive preparation. Captured invocations + // remain in memory and later writes merge after successful preparation. + expect(store.take([])).toEqual({}); + store.beginCapture(); + store.record('c', 'codex resume C'); + expect(read().commands).toEqual({ a: 'codex resume A', b: 'codex resume B', c: 'codex resume C' }); + }); + + it('prepares a successful directory once across multiple captures and writes', () => { + const protect = vi.spyOn(privatePaths, 'ensurePrivateDirectorySync'); + const store = createRecoveryStore(dir, { log }); + store.beginCapture(); + store.record('a', 'claude --continue'); + store.beginCapture(); + store.record('b', 'codex resume B'); + expect(protect).toHaveBeenCalledTimes(1); + expect(read().commands).toEqual({ a: 'claude --continue', b: 'codex resume B' }); + }); + describe('without a state directory', () => { it('keeps the record in memory and says so once', () => { const store = createRecoveryStore(undefined, { log }); diff --git a/lib/src/host/recovery-store.ts b/lib/src/host/recovery-store.ts index 95f96199c..41a6366d1 100644 --- a/lib/src/host/recovery-store.ts +++ b/lib/src/host/recovery-store.ts @@ -14,6 +14,7 @@ import { randomUUID } from 'node:crypto'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { noCommands, silent, type RecoveryLog } from './recovery-capture'; +import { ensurePrivateDirectorySync, ensurePrivateFileSync } from './private-path'; const FILE_NAME = 'recovery.json'; @@ -58,9 +59,32 @@ export function createRecoveryStore(dir?: string, opts: { log?: RecoveryLog } = // What this process has captured. Also the memory-only store's whole content. let captured: Record = noCommands(); + let captureStarted = false; let clearedThisProcess = false; // What is left of the record on disk, once read. `null` until the first `take`. let unclaimed: Record | null = null; + let directoryPrepared = false; + const prepareDirectory = (): void => { + if (!dir || directoryPrepared) return; + ensurePrivateDirectorySync(dir); + directoryPrepared = true; + }; + // Prime outside teardown: Windows ACL setup launches a bounded system process. + // Failed preparation remains retryable and never permits record bytes. + try { prepareDirectory(); } catch (err) { + log.error(`[recovery] private directory unavailable: ${String(err)}`); + } + const requirePrivateDirectory = (): void => { + // Never launch a permission helper inside the bounded teardown capture. + // Cold-start claims can retry a failed startup preparation. + if (dir && !directoryPrepared) throw new Error('Recovery directory is not private'); + }; + const clearPreviousRecord = (): void => { + if (clearedThisProcess) return; + requirePrivateDirectory(); + if (file) fs.rmSync(file, { force: true }); + clearedThisProcess = true; + }; const persist = (): void => { if (!file) return; @@ -70,7 +94,8 @@ export function createRecoveryStore(dir?: string, opts: { log?: RecoveryLog } = // record. The `finally` is what keeps a failed write from leaving one behind. const tmp = `${file}.${randomUUID()}.tmp`; try { - fs.mkdirSync(path.dirname(file), { recursive: true, mode: 0o700 }); + requirePrivateDirectory(); + if (captureStarted) clearPreviousRecord(); // Mode on create, so the bytes are never briefly world-readable; the rename // preserves it. fs.writeFileSync(tmp, JSON.stringify(payload), { encoding: 'utf8', mode: 0o600 }); @@ -86,18 +111,18 @@ export function createRecoveryStore(dir?: string, opts: { log?: RecoveryLog } = persistent: file !== null, beginCapture(): void { - if (clearedThisProcess) return; - clearedThisProcess = true; // Clear before anything can return early. A record is only ever consumed by // a cold start that actually restores, so a teardown that captures nothing // must not leave the last one sitting there — otherwise a run that restores // nothing carries the record forward and a much later restore auto-runs a // week-old invocation unprompted. `record` re-creates it the moment // anything is detected. - captured = noCommands(); - if (!file) return; + if (!captureStarted) { + captureStarted = true; + captured = noCommands(); + } try { - fs.rmSync(file, { force: true }); + clearPreviousRecord(); } catch (err) { log.error(`[recovery] could not clear the previous record: ${String(err)}`); } @@ -105,14 +130,22 @@ export function createRecoveryStore(dir?: string, opts: { log?: RecoveryLog } = record(id: string, command: string): void { captured[id] = command; - // Persist on every change rather than once at the end. The write is a few - // hundred bytes and costs well under a millisecond, and the shutdown budget - // can end the capture at any instant. + // Persist each detection: the capture deadline can end the next scan. persist(); }, take(paneIds: Iterable): Record { - unclaimed ??= file ? readAndClearRecord(file, log) : captured; + if (unclaimed === null) { + try { + prepareDirectory(); + unclaimed = file ? readAndClearRecord(file, log) : captured; + } catch (err) { + // Preserve the durable record for a later successful preparation; + // nothing from an unprotected path is handed to a shell. + log.error(`[recovery] private record unavailable: ${String(err)}`); + return noCommands(); + } + } const claimed: Record = noCommands(); for (const id of paneIds) { const command = unclaimed[id]; @@ -136,6 +169,7 @@ export function createRecoveryStore(dir?: string, opts: { log?: RecoveryLog } = */ function readAndClearRecord(file: string, log: RecoveryLog): Record { if (!fs.existsSync(file)) return noCommands(); + ensurePrivateFileSync(file); let recovery: PersistedRecovery | null = null; try { diff --git a/lib/src/host/remote/enroll-offer.ts b/lib/src/host/remote/enroll-offer.ts index 2c6be8d02..38b2bbc9a 100644 --- a/lib/src/host/remote/enroll-offer.ts +++ b/lib/src/host/remote/enroll-offer.ts @@ -16,7 +16,7 @@ import { readFile } from 'node:fs/promises'; import { homedir } from 'node:os'; -import { join } from 'node:path'; +import * as path from 'node:path'; import { isEnrollmentOfferFresh, parseEnrollmentOffer, @@ -25,7 +25,7 @@ import { export type { EnrollmentOffer }; -const OFFER_FILE = join('run', 'enroll-offer.json'); +const OFFER_FILE = 'enroll-offer.json'; /** * Where each installer's offer lands, mirroring the install root that installer @@ -44,17 +44,18 @@ export function enrollmentOfferPath( env: NodeJS.ProcessEnv = process.env, home: string = homedir(), ): string | null { + const { join } = platform === 'win32' ? path.win32 : path.posix; switch (platform) { case 'darwin': - return join(home, 'Library', 'Application Support', 'Dormouse Relay', OFFER_FILE); + return join(home, 'Library', 'Application Support', 'Dormouse Relay', 'run', OFFER_FILE); case 'win32': // No `%LOCALAPPDATA%` is not a path to guess at: the installer joins onto // that variable, so without it this machine's install root is unknown. - return env.LOCALAPPDATA ? join(env.LOCALAPPDATA, 'Dormouse Relay', OFFER_FILE) : null; + return env.LOCALAPPDATA ? join(env.LOCALAPPDATA, 'Dormouse Relay', 'run', OFFER_FILE) : null; default: // `||` and not `??`, matching the installers' `${XDG_DATA_HOME:-…}`: an // empty value is unset, not a root at the filesystem's top. - return join(env.XDG_DATA_HOME || join(home, '.local', 'share'), 'dormouse-relay', OFFER_FILE); + return join(env.XDG_DATA_HOME || join(home, '.local', 'share'), 'dormouse-relay', 'run', OFFER_FILE); } } diff --git a/lib/src/lib/mirrored-constants.test.ts b/lib/src/lib/mirrored-constants.test.ts index 5ad87f0e1..1138196ce 100644 --- a/lib/src/lib/mirrored-constants.test.ts +++ b/lib/src/lib/mirrored-constants.test.ts @@ -1,6 +1,6 @@ import { readFileSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; -import { dirname, join, resolve } from 'node:path'; +import { dirname, join, resolve, win32 } from 'node:path'; import { describe, expect, it } from 'vitest'; import { PAIRING_OUTCOME_COPY, @@ -137,12 +137,12 @@ describe('enrollment-offer path mirrors the installers', () => { const source = readRepoFile(file); const variable = extract(source, file, /^\$INSTALL_ROOT = Join-Path \$env:(\w+) '[^']+'$/m); const local = 'C:\\Users\\ned\\AppData\\Local'; - const root = join( + const root = win32.join( local, extract(source, file, /^\$INSTALL_ROOT = Join-Path \$env:\w+ '([^']+)'$/m), ); - const run = join(root, extract(source, file, /^\$RUN_DIR = Join-Path \$INSTALL_ROOT '([^']+)'$/m)); - const offerFile = join( + const run = win32.join(root, extract(source, file, /^\$RUN_DIR = Join-Path \$INSTALL_ROOT '([^']+)'$/m)); + const offerFile = win32.join( run, extract(source, file, /^\$ENROLL_OFFER_FILE = Join-Path \$RUN_DIR '([^']+)'$/m), ); diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index 680f1505f..4fa85606c 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -2,7 +2,7 @@ "AGENTS.md": 3600, "SECURITY.md": 200, "SELF_HOST.md": 6100, - "docs/compatible-agents.md": 1750, + "docs/compatible-agents.md": 1850, "docs/specs/alert.md": 8650, "docs/specs/auto-update.md": 1450, "docs/specs/deploy.md": 1950, @@ -36,9 +36,9 @@ "docs/specs/terminal-state.md": 2400, "docs/specs/theme.md": 2400, "docs/specs/tiling-engine.md": 4450, - "docs/specs/transport.md": 5050, + "docs/specs/transport.md": 4750, "docs/specs/tutorial.md": 2050, - "docs/specs/vscode.md": 7300, + "docs/specs/vscode.md": 7150, "docs/specs/webgl-text.md": 1200, "docs/specs/website-docs.md": 5000 } diff --git a/standalone/sidecar/pty-core.js b/standalone/sidecar/pty-core.js index bd2709448..ac5edfcd7 100644 --- a/standalone/sidecar/pty-core.js +++ b/standalone/sidecar/pty-core.js @@ -209,7 +209,8 @@ function withoutInheritedMsysOriginalPath(env, platform = process.platform) { // glob); `DORMOUSE_SHELL_INTEGRATION_DIR` overrides it for hosts that stage the // sidecar elsewhere (e.g. the VS Code bundle) and for tests. function resolveShellIntegrationDir(env, runtime = {}) { - return env.DORMOUSE_SHELL_INTEGRATION_DIR || path.join(runtime.dirname || __dirname, 'shell-integration'); + const platformPath = (runtime.platform || process.platform) === 'win32' ? path.win32 : path.posix; + return env.DORMOUSE_SHELL_INTEGRATION_DIR || platformPath.join(runtime.dirname || __dirname, 'shell-integration'); } // Basename of a shell path, lowercased and with any `.exe` dropped, handling @@ -258,11 +259,12 @@ function winPathToWslMount(winPath) { // their shell. bash is the only WSL shell we integrate for now. function applyShellIntegration(shell, env, shellArgs, integrationDir, runtime = {}) { const fsModule = runtime.fsModule || fs; + const platformPath = (runtime.platform || process.platform) === 'win32' ? path.win32 : path.posix; const stem = shellStem(shell); if (stem === 'zsh') { - const zshDir = path.join(integrationDir, 'zsh'); - if (fileExists(path.join(zshDir, '.zshrc'), fsModule)) { + const zshDir = platformPath.join(integrationDir, 'zsh'); + if (fileExists(platformPath.join(zshDir, '.zshrc'), fsModule)) { return { env: { ...env, ZDOTDIR: zshDir, USER_ZDOTDIR: env.ZDOTDIR || env.HOME || '' }, shellArgs, @@ -271,14 +273,14 @@ function applyShellIntegration(shell, env, shellArgs, integrationDir, runtime = } if (stem === 'bash' && bashArgsAreInjectable(shellArgs)) { - const script = path.join(integrationDir, 'bash', 'shellIntegration.bash'); + const script = platformPath.join(integrationDir, 'bash', 'shellIntegration.bash'); if (fileExists(script, fsModule)) { return { env, shellArgs: ['--init-file', script] }; } } if (stem === 'pwsh' || stem === 'powershell') { - const script = path.join(integrationDir, 'pwsh', 'shellIntegration.ps1'); + const script = platformPath.join(integrationDir, 'pwsh', 'shellIntegration.ps1'); if (fileExists(script, fsModule)) { const integratedArgs = powerShellIntegratedArgs(shellArgs, script); if (integratedArgs) return { env, shellArgs: integratedArgs }; @@ -287,7 +289,7 @@ function applyShellIntegration(shell, env, shellArgs, integrationDir, runtime = // WSL: only the standard `-d ` launch (the shape the picker emits). if (stem === 'wsl' && shellArgs.length === 2 && shellArgs[0] === '-d') { - const script = path.join(integrationDir, 'bash', 'shellIntegration.bash'); + const script = platformPath.join(integrationDir, 'bash', 'shellIntegration.bash'); const mount = winPathToWslMount(script); if (mount && fileExists(script, fsModule)) { // A `sh -c` detector, passed as one argv element so node-pty hands it to diff --git a/vscode-ext/src/extension.ts b/vscode-ext/src/extension.ts index 5f6f50225..a3bf6eef9 100644 --- a/vscode-ext/src/extension.ts +++ b/vscode-ext/src/extension.ts @@ -8,7 +8,7 @@ import { serveWebview } from './webview-messaging'; import { log } from './log'; import { initToolHost } from './tool-host'; import { forgetRetiredState } from './retired-state'; -import { captureAgentRecoveryCommands, mergeAlertStates, refreshSavedSessionStateFromPtys, takeRecoveryCommands } from './session-state'; +import { captureAgentRecoveryCommands, mergeAlertStates, prepareRecoveryStorage, refreshSavedSessionStateFromPtys, takeRecoveryCommands } from './session-state'; import { readPersistedSession } from '../../lib/src/lib/session-types'; import { workspaceTitle } from './workspace-chrome'; import { resolveSelectedShell, setSelectedShellPath, getSelectedShellPath } from './shell-selection'; @@ -91,6 +91,7 @@ export function activate(context: vscode.ExtensionContext) { context.subscriptions.push(vscode.window.onDidChangeWindowState(reportWindowPresence)); initToolHost(context.globalStorageUri?.fsPath); log.init(); + prepareRecoveryStorage(context); extensionContext = context; ptyManager.setExtensionPath(context.extensionPath); const dorRuntime = ptyManager.getDorRuntimeEnv(context.extensionPath); diff --git a/vscode-ext/src/peer-link-protocol.ts b/vscode-ext/src/peer-link-protocol.ts index 0ca0ab850..dc52005f4 100644 --- a/vscode-ext/src/peer-link-protocol.ts +++ b/vscode-ext/src/peer-link-protocol.ts @@ -103,7 +103,7 @@ export class FrameDecoder { #discarding = false; readonly #maxFrameBytes: number; - /** Bounds a peer that never sends a newline; the default fits a screenful. */ + /** Bounds each UTF-8 frame before JSON parsing; the default fits a screenful. */ constructor(maxFrameBytes = 4 * 1024 * 1024) { this.#maxFrameBytes = maxFrameBytes; } @@ -122,6 +122,7 @@ export class FrameDecoder { this.#discarding = false; continue; } + if (Buffer.byteLength(line, 'utf8') > this.#maxFrameBytes) continue; if (!line.trim()) continue; try { frames.push(JSON.parse(line)); @@ -133,7 +134,7 @@ export class FrameDecoder { // can never read, so it goes — but the whole frames already taken out of // the buffer above are real, and dropping them with it would lose traffic // from a link that is otherwise healthy. - if (this.#buffer.length > this.#maxFrameBytes) this.#discarding = true; + if (Buffer.byteLength(this.#buffer, 'utf8') > this.#maxFrameBytes) this.#discarding = true; if (this.#discarding) this.#buffer = ''; return frames; } diff --git a/vscode-ext/src/session-state.ts b/vscode-ext/src/session-state.ts index b629c6a81..a85284c3a 100644 --- a/vscode-ext/src/session-state.ts +++ b/vscode-ext/src/session-state.ts @@ -85,23 +85,10 @@ export async function refreshSavedSessionStateFromPtys( log.info(`[session] refreshFromPtys: saved ${panes.length} panes`); } -/** - * This activation's recovery record, in extension storage. - * - * A plain file, written synchronously — NOT `workspaceState`. - * `workspaceState.update()` hands the value to VS Code's storage service, which - * batches its SQLite flush on its own schedule. By the time `deactivate()` runs - * that service is already tearing down, so the write never reaches disk however - * early it is issued: measured on a real machine, detection completed at +276ms - * and the record still never appeared. A synchronous `writeFileSync` is durable - * the instant it returns and needs no budget at all. - * - * The store itself is the Tauri sidecar's (`lib/src/host/recovery-store.ts`) — - * one record format, one destructive read, one set of file modes for both hosts. - * Created once per activation, because the remainder of a claimed record lives in - * it: `captureAgentRecoveryCommands` interrupts every live PTY, and those panes - * are spread across the Dormouse view and any number of editor panels, each - * restoring its own pane ids from its own saved state. +/** One recovery store per activation, prepared before teardown. It shares the + * sidecar's private record and destructive claim implementation; each webview + * claims only its saved pane ids. Synchronous file writes avoid VS Code's + * teardown storage-service flush (vscode.md -> Capturing agent recovery). */ let store: RecoveryStore | null = null; function recoveryStore(context: vscode.ExtensionContext): RecoveryStore { @@ -112,6 +99,12 @@ function recoveryStore(context: vscode.ExtensionContext): RecoveryStore { return store; } +/** Prepare private storage during activation, before a bounded teardown can + * need it. A failed permission setup is logged and persistence fails closed. */ +export function prepareRecoveryStorage(context: vscode.ExtensionContext): void { + recoveryStore(context); +} + /** * Interrupt the live PTYs, then record each pane's agent resume invocation. * diff --git a/vscode-ext/test/helpers.ts b/vscode-ext/test/helpers.ts index 8ce8520a3..e56f6769e 100644 --- a/vscode-ext/test/helpers.ts +++ b/vscode-ext/test/helpers.ts @@ -33,7 +33,9 @@ export async function tempStorageDir(): Promise { */ export function derivedSocketPath(storageDir: string): string { const id = createHash('sha256').update(storageDir).digest('hex').slice(0, 12); - return join(tmpdir(), `dormouse-peer-${process.getuid?.() ?? 0}`, `${id}.sock`); + return process.platform === 'win32' + ? `\\\\.\\pipe\\dormouse-peer-${id}` + : join(tmpdir(), `dormouse-peer-${process.getuid?.() ?? 0}`, `${id}.sock`); } export async function removeDir(dir: string): Promise { diff --git a/vscode-ext/test/peer-link-protocol.test.ts b/vscode-ext/test/peer-link-protocol.test.ts index 221655aa1..233d7fc83 100644 --- a/vscode-ext/test/peer-link-protocol.test.ts +++ b/vscode-ext/test/peer-link-protocol.test.ts @@ -18,6 +18,31 @@ import { } from '../src/peer-link-protocol'; describe('FrameDecoder', () => { + it('caps complete UTF-8 frames before parsing and preserves surrounding frames', () => { + const good = { kind: 'notify' } as const; + for (const data of ['x'.repeat(100), '\u00e9'.repeat(20)]) { + const encoded = encodeFrame({ kind: 'data', ptyId: 'p', data }); + expect(Buffer.byteLength(encoded.slice(0, -1), 'utf8')).toBeGreaterThan(64); + const decoder = new FrameDecoder(64); + expect(decoder.push(encodeFrame(good) + encoded + encodeFrame(good))).toEqual([good, good]); + } + }); + + it('discards an oversized multibyte partial frame through its newline', () => { + const decoder = new FrameDecoder(64); + const encoded = encodeFrame({ kind: 'data', ptyId: 'p', data: '\u00e9'.repeat(20) }); + expect(decoder.push(encoded.slice(0, -1))).toEqual([]); + expect(decoder.push('\n' + encodeFrame({ kind: 'notify' }))).toEqual([{ kind: 'notify' }]); + }); + + it('accepts exactly the UTF-8 byte cap and rejects one byte more', () => { + const frame = { kind: 'data', ptyId: 'p', data: '\u00e9'.repeat(20) } as const; + const encoded = encodeFrame(frame); + const cap = Buffer.byteLength(encoded.slice(0, -1), 'utf8'); + expect(new FrameDecoder(cap).push(encoded)).toEqual([frame]); + expect(new FrameDecoder(cap - 1).push(encoded)).toEqual([]); + }); + it('reads one frame per line', () => { const decoder = new FrameDecoder(); const frames = decoder.push( diff --git a/vscode-ext/test/peer-link.test.ts b/vscode-ext/test/peer-link.test.ts index 1913b0199..cfda1ca57 100644 --- a/vscode-ext/test/peer-link.test.ts +++ b/vscode-ext/test/peer-link.test.ts @@ -170,6 +170,44 @@ afterEach(async () => { }); describe('bind-as-lease', () => { + it.skipIf(process.platform !== 'win32')('reclaims a dead named pipe and elects exactly one broker for racing windows', async () => { + // Windows removes the pipe when its process dies; there is no Unix corpse + // inode to unlink. Exercise the actual transport, not a fake socket path. + const corpse = spawn(process.execPath, ['-e', + "require('node:net').createServer().listen(process.argv[1], () => process.send('listening'))", + derivedSocketPath()], { stdio: ['ignore', 'ignore', 'pipe', 'ipc'] }); + try { + await new Promise((resolve, reject) => { + corpse.once('message', () => resolve()); + corpse.once('error', reject); + corpse.once('exit', (code) => reject(new Error(`pipe fixture exited ${code}`))); + }); + const exited = new Promise((resolve) => corpse.once('exit', () => resolve())); + corpse.kill('SIGKILL'); + await exited; + const firstSide = fakeWindow({ entries: [{ surfaceId: 'first' }] }); + const secondSide = fakeWindow({ entries: [{ surfaceId: 'second' }] }); + const first = await openWindow(firstSide); + const second = await openWindow(secondSide); + const roles: boolean[] = []; + await Promise.all([ + first.ensurePeerNet((held) => roles.push(held)), + second.ensurePeerNet((held) => roles.push(held)), + ]); + const brokers = [first, second].filter((mod) => mod.isPeerBroker()); + expect(brokers).toHaveLength(1); + expect(roles).toEqual([true]); + const expected = first.isPeerBroker() ? secondSide.entries : firstSide.entries; + await waitFor(async () => JSON.stringify(await brokers[0].remoteRequest('directory', {})) === JSON.stringify(expected)); + } finally { + if (corpse.exitCode === null && corpse.signalCode === null) { + const exited = new Promise((resolve) => corpse.once('exit', () => resolve())); + corpse.kill('SIGKILL'); + await exited; + } + } + }, 15_000); + it('rejects when the peer socket cannot be bound', async () => { const mod = await openWindow(fakeWindow()); const failingServer = createServer(); @@ -184,7 +222,7 @@ describe('bind-as-lease', () => { // libuv callback with nothing to catch it. const mod = await openWindow(fakeWindow()); const server = createServer(); - const path = join(dir, 'accept-error.sock'); + const path = derivedSocketPath(); await mod.listenServer(server, path); try { expect(() => server.emit('error', Object.assign(new Error('EMFILE'), { code: 'EMFILE' }))) @@ -258,13 +296,13 @@ describe('bind-as-lease', () => { releaseStuck(); }, 15_000); - it('does not answer broker while a reclaimed bind is still unverified', async () => { + it.skipIf(process.platform === 'win32')('does not answer broker while a reclaimed bind is still unverified', async () => { // `stillOurs` spends 250 ms watching for a window that cleared the same // corpse and bound after us. An enroll landing inside that window used to // see a bound socket, start a service, and the stand-down path // (`closeServer(false)`) never tears one down — two Burrows under one burrowId. const path = derivedSocketPath(); - await mkdir(dirname(path), { recursive: true, mode: 0o700 }); + if (process.platform !== 'win32') await mkdir(dirname(path), { recursive: true, mode: 0o700 }); const corpse = spawn(process.execPath, [ '-e', `require('node:net').createServer().listen(${JSON.stringify(path)})`, @@ -382,9 +420,9 @@ describe('bind-as-lease', () => { expect(roles).toEqual([true, true]); }); - it('takes over a socket whose broker died without unlinking it', async () => { + it.skipIf(process.platform === 'win32')('takes over a socket whose broker died without unlinking it', async () => { const path = derivedSocketPath(); - await mkdir(dirname(path), { recursive: true, mode: 0o700 }); + if (process.platform !== 'win32') await mkdir(dirname(path), { recursive: true, mode: 0o700 }); // A killed process leaves the inode behind — `close()` would unlink it, so // the only way to produce this state is to not let the owner close. const corpse = spawn(process.execPath, [ @@ -405,13 +443,13 @@ describe('bind-as-lease', () => { expect(mod.isPeerBroker()).toBe(true); }); - it('re-binds when the socket it reclaimed is unlinked out from under it', async () => { + it.skipIf(process.platform === 'win32')('re-binds when the socket it reclaimed is unlinked out from under it', async () => { // Two windows can clear the same corpse and the second bind displaces the // first without any error — the loser keeps serving an inode no client can // reach. On unix a path that has *gone* after our bind is the same failure, // and reading it as "still ours" leaves a broker nobody can dial. const path = derivedSocketPath(); - await mkdir(dirname(path), { recursive: true, mode: 0o700 }); + if (process.platform !== 'win32') await mkdir(dirname(path), { recursive: true, mode: 0o700 }); const corpse = spawn(process.execPath, [ '-e', `require('node:net').createServer().listen(${JSON.stringify(path)})`, @@ -446,13 +484,13 @@ describe('bind-as-lease', () => { expect(peer.isPeerBroker()).toBe(false); }, 30_000); - it('settles two windows racing for one corpse into a broker and a client', async () => { + it.skipIf(process.platform === 'win32')('settles two windows racing for one corpse into a broker and a client', async () => { // Both find the same dead socket, both may unlink it, and the second bind // silently displaces the first. Whoever loses that has to notice and stand // down rather than serve an inode nobody can reach — and must then end up a // client, not wedged. const path = derivedSocketPath(); - await mkdir(dirname(path), { recursive: true, mode: 0o700 }); + if (process.platform !== 'win32') await mkdir(dirname(path), { recursive: true, mode: 0o700 }); const corpse = spawn(process.execPath, [ '-e', `require('node:net').createServer().listen(${JSON.stringify(path)})`, @@ -478,14 +516,14 @@ describe('bind-as-lease', () => { expect(firstRoles.concat(secondRoles)).toEqual([true]); }, 30_000); - it('stands down when a competing reclaim displaces it before its verification reads the path', async () => { + it.skipIf(process.platform === 'win32')('stands down when a competing reclaim displaces it before its verification reads the path', async () => { // The interleaving the racing test above reaches only by luck, forced: a // competing window's unlink and rebind land after our bind but before // `stillOurs` first reads the path. Anchored to that read rather than to // our own bind, both windows would name the competitor's socket as "ours" // and both would broker. const path = derivedSocketPath(); - await mkdir(dirname(path), { recursive: true, mode: 0o700 }); + if (process.platform !== 'win32') await mkdir(dirname(path), { recursive: true, mode: 0o700 }); const corpse = spawn(process.execPath, [ '-e', `require('node:net').createServer().listen(${JSON.stringify(path)})`, @@ -878,7 +916,7 @@ describe('bind-as-lease', () => { } }); }); - await mkdir(dirname(derivedSocketPath()), { recursive: true, mode: 0o700 }); + if (process.platform !== 'win32') await mkdir(dirname(derivedSocketPath()), { recursive: true, mode: 0o700 }); await new Promise((resolve, reject) => { server.once('error', reject); server.listen(derivedSocketPath(), () => { @@ -1205,7 +1243,7 @@ describe('peer handshake', () => { })(); }); const path = derivedSocketPath(); - await mkdir(dirname(path), { recursive: true, mode: 0o700 }); + if (process.platform !== 'win32') await mkdir(dirname(path), { recursive: true, mode: 0o700 }); await new Promise((resolve) => squatter.listen(path, resolve)); try { @@ -1245,7 +1283,7 @@ describe('peer handshake', () => { socket.write('null\n'); }); const path = derivedSocketPath(); - await mkdir(dirname(path), { recursive: true, mode: 0o700 }); + if (process.platform !== 'win32') await mkdir(dirname(path), { recursive: true, mode: 0o700 }); await new Promise((resolve) => squatter.listen(path, resolve)); try { @@ -1266,7 +1304,7 @@ describe('peer handshake', () => { } }); - it('keeps the socket directory private to this user', async () => { + it.skipIf(process.platform === 'win32')('keeps the socket directory private to this user', async () => { // The layer below the handshake: in a shared tmpdir, a directory anyone can // write to is one where a co-resident user can create the path first. const peerDir = dirname(derivedSocketPath()); @@ -1281,7 +1319,7 @@ describe('peer handshake', () => { expect((await stat(peerDir)).mode & 0o777).toBe(0o700); }); - it('stands down for good when the socket directory is not one', async () => { + it.skipIf(process.platform === 'win32')('stands down for good when the socket directory is not one', async () => { // Something else holds the only place these sockets may live. No amount of // retrying changes that, so the link stops rather than spinning — and the // waiting caller is released rather than left hanging. From 9a2e1ca69430f67856bff3ce36c8444821151fc0 Mon Sep 17 00:00:00 2001 From: Ned Date: Thu, 1 Oct 2026 19:55:40 -0700 Subject: [PATCH 02/10] Use Windows paths in Windows-target shell fixtures on every CI host --- standalone/sidecar/pty-core.test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/standalone/sidecar/pty-core.test.js b/standalone/sidecar/pty-core.test.js index 94cf5ee50..7d467141b 100644 --- a/standalone/sidecar/pty-core.test.js +++ b/standalone/sidecar/pty-core.test.js @@ -1075,7 +1075,7 @@ test('resolveSpawnConfig injects bash integration for Git Bash despite its --log // The --login -i defaults are subsumed by the init-file script, which sources // the login profile itself. - const script = path.join(integrationDir, 'bash', 'shellIntegration.bash'); + const script = path.win32.join(integrationDir, 'bash', 'shellIntegration.bash'); assert.deepEqual(config.shellArgs, ['--init-file', script]); }); @@ -1128,7 +1128,7 @@ test('resolveSpawnConfig injects pwsh integration via -NoExit -Command dot-sourc }, ); - const script = path.join(integrationDir, 'pwsh', 'shellIntegration.ps1'); + const script = path.win32.join(integrationDir, 'pwsh', 'shellIntegration.ps1'); assert.deepEqual(config.shellArgs, ['-NoExit', '-Command', `. '${script}'`]); }); @@ -1149,7 +1149,7 @@ test('resolveSpawnConfig injects Windows PowerShell (powershell.exe) too', () => }, ); - const script = path.join(integrationDir, 'pwsh', 'shellIntegration.ps1'); + const script = path.win32.join(integrationDir, 'pwsh', 'shellIntegration.ps1'); assert.deepEqual(config.shellArgs, ['-NoExit', '-Command', `. '${script}'`]); }); @@ -1172,7 +1172,7 @@ test('resolveSpawnConfig merges integration into an interactive pwsh -Command (e ); // The dev-shell command runs first, then our dot-source installs the prompt wrapper. - const script = path.join(integrationDir, 'pwsh', 'shellIntegration.ps1'); + const script = path.win32.join(integrationDir, 'pwsh', 'shellIntegration.ps1'); assert.deepEqual(config.shellArgs, [ '-NoExit', '-Command', @@ -1195,7 +1195,7 @@ test('resolveSpawnConfig adds a -Command to an interactive pwsh launch that has }, ); - const script = path.join(integrationDir, 'pwsh', 'shellIntegration.ps1'); + const script = path.win32.join(integrationDir, 'pwsh', 'shellIntegration.ps1'); assert.deepEqual(config.shellArgs, ['-NoExit', '-NoLogo', '-Command', `. '${script}'`]); }); From e9a7045e90a5b15052b432e0572c0c04f1955b63 Mon Sep 17 00:00:00 2001 From: Ned Date: Thu, 1 Oct 2026 20:22:44 -0700 Subject: [PATCH 03/10] test(vscode): prepare Unix peer socket parent --- vscode-ext/test/peer-link.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/vscode-ext/test/peer-link.test.ts b/vscode-ext/test/peer-link.test.ts index cfda1ca57..d4767111b 100644 --- a/vscode-ext/test/peer-link.test.ts +++ b/vscode-ext/test/peer-link.test.ts @@ -223,6 +223,7 @@ describe('bind-as-lease', () => { const mod = await openWindow(fakeWindow()); const server = createServer(); const path = derivedSocketPath(); + if (process.platform !== 'win32') await mkdir(dirname(path), { recursive: true, mode: 0o700 }); await mod.listenServer(server, path); try { expect(() => server.emit('error', Object.assign(new Error('EMFILE'), { code: 'EMFILE' }))) From b8fcdbd4743d6933c6a515a86fd48c5c97216d20 Mon Sep 17 00:00:00 2001 From: Ned Date: Thu, 1 Oct 2026 21:35:53 -0700 Subject: [PATCH 04/10] Address recovery privacy and async startup review findings --- docs/compatible-agents.md | 6 +- docs/compatible-agents.rationale.md | 2 +- docs/specs/security-local.md | 2 +- docs/specs/vscode.md | 2 +- lib/src/host/private-path.test.ts | 28 ++- lib/src/host/private-path.ts | 42 ++++- lib/src/host/recovery-store.test.ts | 159 ++++++++++++------ lib/src/host/recovery-store.ts | 57 ++++--- standalone/sidecar/main.js | 2 +- vscode-ext/src/extension.ts | 17 +- vscode-ext/src/session-state.ts | 4 +- vscode-ext/src/webview-view-provider.ts | 26 ++- vscode-ext/test/webview-view-provider.test.ts | 92 ++++++++++ 13 files changed, 329 insertions(+), 110 deletions(-) create mode 100644 vscode-ext/test/webview-view-provider.test.ts diff --git a/docs/compatible-agents.md b/docs/compatible-agents.md index 8e72c026b..8e7c51b60 100644 --- a/docs/compatible-agents.md +++ b/docs/compatible-agents.md @@ -101,12 +101,12 @@ Source of truth: `CODING_AGENTS` in `lib/src/lib/coding-agents.ts`; `detectResum ### Recovery record - **Must keep one rebuilt invocation per Surface in a host-owned, single-use record outside the persisted Session.** The renderer save path never derives or writes it. (rationale) -- **Must call `beginCapture` before capture can return early.** The first call per host process clears the previous record when private storage is available; subsequent calls merge, preserving captures from other Windows. (rationale) -- **Must persist every detection synchronously through `createRecoveryStore`**, using `recovery.json` in the host-selected directory, an owner-only temporary file, and atomic rename. **Must prepare the exact host-owned directory before any record bytes and tighten an existing record before claiming it**, using owner-only modes on Unix and a protected current-user-only DACL on Windows. Failed preparation preserves the previous record and permits no read, write, or unlink. Preparation occurs at startup; a cold-start claim may retry, but bounded capture never launches the permission helper. A failed write must not throw through teardown. Without a directory the store is memory-only and logs that limitation once. +- **Must call `beginCapture` before capture can return early.** The first call per host process clears the previous record even if private-storage preparation failed; subsequent calls merge, preserving captures from other Windows. (rationale) +- **Must persist every detection synchronously through `createRecoveryStore`**, through a host-selected `recovery.json`, an owner-only temporary file, and atomic rename. **Must prepare the exact host-owned directory before any record bytes and tighten an existing record before claiming it**, using owner-only modes on Unix and a protected current-user-only DACL on Windows. Failed preparation permits no record-byte read or write; capture still removes the stale record. **Must start preparation asynchronously at startup and await it only for cold-start claims.** Claims may retry failed preparation, but bounded capture never waits for or launches the permission helper. Concurrent claims share one destructive read; if capture starts during setup, the claim must neither return old commands nor remove the new record. Failed writes must not escape teardown. Without a directory, use memory-only storage and log once. - **Must read and unlink the durable record on the first claim**, including on parse failure; if unlink fails, ignore it. Discard records older than 7 days after unlinking. Within the process, each container claims only its saved pane ids, and each entry is handed out once. (rationale) - **Must deliver claimed commands out of band on boot through `PlatformAdapter.getRecoveryCommands()`**; adapters whose hosts capture nothing may omit it. Only cold restore consumes these commands for execution; live resume never executes them. -Source of truth: `createRecoveryStore` in `lib/src/host/recovery-store.ts`; `ensurePrivateDirectorySync` / `ensurePrivateFileSync` in `lib/src/host/private-path.ts`, pinned by `lib/src/host/private-path.test.ts` and `lib/src/host/recovery-store.test.ts`; `PlatformAdapter` in `lib/src/lib/platform/types.ts`. +Source of truth: `createRecoveryStore` in `lib/src/host/recovery-store.ts`; `ensurePrivateDirectory` / `ensurePrivateFile` in `lib/src/host/private-path.ts`, pinned by `lib/src/host/private-path.test.ts` and `lib/src/host/recovery-store.test.ts`; `PlatformAdapter` in `lib/src/lib/platform/types.ts`. ### Cold restore diff --git a/docs/compatible-agents.rationale.md b/docs/compatible-agents.rationale.md index 3f7a43115..fd46735ba 100644 --- a/docs/compatible-agents.rationale.md +++ b/docs/compatible-agents.rationale.md @@ -60,7 +60,7 @@ Rows 1–2 are why a blanket second press is wrong; `Press Ctrl-C again` was abs **Why each webview claims only its own pane ids.** Two containers resolve inside one activation; a claim-everything read would let whichever resolved first delete the other's commands. Per-id claiming also means a disposed-and-re-resolved view restores without re-running the agent — its entries were already taken. -Windows Node mode bits left recovery files inheriting Everyone read access in an actual deliberately loose directory (Windows, 2026-10-01). A protected current-user-only inheritable DACL removed foreign grants from new files; legacy explicit file grants needed separate tightening. System PowerShell setup took about 303 ms in the probe, which motivated startup preparation and successful caching rather than spending the capture budget on each detection. +Windows Node mode bits left recovery files inheriting Everyone read access in an actual deliberately loose directory (Windows, 2026-10-01). A protected current-user-only inheritable DACL removed foreign grants from new files; legacy explicit file grants needed separate tightening. System PowerShell setup took about 303 ms in the probe, which motivated asynchronous startup preparation and successful caching rather than blocking activation or spending the capture budget on each detection. Deleting a stale record exposes no bytes, so capture can clear it even when permission setup fails. Windows may assign an elevated process's new path to the Administrators group; SetOwner and Set-Acl authorization, rather than a preexisting-owner equality test, governs migration to the current user. ## Cold restore diff --git a/docs/specs/security-local.md b/docs/specs/security-local.md index dde7b7b18..e8ac25ebc 100644 --- a/docs/specs/security-local.md +++ b/docs/specs/security-local.md @@ -211,7 +211,7 @@ does. A gap, not an accepted risk. - **FAIL IF** `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, asserted by `restrict_to_owner_leaves_one_owner_only_ace` — or if **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. `session_write_tightens_directory_and_existing_temp_file` pins unix modes; `session_permission_failures_preserve_previous_snapshot_without_writing_bytes` pins both failure gates. The mode reaches the temp file *before* any bytes are written (rationale). -- **FAIL IF** recovery storage is read, written, or unlinked before owner-only permission setup succeeds, or bounded capture launches permission setup. Read `createRecoveryStore` in `lib/src/host/recovery-store.ts` and `ensurePrivateDirectorySync` / `ensurePrivateFileSync` in `lib/src/host/private-path.ts`; `lib/src/host/private-path.test.ts` and `lib/src/host/recovery-store.test.ts` pin real Windows DACLs, Unix modes, legacy file grants, failure preservation, and capture-safe caching. +- **FAIL IF** recovery record bytes are read or written before owner-only permission setup succeeds, bounded capture waits for or launches permission setup, or failed preparation prevents capture from attempting to clear stale records. Read `createRecoveryStore` in `lib/src/host/recovery-store.ts` and `ensurePrivateDirectory` / `ensurePrivateFile` in `lib/src/host/private-path.ts`; `lib/src/host/private-path.test.ts` and `lib/src/host/recovery-store.test.ts` pin real Windows DACLs, Unix modes, legacy file grants, failed setup, capture-safe caching, and claims racing teardown. Source of truth: `SESSION_STATE_KEY` in `vscode-ext/src/session-state.ts`, `ensureToken` in `vscode-ext/src/peer-link.ts`, `default_log_path` in diff --git a/docs/specs/vscode.md b/docs/specs/vscode.md index b66404d6d..ac8134c85 100644 --- a/docs/specs/vscode.md +++ b/docs/specs/vscode.md @@ -138,7 +138,7 @@ needs no host-side per-panel store. #### Capturing agent recovery -**Must offer every live extension-host PTY to shared capture**, across the view and editor panels. **Must store the record under `storageUri`, falling back to `globalStorageUri`; never `workspaceState`** (rationale). If neither directory exists, skip capture. **Must prepare private recovery storage at activation; if preparation fails, bounded teardown must skip disk mutation rather than launch a permission helper.** Cold-start claims may retry. Shared record behavior follows `docs/compatible-agents.md` → Recovery record. +**Must offer every live extension-host PTY to shared capture**, across the view and editor panels. **Must store the record under `storageUri`, falling back to `globalStorageUri`; never `workspaceState`** (rationale). If neither directory exists, skip capture. **Must start private recovery-storage preparation asynchronously at activation.** Cold-start webviews await claims; bounded teardown never waits for or launches permission setup. Shared record behavior follows `docs/compatible-agents.md` → Recovery record. Source of truth: `prepareRecoveryStorage` / `captureAgentRecoveryCommands` / `takeRecoveryCommands` in `vscode-ext/src/session-state.ts`; `interrupt` in `vscode-ext/src/pty-manager.ts`. diff --git a/lib/src/host/private-path.test.ts b/lib/src/host/private-path.test.ts index b4d47d38b..02db04061 100644 --- a/lib/src/host/private-path.test.ts +++ b/lib/src/host/private-path.test.ts @@ -2,8 +2,8 @@ import * as fs from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { ensurePrivateDirectorySync, ensurePrivateFileSync } from './private-path'; -import { readAcl, seedEveryoneRead } from './private-path.test-utils'; +import { ensurePrivateDirectory, ensurePrivateDirectorySync, ensurePrivateFileSync } from './private-path'; +import { readAcl, runAclScript, seedEveryoneRead } from './private-path.test-utils'; let dir: string; beforeEach(() => { dir = fs.mkdtempSync(join(tmpdir(), 'dormouse-private-path-')); }); @@ -57,3 +57,27 @@ describe('private recovery paths', () => { expect(fs.statSync(real).mode & 0o777).toBe(0o755); }); }); + +it.skipIf(process.platform !== 'win32')('can migrate an administrator-owned legacy directory when Windows authorizes it', async (context) => { + const nested = join(dir, 'elevated-legacy'); + fs.mkdirSync(nested); + try { + runAclScript(`$acl=Get-Acl -LiteralPath $p; + $acl.SetOwner([System.Security.Principal.SecurityIdentifier]::new('S-1-5-32-544')); + Set-Acl -LiteralPath $p -AclObject $acl`, nested); + } catch (error) { + // Unelevated Windows CI tokens cannot assign the administrators group owner. + // Do not turn a real ACL setup failure into a platform skip. + if (/privilege|not allowed|UnauthorizedAccess|IdentityNotMapped/.test(String(error))) { + context.skip(); + return; + } + throw error; + } + expect(readAcl(nested).owner).toBe('S-1-5-32-544'); + await ensurePrivateDirectory(nested); + const acl = readAcl(nested); + expect(acl.owner).toBe(acl.currentUser); + expect(acl.protected).toBe(true); + expect(acl.rules).toEqual([{ sid: acl.currentUser, rights: 0x001F01FF, allow: true, inheritance: 3 }]); +}, 10_000); diff --git a/lib/src/host/private-path.ts b/lib/src/host/private-path.ts index 550761e24..08758e8c3 100644 --- a/lib/src/host/private-path.ts +++ b/lib/src/host/private-path.ts @@ -6,16 +6,12 @@ */ import * as fs from 'node:fs'; import * as path from 'node:path'; -import { execFileSync } from 'node:child_process'; +import { execFile, execFileSync } from 'node:child_process'; const WINDOWS_PRIVATE_PATH = ` $ErrorActionPreference = 'Stop' $targetPath = [Console]::In.ReadToEnd() $sid = [System.Security.Principal.WindowsIdentity]::GetCurrent().User -$existing = Get-Acl -LiteralPath $targetPath -if ($existing.GetOwner([System.Security.Principal.SecurityIdentifier]).Value -ne $sid.Value) { - throw 'Recovery path must belong to this user' -} if ((Get-Item -LiteralPath $targetPath -Force).PSIsContainer) { $acl = [System.Security.AccessControl.DirectorySecurity]::new() $inheritance = [System.Security.AccessControl.InheritanceFlags]'ContainerInherit,ObjectInherit' @@ -35,7 +31,7 @@ $acl.AddAccessRule($rule) Set-Acl -LiteralPath $targetPath -AclObject $acl `; -function restrictToOwnerSync(target: string, directory: boolean): void { +function checkPath(target: string, directory: boolean): void { const info = fs.lstatSync(target); if (info.isSymbolicLink() || (directory ? !info.isDirectory() : !info.isFile())) { throw new Error(`Recovery path must be a plain ${directory ? 'directory' : 'file'}`); @@ -45,8 +41,14 @@ function restrictToOwnerSync(target: string, directory: boolean): void { fs.chmodSync(target, directory ? 0o700 : 0o600); return; } - const powershell = path.join(process.env.SystemRoot || 'C:\\Windows', 'System32', 'WindowsPowerShell', 'v1.0', 'powershell.exe'); - execFileSync(powershell, ['-NoProfile', '-NonInteractive', '-Command', WINDOWS_PRIVATE_PATH], { +} + +const powershell = () => path.join(process.env.SystemRoot || 'C:\\Windows', 'System32', 'WindowsPowerShell', 'v1.0', 'powershell.exe'); + +function restrictToOwnerSync(target: string, directory: boolean): void { + checkPath(target, directory); + if (process.platform !== 'win32') return; + execFileSync(powershell(), ['-NoProfile', '-NonInteractive', '-Command', WINDOWS_PRIVATE_PATH], { // Only literal path data crosses stdin: quotes, $, backticks and newlines // never become PowerShell code. Use the system executable, not PATH search. input: path.resolve(target), encoding: 'utf8', windowsHide: true, timeout: 5_000, @@ -64,3 +66,27 @@ export function ensurePrivateDirectorySync(dir: string): void { export function ensurePrivateFileSync(file: string): void { restrictToOwnerSync(file, false); } + +/** Startup and cold claims do not block the host event loop on Windows ACL setup. + * The OS authorizes SetOwner/Set-Acl; an elevated administrator-owned legacy + * path may be rewritten, and any refused operation still fails closed. */ +async function restrictToOwner(target: string, directory: boolean): Promise { + checkPath(target, directory); + if (process.platform !== 'win32') return; + await new Promise((resolve, reject) => { + const child = execFile(powershell(), ['-NoProfile', '-NonInteractive', '-Command', WINDOWS_PRIVATE_PATH], { + encoding: 'utf8', windowsHide: true, timeout: 5_000, maxBuffer: 64 * 1024, + }, (error) => error ? reject(error) : resolve()); + child.stdin!.on('error', () => { /* execFile reports process failure */ }); + child.stdin!.end(path.resolve(target)); + }); +} + +export async function ensurePrivateDirectory(dir: string): Promise { + fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); + await restrictToOwner(dir, true); +} + +export async function ensurePrivateFile(file: string): Promise { + await restrictToOwner(file, false); +} diff --git a/lib/src/host/recovery-store.test.ts b/lib/src/host/recovery-store.test.ts index d190ebb6a..ad76c11b4 100644 --- a/lib/src/host/recovery-store.test.ts +++ b/lib/src/host/recovery-store.test.ts @@ -32,12 +32,13 @@ afterEach(() => { fs.rmSync(dir, { recursive: true, force: true }); }); -describe('recovery store', () => { - describe('capture', () => { - it('replaces the previous record on the first beginCapture and merges after', () => { +describe('recovery store', async () => { + describe('capture', async () => { + it('replaces the previous record on the first beginCapture and merges after', async () => { write({ createdAt: Date.now(), commands: { old: 'claude --continue' } }); const store = createRecoveryStore(dir, { log }); + await store.ready; store.beginCapture(); // The stale record is gone before anything can be detected, so a teardown // that captures nothing cannot carry it forward. @@ -52,8 +53,9 @@ describe('recovery store', () => { expect(read().commands).toEqual({ a: 'claude --resume A', b: 'codex resume B' }); }); - it('writes the record and its directory owner-only', () => { + it('writes the record and its directory owner-only', async () => { const store = createRecoveryStore(dir, { log }); + await store.ready; store.beginCapture(); store.record('a', 'claude --continue'); const mode = (path: string) => fs.statSync(path).mode & 0o777; @@ -68,19 +70,20 @@ describe('recovery store', () => { expect(fs.readdirSync(dir)).toEqual(['recovery.json']); }); - it('does not throw when the record cannot be written', () => { + it('does not throw when the record cannot be written', async () => { // A file where the state directory should be: `mkdirSync` cannot make it. const blocked = join(dir, 'blocked'); fs.writeFileSync(blocked, 'not a directory', 'utf8'); const store = createRecoveryStore(blocked, { log }); + await store.ready; store.beginCapture(); expect(() => store.record('a', 'claude --continue')).not.toThrow(); expect(messages.some((message) => message.startsWith('error [recovery] write failed'))).toBe(true); // Nothing was captured, so nothing can be claimed either. - expect(store.take(['a'])).toEqual({}); + expect(await store.take(['a'])).toEqual({}); }); - it('leaves no temp behind when the rename onto the record fails', () => { + it('leaves no temp behind when the rename onto the record fails', async () => { // A directory where the record should be: the temp is written, the rename // over it cannot succeed. A fixed `recovery.json.tmp` would sit there for // the next run — and for any other host sharing this directory — to rename @@ -88,6 +91,7 @@ describe('recovery store', () => { fs.mkdirSync(file()); const store = createRecoveryStore(dir, { log }); + await store.ready; store.beginCapture(); expect(() => store.record('a', 'claude --continue')).not.toThrow(); @@ -96,119 +100,169 @@ describe('recovery store', () => { }); }); - describe('take', () => { - it('never hands out commands when removing the durable record fails', () => { + describe('take', async () => { + it('never hands out commands when removing the durable record fails', async () => { write({ createdAt: Date.now(), commands: { a: 'claude --continue' } }); const store = createRecoveryStore(dir, { log }); + await store.ready; vi.mocked(fs.unlinkSync).mockImplementationOnce(() => { throw new Error('unlink refused'); }); - expect(store.take(['a'])).toEqual({}); - expect(store.take(['a'])).toEqual({}); + expect(await store.take(['a'])).toEqual({}); + expect(await store.take(['a'])).toEqual({}); expect(read().commands).toEqual({ a: 'claude --continue' }); expect(messages.some((message) => message.includes('could not clear record; ignoring it'))).toBe(true); }); - it.skipIf(process.platform !== 'win32')('tightens explicit legacy file grants before claiming the record', () => { + it.skipIf(process.platform !== 'win32')('tightens explicit legacy file grants before claiming the record', async () => { write({ createdAt: Date.now(), commands: { a: 'claude --continue' } }); seedEveryoneRead(file()); const store = createRecoveryStore(dir, { log }); - const original = privatePaths.ensurePrivateFileSync; + await store.ready; + const original = privatePaths.ensurePrivateFile; // Observe permissions immediately before the actual read and unlink. - const protect = vi.spyOn(privatePaths, 'ensurePrivateFileSync').mockImplementation((target) => { - original(target); + const protect = vi.spyOn(privatePaths, 'ensurePrivateFile').mockImplementation(async (target) => { + await original(target); const acl = readAcl(target); expect(acl.protected).toBe(true); expect(acl.rules).toEqual([{ sid: acl.currentUser, rights: 0x001F01FF, allow: true, inheritance: 0 }]); }); - expect(store.take(['a'])).toEqual({ a: 'claude --continue' }); + expect(await store.take(['a'])).toEqual({ a: 'claude --continue' }); expect(protect).toHaveBeenCalledWith(file()); protect.mockRestore(); expect(fs.existsSync(file())).toBe(false); }); - it('does not read or unlink a record whose private file setup fails', () => { + it('does not read or unlink a record whose private file setup fails', async () => { write({ createdAt: Date.now(), commands: { a: 'claude --continue' } }); const store = createRecoveryStore(dir, { log }); - const protect = vi.spyOn(privatePaths, 'ensurePrivateFileSync').mockImplementation(() => { throw new Error('ACL failed'); }); - expect(store.take(['a'])).toEqual({}); + await store.ready; + const protect = vi.spyOn(privatePaths, 'ensurePrivateFile').mockRejectedValue(new Error('ACL failed')); + expect(await store.take(['a'])).toEqual({}); expect(read().commands).toEqual({ a: 'claude --continue' }); protect.mockRestore(); - expect(store.take(['a'])).toEqual({ a: 'claude --continue' }); + expect(await store.take(['a'])).toEqual({ a: 'claude --continue' }); }); - it('unlinks on the first call and hands out each id exactly once', () => { + it('unlinks on the first call and hands out each id exactly once', async () => { write({ createdAt: Date.now(), commands: { a: 'claude --resume A', b: 'codex resume B' } }); const store = createRecoveryStore(dir, { log }); + await store.ready; - expect(store.take(['a'])).toEqual({ a: 'claude --resume A' }); + expect(await store.take(['a'])).toEqual({ a: 'claude --resume A' }); // The durable copy is gone before anything can act on it, so a failed start // cannot replay it. expect(fs.existsSync(file())).toBe(false); // A second container claims its share of the same read; the first id is // spent. - expect(store.take(['a', 'b'])).toEqual({ b: 'codex resume B' }); - expect(store.take(['b'])).toEqual({}); + expect(await store.take(['a', 'b'])).toEqual({ b: 'codex resume B' }); + expect(await store.take(['b'])).toEqual({}); }); - it('returns nothing when there is no record', () => { - expect(createRecoveryStore(dir, { log }).take(['a'])).toEqual({}); + it('returns nothing when there is no record', async () => { + expect(await createRecoveryStore(dir, { log }).take(['a'])).toEqual({}); }); - it('is destructive even on a record it cannot parse', () => { + it('is destructive even on a record it cannot parse', async () => { fs.writeFileSync(file(), '{ torn', 'utf8'); const store = createRecoveryStore(dir, { log }); - expect(store.take(['a'])).toEqual({}); + await store.ready; + expect(await store.take(['a'])).toEqual({}); expect(fs.existsSync(file())).toBe(false); }); - it('discards a record past its expiry, having removed it', () => { + it('discards a record past its expiry, having removed it', async () => { write({ createdAt: Date.now() - RECOVERY_MAX_AGE_MS - 1, commands: { a: 'claude --continue' } }); const store = createRecoveryStore(dir, { log }); - expect(store.take(['a'])).toEqual({}); + await store.ready; + expect(await store.take(['a'])).toEqual({}); expect(fs.existsSync(file())).toBe(false); }); - it('drops a non-string entry rather than handing it on', () => { + it('drops a non-string entry rather than handing it on', async () => { write({ createdAt: Date.now(), commands: { a: 'claude --continue', b: { evil: true } } }); const store = createRecoveryStore(dir, { log }); - expect(store.take(['a', 'b'])).toEqual({ a: 'claude --continue' }); + await store.ready; + expect(await store.take(['a', 'b'])).toEqual({ a: 'claude --continue' }); }); - it('cannot be tricked by an id that names an Object prototype member', () => { + it('cannot be tricked by an id that names an Object prototype member', async () => { write({ createdAt: Date.now(), commands: { constructor: 'claude --continue' } }); const store = createRecoveryStore(dir, { log }); + await store.ready; // A plain literal would answer `toString` with an inherited function. - expect(store.take(['toString'])).toEqual({}); - expect(store.take(['constructor'])).toEqual({ constructor: 'claude --continue' }); + expect(await store.take(['toString'])).toEqual({}); + expect(await store.take(['constructor'])).toEqual({ constructor: 'claude --continue' }); }); }); - it('never retries a failed startup helper during capture, then recovers on a cold-start claim', () => { + it('clears stale recovery despite failed startup privacy setup, without retrying during capture', async () => { write({ createdAt: Date.now(), commands: { old: 'claude --continue' } }); - const protect = vi.spyOn(privatePaths, 'ensurePrivateDirectorySync').mockImplementation(() => { throw new Error('helper timed out'); }); + const protect = vi.spyOn(privatePaths, 'ensurePrivateDirectory').mockRejectedValue(new Error('helper timed out')); const store = createRecoveryStore(dir, { log }); + await store.ready; expect(protect).toHaveBeenCalledTimes(1); store.beginCapture(); store.record('a', 'codex resume A'); store.beginCapture(); store.record('b', 'codex resume B'); expect(protect).toHaveBeenCalledTimes(1); - expect(read().commands).toEqual({ old: 'claude --continue' }); - expect(fs.readdirSync(dir)).toEqual(['recovery.json']); - expect(store.take([])).toEqual({}); - expect(protect).toHaveBeenCalledTimes(2); + expect(fs.existsSync(file())).toBe(false); + expect(fs.readdirSync(dir)).toEqual([]); + expect(await store.take([])).toEqual({}); + expect(protect).toHaveBeenCalledTimes(1); protect.mockRestore(); - // Only startup/claim retries expensive preparation. Captured invocations - // remain in memory and later writes merge after successful preparation. - expect(store.take([])).toEqual({}); + const next = createRecoveryStore(dir, { log }); + await next.ready; + expect(await next.take(['old'])).toEqual({}); + + }); + + it('starts permission setup without blocking and does not wait for it during capture', async () => { + write({ createdAt: Date.now(), commands: { old: 'claude --continue' } }); + let finish!: () => void; + const original = privatePaths.ensurePrivateDirectory; + const protect = vi.spyOn(privatePaths, 'ensurePrivateDirectory').mockImplementation(async (target) => { + await new Promise((resolve) => { finish = resolve; }); + await original(target); + }); + const store = createRecoveryStore(dir, { log }); + const claim = store.take(['old']); + store.beginCapture(); + store.record('new', 'codex resume NEW'); + expect(fs.existsSync(file())).toBe(false); + expect(protect).toHaveBeenCalledTimes(1); + finish(); + await store.ready; + expect(await claim).toEqual({}); + store.record('later', 'codex resume LATER'); + expect(read().commands).toEqual({ new: 'codex resume NEW', later: 'codex resume LATER' }); + expect(await store.take(['new', 'later'])).toEqual({}); + }); + + it('shares asynchronous legacy claims without removing a newly captured record', async () => { + write({ createdAt: Date.now(), commands: { old: 'claude --continue' } }); + const store = createRecoveryStore(dir, { log }); + await store.ready; + let entered!: () => void, finish!: () => void; + const started = new Promise((resolve) => { entered = resolve; }); + vi.spyOn(privatePaths, 'ensurePrivateFile').mockImplementation(async () => { + entered(); + await new Promise((resolve) => { finish = resolve; }); + }); + const first = store.take(['old']), second = store.take(['old']); + await started; store.beginCapture(); - store.record('c', 'codex resume C'); - expect(read().commands).toEqual({ a: 'codex resume A', b: 'codex resume B', c: 'codex resume C' }); + store.record('new', 'codex resume NEW'); + finish(); + expect(await first).toEqual({}); + expect(await second).toEqual({}); + expect(read().commands).toEqual({ new: 'codex resume NEW' }); }); - it('prepares a successful directory once across multiple captures and writes', () => { - const protect = vi.spyOn(privatePaths, 'ensurePrivateDirectorySync'); + it('prepares a successful directory once across multiple captures and writes', async () => { + const protect = vi.spyOn(privatePaths, 'ensurePrivateDirectory'); const store = createRecoveryStore(dir, { log }); + await store.ready; store.beginCapture(); store.record('a', 'claude --continue'); store.beginCapture(); @@ -217,16 +271,17 @@ describe('recovery store', () => { expect(read().commands).toEqual({ a: 'claude --continue', b: 'codex resume B' }); }); - describe('without a state directory', () => { - it('keeps the record in memory and says so once', () => { + describe('without a state directory', async () => { + it('keeps the record in memory and says so once', async () => { const store = createRecoveryStore(undefined, { log }); + await store.ready; expect(store.persistent).toBe(false); expect(messages.filter((message) => message.includes('no state directory'))).toHaveLength(1); store.beginCapture(); store.record('a', 'claude --continue'); - expect(store.take(['a'])).toEqual({ a: 'claude --continue' }); - expect(store.take(['a'])).toEqual({}); + expect(await store.take(['a'])).toEqual({ a: 'claude --continue' }); + expect(await store.take(['a'])).toEqual({}); }); }); }); diff --git a/lib/src/host/recovery-store.ts b/lib/src/host/recovery-store.ts index 41a6366d1..31fde8bab 100644 --- a/lib/src/host/recovery-store.ts +++ b/lib/src/host/recovery-store.ts @@ -14,7 +14,7 @@ import { randomUUID } from 'node:crypto'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { noCommands, silent, type RecoveryLog } from './recovery-capture'; -import { ensurePrivateDirectorySync, ensurePrivateFileSync } from './private-path'; +import { ensurePrivateDirectory, ensurePrivateFile } from './private-path'; const FILE_NAME = 'recovery.json'; @@ -29,6 +29,8 @@ interface PersistedRecovery { } export interface RecoveryStore { + /** Startup privacy setup has finished; failures are logged, never rejected here. */ + readonly ready: Promise; /** * A teardown is about to capture. The FIRST call of a process replaces * whatever the last run left; later calls merge, because a Window captures @@ -38,7 +40,7 @@ export interface RecoveryStore { /** Merge one detected invocation and persist immediately. */ record(id: string, command: string): void; /** Claim the commands belonging to `paneIds`, removing each as it is handed out. */ - take(paneIds: Iterable): Record; + take(paneIds: Iterable): Promise>; /** Whether a write survives this process. `false` is the no-directory store. */ readonly persistent: boolean; } @@ -64,16 +66,18 @@ export function createRecoveryStore(dir?: string, opts: { log?: RecoveryLog } = // What is left of the record on disk, once read. `null` until the first `take`. let unclaimed: Record | null = null; let directoryPrepared = false; - const prepareDirectory = (): void => { - if (!dir || directoryPrepared) return; - ensurePrivateDirectorySync(dir); - directoryPrepared = true; + let preparation: Promise | null = null; + const prepareDirectory = (): Promise => { + if (!dir || directoryPrepared) return Promise.resolve(); + preparation ??= ensurePrivateDirectory(dir).then(() => { directoryPrepared = true; }) + .finally(() => { preparation = null; }); + return preparation; }; - // Prime outside teardown: Windows ACL setup launches a bounded system process. - // Failed preparation remains retryable and never permits record bytes. - try { prepareDirectory(); } catch (err) { + // Begin startup work immediately without blocking extension activation or sidecar I/O. + const ready = prepareDirectory().catch((err) => { log.error(`[recovery] private directory unavailable: ${String(err)}`); - } + }); + let claim: Promise | null = null; const requirePrivateDirectory = (): void => { // Never launch a permission helper inside the bounded teardown capture. // Cold-start claims can retry a failed startup preparation. @@ -81,7 +85,8 @@ export function createRecoveryStore(dir?: string, opts: { log?: RecoveryLog } = }; const clearPreviousRecord = (): void => { if (clearedThisProcess) return; - requirePrivateDirectory(); + // Unlink exposes no record bytes. Even failed preparation must not preserve + // a stale invocation across a teardown that captured nothing. if (file) fs.rmSync(file, { force: true }); clearedThisProcess = true; }; @@ -109,6 +114,7 @@ export function createRecoveryStore(dir?: string, opts: { log?: RecoveryLog } = return { persistent: file !== null, + ready, beginCapture(): void { // Clear before anything can return early. A record is only ever consumed by @@ -120,6 +126,7 @@ export function createRecoveryStore(dir?: string, opts: { log?: RecoveryLog } = if (!captureStarted) { captureStarted = true; captured = noCommands(); + if (file) unclaimed = noCommands(); } try { clearPreviousRecord(); @@ -134,29 +141,32 @@ export function createRecoveryStore(dir?: string, opts: { log?: RecoveryLog } = persist(); }, - take(paneIds: Iterable): Record { + async take(paneIds: Iterable): Promise> { if (unclaimed === null) { - try { - prepareDirectory(); - unclaimed = file ? readAndClearRecord(file, log) : captured; - } catch (err) { - // Preserve the durable record for a later successful preparation; - // nothing from an unprotected path is handed to a shell. + claim ??= (async () => { + await ready; + await prepareDirectory(); + // Teardown can start while permission setup is pending. Never read + // this activation's newly captured record as a cold-start invocation. + unclaimed = file ? (captureStarted ? noCommands() : await readAndClearRecord(file, log, () => !captureStarted)) : captured; + })().finally(() => { claim = null; }); + try { await claim; } catch (err) { log.error(`[recovery] private record unavailable: ${String(err)}`); return noCommands(); } } + const remaining = unclaimed ?? noCommands(); const claimed: Record = noCommands(); for (const id of paneIds) { - const command = unclaimed[id]; + const command = remaining[id]; if (command === undefined) continue; claimed[id] = command; // Entries leave the map as they are claimed, so no id is ever handed out // twice — a second container claiming its share sees only the remainder. - delete unclaimed[id]; + delete remaining[id]; } log.info(`[recovery] handing ${Object.keys(claimed).length} command(s) to a cold restore` - + ` (${Object.keys(unclaimed).length} unclaimed)`); + + ` (${Object.keys(remaining).length} unclaimed)`); return claimed; }, }; @@ -167,9 +177,10 @@ export function createRecoveryStore(dir?: string, opts: { log?: RecoveryLog } = * the durable copy is gone before anything can act on it and a failed start * cannot replay it. */ -function readAndClearRecord(file: string, log: RecoveryLog): Record { +async function readAndClearRecord(file: string, log: RecoveryLog, mayClaim: () => boolean): Promise> { if (!fs.existsSync(file)) return noCommands(); - ensurePrivateFileSync(file); + await ensurePrivateFile(file); + if (!mayClaim()) return noCommands(); let recovery: PersistedRecovery | null = null; try { diff --git a/standalone/sidecar/main.js b/standalone/sidecar/main.js index cc0de18bd..ec877c9c0 100644 --- a/standalone/sidecar/main.js +++ b/standalone/sidecar/main.js @@ -190,7 +190,7 @@ function handleLine(line) { // on the first call, so nothing can replay them. case 'recovery:take': respondAsync('recovery:commands', data.requestId, async () => ({ - commands: recovery.take(Array.isArray(data.paneIds) ? data.paneIds : []), + commands: await recovery.take(Array.isArray(data.paneIds) ? data.paneIds : []), })); break; case 'pty:gracefulKill': mgr.gracefulKill(data.ids, data.timeout, data.requestId); break; diff --git a/vscode-ext/src/extension.ts b/vscode-ext/src/extension.ts index a3bf6eef9..d231f6a21 100644 --- a/vscode-ext/src/extension.ts +++ b/vscode-ext/src/extension.ts @@ -27,7 +27,7 @@ let extensionContext: vscode.ExtensionContext | null = null; * state VS Code preserved from the panel's `vscode.setState()`; for a fresh * panel opened via `dormouse.open` this is `undefined`. */ -function setupPanel( +async function setupPanel( context: vscode.ExtensionContext, panel: vscode.WebviewPanel, savedState?: unknown, @@ -50,18 +50,22 @@ function setupPanel( light: vscode.Uri.file(path.join(context.extensionPath, 'icon-tiny-light.png')), dark: vscode.Uri.file(path.join(context.extensionPath, 'icon-tiny-dark.png')), }; + let disposed = false; + let router: { dispose(): void } | undefined; + panel.onDidDispose(() => { disposed = true; router?.dispose(); }); const savedSession = readPersistedSession(initialState); // A panel's panes are interrupted by the teardown capture along with every // other live PTY, so they have a recovery command waiting too — claimed by // pane id, since the Dormouse view is claiming its own share of the same // record (docs/compatible-agents.md -> "Cold restore"). - const recoveryCommands = takeRecoveryCommands( + const recoveryCommands = await takeRecoveryCommands( context, (savedSession?.panes ?? []).map((pane) => pane.id), ); + if (disposed) return; const channel = serveWebview(panel.webview, mediaPath, initialState, getSelectedShell?.(), recoveryCommands); - const router = attachRouter(channel, { + router = attachRouter(channel, { reconnect: !!savedState, killOnDispose: true, getSelectedShell, @@ -71,7 +75,6 @@ function setupPanel( // Panels persist via vscode.setState() (per-panel, managed by VS Code). // Don't write to workspaceState — that's for the WebviewView only. }); - panel.onDidDispose(() => router.dispose()); } export function activate(context: vscode.ExtensionContext) { @@ -142,13 +145,13 @@ export function activate(context: vscode.ExtensionContext) { }), vscode.window.registerWebviewPanelSerializer('dormouse', { async deserializeWebviewPanel(panel: vscode.WebviewPanel, state: unknown) { - setupPanel(context, panel, state, () => provider.getSelectedShell()); + await setupPanel(context, panel, state, () => provider.getSelectedShell()); }, }), vscode.commands.registerCommand('dormouse.focus', () => { vscode.commands.executeCommand('dormouse.view.focus'); }), - vscode.commands.registerCommand('dormouse.open', () => { + vscode.commands.registerCommand('dormouse.open', async () => { const mediaPath = path.join(context.extensionPath, 'media'); const panel = vscode.window.createWebviewPanel( 'dormouse', @@ -160,7 +163,7 @@ export function activate(context: vscode.ExtensionContext) { localResourceRoots: [vscode.Uri.file(mediaPath)], }, ); - setupPanel(context, panel, undefined, () => provider.getSelectedShell()); + await setupPanel(context, panel, undefined, () => provider.getSelectedShell()); }), vscode.commands.registerCommand('dormouse.debugTheme', async () => { await vscode.commands.executeCommand('dormouse.view.focus'); diff --git a/vscode-ext/src/session-state.ts b/vscode-ext/src/session-state.ts index a85284c3a..e1316e249 100644 --- a/vscode-ext/src/session-state.ts +++ b/vscode-ext/src/session-state.ts @@ -163,9 +163,9 @@ export async function captureAgentRecoveryCommands( * the webview has nothing to write back and no save/restore cycle can resurrect it * (docs/compatible-agents.md -> "Cold restore"). */ -export function takeRecoveryCommands( +export async function takeRecoveryCommands( context: vscode.ExtensionContext, paneIds: Iterable, -): Record { +): Promise> { return recoveryStore(context).take(paneIds); } diff --git a/vscode-ext/src/webview-view-provider.ts b/vscode-ext/src/webview-view-provider.ts index 59a559a34..11b168d16 100644 --- a/vscode-ext/src/webview-view-provider.ts +++ b/vscode-ext/src/webview-view-provider.ts @@ -48,6 +48,17 @@ export class DormouseViewProvider implements vscode.WebviewViewProvider { _token: vscode.CancellationToken, ): Promise { this.view = view; + let disposed = false; + let ownedRouter: vscode.Disposable | undefined; + view.onDidDispose(() => { + disposed = true; + ownedRouter?.dispose(); + if (this.view !== view) return; + log.info('[view] onDidDispose fired - releasing router (PTYs remain alive)'); + this.routerDisposable = undefined; + this.channel = undefined; + this.view = undefined; + }); if (this.description !== undefined) view.description = this.description; const mediaPath = path.join(this.context.extensionPath, 'media'); @@ -62,6 +73,7 @@ export class DormouseViewProvider implements vscode.WebviewViewProvider { // is cached; this blocks only on a true cold start. if (!this.selectedShell) { const shells = await ptyManager.getAvailableShells(); + if (disposed || this.view !== view) return; const shell = resolveSelectedShell(this.context, shells); this.selectedShell = shell ? { shell: shell.path, args: shell.args } : null; if (shell) { @@ -70,6 +82,7 @@ export class DormouseViewProvider implements vscode.WebviewViewProvider { } } + if (disposed || this.view !== view) return; const savedSession = getSavedSessionState(this.context); // Recovery commands are claimed by this view's pane ids. const savedPaneIds = (savedSession?.panes ?? []).map((pane) => pane.id); @@ -79,13 +92,14 @@ export class DormouseViewProvider implements vscode.WebviewViewProvider { // Scoped to *this* view's panes because the capture interrupts every live PTY, // including any owned by an editor panel — taking the record whole would delete // their commands before the panel ever resolved. - const recoveryCommands = takeRecoveryCommands(this.context, savedPaneIds); + const recoveryCommands = await takeRecoveryCommands(this.context, savedPaneIds); + if (disposed || this.view !== view) return; this.channel = serveWebview( view.webview, mediaPath, savedSession, this.selectedShell, recoveryCommands, ); this.routerDisposable?.dispose(); - this.routerDisposable = attachRouter(this.channel, { + this.routerDisposable = ownedRouter = attachRouter(this.channel, { reconnect: true, onSaveState: (state) => { return saveSessionState(this.context, mergeAlertStates(state, getAlertStates())); @@ -101,13 +115,7 @@ export class DormouseViewProvider implements vscode.WebviewViewProvider { }, }); - view.onDidDispose(() => { - log.info('[view] onDidDispose fired — releasing router (PTYs remain alive)'); - this.routerDisposable?.dispose(); - this.routerDisposable = undefined; - this.channel = undefined; - this.view = undefined; - }); + } focus(): void { diff --git a/vscode-ext/test/webview-view-provider.test.ts b/vscode-ext/test/webview-view-provider.test.ts new file mode 100644 index 000000000..1ccde7877 --- /dev/null +++ b/vscode-ext/test/webview-view-provider.test.ts @@ -0,0 +1,92 @@ +import { beforeEach, expect, it, vi } from 'vitest'; + +const mocks = vi.hoisted(() => ({ + take: vi.fn(), serve: vi.fn(), attach: vi.fn(), shells: vi.fn(), +})); +vi.mock('../src/session-state', () => ({ + takeRecoveryCommands: mocks.take, getSavedSessionState: () => undefined, + mergeAlertStates: (state: unknown) => state, saveSessionState: vi.fn(), +})); +vi.mock('../src/message-router', () => ({ + attachRouter: mocks.attach, getAlertStates: () => new Map(), +})); +vi.mock('../src/webview-messaging', () => ({ serveWebview: mocks.serve })); +vi.mock('../src/pty-manager', () => ({ getAvailableShells: mocks.shells })); +import { DormouseViewProvider } from '../src/webview-view-provider'; + +function view() { + let dispose!: () => void; + const value = { webview: {}, onDidDispose: (callback: () => void) => { dispose = callback; } }; + return { value: value as never, dispose: () => dispose() }; +} +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((done) => { resolve = done; }); + return { promise, resolve }; +} +function provider() { + const value = new DormouseViewProvider({ extensionPath: 'extension' } as never); + value.setSelectedShell({ shell: 'cmd.exe' }); + return value; +} +beforeEach(() => { + vi.clearAllMocks(); + mocks.serve.mockReturnValue({ post: () => Promise.resolve(true) }); + mocks.attach.mockReturnValue({ dispose: vi.fn() }); +}); + +it('waits for the asynchronous recovery claim before serving the boot document', async () => { + const ready = deferred>(); + mocks.take.mockReturnValue(ready.promise); + const pending = provider().resolveWebviewView(view().value, {} as never, {} as never); + expect(mocks.serve).not.toHaveBeenCalled(); + ready.resolve({ pane: 'codex resume ID' }); + await pending; + expect(mocks.serve.mock.calls[0][4]).toEqual({ pane: 'codex resume ID' }); + expect(mocks.attach).toHaveBeenCalledTimes(1); +}); + +it('does not serve or attach a view disposed during its recovery claim', async () => { + const ready = deferred>(), target = view(); + mocks.take.mockReturnValue(ready.promise); + const pending = provider().resolveWebviewView(target.value, {} as never, {} as never); + target.dispose(); + ready.resolve({}); + await pending; + expect(mocks.serve).not.toHaveBeenCalled(); + expect(mocks.attach).not.toHaveBeenCalled(); +}); + +it('a late old claim and old disposal cannot replace or dispose a newer view', async () => { + const ready = deferred>(), first = view(), second = view(), host = provider(); + const newRouter = { dispose: vi.fn() }; + mocks.attach.mockReturnValue(newRouter); + mocks.take.mockReturnValueOnce(ready.promise).mockResolvedValueOnce({}); + const old = host.resolveWebviewView(first.value, {} as never, {} as never); + await host.resolveWebviewView(second.value, {} as never, {} as never); + ready.resolve({}); + await old; + first.dispose(); + expect(mocks.serve).toHaveBeenCalledTimes(1); + expect(mocks.serve.mock.calls[0][0]).toBe((second.value as { webview: unknown }).webview); + expect(newRouter.dispose).not.toHaveBeenCalled(); + expect(await host.postMessage({ type: 'dormouse:newTerminal' } as never)).toBe(true); + second.dispose(); + expect(newRouter.dispose).toHaveBeenCalledTimes(1); + expect(await host.postMessage({ type: 'dormouse:newTerminal' } as never)).toBe(false); +}); + +it('does not touch a disposed view after asynchronous shell discovery', async () => { + const ready = deferred(), target = view(); + const description = vi.fn(); + Object.defineProperty(target.value, 'description', { set: description }); + mocks.shells.mockReturnValue(ready.promise); + const host = new DormouseViewProvider({ extensionPath: 'extension' } as never); + const pending = host.resolveWebviewView(target.value, {} as never, {} as never); + target.dispose(); + ready.resolve([{ name: 'cmd', path: 'cmd.exe', args: [] }]); + await pending; + expect(description).not.toHaveBeenCalled(); + expect(mocks.take).not.toHaveBeenCalled(); + expect(mocks.serve).not.toHaveBeenCalled(); +}); From 1f5912e9952c50edc673ae7c114bbe176e2e4868 Mon Sep 17 00:00:00 2001 From: Ned Date: Thu, 1 Oct 2026 21:44:20 -0700 Subject: [PATCH 05/10] Test recovery permissions through the shipped async path --- lib/src/host/private-path.test.ts | 24 ++++++++++++------------ lib/src/host/private-path.ts | 25 ++----------------------- 2 files changed, 14 insertions(+), 35 deletions(-) diff --git a/lib/src/host/private-path.test.ts b/lib/src/host/private-path.test.ts index 02db04061..721b3e9c4 100644 --- a/lib/src/host/private-path.test.ts +++ b/lib/src/host/private-path.test.ts @@ -2,28 +2,28 @@ import * as fs from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { ensurePrivateDirectory, ensurePrivateDirectorySync, ensurePrivateFileSync } from './private-path'; +import { ensurePrivateDirectory, ensurePrivateFile } from './private-path'; import { readAcl, runAclScript, seedEveryoneRead } from './private-path.test-utils'; let dir: string; beforeEach(() => { dir = fs.mkdtempSync(join(tmpdir(), 'dormouse-private-path-')); }); afterEach(() => { fs.rmSync(dir, { recursive: true, force: true }); }); -describe('private recovery paths', () => { - it.skipIf(process.platform === 'win32')('tightens existing directory and file modes without changing the parent', () => { +describe('private recovery paths', async () => { + it.skipIf(process.platform === 'win32')('tightens existing directory and file modes without changing the parent', async () => { fs.chmodSync(dir, 0o755); const nested = join(dir, 'owned'); fs.mkdirSync(nested, { mode: 0o755 }); - ensurePrivateDirectorySync(nested); + await ensurePrivateDirectory(nested); const file = join(nested, 'recovery.json'); fs.writeFileSync(file, 'secret', { mode: 0o644 }); - ensurePrivateFileSync(file); + await ensurePrivateFile(file); expect(fs.statSync(nested).mode & 0o777).toBe(0o700); expect(fs.statSync(file).mode & 0o777).toBe(0o600); expect(fs.statSync(dir).mode & 0o777).toBe(0o755); }); - it.skipIf(process.platform !== 'win32')('replaces inherited and explicit foreign grants with the current user alone', () => { + it.skipIf(process.platform !== 'win32')('replaces inherited and explicit foreign grants with the current user alone', async () => { // Quotes, $, and backticks must remain literal stdin data, not script code. const nested = join(dir, "literal $ ' ` owned"); fs.mkdirSync(nested); @@ -32,8 +32,8 @@ describe('private recovery paths', () => { fs.writeFileSync(legacy, 'secret'); seedEveryoneRead(legacy); expect(readAcl(legacy).rules.some(({ sid }) => sid === 'S-1-1-0')).toBe(true); - ensurePrivateDirectorySync(nested); - ensurePrivateFileSync(legacy); + await ensurePrivateDirectory(nested); + await ensurePrivateFile(legacy); const fresh = join(nested, 'fresh.tmp'); fs.writeFileSync(fresh, 'secret', { mode: 0o600 }); for (const [target, inheritance] of [[nested, 3], [legacy, 0], [fresh, 0]] as const) { @@ -44,16 +44,16 @@ describe('private recovery paths', () => { } }, 10_000); - it('rejects directories masquerading as recovery files', () => { - expect(() => ensurePrivateFileSync(dir)).toThrow('plain file'); + it('rejects directories masquerading as recovery files', async () => { + await expect(ensurePrivateFile(dir)).rejects.toThrow('plain file'); }); - it.skipIf(process.platform === 'win32')('rejects symlinks without tightening the linked path', () => { + it.skipIf(process.platform === 'win32')('rejects symlinks without tightening the linked path', async () => { const real = join(dir, 'real'); fs.mkdirSync(real, { mode: 0o755 }); const link = join(dir, 'link'); fs.symlinkSync(real, link); - expect(() => ensurePrivateDirectorySync(link)).toThrow('plain directory'); + await expect(ensurePrivateDirectory(link)).rejects.toThrow('plain directory'); expect(fs.statSync(real).mode & 0o777).toBe(0o755); }); }); diff --git a/lib/src/host/private-path.ts b/lib/src/host/private-path.ts index 08758e8c3..1f9284722 100644 --- a/lib/src/host/private-path.ts +++ b/lib/src/host/private-path.ts @@ -6,7 +6,7 @@ */ import * as fs from 'node:fs'; import * as path from 'node:path'; -import { execFile, execFileSync } from 'node:child_process'; +import { execFile } from 'node:child_process'; const WINDOWS_PRIVATE_PATH = ` $ErrorActionPreference = 'Stop' @@ -45,28 +45,6 @@ function checkPath(target: string, directory: boolean): void { const powershell = () => path.join(process.env.SystemRoot || 'C:\\Windows', 'System32', 'WindowsPowerShell', 'v1.0', 'powershell.exe'); -function restrictToOwnerSync(target: string, directory: boolean): void { - checkPath(target, directory); - if (process.platform !== 'win32') return; - execFileSync(powershell(), ['-NoProfile', '-NonInteractive', '-Command', WINDOWS_PRIVATE_PATH], { - // Only literal path data crosses stdin: quotes, $, backticks and newlines - // never become PowerShell code. Use the system executable, not PATH search. - input: path.resolve(target), encoding: 'utf8', windowsHide: true, timeout: 5_000, - stdio: ['pipe', 'pipe', 'pipe'], - }); -} - -export function ensurePrivateDirectorySync(dir: string): void { - fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); - restrictToOwnerSync(dir, true); -} - -/** A legacy record may have explicit grants that directory inheritance cannot - * remove. Tighten it before reading; reject symlinks and non-files outright. */ -export function ensurePrivateFileSync(file: string): void { - restrictToOwnerSync(file, false); -} - /** Startup and cold claims do not block the host event loop on Windows ACL setup. * The OS authorizes SetOwner/Set-Acl; an elevated administrator-owned legacy * path may be rewritten, and any refused operation still fails closed. */ @@ -87,6 +65,7 @@ export async function ensurePrivateDirectory(dir: string): Promise { await restrictToOwner(dir, true); } +/** Tighten explicit legacy grants before reading; reject symlinks and non-files. */ export async function ensurePrivateFile(file: string): Promise { await restrictToOwner(file, false); } From 0d52ba8692ecc36b2210c0d4c2db4071eeb7563c Mon Sep 17 00:00:00 2001 From: Ned Date: Thu, 1 Oct 2026 21:55:22 -0700 Subject: [PATCH 06/10] Align recovery rationale with capture-time stale cleanup --- docs/specs/security-local.rationale.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/specs/security-local.rationale.md b/docs/specs/security-local.rationale.md index fdb74adc0..5c7a8c91b 100644 --- a/docs/specs/security-local.rationale.md +++ b/docs/specs/security-local.rationale.md @@ -173,7 +173,7 @@ true immediately on `win32`, and Node's `mode: 0o600` there touches only the read-only attribute, so unlike `remote_host_state_dir` no DACL work is done for it. -Recovery permissions were reproduced with an actual Windows DACL containing an inherited Everyone read grant (2026-10-01). Node mode `0600` did not remove it. The shared helper now protects the exact host-owned directory before writes and separately tightens explicit legacy file grants before claims; failed setup leaves the previous record untouched. +Recovery permissions were reproduced with an actual Windows DACL containing an inherited Everyone read grant (2026-10-01). Node mode `0600` did not remove it. The shared helper now protects the exact host-owned directory before writes and separately tightens explicit legacy file grants before claims; failed setup prevents record-byte reads and writes. Cold-start claims leave the prior record untouched, while capture still attempts to unlink stale recovery. Where the standalone log is actually exposed. `env::temp_dir()` honors `TMPDIR`, which on macOS is the per-user `/var/folders/.../T` directory at `0700`, so the From a145fc67e278ea0de06d8624a089f530bea34ec4 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 1 Oct 2026 22:32:18 -0700 Subject: [PATCH 07/10] Drop Windows recovery ACL machinery Standalone already locks its recovery directory in Rust before the sidecar starts, and the VS Code record lives under extension storage that inherits a user-only profile ACL. The PowerShell-backed private-path helper cost ~300 ms at every Windows start, silently disabled recovery on failure, and ran in no CI job. Restore the synchronous recovery store, its synchronous take, and the original specs, audit prompt, and known-gap row. Keep the pre-existing race fix in DormouseViewProvider: dispose is registered before the shell-discovery await, so a view disposed or replaced while it is pending is never served and cannot release its successor's router; its tests now drive the delay through shell discovery. Also restore the reconnection steps' message names (`dormouse:init`, `pty:list`, `pty:replay`, `alert:state`) in transport.md, and move the dual-runtime tsconfig paragraph into vscode.rationale.md. Co-Authored-By: Claude Opus 5.5 --- .github/audit/application-security.md | 2 +- docs/compatible-agents.md | 6 +- docs/compatible-agents.rationale.md | 2 - docs/specs/security-local.md | 11 +- docs/specs/security-local.rationale.md | 2 - docs/specs/security.md | 7 +- docs/specs/transport.md | 5 +- docs/specs/vscode.md | 4 +- docs/specs/vscode.rationale.md | 2 + lib/src/host/private-path.test-utils.ts | 37 ---- lib/src/host/private-path.test.ts | 83 -------- lib/src/host/private-path.ts | 71 ------- lib/src/host/recovery-store.test.ts | 199 +++--------------- lib/src/host/recovery-store.ts | 77 ++----- scripts/spec-word-budgets.json | 4 +- standalone/sidecar/main.js | 2 +- vscode-ext/src/extension.ts | 20 +- vscode-ext/src/session-state.ts | 31 +-- vscode-ext/src/webview-view-provider.ts | 11 +- vscode-ext/test/webview-view-provider.test.ts | 60 ++---- 20 files changed, 121 insertions(+), 515 deletions(-) delete mode 100644 lib/src/host/private-path.test-utils.ts delete mode 100644 lib/src/host/private-path.test.ts delete mode 100644 lib/src/host/private-path.ts diff --git a/.github/audit/application-security.md b/.github/audit/application-security.md index 340f2a67f..433aa4384 100644 --- a/.github/audit/application-security.md +++ b/.github/audit/application-security.md @@ -98,7 +98,7 @@ For the rest of `docs/specs/security-local.md`, read each section's owner first `docs/specs/dor-cli.md`, `docs/specs/vscode.md` -> "Webview message authentication", `docs/specs/standalone.md` -> "Persistence" — then the parser, the iframe shim, the control-socket code, and the persistence paths they point at. `## Persisted -state` covers session snapshots, written through `write_file_atomically` on standalone and through VS Code storage in the extension, plus shared recovery storage. Read `lib/src/host/private-path.ts`, `lib/src/host/recovery-store.ts`, their tests, and early activation in `vscode-ext/src/extension.ts`; verify actual Windows ACLs as well as Unix modes, legacy explicit file grants, and no helper retry inside bounded capture. +state` covers session snapshots, written through `write_file_atomically` on standalone and through VS Code storage in the extension. The attacker there is a program printing to the terminal, a page in a browser pane, or another local account, never the network. diff --git a/docs/compatible-agents.md b/docs/compatible-agents.md index 34856f621..9a296fd08 100644 --- a/docs/compatible-agents.md +++ b/docs/compatible-agents.md @@ -110,12 +110,12 @@ Source of truth: `CODING_AGENTS` in `lib/src/lib/coding-agents.ts`; `detectResum ### Recovery record - **Must keep one rebuilt invocation per Surface in a host-owned, single-use record outside the persisted Session.** The renderer save path never derives or writes it. (rationale) -- **Must call `beginCapture` before capture can return early.** The first call per host process clears the previous record even if private-storage preparation failed; subsequent calls merge, preserving captures from other Windows. (rationale) -- **Must persist every detection synchronously through `createRecoveryStore`**, through a host-selected `recovery.json`, an owner-only temporary file, and atomic rename. **Must prepare the exact host-owned directory before any record bytes and tighten an existing record before claiming it**, using owner-only modes on Unix and a protected current-user-only DACL on Windows. Failed preparation permits no record-byte read or write; capture still removes the stale record. **Must start preparation asynchronously at startup and await it only for cold-start claims.** Claims may retry failed preparation, but bounded capture never waits for or launches the permission helper. Concurrent claims share one destructive read; if capture starts during setup, the claim must neither return old commands nor remove the new record. Failed writes must not escape teardown. Without a directory, use memory-only storage and log once. +- **Must call `beginCapture` before capture can return early.** The first call per host process clears the previous record; subsequent calls merge, preserving captures from other Windows. (rationale) +- **Must persist every detection synchronously through `createRecoveryStore`**, using `recovery.json` in the host-selected directory, an owner-only temporary file, and atomic rename. A failed write must not throw through teardown. Without a directory the store is memory-only and logs that limitation once. - **Must read and unlink the durable record on the first claim**, including on parse failure; if unlink fails, ignore it. Discard records older than 7 days after unlinking. Within the process, each container claims only its saved pane ids, and each entry is handed out once. (rationale) - **Must deliver claimed commands out of band on boot through `PlatformAdapter.getRecoveryCommands()`**; adapters whose hosts capture nothing may omit it. Only cold restore consumes these commands for execution; live resume never executes them. -Source of truth: `createRecoveryStore` in `lib/src/host/recovery-store.ts`; `ensurePrivateDirectory` / `ensurePrivateFile` in `lib/src/host/private-path.ts`, pinned by `lib/src/host/private-path.test.ts` and `lib/src/host/recovery-store.test.ts`; `PlatformAdapter` in `lib/src/lib/platform/types.ts`. +Source of truth: `createRecoveryStore` in `lib/src/host/recovery-store.ts`; `PlatformAdapter` in `lib/src/lib/platform/types.ts`; pinned by `lib/src/host/recovery-store.test.ts`. ### Cold restore diff --git a/docs/compatible-agents.rationale.md b/docs/compatible-agents.rationale.md index fd46735ba..f246dba24 100644 --- a/docs/compatible-agents.rationale.md +++ b/docs/compatible-agents.rationale.md @@ -60,8 +60,6 @@ Rows 1–2 are why a blanket second press is wrong; `Press Ctrl-C again` was abs **Why each webview claims only its own pane ids.** Two containers resolve inside one activation; a claim-everything read would let whichever resolved first delete the other's commands. Per-id claiming also means a disposed-and-re-resolved view restores without re-running the agent — its entries were already taken. -Windows Node mode bits left recovery files inheriting Everyone read access in an actual deliberately loose directory (Windows, 2026-10-01). A protected current-user-only inheritable DACL removed foreign grants from new files; legacy explicit file grants needed separate tightening. System PowerShell setup took about 303 ms in the probe, which motivated asynchronous startup preparation and successful caching rather than blocking activation or spending the capture budget on each detection. Deleting a stale record exposes no bytes, so capture can clear it even when permission setup fails. Windows may assign an elevated process's new path to the Administrators group; SetOwner and Set-Acl authorization, rather than a preexisting-owner equality test, governs migration to the current user. - ## Cold restore **Why auto-run needs no confirmation prompt.** The detector rebuilds a known command and restricts the id grammar, excluding shell punctuation from the captured argument. `claude --resume ` restores the conversation, lands at an idle prompt, and makes no request until the user types. It restores *more* context than the scrollback it replaces — the resumed agent renders the real conversation, not a transcript of it — which is what made dropping persisted scrollback affordable. diff --git a/docs/specs/security-local.md b/docs/specs/security-local.md index e8ac25ebc..3d59f6b22 100644 --- a/docs/specs/security-local.md +++ b/docs/specs/security-local.md @@ -188,14 +188,19 @@ upgrade rewrites the snapshot without it, and a boot sweep deletes orphaned `*.json.tmp` files no save would ever overwrite. Snapshots older versions left behind do carry transcripts (rationale). -Recovery storage and single-use claiming follow `docs/compatible-agents.md` → Recovery record. +**Standalone writes `recovery.json` beside its sessions directory**, under the +state root, owner-only: one rebuilt agent-resume invocation per Surface, never a +buffer, unlinked as it is read (`docs/compatible-agents.md` -> "Recovery record"). **The managed-voice token is a bearer credential at rest** — `/managed-voice.json` beside the Burrow's enrollment, written by `writeJsonAtomic` (`0700`/`0600`; on Windows the owner-only DACL `burrow_state_dir` applies before the sidecar spawns), with the voice id (rationale); `docs/specs/alert.md` → "Managed voice" keeps it from any webview. **The token must go only to `hostedVoiceOrigin`'s answer**, never following a redirect (`redirect: 'error'`); a self-host build, answered `null`, sends it nowhere (`docs/specs/relay.md` -> "Relay origin"). **VS Code persists pane structure in VS Code's own storage** — `workspaceState` under `dormouse.session`, and `vscode.setState()`, a WebviewPanel's only store — so the modes there are VS Code's, not ours, and no transcript reaches either -(`docs/specs/vscode.md` -> "Serialization and restore"). +(`docs/specs/vscode.md` -> "Serialization and restore"). Dormouse also writes +`recovery.json` under the extension's storage directory, owner-only and +temp-then-rename: one rebuilt agent-resume invocation per Surface, no buffer, +unlinked as it is read (`docs/compatible-agents.md` -> "Recovery record"). **The VS Code peer-link token is a local credential at rest** — `burrow.peer-token` in the extension's global storage, written mode `0600` @@ -211,8 +216,6 @@ does. A gap, not an accepted risk. - **FAIL IF** `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, asserted by `restrict_to_owner_leaves_one_owner_only_ace` — or if **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. `session_write_tightens_directory_and_existing_temp_file` pins unix modes; `session_permission_failures_preserve_previous_snapshot_without_writing_bytes` pins both failure gates. The mode reaches the temp file *before* any bytes are written (rationale). -- **FAIL IF** recovery record bytes are read or written before owner-only permission setup succeeds, bounded capture waits for or launches permission setup, or failed preparation prevents capture from attempting to clear stale records. Read `createRecoveryStore` in `lib/src/host/recovery-store.ts` and `ensurePrivateDirectory` / `ensurePrivateFile` in `lib/src/host/private-path.ts`; `lib/src/host/private-path.test.ts` and `lib/src/host/recovery-store.test.ts` pin real Windows DACLs, Unix modes, legacy file grants, failed setup, capture-safe caching, and claims racing teardown. - Source of truth: `SESSION_STATE_KEY` in `vscode-ext/src/session-state.ts`, `ensureToken` in `vscode-ext/src/peer-link.ts`, `default_log_path` in `standalone/src-tauri/src/lib.rs`, `createManagedVoiceHost` in diff --git a/docs/specs/security-local.rationale.md b/docs/specs/security-local.rationale.md index 5c7a8c91b..9d8569c99 100644 --- a/docs/specs/security-local.rationale.md +++ b/docs/specs/security-local.rationale.md @@ -173,8 +173,6 @@ true immediately on `win32`, and Node's `mode: 0o600` there touches only the read-only attribute, so unlike `remote_host_state_dir` no DACL work is done for it. -Recovery permissions were reproduced with an actual Windows DACL containing an inherited Everyone read grant (2026-10-01). Node mode `0600` did not remove it. The shared helper now protects the exact host-owned directory before writes and separately tightens explicit legacy file grants before claims; failed setup prevents record-byte reads and writes. Cold-start claims leave the prior record untouched, while capture still attempts to unlink stale recovery. - Where the standalone log is actually exposed. `env::temp_dir()` honors `TMPDIR`, which on macOS is the per-user `/var/folders/.../T` directory at `0700`, so the umask does not matter there (measured on a macOS host, 2026-09). The exposure is diff --git a/docs/specs/security.md b/docs/specs/security.md index 113fdb8b3..8ae66e8d1 100644 --- a/docs/specs/security.md +++ b/docs/specs/security.md @@ -118,9 +118,10 @@ Gaps rather than accepted risks: we intend to close them. WebSocket cookie headers are stripped, but `document.cookie` remains shared; cookie-authenticated iframe pages are unsupported ([Loopback Listeners](./security-local.md#loopback-listeners)). -- **VS Code's peer-link token has no dedicated Windows ACL applied by Dormouse.** - Its unix mode is a no-op on Windows - ([Persisted state](./security-local.md#persisted-state)). +- **Neither VS Code's peer-link token nor the `recovery.json` beside it carries + a Windows ACL applied by Dormouse.** Both are written owner-only by unix + mode, which Windows makes a no-op; standalone locks its state directory + instead ([Persisted state](./security-local.md#persisted-state)). - **The standalone log file is written at the umask** and records the `dor` socket path ([Persisted state](./security-local.md#persisted-state)). - **Revocation has no mechanism.** Revoking a lost phone is editing the Burrow's diff --git a/docs/specs/transport.md b/docs/specs/transport.md index 6dcec1e0d..a71157a52 100644 --- a/docs/specs/transport.md +++ b/docs/specs/transport.md @@ -52,12 +52,11 @@ Source of truth: `pacedInputSegments` and `write` in `standalone/sidecar/pty-cor ### Reconnection protocol -1. The visible or deserialized webview requests initialization. -2. The host lists owned PTYs, then sends their replay and alert state. +1. The visible or deserialized webview calls `requestInit` (VS Code: `{ type: 'dormouse:init' }`). +2. The host answers `pty:list` (one `PtyInfo` per owned PTY: `id`, `alive`, `exitCode`, `shell`), then `pty:replay` for each PTY with buffered output, then `alert:state` for each. 3. The webview resumes terminals with their launch shells for Session-specific clipboard/drop escaping. 4. A saved layout is reused only when its leaves match the live visible pane set; saved minimized PTYs are registered as Doors. - **A collection finishes only on its own answer.** A `requestInit` carries the asking collector's token, and a host serving several windows echoes it on the `pty:list` and on every `pty:replay` behind it; the collector ignores anything diff --git a/docs/specs/vscode.md b/docs/specs/vscode.md index ac8134c85..e094dcf2f 100644 --- a/docs/specs/vscode.md +++ b/docs/specs/vscode.md @@ -138,9 +138,9 @@ needs no host-side per-panel store. #### Capturing agent recovery -**Must offer every live extension-host PTY to shared capture**, across the view and editor panels. **Must store the record under `storageUri`, falling back to `globalStorageUri`; never `workspaceState`** (rationale). If neither directory exists, skip capture. **Must start private recovery-storage preparation asynchronously at activation.** Cold-start webviews await claims; bounded teardown never waits for or launches permission setup. Shared record behavior follows `docs/compatible-agents.md` → Recovery record. +**Must offer every live extension-host PTY to shared capture**, across the view and editor panels. **Must store the record under `storageUri`, falling back to `globalStorageUri`; never `workspaceState`** (rationale). If neither directory exists, skip capture. Shared behavior follows `docs/compatible-agents.md`. -Source of truth: `prepareRecoveryStorage` / `captureAgentRecoveryCommands` / `takeRecoveryCommands` in `vscode-ext/src/session-state.ts`; `interrupt` in `vscode-ext/src/pty-manager.ts`. +Source of truth: `captureAgentRecoveryCommands` / `takeRecoveryCommands` in `vscode-ext/src/session-state.ts`; `interrupt` in `vscode-ext/src/pty-manager.ts`. ### Theme integration diff --git a/docs/specs/vscode.rationale.md b/docs/specs/vscode.rationale.md index 39606645a..7bc202019 100644 --- a/docs/specs/vscode.rationale.md +++ b/docs/specs/vscode.rationale.md @@ -110,3 +110,5 @@ macOS host, hence copying only the declared platform packages. **Why a self-host VSIX needs no update switch of its own.** VS Code treats a VSIX install as a pinned version and leaves it out of Marketplace auto-update (microsoft/vscode#219932, fixed by #219933 in the July 2024 iteration, 1.92; the diff covers the CLI's VSIX path, `code --install-extension`, which `pnpm dogfood:vscode` takes — checked 2026-09). An extension cannot opt itself out of Marketplace updates, and a distinct extension id would collide with the Marketplace build's command, view, and keybinding contributions when both are installed, and would strand the enrollment in another id's `SecretStorage`. **Why the separate typecheck is wired into `test`.** A reference to a deleted function once reached a commit and surfaced only as a runtime throw during `deactivate()`, which — having no `try`/`catch` — skipped every teardown step behind it. `tsc` is the package's only automated check for that class of error. + +**Why the typecheck config carries both DOM and Node libs.** The checked program spans two runtimes — `src/` is extension-host Node code but imports webview modules from `../lib/src/` — so `vscode-ext/tsconfig.json` is looser than either runtime alone; each side is checked precisely by its own project (`lib/tsconfig.app.json` for the webview). What it reliably catches is vscode-ext's own code referring to something that no longer exists. diff --git a/lib/src/host/private-path.test-utils.ts b/lib/src/host/private-path.test-utils.ts deleted file mode 100644 index dc8a12026..000000000 --- a/lib/src/host/private-path.test-utils.ts +++ /dev/null @@ -1,37 +0,0 @@ -import { execFileSync } from 'node:child_process'; -import { join } from 'node:path'; - -export function runAclScript(script: string, target: string): string { - return execFileSync(join(process.env.SystemRoot || 'C:\\Windows', 'System32', 'WindowsPowerShell', 'v1.0', 'powershell.exe'), - ['-NoProfile', '-NonInteractive', '-Command', "$ErrorActionPreference='Stop'; $p=[Console]::In.ReadToEnd(); " + script], - { input: target, encoding: 'utf8', windowsHide: true, timeout: 5_000 }).trim(); -} - -export function seedEveryoneRead(target: string): void { - runAclScript(`$acl=Get-Acl -LiteralPath $p; - $inheritance=[System.Security.AccessControl.InheritanceFlags]::None; - if ((Get-Item -LiteralPath $p -Force).PSIsContainer) { - $inheritance=[System.Security.AccessControl.InheritanceFlags]'ContainerInherit,ObjectInherit' - } - $rule=[System.Security.AccessControl.FileSystemAccessRule]::new( - [System.Security.Principal.SecurityIdentifier]::new('S-1-1-0'), - [System.Security.AccessControl.FileSystemRights]::ReadAndExecute, - $inheritance,[System.Security.AccessControl.PropagationFlags]::None, - [System.Security.AccessControl.AccessControlType]::Allow); - $acl.AddAccessRule($rule); Set-Acl -LiteralPath $p -AclObject $acl`, target); -} - -export function readAcl(target: string): { - currentUser: string; owner: string; protected: boolean; - rules: Array<{ sid: string; rights: number; allow: boolean; inheritance: number }>; -} { - return JSON.parse(runAclScript(`$acl=Get-Acl -LiteralPath $p; - @{ currentUser=[System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value; - owner=$acl.GetOwner([System.Security.Principal.SecurityIdentifier]).Value; - protected=$acl.AreAccessRulesProtected; - rules=@($acl.GetAccessRules($true,$true,[System.Security.Principal.SecurityIdentifier]) | ForEach-Object { - @{ sid=$_.IdentityReference.Value; rights=[int]$_.FileSystemRights; - allow=$_.AccessControlType -eq [System.Security.AccessControl.AccessControlType]::Allow; - inheritance=[int]$_.InheritanceFlags } - }) } | ConvertTo-Json -Depth 4 -Compress`, target)); -} diff --git a/lib/src/host/private-path.test.ts b/lib/src/host/private-path.test.ts deleted file mode 100644 index 721b3e9c4..000000000 --- a/lib/src/host/private-path.test.ts +++ /dev/null @@ -1,83 +0,0 @@ -import * as fs from 'node:fs'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { ensurePrivateDirectory, ensurePrivateFile } from './private-path'; -import { readAcl, runAclScript, seedEveryoneRead } from './private-path.test-utils'; - -let dir: string; -beforeEach(() => { dir = fs.mkdtempSync(join(tmpdir(), 'dormouse-private-path-')); }); -afterEach(() => { fs.rmSync(dir, { recursive: true, force: true }); }); - -describe('private recovery paths', async () => { - it.skipIf(process.platform === 'win32')('tightens existing directory and file modes without changing the parent', async () => { - fs.chmodSync(dir, 0o755); - const nested = join(dir, 'owned'); - fs.mkdirSync(nested, { mode: 0o755 }); - await ensurePrivateDirectory(nested); - const file = join(nested, 'recovery.json'); - fs.writeFileSync(file, 'secret', { mode: 0o644 }); - await ensurePrivateFile(file); - expect(fs.statSync(nested).mode & 0o777).toBe(0o700); - expect(fs.statSync(file).mode & 0o777).toBe(0o600); - expect(fs.statSync(dir).mode & 0o777).toBe(0o755); - }); - - it.skipIf(process.platform !== 'win32')('replaces inherited and explicit foreign grants with the current user alone', async () => { - // Quotes, $, and backticks must remain literal stdin data, not script code. - const nested = join(dir, "literal $ ' ` owned"); - fs.mkdirSync(nested); - seedEveryoneRead(nested); - const legacy = join(nested, 'legacy.json'); - fs.writeFileSync(legacy, 'secret'); - seedEveryoneRead(legacy); - expect(readAcl(legacy).rules.some(({ sid }) => sid === 'S-1-1-0')).toBe(true); - await ensurePrivateDirectory(nested); - await ensurePrivateFile(legacy); - const fresh = join(nested, 'fresh.tmp'); - fs.writeFileSync(fresh, 'secret', { mode: 0o600 }); - for (const [target, inheritance] of [[nested, 3], [legacy, 0], [fresh, 0]] as const) { - const acl = readAcl(target); - expect(acl.owner).toBe(acl.currentUser); - expect(acl.rules).toEqual([{ sid: acl.currentUser, rights: 0x001F01FF, allow: true, inheritance }]); - if (target !== fresh) expect(acl.protected).toBe(true); - } - }, 10_000); - - it('rejects directories masquerading as recovery files', async () => { - await expect(ensurePrivateFile(dir)).rejects.toThrow('plain file'); - }); - - it.skipIf(process.platform === 'win32')('rejects symlinks without tightening the linked path', async () => { - const real = join(dir, 'real'); - fs.mkdirSync(real, { mode: 0o755 }); - const link = join(dir, 'link'); - fs.symlinkSync(real, link); - await expect(ensurePrivateDirectory(link)).rejects.toThrow('plain directory'); - expect(fs.statSync(real).mode & 0o777).toBe(0o755); - }); -}); - -it.skipIf(process.platform !== 'win32')('can migrate an administrator-owned legacy directory when Windows authorizes it', async (context) => { - const nested = join(dir, 'elevated-legacy'); - fs.mkdirSync(nested); - try { - runAclScript(`$acl=Get-Acl -LiteralPath $p; - $acl.SetOwner([System.Security.Principal.SecurityIdentifier]::new('S-1-5-32-544')); - Set-Acl -LiteralPath $p -AclObject $acl`, nested); - } catch (error) { - // Unelevated Windows CI tokens cannot assign the administrators group owner. - // Do not turn a real ACL setup failure into a platform skip. - if (/privilege|not allowed|UnauthorizedAccess|IdentityNotMapped/.test(String(error))) { - context.skip(); - return; - } - throw error; - } - expect(readAcl(nested).owner).toBe('S-1-5-32-544'); - await ensurePrivateDirectory(nested); - const acl = readAcl(nested); - expect(acl.owner).toBe(acl.currentUser); - expect(acl.protected).toBe(true); - expect(acl.rules).toEqual([{ sid: acl.currentUser, rights: 0x001F01FF, allow: true, inheritance: 3 }]); -}, 10_000); diff --git a/lib/src/host/private-path.ts b/lib/src/host/private-path.ts deleted file mode 100644 index 1f9284722..000000000 --- a/lib/src/host/private-path.ts +++ /dev/null @@ -1,71 +0,0 @@ -/** Recovery's exact host-owned directory and legacy record must be private before - * any bytes are written or claimed. Unix mkdir modes do not tighten existing - * directories; Windows mode bits do not control access. Harden the directory - * once at startup so a bounded teardown does not pay for a process per record. - * Never touch ancestors. Failure prevents persistence and automatic recovery. - */ -import * as fs from 'node:fs'; -import * as path from 'node:path'; -import { execFile } from 'node:child_process'; - -const WINDOWS_PRIVATE_PATH = ` -$ErrorActionPreference = 'Stop' -$targetPath = [Console]::In.ReadToEnd() -$sid = [System.Security.Principal.WindowsIdentity]::GetCurrent().User -if ((Get-Item -LiteralPath $targetPath -Force).PSIsContainer) { - $acl = [System.Security.AccessControl.DirectorySecurity]::new() - $inheritance = [System.Security.AccessControl.InheritanceFlags]'ContainerInherit,ObjectInherit' -} else { - $acl = [System.Security.AccessControl.FileSecurity]::new() - $inheritance = [System.Security.AccessControl.InheritanceFlags]::None -} -$acl.SetAccessRuleProtection($true, $false) -$acl.SetOwner($sid) -$rule = [System.Security.AccessControl.FileSystemAccessRule]::new( - $sid, - [System.Security.AccessControl.FileSystemRights]0x001F01FF, - $inheritance, - [System.Security.AccessControl.PropagationFlags]::None, - [System.Security.AccessControl.AccessControlType]::Allow) -$acl.AddAccessRule($rule) -Set-Acl -LiteralPath $targetPath -AclObject $acl -`; - -function checkPath(target: string, directory: boolean): void { - const info = fs.lstatSync(target); - if (info.isSymbolicLink() || (directory ? !info.isDirectory() : !info.isFile())) { - throw new Error(`Recovery path must be a plain ${directory ? 'directory' : 'file'}`); - } - if (process.platform !== 'win32') { - if (process.getuid && info.uid !== process.getuid()) throw new Error('Recovery path must belong to this user'); - fs.chmodSync(target, directory ? 0o700 : 0o600); - return; - } -} - -const powershell = () => path.join(process.env.SystemRoot || 'C:\\Windows', 'System32', 'WindowsPowerShell', 'v1.0', 'powershell.exe'); - -/** Startup and cold claims do not block the host event loop on Windows ACL setup. - * The OS authorizes SetOwner/Set-Acl; an elevated administrator-owned legacy - * path may be rewritten, and any refused operation still fails closed. */ -async function restrictToOwner(target: string, directory: boolean): Promise { - checkPath(target, directory); - if (process.platform !== 'win32') return; - await new Promise((resolve, reject) => { - const child = execFile(powershell(), ['-NoProfile', '-NonInteractive', '-Command', WINDOWS_PRIVATE_PATH], { - encoding: 'utf8', windowsHide: true, timeout: 5_000, maxBuffer: 64 * 1024, - }, (error) => error ? reject(error) : resolve()); - child.stdin!.on('error', () => { /* execFile reports process failure */ }); - child.stdin!.end(path.resolve(target)); - }); -} - -export async function ensurePrivateDirectory(dir: string): Promise { - fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); - await restrictToOwner(dir, true); -} - -/** Tighten explicit legacy grants before reading; reject symlinks and non-files. */ -export async function ensurePrivateFile(file: string): Promise { - await restrictToOwner(file, false); -} diff --git a/lib/src/host/recovery-store.test.ts b/lib/src/host/recovery-store.test.ts index ad76c11b4..75b68a212 100644 --- a/lib/src/host/recovery-store.test.ts +++ b/lib/src/host/recovery-store.test.ts @@ -1,15 +1,8 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import * as fs from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { createRecoveryStore, RECOVERY_MAX_AGE_MS } from './recovery-store'; -import * as privatePaths from './private-path'; -import { readAcl, seedEveryoneRead } from './private-path.test-utils'; - -vi.mock('node:fs', async (importOriginal) => { - const real = await importOriginal(); - return { ...real, unlinkSync: vi.fn(real.unlinkSync) }; -}); let dir: string; const messages: string[] = []; @@ -28,17 +21,15 @@ beforeEach(() => { }); afterEach(() => { - vi.restoreAllMocks(); fs.rmSync(dir, { recursive: true, force: true }); }); -describe('recovery store', async () => { - describe('capture', async () => { - it('replaces the previous record on the first beginCapture and merges after', async () => { +describe('recovery store', () => { + describe('capture', () => { + it('replaces the previous record on the first beginCapture and merges after', () => { write({ createdAt: Date.now(), commands: { old: 'claude --continue' } }); const store = createRecoveryStore(dir, { log }); - await store.ready; store.beginCapture(); // The stale record is gone before anything can be detected, so a teardown // that captures nothing cannot carry it forward. @@ -53,37 +44,29 @@ describe('recovery store', async () => { expect(read().commands).toEqual({ a: 'claude --resume A', b: 'codex resume B' }); }); - it('writes the record and its directory owner-only', async () => { + it('writes the record and its directory owner-only', () => { const store = createRecoveryStore(dir, { log }); - await store.ready; store.beginCapture(); store.record('a', 'claude --continue'); const mode = (path: string) => fs.statSync(path).mode & 0o777; - if (process.platform === 'win32') { - const acl = readAcl(file()); - expect(acl.rules).toEqual([{ sid: acl.currentUser, rights: 0x001F01FF, allow: true, inheritance: 0 }]); - } else { - expect(mode(dir)).toBe(0o700); - expect(mode(file())).toBe(0o600); - } + expect(mode(file())).toBe(0o600); // The temp sibling is renamed over the target, so nothing torn is left. expect(fs.readdirSync(dir)).toEqual(['recovery.json']); }); - it('does not throw when the record cannot be written', async () => { + it('does not throw when the record cannot be written', () => { // A file where the state directory should be: `mkdirSync` cannot make it. const blocked = join(dir, 'blocked'); fs.writeFileSync(blocked, 'not a directory', 'utf8'); const store = createRecoveryStore(blocked, { log }); - await store.ready; store.beginCapture(); expect(() => store.record('a', 'claude --continue')).not.toThrow(); expect(messages.some((message) => message.startsWith('error [recovery] write failed'))).toBe(true); // Nothing was captured, so nothing can be claimed either. - expect(await store.take(['a'])).toEqual({}); + expect(store.take(['a'])).toEqual({}); }); - it('leaves no temp behind when the rename onto the record fails', async () => { + it('leaves no temp behind when the rename onto the record fails', () => { // A directory where the record should be: the temp is written, the rename // over it cannot succeed. A fixed `recovery.json.tmp` would sit there for // the next run — and for any other host sharing this directory — to rename @@ -91,7 +74,6 @@ describe('recovery store', async () => { fs.mkdirSync(file()); const store = createRecoveryStore(dir, { log }); - await store.ready; store.beginCapture(); expect(() => store.record('a', 'claude --continue')).not.toThrow(); @@ -100,188 +82,65 @@ describe('recovery store', async () => { }); }); - describe('take', async () => { - it('never hands out commands when removing the durable record fails', async () => { - write({ createdAt: Date.now(), commands: { a: 'claude --continue' } }); - const store = createRecoveryStore(dir, { log }); - await store.ready; - vi.mocked(fs.unlinkSync).mockImplementationOnce(() => { throw new Error('unlink refused'); }); - expect(await store.take(['a'])).toEqual({}); - expect(await store.take(['a'])).toEqual({}); - expect(read().commands).toEqual({ a: 'claude --continue' }); - expect(messages.some((message) => message.includes('could not clear record; ignoring it'))).toBe(true); - }); - - it.skipIf(process.platform !== 'win32')('tightens explicit legacy file grants before claiming the record', async () => { - write({ createdAt: Date.now(), commands: { a: 'claude --continue' } }); - seedEveryoneRead(file()); - const store = createRecoveryStore(dir, { log }); - await store.ready; - const original = privatePaths.ensurePrivateFile; - // Observe permissions immediately before the actual read and unlink. - const protect = vi.spyOn(privatePaths, 'ensurePrivateFile').mockImplementation(async (target) => { - await original(target); - const acl = readAcl(target); - expect(acl.protected).toBe(true); - expect(acl.rules).toEqual([{ sid: acl.currentUser, rights: 0x001F01FF, allow: true, inheritance: 0 }]); - }); - expect(await store.take(['a'])).toEqual({ a: 'claude --continue' }); - expect(protect).toHaveBeenCalledWith(file()); - protect.mockRestore(); - expect(fs.existsSync(file())).toBe(false); - }); - - it('does not read or unlink a record whose private file setup fails', async () => { - write({ createdAt: Date.now(), commands: { a: 'claude --continue' } }); - const store = createRecoveryStore(dir, { log }); - await store.ready; - const protect = vi.spyOn(privatePaths, 'ensurePrivateFile').mockRejectedValue(new Error('ACL failed')); - expect(await store.take(['a'])).toEqual({}); - expect(read().commands).toEqual({ a: 'claude --continue' }); - protect.mockRestore(); - expect(await store.take(['a'])).toEqual({ a: 'claude --continue' }); - }); - - it('unlinks on the first call and hands out each id exactly once', async () => { + describe('take', () => { + it('unlinks on the first call and hands out each id exactly once', () => { write({ createdAt: Date.now(), commands: { a: 'claude --resume A', b: 'codex resume B' } }); const store = createRecoveryStore(dir, { log }); - await store.ready; - expect(await store.take(['a'])).toEqual({ a: 'claude --resume A' }); + expect(store.take(['a'])).toEqual({ a: 'claude --resume A' }); // The durable copy is gone before anything can act on it, so a failed start // cannot replay it. expect(fs.existsSync(file())).toBe(false); // A second container claims its share of the same read; the first id is // spent. - expect(await store.take(['a', 'b'])).toEqual({ b: 'codex resume B' }); - expect(await store.take(['b'])).toEqual({}); + expect(store.take(['a', 'b'])).toEqual({ b: 'codex resume B' }); + expect(store.take(['b'])).toEqual({}); }); - it('returns nothing when there is no record', async () => { - expect(await createRecoveryStore(dir, { log }).take(['a'])).toEqual({}); + it('returns nothing when there is no record', () => { + expect(createRecoveryStore(dir, { log }).take(['a'])).toEqual({}); }); - it('is destructive even on a record it cannot parse', async () => { + it('is destructive even on a record it cannot parse', () => { fs.writeFileSync(file(), '{ torn', 'utf8'); const store = createRecoveryStore(dir, { log }); - await store.ready; - expect(await store.take(['a'])).toEqual({}); + expect(store.take(['a'])).toEqual({}); expect(fs.existsSync(file())).toBe(false); }); - it('discards a record past its expiry, having removed it', async () => { + it('discards a record past its expiry, having removed it', () => { write({ createdAt: Date.now() - RECOVERY_MAX_AGE_MS - 1, commands: { a: 'claude --continue' } }); const store = createRecoveryStore(dir, { log }); - await store.ready; - expect(await store.take(['a'])).toEqual({}); + expect(store.take(['a'])).toEqual({}); expect(fs.existsSync(file())).toBe(false); }); - it('drops a non-string entry rather than handing it on', async () => { + it('drops a non-string entry rather than handing it on', () => { write({ createdAt: Date.now(), commands: { a: 'claude --continue', b: { evil: true } } }); const store = createRecoveryStore(dir, { log }); - await store.ready; - expect(await store.take(['a', 'b'])).toEqual({ a: 'claude --continue' }); + expect(store.take(['a', 'b'])).toEqual({ a: 'claude --continue' }); }); - it('cannot be tricked by an id that names an Object prototype member', async () => { + it('cannot be tricked by an id that names an Object prototype member', () => { write({ createdAt: Date.now(), commands: { constructor: 'claude --continue' } }); const store = createRecoveryStore(dir, { log }); - await store.ready; // A plain literal would answer `toString` with an inherited function. - expect(await store.take(['toString'])).toEqual({}); - expect(await store.take(['constructor'])).toEqual({ constructor: 'claude --continue' }); + expect(store.take(['toString'])).toEqual({}); + expect(store.take(['constructor'])).toEqual({ constructor: 'claude --continue' }); }); }); - it('clears stale recovery despite failed startup privacy setup, without retrying during capture', async () => { - write({ createdAt: Date.now(), commands: { old: 'claude --continue' } }); - const protect = vi.spyOn(privatePaths, 'ensurePrivateDirectory').mockRejectedValue(new Error('helper timed out')); - const store = createRecoveryStore(dir, { log }); - await store.ready; - expect(protect).toHaveBeenCalledTimes(1); - store.beginCapture(); - store.record('a', 'codex resume A'); - store.beginCapture(); - store.record('b', 'codex resume B'); - expect(protect).toHaveBeenCalledTimes(1); - expect(fs.existsSync(file())).toBe(false); - expect(fs.readdirSync(dir)).toEqual([]); - expect(await store.take([])).toEqual({}); - expect(protect).toHaveBeenCalledTimes(1); - protect.mockRestore(); - const next = createRecoveryStore(dir, { log }); - await next.ready; - expect(await next.take(['old'])).toEqual({}); - - }); - - it('starts permission setup without blocking and does not wait for it during capture', async () => { - write({ createdAt: Date.now(), commands: { old: 'claude --continue' } }); - let finish!: () => void; - const original = privatePaths.ensurePrivateDirectory; - const protect = vi.spyOn(privatePaths, 'ensurePrivateDirectory').mockImplementation(async (target) => { - await new Promise((resolve) => { finish = resolve; }); - await original(target); - }); - const store = createRecoveryStore(dir, { log }); - const claim = store.take(['old']); - store.beginCapture(); - store.record('new', 'codex resume NEW'); - expect(fs.existsSync(file())).toBe(false); - expect(protect).toHaveBeenCalledTimes(1); - finish(); - await store.ready; - expect(await claim).toEqual({}); - store.record('later', 'codex resume LATER'); - expect(read().commands).toEqual({ new: 'codex resume NEW', later: 'codex resume LATER' }); - expect(await store.take(['new', 'later'])).toEqual({}); - }); - - it('shares asynchronous legacy claims without removing a newly captured record', async () => { - write({ createdAt: Date.now(), commands: { old: 'claude --continue' } }); - const store = createRecoveryStore(dir, { log }); - await store.ready; - let entered!: () => void, finish!: () => void; - const started = new Promise((resolve) => { entered = resolve; }); - vi.spyOn(privatePaths, 'ensurePrivateFile').mockImplementation(async () => { - entered(); - await new Promise((resolve) => { finish = resolve; }); - }); - const first = store.take(['old']), second = store.take(['old']); - await started; - store.beginCapture(); - store.record('new', 'codex resume NEW'); - finish(); - expect(await first).toEqual({}); - expect(await second).toEqual({}); - expect(read().commands).toEqual({ new: 'codex resume NEW' }); - }); - - it('prepares a successful directory once across multiple captures and writes', async () => { - const protect = vi.spyOn(privatePaths, 'ensurePrivateDirectory'); - const store = createRecoveryStore(dir, { log }); - await store.ready; - store.beginCapture(); - store.record('a', 'claude --continue'); - store.beginCapture(); - store.record('b', 'codex resume B'); - expect(protect).toHaveBeenCalledTimes(1); - expect(read().commands).toEqual({ a: 'claude --continue', b: 'codex resume B' }); - }); - - describe('without a state directory', async () => { - it('keeps the record in memory and says so once', async () => { + describe('without a state directory', () => { + it('keeps the record in memory and says so once', () => { const store = createRecoveryStore(undefined, { log }); - await store.ready; expect(store.persistent).toBe(false); expect(messages.filter((message) => message.includes('no state directory'))).toHaveLength(1); store.beginCapture(); store.record('a', 'claude --continue'); - expect(await store.take(['a'])).toEqual({ a: 'claude --continue' }); - expect(await store.take(['a'])).toEqual({}); + expect(store.take(['a'])).toEqual({ a: 'claude --continue' }); + expect(store.take(['a'])).toEqual({}); }); }); }); diff --git a/lib/src/host/recovery-store.ts b/lib/src/host/recovery-store.ts index 31fde8bab..95f96199c 100644 --- a/lib/src/host/recovery-store.ts +++ b/lib/src/host/recovery-store.ts @@ -14,7 +14,6 @@ import { randomUUID } from 'node:crypto'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { noCommands, silent, type RecoveryLog } from './recovery-capture'; -import { ensurePrivateDirectory, ensurePrivateFile } from './private-path'; const FILE_NAME = 'recovery.json'; @@ -29,8 +28,6 @@ interface PersistedRecovery { } export interface RecoveryStore { - /** Startup privacy setup has finished; failures are logged, never rejected here. */ - readonly ready: Promise; /** * A teardown is about to capture. The FIRST call of a process replaces * whatever the last run left; later calls merge, because a Window captures @@ -40,7 +37,7 @@ export interface RecoveryStore { /** Merge one detected invocation and persist immediately. */ record(id: string, command: string): void; /** Claim the commands belonging to `paneIds`, removing each as it is handed out. */ - take(paneIds: Iterable): Promise>; + take(paneIds: Iterable): Record; /** Whether a write survives this process. `false` is the no-directory store. */ readonly persistent: boolean; } @@ -61,35 +58,9 @@ export function createRecoveryStore(dir?: string, opts: { log?: RecoveryLog } = // What this process has captured. Also the memory-only store's whole content. let captured: Record = noCommands(); - let captureStarted = false; let clearedThisProcess = false; // What is left of the record on disk, once read. `null` until the first `take`. let unclaimed: Record | null = null; - let directoryPrepared = false; - let preparation: Promise | null = null; - const prepareDirectory = (): Promise => { - if (!dir || directoryPrepared) return Promise.resolve(); - preparation ??= ensurePrivateDirectory(dir).then(() => { directoryPrepared = true; }) - .finally(() => { preparation = null; }); - return preparation; - }; - // Begin startup work immediately without blocking extension activation or sidecar I/O. - const ready = prepareDirectory().catch((err) => { - log.error(`[recovery] private directory unavailable: ${String(err)}`); - }); - let claim: Promise | null = null; - const requirePrivateDirectory = (): void => { - // Never launch a permission helper inside the bounded teardown capture. - // Cold-start claims can retry a failed startup preparation. - if (dir && !directoryPrepared) throw new Error('Recovery directory is not private'); - }; - const clearPreviousRecord = (): void => { - if (clearedThisProcess) return; - // Unlink exposes no record bytes. Even failed preparation must not preserve - // a stale invocation across a teardown that captured nothing. - if (file) fs.rmSync(file, { force: true }); - clearedThisProcess = true; - }; const persist = (): void => { if (!file) return; @@ -99,8 +70,7 @@ export function createRecoveryStore(dir?: string, opts: { log?: RecoveryLog } = // record. The `finally` is what keeps a failed write from leaving one behind. const tmp = `${file}.${randomUUID()}.tmp`; try { - requirePrivateDirectory(); - if (captureStarted) clearPreviousRecord(); + fs.mkdirSync(path.dirname(file), { recursive: true, mode: 0o700 }); // Mode on create, so the bytes are never briefly world-readable; the rename // preserves it. fs.writeFileSync(tmp, JSON.stringify(payload), { encoding: 'utf8', mode: 0o600 }); @@ -114,22 +84,20 @@ export function createRecoveryStore(dir?: string, opts: { log?: RecoveryLog } = return { persistent: file !== null, - ready, beginCapture(): void { + if (clearedThisProcess) return; + clearedThisProcess = true; // Clear before anything can return early. A record is only ever consumed by // a cold start that actually restores, so a teardown that captures nothing // must not leave the last one sitting there — otherwise a run that restores // nothing carries the record forward and a much later restore auto-runs a // week-old invocation unprompted. `record` re-creates it the moment // anything is detected. - if (!captureStarted) { - captureStarted = true; - captured = noCommands(); - if (file) unclaimed = noCommands(); - } + captured = noCommands(); + if (!file) return; try { - clearPreviousRecord(); + fs.rmSync(file, { force: true }); } catch (err) { log.error(`[recovery] could not clear the previous record: ${String(err)}`); } @@ -137,36 +105,25 @@ export function createRecoveryStore(dir?: string, opts: { log?: RecoveryLog } = record(id: string, command: string): void { captured[id] = command; - // Persist each detection: the capture deadline can end the next scan. + // Persist on every change rather than once at the end. The write is a few + // hundred bytes and costs well under a millisecond, and the shutdown budget + // can end the capture at any instant. persist(); }, - async take(paneIds: Iterable): Promise> { - if (unclaimed === null) { - claim ??= (async () => { - await ready; - await prepareDirectory(); - // Teardown can start while permission setup is pending. Never read - // this activation's newly captured record as a cold-start invocation. - unclaimed = file ? (captureStarted ? noCommands() : await readAndClearRecord(file, log, () => !captureStarted)) : captured; - })().finally(() => { claim = null; }); - try { await claim; } catch (err) { - log.error(`[recovery] private record unavailable: ${String(err)}`); - return noCommands(); - } - } - const remaining = unclaimed ?? noCommands(); + take(paneIds: Iterable): Record { + unclaimed ??= file ? readAndClearRecord(file, log) : captured; const claimed: Record = noCommands(); for (const id of paneIds) { - const command = remaining[id]; + const command = unclaimed[id]; if (command === undefined) continue; claimed[id] = command; // Entries leave the map as they are claimed, so no id is ever handed out // twice — a second container claiming its share sees only the remainder. - delete remaining[id]; + delete unclaimed[id]; } log.info(`[recovery] handing ${Object.keys(claimed).length} command(s) to a cold restore` - + ` (${Object.keys(remaining).length} unclaimed)`); + + ` (${Object.keys(unclaimed).length} unclaimed)`); return claimed; }, }; @@ -177,10 +134,8 @@ export function createRecoveryStore(dir?: string, opts: { log?: RecoveryLog } = * the durable copy is gone before anything can act on it and a failed start * cannot replay it. */ -async function readAndClearRecord(file: string, log: RecoveryLog, mayClaim: () => boolean): Promise> { +function readAndClearRecord(file: string, log: RecoveryLog): Record { if (!fs.existsSync(file)) return noCommands(); - await ensurePrivateFile(file); - if (!mayClaim()) return noCommands(); let recovery: PersistedRecovery | null = null; try { diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index 4fa85606c..f9fc124de 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -2,7 +2,7 @@ "AGENTS.md": 3600, "SECURITY.md": 200, "SELF_HOST.md": 6100, - "docs/compatible-agents.md": 1850, + "docs/compatible-agents.md": 1800, "docs/specs/alert.md": 8650, "docs/specs/auto-update.md": 1450, "docs/specs/deploy.md": 1950, @@ -36,7 +36,7 @@ "docs/specs/terminal-state.md": 2400, "docs/specs/theme.md": 2400, "docs/specs/tiling-engine.md": 4450, - "docs/specs/transport.md": 4750, + "docs/specs/transport.md": 4800, "docs/specs/tutorial.md": 2050, "docs/specs/vscode.md": 7150, "docs/specs/webgl-text.md": 1200, diff --git a/standalone/sidecar/main.js b/standalone/sidecar/main.js index ec877c9c0..cc0de18bd 100644 --- a/standalone/sidecar/main.js +++ b/standalone/sidecar/main.js @@ -190,7 +190,7 @@ function handleLine(line) { // on the first call, so nothing can replay them. case 'recovery:take': respondAsync('recovery:commands', data.requestId, async () => ({ - commands: await recovery.take(Array.isArray(data.paneIds) ? data.paneIds : []), + commands: recovery.take(Array.isArray(data.paneIds) ? data.paneIds : []), })); break; case 'pty:gracefulKill': mgr.gracefulKill(data.ids, data.timeout, data.requestId); break; diff --git a/vscode-ext/src/extension.ts b/vscode-ext/src/extension.ts index d231f6a21..5f6f50225 100644 --- a/vscode-ext/src/extension.ts +++ b/vscode-ext/src/extension.ts @@ -8,7 +8,7 @@ import { serveWebview } from './webview-messaging'; import { log } from './log'; import { initToolHost } from './tool-host'; import { forgetRetiredState } from './retired-state'; -import { captureAgentRecoveryCommands, mergeAlertStates, prepareRecoveryStorage, refreshSavedSessionStateFromPtys, takeRecoveryCommands } from './session-state'; +import { captureAgentRecoveryCommands, mergeAlertStates, refreshSavedSessionStateFromPtys, takeRecoveryCommands } from './session-state'; import { readPersistedSession } from '../../lib/src/lib/session-types'; import { workspaceTitle } from './workspace-chrome'; import { resolveSelectedShell, setSelectedShellPath, getSelectedShellPath } from './shell-selection'; @@ -27,7 +27,7 @@ let extensionContext: vscode.ExtensionContext | null = null; * state VS Code preserved from the panel's `vscode.setState()`; for a fresh * panel opened via `dormouse.open` this is `undefined`. */ -async function setupPanel( +function setupPanel( context: vscode.ExtensionContext, panel: vscode.WebviewPanel, savedState?: unknown, @@ -50,22 +50,18 @@ async function setupPanel( light: vscode.Uri.file(path.join(context.extensionPath, 'icon-tiny-light.png')), dark: vscode.Uri.file(path.join(context.extensionPath, 'icon-tiny-dark.png')), }; - let disposed = false; - let router: { dispose(): void } | undefined; - panel.onDidDispose(() => { disposed = true; router?.dispose(); }); const savedSession = readPersistedSession(initialState); // A panel's panes are interrupted by the teardown capture along with every // other live PTY, so they have a recovery command waiting too — claimed by // pane id, since the Dormouse view is claiming its own share of the same // record (docs/compatible-agents.md -> "Cold restore"). - const recoveryCommands = await takeRecoveryCommands( + const recoveryCommands = takeRecoveryCommands( context, (savedSession?.panes ?? []).map((pane) => pane.id), ); - if (disposed) return; const channel = serveWebview(panel.webview, mediaPath, initialState, getSelectedShell?.(), recoveryCommands); - router = attachRouter(channel, { + const router = attachRouter(channel, { reconnect: !!savedState, killOnDispose: true, getSelectedShell, @@ -75,6 +71,7 @@ async function setupPanel( // Panels persist via vscode.setState() (per-panel, managed by VS Code). // Don't write to workspaceState — that's for the WebviewView only. }); + panel.onDidDispose(() => router.dispose()); } export function activate(context: vscode.ExtensionContext) { @@ -94,7 +91,6 @@ export function activate(context: vscode.ExtensionContext) { context.subscriptions.push(vscode.window.onDidChangeWindowState(reportWindowPresence)); initToolHost(context.globalStorageUri?.fsPath); log.init(); - prepareRecoveryStorage(context); extensionContext = context; ptyManager.setExtensionPath(context.extensionPath); const dorRuntime = ptyManager.getDorRuntimeEnv(context.extensionPath); @@ -145,13 +141,13 @@ export function activate(context: vscode.ExtensionContext) { }), vscode.window.registerWebviewPanelSerializer('dormouse', { async deserializeWebviewPanel(panel: vscode.WebviewPanel, state: unknown) { - await setupPanel(context, panel, state, () => provider.getSelectedShell()); + setupPanel(context, panel, state, () => provider.getSelectedShell()); }, }), vscode.commands.registerCommand('dormouse.focus', () => { vscode.commands.executeCommand('dormouse.view.focus'); }), - vscode.commands.registerCommand('dormouse.open', async () => { + vscode.commands.registerCommand('dormouse.open', () => { const mediaPath = path.join(context.extensionPath, 'media'); const panel = vscode.window.createWebviewPanel( 'dormouse', @@ -163,7 +159,7 @@ export function activate(context: vscode.ExtensionContext) { localResourceRoots: [vscode.Uri.file(mediaPath)], }, ); - await setupPanel(context, panel, undefined, () => provider.getSelectedShell()); + setupPanel(context, panel, undefined, () => provider.getSelectedShell()); }), vscode.commands.registerCommand('dormouse.debugTheme', async () => { await vscode.commands.executeCommand('dormouse.view.focus'); diff --git a/vscode-ext/src/session-state.ts b/vscode-ext/src/session-state.ts index e1316e249..b629c6a81 100644 --- a/vscode-ext/src/session-state.ts +++ b/vscode-ext/src/session-state.ts @@ -85,10 +85,23 @@ export async function refreshSavedSessionStateFromPtys( log.info(`[session] refreshFromPtys: saved ${panes.length} panes`); } -/** One recovery store per activation, prepared before teardown. It shares the - * sidecar's private record and destructive claim implementation; each webview - * claims only its saved pane ids. Synchronous file writes avoid VS Code's - * teardown storage-service flush (vscode.md -> Capturing agent recovery). +/** + * This activation's recovery record, in extension storage. + * + * A plain file, written synchronously — NOT `workspaceState`. + * `workspaceState.update()` hands the value to VS Code's storage service, which + * batches its SQLite flush on its own schedule. By the time `deactivate()` runs + * that service is already tearing down, so the write never reaches disk however + * early it is issued: measured on a real machine, detection completed at +276ms + * and the record still never appeared. A synchronous `writeFileSync` is durable + * the instant it returns and needs no budget at all. + * + * The store itself is the Tauri sidecar's (`lib/src/host/recovery-store.ts`) — + * one record format, one destructive read, one set of file modes for both hosts. + * Created once per activation, because the remainder of a claimed record lives in + * it: `captureAgentRecoveryCommands` interrupts every live PTY, and those panes + * are spread across the Dormouse view and any number of editor panels, each + * restoring its own pane ids from its own saved state. */ let store: RecoveryStore | null = null; function recoveryStore(context: vscode.ExtensionContext): RecoveryStore { @@ -99,12 +112,6 @@ function recoveryStore(context: vscode.ExtensionContext): RecoveryStore { return store; } -/** Prepare private storage during activation, before a bounded teardown can - * need it. A failed permission setup is logged and persistence fails closed. */ -export function prepareRecoveryStorage(context: vscode.ExtensionContext): void { - recoveryStore(context); -} - /** * Interrupt the live PTYs, then record each pane's agent resume invocation. * @@ -163,9 +170,9 @@ export async function captureAgentRecoveryCommands( * the webview has nothing to write back and no save/restore cycle can resurrect it * (docs/compatible-agents.md -> "Cold restore"). */ -export async function takeRecoveryCommands( +export function takeRecoveryCommands( context: vscode.ExtensionContext, paneIds: Iterable, -): Promise> { +): Record { return recoveryStore(context).take(paneIds); } diff --git a/vscode-ext/src/webview-view-provider.ts b/vscode-ext/src/webview-view-provider.ts index 11b168d16..20a6810b5 100644 --- a/vscode-ext/src/webview-view-provider.ts +++ b/vscode-ext/src/webview-view-provider.ts @@ -48,13 +48,16 @@ export class DormouseViewProvider implements vscode.WebviewViewProvider { _token: vscode.CancellationToken, ): Promise { this.view = view; + // Registered before the shell-discovery await: a view disposed or replaced + // while it is pending is never served, and a stale view's disposal never + // releases its successor's router. let disposed = false; let ownedRouter: vscode.Disposable | undefined; view.onDidDispose(() => { disposed = true; ownedRouter?.dispose(); if (this.view !== view) return; - log.info('[view] onDidDispose fired - releasing router (PTYs remain alive)'); + log.info('[view] onDidDispose fired — releasing router (PTYs remain alive)'); this.routerDisposable = undefined; this.channel = undefined; this.view = undefined; @@ -82,7 +85,6 @@ export class DormouseViewProvider implements vscode.WebviewViewProvider { } } - if (disposed || this.view !== view) return; const savedSession = getSavedSessionState(this.context); // Recovery commands are claimed by this view's pane ids. const savedPaneIds = (savedSession?.panes ?? []).map((pane) => pane.id); @@ -92,8 +94,7 @@ export class DormouseViewProvider implements vscode.WebviewViewProvider { // Scoped to *this* view's panes because the capture interrupts every live PTY, // including any owned by an editor panel — taking the record whole would delete // their commands before the panel ever resolved. - const recoveryCommands = await takeRecoveryCommands(this.context, savedPaneIds); - if (disposed || this.view !== view) return; + const recoveryCommands = takeRecoveryCommands(this.context, savedPaneIds); this.channel = serveWebview( view.webview, mediaPath, savedSession, this.selectedShell, recoveryCommands, ); @@ -114,8 +115,6 @@ export class DormouseViewProvider implements vscode.WebviewViewProvider { if (this.view) this.view.badge = workspaceBadge(union); }, }); - - } focus(): void { diff --git a/vscode-ext/test/webview-view-provider.test.ts b/vscode-ext/test/webview-view-provider.test.ts index 1ccde7877..dfcab4dfd 100644 --- a/vscode-ext/test/webview-view-provider.test.ts +++ b/vscode-ext/test/webview-view-provider.test.ts @@ -12,6 +12,9 @@ vi.mock('../src/message-router', () => ({ })); vi.mock('../src/webview-messaging', () => ({ serveWebview: mocks.serve })); vi.mock('../src/pty-manager', () => ({ getAvailableShells: mocks.shells })); +vi.mock('../src/shell-selection', () => ({ + resolveSelectedShell: (_context: unknown, shells: unknown[]) => shells[0], +})); import { DormouseViewProvider } from '../src/webview-view-provider'; function view() { @@ -24,47 +27,39 @@ function deferred() { const promise = new Promise((done) => { resolve = done; }); return { promise, resolve }; } -function provider() { - const value = new DormouseViewProvider({ extensionPath: 'extension' } as never); - value.setSelectedShell({ shell: 'cmd.exe' }); - return value; -} +const cmd = [{ name: 'cmd', path: 'cmd.exe', args: [] }]; beforeEach(() => { vi.clearAllMocks(); + mocks.take.mockReturnValue({}); mocks.serve.mockReturnValue({ post: () => Promise.resolve(true) }); mocks.attach.mockReturnValue({ dispose: vi.fn() }); }); -it('waits for the asynchronous recovery claim before serving the boot document', async () => { - const ready = deferred>(); - mocks.take.mockReturnValue(ready.promise); - const pending = provider().resolveWebviewView(view().value, {} as never, {} as never); - expect(mocks.serve).not.toHaveBeenCalled(); - ready.resolve({ pane: 'codex resume ID' }); - await pending; - expect(mocks.serve.mock.calls[0][4]).toEqual({ pane: 'codex resume ID' }); - expect(mocks.attach).toHaveBeenCalledTimes(1); -}); - -it('does not serve or attach a view disposed during its recovery claim', async () => { - const ready = deferred>(), target = view(); - mocks.take.mockReturnValue(ready.promise); - const pending = provider().resolveWebviewView(target.value, {} as never, {} as never); +it('does not touch a disposed view after asynchronous shell discovery', async () => { + const ready = deferred(), target = view(); + const description = vi.fn(); + Object.defineProperty(target.value, 'description', { set: description }); + mocks.shells.mockReturnValue(ready.promise); + const host = new DormouseViewProvider({ extensionPath: 'extension' } as never); + const pending = host.resolveWebviewView(target.value, {} as never, {} as never); target.dispose(); - ready.resolve({}); + ready.resolve(cmd); await pending; + expect(description).not.toHaveBeenCalled(); + expect(mocks.take).not.toHaveBeenCalled(); expect(mocks.serve).not.toHaveBeenCalled(); expect(mocks.attach).not.toHaveBeenCalled(); }); -it('a late old claim and old disposal cannot replace or dispose a newer view', async () => { - const ready = deferred>(), first = view(), second = view(), host = provider(); +it('a late old shell discovery and old disposal cannot replace or dispose a newer view', async () => { + const ready = deferred(), first = view(), second = view(); + const host = new DormouseViewProvider({ extensionPath: 'extension' } as never); const newRouter = { dispose: vi.fn() }; mocks.attach.mockReturnValue(newRouter); - mocks.take.mockReturnValueOnce(ready.promise).mockResolvedValueOnce({}); + mocks.shells.mockReturnValueOnce(ready.promise).mockResolvedValueOnce(cmd); const old = host.resolveWebviewView(first.value, {} as never, {} as never); await host.resolveWebviewView(second.value, {} as never, {} as never); - ready.resolve({}); + ready.resolve(cmd); await old; first.dispose(); expect(mocks.serve).toHaveBeenCalledTimes(1); @@ -75,18 +70,3 @@ it('a late old claim and old disposal cannot replace or dispose a newer view', a expect(newRouter.dispose).toHaveBeenCalledTimes(1); expect(await host.postMessage({ type: 'dormouse:newTerminal' } as never)).toBe(false); }); - -it('does not touch a disposed view after asynchronous shell discovery', async () => { - const ready = deferred(), target = view(); - const description = vi.fn(); - Object.defineProperty(target.value, 'description', { set: description }); - mocks.shells.mockReturnValue(ready.promise); - const host = new DormouseViewProvider({ extensionPath: 'extension' } as never); - const pending = host.resolveWebviewView(target.value, {} as never, {} as never); - target.dispose(); - ready.resolve([{ name: 'cmd', path: 'cmd.exe', args: [] }]); - await pending; - expect(description).not.toHaveBeenCalled(); - expect(mocks.take).not.toHaveBeenCalled(); - expect(mocks.serve).not.toHaveBeenCalled(); -}); From 9cbbe9f9b33e6301e3c7cc9458d0c1f13be3b983 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 1 Oct 2026 22:32:18 -0700 Subject: [PATCH 08/10] Track peer frame bytes incrementally FrameDecoder re-measured the whole pending buffer with Buffer.byteLength on every push, quadratic in a frame that arrives in many chunks. Measure each newline-delimited piece once as it arrives and carry the pending frame's byte count, keeping the cap on complete frames and partial tails and preserving adjacent valid frames. Co-Authored-By: Claude Opus 5.5 --- vscode-ext/src/peer-link-protocol.ts | 36 ++++++++++++++-------- vscode-ext/test/peer-link-protocol.test.ts | 11 +++++++ 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/vscode-ext/src/peer-link-protocol.ts b/vscode-ext/src/peer-link-protocol.ts index dc52005f4..852f2d260 100644 --- a/vscode-ext/src/peer-link-protocol.ts +++ b/vscode-ext/src/peer-link-protocol.ts @@ -93,7 +93,9 @@ export function encodeFrame(frame: PeerLinkFrame | PeerLinkHandshake): string { * killing the link. */ export class FrameDecoder { + /** The unterminated frame so far, and its UTF-8 size, counted per chunk. */ #buffer = ''; + #bufferBytes = 0; /** * Set once one frame has outgrown the cap: everything up to the next newline * belongs to that frame and is dropped, and normal accumulation resumes after @@ -109,20 +111,23 @@ export class FrameDecoder { } push(chunk: string): unknown[] { - this.#buffer += chunk; const frames: unknown[] = []; - for (;;) { - const newline = this.#buffer.indexOf('\n'); - if (newline === -1) break; - const line = this.#buffer.slice(0, newline); - this.#buffer = this.#buffer.slice(newline + 1); + // Each piece is measured once, as it arrives — re-measuring the whole + // buffer on every chunk would be quadratic in a large frame. + const pieces = chunk.split('\n'); + const tail = pieces.pop()!; + for (const piece of pieces) { + const line = this.#buffer + piece; + const bytes = this.#bufferBytes + Buffer.byteLength(piece, 'utf8'); + this.#buffer = ''; + this.#bufferBytes = 0; if (this.#discarding) { // That was the oversized frame's terminator; the bytes after it are a // frame boundary again. this.#discarding = false; continue; } - if (Buffer.byteLength(line, 'utf8') > this.#maxFrameBytes) continue; + if (bytes > this.#maxFrameBytes) continue; if (!line.trim()) continue; try { frames.push(JSON.parse(line)); @@ -131,11 +136,18 @@ export class FrameDecoder { } } // Whatever is left is one unterminated frame. Past the cap it is a frame we - // can never read, so it goes — but the whole frames already taken out of - // the buffer above are real, and dropping them with it would lose traffic - // from a link that is otherwise healthy. - if (Buffer.byteLength(this.#buffer, 'utf8') > this.#maxFrameBytes) this.#discarding = true; - if (this.#discarding) this.#buffer = ''; + // can never read, so it goes — but the whole frames already taken out + // above are real, and dropping them with it would lose traffic from a link + // that is otherwise healthy. + if (!this.#discarding) { + this.#buffer += tail; + this.#bufferBytes += Buffer.byteLength(tail, 'utf8'); + if (this.#bufferBytes > this.#maxFrameBytes) this.#discarding = true; + } + if (this.#discarding) { + this.#buffer = ''; + this.#bufferBytes = 0; + } return frames; } } diff --git a/vscode-ext/test/peer-link-protocol.test.ts b/vscode-ext/test/peer-link-protocol.test.ts index 233d7fc83..639bf2e40 100644 --- a/vscode-ext/test/peer-link-protocol.test.ts +++ b/vscode-ext/test/peer-link-protocol.test.ts @@ -43,6 +43,17 @@ describe('FrameDecoder', () => { expect(new FrameDecoder(cap - 1).push(encoded)).toEqual([]); }); + it('counts UTF-8 bytes across a frame delivered one character at a time', () => { + const frame = { kind: 'data', ptyId: 'p', data: '\u00e9'.repeat(20) } as const; + const encoded = encodeFrame(frame); + const cap = Buffer.byteLength(encoded.slice(0, -1), 'utf8'); + const trickle = (decoder: FrameDecoder) => [...encoded].flatMap((ch) => decoder.push(ch)); + expect(trickle(new FrameDecoder(cap))).toEqual([frame]); + const over = new FrameDecoder(cap - 1); + expect(trickle(over)).toEqual([]); + expect(over.push(encodeFrame({ kind: 'notify' }))).toEqual([{ kind: 'notify' }]); + }); + it('reads one frame per line', () => { const decoder = new FrameDecoder(); const frames = decoder.push( From ad321f628ec77ba69d8c757b323fae39ee2a387c Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 1 Oct 2026 22:32:18 -0700 Subject: [PATCH 09/10] Move Windows test-portability fixes out of this PR The original PR bundled unrelated test-portability work into a host security change: path.win32 fixtures for enroll-offer, pty-core, and mirrored-constants, the named-pipe peer-link tests, and the Unix peer socket parent setup. None of it changes shipped behaviour, and none of the Windows-only cases run in CI, so it obscured the reviewable part of the diff. It now lives on its own PR (#899, based on main), leaving this PR as the spec cleanup plus the peer-frame byte cap. Co-Authored-By: Claude Opus 5.5 --- lib/src/host/remote/enroll-offer.ts | 11 ++-- lib/src/lib/mirrored-constants.test.ts | 8 +-- standalone/sidecar/pty-core.js | 14 +++-- standalone/sidecar/pty-core.test.js | 10 ++-- vscode-ext/test/helpers.ts | 4 +- vscode-ext/test/peer-link.test.ts | 71 ++++++-------------------- 6 files changed, 37 insertions(+), 81 deletions(-) diff --git a/lib/src/host/remote/enroll-offer.ts b/lib/src/host/remote/enroll-offer.ts index 38b2bbc9a..2c6be8d02 100644 --- a/lib/src/host/remote/enroll-offer.ts +++ b/lib/src/host/remote/enroll-offer.ts @@ -16,7 +16,7 @@ import { readFile } from 'node:fs/promises'; import { homedir } from 'node:os'; -import * as path from 'node:path'; +import { join } from 'node:path'; import { isEnrollmentOfferFresh, parseEnrollmentOffer, @@ -25,7 +25,7 @@ import { export type { EnrollmentOffer }; -const OFFER_FILE = 'enroll-offer.json'; +const OFFER_FILE = join('run', 'enroll-offer.json'); /** * Where each installer's offer lands, mirroring the install root that installer @@ -44,18 +44,17 @@ export function enrollmentOfferPath( env: NodeJS.ProcessEnv = process.env, home: string = homedir(), ): string | null { - const { join } = platform === 'win32' ? path.win32 : path.posix; switch (platform) { case 'darwin': - return join(home, 'Library', 'Application Support', 'Dormouse Relay', 'run', OFFER_FILE); + return join(home, 'Library', 'Application Support', 'Dormouse Relay', OFFER_FILE); case 'win32': // No `%LOCALAPPDATA%` is not a path to guess at: the installer joins onto // that variable, so without it this machine's install root is unknown. - return env.LOCALAPPDATA ? join(env.LOCALAPPDATA, 'Dormouse Relay', 'run', OFFER_FILE) : null; + return env.LOCALAPPDATA ? join(env.LOCALAPPDATA, 'Dormouse Relay', OFFER_FILE) : null; default: // `||` and not `??`, matching the installers' `${XDG_DATA_HOME:-…}`: an // empty value is unset, not a root at the filesystem's top. - return join(env.XDG_DATA_HOME || join(home, '.local', 'share'), 'dormouse-relay', 'run', OFFER_FILE); + return join(env.XDG_DATA_HOME || join(home, '.local', 'share'), 'dormouse-relay', OFFER_FILE); } } diff --git a/lib/src/lib/mirrored-constants.test.ts b/lib/src/lib/mirrored-constants.test.ts index 1138196ce..5ad87f0e1 100644 --- a/lib/src/lib/mirrored-constants.test.ts +++ b/lib/src/lib/mirrored-constants.test.ts @@ -1,6 +1,6 @@ import { readFileSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; -import { dirname, join, resolve, win32 } from 'node:path'; +import { dirname, join, resolve } from 'node:path'; import { describe, expect, it } from 'vitest'; import { PAIRING_OUTCOME_COPY, @@ -137,12 +137,12 @@ describe('enrollment-offer path mirrors the installers', () => { const source = readRepoFile(file); const variable = extract(source, file, /^\$INSTALL_ROOT = Join-Path \$env:(\w+) '[^']+'$/m); const local = 'C:\\Users\\ned\\AppData\\Local'; - const root = win32.join( + const root = join( local, extract(source, file, /^\$INSTALL_ROOT = Join-Path \$env:\w+ '([^']+)'$/m), ); - const run = win32.join(root, extract(source, file, /^\$RUN_DIR = Join-Path \$INSTALL_ROOT '([^']+)'$/m)); - const offerFile = win32.join( + const run = join(root, extract(source, file, /^\$RUN_DIR = Join-Path \$INSTALL_ROOT '([^']+)'$/m)); + const offerFile = join( run, extract(source, file, /^\$ENROLL_OFFER_FILE = Join-Path \$RUN_DIR '([^']+)'$/m), ); diff --git a/standalone/sidecar/pty-core.js b/standalone/sidecar/pty-core.js index ac5edfcd7..bd2709448 100644 --- a/standalone/sidecar/pty-core.js +++ b/standalone/sidecar/pty-core.js @@ -209,8 +209,7 @@ function withoutInheritedMsysOriginalPath(env, platform = process.platform) { // glob); `DORMOUSE_SHELL_INTEGRATION_DIR` overrides it for hosts that stage the // sidecar elsewhere (e.g. the VS Code bundle) and for tests. function resolveShellIntegrationDir(env, runtime = {}) { - const platformPath = (runtime.platform || process.platform) === 'win32' ? path.win32 : path.posix; - return env.DORMOUSE_SHELL_INTEGRATION_DIR || platformPath.join(runtime.dirname || __dirname, 'shell-integration'); + return env.DORMOUSE_SHELL_INTEGRATION_DIR || path.join(runtime.dirname || __dirname, 'shell-integration'); } // Basename of a shell path, lowercased and with any `.exe` dropped, handling @@ -259,12 +258,11 @@ function winPathToWslMount(winPath) { // their shell. bash is the only WSL shell we integrate for now. function applyShellIntegration(shell, env, shellArgs, integrationDir, runtime = {}) { const fsModule = runtime.fsModule || fs; - const platformPath = (runtime.platform || process.platform) === 'win32' ? path.win32 : path.posix; const stem = shellStem(shell); if (stem === 'zsh') { - const zshDir = platformPath.join(integrationDir, 'zsh'); - if (fileExists(platformPath.join(zshDir, '.zshrc'), fsModule)) { + const zshDir = path.join(integrationDir, 'zsh'); + if (fileExists(path.join(zshDir, '.zshrc'), fsModule)) { return { env: { ...env, ZDOTDIR: zshDir, USER_ZDOTDIR: env.ZDOTDIR || env.HOME || '' }, shellArgs, @@ -273,14 +271,14 @@ function applyShellIntegration(shell, env, shellArgs, integrationDir, runtime = } if (stem === 'bash' && bashArgsAreInjectable(shellArgs)) { - const script = platformPath.join(integrationDir, 'bash', 'shellIntegration.bash'); + const script = path.join(integrationDir, 'bash', 'shellIntegration.bash'); if (fileExists(script, fsModule)) { return { env, shellArgs: ['--init-file', script] }; } } if (stem === 'pwsh' || stem === 'powershell') { - const script = platformPath.join(integrationDir, 'pwsh', 'shellIntegration.ps1'); + const script = path.join(integrationDir, 'pwsh', 'shellIntegration.ps1'); if (fileExists(script, fsModule)) { const integratedArgs = powerShellIntegratedArgs(shellArgs, script); if (integratedArgs) return { env, shellArgs: integratedArgs }; @@ -289,7 +287,7 @@ function applyShellIntegration(shell, env, shellArgs, integrationDir, runtime = // WSL: only the standard `-d ` launch (the shape the picker emits). if (stem === 'wsl' && shellArgs.length === 2 && shellArgs[0] === '-d') { - const script = platformPath.join(integrationDir, 'bash', 'shellIntegration.bash'); + const script = path.join(integrationDir, 'bash', 'shellIntegration.bash'); const mount = winPathToWslMount(script); if (mount && fileExists(script, fsModule)) { // A `sh -c` detector, passed as one argv element so node-pty hands it to diff --git a/standalone/sidecar/pty-core.test.js b/standalone/sidecar/pty-core.test.js index 7d467141b..94cf5ee50 100644 --- a/standalone/sidecar/pty-core.test.js +++ b/standalone/sidecar/pty-core.test.js @@ -1075,7 +1075,7 @@ test('resolveSpawnConfig injects bash integration for Git Bash despite its --log // The --login -i defaults are subsumed by the init-file script, which sources // the login profile itself. - const script = path.win32.join(integrationDir, 'bash', 'shellIntegration.bash'); + const script = path.join(integrationDir, 'bash', 'shellIntegration.bash'); assert.deepEqual(config.shellArgs, ['--init-file', script]); }); @@ -1128,7 +1128,7 @@ test('resolveSpawnConfig injects pwsh integration via -NoExit -Command dot-sourc }, ); - const script = path.win32.join(integrationDir, 'pwsh', 'shellIntegration.ps1'); + const script = path.join(integrationDir, 'pwsh', 'shellIntegration.ps1'); assert.deepEqual(config.shellArgs, ['-NoExit', '-Command', `. '${script}'`]); }); @@ -1149,7 +1149,7 @@ test('resolveSpawnConfig injects Windows PowerShell (powershell.exe) too', () => }, ); - const script = path.win32.join(integrationDir, 'pwsh', 'shellIntegration.ps1'); + const script = path.join(integrationDir, 'pwsh', 'shellIntegration.ps1'); assert.deepEqual(config.shellArgs, ['-NoExit', '-Command', `. '${script}'`]); }); @@ -1172,7 +1172,7 @@ test('resolveSpawnConfig merges integration into an interactive pwsh -Command (e ); // The dev-shell command runs first, then our dot-source installs the prompt wrapper. - const script = path.win32.join(integrationDir, 'pwsh', 'shellIntegration.ps1'); + const script = path.join(integrationDir, 'pwsh', 'shellIntegration.ps1'); assert.deepEqual(config.shellArgs, [ '-NoExit', '-Command', @@ -1195,7 +1195,7 @@ test('resolveSpawnConfig adds a -Command to an interactive pwsh launch that has }, ); - const script = path.win32.join(integrationDir, 'pwsh', 'shellIntegration.ps1'); + const script = path.join(integrationDir, 'pwsh', 'shellIntegration.ps1'); assert.deepEqual(config.shellArgs, ['-NoExit', '-NoLogo', '-Command', `. '${script}'`]); }); diff --git a/vscode-ext/test/helpers.ts b/vscode-ext/test/helpers.ts index e56f6769e..8ce8520a3 100644 --- a/vscode-ext/test/helpers.ts +++ b/vscode-ext/test/helpers.ts @@ -33,9 +33,7 @@ export async function tempStorageDir(): Promise { */ export function derivedSocketPath(storageDir: string): string { const id = createHash('sha256').update(storageDir).digest('hex').slice(0, 12); - return process.platform === 'win32' - ? `\\\\.\\pipe\\dormouse-peer-${id}` - : join(tmpdir(), `dormouse-peer-${process.getuid?.() ?? 0}`, `${id}.sock`); + return join(tmpdir(), `dormouse-peer-${process.getuid?.() ?? 0}`, `${id}.sock`); } export async function removeDir(dir: string): Promise { diff --git a/vscode-ext/test/peer-link.test.ts b/vscode-ext/test/peer-link.test.ts index d4767111b..1913b0199 100644 --- a/vscode-ext/test/peer-link.test.ts +++ b/vscode-ext/test/peer-link.test.ts @@ -170,44 +170,6 @@ afterEach(async () => { }); describe('bind-as-lease', () => { - it.skipIf(process.platform !== 'win32')('reclaims a dead named pipe and elects exactly one broker for racing windows', async () => { - // Windows removes the pipe when its process dies; there is no Unix corpse - // inode to unlink. Exercise the actual transport, not a fake socket path. - const corpse = spawn(process.execPath, ['-e', - "require('node:net').createServer().listen(process.argv[1], () => process.send('listening'))", - derivedSocketPath()], { stdio: ['ignore', 'ignore', 'pipe', 'ipc'] }); - try { - await new Promise((resolve, reject) => { - corpse.once('message', () => resolve()); - corpse.once('error', reject); - corpse.once('exit', (code) => reject(new Error(`pipe fixture exited ${code}`))); - }); - const exited = new Promise((resolve) => corpse.once('exit', () => resolve())); - corpse.kill('SIGKILL'); - await exited; - const firstSide = fakeWindow({ entries: [{ surfaceId: 'first' }] }); - const secondSide = fakeWindow({ entries: [{ surfaceId: 'second' }] }); - const first = await openWindow(firstSide); - const second = await openWindow(secondSide); - const roles: boolean[] = []; - await Promise.all([ - first.ensurePeerNet((held) => roles.push(held)), - second.ensurePeerNet((held) => roles.push(held)), - ]); - const brokers = [first, second].filter((mod) => mod.isPeerBroker()); - expect(brokers).toHaveLength(1); - expect(roles).toEqual([true]); - const expected = first.isPeerBroker() ? secondSide.entries : firstSide.entries; - await waitFor(async () => JSON.stringify(await brokers[0].remoteRequest('directory', {})) === JSON.stringify(expected)); - } finally { - if (corpse.exitCode === null && corpse.signalCode === null) { - const exited = new Promise((resolve) => corpse.once('exit', () => resolve())); - corpse.kill('SIGKILL'); - await exited; - } - } - }, 15_000); - it('rejects when the peer socket cannot be bound', async () => { const mod = await openWindow(fakeWindow()); const failingServer = createServer(); @@ -222,8 +184,7 @@ describe('bind-as-lease', () => { // libuv callback with nothing to catch it. const mod = await openWindow(fakeWindow()); const server = createServer(); - const path = derivedSocketPath(); - if (process.platform !== 'win32') await mkdir(dirname(path), { recursive: true, mode: 0o700 }); + const path = join(dir, 'accept-error.sock'); await mod.listenServer(server, path); try { expect(() => server.emit('error', Object.assign(new Error('EMFILE'), { code: 'EMFILE' }))) @@ -297,13 +258,13 @@ describe('bind-as-lease', () => { releaseStuck(); }, 15_000); - it.skipIf(process.platform === 'win32')('does not answer broker while a reclaimed bind is still unverified', async () => { + it('does not answer broker while a reclaimed bind is still unverified', async () => { // `stillOurs` spends 250 ms watching for a window that cleared the same // corpse and bound after us. An enroll landing inside that window used to // see a bound socket, start a service, and the stand-down path // (`closeServer(false)`) never tears one down — two Burrows under one burrowId. const path = derivedSocketPath(); - if (process.platform !== 'win32') await mkdir(dirname(path), { recursive: true, mode: 0o700 }); + await mkdir(dirname(path), { recursive: true, mode: 0o700 }); const corpse = spawn(process.execPath, [ '-e', `require('node:net').createServer().listen(${JSON.stringify(path)})`, @@ -421,9 +382,9 @@ describe('bind-as-lease', () => { expect(roles).toEqual([true, true]); }); - it.skipIf(process.platform === 'win32')('takes over a socket whose broker died without unlinking it', async () => { + it('takes over a socket whose broker died without unlinking it', async () => { const path = derivedSocketPath(); - if (process.platform !== 'win32') await mkdir(dirname(path), { recursive: true, mode: 0o700 }); + await mkdir(dirname(path), { recursive: true, mode: 0o700 }); // A killed process leaves the inode behind — `close()` would unlink it, so // the only way to produce this state is to not let the owner close. const corpse = spawn(process.execPath, [ @@ -444,13 +405,13 @@ describe('bind-as-lease', () => { expect(mod.isPeerBroker()).toBe(true); }); - it.skipIf(process.platform === 'win32')('re-binds when the socket it reclaimed is unlinked out from under it', async () => { + it('re-binds when the socket it reclaimed is unlinked out from under it', async () => { // Two windows can clear the same corpse and the second bind displaces the // first without any error — the loser keeps serving an inode no client can // reach. On unix a path that has *gone* after our bind is the same failure, // and reading it as "still ours" leaves a broker nobody can dial. const path = derivedSocketPath(); - if (process.platform !== 'win32') await mkdir(dirname(path), { recursive: true, mode: 0o700 }); + await mkdir(dirname(path), { recursive: true, mode: 0o700 }); const corpse = spawn(process.execPath, [ '-e', `require('node:net').createServer().listen(${JSON.stringify(path)})`, @@ -485,13 +446,13 @@ describe('bind-as-lease', () => { expect(peer.isPeerBroker()).toBe(false); }, 30_000); - it.skipIf(process.platform === 'win32')('settles two windows racing for one corpse into a broker and a client', async () => { + it('settles two windows racing for one corpse into a broker and a client', async () => { // Both find the same dead socket, both may unlink it, and the second bind // silently displaces the first. Whoever loses that has to notice and stand // down rather than serve an inode nobody can reach — and must then end up a // client, not wedged. const path = derivedSocketPath(); - if (process.platform !== 'win32') await mkdir(dirname(path), { recursive: true, mode: 0o700 }); + await mkdir(dirname(path), { recursive: true, mode: 0o700 }); const corpse = spawn(process.execPath, [ '-e', `require('node:net').createServer().listen(${JSON.stringify(path)})`, @@ -517,14 +478,14 @@ describe('bind-as-lease', () => { expect(firstRoles.concat(secondRoles)).toEqual([true]); }, 30_000); - it.skipIf(process.platform === 'win32')('stands down when a competing reclaim displaces it before its verification reads the path', async () => { + it('stands down when a competing reclaim displaces it before its verification reads the path', async () => { // The interleaving the racing test above reaches only by luck, forced: a // competing window's unlink and rebind land after our bind but before // `stillOurs` first reads the path. Anchored to that read rather than to // our own bind, both windows would name the competitor's socket as "ours" // and both would broker. const path = derivedSocketPath(); - if (process.platform !== 'win32') await mkdir(dirname(path), { recursive: true, mode: 0o700 }); + await mkdir(dirname(path), { recursive: true, mode: 0o700 }); const corpse = spawn(process.execPath, [ '-e', `require('node:net').createServer().listen(${JSON.stringify(path)})`, @@ -917,7 +878,7 @@ describe('bind-as-lease', () => { } }); }); - if (process.platform !== 'win32') await mkdir(dirname(derivedSocketPath()), { recursive: true, mode: 0o700 }); + await mkdir(dirname(derivedSocketPath()), { recursive: true, mode: 0o700 }); await new Promise((resolve, reject) => { server.once('error', reject); server.listen(derivedSocketPath(), () => { @@ -1244,7 +1205,7 @@ describe('peer handshake', () => { })(); }); const path = derivedSocketPath(); - if (process.platform !== 'win32') await mkdir(dirname(path), { recursive: true, mode: 0o700 }); + await mkdir(dirname(path), { recursive: true, mode: 0o700 }); await new Promise((resolve) => squatter.listen(path, resolve)); try { @@ -1284,7 +1245,7 @@ describe('peer handshake', () => { socket.write('null\n'); }); const path = derivedSocketPath(); - if (process.platform !== 'win32') await mkdir(dirname(path), { recursive: true, mode: 0o700 }); + await mkdir(dirname(path), { recursive: true, mode: 0o700 }); await new Promise((resolve) => squatter.listen(path, resolve)); try { @@ -1305,7 +1266,7 @@ describe('peer handshake', () => { } }); - it.skipIf(process.platform === 'win32')('keeps the socket directory private to this user', async () => { + it('keeps the socket directory private to this user', async () => { // The layer below the handshake: in a shared tmpdir, a directory anyone can // write to is one where a co-resident user can create the path first. const peerDir = dirname(derivedSocketPath()); @@ -1320,7 +1281,7 @@ describe('peer handshake', () => { expect((await stat(peerDir)).mode & 0o777).toBe(0o700); }); - it.skipIf(process.platform === 'win32')('stands down for good when the socket directory is not one', async () => { + it('stands down for good when the socket directory is not one', async () => { // Something else holds the only place these sockets may live. No amount of // retrying changes that, so the link stops rather than spinning — and the // waiting caller is released rather than left hanging. From f231bb057a3349f829e31c4e6b8903c567bdfbba Mon Sep 17 00:00:00 2001 From: Ned Date: Thu, 1 Oct 2026 23:10:39 -0700 Subject: [PATCH 10/10] Keep generated contributor links covered after spec compression --- website/scripts/generate-docs.test.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/website/scripts/generate-docs.test.js b/website/scripts/generate-docs.test.js index f036005df..880f096f6 100644 --- a/website/scripts/generate-docs.test.js +++ b/website/scripts/generate-docs.test.js @@ -52,12 +52,15 @@ describe('compatible agents', () => { expect(body).not.toContain('If automatic agent startup becomes disruptive'); }); - it('sends the contributor link to the withheld contract on GitHub', () => { + it('sends contributor links to withheld headings on GitHub', () => { expect(data.agents.withheldLinks).toEqual([{ + from: '#detection', + to: `${REPO_BLOB_BASE}/docs/compatible-agents.md#detection`, + }, { from: '#recovery-contract-maintainers', to: `${REPO_BLOB_BASE}/docs/compatible-agents.md#recovery-contract-maintainers`, }]); - expect(generatedHrefs()).toContain(data.agents.withheldLinks[0].to); + expect(generatedHrefs()).toEqual(expect.arrayContaining(data.agents.withheldLinks.map(({ to }) => to))); }); it('keeps the supported-agent table aligned with executable and resume definitions', () => {