# HG changeset patch # User RhodeCode Admin # Date 2023-12-11 19:13:48 # Node ID 652285bbf5f4e5b480558028f22f70543eeb6b29 # Parent 729a204e11db4288785f15bd9f894e7efc15a910 fix(hooks): make compat for WSL filesystem better by explicity only changing permissions if they are lower at creation time. diff --git a/vcsserver/hook_utils/__init__.py b/vcsserver/hook_utils/__init__.py --- a/vcsserver/hook_utils/__init__.py +++ b/vcsserver/hook_utils/__init__.py @@ -27,6 +27,19 @@ from vcsserver.str_utils import safe_byt log = logging.getLogger(__name__) +HOOKS_DIR_MODE = 0o755 +HOOKS_FILE_MODE = 0o755 + + +def set_permissions_if_needed(path_to_check, perms: oct): + # Get current permissions + current_permissions = os.stat(path_to_check).st_mode & 0o777 # Extract permission bits + + # Check if current permissions are lower than required + if current_permissions < int(perms): + # Change the permissions if they are lower than required + os.chmod(path_to_check, perms) + def get_git_hooks_path(repo_path, bare): hooks_path = os.path.join(repo_path, 'hooks') @@ -51,10 +64,9 @@ def install_git_hooks(repo_path, bare, e # we always call it to ensure dir exists and it has a proper mode if not os.path.exists(hooks_path): # If it doesn't exist, create a new directory with the specified mode - os.makedirs(hooks_path, mode=0o777, exist_ok=True) - else: - # If it exists, change the directory's mode to the specified mode - os.chmod(hooks_path, mode=0o777) + os.makedirs(hooks_path, mode=HOOKS_DIR_MODE, exist_ok=True) + # If it exists, change the directory's mode to the specified mode + set_permissions_if_needed(hooks_path, perms=HOOKS_DIR_MODE) tmpl_post = pkg_resources.resource_string( 'vcsserver', '/'.join( @@ -80,7 +92,7 @@ def install_git_hooks(repo_path, bare, e template = template.replace(b'_ENV_', safe_bytes(executable)) template = template.replace(b'_PATH_', safe_bytes(path)) f.write(template) - os.chmod(_hook_file, 0o755) + set_permissions_if_needed(_hook_file, perms=HOOKS_FILE_MODE) except OSError: log.exception('error writing hook file %s', _hook_file) else: diff --git a/vcsserver/tests/test_install_hooks.py b/vcsserver/tests/test_install_hooks.py --- a/vcsserver/tests/test_install_hooks.py +++ b/vcsserver/tests/test_install_hooks.py @@ -22,8 +22,9 @@ import pytest import vcsserver import tempfile from vcsserver import hook_utils +from vcsserver.hook_utils import set_permissions_if_needed, HOOKS_DIR_MODE, HOOKS_FILE_MODE from vcsserver.tests.fixture import no_newline_id_generator -from vcsserver.str_utils import safe_bytes, safe_str +from vcsserver.str_utils import safe_bytes from vcsserver.utils import AttributeDict @@ -58,13 +59,22 @@ class TestCheckRhodecodeHook: class BaseInstallHooks: HOOK_FILES = () + def _check_hook_file_dir_mode(self, file_path): + dir_path = os.path.dirname(file_path) + assert os.path.exists(dir_path), f'dir {file_path} missing' + stat_info = os.stat(dir_path) + + file_mode = stat.S_IMODE(stat_info.st_mode) + expected_mode = int(HOOKS_DIR_MODE) + assert expected_mode == file_mode, f'expected mode: {oct(expected_mode)} got: {oct(file_mode)} for {dir_path}' + def _check_hook_file_mode(self, file_path): assert os.path.exists(file_path), f'path {file_path} missing' stat_info = os.stat(file_path) file_mode = stat.S_IMODE(stat_info.st_mode) - expected_mode = int('755', 8) - assert expected_mode == file_mode + expected_mode = int(HOOKS_FILE_MODE) + assert expected_mode == file_mode, f'expected mode: {oct(expected_mode)} got: {oct(file_mode)} for {file_path}' def _check_hook_file_content(self, file_path, executable): executable = executable or sys.executable @@ -102,6 +112,8 @@ class BaseInstallHooks: file_path = os.path.join(repo_path, 'hooks', file_name) else: file_path = os.path.join(repo_path, '.git', 'hooks', file_name) + + self._check_hook_file_dir_mode(file_path) self._check_hook_file_mode(file_path) self._check_hook_file_content(file_path, sys.executable) @@ -204,3 +216,74 @@ class TestInstallSvnHooks(BaseInstallHoo repo.path, force_create=True) assert result self.check_hooks(repo.path, ) + + +def create_test_file(filename): + """Utility function to create a test file.""" + with open(filename, 'w') as f: + f.write("Test file") + + +def remove_test_file(filename): + """Utility function to remove a test file.""" + if os.path.exists(filename): + os.remove(filename) + + +@pytest.fixture +def test_file(): + filename = 'test_file.txt' + create_test_file(filename) + yield filename + remove_test_file(filename) + + +def test_increase_permissions(test_file): + # Set initial lower permissions + initial_perms = 0o644 + os.chmod(test_file, initial_perms) + + # Set higher permissions + new_perms = 0o666 + set_permissions_if_needed(test_file, new_perms) + + # Check if permissions were updated + assert (os.stat(test_file).st_mode & 0o777) == new_perms + + +def test_no_permission_change_needed(test_file): + # Set initial permissions + initial_perms = 0o666 + os.chmod(test_file, initial_perms) + + # Attempt to set the same permissions + set_permissions_if_needed(test_file, initial_perms) + + # Check if permissions were unchanged + assert (os.stat(test_file).st_mode & 0o777) == initial_perms + + +def test_no_permission_reduction(test_file): + # Set initial higher permissions + initial_perms = 0o666 + os.chmod(test_file, initial_perms) + + # Attempt to set lower permissions + lower_perms = 0o644 + set_permissions_if_needed(test_file, lower_perms) + + # Check if permissions were not reduced + assert (os.stat(test_file).st_mode & 0o777) == initial_perms + + +def test_no_permission_reduction_when_on_777(test_file): + # Set initial higher permissions + initial_perms = 0o777 + os.chmod(test_file, initial_perms) + + # Attempt to set lower permissions + lower_perms = 0o755 + set_permissions_if_needed(test_file, lower_perms) + + # Check if permissions were not reduced + assert (os.stat(test_file).st_mode & 0o777) == initial_perms