Shorten long shebangs only if the execute permission is set (#26899)

The OS should only interpret shebangs, if a file is executable. 

Thus, there should be no need to modify files where no execute bit is set. 

This solves issues that are e.g. encountered while packaging software as 
COVISE (https://github.com/hlrs-vis/covise), which includes example data
in Tecplot format. The sbang post-install hook is applied to every installed
file that starts with the two characters #!, but this fails on the binary Tecplot
files, as they happen to start with #!TDV. Decoding them with UTF-8 fails 
and an exception is thrown during post_install.

Co-authored-by: Martin Aumüller <aumuell@reserv.at>

2 files changed, 54 insertions, 0 deletions

diff --git a/lib/spack/spack/hooks/sbang.py b/lib/spack/spack/hooks/sbang.py
index 33aa29691f..8b99ee7873 100644
--- a/lib/spack/spack/hooks/sbang.py
+++ b/lib/spack/spack/hooks/sbang.py
@@ -126,6 +126,11 @@ def filter_shebangs_in_directory(directory, filenames=None):
         if not os.path.isfile(path):
             continue
 
+        # only handle executable files
+        st = os.stat(path)
+        if not st.st_mode & (stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH):
+            continue
+
         # only handle links that resolve within THIS package's prefix.
         if os.path.islink(path):
             real_path = os.path.realpath(path)
diff --git a/lib/spack/spack/test/sbang.py b/lib/spack/spack/test/sbang.py
index e0d7a8ccf3..4eb3b07a60 100644
--- a/lib/spack/spack/test/sbang.py
+++ b/lib/spack/spack/test/sbang.py
@@ -62,18 +62,27 @@ class ScriptDirectory(object):
         with open(self.short_shebang, 'w') as f:
             f.write(short_line)
             f.write(last_line)
+        self.make_executable(self.short_shebang)
 
         # Script with long shebang
         self.long_shebang = os.path.join(self.tempdir, 'long')
         with open(self.long_shebang, 'w') as f:
             f.write(long_line)
             f.write(last_line)
+        self.make_executable(self.long_shebang)
+
+        # Non-executable script with long shebang
+        self.nonexec_long_shebang = os.path.join(self.tempdir, 'nonexec_long')
+        with open(self.nonexec_long_shebang, 'w') as f:
+            f.write(long_line)
+            f.write(last_line)
 
         # Lua script with long shebang
         self.lua_shebang = os.path.join(self.tempdir, 'lua')
         with open(self.lua_shebang, 'w') as f:
             f.write(lua_line)
             f.write(last_line)
+        self.make_executable(self.lua_shebang)
 
         # Lua script with long shebang
         self.lua_textbang = os.path.join(self.tempdir, 'lua_in_text')
@@ -81,12 +90,14 @@ class ScriptDirectory(object):
             f.write(short_line)
             f.write(lua_in_text)
             f.write(last_line)
+        self.make_executable(self.lua_textbang)
 
         # Node script with long shebang
         self.node_shebang = os.path.join(self.tempdir, 'node')
         with open(self.node_shebang, 'w') as f:
             f.write(node_line)
             f.write(last_line)
+        self.make_executable(self.node_shebang)
 
         # Node script with long shebang
         self.node_textbang = os.path.join(self.tempdir, 'node_in_text')
@@ -94,12 +105,14 @@ class ScriptDirectory(object):
             f.write(short_line)
             f.write(node_in_text)
             f.write(last_line)
+        self.make_executable(self.node_textbang)
 
         # php script with long shebang
         self.php_shebang = os.path.join(self.tempdir, 'php')
         with open(self.php_shebang, 'w') as f:
             f.write(php_line)
             f.write(last_line)
+        self.make_executable(self.php_shebang)
 
         # php script with long shebang
         self.php_textbang = os.path.join(self.tempdir, 'php_in_text')
@@ -107,6 +120,7 @@ class ScriptDirectory(object):
             f.write(short_line)
             f.write(php_in_text)
             f.write(last_line)
+        self.make_executable(self.php_textbang)
 
         # Script already using sbang.
         self.has_sbang = os.path.join(self.tempdir, 'shebang')
@@ -114,15 +128,27 @@ class ScriptDirectory(object):
             f.write(sbang_line)
             f.write(long_line)
             f.write(last_line)
+        self.make_executable(self.has_sbang)
 
         # Fake binary file.
         self.binary = os.path.join(self.tempdir, 'binary')
         tar = which('tar', required=True)
         tar('czf', self.binary, self.has_sbang)
+        self.make_executable(self.binary)
 
     def destroy(self):
         shutil.rmtree(self.tempdir, ignore_errors=True)
 
+    def make_executable(self, path):
+        # make a file executable
+        st = os.stat(path)
+        executable_mode = st.st_mode \
+            | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH
+        os.chmod(path, executable_mode)
+
+        st = os.stat(path)
+        assert oct(executable_mode) == oct(st.st_mode & executable_mode)
+
 
 @pytest.fixture
 def script_dir(sbang_line):
@@ -134,6 +160,7 @@ def script_dir(sbang_line):
 def test_shebang_handling(script_dir, sbang_line):
     assert sbang.shebang_too_long(script_dir.lua_shebang)
     assert sbang.shebang_too_long(script_dir.long_shebang)
+    assert sbang.shebang_too_long(script_dir.nonexec_long_shebang)
 
     assert not sbang.shebang_too_long(script_dir.short_shebang)
     assert not sbang.shebang_too_long(script_dir.has_sbang)
@@ -153,6 +180,11 @@ def test_shebang_handling(script_dir, sbang_line):
         assert f.readline() == long_line
         assert f.readline() == last_line
 
+    # Make sure this is untouched
+    with open(script_dir.nonexec_long_shebang, 'r') as f:
+        assert f.readline() == long_line
+        assert f.readline() == last_line
+
     # Make sure this got patched.
     with open(script_dir.lua_shebang, 'r') as f:
         assert f.readline() == sbang_line
@@ -243,3 +275,20 @@ def test_install_sbang_too_long(tmpdir):
     assert 'root is too long' in err
     assert 'exceeds limit' in err
     assert 'cannot patch' in err
+
+
+def test_sbang_hook_skips_nonexecutable_blobs(tmpdir):
+    # Write a binary blob to non-executable.sh, with a long interpreter "path"
+    # consisting of invalid UTF-8. The latter is technically not really necessary for
+    # the test, but binary blobs accidentally starting with b'#!' usually do not contain
+    # valid UTF-8, so we also ensure that Spack does not attempt to decode as UTF-8.
+    contents = b'#!' + b'\x80' * sbang.shebang_limit
+    file = str(tmpdir.join('non-executable.sh'))
+    with open(file, 'wb') as f:
+        f.write(contents)
+
+    sbang.filter_shebangs_in_directory(str(tmpdir))
+
+    # Make sure there is no sbang shebang.
+    with open(file, 'rb') as f:
+        assert b'sbang' not in f.readline()