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

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

- Bump cross-spawn to ^7.0.5 and which to ^4; drop unused spawn-sync.
- Fix spawnSync result checks (use status/signal/error, not .code).
- Treat non-zero / null close codes from npm run spawns reliably.
- Install hook via absolute path to package hook script for Yarn PnP; chmod 0755.
- Hook: cd to git root before require.resolve; use exec for node.
- Install: guard gitdir parse; avoid fs.existsSync(null) on missing .git.
- Dev: mocha 10, assume 2, nyc; engines node>=16; stub tty in tests.
- Version 1.2.3; add package-lock.json; ignore .nyc_output.

Addresses GH-167, GH-160, GH-157; mitigates GH-166 (cwd / exit handling).

Made-with: Cursor

* chore: fold in fixes from open PRs (#127, #148, #112)

Addresses long-standing open PRs whose intent fits the cross-spawn /
hook-hardening cleanup:

* #127 - install.js: "backuped" -> "backed up".
* #148 - magit workaround: emit `unset GIT_LITERAL_PATHSPECS` in the
  generated `.git/hooks/pre-commit` wrapper, and also at the top of
  the package's `hook` script for defense-in-depth, so hooks invoked
  from emacs/magit behave the same as on the command line.
* #112 - resilience for missing pre-commit package: when a user
  switches to a branch without `node_modules`, the generated wrapper
  now exits 0 instead of failing the commit, and the `hook` script
  detects an unresolvable `pre-commit` package and skips with a
  friendly warning instead of throwing a Node module-not-found stack.

Made-with: Cursor

* fix(install): handle submodules and tighten engines.node

* getGitFolderPath previously recursed past a `.git` *file*, which made
  the existing submodule-aware gitdir parsing block unreachable -- in a
  submodule we silently walked up to the super-project's `.git` dir and
  installed the hook there instead. Return the `.git` path whether it is
  a file or directory.
* Resolve `gitdir:` pointers against the directory containing the `.git`
  file (path.dirname(git)) instead of the package root, since git stores
  those paths relative to the file. Fixes submodules whose `.git` file
  is not at the package root, and linked worktrees.
* Tighten engines.node to >=16.13.0 to match `which@4`'s minimum
  (`^16.13.0 || >=18.0.0`); the previous `>=16` allowed Node 16.0-16.12
  where `which@4` will warn/fail at install time.

Addresses copilot-pull-request-reviewer feedback on PR #169.
Supersedes the submodule-install half of #75.

Made-with: Cursor

* chore: regenerate package-lock

* chore(release): bump to 2.0.0

This release contains breaking changes that warrant a major bump:

* Drops support for Node < 16.13 by introducing
  `engines.node: ">=16.13.0"` (matches `which@4`'s minimum). Previous
  releases declared no engine, so anything old enough to run them is
  now incompatible.
* Production deps make jumps that raise the floor and change result
  shapes: `cross-spawn` ^5 -> ^7 (`spawnSync` returns `status`, not
  `code`), `which` 1.2.x -> ^4. The runtime `spawn-sync` dep is dropped
  in favor of `cross-spawn.spawnSync`.
* The generated `.git/hooks/pre-commit` wrapper format is rewritten
  (single `exec bash <abs-hook> "\$@"` instead of the previous inline
  bash). Anyone parsing or scripting against the previous wrapper
  shape will see a different file.
* Hook file mode tightened from 0777 to 0755 (CIS 6.1.10).
* Submodule install location now resolves correctly to
  `<super>/.git/modules/<sub>/hooks` (previously the unreachable
  gitdir-parsing block silently caused submodule hooks to land in the
  super-project).

Minor / patch additions in the same release (carried by the same PR):

* Magit fix: generated wrapper and hook script `unset
  GIT_LITERAL_PATHSPECS` so hooks behave the same way under emacs.
* Resilient missing-package handling: a removed `pre-commit` (e.g.
  branch-switch without `node_modules`) skips with a friendly warning
  instead of breaking the commit.
* Internal: harden `index.js` spawn result checks; guard install-time
  gitdir parsing against null matches; spelling fix
  ("backuped" -> "backed up"); drop `istanbul` for `nyc`; modernize
  `mocha` and `assume` test deps.

Made-with: Cursor

Author: 3rd-Eden

.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:');
 }