Skip to content

Commit 07e2508

Browse files
authored
fix: contain managed ADB transport to its lease target (#2311)
* fix: contain managed ADB transport to its lease target * fix: block managed ADB server administration and inherited sockets
1 parent 6b0e5d6 commit 07e2508

5 files changed

Lines changed: 227 additions & 41 deletions

File tree

packages/platform-android/src/adb-provider-scope.test.ts

Lines changed: 74 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@ import type { DeviceInfo } from '@agent-device/kernel/device';
33
import { bindAndroidAdbHostStub } from './adb-host.fixtures.ts';
44
import {
55
createLocalAndroidAdbProvider,
6+
createDeviceAdbExecutor,
67
resolveAndroidAdbExecutor,
78
resolveAndroidAdbProvider,
89
resolveAndroidTextInjector,
10+
resolveAndroidTouchProvider,
911
resolveScopedAndroidAdbBackgroundTransport,
1012
withAndroidAdbProvider,
1113
} from './adb-provider-scope.ts';
@@ -95,7 +97,7 @@ test('the installed override routes only normalized device-scoped adb calls to t
9597
expect(providerCalls).toEqual([['shell', 'ls']]);
9698
});
9799

98-
test('a managed port scope routes host adb and matching serial calls to its private server', async () => {
100+
test('a managed port scope rejects foreign serials before host adb execution', async () => {
99101
const hostCalls: Array<{ args: string[]; serverPort?: number }> = [];
100102
bindAndroidAdbHostStub({
101103
execHostAdb: async (args, options) => {
@@ -109,16 +111,19 @@ test('a managed port scope routes host adb and matching serial calls to its priv
109111
{ serial: DEVICE.id, serverPort: 15_037 },
110112
async () => {
111113
await runAndroidHostAdb(['devices']);
114+
await runAndroidHostAdb(['shell', 'id'], { env: { ANDROID_SERIAL: OTHER.id } });
112115
await runAndroidHostAdb(['-s', DEVICE.id, 'shell', 'getprop']);
113-
await runAndroidHostAdb(['-s', OTHER.id, 'shell', 'getprop']);
116+
await expect(runAndroidHostAdb(['-s', OTHER.id, 'shell', 'getprop'])).rejects.toMatchObject({
117+
details: { reason: 'managed-device-transport-mismatch' },
118+
});
114119
},
115120
);
116121
await runAndroidHostAdb(['devices']);
117122

118123
expect(hostCalls).toEqual([
119-
{ args: ['devices'], serverPort: 15_037 },
124+
{ args: ['-s', DEVICE.id, 'devices'], serverPort: 15_037 },
125+
{ args: ['-s', DEVICE.id, 'shell', 'id'], serverPort: 15_037 },
120126
{ args: ['-s', DEVICE.id, 'shell', 'getprop'], serverPort: 15_037 },
121-
{ args: ['-s', OTHER.id, 'shell', 'getprop'] },
122127
{ args: ['devices'] },
123128
]);
124129
});
@@ -155,7 +160,9 @@ test('a managed port scope classifies absolute adb commands and preserves the de
155160
['-s', DEVICE.id, 'shell', 'ls'],
156161
{},
157162
);
158-
expect(captured?.('adb', ['-s', OTHER.id, 'shell', 'ls'], {})).toBeUndefined();
163+
expect(() => captured?.('adb', ['-s', OTHER.id, 'shell', 'ls'], {})).toThrowError(
164+
expect.objectContaining({ details: { reason: 'managed-device-transport-mismatch' } }),
165+
);
159166
expect(captured?.('emulator', ['-list-avds'], {})).toBeUndefined();
160167
expect(global).toBeDefined();
161168
expect(matching).toBeDefined();
@@ -164,10 +171,69 @@ test('a managed port scope classifies absolute adb commands and preserves the de
164171
},
165172
);
166173

167-
expect(hostCalls).toEqual([['devices', '-l']]);
174+
expect(hostCalls).toEqual([['-s', DEVICE.id, 'devices', '-l']]);
168175
expect(providerCalls).toEqual([['shell', 'ls']]);
169176
});
170177

178+
test('managed port scopes refuse foreign device resolvers before returning a local transport', async () => {
179+
bindAndroidAdbHostStub();
180+
await withAndroidAdbProvider(
181+
{ exec: async () => ok() },
182+
{ serial: DEVICE.id, serverPort: 15_037 },
183+
async () => {
184+
for (const resolve of [
185+
resolveAndroidAdbExecutor,
186+
resolveAndroidAdbProvider,
187+
resolveScopedAndroidAdbBackgroundTransport,
188+
resolveAndroidTextInjector,
189+
resolveAndroidTouchProvider,
190+
]) {
191+
expect(() => resolve(OTHER)).toThrowError(
192+
expect.objectContaining({ details: { reason: 'managed-device-transport-mismatch' } }),
193+
);
194+
}
195+
},
196+
);
197+
});
198+
199+
test('private-port execution contains local transports constructed before entering the scope', async () => {
200+
const calls: Array<{ serial: string; serverPort?: number }> = [];
201+
bindAndroidAdbHostStub({
202+
execSerialAdb: async (serial, _args, options) => {
203+
calls.push({ serial, serverPort: options?.serverPort });
204+
return ok();
205+
},
206+
spawnSerialAdb: (serial, _args, options) => {
207+
calls.push({ serial, serverPort: options?.serverPort });
208+
return undefined as never;
209+
},
210+
});
211+
const matching = createLocalAndroidAdbProvider(DEVICE);
212+
const foreign = createLocalAndroidAdbProvider(OTHER);
213+
const wrongPort = createDeviceAdbExecutor(DEVICE, { serverPort: 15_038 });
214+
await withAndroidAdbProvider(
215+
{ exec: async () => ok() },
216+
{ serial: DEVICE.id, serverPort: 15_037 },
217+
async () => {
218+
await matching.exec(['shell', 'id']);
219+
matching.spawn?.(['logcat']);
220+
await expect(foreign.exec(['shell', 'id'])).rejects.toMatchObject({
221+
details: { reason: 'managed-device-transport-mismatch' },
222+
});
223+
expect(() => foreign.spawn?.(['logcat'])).toThrowError(
224+
expect.objectContaining({ details: { reason: 'managed-device-transport-mismatch' } }),
225+
);
226+
await expect(wrongPort(['shell', 'id'])).rejects.toMatchObject({
227+
details: { reason: 'managed-device-transport-mismatch' },
228+
});
229+
},
230+
);
231+
expect(calls).toEqual([
232+
{ serial: DEVICE.id, serverPort: 15_037 },
233+
{ serial: DEVICE.id, serverPort: 15_037 },
234+
]);
235+
});
236+
171237
test('a managed port scope keeps shell -s arguments on the private transport', async () => {
172238
const hostCalls: Array<{ args: string[]; serverPort?: number }> = [];
173239
let captured:
@@ -196,8 +262,8 @@ test('a managed port scope keeps shell -s arguments on the private transport', a
196262
);
197263

198264
expect(hostCalls).toEqual([
199-
{ args: ['shell', 'echo', '-s', OTHER.id], serverPort: 15_037 },
200-
{ args: ['shell', 'echo', '-s', OTHER.id], serverPort: 15_037 },
265+
{ args: ['-s', DEVICE.id, 'shell', 'echo', '-s', OTHER.id], serverPort: 15_037 },
266+
{ args: ['-s', DEVICE.id, 'shell', 'echo', '-s', OTHER.id], serverPort: 15_037 },
201267
]);
202268
});
203269

packages/platform-android/src/adb-provider-scope.ts

Lines changed: 53 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { AsyncLocalStorage } from 'node:async_hooks';
22
import path from 'node:path';
33
import type { DeviceInfo } from '@agent-device/kernel/device';
4+
import { AppError } from '@agent-device/kernel/errors';
45
import {
56
requireAndroidAdbHost,
67
withAndroidHostAdbTransport,
@@ -42,23 +43,25 @@ export function createDeviceAdbExecutor(
4243
}
4344

4445
function createSerialAdbExecutor(serial: string, serverPort?: number): AndroidAdbExecutor {
45-
return withAdbFailureHints(
46-
async (args, options) =>
47-
await requireAndroidAdbHost().execSerialAdb(
48-
serial,
49-
args,
50-
serverPort === undefined ? options : { ...options, serverPort },
51-
),
52-
);
46+
return withAdbFailureHints(async (args, options) => {
47+
const port = scopedServerPort(serial, serverPort);
48+
return await requireAndroidAdbHost().execSerialAdb(
49+
serial,
50+
args,
51+
port === undefined ? options : { ...options, serverPort: port },
52+
);
53+
});
5354
}
5455

5556
function createSerialAdbSpawner(serial: string, serverPort?: number): AndroidAdbSpawner {
56-
return (args, options) =>
57-
requireAndroidAdbHost().spawnSerialAdb(
57+
return (args, options) => {
58+
const port = scopedServerPort(serial, serverPort);
59+
return requireAndroidAdbHost().spawnSerialAdb(
5860
serial,
5961
args,
60-
serverPort === undefined ? options : { ...options, serverPort },
62+
port === undefined ? options : { ...options, serverPort: port },
6163
);
64+
};
6265
}
6366

6467
export function createLocalAndroidAdbProvider(
@@ -83,7 +86,7 @@ export function resolveAndroidAdbExecutor(
8386
device: DeviceInfo,
8487
executor?: AndroidAdbExecutor,
8588
): AndroidAdbExecutor {
86-
const scoped = androidAdbProviderScope.getStore();
89+
const scoped = scopeForDevice(device);
8790
if (executor) return executor;
8891
if (scoped?.serial === device.id) return scoped.provider.exec;
8992
return createDeviceAdbExecutor(device);
@@ -93,8 +96,8 @@ export function resolveAndroidAdbProvider(
9396
device: DeviceInfo,
9497
provider?: AndroidAdbProvider | AndroidAdbExecutor,
9598
): AndroidAdbProvider {
99+
const scoped = scopeForDevice(device);
96100
if (provider) return normalizeAndroidAdbProvider(provider);
97-
const scoped = androidAdbProviderScope.getStore();
98101
return scoped?.serial === device.id
99102
? normalizeAndroidAdbProvider(scoped.provider)
100103
: createLocalAndroidAdbProvider(device);
@@ -108,7 +111,7 @@ export function resolveAndroidAdbProvider(
108111
export function resolveScopedAndroidAdbBackgroundTransport(
109112
device: DeviceInfo,
110113
): ScopedAndroidAdbBackgroundTransport {
111-
const scoped = androidAdbProviderScope.getStore();
114+
const scoped = scopeForDevice(device);
112115
if (scoped?.serial !== device.id) return { mode: 'local' };
113116
return {
114117
mode: 'transport-composed',
@@ -117,12 +120,12 @@ export function resolveScopedAndroidAdbBackgroundTransport(
117120
}
118121

119122
export function resolveAndroidTextInjector(device: DeviceInfo): AndroidTextInjector | undefined {
120-
const scoped = androidAdbProviderScope.getStore();
123+
const scoped = scopeForDevice(device);
121124
return scoped?.serial === device.id ? scoped.provider.text : undefined;
122125
}
123126

124127
export function resolveAndroidTouchProvider(device: DeviceInfo): AndroidTouchProvider | undefined {
125-
const scoped = androidAdbProviderScope.getStore();
128+
const scoped = scopeForDevice(device);
126129
return scoped?.serial === device.id && scoped.provider.touch ? scoped.provider : undefined;
127130
}
128131

@@ -170,6 +173,7 @@ function createAndroidCommandExecutorOverride(
170173
if (!isAdbCommand(cmd)) return undefined;
171174
if (scope.serverPort === undefined && cmd !== 'adb') return undefined;
172175
const serial = readAdbSerial(args);
176+
requireScopedSerial(scope, serial);
173177
if (serial && serial !== scope.serial) return undefined;
174178
if (serial === scope.serial) {
175179
const providerArgs = stripAdbSerialArgs(args, scope.serial);
@@ -181,7 +185,7 @@ function createAndroidCommandExecutorOverride(
181185
if (scope.serverPort === undefined) return undefined;
182186
return requireAndroidAdbHost().withoutAdbCommandExecutorOverride(
183187
async () =>
184-
await requireAndroidAdbHost().execHostAdb(args, {
188+
await requireAndroidAdbHost().execHostAdb(['-s', scope.serial, ...args], {
185189
...options,
186190
allowFailure: true,
187191
serverPort: scope.serverPort,
@@ -193,18 +197,48 @@ function createAndroidCommandExecutorOverride(
193197
function createScopedHostTransport(scope: AndroidAdbProviderScope): AndroidAdbHostTransport {
194198
return async (args: string[], options?: AndroidAdbExecutorOptions) => {
195199
const serial = readAdbSerial(args);
200+
requireScopedSerial(scope, serial);
196201
const host = requireAndroidAdbHost();
197202
return await host.withoutAdbCommandExecutorOverride(
198203
async () =>
199-
await host.execHostAdb(args, {
204+
await host.execHostAdb(serial === undefined ? ['-s', scope.serial, ...args] : args, {
200205
...options,
201206
allowFailure: true,
202-
...(serial && serial !== scope.serial ? {} : { serverPort: scope.serverPort }),
207+
serverPort: scope.serverPort,
203208
}),
204209
);
205210
};
206211
}
207212

213+
function scopeForDevice(device: DeviceInfo): AndroidAdbProviderScope | undefined {
214+
const scoped = androidAdbProviderScope.getStore();
215+
requireScopedSerial(scoped, device.id);
216+
return scoped;
217+
}
218+
219+
function requireScopedSerial(
220+
scope: AndroidAdbProviderScope | undefined,
221+
serial: string | undefined,
222+
) {
223+
if (scope?.serverPort !== undefined && serial !== undefined && serial !== scope.serial) {
224+
throw new AppError('COMMAND_FAILED', 'Managed ADB transport cannot address another device.', {
225+
reason: 'managed-device-transport-mismatch',
226+
});
227+
}
228+
}
229+
230+
function scopedServerPort(serial: string, requested: number | undefined): number | undefined {
231+
const scope = androidAdbProviderScope.getStore();
232+
requireScopedSerial(scope, serial);
233+
if (scope?.serverPort === undefined) return requested;
234+
if (requested !== undefined && requested !== scope.serverPort) {
235+
throw new AppError('COMMAND_FAILED', 'Managed ADB transport cannot select another server.', {
236+
reason: 'managed-device-transport-mismatch',
237+
});
238+
}
239+
return scope.serverPort;
240+
}
241+
208242
function isAdbCommand(command: string): boolean {
209243
const executable = path.basename(command).replace(/\.(?:com|exe|bat|cmd)$/i, '');
210244
return executable === 'adb';

src/managed-device-reachability.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,11 @@ test.skipIf(process.platform === 'win32')(
166166
booted: true,
167167
},
168168
],
169-
host: { args: ['-P', '15037', 'devices'], port: '15037' },
170-
hostWithWrongPort: { args: ['-P', '15037', 'devices'], port: '15037' },
169+
host: { args: ['-P', '15037', '-s', 'emulator-15037', 'devices'], port: '15037' },
170+
hostWithWrongPort: {
171+
args: ['-P', '15037', '-s', 'emulator-15037', 'devices'],
172+
port: '15037',
173+
},
171174
serial: {
172175
args: ['-P', '15037', '-s', 'emulator-15037', 'shell', 'id'],
173176
port: '15037',

0 commit comments

Comments
 (0)