From 4573922c58dcb9b5d73dd6a1d07f7cb625d428cf Mon Sep 17 00:00:00 2001 From: dormouse-bot <287024035+dormouse-bot@users.noreply.github.com> Date: Sat, 31 Oct 2026 13:01:24 +0000 Subject: [PATCH 1/2] fix: say an unsaved VS Code revocation lasts until the broker window closes VS Code runs BurrowService only in the broker window, so a revocation whose ACL save failed is lost when that window closes and the next broker loads the stored ACL. The error and the spec said it lasted until Dormouse quits, which holds only for standalone. Refs #1484 --- docs/specs/remote-security-model.md | 2 +- docs/specs/remote-security-model.rationale.md | 2 +- lib/src/host/remote/service.test.ts | 14 +++++++++++++- lib/src/host/remote/service.ts | 7 +++++-- 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/docs/specs/remote-security-model.md b/docs/specs/remote-security-model.md index 5eb932edf..b8e3b28e9 100644 --- a/docs/specs/remote-security-model.md +++ b/docs/specs/remote-security-model.md @@ -154,7 +154,7 @@ Source of truth: `OneTimeRuntime` in `lib/src/remote/burrow/one-time-runtime.ts` Reserved: a Relay- or Pocket-initiated revocation calls this entry point with a requester of its own (`remote-trust-followups` -> [Revocation propagation](#revocation-propagation)). -- **A running Burrow's revocation takes effect before its save**: the live ACL drops the records, every established session no active record still authorizes ends with the goodbye, every pending connection whose Client static it left unpaired is refused `pairing-required`, and those statics' [presence windows](#presence-window) close. It queues with approvals, so neither overwrites the other's snapshot. A failed save rejects with the phone already refused, and the service applies the revocation to every ACL it reads for that Burrow, so a network-policy change that restarts the runtime keeps it; the next ACL write carries it, and quitting Dormouse before one undoes it (rationale). +- **A running Burrow's revocation takes effect before its save**: the live ACL drops the records, every established session no active record still authorizes ends with the goodbye, every pending connection whose Client static it left unpaired is refused `pairing-required`, and those statics' [presence windows](#presence-window) close. It queues with approvals, so neither overwrites the other's snapshot. A failed save rejects with the phone already refused, and the service applies the revocation to every ACL it reads for that Burrow, so a network-policy change that restarts the runtime keeps it; the next ACL write carries it, and the service stopping before one undoes it — Dormouse quitting, or in VS Code the broker window closing (`docs/specs/vscode.md` -> "Which window: bind-as-lease"), since the next broker loads the stored ACL (rationale). - **A stopped Burrow's revocation is its save**, on the service's lifecycle chain, so a Burrow starting meanwhile loads the ACL after it. - **A paired browser is named by `deviceIdFor`**: the first 64 bits of its Client static in hex, a name that authorizes nothing and the only identifier of a record that reaches a webview or `dor` (rationale). **An id that matches more than one Client static revokes nothing.** diff --git a/docs/specs/remote-security-model.rationale.md b/docs/specs/remote-security-model.rationale.md index d537e4db8..1bfb5ca8d 100644 --- a/docs/specs/remote-security-model.rationale.md +++ b/docs/specs/remote-security-model.rationale.md @@ -82,7 +82,7 @@ The self-host operator explicitly accepted the fallback tradeoff: same-origin ma ## Revocation -**Why a revocation acts before it saves, when an approval saves first.** The two fail in opposite directions. An approval that published before a failed save would grant access nothing durable records; a revocation that waited for its save would leave a phone the person just cut off connected for as long as the disk refuses. Acting first fails closed, and the cost of a failed save is a phone that comes back once Dormouse quits, which the error says. The service holds the revocation over every runtime it starts meanwhile, since a network-policy change restarts one from the store. +**Why a revocation acts before it saves, when an approval saves first.** The two fail in opposite directions. An approval that published before a failed save would grant access nothing durable records; a revocation that waited for its save would leave a phone the person just cut off connected for as long as the disk refuses. Acting first fails closed, and the cost of a failed save is a phone that comes back once the service stops — Dormouse quitting, or in VS Code the broker window closing — which the error says. The service holds the revocation over every runtime it starts meanwhile, since a network-policy change restarts one from the store. **Why a device id is 64 bits of the static, in hex.** The static is a public key and authorizes nothing, but it is long and stable, so it stays in the service; a prefix is short enough to type into `dor`. A paired phone chooses its own static and could grind one sharing a shorter prefix with another phone's, so that the person revoking one cuts off the other; 64 bits puts that out of reach, and the ambiguity refusal covers what is left. Hex never begins with the `-` a command line reads as a flag, which base64url can. diff --git a/lib/src/host/remote/service.test.ts b/lib/src/host/remote/service.test.ts index 6619e1607..bd4bd41bc 100644 --- a/lib/src/host/remote/service.test.ts +++ b/lib/src/host/remote/service.test.ts @@ -1790,13 +1790,25 @@ describe('paired devices and revocation', () => { expect((await command('status')).result).toMatchObject({ pairedClients: 0 }); }); + it('names Dormouse quitting as the end of an unsaved revocation in standalone', async () => { + createService({ enrollment: ENROLLMENT, acl: { [BURROW_ID]: [PHONE] } }, { kind: 'standalone' }); + await service.start(); + sockets[0]!.open(); + vi.spyOn(store, 'saveAcl').mockRejectedValueOnce(new Error('disk full')); + + expect((await command('revokeDevice', { id: phoneId })).error).toMatch(/refused until Dormouse quits.*disk full/); + }); + it('keeps a revocation whose save failed across a network-policy restart, until a save lands', async () => { createService({ enrollment: ENROLLMENT, acl: { [BURROW_ID]: [PHONE, TABLET] } }); await service.start(); sockets[0]!.open(); const save = vi.spyOn(store, 'saveAcl').mockRejectedValueOnce(new Error('disk full')); - expect((await command('revokeDevice', { id: phoneId })).error).toMatch(/refused until Dormouse quits.*disk full/); + // VS Code's next broker window loads the stored ACL, so the hold ends with this window. + expect((await command('revokeDevice', { id: phoneId })).error).toMatch( + /refused until the window running remote control closes.*disk full/, + ); expect(store.acl[BURROW_ID]!.every((record) => record.revokedAt === null)).toBe(true); // The hold covers every record of the target approved by the failure — one // an approval queued ahead of the revocation committed included — and none diff --git a/lib/src/host/remote/service.ts b/lib/src/host/remote/service.ts index a2a55073e..5bb1d68cf 100644 --- a/lib/src/host/remote/service.ts +++ b/lib/src/host/remote/service.ts @@ -534,7 +534,9 @@ export class BurrowService { * the store has them applied ({@link #loadAcl}) — a network-policy change * restarts the runtime from the store, and must not re-admit a phone the * person just cut off — until an ACL save for that Burrow succeeds, which - * carries them. Memory only: quitting Dormouse first undoes them. + * carries them. Memory only: stopping this service first undoes them — quitting + * Dormouse, or in VS Code closing the broker window, whose successor loads + * the stored ACL. */ readonly #unsavedRevocations = new Map>(); @@ -1195,8 +1197,9 @@ export class BurrowService { const held = this.#unsavedRevocations.get(enrollment.burrowId) ?? []; this.#unsavedRevocations.set(enrollment.burrowId, [...held, { target, at: this.#now() }]); const detail = error instanceof Error ? error.message : String(error); + const until = this.#kind === 'vscode' ? 'the window running remote control closes' : 'Dormouse quits'; throw new Error( - `The phone is disconnected and refused until Dormouse quits, but the revocation could not be saved: ${detail}`, + `The phone is disconnected and refused until ${until}, but the revocation could not be saved: ${detail}`, ); } } else { From 7a1c721d43d8418cf645f809489d1bc43080fc73 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 31 Oct 2026 09:25:06 -0700 Subject: [PATCH 2/2] Pin the access-log story's timestamps to the story clock `PairedPhonesAndAccessLog` built its log in a `parameters` literal, which runs at module load, before the meta's `beforeEach` pins `Date.now` to `STORY_NOW`. The log therefore carried the CI runner's wall-clock time, and every PR showed an Argos diff on `modals-remotecontrolsection--paired-phones-and-access-log`. `pairedDevices` and `accessLogEntries` now take an optional `now`, and the story passes `STORY_NOW`; `OneTimeEndedNetworkNotAllowed`'s refusal time gets the same treatment. Co-Authored-By: Claude Opus 5.5 --- lib/src/host/remote/test-burrow-link.ts | 11 +++++++---- lib/src/stories/RemoteControlSection.stories.tsx | 8 +++++--- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/src/host/remote/test-burrow-link.ts b/lib/src/host/remote/test-burrow-link.ts index a916957b2..cada13061 100644 --- a/lib/src/host/remote/test-burrow-link.ts +++ b/lib/src/host/remote/test-burrow-link.ts @@ -155,9 +155,10 @@ const HOUR_MS = 60 * 60_000; * first paired an hour ago and connected now; each after it paired a day * before the last and was last seen two hours after pairing. Relative to now, * like {@link setupQrResult}, so "Last seen" reads the same in every story. + * A caller that runs before a story pins the clock (a `parameters` literal is + * built at module load) passes its pinned `now`. */ -export function pairedDevices(count: number): PairedDevice[] { - const now = Date.now(); +export function pairedDevices(count: number, now = Date.now()): PairedDevice[] { return Array.from({ length: count }, (_, index) => { const pairedAt = now - HOUR_MS - index * 24 * HOUR_MS; return { @@ -176,7 +177,10 @@ export function pairedDevices(count: number): PairedDevice[] { * one's pairing, the second's session that went direct and ended, a one-time * connection, an unpaired phone refused, and the first's session open now. */ -export function accessLogEntries(devices: PairedDevice[] = pairedDevices(2)): AccessLogEntry[] { +export function accessLogEntries( + devices: PairedDevice[] = pairedDevices(2), + now = Date.now(), +): AccessLogEntry[] { const entries: AccessLogEntry[] = []; for (const [index, { label: device, id: deviceId, pairedAt, lastSeenAt }] of devices.entries()) { entries.push({ at: pairedAt, event: 'paired', device, deviceId }); @@ -192,7 +196,6 @@ export function accessLogEntries(devices: PairedDevice[] = pairedDevices(2)): Ac { at: lastSeenAt, event: 'ended', device, deviceId, path: 'direct', durationMs: 4 * 60_000 }, ); } - const now = Date.now(); const oneTime = 'Dormouse Pocket (browser)'; entries.push( { at: now - 3 * HOUR_MS, event: 'one-time-connected', device: oneTime, path: 'direct' }, diff --git a/lib/src/stories/RemoteControlSection.stories.tsx b/lib/src/stories/RemoteControlSection.stories.tsx index 8e6dcdd7d..c465d3f92 100644 --- a/lib/src/stories/RemoteControlSection.stories.tsx +++ b/lib/src/stories/RemoteControlSection.stories.tsx @@ -39,7 +39,9 @@ function RemoteControlStory() { } /** The clock every story here reads. A setup code encodes its expiry, so a real - * clock would draw a different QR on every run. */ + * clock would draw a different QR on every run. `beforeEach` pins it only + * around render: a `parameters` literal is built at module load, under the + * real clock, so a timestamp there must use `STORY_NOW` itself. */ const STORY_NOW = Date.UTC(2026, 0, 1); const meta: Meta = { @@ -342,7 +344,7 @@ export const PairedPhonesAndAccessLog: Story = { parameters: { primedBurrow: { status: enrolledStatus({ pairedClients: 2 }), - accessLog: accessLogEntries(pairedDevices(2)), + accessLog: accessLogEntries(pairedDevices(2, STORY_NOW), STORY_NOW), }, docs: { story: { height: '620px' } }, }, @@ -703,7 +705,7 @@ export const OneTimeEndedNetworkNotAllowed: Story = { oneTime: { status: 'ended', reason: 'network-not-allowed', - refusal: { at: Date.now(), kind: 'path-refused', end: 'remote', address: '172.58.12.9', addressSource: 'observed' }, + refusal: { at: STORY_NOW, kind: 'path-refused', end: 'remote', address: '172.58.12.9', addressSource: 'observed' }, }, }, docs: { story: { height: '360px' } },