diff --git a/packages/data/src/hooks/_exec.test.ts b/packages/data/src/hooks/_exec.test.ts index 9ebbeabf6..84b8b6d3e 100644 --- a/packages/data/src/hooks/_exec.test.ts +++ b/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); + }); }); + diff --git a/packages/data/src/hooks/_exec.ts b/packages/data/src/hooks/_exec.ts index 4cfbaa6d8..ae82f2b92 100644 --- a/packages/data/src/hooks/_exec.ts +++ b/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) }); }); diff --git a/packages/jsx/src/hooks/useSubprocess.test.ts b/packages/jsx/src/hooks/useSubprocess.test.ts index f708ddc98..2b14a5ef6 100644 --- a/packages/jsx/src/hooks/useSubprocess.test.ts +++ b/packages/jsx/src/hooks/useSubprocess.test.ts @@ -3,6 +3,7 @@ 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(), @@ -10,6 +11,24 @@ vi.mock('node:child_process', () => ({ 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); 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); 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); 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', + ); + }); }); \ No newline at end of file diff --git a/packages/jsx/src/hooks/useSubprocess.ts b/packages/jsx/src/hooks/useSubprocess.ts index 422cb5e30..3c46e807e 100644 --- a/packages/jsx/src/hooks/useSubprocess.ts +++ b/packages/jsx/src/hooks/useSubprocess.ts @@ -21,9 +21,14 @@ function spawnProcess(cmd: string[]): Promise { export function useSubprocess(): UseSubprocessResult { async function run(cmd: string[]): Promise { - 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();