Skip to content

Commit f519034

Browse files
committed
fix: harden executable override policy
1 parent 406b3ed commit f519034

12 files changed

Lines changed: 160 additions & 19 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ npm install -g agent-device
8282

8383
Set `AGENT_DEVICE_NO_UPDATE_NOTIFIER=1` to disable the notice.
8484

85-
On macOS, `agent-device` includes a local `agent-device-macos-helper` source package that is built on demand for desktop permission checks, alert handling, and helper-backed desktop snapshot surfaces. Release distribution should use a signed/notarized helper build; source checkouts fall back to a local Swift build.
85+
On macOS, `agent-device` includes a local `agent-device-macos-helper` source package that is built on demand for desktop permission checks, alert handling, and helper-backed desktop snapshot surfaces. Release distribution should use a signed/notarized helper build; source checkouts fall back to a local Swift build. Local helper overrides through `AGENT_DEVICE_MACOS_HELPER_BIN` must use an absolute executable path.
8686

8787
## Contributing
8888

skills/agent-device/references/macos-desktop.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,3 +85,4 @@ Troubleshooting:
8585
- If `menubar` is missing the expected menu, retry with `open <app> --platform macos --surface menubar` for menu bar apps, or make the app frontmost first and retry the generic menubar surface.
8686
- If the wrong menu opened, retry secondary-clicking the row or cell wrapper rather than the nested text node.
8787
- If the app has multiple windows, make the correct window frontmost before relying on refs.
88+
- If overriding the local helper, set `AGENT_DEVICE_MACOS_HELPER_BIN` to an absolute executable path; relative helper paths are rejected.

src/platforms/android/__tests__/index.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -617,6 +617,42 @@ test('installAndroidApp .aab reports missing bundletool tooling', async () => {
617617
}
618618
});
619619

620+
test('installAndroidApp .aab rejects relative AGENT_DEVICE_BUNDLETOOL_JAR overrides', async () => {
621+
const tmpDir = await fs.mkdtemp(
622+
path.join(os.tmpdir(), 'agent-device-android-install-aab-relative-jar-'),
623+
);
624+
const adbPath = path.join(tmpDir, 'adb');
625+
const aabPath = path.join(tmpDir, 'Sample.aab');
626+
await fs.writeFile(aabPath, 'placeholder', 'utf8');
627+
await fs.writeFile(adbPath, '#!/bin/sh\nexit 0\n', 'utf8');
628+
await fs.chmod(adbPath, 0o755);
629+
630+
const previousPath = process.env.PATH;
631+
const previousBundletoolJar = process.env.AGENT_DEVICE_BUNDLETOOL_JAR;
632+
process.env.PATH = tmpDir;
633+
process.env.AGENT_DEVICE_BUNDLETOOL_JAR = './bundletool-all.jar';
634+
635+
const device: DeviceInfo = {
636+
platform: 'android',
637+
id: 'emulator-5554',
638+
name: 'Pixel',
639+
kind: 'emulator',
640+
booted: true,
641+
};
642+
643+
try {
644+
await assert.rejects(() => installAndroidApp(device, aabPath), { code: 'INVALID_ARGS' });
645+
} finally {
646+
process.env.PATH = previousPath;
647+
if (previousBundletoolJar === undefined) {
648+
delete process.env.AGENT_DEVICE_BUNDLETOOL_JAR;
649+
} else {
650+
process.env.AGENT_DEVICE_BUNDLETOOL_JAR = previousBundletoolJar;
651+
}
652+
await fs.rm(tmpDir, { recursive: true, force: true });
653+
}
654+
});
655+
620656
test('openAndroidApp rejects activity override for deep link URLs', async () => {
621657
const device: DeviceInfo = {
622658
platform: 'android',

src/platforms/android/app-lifecycle.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -506,12 +506,22 @@ async function resolveBundletoolInvocation(): Promise<BundletoolInvocation> {
506506
'bundletool not found in PATH. Install bundletool or set AGENT_DEVICE_BUNDLETOOL_JAR to a bundletool-all.jar path.',
507507
);
508508
}
509+
if (!path.isAbsolute(bundletoolJar) || bundletoolJar.includes('\0')) {
510+
throw new AppError(
511+
'INVALID_ARGS',
512+
`AGENT_DEVICE_BUNDLETOOL_JAR must be an absolute file path, not ${JSON.stringify(process.env.AGENT_DEVICE_BUNDLETOOL_JAR)}`,
513+
{ envName: 'AGENT_DEVICE_BUNDLETOOL_JAR', path: process.env.AGENT_DEVICE_BUNDLETOOL_JAR },
514+
);
515+
}
509516
try {
510-
await fs.access(bundletoolJar);
517+
const jarStat = await fs.stat(bundletoolJar);
518+
if (!jarStat.isFile()) {
519+
throw new Error('not a file');
520+
}
511521
} catch {
512522
throw new AppError(
513523
'TOOL_MISSING',
514-
`AGENT_DEVICE_BUNDLETOOL_JAR points to a missing file: ${bundletoolJar}`,
524+
`AGENT_DEVICE_BUNDLETOOL_JAR points to a missing or non-file path: ${bundletoolJar}`,
515525
);
516526
}
517527
const invocation = { cmd: 'java', prefixArgs: ['-jar', bundletoolJar] } as const;

src/platforms/android/manifest.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import fs from 'node:fs/promises';
22
import path from 'node:path';
33
import { TextDecoder } from 'node:util';
4-
import { runCmd } from '../../utils/exec.ts';
4+
import { isExecutablePath, runCmd } from '../../utils/exec.ts';
55
import { resolveAndroidSdkRoots } from './sdk.ts';
66

77
const RES_XML_TYPE = 0x0003;
@@ -197,12 +197,9 @@ async function resolveAaptPath(): Promise<string | undefined> {
197197
);
198198
for (const version of sortedVersions) {
199199
const candidate = path.join(buildToolsDir, version, 'aapt');
200-
try {
201-
await fs.access(candidate);
200+
if (path.isAbsolute(candidate) && (await isExecutablePath(candidate))) {
202201
aaptPathCache = candidate;
203202
return candidate;
204-
} catch {
205-
// Continue searching other build-tools versions.
206203
}
207204
}
208205
} catch {

src/platforms/ios/__tests__/index.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,21 @@ test('resolveMacOsHelperPackageRootFrom finds helper package from source and dis
141141
}
142142
});
143143

144+
test('AGENT_DEVICE_MACOS_HELPER_BIN rejects relative override paths', async () => {
145+
const previousHelperPath = process.env.AGENT_DEVICE_MACOS_HELPER_BIN;
146+
process.env.AGENT_DEVICE_MACOS_HELPER_BIN = './agent-device-macos-helper';
147+
148+
try {
149+
await assert.rejects(() => quitMacOsApp('com.example.App'), { code: 'INVALID_ARGS' });
150+
} finally {
151+
if (previousHelperPath === undefined) {
152+
delete process.env.AGENT_DEVICE_MACOS_HELPER_BIN;
153+
} else {
154+
process.env.AGENT_DEVICE_MACOS_HELPER_BIN = previousHelperPath;
155+
}
156+
}
157+
});
158+
144159
async function withMockedXcrun(
145160
tempPrefix: string,
146161
script: string,

src/platforms/ios/macos-helper.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import os from 'node:os';
44
import path from 'node:path';
55
import { fileURLToPath } from 'node:url';
66
import { AppError } from '../../utils/errors.ts';
7-
import { runCmd } from '../../utils/exec.ts';
7+
import { resolveExecutableOverridePath, runCmd } from '../../utils/exec.ts';
88
import type { SessionSurface } from '../../core/session-surface.ts';
99

1010
export type MacOsPermissionTarget = 'accessibility' | 'screen-recording' | 'input-monitoring';
@@ -155,7 +155,10 @@ async function readInstalledMacOsHelperFingerprint(): Promise<string | null> {
155155
}
156156

157157
async function ensureMacOsHelperBinary(): Promise<string> {
158-
const configuredPath = process.env[MACOS_HELPER_ENV_PATH]?.trim();
158+
const configuredPath = await resolveExecutableOverridePath(
159+
process.env[MACOS_HELPER_ENV_PATH],
160+
MACOS_HELPER_ENV_PATH,
161+
);
159162
if (configuredPath) {
160163
return configuredPath;
161164
}

src/utils/__tests__/exec.test.ts

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,14 @@ import assert from 'node:assert/strict';
33
import fs from 'node:fs';
44
import os from 'node:os';
55
import path from 'node:path';
6-
import { runCmd, whichCmd } from '../exec.ts';
6+
import {
7+
runCmd,
8+
runCmdBackground,
9+
runCmdDetached,
10+
runCmdStreaming,
11+
runCmdSync,
12+
whichCmd,
13+
} from '../exec.ts';
714

815
test('runCmd enforces timeoutMs and rejects with COMMAND_FAILED', async () => {
916
await assert.rejects(
@@ -29,14 +36,61 @@ test('whichCmd resolves bare commands from PATH', async () => {
2936
});
3037

3138
test.runIf(process.platform !== 'win32')(
32-
'runCmd allows explicit relative executable paths when shell execution is disabled',
39+
'process helpers reject relative executable paths',
3340
async () => {
3441
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-runcmd-relative-'));
3542
const target = path.join(root, 'local-node');
3643
fs.symlinkSync(process.execPath, target);
3744

3845
try {
39-
const result = await runCmd('./local-node', ['-e', 'process.stdout.write("ok")'], {
46+
await assert.rejects(
47+
runCmd('./local-node', ['-e', 'process.stdout.write("ok")'], {
48+
cwd: root,
49+
}),
50+
{ code: 'INVALID_ARGS' },
51+
);
52+
await assert.rejects(
53+
runCmdStreaming('./local-node', ['-e', 'process.stdout.write("ok")'], {
54+
cwd: root,
55+
}),
56+
{ code: 'INVALID_ARGS' },
57+
);
58+
assert.throws(
59+
() =>
60+
runCmdSync('./local-node', ['-e', 'process.stdout.write("ok")'], {
61+
cwd: root,
62+
}),
63+
{ code: 'INVALID_ARGS' },
64+
);
65+
assert.throws(
66+
() =>
67+
runCmdDetached('./local-node', ['-e', 'process.stdout.write("ok")'], {
68+
cwd: root,
69+
}),
70+
{ code: 'INVALID_ARGS' },
71+
);
72+
assert.throws(
73+
() =>
74+
runCmdBackground('./local-node', ['-e', 'process.stdout.write("ok")'], {
75+
cwd: root,
76+
}),
77+
{ code: 'INVALID_ARGS' },
78+
);
79+
} finally {
80+
fs.rmSync(root, { recursive: true, force: true });
81+
}
82+
},
83+
);
84+
85+
test.runIf(process.platform !== 'win32')(
86+
'runCmd accepts absolute executable paths without shell execution',
87+
async () => {
88+
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-runcmd-absolute-'));
89+
const target = path.join(root, 'local-node');
90+
fs.symlinkSync(process.execPath, target);
91+
92+
try {
93+
const result = await runCmd(target, ['-e', 'process.stdout.write("ok")'], {
4094
cwd: root,
4195
});
4296
assert.equal(result.stdout, 'ok');

src/utils/exec.ts

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ export async function whichCmd(cmd: string): Promise<boolean> {
138138
if (!candidate) return false;
139139

140140
if (path.isAbsolute(candidate)) {
141-
return isExecutable(candidate);
141+
return isExecutablePath(candidate);
142142
}
143143

144144
const pathValue = process.env.PATH;
@@ -148,7 +148,7 @@ export async function whichCmd(cmd: string): Promise<boolean> {
148148
const trimmedDirectory = directory.trim();
149149
if (!trimmedDirectory) continue;
150150
for (const entry of resolveExecutableCandidates(candidate, pathExtensions)) {
151-
if (await isExecutable(path.join(trimmedDirectory, entry))) {
151+
if (await isExecutablePath(path.join(trimmedDirectory, entry))) {
152152
return true;
153153
}
154154
}
@@ -157,6 +157,29 @@ export async function whichCmd(cmd: string): Promise<boolean> {
157157
return false;
158158
}
159159

160+
export async function resolveExecutableOverridePath(
161+
rawPath: string | undefined,
162+
envName: string,
163+
): Promise<string | undefined> {
164+
const candidate = rawPath?.trim();
165+
if (!candidate) return undefined;
166+
if (!path.isAbsolute(candidate) || candidate.includes('\0')) {
167+
throw new AppError(
168+
'INVALID_ARGS',
169+
`${envName} must be an absolute executable path, not ${JSON.stringify(rawPath)}`,
170+
{ envName, path: rawPath },
171+
);
172+
}
173+
if (!(await isExecutablePath(candidate))) {
174+
throw new AppError(
175+
'TOOL_MISSING',
176+
`${envName} points to a missing or non-executable file: ${candidate}`,
177+
{ envName, path: candidate },
178+
);
179+
}
180+
return candidate;
181+
}
182+
160183
export function runCmdSync(cmd: string, args: string[], options: ExecOptions = {}): ExecResult {
161184
const executable = normalizeExecutableCommand(cmd);
162185
const result = spawnSync(executable, args, {
@@ -397,10 +420,11 @@ export function runCmdBackground(
397420
}
398421

399422
function normalizeExecutableCommand(cmd: string): string {
400-
const candidate = normalizeExecutableLookup(cmd, { allowRelativePath: true });
423+
const candidate = normalizeExecutableLookup(cmd, { allowRelativePath: false });
401424
if (!candidate) {
402425
throw new AppError('INVALID_ARGS', `Invalid executable command: ${JSON.stringify(cmd)}`, {
403426
cmd,
427+
hint: 'Use a bare command name from PATH or an absolute executable path.',
404428
});
405429
}
406430
return candidate;
@@ -439,7 +463,7 @@ function resolveExecutableCandidates(cmd: string, pathExtensions: string[]): str
439463
return pathExtensions.map((extension) => `${cmd}${extension}`);
440464
}
441465

442-
async function isExecutable(filePath: string): Promise<boolean> {
466+
export async function isExecutablePath(filePath: string): Promise<boolean> {
443467
try {
444468
const fileStat = await stat(filePath);
445469
if (!fileStat.isFile()) return false;

website/docs/docs/commands.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ agent-device install com.example.app ./build/MyApp.app --platform ios
346346
- Useful for upgrade flows where you want to keep existing app data when supported by the platform.
347347
- Remote daemons automatically upload local app artifacts for `install`; prefix the path with `remote:` to use a daemon-side path verbatim.
348348
- Supported binary formats: Android `.apk`/`.aab`, iOS `.app`/`.ipa`.
349-
- `.aab` requires `bundletool` in `PATH`, or `AGENT_DEVICE_BUNDLETOOL_JAR=<path-to-bundletool-all.jar>` with `java` in `PATH`.
349+
- `.aab` requires `bundletool` in `PATH`, or `AGENT_DEVICE_BUNDLETOOL_JAR=<absolute-path-to-bundletool-all.jar>` with `java` in `PATH`.
350350
- Optional: `AGENT_DEVICE_ANDROID_BUNDLETOOL_MODE=<mode>` overrides bundletool `build-apks --mode` (default: `universal`).
351351
- `.ipa` installs by extracting `Payload/*.app`; if multiple app bundles exist, `<app>` is used as a bundle id/name hint to select one.
352352

0 commit comments

Comments
 (0)