From 9adab75ac84b5c2a7b84702fae02c2484abb50ad Mon Sep 17 00:00:00 2001
From: Mahmood Ali <mahmood@hashicorp.com>
Date: Tue, 2 Apr 2019 19:37:30 -0400
Subject: [PATCH 1/3] basic test for #4809

---
 drivers/shared/executor/executor_test.go | 59 ++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/drivers/shared/executor/executor_test.go b/drivers/shared/executor/executor_test.go
index 9d3b4c24bdca..3da4c7b377e5 100644
--- a/drivers/shared/executor/executor_test.go
+++ b/drivers/shared/executor/executor_test.go
@@ -544,3 +544,62 @@ func TestExecutor_Start_Kill_Immediately_WithGrace(pt *testing.T) {
 		})
 	}
 }
+
+// TestExecutor_Start_NonExecutableBinaries asserts that executor marks binary as executable
+// before starting
+func TestExecutor_Start_NonExecutableBinaries(pt *testing.T) {
+	pt.Parallel()
+
+	for name, factory := range executorFactories {
+		pt.Run(name, func(t *testing.T) {
+			require := require.New(t)
+
+			tmpDir, err := ioutil.TempDir("", "nomad-executor-tests")
+			require.NoError(err)
+			defer os.RemoveAll(tmpDir)
+
+			nonExecutablePath := filepath.Join(tmpDir, "nonexecutablefile")
+			ioutil.WriteFile(nonExecutablePath,
+				[]byte("#!/bin/sh\necho hello world"),
+				0600)
+
+			testExecCmd := testExecutorCommand(t)
+			execCmd, allocDir := testExecCmd.command, testExecCmd.allocDir
+			execCmd.Cmd = nonExecutablePath
+			factory.configureExecCmd(t, execCmd)
+
+			executor := factory.new(testlog.HCLogger(t))
+			defer executor.Shutdown("", 0)
+
+			// need to configure path in chroot with that file if using isolation executor
+			if _, ok := executor.(*UniversalExecutor); !ok {
+				taskName := filepath.Base(testExecCmd.command.TaskDir)
+				err := allocDir.NewTaskDir(taskName).Build(true, map[string]string{
+					tmpDir: tmpDir,
+				})
+				require.NoError(err)
+			}
+
+			defer allocDir.Destroy()
+			ps, err := executor.Launch(execCmd)
+			require.NoError(err)
+			require.NotZero(ps.Pid)
+
+			ps, err = executor.Wait(context.Background())
+			require.NoError(err)
+			require.NoError(executor.Shutdown("SIGINT", 100*time.Millisecond))
+
+			expected := "hello world"
+			tu.WaitForResult(func() (bool, error) {
+				act := strings.TrimSpace(string(testExecCmd.stdout.String()))
+				if expected != act {
+					return false, fmt.Errorf("expected: '%s' actual: '%s'", expected, act)
+				}
+				return true, nil
+			}, func(err error) {
+				require.NoError(err)
+			})
+		})
+	}
+
+}

From d441cdd52f68912e96dc7ee5baf2dcddb0ac8caa Mon Sep 17 00:00:00 2001
From: Mahmood Ali <mahmood@hashicorp.com>
Date: Tue, 2 Apr 2019 19:55:44 -0400
Subject: [PATCH 2/3] try not without checking stat first

---
 drivers/shared/executor/executor.go | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/shared/executor/executor.go b/drivers/shared/executor/executor.go
index ced883f58c2c..7eea886a8fe4 100644
--- a/drivers/shared/executor/executor.go
+++ b/drivers/shared/executor/executor.go
@@ -549,12 +549,6 @@ func (e *UniversalExecutor) handleStats(ch chan *cstructs.TaskResourceUsage, ctx
 // the following locations, in-order: task/local/, task/, based on host $PATH.
 // The return path is absolute.
 func lookupBin(taskDir string, bin string) (string, error) {
-	// Check the binary path first
-	// This handles the case where the job spec sends a fully interpolated path to the binary
-	if _, err := os.Stat(bin); err == nil {
-		return bin, nil
-	}
-
 	// Check in the local directory
 	local := filepath.Join(taskDir, allocdir.TaskLocal, bin)
 	if _, err := os.Stat(local); err == nil {

From 244544b735a8dbac0e14e7ee85e12a39780cbbb9 Mon Sep 17 00:00:00 2001
From: Mahmood Ali <mahmood@hashicorp.com>
Date: Tue, 2 Apr 2019 20:00:54 -0400
Subject: [PATCH 3/3] an alternative order

---
 drivers/shared/executor/executor.go | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/shared/executor/executor.go b/drivers/shared/executor/executor.go
index 7eea886a8fe4..699d3247ae65 100644
--- a/drivers/shared/executor/executor.go
+++ b/drivers/shared/executor/executor.go
@@ -561,6 +561,14 @@ func lookupBin(taskDir string, bin string) (string, error) {
 		return root, nil
 	}
 
+	// when checking host paths, check with Stat first if path is absolute
+	// as exec.LookPath only considers files already marked as executable
+	// and only consider this for absolute paths to avoid depending on
+	// current directory of nomad which may cause unexpected behavior
+	if _, err := os.Stat(bin); err == nil && filepath.IsAbs(bin) {
+		return bin, nil
+	}
+
 	// Check the $PATH
 	if host, err := exec.LookPath(bin); err == nil {
 		return host, nil