From deef63c1cf369d4fa1a8946ab455aa1b2988a408 Mon Sep 17 00:00:00 2001
From: Michael Haggerty <mhagger@github.com>
Date: Wed, 21 Apr 2021 09:36:23 +0200
Subject: [PATCH] Use a safer method to locate the `git` executable

`exec.LookPath()` from the Go standard library, which is used by
`exec.Cmd`, implicitly searches for executables in the current
directory before searching `PATH`. This could be awkward if the Git
repository being analyzed contains a file like `git.exe` that could be
run instead of the standard system `git` binary. So introduce a way to
look for the "correct" `git` binary, record it in the `Repository`
instance, and use that binary whenever we need to run `git`.

Don't bother to make the same change in the test code, since tests are
not run inside of a potentially hostile repository.
---
 git/git.go     | 31 +++++++++++++++++++++++--------
 git/git_bin.go | 27 +++++++++++++++++++++++++++
 go.mod         |  1 +
 go.sum         |  2 ++
 4 files changed, 53 insertions(+), 8 deletions(-)
 create mode 100644 git/git_bin.go

diff --git a/git/git.go b/git/git.go
index 7883cf9..34c7c53 100644
--- a/git/git.go
+++ b/git/git.go
@@ -62,6 +62,10 @@ func (oid OID) MarshalJSON() ([]byte, error) {
 
 type Repository struct {
 	path string
+
+	// gitBin is the path of the `git` executable that should be used
+	// when running commands in this repository.
+	gitBin string
 }
 
 // smartJoin returns the path that can be described as `relPath`
@@ -74,20 +78,28 @@ func smartJoin(path, relPath string) string {
 	return filepath.Join(path, relPath)
 }
 
+// NewRepository creates a new repository object that can be used for
+// running `git` commands within that repository.
 func NewRepository(path string) (*Repository, error) {
-	cmd := exec.Command("git", "-C", path, "rev-parse", "--git-dir")
+	// Find the `git` executable to be used:
+	gitBin, err := findGitBin()
+	if err != nil {
+		return nil, fmt.Errorf(
+			"could not find 'git' executable (is it in your PATH?): %v", err,
+		)
+	}
+
+	cmd := exec.Command(gitBin, "-C", path, "rev-parse", "--git-dir")
 	out, err := cmd.Output()
 	if err != nil {
 		switch err := err.(type) {
 		case *exec.Error:
 			return nil, fmt.Errorf(
-				"could not run git (is it in your PATH?): %s",
-				err.Err,
+				"could not run '%s': %v", gitBin, err.Err,
 			)
 		case *exec.ExitError:
 			return nil, fmt.Errorf(
-				"git rev-parse failed: %s",
-				err.Stderr,
+				"git rev-parse failed: %s", err.Stderr,
 			)
 		default:
 			return nil, err
@@ -95,7 +107,7 @@ func NewRepository(path string) (*Repository, error) {
 	}
 	gitDir := smartJoin(path, string(bytes.TrimSpace(out)))
 
-	cmd = exec.Command("git", "rev-parse", "--git-path", "shallow")
+	cmd = exec.Command(gitBin, "rev-parse", "--git-path", "shallow")
 	cmd.Dir = gitDir
 	out, err = cmd.Output()
 	if err != nil {
@@ -109,7 +121,10 @@ func NewRepository(path string) (*Repository, error) {
 		return nil, errors.New("this appears to be a shallow clone; full clone required")
 	}
 
-	return &Repository{path: gitDir}, nil
+	return &Repository{
+		path:   gitDir,
+		gitBin: gitBin,
+	}, nil
 }
 
 func (repo *Repository) gitCommand(callerArgs ...string) *exec.Cmd {
@@ -125,7 +140,7 @@ func (repo *Repository) gitCommand(callerArgs ...string) *exec.Cmd {
 
 	args = append(args, callerArgs...)
 
-	cmd := exec.Command("git", args...)
+	cmd := exec.Command(repo.gitBin, args...)
 
 	cmd.Env = append(
 		os.Environ(),
diff --git a/git/git_bin.go b/git/git_bin.go
new file mode 100644
index 0000000..fc03435
--- /dev/null
+++ b/git/git_bin.go
@@ -0,0 +1,27 @@
+package git
+
+import (
+	"path/filepath"
+
+	"github.com/cli/safeexec"
+)
+
+// findGitBin finds the `git` binary in PATH that should be used by
+// the rest of `git-sizer`. It uses `safeexec` to find the executable,
+// because on Windows, `exec.Cmd` looks not only in PATH, but also in
+// the current directory. This is a potential risk if the repository
+// being scanned is hostile and non-bare because it might possibly
+// contain an executable file named `git`.
+func findGitBin() (string, error) {
+	gitBin, err := safeexec.LookPath("git")
+	if err != nil {
+		return "", err
+	}
+
+	gitBin, err = filepath.Abs(gitBin)
+	if err != nil {
+		return "", err
+	}
+
+	return gitBin, nil
+}
diff --git a/go.mod b/go.mod
index 20bf4b8..293770e 100644
--- a/go.mod
+++ b/go.mod
@@ -3,6 +3,7 @@ module github.com/github/git-sizer
 go 1.13
 
 require (
+	github.com/cli/safeexec v1.0.0
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.4.0
diff --git a/go.sum b/go.sum
index dff9970..590e4f5 100644
--- a/go.sum
+++ b/go.sum
@@ -1,3 +1,5 @@
+github.com/cli/safeexec v1.0.0 h1:0VngyaIyqACHdcMNWfo6+KdUYnqEr2Sg+bSP1pdF+dI=
+github.com/cli/safeexec v1.0.0/go.mod h1:Z/D4tTN8Vs5gXYHDCbaM1S/anmEDnJb1iW0+EJ5zx3Q=
 github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=