fix: pr-bug-scan findings from #1475 (#1562)

host-store: write AsyncStorage metadata before SecureStore token to avoid orphaned keychain tokens on crash. mobile IPC: coalesce repeated getPairingQR calls onto a single pending device token via new DeviceRegistry.getOrCreatePendingDevice.

Findings addressed:
- [high] mobile/src/transport/host-store.ts — saveHost orders Keychain write before AsyncStorage — orphaned tokens on crash
- [low] src/main/ipc/mobile.ts:84-95 — getPairingQR creates a device token on every call, leaking pre-paired entries

Rebased onto current main to resolve conflicts; preserved tokenCache.set added on main.

Co-authored-by: orca-bot <bot@stably.ai>
This commit is contained in:
buf0-bot[bot] 2026-05-07 23:11:03 -07:00 committed by GitHub
parent 74159b43d1
commit 57d095bf6f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 25 additions and 2 deletions

View File

@ -132,8 +132,13 @@ export async function saveHost(host: HostProfile): Promise<void> {
} else {
hosts.push(stored)
}
await SecureStore.setItemAsync(tokenKey(stored.id), validated.deviceToken, KEYCHAIN_OPTIONS)
// Why: write metadata BEFORE the keychain token so a crash between the two
// leaves orphaned metadata (which loadHosts skips and removeHost can clean
// up) rather than an orphaned keychain token with no metadata pointer —
// the latter would persist forever since removeHost only deletes by hostId
// from current metadata.
await AsyncStorage.setItem(STORAGE_KEY, JSON.stringify(hosts))
await SecureStore.setItemAsync(tokenKey(stored.id), validated.deviceToken, KEYCHAIN_OPTIONS)
tokenCache.set(stored.id, validated.deviceToken)
}

View File

@ -59,7 +59,11 @@ export function registerMobileHandlers(rpcServer: OrcaRuntimeRpcServer): void {
}
const endpoint = rawEndpoint.replace('0.0.0.0', ip)
const device = registry.addDevice(`Mobile ${new Date().toLocaleDateString()}`)
// Why: coalesce repeated QR regenerations onto a single never-scanned
// pending token so the copy-button flow doesn't accumulate orphaned
// device credentials forever. The token graduates to a real entry when
// a phone actually connects (lastSeenAt > 0).
const device = registry.getOrCreatePendingDevice(`Mobile ${new Date().toLocaleDateString()}`)
const publicKeyB64 = rpcServer.getE2EEPublicKey()
if (!publicKeyB64) {

View File

@ -38,6 +38,20 @@ export class DeviceRegistry {
return entry
}
// Why: coalesce repeated QR-regenerate clicks onto a single pending token.
// Each call to addDevice() produces a valid auth credential; without
// coalescing, every renderer call to mobile:getPairingQR (e.g. the new
// copy-button flow that encourages regeneration) leaves an orphaned token
// forever. Returns an existing never-scanned entry if present; otherwise
// mints a new one and drops any stale pending entries.
getOrCreatePendingDevice(name: string): DeviceEntry {
const existing = this.devices.find((d) => d.lastSeenAt === 0)
if (existing) {
return existing
}
return this.addDevice(name)
}
removeDevice(deviceId: string): boolean {
const before = this.devices.length
this.devices = this.devices.filter((d) => d.deviceId !== deviceId)