From e1ff5bb3ff96cc173f0e8c56804118887292e194 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 19 Oct 2026 17:04:48 -0700 Subject: [PATCH] Delete Natural Voice tokens the desktop discards, and never reset the saved voice on a failed read A token an approval delivers but the desktop never keeps (the poll answer lands after a cancel, a Turn off overtakes it, or the save fails) is now deleted at Hosted with the computer's own DELETE /api/voice/token, best effort and never under Nowhere. Turn off reads the token to delete inside the serialized section, so a token it clears mid-save is deleted too. A Turn off whose read failed wrote the defaults back, and standalone's file store rewrites the whole file, so the chosen voice reset. The file store now merges the one changed field into a fresh read (failing rather than resetting), and clearToken never caches defaults it did not read. VS Code's store already writes each field under its own key. Co-Authored-By: Claude Opus 5.5 --- docs/specs/alert.md | 2 +- lib/src/host/managed-voice-host.test.ts | 50 ++++++++++++-- lib/src/host/managed-voice-host.ts | 92 +++++++++++++++---------- 3 files changed, 103 insertions(+), 41 deletions(-) diff --git a/docs/specs/alert.md b/docs/specs/alert.md index 9105c2480..1929aeef9 100644 --- a/docs/specs/alert.md +++ b/docs/specs/alert.md @@ -410,7 +410,7 @@ Every rule above holds for both engines. - **Never let the voice token reach a renderer**; the host adds it to the request (rationale), and only managed-voice approval writes it (`docs/specs/hosted.md` -> "Managed voice"). Where it may go: `docs/specs/security-local.md` → "Persisted state". - **Must bound the host request** at `MANAGED_VOICE_REQUEST_TIMEOUT_MS` (inside `SPEECH_ENGINE_TIMEOUT_MS`, so a fallback still fits) and `MAX_AUDIO_BYTES`, accepting only `audio/mpeg`, and store only a token of the `dmv_…` shape and a voice of the curated set (`MANAGED_VOICES`). - **Must cache clips by voice and text in the host's memory, bounded in count and bytes, never persisted**; a hit makes no request, and a token change empties it. **A 403 latches `notEntitled`**, which bypasses the cache until a speak succeeds, so a lapsed member hears the system voice; **a 401 forgets the token it refused, never one saved since**. An answer to an older token changes nothing. -- **Must disable managed voice in memory immediately on Disconnect, including cached and in-flight results, even if clearing its persisted token fails.** A failure stays visible and retryable; it must never report a successful disconnect while the token remains on disk. **Disconnect must also delete the token at the voice origin, best effort and never awaited**, its retry included; under `nothing` it asks nothing, and the token lapses unused (`docs/specs/hosted.md` -> "Managed voice"). +- **Must disable managed voice in memory immediately on Disconnect, including cached and in-flight results, even if clearing its persisted token fails.** A failure stays visible and retryable; it must never report a successful disconnect while the token remains on disk. **Disconnect must also delete the token at the voice origin, best effort and never awaited**, its retry included, **as must the host for every approved token it discards unsaved** (approval canceled or overtaken, save failed); under `nothing` it asks nothing, and the token lapses unused (`docs/specs/hosted.md` -> "Managed voice"). Source of truth: `withFallback` in `lib/src/lib/speech-engine.ts`; `createManagedVoiceEngine` in `lib/src/lib/managed-voice-engine.ts`; `createManagedVoiceHost` in `lib/src/host/managed-voice-host.ts`; `MANAGED_VOICES` in `remote-lib-common/src/remote/managed-voice.ts`. diff --git a/lib/src/host/managed-voice-host.test.ts b/lib/src/host/managed-voice-host.test.ts index 1a0a1472c..c6bcd5457 100644 --- a/lib/src/host/managed-voice-host.test.ts +++ b/lib/src/host/managed-voice-host.test.ts @@ -23,8 +23,8 @@ const MANAGED_VOICE_TOKEN_URL = 'https://voice.dormouse.sh/api/voice/token'; const requests = () => fetchMock.mock.calls.map(([url, init]: [string, RequestInit]) => [url, init.method, (init.headers as Record).authorization]); -/** The next `readFile` fails with this, once: a Windows antivirus lock, say. */ -const readFault = vi.hoisted(() => ({ next: null as NodeJS.ErrnoException | null })); +/** The next `readFile` fails with this, once and then `repeats` more times: a Windows antivirus lock, say. */ +const readFault = vi.hoisted(() => ({ next: null as NodeJS.ErrnoException | null, repeats: 0 })); const renameFault = vi.hoisted(() => ({ next: null as NodeJS.ErrnoException | null })); vi.mock('node:fs/promises', async (importOriginal) => { const actual = await importOriginal(); @@ -38,7 +38,8 @@ vi.mock('node:fs/promises', async (importOriginal) => { }) as typeof actual.rename, readFile: (async (...args: Parameters) => { const fault = readFault.next; - readFault.next = null; + if (readFault.repeats > 0) readFault.repeats--; + else readFault.next = null; if (fault) throw fault; return actual.readFile(...args); }) as typeof actual.readFile, @@ -205,6 +206,18 @@ describe('configuration', () => { const unavailable = make({ stateDir: undefined }); await approveVoice(unavailable); expect(await unavailable.handle({ op: 'status' })).toMatchObject({ configured: false, enrollment: { status: 'ended', message: expect.stringContaining('Could not save') } }); + // Held by no one, so deleted at Hosted. + await vi.waitFor(() => expect(requests().at(-1)).toEqual([MANAGED_VOICE_TOKEN_URL, 'DELETE', `Bearer ${TOKEN}`])); + }); + + it('keeps the saved voice through a disconnect whose reads fail', async () => { + await writeFile(join(dir, MANAGED_VOICE_FILE), JSON.stringify({ token: TOKEN, voiceId: OTHER_VOICE }), { mode: 0o600 }); + const restarted = make(); + readFault.next = Object.assign(new Error('EBUSY: resource busy or locked'), { code: 'EBUSY' }); + readFault.repeats = 1; + expect(await restarted.handle({ op: 'disconnect' })).toMatchObject({ ok: true, configured: false, voiceId: OTHER_VOICE }); + expect(JSON.parse(await readFile(join(dir, MANAGED_VOICE_FILE), 'utf8'))).toEqual({ token: null, voiceId: OTHER_VOICE }); + expect(broadcasts.at(-1)).toEqual(idle(false, OTHER_VOICE)); }); }); @@ -641,7 +654,9 @@ describe('independent managed-voice lifecycle', () => { answer(Response.json({ status: 'enabled', voiceToken: TOKEN })); await vi.advanceTimersByTimeAsync(5000); expect(await host.handle({ op: 'status' })).toEqual(idle(false)); - expect(fetchMock).toHaveBeenCalledTimes(2); + // Polling stopped, and the late token is deleted at Hosted. + expect(fetchMock).toHaveBeenCalledTimes(3); + expect(requests().at(-1)).toEqual([MANAGED_VOICE_TOKEN_URL, 'DELETE', `Bearer ${TOKEN}`]); }); it('disconnect during the saving notification cannot re-enable a late approval', async () => { @@ -663,6 +678,33 @@ describe('independent managed-voice lifecycle', () => { await disconnect; expect(await host.handle({ op: 'status' })).toEqual(idle(false)); expect(await make().handle({ op: 'status' })).toEqual(idle(false)); + // The token the disconnect overtook, never saved, is deleted at Hosted. + await vi.waitFor(() => expect(requests().at(-1)).toEqual([MANAGED_VOICE_TOKEN_URL, 'DELETE', `Bearer ${TOKEN}`])); + }); + + it('deletes at Hosted a token a disconnect clears while it is being saved', async () => { + vi.useFakeTimers(); + const files = fileManagedVoiceStore(dir); + host = createManagedVoiceHost({ + store: { + read: files.read, + write(next, changed) { + // The disconnect queues behind this save, before it lands. + if (next.token !== null) void host.handle({ op: 'disconnect' }); + return files.write(next, changed); + }, + }, + onStatus: (status) => void broadcasts.push(status), + fetch: fetchMock as unknown as typeof fetch, + relay: { origin: DEFAULT_RELAY_ORIGIN, mode: 'hosted' }, + networkAllowed: async () => true, + }); + fetchMock.mockResolvedValueOnce(Response.json(begun())).mockResolvedValueOnce(Response.json({ status: 'enabled', voiceToken: TOKEN })); + await host.handle({ op: 'beginEnrollment' }); + await vi.advanceTimersByTimeAsync(1000); + await vi.waitFor(() => expect(requests().at(-1)).toEqual([MANAGED_VOICE_TOKEN_URL, 'DELETE', `Bearer ${TOKEN}`])); + expect(await host.handle({ op: 'status' })).toEqual(idle(false)); + expect(JSON.parse(await readFile(join(dir, MANAGED_VOICE_FILE), 'utf8')).token).toBeNull(); }); it('stops approval polling when the network policy changes to Nowhere', async () => { diff --git a/lib/src/host/managed-voice-host.ts b/lib/src/host/managed-voice-host.ts index cab81c794..2f8af91da 100644 --- a/lib/src/host/managed-voice-host.ts +++ b/lib/src/host/managed-voice-host.ts @@ -55,25 +55,34 @@ export interface ManagedVoiceStore { * lock cached as defaults would let the next edit overwrite the token. */ read(): Promise; - /** Persist `next`, of which only `changed` differs from what was read. */ + /** + * Persist `next[changed]` alone, keeping the other field as stored: a + * caller whose read failed passes defaults there, which must never land. + */ write(next: StoredManagedVoice, changed: keyof StoredManagedVoice): Promise; } -/** The sidecar's store: `managed-voice.json` in the owner-only state directory, rewritten whole. */ +/** + * The sidecar's store: `managed-voice.json` in the owner-only state directory, + * rewritten whole from a fresh read, so a write fails rather than reset the + * field it was not given. + */ export function fileManagedVoiceStore(stateDir: string): ManagedVoiceStore { const file = join(stateDir, MANAGED_VOICE_FILE); + async function read(): Promise { + try { + return JSON.parse(await readFile(file, 'utf8')); + } catch (error) { + // Only a missing or unparsable file is "no config": a transient lock + // (EBUSY, EPERM, EMFILE) is retried on the next read. + if ((error as NodeJS.ErrnoException).code === 'ENOENT' || error instanceof SyntaxError) return null; + throw error; + } + } return { - async read() { - try { - return JSON.parse(await readFile(file, 'utf8')); - } catch (error) { - // Only a missing or unparsable file is "no config": a transient lock - // (EBUSY, EPERM, EMFILE) is retried on the next read. - if ((error as NodeJS.ErrnoException).code === 'ENOENT' || error instanceof SyntaxError) return null; - throw error; - } - }, - write: (next) => writeJsonAtomic(stateDir, file, next), + read, + write: async (next, changed) => + writeJsonAtomic(stateDir, file, { ...normalizeStored(await read()), [changed]: next[changed] }), }; } @@ -224,17 +233,17 @@ export function createManagedVoiceHost(options: { /** Clear the stored token, memory before disk: a failed write must never leave speech armed. Runs on `serialize`. */ async function clearToken(): Promise { - const current = await load().catch(() => normalizeStored(null)); - const next = { ...current, token: null }; - loaded = Promise.resolve(next); + // Unread, the voice is unknown: never cached, and the store keeps it. + const current = await load().catch(() => null); + loaded = current ? Promise.resolve({ ...current, token: null }) : null; disconnectFailed = true; - uncleared = current.token ?? uncleared; - options.onStatus(status(next)); + uncleared = current?.token ?? uncleared; + await announce(); if (!store) throw new Error('no owner-only store'); - await store.write(next, 'token'); + await store.write({ ...current ?? normalizeStored(null), token: null }, 'token'); disconnectFailed = false; uncleared = null; - options.onStatus(status(next)); + await announce(); } /** Any token change forgets every clip and every refusal the old one earned. */ @@ -246,7 +255,15 @@ export function createManagedVoiceHost(options: { return serialize(async () => { if (speakUrl === null) throw new Error('this build has no managed voice'); if (token === null) { - await clearToken(); + // Read here, after any save queued first: a token a disconnect + // overtakes mid-save is deleted too. A retry's is the one the failed + // clear left behind. + const held = (await load().catch(() => null))?.token ?? uncleared; + try { + await clearToken(); + } finally { + if (held !== null) void forgetAtHosted(held); + } } else { await save('token', (config) => ({ ...config, token }), false); if (generation === tokenGeneration) disabled = false; @@ -354,9 +371,9 @@ export function createManagedVoiceHost(options: { } /** - * Best effort, never awaited by turning off: delete `token` at Hosted, so a - * computer that turned Natural Voice off leaves nothing live there. Skipped - * under Nowhere, failed offline; either way the token lapses unused. + * Best effort, never awaited: delete `token` at Hosted, so a token this + * computer turned off or discarded unsaved leaves nothing live there. + * Skipped under Nowhere, failed offline; either way the token lapses unused. */ async function forgetAtHosted(token: string): Promise { try { @@ -416,7 +433,12 @@ export function createManagedVoiceHost(options: { // Check outside the retrying exchange too: Nowhere ends this run. await requireEnrollmentNetwork(); const answer = await pollHostedApproval(options.relay.origin, body.deviceCode, 'voice', enrollmentFetch, parseVoiceApproval); - if (sequence !== enrollmentSequence) return; + // Below, a token this host discards (canceled, overtaken, or never + // saved) is held by no one, so it is deleted at Hosted. + if (sequence !== enrollmentSequence) { + if (answer.status === 'enabled') void forgetAtHosted(answer.voiceToken); + return; + } if (answer.status === 'retry') { if (answer.slowDown) intervalMs = slowDownHostedApproval(intervalMs); schedule(); @@ -428,11 +450,15 @@ export function createManagedVoiceHost(options: { if (answer.status === 'enabled') { enrollment = { status: 'saving' }; await announce(); - if (sequence !== enrollmentSequence) return; - // A token never saved, or never received below, is held by no one, - // so the user is never asked to revoke it. + if (sequence !== enrollmentSequence) { + void forgetAtHosted(answer.voiceToken); + return; + } try { await setToken(answer.voiceToken); } - catch { throw new Error('Could not save Natural Voice on this computer. Try again.'); } + catch { + void forgetAtHosted(answer.voiceToken); + throw new Error('Could not save Natural Voice on this computer. Try again.'); + } if (sequence === enrollmentSequence) { enrollment = null; await announce(); } return; } @@ -459,13 +485,7 @@ export function createManagedVoiceHost(options: { cancelEnrollment(); if (op === 'disconnect') { disabled = true; - // A retry's token is the one the failed clear left behind. - const held = (await load().catch(() => null))?.token ?? uncleared; - try { - await setToken(null); - } finally { - if (held !== null) void forgetAtHosted(held); - } + await setToken(null); } else await announce(); } return { ok: true, ...status(await load()) };