Skip to content

Commit c610f14

Browse files
committed
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
1 parent a84bdc8 commit c610f14

7 files changed

Lines changed: 2900 additions & 31 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
node_modules
22
npm-debug.log
33
coverage
4+
.nyc_output
45
.tern-port

hook

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,15 @@ elif [[ -x "$LOCAL" ]]; then
3737
BINARY="$LOCAL"
3838
fi
3939

40+
#
41+
# Run from the repository root so `require.resolve('pre-commit')` works for Yarn PnP,
42+
# and GUI git clients that invoke hooks with an unexpected cwd still resolve deps.
43+
#
44+
REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null)" || REPO_ROOT=""
45+
if [[ -n "$REPO_ROOT" ]]; then
46+
cd "$REPO_ROOT" || exit 1
47+
fi
48+
4049
#
4150
# Add --dry-run cli flag support so we can execute this hook without side effects
4251
# and see if it works in the current environment
@@ -46,5 +55,5 @@ if [[ $* == *--dry-run* ]]; then
4655
exit 1
4756
fi
4857
else
49-
"$BINARY" "$("$BINARY" -e "console.log(require.resolve('pre-commit'))")"
58+
exec "$BINARY" "$("$BINARY" -e "console.log(require.resolve('pre-commit'))")"
5059
fi

index.js

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,16 @@
11
'use strict';
22

3+
//
4+
// cross-spawn.spawnSync returns the same shape as child_process.spawnSync
5+
// (`status`, not `code`).
6+
//
7+
function failedSpawn(result) {
8+
if (!result) return true;
9+
if (result.error) return true;
10+
if (result.signal) return true;
11+
return result.status !== 0;
12+
}
13+
314
var spawn = require('cross-spawn')
415
, which = require('which')
516
, path = require('path')
@@ -173,8 +184,8 @@ Hook.prototype.initialize = function initialize() {
173184
this.root = this.exec(this.git, ['rev-parse', '--show-toplevel']);
174185
this.status = this.exec(this.git, ['status', '--porcelain']);
175186

176-
if (this.status.code) return this.log(Hook.log.status, 0);
177-
if (this.root.code) return this.log(Hook.log.root, 0);
187+
if (failedSpawn(this.status)) return this.log(Hook.log.status, 0);
188+
if (failedSpawn(this.root)) return this.log(Hook.log.root, 0);
178189

179190
this.status = this.status.stdout.toString().trim();
180191
this.root = this.root.stdout.toString().trim();
@@ -229,8 +240,11 @@ Hook.prototype.run = function runner() {
229240
env: process.env,
230241
cwd: hooked.root,
231242
stdio: [0, 1, 2]
232-
}).once('close', function closed(code) {
233-
if (code) return hooked.log(hooked.format(Hook.log.failure, script, code));
243+
}).once('close', function closed(code, signal) {
244+
var exitCode = typeof code === 'number' ? code : (signal ? 1 : 0);
245+
if (exitCode !== 0) {
246+
return hooked.log(hooked.format(Hook.log.failure, script, exitCode));
247+
}
234248

235249
again(scripts);
236250
});

install.js

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,17 @@ var fs = require('fs')
77
, path = require('path')
88
, os = require('os')
99
, hook = path.join(__dirname, 'hook')
10+
, hookAbs = path.resolve(hook)
1011
, root = path.resolve(__dirname, '..', '..')
1112
, exists = fs.existsSync || path.existsSync;
1213

14+
//
15+
// POSIX single-quoted string for embedding paths in generated shell scripts.
16+
//
17+
function shellSingleQuote(str) {
18+
return '\'' + str.replace(/'/g, '\'\\\'\'') + '\'';
19+
}
20+
1321
//
1422
// Gather the location of the possible hidden .git directory, the hooks
1523
// directory which contains all git hooks and the absolute location of the
@@ -45,10 +53,10 @@ function getGitFolderPath(currentPath) {
4553
//
4654
// Resolve git directory for submodules
4755
//
48-
if (exists(git) && fs.lstatSync(git).isFile()) {
56+
if (git && exists(git) && fs.lstatSync(git).isFile()) {
4957
var gitinfo = fs.readFileSync(git).toString()
5058
, gitdirmatch = /gitdir: (.+)/.exec(gitinfo)
51-
, gitdir = gitdirmatch.length == 2 ? gitdirmatch[1] : null;
59+
, gitdir = gitdirmatch && gitdirmatch.length === 2 ? gitdirmatch[1].trim() : null;
5260

5361
if (gitdir !== null) {
5462
git = path.resolve(root, gitdir);
@@ -92,20 +100,17 @@ if (exists(precommit) && !fs.lstatSync(precommit).isSymbolicLink()) {
92100
try { fs.unlinkSync(precommit); }
93101
catch (e) {}
94102

95-
// Create generic precommit hook that launches this modules hook (as well
96-
// as stashing - unstashing the unstaged changes)
97-
// TODO: we could keep launching the old pre-commit scripts
98-
var hookRelativeUnixPath = hook.replace(root, '.');
99-
100-
if(os.platform() === 'win32') {
101-
hookRelativeUnixPath = hookRelativeUnixPath.replace(/[\\\/]+/g, '/');
103+
// Delegate to this package's `hook` script using an absolute path so Yarn Plug'n'Play
104+
// and other layouts without `node_modules/pre-commit` still work. The hook script
105+
// changes to the git root before resolving `pre-commit` via Node.
106+
//
107+
var hookLauncher = hookAbs;
108+
if (os.platform() === 'win32') {
109+
hookLauncher = hookLauncher.replace(/\\/g, '/');
102110
}
103111

104112
var precommitContent = '#!/usr/bin/env bash' + os.EOL
105-
+ hookRelativeUnixPath + os.EOL
106-
+ 'RESULT=$?' + os.EOL
107-
+ '[ $RESULT -ne 0 ] && exit 1' + os.EOL
108-
+ 'exit 0' + os.EOL;
113+
+ 'exec bash ' + shellSingleQuote(hookLauncher) + ' "$@"' + os.EOL;
109114

110115
//
111116
// It could be that we do not have rights to this folder which could cause the
@@ -121,10 +126,10 @@ catch (e) {
121126
console.error('pre-commit:');
122127
}
123128

124-
try { fs.chmodSync(precommit, '777'); }
129+
try { fs.chmodSync(precommit, 0o755); }
125130
catch (e) {
126131
console.error('pre-commit:');
127-
console.error('pre-commit: chmod 0777 the pre-commit file in your .git/hooks folder because:');
132+
console.error('pre-commit: chmod 0755 the pre-commit file in your .git/hooks folder because:');
128133
console.error('pre-commit: '+ e.message);
129134
console.error('pre-commit:');
130135
}

0 commit comments

Comments
 (0)