diff --git a/packages/platform-android/src/adb-provider-scope.test.ts b/packages/platform-android/src/adb-provider-scope.test.ts index af4eb66b2..22cacbce4 100644 --- a/packages/platform-android/src/adb-provider-scope.test.ts +++ b/packages/platform-android/src/adb-provider-scope.test.ts @@ -3,9 +3,11 @@ import type { DeviceInfo } from '@agent-device/kernel/device'; import { bindAndroidAdbHostStub } from './adb-host.fixtures.ts'; import { createLocalAndroidAdbProvider, + createDeviceAdbExecutor, resolveAndroidAdbExecutor, resolveAndroidAdbProvider, resolveAndroidTextInjector, + resolveAndroidTouchProvider, resolveScopedAndroidAdbBackgroundTransport, withAndroidAdbProvider, } from './adb-provider-scope.ts'; @@ -95,7 +97,7 @@ test('the installed override routes only normalized device-scoped adb calls to t expect(providerCalls).toEqual([['shell', 'ls']]); }); -test('a managed port scope routes host adb and matching serial calls to its private server', async () => { +test('a managed port scope rejects foreign serials before host adb execution', async () => { const hostCalls: Array<{ args: string[]; serverPort?: number }> = []; bindAndroidAdbHostStub({ execHostAdb: async (args, options) => { @@ -109,16 +111,19 @@ test('a managed port scope routes host adb and matching serial calls to its priv { serial: DEVICE.id, serverPort: 15_037 }, async () => { await runAndroidHostAdb(['devices']); + await runAndroidHostAdb(['shell', 'id'], { env: { ANDROID_SERIAL: OTHER.id } }); await runAndroidHostAdb(['-s', DEVICE.id, 'shell', 'getprop']); - await runAndroidHostAdb(['-s', OTHER.id, 'shell', 'getprop']); + await expect(runAndroidHostAdb(['-s', OTHER.id, 'shell', 'getprop'])).rejects.toMatchObject({ + details: { reason: 'managed-device-transport-mismatch' }, + }); }, ); await runAndroidHostAdb(['devices']); expect(hostCalls).toEqual([ - { args: ['devices'], serverPort: 15_037 }, + { args: ['-s', DEVICE.id, 'devices'], serverPort: 15_037 }, + { args: ['-s', DEVICE.id, 'shell', 'id'], serverPort: 15_037 }, { args: ['-s', DEVICE.id, 'shell', 'getprop'], serverPort: 15_037 }, - { args: ['-s', OTHER.id, 'shell', 'getprop'] }, { args: ['devices'] }, ]); }); @@ -155,7 +160,9 @@ test('a managed port scope classifies absolute adb commands and preserves the de ['-s', DEVICE.id, 'shell', 'ls'], {}, ); - expect(captured?.('adb', ['-s', OTHER.id, 'shell', 'ls'], {})).toBeUndefined(); + expect(() => captured?.('adb', ['-s', OTHER.id, 'shell', 'ls'], {})).toThrowError( + expect.objectContaining({ details: { reason: 'managed-device-transport-mismatch' } }), + ); expect(captured?.('emulator', ['-list-avds'], {})).toBeUndefined(); expect(global).toBeDefined(); expect(matching).toBeDefined(); @@ -164,10 +171,69 @@ test('a managed port scope classifies absolute adb commands and preserves the de }, ); - expect(hostCalls).toEqual([['devices', '-l']]); + expect(hostCalls).toEqual([['-s', DEVICE.id, 'devices', '-l']]); expect(providerCalls).toEqual([['shell', 'ls']]); }); +test('managed port scopes refuse foreign device resolvers before returning a local transport', async () => { + bindAndroidAdbHostStub(); + await withAndroidAdbProvider( + { exec: async () => ok() }, + { serial: DEVICE.id, serverPort: 15_037 }, + async () => { + for (const resolve of [ + resolveAndroidAdbExecutor, + resolveAndroidAdbProvider, + resolveScopedAndroidAdbBackgroundTransport, + resolveAndroidTextInjector, + resolveAndroidTouchProvider, + ]) { + expect(() => resolve(OTHER)).toThrowError( + expect.objectContaining({ details: { reason: 'managed-device-transport-mismatch' } }), + ); + } + }, + ); +}); + +test('private-port execution contains local transports constructed before entering the scope', async () => { + const calls: Array<{ serial: string; serverPort?: number }> = []; + bindAndroidAdbHostStub({ + execSerialAdb: async (serial, _args, options) => { + calls.push({ serial, serverPort: options?.serverPort }); + return ok(); + }, + spawnSerialAdb: (serial, _args, options) => { + calls.push({ serial, serverPort: options?.serverPort }); + return undefined as never; + }, + }); + const matching = createLocalAndroidAdbProvider(DEVICE); + const foreign = createLocalAndroidAdbProvider(OTHER); + const wrongPort = createDeviceAdbExecutor(DEVICE, { serverPort: 15_038 }); + await withAndroidAdbProvider( + { exec: async () => ok() }, + { serial: DEVICE.id, serverPort: 15_037 }, + async () => { + await matching.exec(['shell', 'id']); + matching.spawn?.(['logcat']); + await expect(foreign.exec(['shell', 'id'])).rejects.toMatchObject({ + details: { reason: 'managed-device-transport-mismatch' }, + }); + expect(() => foreign.spawn?.(['logcat'])).toThrowError( + expect.objectContaining({ details: { reason: 'managed-device-transport-mismatch' } }), + ); + await expect(wrongPort(['shell', 'id'])).rejects.toMatchObject({ + details: { reason: 'managed-device-transport-mismatch' }, + }); + }, + ); + expect(calls).toEqual([ + { serial: DEVICE.id, serverPort: 15_037 }, + { serial: DEVICE.id, serverPort: 15_037 }, + ]); +}); + test('a managed port scope keeps shell -s arguments on the private transport', async () => { const hostCalls: Array<{ args: string[]; serverPort?: number }> = []; let captured: @@ -196,8 +262,8 @@ test('a managed port scope keeps shell -s arguments on the private transport', a ); expect(hostCalls).toEqual([ - { args: ['shell', 'echo', '-s', OTHER.id], serverPort: 15_037 }, - { args: ['shell', 'echo', '-s', OTHER.id], serverPort: 15_037 }, + { args: ['-s', DEVICE.id, 'shell', 'echo', '-s', OTHER.id], serverPort: 15_037 }, + { args: ['-s', DEVICE.id, 'shell', 'echo', '-s', OTHER.id], serverPort: 15_037 }, ]); }); diff --git a/packages/platform-android/src/adb-provider-scope.ts b/packages/platform-android/src/adb-provider-scope.ts index 760269943..d2a63b2b9 100644 --- a/packages/platform-android/src/adb-provider-scope.ts +++ b/packages/platform-android/src/adb-provider-scope.ts @@ -1,6 +1,7 @@ import { AsyncLocalStorage } from 'node:async_hooks'; import path from 'node:path'; import type { DeviceInfo } from '@agent-device/kernel/device'; +import { AppError } from '@agent-device/kernel/errors'; import { requireAndroidAdbHost, withAndroidHostAdbTransport, @@ -42,23 +43,25 @@ export function createDeviceAdbExecutor( } function createSerialAdbExecutor(serial: string, serverPort?: number): AndroidAdbExecutor { - return withAdbFailureHints( - async (args, options) => - await requireAndroidAdbHost().execSerialAdb( - serial, - args, - serverPort === undefined ? options : { ...options, serverPort }, - ), - ); + return withAdbFailureHints(async (args, options) => { + const port = scopedServerPort(serial, serverPort); + return await requireAndroidAdbHost().execSerialAdb( + serial, + args, + port === undefined ? options : { ...options, serverPort: port }, + ); + }); } function createSerialAdbSpawner(serial: string, serverPort?: number): AndroidAdbSpawner { - return (args, options) => - requireAndroidAdbHost().spawnSerialAdb( + return (args, options) => { + const port = scopedServerPort(serial, serverPort); + return requireAndroidAdbHost().spawnSerialAdb( serial, args, - serverPort === undefined ? options : { ...options, serverPort }, + port === undefined ? options : { ...options, serverPort: port }, ); + }; } export function createLocalAndroidAdbProvider( @@ -83,7 +86,7 @@ export function resolveAndroidAdbExecutor( device: DeviceInfo, executor?: AndroidAdbExecutor, ): AndroidAdbExecutor { - const scoped = androidAdbProviderScope.getStore(); + const scoped = scopeForDevice(device); if (executor) return executor; if (scoped?.serial === device.id) return scoped.provider.exec; return createDeviceAdbExecutor(device); @@ -93,8 +96,8 @@ export function resolveAndroidAdbProvider( device: DeviceInfo, provider?: AndroidAdbProvider | AndroidAdbExecutor, ): AndroidAdbProvider { + const scoped = scopeForDevice(device); if (provider) return normalizeAndroidAdbProvider(provider); - const scoped = androidAdbProviderScope.getStore(); return scoped?.serial === device.id ? normalizeAndroidAdbProvider(scoped.provider) : createLocalAndroidAdbProvider(device); @@ -108,7 +111,7 @@ export function resolveAndroidAdbProvider( export function resolveScopedAndroidAdbBackgroundTransport( device: DeviceInfo, ): ScopedAndroidAdbBackgroundTransport { - const scoped = androidAdbProviderScope.getStore(); + const scoped = scopeForDevice(device); if (scoped?.serial !== device.id) return { mode: 'local' }; return { mode: 'transport-composed', @@ -117,12 +120,12 @@ export function resolveScopedAndroidAdbBackgroundTransport( } export function resolveAndroidTextInjector(device: DeviceInfo): AndroidTextInjector | undefined { - const scoped = androidAdbProviderScope.getStore(); + const scoped = scopeForDevice(device); return scoped?.serial === device.id ? scoped.provider.text : undefined; } export function resolveAndroidTouchProvider(device: DeviceInfo): AndroidTouchProvider | undefined { - const scoped = androidAdbProviderScope.getStore(); + const scoped = scopeForDevice(device); return scoped?.serial === device.id && scoped.provider.touch ? scoped.provider : undefined; } @@ -170,6 +173,7 @@ function createAndroidCommandExecutorOverride( if (!isAdbCommand(cmd)) return undefined; if (scope.serverPort === undefined && cmd !== 'adb') return undefined; const serial = readAdbSerial(args); + requireScopedSerial(scope, serial); if (serial && serial !== scope.serial) return undefined; if (serial === scope.serial) { const providerArgs = stripAdbSerialArgs(args, scope.serial); @@ -181,7 +185,7 @@ function createAndroidCommandExecutorOverride( if (scope.serverPort === undefined) return undefined; return requireAndroidAdbHost().withoutAdbCommandExecutorOverride( async () => - await requireAndroidAdbHost().execHostAdb(args, { + await requireAndroidAdbHost().execHostAdb(['-s', scope.serial, ...args], { ...options, allowFailure: true, serverPort: scope.serverPort, @@ -193,18 +197,48 @@ function createAndroidCommandExecutorOverride( function createScopedHostTransport(scope: AndroidAdbProviderScope): AndroidAdbHostTransport { return async (args: string[], options?: AndroidAdbExecutorOptions) => { const serial = readAdbSerial(args); + requireScopedSerial(scope, serial); const host = requireAndroidAdbHost(); return await host.withoutAdbCommandExecutorOverride( async () => - await host.execHostAdb(args, { + await host.execHostAdb(serial === undefined ? ['-s', scope.serial, ...args] : args, { ...options, allowFailure: true, - ...(serial && serial !== scope.serial ? {} : { serverPort: scope.serverPort }), + serverPort: scope.serverPort, }), ); }; } +function scopeForDevice(device: DeviceInfo): AndroidAdbProviderScope | undefined { + const scoped = androidAdbProviderScope.getStore(); + requireScopedSerial(scoped, device.id); + return scoped; +} + +function requireScopedSerial( + scope: AndroidAdbProviderScope | undefined, + serial: string | undefined, +) { + if (scope?.serverPort !== undefined && serial !== undefined && serial !== scope.serial) { + throw new AppError('COMMAND_FAILED', 'Managed ADB transport cannot address another device.', { + reason: 'managed-device-transport-mismatch', + }); + } +} + +function scopedServerPort(serial: string, requested: number | undefined): number | undefined { + const scope = androidAdbProviderScope.getStore(); + requireScopedSerial(scope, serial); + if (scope?.serverPort === undefined) return requested; + if (requested !== undefined && requested !== scope.serverPort) { + throw new AppError('COMMAND_FAILED', 'Managed ADB transport cannot select another server.', { + reason: 'managed-device-transport-mismatch', + }); + } + return scope.serverPort; +} + function isAdbCommand(command: string): boolean { const executable = path.basename(command).replace(/\.(?:com|exe|bat|cmd)$/i, ''); return executable === 'adb'; diff --git a/src/managed-device-reachability.test.ts b/src/managed-device-reachability.test.ts index e35b253d5..7dd762991 100644 --- a/src/managed-device-reachability.test.ts +++ b/src/managed-device-reachability.test.ts @@ -166,8 +166,11 @@ test.skipIf(process.platform === 'win32')( booted: true, }, ], - host: { args: ['-P', '15037', 'devices'], port: '15037' }, - hostWithWrongPort: { args: ['-P', '15037', 'devices'], port: '15037' }, + host: { args: ['-P', '15037', '-s', 'emulator-15037', 'devices'], port: '15037' }, + hostWithWrongPort: { + args: ['-P', '15037', '-s', 'emulator-15037', 'devices'], + port: '15037', + }, serial: { args: ['-P', '15037', '-s', 'emulator-15037', 'shell', 'id'], port: '15037', diff --git a/src/platform-runtime-android-adb-host.test.ts b/src/platform-runtime-android-adb-host.test.ts index b56390130..27ff6068d 100644 --- a/src/platform-runtime-android-adb-host.test.ts +++ b/src/platform-runtime-android-adb-host.test.ts @@ -4,7 +4,7 @@ import path from 'node:path'; import { test } from 'vitest'; import { AppError } from '@agent-device/kernel/errors'; import { - createDeviceAdbExecutor, + createLocalAndroidAdbProvider, runAndroidHostAdb, } from '@agent-device/platform-android/mechanics'; import { mkdtempForTestSync } from './__tests__/test-utils/tmp-dir.ts'; @@ -50,14 +50,16 @@ test.skipIf(process.platform === 'win32')( fs.writeFileSync( adbPath, '#!/usr/bin/env node\n' + - 'process.stdout.write(JSON.stringify({args: process.argv.slice(2), port: process.env.ANDROID_ADB_SERVER_PORT ?? null}));\n', + 'process.stdout.write(JSON.stringify({args: process.argv.slice(2), port: process.env.ANDROID_ADB_SERVER_PORT ?? null, address: process.env.ANDROID_ADB_SERVER_ADDRESS ?? null, socket: process.env.ADB_SERVER_SOCKET ?? null}));\n', ); fs.chmodSync(adbPath, 0o755); const previousPath = process.env.PATH; const previousPort = process.env.ANDROID_ADB_SERVER_PORT; + const previousSocket = process.env.ADB_SERVER_SOCKET; + process.env.ADB_SERVER_SOCKET = 'tcp:inherited.example:9999'; process.env.PATH = `${tmpDir}${path.delimiter}${previousPath ?? ''}`; try { - const adb = createDeviceAdbExecutor( + const provider = createLocalAndroidAdbProvider( { platform: 'android', id: 'emulator-5554', @@ -67,6 +69,7 @@ test.skipIf(process.platform === 'win32')( }, { serverPort: 15_037 }, ); + const adb = provider.exec; const serial = JSON.parse((await adb(['shell', 'id'])).stdout) as { args: string[]; port: string | null; @@ -78,23 +81,72 @@ test.skipIf(process.platform === 'win32')( const serialWithWrongEnvironment = JSON.parse( ( await adb(['shell', 'id'], { - env: { ANDROID_ADB_SERVER_PORT: '9999' }, + env: { + ANDROID_ADB_SERVER_PORT: '9999', + ANDROID_ADB_SERVER_ADDRESS: 'foreign.example', + ADB_SERVER_SOCKET: 'tcp:foreign.example:9999', + }, }) ).stdout, ) as { args: string[]; port: string | null }; const host = JSON.parse( (await runAndroidHostAdb(['-P', '9999', 'devices'], { serverPort: 15_038 })).stdout, ) as { args: string[]; port: string | null }; + for (const selector of [ + ['-H', 'foreign.example'], + ['-L', 'tcp:foreign.example:5037'], + ['-t', '42'], + ['-s', 'foreign-device'], + ['-P9999'], + ['-d'], + ['-e'], + ['nodaemon', '-H', 'foreign.example'], + ['server', '-P', '9999'], + ['fork-server', '-s', 'foreign-device'], + ['kill-server'], + ['start-server'], + ['connect', 'foreign.example'], + ['disconnect'], + ['reconnect', 'offline'], + ['attach', 'foreign-device'], + ['detach', 'foreign-device'], + ['pair', 'foreign.example', '123456'], + ['wait-for-device', 'kill-server'], + ['wait-for-device', 'disconnect'], + ['wait-for-any-device', 'pair', 'foreign.example', '123456'], + ]) { + await assert.rejects(adb([...selector, 'shell', 'id']), { + details: { reason: 'managed-device-transport-mismatch' }, + }); + assert.throws(() => provider.spawn?.([...selector, 'shell', 'id']), { + details: { reason: 'managed-device-transport-mismatch' }, + }); + } assert.deepEqual(serial, { args: ['-P', '15037', '-s', 'emulator-5554', 'shell', 'id'], port: '15037', + address: '127.0.0.1', + socket: null, }); assert.deepEqual(serialWithWrongPort, serial); assert.deepEqual(serialWithWrongEnvironment, serial); - assert.deepEqual(host, { args: ['-P', '15038', 'devices'], port: '15038' }); + const waited = JSON.parse((await adb(['wait-for-device', 'shell', 'id'])).stdout); + assert.deepEqual(waited, { + ...serial, + args: ['-P', '15037', '-s', 'emulator-5554', 'wait-for-device', 'shell', 'id'], + }); + assert.deepEqual(host, { + args: ['-P', '15038', 'devices'], + port: '15038', + address: '127.0.0.1', + socket: null, + }); assert.equal(process.env.ANDROID_ADB_SERVER_PORT, previousPort); + assert.equal(process.env.ADB_SERVER_SOCKET, 'tcp:inherited.example:9999'); } finally { + if (previousSocket === undefined) delete process.env.ADB_SERVER_SOCKET; + else process.env.ADB_SERVER_SOCKET = previousSocket; if (previousPath === undefined) delete process.env.PATH; else process.env.PATH = previousPath; } diff --git a/src/platform-runtime-android-adb-host.ts b/src/platform-runtime-android-adb-host.ts index b80ef02a5..a3df81969 100644 --- a/src/platform-runtime-android-adb-host.ts +++ b/src/platform-runtime-android-adb-host.ts @@ -1,5 +1,6 @@ import { bindAndroidAdbHost } from '@agent-device/platform-android/adb-host'; import type { AndroidAdbExecutorOptions } from '@agent-device/platform-android/mechanics'; +import { AppError } from '@agent-device/kernel/errors'; import { createHash, randomUUID } from 'node:crypto'; import { coerceExecResult, @@ -152,7 +153,9 @@ function adbInvocation( env: { ...environment, ...(withoutServerPort.env ?? {}), + ADB_SERVER_SOCKET: undefined, ANDROID_ADB_SERVER_PORT: String(serverPort), + ANDROID_ADB_SERVER_ADDRESS: '127.0.0.1', }, }, }; @@ -161,23 +164,51 @@ function adbInvocation( function withServerPort(args: string[], serverPort: number): string[] { const normalized = ['-P', String(serverPort)]; let index = 0; + let serial: string | undefined; while (index < args.length) { const argument = args[index]; if (argument === '-P') { index += 2; continue; } - if (argument === '-s' || argument === '-H' || argument === '-L') { + if (argument === '-s') { + if (serial !== undefined && serial !== args[index + 1]) throw transportMismatch(); + serial = args[index + 1]; normalized.push(argument, args[index + 1]!); index += 2; continue; } - if (argument === '-a' || argument === '-d' || argument === '-e') { - normalized.push(argument); - index += 1; - continue; - } + if (argument?.startsWith('-')) throw transportMismatch(); break; } - return [...normalized, ...args.slice(index)]; + const command = args.slice(index); + assertManagedAdbCommand(command); + return [...normalized, ...command]; +} + +function assertManagedAdbCommand(args: string[]): void { + const command = args.find((argument) => !argument.startsWith('wait-for-')); + if ( + [ + 'nodaemon', + 'server', + 'fork-server', + 'kill-server', + 'start-server', + 'connect', + 'disconnect', + 'reconnect', + 'attach', + 'detach', + 'pair', + ].includes(command ?? '') + ) { + throw transportMismatch(); + } +} + +function transportMismatch(): AppError { + return new AppError('COMMAND_FAILED', 'Managed ADB transport cannot select another target.', { + reason: 'managed-device-transport-mismatch', + }); }