fix: replace vulnerable double-quote shell escaping with POSIX single-quote method (CWE-78)

src/utils/__tests__/shell-escape.test.ts

@@ -0,0 +1,70 @@
+import { describe, it, expect } from 'vitest';
+import { shellEscapeArg } from '../shell-escape.ts';
+
+describe('shellEscapeArg', () => {
+  it('wraps a simple string in single quotes', () => {
+    expect(shellEscapeArg('hello')).toBe("'hello'");
+  });
+
+  it('returns empty single-quoted string for empty input', () => {
+    expect(shellEscapeArg('')).toBe("''");
+  });
+
+  it('escapes embedded single quotes using the POSIX technique', () => {
+    expect(shellEscapeArg("it's")).toBe("'it'\\''s'");
+  });
+
+  it('handles multiple single quotes', () => {
+    expect(shellEscapeArg("a'b'c")).toBe("'a'\\''b'\\''c'");
+  });
+
+  it('passes through double quotes safely inside single quotes', () => {
+    expect(shellEscapeArg('say "hi"')).toBe('\'say "hi"\'');
+  });
+
+  it('neutralises dollar-sign variable expansion', () => {
+    const escaped = shellEscapeArg('$HOME');
+    expect(escaped).toBe("'$HOME'");
+  });
+
+  it('neutralises backtick command substitution', () => {
+    const escaped = shellEscapeArg('`id`');
+    expect(escaped).toBe("'`id`'");
+  });
+
+  it('neutralises $() command substitution', () => {
+    const escaped = shellEscapeArg('$(whoami)');
+    expect(escaped).toBe("'$(whoami)'");
+  });
+
+  it('neutralises semicolon command chaining', () => {
+    const escaped = shellEscapeArg('foo; rm -rf /');
+    expect(escaped).toBe("'foo; rm -rf /'");
+  });
+
+  it('handles newlines (cannot break out of single quotes)', () => {
+    const escaped = shellEscapeArg('line1\nline2');
+    expect(escaped).toBe("'line1\nline2'");
+  });
+
+  it('handles backslashes', () => {
+    const escaped = shellEscapeArg('path\\to\\file');
+    expect(escaped).toBe("'path\\to\\file'");
+  });
+
+  it('handles pipe and redirection metacharacters', () => {
+    const escaped = shellEscapeArg('a | b > c < d');
+    expect(escaped).toBe("'a | b > c < d'");
+  });
+
+  it('handles a realistic malicious scheme name (CWE-78 PoC)', () => {
+    // An attacker might supply this as a scheme parameter via MCP tool call
+    const malicious = '$(touch /tmp/pwned)';
+    const escaped = shellEscapeArg(malicious);
+    // The result should be a valid single-quoted string that cannot execute $(touch)
+    expect(escaped).toBe("'$(touch /tmp/pwned)'");
+    // Verify no unquoted regions exist
+    expect(escaped.startsWith("'")).toBe(true);
+    expect(escaped.endsWith("'")).toBe(true);
+  });
+});

src/utils/command.ts

@@ -13,6 +13,7 @@ import { spawn } from 'child_process';
 import { createWriteStream, existsSync } from 'fs';
 import { tmpdir as osTmpdir } from 'os';
 import { log } from './logger.ts';
+import { shellEscapeArg } from './shell-escape.ts';
 import type { FileSystemExecutor } from './FileSystemExecutor.ts';
 import type { CommandExecutor, CommandResponse, CommandExecOptions } from './CommandExecutor.ts';
 
@@ -41,16 +42,8 @@ async function defaultExecutor(
   let escapedCommand = command;
   if (useShell) {
     // For shell execution, we need to format as ['/bin/sh', '-c', 'full command string']
-    const commandString = command
-      .map((arg) => {
-        // Shell metacharacters that require quoting: space, quotes, equals, dollar, backticks, semicolons, pipes, etc.
-        if (/[\s,"'=$`;&|<>(){}[\]\\*?~]/.test(arg) && !/^".*"$/.test(arg)) {
-          // Escape all quotes and backslashes, then wrap in double quotes
-          return `"${arg.replace(/(["\\])/g, '\\$1')}"`;
-        }
-        return arg;
-      })
-      .join(' ');
+    // Use POSIX single-quote escaping for each argument to prevent injection
+    const commandString = command.map((arg) => shellEscapeArg(arg)).join(' ');
 
     escapedCommand = ['/bin/sh', '-c', commandString];
   }

src/utils/shell-escape.ts

@@ -0,0 +1,15 @@
+/**
+ * POSIX-safe shell argument escaping.
+ *
+ * Wraps a string in single quotes and escapes any embedded single quotes
+ * using the standard `'\''` technique. This is the safest way to pass
+ * arbitrary strings as arguments to `/bin/sh -c` commands.
+ *
+ * @param arg The argument to escape for safe shell interpolation
+ * @returns A single-quoted, safely escaped string
+ */
+export function shellEscapeArg(arg: string): string {
+  // Replace each single quote with: end current quote, escaped single quote, start new quote
+  // Then wrap the whole thing in single quotes
+  return "'" + arg.replace(/'/g, "'\\''") + "'";
+}