From e1cc28a30a21da9d17967bc21e93d94c7fa03466 Mon Sep 17 00:00:00 2001 From: Paul Spencer <32964672+pwablito@users.noreply.github.com> Date: Sat, 18 Dec 2021 23:07:20 -0700 Subject: sbang: respect package permissive package permissions for sbang (#25764) Co-authored-by: Todd Gamblin Co-authored-by: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com> --- lib/spack/spack/hooks/sbang.py | 45 ++++++++++++++++++++++++++++-- lib/spack/spack/test/sbang.py | 63 ++++++++++++++++++++++++++++++++++++------ 2 files changed, 96 insertions(+), 12 deletions(-) diff --git a/lib/spack/spack/hooks/sbang.py b/lib/spack/spack/hooks/sbang.py index ce2cf3435d..179416f03c 100644 --- a/lib/spack/spack/hooks/sbang.py +++ b/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 tempfile 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): diff --git a/lib/spack/spack/test/sbang.py b/lib/spack/spack/test/sbang.py index 6d26bd1ba8..6e0d00eba0 100644 --- a/lib/spack/spack/test/sbang.py +++ b/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 llnl.util.filesystem as fs 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): -- cgit v1.2.3-60-g2f50