fix: harden command execution and replay script escaping (#364)

* fix: harden command execution and replay script escaping

* fix: tighten whichCmd executable checks

* fix: allow explicit relative executables

Author: thymikee

src/daemon/__tests__/session-store.test.ts

@@ -345,3 +345,25 @@ test('writeSessionLog preserves interaction series flags for click/press/swipe',
   assert.match(script, /swipe 10 20 30 40 --count 3 --pause-ms 12 --pattern ping-pong/);
   assert.match(script, /fill @e5 "search" --delay-ms 40/);
 });
+
+test('writeSessionLog escapes device labels with quotes and backslashes', () => {
+  const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-session-log-device-label-'));
+  const store = new SessionStore(root);
+  const session = makeSession('default');
+  session.device.name = 'QA "Lab" \\ Shelf';
+  store.recordAction(session, {
+    command: 'open',
+    positionals: ['Settings'],
+    flags: { platform: 'ios', saveScript: true },
+    result: {},
+  });
+
+  store.writeSessionLog(session);
+  const scriptFile = fs.readdirSync(root).find((file) => file.endsWith('.ad'));
+  assert.ok(scriptFile);
+  const script = fs.readFileSync(path.join(root, scriptFile!), 'utf8');
+  assert.match(
+    script,
+    /context platform=ios device="QA \\"Lab\\" \\\\ Shelf" kind=simulator theme=unknown/,
+  );
+});

src/daemon/handlers/__tests__/session-replay-script.test.ts

@@ -130,6 +130,21 @@ test('type replay script preserves literal delay flag tokens', () => {
   assert.equal(parsed[0]?.flags.delayMs, undefined);
 });
 
+test('writeReplayScript escapes device labels with quotes and backslashes', () => {
+  const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-script-device-label-'));
+  const replayPath = path.join(root, 'flow.ad');
+  const session = makeSession();
+  session.device.name = 'Pixel "QA" \\ Lab';
+
+  writeReplayScript(replayPath, [], session);
+  const script = fs.readFileSync(replayPath, 'utf8');
+
+  assert.match(
+    script,
+    /context platform=android device="Pixel \\"QA\\" \\\\ Lab" kind=emulator theme=unknown/,
+  );
+});
+
 test('readReplayScriptMetadata extracts platform from context header', () => {
   const metadata = readReplayScriptMetadata(
     '# comment\n\ncontext platform=android device="Pixel 9 Pro"\nopen "Demo"\n',

src/daemon/handlers/session-replay-script.ts

@@ -9,14 +9,20 @@ import {
   appendScriptSeriesFlags,
   formatScriptArgQuoteIfNeeded,
   formatScriptArg,
+  formatScriptStringLiteral,
   isClickLikeCommand,
   parseReplaySeriesFlags,
   parseReplayRuntimeFlags,
 } from '../script-utils.ts';
 
 type ReplayScriptPlatform = Exclude<PlatformSelector, 'apple'>;
 
-const REPLAY_METADATA_PLATFORMS = new Set<ReplayScriptPlatform>(['ios', 'android', 'macos', 'linux']);
+const REPLAY_METADATA_PLATFORMS = new Set<ReplayScriptPlatform>([
+  'ios',
+  'android',
+  'macos',
+  'linux',
+]);
 
 export type ReplayScriptMetadata = {
   platform?: ReplayScriptPlatform;
@@ -310,11 +316,10 @@ export function writeReplayScript(
   // Session can be missing if the replay session is closed/deleted between execution and update write.
   // In that case we still persist healed actions and omit only the context header.
   if (session) {
-    const deviceLabel = session.device.name.replace(/"/g, '\\"');
     const kind = session.device.kind ? ` kind=${session.device.kind}` : '';
     const target = session.device.target ? ` target=${session.device.target}` : '';
     lines.push(
-      `context platform=${session.device.platform}${target} device="${deviceLabel}"${kind} theme=unknown`,
+      `context platform=${session.device.platform}${target} device=${formatScriptStringLiteral(session.device.name)}${kind} theme=unknown`,
     );
   }
   for (const action of actions) {

src/daemon/script-utils.ts

@@ -33,6 +33,11 @@ export function formatScriptArg(value: string): string {
   return JSON.stringify(trimmed);
 }
 
+// Use for literal values such as device labels where leading/trailing whitespace must survive round-trips.
+export function formatScriptStringLiteral(value: string): string {
+  return JSON.stringify(value);
+}
+
 // Preserve readable CLI-ish script output for ordinary tokens while still quoting whitespace.
 export function formatScriptArgQuoteIfNeeded(value: string): string {
   const trimmed = value.trim();

src/daemon/session-store.ts

@@ -11,6 +11,7 @@ import {
   appendScriptSeriesFlags,
   formatScriptArgQuoteIfNeeded,
   formatScriptArg,
+  formatScriptStringLiteral,
   isClickLikeCommand,
 } from './script-utils.ts';
 import { emitDiagnostic } from '../utils/diagnostics.ts';
@@ -285,11 +286,10 @@ function sanitizeFlags(flags: CommandFlags | undefined): SessionAction['flags']
 
 function formatScript(session: SessionState, actions: SessionAction[]): string {
   const lines: string[] = [];
-  const deviceLabel = session.device.name.replace(/"/g, '\\"');
   const kind = session.device.kind ? ` kind=${session.device.kind}` : '';
   const theme = 'unknown';
   lines.push(
-    `context platform=${session.device.platform} device="${deviceLabel}"${kind} theme=${theme}`,
+    `context platform=${session.device.platform} device=${formatScriptStringLiteral(session.device.name)}${kind} theme=${theme}`,
   );
   for (const action of actions) {
     if (action.flags?.noRecord) continue;

src/utils/__tests__/exec.test.ts

@@ -1,6 +1,9 @@
 import { test } from 'vitest';
 import assert from 'node:assert/strict';
-import { runCmd } from '../exec.ts';
+import fs from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
+import { runCmd, whichCmd } from '../exec.ts';
 
 test('runCmd enforces timeoutMs and rejects with COMMAND_FAILED', async () => {
   await assert.rejects(
@@ -16,3 +19,50 @@ test('runCmd enforces timeoutMs and rejects with COMMAND_FAILED', async () => {
     },
   );
 });
+
+test('whichCmd resolves absolute executable paths without invoking a shell', async () => {
+  assert.equal(await whichCmd(process.execPath), true);
+});
+
+test('whichCmd resolves bare commands from PATH', async () => {
+  assert.equal(await whichCmd('node'), true);
+});
+
+test.runIf(process.platform !== 'win32')(
+  'runCmd allows explicit relative executable paths when shell execution is disabled',
+  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")'], {
+        cwd: root,
+      });
+      assert.equal(result.stdout, 'ok');
+    } finally {
+      fs.rmSync(root, { recursive: true, force: true });
+    }
+  },
+);
+
+test('whichCmd rejects suspicious command strings', async () => {
+  assert.equal(await whichCmd('node; rm -rf /'), false);
+  assert.equal(await whichCmd('./node'), false);
+});
+
+test.sequential('whichCmd ignores directories that match a command name in PATH', async () => {
+  const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-whichcmd-'));
+  const fakeCommandDir = path.join(root, 'fake-tool');
+  fs.mkdirSync(fakeCommandDir);
+
+  const previousPath = process.env.PATH;
+  process.env.PATH = `${root}${path.delimiter}${previousPath ?? ''}`;
+
+  try {
+    assert.equal(await whichCmd('fake-tool'), false);
+  } finally {
+    process.env.PATH = previousPath;
+    fs.rmSync(root, { recursive: true, force: true });
+  }
+});