mirror of
https://github.com/stablyai/orca.git
synced 2026-09-28 16:02:45 +00:00
fix(mobile): a scope refusal is not a missing method on the pairing probes
A phone decides whether a desktop can serve Relay by calling pairing.getEndpoints and pairing.provisionRelay and reading the failure. Both call sites treated only `method_not_found` as "this desktop is too old for Relay, stay on LAN" — and that code is the one answer a genuinely old desktop can never give. runtime-rpc-websocket-dispatch.ts runs the mobile allowlist gate BEFORE the RPC 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 absent from both lists and the phone is refused with `forbidden`. The fallback could never fire against the exact desktop it exists for. In pre-profile-pairing-coordinator.ts that is the first-time QR pairing flow, so the failure is not a degraded mode — it throws and the phone ends up with no host profile at all: AssertionError: promise rejected "Error: forbidden: forbidden" instead of resolving ❯ requireSuccess src/transport/pre-profile-pairing-coordinator.ts:288 Both local `isMethodNotFound` predicates are replaced by one shared `isPairingRelayRpcUnavailable` that accepts either code. Safe in both directions: a desktop that serves the probes allowlists them, so `forbidden` on one can only mean "this desktop will not expose Relay pairing to a phone", which is exactly what the caller falls back for. See docs/reference/remote-wire-compatibility.md. The desktop-side test pins the mechanism rather than restating it: a mobile-scoped device gets `forbidden` for an unregistered method while a runtime-scoped peer gets `method_not_found` for the same one. Measured: mobile `vitest run src/transport` 98 files / 723 passed. Mutating away the `forbidden` arm fails both new cases; mutating away the `method_not_found` arm fails both pre-existing cases; mutating the desktop gate to emit `method_not_found` fails the new desktop case. No survivors.
This commit is contained in:
@@ -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 })])
|
||||
|
||||
@@ -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<PairingGetEndpointsResult | 'method-not-found'> {
|
||||
const response = await client.sendRequest('pairing.getEndpoints', { installReqId })
|
||||
if (isMethodNotFoundRefusal(response)) {
|
||||
if (isPairingRelayRpcUnavailable(response)) {
|
||||
return 'method-not-found'
|
||||
}
|
||||
return PairingGetEndpointsResultSchema.parse(requireRpcResultOrThrowCodedError(response))
|
||||
|
||||
@@ -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')
|
||||
)
|
||||
}
|
||||
@@ -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<typeof vi.fn>).mockRejectedValue(new Error('LAN down'))
|
||||
|
||||
@@ -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')
|
||||
}
|
||||
|
||||
@@ -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<Record<string, unknown>> => {
|
||||
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<string, unknown>[] = []
|
||||
await server['handleWebSocketMessage'](
|
||||
JSON.stringify({ id: 'req_1', method, deviceToken: device.token, params: {} }),
|
||||
(response) => replies.push(JSON.parse(response) as Record<string, unknown>),
|
||||
() => {}
|
||||
)
|
||||
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)
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user