fix: resolve git executable path to avoid PATH substitution (Fixes #254)

Merged
40
3mo ago
+31-33

Summary

Fixes code scanning alerts S4036 (#7–#11) where git was invoked by bare name, letting PATH decide which binary runs. An attacker who controls PATH could substitute a malicious git binary.

All git invocations now use a resolved, verified absolute executable path from the existing getGitExecutable() in src/git/diff.ts (the same logic that already fixed the equivalent alerts in diff.ts).

Changes

  • src/git/diff.ts — export getGitExecutable() (previously module-private).
  • src/git/hook.ts
    • resolveGitPath() → execFileSync(getGitExecutable(), ['rev-parse', '--git-path', ...])
    • runPostCommitHook() default readLatestCommitMessage → execFileSync(getGitExecutable(), ['log', '-1', '--pretty=%B'], ...) (was execSync('git log ...'))
    • dropped the now-unused execSync import
  • src/commands/batch.ts
    • gitHasChanges() — staged + unstaged --quiet checks → execFileSync(getGitExecutable(), ['diff', ...], { cwd })
    • getGitDiff() — replaced execSync('git diff ...') shell string with execFileSync and the resolved executable (same unsafe pattern, not independently flagged)
    • gitCommit() → spawnSync(getGitExecutable(), ['commit', '-F', tmpFile], { cwd })

Behavior is preserved: execFileSync/spawnSync throw with the same .status/.stderr shape, so existing error handling and the cwd support for arbitrary repos in batch mode are unchanged.

Validation

  • npm run build ✅
  • npm test — 356 pass, 0 fail, 2 skipped ✅
  • npm run format + npm run format:check ✅

Closes #254

Summary by CodeRabbit

  • Bug Fixes
    • Improved Git command handling for greater reliability across different environments.
    • Enhanced command execution safety by avoiding shell-based Git invocation.
    • Preserved existing staging, diff, commit, repository-path, and latest-commit behavior.