bin: Do not expose filenames to shell expansion

isaacs · isaacs · commit 1e4e297342a0 · 2025-11-17T17:33:59.000-08:00
Backport of 47473c0 from main to v10 branch fix: #636
diff --git a/changelog.md b/changelog.md
@@ -1,5 +1,19 @@
 # changeglob
 
+## 10.5
+
+Backport fix for
+[GHSA-5j98-mcp5-4vw2](https://github.com/isaacs/node-glob/security/advisories/GHSA-5j98-mcp5-4vw2)
+to v10 branch.
+
+- Add the `--shell` option for the command line, with a warning
+  that this is unsafe. (It will be removed in v12.)
+- Add the `--cmd-arg`/`-g` as a way to _safely_ add positional
+  arguments to the command provided to the CLI tool.
+- Detect commands with space or quote characters on known shells,
+  and pass positional arguments to them safely, avoiding
+  `shell:true` execution.
+
 ## 10.4
 
 - Add `includeChildMatches: false` option
diff --git a/package.json b/package.json
@@ -21,12 +21,10 @@
     "./package.json": "./package.json",
     ".": {
       "import": {
-        "source": "./src/index.ts",
         "types": "./dist/esm/index.d.ts",
         "default": "./dist/esm/index.js"
       },
       "require": {
-        "source": "./src/index.ts",
         "types": "./dist/commonjs/index.d.ts",
         "default": "./dist/commonjs/index.js"
       }
diff --git a/src/bin.mts b/src/bin.mts
@@ -3,7 +3,7 @@ import { foregroundChild } from 'foreground-child'
 import { existsSync } from 'fs'
 import { jack } from 'jackspeak'
 import { loadPackageJson } from 'package-json-from-dist'
-import { join } from 'path'
+import { basename, join } from 'path'
 import { globStream } from './index.js'
 
 const { version } = loadPackageJson(import.meta.url, '../package.json')
@@ -35,6 +35,50 @@ const j = jack({
                     this pattern`,
     },
   })
+  .flag({
+    shell: {
+      default: false,
+      description: `Interpret the command as a shell command by passing it
+                    to the shell, with all matched filesystem paths appended,
+                    **even if this cannot be done safely**.
+
+                    This is **not** unsafe (and usually unnecessary) when using
+                    the known Unix shells sh, bash, zsh, and fish, as these can
+                    all be executed in such a way as to pass positional
+                    arguments safely.
+
+                    **Note**: THIS IS UNSAFE IF THE FILE PATHS ARE UNTRUSTED,
+                    because a path like \`'some/path/\\$\\(cmd)'\` will be
+                    executed by the shell.
+
+                    If you do have positional arguments that you wish to pass to
+                    the command ahead of the glob pattern matches, use the
+                    \`--cmd-arg\`/\`-g\` option instead.
+
+                    The next major release of glob will fully remove the ability
+                    to use this option unsafely.`,
+    },
+  })
+  .optList({
+    'cmd-arg': {
+      short: 'g',
+      hint: 'arg',
+      default: [],
+      description: `Pass the provided values to the supplied command, ahead of
+                    the glob matches.
+
+                    For example, the command:
+
+                        glob -c echo -g"hello" -g"world" *.txt
+
+                    might output:
+
+                        hello world a.txt b.txt
+
+                    This is a safer (and future-proof) alternative than putting
+                    positional arguments in the \`-c\`/\`--cmd\` option.`,
+    },
+  })
   .flag({
     all: {
       short: 'A',
@@ -78,7 +122,7 @@ const j = jack({
       description: `Always resolve to posix style paths, using '/' as the
                     directory separator, even on Windows. Drive letter
                     absolute matches on Windows will be expanded to their
-                    full resolved UNC maths, eg instead of 'C:\\foo\\bar',
+                    full resolved UNC paths, eg instead of 'C:\\foo\\bar',
                     it will expand to '//?/C:/foo/bar'.
       `,
     },
@@ -215,8 +259,10 @@ const j = jack({
       description: `Output a huge amount of noisy debug information about
                     patterns as they are parsed and used to match files.`,
     },
-  })
-  .flag({
+    version: {
+      short: 'V',
+      description: `Output the version (${version})`,
+    },
     help: {
       short: 'h',
       description: 'Show this usage information',
@@ -225,50 +271,113 @@ const j = jack({
 
 try {
   const { positionals, values } = j.parse()
-  if (values.help) {
+  const {
+    cmd,
+    shell,
+    all,
+    default: def,
+    version: showVersion,
+    help,
+    absolute,
+    cwd,
+    dot,
+
+    'dot-relative': dotRelative,
+    follow,
+    ignore,
+    'match-base': matchBase,
+    'max-depth': maxDepth,
+    mark,
+    nobrace,
+    nocase,
+    nodir,
+    noext,
+    noglobstar,
+    platform,
+    realpath,
+    root,
+    stat,
+    debug,
+    posix,
+    'cmd-arg': cmdArg,
+  } = values
+  if (showVersion) {
+    console.log(version)
+    process.exit(0)
+  }
+  if (help) {
     console.log(j.usage())
     process.exit(0)
   }
-  if (positionals.length === 0 && !values.default)
-    throw 'No patterns provided'
-  if (positionals.length === 0 && values.default)
-    positionals.push(values.default)
+  //const { shell, help } = values
+  if (positionals.length === 0 && !def) throw 'No patterns provided'
+  if (positionals.length === 0 && def) positionals.push(def)
   const patterns =
-    values.all ? positionals : positionals.filter(p => !existsSync(p))
+    all ? positionals : positionals.filter(p => !existsSync(p))
   const matches =
-    values.all ?
-      []
-    : positionals.filter(p => existsSync(p)).map(p => join(p))
+    all ? [] : positionals.filter(p => existsSync(p)).map(p => join(p))
+
   const stream = globStream(patterns, {
-    absolute: values.absolute,
-    cwd: values.cwd,
-    dot: values.dot,
-    dotRelative: values['dot-relative'],
-    follow: values.follow,
-    ignore: values.ignore,
-    mark: values.mark,
-    matchBase: values['match-base'],
-    maxDepth: values['max-depth'],
-    nobrace: values.nobrace,
-    nocase: values.nocase,
-    nodir: values.nodir,
-    noext: values.noext,
-    noglobstar: values.noglobstar,
-    platform: values.platform as undefined | NodeJS.Platform,
-    realpath: values.realpath,
-    root: values.root,
-    stat: values.stat,
-    debug: values.debug,
-    posix: values.posix,
+    absolute,
+    cwd,
+    dot,
+    dotRelative,
+    follow,
+    ignore,
+    mark,
+    matchBase,
+    maxDepth,
+    nobrace,
+    nocase,
+    nodir,
+    noext,
+    noglobstar,
+    platform: platform as undefined | NodeJS.Platform,
+    realpath,
+    root,
+    stat,
+    debug,
+    posix,
   })
 
-  const cmd = values.cmd
   if (!cmd) {
     matches.forEach(m => console.log(m))
     stream.on('data', f => console.log(f))
   } else {
-    stream.on('data', f => matches.push(f))
-    stream.on('end', () => foregroundChild(cmd, matches, { shell: true }))
+    cmdArg.push(...matches)
+    stream.on('data', f => cmdArg.push(f))
+    // Attempt to support commands that contain spaces and otherwise require
+    // shell interpretation, but do NOT shell-interpret the arguments, to avoid
+    // injections via filenames. This affordance can only be done on known Unix
+    // shells, unfortunately.
+    //
+    // 'bash', ['-c', cmd + ' "$@"', 'bash', ...matches]
+    // 'zsh', ['-c', cmd + ' "$@"', 'zsh', ...matches]
+    // 'fish', ['-c', cmd + ' "$argv"', ...matches]
+    const { SHELL = 'unknown' } = process.env
+    const shellBase = basename(SHELL)
+    const knownShells = ['sh', 'ksh', 'zsh', 'bash', 'fish']
+    if (
+      (shell || /[ "']/.test(cmd)) &&
+      knownShells.includes(shellBase)
+    ) {
+      const cmdWithArgs = `${cmd} "\$${shellBase === 'fish' ? 'argv' : '@'}"`
+      if (shellBase !== 'fish') {
+        cmdArg.unshift(SHELL)
+      }
+      cmdArg.unshift('-c', cmdWithArgs)
+      stream.on('end', () => foregroundChild(SHELL, cmdArg))
+    } else {
+      if (shell) {
+        process.emitWarning(
+          'The --shell option is unsafe, and will be removed. To pass ' +
+            'positional arguments to the subprocess, use -g/--cmd-arg instead.',
+          'DeprecationWarning',
+          'GLOB_SHELL',
+        )
+      }
+      stream.on('end', () => foregroundChild(cmd, cmdArg, { shell }))
+    }
   }
 } catch (e) {
   console.error(j.usage())
diff --git a/tap-snapshots/test/bin.ts.test.cjs b/tap-snapshots/test/bin.ts.test.cjs
@@ -31,6 +31,45 @@ Object {
                              If no positional arguments are provided, glob will use
                              this pattern
     
+      --shell                Interpret the command as a shell command by passing it
+                             to the shell, with all matched filesystem paths
+                             appended,
+                             **even if this cannot be done safely**.
+    
+                             This is **not** unsafe (and usually unnecessary) when
+                             using the known Unix shells sh, bash, zsh, and fish, as
+                             these can all be executed in such a way as to pass
+                             positional arguments safely.
+    
+                             **Note**: THIS IS UNSAFE IF THE FILE PATHS ARE
+                             UNTRUSTED, because a path like \`'some/path/\\\\$\\\\(cmd)'\`
+                             will be executed by the shell.
+    
+                             If you do have positional arguments that you wish to
+                             pass to the command ahead of the glob pattern matches,
+                             use the \`--cmd-arg\`/\`-g\` option instead.
+    
+                             The next major release of glob will fully remove the
+                             ability to use this option unsafely.
+    
+      -g<arg> --cmd-arg=<arg>
+                             Pass the provided values to the supplied command, ahead
+                             of the glob matches.
+    
+                             For example, the command:
+    
+                             glob -c echo -g"hello" -g"world" *.txt
+    
+                             might output:
+    
+                             hello world a.txt b.txt
+    
+                             This is a safer (and future-proof) alternative than
+                             putting positional arguments in the \`-c\`/\`--cmd\`
+                             option.
+    
+                             Can be set multiple times
+    
       -A --all               By default, the glob cli command will not expand any
                              arguments that are an exact match to a file on disk.
     
@@ -60,7 +99,7 @@ Object {
       -x --posix             Always resolve to posix style paths, using '/' as the
                              directory separator, even on Windows. Drive letter
                              absolute matches on Windows will be expanded to their
-                             full resolved UNC maths, eg instead of 'C:\\\\foo\\\\bar', it
+                             full resolved UNC paths, eg instead of 'C:\\\\foo\\\\bar', it
                              will expand to '//?/C:/foo/bar'.
     
       -f --follow            Follow symlinked directories when expanding '**'
@@ -143,8 +182,35 @@ Object {
       -v --debug             Output a huge amount of noisy debug information about
                              patterns as they are parsed and used to match files.
     
+      -V --version           Output the version ({VERSION})
       -h --help              Show this usage information
     
   ),
 }
 `
+
+exports[`test/bin.ts > TAP > version > --version shows version 1`] = `
+Object {
+  "args": Array [
+    "--version",
+  ],
+  "code": 0,
+  "options": Object {},
+  "signal": null,
+  "stderr": "",
+  "stdout": "{VERSION}\\n",
+}
+`
+
+exports[`test/bin.ts > TAP > version > -V shows version 1`] = `
+Object {
+  "args": Array [
+    "-V",
+  ],
+  "code": 0,
+  "options": Object {},
+  "signal": null,
+  "stderr": "",
+  "stdout": "{VERSION}\\n",
+}
+`
diff --git a/test/bin.ts b/test/bin.ts
