fix: safe execute git commands without sh wrapper

internal/git/exec.go

@@ -3,18 +3,15 @@ package git
 import (
 	"os"
 	"os/exec"
-	"runtime"
 	"strings"
 
 	"github.com/evilmartians/lefthook/internal/log"
 )
 
 type Exec interface {
 	SetRootPath(root string)
-	Cmd(cmd string) (string, error)
-	CmdArgs(args ...string) (string, error)
-	CmdLines(cmd string) ([]string, error)
-	RawCmd(cmd string) (string, error)
+	Cmd(cmd []string) (string, error)
+	CmdLines(cmd []string) ([]string, error)
 }
 
 type OsExec struct {
@@ -32,46 +29,28 @@ func (o *OsExec) SetRootPath(root string) {
 }
 
 // Cmd runs plain string command. Trims spaces around output.
-func (o *OsExec) Cmd(cmd string) (string, error) {
-	args := strings.Split(cmd, " ")
-	return o.CmdArgs(args...)
-}
-
-// CmdLines runs plain string command, returns its output split by newline.
-func (o *OsExec) CmdLines(cmd string) ([]string, error) {
-	out, err := o.RawCmd(cmd)
-	if err != nil {
-		return nil, err
-	}
-
-	return strings.Split(out, "\n"), nil
-}
-
-// CmdArgs runs a command provided with separated words. Trims spaces around output.
-func (o *OsExec) CmdArgs(args ...string) (string, error) {
-	out, err := o.rawExecArgs(args...)
+func (o *OsExec) Cmd(cmd []string) (string, error) {
+	out, err := o.rawExecArgs(cmd)
 	if err != nil {
 		return "", err
 	}
 
 	return strings.TrimSpace(out), nil
 }
 
-// RawCmd runs a plain string command returning unprocessed output as string.
-func (o *OsExec) RawCmd(cmd string) (string, error) {
-	var args []string
-	if runtime.GOOS == "windows" {
-		args = strings.Split(cmd, " ")
-	} else {
-		args = []string{"sh", "-c", cmd}
+// CmdLines runs plain string command, returns its output split by newline.
+func (o *OsExec) CmdLines(cmd []string) ([]string, error) {
+	out, err := o.rawExecArgs(cmd)
+	if err != nil {
+		return nil, err
 	}
 
-	return o.rawExecArgs(args...)
+	return strings.Split(strings.TrimSpace(out), "\n"), nil
 }
 
 // rawExecArgs executes git command with LEFTHOOK=0 in order
 // to prevent calling subsequent lefthook hooks.
-func (o *OsExec) rawExecArgs(args ...string) (string, error) {
+func (o *OsExec) rawExecArgs(args []string) (string, error) {
 	log.Debug("[lefthook] cmd: ", args)
 
 	cmd := exec.Command(args[0], args[1:]...)

internal/git/remote.go

@@ -60,22 +60,22 @@ func (r *Repository) updateRemote(path, ref string) error {
 	log.Debugf("Updating remote config repository: %s", path)
 
 	if len(ref) != 0 {
-		_, err := r.Git.CmdArgs(
+		_, err := r.Git.Cmd([]string{
 			"git", "-C", path, "fetch", "--quiet", "--depth", "1",
 			"origin", ref,
-		)
+		})
 		if err != nil {
 			return err
 		}
 
-		_, err = r.Git.CmdArgs(
+		_, err = r.Git.Cmd([]string{
 			"git", "-C", path, "checkout", "FETCH_HEAD",
-		)
+		})
 		if err != nil {
 			return err
 		}
 	} else {
-		_, err := r.Git.CmdArgs("git", "-C", path, "pull", "--quiet")
+		_, err := r.Git.Cmd([]string{"git", "-C", path, "pull", "--quiet"})
 		if err != nil {
 			return err
 		}
@@ -93,7 +93,7 @@ func (r *Repository) cloneRemote(path, url, ref string) error {
 	}
 	cmdClone = append(cmdClone, url)
 
-	_, err := r.Git.CmdArgs(cmdClone...)
+	_, err := r.Git.Cmd(cmdClone)
 	if err != nil {
 		return err
 	}

internal/git/repository.go

@@ -1,7 +1,6 @@
 package git
 
 import (
-	"fmt"
 	"os"
 	"path/filepath"
 	"regexp"
@@ -11,25 +10,26 @@ import (
 )
 
 const (
-	cmdRootPath      = "git rev-parse --show-toplevel"
-	cmdHooksPath     = "git rev-parse --git-path hooks"
-	cmdInfoPath      = "git rev-parse --git-path info"
-	cmdGitPath       = "git rev-parse --git-dir"
-	cmdStagedFiles   = "git diff --name-only --cached --diff-filter=ACMR"
-	cmdAllFiles      = "git ls-files --cached"
-	cmdPushFilesBase = "git diff --name-only HEAD @{push}"
-	cmdPushFilesHead = "git diff --name-only HEAD %s"
-	cmdStatusShort   = "git status --short"
-	cmdCreateStash   = "git stash create"
-	cmdListStash     = "git stash list"
-
 	stashMessage      = "lefthook auto backup"
 	unstagedPatchName = "lefthook-unstaged.patch"
 	infoDirMode       = 0o775
 	minStatusLen      = 3
 )
 
-var headBranchRegexp = regexp.MustCompile(`HEAD -> (?P<name>.*)$`)
+var (
+	headBranchRegexp = regexp.MustCompile(`HEAD -> (?P<name>.*)$`)
+	cmdPushFilesBase = []string{"git", "diff", "--name-only", "HEAD", "@{push}"}
+	cmdPushFilesHead = []string{"git", "diff", "--name-only", "HEAD"}
+	cmdStagedFiles   = []string{"git", "diff", "--name-only", "--cached", "--diff-filter=ACMR"}
+	cmdStatusShort   = []string{"git", "status", "--short"}
+	cmdListStash     = []string{"git", "stash", "list"}
+	cmdRootPath      = []string{"git", "rev-parse", "--show-toplevel"}
+	cmdHooksPath     = []string{"git", "rev-parse", "--git-path", "hooks"}
+	cmdInfoPath      = []string{"git", "rev-parse", "--git-path", "info"}
+	cmdGitPath       = []string{"git", "rev-parse", "--git-dir"}
+	cmdAllFiles      = []string{"git", "ls-files", "--cached"}
+	cmdCreateStash   = []string{"git", "stash", "create"}
+)
 
 // Repository represents a git repository.
 type Repository struct {
@@ -112,7 +112,7 @@ func (r *Repository) PushFiles() ([]string, error) {
 	}
 
 	if len(r.headBranch) == 0 {
-		branches, err := r.Git.CmdLines("git branch --remotes")
+		branches, err := r.Git.CmdLines([]string{"git", "branch", " --remotes"})
 		if err != nil {
 			return nil, err
 		}
@@ -126,7 +126,7 @@ func (r *Repository) PushFiles() ([]string, error) {
 			break
 		}
 	}
-	return r.FilesByCommand(fmt.Sprintf(cmdPushFilesHead, r.headBranch))
+	return r.FilesByCommand(append(cmdPushFilesHead, r.headBranch))
 }
 
 // PartiallyStagedFiles returns the list of files that have both staged and
@@ -163,7 +163,7 @@ func (r *Repository) PartiallyStagedFiles() ([]string, error) {
 }
 
 func (r *Repository) SaveUnstaged(files []string) error {
-	_, err := r.Git.CmdArgs(
+	_, err := r.Git.Cmd(
 		append([]string{
 			"git",
 			"diff",
@@ -178,20 +178,20 @@ func (r *Repository) SaveUnstaged(files []string) error {
 			"--output",
 			r.unstagedPatchPath,
 			"--",
-		}, files...)...,
+		}, files...),
 	)
 
 	return err
 }
 
 func (r *Repository) HideUnstaged(files []string) error {
-	_, err := r.Git.CmdArgs(
+	_, err := r.Git.Cmd(
 		append([]string{
 			"git",
 			"checkout",
 			"--force",
 			"--",
-		}, files...)...,
+		}, files...),
 	)
 
 	return err
@@ -202,15 +202,15 @@ func (r *Repository) RestoreUnstaged() error {
 		return nil
 	}
 
-	_, err := r.Git.CmdArgs(
+	_, err := r.Git.Cmd([]string{
 		"git",
 		"apply",
 		"-v",
 		"--whitespace=nowarn",
 		"--recount",
 		"--unidiff-zero",
 		r.unstagedPatchPath,
-	)
+	})
 
 	if err == nil {
 		err = r.Fs.Remove(r.unstagedPatchPath)
@@ -225,15 +225,15 @@ func (r *Repository) StashUnstaged() error {
 		return err
 	}
 
-	_, err = r.Git.CmdArgs(
+	_, err = r.Git.Cmd([]string{
 		"git",
 		"stash",
 		"store",
 		"--quiet",
 		"--message",
 		stashMessage,
 		stashHash,
-	)
+	})
 	if err != nil {
 		return err
 	}
@@ -258,13 +258,13 @@ func (r *Repository) DropUnstagedStash() error {
 		stashID := stashRegexp.SubexpIndex("stash")
 
 		if len(matches[stashID]) > 0 {
-			_, err := r.Git.CmdArgs(
+			_, err := r.Git.Cmd([]string{
 				"git",
 				"stash",
 				"drop",
 				"--quiet",
 				matches[stashID],
-			)
+			})
 			if err != nil {
 				return err
 			}
@@ -279,15 +279,15 @@ func (r *Repository) AddFiles(files []string) error {
 		return nil
 	}
 
-	_, err := r.Git.CmdArgs(
-		append([]string{"git", "add"}, files...)...,
+	_, err := r.Git.Cmd(
+		append([]string{"git", "add"}, files...),
 	)
 
 	return err
 }
 
 // FilesByCommand accepts git command and returns its result as a list of filepaths.
-func (r *Repository) FilesByCommand(command string) ([]string, error) {
+func (r *Repository) FilesByCommand(command []string) ([]string, error) {
 	lines, err := r.Git.CmdLines(command)
 	if err != nil {
 		return nil, err

internal/git/repository_test.go

@@ -13,20 +13,16 @@ type GitMock struct {
 
 func (g GitMock) SetRootPath(_root string) {}
 
-func (g GitMock) Cmd(cmd string) (string, error) {
-	res, err := g.RawCmd(cmd)
-	if err != nil {
-		return "", err
+func (g GitMock) Cmd(cmd []string) (string, error) {
+	res, ok := g.cases[(strings.Join(cmd, " "))]
+	if !ok {
+		return "", errors.New("doesn't exist")
 	}
 
 	return strings.TrimSpace(res), nil
 }
 
-func (g GitMock) CmdArgs(args ...string) (string, error) {
-	return g.Cmd(strings.Join(args, " "))
-}
-
-func (g GitMock) CmdLines(cmd string) ([]string, error) {
+func (g GitMock) CmdLines(cmd []string) ([]string, error) {
 	res, err := g.Cmd(cmd)
 	if err != nil {
 		return nil, err
@@ -35,15 +31,6 @@ func (g GitMock) CmdLines(cmd string) ([]string, error) {
 	return strings.Split(res, "\n"), nil
 }
 
-func (g GitMock) RawCmd(cmd string) (string, error) {
-	res, ok := g.cases[cmd]
-	if !ok {
-		return "", errors.New("doesn't exist")
-	}
-
-	return res, nil
-}
-
 func TestPartiallyStagedFiles(t *testing.T) {
 	for i, tt := range [...]struct {
 		name, gitOut string

internal/lefthook/run/prepare_command.go

@@ -88,7 +88,13 @@ func (r *Runner) buildRun(command *config.Command) (*run, error, error) {
 		config.PushFiles:      r.Repo.PushFiles,
 		config.SubAllFiles:    r.Repo.AllFiles,
 		config.SubFiles: func() ([]string, error) {
-			return r.Repo.FilesByCommand(filesCmd)
+			var cmd []string
+			if runtime.GOOS == "windows" {
+				cmd = strings.Split(filesCmd, " ")
+			} else {
+				cmd = []string{"sh", "-c", filesCmd}
+			}
+			return r.Repo.FilesByCommand(cmd)
 		},
 	}
 

internal/lefthook/run/runner_test.go

@@ -41,23 +41,16 @@ type GitMock struct {
 
 func (g *GitMock) SetRootPath(_root string) {}
 
-func (g *GitMock) Cmd(cmd string) (string, error) {
+func (g *GitMock) Cmd(cmd []string) (string, error) {
 	g.mux.Lock()
-	g.commands = append(g.commands, cmd)
-	g.mux.Unlock()
-
-	return "", nil
-}
-
-func (g *GitMock) CmdArgs(args ...string) (string, error) {
-	g.mux.Lock()
-	g.commands = append(g.commands, strings.Join(args, " "))
+	g.commands = append(g.commands, strings.Join(cmd, " "))
 	g.mux.Unlock()
 
 	return "", nil
 }
 
-func (g *GitMock) CmdLines(cmd string) ([]string, error) {
+func (g *GitMock) CmdLines(args []string) ([]string, error) {
+	cmd := strings.Join(args, " ")
 	g.mux.Lock()
 	g.commands = append(g.commands, cmd)
 	g.mux.Unlock()
@@ -74,14 +67,6 @@ func (g *GitMock) CmdLines(cmd string) ([]string, error) {
 	return nil, nil
 }
 
-func (g *GitMock) RawCmd(cmd string) (string, error) {
-	g.mux.Lock()
-	g.commands = append(g.commands, cmd)
-	g.mux.Unlock()
-
-	return "", nil
-}
-
 func (g *GitMock) reset() {
 	g.mux.Lock()
 	g.commands = []string{}