diff --git a/src/daemon/__tests__/session-store.test.ts b/src/daemon/__tests__/session-store.test.ts index 74f8825f..161c8367 100644 --- a/src/daemon/__tests__/session-store.test.ts +++ b/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/, + ); +}); diff --git a/src/daemon/handlers/__tests__/session-replay-script.test.ts b/src/daemon/handlers/__tests__/session-replay-script.test.ts index 311e2392..9289afd3 100644 --- a/src/daemon/handlers/__tests__/session-replay-script.test.ts +++ b/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', diff --git a/src/daemon/handlers/session-replay-script.ts b/src/daemon/handlers/session-replay-script.ts index 9bd4486b..f46ec2a3 100644 --- a/src/daemon/handlers/session-replay-script.ts +++ b/src/daemon/handlers/session-replay-script.ts @@ -9,6 +9,7 @@ import { appendScriptSeriesFlags, formatScriptArgQuoteIfNeeded, formatScriptArg, + formatScriptStringLiteral, isClickLikeCommand, parseReplaySeriesFlags, parseReplayRuntimeFlags, @@ -16,7 +17,12 @@ import { type ReplayScriptPlatform = Exclude; -const REPLAY_METADATA_PLATFORMS = new Set(['ios', 'android', 'macos', 'linux']); +const REPLAY_METADATA_PLATFORMS = new Set([ + '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) { diff --git a/src/daemon/script-utils.ts b/src/daemon/script-utils.ts index ef1d1283..bafa3ebb 100644 --- a/src/daemon/script-utils.ts +++ b/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(); diff --git a/src/daemon/session-store.ts b/src/daemon/session-store.ts index 328cd323..be1fb586 100644 --- a/src/daemon/session-store.ts +++ b/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; diff --git a/src/utils/__tests__/exec.test.ts b/src/utils/__tests__/exec.test.ts index e090bcef..8eb82230 100644 --- a/src/utils/__tests__/exec.test.ts +++ b/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 }); + } +}); diff --git a/src/utils/exec.ts b/src/utils/exec.ts index 210cbf4b..e52c93ba 100644 --- a/src/utils/exec.ts +++ b/src/utils/exec.ts @@ -1,3 +1,6 @@ +import { constants } from 'node:fs'; +import { access, stat } from 'node:fs/promises'; +import path from 'node:path'; import { spawn, spawnSync, type StdioOptions } from 'node:child_process'; import { AppError } from './errors.ts'; @@ -33,17 +36,22 @@ type ExecDetachedOptions = ExecOptions & { stdio?: StdioOptions; }; +const BARE_COMMAND_RE = /^[A-Za-z0-9][A-Za-z0-9._+-]*$/; +const WINDOWS_PATH_EXTENSIONS = ['.com', '.exe', '.bat', '.cmd']; + export async function runCmd( cmd: string, args: string[], options: ExecOptions = {}, ): Promise { + const executable = normalizeExecutableCommand(cmd); return new Promise((resolve, reject) => { - const child = spawn(cmd, args, { + const child = spawn(executable, args, { cwd: options.cwd, env: options.env, stdio: ['pipe', 'pipe', 'pipe'], detached: options.detached, + shell: false, }); let stdout = ''; @@ -85,10 +93,10 @@ export async function runCmd( if (timeoutHandle) clearTimeout(timeoutHandle); const code = (err as NodeJS.ErrnoException).code; if (code === 'ENOENT') { - reject(new AppError('TOOL_MISSING', `${cmd} not found in PATH`, { cmd }, err)); + reject(new AppError('TOOL_MISSING', `${executable} not found in PATH`, { cmd }, err)); return; } - reject(new AppError('COMMAND_FAILED', `Failed to run ${cmd}`, { cmd, args }, err)); + reject(new AppError('COMMAND_FAILED', `Failed to run ${executable}`, { cmd, args }, err)); }); child.on('close', (code) => { @@ -96,7 +104,7 @@ export async function runCmd( const exitCode = code ?? 1; if (didTimeout && timeoutMs) { reject( - new AppError('COMMAND_FAILED', `${cmd} timed out after ${timeoutMs}ms`, { + new AppError('COMMAND_FAILED', `${executable} timed out after ${timeoutMs}ms`, { cmd, args, stdout, @@ -109,7 +117,7 @@ export async function runCmd( } if (exitCode !== 0 && !options.allowFailure) { reject( - new AppError('COMMAND_FAILED', `${cmd} exited with code ${exitCode}`, { + new AppError('COMMAND_FAILED', `${executable} exited with code ${exitCode}`, { cmd, args, stdout, @@ -126,23 +134,39 @@ export async function runCmd( } export async function whichCmd(cmd: string): Promise { - try { - const { shell, args } = resolveWhichArgs(cmd); - const result = await runCmd(shell, args, { allowFailure: true }); - return result.exitCode === 0 && result.stdout.trim().length > 0; - } catch { - return false; + const candidate = normalizeExecutableLookup(cmd, { allowRelativePath: false }); + if (!candidate) return false; + + if (path.isAbsolute(candidate)) { + return isExecutable(candidate); + } + + const pathValue = process.env.PATH; + if (!pathValue) return false; + const pathExtensions = resolvePathExtensions(); + for (const directory of pathValue.split(path.delimiter)) { + const trimmedDirectory = directory.trim(); + if (!trimmedDirectory) continue; + for (const entry of resolveExecutableCandidates(candidate, pathExtensions)) { + if (await isExecutable(path.join(trimmedDirectory, entry))) { + return true; + } + } } + + return false; } export function runCmdSync(cmd: string, args: string[], options: ExecOptions = {}): ExecResult { - const result = spawnSync(cmd, args, { + const executable = normalizeExecutableCommand(cmd); + const result = spawnSync(executable, args, { cwd: options.cwd, env: options.env, stdio: ['pipe', 'pipe', 'pipe'], encoding: options.binaryStdout ? undefined : 'utf8', input: options.stdin, timeout: normalizeTimeoutMs(options.timeoutMs), + shell: false, }); if (result.error) { @@ -150,7 +174,7 @@ export function runCmdSync(cmd: string, args: string[], options: ExecOptions = { if (code === 'ETIMEDOUT') { throw new AppError( 'COMMAND_FAILED', - `${cmd} timed out after ${normalizeTimeoutMs(options.timeoutMs)}ms`, + `${executable} timed out after ${normalizeTimeoutMs(options.timeoutMs)}ms`, { cmd, args, @@ -160,9 +184,14 @@ export function runCmdSync(cmd: string, args: string[], options: ExecOptions = { ); } if (code === 'ENOENT') { - throw new AppError('TOOL_MISSING', `${cmd} not found in PATH`, { cmd }, result.error); + throw new AppError('TOOL_MISSING', `${executable} not found in PATH`, { cmd }, result.error); } - throw new AppError('COMMAND_FAILED', `Failed to run ${cmd}`, { cmd, args }, result.error); + throw new AppError( + 'COMMAND_FAILED', + `Failed to run ${executable}`, + { cmd, args }, + result.error, + ); } const stdoutBuffer = options.binaryStdout @@ -180,7 +209,7 @@ export function runCmdSync(cmd: string, args: string[], options: ExecOptions = { const exitCode = result.status ?? 1; if (exitCode !== 0 && !options.allowFailure) { - throw new AppError('COMMAND_FAILED', `${cmd} exited with code ${exitCode}`, { + throw new AppError('COMMAND_FAILED', `${executable} exited with code ${exitCode}`, { cmd, args, stdout, @@ -198,11 +227,13 @@ export function runCmdDetached( args: string[], options: ExecDetachedOptions = {}, ): number { - const child = spawn(cmd, args, { + const executable = normalizeExecutableCommand(cmd); + const child = spawn(executable, args, { cwd: options.cwd, env: options.env, stdio: options.stdio ?? 'ignore', detached: true, + shell: false, }); child.unref(); return child.pid ?? 0; @@ -213,12 +244,14 @@ export async function runCmdStreaming( args: string[], options: ExecStreamOptions = {}, ): Promise { + const executable = normalizeExecutableCommand(cmd); return new Promise((resolve, reject) => { - const child = spawn(cmd, args, { + const child = spawn(executable, args, { cwd: options.cwd, env: options.env, stdio: ['pipe', 'pipe', 'pipe'], detached: options.detached, + shell: false, }); options.onSpawn?.(child); @@ -265,10 +298,10 @@ export async function runCmdStreaming( if (timeoutHandle) clearTimeout(timeoutHandle); const code = (err as NodeJS.ErrnoException).code; if (code === 'ENOENT') { - reject(new AppError('TOOL_MISSING', `${cmd} not found in PATH`, { cmd }, err)); + reject(new AppError('TOOL_MISSING', `${executable} not found in PATH`, { cmd }, err)); return; } - reject(new AppError('COMMAND_FAILED', `Failed to run ${cmd}`, { cmd, args }, err)); + reject(new AppError('COMMAND_FAILED', `Failed to run ${executable}`, { cmd, args }, err)); }); child.on('close', (code) => { @@ -276,7 +309,7 @@ export async function runCmdStreaming( const exitCode = code ?? 1; if (didTimeout && timeoutMs) { reject( - new AppError('COMMAND_FAILED', `${cmd} timed out after ${timeoutMs}ms`, { + new AppError('COMMAND_FAILED', `${executable} timed out after ${timeoutMs}ms`, { cmd, args, stdout, @@ -289,7 +322,7 @@ export async function runCmdStreaming( } if (exitCode !== 0 && !options.allowFailure) { reject( - new AppError('COMMAND_FAILED', `${cmd} exited with code ${exitCode}`, { + new AppError('COMMAND_FAILED', `${executable} exited with code ${exitCode}`, { cmd, args, stdout, @@ -310,11 +343,13 @@ export function runCmdBackground( args: string[], options: ExecOptions = {}, ): ExecBackgroundResult { - const child = spawn(cmd, args, { + const executable = normalizeExecutableCommand(cmd); + const child = spawn(executable, args, { cwd: options.cwd, env: options.env, stdio: ['ignore', 'pipe', 'pipe'], detached: options.detached, + shell: false, }); let stdout = ''; @@ -334,16 +369,16 @@ export function runCmdBackground( child.on('error', (err) => { const code = (err as NodeJS.ErrnoException).code; if (code === 'ENOENT') { - reject(new AppError('TOOL_MISSING', `${cmd} not found in PATH`, { cmd }, err)); + reject(new AppError('TOOL_MISSING', `${executable} not found in PATH`, { cmd }, err)); return; } - reject(new AppError('COMMAND_FAILED', `Failed to run ${cmd}`, { cmd, args }, err)); + reject(new AppError('COMMAND_FAILED', `Failed to run ${executable}`, { cmd, args }, err)); }); child.on('close', (code) => { const exitCode = code ?? 1; if (exitCode !== 0 && !options.allowFailure) { reject( - new AppError('COMMAND_FAILED', `${cmd} exited with code ${exitCode}`, { + new AppError('COMMAND_FAILED', `${executable} exited with code ${exitCode}`, { cmd, args, stdout, @@ -361,11 +396,58 @@ export function runCmdBackground( return { child, wait }; } -function resolveWhichArgs(cmd: string): { shell: string; args: string[] } { - if (process.platform === 'win32') { - return { shell: 'cmd.exe', args: ['/c', 'where', cmd] }; +function normalizeExecutableCommand(cmd: string): string { + const candidate = normalizeExecutableLookup(cmd, { allowRelativePath: true }); + if (!candidate) { + throw new AppError('INVALID_ARGS', `Invalid executable command: ${JSON.stringify(cmd)}`, { + cmd, + }); + } + return candidate; +} + +function normalizeExecutableLookup( + cmd: string, + options: { allowRelativePath: boolean }, +): 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 BARE_COMMAND_RE.test(candidate) ? candidate : null; +} + +function resolvePathExtensions(): string[] { + if (process.platform !== 'win32') return ['']; + const rawPathExt = process.env.PATHEXT; + if (!rawPathExt) return WINDOWS_PATH_EXTENSIONS; + const extensions = rawPathExt + .split(';') + .map((value) => value.trim().toLowerCase()) + .filter((value) => value.length > 0); + return extensions.length > 0 ? extensions : WINDOWS_PATH_EXTENSIONS; +} + +function resolveExecutableCandidates(cmd: string, pathExtensions: string[]): string[] { + if (process.platform !== 'win32') return [cmd]; + const lowered = cmd.toLowerCase(); + if (pathExtensions.some((extension) => lowered.endsWith(extension))) { + return [cmd]; + } + return pathExtensions.map((extension) => `${cmd}${extension}`); +} + +async function isExecutable(filePath: string): Promise { + try { + const fileStat = await stat(filePath); + if (!fileStat.isFile()) return false; + await access(filePath, process.platform === 'win32' ? constants.F_OK : constants.X_OK); + return true; + } catch { + return false; } - return { shell: 'bash', args: ['-lc', `command -v ${cmd}`] }; } function normalizeTimeoutMs(value: number | undefined): number | undefined {