summaryrefslogtreecommitdiffstats
path: root/pkg
diff options
context:
space:
mode:
authorJesse Duffield <jessedduffield@gmail.com>2023-07-30 19:33:20 +1000
committerJesse Duffield <jessedduffield@gmail.com>2023-07-30 19:59:51 +1000
commit975d2bedb67c0c87203cfc7bdf2cd67d6d40d0f3 (patch)
tree8e208fc96d560ab50a8c450a337eaa29b7db0246 /pkg
parent5c7839429909c411504351da8c1b204b3f5fa2f2 (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.go4
-rw-r--r--pkg/app/entry_point.go4
-rw-r--r--pkg/commands/git_config/get_key.go6
-rw-r--r--pkg/commands/oscommands/cmd_obj.go8
-rw-r--r--pkg/commands/oscommands/cmd_obj_builder.go5
-rw-r--r--pkg/commands/oscommands/cmd_obj_test.go3
-rw-r--r--pkg/commands/oscommands/fake_cmd_obj_runner.go37
-rw-r--r--pkg/commands/oscommands/os_windows_test.go13
-rw-r--r--pkg/integration/clients/tui.go6
-rw-r--r--pkg/integration/components/shell.go7
-rw-r--r--pkg/logs/tail/logs_default.go4
-rw-r--r--pkg/secureexec/secureexec_default.go12
-rw-r--r--pkg/secureexec/secureexec_windows.go45
-rw-r--r--pkg/tasks/tasks_test.go7
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
}