mirror of
https://github.com/stablyai/orca.git
synced 2026-10-17 16:02:55 +00:00
fix(mobile): harden host re-pair deduplication
This commit is contained in:
@@ -34,12 +34,13 @@ vi.mock('./host-credential-cleanup', () => ({
|
||||
}))
|
||||
|
||||
import {
|
||||
findStoredHostByPublicKey,
|
||||
loadHosts,
|
||||
MobileRelayUpgradeHostRemovedError,
|
||||
removeHost,
|
||||
renameHost,
|
||||
resolvePairingHostIdentity,
|
||||
resetHostStoreForTests,
|
||||
saveHost,
|
||||
saveExistingHostRelayUpgrade,
|
||||
updateLastConnected
|
||||
} from './host-store'
|
||||
@@ -86,6 +87,8 @@ describe('host-store list mutations', () => {
|
||||
asyncStorageMock.setItem.mockImplementation(async (key: string, raw: string) => {
|
||||
if (key === HOSTS_STORAGE_KEY) {
|
||||
storedHostsRaw = raw
|
||||
} else if (key === OVERLAY_STORAGE_KEY) {
|
||||
storedOverlayRaw = raw
|
||||
}
|
||||
})
|
||||
secureStoreMock.getItemAsync.mockImplementation(async (key: string) =>
|
||||
@@ -93,16 +96,87 @@ describe('host-store list mutations', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('finds a stored host by its pinned public key (STA-1840 re-pair dedup)', async () => {
|
||||
await expect(findStoredHostByPublicKey(HOST_TWO.publicKeyB64)).resolves.toEqual({
|
||||
it('resolves an existing host by pinned key with one durable read', async () => {
|
||||
await expect(resolvePairingHostIdentity(HOST_TWO.publicKeyB64, 'host-new')).resolves.toEqual({
|
||||
id: HOST_TWO.id,
|
||||
name: HOST_TWO.name
|
||||
})
|
||||
expect(asyncStorageMock.getItem).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('returns null when no stored host has the public key, or the key is empty', async () => {
|
||||
await expect(findStoredHostByPublicKey('unpaired-key')).resolves.toBeNull()
|
||||
await expect(findStoredHostByPublicKey('')).resolves.toBeNull()
|
||||
it('names a new host from the same durable read used for identity lookup', async () => {
|
||||
await expect(resolvePairingHostIdentity('unpaired-key', 'host-new')).resolves.toEqual({
|
||||
id: 'host-new',
|
||||
name: 'Host 3'
|
||||
})
|
||||
expect(asyncStorageMock.getItem).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('fails closed when durable host identity storage is unreadable', async () => {
|
||||
asyncStorageMock.getItem.mockRejectedValueOnce(new Error('storage unavailable'))
|
||||
|
||||
await expect(resolvePairingHostIdentity('key-new', 'host-new')).rejects.toThrow(/unreadable/)
|
||||
expect(asyncStorageMock.setItem).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('collapses already-persisted duplicates when the desktop is re-paired', async () => {
|
||||
storedHostsRaw = JSON.stringify([
|
||||
HOST_ONE,
|
||||
{ ...HOST_TWO, id: 'host-duplicate', publicKeyB64: HOST_ONE.publicKeyB64 }
|
||||
])
|
||||
|
||||
await saveHost({
|
||||
...HOST_ONE,
|
||||
endpoint: 'ws://127.0.0.1:3',
|
||||
deviceToken: 'replacement-token'
|
||||
})
|
||||
|
||||
expect(JSON.parse(storedHostsRaw)).toEqual([{ ...HOST_ONE, endpoint: 'ws://127.0.0.1:3' }])
|
||||
expect(scheduleCleanupMock).toHaveBeenCalledWith('host-duplicate', expect.any(Function))
|
||||
})
|
||||
|
||||
it('clears stale relay state when an existing host is re-paired direct-only', async () => {
|
||||
const overlay: MobileRelayHostOverlay = {
|
||||
v: 2,
|
||||
hostId: HOST_ONE.id,
|
||||
endpoints: [
|
||||
{ id: 'direct-primary', kind: 'lan', url: HOST_ONE.endpoint },
|
||||
{
|
||||
id: 'relay-primary',
|
||||
kind: 'relay',
|
||||
url: 'wss://relay-c1.onorca.dev/v1/connect/AbCdEf0123_-xyZ9'
|
||||
}
|
||||
],
|
||||
relayHostId: 'AbCdEf0123_-xyZ9',
|
||||
relay: {
|
||||
v: 1,
|
||||
directorUrl: 'https://relay.onorca.dev',
|
||||
cellUrl: 'https://relay-c1.onorca.dev',
|
||||
assignmentEpoch: 7,
|
||||
relayHostId: 'AbCdEf0123_-xyZ9',
|
||||
e2eeFraming: 2
|
||||
}
|
||||
}
|
||||
storedOverlayRaw = JSON.stringify([overlay])
|
||||
|
||||
await saveHost({ ...HOST_ONE, deviceToken: 'replacement-token' })
|
||||
|
||||
expect(JSON.parse(storedOverlayRaw!)).toEqual([])
|
||||
expect(secureStoreMock.deleteItemAsync).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not touch relay storage when saving a new direct-only host', async () => {
|
||||
await saveHost({
|
||||
id: 'host-new',
|
||||
name: 'New Host',
|
||||
endpoint: 'ws://127.0.0.1:3',
|
||||
publicKeyB64: 'key-new',
|
||||
deviceToken: 'new-token',
|
||||
lastConnected: 0
|
||||
})
|
||||
|
||||
expect(asyncStorageMock.getItem).not.toHaveBeenCalledWith(OVERLAY_STORAGE_KEY)
|
||||
expect(secureStoreMock.deleteItemAsync).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('commits the removal when credential cleanup scheduling rejects', async () => {
|
||||
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
import {
|
||||
loadMobileRelayHostOverlayState,
|
||||
removeMobileRelayHostOverlay,
|
||||
removeMobileRelayHostOverlays,
|
||||
saveMobileRelayHostOverlay
|
||||
} from './mobile-relay-host-overlay-store'
|
||||
import { deleteMobileRelayCredentialBundle } from './mobile-relay-credential-bundle'
|
||||
@@ -182,31 +183,18 @@ async function doLoadHosts(): Promise<HostProfile[]> {
|
||||
return out
|
||||
}
|
||||
|
||||
async function loadStoredHosts(): Promise<StoredHostProfile[]> {
|
||||
try {
|
||||
return parseStoredHosts(await AsyncStorage.getItem(STORAGE_KEY)) ?? []
|
||||
} catch {
|
||||
return []
|
||||
}
|
||||
}
|
||||
|
||||
// Why: re-pairing the same desktop must merge into its existing card, not mint
|
||||
// a new host id and duplicate the row (STA-1840). publicKeyB64 is the desktop's
|
||||
// pinned identity key — stable across re-pairs, unlike id/endpoint/relay — so
|
||||
// it is the right dedup key. Reads the durable stored list (not loadHosts) so a
|
||||
// host whose keychain token read is transiently failing still matches and is
|
||||
// not duplicated.
|
||||
export async function findStoredHostByPublicKey(
|
||||
publicKeyB64: string
|
||||
): Promise<{ id: string; name: string } | null> {
|
||||
if (!publicKeyB64) {
|
||||
return null
|
||||
}
|
||||
// Why: settle any in-flight save/remove first so a re-pair racing its own
|
||||
// prior write matches the just-persisted host rather than an empty snapshot.
|
||||
export async function resolvePairingHostIdentity(
|
||||
publicKeyB64: string,
|
||||
newHostId: string
|
||||
): Promise<{ id: string; name: string }> {
|
||||
// Why: one durable read both preserves an existing identity and names a new host,
|
||||
// avoiding duplicate cards and a second serial storage read before connecting.
|
||||
await hostListMutation
|
||||
const match = (await loadStoredHosts()).find((h) => h.publicKeyB64 === publicKeyB64)
|
||||
return match ? { id: match.id, name: match.name } : null
|
||||
const hosts = await readStoredHostsForMutation()
|
||||
const match = hosts.find((host) => host.publicKeyB64 === publicKeyB64)
|
||||
return match
|
||||
? { id: match.id, name: match.name }
|
||||
: { id: newHostId, name: getNextHostNameFromHosts(hosts) }
|
||||
}
|
||||
|
||||
async function readStoredHostsForMutation(): Promise<StoredHostProfile[]> {
|
||||
@@ -261,18 +249,28 @@ export async function saveExistingHostRelayUpgrade(host: HostProfile): Promise<v
|
||||
async function persistHost(host: HostProfile, requireExisting: boolean): Promise<void> {
|
||||
const validated = HostProfileSchema.parse(host)
|
||||
const stored = toStored(validated)
|
||||
const duplicateHostIds = new Set<string>()
|
||||
let updatedExistingHost = false
|
||||
await mutateStoredHosts((hosts) => {
|
||||
const index = hosts.findIndex((h) => h.id === stored.id)
|
||||
for (const candidate of hosts) {
|
||||
if (candidate.id !== stored.id && candidate.publicKeyB64 === stored.publicKeyB64) {
|
||||
duplicateHostIds.add(candidate.id)
|
||||
}
|
||||
}
|
||||
if (index >= 0) {
|
||||
const next = hosts.slice()
|
||||
next[index] = stored
|
||||
return next
|
||||
updatedExistingHost = true
|
||||
// Why: affected installs may already contain duplicate rows; an authoritative
|
||||
// save is the safe point to collapse them to the preserved host id.
|
||||
return hosts
|
||||
.filter(({ id }) => !duplicateHostIds.has(id))
|
||||
.map((candidate) => (candidate.id === stored.id ? stored : candidate))
|
||||
}
|
||||
if (requireExisting) {
|
||||
// Why: an in-flight relay upgrade must not resurrect a host the user removed.
|
||||
throw new MobileRelayUpgradeHostRemovedError('mobile relay upgrade host was removed')
|
||||
}
|
||||
return [...hosts, stored]
|
||||
return [...hosts.filter(({ id }) => !duplicateHostIds.has(id)), stored]
|
||||
})
|
||||
// Why: write metadata BEFORE the keychain token so a crash between the two
|
||||
// leaves orphaned metadata (which loadHosts skips and removeHost can clean
|
||||
@@ -290,6 +288,23 @@ async function persistHost(host: HostProfile, requireExisting: boolean): Promise
|
||||
relay: validated.relay
|
||||
})
|
||||
}
|
||||
const overlayRemovalIds = [...duplicateHostIds]
|
||||
if (!validated.endpoints && updatedExistingHost) {
|
||||
overlayRemovalIds.push(stored.id)
|
||||
}
|
||||
if (overlayRemovalIds.length > 0) {
|
||||
// Why: reusing an id for direct-only re-pairing must not retain routing
|
||||
// metadata from the host's previous transport state.
|
||||
await removeMobileRelayHostOverlays(overlayRemovalIds)
|
||||
}
|
||||
for (const duplicateHostId of duplicateHostIds) {
|
||||
tokenCache.delete(duplicateHostId)
|
||||
try {
|
||||
await scheduleHostCredentialCleanup(duplicateHostId, deleteHostCredentials)
|
||||
} catch {
|
||||
// Metadata is already deduplicated; orphan-token recovery is best-effort.
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export async function removeHost(hostId: string): Promise<void> {
|
||||
@@ -330,12 +345,6 @@ export async function renameHost(hostId: string, newName: string): Promise<void>
|
||||
})
|
||||
}
|
||||
|
||||
export async function getNextHostName(): Promise<string> {
|
||||
await hostListMutation
|
||||
const hosts = await loadStoredHosts()
|
||||
return getNextHostNameFromHosts(hosts)
|
||||
}
|
||||
|
||||
export async function updateLastConnected(hostId: string): Promise<void> {
|
||||
try {
|
||||
await mutateStoredHosts((hosts) => {
|
||||
|
||||
@@ -9,6 +9,7 @@ vi.mock('@react-native-async-storage/async-storage', () => ({ default: asyncStor
|
||||
|
||||
import {
|
||||
loadMobileRelayHostOverlays,
|
||||
removeMobileRelayHostOverlays,
|
||||
resetMobileRelayHostOverlayStoreForTests,
|
||||
saveMobileRelayHostOverlay
|
||||
} from './mobile-relay-host-overlay-store'
|
||||
@@ -77,4 +78,24 @@ describe('mobile relay host overlay store', () => {
|
||||
await expect(saveMobileRelayHostOverlay(OVERLAY)).rejects.toThrow(/unreadable/)
|
||||
expect(asyncStorage.setItem).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('removes requested overlays in one storage write', async () => {
|
||||
const second = { ...OVERLAY, hostId: 'host-2' }
|
||||
stored = JSON.stringify([OVERLAY, second])
|
||||
|
||||
await expect(removeMobileRelayHostOverlays(['host-1', 'host-missing'])).resolves.toBeUndefined()
|
||||
|
||||
expect(JSON.parse(stored!)).toEqual([second])
|
||||
expect(asyncStorage.getItem).toHaveBeenCalledOnce()
|
||||
expect(asyncStorage.setItem).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('skips the storage write when no requested overlay exists', async () => {
|
||||
stored = JSON.stringify([OVERLAY])
|
||||
|
||||
await expect(removeMobileRelayHostOverlays(['host-missing'])).resolves.toBeUndefined()
|
||||
|
||||
expect(asyncStorage.getItem).toHaveBeenCalledOnce()
|
||||
expect(asyncStorage.setItem).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -40,7 +40,12 @@ async function mutateOverlays(
|
||||
): Promise<void> {
|
||||
const mutation = overlayMutation.then(async () => {
|
||||
const current = await readOverlaysForMutation()
|
||||
await AsyncStorage.setItem(OVERLAY_STORAGE_KEY, JSON.stringify(update(current)))
|
||||
const next = update(current)
|
||||
// Why: direct-only saves commonly have no overlay to remove; avoid a full
|
||||
// AsyncStorage write when cleanup leaves the durable list unchanged.
|
||||
if (next !== current) {
|
||||
await AsyncStorage.setItem(OVERLAY_STORAGE_KEY, JSON.stringify(next))
|
||||
}
|
||||
})
|
||||
overlayMutation = mutation.catch(() => {})
|
||||
return mutation
|
||||
@@ -85,7 +90,22 @@ export async function saveMobileRelayHostOverlay(overlay: MobileRelayHostOverlay
|
||||
}
|
||||
|
||||
export function removeMobileRelayHostOverlay(hostId: string): Promise<void> {
|
||||
return mutateOverlays((overlays) => overlays.filter((overlay) => overlay.hostId !== hostId))
|
||||
return removeMobileRelayHostOverlays([hostId])
|
||||
}
|
||||
|
||||
export function removeMobileRelayHostOverlays(hostIds: readonly string[]): Promise<void> {
|
||||
const targets = new Set(hostIds)
|
||||
let removed = false
|
||||
return mutateOverlays((overlays) => {
|
||||
const next = overlays.filter((overlay) => {
|
||||
if (!targets.has(overlay.hostId)) {
|
||||
return true
|
||||
}
|
||||
removed = true
|
||||
return false
|
||||
})
|
||||
return removed ? next : overlays
|
||||
})
|
||||
}
|
||||
|
||||
/** Test-only: drain the module mutation chain between cases. */
|
||||
|
||||
@@ -64,8 +64,10 @@ function dependencies(client: RpcClient, events: string[]) {
|
||||
resolveInviteDirector: vi.fn(async () => {
|
||||
throw new Error('director unavailable')
|
||||
}),
|
||||
getNextHostName: vi.fn(async () => 'Blue Whale'),
|
||||
findExistingHost: vi.fn(async (_publicKeyB64: string) => null),
|
||||
resolveHostIdentity: vi.fn(async (_publicKeyB64: string, hostId: string) => ({
|
||||
id: hostId,
|
||||
name: 'Blue Whale'
|
||||
})),
|
||||
saveHost: vi.fn(async (_host: HostProfile) => {
|
||||
events.push('save-host')
|
||||
}),
|
||||
@@ -143,8 +145,9 @@ describe('pre-profile pairing coordinator', () => {
|
||||
const events: string[] = []
|
||||
const client = fakeClient([success({ version: '1.0.0' })])
|
||||
const deps = dependencies(client, events)
|
||||
deps.findExistingHost = vi.fn(async (publicKeyB64: string) => {
|
||||
deps.resolveHostIdentity = vi.fn(async (publicKeyB64: string, newHostId: string) => {
|
||||
expect(publicKeyB64).toBe(directOffer.publicKeyB64)
|
||||
expect(newHostId).toBe(`host-${now}`)
|
||||
return { id: 'host-existing', name: 'Studio Mac' }
|
||||
})
|
||||
|
||||
@@ -163,8 +166,6 @@ describe('pre-profile pairing coordinator', () => {
|
||||
publicKeyB64: directOffer.publicKeyB64,
|
||||
lastConnected: now
|
||||
})
|
||||
// Existing host keeps its name — no fresh name minted.
|
||||
expect(deps.getNextHostName).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('journals before connecting and publishes only after authoritative direct install', async () => {
|
||||
|
||||
@@ -6,7 +6,7 @@ import {
|
||||
type MobileRelayEndpoint
|
||||
} from '../../../src/shared/mobile-relay-credential-contract'
|
||||
import { connect, type ConnectOptions } from './rpc-client'
|
||||
import { findStoredHostByPublicKey, getNextHostName, saveHost } from './host-store'
|
||||
import { resolvePairingHostIdentity, saveHost } from './host-store'
|
||||
import type { HostProfile, PairingOffer, RpcResponse } from './types'
|
||||
import {
|
||||
createMobileRelayPairingJournal,
|
||||
@@ -39,8 +39,7 @@ type Dependencies = {
|
||||
connectDirect: typeof connect
|
||||
connectRelay: typeof connectMobileRelayForPairing
|
||||
resolveInviteDirector: typeof resolvePairingInviteThroughDirector
|
||||
getNextHostName: typeof getNextHostName
|
||||
findExistingHost: typeof findStoredHostByPublicKey
|
||||
resolveHostIdentity: typeof resolvePairingHostIdentity
|
||||
saveHost: typeof saveHost
|
||||
saveJournal: typeof saveMobileRelayPairingJournal
|
||||
updateJournal: typeof updateMobileRelayPairingJournal
|
||||
@@ -54,8 +53,7 @@ const defaultDependencies: Dependencies = {
|
||||
connectDirect: connect,
|
||||
connectRelay: connectMobileRelayForPairing,
|
||||
resolveInviteDirector: resolvePairingInviteThroughDirector,
|
||||
getNextHostName,
|
||||
findExistingHost: findStoredHostByPublicKey,
|
||||
resolveHostIdentity: resolvePairingHostIdentity,
|
||||
saveHost,
|
||||
saveJournal: saveMobileRelayPairingJournal,
|
||||
updateJournal: updateMobileRelayPairingJournal,
|
||||
@@ -132,16 +130,12 @@ async function runPairing(
|
||||
isDisposed: () => boolean
|
||||
): Promise<{ hostId: string }> {
|
||||
const now = dependencies.now()
|
||||
// Why: if this desktop (by its pinned public key) is already paired, reuse
|
||||
// that host's id and name so the pairing merges into the existing card
|
||||
// instead of minting a fresh host-${now} and duplicating the row (STA-1840).
|
||||
// Every downstream write (journal, credential bundle, saveHost) then keys to
|
||||
// the same id, so persistHost updates in place. A new desktop key falls
|
||||
// through to a fresh id/name.
|
||||
const existing = await dependencies.findExistingHost(offer.publicKeyB64)
|
||||
assertActive(isDisposed)
|
||||
const hostId = existing?.id ?? `host-${now}`
|
||||
const hostName = existing?.name ?? (await dependencies.getNextHostName())
|
||||
// Why: every pairing artifact must share the preserved host id so re-pairing
|
||||
// updates one card instead of publishing a second identity (STA-1840).
|
||||
const { id: hostId, name: hostName } = await dependencies.resolveHostIdentity(
|
||||
offer.publicKeyB64,
|
||||
`host-${now}`
|
||||
)
|
||||
assertActive(isDisposed)
|
||||
let journal: MobileRelayPairingJournal | null = null
|
||||
if (offer.relay && dependencies.platform !== 'web') {
|
||||
|
||||
Reference in New Issue
Block a user