sbang: respect package permissive package permissions for sbang (#25764)

Co-authored-by: Todd Gamblin <tgamblin@llnl.gov>
Co-authored-by: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com>

lib/spack/spack/hooks/sbang.py

@@ -4,6 +4,7 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
 import filecmp
+import grp
 import os
 import re
 import shutil
@@ -14,7 +15,9 @@
 import llnl.util.filesystem as fs
 import llnl.util.tty as tty
 
+import spack.package_prefs
 import spack.paths
+import spack.spec
 import spack.store
 
 #: OS-imposed character limit for shebang line: 127 for Linux; 511 for Mac.
@@ -187,11 +190,47 @@ def install_sbang():
             spack.paths.sbang_script, sbang_path):
         return
 
-    # make $install_tree/bin and copy in a new version of sbang if needed
+    # make $install_tree/bin
     sbang_bin_dir = os.path.dirname(sbang_path)
     fs.mkdirp(sbang_bin_dir)
-    fs.install(spack.paths.sbang_script, sbang_path)
+
-    fs.set_install_permissions(sbang_bin_dir)
+    # get permissions for bin dir from configuration files
+    group_name = spack.package_prefs.get_package_group(spack.spec.Spec("all"))
+    config_mode = spack.package_prefs.get_package_dir_permissions(
+        spack.spec.Spec("all")
+    )
+
+    if group_name:
+        os.chmod(sbang_bin_dir, config_mode)   # Use package directory permissions
+    else:
+        fs.set_install_permissions(sbang_bin_dir)
+
+    # set group on sbang_bin_dir if not already set (only if set in configuration)
+    if group_name and grp.getgrgid(os.stat(sbang_bin_dir).st_gid).gr_name != group_name:
+        os.chown(
+            sbang_bin_dir,
+            os.stat(sbang_bin_dir).st_uid,
+            grp.getgrnam(group_name).gr_gid
+        )
+
+    # copy over the fresh copy of `sbang`
+    sbang_tmp_path = os.path.join(
+        os.path.dirname(sbang_path),
+        ".%s.tmp" % os.path.basename(sbang_path),
+    )
+    shutil.copy(spack.paths.sbang_script, sbang_tmp_path)
+
+    # set permissions on `sbang` (including group if set in configuration)
+    os.chmod(sbang_tmp_path, config_mode)
+    if group_name:
+        os.chown(
+            sbang_tmp_path,
+            os.stat(sbang_tmp_path).st_uid,
+            grp.getgrnam(group_name).gr_gid
+        )
+
+    # Finally, move the new `sbang` into place atomically
+    os.rename(sbang_tmp_path, sbang_path)
 
 
 def post_install(spec):

lib/spack/spack/test/sbang.py

@@ -7,6 +7,7 @@
 Test that Spack's shebang filtering works correctly.
 """
 import filecmp
+import grp
 import os
 import shutil
 import stat
@@ -19,6 +20,7 @@
 import spack.hooks.sbang as sbang
 import spack.paths
 import spack.store
+import spack.util.spack_yaml as syaml
 from spack.util.executable import which
 
 too_long = sbang.system_shebang_limit + 1
@@ -256,7 +258,34 @@ def test_shebang_handles_non_writable_files(script_dir, sbang_line):
     assert oct(not_writable_mode) == oct(st.st_mode)
 
 
-def check_sbang_installation():
+@pytest.fixture(scope='function')
+def configure_group_perms():
+    conf = syaml.load_config("""\
+all:
+  permissions:
+    read: world
+    write: group
+    group: {0}
+""".format(grp.getgrgid(os.getegid()).gr_name))
+    spack.config.set('packages', conf, scope='user')
+
+    yield
+
+
+@pytest.fixture(scope='function')
+def configure_user_perms():
+    conf = syaml.load_config("""\
+all:
+  permissions:
+    read: world
+    write: user
+""")
+    spack.config.set('packages', conf, scope='user')
+
+    yield
+
+
+def check_sbang_installation(group=False):
     sbang_path = sbang.sbang_install_path()
     sbang_bin_dir = os.path.dirname(sbang_path)
     assert sbang_path.startswith(spack.store.store.unpadded_root)
@@ -264,14 +293,22 @@ def check_sbang_installation():
     assert os.path.exists(sbang_path)
     assert fs.is_exe(sbang_path)
 
-    status = os.stat(sbang_path)
-    assert (status.st_mode & 0o777) == 0o755
-
     status = os.stat(sbang_bin_dir)
-    assert (status.st_mode & 0o777) == 0o755
+    mode = (status.st_mode & 0o777)
+    if group:
+        assert mode == 0o775, 'Unexpected {0}'.format(oct(mode))
+    else:
+        assert mode == 0o755, 'Unexpected {0}'.format(oct(mode))
+
+    status = os.stat(sbang_path)
+    mode = (status.st_mode & 0o777)
+    if group:
+        assert mode == 0o775, 'Unexpected {0}'.format(oct(mode))
+    else:
+        assert mode == 0o755, 'Unexpected {0}'.format(oct(mode))
 
 
-def test_install_sbang(install_mockery):
+def run_test_install_sbang(group):
     sbang_path = sbang.sbang_install_path()
     sbang_bin_dir = os.path.dirname(sbang_path)
 
@@ -279,7 +316,7 @@ def test_install_sbang(install_mockery):
     assert not os.path.exists(sbang_bin_dir)
 
     sbang.install_sbang()
-    check_sbang_installation()
+    check_sbang_installation(group)
 
     # put an invalid file in for sbang
     fs.mkdirp(sbang_bin_dir)
@@ -287,11 +324,19 @@ def test_install_sbang(install_mockery):
         f.write("foo")
 
     sbang.install_sbang()
-    check_sbang_installation()
+    check_sbang_installation(group)
 
     # install again and make sure sbang is still fine
     sbang.install_sbang()
-    check_sbang_installation()
+    check_sbang_installation(group)
+
+
+def test_install_group_sbang(install_mockery, configure_group_perms):
+    run_test_install_sbang(True)
+
+
+def test_install_user_sbang(install_mockery, configure_user_perms):
+    run_test_install_sbang(False)
 
 
 def test_install_sbang_too_long(tmpdir):