From 57d095bf6f59a7b177e4bbb2317e36bbcb18374b Mon Sep 17 00:00:00 2001
From: "buf0-bot[bot]" <252831055+buf0-bot[bot]@users.noreply.github.com>
Date: Thu, 7 May 2026 23:11:03 -0700
Subject: [PATCH] fix: pr-bug-scan findings from #1475 (#1562)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 mobile/src/transport/host-store.ts  |  7 ++++++-
 src/main/ipc/mobile.ts              |  6 +++++-
 src/main/runtime/device-registry.ts | 14 ++++++++++++++
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/mobile/src/transport/host-store.ts b/mobile/src/transport/host-store.ts
index 13fd665e4..af21723a6 100644
--- a/mobile/src/transport/host-store.ts
+++ b/mobile/src/transport/host-store.ts
@@ -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)
 }
 
diff --git a/src/main/ipc/mobile.ts b/src/main/ipc/mobile.ts
index 528749c29..7a9e0e6f4 100644
--- a/src/main/ipc/mobile.ts
+++ b/src/main/ipc/mobile.ts
@@ -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) {
diff --git a/src/main/runtime/device-registry.ts b/src/main/runtime/device-registry.ts
index fed25b622..935e93eeb 100644
--- a/src/main/runtime/device-registry.ts
+++ b/src/main/runtime/device-registry.ts
@@ -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)