Skip to content

Commit deef63c

Browse files
committed
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.
1 parent 951a51b commit deef63c

File tree

4 files changed

+53
-8
lines changed

4 files changed

+53
-8
lines changed

git/git.go

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,10 @@ func (oid OID) MarshalJSON() ([]byte, error) {
6262

6363
type Repository struct {
6464
path string
65+
66+
// gitBin is the path of the `git` executable that should be used
67+
// when running commands in this repository.
68+
gitBin string
6569
}
6670

6771
// smartJoin returns the path that can be described as `relPath`
@@ -74,28 +78,36 @@ func smartJoin(path, relPath string) string {
7478
return filepath.Join(path, relPath)
7579
}
7680

81+
// NewRepository creates a new repository object that can be used for
82+
// running `git` commands within that repository.
7783
func NewRepository(path string) (*Repository, error) {
78-
cmd := exec.Command("git", "-C", path, "rev-parse", "--git-dir")
84+
// Find the `git` executable to be used:
85+
gitBin, err := findGitBin()
86+
if err != nil {
87+
return nil, fmt.Errorf(
88+
"could not find 'git' executable (is it in your PATH?): %v", err,
89+
)
90+
}
91+
92+
cmd := exec.Command(gitBin, "-C", path, "rev-parse", "--git-dir")
7993
out, err := cmd.Output()
8094
if err != nil {
8195
switch err := err.(type) {
8296
case *exec.Error:
8397
return nil, fmt.Errorf(
84-
"could not run git (is it in your PATH?): %s",
85-
err.Err,
98+
"could not run '%s': %v", gitBin, err.Err,
8699
)
87100
case *exec.ExitError:
88101
return nil, fmt.Errorf(
89-
"git rev-parse failed: %s",
90-
err.Stderr,
102+
"git rev-parse failed: %s", err.Stderr,
91103
)
92104
default:
93105
return nil, err
94106
}
95107
}
96108
gitDir := smartJoin(path, string(bytes.TrimSpace(out)))
97109

98-
cmd = exec.Command("git", "rev-parse", "--git-path", "shallow")
110+
cmd = exec.Command(gitBin, "rev-parse", "--git-path", "shallow")
99111
cmd.Dir = gitDir
100112
out, err = cmd.Output()
101113
if err != nil {
@@ -109,7 +121,10 @@ func NewRepository(path string) (*Repository, error) {
109121
return nil, errors.New("this appears to be a shallow clone; full clone required")
110122
}
111123

112-
return &Repository{path: gitDir}, nil
124+
return &Repository{
125+
path: gitDir,
126+
gitBin: gitBin,
127+
}, nil
113128
}
114129

115130
func (repo *Repository) gitCommand(callerArgs ...string) *exec.Cmd {
@@ -125,7 +140,7 @@ func (repo *Repository) gitCommand(callerArgs ...string) *exec.Cmd {
125140

126141
args = append(args, callerArgs...)
127142

128-
cmd := exec.Command("git", args...)
143+
cmd := exec.Command(repo.gitBin, args...)
129144

130145
cmd.Env = append(
131146
os.Environ(),

git/git_bin.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package git
2+
3+
import (
4+
"path/filepath"
5+
6+
"github.com/cli/safeexec"
7+
)
8+
9+
// findGitBin finds the `git` binary in PATH that should be used by
10+
// the rest of `git-sizer`. It uses `safeexec` to find the executable,
11+
// because on Windows, `exec.Cmd` looks not only in PATH, but also in
12+
// the current directory. This is a potential risk if the repository
13+
// being scanned is hostile and non-bare because it might possibly
14+
// contain an executable file named `git`.
15+
func findGitBin() (string, error) {
16+
gitBin, err := safeexec.LookPath("git")
17+
if err != nil {
18+
return "", err
19+
}
20+
21+
gitBin, err = filepath.Abs(gitBin)
22+
if err != nil {
23+
return "", err
24+
}
25+
26+
return gitBin, nil
27+
}

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ module github.com/github/git-sizer
33
go 1.13
44

55
require (
6+
github.com/cli/safeexec v1.0.0
67
github.com/davecgh/go-spew v1.1.1 // indirect
78
github.com/spf13/pflag v1.0.5
89
github.com/stretchr/testify v1.4.0

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
github.com/cli/safeexec v1.0.0 h1:0VngyaIyqACHdcMNWfo6+KdUYnqEr2Sg+bSP1pdF+dI=
2+
github.com/cli/safeexec v1.0.0/go.mod h1:Z/D4tTN8Vs5gXYHDCbaM1S/anmEDnJb1iW0+EJ5zx3Q=
13
github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
24
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
35
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=

0 commit comments

Comments
 (0)