diff --git a/src/main/runtime/device-registry-corruption.ts b/src/main/runtime/device-registry-corruption.ts index 7e13f3818ff..253972954f9 100644 --- a/src/main/runtime/device-registry-corruption.ts +++ b/src/main/runtime/device-registry-corruption.ts @@ -15,14 +15,17 @@ export function corruptRegistryCopyExists(registryPath: string): boolean { } } -export function quarantineCorruptRegistry(registryPath: string): void { +/** True when this call moved the file aside; false when it was already gone. */ +export function quarantineCorruptRegistry(registryPath: string): boolean { const target = join(dirname(registryPath), `${corruptCopyPrefix(registryPath)}${Date.now()}`) try { renameSync(registryPath, target) + return true } catch (error) { if (!isMissingFileError(error)) { throw error } + return false } } diff --git a/src/main/runtime/device-registry-disk-merge.ts b/src/main/runtime/device-registry-disk-merge.ts index 70bdd5a4d77..06c962eff00 100644 --- a/src/main/runtime/device-registry-disk-merge.ts +++ b/src/main/runtime/device-registry-disk-merge.ts @@ -1,5 +1,6 @@ import { writeDurableSecureJsonFile, writeSecureJsonFile } from '../../shared/secure-file' import type { DeviceEntry } from './device-entry' +import { quarantineCorruptRegistry } from './device-registry-corruption' import { readDeviceRegistryFile, type DeviceRegistryFileRead } from './device-registry-file-read' /** @@ -29,14 +30,22 @@ export function mergeDeviceRegistrySave( * Writes saves merged over the file as it is now (atomically, through the secure-file stage and * rename). Remembers every id this process removed, recorded before the write so a failed save * (such as the load-time cleanup of session-only rows) still counts: a stale writer putting one of - * those back must not get it re-adopted. A file that became unreadable or corrupt proves nothing - * about removals, so then the save is written as is, as before. + * those back must not get it re-adopted. A file that became unreadable or corrupt, or that this + * process quarantined itself, proves nothing about removals, so then the save is written as is. */ export class DeviceRegistryWriter { private readonly removedIds = new Set() + /** This process moved the file aside and no write has landed since: its absence is ours, not a revoke. */ + private quarantinedUntilWritten = false constructor(private readonly registryPath: string) {} + quarantineCorrupt(): void { + if (quarantineCorruptRegistry(this.registryPath)) { + this.quarantinedUntilWritten = true + } + } + /** Returns what was written. Security transitions pass `durable`. */ write( before: readonly DeviceEntry[], @@ -49,13 +58,14 @@ export class DeviceRegistryWriter { this.removedIds.add(deviceId) } } - const disk = readDeviceRegistryFile(this.registryPath) + const disk = this.quarantinedUntilWritten ? null : readDeviceRegistryFile(this.registryPath) const merged = - disk.unreadable || disk.corruptFilePending + !disk || disk.unreadable || disk.corruptFilePending ? [...next] : mergeDeviceRegistrySave(before, next, disk, this.removedIds) const write = options.durable ? writeDurableSecureJsonFile : writeSecureJsonFile write(this.registryPath, merged) + this.quarantinedUntilWritten = false return merged } } diff --git a/src/main/runtime/device-registry-own-quarantine.test.ts b/src/main/runtime/device-registry-own-quarantine.test.ts new file mode 100644 index 00000000000..d55e55ef24d --- /dev/null +++ b/src/main/runtime/device-registry-own-quarantine.test.ts @@ -0,0 +1,107 @@ +// A load that drops an unreadable row (e.g. one a newer Orca wrote) quarantines the file at the +// first save; that missing file is this process's own doing and must not revoke the valid rows. +import { existsSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import * as secureFile from '../../shared/secure-file' +import { DeviceRegistry } from './device-registry' +import { DEVICE_REGISTRY_FILENAME } from './mobile-pairing-files' + +let dir: string + +beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'orca-device-registry-quarantine-')) +}) + +afterEach(() => { + vi.restoreAllMocks() + rmSync(dir, { recursive: true, force: true }) +}) + +function registryPath(): string { + return join(dir, DEVICE_REGISTRY_FILENAME) +} + +function fileIds(): string[] { + const rows: { deviceId: string }[] = JSON.parse(readFileSync(registryPath(), 'utf8')) + return rows.map((row) => row.deviceId) +} + +function quarantinedCopies(): string[] { + return readdirSync(dir).filter((name) => name.startsWith(`${DEVICE_REGISTRY_FILENAME}.corrupt-`)) +} + +/** A paired, seen phone in a file that also holds a row this build can't read. */ +function partiallyCorruptRegistry() { + const first = new DeviceRegistry(dir) + const phone = first.addDevice('phone', 'mobile') + first.updateLastSeen(phone.deviceId) + const rows: unknown[] = JSON.parse(readFileSync(registryPath(), 'utf8')) + writeFileSync(registryPath(), JSON.stringify([...rows, { deviceId: 'x', bogus: true }])) + const registry = new DeviceRegistry(dir) + expect(registry.validateToken(phone.token)?.deviceId).toBe(phone.deviceId) + return { registry, phone } +} + +describe('device registry after quarantining its own corrupt file', () => { + it('keeps every valid device through the first save', () => { + const { registry, phone } = partiallyCorruptRegistry() + + const added = registry.addDevice('new desktop', 'runtime') + + expect(quarantinedCopies()).toHaveLength(1) + expect(fileIds()).toEqual([phone.deviceId, added.deviceId]) + expect(registry.validateToken(phone.token)?.deviceId).toBe(phone.deviceId) + expect(new DeviceRegistry(dir).validateToken(phone.token)?.deviceId).toBe(phone.deviceId) + }) + + it('keeps them when the write after the quarantine failed, on the next save', () => { + const { registry, phone } = partiallyCorruptRegistry() + vi.spyOn(secureFile, 'writeSecureJsonFile').mockImplementationOnce(() => { + throw new Error('disk full') + }) + + expect(() => registry.addDevice('new desktop', 'runtime')).toThrow('disk full') + expect(quarantinedCopies()).toHaveLength(1) + expect(existsSync(registryPath())).toBe(false) + const added = registry.addDevice('new desktop', 'runtime') + + expect(fileIds()).toEqual([phone.deviceId, added.deviceId]) + expect(registry.validateToken(phone.token)?.deviceId).toBe(phone.deviceId) + }) + + it('still revokes everything when the file is deleted after its own write landed', () => { + const { registry, phone } = partiallyCorruptRegistry() + registry.addDevice('new desktop', 'runtime') + + rmSync(registryPath()) + const added = registry.addDevice('second desktop', 'runtime') + + expect(fileIds()).toEqual([added.deviceId]) + expect(registry.validateToken(phone.token)).toBeNull() + }) + + it('revokes everything when an intact file is deleted, as before', () => { + const registry = new DeviceRegistry(dir) + const phone = registry.addDevice('phone', 'mobile') + + rmSync(registryPath()) + const added = registry.addDevice('desktop', 'runtime') + + expect(fileIds()).toEqual([added.deviceId]) + expect(registry.getDevice(phone.deviceId)).toBeNull() + expect(registry.validateToken(phone.token)).toBeNull() + }) + + it('treats a corrupt file deleted by someone else before the quarantine as a revoke', () => { + const { registry, phone } = partiallyCorruptRegistry() + + rmSync(registryPath()) + const added = registry.addDevice('new desktop', 'runtime') + + expect(quarantinedCopies()).toEqual([]) + expect(fileIds()).toEqual([added.deviceId]) + expect(registry.validateToken(phone.token)).toBeNull() + }) +}) diff --git a/src/main/runtime/device-registry.ts b/src/main/runtime/device-registry.ts index 8d533733fbf..397e48e34ab 100644 --- a/src/main/runtime/device-registry.ts +++ b/src/main/runtime/device-registry.ts @@ -6,7 +6,6 @@ import { DEVICE_REGISTRY_FILENAME } from './mobile-pairing-files' import { mintDeviceCredential } from './device-credential' import { planDeviceKeyFork } from './device-key-fork' import { planIdleEphemeralPrune, withoutEphemeralRows } from './ephemeral-device-rows' -import { quarantineCorruptRegistry } from './device-registry-corruption' import { readDeviceRegistryFile } from './device-registry-file-read' import { DeviceRegistryWriter } from './device-registry-disk-merge' import type { RelayDeviceBinding } from './mobile-cloud-relay/relay-revoke-outbox' @@ -339,7 +338,7 @@ export class DeviceRegistry { ) } if (this.corruptFilePending) { - quarantineCorruptRegistry(this.registryPath) + this.writer.quarantineCorrupt() this.corruptFilePending = false } }