diff --git a/README.md b/README.md index b7cdc56..f62c544 100644 --- a/README.md +++ b/README.md @@ -30,6 +30,7 @@ import spawnAsync from '@expo/spawn-async'; `spawnAsync` takes the same arguments as [`child_process.spawn`](https://nodejs.org/api/child_process.html#child_process_child_process_spawn_command_args_options). Its options are the same as those of `child_process.spawn` plus: - `ignoreStdio`: whether to ignore waiting for the child process's stdio streams to close before resolving the result promise. When ignoring stdio, the returned values for `stdout` and `stderr` will be empty strings. The default value of this option is `false`. +- `maxBuffer`: the maximum bytes retained from `stdout` and `stderr` (independently). Output is collected with a sliding window. When set explicitly, exceeding it rejects the promise with an error whose `code` is `ERR_CHILD_PROCESS_STDIO_MAXBUFFER` and whose `stdout`/`stderr` carry the truncated tail. When omitted, the default is `buffer.constants.MAX_STRING_LENGTH` (~512 MiB). It returns a promise whose result is an object with these properties: @@ -64,3 +65,13 @@ Here is an example: })(); ``` + +## Notes + +### `maxBuffer` + +`maxBuffer` is a later addition to the API. Set it when child output could exhaust memory and crash the parent process, or when the command or arguments are influenced by untrusted input — an attacker can otherwise force unbounded output to crash the parent. + +The default of `buffer.constants.MAX_STRING_LENGTH` (~512 MiB) is a crash-safe floor, not a memory bound: at that size the materialized string itself can still exhaust process memory. + +When `maxBuffer` is set explicitly, exceeding it rejects the promise immediately with `ERR_CHILD_PROCESS_STDIO_MAXBUFFER`. When left at the default, exceeding it doesn't reject; the sliding-window tail is still readable, but reading `stdout`/`stderr` throws `ERR_CHILD_PROCESS_STDIO_MAXBUFFER` with the truncated tail attached. diff --git a/src/__tests__/spawnAsync-test.ts b/src/__tests__/spawnAsync-test.ts index 56b7d9b..5712f69 100644 --- a/src/__tests__/spawnAsync-test.ts +++ b/src/__tests__/spawnAsync-test.ts @@ -138,6 +138,153 @@ it('throws errors with preserved stack traces when processes return non-zero exi } }); +it(`rejects with ERR_CHILD_PROCESS_STDIO_MAXBUFFER when stdout exceeds maxBuffer`, async () => { + await expect( + spawnAsync( + process.execPath, + ['-e', 'process.stdout.write("a".repeat(1000));'], + { maxBuffer: 100 } + ) + ).rejects.toMatchObject({ + code: 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER', + message: expect.stringMatching(/exceeded maxBuffer of 100 bytes/), + stdout: 'a'.repeat(100), + stderr: '', + status: 0, + }); +}); + +it(`rejects with ERR_CHILD_PROCESS_STDIO_MAXBUFFER when stderr exceeds maxBuffer`, async () => { + await expect( + spawnAsync( + process.execPath, + ['-e', 'process.stderr.write("b".repeat(1000));'], + { maxBuffer: 50 } + ) + ).rejects.toMatchObject({ + code: 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER', + stdout: '', + stderr: 'b'.repeat(50), + }); +}); + +it(`preserves the most recent bytes via sliding window when maxBuffer is exceeded`, async () => { + await expect( + spawnAsync( + process.execPath, + ['-e', 'process.stdout.write("a".repeat(100), () => process.stdout.write("b".repeat(50)));'], + { maxBuffer: 100 } + ) + ).rejects.toMatchObject({ + code: 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER', + stdout: 'a'.repeat(50) + 'b'.repeat(50), + }); +}); + +it(`prefers the exit-code error over the maxBuffer error and exposes the truncated tail`, async () => { + await expect( + spawnAsync( + process.execPath, + ['-e', 'process.stdout.write("a".repeat(1000)); process.exit(2);'], + { maxBuffer: 100 } + ) + ).rejects.toMatchObject({ + message: expect.stringContaining('exited with non-zero code: 2'), + stdout: 'a'.repeat(100), + status: 2, + }); +}); + +it(`allows output up to but not exceeding maxBuffer`, async () => { + const result = await spawnAsync( + process.execPath, + ['-e', 'process.stdout.write("x".repeat(100));'], + { maxBuffer: 100 } + ); + expect(result.stdout).toBe('x'.repeat(100)); +}); + +it(`does not enforce maxBuffer when ignoreStdio is true`, async () => { + const result = await spawnAsync( + process.execPath, + ['-e', 'process.stdout.write("a".repeat(10000));'], + { ignoreStdio: true, maxBuffer: 10 } + ); + expect(result.status).toBe(0); + expect(result.stdout).toBe(''); +}); + +it(`does not enforce maxBuffer when stdio bypasses pipe capture`, async () => { + const result = await spawnAsync( + process.execPath, + ['-e', 'process.stdout.write("a".repeat(10000));'], + { stdio: 'ignore', maxBuffer: 10 } + ); + expect(result.status).toBe(0); + expect(result.stdout).toBe(''); +}); + +it(`listens on 'exit' (not 'close') when stdio is not piped to us`, async () => { + const task = spawnAsync('echo', ['hi'], { stdio: 'ignore' }); + expect(task.child.listenerCount('exit')).toBe(1); + expect(task.child.listenerCount('close')).toBe(0); + await task; +}); + +it(`listens on 'close' (not 'exit') when stdio is piped`, async () => { + const task = spawnAsync('echo', ['hi']); + expect(task.child.listenerCount('close')).toBe(1); + expect(task.child.listenerCount('exit')).toBe(0); + await task; +}); + +describe(`default-cap (lazy) maxBuffer path`, () => { + // The lazy path only triggers against MAX_STRING_LENGTH (~512 MiB), which is + // impractical to generate. Mock the constant so the same code path activates + // at a testable size. + function spawnAsyncWithCap(cap: number) { + let task: any; + jest.isolateModules(() => { + jest.doMock('buffer', () => { + const actual = jest.requireActual('buffer'); + return { + ...actual, + constants: { ...actual.constants, MAX_STRING_LENGTH: cap }, + }; + }); + const localSpawnAsync = require('../spawnAsync'); + task = localSpawnAsync( + process.execPath, + ['-e', 'process.stdout.write("a".repeat(100), () => process.stdout.write("b".repeat(50)));'] + ); + }); + return task as Promise & { child: any }; + } + + it(`resolves the promise without rejecting`, async () => { + const result = await spawnAsyncWithCap(100); + expect(result.status).toBe(0); + expect(result.signal).toBe(null); + }); + + it(`throws ERR_CHILD_PROCESS_STDIO_MAXBUFFER on stdout access with the truncated tail`, async () => { + const result = await spawnAsyncWithCap(100); + let error: any; + try { void result.stdout; } catch (e) { error = e; } + expect(error).toMatchObject({ + code: 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER', + stdout: 'a'.repeat(50) + 'b'.repeat(50), + stderr: '', + }); + }); + + it(`only throws on the overflowed stream; the other reads normally`, async () => { + const result = await spawnAsyncWithCap(100); + expect(result.stderr).toBe(''); + expect(() => result.stdout).toThrow(); + }); +}); + it(`exports TypeScript types`, async () => { let options: SpawnOptions = {}; let promise: SpawnPromise = spawnAsync('echo', ['hi'], options); diff --git a/src/spawnAsync.ts b/src/spawnAsync.ts index 0d8d21a..6f44a55 100644 --- a/src/spawnAsync.ts +++ b/src/spawnAsync.ts @@ -1,9 +1,11 @@ import { ChildProcess, SpawnOptions as NodeSpawnOptions } from 'child_process'; +import { constants as bufferConstants } from 'buffer'; import spawn from 'cross-spawn'; namespace spawnAsync { export interface SpawnOptions extends NodeSpawnOptions { ignoreStdio?: boolean; + maxBuffer?: number; } export interface SpawnPromise extends Promise { @@ -20,6 +22,15 @@ namespace spawnAsync { } } +type IOChunk = string | Buffer; + +interface IOChunksState { + buffer: IOChunk[]; + maxExceeded: boolean; +} + +const DEFAULT_MAX_BUFFER = bufferConstants.MAX_STRING_LENGTH; + function spawnAsync( command: string, args?: ReadonlyArray, @@ -28,79 +39,168 @@ function spawnAsync( const stubError = new Error(); const callerStack = stubError.stack ? stubError.stack.replace(/^.*/, ' ...') : null; - let child: ChildProcess; + const { + ignoreStdio: optionsIgnoreStdio, + maxBuffer: optionsMaxBuffer, + ...nodeOptions + } = options; + + // NOTE(@kitten): When `maxBuffer` is set explicitly, we enforce it strictly + // and don't produce a result without it being strictly enforced + const enforceMaxBufferStrictly = options.maxBuffer != null; + + const ignoreStdio = !!optionsIgnoreStdio; + const maxBuffer = Math.min( + optionsMaxBuffer ?? DEFAULT_MAX_BUFFER, + bufferConstants.MAX_STRING_LENGTH, + ); + + let child: ChildProcess = spawn(command, args, nodeOptions); let promise = new Promise((resolve, reject) => { - let { ignoreStdio, ...nodeOptions } = options; - // @ts-ignore: cross-spawn declares "args" to be a regular array instead of a read-only one - child = spawn(command, args, nodeOptions); - let stdout = ''; - let stderr = ''; + const stdoutChunks: IOChunksState = { buffer: [], maxExceeded: false }; + const stderrChunks: IOChunksState = { buffer: [], maxExceeded: false }; - if (!ignoreStdio) { - if (child.stdout) { - child.stdout.on('data', (data) => { - stdout += data; - }); + function makeHandler(chunks: IOChunksState) { + let length = 0; + return (chunk: IOChunk) => { + chunks.buffer.push(chunk); + length += typeof chunk === 'string' ? Buffer.byteLength(chunk) : chunk.byteLength; + while (chunks.buffer.length > 0 && length > maxBuffer) { + chunks.maxExceeded = true; + chunk = chunks.buffer[0]; + const chunkLength = typeof chunk === 'string' ? Buffer.byteLength(chunk) : chunk.byteLength; + if (length - chunkLength <= maxBuffer) { + const replacement = typeof chunk === 'string' ? Buffer.from(chunk) : chunk; + const excess = length - maxBuffer; + chunks.buffer[0] = replacement.subarray(excess); + length -= excess; + break; + } else { + chunks.buffer.shift(); + length -= chunkLength; + } + } + }; + } + + function attachResult)>( + target: T, + assign: Partial, + stdoutChunks: IOChunksState, + stderrChunks: IOChunksState, + skipMaxBufferCheck?: boolean, + ): T { + function makeMaxBufferError() { + const argumentString = args && args.length > 0 ? ` ${args.join(' ')}` : ''; + const error: Error & { code?: string } = new Error(`${command}${argumentString} exceeded maxBuffer of ${maxBuffer} bytes`); + error.code = 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER'; + return attachResult(error, assign, stdoutChunks, stderrChunks, true); } - if (child.stderr) { - child.stderr.on('data', (data) => { - stderr += data; - }); + let _stdout: string | undefined; + let _stderr: string | undefined; + const map: PropertyDescriptorMap = { + stdout: { + enumerable: true, + configurable: true, + get() { + if (!skipMaxBufferCheck && stdoutChunks.maxExceeded) { + throw makeMaxBufferError(); + } else if (_stdout === undefined) { + _stdout = Buffer.concat( + stdoutChunks.buffer.map((chunk) => typeof chunk === 'string' ? Buffer.from(chunk) : chunk) + ).toString('utf8'); + } + return _stdout; + }, + }, + stderr: { + enumerable: true, + configurable: true, + get() { + if (!skipMaxBufferCheck && stderrChunks.maxExceeded) { + throw makeMaxBufferError(); + } else if (_stderr === undefined) { + _stderr = Buffer.concat( + stderrChunks.buffer.map((chunk) => typeof chunk === 'string' ? Buffer.from(chunk) : chunk) + ).toString('utf8'); + } + return _stderr; + }, + }, + output: { + enumerable: true, + configurable: true, + get: () => [target.stdout, target.stderr], + }, + }; + for (const key in assign) { + map[key] = { + value: assign[key as keyof spawnAsync.SpawnResult], + enumerable: true, + writable: true, + configurable: true, + }; } + Object.defineProperties(target, map); + return target; + } + + if (!ignoreStdio) { + child.stdout?.on('data', makeHandler(stdoutChunks)); + child.stderr?.on('data', makeHandler(stderrChunks)); } + // Use 'exit' instead of 'close' when there are no piped stdio streams for us to drain; + // 'close' can be deferred past 'exit' when the child has grandchildren that inherit its + // stdio fds, so waiting on it without anything to read just stalls + const completionEvent = + ignoreStdio || (!child.stdout && !child.stderr) ? 'exit' : 'close'; + let completionListener = (code: number | null, signal: string | null) => { child.removeListener('error', errorListener); - let result: spawnAsync.SpawnResult = { + const argumentString = args && args.length > 0 ? ` ${args.join(' ')}` : ''; + let error: (Error & { code?: string }) | null = null; + if (code !== 0) { + error = signal + ? new Error(`${command}${argumentString} exited with signal: ${signal}`) + : new Error(`${command}${argumentString} exited with non-zero code: ${code}`); + } + const assignResult: Partial = { pid: child.pid, - output: [stdout, stderr], - stdout, - stderr, status: code, signal, }; - if (code !== 0) { - let argumentString = args && args.length > 0 ? ` ${args.join(' ')}` : ''; - let error = signal - ? new Error(`${command}${argumentString} exited with signal: ${signal}`) - : new Error(`${command}${argumentString} exited with non-zero code: ${code}`); - if (error.stack && callerStack) { - error.stack += `\n${callerStack}`; - } - Object.assign(error, result); - reject(error); + if (error) { + if (error.stack && callerStack) error.stack += `\n${callerStack}`; + // When we're already rejecting, we don't enforce the max buffer error, and accept that we + // may truncate stderr/stdout + reject(attachResult(error, assignResult, stdoutChunks, stderrChunks, true)); + } else if (enforceMaxBufferStrictly && (stdoutChunks.maxExceeded || stderrChunks.maxExceeded)) { + // When a `maxBuffer` is passed, we enforce the maximum on stdout and stderr strictly + const error: Error & { code?: string } = new Error(`${command}${argumentString} exceeded maxBuffer of ${maxBuffer} bytes`); + error.code = 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER'; + reject(attachResult(error, assignResult, stdoutChunks, stderrChunks, true)); } else { - resolve(result); + const result = {} as spawnAsync.SpawnResult; + resolve(attachResult(result, assignResult, stdoutChunks, stderrChunks)); } }; let errorListener = (error: Error) => { - if (ignoreStdio) { - child.removeListener('exit', completionListener); - } else { - child.removeListener('close', completionListener); - } - Object.assign(error, { + child.removeListener(completionEvent, completionListener); + const assignResult: Partial = { pid: child.pid, - output: [stdout, stderr], - stdout, - stderr, status: null, signal: null, - }); - reject(error); + }; + reject(attachResult(error, assignResult, stdoutChunks, stderrChunks)); }; - if (ignoreStdio) { - child.once('exit', completionListener); - } else { - child.once('close', completionListener); - } + child.once(completionEvent, completionListener); child.once('error', errorListener); }) as spawnAsync.SpawnPromise; - // @ts-ignore: TypeScript isn't aware the Promise constructor argument runs synchronously and - // thinks `child` is not yet defined + promise.child = child; return promise; }