diff --git a/go.mod b/go.mod index 23babf27e..270763e7c 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( github.com/OpenPeeDeeP/xdg v1.0.0 github.com/atotto/clipboard v0.1.2 github.com/aybabtme/humanlog v0.4.1 + github.com/cli/safeexec v1.0.0 github.com/cloudfoundry/jibber_jabber v0.0.0-20151120183258-bcc4c8345a21 github.com/creack/pty v1.1.11 github.com/fatih/color v1.9.0 diff --git a/go.sum b/go.sum index 01eb8e3fa..73c04ec83 100644 --- a/go.sum +++ b/go.sum @@ -11,6 +11,8 @@ github.com/atotto/clipboard v0.1.2/go.mod h1:ZY9tmq7sm5xIbd9bOK4onWV4S6X0u6GY7Vn github.com/aybabtme/humanlog v0.4.1 h1:D8d9um55rrthJsP8IGSHBcti9lTb/XknmDAX6Zy8tek= github.com/aybabtme/humanlog v0.4.1/go.mod h1:B0bnQX4FTSU3oftPMTTPvENCy8LqixLDvYJA9TUCAGo= github.com/aybabtme/rgbterm v0.0.0-20170906152045-cc83f3b3ce59/go.mod h1:q/89r3U2H7sSsE2t6Kca0lfwTK8JdoNGS/yzM/4iH5I= +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/cloudfoundry/jibber_jabber v0.0.0-20151120183258-bcc4c8345a21 h1:tuijfIjZyjZaHq9xDUh0tNitwXshJpbLkqMOJv4H3do= github.com/cloudfoundry/jibber_jabber v0.0.0-20151120183258-bcc4c8345a21/go.mod h1:po7NpZ/QiTKzBKyrsEAxwnTamCoh8uDk/egRpQ7siIc= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= diff --git a/pkg/app/app.go b/pkg/app/app.go index a53e4db8c..22f5913dd 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -8,7 +8,6 @@ import ( "io/ioutil" "log" "os" - "os/exec" "path/filepath" "regexp" "strconv" @@ -21,6 +20,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/env" "github.com/jesseduffield/lazygit/pkg/gui" "github.com/jesseduffield/lazygit/pkg/i18n" + "github.com/jesseduffield/lazygit/pkg/secureexec" "github.com/jesseduffield/lazygit/pkg/updates" "github.com/sirupsen/logrus" ) @@ -324,7 +324,7 @@ func TailLogs() { log.Fatal(err) } - cmd := exec.Command("tail", "-f", logFilePath) + cmd := secureexec.Command("tail", "-f", logFilePath) stdout, _ := cmd.StdoutPipe() if err := cmd.Start(); err != nil { diff --git a/pkg/commands/git_test.go b/pkg/commands/git_test.go index ac34d195c..df25ffe3a 100644 --- a/pkg/commands/git_test.go +++ b/pkg/commands/git_test.go @@ -16,6 +16,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/config" "github.com/jesseduffield/lazygit/pkg/i18n" + "github.com/jesseduffield/lazygit/pkg/secureexec" "github.com/jesseduffield/lazygit/pkg/test" "github.com/jesseduffield/lazygit/pkg/utils" "github.com/stretchr/testify/assert" @@ -269,7 +270,7 @@ func TestGitCommandGetStashEntries(t *testing.T) { { "No stash entries found", func(string, ...string) *exec.Cmd { - return exec.Command("echo") + return secureexec.Command("echo") }, func(entries []*models.StashEntry) { assert.Len(t, entries, 0) @@ -278,7 +279,7 @@ func TestGitCommandGetStashEntries(t *testing.T) { { "Several stash entries found", func(string, ...string) *exec.Cmd { - return exec.Command("echo", "WIP on add-pkg-commands-test: 55c6af2 increase parallel build\nWIP on master: bb86a3f update github template") + return secureexec.Command("echo", "WIP on add-pkg-commands-test: 55c6af2 increase parallel build\nWIP on master: bb86a3f update github template") }, func(entries []*models.StashEntry) { expected := []*models.StashEntry{ @@ -320,7 +321,7 @@ func TestGitCommandGetStatusFiles(t *testing.T) { { "No files found", func(cmd string, args ...string) *exec.Cmd { - return exec.Command("echo") + return secureexec.Command("echo") }, func(files []*models.File) { assert.Len(t, files, 0) @@ -329,7 +330,7 @@ func TestGitCommandGetStatusFiles(t *testing.T) { { "Several files found", func(cmd string, args ...string) *exec.Cmd { - return exec.Command( + return secureexec.Command( "echo", "MM file1.txt\nA file3.txt\nAM file2.txt\n?? file4.txt\nUU file5.txt", ) @@ -422,7 +423,7 @@ func TestGitCommandStashDo(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"stash", "drop", "stash@{1}"}, args) - return exec.Command("echo") + return secureexec.Command("echo") } assert.NoError(t, gitCmd.StashDo(1, "drop")) @@ -435,7 +436,7 @@ func TestGitCommandStashSave(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"stash", "save", "A stash message"}, args) - return exec.Command("echo") + return secureexec.Command("echo") } assert.NoError(t, gitCmd.StashSave("A stash message")) @@ -448,7 +449,7 @@ func TestGitCommandCommitAmend(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"commit", "--amend", "--allow-empty"}, args) - return exec.Command("echo") + return secureexec.Command("echo") } _, err := gitCmd.PrepareCommitAmendSubProcess().CombinedOutput() @@ -548,7 +549,7 @@ func TestGitCommandGetCommitDifferences(t *testing.T) { { "Can't retrieve pushable count", func(string, ...string) *exec.Cmd { - return exec.Command("test") + return secureexec.Command("test") }, func(pushableCount string, pullableCount string) { assert.EqualValues(t, "?", pushableCount) @@ -559,10 +560,10 @@ func TestGitCommandGetCommitDifferences(t *testing.T) { "Can't retrieve pullable count", func(cmd string, args ...string) *exec.Cmd { if args[1] == "HEAD..@{u}" { - return exec.Command("test") + return secureexec.Command("test") } - return exec.Command("echo") + return secureexec.Command("echo") }, func(pushableCount string, pullableCount string) { assert.EqualValues(t, "?", pushableCount) @@ -573,10 +574,10 @@ func TestGitCommandGetCommitDifferences(t *testing.T) { "Retrieve pullable and pushable count", func(cmd string, args ...string) *exec.Cmd { if args[1] == "HEAD..@{u}" { - return exec.Command("echo", "10") + return secureexec.Command("echo", "10") } - return exec.Command("echo", "11") + return secureexec.Command("echo", "11") }, func(pushableCount string, pullableCount string) { assert.EqualValues(t, "11", pushableCount) @@ -601,7 +602,7 @@ func TestGitCommandRenameCommit(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"commit", "--allow-empty", "--amend", "-m", "test"}, args) - return exec.Command("echo") + return secureexec.Command("echo") } assert.NoError(t, gitCmd.RenameCommit("test")) @@ -614,7 +615,7 @@ func TestGitCommandResetToCommit(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"reset", "--hard", "78976bc"}, args) - return exec.Command("echo") + return secureexec.Command("echo") } assert.NoError(t, gitCmd.ResetToCommit("78976bc", "hard", oscommands.RunCommandOptions{})) @@ -627,7 +628,7 @@ func TestGitCommandNewBranch(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"checkout", "-b", "test", "master"}, args) - return exec.Command("echo") + return secureexec.Command("echo") } assert.NoError(t, gitCmd.NewBranch("test", "master")) @@ -652,7 +653,7 @@ func TestGitCommandDeleteBranch(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"branch", "-d", "test"}, args) - return exec.Command("echo") + return secureexec.Command("echo") }, func(err error) { assert.NoError(t, err) @@ -666,7 +667,7 @@ func TestGitCommandDeleteBranch(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"branch", "-D", "test"}, args) - return exec.Command("echo") + return secureexec.Command("echo") }, func(err error) { assert.NoError(t, err) @@ -690,7 +691,7 @@ func TestGitCommandMerge(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"merge", "--no-edit", "test"}, args) - return exec.Command("echo") + return secureexec.Command("echo") } assert.NoError(t, gitCmd.Merge("test", MergeOpts{})) @@ -807,7 +808,7 @@ func TestGitCommandCommit(t *testing.T) { assert.EqualValues(t, "bash", cmd) assert.EqualValues(t, []string{"-c", "git commit -m \"test\""}, args) - return exec.Command("echo") + return secureexec.Command("echo") }, func(string) (string, error) { return "true", nil @@ -824,7 +825,7 @@ func TestGitCommandCommit(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"commit", "-m", "test"}, args) - return exec.Command("echo") + return secureexec.Command("echo") }, func(string) (string, error) { return "false", nil @@ -841,7 +842,7 @@ func TestGitCommandCommit(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"commit", "--no-verify", "-m", "test"}, args) - return exec.Command("echo") + return secureexec.Command("echo") }, func(string) (string, error) { return "false", nil @@ -858,7 +859,7 @@ func TestGitCommandCommit(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"commit", "-m", "test"}, args) - return exec.Command("test") + return secureexec.Command("test") }, func(string) (string, error) { return "false", nil @@ -897,7 +898,7 @@ func TestGitCommandAmendHead(t *testing.T) { assert.EqualValues(t, "bash", cmd) assert.EqualValues(t, []string{"-c", "git commit --amend --no-edit --allow-empty"}, args) - return exec.Command("echo") + return secureexec.Command("echo") }, func(string) (string, error) { return "true", nil @@ -913,7 +914,7 @@ func TestGitCommandAmendHead(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"commit", "--amend", "--no-edit", "--allow-empty"}, args) - return exec.Command("echo") + return secureexec.Command("echo") }, func(string) (string, error) { return "false", nil @@ -929,7 +930,7 @@ func TestGitCommandAmendHead(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"commit", "--amend", "--no-edit", "--allow-empty"}, args) - return exec.Command("test") + return secureexec.Command("test") }, func(string) (string, error) { return "false", nil @@ -975,7 +976,7 @@ func TestGitCommandPush(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"push", "--follow-tags"}, args) - return exec.Command("echo") + return secureexec.Command("echo") }, false, func(err error) { @@ -994,7 +995,7 @@ func TestGitCommandPush(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"push", "--follow-tags", "--force-with-lease"}, args) - return exec.Command("echo") + return secureexec.Command("echo") }, true, func(err error) { @@ -1013,7 +1014,7 @@ func TestGitCommandPush(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"push"}, args) - return exec.Command("echo") + return secureexec.Command("echo") }, false, func(err error) { @@ -1032,7 +1033,7 @@ func TestGitCommandPush(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"push", "--force-with-lease"}, args) - return exec.Command("echo") + return secureexec.Command("echo") }, true, func(err error) { @@ -1050,7 +1051,7 @@ func TestGitCommandPush(t *testing.T) { func(cmd string, args ...string) *exec.Cmd { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"push", "--follow-tags"}, args) - return exec.Command("test") + return secureexec.Command("test") }, false, func(err error) { @@ -1087,7 +1088,7 @@ func TestGitCommandCatFile(t *testing.T) { assert.EqualValues(t, osCmd, cmd) assert.EqualValues(t, []string{"test.txt"}, args) - return exec.Command("echo", "-n", "test") + return secureexec.Command("echo", "-n", "test") } o, err := gitCmd.CatFile("test.txt") @@ -1102,7 +1103,7 @@ func TestGitCommandStageFile(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"add", "test.txt"}, args) - return exec.Command("echo") + return secureexec.Command("echo") } assert.NoError(t, gitCmd.StageFile("test.txt")) @@ -1124,7 +1125,7 @@ func TestGitCommandUnstageFile(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"rm", "--cached", "--force", "test.txt"}, args) - return exec.Command("echo") + return secureexec.Command("echo") }, func(err error) { assert.NoError(t, err) @@ -1137,7 +1138,7 @@ func TestGitCommandUnstageFile(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"reset", "HEAD", "test.txt"}, args) - return exec.Command("echo") + return secureexec.Command("echo") }, func(err error) { assert.NoError(t, err) @@ -1173,7 +1174,7 @@ func TestGitCommandDiscardAllFileChanges(t *testing.T) { return func(cmd string, args ...string) *exec.Cmd { cmdsCalled = append(cmdsCalled, args) - return exec.Command("test") + return secureexec.Command("test") }, &cmdsCalled }, func(cmdsCalled *[][]string, err error) { @@ -1198,7 +1199,7 @@ func TestGitCommandDiscardAllFileChanges(t *testing.T) { return func(cmd string, args ...string) *exec.Cmd { cmdsCalled = append(cmdsCalled, args) - return exec.Command("test") + return secureexec.Command("test") }, &cmdsCalled }, func(cmdsCalled *[][]string, err error) { @@ -1221,7 +1222,7 @@ func TestGitCommandDiscardAllFileChanges(t *testing.T) { return func(cmd string, args ...string) *exec.Cmd { cmdsCalled = append(cmdsCalled, args) - return exec.Command("test") + return secureexec.Command("test") }, &cmdsCalled }, func(cmdsCalled *[][]string, err error) { @@ -1247,7 +1248,7 @@ func TestGitCommandDiscardAllFileChanges(t *testing.T) { return func(cmd string, args ...string) *exec.Cmd { cmdsCalled = append(cmdsCalled, args) - return exec.Command("echo") + return secureexec.Command("echo") }, &cmdsCalled }, func(cmdsCalled *[][]string, err error) { @@ -1273,7 +1274,7 @@ func TestGitCommandDiscardAllFileChanges(t *testing.T) { return func(cmd string, args ...string) *exec.Cmd { cmdsCalled = append(cmdsCalled, args) - return exec.Command("echo") + return secureexec.Command("echo") }, &cmdsCalled }, func(cmdsCalled *[][]string, err error) { @@ -1300,7 +1301,7 @@ func TestGitCommandDiscardAllFileChanges(t *testing.T) { return func(cmd string, args ...string) *exec.Cmd { cmdsCalled = append(cmdsCalled, args) - return exec.Command("echo") + return secureexec.Command("echo") }, &cmdsCalled }, func(cmdsCalled *[][]string, err error) { @@ -1327,7 +1328,7 @@ func TestGitCommandDiscardAllFileChanges(t *testing.T) { return func(cmd string, args ...string) *exec.Cmd { cmdsCalled = append(cmdsCalled, args) - return exec.Command("echo") + return secureexec.Command("echo") }, &cmdsCalled }, func(cmdsCalled *[][]string, err error) { @@ -1354,7 +1355,7 @@ func TestGitCommandDiscardAllFileChanges(t *testing.T) { return func(cmd string, args ...string) *exec.Cmd { cmdsCalled = append(cmdsCalled, args) - return exec.Command("echo") + return secureexec.Command("echo") }, &cmdsCalled }, func(cmdsCalled *[][]string, err error) { @@ -1400,7 +1401,7 @@ func TestGitCommandCheckout(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"checkout", "test"}, args) - return exec.Command("echo") + return secureexec.Command("echo") }, func(err error) { assert.NoError(t, err) @@ -1413,7 +1414,7 @@ func TestGitCommandCheckout(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"checkout", "--force", "test"}, args) - return exec.Command("echo") + return secureexec.Command("echo") }, func(err error) { assert.NoError(t, err) @@ -1437,7 +1438,7 @@ func TestGitCommandGetBranchGraph(t *testing.T) { gitCmd.OSCommand.Command = func(cmd string, args ...string) *exec.Cmd { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"log", "--graph", "--color=always", "--abbrev-commit", "--decorate", "--date=relative", "--pretty=medium", "test", "--"}, args) - return exec.Command("echo") + return secureexec.Command("echo") } _, err := gitCmd.GetBranchGraph("test") assert.NoError(t, err) @@ -1448,7 +1449,7 @@ func TestGitCommandGetAllBranchGraph(t *testing.T) { gitCmd.OSCommand.Command = func(cmd string, args ...string) *exec.Cmd { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"log", "--graph", "--all", "--color=always", "--abbrev-commit", "--decorate", "--date=relative", "--pretty=medium"}, args) - return exec.Command("echo") + return secureexec.Command("echo") } cmdStr := gitCmd.Config.GetUserConfig().Git.AllBranchesLogCmd _, err := gitCmd.OSCommand.RunCommandWithOutput(cmdStr) @@ -1472,7 +1473,7 @@ func TestGitCommandDiff(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"diff", "--submodule", "--no-ext-diff", "--color=always", "--", "test.txt"}, args) - return exec.Command("echo") + return secureexec.Command("echo") }, &models.File{ Name: "test.txt", @@ -1488,7 +1489,7 @@ func TestGitCommandDiff(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"diff", "--submodule", "--no-ext-diff", "--color=always", "--cached", "--", "test.txt"}, args) - return exec.Command("echo") + return secureexec.Command("echo") }, &models.File{ Name: "test.txt", @@ -1504,7 +1505,7 @@ func TestGitCommandDiff(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"diff", "--submodule", "--no-ext-diff", "--color=never", "--", "test.txt"}, args) - return exec.Command("echo") + return secureexec.Command("echo") }, &models.File{ Name: "test.txt", @@ -1520,7 +1521,7 @@ func TestGitCommandDiff(t *testing.T) { assert.EqualValues(t, "git", cmd) assert.EqualValues(t, []string{"diff", "--submodule", "--no-ext-diff", "--color=always", "--no-index", "/dev/null", "test.txt"}, args) - return exec.Command("echo") + return secureexec.Command("echo") }, &models.File{ Name: "test.txt", @@ -1554,7 +1555,7 @@ func TestGitCommandCurrentBranchName(t *testing.T) { "says we are on the master branch if we are", func(cmd string, args ...string) *exec.Cmd { assert.Equal(t, "git", cmd) - return exec.Command("echo", "master") + return secureexec.Command("echo", "master") }, func(name string, displayname string, err error) { assert.NoError(t, err) @@ -1570,10 +1571,10 @@ func TestGitCommandCurrentBranchName(t *testing.T) { switch args[0] { case "symbolic-ref": assert.EqualValues(t, []string{"symbolic-ref", "--short", "HEAD"}, args) - return exec.Command("test") + return secureexec.Command("test") case "branch": assert.EqualValues(t, []string{"branch", "--contains"}, args) - return exec.Command("echo", "* master") + return secureexec.Command("echo", "* master") } return nil @@ -1592,10 +1593,10 @@ func TestGitCommandCurrentBranchName(t *testing.T) { switch args[0] { case "symbolic-ref": assert.EqualValues(t, []string{"symbolic-ref", "--short", "HEAD"}, args) - return exec.Command("test") + return secureexec.Command("test") case "branch": assert.EqualValues(t, []string{"branch", "--contains"}, args) - return exec.Command("echo", "* (HEAD detached at 123abcd)") + return secureexec.Command("echo", "* (HEAD detached at 123abcd)") } return nil @@ -1610,7 +1611,7 @@ func TestGitCommandCurrentBranchName(t *testing.T) { "bubbles up error if there is one", func(cmd string, args ...string) *exec.Cmd { assert.Equal(t, "git", cmd) - return exec.Command("test") + return secureexec.Command("test") }, func(name string, displayname string, err error) { assert.Error(t, err) @@ -1648,7 +1649,7 @@ func TestGitCommandApplyPatch(t *testing.T) { assert.Equal(t, "test", string(content)) - return exec.Command("echo", "done") + return secureexec.Command("echo", "done") }, func(err error) { assert.NoError(t, err) @@ -1669,7 +1670,7 @@ func TestGitCommandApplyPatch(t *testing.T) { assert.Equal(t, "test", string(content)) - return exec.Command("test") + return secureexec.Command("test") }, func(err error) { assert.Error(t, err) @@ -2174,7 +2175,7 @@ func TestEditFile(t *testing.T) { { "test", func(name string, arg ...string) *exec.Cmd { - return exec.Command("exit", "1") + return secureexec.Command("exit", "1") }, func(env string) string { return "" @@ -2190,7 +2191,7 @@ func TestEditFile(t *testing.T) { "test", func(name string, arg ...string) *exec.Cmd { if name == "which" { - return exec.Command("exit", "1") + return secureexec.Command("exit", "1") } assert.EqualValues(t, "nano", name) @@ -2211,7 +2212,7 @@ func TestEditFile(t *testing.T) { "test", func(name string, arg ...string) *exec.Cmd { if name == "which" { - return exec.Command("exit", "1") + return secureexec.Command("exit", "1") } assert.EqualValues(t, "nano", name) @@ -2236,7 +2237,7 @@ func TestEditFile(t *testing.T) { "test", func(name string, arg ...string) *exec.Cmd { if name == "which" { - return exec.Command("exit", "1") + return secureexec.Command("exit", "1") } assert.EqualValues(t, "emacs", name) @@ -2261,7 +2262,7 @@ func TestEditFile(t *testing.T) { "test", func(name string, arg ...string) *exec.Cmd { if name == "which" { - return exec.Command("echo") + return secureexec.Command("echo") } assert.EqualValues(t, "vi", name) @@ -2282,7 +2283,7 @@ func TestEditFile(t *testing.T) { "file/with space", func(name string, args ...string) *exec.Cmd { if name == "which" { - return exec.Command("echo") + return secureexec.Command("echo") } assert.EqualValues(t, "vi", name) diff --git a/pkg/commands/loading_commits_test.go b/pkg/commands/loading_commits_test.go index c6c77f259..a637f5bd5 100644 --- a/pkg/commands/loading_commits_test.go +++ b/pkg/commands/loading_commits_test.go @@ -6,6 +6,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/i18n" + "github.com/jesseduffield/lazygit/pkg/secureexec" "github.com/jesseduffield/lazygit/pkg/utils" "github.com/stretchr/testify/assert" ) @@ -39,10 +40,10 @@ func TestCommitListBuilderGetMergeBase(t *testing.T) { switch args[0] { case "symbolic-ref": assert.EqualValues(t, []string{"symbolic-ref", "--short", "HEAD"}, args) - return exec.Command("echo", "master") + return secureexec.Command("echo", "master") case "merge-base": assert.EqualValues(t, []string{"merge-base", "HEAD", "master"}, args) - return exec.Command("test") + return secureexec.Command("test") } return nil }, @@ -59,10 +60,10 @@ func TestCommitListBuilderGetMergeBase(t *testing.T) { switch args[0] { case "symbolic-ref": assert.EqualValues(t, []string{"symbolic-ref", "--short", "HEAD"}, args) - return exec.Command("echo", "master") + return secureexec.Command("echo", "master") case "merge-base": assert.EqualValues(t, []string{"merge-base", "HEAD", "master"}, args) - return exec.Command("echo", "blah") + return secureexec.Command("echo", "blah") } return nil }, @@ -79,10 +80,10 @@ func TestCommitListBuilderGetMergeBase(t *testing.T) { switch args[0] { case "symbolic-ref": assert.EqualValues(t, []string{"symbolic-ref", "--short", "HEAD"}, args) - return exec.Command("echo", "feature/test") + return secureexec.Command("echo", "feature/test") case "merge-base": assert.EqualValues(t, []string{"merge-base", "HEAD", "develop"}, args) - return exec.Command("echo", "blah") + return secureexec.Command("echo", "blah") } return nil }, @@ -94,7 +95,7 @@ func TestCommitListBuilderGetMergeBase(t *testing.T) { { "bubbles up error if there is one", func(cmd string, args ...string) *exec.Cmd { - return exec.Command("test") + return secureexec.Command("test") }, func(output string, err error) { assert.Error(t, err) diff --git a/pkg/commands/oscommands/os.go b/pkg/commands/oscommands/os.go index bbfcb8095..2638f3c49 100644 --- a/pkg/commands/oscommands/os.go +++ b/pkg/commands/oscommands/os.go @@ -16,6 +16,7 @@ import ( "github.com/atotto/clipboard" "github.com/jesseduffield/lazygit/pkg/config" + "github.com/jesseduffield/lazygit/pkg/secureexec" "github.com/jesseduffield/lazygit/pkg/utils" "github.com/mgutz/str" "github.com/sirupsen/logrus" @@ -49,7 +50,7 @@ func NewOSCommand(log *logrus.Entry, config config.AppConfigurer) *OSCommand { Log: log, Platform: getPlatform(), Config: config, - Command: exec.Command, + Command: secureexec.Command, BeforeExecuteCmd: func(*exec.Cmd) {}, Getenv: os.Getenv, } diff --git a/pkg/commands/oscommands/os_test.go b/pkg/commands/oscommands/os_test.go index f8b3767bf..1504e4322 100644 --- a/pkg/commands/oscommands/os_test.go +++ b/pkg/commands/oscommands/os_test.go @@ -6,6 +6,7 @@ import ( "os/exec" "testing" + "github.com/jesseduffield/lazygit/pkg/secureexec" "github.com/stretchr/testify/assert" ) @@ -70,7 +71,7 @@ func TestOSCommandOpenFile(t *testing.T) { { "test", func(name string, arg ...string) *exec.Cmd { - return exec.Command("exit", "1") + return secureexec.Command("exit", "1") }, func(err error) { assert.Error(t, err) @@ -81,7 +82,7 @@ func TestOSCommandOpenFile(t *testing.T) { func(name string, arg ...string) *exec.Cmd { assert.Equal(t, "open", name) assert.Equal(t, []string{"test"}, arg) - return exec.Command("echo") + return secureexec.Command("echo") }, func(err error) { assert.NoError(t, err) @@ -92,7 +93,7 @@ func TestOSCommandOpenFile(t *testing.T) { func(name string, arg ...string) *exec.Cmd { assert.Equal(t, "open", name) assert.Equal(t, []string{"filename with spaces"}, arg) - return exec.Command("echo") + return secureexec.Command("echo") }, func(err error) { assert.NoError(t, err) diff --git a/pkg/commands/pull_request_test.go b/pkg/commands/pull_request_test.go index 110f5b58a..752bc53e2 100644 --- a/pkg/commands/pull_request_test.go +++ b/pkg/commands/pull_request_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/jesseduffield/lazygit/pkg/commands/models" + "github.com/jesseduffield/lazygit/pkg/secureexec" "github.com/stretchr/testify/assert" ) @@ -63,12 +64,12 @@ func TestCreatePullRequest(t *testing.T) { command: func(cmd string, args ...string) *exec.Cmd { // Handle git remote url call if strings.HasPrefix(cmd, "git") { - return exec.Command("echo", "git@bitbucket.org:johndoe/social_network.git") + return secureexec.Command("echo", "git@bitbucket.org:johndoe/social_network.git") } assert.Equal(t, cmd, "open") assert.Equal(t, args, []string{"https://bitbucket.org/johndoe/social_network/pull-requests/new?source=feature/profile-page&t=1"}) - return exec.Command("echo") + return secureexec.Command("echo") }, test: func(err error) { assert.NoError(t, err) @@ -83,12 +84,12 @@ func TestCreatePullRequest(t *testing.T) { command: func(cmd string, args ...string) *exec.Cmd { // Handle git remote url call if strings.HasPrefix(cmd, "git") { - return exec.Command("echo", "https://my_username@bitbucket.org/johndoe/social_network.git") + return secureexec.Command("echo", "https://my_username@bitbucket.org/johndoe/social_network.git") } assert.Equal(t, cmd, "open") assert.Equal(t, args, []string{"https://bitbucket.org/johndoe/social_network/pull-requests/new?source=feature/events&t=1"}) - return exec.Command("echo") + return secureexec.Command("echo") }, test: func(err error) { assert.NoError(t, err) @@ -103,12 +104,12 @@ func TestCreatePullRequest(t *testing.T) { command: func(cmd string, args ...string) *exec.Cmd { // Handle git remote url call if strings.HasPrefix(cmd, "git") { - return exec.Command("echo", "git@github.com:peter/calculator.git") + return secureexec.Command("echo", "git@github.com:peter/calculator.git") } assert.Equal(t, cmd, "open") assert.Equal(t, args, []string{"https://github.com/peter/calculator/compare/feature/sum-operation?expand=1"}) - return exec.Command("echo") + return secureexec.Command("echo") }, test: func(err error) { assert.NoError(t, err) @@ -123,12 +124,12 @@ func TestCreatePullRequest(t *testing.T) { command: func(cmd string, args ...string) *exec.Cmd { // Handle git remote url call if strings.HasPrefix(cmd, "git") { - return exec.Command("echo", "git@gitlab.com:peter/calculator.git") + return secureexec.Command("echo", "git@gitlab.com:peter/calculator.git") } assert.Equal(t, cmd, "open") assert.Equal(t, args, []string{"https://gitlab.com/peter/calculator/merge_requests/new?merge_request[source_branch]=feature/ui"}) - return exec.Command("echo") + return secureexec.Command("echo") }, test: func(err error) { assert.NoError(t, err) @@ -141,7 +142,7 @@ func TestCreatePullRequest(t *testing.T) { }, remoteUrl: "git@something.com:peter/calculator.git", command: func(cmd string, args ...string) *exec.Cmd { - return exec.Command("echo") + return secureexec.Command("echo") }, test: func(err error) { assert.Error(t, err) diff --git a/pkg/gui/gui_test.go b/pkg/gui/gui_test.go index f632f971a..c513b8399 100644 --- a/pkg/gui/gui_test.go +++ b/pkg/gui/gui_test.go @@ -6,7 +6,6 @@ import ( "io" "io/ioutil" "os" - "os/exec" "path/filepath" "strconv" "strings" @@ -14,6 +13,7 @@ import ( "github.com/creack/pty" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" + "github.com/jesseduffield/lazygit/pkg/secureexec" "github.com/stretchr/testify/assert" ) @@ -250,7 +250,7 @@ func Test(t *testing.T) { func createFixture(testPath, actualDir string) error { osCommand := oscommands.NewDummyOSCommand() bashScriptPath := filepath.Join(testPath, "setup.sh") - cmd := exec.Command("bash", bashScriptPath, actualDir) + cmd := secureexec.Command("bash", bashScriptPath, actualDir) if err := osCommand.RunExecutable(cmd); err != nil { return err diff --git a/pkg/secureexec/secureexec_default.go b/pkg/secureexec/secureexec_default.go new file mode 100644 index 000000000..1992358ce --- /dev/null +++ b/pkg/secureexec/secureexec_default.go @@ -0,0 +1,11 @@ +// +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 new file mode 100644 index 000000000..537e0bfc1 --- /dev/null +++ b/pkg/secureexec/secureexec_windows.go @@ -0,0 +1,30 @@ +// +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. + +func Command(name string, args ...string) *exec.Cmd { + bin, err := safeexec.LookPath(name) + if err != nil { + bin = name + } + + return exec.Command(bin, args...) +} diff --git a/pkg/test/test.go b/pkg/test/test.go index 38eabc6a4..da476b95c 100644 --- a/pkg/test/test.go +++ b/pkg/test/test.go @@ -2,11 +2,11 @@ package test import ( "os" - "os/exec" "path/filepath" "github.com/go-errors/errors" + "github.com/jesseduffield/lazygit/pkg/secureexec" "github.com/jesseduffield/lazygit/pkg/utils" ) @@ -24,7 +24,7 @@ func GenerateRepo(filename string) error { if err := os.Chdir(testPath); err != nil { return err } - if output, err := exec.Command("bash", filename).CombinedOutput(); err != nil { + if output, err := secureexec.Command("bash", filename).CombinedOutput(); err != nil { return errors.New(string(output)) } diff --git a/pkg/test/utils.go b/pkg/test/utils.go index b27bdbbab..47f2c1146 100644 --- a/pkg/test/utils.go +++ b/pkg/test/utils.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "github.com/jesseduffield/lazygit/pkg/secureexec" "github.com/mgutz/str" "github.com/stretchr/testify/assert" ) @@ -27,7 +28,7 @@ func (i *CommandSwapper) SwapCommand(t *testing.T, cmd string, args []string) *e } splitCmd = str.ToArgv(i.Replace) - return exec.Command(splitCmd[0], splitCmd[1:]...) + return secureexec.Command(splitCmd[0], splitCmd[1:]...) } // CreateMockCommand creates a command function that will verify its receiving the right sequence of commands from lazygit diff --git a/scripts/push_new_patch/main.go b/scripts/push_new_patch/main.go index 01b844c8d..40d95d9a0 100755 --- a/scripts/push_new_patch/main.go +++ b/scripts/push_new_patch/main.go @@ -11,9 +11,10 @@ import ( "io/ioutil" "log" "os" - "os/exec" "strconv" "strings" + + "github.com/jesseduffield/lazygit/pkg/secureexec" ) func main() { @@ -49,7 +50,7 @@ func main() { func runCommand(args ...string) { fmt.Println(strings.Join(args, " ")) - cmd := exec.Command(args[0], args[1:]...) + cmd := secureexec.Command(args[0], args[1:]...) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr err := cmd.Start() diff --git a/vendor/github.com/cli/safeexec/LICENSE b/vendor/github.com/cli/safeexec/LICENSE new file mode 100644 index 000000000..ca498575a --- /dev/null +++ b/vendor/github.com/cli/safeexec/LICENSE @@ -0,0 +1,25 @@ +BSD 2-Clause License + +Copyright (c) 2020, GitHub Inc. +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are met: + +1. Redistributions of source code must retain the above copyright notice, this + list of conditions and the following disclaimer. + +2. Redistributions in binary form must reproduce the above copyright notice, + this list of conditions and the following disclaimer in the documentation + and/or other materials provided with the distribution. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/vendor/github.com/cli/safeexec/README.md b/vendor/github.com/cli/safeexec/README.md new file mode 100644 index 000000000..bd73e9ad6 --- /dev/null +++ b/vendor/github.com/cli/safeexec/README.md @@ -0,0 +1,40 @@ +# safeexec + +A Go module that provides a safer alternative to `exec.LookPath()` on Windows. + +The following, relatively common approach to running external commands has a subtle vulnerability on Windows: +```go +import "os/exec" + +func gitStatus() error { + // On Windows, this will result in `.\git.exe` or `.\git.bat` being executed + // if either were found in the current working directory. + cmd := exec.Command("git", "status") + return cmd.Run() +} +``` + +Searching the current directory (surprising behavior) before searching folders listed in the PATH environment variable (expected behavior) seems to be intended in Go and unlikely to be changed: https://github.com/golang/go/issues/38736 + +Since Go does not provide a version of [`exec.LookPath()`](https://golang.org/pkg/os/exec/#LookPath) that only searches PATH and does not search the current working directory, this module provides a `LookPath` function that works consistently across platforms. + +Example use: +```go +import ( + "os/exec" + "github.com/cli/safeexec" +) + +func gitStatus() error { + gitBin, err := safeexec.LookPath("git") + if err != nil { + return err + } + cmd := exec.Command(gitBin, "status") + return cmd.Run() +} +``` + +## TODO + +Ideally, this module would also provide `exec.Command()` and `exec.CommandContext()` equivalents that delegate to the patched version of `LookPath`. However, this doesn't seem possible since `LookPath` may return an error, while `exec.Command/CommandContext()` themselves do not return an error. In the standard library, the resulting `exec.Cmd` struct stores the LookPath error in a private field, but that functionality isn't available to us. diff --git a/vendor/github.com/cli/safeexec/go.mod b/vendor/github.com/cli/safeexec/go.mod new file mode 100644 index 000000000..266fab447 --- /dev/null +++ b/vendor/github.com/cli/safeexec/go.mod @@ -0,0 +1,3 @@ +module github.com/cli/safeexec + +go 1.15 diff --git a/vendor/github.com/cli/safeexec/lookpath.go b/vendor/github.com/cli/safeexec/lookpath.go new file mode 100644 index 000000000..41b777078 --- /dev/null +++ b/vendor/github.com/cli/safeexec/lookpath.go @@ -0,0 +1,9 @@ +// +build !windows + +package safeexec + +import "os/exec" + +func LookPath(file string) (string, error) { + return exec.LookPath(file) +} diff --git a/vendor/github.com/cli/safeexec/lookpath_windows.go b/vendor/github.com/cli/safeexec/lookpath_windows.go new file mode 100644 index 000000000..19b3e52f7 --- /dev/null +++ b/vendor/github.com/cli/safeexec/lookpath_windows.go @@ -0,0 +1,120 @@ +// Copyright (c) 2009 The Go Authors. All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +// Package safeexec provides alternatives for exec package functions to avoid +// accidentally executing binaries found in the current working directory on +// Windows. +package safeexec + +import ( + "os" + "os/exec" + "path/filepath" + "strings" +) + +func chkStat(file string) error { + d, err := os.Stat(file) + if err != nil { + return err + } + if d.IsDir() { + return os.ErrPermission + } + return nil +} + +func hasExt(file string) bool { + i := strings.LastIndex(file, ".") + if i < 0 { + return false + } + return strings.LastIndexAny(file, `:\/`) < i +} + +func findExecutable(file string, exts []string) (string, error) { + if len(exts) == 0 { + return file, chkStat(file) + } + if hasExt(file) { + if chkStat(file) == nil { + return file, nil + } + } + for _, e := range exts { + if f := file + e; chkStat(f) == nil { + return f, nil + } + } + return "", os.ErrNotExist +} + +// LookPath searches for an executable named file in the +// directories named by the PATH environment variable. +// If file contains a slash, it is tried directly and the PATH is not consulted. +// LookPath also uses PATHEXT environment variable to match +// a suitable candidate. +// The result may be an absolute path or a path relative to the current directory. +func LookPath(file string) (string, error) { + var exts []string + x := os.Getenv(`PATHEXT`) + if x != "" { + for _, e := range strings.Split(strings.ToLower(x), `;`) { + if e == "" { + continue + } + if e[0] != '.' { + e = "." + e + } + exts = append(exts, e) + } + } else { + exts = []string{".com", ".exe", ".bat", ".cmd"} + } + + if strings.ContainsAny(file, `:\/`) { + if f, err := findExecutable(file, exts); err == nil { + return f, nil + } else { + return "", &exec.Error{file, err} + } + } + + // https://github.com/golang/go/issues/38736 + // if f, err := findExecutable(filepath.Join(".", file), exts); err == nil { + // return f, nil + // } + + path := os.Getenv("path") + for _, dir := range filepath.SplitList(path) { + if f, err := findExecutable(filepath.Join(dir, file), exts); err == nil { + return f, nil + } + } + return "", &exec.Error{file, exec.ErrNotFound} +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 141091a4e..87c7fd91b 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -7,6 +7,9 @@ github.com/atotto/clipboard # github.com/aybabtme/humanlog v0.4.1 ## explicit github.com/aybabtme/humanlog +# github.com/cli/safeexec v1.0.0 +## explicit +github.com/cli/safeexec # github.com/cloudfoundry/jibber_jabber v0.0.0-20151120183258-bcc4c8345a21 ## explicit github.com/cloudfoundry/jibber_jabber