sec: Prevent Arbitrary Command Injection in Terminal Execution Helpers (#3202)

packages/data/src/hooks/_exec.test.ts

@@ -27,7 +27,7 @@ describe('execFileAsync', () => {
             stdout: 'hello stdout',
             stderr: 'hello stderr',
         });
-        expect(spy).toHaveBeenCalledWith('test-bin', ['arg1'], { cwd: '/tmp' }, expect.any(Function));
+        expect(spy).toHaveBeenCalledWith('test-bin', ['arg1'], { cwd: '/tmp', shell: false }, expect.any(Function));
     });
 
     it('should reject with error when execution fails', async () => {
@@ -42,4 +42,29 @@ describe('execFileAsync', () => {
 
         await expect(execFileAsync('test-bin', [])).rejects.toThrow('Spawn failed');
     });
+
+    it('should reject when file path contains null bytes', async () => {
+        await expect(execFileAsync('test-bin\0malicious', [])).rejects.toThrow('execFileAsync: file path contains null bytes');
+    });
+
+    it('should reject when an argument contains null bytes', async () => {
+        await expect(execFileAsync('test-bin', ['safe', 'malicious\0arg'])).rejects.toThrow('execFileAsync: argument contains null bytes');
+    });
+
+    it('should always enforce shell: false even if caller passes shell: true in opts', async () => {
+        // Security invariant: caller must never be able to re-enable shell execution
+        const spy = vi.spyOn(childProcess, 'execFile').mockImplementation(
+            (file, args, opts, callback) => {
+                const cb = typeof opts === 'function' ? opts : callback;
+                cb(null, '', '');
+                return {} as any;
+            }
+        );
+
+        // Even if caller attempts to pass shell: true, it must be overridden to false
+        await execFileAsync('test-bin', [], { shell: true });
+        const calledOpts = spy.mock.calls[0][2] as { shell: boolean };
+        expect(calledOpts.shell).toBe(false);
+    });
 });
+

packages/data/src/hooks/_exec.ts

@@ -1,8 +1,21 @@
 import { execFile } from 'node:child_process';
 
 export const execFileAsync = (file: string, args: string[], opts?: any): Promise<{ stdout: string; stderr: string }> => {
+    if (!file || typeof file !== 'string') {
+        return Promise.reject(new TypeError('execFileAsync: file path must be a non-empty string'));
+    }
+    if (file.includes('\0')) {
+        return Promise.reject(new Error('execFileAsync: file path contains null bytes'));
+    }
+    if (Array.isArray(args)) {
+        for (const arg of args) {
+            if (typeof arg === 'string' && arg.includes('\0')) {
+                return Promise.reject(new Error('execFileAsync: argument contains null bytes'));
+            }
+        }
+    }
     return new Promise((resolve, reject) => {
-        execFile(file, args, opts, (err, stdout, stderr) => {
+        execFile(file, args, { ...opts, shell: false }, (err, stdout, stderr) => {
             if (err) reject(err);
             else resolve({ stdout: String(stdout), stderr: String(stderr) });
         });

packages/jsx/src/hooks/useSubprocess.test.ts

@@ -3,13 +3,32 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 import { useSubprocess } from './useSubprocess.js';
 import { setCurrentApp } from '../runtime.js';
 import { spawn } from 'node:child_process';
+import type { App } from '@termuijs/core';
 
 vi.mock('node:child_process', () => ({
     spawn: vi.fn(),
 }));
 
 const mockSpawn = vi.mocked(spawn);
 
+/**
+ * Create a minimal typed App stub for useSubprocess tests.
+ * We only stub the three properties that useSubprocess accesses:
+ * terminal.exitRawMode, terminal.enterRawMode, screen.invalidate, requestRender.
+ */
+function makeAppStub(): App {
+    return {
+        terminal: {
+            exitRawMode: vi.fn(),
+            enterRawMode: vi.fn(),
+        },
+        screen: {
+            invalidate: vi.fn(),
+        },
+        requestRender: vi.fn(),
+    } as unknown as App; // unknown-first cast is intentional: we supply a typed partial stub
+}
+
 describe('useSubprocess', () => {
     beforeEach(() => {
         mockSpawn.mockReset();
@@ -22,7 +41,7 @@ describe('useSubprocess', () => {
     it('spawns a subprocess with inherited stdio and returns the exit code', async () => {
         const proc = new EventEmitter();
 
-        mockSpawn.mockReturnValue(proc as any);
+        mockSpawn.mockReturnValue(proc as ReturnType<typeof spawn>);
 
         const subprocess = useSubprocess();
         const promise = subprocess.run(['git', 'status']);
@@ -38,21 +57,12 @@ describe('useSubprocess', () => {
     });
 
     it('exits raw mode before spawning and restores the TUI after exit', async () => {
-        const app = {
-            terminal: {
-                exitRawMode: vi.fn(),
-                enterRawMode: vi.fn(),
-            },
-            screen: {
-                invalidate: vi.fn(),
-            },
-            requestRender: vi.fn(),
-        } as any;
+        const app = makeAppStub();
 
         setCurrentApp(app);
 
         const proc = new EventEmitter();
-        mockSpawn.mockReturnValue(proc as any);
+        mockSpawn.mockReturnValue(proc as ReturnType<typeof spawn>);
 
         const subprocess = useSubprocess();
         const promise = subprocess.run(['vim', 'file.txt']);
@@ -70,21 +80,12 @@ describe('useSubprocess', () => {
     });
 
     it('restores raw mode and re-renders when the subprocess emits an error', async () => {
-        const app = {
-            terminal: {
-                exitRawMode: vi.fn(),
-                enterRawMode: vi.fn(),
-            },
-            screen: {
-                invalidate: vi.fn(),
-            },
-            requestRender: vi.fn(),
-        } as any;
+        const app = makeAppStub();
 
         setCurrentApp(app);
 
         const proc = new EventEmitter();
-        mockSpawn.mockReturnValue(proc as any);
+        mockSpawn.mockReturnValue(proc as ReturnType<typeof spawn>);
 
         const subprocess = useSubprocess();
         const promise = subprocess.run(['bad-command']);
@@ -105,4 +106,12 @@ describe('useSubprocess', () => {
             'useSubprocess.run requires a command',
         );
     });
+
+    it('throws when command contains null bytes', async () => {
+        const subprocess = useSubprocess();
+
+        await expect(subprocess.run(['ls', 'dir\0malicious'])).rejects.toThrow(
+            'useSubprocess: command contains null bytes',
+        );
+    });
 });

packages/jsx/src/hooks/useSubprocess.ts

@@ -21,9 +21,14 @@ function spawnProcess(cmd: string[]): Promise<number> {
 
 export function useSubprocess(): UseSubprocessResult {
     async function run(cmd: string[]): Promise<number> {
-        if (cmd.length === 0) {
+        if (!cmd || cmd.length === 0) {
             throw new Error('useSubprocess.run requires a command');
         }
+        for (const part of cmd) {
+            if (typeof part === 'string' && part.includes('\0')) {
+                throw new Error('useSubprocess: command contains null bytes');
+            }
+        }
 
         const app = getCurrentApp();