mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 00:03:45 +00:00
fix(mobile): protect host token cache from stale reads
This commit is contained in:
@@ -538,7 +538,6 @@ export default function HomeScreen() {
|
||||
}
|
||||
}
|
||||
// Why: key on host-id set + each client's identity so resubs fire when forceReconnect swaps a host's client, not on every render.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [
|
||||
allClients
|
||||
.map((e) => `${e.hostId}:${clientKey(e.client)}`)
|
||||
|
||||
@@ -1,12 +1,13 @@
|
||||
import type { HostProfile } from './types'
|
||||
|
||||
// Why: Keychain reads are slow (50-200ms) and loadHosts() runs on every screen
|
||||
// mount, so concurrent callers share one pass instead of each paying for their
|
||||
// own. A pass that started before a write still holds the pre-write snapshot,
|
||||
// though — handing it to a caller that loaded AFTER the write returns the host
|
||||
// that caller just removed, or the name it just replaced (issue #8791). So every
|
||||
// durable commit drops the shared pass and the next caller reads fresh.
|
||||
// Why: concurrent callers share a slow Keychain pass; durable writes invalidate
|
||||
// it so later loads cannot receive stale snapshots (#8791).
|
||||
let inflight: Promise<HostProfile[]> | null = null
|
||||
let revision = 0
|
||||
|
||||
export function getHostListLoadRevision(): number {
|
||||
return revision
|
||||
}
|
||||
|
||||
export function shareHostListLoad(load: () => Promise<HostProfile[]>): Promise<HostProfile[]> {
|
||||
if (inflight) {
|
||||
@@ -25,5 +26,6 @@ export function shareHostListLoad(load: () => Promise<HostProfile[]>): Promise<H
|
||||
|
||||
/** Call after every durable host write so no later read is served a pre-write pass. */
|
||||
export function dropSharedHostListLoad(): void {
|
||||
revision += 1
|
||||
inflight = null
|
||||
}
|
||||
|
||||
@@ -415,6 +415,35 @@ describe('host-store list mutations', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('does not let a late pre-write token read poison the token cache', async () => {
|
||||
const newHost = {
|
||||
id: 'host-new',
|
||||
name: 'New Host',
|
||||
endpoint: 'ws://127.0.0.1:3',
|
||||
publicKeyB64: 'key-new',
|
||||
deviceToken: 'token-new',
|
||||
lastConnected: 0
|
||||
}
|
||||
storedHostsRaw = JSON.stringify([{ ...newHost, deviceToken: undefined }])
|
||||
let resolvePrewriteTokenRead: (token: string) => void = () => {}
|
||||
secureStoreMock.getItemAsync.mockReturnValue(
|
||||
new Promise<string>((resolve) => {
|
||||
resolvePrewriteTokenRead = resolve
|
||||
})
|
||||
)
|
||||
|
||||
const parkedLoad = loadHosts()
|
||||
await vi.waitFor(() => {
|
||||
expect(secureStoreMock.getItemAsync).toHaveBeenCalled()
|
||||
})
|
||||
await saveHost(newHost)
|
||||
resolvePrewriteTokenRead('token-old')
|
||||
await parkedLoad
|
||||
|
||||
const hosts = await loadHosts()
|
||||
expect(hosts.find((host) => host.id === newHost.id)?.deviceToken).toBe(newHost.deviceToken)
|
||||
})
|
||||
|
||||
it('still shares one Keychain pass across loads with no write between them', async () => {
|
||||
const releaseKeychain = gateKeychainReads()
|
||||
const first = loadHosts()
|
||||
|
||||
@@ -8,7 +8,7 @@ import {
|
||||
type StoredHostProfile
|
||||
} from './types'
|
||||
import { getNextHostNameFromHosts } from './host-names'
|
||||
import { dropSharedHostListLoad, shareHostListLoad } from './host-list-load-sharing'
|
||||
import * as hostListLoads from './host-list-load-sharing'
|
||||
import {
|
||||
retryPendingHostCredentialCleanups,
|
||||
scheduleHostCredentialCleanup
|
||||
@@ -103,7 +103,7 @@ export async function loadHosts(): Promise<HostProfile[]> {
|
||||
// Why: writers hold the mutation chain across their full RMW; wait so a load doesn't race a half-written list.
|
||||
await hostListMutation
|
||||
// Why: deduplicate concurrent loadHosts() calls so simultaneously mounting screens share one Keychain read pass.
|
||||
return shareHostListLoad(doLoadHosts)
|
||||
return hostListLoads.shareHostListLoad(doLoadHosts)
|
||||
}
|
||||
|
||||
async function doLoadHosts(): Promise<HostProfile[]> {
|
||||
@@ -125,6 +125,7 @@ async function doLoadHosts(): Promise<HostProfile[]> {
|
||||
for (const stored of storedHosts) {
|
||||
let token = tokenCache.get(stored.id)
|
||||
if (!token) {
|
||||
const readRevision = hostListLoads.getHostListLoadRevision()
|
||||
let fetched: string | null
|
||||
try {
|
||||
fetched = await readDeviceToken(stored.id)
|
||||
@@ -137,7 +138,9 @@ async function doLoadHosts(): Promise<HostProfile[]> {
|
||||
continue
|
||||
}
|
||||
token = fetched
|
||||
tokenCache.set(stored.id, token)
|
||||
if (readRevision === hostListLoads.getHostListLoadRevision()) {
|
||||
tokenCache.set(stored.id, token)
|
||||
}
|
||||
}
|
||||
const overlay = overlays.get(stored.id)
|
||||
out.push({
|
||||
@@ -191,7 +194,7 @@ async function mutateStoredHosts(
|
||||
const current = await readStoredHostsForMutation()
|
||||
const next = update(current)
|
||||
await AsyncStorage.setItem(STORAGE_KEY, JSON.stringify(next))
|
||||
dropSharedHostListLoad()
|
||||
hostListLoads.dropSharedHostListLoad()
|
||||
})
|
||||
hostListMutation = mutation.catch(() => {})
|
||||
return mutation
|
||||
@@ -245,7 +248,7 @@ async function persistHost(host: HostProfile, requireExisting: boolean): Promise
|
||||
// Why: write metadata before the keychain token so a crash leaves recoverable orphaned metadata, not an orphaned token that persists forever.
|
||||
await writeDeviceToken(stored.id, validated.deviceToken)
|
||||
tokenCache.set(stored.id, validated.deviceToken)
|
||||
dropSharedHostListLoad()
|
||||
hostListLoads.dropSharedHostListLoad()
|
||||
if (validated.endpoints) {
|
||||
await saveMobileRelayHostOverlay({
|
||||
v: 2,
|
||||
@@ -254,7 +257,7 @@ async function persistHost(host: HostProfile, requireExisting: boolean): Promise
|
||||
relayHostId: validated.relayHostId,
|
||||
relay: validated.relay
|
||||
})
|
||||
dropSharedHostListLoad()
|
||||
hostListLoads.dropSharedHostListLoad()
|
||||
}
|
||||
const overlayRemovalIds = [...duplicateHostIds]
|
||||
if (!validated.endpoints && updatedExistingHost) {
|
||||
@@ -263,7 +266,7 @@ async function persistHost(host: HostProfile, requireExisting: boolean): Promise
|
||||
if (overlayRemovalIds.length > 0) {
|
||||
// Why: reusing an id for direct-only re-pairing must not retain routing metadata from the previous transport state.
|
||||
await removeMobileRelayHostOverlays(overlayRemovalIds)
|
||||
dropSharedHostListLoad()
|
||||
hostListLoads.dropSharedHostListLoad()
|
||||
}
|
||||
for (const duplicateHostId of duplicateHostIds) {
|
||||
tokenCache.delete(duplicateHostId)
|
||||
@@ -280,7 +283,7 @@ export async function removeHost(hostId: string): Promise<void> {
|
||||
tokenCache.delete(hostId)
|
||||
try {
|
||||
await removeMobileRelayHostOverlay(hostId)
|
||||
dropSharedHostListLoad()
|
||||
hostListLoads.dropSharedHostListLoad()
|
||||
} catch {
|
||||
// Base removal is authoritative; a retained overlay can't resurrect the host and is cleaned on a later retry.
|
||||
}
|
||||
@@ -340,5 +343,5 @@ export async function updateLastConnected(hostId: string): Promise<void> {
|
||||
export function resetHostStoreForTests(): void {
|
||||
hostListMutation = Promise.resolve()
|
||||
tokenCache.clear()
|
||||
dropSharedHostListLoad()
|
||||
hostListLoads.dropSharedHostListLoad()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user