From cc222f1fa77f2891854d2d777cd3fdba3f652929 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 30 Jul 2026 22:00:46 -0700 Subject: [PATCH] fix(mobile): restore pairing self-heal and recover a wedged handshake readPairingKeychainItem threw when an Android presence record pointed at a SecureStore entry that read back null. Android reports absent and undecryptable identically, so the keystore fault the presence record was added to survive latched every caller out of its own orphan cleanup: the pairing journal store never reached its null-secret branch, stale winner-stamped metadata survived, and every later QR scan failed with "mobile relay pairing recovery pending". Report absent instead and drop the stale presence claim, still without falling back to the superseded older generation. The handshake-timeout path closed the socket with no handleSocketClosed fallback, unlike the connect-timeout and activity-probe paths. When React Native omits onclose for a wedged transport the client stayed in 'handshaking' forever with no reconnect armed. --- .../cellular-connecting-label-stall.test.ts | 42 ++++++++++++++++++- ...mobile-relay-pairing-journal-store.test.ts | 37 ++++++++++++++++ mobile/src/transport/pairing-keychain.test.ts | 12 +++++- mobile/src/transport/pairing-keychain.ts | 17 +++++++- mobile/src/transport/rpc-client.ts | 4 ++ 5 files changed, 108 insertions(+), 4 deletions(-) diff --git a/mobile/src/transport/cellular-connecting-label-stall.test.ts b/mobile/src/transport/cellular-connecting-label-stall.test.ts index e7bc6731462..e9a46d3b406 100644 --- a/mobile/src/transport/cellular-connecting-label-stall.test.ts +++ b/mobile/src/transport/cellular-connecting-label-stall.test.ts @@ -31,6 +31,9 @@ type CarrierBehavior = // Endpoint is healthy but its e2ee_ready lands after readyAfterMs, which a // high-latency / lossy cellular link readily pushes past the 5s budget. | { kind: 'slow-handshake'; readyAfterMs: number } + // Upgrade completes, nothing is ever answered, and the native socket is already + // half-open, so RN never delivers a close event for the client's own close(). + | { kind: 'wedged-open-then-silent' } let carrier: CarrierBehavior = { kind: 'blackhole' } const sockets: CarrierWebSocket[] = [] @@ -54,7 +57,11 @@ class CarrierWebSocket { setTimeout(() => this.fail(), 1) return } - if (carrier.kind === 'open-then-silent' || carrier.kind === 'slow-handshake') { + if ( + carrier.kind === 'open-then-silent' || + carrier.kind === 'slow-handshake' || + carrier.kind === 'wedged-open-then-silent' + ) { setTimeout(() => { this.readyState = 1 this.onopen?.() @@ -88,6 +95,10 @@ class CarrierWebSocket { if (this.readyState === 3) { return } + if (carrier.kind === 'wedged-open-then-silent') { + this.readyState = 3 + return + } this.fail() } @@ -235,6 +246,35 @@ describe('issue #10119 — what a phone shows while it cannot reach the desktop' client.close() }) + it('redials after a handshake timeout the wedged transport never reports as closed', () => { + carrier = { kind: 'wedged-open-then-silent' } + const client = connect('ws://192.168.0.56:6769', 'device-token', 'server-public-key') + + // 200ms upgrade plus the 5s handshake budget, still inside the reconnect delay. + vi.advanceTimersByTime(5_400) + + // Without the onclose fallback nothing else leaves 'handshaking': no reconnect + // timer is armed, the activity probe bails, and foregrounding is a no-op. + expect(sockets).toHaveLength(1) + expect(client.getState()).toBe('reconnecting') + + vi.advanceTimersByTime(1_000) + expect(sockets).toHaveLength(2) + + client.close() + }) + + it('escalates past "Connecting…" while every wedged dial swallows its close event', () => { + carrier = { kind: 'wedged-open-then-silent' } + const samples = observe('ws://192.168.0.56:6769', 900_000) + + expect(Math.max(...samples.map((s) => s.attempts))).toBeGreaterThanOrEqual(12) + expect(samples.some((s) => s.label === "Can't reach desktop")).toBe(true) + + const after = labelsAfterFirstEscalation(samples) + expect(after.every((s) => s.label !== 'Connecting…')).toBe(true) + }) + it('contrast: an endpoint that RSTs escalates the same way', () => { carrier = { kind: 'refused' } const samples = observe('ws://192.168.0.56:6769', 60_000) diff --git a/mobile/src/transport/mobile-relay-pairing-journal-store.test.ts b/mobile/src/transport/mobile-relay-pairing-journal-store.test.ts index cf249a90506..db67a2ac0d7 100644 --- a/mobile/src/transport/mobile-relay-pairing-journal-store.test.ts +++ b/mobile/src/transport/mobile-relay-pairing-journal-store.test.ts @@ -339,6 +339,43 @@ describe('mobile relay pairing journal store', () => { await expect(loadMobileRelayPairingJournal()).resolves.toEqual(journal) }) + it('self-heals an undecryptable Android secret so the next QR scan can pair', async () => { + platform.OS = 'android' + const created = createMobileRelayPairingJournal({ + offer: offer as PairingOffer & { relay: NonNullable }, + hostId: 'host-1', + hostName: 'Blue Whale', + randomBytes: (length) => new Uint8Array(length).fill(13) + }) + // Why: the candidate race stamps winner/authorizationMode long before pairing completes. + const journal = { + ...created, + metadata: { + ...created.metadata, + winner: 'relay' as const, + authorizationMode: 'relay-basis' as const + } + } + await saveMobileRelayPairingJournal(journal) + expect(presenceRaw).toBe('0') + + // Why: Android SecureStore reports an undecryptable keystore entry as null (#6600). + secretRaw = null + + await expect(loadMobileRelayPairingJournal()).resolves.toBeNull() + expect(metadataRaw).toBeNull() + expect(presenceRaw).toBeNull() + + const rescan = createMobileRelayPairingJournal({ + offer: offer as PairingOffer & { relay: NonNullable }, + hostId: 'host-2', + hostName: 'Red Panda', + randomBytes: (length) => new Uint8Array(length).fill(14) + }) + await expect(saveMobileRelayPairingJournal(rescan)).resolves.toBeUndefined() + await expect(loadMobileRelayPairingJournal()).resolves.toEqual(rescan) + }) + it('clears metadata before deleting its secret and keeps relay unavailable on web', async () => { const journal = createMobileRelayPairingJournal({ offer: offer as PairingOffer & { relay: NonNullable }, diff --git a/mobile/src/transport/pairing-keychain.test.ts b/mobile/src/transport/pairing-keychain.test.ts index fda46d0a328..b656fe316d7 100644 --- a/mobile/src/transport/pairing-keychain.test.ts +++ b/mobile/src/transport/pairing-keychain.test.ts @@ -246,7 +246,7 @@ describe('pairing keychain', () => { expect(secureStoreMock.getItemAsync).toHaveBeenCalledTimes(1) }) - it('does not return a stale older value when a recorded current item decrypts as null', async () => { + it('self-heals to absent, never a stale older value, when a recorded item decrypts as null', async () => { generationRecord = '1' let presenceRecord: string | null = null asyncStorageMock.getItem.mockImplementation(async (key: string) => { @@ -263,6 +263,11 @@ describe('pairing keychain', () => { presenceRecord = raw } }) + asyncStorageMock.removeItem.mockImplementation(async (key: string) => { + if (key === TOKEN_PRESENCE_KEY) { + presenceRecord = null + } + }) await writePairingKeychainItem(TOKEN_KEY, 'rotated-token') expect(presenceRecord).toBe('1') @@ -275,8 +280,11 @@ describe('pairing keychain', () => { serviceOf(options) === undefined ? 'legacy-token' : null ) - await expect(readPairingKeychainItem(TOKEN_KEY)).rejects.toThrow(/recorded generation/) + // Why: Android cannot distinguish absent from undecryptable, and a throw here left every + // caller's orphan cleanup unreachable; absent is the only self-healing answer. + await expect(readPairingKeychainItem(TOKEN_KEY)).resolves.toBeNull() expect(secureStoreMock.getItemAsync).toHaveBeenCalledTimes(1) + expect(presenceRecord).toBeNull() }) it('reads an older value while a newly recorded alias has no item yet', async () => { diff --git a/mobile/src/transport/pairing-keychain.ts b/mobile/src/transport/pairing-keychain.ts index bf3b1810807..f7884bbc665 100644 --- a/mobile/src/transport/pairing-keychain.ts +++ b/mobile/src/transport/pairing-keychain.ts @@ -134,6 +134,17 @@ async function preparePresenceWrite( return { storageKey, previousRaw } } +async function clearPresence(key: string): Promise { + if (Platform.OS !== 'android') { + return + } + try { + await AsyncStorage.removeItem(presenceStorageKey(key)) + } catch { + // Why: the read already self-healed; a stuck presence record must not fail the caller. + } +} + async function restorePresence(change: PresenceChange | null): Promise { if (!change) { return @@ -162,7 +173,11 @@ export async function readPairingKeychainItem(key: string): Promise