From ba9d9f390ec3fd07536d216f3ecccb8b617fc0af Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur200126@gmail.com>
Date: Thu, 12 Sep 2019 13:53:19 +0800
Subject: [PATCH 01/20] child_process: escape args[] for shell

BREAKING CHANGE: This changes the behavior of args[] in `shell: true` to
escape globs and other metacharacters. If you want to keep your scripts,
do it the proper way and stuff them in "command."

Refs: https://github.com/nodejs/node/pull/29576
Fixes: https://github.com/nodejs/node/issues/29532
---
 doc/api/child_process.md                      | 73 ++++++++++++++---
 lib/child_process.js                          | 77 ++++++++++++------
 .../test-child-process-spawn-shell.js         |  2 +-
 ...test-child-process-spawnsync-shell-args.js | 79 +++++++++++++++++++
 .../test-child-process-spawnsync-shell.js     |  2 +-
 5 files changed, 200 insertions(+), 33 deletions(-)
 create mode 100644 test/parallel/test-child-process-spawnsync-shell-args.js

diff --git a/doc/api/child_process.md b/doc/api/child_process.md
index afd756db79ec..1f70c7913895 100644
--- a/doc/api/child_process.md
+++ b/doc/api/child_process.md
@@ -469,8 +469,10 @@ changes:
     See [Advanced serialization][] for more details. **Default:** `'json'`.
   * `shell` {boolean|string} If `true`, runs `command` inside of a shell. Uses
     `'/bin/sh'` on Unix, and `process.env.ComSpec` on Windows. A different
-    shell can be specified as a string. See [Shell requirements][] and
-    [Default Windows shell][]. **Default:** `false` (no shell).
+    shell can be specified as a string. Unlike `command`, `args` will be quoted
+    and no metacharacters will be effective inside of it. See
+    [Shell Requirements][] and [Default Windows Shell][]. **Default:** `false`
+    (no shell).
   * `windowsVerbatimArguments` {boolean} No quoting or escaping of arguments is
     done on Windows. Ignored on Unix. This is set to `true` automatically
     when `shell` is specified and is CMD. **Default:** `false`.
@@ -818,8 +820,10 @@ changes:
     normally be created on Windows systems. **Default:** `false`.
   * `shell` {boolean|string} If `true`, runs `command` inside of a shell. Uses
     `'/bin/sh'` on Unix, and `process.env.ComSpec` on Windows. A different
-    shell can be specified as a string. See [Shell requirements][] and
-    [Default Windows shell][]. **Default:** `false` (no shell).
+    shell can be specified as a string. Unlike `command`, `args` will be quoted
+    and no metacharacters will be effective inside of it. See
+    [Shell Requirements][] and [Default Windows Shell][]. **Default:** `false`
+    (no shell).
 * Returns: {Buffer|string} The stdout from the command.
 
 The `child_process.execFileSync()` method is generally identical to
@@ -949,8 +953,10 @@ changes:
     **Default:** `'buffer'`.
   * `shell` {boolean|string} If `true`, runs `command` inside of a shell. Uses
     `'/bin/sh'` on Unix, and `process.env.ComSpec` on Windows. A different
-    shell can be specified as a string. See [Shell requirements][] and
-    [Default Windows shell][]. **Default:** `false` (no shell).
+    shell can be specified as a string. Unlike `command`, `args` will be quoted
+    and no metacharacters will be effective inside of it. See
+    [Shell Requirements][] and [Default Windows Shell][]. **Default:** `false`
+    (no shell).
   * `windowsVerbatimArguments` {boolean} No quoting or escaping of arguments is
     done on Windows. Ignored on Unix. This is set to `true` automatically
     when `shell` is specified and is CMD. **Default:** `false`.
@@ -1640,7 +1646,35 @@ The shell should understand the `-c` switch. If the shell is `'cmd.exe'`, it
 should understand the `/d /s /c` switches and command-line parsing should be
 compatible.
 
-## Default Windows shell
+If `shell` and `args` are both specified, the shell should understand
+[POSIX single-quotes][]. If the shell is `'cmd.exe'`, it should understand the
+double-quoting used by Windows `cmd`. If the shell is `'powershell.exe'` or
+`'pwsh'`, it should understand a powershell-compatible single quoting.
+
+Since the `command` part is literally passed to the shell, you will be
+responsible for any escaping you perform. The specific way to call a file
+as a command-line program depends on the shell:
+
+* With POSIX shell, you quote the path using the usual single-quotes. You
+  may want to prefix it with `command --` or `env --` to suppress syntax,
+  function, and/or alias lookups. You can simply use `'command --'` as your
+  `command` argument and put the path on the first element of `args`.
+* With PowerShell, it is often needed to prefix the command with `&` when the
+  executable path is quoted. You can use a similar technique with `command`
+  set to `'&'`.
+* With CMD, you should wrap double quotes around the paths to the executable.
+  Do NOT escape anything inside of it.
+
+As a caveat for `'cmd'`, `args` should additionally not contain any newlines
+because `cmd` cannot handle such. See also [Windows Command Line][] for when
+else not to rely on the builtin escaping mechanism on Windows.
+
+In addition, PowerShell has an [ongoing, cross-platform issue][] with how it
+translates the `argv` for external programs to a Windows-style command-line
+string. Until it is fixed, the built-in PowerShell escape provided by Node.js
+is guaranteed to work for built-in commands only.
+
+## Default Windows Shell
 
 Although Microsoft specifies `%COMSPEC%` must contain the path to
 `'cmd.exe'` in the root environment, child processes are not always subject to
@@ -1648,7 +1682,21 @@ the same requirement. Thus, in `child_process` functions where a shell can be
 spawned, `'cmd.exe'` is used as a fallback if `process.env.ComSpec` is
 unavailable.
 
-## Advanced serialization
+## Windows Command Line
+
+There is no universal way in Windows to do command-line escapes as every
+program are exposed natively to its full cmdline as a string, and each of them
+can have their own parsing rules, which includes optional glob expansion,
+officially provided as `_setargv`.
+
+The most common parsing is defined in the C runtime library as
+`CommandLineToArgvW`. Microsoft describes the rules in
+[Parsing C++ Command-Line Arguments][]. When any other rules are known to be
+used, `windowsVerbatimArguments` should be specified with manual argument
+quoting. Since `cmd` has a `/s` switch that allows for verbatim processing
+of the command string, we turn on the flag to take advantage of that feature.
+
+## Advanced Serialization
 <!-- YAML
 added:
  - v13.2.0
@@ -1704,6 +1752,13 @@ or [`child_process.fork()`][].
 [`subprocess.stdin`]: #child_process_subprocess_stdin
 [`subprocess.stdio`]: #child_process_subprocess_stdio
 [`subprocess.stdout`]: #child_process_subprocess_stdout
-[`util.promisify()`]: util.md#util_util_promisify_original
+[`util.promisify()`]: util.html#util_util_promisify_original
+[Default Windows Shell]: #child_process_default_windows_shell
+[HTML structured clone algorithm]: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm
+[Shell Requirements]: #child_process_shell_requirements
+[POSIX single-quotes]: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_02_02
+[Parsing C++ Command-Line Arguments]: https://docs.microsoft.com/en-us/cpp/cpp/parsing-cpp-command-line-arguments
+[Windows Command Line]: #child_process_windows_command_line
+[ongoing, cross-platform issue]: https://github.com/PowerShell/PowerShell/issues/1995
 [synchronous counterparts]: #child_process_synchronous_process_creation
 [v8.serdes]: v8.md#v8_serialization_api
diff --git a/lib/child_process.js b/lib/child_process.js
index daa1d44e8974..a28c6ea6272a 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -527,29 +527,40 @@ function normalizeSpawnArguments(file, args, options) {
   }
 
   if (options.shell) {
-    const command = ArrayPrototypeJoin([file, ...args], ' ');
-    // Set the shell, switches, and commands.
-    if (process.platform === 'win32') {
-      if (typeof options.shell === 'string')
-        file = options.shell;
-      else
-        file = process.env.comspec || 'cmd.exe';
-      // '/d /s /c' is used only for cmd.exe.
-      if (RegExpPrototypeTest(/^(?:.*\\)?cmd(?:\.exe)?$/i, file)) {
-        args = ['/d', '/s', '/c', `"${command}"`];
-        windowsVerbatimArguments = true;
-      } else {
-        args = ['-c', command];
-      }
-    } else {
-      if (typeof options.shell === 'string')
-        file = options.shell;
-      else if (process.platform === 'android')
-        file = '/system/bin/sh';
-      else
-        file = '/bin/sh';
-      args = ['-c', command];
+    const command = file;
+    const commandArgs = args.map((i) => i + '');
+    args = ['-c'];
+    let quote = quotePosixShArg;
+    let isCmd = false;
+    let testWindowsExe = (_c, _f) => false;
+    // Set the shell.
+    if (typeof options.shell === 'string')
+      file = options.shell;
+    else if (process.platform === 'win32') {
+      file = process.env.comspec || 'cmd.exe';
+      testWindowsExe = (cmd, file) =>
+        new RegExp(`^(?:.*\\\\)?${cmd}(?:\\.exe)?$`, 'i').test(file);
+    } else if (process.platform === 'android')
+      file = '/system/bin/sh';
+    else
+      file = '/bin/sh';
+
+    // Handle special args for special shells.
+    // '/d /s /c' is used only for cmd.exe.
+    if (testWindowsExe('cmd', file)) {
+      args = ['/d', '/s', '/c'];
+      quote = quoteCmdArg;
+      isCmd = windowsVerbatimArguments = true;
+    } else if (
+      testWindowsExe('(powershell|pwsh)', file) ||
+      file.endsWith('/pwsh')
+    ) {
+      quote = quotePwshArg;
     }
+
+    // Cmd accepts quoted things verbatim under /s.
+    const commandArg = [command].concat(commandArgs.map(quote)).join(' ');
+    args.push(isCmd ? `"${commandArg}"` : commandArg);
   }
 
   if (typeof options.argv0 === 'string') {
@@ -775,6 +786,28 @@ function spawnWithSignal(file, args, options) {
   }
   return child;
 }
+
+
+function quotePosixShArg(arg) {
+  return `'${arg.replace(/'/g, "'\\''")}'`;
+}
+
+
+function quoteCmdArg(arg) {
+  // We don't do the verbatim case here, because we need to quote
+  // (but not escape) stuff like & and | too, and that's a long list
+  // to check for on indexOf()
+  return `"${arg.replace(/\\/g, '\\\\').replace(/"/g, '\\"')}"`;
+}
+
+
+function quotePwshArg(arg) {
+  // We need to specifically check for powershell now; it does -c but does
+  // not quote in the same way
+  return `'${arg.replace(/'/g, "''")}'`;
+}
+
+
 module.exports = {
   _forkChild,
   ChildProcess,
diff --git a/test/parallel/test-child-process-spawn-shell.js b/test/parallel/test-child-process-spawn-shell.js
index 9b8de0507130..ca238969cbf8 100644
--- a/test/parallel/test-child-process-spawn-shell.js
+++ b/test/parallel/test-child-process-spawn-shell.js
@@ -24,7 +24,7 @@ const echo = cp.spawn('echo', ['foo'], {
 });
 let echoOutput = '';
 
-assert.strictEqual(echo.spawnargs[echo.spawnargs.length - 1].replace(/"/g, ''),
+assert.strictEqual(echo.spawnargs[echo.spawnargs.length - 1].replace(/['"]/g, ''),
                    'echo foo');
 echo.stdout.on('data', (data) => {
   echoOutput += data;
diff --git a/test/parallel/test-child-process-spawnsync-shell-args.js b/test/parallel/test-child-process-spawnsync-shell-args.js
new file mode 100644
index 000000000000..c442938a0acc
--- /dev/null
+++ b/test/parallel/test-child-process-spawnsync-shell-args.js
@@ -0,0 +1,79 @@
+'use strict';
+const common = require('../common');
+const { ok: mkay, strictEqual: same } = require('assert');
+const cp = require('child_process');
+
+// This test ensures that child_process.spawnSync escapes shell args as
+// required since #29532.
+const run = (shell, command, args, ok = mkay) => {
+  // eslint-disable-next-line prefer-const
+  let { stdout, stderr, error, status } = cp.spawnSync(command, args, {
+    shell: shell,
+  });
+  [stdout, stderr] = [stdout, stderr].map((i) => i.toString());
+  ok(!error && !stderr, [error, status, stderr, stdout].join(' ||| '));
+  return common.isWindows ? stdout.replace(/\r/g, '') : stdout;
+};
+
+// find bash
+const silent = () => {};
+let bashPath = '';
+if (common.isWindows) {
+  const bashPaths = run(true, 'where bash', null, silent)
+    .trim()
+    .split(/[\n]+/g);
+  // We may have multiple results. Pick the one that looks right.
+  for (const i of bashPaths) {
+    // Not you, Bash on Windows stub.
+    // (It parses the cmdline... quite differently.)
+    if (i.indexOf('Windows\\System32') < 0) {
+      bashPath = i;
+      break;
+    }
+  }
+} else {
+  bashPath = run(true, 'command -v bash', null, silent).trim();
+}
+
+if (!bashPath) common.skip('Skipping test because bash is not found.');
+else console.info(`Using bash from ${bashPath}.`);
+
+const reprint = "printf '%q '";
+const testArgs = ['  Lore"m  Ipsu""m\ndolor\tsit', 'on \\a\x07 ringing chair.'];
+const testArgsLite = ['"Hello \\"World|&*', "y'all"];
+const reprintArgs = ['-c', `${reprint} "$@"`, '--', ...testArgs];
+const nodeEcho = [
+  '-e',
+  'process.argv.slice(1).forEach((x) => console.log(x))',
+  '--',
+];
+
+const echoOut = testArgs.concat(['']).join('\n');
+same(run(bashPath, 'printf "%s\n"', testArgs), echoOut);
+same(run(false, process.execPath, nodeEcho.concat(testArgs)), echoOut);
+
+const echoOutLite = testArgsLite.concat(['']).join('\n');
+same(run(bashPath, 'printf "%s\n"', testArgsLite), echoOutLite);
+
+const reprintOut = run(bashPath, reprint, testArgs);
+same(run(false, 'bash', reprintArgs), reprintOut);
+
+if (common.isWindows) {
+  same(run('powershell', 'Write-Output', testArgs), echoOut);
+
+  // XXX: Here we see where pwsh internal cmdline quoting clashes with actual
+  // Windows cmdline handling. Microsoft, don't you have the source for both?
+  //
+  // Ref: PowerShell/PowerShell#1995
+  // same(
+  //   run('powershell', `& "${process.execPath}"`, nodeEcho.concat(testArgs)),
+  //   echoOut
+  // );
+
+  // Cmd has troubles with newlines, so we lower the bar
+  // (not using 'echo' because cmd handles builtin parsing differently)
+  same(
+    run('cmd', `"${process.execPath}"`, nodeEcho.concat(testArgsLite)),
+    echoOutLite
+  );
+}
diff --git a/test/parallel/test-child-process-spawnsync-shell.js b/test/parallel/test-child-process-spawnsync-shell.js
index 0dd50cedd123..573a9419e991 100644
--- a/test/parallel/test-child-process-spawnsync-shell.js
+++ b/test/parallel/test-child-process-spawnsync-shell.js
@@ -20,7 +20,7 @@ else
 
 // Verify that passing arguments works
 internalCp.spawnSync = common.mustCall(function(opts) {
-  assert.strictEqual(opts.args[opts.args.length - 1].replace(/"/g, ''),
+  assert.strictEqual(opts.args[opts.args.length - 1].replace(/["']/g, ''),
                      'echo foo');
   return oldSpawnSync(opts);
 });

From fe443d2ff6adb1d4791443707d8a08253505f5cf Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur200126@gmail.com>
Date: Sat, 21 Sep 2019 00:39:25 +0800
Subject: [PATCH 02/20] child_process: improve escape on cmd and docs

node-cross-spawn points me to a page with a not entirely correct Perl
thing for escaping cmd arguments. Throwing its metacharacters in does
improve our coverage of cmd cases. It also lead me to the unfortunate
conclusion that I have to use an undocumented trivia to make quotes work
consistently on CMD.

Applied the working change about sh env/command stuff.
---
 doc/api/child_process.md                           |  7 +++----
 lib/child_process.js                               | 14 ++++++++++----
 .../test-child-process-spawnsync-shell-args.js     | 10 ++++++----
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/doc/api/child_process.md b/doc/api/child_process.md
index 1f70c7913895..560d4eef5fac 100644
--- a/doc/api/child_process.md
+++ b/doc/api/child_process.md
@@ -1655,10 +1655,9 @@ Since the `command` part is literally passed to the shell, you will be
 responsible for any escaping you perform. The specific way to call a file
 as a command-line program depends on the shell:
 
-* With POSIX shell, you quote the path using the usual single-quotes. You
-  may want to prefix it with `command --` or `env --` to suppress syntax,
-  function, and/or alias lookups. You can simply use `'command --'` as your
-  `command` argument and put the path on the first element of `args`.
+* With POSIX shell, you quote the path using the usual single-quotes.
+  To auto-escape the path, use `'command --'` as your `command` argument and
+  put the path on the first element of `args`.
 * With PowerShell, it is often needed to prefix the command with `&` when the
   executable path is quoted. You can use a similar technique with `command`
   set to `'&'`.
diff --git a/lib/child_process.js b/lib/child_process.js
index a28c6ea6272a..d6464524b340 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -794,10 +794,16 @@ function quotePosixShArg(arg) {
 
 
 function quoteCmdArg(arg) {
-  // We don't do the verbatim case here, because we need to quote
-  // (but not escape) stuff like & and | too, and that's a long list
-  // to check for on indexOf()
-  return `"${arg.replace(/\\/g, '\\\\').replace(/"/g, '\\"')}"`;
+  // We are doing a bit of magic here:
+  //  1. The backslash-escape only cares about the consecutive ones *before*
+  //     a quote. That is, before a quote ends or before our magic escape.
+  //  2. The same applies to *not* closing a quote. Throwing a backslash in
+  //     there does not really deter CMD, so we use the UNDOCUMENTED double-
+  //     the-quotes escape I picked up from CoreFx Sys.Diag.Process.Unix.
+  //
+  // Don't put newlines in here; there is still no way to escape it. ^ before
+  // EOL only does continuation.
+  return `"${arg.replace(/(\\*)($|")/g, '$1$1$2').replace(/"/g, '""')}"`;
 }
 
 
diff --git a/test/parallel/test-child-process-spawnsync-shell-args.js b/test/parallel/test-child-process-spawnsync-shell-args.js
index c442938a0acc..fcceeb373935 100644
--- a/test/parallel/test-child-process-spawnsync-shell-args.js
+++ b/test/parallel/test-child-process-spawnsync-shell-args.js
@@ -39,12 +39,12 @@ if (!bashPath) common.skip('Skipping test because bash is not found.');
 else console.info(`Using bash from ${bashPath}.`);
 
 const reprint = "printf '%q '";
-const testArgs = ['  Lore"m  Ipsu""m\ndolor\tsit', 'on \\a\x07 ringing chair.'];
-const testArgsLite = ['"Hello \\"World|&*', "y'all"];
+const testArgs = ['  Lore"> Ipsu""<\nd|or\tsit', 'on \\a\x07 ringing ch&air'];
+const testArgsLite = ['He\\\\o \\"Wor\tld\\|<>&*\\', "y'all\x07"];
 const reprintArgs = ['-c', `${reprint} "$@"`, '--', ...testArgs];
 const nodeEcho = [
   '-e',
-  'process.argv.slice(1).forEach((x) => console.log(x))',
+  'console.log(process.argv.slice(1).join("\\n"))',
   '--',
 ];
 
@@ -53,7 +53,9 @@ same(run(bashPath, 'printf "%s\n"', testArgs), echoOut);
 same(run(false, process.execPath, nodeEcho.concat(testArgs)), echoOut);
 
 const echoOutLite = testArgsLite.concat(['']).join('\n');
-same(run(bashPath, 'printf "%s\n"', testArgsLite), echoOutLite);
+// Git MSYS Bash cmdline parsing quirk
+if (!common.isWindows)
+  same(run(bashPath, 'printf "%s\n"', testArgsLite), echoOutLite);
 
 const reprintOut = run(bashPath, reprint, testArgs);
 same(run(false, 'bash', reprintArgs), reprintOut);

From 0d8de7db585a1c098da67f4f5f7f6853c8404e15 Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur200126@gmail.com>
Date: Sat, 21 Sep 2019 00:50:49 +0800
Subject: [PATCH 03/20] child_process: make escaping opt-in with shellEscape

---
 doc/api/child_process.md                      | 55 +++++++++++--------
 lib/child_process.js                          | 11 ++++
 ...test-child-process-spawnsync-shell-args.js |  1 +
 3 files changed, 45 insertions(+), 22 deletions(-)

diff --git a/doc/api/child_process.md b/doc/api/child_process.md
index 560d4eef5fac..f9b3017f5fb2 100644
--- a/doc/api/child_process.md
+++ b/doc/api/child_process.md
@@ -103,7 +103,7 @@ When running on Windows, `.bat` and `.cmd` files can be invoked using
 [`child_process.exec()`][], or by spawning `cmd.exe` and passing the `.bat` or
 `.cmd` file as an argument (which is what the `shell` option and
 [`child_process.exec()`][] do). In any case, if the script filename contains
-spaces it needs to be quoted.
+spaces it needs to be quoted or invoked with `shellEscape: true`.
 
 ```js
 // On Windows Only...
@@ -278,8 +278,10 @@ changes:
     done on Windows. Ignored on Unix. **Default:** `false`.
   * `shell` {boolean|string} If `true`, runs `command` inside of a shell. Uses
     `'/bin/sh'` on Unix, and `process.env.ComSpec` on Windows. A different
-    shell can be specified as a string. See [Shell requirements][] and
-    [Default Windows shell][]. **Default:** `false` (no shell).
+    shell can be specified as a string. See [Shell Requirements][] and
+    [Default Windows Shell][]. **Default:** `false` (no shell).
+  * `shellEscape` {boolean} If `true`, escape `args[]`, but not `command`,
+    for the shell selected. **Default:** `false` (no escaping).
   * `signal` {AbortSignal} allows aborting the execFile using an AbortSignal.
 * `callback` {Function} Called with the output when process terminates.
   * `error` {Error}
@@ -469,10 +471,10 @@ changes:
     See [Advanced serialization][] for more details. **Default:** `'json'`.
   * `shell` {boolean|string} If `true`, runs `command` inside of a shell. Uses
     `'/bin/sh'` on Unix, and `process.env.ComSpec` on Windows. A different
-    shell can be specified as a string. Unlike `command`, `args` will be quoted
-    and no metacharacters will be effective inside of it. See
-    [Shell Requirements][] and [Default Windows Shell][]. **Default:** `false`
-    (no shell).
+    shell can be specified as a string. See [Shell Requirements][] and
+    [Default Windows Shell][]. **Default:** `false` (no shell).
+  * `shellEscape` {boolean} If `true`, escape `args[]`, but not `command`,
+    for the shell selected. **Default:** `false` (no escaping).
   * `windowsVerbatimArguments` {boolean} No quoting or escaping of arguments is
     done on Windows. Ignored on Unix. This is set to `true` automatically
     when `shell` is specified and is CMD. **Default:** `false`.
@@ -820,10 +822,10 @@ changes:
     normally be created on Windows systems. **Default:** `false`.
   * `shell` {boolean|string} If `true`, runs `command` inside of a shell. Uses
     `'/bin/sh'` on Unix, and `process.env.ComSpec` on Windows. A different
-    shell can be specified as a string. Unlike `command`, `args` will be quoted
-    and no metacharacters will be effective inside of it. See
-    [Shell Requirements][] and [Default Windows Shell][]. **Default:** `false`
-    (no shell).
+    shell can be specified as a string. See [Shell Requirements][] and
+    [Default Windows Shell][]. **Default:** `false` (no shell).
+  * `shellEscape` {boolean} If `true`, escape `args[]`, but not `command`,
+    for the shell selected. **Default:** `false` (no escaping).
 * Returns: {Buffer|string} The stdout from the command.
 
 The `child_process.execFileSync()` method is generally identical to
@@ -953,10 +955,10 @@ changes:
     **Default:** `'buffer'`.
   * `shell` {boolean|string} If `true`, runs `command` inside of a shell. Uses
     `'/bin/sh'` on Unix, and `process.env.ComSpec` on Windows. A different
-    shell can be specified as a string. Unlike `command`, `args` will be quoted
-    and no metacharacters will be effective inside of it. See
-    [Shell Requirements][] and [Default Windows Shell][]. **Default:** `false`
-    (no shell).
+    shell can be specified as a string. See [Shell Requirements][] and
+    [Default Windows Shell][]. **Default:** `false` (no shell).
+  * `shellEscape` {boolean} If `true`, escape `args[]`, but not `command`,
+    for the shell selected. **Default:** `false` (no escaping).
   * `windowsVerbatimArguments` {boolean} No quoting or escaping of arguments is
     done on Windows. Ignored on Unix. This is set to `true` automatically
     when `shell` is specified and is CMD. **Default:** `false`.
@@ -1646,14 +1648,23 @@ The shell should understand the `-c` switch. If the shell is `'cmd.exe'`, it
 should understand the `/d /s /c` switches and command-line parsing should be
 compatible.
 
-If `shell` and `args` are both specified, the shell should understand
-[POSIX single-quotes][]. If the shell is `'cmd.exe'`, it should understand the
-double-quoting used by Windows `cmd`. If the shell is `'powershell.exe'` or
-`'pwsh'`, it should understand a powershell-compatible single quoting.
+### Shell Escaping
 
-Since the `command` part is literally passed to the shell, you will be
-responsible for any escaping you perform. The specific way to call a file
-as a command-line program depends on the shell:
+`shellEscape` is an option introduced in Node 1X.Y.Z. It is intended to improve
+the safety of shell command-line by unifying the behavior of `args[]` with
+regard to metacharacters with the `shell: false` case. All metacharacters and
+shell-specific scripting should be placed in `command` instead. To preserve
+backward compatibility, it is designed as an opt-in feature.
+
+If all of `shell`, `args`, and `shellEscape` are specified, the shell should
+understand [POSIX single-quotes][]. If the shell is `'cmd.exe'`, it should
+understand the double-quoting used by Windows `cmd`. If the shell is
+`'powershell.exe'` or `'pwsh'`, it should understand a powershell-compatible
+single quoting.
+
+Since the `command` part is literally passed to the shell, you are responsible
+for any escaping you perform. The specific way to call a file as a command-line
+program depends on the shell:
 
 * With POSIX shell, you quote the path using the usual single-quotes.
   To auto-escape the path, use `'command --'` as your `command` argument and
diff --git a/lib/child_process.js b/lib/child_process.js
index d6464524b340..c83b6bf59947 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -504,6 +504,13 @@ function normalizeSpawnArguments(file, args, options) {
                                    ['boolean', 'string'], options.shell);
   }
 
+  // Validate shellEscape, if present.
+  if (options.shellEscape != null &&
+      typeof options.shellEscape !== 'boolean') {
+    throw new ERR_INVALID_ARG_TYPE('options.shellEscape',
+                                   ['boolean'], options.shellEscape);
+  }
+
   // Validate argv0, if present.
   if (options.argv0 != null &&
       typeof options.argv0 !== 'string') {
@@ -558,6 +565,10 @@ function normalizeSpawnArguments(file, args, options) {
       quote = quotePwshArg;
     }
 
+    // Make it opt-in... for now.
+    if (!options.shellEscape)
+      quote = (s) => s;
+
     // Cmd accepts quoted things verbatim under /s.
     const commandArg = [command].concat(commandArgs.map(quote)).join(' ');
     args.push(isCmd ? `"${commandArg}"` : commandArg);
diff --git a/test/parallel/test-child-process-spawnsync-shell-args.js b/test/parallel/test-child-process-spawnsync-shell-args.js
index fcceeb373935..d17e2c1610be 100644
--- a/test/parallel/test-child-process-spawnsync-shell-args.js
+++ b/test/parallel/test-child-process-spawnsync-shell-args.js
@@ -9,6 +9,7 @@ const run = (shell, command, args, ok = mkay) => {
   // eslint-disable-next-line prefer-const
   let { stdout, stderr, error, status } = cp.spawnSync(command, args, {
     shell: shell,
+    shellEscape: true
   });
   [stdout, stderr] = [stdout, stderr].map((i) => i.toString());
   ok(!error && !stderr, [error, status, stderr, stdout].join(' ||| '));

From 242ecd9c2545d138384e3aa41cc1288df2365c1b Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur200126@gmail.com>
Date: Tue, 8 Oct 2019 15:13:50 +0800
Subject: [PATCH 04/20] child_process: document Cygwin/MSYS, one case when you
 MUST go verbatim

---
 doc/api/child_process.md | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/doc/api/child_process.md b/doc/api/child_process.md
index f9b3017f5fb2..9d69771cfb84 100644
--- a/doc/api/child_process.md
+++ b/doc/api/child_process.md
@@ -1697,7 +1697,8 @@ unavailable.
 There is no universal way in Windows to do command-line escapes as every
 program are exposed natively to its full cmdline as a string, and each of them
 can have their own parsing rules, which includes optional glob expansion,
-officially provided as `_setargv`.
+officially provided as `_setargv`. There is no reliable way of escaping the
+MS CRT glob, and we do not try to do that in the underlying `uv` library.
 
 The most common parsing is defined in the C runtime library as
 `CommandLineToArgvW`. Microsoft describes the rules in
@@ -1706,6 +1707,38 @@ used, `windowsVerbatimArguments` should be specified with manual argument
 quoting. Since `cmd` has a `/s` switch that allows for verbatim processing
 of the command string, we turn on the flag to take advantage of that feature.
 
+A very common, alternative command-line parsing method is the Cygwin/MSYS2
+[`build_argv`]. It uses always-on globbing based on the POSIX glob(3),
+with special provisions for dos-like paths. You may escape it as:
+
+```js
+function quoteCygwinArg(arg, mayBePath = true) {
+  // Cygwin escapes the backslashes on paths to handle the Windows verbatim
+  // passing-in of them.
+  // (There is a Cygwin bug in here about " never being escapable on these
+  // paths：https://cygwin.com/ml/cygwin/2019-10/msg00033.html)
+  const isPath = mayBePath ?
+    !!(/^\\\\[a-z][^\\]+\\|^[a-z]:/i.exec(arg)) :
+    /^[a-z]:/i.exec(arg);
+  if (!isPath) {
+    arg = arg.replace(/[\\]/g, '\\\\');
+    if (/^\\\\[a-z][^\\]+\\/i.exec(arg))
+      // If Cygwin sees this as an SMB network path...
+      // Chop off the first \\ (it seems to not eat up the a in \a)
+      arg = arg.substring(2);
+  }
+  return `"${arg.replace(/"/g, '""')}"`;
+}
+
+// For shell (set { shellEscape: false } for spawn):
+exec([command].concat(args.map(quoteCygwinArg)).join(' '));
+// For non-shell:
+spawn(command, args.map(quoteCygwinArg), { windowsVerbatimArguments: true });
+```
+
+MinGW uses the native Windows mechanism and can be used safely with the
+default escapes.
+
 ## Advanced Serialization
 <!-- YAML
 added:
@@ -1770,5 +1803,6 @@ or [`child_process.fork()`][].
 [Parsing C++ Command-Line Arguments]: https://docs.microsoft.com/en-us/cpp/cpp/parsing-cpp-command-line-arguments
 [Windows Command Line]: #child_process_windows_command_line
 [ongoing, cross-platform issue]: https://github.com/PowerShell/PowerShell/issues/1995
+[`build_argv`]: https://github.com/mirror/newlib-cygwin/blob/b39cd00/winsup/cygwin/dcrt0.cc#L292
 [synchronous counterparts]: #child_process_synchronous_process_creation
 [v8.serdes]: v8.md#v8_serialization_api

From d75fe07681930765ce4aa26fc194f4d68d66f6af Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur200126@gmail.com>
Date: Mon, 21 Oct 2019 19:28:26 +0800
Subject: [PATCH 05/20] doc: add big flashing bolded <marquee> security warning

---
 doc/api/child_process.md | 13 ++++++++-----
 lib/child_process.js     |  2 ++
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/doc/api/child_process.md b/doc/api/child_process.md
index 9d69771cfb84..23539f716416 100644
--- a/doc/api/child_process.md
+++ b/doc/api/child_process.md
@@ -1650,10 +1650,10 @@ compatible.
 
 ### Shell Escaping
 
-`shellEscape` is an option introduced in Node 1X.Y.Z. It is intended to improve
-the safety of shell command-line by unifying the behavior of `args[]` with
-regard to metacharacters with the `shell: false` case. All metacharacters and
-shell-specific scripting should be placed in `command` instead. To preserve
+`shellEscape` is an option introduced in Node.js 1X.Y.Z. It is intended to
+improve the safety of shell command-line by unifying the behavior of `args[]`
+with regard to metacharacters with the `shell: false` case. All metacharacters
+and shell-specific scripting should be placed in `command` instead. To preserve
 backward compatibility, it is designed as an opt-in feature.
 
 If all of `shell`, `args`, and `shellEscape` are specified, the shell should
@@ -1684,6 +1684,9 @@ translates the `argv` for external programs to a Windows-style command-line
 string. Until it is fixed, the built-in PowerShell escape provided by Node.js
 is guaranteed to work for built-in commands only.
 
+:*WARNING*: Use caution while using this API. The shell-escaping is not a
+security feature and you should never pass user input into child_process APIs.
+
 ## Default Windows Shell
 
 Although Microsoft specifies `%COMSPEC%` must contain the path to
@@ -1708,7 +1711,7 @@ quoting. Since `cmd` has a `/s` switch that allows for verbatim processing
 of the command string, we turn on the flag to take advantage of that feature.
 
 A very common, alternative command-line parsing method is the Cygwin/MSYS2
-[`build_argv`]. It uses always-on globbing based on the POSIX glob(3),
+[`build_argv`][]. It uses always-on globbing based on the POSIX glob(3),
 with special provisions for dos-like paths. You may escape it as:
 
 ```js
diff --git a/lib/child_process.js b/lib/child_process.js
index c83b6bf59947..29be53ce4933 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -42,6 +42,8 @@ const {
   SafeSet,
   StringPrototypeSlice,
   StringPrototypeToUpperCase,
+  Set,
+  RegExp
 } = primordials;
 
 const {

From a01ac853c1d45356a3024219594bb5cd19fa8cef Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur200126@gmail.com>
Date: Sun, 25 Oct 2020 02:27:27 +0800
Subject: [PATCH 06/20] Apply suggestions from code review [fixme]

(NOT INCLUDING THE ARRAY STUFF)

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
---
 lib/child_process.js                          | 35 ++++++++++++-------
 ...test-child-process-spawnsync-shell-args.js |  2 +-
 2 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/lib/child_process.js b/lib/child_process.js
index 29be53ce4933..b3e41e638997 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -43,7 +43,11 @@ const {
   StringPrototypeSlice,
   StringPrototypeToUpperCase,
   Set,
-  RegExp
+  RegExp,
+  RegExpPrototypeTest,
+  String,
+  StringPrototypeEndsWith,
+  StringPrototypeReplace,
 } = primordials;
 
 const {
@@ -537,22 +541,23 @@ function normalizeSpawnArguments(file, args, options) {
 
   if (options.shell) {
     const command = file;
-    const commandArgs = args.map((i) => i + '');
+    const commandArgs = ArrayPrototypeMap(args, String);
     args = ['-c'];
     let quote = quotePosixShArg;
     let isCmd = false;
     let testWindowsExe = (_c, _f) => false;
     // Set the shell.
-    if (typeof options.shell === 'string')
+    if (typeof options.shell === 'string') {
       file = options.shell;
-    else if (process.platform === 'win32') {
+    } else if (process.platform === 'win32') {
       file = process.env.comspec || 'cmd.exe';
       testWindowsExe = (cmd, file) =>
-        new RegExp(`^(?:.*\\\\)?${cmd}(?:\\.exe)?$`, 'i').test(file);
-    } else if (process.platform === 'android')
+        RegExpPrototypeTest(new RegExp(`^(?:.*\\\\)?${cmd}(?:\\.exe)?$`, 'i'), file);
+    } else if (process.platform === 'android') {
       file = '/system/bin/sh';
-    else
+    } else {
       file = '/bin/sh';
+    }
 
     // Handle special args for special shells.
     // '/d /s /c' is used only for cmd.exe.
@@ -562,7 +567,7 @@ function normalizeSpawnArguments(file, args, options) {
       isCmd = windowsVerbatimArguments = true;
     } else if (
       testWindowsExe('(powershell|pwsh)', file) ||
-      file.endsWith('/pwsh')
+      StringPrototypeEndsWith(file, '/pwsh')
     ) {
       quote = quotePwshArg;
     }
@@ -572,8 +577,11 @@ function normalizeSpawnArguments(file, args, options) {
       quote = (s) => s;
 
     // Cmd accepts quoted things verbatim under /s.
-    const commandArg = [command].concat(commandArgs.map(quote)).join(' ');
-    args.push(isCmd ? `"${commandArg}"` : commandArg);
+    const commandArg = ArrayPrototypeJoin(
+      [command, ...ArrayPrototypeMap(commandArgs, quote)],
+      ' '
+    );
+    ArrayPrototypePush(args, isCmd ? `"${commandArg}"` : commandArg);
   }
 
   if (typeof options.argv0 === 'string') {
@@ -802,7 +810,7 @@ function spawnWithSignal(file, args, options) {
 
 
 function quotePosixShArg(arg) {
-  return `'${arg.replace(/'/g, "'\\''")}'`;
+  return `'${StringPrototypeReplace(arg, /'/g, "'\\''")}'`;
 }
 
 
@@ -816,14 +824,15 @@ function quoteCmdArg(arg) {
   //
   // Don't put newlines in here; there is still no way to escape it. ^ before
   // EOL only does continuation.
-  return `"${arg.replace(/(\\*)($|")/g, '$1$1$2').replace(/"/g, '""')}"`;
+  return `"${StringPrototypeReplace(
+    StringPrototypeReplace(arg, /(\\*)($|")/g, '$1$1$2'), /"/g, '""')}"`;
 }
 
 
 function quotePwshArg(arg) {
   // We need to specifically check for powershell now; it does -c but does
   // not quote in the same way
-  return `'${arg.replace(/'/g, "''")}'`;
+  return `'${StringPrototypeReplace(arg, /'/g, "''")}'`;
 }
 
 
diff --git a/test/parallel/test-child-process-spawnsync-shell-args.js b/test/parallel/test-child-process-spawnsync-shell-args.js
index d17e2c1610be..428b69c870ac 100644
--- a/test/parallel/test-child-process-spawnsync-shell-args.js
+++ b/test/parallel/test-child-process-spawnsync-shell-args.js
@@ -13,7 +13,7 @@ const run = (shell, command, args, ok = mkay) => {
   });
   [stdout, stderr] = [stdout, stderr].map((i) => i.toString());
   ok(!error && !stderr, [error, status, stderr, stdout].join(' ||| '));
-  return common.isWindows ? stdout.replace(/\r/g, '') : stdout;
+  return common.isWindows ? stdout.replaceAll('\r', '') : stdout;
 };
 
 // find bash

From 0ccc509daae6c0c85945accafa6e393cd15a5e87 Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur200126@gmail.com>
Date: Sun, 25 Oct 2020 14:32:35 +0800
Subject: [PATCH 07/20] Apply more suggestions from code review

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
---
 doc/api/child_process.md | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/doc/api/child_process.md b/doc/api/child_process.md
index 23539f716416..5d071f33dda0 100644
--- a/doc/api/child_process.md
+++ b/doc/api/child_process.md
@@ -1649,8 +1649,11 @@ should understand the `/d /s /c` switches and command-line parsing should be
 compatible.
 
 ### Shell Escaping
+<!-- YAML
+added: REPLACEME
+-->
 
-`shellEscape` is an option introduced in Node.js 1X.Y.Z. It is intended to
+`shellEscape` is an option intended to
 improve the safety of shell command-line by unifying the behavior of `args[]`
 with regard to metacharacters with the `shell: false` case. All metacharacters
 and shell-specific scripting should be placed in `command` instead. To preserve
@@ -1720,17 +1723,16 @@ function quoteCygwinArg(arg, mayBePath = true) {
   // passing-in of them.
   // (There is a Cygwin bug in here about " never being escapable on these
   // paths：https://cygwin.com/ml/cygwin/2019-10/msg00033.html)
-  const isPath = mayBePath ?
-    !!(/^\\\\[a-z][^\\]+\\|^[a-z]:/i.exec(arg)) :
-    /^[a-z]:/i.exec(arg);
+  const regex = mayBePath ? /^\\\\[a-z][^\\]+\\|^[a-z]:/i : /^[a-z]:/i;
+  const isPath = regex.test(arg);
   if (!isPath) {
-    arg = arg.replace(/[\\]/g, '\\\\');
-    if (/^\\\\[a-z][^\\]+\\/i.exec(arg))
+    arg = arg.replaceAll('\\', '\\\\');
+    if (/^\\\\[a-z][^\\]+\\/i.test(arg))
       // If Cygwin sees this as an SMB network path...
       // Chop off the first \\ (it seems to not eat up the a in \a)
       arg = arg.substring(2);
   }
-  return `"${arg.replace(/"/g, '""')}"`;
+  return `"${arg.replaceAll('"', '""')}"`;
 }
 
 // For shell (set { shellEscape: false } for spawn):

From 240fddf83baff4f9dfc29f03d7aa1e0aa542d306 Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur2e5@aosc.io>
Date: Sun, 25 Oct 2020 14:29:07 +0800
Subject: [PATCH 08/20] child_process.md: link ordering (incomplete)

Is there a way to automatically do this? Running lint-md -o rewrites too much.
---
 doc/api/child_process.md | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/doc/api/child_process.md b/doc/api/child_process.md
index 5d071f33dda0..d82f420dc8f7 100644
--- a/doc/api/child_process.md
+++ b/doc/api/child_process.md
@@ -1717,6 +1717,8 @@ A very common, alternative command-line parsing method is the Cygwin/MSYS2
 [`build_argv`][]. It uses always-on globbing based on the POSIX glob(3),
 with special provisions for dos-like paths. You may escape it as:
 
+<!-- FIXME: Trim this section as soon as the new Cygwin parser is up.
+     I know it's my PR and I've been sloppy there too... -->
 ```js
 function quoteCygwinArg(arg, mayBePath = true) {
   // Cygwin escapes the backslashes on paths to handle the Windows verbatim
@@ -1768,7 +1770,12 @@ or [`child_process.fork()`][].
 [Advanced serialization]: #child_process_advanced_serialization
 [Default Windows shell]: #child_process_default_windows_shell
 [HTML structured clone algorithm]: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm
+[`build_argv`]: https://github.com/mirror/newlib-cygwin/blob/b39cd00/winsup/cygwin/dcrt0.cc#L292
 [Shell requirements]: #child_process_shell_requirements
+[ongoing, cross-platform issue]: https://github.com/PowerShell/PowerShell/issues/1995
+[`options.detached`]: #child_process_options_detached
+[POSIX single-quotes]: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_02_02
+[Parsing C++ Command-Line Arguments]: https://docs.microsoft.com/en-us/cpp/cpp/parsing-cpp-command-line-arguments
 [`'disconnect'`]: process.md#process_event_disconnect
 [`'error'`]: #child_process_event_error
 [`'exit'`]: #child_process_event_exit
@@ -1786,7 +1793,6 @@ or [`child_process.fork()`][].
 [`maxBuffer` and Unicode]: #child_process_maxbuffer_and_unicode
 [`net.Server`]: net.md#net_class_net_server
 [`net.Socket`]: net.md#net_class_net_socket
-[`options.detached`]: #child_process_options_detached
 [`process.disconnect()`]: process.md#process_process_disconnect
 [`process.env`]: process.md#process_process_env
 [`process.execPath`]: process.md#process_process_execpath
@@ -1800,14 +1806,7 @@ or [`child_process.fork()`][].
 [`subprocess.stdin`]: #child_process_subprocess_stdin
 [`subprocess.stdio`]: #child_process_subprocess_stdio
 [`subprocess.stdout`]: #child_process_subprocess_stdout
-[`util.promisify()`]: util.html#util_util_promisify_original
-[Default Windows Shell]: #child_process_default_windows_shell
-[HTML structured clone algorithm]: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm
-[Shell Requirements]: #child_process_shell_requirements
-[POSIX single-quotes]: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_02_02
-[Parsing C++ Command-Line Arguments]: https://docs.microsoft.com/en-us/cpp/cpp/parsing-cpp-command-line-arguments
+[`util.promisify()`]: util.md#util_util_promisify_original
 [Windows Command Line]: #child_process_windows_command_line
-[ongoing, cross-platform issue]: https://github.com/PowerShell/PowerShell/issues/1995
-[`build_argv`]: https://github.com/mirror/newlib-cygwin/blob/b39cd00/winsup/cygwin/dcrt0.cc#L292
 [synchronous counterparts]: #child_process_synchronous_process_creation
 [v8.serdes]: v8.md#v8_serialization_api

From a0c791a2c3f55f6dee340ae9edc1264986fec766 Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur2e5@aosc.io>
Date: Sun, 25 Oct 2020 14:31:40 +0800
Subject: [PATCH 09/20] js: fixup for suggestion

---
 lib/child_process.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/child_process.js b/lib/child_process.js
index b3e41e638997..282929d3205b 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -27,6 +27,7 @@ const {
   ArrayPrototypeIncludes,
   ArrayPrototypeJoin,
   ArrayPrototypeLastIndexOf,
+  ArrayPrototypeMap,
   ArrayPrototypePush,
   ArrayPrototypeSlice,
   ArrayPrototypeSort,

From 551417b95e96ac70074e240237ae6a5e80859280 Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur2e5@aosc.io>
Date: Sat, 7 Nov 2020 19:57:29 +0800
Subject: [PATCH 10/20] really fix the link ordering

---
 doc/api/child_process.md | 10 +++++-----
 tools/doc/checkLinks.js  |  4 ++++
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/doc/api/child_process.md b/doc/api/child_process.md
index d82f420dc8f7..f1a42095107e 100644
--- a/doc/api/child_process.md
+++ b/doc/api/child_process.md
@@ -1770,12 +1770,10 @@ or [`child_process.fork()`][].
 [Advanced serialization]: #child_process_advanced_serialization
 [Default Windows shell]: #child_process_default_windows_shell
 [HTML structured clone algorithm]: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm
-[`build_argv`]: https://github.com/mirror/newlib-cygwin/blob/b39cd00/winsup/cygwin/dcrt0.cc#L292
-[Shell requirements]: #child_process_shell_requirements
-[ongoing, cross-platform issue]: https://github.com/PowerShell/PowerShell/issues/1995
-[`options.detached`]: #child_process_options_detached
 [POSIX single-quotes]: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_02_02
 [Parsing C++ Command-Line Arguments]: https://docs.microsoft.com/en-us/cpp/cpp/parsing-cpp-command-line-arguments
+[Shell requirements]: #child_process_shell_requirements
+[Windows Command Line]: #child_process_windows_command_line
 [`'disconnect'`]: process.md#process_event_disconnect
 [`'error'`]: #child_process_event_error
 [`'exit'`]: #child_process_event_exit
@@ -1783,6 +1781,7 @@ or [`child_process.fork()`][].
 [`ChildProcess`]: #child_process_class_childprocess
 [`Error`]: errors.md#errors_class_error
 [`EventEmitter`]: events.md#events_class_eventemitter
+[`build_argv`]: https://github.com/mirror/newlib-cygwin/blob/b39cd00/winsup/cygwin/dcrt0.cc#L292
 [`child_process.exec()`]: #child_process_child_process_exec_command_options_callback
 [`child_process.execFile()`]: #child_process_child_process_execfile_file_args_options_callback
 [`child_process.execFileSync()`]: #child_process_child_process_execfilesync_file_args_options
@@ -1793,6 +1792,7 @@ or [`child_process.fork()`][].
 [`maxBuffer` and Unicode]: #child_process_maxbuffer_and_unicode
 [`net.Server`]: net.md#net_class_net_server
 [`net.Socket`]: net.md#net_class_net_socket
+[`options.detached`]: #child_process_options_detached
 [`process.disconnect()`]: process.md#process_process_disconnect
 [`process.env`]: process.md#process_process_env
 [`process.execPath`]: process.md#process_process_execpath
@@ -1807,6 +1807,6 @@ or [`child_process.fork()`][].
 [`subprocess.stdio`]: #child_process_subprocess_stdio
 [`subprocess.stdout`]: #child_process_subprocess_stdout
 [`util.promisify()`]: util.md#util_util_promisify_original
-[Windows Command Line]: #child_process_windows_command_line
+[ongoing, cross-platform issue]: https://github.com/PowerShell/PowerShell/issues/1995
 [synchronous counterparts]: #child_process_synchronous_process_creation
 [v8.serdes]: v8.md#v8_serialization_api
diff --git a/tools/doc/checkLinks.js b/tools/doc/checkLinks.js
index cdee7ca600d0..967d5bcc77ae 100644
--- a/tools/doc/checkLinks.js
+++ b/tools/doc/checkLinks.js
@@ -1,3 +1,7 @@
+// You are probably here because you failed a PR and wants to see if this thing
+// has a --fix option.
+// It doesn't. But if you have some Unix utilities, you can just feed the last
+// lines of your markdown file to `sort`.
 'use strict';
 
 const fs = require('fs');

From 68018aa1af01f20c8e473679bb68b4064a2edf34 Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur2e5@aosc.io>
Date: Sat, 7 Nov 2020 20:01:30 +0800
Subject: [PATCH 11/20] LINT GOD PLEASE ACCEPT MY OFFERING

---
 lib/child_process.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/child_process.js b/lib/child_process.js
index 282929d3205b..120bcab078bc 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -553,7 +553,8 @@ function normalizeSpawnArguments(file, args, options) {
     } else if (process.platform === 'win32') {
       file = process.env.comspec || 'cmd.exe';
       testWindowsExe = (cmd, file) =>
-        RegExpPrototypeTest(new RegExp(`^(?:.*\\\\)?${cmd}(?:\\.exe)?$`, 'i'), file);
+        RegExpPrototypeTest(new RegExp(`^(?:.*\\\\)?${cmd}(?:\\.exe)?$`, 'i'),
+                            file);
     } else if (process.platform === 'android') {
       file = '/system/bin/sh';
     } else {

From d66746c0b5bb7bc10119cb0d8f7f2c7f98892b35 Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur2e5@aosc.io>
Date: Mon, 9 Nov 2020 08:25:58 +0800
Subject: [PATCH 12/20] Allow custom escapers

---
 doc/api/child_process.md | 27 ++++++++++++++++-----------
 lib/child_process.js     |  7 +++++--
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/doc/api/child_process.md b/doc/api/child_process.md
index f1a42095107e..eda5aa310482 100644
--- a/doc/api/child_process.md
+++ b/doc/api/child_process.md
@@ -280,8 +280,9 @@ changes:
     `'/bin/sh'` on Unix, and `process.env.ComSpec` on Windows. A different
     shell can be specified as a string. See [Shell Requirements][] and
     [Default Windows Shell][]. **Default:** `false` (no shell).
-  * `shellEscape` {boolean} If `true`, escape `args[]`, but not `command`,
-    for the shell selected. **Default:** `false` (no escaping).
+  * `shellEscape` {boolean|Function} If `true`, escape `args[]`, but not
+    `command`, for the shell selected. If a function, use it as the escaper.
+    **Default:** `false` (no escaping).
   * `signal` {AbortSignal} allows aborting the execFile using an AbortSignal.
 * `callback` {Function} Called with the output when process terminates.
   * `error` {Error}
@@ -473,8 +474,9 @@ changes:
     `'/bin/sh'` on Unix, and `process.env.ComSpec` on Windows. A different
     shell can be specified as a string. See [Shell Requirements][] and
     [Default Windows Shell][]. **Default:** `false` (no shell).
-  * `shellEscape` {boolean} If `true`, escape `args[]`, but not `command`,
-    for the shell selected. **Default:** `false` (no escaping).
+  * `shellEscape` {boolean|Function} If `true`, escape `args[]`, but not
+    `command`, for the shell selected. If a function, use it as the escaper.
+    **Default:** `false` (no escaping).
   * `windowsVerbatimArguments` {boolean} No quoting or escaping of arguments is
     done on Windows. Ignored on Unix. This is set to `true` automatically
     when `shell` is specified and is CMD. **Default:** `false`.
@@ -824,8 +826,9 @@ changes:
     `'/bin/sh'` on Unix, and `process.env.ComSpec` on Windows. A different
     shell can be specified as a string. See [Shell Requirements][] and
     [Default Windows Shell][]. **Default:** `false` (no shell).
-  * `shellEscape` {boolean} If `true`, escape `args[]`, but not `command`,
-    for the shell selected. **Default:** `false` (no escaping).
+  * `shellEscape` {boolean|Function} If `true`, escape `args[]`, but not
+    `command`, for the shell selected. If a function, use it as the escaper.
+    **Default:** `false` (no escaping).
 * Returns: {Buffer|string} The stdout from the command.
 
 The `child_process.execFileSync()` method is generally identical to
@@ -957,8 +960,9 @@ changes:
     `'/bin/sh'` on Unix, and `process.env.ComSpec` on Windows. A different
     shell can be specified as a string. See [Shell Requirements][] and
     [Default Windows Shell][]. **Default:** `false` (no shell).
-  * `shellEscape` {boolean} If `true`, escape `args[]`, but not `command`,
-    for the shell selected. **Default:** `false` (no escaping).
+  * `shellEscape` {boolean|Function} If `true`, escape `args[]`, but not
+    `command`, for the shell selected. If a function, use it as the escaper.
+    **Default:** `false` (no escaping).
   * `windowsVerbatimArguments` {boolean} No quoting or escaping of arguments is
     done on Windows. Ignored on Unix. This is set to `true` automatically
     when `shell` is specified and is CMD. **Default:** `false`.
@@ -1737,10 +1741,11 @@ function quoteCygwinArg(arg, mayBePath = true) {
   return `"${arg.replaceAll('"', '""')}"`;
 }
 
-// For shell (set { shellEscape: false } for spawn):
-exec([command].concat(args.map(quoteCygwinArg)).join(' '));
 // For non-shell:
-spawn(command, args.map(quoteCygwinArg), { windowsVerbatimArguments: true });
+spawn(command, args, {
+  shellEscape: quoteCygwinArg,
+  windowsVerbatimArguments: true,
+});
 ```
 
 MinGW uses the native Windows mechanism and can be used safely with the
diff --git a/lib/child_process.js b/lib/child_process.js
index 120bcab078bc..eea079dab93f 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -513,9 +513,10 @@ function normalizeSpawnArguments(file, args, options) {
 
   // Validate shellEscape, if present.
   if (options.shellEscape != null &&
-      typeof options.shellEscape !== 'boolean') {
+      typeof options.shellEscape !== 'boolean' &&
+      typeof options.shellEscape !== 'function') {
     throw new ERR_INVALID_ARG_TYPE('options.shellEscape',
-                                   ['boolean'], options.shellEscape);
+                                   ['boolean', 'function'], options.shellEscape);
   }
 
   // Validate argv0, if present.
@@ -577,6 +578,8 @@ function normalizeSpawnArguments(file, args, options) {
     // Make it opt-in... for now.
     if (!options.shellEscape)
       quote = (s) => s;
+    else if (typeof options.shellEscape == 'function')
+      quote = options.shellEscape;
 
     // Cmd accepts quoted things verbatim under /s.
     const commandArg = ArrayPrototypeJoin(

From 19c2125ad5c6ca2520e2f6353f115a11f3bda25d Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur2e5@aosc.io>
Date: Mon, 9 Nov 2020 08:28:06 +0800
Subject: [PATCH 13/20] testWindowsExe should always trigger on win32

---
 lib/child_process.js | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/child_process.js b/lib/child_process.js
index eea079dab93f..73dd077aa451 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -516,7 +516,8 @@ function normalizeSpawnArguments(file, args, options) {
       typeof options.shellEscape !== 'boolean' &&
       typeof options.shellEscape !== 'function') {
     throw new ERR_INVALID_ARG_TYPE('options.shellEscape',
-                                   ['boolean', 'function'], options.shellEscape);
+                                   ['boolean', 'function'],
+                                   options.shellEscape);
   }
 
   // Validate argv0, if present.
@@ -547,15 +548,19 @@ function normalizeSpawnArguments(file, args, options) {
     args = ['-c'];
     let quote = quotePosixShArg;
     let isCmd = false;
-    let testWindowsExe = (_c, _f) => false;
+    const testWindowsExe =
+      process.platform === 'win32' ?
+        (cmd, file) =>
+          RegExpPrototypeTest(
+            new RegExp(`^(?:.*\\\\)?${cmd}(?:\\.exe)?$`, 'i'),
+            file
+          ) :
+        (_c, _f) => false;
     // Set the shell.
     if (typeof options.shell === 'string') {
       file = options.shell;
     } else if (process.platform === 'win32') {
       file = process.env.comspec || 'cmd.exe';
-      testWindowsExe = (cmd, file) =>
-        RegExpPrototypeTest(new RegExp(`^(?:.*\\\\)?${cmd}(?:\\.exe)?$`, 'i'),
-                            file);
     } else if (process.platform === 'android') {
       file = '/system/bin/sh';
     } else {

From 1e54c83922cd9e82c8fc33ba771e4474f335254a Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur2e5@aosc.io>
Date: Mon, 9 Nov 2020 08:34:02 +0800
Subject: [PATCH 14/20] doc

---
 doc/api/child_process.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/api/child_process.md b/doc/api/child_process.md
index eda5aa310482..99539b0a1282 100644
--- a/doc/api/child_process.md
+++ b/doc/api/child_process.md
@@ -1667,7 +1667,7 @@ If all of `shell`, `args`, and `shellEscape` are specified, the shell should
 understand [POSIX single-quotes][]. If the shell is `'cmd.exe'`, it should
 understand the double-quoting used by Windows `cmd`. If the shell is
 `'powershell.exe'` or `'pwsh'`, it should understand a powershell-compatible
-single quoting.
+single quoting. If all fails, you may provide your own quoting function.
 
 Since the `command` part is literally passed to the shell, you are responsible
 for any escaping you perform. The specific way to call a file as a command-line
@@ -1722,7 +1722,7 @@ A very common, alternative command-line parsing method is the Cygwin/MSYS2
 with special provisions for dos-like paths. You may escape it as:
 
 <!-- FIXME: Trim this section as soon as the new Cygwin parser is up.
-     I know it's my PR and I've been sloppy there too... -->
+     It's very annoying to read. -->
 ```js
 function quoteCygwinArg(arg, mayBePath = true) {
   // Cygwin escapes the backslashes on paths to handle the Windows verbatim

From 5821e0aa184245a6493503de1acab88e99912cd3 Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur2e5@aosc.io>
Date: Fri, 13 Nov 2020 00:19:02 +0800
Subject: [PATCH 15/20] remove irrlevant comment

---
 tools/doc/checkLinks.js | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/tools/doc/checkLinks.js b/tools/doc/checkLinks.js
index 967d5bcc77ae..cdee7ca600d0 100644
--- a/tools/doc/checkLinks.js
+++ b/tools/doc/checkLinks.js
@@ -1,7 +1,3 @@
-// You are probably here because you failed a PR and wants to see if this thing
-// has a --fix option.
-// It doesn't. But if you have some Unix utilities, you can just feed the last
-// lines of your markdown file to `sort`.
 'use strict';
 
 const fs = require('fs');

From cdae7ffef0cbc175e26acc6fb1d951f6fad443b0 Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur2e5@aosc.io>
Date: Fri, 13 Nov 2020 00:26:01 +0800
Subject: [PATCH 16/20] add node for cmd arg

---
 lib/child_process.js | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lib/child_process.js b/lib/child_process.js
index 73dd077aa451..8f0a53086c3d 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -834,6 +834,14 @@ function quoteCmdArg(arg) {
   //
   // Don't put newlines in here; there is still no way to escape it. ^ before
   // EOL only does continuation.
+  //
+  // As with POSIX, always quoting is simpler and less prone to breakage.
+  // This also makes behavior consistent wrt globs.
+  //
+  // This is written assuming the receiving program is a normal MSVCRT exe.
+  // Batch files have differences in that cmd double-interprets the string,
+  // but that should not affect us as the result is simply inert to CMD here.
+  // cf. https://daviddeley.com/autohotkey/parameters/parameters.htm#BATCH
   return `"${StringPrototypeReplace(
     StringPrototypeReplace(arg, /(\\*)($|")/g, '$1$1$2'), /"/g, '""')}"`;
 }

From 770d59a6ed6fe67f44ddab07a966b378197bee44 Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur2e5@aosc.io>
Date: Fri, 13 Nov 2020 00:33:16 +0800
Subject: [PATCH 17/20] validate for lack of newline in quoteCmdArg

---
 lib/child_process.js | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/child_process.js b/lib/child_process.js
index 8f0a53086c3d..33006de0adc2 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -842,6 +842,10 @@ function quoteCmdArg(arg) {
   // Batch files have differences in that cmd double-interprets the string,
   // but that should not affect us as the result is simply inert to CMD here.
   // cf. https://daviddeley.com/autohotkey/parameters/parameters.htm#BATCH
+  if (arg.indexOf('\n') != -1 || arg.indexOf('\r') != -1) {
+    throw new ERR_INVALID_ARG_VALUE(`arg`, arg,
+                                    'cannot contain newlines for cmd');
+  }
   return `"${StringPrototypeReplace(
     StringPrototypeReplace(arg, /(\\*)($|")/g, '$1$1$2'), /"/g, '""')}"`;
 }

From 46d190b59ff691e37cd3a7e7920597c355b31c69 Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur2e5@aosc.io>
Date: Mon, 11 Jan 2021 18:58:20 +0800
Subject: [PATCH 18/20] Appease the linter god

---
 lib/child_process.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/child_process.js b/lib/child_process.js
index 33006de0adc2..2c7a9344cbf1 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -842,7 +842,7 @@ function quoteCmdArg(arg) {
   // Batch files have differences in that cmd double-interprets the string,
   // but that should not affect us as the result is simply inert to CMD here.
   // cf. https://daviddeley.com/autohotkey/parameters/parameters.htm#BATCH
-  if (arg.indexOf('\n') != -1 || arg.indexOf('\r') != -1) {
+  if (arg.indexOf('\n') !== -1 || arg.indexOf('\r') !== -1) {
     throw new ERR_INVALID_ARG_VALUE(`arg`, arg,
                                     'cannot contain newlines for cmd');
   }

From 804a34637502961eb48cfcbab37b0809de8444a0 Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur2e5@aosc.io>
Date: Tue, 12 Jan 2021 18:54:24 +0800
Subject: [PATCH 19/20] fix duplicate import

---
 lib/child_process.js | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/lib/child_process.js b/lib/child_process.js
index 2c7a9344cbf1..a297d2598ed4 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -39,16 +39,14 @@ const {
   ObjectDefineProperty,
   ObjectPrototypeHasOwnProperty,
   Promise,
-  RegExpPrototypeTest,
-  SafeSet,
-  StringPrototypeSlice,
-  StringPrototypeToUpperCase,
-  Set,
   RegExp,
   RegExpPrototypeTest,
+  SafeSet,
   String,
   StringPrototypeEndsWith,
   StringPrototypeReplace,
+  StringPrototypeSlice,
+  StringPrototypeToUpperCase,
 } = primordials;
 
 const {

From 0dfba452583c2c9ac46aef681f0456fccdd74cbc Mon Sep 17 00:00:00 2001
From: Mingye Wang <arthur200126@gmail.com>
Date: Fri, 7 May 2021 21:38:49 +0800
Subject: [PATCH 20/20] Apply suggestions from code review

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
---
 lib/child_process.js                                | 11 +++++------
 test/parallel/test-child-process-spawn-shell.js     |  2 +-
 test/parallel/test-child-process-spawnsync-shell.js |  2 +-
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/lib/child_process.js b/lib/child_process.js
index a297d2598ed4..2639b547b3ab 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -585,10 +585,9 @@ function normalizeSpawnArguments(file, args, options) {
       quote = options.shellEscape;
 
     // Cmd accepts quoted things verbatim under /s.
-    const commandArg = ArrayPrototypeJoin(
-      [command, ...ArrayPrototypeMap(commandArgs, quote)],
-      ' '
-    );
+    const commandArg = commandArgs.length ?
+      `${command} ${ArrayPrototypeJoin(ArrayPrototypeMap(commandArgs, quote), ' ')}`:
+      command;
     ArrayPrototypePush(args, isCmd ? `"${commandArg}"` : commandArg);
   }
 
@@ -840,8 +839,8 @@ function quoteCmdArg(arg) {
   // Batch files have differences in that cmd double-interprets the string,
   // but that should not affect us as the result is simply inert to CMD here.
   // cf. https://daviddeley.com/autohotkey/parameters/parameters.htm#BATCH
-  if (arg.indexOf('\n') !== -1 || arg.indexOf('\r') !== -1) {
-    throw new ERR_INVALID_ARG_VALUE(`arg`, arg,
+  if (StringPrototypeIncludes(arg, '\n') || StringPrototypeIncludes(arg, '\r')) {
+    throw new ERR_INVALID_ARG_VALUE('arg', arg,
                                     'cannot contain newlines for cmd');
   }
   return `"${StringPrototypeReplace(
diff --git a/test/parallel/test-child-process-spawn-shell.js b/test/parallel/test-child-process-spawn-shell.js
index ca238969cbf8..71df515db6eb 100644
--- a/test/parallel/test-child-process-spawn-shell.js
+++ b/test/parallel/test-child-process-spawn-shell.js
@@ -24,7 +24,7 @@ const echo = cp.spawn('echo', ['foo'], {
 });
 let echoOutput = '';
 
-assert.strictEqual(echo.spawnargs[echo.spawnargs.length - 1].replace(/['"]/g, ''),
+assert.strictEqual(echo.spawnargs[echo.spawnargs.length - 1].replace(/['"]/g, '', { shellEscape: true }),
                    'echo foo');
 echo.stdout.on('data', (data) => {
   echoOutput += data;
diff --git a/test/parallel/test-child-process-spawnsync-shell.js b/test/parallel/test-child-process-spawnsync-shell.js
index 573a9419e991..861b4631cf6d 100644
--- a/test/parallel/test-child-process-spawnsync-shell.js
+++ b/test/parallel/test-child-process-spawnsync-shell.js
@@ -20,7 +20,7 @@ else
 
 // Verify that passing arguments works
 internalCp.spawnSync = common.mustCall(function(opts) {
-  assert.strictEqual(opts.args[opts.args.length - 1].replace(/["']/g, ''),
+  assert.strictEqual(opts.args[opts.args.length - 1].replace(/["']/g, '', { shellEscape: true }),
                      'echo foo');
   return oldSpawnSync(opts);
 });