raise instead of skipping model directories that cannot be listed

CHANGELOG.md

@@ -17,6 +17,7 @@ All versions prior to 1.0.0 are untracked.
 - Standardized CLI flags to use hyphens (e.g., `--trust-config` instead of `--trust_config`). Underscore variants are still accepted for backwards compatibility via token normalization.
 
 ### Fixed
+- Fixed a bug where a directory that could not be listed was skipped during serialization instead of raising. Every file under it was left out of the manifest, so a model carrying such a directory verified successfully even though its contents were never hashed. ([#652](https://github.com/sigstore/model-transparency/pull/652))
 - Fixed a bug where reusing a single `verifying.Config` across models let the ignore paths and guessed hashing configuration from one verification carry over into later ones. A file that a later model's signature never excluded could be silently skipped instead of reported as unsigned. ([#650](https://github.com/sigstore/model-transparency/pull/650))
 - Fixed a bug where installing from the sdist produced an empty wheel with zero Python modules. The hatch `packages` directive was scoped to all build targets instead of the wheel target only, causing the sdist's flattened layout to not match the expected `src/` path. ([#636](https://github.com/sigstore/model-transparency/issues/636))
 - 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))

src/model_signing/_serialization/serialization.py

@@ -16,6 +16,7 @@
 
 import abc
 from collections.abc import Iterable
+import os
 import pathlib
 
 from model_signing import manifest
@@ -39,8 +40,9 @@ def check_file_or_directory(
           serialization would raise an error.
 
     Raises:
-        ValueError: The path is neither a file or a directory, or the path
-          is a symlink and `allow_symlinks` is false.
+        ValueError: The path is neither a file or a directory, the path is a
+          symlink and `allow_symlinks` is false, or the path is a directory
+          whose entries cannot be listed.
     """
     if not allow_symlinks and path.is_symlink():
         raise ValueError(
@@ -53,6 +55,19 @@ def check_file_or_directory(
             " special file, it could be missing, or there might be a"
             " permission issue."
         )
+    if path.is_dir():
+        # Listing a directory needs read permission, while `is_dir` only needs
+        # search permission on the parent. `glob` drops a subtree it cannot
+        # list without reporting it, so check here instead of silently leaving
+        # the files it holds out of the manifest.
+        try:
+            with os.scandir(path):
+                pass
+        except OSError as err:
+            raise ValueError(
+                f"Cannot list the contents of '{path}'. Any file under it"
+                " would be missing from the manifest."
+            ) from err
 
 
 def should_ignore(

tests/_serialization/file_test.py

@@ -326,6 +326,20 @@ def test_ignored_symlinks_dont_raise_error(self, symlink_model_folder):
             symlink_model_folder, ignore_paths=[symlink_model_folder]
         )
 
+    def test_unlistable_directory_raises(self, sample_model_folder):
+        hidden_dir = sample_model_folder / "hidden"
+        hidden_dir.mkdir()
+        (hidden_dir / "extra_file").write_bytes(b"not in the manifest")
+        if not test_support.make_unlistable(hidden_dir):
+            return  # trivially pass where permissions cannot restrict listing
+
+        serializer = file.Serializer(self._hasher_factory)
+        try:
+            with pytest.raises(ValueError, match="Cannot list the contents of"):
+                _ = serializer.serialize(sample_model_folder)
+        finally:
+            hidden_dir.chmod(0o755)
+
 
 class TestUtilities:
     def test_check_file_or_directory_raises_on_pipes(self, sample_model_file):
@@ -341,3 +355,17 @@ def test_check_file_or_directory_raises_on_pipes(self, sample_model_file):
             ValueError, match="Cannot use .* as file or directory"
         ):
             serialization.check_file_or_directory(pipe)
+
+    def test_check_file_or_directory_raises_on_unlistable_directory(
+        self, sample_model_folder
+    ):
+        directory = sample_model_folder / "unlistable"
+        directory.mkdir()
+        if not test_support.make_unlistable(directory):
+            return  # trivially pass where permissions cannot restrict listing
+
+        try:
+            with pytest.raises(ValueError, match="Cannot list the contents of"):
+                serialization.check_file_or_directory(directory)
+        finally:
+            directory.chmod(0o755)

tests/test_support.py

@@ -15,6 +15,7 @@
 """Helpers and constants used in fixtures and tests. Not in the public API."""
 
 import itertools
+import os
 import pathlib
 
 from model_signing import manifest
@@ -113,3 +114,20 @@ def count_files(path: pathlib.Path) -> int:
         if child_path.is_file():
             count += 1
     return count
+
+
+def make_unlistable(path: pathlib.Path) -> bool:
+    """Drops read permission on a directory, keeping it searchable.
+
+    Returns whether the directory actually became unlistable. Windows does not
+    map this onto file modes, and a user with CAP_DAC_READ_SEARCH (such as
+    root) bypasses the check, so callers must skip when this returns False.
+    """
+    path.chmod(0o111)
+    try:
+        with os.scandir(path):
+            pass
+    except OSError:
+        return True
+    path.chmod(0o755)
+    return False