fix(runtime): quarantining a partly corrupt device registry no longer revokes every device (#35539)

A load that drops an unreadable row quarantines the file at the first
save. The save then re-read the now-missing file, took the empty list
as an admin deleting it, and dropped every valid device. The writer now
records its own quarantine and writes the save as is until a write
lands; a file someone else deleted still revokes everything.
This commit is contained in:
Neil
2026-10-28 18:27:12 -07:00
committed by GitHub
parent 3a68805eee
commit 0978e3e049
4 changed files with 126 additions and 7 deletions
@@ -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
}
}
+14 -4
View File
@@ -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<string>()
/** 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
}
}
@@ -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()
})
})
+1 -2
View File
@@ -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
}
}