fix(hooks): make compat for WSL filesystem better by explicity only changing permissions if they are lower at creation time.

vcsserver/hook_utils/__init__.py

@@ -27,6 +27,19 b' 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 b' 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:
+        os.makedirs(hooks_path, mode=HOOKS_DIR_MODE, exist_ok=True)
         # If it exists, change the directory's mode to the specified mode
-        os.chmod(hooks_path, mode=0o777)
+    set_permissions_if_needed(hooks_path, perms=HOOKS_DIR_MODE)
 
     tmpl_post = pkg_resources.resource_string(
         'vcsserver', '/'.join(
@@ -80,7 +92,7 b' 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:

vcsserver/tests/test_install_hooks.py

@@ -22,8 +22,9 b' 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 b' 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 b' 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 b' 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)
+
+
[e-mail]
+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