fix: harden executable override policy (#398)

Author: thymikee

README.md

@@ -82,7 +82,7 @@ npm install -g agent-device
 
 Set `AGENT_DEVICE_NO_UPDATE_NOTIFIER=1` to disable the notice.
 
-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.
+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.
 
 ## Contributing
 

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

@@ -85,3 +85,4 @@ Troubleshooting:
 - 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.
 - If the wrong menu opened, retry secondary-clicking the row or cell wrapper rather than the nested text node.
 - If the app has multiple windows, make the correct window frontmost before relying on refs.
+- 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

@@ -612,6 +612,42 @@ test('installAndroidApp .aab reports missing bundletool tooling', async () => {
   }
 });
 
+test('installAndroidApp .aab rejects relative AGENT_DEVICE_BUNDLETOOL_JAR overrides', async () => {
+  const tmpDir = await fs.mkdtemp(
+    path.join(os.tmpdir(), 'agent-device-android-install-aab-relative-jar-'),
+  );
+  const adbPath = path.join(tmpDir, 'adb');
+  const aabPath = path.join(tmpDir, 'Sample.aab');
+  await fs.writeFile(aabPath, 'placeholder', 'utf8');
+  await fs.writeFile(adbPath, '#!/bin/sh\nexit 0\n', 'utf8');
+  await fs.chmod(adbPath, 0o755);
+
+  const previousPath = process.env.PATH;
+  const previousBundletoolJar = process.env.AGENT_DEVICE_BUNDLETOOL_JAR;
+  process.env.PATH = tmpDir;
+  process.env.AGENT_DEVICE_BUNDLETOOL_JAR = './bundletool-all.jar';
+
+  const device: DeviceInfo = {
+    platform: 'android',
+    id: 'emulator-5554',
+    name: 'Pixel',
+    kind: 'emulator',
+    booted: true,
+  };
+
+  try {
+    await assert.rejects(() => installAndroidApp(device, aabPath), { code: 'INVALID_ARGS' });
+  } finally {
+    process.env.PATH = previousPath;
+    if (previousBundletoolJar === undefined) {
+      delete process.env.AGENT_DEVICE_BUNDLETOOL_JAR;
+    } else {
+      process.env.AGENT_DEVICE_BUNDLETOOL_JAR = previousBundletoolJar;
+    }
+    await fs.rm(tmpDir, { recursive: true, force: true });
+  }
+});
+
 test('openAndroidApp rejects activity override for deep link URLs', async () => {
   const device: DeviceInfo = {
     platform: 'android',

src/platforms/android/app-lifecycle.ts

@@ -1,7 +1,7 @@
 import { promises as fs } from 'node:fs';
 import os from 'node:os';
 import path from 'node:path';
-import { runCmd, whichCmd } from '../../utils/exec.ts';
+import { resolveFileOverridePath, runCmd, whichCmd } from '../../utils/exec.ts';
 import { AppError } from '../../utils/errors.ts';
 import type { DeviceInfo } from '../../utils/device.ts';
 import { isDeepLinkTarget } from '../../core/open-target.ts';
@@ -499,21 +499,16 @@ async function resolveBundletoolInvocation(): Promise<BundletoolInvocation> {
     return invocation;
   }
 
-  const bundletoolJar = process.env.AGENT_DEVICE_BUNDLETOOL_JAR?.trim();
+  const bundletoolJar = await resolveFileOverridePath(
+    process.env.AGENT_DEVICE_BUNDLETOOL_JAR,
+    'AGENT_DEVICE_BUNDLETOOL_JAR',
+  );
   if (!bundletoolJar) {
     throw new AppError(
       'TOOL_MISSING',
       'bundletool not found in PATH. Install bundletool or set AGENT_DEVICE_BUNDLETOOL_JAR to a bundletool-all.jar path.',
     );
   }
-  try {
-    await fs.access(bundletoolJar);
-  } catch {
-    throw new AppError(
-      'TOOL_MISSING',
-      `AGENT_DEVICE_BUNDLETOOL_JAR points to a missing file: ${bundletoolJar}`,
-    );
-  }
   const invocation = { cmd: 'java', prefixArgs: ['-jar', bundletoolJar] } as const;
   cachedBundletoolInvocation = { key: cacheKey, invocation };
   return invocation;

src/platforms/android/manifest.ts

@@ -1,7 +1,7 @@
 import fs from 'node:fs/promises';
 import path from 'node:path';
 import { TextDecoder } from 'node:util';
-import { runCmd } from '../../utils/exec.ts';
+import { isExecutablePath, runCmd } from '../../utils/exec.ts';
 import { resolveAndroidSdkRoots } from './sdk.ts';
 
 const RES_XML_TYPE = 0x0003;
@@ -197,12 +197,10 @@ async function resolveAaptPath(): Promise<string | undefined> {
         );
         for (const version of sortedVersions) {
           const candidate = path.join(buildToolsDir, version, 'aapt');
-          try {
-            await fs.access(candidate);
+          // SDK roots can come from env vars; reject relative roots before returning an executable.
+          if (path.isAbsolute(candidate) && (await isExecutablePath(candidate))) {
             aaptPathCache = candidate;
             return candidate;
-          } catch {
-            // Continue searching other build-tools versions.
           }
         }
       } catch {

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

@@ -141,6 +141,21 @@ test('resolveMacOsHelperPackageRootFrom finds helper package from source and dis
   }
 });
 
+test('AGENT_DEVICE_MACOS_HELPER_BIN rejects relative override paths', async () => {
+  const previousHelperPath = process.env.AGENT_DEVICE_MACOS_HELPER_BIN;
+  process.env.AGENT_DEVICE_MACOS_HELPER_BIN = './agent-device-macos-helper';
+
+  try {
+    await assert.rejects(() => quitMacOsApp('com.example.App'), { code: 'INVALID_ARGS' });
+  } finally {
+    if (previousHelperPath === undefined) {
+      delete process.env.AGENT_DEVICE_MACOS_HELPER_BIN;
+    } else {
+      process.env.AGENT_DEVICE_MACOS_HELPER_BIN = previousHelperPath;
+    }
+  }
+});
+
 async function withMockedXcrun(
   tempPrefix: string,
   script: string,

src/platforms/ios/macos-helper.ts

@@ -4,7 +4,7 @@ import os from 'node:os';
 import path from 'node:path';
 import { fileURLToPath } from 'node:url';
 import { AppError } from '../../utils/errors.ts';
-import { runCmd } from '../../utils/exec.ts';
+import { resolveExecutableOverridePath, runCmd } from '../../utils/exec.ts';
 import type { SessionSurface } from '../../core/session-surface.ts';
 
 export type MacOsPermissionTarget = 'accessibility' | 'screen-recording' | 'input-monitoring';
@@ -155,7 +155,10 @@ async function readInstalledMacOsHelperFingerprint(): Promise<string | null> {
 }
 
 async function ensureMacOsHelperBinary(): Promise<string> {
-  const configuredPath = process.env[MACOS_HELPER_ENV_PATH]?.trim();
+  const configuredPath = await resolveExecutableOverridePath(
+    process.env[MACOS_HELPER_ENV_PATH],
+    MACOS_HELPER_ENV_PATH,
+  );
   if (configuredPath) {
     return configuredPath;
   }

src/utils/__tests__/exec.test.ts

@@ -3,7 +3,14 @@ import assert from 'node:assert/strict';
 import fs from 'node:fs';
 import os from 'node:os';
 import path from 'node:path';
-import { runCmd, whichCmd } from '../exec.ts';
+import {
+  runCmd,
+  runCmdBackground,
+  runCmdDetached,
+  runCmdStreaming,
+  runCmdSync,
+  whichCmd,
+} from '../exec.ts';
 
 test('runCmd enforces timeoutMs and rejects with COMMAND_FAILED', async () => {
   await assert.rejects(
@@ -29,14 +36,61 @@ test('whichCmd resolves bare commands from PATH', async () => {
 });
 
 test.runIf(process.platform !== 'win32')(
-  'runCmd allows explicit relative executable paths when shell execution is disabled',
+  'process helpers reject relative executable paths',
   async () => {
     const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-runcmd-relative-'));
     const target = path.join(root, 'local-node');
     fs.symlinkSync(process.execPath, target);
 
     try {
-      const result = await runCmd('./local-node', ['-e', 'process.stdout.write("ok")'], {
+      await assert.rejects(
+        runCmd('./local-node', ['-e', 'process.stdout.write("ok")'], {
+          cwd: root,
+        }),
+        { code: 'INVALID_ARGS' },
+      );
+      await assert.rejects(
+        runCmdStreaming('./local-node', ['-e', 'process.stdout.write("ok")'], {
+          cwd: root,
+        }),
+        { code: 'INVALID_ARGS' },
+      );
+      assert.throws(
+        () =>
+          runCmdSync('./local-node', ['-e', 'process.stdout.write("ok")'], {
+            cwd: root,
+          }),
+        { code: 'INVALID_ARGS' },
+      );
+      assert.throws(
+        () =>
+          runCmdDetached('./local-node', ['-e', 'process.stdout.write("ok")'], {
+            cwd: root,
+          }),
+        { code: 'INVALID_ARGS' },
+      );
+      assert.throws(
+        () =>
+          runCmdBackground('./local-node', ['-e', 'process.stdout.write("ok")'], {
+            cwd: root,
+          }),
+        { code: 'INVALID_ARGS' },
+      );
+    } finally {
+      fs.rmSync(root, { recursive: true, force: true });
+    }
+  },
+);
+
+test.runIf(process.platform !== 'win32')(
+  'runCmd accepts absolute executable paths without shell execution',
+  async () => {
+    const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-runcmd-absolute-'));
+    const target = path.join(root, 'local-node');
+    fs.symlinkSync(process.execPath, target);
+
+    try {
+      const result = await runCmd(target, ['-e', 'process.stdout.write("ok")'], {
         cwd: root,
       });
       assert.equal(result.stdout, 'ok');

src/utils/exec.ts

@@ -134,11 +134,11 @@ export async function runCmd(
 }
 
 export async function whichCmd(cmd: string): Promise<boolean> {
-  const candidate = normalizeExecutableLookup(cmd, { allowRelativePath: false });
+  const candidate = normalizeExecutableLookup(cmd);
   if (!candidate) return false;
 
   if (path.isAbsolute(candidate)) {
-    return isExecutable(candidate);
+    return isExecutablePath(candidate);
   }
 
   const pathValue = process.env.PATH;
@@ -148,7 +148,7 @@ export async function whichCmd(cmd: string): Promise<boolean> {
     const trimmedDirectory = directory.trim();
     if (!trimmedDirectory) continue;
     for (const entry of resolveExecutableCandidates(candidate, pathExtensions)) {
-      if (await isExecutable(path.join(trimmedDirectory, entry))) {
+      if (await isExecutablePath(path.join(trimmedDirectory, entry))) {
         return true;
       }
     }
@@ -157,6 +157,38 @@ export async function whichCmd(cmd: string): Promise<boolean> {
   return false;
 }
 
+export async function resolveExecutableOverridePath(
+  rawPath: string | undefined,
+  envName: string,
+): Promise<string | undefined> {
+  const candidate = normalizeOverridePath(rawPath, envName, 'executable');
+  if (!candidate) return undefined;
+  if (!(await isExecutablePath(candidate))) {
+    throw new AppError(
+      'TOOL_MISSING',
+      `${envName} points to a missing or non-executable file: ${candidate}`,
+      { envName, path: candidate },
+    );
+  }
+  return candidate;
+}
+
+export async function resolveFileOverridePath(
+  rawPath: string | undefined,
+  envName: string,
+): Promise<string | undefined> {
+  const candidate = normalizeOverridePath(rawPath, envName, 'file');
+  if (!candidate) return undefined;
+  if (!(await isFilePath(candidate))) {
+    throw new AppError(
+      'TOOL_MISSING',
+      `${envName} points to a missing or non-file path: ${candidate}`,
+      { envName, path: candidate },
+    );
+  }
+  return candidate;
+}
+
 export function runCmdSync(cmd: string, args: string[], options: ExecOptions = {}): ExecResult {
   const executable = normalizeExecutableCommand(cmd);
   const result = spawnSync(executable, args, {
@@ -397,24 +429,39 @@ export function runCmdBackground(
 }
 
 function normalizeExecutableCommand(cmd: string): string {
-  const candidate = normalizeExecutableLookup(cmd, { allowRelativePath: true });
+  const candidate = normalizeExecutableLookup(cmd);
   if (!candidate) {
     throw new AppError('INVALID_ARGS', `Invalid executable command: ${JSON.stringify(cmd)}`, {
       cmd,
+      hint: 'Use a bare command name from PATH or an absolute executable path.',
     });
   }
   return candidate;
 }
 
-function normalizeExecutableLookup(
-  cmd: string,
-  options: { allowRelativePath: boolean },
-): string | null {
+function normalizeOverridePath(
+  rawPath: string | undefined,
+  envName: string,
+  kind: 'executable' | 'file',
+): string | undefined {
+  const candidate = rawPath?.trim();
+  if (!candidate) return undefined;
+  if (!path.isAbsolute(candidate) || candidate.includes('\0')) {
+    throw new AppError(
+      'INVALID_ARGS',
+      `${envName} must be an absolute ${kind} path, not ${JSON.stringify(rawPath)}`,
+      { envName, path: rawPath },
+    );
+  }
+  return candidate;
+}
+
+function normalizeExecutableLookup(cmd: string): string | null {
   const candidate = cmd.trim();
   if (!candidate || candidate.includes('\0')) return null;
   if (path.isAbsolute(candidate)) return candidate;
   if (candidate.includes('/') || candidate.includes('\\')) {
-    return options.allowRelativePath ? candidate : null;
+    return null;
   }
   return BARE_COMMAND_RE.test(candidate) ? candidate : null;
 }
@@ -439,17 +486,25 @@ function resolveExecutableCandidates(cmd: string, pathExtensions: string[]): str
   return pathExtensions.map((extension) => `${cmd}${extension}`);
 }
 
-async function isExecutable(filePath: string): Promise<boolean> {
+export async function isExecutablePath(filePath: string): Promise<boolean> {
   try {
-    const fileStat = await stat(filePath);
-    if (!fileStat.isFile()) return false;
+    if (!(await isFilePath(filePath))) return false;
     await access(filePath, process.platform === 'win32' ? constants.F_OK : constants.X_OK);
     return true;
   } catch {
     return false;
   }
 }
 
+async function isFilePath(filePath: string): Promise<boolean> {
+  try {
+    const fileStat = await stat(filePath);
+    return fileStat.isFile();
+  } catch {
+    return false;
+  }
+}
+
 function normalizeTimeoutMs(value: number | undefined): number | undefined {
   if (!Number.isFinite(value)) return undefined;
   const timeout = Math.floor(value as number);

website/docs/docs/commands.md

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