diff --git a/packages/data/src/hooks/_exec.test.ts b/packages/data/src/hooks/_exec.test.ts index 9ebbeabf6..c48943ce9 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,13 @@ 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'); + }); }); + diff --git a/packages/data/src/hooks/_exec.ts b/packages/data/src/hooks/_exec.ts index 4cfbaa6d8..80c262abe 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, { shell: false, ...opts }, (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..8c486d63a 100644 --- a/packages/jsx/src/hooks/useSubprocess.test.ts +++ b/packages/jsx/src/hooks/useSubprocess.test.ts @@ -1,108 +1,116 @@ -import { EventEmitter } from 'node:events'; -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'; - -vi.mock('node:child_process', () => ({ - spawn: vi.fn(), -})); - -const mockSpawn = vi.mocked(spawn); - -describe('useSubprocess', () => { - beforeEach(() => { - mockSpawn.mockReset(); - }); - - afterEach(() => { - setCurrentApp(null); - }); - - it('spawns a subprocess with inherited stdio and returns the exit code', async () => { - const proc = new EventEmitter(); - - mockSpawn.mockReturnValue(proc as any); - - const subprocess = useSubprocess(); - const promise = subprocess.run(['git', 'status']); - - proc.emit('close', 7); - - const code = await promise; - - expect(mockSpawn).toHaveBeenCalledWith('git', ['status'], { - stdio: 'inherit', - }); - expect(code).toBe(7); - }); - - 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; - - setCurrentApp(app); - - const proc = new EventEmitter(); - mockSpawn.mockReturnValue(proc as any); - - const subprocess = useSubprocess(); - const promise = subprocess.run(['vim', 'file.txt']); - - expect(app.terminal.exitRawMode).toHaveBeenCalledOnce(); - - proc.emit('close', 0); - - const code = await promise; - - expect(app.terminal.enterRawMode).toHaveBeenCalledOnce(); - expect(app.screen.invalidate).toHaveBeenCalledOnce(); - expect(app.requestRender).toHaveBeenCalledOnce(); - expect(code).toBe(0); - }); - - 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; - - setCurrentApp(app); - - const proc = new EventEmitter(); - mockSpawn.mockReturnValue(proc as any); - - const subprocess = useSubprocess(); - const promise = subprocess.run(['bad-command']); - - proc.emit('error', new Error('spawn failed')); - - await expect(promise).rejects.toThrow('spawn failed'); - - expect(app.terminal.enterRawMode).toHaveBeenCalledOnce(); - expect(app.screen.invalidate).toHaveBeenCalledOnce(); - expect(app.requestRender).toHaveBeenCalledOnce(); - }); - - it('throws when command is empty', async () => { - const subprocess = useSubprocess(); - - await expect(subprocess.run([])).rejects.toThrow( - 'useSubprocess.run requires a command', - ); - }); -}); \ No newline at end of file +import { EventEmitter } from 'node:events'; +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'; + +vi.mock('node:child_process', () => ({ + spawn: vi.fn(), +})); + +const mockSpawn = vi.mocked(spawn); + +describe('useSubprocess', () => { + beforeEach(() => { + mockSpawn.mockReset(); + }); + + afterEach(() => { + setCurrentApp(null); + }); + + it('spawns a subprocess with inherited stdio and returns the exit code', async () => { + const proc = new EventEmitter(); + + mockSpawn.mockReturnValue(proc as any); + + const subprocess = useSubprocess(); + const promise = subprocess.run(['git', 'status']); + + proc.emit('close', 7); + + const code = await promise; + + expect(mockSpawn).toHaveBeenCalledWith('git', ['status'], { + stdio: 'inherit', + }); + expect(code).toBe(7); + }); + + 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; + + setCurrentApp(app); + + const proc = new EventEmitter(); + mockSpawn.mockReturnValue(proc as any); + + const subprocess = useSubprocess(); + const promise = subprocess.run(['vim', 'file.txt']); + + expect(app.terminal.exitRawMode).toHaveBeenCalledOnce(); + + proc.emit('close', 0); + + const code = await promise; + + expect(app.terminal.enterRawMode).toHaveBeenCalledOnce(); + expect(app.screen.invalidate).toHaveBeenCalledOnce(); + expect(app.requestRender).toHaveBeenCalledOnce(); + expect(code).toBe(0); + }); + + 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; + + setCurrentApp(app); + + const proc = new EventEmitter(); + mockSpawn.mockReturnValue(proc as any); + + const subprocess = useSubprocess(); + const promise = subprocess.run(['bad-command']); + + proc.emit('error', new Error('spawn failed')); + + await expect(promise).rejects.toThrow('spawn failed'); + + expect(app.terminal.enterRawMode).toHaveBeenCalledOnce(); + expect(app.screen.invalidate).toHaveBeenCalledOnce(); + expect(app.requestRender).toHaveBeenCalledOnce(); + }); + + it('throws when command is empty', async () => { + const subprocess = useSubprocess(); + + await expect(subprocess.run([])).rejects.toThrow( + '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();