Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add .escapedCommand property #466

Merged
merged 7 commits into from
Jun 2, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,10 +252,20 @@ declare namespace execa {

interface ExecaReturnBase<StdoutStderrType> {
/**
The file and arguments that were run.
The file and arguments that were run, for logging purpose.
ehmicky marked this conversation as resolved.
Show resolved Hide resolved

This is not escaped and should be passed to neither `execa()` nor `execa.command()`
*/
command: string;

/**
Same as `command` but escaped.

This is meant to be copy/pasted in a shell, for debugging purpose.
ehmicky marked this conversation as resolved.
Show resolved Hide resolved
Since the escaping is fairly basic, this should be passed to neither `execa()` nor `execa.command()`
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To prevent any vulnerability report coming from this, I think we should say that it should not be used with any child process type method. Same with .command.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed 👍

*/
escapedCommand: string;

/**
The numeric exit code of the process that was run.
*/
Expand Down
10 changes: 9 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const normalizeStdio = require('./lib/stdio');
const {spawnedKill, spawnedCancel, setupTimeout, validateTimeout, setExitHandler} = require('./lib/kill');
const {handleInput, getSpawnedResult, makeAllStream, validateInputSync} = require('./lib/stream');
const {mergePromise, getSpawnedPromise} = require('./lib/promise');
const {joinCommand, parseCommand} = require('./lib/command');
const {joinCommand, parseCommand, getEscapedCommand} = require('./lib/command');

const DEFAULT_MAX_BUFFER = 1000 * 1000 * 100;

Expand Down Expand Up @@ -74,6 +74,7 @@ const handleOutput = (options, value, error) => {
const execa = (file, args, options) => {
const parsed = handleArguments(file, args, options);
const command = joinCommand(file, args);
const escapedCommand = getEscapedCommand(file, args);

validateTimeout(parsed.options);

Expand All @@ -89,6 +90,7 @@ const execa = (file, args, options) => {
stderr: '',
all: '',
command,
escapedCommand,
parsed,
timedOut: false,
isCanceled: false,
Expand Down Expand Up @@ -121,6 +123,7 @@ const execa = (file, args, options) => {
stderr,
all,
command,
escapedCommand,
parsed,
timedOut,
isCanceled: context.isCanceled,
Expand All @@ -136,6 +139,7 @@ const execa = (file, args, options) => {

return {
command,
escapedCommand,
exitCode: 0,
stdout,
stderr,
Expand All @@ -161,6 +165,7 @@ module.exports = execa;
module.exports.sync = (file, args, options) => {
const parsed = handleArguments(file, args, options);
const command = joinCommand(file, args);
const escapedCommand = getEscapedCommand(file, args);

validateInputSync(parsed.options);

Expand All @@ -174,6 +179,7 @@ module.exports.sync = (file, args, options) => {
stderr: '',
all: '',
command,
escapedCommand,
parsed,
timedOut: false,
isCanceled: false,
Expand All @@ -192,6 +198,7 @@ module.exports.sync = (file, args, options) => {
signal: result.signal,
exitCode: result.status,
command,
escapedCommand,
parsed,
timedOut: result.error && result.error.code === 'ETIMEDOUT',
isCanceled: false,
Expand All @@ -207,6 +214,7 @@ module.exports.sync = (file, args, options) => {

return {
command,
escapedCommand,
exitCode: 0,
stdout,
stderr,
Expand Down
2 changes: 2 additions & 0 deletions index.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ try {

const unicornsResult = await execaPromise;
expectType<string>(unicornsResult.command);
expectType<string>(unicornsResult.escapedCommand);
expectType<number>(unicornsResult.exitCode);
expectType<string>(unicornsResult.stdout);
expectType<string>(unicornsResult.stderr);
Expand Down Expand Up @@ -47,6 +48,7 @@ try {
try {
const unicornsResult = execa.sync('unicorns');
expectType<string>(unicornsResult.command);
expectType<string>(unicornsResult.escapedCommand);
expectType<number>(unicornsResult.exitCode);
expectType<string>(unicornsResult.stdout);
expectType<string>(unicornsResult.stderr);
Expand Down
30 changes: 25 additions & 5 deletions lib/command.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,33 @@
'use strict';
const SPACES_REGEXP = / +/g;

const joinCommand = (file, args = []) => {
const normalizeArgs = (file, args = []) => {
if (!Array.isArray(args)) {
return file;
return [file];
}

return [file, ...args];
};

const NO_ESCAPE_REGEXP = /^[\w.-]+$/;
const DOUBLE_QUOTES_REGEXP = /"/g;

const escapeArg = arg => {
if (NO_ESCAPE_REGEXP.test(arg)) {
return arg;
}

return [file, ...args].join(' ');
return `"${arg.replace(DOUBLE_QUOTES_REGEXP, '\\"')}"`;
};

const joinCommand = (file, args) => {
return normalizeArgs(file, args).join(' ');
};

const getEscapedCommand = (file, args) => {
return normalizeArgs(file, args).map(arg => escapeArg(arg)).join(' ');
};

const SPACES_REGEXP = / +/g;

// Handle `execa.command()`
const parseCommand = command => {
const tokens = [];
Expand All @@ -28,5 +47,6 @@ const parseCommand = command => {

module.exports = {
joinCommand,
getEscapedCommand,
parseCommand
};
2 changes: 2 additions & 0 deletions lib/error.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const makeError = ({
signal,
exitCode,
command,
escapedCommand,
timedOut,
isCanceled,
killed,
Expand Down Expand Up @@ -61,6 +62,7 @@ const makeError = ({

error.shortMessage = shortMessage;
error.command = command;
error.escapedCommand = escapedCommand;
error.exitCode = exitCode;
error.signal = signal;
error.signalDescription = signalDescription;
Expand Down
15 changes: 14 additions & 1 deletion readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ const execa = require('execa');
originalMessage: 'spawn unknown ENOENT',
shortMessage: 'Command failed with ENOENT: unknown command spawn unknown ENOENT',
command: 'unknown command',
escapedCommand: 'unknown command',
stdout: '',
stderr: '',
all: '',
Expand Down Expand Up @@ -121,6 +122,7 @@ try {
originalMessage: 'spawnSync unknown ENOENT',
shortMessage: 'Command failed with ENOENT: unknown command spawnSync unknown ENOENT',
command: 'unknown command',
escapedCommand: 'unknown command',
stdout: '',
stderr: '',
all: '',
Expand Down Expand Up @@ -234,7 +236,18 @@ The child process [fails](#failed) when:

Type: `string`

The file and arguments that were run.
The file and arguments that were run, for logging purpose.

This is not escaped and should be passed to neither [`execa()`](#execafile-arguments-options) nor [`execa.command()`](#execacommandcommand-options).

#### escapedCommand

Type: `string`

Same as [`command`](#command) but escaped.

This is meant to be copy/pasted in a shell, for debugging purpose.
Since the escaping is fairly basic, this should be passed to neither [`execa()`](#execafile-arguments-options) nor [`execa.command()`](#execacommandcommand-options).

#### exitCode

Expand Down
23 changes: 23 additions & 0 deletions test/command.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,29 @@ test(command, ' foo bar', 'foo', 'bar');
test(command, ' baz quz', 'baz', 'quz');
test(command, '');

const testEscapedCommand = async (t, expected, args) => {
const {escapedCommand: failEscapedCommand} = await t.throwsAsync(execa('fail', args));
t.is(failEscapedCommand, `fail ${expected}`);

const {escapedCommand: failEscapedCommandSync} = t.throws(() => {
execa.sync('fail', args);
});
t.is(failEscapedCommandSync, `fail ${expected}`);

const {escapedCommand} = await execa('noop', args);
t.is(escapedCommand, `noop ${expected}`);

const {escapedCommand: escapedCommandSync} = execa.sync('noop', args);
t.is(escapedCommandSync, `noop ${expected}`);
};

testEscapedCommand.title = (message, expected) => `escapedCommand is: ${JSON.stringify(expected)}`;

test(testEscapedCommand, 'foo bar', ['foo', 'bar']);
test(testEscapedCommand, '"foo bar"', ['foo bar']);
test(testEscapedCommand, '"\\"foo\\""', ['"foo"']);
test(testEscapedCommand, '"*"', ['*']);

test('allow commands with spaces and no array arguments', async t => {
const {stdout} = await execa('command with space');
t.is(stdout, '');
Expand Down