Use spawn over exec

src/install.ts

@@ -1,6 +1,6 @@
 /* eslint-disable no-console */
 
-import { exec, spawn } from "child_process";
+import { execFile, spawn } from "child_process";
 import { existsSync, readFileSync } from "fs";
 import inquirer from "inquirer";
 import os from "os";
@@ -60,13 +60,24 @@ const TOOL_CONFIGS = {
   cursor: path.join(os.homedir(), ".cursor", "mcp.json"),
 } as const;
 
+const execFileAsync = promisify(execFile);
+
 // Cross-platform command finder
-const findCommand = async (command: string): Promise<string> => {
+// Uses execFile to prevent shell injection vulnerabilities
+// Exported for testing
+export const findCommand = async (command: string): Promise<string> => {
   const finder = process.platform === "win32" ? "where" : "which";
-  const { stdout } = await promisify(exec)(`${finder} ${command}`);
-  const lines = stdout.trim().split("\n");
-  if (!lines?.[0]) throw new Error(`${command} not found`);
-  return lines[0]!;
+
+  try {
+    const { stdout } = await execFileAsync(finder, [command]);
+    const lines = stdout.trim().split("\n");
+    if (!lines?.[0]) {
+      throw new Error(`${command} not found`);
+    }
+    return lines[0]!;
+  } catch (_error) {
+    throw new Error(`${command} not found`);
+  }
 };
 
 // Auto-detect if running from a local development build
@@ -945,12 +956,10 @@ export const setupMcpServer = async (): Promise<void> => {
 
       const configJson = JSON.stringify(iterableMcpConfig);
 
-      // Remove existing config
-      try {
-        await promisify(exec)("claude mcp remove iterable");
-      } catch {
-        // Ignore errors
-      }
+      // Remove existing config (ignore errors)
+      await execFileAsync("claude", ["mcp", "remove", "iterable"]).catch(
+        () => {}
+      );
 
       spinner.start("Configuring Claude Code...");
 

src/key-manager.ts

@@ -22,11 +22,12 @@
  */
 
 import { logger } from "@iterable/api";
-import { spawn } from "child_process";
+import { execFile } from "child_process";
 import { randomUUID } from "crypto";
 import { promises as fs } from "fs";
 import os from "os";
 import path from "path";
+import { promisify } from "util";
 
 import { isHttpsOrLocalhost, isLocalhostHost } from "./utils/url.js";
 
@@ -35,43 +36,21 @@ import { isHttpsOrLocalhost, isLocalhostHost } from "./utils/url.js";
 // Tests should use dependency injection with mock execSecurity instead.
 const SERVICE_NAME = "iterable-mcp";
 
+const execFileAsync = promisify(execFile);
+
 /**
  * Safely execute macOS security command with proper argument escaping
- * Uses spawn to prevent shell injection vulnerabilities
+ * Uses execFile to prevent shell injection vulnerabilities
  */
 async function execSecurityDefault(args: string[]): Promise<string> {
-  return new Promise<string>((resolve, reject) => {
-    const child = spawn("security", args, {
-      stdio: ["ignore", "pipe", "pipe"],
-    });
-
-    let stdout = "";
-    let stderr = "";
-
-    child.stdout.on("data", (data) => {
-      stdout += data.toString();
-    });
-
-    child.stderr.on("data", (data) => {
-      stderr += data.toString();
-    });
-
-    child.on("error", (error) => {
-      reject(new Error(`Failed to execute security command: ${error.message}`));
-    });
-
-    child.on("close", (code) => {
-      if (code === 0) {
-        resolve(stdout.trim());
-      } else {
-        reject(
-          new Error(
-            `Security command failed with code ${code}: ${stderr.trim() || stdout.trim()}`
-          )
-        );
-      }
-    });
-  });
+  try {
+    const { stdout } = await execFileAsync("security", args);
+    return stdout.trim();
+  } catch (error: any) {
+    throw new Error(
+      `Security command failed: ${error.stderr?.toString().trim() || error.message}`
+    );
+  }
 }
 
 // Type for the security executor function (for dependency injection in tests)

src/keys-cli.ts

@@ -3,7 +3,7 @@
  * CLI commands for API key management with beautiful modern UI
  */
 
-import { exec, spawn } from "child_process";
+import { execFile, spawn } from "child_process";
 import { promises as fs, readFileSync } from "fs";
 import inquirer from "inquirer";
 import os from "os";
@@ -16,6 +16,8 @@ const { dirname, join } = path;
 import { getSpinner, loadUi } from "./utils/cli-env.js";
 import { promptForApiKey } from "./utils/password-prompt.js";
 
+const execFileAsync = promisify(execFile);
+
 // Get package version
 const packageJson = JSON.parse(
   readFileSync(
@@ -471,17 +473,20 @@ export async function handleKeysCommand(): Promise<void> {
 
             // Update Claude Code CLI registry if available
             try {
-              await promisify(exec)("claude --version");
+              await execFileAsync("claude", ["--version"]);
 
               // Build config using existing helper (keeps local/npx logic consistent)
               const iterableMcpConfig = buildMcpConfig({ env: mcpEnv });
               const configJson = JSON.stringify(iterableMcpConfig);
 
               // Remove existing registration (ignore errors)
-              await promisify(exec)("claude mcp remove iterable").catch(
-                () => {}
-              );
+              await execFileAsync("claude", [
+                "mcp",
+                "remove",
+                "iterable",
+              ]).catch(() => {});
 
+              // Add new registration with inherited stdio to show Claude CLI output
               await new Promise<void>((resolve, reject) => {
                 const child = spawn(
                   "claude",

tests/unit/execfile-security.test.ts

@@ -0,0 +1,32 @@
+/**
+ * Tests for execFile security refactoring
+ * Verifies that we use execFile instead of shell-spawning commands
+ */
+
+import { describe, expect, it } from "@jest/globals";
+
+import { findCommand } from "../../src/install.js";
+
+describe("execFile Security", () => {
+  it("should find valid system commands without using shell", async () => {
+    // Test that findCommand works with a known command
+    // This verifies execFile is working correctly
+    const result = await findCommand("node");
+    expect(result).toBeTruthy();
+    expect(typeof result).toBe("string");
+    expect(result.length).toBeGreaterThan(0);
+  });
+
+  it("should reject commands with shell metacharacters gracefully", async () => {
+    // Attempt to find a command with shell injection characters
+    // execFile should treat this as a literal command name (which won't exist)
+    await expect(findCommand("node; echo pwned")).rejects.toThrow("not found");
+  });
+
+  it("should handle non-existent commands without shell execution", async () => {
+    // This should fail cleanly without any shell interpretation
+    await expect(
+      findCommand("this-command-does-not-exist-xyz")
+    ).rejects.toThrow("not found");
+  });
+});

tests/unit/find-command.test.ts

@@ -0,0 +1,17 @@
+/* eslint-disable simple-import-sort/imports */
+import { describe, expect, it } from "@jest/globals";
+import { findCommand } from "../../src/install";
+
+describe("findCommand", () => {
+  it("finds node command successfully", async () => {
+    const nodePath = await findCommand("node");
+    expect(nodePath).toBeTruthy();
+    expect(nodePath).toContain("node");
+  });
+
+  it("throws error for non-existent command", async () => {
+    await expect(
+      findCommand("this-command-does-not-exist-xyz")
+    ).rejects.toThrow("not found");
+  });
+});