diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ff870d0..c430f381 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/src/model_signing/_serialization/serialization.py b/src/model_signing/_serialization/serialization.py index ba47bb43..1ca65cbc 100644 --- a/src/model_signing/_serialization/serialization.py +++ b/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( diff --git a/tests/_serialization/file_test.py b/tests/_serialization/file_test.py index 9659e8e9..63d74308 100644 --- a/tests/_serialization/file_test.py +++ b/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) diff --git a/tests/test_support.py b/tests/test_support.py index 7943610a..f88f4a33 100644 --- a/tests/test_support.py +++ b/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