Skip to content

Repository navigation

Commit b57d643

Browse files
sindresorhussindresorhus
committed
Fix wrong-typed options and arguments being silently ignored or coerced
1 parent 74b854a commit b57d643

32 files changed

Lines changed: 342 additions & 35 deletions

‎lib/arguments/options.js‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {validateGracefulCancel} from '../terminate/graceful.js';
88
import {validateTimeout} from '../terminate/timeout.js';
99
import {handleNodeOption} from '../methods/node.js';
1010
import {validateIpcInputOption} from '../ipc/ipc-input.js';
11+
import {checkBooleanOption} from '../utils/options.js';
1112
import {validateEncoding, BINARY_ENCODINGS} from './encoding-option.js';
1213
import {parseCommandFile} from './command-file.js';
1314
import {normalizeCwd} from './cwd.js';
@@ -25,6 +26,8 @@ export const normalizeOptions = (filePath, rawArguments, rawOptions) => {
2526
const [processedFile, processedArguments, processedOptions] = handleNodeOption(filePath, rawArguments, sanitizedOptions);
2627
const fdOptions = normalizeFdSpecificOptions(processedOptions);
2728
const options = addDefaultOptions(fdOptions);
29+
validateBooleanOptions(options);
30+
validateEnv(options);
2831
options.env = getEnv(options);
2932

3033
const {file, commandArguments} = parseCommandFile(processedFile, processedArguments, options);
@@ -49,6 +52,32 @@ export const normalizeOptions = (filePath, rawArguments, rawOptions) => {
4952
return {file, commandArguments, options};
5053
};
5154

55+
const BOOLEAN_OPTIONS = [
56+
'extendEnv',
57+
'preferLocal',
58+
'reject',
59+
'cleanup',
60+
'killDescendants',
61+
'all',
62+
'gracefulCancel',
63+
'ipc',
64+
'node',
65+
];
66+
67+
// Wrong types must be reported instead of being silently coerced, e.g. the string 'false' enabling an option
68+
const validateBooleanOptions = options => {
69+
for (const optionName of BOOLEAN_OPTIONS) {
70+
checkBooleanOption(options[optionName], optionName);
71+
}
72+
};
73+
74+
// A non-object `env` would be spread into unrelated properties, e.g. its characters when a string
75+
const validateEnv = ({env}) => {
76+
if (env !== undefined && (typeof env !== 'object' || env === null || Array.isArray(env))) {
77+
throw new TypeError(`The \`env\` option must be an object: ${String(env)}.`);
78+
}
79+
};
80+
5281
// Use null prototype to prevent prototype pollution from leaking through
5382
const addDefaultOptions = ({
5483
extendEnv = true,

‎lib/arguments/specific.js‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {debuglog} from 'node:util';
22
import isPlainObject from 'is-plain-obj';
33
import {STANDARD_STREAMS_ALIASES} from '../utils/standard-stream.js';
4+
import {checkBooleanOption} from '../utils/options.js';
45

56
// Some options can have different values for `stdout`/`stderr`/`fd3`.
67
// This normalizes those to array of values.
@@ -27,12 +28,18 @@ const getStdioLength = ({stdio}) => Array.isArray(stdio)
2728
? Math.max(stdio.length, STANDARD_STREAMS_ALIASES.length)
2829
: STANDARD_STREAMS_ALIASES.length;
2930

30-
const normalizeFdSpecificValue = (optionValue, optionArray, optionName, stdioLength) => isPlainObject(optionValue)
31-
? normalizeOptionObject(optionValue, optionArray, optionName, stdioLength)
32-
: optionArray.fill(optionValue);
31+
const normalizeFdSpecificValue = (optionValue, optionArray, optionName, stdioLength) => {
32+
if (isPlainObject(optionValue)) {
33+
return normalizeOptionObject(optionValue, optionArray, optionName, stdioLength);
34+
}
35+
36+
validateFdSpecificValue(optionValue, optionName);
37+
return optionArray.fill(optionValue);
38+
};
3339

3440
const normalizeOptionObject = (optionValue, optionArray, optionName, stdioLength) => {
3541
for (const fdName of Object.keys(optionValue).sort(compareFdName)) {
42+
validateFdSpecificValue(optionValue[fdName], optionName, `${optionName}.${fdName}`);
3643
for (const fdNumber of parseFdName(fdName, optionName, stdioLength)) {
3744
optionArray[fdNumber] = optionValue[fdName];
3845
}
@@ -93,6 +100,18 @@ const addDefaultValue = (optionArray, optionName) => optionArray.map(optionValue
93100
? DEFAULT_OPTIONS[optionName]
94101
: optionValue);
95102

103+
const BOOLEAN_FD_SPECIFIC_OPTIONS = new Set(['lines', 'buffer', 'stripFinalNewline']);
104+
105+
// Wrong types must be reported instead of being silently coerced, e.g. a truthy string enabling the `buffer` option.
106+
// `undefined` uses the default value.
107+
const validateFdSpecificValue = (optionValue, optionName, fullOptionName = optionName) => {
108+
if (BOOLEAN_FD_SPECIFIC_OPTIONS.has(optionName)) {
109+
checkBooleanOption(optionValue, fullOptionName);
110+
} else if (optionName === 'maxBuffer' && optionValue !== undefined && (typeof optionValue !== 'number' || Number.isNaN(optionValue))) {
111+
throw new TypeError(`The \`${fullOptionName}\` option must use a number.`);
112+
}
113+
};
114+
96115
// Default value for the `verbose` option
97116
const verboseDefault = debuglog('execa').enabled ? 'full' : 'none';
98117

‎lib/convert/duplex.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import {Duplex} from 'node:stream';
22
import {callbackify} from 'node:util';
33
import {BINARY_ENCODINGS} from '../arguments/encoding-option.js';
4-
import {copyOptions} from '../utils/options.js';
4+
import {copyOptions, checkBooleanOption} from '../utils/options.js';
55
import {
66
getSubprocessStdout,
77
getReadableOptions,
@@ -19,6 +19,8 @@ import {
1919
// Create a `Duplex` stream combining both `subprocess.readable()` and `subprocess.writable()`
2020
export const createDuplex = ({subprocess, concurrentStreams, encoding}, options) => {
2121
const {from, to, binary: binaryOption = true, preserveNewlines = true} = copyOptions(options);
22+
checkBooleanOption(binaryOption, 'binary');
23+
checkBooleanOption(preserveNewlines, 'preserveNewlines');
2224
const binary = binaryOption || BINARY_ENCODINGS.has(encoding);
2325
const {subprocessStdout, waitReadableDestroy} = getSubprocessStdout(subprocess, from, concurrentStreams);
2426
const {subprocessStdin, waitWritableFinal, waitWritableDestroy} = getSubprocessStdin(subprocess, to, concurrentStreams);

‎lib/convert/iterable.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import {BINARY_ENCODINGS} from '../arguments/encoding-option.js';
22
import {getFromStream} from '../arguments/fd-options.js';
33
import {iterateOnSubprocessStream} from '../io/iterate.js';
4-
import {copyOptions} from '../utils/options.js';
4+
import {copyOptions, checkBooleanOption} from '../utils/options.js';
55

66
// Convert the subprocess to an async iterable
77
export const createIterable = (subprocess, encoding, options) => {
88
const {from, binary: binaryOption = false, preserveNewlines = false} = copyOptions(options);
9+
checkBooleanOption(binaryOption, 'binary');
10+
checkBooleanOption(preserveNewlines, 'preserveNewlines');
911
const binary = binaryOption || BINARY_ENCODINGS.has(encoding);
1012
const subprocessStdout = getFromStream(subprocess, from);
1113
const onStdoutData = iterateOnSubprocessStream({

‎lib/convert/readable.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import {BINARY_ENCODINGS} from '../arguments/encoding-option.js';
44
import {getFromStream} from '../arguments/fd-options.js';
55
import {iterateOnSubprocessStream, DEFAULT_OBJECT_HIGH_WATER_MARK} from '../io/iterate.js';
66
import {createDeferred} from '../utils/deferred.js';
7-
import {copyOptions} from '../utils/options.js';
7+
import {copyOptions, checkBooleanOption} from '../utils/options.js';
88
import {addConcurrentStream, waitForConcurrentStreams} from './concurrent.js';
99
import {
1010
safeWaitForSubprocessStdin,
@@ -16,6 +16,8 @@ import {
1616
// Create a `Readable` stream that forwards from `stdout` and awaits the subprocess
1717
export const createReadable = ({subprocess, concurrentStreams, encoding}, options) => {
1818
const {from, binary: binaryOption = true, preserveNewlines = true} = copyOptions(options);
19+
checkBooleanOption(binaryOption, 'binary');
20+
checkBooleanOption(preserveNewlines, 'preserveNewlines');
1921
const binary = binaryOption || BINARY_ENCODINGS.has(encoding);
2022
const {subprocessStdout, waitReadableDestroy} = getSubprocessStdout(subprocess, from, concurrentStreams);
2123
const {readableEncoding, readableObjectMode, readableHighWaterMark} = getReadableOptions(subprocessStdout, binary);

‎lib/ipc/send.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {promisify} from 'node:util';
2-
import {copyOptions} from '../utils/options.js';
2+
import {copyOptions, checkBooleanOption} from '../utils/options.js';
33
import {
44
validateIpcMethod,
55
handleEpipeError,
@@ -15,6 +15,7 @@ import {handleSendStrict, waitForStrictResponse} from './strict.js';
1515
// Also, this would prevent `unhandledRejection` event from being emitted, making it silent.
1616
export const sendMessage = ({anyProcess, channel, isSubprocess, ipc}, message, options) => {
1717
const {strict = false} = copyOptions(options);
18+
checkBooleanOption(strict, 'strict');
1819
const methodName = 'sendMessage';
1920
validateIpcMethod({
2021
methodName,

‎lib/methods/node.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ export const handleNodeOption = (file, commandArguments, {
4949
throw new TypeError('The "execPath" option has been removed. Please use the "nodePath" option instead.');
5050
}
5151

52+
if (!Array.isArray(nodeOptions) || nodeOptions.some(nodeOption => typeof nodeOption !== 'string')) {
53+
throw new TypeError('The "nodeOptions" option must be an array of strings.');
54+
}
55+
5256
const normalizedNodePath = safeNormalizeFileUrl(nodePath, 'The "nodePath" option');
5357
const resolvedNodePath = path.resolve(cwd, normalizedNodePath);
5458
// Use spread (which only copies own properties) to safely get shell without reading polluted prototype

‎lib/methods/parameters.js‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,17 @@ import {safeNormalizeFileUrl} from '../arguments/file-url.js';
33

44
// The command `arguments` and `options` are both optional.
55
// This also does basic validation on them and on the command file.
6-
export const normalizeParameters = (rawFile, rawArguments = [], rawOptions = {}) => {
6+
export const normalizeParameters = (rawFile, rawArguments = [], rawOptions, ...rawExtraArguments) => {
7+
if (rawExtraArguments.length > 0) {
8+
throw new TypeError('Execa accepts at most three arguments: file, arguments, options.');
9+
}
10+
11+
if (rawOptions !== undefined && isPlainObject(rawArguments)) {
12+
throw new TypeError('Execa accepts at most one options object. Please merge the second and third arguments.');
13+
}
14+
715
const filePath = safeNormalizeFileUrl(rawFile, 'First argument');
8-
const [commandArguments, options] = isPlainObject(rawArguments)
16+
const [commandArguments, options = {}] = isPlainObject(rawArguments)
917
? [[], rawArguments]
1018
: [rawArguments, rawOptions];
1119

‎lib/methods/template.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import isPlainObject from 'is-plain-obj';
33
import {ASTRAL_START} from '../arguments/escape.js';
44
import {isUint8Array, uint8ArrayToString} from '../utils/uint-array.js';
55
import {isExecaError} from '../return/final-error.js';
6+
import {getTypeName} from '../utils/type-name.js';
67

78
// Check whether the template string syntax is being used
89
export const isTemplateString = templates => Array.isArray(templates) && Array.isArray(templates.raw);
@@ -139,7 +140,7 @@ const concatTokens = (tokens, nextTokens, isSeparated) => isSeparated
139140

140141
// Handle `${expression}` inside the template string syntax
141142
const parseExpression = expression => {
142-
const typeOfExpression = typeof expression;
143+
const typeOfExpression = getTypeName(expression);
143144

144145
if (typeOfExpression === 'string') {
145146
return expression;
@@ -178,5 +179,5 @@ const getSubprocessResult = result => {
178179
throw new TypeError('Missing result.stdout in template expression. This is probably due to the previous subprocess\' "stdout" option.');
179180
}
180181

181-
throw new TypeError(`Unexpected "${typeof stdout}" stdout in template expression`);
182+
throw new TypeError(`Unexpected "${getTypeName(stdout)}" stdout in template expression`);
182183
};

‎lib/pipe/pipe-arguments.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {getStartTime} from '../return/duration.js';
33
import {SUBPROCESS_OPTIONS, getToStream, getFromStream} from '../arguments/fd-options.js';
44
import {isDenoExecPath} from '../arguments/file-url.js';
55
import {copyOptions} from '../utils/options.js';
6+
import {validateAbortSignal} from '../utils/abort-signal.js';
67

78
// Normalize and validate arguments passed to `source.pipe(destination)`
89
export const normalizePipeArguments = ({source, sourcePromise, boundOptions, createNested}, ...pipeArguments) => {
@@ -44,6 +45,7 @@ const getDestinationStream = (boundOptions, createNested, pipeArguments) => {
4445

4546
try {
4647
const {from, to, unpipeSignal} = copyOptions(pipeOptions);
48+
validateAbortSignal(unpipeSignal, 'unpipeSignal');
4749
const destinationStream = getToStream(destination, to);
4850
return {
4951
destination,

0 commit comments

Comments
 (0)