diff options
author | Jesse Duffield <jessedduffield@gmail.com> | 2023-07-30 19:33:20 +1000 |
---|---|---|
committer | Jesse Duffield <jessedduffield@gmail.com> | 2023-07-30 19:59:51 +1000 |
commit | 975d2bedb67c0c87203cfc7bdf2cd67d6d40d0f3 (patch) | |
tree | 8e208fc96d560ab50a8c450a337eaa29b7db0246 /pkg | |
parent | 5c7839429909c411504351da8c1b204b3f5fa2f2 (diff) |
Remove secureexec package
From the go 1.19 release notes:
Command and LookPath no longer allow results from a PATH search to be found relative to the current directory. This removes a common source of security problems but may also break existing programs that depend on using, say, exec.Command("prog") to run a binary named prog (or, on Windows, prog.exe) in the current directory. See the os/exec package documentation for information about how best to update such programs.
Diffstat (limited to 'pkg')
-rw-r--r-- | pkg/app/daemon/daemon.go | 4 | ||||
-rw-r--r-- | pkg/app/entry_point.go | 4 | ||||
-rw-r--r-- | pkg/commands/git_config/get_key.go | 6 | ||||
-rw-r--r-- | pkg/commands/oscommands/cmd_obj.go | 8 | ||||
-rw-r--r-- | pkg/commands/oscommands/cmd_obj_builder.go | 5 | ||||
-rw-r--r-- | pkg/commands/oscommands/cmd_obj_test.go | 3 | ||||
-rw-r--r-- | pkg/commands/oscommands/fake_cmd_obj_runner.go | 37 | ||||
-rw-r--r-- | pkg/commands/oscommands/os_windows_test.go | 13 | ||||
-rw-r--r-- | pkg/integration/clients/tui.go | 6 | ||||
-rw-r--r-- | pkg/integration/components/shell.go | 7 | ||||
-rw-r--r-- | pkg/logs/tail/logs_default.go | 4 | ||||
-rw-r--r-- | pkg/secureexec/secureexec_default.go | 12 | ||||
-rw-r--r-- | pkg/secureexec/secureexec_windows.go | 45 | ||||
-rw-r--r-- | pkg/tasks/tasks_test.go | 7 |
14 files changed, 30 insertions, 131 deletions
diff --git a/pkg/app/daemon/daemon.go b/pkg/app/daemon/daemon.go index cf10e9a18..95fa6bc9e 100644 --- a/pkg/app/daemon/daemon.go +++ b/pkg/app/daemon/daemon.go @@ -5,12 +5,12 @@ import ( "fmt" "log" "os" + "os/exec" "strconv" "github.com/fsmiamoto/git-todo-parser/todo" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/common" - "github.com/jesseduffield/lazygit/pkg/secureexec" "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" ) @@ -92,7 +92,7 @@ func getDaemonKind() DaemonKind { } func getCommentChar() byte { - cmd := secureexec.Command("git", "config", "--get", "--null", "core.commentChar") + cmd := exec.Command("git", "config", "--get", "--null", "core.commentChar") if output, err := cmd.Output(); err == nil && len(output) == 2 { return output[0] } diff --git a/pkg/app/entry_point.go b/pkg/app/entry_point.go index 3f8de0ce3..fc2a4c7c4 100644 --- a/pkg/app/entry_point.go +++ b/pkg/app/entry_point.go @@ -5,6 +5,7 @@ import ( "fmt" "log" "os" + "os/exec" "path/filepath" "runtime" "runtime/debug" @@ -17,7 +18,6 @@ import ( "github.com/jesseduffield/lazygit/pkg/env" integrationTypes "github.com/jesseduffield/lazygit/pkg/integration/types" "github.com/jesseduffield/lazygit/pkg/logs/tail" - "github.com/jesseduffield/lazygit/pkg/secureexec" "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" "gopkg.in/yaml.v3" @@ -280,7 +280,7 @@ func mergeBuildInfo(buildInfo *BuildInfo) { } func getGitVersionInfo() string { - cmd := secureexec.Command("git", "--version") + cmd := exec.Command("git", "--version") stdout, _ := cmd.Output() gitVersion := strings.Trim(strings.TrimPrefix(string(stdout), "git version "), " \r\n") return gitVersion diff --git a/pkg/commands/git_config/get_key.go b/pkg/commands/git_config/get_key.go index c3156a2db..cd5a678ef 100644 --- a/pkg/commands/git_config/get_key.go +++ b/pkg/commands/git_config/get_key.go @@ -7,8 +7,6 @@ import ( "os/exec" "strings" "syscall" - - "github.com/jesseduffield/lazygit/pkg/secureexec" ) // including license from https://github.com/tcnksm/go-gitconfig because this file is an adaptation of that repo's code @@ -55,10 +53,10 @@ func runGitConfigCmd(cmd *exec.Cmd) (string, error) { func getGitConfigCmd(key string) *exec.Cmd { gitArgs := []string{"config", "--get", "--null", key} - return secureexec.Command("git", gitArgs...) + return exec.Command("git", gitArgs...) } func getGitConfigGeneralCmd(args string) *exec.Cmd { gitArgs := append([]string{"config"}, strings.Split(args, " ")...) - return secureexec.Command("git", gitArgs...) + return exec.Command("git", gitArgs...) } diff --git a/pkg/commands/oscommands/cmd_obj.go b/pkg/commands/oscommands/cmd_obj.go index d1cc23c67..06b07cd84 100644 --- a/pkg/commands/oscommands/cmd_obj.go +++ b/pkg/commands/oscommands/cmd_obj.go @@ -73,10 +73,6 @@ type ICmdObj interface { } type CmdObj struct { - // the secureexec package will swap out the first arg with the full path to the binary, - // so we store these args separately so that ToString() will output the original - args []string - cmd *exec.Cmd runner ICmdObjRunner @@ -121,7 +117,7 @@ func (self *CmdObj) GetCmd() *exec.Cmd { func (self *CmdObj) ToString() string { // if a given arg contains a space, we need to wrap it in quotes - quotedArgs := lo.Map(self.args, func(arg string, _ int) string { + quotedArgs := lo.Map(self.cmd.Args, func(arg string, _ int) string { if strings.Contains(arg, " ") { return `"` + arg + `"` } @@ -132,7 +128,7 @@ func (self *CmdObj) ToString() string { } func (self *CmdObj) Args() []string { - return self.args + return self.cmd.Args } func (self *CmdObj) AddEnvVars(vars ...string) ICmdObj { diff --git a/pkg/commands/oscommands/cmd_obj_builder.go b/pkg/commands/oscommands/cmd_obj_builder.go index 40aadaf1d..77fb7e7c9 100644 --- a/pkg/commands/oscommands/cmd_obj_builder.go +++ b/pkg/commands/oscommands/cmd_obj_builder.go @@ -3,9 +3,9 @@ package oscommands import ( "fmt" "os" + "os/exec" "strings" - "github.com/jesseduffield/lazygit/pkg/secureexec" "github.com/mgutz/str" ) @@ -27,11 +27,10 @@ type CmdObjBuilder struct { var _ ICmdObjBuilder = &CmdObjBuilder{} func (self *CmdObjBuilder) New(args []string) ICmdObj { - cmd := secureexec.Command(args[0], args[1:]...) + cmd := exec.Command(args[0], args[1:]...) cmd.Env = os.Environ() return &CmdObj{ - args: args, cmd: cmd, runner: self.runner, } diff --git a/pkg/commands/oscommands/cmd_obj_test.go b/pkg/commands/oscommands/cmd_obj_test.go index c9cb92eb5..b135f1b74 100644 --- a/pkg/commands/oscommands/cmd_obj_test.go +++ b/pkg/commands/oscommands/cmd_obj_test.go @@ -27,7 +27,8 @@ func TestCmdObjToString(t *testing.T) { } for _, scenario := range scenarios { - cmdObj := &CmdObj{args: scenario.cmdArgs} + cmd := exec.Command(scenario.cmdArgs[0], scenario.cmdArgs[1:]...) + cmdObj := &CmdObj{cmd: cmd} actual := cmdObj.ToString() if actual != scenario.expected { t.Errorf("Expected %s, got %s", quote(scenario.expected), quote(actual)) diff --git a/pkg/commands/oscommands/fake_cmd_obj_runner.go b/pkg/commands/oscommands/fake_cmd_obj_runner.go index dc75228a0..3e44bef3d 100644 --- a/pkg/commands/oscommands/fake_cmd_obj_runner.go +++ b/pkg/commands/oscommands/fake_cmd_obj_runner.go @@ -3,8 +3,6 @@ package oscommands import ( "bufio" "fmt" - "regexp" - "runtime" "strings" "sync" "testing" @@ -124,27 +122,7 @@ func (self *FakeCmdObjRunner) ExpectFunc(description string, fn func(cmdObj ICmd func (self *FakeCmdObjRunner) ExpectArgs(expectedArgs []string, output string, err error) *FakeCmdObjRunner { description := fmt.Sprintf("matches args %s", strings.Join(expectedArgs, " ")) self.ExpectFunc(description, func(cmdObj ICmdObj) bool { - args := cmdObj.GetCmd().Args - - if runtime.GOOS == "windows" { - // thanks to the secureexec package, the first arg is something like - // '"C:\\Program Files\\Git\\mingw64\\bin\\<command>.exe" - // on windows so we'll just ensure it contains our program - if !strings.Contains(args[0], expectedArgs[0]) { - return false - } - } else { - // first arg is the program name - if expectedArgs[0] != args[0] { - return false - } - } - - if !slices.Equal(expectedArgs[1:], args[1:]) { - return false - } - - return true + return slices.Equal(expectedArgs, cmdObj.GetCmd().Args) }, output, err) return self @@ -153,18 +131,7 @@ func (self *FakeCmdObjRunner) ExpectArgs(expectedArgs []string, output string, e func (self *FakeCmdObjRunner) ExpectGitArgs(expectedArgs []string, output string, err error) *FakeCmdObjRunner { description := fmt.Sprintf("matches git args %s", strings.Join(expectedArgs, " ")) self.ExpectFunc(description, func(cmdObj ICmdObj) bool { - // first arg is 'git' on unix and something like '"C:\\Program Files\\Git\\mingw64\\bin\\git.exe" on windows so we'll just ensure it ends in either 'git' or 'git.exe' - re := regexp.MustCompile(`git(\.exe)?$`) - args := cmdObj.GetCmd().Args - if !re.MatchString(args[0]) { - return false - } - - if !slices.Equal(expectedArgs, args[1:]) { - return false - } - - return true + return slices.Equal(expectedArgs, cmdObj.GetCmd().Args[1:]) }, output, err) return self diff --git a/pkg/commands/oscommands/os_windows_test.go b/pkg/commands/oscommands/os_windows_test.go index acb971b59..cf9c1e68a 100644 --- a/pkg/commands/oscommands/os_windows_test.go +++ b/pkg/commands/oscommands/os_windows_test.go @@ -6,7 +6,6 @@ package oscommands import ( "testing" - "github.com/cli/safeexec" "github.com/go-errors/errors" "github.com/stretchr/testify/assert" ) @@ -20,13 +19,11 @@ func TestOSCommandOpenFileWindows(t *testing.T) { test func(error) } - fullCmdPath, _ := safeexec.LookPath("cmd") - scenarios := []scenario{ { filename: "test", runner: NewFakeRunner(t). - ExpectArgs([]string{fullCmdPath, "/c", "start", "", "test"}, "", errors.New("error")), + ExpectArgs([]string{"cmd", "/c", "start", "", "test"}, "", errors.New("error")), test: func(err error) { assert.Error(t, err) }, @@ -34,7 +31,7 @@ func TestOSCommandOpenFileWindows(t *testing.T) { { filename: "test", runner: NewFakeRunner(t). - ExpectArgs([]string{fullCmdPath, "/c", "start", "", "test"}, "", nil), + ExpectArgs([]string{"cmd", "/c", "start", "", "test"}, "", nil), test: func(err error) { assert.NoError(t, err) }, @@ -42,7 +39,7 @@ func TestOSCommandOpenFileWindows(t *testing.T) { { filename: "filename with spaces", runner: NewFakeRunner(t). - ExpectArgs([]string{fullCmdPath, "/c", "start", "", "filename with spaces"}, "", nil), + ExpectArgs([]string{"cmd", "/c", "start", "", "filename with spaces"}, "", nil), test: func(err error) { assert.NoError(t, err) }, @@ -50,7 +47,7 @@ func TestOSCommandOpenFileWindows(t *testing.T) { { filename: "let's_test_with_single_quote", runner: NewFakeRunner(t). - ExpectArgs([]string{fullCmdPath, "/c", "start", "", "let's_test_with_single_quote"}, "", nil), + ExpectArgs([]string{"cmd", "/c", "start", "", "let's_test_with_single_quote"}, "", nil), test: func(err error) { assert.NoError(t, err) }, @@ -58,7 +55,7 @@ func TestOSCommandOpenFileWindows(t *testing.T) { { filename: "$USER.txt", runner: NewFakeRunner(t). - ExpectArgs([]string{fullCmdPath, "/c", "start", "", "$USER.txt"}, "", nil), + ExpectArgs([]string{"cmd", "/c", "start", "", "$USER.txt"}, "", nil), test: func(err error) { assert.NoError(t, err) }, diff --git a/pkg/integration/clients/tui.go b/pkg/integration/clients/tui.go index 78180cae6..47f1a2dc2 100644 --- a/pkg/integration/clients/tui.go +++ b/pkg/integration/clients/tui.go @@ -4,6 +4,7 @@ import ( "fmt" "log" "os" + "os/exec" "path/filepath" "strings" @@ -13,7 +14,6 @@ import ( "github.com/jesseduffield/lazygit/pkg/gui/style" "github.com/jesseduffield/lazygit/pkg/integration/components" "github.com/jesseduffield/lazygit/pkg/integration/tests" - "github.com/jesseduffield/lazygit/pkg/secureexec" "github.com/samber/lo" ) @@ -124,7 +124,7 @@ func RunTUI() { return nil } - cmd := secureexec.Command("sh", "-c", fmt.Sprintf("code -r pkg/integration/tests/%s.go", currentTest.Name())) + cmd := exec.Command("sh", "-c", fmt.Sprintf("code -r pkg/integration/tests/%s.go", currentTest.Name())) if err := cmd.Run(); err != nil { return err } @@ -140,7 +140,7 @@ func RunTUI() { return nil } - cmd := secureexec.Command("sh", "-c", fmt.Sprintf("code test/results/%s", currentTest.Name())) + cmd := exec.Command("sh", "-c", fmt.Sprintf("code test/results/%s", currentTest.Name())) if err := cmd.Run(); err != nil { return err } diff --git a/pkg/integration/components/shell.go b/pkg/integration/components/shell.go index ce98e575a..51fa2310b 100644 --- a/pkg/integration/components/shell.go +++ b/pkg/integration/components/shell.go @@ -4,10 +4,9 @@ import ( "fmt" "io" "os" + "os/exec" "path/filepath" "runtime" - - "github.com/jesseduffield/lazygit/pkg/secureexec" ) // this is for running shell commands, mostly for the sake of setting up the repo @@ -44,7 +43,7 @@ func (self *Shell) RunCommandExpectError(args []string) *Shell { } func (self *Shell) runCommandWithOutput(args []string) (string, error) { - cmd := secureexec.Command(args[0], args[1:]...) + cmd := exec.Command(args[0], args[1:]...) cmd.Env = os.Environ() cmd.Dir = self.dir @@ -61,7 +60,7 @@ func (self *Shell) RunShellCommand(cmdStr string) *Shell { shellArg = "/C" } - cmd := secureexec.Command(shell, shellArg, cmdStr) + cmd := exec.Command(shell, shellArg, cmdStr) cmd.Env = os.Environ() cmd.Dir = self.dir diff --git a/pkg/logs/tail/logs_default.go b/pkg/logs/tail/logs_default.go index 87965a1e7..1b866500f 100644 --- a/pkg/logs/tail/logs_default.go +++ b/pkg/logs/tail/logs_default.go @@ -6,13 +6,13 @@ package tail import ( "log" "os" + "os/exec" "github.com/aybabtme/humanlog" - "github.com/jesseduffield/lazygit/pkg/secureexec" ) func tailLogsForPlatform(logFilePath string, opts *humanlog.HandlerOptions) { - cmd := secureexec.Command("tail", "-f", logFilePath) + cmd := exec.Command("tail", "-f", logFilePath) stdout, _ := cmd.StdoutPipe() if err := cmd.Start(); err != nil { diff --git a/pkg/secureexec/secureexec_default.go b/pkg/secureexec/secureexec_default.go deleted file mode 100644 index f6b843d45..000000000 --- a/pkg/secureexec/secureexec_default.go +++ /dev/null @@ -1,12 +0,0 @@ -//go:build !windows -// +build !windows - -package secureexec - -import ( - "os/exec" -) - -func Command(name string, args ...string) *exec.Cmd { - return exec.Command(name, args...) -} diff --git a/pkg/secureexec/secureexec_windows.go b/pkg/secureexec/secureexec_windows.go deleted file mode 100644 index ed4339cd3..000000000 --- a/pkg/secureexec/secureexec_windows.go +++ /dev/null @@ -1,45 +0,0 @@ -//go:build windows -// +build windows - -package secureexec - -import ( - "os/exec" - - "github.com/cli/safeexec" -) - -// calling exec.Command directly on a windows machine poses a security risk due to -// the current directory being searched first before any directories in the PATH -// variable, meaning you might clone a repo that contains a program called 'git' -// which does something malicious when executed. - -// see https://github.com/golang/go/issues/38736 for more context. We'll likely -// be able to just throw out this code and switch to the official solution when it exists. - -// I consider this a minor security concern because you're just as vulnerable if -// you call `git status` from the command line directly but no harm in playing it -// safe. - -var pathCache = map[string]string{} - -func Command(name string, args ...string) *exec.Cmd { - path := getPath(name) - - return exec.Command(path, args...) -} - -func getPath(name string) string { - if path, ok := pathCache[name]; ok { - return path - } - - path, err := safeexec.LookPath(name) - if err != nil { - pathCache[name] = name - return name - } - - pathCache[name] = path - return path -} diff --git a/pkg/tasks/tasks_test.go b/pkg/tasks/tasks_test.go index 6f2edbbf4..89056807f 100644 --- a/pkg/tasks/tasks_test.go +++ b/pkg/tasks/tasks_test.go @@ -11,7 +11,6 @@ import ( "time" "github.com/jesseduffield/gocui" - "github.com/jesseduffield/lazygit/pkg/secureexec" "github.com/jesseduffield/lazygit/pkg/utils" ) @@ -46,7 +45,7 @@ func TestNewCmdTaskInstantStop(t *testing.T) { reader := bytes.NewBufferString("test") start := func() (*exec.Cmd, io.Reader) { // not actually starting this because it's not necessary - cmd := secureexec.Command("blah blah") + cmd := exec.Command("blah") close(stop) @@ -111,7 +110,7 @@ func TestNewCmdTask(t *testing.T) { reader := bytes.NewBufferString("test") start := func() (*exec.Cmd, io.Reader) { // not actually starting this because it's not necessary - cmd := secureexec.Command("blah blah") + cmd := exec.Command("blah") return cmd, reader } @@ -246,7 +245,7 @@ func TestNewCmdTaskRefresh(t *testing.T) { reader := BlankLineReader{totalLinesToYield: s.totalTaskLines} start := func() (*exec.Cmd, io.Reader) { // not actually starting this because it's not necessary - cmd := secureexec.Command("blah blah") + cmd := exec.Command("blah") return cmd, &reader } |