fix: harden external CLI hub — command injection, denylist, sync API, build-copy

scripts/copy-yaml.cjs

@@ -19,3 +19,10 @@ function walk(src, dst) {
 }
 
 walk('src/clis', 'dist/clis');
+
+// Copy external CLI registry to dist/
+const extSrc = 'src/external-clis.yaml';
+if (existsSync(extSrc)) {
+  mkdirSync('dist', { recursive: true });
+  copyFileSync(extSrc, 'dist/external-clis.yaml');
+}

src/cli.ts

@@ -158,25 +158,28 @@ export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void {
     .option('--install <cmd>', 'Auto-install command')
     .option('--desc <text>', 'Description')
     .action((name, opts) => {
-      registerExternalCli(name, opts.binary, opts.install, opts.desc);
+      registerExternalCli(name, { binary: opts.binary, install: opts.install, description: opts.desc });
     });
 
+  // Helper: extract args from process.argv and passthrough to external CLI
+  function passthroughExternal(name: string) {
+    const idx = process.argv.indexOf(name);
+    const args = process.argv.slice(idx + 1);
+    try {
+      executeExternalCli(name, args, externalClis);
+    } catch (err: any) {
+      console.error(chalk.red(`Error: ${err.message}`));
+      process.exitCode = 1;
+    }
+  }
+
   for (const ext of externalClis) {
     if (program.commands.some(c => c.name() === ext.name)) continue;
     program.command(ext.name)
       .description(`(External) ${ext.description || ext.name}`)
       .allowUnknownOption()
       .allowExcessArguments()
-      .action(() => {
-        // Retrieve args passed to the external CLI
-        // Commander consumes standard args before the action, so we must slice process.argv directly.
-        const extIndex = process.argv.indexOf(ext.name);
-        const args = process.argv.slice(extIndex + 1);
-        executeExternalCli(ext.name, args).catch(err => {
-          console.error(chalk.red(`Error: ${err.message}`));
-          process.exitCode = 1;
-        });
-      });
+      .action(() => passthroughExternal(ext.name));
   }
 
   // ── Antigravity serve (built-in, long-running) ──────────────────────────────
@@ -291,18 +294,24 @@ export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void {
     });
   }
 
+  // Dangerous system commands that should never be auto-registered
+  const DENY_LIST = new Set([
+    'rm', 'sudo', 'dd', 'mkfs', 'fdisk', 'shutdown', 'reboot',
+    'kill', 'killall', 'chmod', 'chown', 'passwd', 'su', 'mount',
+    'umount', 'format', 'diskutil',
+  ]);
+
   program.on('command:*', (operands: string[]) => {
     const binary = operands[0];
+    if (DENY_LIST.has(binary)) {
+      console.error(chalk.red(`Refusing to register system command '${binary}'.`));
+      process.exitCode = 1;
+      return;
+    }
     if (isBinaryInstalled(binary)) {
       console.log(chalk.cyan(`🔹 Auto-discovered local CLI '${binary}'. Registering...`));
       registerExternalCli(binary);
-      // Execute it
-      const extIndex = process.argv.indexOf(binary);
-      const args = process.argv.slice(extIndex + 1);
-      executeExternalCli(binary, args).catch(err => {
-        console.error(chalk.red(`Error: ${err.message}`));
-        process.exitCode = 1;
-      });
+      passthroughExternal(binary);
     } else {
       console.error(chalk.red(`error: unknown command '${binary}'`));
       program.outputHelp();

src/external.ts

@@ -2,7 +2,7 @@ import * as fs from 'node:fs';
 import * as path from 'node:path';
 import * as os from 'node:os';
 import { fileURLToPath } from 'node:url';
-import { spawnSync, execSync } from 'node:child_process';
+import { spawnSync, execSync, execFileSync } from 'node:child_process';
 import yaml from 'js-yaml';
 import chalk from 'chalk';
 import { log } from './logger.js';
@@ -65,8 +65,7 @@ export function loadExternalClis(): ExternalCliConfig[] {
 export function isBinaryInstalled(binary: string): boolean {
   try {
     const isWindows = os.platform() === 'win32';
-    const cmd = isWindows ? 'where' : 'command -v';
-    execSync(`${cmd} ${binary}`, { stdio: 'ignore' });
+    execFileSync(isWindows ? 'where' : 'which', [binary], { stdio: 'ignore' });
     return true;
   } catch {
     return false;
@@ -109,8 +108,8 @@ export function installExternalCli(cli: ExternalCliConfig): boolean {
   }
 }
 
-export async function executeExternalCli(name: string, args: string[]): Promise<void> {
-  const configs = loadExternalClis();
+export function executeExternalCli(name: string, args: string[], preloaded?: ExternalCliConfig[]): void {
+  const configs = preloaded ?? loadExternalClis();
   const cli = configs.find((c) => c.name === name);
   if (!cli) {
     throw new Error(`External CLI '${name}' not found in registry.`);
@@ -126,8 +125,7 @@ export async function executeExternalCli(name: string, args: string[]): Promise<
     }
   }
 
-  // 3. Passthrough execution
-  // We use spawnSync to properly inherit stdio and block until completion
+  // 3. Passthrough execution with stdio inherited
   const result = spawnSync(cli.binary, args, { stdio: 'inherit' });
   if (result.error) {
     console.error(chalk.red(`Failed to execute '${cli.binary}': ${result.error.message}`));
@@ -140,7 +138,13 @@ export async function executeExternalCli(name: string, args: string[]): Promise<
   }
 }
 
-export function registerExternalCli(name: string, binary?: string, install?: string, description?: string): void {
+export interface RegisterOptions {
+  binary?: string;
+  install?: string;
+  description?: string;
+}
+
+export function registerExternalCli(name: string, opts?: RegisterOptions): void {
   const userPath = getUserRegistryPath();
   const configDir = path.dirname(userPath);
 
@@ -162,13 +166,12 @@ export function registerExternalCli(name: string, binary?: string, install?: str
   
   const newItem: ExternalCliConfig = {
     name,
-    binary: binary || name,
+    binary: opts?.binary || name,
   };
-  if (description) newItem.description = description;
-  if (install) newItem.install = { default: install };
+  if (opts?.description) newItem.description = opts.description;
+  if (opts?.install) newItem.install = { default: opts.install };
 
   if (existingIndex >= 0) {
-    // Merge
     items[existingIndex] = { ...items[existingIndex], ...newItem };
     console.log(chalk.green(`Updated '${name}' in user registry.`));
   } else {