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()) };