It's gitfs not gtfs, plus some code fixes and cleanup

Signed-off-by: Pedro Algarvio <palgarvio@vmware.com>

Author: s0undt3ch

changelog/cve-2023-20898.security

@@ -1 +1 @@
-Fixed gtfs cachedir_basename to avoid hash collisions. Added MP Lock to gtfs. These changes should stop race conditions.
+Fixed gitfs cachedir_basename to avoid hash collisions. Added MP Lock to gitfs. These changes should stop race conditions.

salt/utils/gitfs.py

@@ -457,7 +457,7 @@ def __init__(
 
         hash_type = getattr(hashlib, self.opts.get("hash_type", "md5"))
         # Generate full id. The full id is made from these parts name-id-env-_root.
-        # Full id stops collections in the gtfs cache.
+        # Full id stops collections in the gitfs cache.
         self._full_id = "-".join(
             [
                 getattr(self, "name", ""),
@@ -683,9 +683,9 @@ def clear_lock(self, lock_type="update"):
         Clear update.lk
         """
         if self.__class__._master_lock.acquire(timeout=60) is False:
-            # if gtfs works right we should never see this timeout error.
-            log.error("gtfs master lock timeout!")
-            raise TimeoutError("gtfs master lock timeout!")
+            # if gitfs works right we should never see this timeout error.
+            log.error("gitfs master lock timeout!")
+            raise TimeoutError("gitfs master lock timeout!")
         try:
             return self._clear_lock(lock_type)
         finally:
@@ -871,9 +871,9 @@ def _lock(self, lock_type="update", failhard=False):
         Place a lock file if (and only if) it does not already exist.
         """
         if self.__class__._master_lock.acquire(timeout=60) is False:
-            # if gtfs works right we should never see this timeout error.
-            log.error("gtfs master lock timeout!")
-            raise TimeoutError("gtfs master lock timeout!")
+            # if gitfs works right we should never see this timeout error.
+            log.error("gitfs master lock timeout!")
+            raise TimeoutError("gitfs master lock timeout!")
         try:
             return self.__lock(lock_type, failhard)
         finally:

tests/pytests/unit/utils/test_gitfs.py

@@ -1,6 +1,6 @@
 import os
 import string
-from time import time
+import time
 
 import pytest
 import salt.config
@@ -70,14 +70,6 @@ def test_provider_case_insensitive_gitfs_provider(minion_opts, role_name, role_c
             # Now try to instantiate an instance with all lowercase
             # letters. Again, no need for an assert here.
             role_class(*args, **kwargs)
-            pytest.mark.parametrize(
-                "role_name,role_class",
-                (
-                    ("gitfs", salt.utils.gitfs.GitFS),
-                    ("git_pillar", salt.utils.gitfs.GitPillar),
-                    ("winrepo", salt.utils.gitfs.WinRepo),
-                ),
-            )
 
 
 @pytest.mark.parametrize(
@@ -126,7 +118,9 @@ def _prepare_remote_repository_pygit2(tmp_path):
     remote = os.path.join(tmp_path, "pygit2-repo")
     filecontent = "This is an empty README file"
     filename = "README"
-    signature = pygit2.Signature("Dummy Commiter", "dummy@dummy.com", int(time()), 0)
+    signature = pygit2.Signature(
+        "Dummy Commiter", "dummy@dummy.com", int(time.time()), 0
+    )
     repository = pygit2.init_repository(remote, False)
     builder = repository.TreeBuilder()
     tree = builder.write()
@@ -161,10 +155,10 @@ def _prepare_remote_repository_pygit2(tmp_path):
 
 @pytest.fixture
 def _prepare_provider(tmp_path, minion_opts, _prepare_remote_repository_pygit2):
-    cache = os.path.join(tmp_path, "pygit2-repo-cache")
+    cache = tmp_path / "pygit2-repo-cache"
     minion_opts.update(
         {
-            "cachedir": cache,
+            "cachedir": str(cache),
             "gitfs_disable_saltenv_mapping": False,
             "gitfs_base": "master",
             "gitfs_insecure_auth": False,
@@ -210,16 +204,16 @@ def _prepare_provider(tmp_path, minion_opts, _prepare_remote_repository_pygit2):
         "user": "",
     }
     per_remote_only = ("all_saltenvs", "name", "saltenv")
-    override_params = tuple(per_remote_defaults.keys())
-    cache_root = os.path.join(cache, "gitfs")
+    override_params = tuple(per_remote_defaults)
+    cache_root = cache / "gitfs"
     role = "gitfs"
     provider = salt.utils.gitfs.Pygit2(
         minion_opts,
         _prepare_remote_repository_pygit2,
         per_remote_defaults,
         per_remote_only,
         override_params,
-        cache_root,
+        str(cache_root),
         role,
     )
     return provider

tests/unit/utils/test_gitfs.py

@@ -6,7 +6,6 @@
 import tempfile
 
 import pytest
-
 import salt.ext.tornado.ioloop
 import salt.fileserver.gitfs
 import salt.utils.files