From 43c415e424137547d0cc896f10d6b5d6d7b4542d Mon Sep 17 00:00:00 2001
From: David Trudgian <david.trudgian@sylabs.io>
Date: Mon, 4 Apr 2022 08:23:07 -0500
Subject: [PATCH] fix: Show informative error for non-executable exec target

When the target executable of `singularity exec` lacks `+x`
permissions, show a sensible error message rather than `permission
denied`.

Fixes #607
---
 CHANGELOG.md                                 |  1 +
 e2e/actions/actions.go                       | 20 ++++++++++++++++----
 internal/pkg/util/fs/files/action_scripts.go |  9 ++++++++-
 3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 95c22e048b..ccded11b28 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -47,6 +47,7 @@
 
 - Support nvidia-container-cli v1.8.0 and above, via fix to capability set.
 - Do not truncate environment variables with commas
+- Show an informative error when `exec` target is not executable.
 
 ## v3.9.6 \[2022-03-10\]
 
diff --git a/e2e/actions/actions.go b/e2e/actions/actions.go
index 45cb385af3..b55b2db3fa 100644
--- a/e2e/actions/actions.go
+++ b/e2e/actions/actions.go
@@ -109,9 +109,10 @@ func (c actionTests) actionExec(t *testing.T) {
 	homePath := filepath.Join("/home", basename)
 
 	tests := []struct {
-		name string
-		argv []string
-		exit int
+		name          string
+		argv          []string
+		exit          int
+		errorContains string
 	}{
 		{
 			name: "NoCommand",
@@ -250,9 +251,20 @@ func (c actionTests) actionExec(t *testing.T) {
 			argv: []string{"--no-home", c.env.ImagePath, "ls", "-ld", user.Dir},
 			exit: 1,
 		},
+		{
+			name:          "NotExecutable",
+			argv:          []string{c.env.ImagePath, "/etc/hosts"},
+			exit:          1,
+			errorContains: "/etc/hosts is not an executable file in the container. Check it exists and has executable permissions.",
+		},
 	}
 
 	for _, tt := range tests {
+		resultOps := []e2e.SingularityCmdResultOp{}
+		if tt.errorContains != "" {
+			resultOps = append(resultOps, e2e.ExpectError(e2e.ContainMatch, tt.errorContains))
+		}
+
 		c.env.RunSingularity(
 			t,
 			e2e.AsSubtest(tt.name),
@@ -260,7 +272,7 @@ func (c actionTests) actionExec(t *testing.T) {
 			e2e.WithCommand("exec"),
 			e2e.WithDir("/tmp"),
 			e2e.WithArgs(tt.argv...),
-			e2e.ExpectExit(tt.exit),
+			e2e.ExpectExit(tt.exit, resultOps...),
 		)
 	}
 }
diff --git a/internal/pkg/util/fs/files/action_scripts.go b/internal/pkg/util/fs/files/action_scripts.go
index 297441a3dc..5924b82547 100644
--- a/internal/pkg/util/fs/files/action_scripts.go
+++ b/internal/pkg/util/fs/files/action_scripts.go
@@ -157,7 +157,14 @@ sylog debug "Running action command ${__singularity_cmd__}"
 
 case "${__singularity_cmd__}" in
 exec)
-    exec "$@" ;;
+    # This uses the limited type -p behavior of mvdan.cc/sh and is *not* POSIX.
+    execbin=$(type -p "$1")
+    if test $? -ne 0 || test -z "${execbin}" ; then
+        sylog error "$1 is not an executable file in the container. Check it exists and has executable permissions."
+        exit 1
+    fi
+    exec "$@"   
+    ;;
 shell)
     if test -n "${SINGULARITY_SHELL:-}" -a -x "${SINGULARITY_SHELL:-}"; then
         exec "${SINGULARITY_SHELL:-}" "$@"