diff --git a/mobile/src/transport/mobile-relay-direct-upgrade.test.ts b/mobile/src/transport/mobile-relay-direct-upgrade.test.ts index d77848b6e17..63b9f42318b 100644 --- a/mobile/src/transport/mobile-relay-direct-upgrade.test.ts +++ b/mobile/src/transport/mobile-relay-direct-upgrade.test.ts @@ -156,6 +156,26 @@ describe('existing direct pairing relay upgrade', () => { expect(deps.saveHost).not.toHaveBeenCalled() }) + // Why 'forbidden': a desktop that predates pairing.getEndpoints has it on neither its mobile + // allowlist nor its dispatcher, and the allowlist gate answers first — so scope refusal, not + // absence, is what an old desktop actually sends. See pairing-relay-rpc-unavailable.ts. + it('cleans pending state and leaves direct access unchanged for a scope-refusing old desktop', async () => { + const deps = dependencies() + const client = clientWith([ + { + id: 'rpc', + ok: false, + error: { code: 'forbidden', message: "Method 'pairing.getEndpoints' is not available" }, + _meta: { runtimeId: 'runtime' } + } + ]) + + await expect(upgradeDirectMobileRelay({ client, host, dependencies: deps })).resolves.toBeNull() + expect(deps.clearJournal).toHaveBeenCalledWith(host.id) + expect(deps.writeBundle).not.toHaveBeenCalled() + expect(deps.saveHost).not.toHaveBeenCalled() + }) + it('retains the durable journal when relay registration is temporarily unavailable', async () => { const deps = dependencies() const client = clientWith([success({ v: 1, relay: null })]) diff --git a/mobile/src/transport/mobile-relay-direct-upgrade.ts b/mobile/src/transport/mobile-relay-direct-upgrade.ts index ed019262bca..50fc496a9f6 100644 --- a/mobile/src/transport/mobile-relay-direct-upgrade.ts +++ b/mobile/src/transport/mobile-relay-direct-upgrade.ts @@ -22,10 +22,8 @@ import { } from './mobile-relay-direct-upgrade-journal' import type { RpcClient } from './rpc-client' import type { HostProfile } from './types' -import { - isMethodNotFoundRefusal, - requireRpcResultOrThrowCodedError -} from './rpc-acceptance-policies' +import { requireRpcResultOrThrowCodedError } from './rpc-acceptance-policies' +import { isPairingRelayRpcUnavailable } from './pairing-relay-rpc-unavailable' export type MobileRelayDirectUpgradeResult = { host: HostProfile @@ -83,7 +81,7 @@ export async function upgradeDirectMobileRelay(args: { reqId: journal.reqId, newResumeTokenHash: journal.pendingResumeTokenHash }) - if (isMethodNotFoundRefusal(provisionResponse)) { + if (isPairingRelayRpcUnavailable(provisionResponse)) { await dependencies.clearJournal(args.host.id) return null } @@ -142,7 +140,7 @@ async function getEndpoints( installReqId: string ): Promise { const response = await client.sendRequest('pairing.getEndpoints', { installReqId }) - if (isMethodNotFoundRefusal(response)) { + if (isPairingRelayRpcUnavailable(response)) { return 'method-not-found' } return PairingGetEndpointsResultSchema.parse(requireRpcResultOrThrowCodedError(response)) diff --git a/mobile/src/transport/pairing-relay-rpc-unavailable.ts b/mobile/src/transport/pairing-relay-rpc-unavailable.ts new file mode 100644 index 00000000000..e58a8e1dc8e --- /dev/null +++ b/mobile/src/transport/pairing-relay-rpc-unavailable.ts @@ -0,0 +1,25 @@ +import type { RpcResponse } from './types' + +/** + * Whether a desktop has told this phone it will not serve a Relay pairing RPC at all. + * + * Two codes mean the same thing here, and only one of them is reachable from an old desktop: + * + * - `method_not_found` — the method is on that desktop's mobile allowlist but nothing registers it + * (Relay compiled out, or the provider not wired). + * - `forbidden` — the method is not on its mobile allowlist. The allowlist gate runs *before* the + * RPC dispatcher (`runtime-rpc-websocket-dispatch.ts`), so a method a desktop predates is absent + * from both and this is the only answer a phone can get from it. Keying the "too old for Relay, + * stay on LAN" fallback on `method_not_found` alone therefore never fired against the exact + * desktop the fallback exists for — first-time pairing threw instead of committing a LAN host. + * + * A desktop that does serve the probes allowlists them, so `forbidden` can only ever mean "this + * desktop will not expose Relay pairing to a phone", which is what the caller falls back for. + * See docs/reference/remote-wire-compatibility.md — a scope refusal is not a missing method. + */ +export function isPairingRelayRpcUnavailable(response: RpcResponse): boolean { + return ( + !response.ok && + (response.error.code === 'method_not_found' || response.error.code === 'forbidden') + ) +} diff --git a/mobile/src/transport/pre-profile-pairing-coordinator.test.ts b/mobile/src/transport/pre-profile-pairing-coordinator.test.ts index 1729ae65dc7..6df10cdbdcd 100644 --- a/mobile/src/transport/pre-profile-pairing-coordinator.test.ts +++ b/mobile/src/transport/pre-profile-pairing-coordinator.test.ts @@ -316,6 +316,34 @@ describe('pre-profile pairing coordinator', () => { ]) }) + // Why 'forbidden' and not only 'method_not_found': the desktop's mobile allowlist gate runs + // before its RPC dispatcher, so a method a desktop predates is missing from both and the phone + // is refused by scope, never by absence. Keying the fallback on absence alone made the QR pair + // fail outright against the exact desktop the fallback exists for. + it('tolerates an old desktop scope refusal and commits a direct-only host', async () => { + const events: string[] = [] + const client = fakeClient([success({ version: '1.0.0' }), failure('forbidden')]) + const deps = dependencies(client, events) + + const attempt = startPreProfilePairing({ + offer: relayOffer, + timeoutMs: 5_000, + dependencies: deps + }) + await expect(attempt.result).resolves.toEqual({ hostId: `host-${now}` }) + + expect(deps.saveHost).toHaveBeenCalledWith( + expect.not.objectContaining({ endpoints: expect.anything() }) + ) + expect(events).toEqual([ + 'save-journal', + 'connect', + 'update-journal', + 'save-host', + 'clear-journal' + ]) + }) + it('uses relay-basis provisioning when only the relay reaches post-E2EE status', async () => { const direct = fakeClient([]) ;(direct.sendRequest as ReturnType).mockRejectedValue(new Error('LAN down')) diff --git a/mobile/src/transport/pre-profile-pairing-coordinator.ts b/mobile/src/transport/pre-profile-pairing-coordinator.ts index eb1f524334e..f6603389a5b 100644 --- a/mobile/src/transport/pre-profile-pairing-coordinator.ts +++ b/mobile/src/transport/pre-profile-pairing-coordinator.ts @@ -8,10 +8,7 @@ import { import { connect, type ConnectOptions } from './rpc-client' import { resolvePairingHostIdentity, saveHost } from './host-store' import type { HostProfile, PairingOffer } from './types' -import { - isMethodNotFoundRefusal, - requireRpcResultOrThrowCodedError -} from './rpc-acceptance-policies' +import { requireRpcResultOrThrowCodedError } from './rpc-acceptance-policies' import { createMobileRelayPairingJournal, type MobileRelayPairingJournal @@ -35,6 +32,7 @@ import { resolvePairingInviteThroughDirector } from './mobile-relay-invite-direc import { createRecoveringPairingRelayCandidate } from './pairing-relay-candidate' import { createPairingRelayLogger } from './pairing-relay-log' import { redactSocketEndpoint } from './socket-event-debug' +import { isPairingRelayRpcUnavailable } from './pairing-relay-rpc-unavailable' export type PreProfilePairingAttempt = { readonly result: Promise<{ hostId: string }> @@ -223,7 +221,7 @@ async function runPairing( reqId: journal.metadata.installReqId, newResumeTokenHash: journal.metadata.pendingResumeTokenHash }) - if (isMethodNotFoundRefusal(provision)) { + if (isPairingRelayRpcUnavailable(provision)) { if (winner.path !== 'direct') { throw new Error('relay pairing RPC unavailable after relay path authentication') } diff --git a/src/main/runtime/runtime-rpc-mobile-unknown-method-scope-refusal.test.ts b/src/main/runtime/runtime-rpc-mobile-unknown-method-scope-refusal.test.ts new file mode 100644 index 00000000000..89b8b74f9a8 --- /dev/null +++ b/src/main/runtime/runtime-rpc-mobile-unknown-method-scope-refusal.test.ts @@ -0,0 +1,61 @@ +import { mkdtempSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import { OrcaRuntimeRpcServer } from './runtime-rpc' +import { DeviceRegistry } from './device-registry' +import { createMobileRpcSurfaceRuntime } from './runtime-rpc-mobile-method-allowlist-fixtures' +import { MOBILE_RPC_METHOD_ALLOWLIST } from './runtime-rpc/runtime-rpc-mobile-method-allowlist' + +/** + * What a phone is told when this desktop has never heard of the method it called. + * + * The mobile allowlist gate runs before the dispatcher, so for a mobile-scoped device + * `method_not_found` is reachable only for a method that IS allowlisted but unregistered. + * A method an older desktop predates is missing from both, and the phone is told `forbidden`. + * + * That distinction is the whole skew contract for a phone probing a method to decide whether the + * desktop can serve it (`pairing.provisionRelay`, `pairing.getEndpoints`). Keying the "too old, + * stay on LAN" fallback on `method_not_found` alone never fires against a genuinely old desktop. + * See docs/reference/remote-wire-compatibility.md — a scope refusal is not a missing method. + */ +describe('an unknown method reaching a mobile-scoped device', () => { + const dispatchAs = async ( + scope: 'mobile' | 'runtime', + method: string + ): Promise> => { + const userDataPath = mkdtempSync(join(tmpdir(), 'orca-runtime-rpc-scope-')) + const { runtime } = createMobileRpcSurfaceRuntime() + const server = new OrcaRuntimeRpcServer({ runtime, userDataPath, enableWebSocket: false }) + server['deviceRegistry'] = new DeviceRegistry(userDataPath) + const device = server['deviceRegistry']!.addDevice('peer', scope) + const replies: Record[] = [] + await server['handleWebSocketMessage']( + JSON.stringify({ id: 'req_1', method, deviceToken: device.token, params: {} }), + (response) => replies.push(JSON.parse(response) as Record), + () => {} + ) + return replies[0] ?? {} + } + + it('answers forbidden, not method_not_found, for a method this build does not register', async () => { + const reply = await dispatchAs('mobile', 'pairing.methodThisBuildHasNeverHeardOf') + expect(reply.ok).toBe(false) + expect((reply.error as { code?: string }).code).toBe('forbidden') + expect((reply.error as { code?: string }).code).not.toBe('method_not_found') + }) + + it('answers method_not_found to a non-mobile peer for the same unknown method', async () => { + const reply = await dispatchAs('runtime', 'pairing.methodThisBuildHasNeverHeardOf') + expect(reply.ok).toBe(false) + expect((reply.error as { code?: string }).code).toBe('method_not_found') + }) + + // Why this build's own allowlist and not an older one: a desktop that serves the pairing probes + // allowlists them, so `forbidden` on either can only come from a build that predates them. That + // is what makes the phone's fallback a skew contract rather than a local error path. + it('allowlists both pairing probes, so forbidden on one can only mean an older desktop', () => { + expect(MOBILE_RPC_METHOD_ALLOWLIST.has('pairing.getEndpoints')).toBe(true) + expect(MOBILE_RPC_METHOD_ALLOWLIST.has('pairing.provisionRelay')).toBe(true) + }) +})