From 013bdbb02be919677fea8f4aee24e85a3771a996 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Thu, 6 Aug 2026 17:57:23 -0700 Subject: [PATCH] fix(mobile): never auto-advertise virtual bridge addresses for pairing Container/VM bridges stay manually pickable, but automatic defaults skip them so QR codes do not race an unreachable direct path. Relay pairs without a local address; LAN-only and runtime pairing fail closed on bridge-only hosts. --- src/main/ipc/mobile.test.ts | 95 +++++++++++++++++++ src/main/ipc/mobile.ts | 30 +++--- ...mobile-network-interface-selection.test.ts | 15 +++ .../mobile-network-interface-selection.ts | 9 +- src/shared/pairing-address-auto-selection.ts | 32 +++++++ 5 files changed, 159 insertions(+), 22 deletions(-) create mode 100644 src/shared/pairing-address-auto-selection.ts diff --git a/src/main/ipc/mobile.test.ts b/src/main/ipc/mobile.test.ts index bc90f0b2b41..6f17a9f310b 100644 --- a/src/main/ipc/mobile.test.ts +++ b/src/main/ipc/mobile.test.ts @@ -171,6 +171,84 @@ describe('registerMobileHandlers', () => { }) }) + it('never auto-advertises a bridge: a bridge-only host pairs over Relay with no address', async () => { + // Why: a bridge address the phone provably cannot reach must not become the default, and Relay + // needs no local address — so the QR ships without a direct path instead of an unreachable one. + networkInterfacesMock.mockReturnValue({ + docker0: [{ family: 'IPv4', internal: false, address: '172.17.0.1' }], + 'vEthernet (WSL)': [{ family: 'IPv4', internal: false, address: '172.28.80.1' }] + }) + const createMobilePairingOffer = vi.fn().mockResolvedValue({ + available: true, + pairingUrl: 'orca://pair#relay', + endpoint: 'ws://127.0.0.1:6768', + deviceId: 'mobile-bridge-only', + connectionMode: 'automatic' + }) + + registerMobileHandlers({ createMobilePairingOffer } as never) + + await expect(handlers.get('mobile:getPairingQR')?.(null, {})).resolves.toMatchObject({ + available: true, + connectionMode: 'automatic' + }) + expect(createMobilePairingOffer).toHaveBeenCalledWith( + expect.objectContaining({ address: null }) + ) + // The bridges stay pickable, just never automatically. + expect(handlers.get('mobile:listNetworkInterfaces')?.()).toEqual({ + interfaces: [ + { name: 'docker0', address: '172.17.0.1' }, + { name: 'vEthernet (WSL)', address: '172.28.80.1' } + ] + }) + }) + + it('refuses a LAN-only QR on a bridge-only host instead of advertising the bridge', async () => { + // Why: LAN has no Relay to fall back on, so a dead direct endpoint is worse than saying so — + // the guidance points at the picker, where the bridge is still selectable on purpose. + networkInterfacesMock.mockReturnValue({ + docker0: [{ family: 'IPv4', internal: false, address: '172.17.0.1' }] + }) + const createMobilePairingOffer = vi.fn() + + registerMobileHandlers({ createMobilePairingOffer } as never) + + await expect( + handlers.get('mobile:getPairingQR')?.(null, { connectionMode: 'local-only' }) + ).resolves.toMatchObject({ + available: false, + reason: 'invalid_advertised_endpoint' + }) + expect(createMobilePairingOffer).not.toHaveBeenCalled() + }) + + it('honors an explicitly picked bridge address', async () => { + // Why: exclusion is about the automatic default only — a user who knows their bridge is routable + // (a VM guest pairing with the host) must still be able to advertise it. + networkInterfacesMock.mockReturnValue({ + docker0: [{ family: 'IPv4', internal: false, address: '172.17.0.1' }], + en0: [{ family: 'IPv4', internal: false, address: '192.168.1.24' }] + }) + const createMobilePairingOffer = vi.fn().mockResolvedValue({ + available: true, + pairingUrl: 'orca://pair#bridge', + endpoint: 'ws://172.17.0.1:6768', + deviceId: 'mobile-bridge-pick', + connectionMode: 'local-only' + }) + + registerMobileHandlers({ createMobilePairingOffer } as never) + await handlers.get('mobile:getPairingQR')?.(null, { + address: '172.17.0.1', + connectionMode: 'local-only' + }) + + expect(createMobilePairingOffer).toHaveBeenCalledWith( + expect.objectContaining({ address: '172.17.0.1' }) + ) + }) + it('returns an IPv6 interface on an IPv6-only host (regression: was empty, breaking mobile pairing)', () => { networkInterfacesMock.mockReturnValue({ eth0: [ @@ -388,6 +466,23 @@ describe('registerMobileHandlers', () => { ensureNetworkExposure: vi.fn().mockResolvedValue(undefined) }) + it('reports runtime pairing unavailable rather than advertising a bridge', async () => { + // Why: runtime clients have no Relay fallback, so a bridge-only host has nothing reachable to + // advertise — and no widen should happen for a link that would be dead anyway. + networkInterfacesMock.mockReturnValue({ + docker0: [{ family: 'IPv4', internal: false, address: '172.17.0.1' }] + }) + const rpcServer = stubRuntimePairingServer('172.17.0.1:6768') + + registerMobileHandlers(rpcServer as never) + + await expect(handlers.get('mobile:getRuntimePairingUrl')?.(null, {})).resolves.toEqual({ + available: false + }) + expect(rpcServer.createPairingOffer).not.toHaveBeenCalled() + expect(rpcServer.ensureNetworkExposure).not.toHaveBeenCalled() + }) + // Why: every loopback form the address field accepts must behave identically once the user declared // "This computer only" — `splitHostPort`/`ws://` overrides all reach the handler verbatim, so gating on // the raw string alone treated `localhost:8443` and `ws://127.0.0.1:6768` as off-host. diff --git a/src/main/ipc/mobile.ts b/src/main/ipc/mobile.ts index 0b391acc71a..fa2c7ff8f57 100644 --- a/src/main/ipc/mobile.ts +++ b/src/main/ipc/mobile.ts @@ -2,6 +2,10 @@ import { app, ipcMain, shell, type IpcMainInvokeEvent } from 'electron' import { networkInterfaces } from 'node:os' import type { RuntimeAccessGrant } from '../../shared/runtime-access-grants' import type { MobilePairingConnectionMode } from '../../shared/mobile-pairing-connection-mode' +import { + isVirtualBridgeInterface, + selectAutoAdvertisedPairingAddress +} from '../../shared/pairing-address-auto-selection' import { classifyRemotePairingHostname } from '../../shared/remote-pairing-address' import type { RuntimePairingReach } from '../../shared/runtime-pairing-reach' import { isTailnetIPv4Address } from '../../shared/tailnet-address' @@ -35,19 +39,6 @@ function isProxyFakeIpIPv4Address(address: string): boolean { return /^198\.(?:18|19)\./.test(address) } -// Why: container/VM bridges are host-local — a phone can never reach docker0 or -// vmnet8 — but they enumerate as ordinary non-internal IPv4, so advertising one -// makes the direct path silently lose the pairing race and every session relay. -// Keyed on interface name, not subnet: Docker's 172.16/12 pool overlaps real -// corporate LANs, so an address test would demote genuine addresses. These stay -// pickable in the UI; they are only ranked below a real LAN address. -const VIRTUAL_BRIDGE_INTERFACE_PATTERN = - /^(?:docker|br-|virbr|vmnet|vboxnet|veth|lxcbr|cni|flannel|cali|bridge)|^vEthernet |VMware Network Adapter|VirtualBox Host-Only/i - -function isVirtualBridgeInterface(name: string): boolean { - return VIRTUAL_BRIDGE_INTERFACE_PATTERN.test(name) -} - // Why: the WebSocket transport advertises 0.0.0.0 as its endpoint, which isn't // connectable from a mobile device. We enumerate all non-internal IPv4 and // (non-link-local) IPv6 addresses so the user can choose which one to advertise @@ -78,7 +69,8 @@ function getNetworkInterfaces(): NetworkInterface[] { } // Why: prefer tailnet IPv4 first (most portable across networks), then other // IPv4, then IPv6 as a fallback for IPv6-only environments. Virtual bridges - // sort below both so they are never the auto-advertised default. + // sort below all of them and stay in the list so they remain explicitly + // pickable; the automatic default skips them entirely. return result.sort((a, b) => rankInterface(a) - rankInterface(b)) } @@ -90,9 +82,10 @@ function rankInterface({ name, address }: NetworkInterface): number { return (address.includes(':') ? 2 : 1) + bridgePenalty } +// Why: null when every enumerated interface is a host-local bridge, so a docker0-only host +// advertises no direct address instead of one the phone provably cannot reach. function getDefaultPairingAddress(): string | null { - const ifaces = getNetworkInterfaces() - return ifaces.length > 0 ? ifaces[0]!.address : null + return selectAutoAdvertisedPairingAddress(getNetworkInterfaces()) ?? null } // Why: only an explicit "This computer only" pick skips the one-way widen, and only when the address it @@ -156,7 +149,10 @@ export function registerMobileHandlers( // embed in the QR code. This supports overlay networks (Tailscale, // ZeroTier) where the default LAN IP isn't reachable from the phone. const ip = args?.address ?? getDefaultPairingAddress() - if (!ip) { + // Why: the local address is optional under Relay — the QR carries the relay invite, so a host + // with nothing auto-advertisable (only container bridges, or no interface at all) still pairs, + // just without a direct path to race. LAN-only has nothing to fall back on, so it fails closed. + if (!ip && args?.connectionMode === 'local-only') { return { available: false as const, reason: 'invalid_advertised_endpoint', diff --git a/src/renderer/src/components/settings/mobile-network-interface-selection.test.ts b/src/renderer/src/components/settings/mobile-network-interface-selection.test.ts index 0feaeaeeaef..7fa81a72b37 100644 --- a/src/renderer/src/components/settings/mobile-network-interface-selection.test.ts +++ b/src/renderer/src/components/settings/mobile-network-interface-selection.test.ts @@ -6,6 +6,7 @@ import { const LAN: MobileNetworkInterface = { name: 'en0', address: '192.168.1.24' } const TAILNET: MobileNetworkInterface = { name: 'tailscale0', address: '100.64.1.20' } +const BRIDGE: MobileNetworkInterface = { name: 'docker0', address: '172.17.0.1' } describe('selectRefreshedNetworkAddress', () => { // Why: regression for the manual-address branch the PR adds — a @@ -38,4 +39,18 @@ describe('selectRefreshedNetworkAddress', () => { it('clears the selection when no interfaces are available', () => { expect(selectRefreshedNetworkAddress(LAN.address, [])).toBeUndefined() }) + + it('skips a container bridge in favor of a reachable address', () => { + expect(selectRefreshedNetworkAddress(undefined, [BRIDGE, LAN])).toBe(LAN.address) + }) + + // Why: matches main's default — advertising nothing beats advertising docker0, and Relay + // pairs without a direct address. A divergence here would show an address the QR never carried. + it('selects no address when every interface is a container bridge', () => { + expect(selectRefreshedNetworkAddress(undefined, [BRIDGE])).toBeUndefined() + }) + + it('keeps a bridge the user picked explicitly', () => { + expect(selectRefreshedNetworkAddress(BRIDGE.address, [BRIDGE, LAN])).toBe(BRIDGE.address) + }) }) diff --git a/src/renderer/src/components/settings/mobile-network-interface-selection.ts b/src/renderer/src/components/settings/mobile-network-interface-selection.ts index 2edd817b68f..4b9e4782740 100644 --- a/src/renderer/src/components/settings/mobile-network-interface-selection.ts +++ b/src/renderer/src/components/settings/mobile-network-interface-selection.ts @@ -1,4 +1,4 @@ -import { isTailnetIPv4Address } from '../../../../shared/tailnet-address' +import { selectAutoAdvertisedPairingAddress } from '../../../../shared/pairing-address-auto-selection' export type MobileNetworkInterface = { name: string @@ -25,8 +25,7 @@ export function selectRefreshedNetworkAddress( ) { return currentAddress } - return ( - interfaces.find((iface) => isTailnetIPv4Address(iface.address))?.address ?? - interfaces[0]!.address - ) + // Why: shared with main so the picker never shows a default the QR didn't advertise. Undefined on + // a bridge-only host: Relay pairs without a direct address rather than offering an unreachable one. + return selectAutoAdvertisedPairingAddress(interfaces) } diff --git a/src/shared/pairing-address-auto-selection.ts b/src/shared/pairing-address-auto-selection.ts new file mode 100644 index 00000000000..b263b670466 --- /dev/null +++ b/src/shared/pairing-address-auto-selection.ts @@ -0,0 +1,32 @@ +import { isTailnetIPv4Address } from './tailnet-address' + +export type PairingNetworkInterface = { + name: string + address: string +} + +// Why: container/VM bridges are host-local — a phone can never reach docker0 or +// vmnet8 — but they enumerate as ordinary non-internal IPv4, so advertising one +// makes the direct path silently lose the pairing race and every session relay. +// Keyed on interface name, not subnet: Docker's 172.16/12 pool overlaps real +// corporate LANs, so an address test would demote genuine addresses. +const VIRTUAL_BRIDGE_INTERFACE_PATTERN = + /^(?:docker|br-|virbr|vmnet|vboxnet|veth|lxcbr|cni|flannel|cali|bridge)|^vEthernet |VMware Network Adapter|VirtualBox Host-Only/i + +export function isVirtualBridgeInterface(name: string): boolean { + return VIRTUAL_BRIDGE_INTERFACE_PATTERN.test(name) +} + +// Why: main mints the QR from this and the renderer's picker shows its result, so both sides must +// agree — a divergence would display one address while the QR advertises another. Bridges stay +// pickable for an explicit choice but are never chosen here; `undefined` means "advertise no direct +// address", which Relay tolerates (it carries its own invite) and the LAN-only path refuses. +export function selectAutoAdvertisedPairingAddress( + interfaces: readonly PairingNetworkInterface[] +): string | undefined { + const advertisable = interfaces.filter((iface) => !isVirtualBridgeInterface(iface.name)) + return ( + advertisable.find((iface) => isTailnetIPv4Address(iface.address))?.address ?? + advertisable[0]?.address + ) +}