diff --git a/docs/specs/auto-update.md b/docs/specs/auto-update.md index 3da2af03e..858c6544f 100644 --- a/docs/specs/auto-update.md +++ b/docs/specs/auto-update.md @@ -6,7 +6,7 @@ The standalone app checks for updates on launch, where the network policy allows ## How it works -**Must read and clear the post-install marker on launch** (§localStorage) and show its banner; a reported failure suppresses this launch's check. Otherwise wait 5 seconds, then read the network policy with `networkPolicy` over the Burrow link and, where it allows (`docs/specs/remote-network.md` → "Updates"), `check()` — no update is silent, an update raises the approval prompt; then the reminder, if due: `check-due`, recording `remindedAt`. **The reminder is re-evaluated hourly while the app runs**, reading no policy and never checking; **never over an undismissed notice, nor while the clock reads before 2026-09**, not yet set. Version-lookup and check failures are logged. **Only approval starts the background `download()`**; a failed one is logged and the prompt returns. +**Must read and clear the post-install marker on launch** (§localStorage) and show its banner; a reported failure suppresses this launch's check. Otherwise wait 5 seconds, then read the network policy with `networkPolicy` over the Burrow link and, where it allows (`docs/specs/remote-network.md` → "Updates"), `check()` — no update is silent, an update raises the approval prompt; then the reminder, if due: `check-due`, recording `remindedAt`. **Must skip that `check()` once an update is approved**, including through Check now during the wait or the policy read. **The reminder is re-evaluated hourly while the app runs**, reading no policy and never checking; **never over an undismissed notice, nor while the clock reads before 2026-09**, not yet set. Version-lookup and check failures are logged. **Only approval starts the background `download()`**; a failed one is logged and the prompt returns. **Check now** — the `check-due` and `check-failed` links, and the `updates` port — shows `checking`, then `available`, `up-to-date`, or `check-failed`. **A second ask joins the check in flight. An update already approved is shown again, `downloading` or `downloaded`, instead of checked for**, which would offer it for approval twice. **Every successful check, automatic or asked for, records `checkedAt`** (§localStorage). diff --git a/docs/specs/security-remote.md b/docs/specs/security-remote.md index abb14b771..e8db112b1 100644 --- a/docs/specs/security-remote.md +++ b/docs/specs/security-remote.md @@ -111,7 +111,7 @@ per-Burrow browser storage follows `docs/specs/remote-security-model.md` -> - **FAIL IF** `relay/src/state.ts` stops creating `$DORMOUSE_STATE_DIR` mode `0o700`, or stops writing every file through `writeAtomic` at mode `0o600`. The "every file" clause is a negative search over `relay/src/`: no `writeFile`, `appendFile`, or `createWriteStream` may target the state directory outside `writeAtomic`. A cheap default, not a cross-platform guarantee; the installer's directory permissions below protect the installed Relay's state (rationale). - **FAIL IF** `FileBurrowStateStore` (`lib/src/host/remote/burrow-state-store.ts`) stops creating its directory `0o700` and writing `0o600` on non-Windows platforms, or if `VsCodeBurrowStateStore` stops keeping the **enrollment** in `SecretStorage`. The ACL's home in `globalState` is deliberate and is not a finding; the enrollment's is what carries `burrowToken`. - **FAIL IF** a credential the Host→Burrow rename retired stops being deleted unread at boot: `state/hosts.json` on the Relay (`forgetRetiredState` in `relay/src/state.ts`, called from `relay/src/start.ts`), `remote-host.json` on a Node-resident Burrow (`forgetRetiredState` in `lib/src/host/remote/burrow-state-store.ts`, called from `sidecar-entry.ts`), and `dormouse.remote-host.enrollment`, `dormouse.remote-host.acl.*`, `remote-host.peer-token` in VS Code (`vscode-ext/src/retired-state.ts`, called from `activate()`). Each held a live `burrowToken` or peer secret, and `SecretStorage` cannot be enumerated — a key nothing removes *by name* outlives every build that knew it. Pinned by `relay/test/state-records.test.mjs`, `lib/src/host/remote/burrow-state-store.test.ts` and `vscode-ext/test/retired-state.test.ts`. -- **FAIL IF** `burrow_state_dir` in `standalone/src-tauri/src/lib.rs` stops calling `restrict_to_owner` on the state directory **before** spawning the sidecar — on Windows those Node modes are no-ops and Node cannot set an ACL, so the guarantee is held one layer down. That call carries both legs: a newly written enrollment file *inherits* the owner-only entry, and one a prior version already left under the `%LOCALAPPDATA%` ACL — with a live `burrowToken` in it — has that entry *propagated* onto it, the half `restrict_to_owner_leaves_one_owner_only_ace` covers with its pre-existing `before.json`. +- **FAIL IF** `burrow_state_dir` in `standalone/src-tauri/src/lib.rs` passes the sidecar a state directory `restrict_to_owner` did not lock — on Windows those Node modes are no-ops and Node cannot set an ACL, so the guarantee is held one layer down; a refusal keeps the Burrow in memory (`burrow_directory_permission_failure_disables_durable_state`). That call carries both legs: a newly written enrollment file *inherits* the owner-only entry, and one a prior version already left under the `%LOCALAPPDATA%` ACL — with a live `burrowToken` in it — has that entry *propagated* onto it, the half `restrict_to_owner_leaves_one_owner_only_ace` covers with its pre-existing `before.json`. - **FAIL IF** `relay/src/start.ts` stops obtaining the setup password from `SetupPasswordStore.loadOrCreate(generateSetupPassword)`, `generateSetupPassword` stops using `crypto.randomBytes(32)`, `readConfig` reads `DORMOUSE_SETUP_PASSWORD` or any other setup-password input, or `SetupPasswordStore` stops refusing a persisted or generated value outside 64 lowercase hexadecimal characters. Pinned by `relay/test/config.test.mjs` and `relay/test/setup-password-store.test.mjs`. - **FAIL IF** `createApp` accepts anything but 64 lowercase hexadecimal characters as the setup password injected by the entrypoint; pinned by `relay/test/app.test.mjs`. - **FAIL IF** any installer stops making `config/`, `state/`, and `config/relay.env` reachable only by the installing user — the effective property `manage verify` tests: no principal other than that user may appear in the effective permissions. macOS and Linux achieve it with `0700`/`0600` under `umask 077`; Windows with a single owner-only ACE, whether the path carries it directly or inherits it from an already-locked parent. The Windows and Linux installers create `relay.env` and lock it before writing its contents (rationale). diff --git a/docs/specs/standalone.md b/docs/specs/standalone.md index 709cc8056..73d011e82 100644 --- a/docs/specs/standalone.md +++ b/docs/specs/standalone.md @@ -101,7 +101,7 @@ constants in `lib/src/lib/platform/types.ts` (and `standalone/sidecar/pty-core.j `#[tauri::command]` over an `async fn`, which the guard below accepts equally. Tauri runs a *sync* command on the main thread, where the `recv_timeout` inside `request_from_sidecar` / `request_from_sidecar_timeout` stops the webview painting -for the whole round trip, up to `AGENT_BROWSER_TIMEOUT` (30s) (rationale). **The +for the whole round trip, up to `BROWSER_REQUEST_TIMEOUT` (40s) (rationale). **The three clipboard readers included**: their non-Windows branches round-trip through the sidecar, and the declaration is per command, not per branch. A unit test in `lib.rs` scans the source and fails on any command that reaches the blocking @@ -574,8 +574,12 @@ checks in the debounce flush. - **The flush slot is released in the same step as the drain.** A `Moved` landing between the two was marked dirty with no thread left to write it — and that move is exactly a window's final position. +- **Must recheck the save refusal under the journal lock before writing + geometry.** The flush reads the window and rect first; a close in between + removed the file (§Per-window close), and the write would put it back + (`a_geometry_flush_captured_before_close_cannot_recreate_removed_geometry`). -Source of truth: `CachedRect` / `GeometryState` / `note_geometry` / +Source of truth: `CachedRect` / `GeometryState` / `note_geometry` / `write_open_window_geometry` / `restore_windows` in `standalone/src-tauri/src/lib.rs`; the sequencing is pinned by `the_geometry_flush_slot_is_released_with_the_drain`. @@ -604,12 +608,12 @@ Source of truth: `CleanupGate` and `WindowEvent::Destroyed` in **Closing a window with siblings alive ends that window alone**; only the last window's close is the quit. Rust prevents the close and emits `dormouse://window-close-requested`; the webview acks (a ~2 s watchdog closes it -anyway if that listener is dead), asks about *its own* running work, removes its snapshot, kills the PTYs it owns, and calls back +anyway if that listener is dead), asks about *its own* running work, attempts snapshot removal, kills its PTYs, and calls back `close_window`. -- **A close is deliberate, so it removes the blob** — geometry - and temp sibling included — and the next launch does not reopen the window - (`docs/specs/transport.md` → "The governing rule"). +- **Must attempt to remove the blob before killing this Window's PTYs** — geometry + and temp sibling included; successful removal prevents reopening + (`docs/specs/transport.md` → "The governing rule"). Removal failures are logged and close proceeds; an old snapshot may reopen. - **It runs no agent-recovery capture**: nothing is coming back. - **A cancelled close retires its watchdog's token and never reuses it**: the next close on that window is a fresh seq, so a watchdog still sleeping on the @@ -618,11 +622,13 @@ anyway if that listener is dead), asks about *its own* running work, removes its - **It confirms on a pending download as well as on running work.** An approved, downloaded update lives in this webview's memory, so closing the window throws it away and nothing else can install it (`docs/specs/auto-update.md`). -- **The snapshot is removed before the kill**, and Rust refuses every later save - for that label, so a PTY exit's save cannot write it back. **Both close paths +- **Must refuse every later save for a closing label**, so a PTY exit cannot recreate its snapshot. **Both close paths set that refusal** — the webview's own `remove_window_session`, and `finish_window_close` for the ack-timeout path, where the webview never ran at - all. It is dropped when the webview is destroyed and can no longer save. + all. **Must keep that refusal for the process lifetime**, geometry writes + included: a save dispatched before `Destroyed` can reach the disk lock after + it, and no label is reused within a process + (`a_closed_window_refuses_saves_for_the_process_lifetime`). - **`close_window` is the one Rust half both endings share** — a deliberate close and a window whose last Workspace moved away (§Transfer) — because what separates them is entirely what the webview did before calling it. @@ -700,7 +706,7 @@ below reads that record rather than inferring itself from the suppression map. `transfer_workspace` / `open_workspace_window`. **Must return preparation refusals as `{ moved: false, reason }` without changing ownership.** On `Ok` it marks the Workspace **transferring**: the Wall stays mounted, nothing is released, and `getWindowSnapshot` omits it. -2. **Rust** reassigns `terminalIds` to the target, keeps routing their output to +2. **Rust** journals the arrival (below), then reassigns `terminalIds` to the target, keeps routing their output to the source, and asks the sidecar to stamp a `pty:marked` line per id; at that line the id's suppression begins, until its replay has been emitted to the target. The source serializes each buffer at its mark and invokes @@ -796,14 +802,17 @@ below reads that record rather than inferring itself from the suppression map. - **A boot's `pty_request_init` excludes every id an arrival claims.** Ownership moves at the invoke, so those shells would otherwise be listed as top-level panes beside the Workspace about to mount them. -- **`begin_arrival` records the arrival in `sessions/arrivals.json`** — a JSON - array of `{ workspaceId, from, to, workspace, settled }`, never an entry in - either window's snapshot (rationale); the tombstone rules below read `settled`. **Must retain an adopted record until target +- **`begin_arrival` records the arrival in `sessions/arrivals.json` before + ownership moves** — a JSON array of `{ workspaceId, from, to, workspace, + settled }`, never an entry in either window's snapshot (rationale). **A failed + write must refuse the move with nothing changed**; the write runs outside + `arrivals`, so admission is rechecked after it and a refusal withdraws the + record (`a_failed_arrival_journal_refuses_the_move_with_nothing_changed`); the tombstone rules below read `settled`. **Must retain an adopted record until target and source snapshots both reflect the move**, marking it settled at `adopt_done` and checking after each `save_session` or source-window close (`adoption_keeps_the_journal_until_both_snapshots_are_durable`). **Must reverse the durable destination on hand-back and retain the record until both - snapshots reflect the return** (`a_hand_back_is_recovered_in_the_source_before_its_next_flush`). + snapshots reflect the return** (`a_hand_back_is_recovered_in_the_source_before_its_next_flush`). Failed settlement-marker or hand-back writes are logged and do not block live adoption or return. **Must tombstone settled arrivals into a deliberately closed Window until both snapshots omit them**, including during boot recovery (`closing_an_adopted_target_never_resurrects_either_copy`). @@ -943,7 +952,7 @@ written. - **The label is sanitized** so it cannot escape the directory. - **Temp-then-rename**, so a crash cannot truncate the previous snapshot. The temp file is fsynced before the rename and, on unix only, the sessions directory - *after* it (rationale). + *after* it, best-effort (rationale). - **Window identity is implicit**: each command keys by the invoking `tauri::Window`'s `label()`, so the frontend stays window-agnostic and every window (`ws-2`, …) persists to its own file rather than rewriting a sibling's. @@ -951,8 +960,7 @@ written. blob (rationale). - **The writer removes its own temp file on every error path**, so only a crash can leave one behind. -- **A per-window close removes the blob, its temp sibling and its geometry** - (§Per-window close); nothing else deletes a snapshot but the boot merge +- Per-window cleanup follows §Per-window close; only the boot merge otherwise deletes a snapshot (§Arrival queue). - **Must sweep orphan session temp files once at boot** in the active sessions directory and, for debug builds, the legacy `/sessions` directory. @@ -975,7 +983,7 @@ owner-only first and each an empty string when it could not be: `DORMOUSE_STATE_DIR` (the Burrow store, `app_data_dir`) and `DORMOUSE_RECOVERY_DIR` (the recovery record, the state root — so a dev run's record cannot reach the installed app). The browser-dev harness sets both to its -own per-run temp directory. Source of truth: `recovery_state_dir` in +own per-run temp directory. Source of truth: `prepare_owner_only_dir` / `recovery_state_dir` in `standalone/src-tauri/src/lib.rs`. **Must restrict the session store to the owner before any bytes are written** @@ -985,7 +993,7 @@ own per-run temp directory. Source of truth: `recovery_state_dir` in silent no-op, it applies a protected single-entry DACL instead (mechanism in its doc comment). `burrow_state_dir` locks the sidecar's state directory with the same call and relies on it reaching a file that already *existed*, which -`restrict_to_owner_leaves_one_owner_only_ace` pins (rationale). **Must abort a snapshot save if either permission change fails**, preserving the previous snapshot. The state-directory call remains nonfatal and logs a `WARNING` naming the path. Pinned by `session_permission_failures_preserve_previous_snapshot_without_writing_bytes` and `session_write_tightens_directory_and_existing_temp_file`. +`restrict_to_owner_leaves_one_owner_only_ace` pins (rationale). **Must abort a snapshot save if either permission change fails**, preserving the previous snapshot. **Must withhold a state directory whose restriction fails**, logging a `WARNING` naming the path; the Burrow store and recovery record then stay in memory (`burrow_directory_permission_failure_disables_durable_state`). Pinned by `session_permission_failures_preserve_previous_snapshot_without_writing_bytes` and `session_write_tightens_directory_and_existing_temp_file`. **Boot + the synchronous-read constraint.** `getState()` is synchronous — cold-start restore reads it before React mounts — but a Tauri `invoke` is async, so @@ -1140,7 +1148,7 @@ asks before discarding a pending download (§Per-window close). `deferred_quit_and_close_requests_wait_for_membership_then_run_once` in `standalone/src-tauri/src/quit_state.rs`, and `transfers_cannot_change_membership_after_close_or_quit_confirmation_begins` and - `begin_arrival_admits_under_the_arrivals_lock_before_queueing` in + `begin_arrival_journals_then_admits_under_the_arrivals_lock_before_queueing` in `standalone/src-tauri/src/lib.rs`. - **Must collect votes before killing any window's Sessions.** Confirmation consumes its callback once; a noninteractive full-window progress overlay diff --git a/docs/specs/standalone.rationale.md b/docs/specs/standalone.rationale.md index a22e726dd..3132c8ba0 100644 --- a/docs/specs/standalone.rationale.md +++ b/docs/specs/standalone.rationale.md @@ -222,7 +222,7 @@ stays the webview's throughout and no polling loop is needed. **The WKWebView WAL measurement.** WKWebView stores `localStorage` as SQLite in WAL mode, and WebKit pins that WAL with a long-lived reader that never advances during a running session — so it is never checkpointed, and an external checkpoint is blocked by the same reader. Rewriting the multi-MB scrollback-bearing session blob on every save grew the WAL to ~1 GB within a few hours (recorded 2026-07); a days-long session made it pathological. The Rust file store that replaced it has no WAL and rewrites the same file each time. -**Why the sessions directory is fsynced after the rename.** Fsyncing only the temp file leaves the new name recoverable-but-absent after a power loss; the directory-entry fsync is what makes the rename itself durable. Windows has no equivalent concept, hence unix-only. +**Why the sessions directory is fsynced after the rename.** Fsyncing only the temp file leaves the new name recoverable-but-absent after a power loss; a successful directory-entry fsync makes the rename durable. Its failure is ignored, so this step is best-effort. Windows has no equivalent concept, hence unix-only. **Why the mode is set before the bytes.** Under the bare umask the transcript-bearing blob lands `0644` in a `0755` directory any other local account can read, and tightening after the write would leave a window in which it was readable. Continuing after a permission failure would contradict the owner-only guarantee; aborting before writing preserves the previous snapshot and leaves at most an empty temp file. diff --git a/lib/src/host/remote/burrow-state-store.ts b/lib/src/host/remote/burrow-state-store.ts index 7b4438a43..41d5137f4 100644 --- a/lib/src/host/remote/burrow-state-store.ts +++ b/lib/src/host/remote/burrow-state-store.ts @@ -8,7 +8,8 @@ * The interface is async because the hosts that implement it are: files the * sidecar owns here, `VsCodeBurrowStateStore` there (enrollment in * `SecretStorage`, ACL in `globalState` — `docs/specs/vscode.md`). {@link FileBurrowStateStore} - * is the sidecar's: two files, 0600, under a directory the app passes in. + * is the sidecar's: private JSON state under a directory the app passes in + * only after establishing owner-only access (POSIX modes or a Windows DACL). */ import { readFile, rm } from 'node:fs/promises'; diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index 19f48d5fe..d00c4cefa 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -30,7 +30,7 @@ "docs/specs/security-supply-chain.md": 1250, "docs/specs/security.md": 2150, "docs/specs/shortcuts.md": 1100, - "docs/specs/standalone.md": 11950, + "docs/specs/standalone.md": 12050, "docs/specs/terminal-context.md": 1100, "docs/specs/terminal-escapes.md": 4050, "docs/specs/terminal-state.md": 2400, diff --git a/standalone/scripts/dev-agent-browser.test.mjs b/standalone/scripts/dev-agent-browser.test.mjs index bc7916c17..ddac0e687 100644 --- a/standalone/scripts/dev-agent-browser.test.mjs +++ b/standalone/scripts/dev-agent-browser.test.mjs @@ -2,7 +2,7 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import { access, copyFile, mkdir, readFile, rm, writeFile } from 'node:fs/promises'; import path from 'node:path'; -import { fileURLToPath } from 'node:url'; +import { fileURLToPath, pathToFileURL } from 'node:url'; import { spawn } from 'node:child_process'; import { get } from 'node:http'; import { setTimeout as delay } from 'node:timers/promises'; @@ -30,6 +30,10 @@ async function fixture(t) { }); `); const cli = path.join(bin, 'cli.cjs'); + // Windows kill('SIGTERM') bypasses JS handlers. Exercise the same shutdown + // handler over IPC there; POSIX continues exercising the actual signal. + const signals = path.join(bin, 'signals.mjs'); + await writeFile(signals, "process.on('message', signal => process.emit(signal));"); await writeFile(cli, ` if (process.argv[2] === 'list' && process.env.TEST_DOR_LIST) { console.log(process.env.TEST_DOR_LIST); @@ -54,8 +58,8 @@ async function fixture(t) { return { root, start(overrides = {}) { - const child = spawn(process.execPath, [path.join(standalone, 'scripts/dev-agent-browser.mjs')], { - cwd: root, env: { ...cleanEnv(bin), ...overrides }, stdio: ['ignore', 'pipe', 'pipe'], + const child = spawn(process.execPath, ['--import', pathToFileURL(signals).href, path.join(standalone, 'scripts/dev-agent-browser.mjs')], { + cwd: root, env: { ...cleanEnv(bin), ...overrides }, stdio: ['ignore', 'pipe', 'pipe', 'ipc'], }); // Object.assign, not a spread: `runner`'s `output`/`closed` are getters // over live state, and spreading would snapshot them once. @@ -73,7 +77,10 @@ async function fixture(t) { return this; }, async stop() { - if (child.exitCode === null && child.signalCode === null) child.kill('SIGTERM'); + if (child.exitCode === null && child.signalCode === null) { + if (process.platform === 'win32' && child.connected) child.send('SIGTERM', () => {}); + else child.kill('SIGTERM'); + } const timer = setTimeout(() => child.kill('SIGKILL'), 5000); try { return await this.exited; } finally { clearTimeout(timer); } }, diff --git a/standalone/scripts/dev-standalone.test.mjs b/standalone/scripts/dev-standalone.test.mjs index 4894deb2e..039a7c07a 100644 --- a/standalone/scripts/dev-standalone.test.mjs +++ b/standalone/scripts/dev-standalone.test.mjs @@ -2,7 +2,7 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import { copyFile, mkdir, rm, writeFile } from 'node:fs/promises'; import path from 'node:path'; -import { fileURLToPath } from 'node:url'; +import { fileURLToPath, pathToFileURL } from 'node:url'; import { spawn } from 'node:child_process'; import { setTimeout as delay } from 'node:timers/promises'; import { cleanEnv, devWorkspace, runner, writeShims } from './dev-fixture.mjs'; @@ -54,7 +54,7 @@ async function fixture(t) { return { root, start(args = ['dev'], overrides = {}) { - const child = spawn(process.execPath, ['--import', signals, path.join(standalone, 'scripts/tauri.mjs'), ...args], { + const child = spawn(process.execPath, ['--import', pathToFileURL(signals).href, path.join(standalone, 'scripts/tauri.mjs'), ...args], { cwd: standalone, env: { ...cleanEnv(bin), ...overrides }, stdio: ['ignore', 'pipe', 'pipe', 'ipc'], }); // Object.assign, not a spread: `runner`'s `output`/`closed` are getters diff --git a/standalone/src-tauri/src/lib.rs b/standalone/src-tauri/src/lib.rs index 8fda5925c..2cdc4d1e2 100644 --- a/standalone/src-tauri/src/lib.rs +++ b/standalone/src-tauri/src/lib.rs @@ -139,8 +139,9 @@ struct WindowState { /// one can be told to clear it. hover_target: Mutex>, /// Labels whose snapshot has been deliberately removed. A save arriving - /// from a webview that is going away must not put the file back; the entry - /// is dropped once that webview is destroyed and can no longer save. + /// from a webview that is going away must not put the file back. Kept for + /// the process lifetime: a save dispatched before `Destroyed` can still + /// reach the disk lock after it, and labels are never reused in a process. closing: Mutex>, /// The next `ws-`, seeded above every live and saved label at setup. next_ws: AtomicU64, @@ -181,8 +182,8 @@ impl WindowState { .store(routing.awaiting_replay.len(), Ordering::Relaxed); } - /// Refuse every later `save_session` for `label` (a deliberate close removed - /// its snapshot). Cleared by `Destroyed`, after which no save can arrive. + /// Refuse every later `save_session` and geometry write for `label` (a + /// deliberate close removed its snapshot), for the rest of the process. fn begin_closing(&self, label: &str) { guard(&self.closing).insert(label.to_string()); } @@ -2165,6 +2166,16 @@ fn geometry_path(dir: &Path, label: &str) -> PathBuf { dir.join(format!("{stem}.geometry.json")) } +/// The window and rect were read before this lock, so a close may have removed +/// the geometry since: recheck the save refusal under the lock that removal +/// holds, or a debounce captured before the close writes the file back. +fn write_open_window_geometry(windows: Option<&WindowState>, dir: &Path, label: &str, json: &str) -> Result { + let _disk = guard(&ARRIVAL_DISK_LOCK); + if windows.is_some_and(|windows| windows.refuses_save(label)) { return Ok(false); } + write_file_atomically(&geometry_path(dir, label), json)?; + Ok(true) +} + fn read_geometry(dir: &Path, label: &str) -> Option { let raw = std::fs::read_to_string(geometry_path(dir, label)).ok()?; serde_json::from_str(&raw).ok() @@ -2245,7 +2256,8 @@ fn note_geometry(app: &AppHandle, label: &str, origin: Option<(i32, i32)>, size: let Ok(json) = serde_json::to_string(&rect.to_logical()) else { continue; }; - if let Err(err) = write_file_atomically(&geometry_path(&dir, &label), &json) { + let windows = app.try_state::(); + if let Err(err) = write_open_window_geometry(windows.as_deref(), &dir, &label, &json) { append_log(format!("[window] geometry write for {label}: {err}")); } } @@ -2492,20 +2504,35 @@ fn record_workspace_id(record: &JsonValue) -> Option<&str> { record.get("workspaceId").and_then(JsonValue::as_str) } -/// Append one arrival's record, replacing any earlier record of the same id. -fn record_arrival_on_disk(dir: &Path, arrival: &routing::Arrival) -> Result<(), String> { +/// Append one arrival's record, replacing and returning any earlier record of +/// the same id. +fn record_arrival_on_disk(dir: &Path, arrival: &routing::Arrival) -> Result, String> { let _disk = guard(&ARRIVAL_DISK_LOCK); let Some(workspace) = arrival.payload.get("workspace") else { return Err("arrival payload carries no workspace".to_string()); }; let mut records = read_arrivals_from(dir)?; - records.retain(|record| record_workspace_id(record) != Some(&arrival.workspace_id)); + let previous = records.iter().position(|record| record_workspace_id(record) == Some(&arrival.workspace_id)) + .map(|at| records.remove(at)); records.push(serde_json::json!({ "workspaceId": arrival.workspace_id, "from": arrival.from, "to": arrival.to, "workspace": workspace, })); + write_arrivals_to(dir, &records)?; + Ok(previous) +} + +/// Undo `record_arrival_on_disk` for an arrival refused after its write, +/// unless a later drop of the same id has since replaced the record. +fn withdraw_arrival_on_disk(dir: &Path, arrival: &routing::Arrival, previous: Option) -> Result<(), String> { + let _disk = guard(&ARRIVAL_DISK_LOCK); + let mut records = read_arrivals_from(dir)?; + let ours = |r: &JsonValue| record_workspace_id(r) == Some(&arrival.workspace_id) + && r["from"] == arrival.from.as_str() && r["to"] == arrival.to.as_str() && r.get("settled").is_none(); + let Some(at) = records.iter().position(ours) else { return Ok(()); }; + match previous { Some(record) => records[at] = record, None => { records.remove(at); } } write_arrivals_to(dir, &records) } @@ -2676,8 +2703,65 @@ fn transfer_admitted( && [from, to].into_iter().all(|label| !close.active(label) && !closing.contains(label)) } -/// Open one arrival: reassign its shells to the target and suppress them, then -/// queue the record. **Ownership moves synchronously here**, before either +/// Whether `arrival` may begin: no quit or close of either end (see +/// `transfer_admitted`) and its Workspace not already in flight. +fn admit_arrival( + quit: Option<&QuitState>, + windows: &WindowState, + arrivals: &ArrivalQueue, + arrival: &routing::Arrival, +) -> Result<(), String> { + if let Some(state) = quit { + // Lock order: arrivals, quit machine, close machine, closing. + let admitted = transfer_admitted( + arrivals, + &guard(&state.machine), + &guard(&state.close), + &guard(&windows.closing), + &arrival.from, + &arrival.to, + ); + if !admitted { + return Err("cannot transfer a Workspace while its window is closing or Dormouse is quitting".to_string()); + } + } + if routing::has_arrival(arrivals, &arrival.workspace_id) { + return Err(format!("Workspace '{}' is already in flight", arrival.workspace_id)); + } + Ok(()) +} + +/// Journal one arrival, then reassign its shells to the target and queue it. +/// **The journal comes first**: the source omits a transferring Workspace +/// from its saves and the target writes only after adoption, so a crash in +/// the gap is recovered from this record alone (§Arrival queue). A failed +/// write refuses the move with nothing changed. The write runs outside +/// `arrivals`, whose waits reach the main thread, so admission is checked +/// again under it and a refusal then withdraws the record. +fn journal_and_open_arrival( + quit: Option<&QuitState>, + windows: &WindowState, + dir: &Path, + arrival: &routing::Arrival, +) -> Result<(), String> { + admit_arrival(quit, windows, &guard(&windows.arrivals), arrival)?; + let previous = record_arrival_on_disk(dir, arrival) + .map_err(|e| format!("could not record the Workspace transfer: {e}"))?; + let mut arrivals = guard(&windows.arrivals); + if let Err(refused) = admit_arrival(quit, windows, &arrivals, arrival) { + drop(arrivals); + if let Err(e) = withdraw_arrival_on_disk(dir, arrival, previous) { + append_log(format!("[window] could not withdraw {}'s record: {e}", arrival.workspace_id)); + } + return Err(refused); + } + // Ownership moves now; suppression waits for each id's `marked` line. + windows.begin_transfer(&arrival.terminal_ids, &arrival.from, &arrival.to); + routing::queue_arrival(&mut arrivals, arrival.clone()); + Ok(()) +} + +/// Open one arrival. **Ownership moves synchronously here**, before either /// window is told anything — the single Rust reader thread processes sidecar /// lines in order, so every byte after this point is either dropped (and present /// in the replay the target is about to get) or delivered to the target @@ -2687,32 +2771,8 @@ fn begin_arrival( windows: &WindowState, arrival: routing::Arrival, ) -> Result<(), String> { - { - let mut arrivals = guard(&windows.arrivals); - if let Some(state) = app.try_state::() { - // Lock order: arrivals, quit machine, close machine, closing. - let admitted = transfer_admitted( - &arrivals, - &guard(&state.machine), - &guard(&state.close), - &guard(&windows.closing), - &arrival.from, - &arrival.to, - ); - if !admitted { - return Err("cannot transfer a Workspace while its window is closing or Dormouse is quitting".to_string()); - } - } - if routing::has_arrival(&arrivals, &arrival.workspace_id) { - return Err(format!( - "Workspace '{}' is already in flight", - arrival.workspace_id - )); - } - // Ownership moves now; suppression waits for each id's `marked` line. - windows.begin_transfer(&arrival.terminal_ids, &arrival.from, &arrival.to); - routing::queue_arrival(&mut arrivals, arrival.clone()); - } + let quit = app.try_state::(); + journal_and_open_arrival(quit.as_deref(), windows, &sessions_dir(app)?, &arrival)?; // The split point, stamped in the stream by the sidecar and routed to the // source, which serializes what it holds when it sees it (§Transfer). A // Workspace of browser panes alone has no ids to mark; its source sends @@ -2724,19 +2784,6 @@ fn begin_arrival( }); send_to_sidecar(&sidecar, msg.to_string()); } - // Recorded on disk here, in neither window's snapshot: the source omits a - // transferring Workspace from its saves and the target writes only after - // adoption, so a crash in the gap would otherwise restore it nowhere - // (§Arrival queue). Never fatal: a failed write is logged and the transfer - // proceeds. - match sessions_dir(app) { - Ok(dir) => { - if let Err(e) = record_arrival_on_disk(&dir, &arrival) { - append_log(format!("[window] could not record {} on disk: {e}", arrival.workspace_id)); - } - } - Err(e) => append_log(format!("[window] {e}")), - } spawn_arrival_watchdog(app.clone(), &arrival); Ok(()) } @@ -3610,8 +3657,8 @@ fn resolve_dor_cli_paths(sidecar_path: &Path, manifest_dir: &Path) -> DorCliPath // Where the sidecar's Burrow persists its enrollment (a bearer credential) // and its ACL, as one 0600 file it writes itself // (lib/src/host/remote/burrow-state-store.ts). Created here so a first launch -// hands the sidecar a directory that exists; if it can't be made, the sidecar is -// told nothing and runs without persistence rather than not at all. +// hands the sidecar a directory that exists; if it can't be made or locked, the +// sidecar is told nothing and keeps that state in memory rather than not at all. fn burrow_state_dir(app: &AppHandle) -> Option { let dir = match app.path().app_data_dir() { Ok(dir) => dir, @@ -3620,10 +3667,6 @@ fn burrow_state_dir(app: &AppHandle) -> Option { return None; } }; - if let Err(e) = create_dir_all(&dir) { - append_log(format!("[sidecar] create state dir: {e}")); - return None; - } // The Node sidecar writes the Burrow enrollment here, and that record carries // `burrowToken` — a bearer credential for `/ws/burrow`. `FileBurrowStateStore` // asks for `0700`/`0600`, which Windows ignores entirely, so on Windows this @@ -3634,24 +3677,33 @@ fn burrow_state_dir(app: &AppHandle) -> Option { // `restrict_to_owner_leaves_one_owner_only_ace` covers with `before.json`. // On unix the store's own modes already do the job and this is a harmless // re-assert of the same intent. - if let Err(e) = restrict_to_owner(&dir, 0o700) { - // Not fatal — a Burrow that cannot start is worse than one whose state - // directory kept the OS default — but never silent: on Windows this - // call is the only thing restricting `burrowToken`, so its failure is a - // downgrade of the sole control and has to be visible. - append_log(format!( - "[sidecar] WARNING could not restrict state dir {}: {e}", - dir.display() - )); + match prepare_owner_only_dir(&dir, restrict_to_owner) { + Ok(path) => Some(path), + Err(e) => { + append_log(format!("[sidecar] WARNING {e}; Burrow state stays in memory")); + None + } } - Some(dir.to_string_lossy().into_owned()) +} + +/// Create `dir` and lock it owner-only, publishing its path only once both +/// succeeded. A refused restriction publishes nothing: on Windows the Node +/// stores' modes are no-ops, so an unlocked directory would hold their +/// credentials and commands under the inherited ACL. +fn prepare_owner_only_dir( + dir: &Path, + restrict: impl FnOnce(&Path, u32) -> Result<(), String>, +) -> Result { + create_dir_all(dir).map_err(|e| format!("create state dir: {e}"))?; + restrict(dir, 0o700).map_err(|e| format!("could not restrict state dir {}: {e}", dir.display()))?; + Ok(dir.to_string_lossy().into_owned()) } /// Where the sidecar writes the single-use agent-recovery record. Under the -/// state root, so a dev run never consumes the installed app's. Created here so -/// a first launch hands the sidecar a directory that exists; owner-only for the -/// same reason the Burrow's is — the record holds command lines the user typed, -/// and a unix mode is a silent no-op on Windows. +/// state root, so a dev run never consumes the installed app's. Owner-only for +/// the same reason the Burrow's is — the record holds command lines the user +/// typed, and a unix mode is a silent no-op on Windows — so a directory that +/// cannot be locked is withheld and the record stays in memory. fn recovery_state_dir(app: &AppHandle) -> Option { let dir = match state_root(app) { Ok(dir) => dir, @@ -3660,17 +3712,13 @@ fn recovery_state_dir(app: &AppHandle) -> Option { return None; } }; - if let Err(e) = create_dir_all(&dir) { - append_log(format!("[recovery] create state dir: {e}")); - return None; - } - if let Err(e) = restrict_to_owner(&dir, 0o700) { - append_log(format!( - "[recovery] WARNING could not restrict state dir {}: {e}", - dir.display() - )); + match prepare_owner_only_dir(&dir, restrict_to_owner) { + Ok(path) => Some(path), + Err(e) => { + append_log(format!("[recovery] WARNING {e}; recovery stays in memory")); + None + } } - Some(dir.to_string_lossy().into_owned()) } fn start_sidecar(app: &AppHandle) -> Result { @@ -3996,7 +4044,6 @@ pub fn run() { // Drop label-keyed ownership synchronously; only the // returned arrivals need the blocking journal worker. let (lost, orphaned) = state.drop_window(&label); - guard(&state.closing).remove(&label); reap_orphaned_ptys(app, &label, orphaned); let changed = workspaces::forget_window(&mut guard(&state.registry), &label); if changed { broadcast_registry(app, &state); } @@ -4295,6 +4342,53 @@ mod tests { } } + #[test] + fn state_directory_creation_failure_never_attempts_permissions() { + let root = TempDir::new("state-dir-create-failure"); + let occupied = root.path().join("not-a-directory"); + fs::write(&occupied, b"previous").unwrap(); + let called = std::cell::Cell::new(false); + let result = super::prepare_owner_only_dir(&occupied, |_, _| { + called.set(true); + Ok(()) + }); + assert!(result.is_err()); + assert!(!called.get()); + assert_eq!(fs::read(occupied).unwrap(), b"previous"); + } + + #[test] + fn burrow_directory_permission_failure_disables_durable_state() { + let root = TempDir::new("burrow-state-restrict-failure"); + let target = root.path().join("state"); + fs::create_dir(&target).unwrap(); + fs::write(target.join("burrow.json"), b"existing enrollment").unwrap(); + let result = super::prepare_owner_only_dir(&target, |path, mode| { + assert_eq!(path, target); + assert_eq!(mode, 0o700); + Err("DACL refused".into()) + }); + assert!(result.unwrap_err().contains("DACL refused")); + assert_eq!(fs::read(target.join("burrow.json")).unwrap(), b"existing enrollment"); + assert_eq!(fs::read_dir(target).unwrap().count(), 1); + } + + #[test] + fn state_directory_is_created_and_restricted_before_publication() { + let root = TempDir::new("state-dir-order"); + let target = root.path().join("nested").join("state"); + let called = std::cell::Cell::new(false); + let result = super::prepare_owner_only_dir(&target, |path, mode| { + assert!(path.is_dir()); + assert_eq!(fs::read_dir(path).unwrap().count(), 0); + assert_eq!(mode, 0o700); + called.set(true); + Ok(()) + }); + assert!(called.get()); + assert_eq!(result.unwrap(), target.to_string_lossy()); + } + // --- Pending arrivals on disk (§Arrival queue) --------------------------- fn workspace_json(id: &str, name: &str) -> JsonValue { @@ -4667,15 +4761,47 @@ mod tests { } #[test] - fn begin_arrival_admits_under_the_arrivals_lock_before_queueing() { + fn begin_arrival_journals_then_admits_under_the_arrivals_lock_before_queueing() { let src = include_str!("lib.rs").split("#[cfg(test)]").next().unwrap(); - let body = src.split("fn begin_arrival(").nth(1).unwrap().split("\n}").next().unwrap(); + let body = src.split("fn journal_and_open_arrival(").nth(1).unwrap().split("\n}").next().unwrap(); + let journal = body.find("record_arrival_on_disk(").unwrap(); let lock = body.find("let mut arrivals = guard(&windows.arrivals)").unwrap(); - let check = body.find("transfer_admitted(").unwrap(); - let refuse = body.find("if !admitted {").unwrap(); + let check = lock + body[lock..].find("admit_arrival(").unwrap(); + let refuse = body.find("return Err(refused)").unwrap(); + let transfer = body.find("windows.begin_transfer(").unwrap(); let queue = body.find("routing::queue_arrival").unwrap(); - assert!(lock < check && check < refuse && refuse < queue); - assert!(body[refuse..queue].contains("return Err(")); + assert!(journal < lock && lock < check && check < refuse && refuse < transfer && transfer < queue); + } + + #[test] + fn a_failed_arrival_journal_refuses_the_move_with_nothing_changed() { + let dir = TempDir::new("arrival-journal-first"); + let windows = super::WindowState::default(); + let mut arrival = arrival_of("workspace-7", "main", "ws-2"); + arrival.terminal_ids = vec!["pane-a".to_string()]; + windows.mint("pane-a", "main"); + // A directory where the journal belongs fails its read on every platform. + fs::create_dir(arrivals_path(dir.path())).unwrap(); + assert!(super::journal_and_open_arrival(None, &windows, dir.path(), &arrival).is_err()); + assert_eq!(windows.owned_by("main"), vec!["pane-a"]); + assert!(guard(&windows.arrivals).is_empty()); + assert!(guard(&windows.routing).marking.is_empty()); + fs::remove_dir(arrivals_path(dir.path())).unwrap(); + super::journal_and_open_arrival(None, &windows, dir.path(), &arrival).unwrap(); + assert_eq!(windows.owned_by("ws-2"), vec!["pane-a"]); + assert_eq!(guard(&windows.arrivals).len(), 1); + // A refusal after the write puts back the record it replaced, and + // never withdraws a newer drop's record. + let mut later = arrival_of("workspace-7", "ws-2", "ws-3"); + let previous = record_arrival_on_disk(dir.path(), &later).unwrap(); + super::withdraw_arrival_on_disk(dir.path(), &arrival, None).unwrap(); + assert_eq!(read_arrivals_from(dir.path()).unwrap()[0]["to"], "ws-3"); + super::withdraw_arrival_on_disk(dir.path(), &later, previous).unwrap(); + assert_eq!(read_arrivals_from(dir.path()).unwrap()[0]["to"], "ws-2"); + later.to = "ws-4".to_string(); + record_arrival_on_disk(dir.path(), &later).unwrap(); + super::withdraw_arrival_on_disk(dir.path(), &later, None).unwrap(); + assert!(read_arrivals_from(dir.path()).unwrap().is_empty()); } #[test] @@ -5400,16 +5526,37 @@ mod tests { /// paths set it: the webview's own `remove_window_session`, and /// `finish_window_close` for the ack-timeout path where it never ran. #[test] - fn a_closing_window_refuses_every_later_save_until_it_is_destroyed() { + fn a_closed_window_refuses_saves_for_the_process_lifetime() { let state = super::WindowState::default(); assert!(!state.refuses_save("ws-2")); state.begin_closing("ws-2"); assert!(state.refuses_save("ws-2")); // Never a sibling's. assert!(!state.refuses_save("main")); - // `Destroyed` drops the refusal: no save can arrive under a dead label. - guard(&state.closing).remove("ws-2"); - assert!(!state.refuses_save("ws-2")); + // A save dispatched before `Destroyed` can reach the disk lock after + // it, so neither the label sweep nor the arm itself drops the refusal. + state.drop_window("ws-2"); + assert!(state.refuses_save("ws-2")); + let src = include_str!("lib.rs").split("#[cfg(test)]").next().unwrap(); + let destroyed = src.split("WindowEvent::Destroyed => {").nth(1).unwrap() + .split("if let Some(state) = app.try_state::()").next().unwrap(); + assert!(!destroyed.contains(".closing"), "Destroyed must not clear save refusal"); + } + + #[test] + fn a_geometry_flush_captured_before_close_cannot_recreate_removed_geometry() { + let dir = TempDir::new("geometry-close-fence"); + let windows = super::WindowState::default(); + write_session_to(dir.path(), "ws-2", "snapshot").unwrap(); + assert!(super::write_open_window_geometry(Some(&windows), dir.path(), "ws-2", "previous").unwrap()); + assert_eq!(fs::read_to_string(super::geometry_path(dir.path(), "ws-2")).unwrap(), "previous"); + windows.begin_closing("ws-2"); + super::close_window_snapshot(dir.path(), "ws-2").unwrap(); + assert!(!super::write_open_window_geometry(Some(&windows), dir.path(), "ws-2", "captured before close").unwrap()); + assert!(!super::geometry_path(dir.path(), "ws-2").exists()); + windows.drop_window("ws-2"); + assert!(!super::write_open_window_geometry(Some(&windows), dir.path(), "ws-2", "captured before close").unwrap()); + assert!(!super::geometry_path(dir.path(), "ws-2").exists()); } fn queue_test_suppression(state: &super::WindowState, id: &str) { diff --git a/standalone/src/updater.test.ts b/standalone/src/updater.test.ts index 05571c864..1ee5a6e24 100644 --- a/standalone/src/updater.test.ts +++ b/standalone/src/updater.test.ts @@ -121,6 +121,53 @@ describe('updater', () => { mocks.platform = { requestAppRestart: mocks.requestAppRestart, burrow: { command: mocks.burrowCommand } }; }); + it('does not reoffer an approved download when the delayed launch check begins', async () => { + mocks.check.mockResolvedValue(makeUpdate('0.5.0')); + startUpdateCheck(); + await vi.advanceTimersByTimeAsync(0); + checkNow(); + await vi.advanceTimersByTimeAsync(0); + approveUpdate(); + await vi.advanceTimersByTimeAsync(0); + expect(hasPendingUpdate()).toBe(true); + + await vi.advanceTimersByTimeAsync(5_000); + expect(mocks.check).toHaveBeenCalledOnce(); + expect(readBannerState()).toEqual({ status: 'downloaded', version: '0.5.0' }); + }); + + it('does not reoffer an approval while its download is pending at the launch check', async () => { + const update = makeUpdate('0.5.0'); + update.download.mockImplementation(() => new Promise(() => {})); + mocks.check.mockResolvedValue(update); + startUpdateCheck(); + await vi.advanceTimersByTimeAsync(0); + checkNow(); + await vi.advanceTimersByTimeAsync(0); + approveUpdate(); + await vi.advanceTimersByTimeAsync(5_000); + + expect(mocks.check).toHaveBeenCalledOnce(); + expect(readBannerState()).toEqual({ status: 'downloading', version: '0.5.0' }); + }); + + it('keeps an approval made while the delayed launch policy read is pending', async () => { + let answerPolicy!: (value: typeof CHECKS_ON) => void; + mocks.burrowCommand.mockImplementation(() => new Promise(resolve => { answerPolicy = resolve; })); + mocks.check.mockResolvedValue(makeUpdate('0.5.0')); + startUpdateCheck(); + await vi.advanceTimersByTimeAsync(5_000); + checkNow(); + await vi.advanceTimersByTimeAsync(0); + approveUpdate(); + await vi.advanceTimersByTimeAsync(0); + answerPolicy(CHECKS_ON); + await vi.advanceTimersByTimeAsync(0); + + expect(mocks.check).toHaveBeenCalledOnce(); + expect(readBannerState()).toEqual({ status: 'downloaded', version: '0.5.0' }); + }); + // Drive check → approve → download so an approved, downloaded update is pending. async function reachDownloadedUpdate(update: ReturnType) { mocks.check.mockResolvedValue(update); diff --git a/standalone/src/updater.ts b/standalone/src/updater.ts index 4c6c91715..25043ab1a 100644 --- a/standalone/src/updater.ts +++ b/standalone/src/updater.ts @@ -414,7 +414,10 @@ async function runUpdateCheck(): Promise { // Read at the check, so a change made meanwhile counts // (`docs/specs/remote-network.md` → "Updates"). const policy = await readNetworkPolicy(); - if (policy && checksForUpdates(policy)) { + // A manual approval during the delay or the policy read owns this session's + // update; checking again would offer it for approval a second time. + const approved = pendingUpdate !== null || downloadPromise !== null; + if (policy && checksForUpdates(policy) && !approved) { // An update found is offered by `performCheck`. await performCheck().catch((e) => console.error('[updater] Check failed:', e)); }