Skip to content
This repository has been archived by the owner on Nov 22, 2022. It is now read-only.

Commit

Permalink
Merge pull request #332 from profclems/fix-lookpath
Browse files Browse the repository at this point in the history
Ensure only $PATH is searched when switching to external commands
  • Loading branch information
profclems authored Nov 26, 2020
2 parents 9eae91f + ac0af10 commit 2e8c423
Show file tree
Hide file tree
Showing 14 changed files with 276 additions and 18 deletions.
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ lint: bin/golangci-lint ## Run linter

.PHONY: fix
fix: bin/golangci-lint ## Fix lint violations
go mod tidy
$(GOLINT) run --fix
gofmt -s -w .
goimports -w .
Expand Down
7 changes: 4 additions & 3 deletions commands/alias/expand/alias_expand.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ import (
"errors"
"fmt"
"os"
"os/exec"
"path/filepath"
"regexp"
"runtime"
"strings"

"github.com/profclems/glab/pkg/execext"

"github.com/google/shlex"
"github.com/profclems/glab/internal/config"
)
Expand Down Expand Up @@ -80,15 +81,15 @@ func ExpandAlias(cfg config.Config, args []string, findShFunc func() (string, er
}

func findSh() (string, error) {
shPath, err := exec.LookPath("sh")
shPath, err := execext.LookPath("sh")
if err == nil {
return shPath, nil
}

if runtime.GOOS == "windows" {
winNotFoundErr := errors.New("unable to locate sh to execute the shell alias with. The sh.exe interpreter is typically distributed with Git for Windows")
// We can try and find a sh executable in a Git for Windows install
gitPath, err := exec.LookPath("git")
gitPath, err := execext.LookPath("git")
if err != nil {
return "", winNotFoundErr
}
Expand Down
8 changes: 3 additions & 5 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ require (
github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d
github.com/alecthomas/assert v0.0.0-20170929043011-405dbfeb8e38
github.com/charmbracelet/glamour v0.2.1-0.20200724174618-1246d13c1684
github.com/cli/safeexec v1.0.0
github.com/dustin/go-humanize v1.0.0
github.com/gdamore/tcell v1.4.0
github.com/google/go-querystring v1.0.0
Expand All @@ -18,6 +17,7 @@ require (
github.com/hashicorp/go-retryablehttp v0.6.4
github.com/hashicorp/go-version v1.2.1
github.com/jarcoal/httpmock v1.0.6
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51
github.com/lunixbochs/vtclean v1.0.0
github.com/mattn/go-colorable v0.1.8
github.com/mattn/go-isatty v0.0.12
Expand All @@ -35,9 +35,7 @@ require (
github.com/tcnksm/go-gitconfig v0.1.2
github.com/tidwall/pretty v1.0.2
github.com/xanzy/go-gitlab v0.39.0
github.com/yuin/goldmark v1.2.1 // indirect
golang.org/x/crypto v0.0.0-20201016220609-9e8e0b390897
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9
golang.org/x/sync v0.0.0-20190423024810-112230192c58
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c
)
9 changes: 2 additions & 7 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,9 @@ github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+Ce
github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs=
github.com/bketelsen/crypt v0.0.3-0.20200106085610-5cbc8cc4026c/go.mod h1:MKsuJmJgSg28kpZDP6UIiPt0e0Oz0kqKNGyRaWEPv84=
github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc=
github.com/charmbracelet/glamour v0.2.1-0.20200724174618-1246d13c1684 h1:YMyvXRstOQc7n6eneHfudVMbARSCmZ7EZGjtTkkeB3A=
github.com/charmbracelet/glamour v0.2.0 h1:mTgaiNiumpqTZp3qVM6DH9UB0NlbY17wejoMf1kM8Pg=
github.com/charmbracelet/glamour v0.2.0/go.mod h1:UA27Kwj3QHialP74iU6C+Gpc8Y7IOAKupeKMLLBURWM=
github.com/charmbracelet/glamour v0.2.1-0.20200724174618-1246d13c1684/go.mod h1:UA27Kwj3QHialP74iU6C+Gpc8Y7IOAKupeKMLLBURWM=
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/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=
github.com/coreos/etcd v3.3.13+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
Expand Down Expand Up @@ -320,8 +319,6 @@ github.com/xanzy/go-gitlab v0.39.0/go.mod h1:sPLojNBn68fMUWSxIJtdVVIP8uSBYqesTfD
github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
github.com/yuin/goldmark v1.2.0 h1:WOOcyaJPlzb8fZ8TloxFe8QZkhOOJx87leDa9MIT9dc=
github.com/yuin/goldmark v1.2.0/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/yuin/goldmark v1.2.1 h1:ruQGxdhGHe7FWOJPT0mKs5+pD2Xs1Bm/kdGlHO04FmM=
github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
go.etcd.io/bbolt v1.3.2/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU=
go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU=
go.opencensus.io v0.22.0/go.mod h1:+kGneAE2xo2IficOXnaByMWTGM9T73dGwxeWcUqIpI8=
Expand Down Expand Up @@ -449,8 +446,6 @@ golang.org/x/tools v0.0.0-20191112195655-aa38f8e97acc/go.mod h1:b+2E5dAYhXwXZwtn
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
google.golang.org/api v0.4.0/go.mod h1:8k5glujaEP+g9n7WNsDg8QP6cUVNI86fCNMcbazEtwE=
google.golang.org/api v0.7.0/go.mod h1:WtwebWUNSVBH/HAw79HIFXZNqEvBhG+Ra+ax0hx3E3M=
google.golang.org/api v0.8.0/go.mod h1:o4eAsZoiT+ibD93RtjEohWalFOjRDx6CVaqeizhEnKg=
Expand Down
5 changes: 2 additions & 3 deletions internal/utils/terminal.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@ import (
"strconv"
"strings"

"github.com/cli/safeexec"

"github.com/mattn/go-isatty"
"github.com/profclems/glab/pkg/execext"
"golang.org/x/crypto/ssh/terminal"
)

Expand Down Expand Up @@ -59,7 +58,7 @@ func TerminalWidth(out io.Writer) int {
}

if isCygwinTerminal(out) {
tputExe, err := safeexec.LookPath("tput")
tputExe, err := execext.LookPath("tput")
if err != nil {
return defaultWidth
}
Expand Down
13 changes: 13 additions & 0 deletions pkg/execext/lp.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// +build !windows

package execext

import "os/exec"

// 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.
// The result may be an absolute path or a path relative to the current directory.
func LookPath(file string) (string, error) {
return exec.LookPath(file)
}
131 changes: 131 additions & 0 deletions pkg/execext/lp_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
package execext

import (
"errors"
"os"
"os/exec"
"path/filepath"
"runtime"
"strings"
"testing"
)

func winExecutable(s string) string {
if runtime.GOOS == "windows" {
return s
}
return ""
}

func TestLookPath(t *testing.T) {
root, wderr := os.Getwd()
if wderr != nil {
t.Fatal(wderr)
}
defaultPath := os.Getenv("PATH")
paths := []string{
filepath.Join(root, "testdata", "nonexist"),
filepath.Join(root, "testdata", "PATH"),
}
os.Setenv("PATH", strings.Join(paths, string(filepath.ListSeparator)))

if err := os.Chdir(filepath.Join(root, "testdata", "cwd")); err != nil {
t.Fatal(err)
}

testCases := []struct {
desc string
pathext string
arg string
wants string
wantErr bool
}{
{
desc: "has extension",
pathext: "",
arg: "git.exe",
wants: filepath.Join(root, "testdata", "PATH", "git.exe"),
wantErr: false,
},
{
desc: "has path",
pathext: "",
arg: filepath.Join("..", "PATH", "git"),
wants: filepath.Join("..", "PATH", "git"+winExecutable(".exe")),
wantErr: false,
},
{
desc: "no extension",
pathext: "",
arg: "git",
wants: filepath.Join(root, "testdata", "PATH", "git"+winExecutable(".exe")),
wantErr: false,
},
{
desc: "has path+extension",
pathext: "",
arg: filepath.Join("..", "PATH", "git.bat"),
wants: filepath.Join("..", "PATH", "git.bat"),
wantErr: false,
},
{
desc: "no extension, PATHEXT",
pathext: ".com;.bat",
arg: "git",
wants: filepath.Join(root, "testdata", "PATH", "git"+winExecutable(".bat")),
wantErr: false,
},
{
desc: "has extension, PATHEXT",
pathext: ".com;.bat",
arg: "git.exe",
wants: filepath.Join(root, "testdata", "PATH", "git.exe"),
wantErr: false,
},
{
desc: "no extension, not found",
pathext: "",
arg: "ls",
wants: "",
wantErr: true,
},
{
desc: "has extension, not found",
pathext: "",
arg: "ls.exe",
wants: "",
wantErr: true,
},
{
desc: "no extension, PATHEXT, not found",
pathext: ".com;.bat",
arg: "ls",
wants: "",
wantErr: true,
},
{
desc: "has extension, PATHEXT, not found",
pathext: ".com;.bat",
arg: "ls.exe",
wants: "",
wantErr: true,
},
}
for _, tC := range testCases {
t.Run(tC.desc, func(t *testing.T) {
os.Setenv("PATHEXT", tC.pathext)
got, err := LookPath(tC.arg)

if tC.wantErr != (err != nil) {
t.Errorf("expects error: %v, got: %v", tC.wantErr, err)
}
if err != nil && !errors.Is(err, exec.ErrNotFound) {
t.Errorf("expected exec.ErrNotFound; got %#v", err)
}
if got != tC.wants {
t.Errorf("expected result %q, got %q", tC.wants, got)
}
})
}
_ = os.Setenv("PATH", defaultPath)
}
120 changes: 120 additions & 0 deletions pkg/execext/lp_windows.go
Original file line number Diff line number Diff line change
@@ -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.

// EXTENDED:
// This package provides an alternative LookPath function to avoid accidentally
// executing matching binaries in the current directory on windows
package execext

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}
}
}
// EXTENDED: this is the only part removed to avoid searching for executables in the
// current directory
// Check 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}
}
Empty file added pkg/execext/testdata/PATH/git
Empty file.
Empty file.
Empty file.
Empty file added pkg/execext/testdata/cwd/git
Empty file.
Empty file.
Empty file.

0 comments on commit 2e8c423

Please sign in to comment.