serialization: skip ignored files before checking for symlinks (#550)

Ignored files aren't serialized, so they shouldn't raise a ValueError if
allow_symlinks wasn't set.

Signed-off-by: Spencer Schrock <[email protected]>

Author: spencerschrock

CHANGELOG.md

@@ -16,7 +16,7 @@ All versions prior to 1.0.0 are untracked.
 - ...
 
 ### Fixed
-- ...
+- Fixed a bug where ignored symlinks could raise `ValueError`s if allow_symlinks was unset, even though they were skipped during serialization. ([#550](https://github.com/sigstore/model-transparency/pull/550))
 
 ### Removed
 - ...

src/model_signing/_serialization/file.py

@@ -111,12 +111,12 @@ def serialize(
                 (model_path,), model_path.glob("**/*")
             )
         for path in files_to_hash:
+            if serialization.should_ignore(path, ignore_paths):
+                continue
             serialization.check_file_or_directory(
                 path, allow_symlinks=self._allow_symlinks
             )
-            if path.is_file() and not serialization.should_ignore(
-                path, ignore_paths
-            ):
+            if path.is_file():
                 paths.append(path)
 
         manifest_items = []

src/model_signing/_serialization/file_shard.py

@@ -145,12 +145,12 @@ def serialize(
                 (model_path,), model_path.glob("**/*")
             )
         for path in files_to_hash:
+            if serialization.should_ignore(path, ignore_paths):
+                continue
             serialization.check_file_or_directory(
                 path, allow_symlinks=self._allow_symlinks
             )
-            if path.is_file() and not serialization.should_ignore(
-                path, ignore_paths
-            ):
+            if path.is_file():
                 shards.extend(self._get_shards(path))
 
         manifest_items = []

tests/_serialization/file_shard_test.py

@@ -387,3 +387,9 @@ def test_ignore_list_respects_directories(self, sample_model_folder):
         )
         assert manifest1 != manifest2
         assert len(manifest1._item_to_digest) > len(manifest2._item_to_digest)
+
+    def test_ignored_symlinks_dont_raise_error(self, symlink_model_folder):
+        serializer = file_shard.Serializer(self._hasher_factory)
+        _ = serializer.serialize(
+            symlink_model_folder, ignore_paths=[symlink_model_folder]
+        )

tests/_serialization/file_test.py

@@ -320,6 +320,12 @@ def test_ignore_list_respects_directories(self, sample_model_folder):
         diff = len(manifest1._item_to_digest) - len(manifest2._item_to_digest)
         assert diff == ignored_file_count
 
+    def test_ignored_symlinks_dont_raise_error(self, symlink_model_folder):
+        serializer = file.Serializer(self._hasher_factory)
+        _ = serializer.serialize(
+            symlink_model_folder, ignore_paths=[symlink_model_folder]
+        )
+
 
 class TestUtilities:
     def test_check_file_or_directory_raises_on_pipes(self, sample_model_file):