fix: upgrade cross-spawn (ReDoS), harden hook install and spawn handling

.gitignore

@@ -1,4 +1,5 @@
 node_modules
 npm-debug.log
 coverage
+.nyc_output
 .tern-port

hook

@@ -1,5 +1,12 @@
 #!/usr/bin/env bash
 
+#
+# Defense-in-depth for magit (the .git/hooks wrapper unsets it too); ensures
+# hooks invoked from emacs/magit behave the same as on the CLI.
+# https://magit.vc/manual/magit/My-Git-hooks-work-on-the-command_002dline-but-not-inside-Magit.html
+#
+unset GIT_LITERAL_PATHSPECS
+
 HAS_NODE=`which node 2> /dev/null || which nodejs 2> /dev/null || which iojs 2> /dev/null`
 
 #
@@ -37,14 +44,41 @@ elif [[ -x "$LOCAL" ]]; then
   BINARY="$LOCAL"
 fi
 
+#
+# Run from the repository root so `require.resolve('pre-commit')` works for Yarn PnP,
+# and GUI git clients that invoke hooks with an unexpected cwd still resolve deps.
+#
+REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null)" || REPO_ROOT=""
+if [[ -n "$REPO_ROOT" ]]; then
+  cd "$REPO_ROOT" || exit 1
+fi
+
+#
+# Resolve the entry point of the `pre-commit` package via Node so we work for
+# Yarn Plug'n'Play, hoisted, and nested layouts. If the package cannot be
+# resolved (e.g. the user switched to a branch with no `node_modules`, or
+# uninstalled `pre-commit`) skip the hook with exit 0 instead of failing the
+# commit -- a missing dev dependency must not block work.
+#
+RESOLVED=
+RESOLVE_RC=1
+if [[ -n "$BINARY" ]]; then
+  RESOLVED="$("$BINARY" -e "try { console.log(require.resolve('pre-commit')); } catch (e) { process.exit(2); }" 2>/dev/null)"
+  RESOLVE_RC=$?
+fi
+
 #
 # Add --dry-run cli flag support so we can execute this hook without side effects
 # and see if it works in the current environment
 #
 if [[ $* == *--dry-run* ]]; then
-  if [[ -z "$BINARY" ]]; then
+  if [[ -z "$BINARY" ]] || [[ "$RESOLVE_RC" -ne 0 ]] || [[ -z "$RESOLVED" ]]; then
     exit 1
   fi
 else
-  "$BINARY" "$("$BINARY" -e "console.log(require.resolve('pre-commit'))")"
+  if [[ "$RESOLVE_RC" -ne 0 ]] || [[ -z "$RESOLVED" ]]; then
+    echo "pre-commit: 'pre-commit' package is not installed; skipping hooks (run \`npm install\` to re-enable)." >&2
+    exit 0
+  fi
+  exec "$BINARY" "$RESOLVED"
 fi

index.js

@@ -1,5 +1,16 @@
 'use strict';
 
+//
+// cross-spawn.spawnSync returns the same shape as child_process.spawnSync
+// (`status`, not `code`).
+//
+function failedSpawn(result) {
+  if (!result) return true;
+  if (result.error) return true;
+  if (result.signal) return true;
+  return result.status !== 0;
+}
+
 var spawn = require('cross-spawn')
   , which = require('which')
   , path = require('path')
@@ -173,8 +184,8 @@ Hook.prototype.initialize = function initialize() {
   this.root = this.exec(this.git, ['rev-parse', '--show-toplevel']);
   this.status = this.exec(this.git, ['status', '--porcelain']);
 
-  if (this.status.code) return this.log(Hook.log.status, 0);
-  if (this.root.code) return this.log(Hook.log.root, 0);
+  if (failedSpawn(this.status)) return this.log(Hook.log.status, 0);
+  if (failedSpawn(this.root)) return this.log(Hook.log.root, 0);
 
   this.status = this.status.stdout.toString().trim();
   this.root = this.root.stdout.toString().trim();
@@ -229,8 +240,11 @@ Hook.prototype.run = function runner() {
       env: process.env,
       cwd: hooked.root,
       stdio: [0, 1, 2]
-    }).once('close', function closed(code) {
-      if (code) return hooked.log(hooked.format(Hook.log.failure, script, code));
+    }).once('close', function closed(code, signal) {
+      var exitCode = typeof code === 'number' ? code : (signal ? 1 : 0);
+      if (exitCode !== 0) {
+        return hooked.log(hooked.format(Hook.log.failure, script, exitCode));
+      }
 
       again(scripts);
     });

install.js

@@ -7,9 +7,17 @@ var fs = require('fs')
   , path = require('path')
   , os = require('os')
   , hook = path.join(__dirname, 'hook')
+  , hookAbs = path.resolve(hook)
   , root = path.resolve(__dirname, '..', '..')
   , exists = fs.existsSync || path.existsSync;
 
+//
+// POSIX single-quoted string for embedding paths in generated shell scripts.
+//
+function shellSingleQuote(str) {
+  return '\'' + str.replace(/'/g, '\'\\\'\'') + '\'';
+}
+
 //
 // Gather the location of the possible hidden .git directory, the hooks
 // directory which contains all git hooks and the absolute location of the
@@ -19,41 +27,50 @@ var fs = require('fs')
 
 var git = getGitFolderPath(root);
 
-// Function to recursively finding .git folder
+//
+// Walk up from `currentPath` looking for `.git`. Returns the path to the `.git`
+// entry as soon as one is found, regardless of whether it is a directory (the
+// regular case) or a file (submodules, linked worktrees, where `.git` contains
+// `gitdir: <path>`).
+//
 function getGitFolderPath(currentPath) {
-  var git = path.resolve(currentPath, '.git')
-
-  if (!exists(git) || !fs.lstatSync(git).isDirectory()) {
-    console.log('pre-commit:');
-    console.log('pre-commit: Not found .git folder in', git);
-
-    var newPath = path.resolve(currentPath, '..');
-
-    // Stop if we on top folder
-    if (currentPath === newPath) {
-      return null;
+  var git = path.resolve(currentPath, '.git');
+
+  if (exists(git)) {
+    var stat = fs.lstatSync(git);
+    if (stat.isDirectory() || stat.isFile()) {
+      console.log('pre-commit:');
+      console.log('pre-commit: Found .git in', git);
+      return git;
     }
-
-    return getGitFolderPath(newPath);
   }
 
   console.log('pre-commit:');
-  console.log('pre-commit: Found .git folder in', git);
-  return git;
+  console.log('pre-commit: No .git found in', currentPath);
+
+  var newPath = path.resolve(currentPath, '..');
+  if (currentPath === newPath) return null;
+
+  return getGitFolderPath(newPath);
 }
 
 //
-// Resolve git directory for submodules
+// When `.git` is a file (submodules and linked worktrees) it contains a
+// `gitdir: <path>` pointer to the real git directory. Paths inside that file
+// are resolved relative to the directory containing the `.git` file, not
+// relative to the package root, so we use `path.dirname(git)` as the base.
 //
-if (exists(git) && fs.lstatSync(git).isFile()) {
-  var gitinfo = fs.readFileSync(git).toString()
-    , gitdirmatch = /gitdir: (.+)/.exec(gitinfo)
-    , gitdir = gitdirmatch.length == 2 ? gitdirmatch[1] : null;
-
-  if (gitdir !== null) {
-    git = path.resolve(root, gitdir);
-    hooks = path.resolve(git, 'hooks');
-    precommit = path.resolve(hooks, 'pre-commit');
+if (git && fs.lstatSync(git).isFile()) {
+  var gitinfo = fs.readFileSync(git, 'utf8')
+    , gitdirmatch = /^gitdir:\s*(.+)$/m.exec(gitinfo)
+    , gitdir = gitdirmatch ? gitdirmatch[1].trim() : null;
+
+  if (gitdir) {
+    git = path.resolve(path.dirname(git), gitdir);
+  } else {
+    console.log('pre-commit:');
+    console.log('pre-commit: .git file did not contain a gitdir pointer; aborting.');
+    return;
   }
 }
 
@@ -81,7 +98,7 @@ if (exists(precommit) && !fs.lstatSync(precommit).isSymbolicLink()) {
   console.log('pre-commit:');
   console.log('pre-commit: Detected an existing git pre-commit hook');
   fs.writeFileSync(precommit +'.old', fs.readFileSync(precommit));
-  console.log('pre-commit: Old pre-commit hook backuped to pre-commit.old');
+  console.log('pre-commit: Old pre-commit hook backed up to pre-commit.old');
   console.log('pre-commit:');
 }
 
@@ -92,20 +109,34 @@ if (exists(precommit) && !fs.lstatSync(precommit).isSymbolicLink()) {
 try { fs.unlinkSync(precommit); }
 catch (e) {}
 
-// Create generic precommit hook that launches this modules hook (as well
-// as stashing - unstashing the unstaged changes)
-// TODO: we could keep launching the old pre-commit scripts
-var hookRelativeUnixPath = hook.replace(root, '.');
-
-if(os.platform() === 'win32') {
-  hookRelativeUnixPath = hookRelativeUnixPath.replace(/[\\\/]+/g, '/');
+// Delegate to this package's `hook` script using an absolute path so Yarn Plug'n'Play
+// and other layouts without `node_modules/pre-commit` still work. The hook script
+// changes to the git root before resolving `pre-commit` via Node.
+//
+var hookLauncher = hookAbs;
+if (os.platform() === 'win32') {
+  hookLauncher = hookLauncher.replace(/\\/g, '/');
 }
 
-var precommitContent = '#!/usr/bin/env bash' + os.EOL
-  +  hookRelativeUnixPath + os.EOL
-  + 'RESULT=$?' + os.EOL
-  + '[ $RESULT -ne 0 ] && exit 1' + os.EOL
-  + 'exit 0' + os.EOL;
+//
+// Generated wrapper:
+//   * Unsets GIT_LITERAL_PATHSPECS so hooks invoked from magit/emacs behave the
+//     same as on the command line. See:
+//     https://magit.vc/manual/magit/My-Git-hooks-work-on-the-command_002dline-but-not-inside-Magit.html
+//   * If the package's `hook` script is missing (e.g. user switched to a branch
+//     without `node_modules`, or removed the `pre-commit` package), skip
+//     silently with exit 0 so commits are not blocked.
+//
+var precommitContent = [
+  '#!/usr/bin/env bash',
+  'unset GIT_LITERAL_PATHSPECS',
+  'HOOK=' + shellSingleQuote(hookLauncher),
+  'if [ ! -f "$HOOK" ]; then',
+  '  exit 0',
+  'fi',
+  'exec bash "$HOOK" "$@"',
+  ''
+].join(os.EOL);
 
 //
 // It could be that we do not have rights to this folder which could cause the
@@ -121,10 +152,10 @@ catch (e) {
   console.error('pre-commit:');
 }
 
-try { fs.chmodSync(precommit, '777'); }
+try { fs.chmodSync(precommit, 0o755); }
 catch (e) {
   console.error('pre-commit:');
-  console.error('pre-commit: chmod 0777 the pre-commit file in your .git/hooks folder because:');
+  console.error('pre-commit: chmod 0755 the pre-commit file in your .git/hooks folder because:');
   console.error('pre-commit: '+ e.message);
   console.error('pre-commit:');
 }