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.
2026-10-20 16:03:20 +00:00 · 2026-08-06 17:57:25 -07:00
parent 87d1d1a8dc
commit 013bdbb02b
5 changed files with 159 additions and 22 deletions
@@ -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.
@@ -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',
@@ -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)
+  })
 })
@@ -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)
 }
@@ -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
+  )
+}