Skip to content

Commit 5c958e1

Browse files
janicduplessisjanicduplessis
committed
fix(remote): refuse device selectors and require platform macos under a macos-app lease
1 parent 4e13a8d commit 5c958e1

3 files changed

Lines changed: 127 additions & 12 deletions

File tree

‎src/daemon/__tests__/macos-app-lease.test.ts‎

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,12 @@ function request(
2121
flags: Record<string, unknown> = {},
2222
extra: Pick<DaemonRequest, 'input' | 'runtime'> = {},
2323
): Pick<DaemonRequest, 'command' | 'positionals' | 'flags' | 'input' | 'runtime'> {
24-
return { command, positionals, flags: flags as DaemonRequest['flags'], ...extra };
24+
return {
25+
command,
26+
positionals,
27+
flags: { platform: 'macos', ...flags } as DaemonRequest['flags'],
28+
...extra,
29+
};
2530
}
2631

2732
function assertDenied(run: () => unknown, rule: string): void {
@@ -244,6 +249,66 @@ test('a batch is refused whole when any step is not allowed', () => {
244249
);
245250
});
246251

252+
test('no device selector reaches a host device, and open and batch must name platform macos', () => {
253+
for (const selector of [
254+
{ udid: 'ABCD-1234' },
255+
{ serial: 'emulator-5554' },
256+
{ device: 'iPhone 17' },
257+
{ target: 'mobile' },
258+
{ iosSimulatorDeviceSet: '/tmp/set' },
259+
{ androidDeviceAllowlist: 'emulator-5554' },
260+
]) {
261+
assertDenied(
262+
() => assertMacOsAppLeaseAdmitsRequest(lease, request('open', ['com.example.app'], selector)),
263+
'device',
264+
);
265+
assertDenied(
266+
() =>
267+
assertMacOsAppLeaseAdmitsRequest(lease, request('snapshot', [], selector), leasedSession),
268+
'device',
269+
);
270+
assertDenied(
271+
() =>
272+
assertMacOsAppLeaseAdmitsRequest(
273+
lease,
274+
request('batch', [], {
275+
batchSteps: [{ command: 'snapshot', input: {}, flags: selector }],
276+
}),
277+
leasedSession,
278+
),
279+
'device',
280+
);
281+
}
282+
for (const command of ['open', 'batch']) {
283+
assertDenied(
284+
() =>
285+
assertMacOsAppLeaseAdmitsRequest(lease, {
286+
command,
287+
positionals: command === 'open' ? ['com.example.app'] : [],
288+
flags: {},
289+
}),
290+
'device',
291+
);
292+
}
293+
});
294+
295+
test('a batch is refused whole when a later step carries a launch URL in its runtime', () => {
296+
assertDenied(
297+
() =>
298+
assertMacOsAppLeaseAdmitsRequest(
299+
lease,
300+
request('batch', [], {
301+
batchSteps: [
302+
{ command: 'snapshot', input: {} },
303+
{ command: 'open', positionals: ['com.example.app'], runtime: { launchUrl: 'x://y' } },
304+
],
305+
}),
306+
leasedSession,
307+
),
308+
'host-path',
309+
);
310+
});
311+
247312
test('open needs the native app backend, which resolves every action inside the app', () => {
248313
vi.stubEnv('AGENT_DEVICE_MACOS_APP_BACKEND', 'xctest');
249314
assertDenied(

‎src/daemon/macos-app-lease.ts‎

Lines changed: 59 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,16 @@ const MACOS_APP_LEASE_COMMANDS: ReadonlySet<string> = new Set([
5656
'lease_release',
5757
]);
5858

59+
/** Commands that can resolve a device when no session exists yet; a batch's steps inherit its platform. */
60+
const DEVICE_RESOLVING_COMMANDS: ReadonlySet<string> = new Set(['open', 'batch']);
61+
5962
type MacOsAppLeaseRule =
6063
| 'command'
6164
| 'app'
6265
| 'surface'
6366
| 'capture'
6467
| 'host-path'
68+
| 'device'
6569
| 'backend'
6670
| 'process';
6771

@@ -90,6 +94,16 @@ const HOST_INPUT_KEYS = [
9094
'iosXctestEnvDir',
9195
] as const;
9296

97+
/** Flags that pick a device other than the leased app's own; a lease never takes a device selector. */
98+
const DEVICE_SELECTOR_KEYS = [
99+
'device',
100+
'udid',
101+
'serial',
102+
'target',
103+
'iosSimulatorDeviceSet',
104+
'androidDeviceAllowlist',
105+
] as const;
106+
93107
/** The only screenshot path a remote client sends: the temp file `agent-device` names for it. */
94108
const REMOTE_SCREENSHOT_PATH = /^\/tmp\/agent-device-screenshot-[0-9]+-[a-z0-9]+\.png$/;
95109

@@ -119,24 +133,46 @@ export function assertMacOsAppLeaseAdmitsRequest(
119133
): void {
120134
if (lease.backend !== 'macos-app') return;
121135
const key = parseMacOsAppLeaseKey(lease.deviceKey);
122-
assertInvocation(key, req.command, req.positionals ?? [], {
123-
...req.input,
124-
...req.flags,
125-
...(req.runtime?.launchUrl ? { launchUrl: req.runtime.launchUrl } : {}),
126-
...(req.runtime?.bundleUrl ? { bundleUrl: req.runtime.bundleUrl } : {}),
127-
});
136+
assertInvocation(
137+
key,
138+
req.command,
139+
req.positionals ?? [],
140+
withRuntimeInputs(req.runtime, { ...req.input, ...req.flags }),
141+
);
142+
if (DEVICE_RESOLVING_COMMANDS.has(req.command) && req.flags?.platform !== 'macos') {
143+
throw macOsAppLeaseDenied(
144+
'device',
145+
`A macos-app lease needs platform macos on ${req.command}.`,
146+
);
147+
}
128148
if (req.command === 'batch') {
129149
for (const step of (req.flags?.batchSteps ?? []) as readonly Record<string, unknown>[]) {
130150
const positionals = Array.isArray(step.positionals) ? step.positionals.map(String) : [];
131-
assertInvocation(key, normalizeBatchCommandName(step.command), positionals, {
132-
...(step.input as Record<string, unknown> | undefined),
133-
...(step.flags as Record<string, unknown> | undefined),
134-
});
151+
assertInvocation(
152+
key,
153+
normalizeBatchCommandName(step.command),
154+
positionals,
155+
withRuntimeInputs(step.runtime as DaemonRequest['runtime'], {
156+
...(step.input as Record<string, unknown> | undefined),
157+
...(step.flags as Record<string, unknown> | undefined),
158+
}),
159+
);
135160
}
136161
}
137162
if (session) assertSessionIsLeasedApp(key, session);
138163
}
139164

165+
function withRuntimeInputs(
166+
runtime: DaemonRequest['runtime'],
167+
fields: Record<string, unknown>,
168+
): Record<string, unknown> {
169+
return {
170+
...fields,
171+
...(runtime?.launchUrl ? { launchUrl: runtime.launchUrl } : {}),
172+
...(runtime?.bundleUrl ? { bundleUrl: runtime.bundleUrl } : {}),
173+
};
174+
}
175+
140176
function assertInvocation(
141177
key: MacOsAppLeaseKey,
142178
command: string,
@@ -149,6 +185,7 @@ function assertInvocation(
149185
});
150186
}
151187
assertAppSurface(fields);
188+
assertNoDeviceSelector(fields);
152189
if (command === 'screenshot') assertWindowCapture(positionals, fields);
153190
else assertNoHostInputs(fields);
154191
if (command === 'open') assertOpensLeasedApp(key, positionals);
@@ -185,6 +222,18 @@ function assertNoHostInputs(fields: Readonly<Record<string, unknown>>): void {
185222
);
186223
}
187224

225+
function assertNoDeviceSelector(fields: Readonly<Record<string, unknown>>): void {
226+
const named = DEVICE_SELECTOR_KEYS.find((key) => fields[key] !== undefined && fields[key] !== '');
227+
if (!named) return;
228+
throw macOsAppLeaseDenied(
229+
'device',
230+
`A macos-app lease does not accept the ${named} device selector.`,
231+
{
232+
field: named,
233+
},
234+
);
235+
}
236+
188237
function assertAppSurface(fields: Readonly<Record<string, unknown>>): void {
189238
if (fields.surface !== undefined && fields.surface !== 'app') {
190239
throw macOsAppLeaseDenied(

‎website/docs/docs/remote-proxy.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,8 @@ Requests under a `macos-app` lease are limited to `open`, `close`, `snapshot`, `
164164
`open` and `close` accept only the leased bundle id, only the `app` surface is allowed, screenshots
165165
capture only the app window, inputs that name a host path or a launch (`--save-script`,
166166
`--launch-url`, `--launch-console`, a screenshot path other than the client's own temp file) are
167-
refused, and a pid-pinned lease stops working when that process exits. `open`
167+
refused, device selectors (`--udid`, `--serial`, `--device`, `--target`) are refused, `open` and
168+
`batch` must carry `--platform macos`, and a pid-pinned lease stops working when that process exits. `open`
168169
requires the daemon to run the native macOS app backend (`AGENT_DEVICE_MACOS_APP_BACKEND=native`).
169170
A refusal fails with `UNAUTHORIZED` and `details.reason: "MACOS_APP_LEASE_DENIED"`.
170171

0 commit comments

Comments
 (0)