record zero-length files in the shard serialization manifest

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 shard serialization left zero-length files out of the manifest entirely, so a signature did not cover them and adding, removing or filling in an empty file did not invalidate it. Empty files now get one empty shard, matching file serialization. Shard signatures over models that contain empty files need to be regenerated. ([#653](https://github.com/sigstore/model-transparency/pull/653))
 - 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/_hashing/io.py

@@ -219,9 +219,10 @@ def __init__(
               digest of the file shard.
             start: The file offset to start reading from. Must be valid. Reset
               with `set_shard`.
-            end: The file offset to stop reading at. Must be stricly greater
-              than start. The entire shard length must be less than the
-              configured `shard_size`. Reset with `set_shard`.
+            end: The file offset to stop reading at. Must not be lower than
+              start; equal to start means an empty shard, as used for an empty
+              file. The entire shard length must be less than the configured
+              `shard_size`. Reset with `set_shard`.
             chunk_size: The amount of file to read at once. Default is 1MB. A
               special value of 0 signals to attempt to read everything in a
               single call.
@@ -249,18 +250,19 @@ def set_shard(self, *, start: int, end: int) -> None:
 
         Args:
             start: The file offset to start reading from. Must be valid.
-            end: The file offset to stop reading at. Must be stricly greater
-              than start. The entire shard length must be less than the
-              configured `shard_size`.
+            end: The file offset to stop reading at. Must not be lower than
+              start; equal to start means an empty shard, as used for an empty
+              file. The entire shard length must be less than the configured
+              `shard_size`.
         """
         if start < 0:
             raise ValueError(
                 f"File start offset must be non-negative, got {start}."
             )
-        if end <= start:
+        if end < start:
             raise ValueError(
-                "File end offset must be stricly higher that file start offset,"
-                f" got {start=}, {end=}."
+                "File end offset must not be lower than file start offset, got"
+                f" {start=}, {end=}."
             )
         read_length = end - start
         if read_length > self.shard_size:

src/model_signing/_serialization/file_shard.py

@@ -41,6 +41,8 @@
     [2, 4, 6, 8, 9]
     >>> list(_endpoints(2, 2))
     [2]
+    >>> list(_endpoints(2, 0))
+    [0]
 
     Yields:
         Values in the range, from `step` and up to `end`.
@@ -182,7 +184,7 @@
 
         model_name = model_path.name
         if not model_name or model_name == "..":
             model_name = os.path.basename(model_path.resolve())
 
         return manifest.Manifest(
             model_name, manifest_items, self._serialization_description
@@ -191,14 +193,17 @@
     def _get_shards(
         self, path: pathlib.Path
     ) -> list[tuple[pathlib.Path, int, int]]:
-        """Determines the shards of a given file path."""
+        """Determines the shards of a given file path.
+
+        An empty file gets a single empty shard, so that it is still recorded in
+        the manifest and covered by the signature.
+        """
         shards = []
         path_size = path.stat().st_size
-        if path_size > 0:
-            start = 0
-            for end in _endpoints(self._shard_size, path_size):
-                shards.append((path, start, end))
-                start = end
+        start = 0
+        for end in _endpoints(self._shard_size, path_size):
+            shards.append((path, start, end))
+            start = end
         return shards
 
     def _compute_hash(

tests/_hashing/io_hashing_test.py

@@ -42,6 +42,13 @@ def sample_file_content_only(tmp_path_factory):
     return file_path
 
 
+@pytest.fixture(scope="class")
+def empty_file(tmp_path_factory):
+    file_path = tmp_path_factory.mktemp("dir") / "empty.txt"
+    file_path.write_bytes(b"")
+    return file_path
+
+
 @pytest.fixture(scope="class")
 def expected_digest():
     # To ensure that the expected file digest is always up to date, use the
@@ -182,9 +189,7 @@ def test_set_fails_with_negative_start(self):
     def test_fails_with_end_lower_than_start(self):
         with pytest.raises(
             ValueError,
-            match=(
-                "File end offset must be stricly higher that file start offset"
-            ),
+            match="File end offset must not be lower than file start offset",
         ):
             io.ShardedFileHasher(_UNUSED_PATH, memory.SHA256(), start=42, end=2)
 
@@ -194,34 +199,29 @@ def test_set_fails_with_end_lower_than_start(self):
         )
         with pytest.raises(
             ValueError,
-            match=(
-                "File end offset must be stricly higher that file start offset"
-            ),
+            match="File end offset must not be lower than file start offset",
         ):
             hasher.set_shard(start=42, end=2)
 
-    def test_fails_with_zero_read_span(self):
-        with pytest.raises(
-            ValueError,
-            match=(
-                "File end offset must be stricly higher that file start offset"
-            ),
-        ):
-            io.ShardedFileHasher(
-                _UNUSED_PATH, memory.SHA256(), start=42, end=42
-            )
+    def test_hash_of_empty_file(self, empty_file):
+        hasher = io.ShardedFileHasher(
+            empty_file, memory.SHA256(), start=0, end=0
+        )
+        expected = memory.SHA256(b"").compute()
 
-    def test_set_fails_with_zero_read_span(self):
+        digest = hasher.compute()
+
+        assert digest.digest_value == expected.digest_value
+
+    def test_set_allows_zero_read_span(self, sample_file):
         hasher = io.ShardedFileHasher(
-            _UNUSED_PATH, memory.SHA256(), start=0, end=42
+            sample_file, memory.SHA256(), start=0, end=_SHARD_SIZE
         )
-        with pytest.raises(
-            ValueError,
-            match=(
-                "File end offset must be stricly higher that file start offset"
-            ),
-        ):
-            hasher.set_shard(start=42, end=42)
+        expected = memory.SHA256(b"").compute()
+
+        hasher.set_shard(start=_SHARD_SIZE, end=_SHARD_SIZE)
+
+        assert hasher.compute().digest_value == expected.digest_value
 
     def test_fails_with_read_span_too_large(self):
         with pytest.raises(

tests/_serialization/file_shard_test.py

@@ -197,7 +197,7 @@ def test_folder_model_empty_folder_not_included(self, sample_model_folder):
 
         assert manifest == new_manifest
 
-    def test_folder_model_empty_file_not_included(self, sample_model_folder):
+    def test_folder_model_empty_file_gets_included(self, sample_model_folder):
         serializer = file_shard.Serializer(self._hasher_factory)
         manifest = serializer.serialize(sample_model_folder)
 
@@ -206,7 +206,13 @@ def test_folder_model_empty_file_not_included(self, sample_model_folder):
         new_empty_file.write_text("")
         new_manifest = serializer.serialize(sample_model_folder)
 
-        assert manifest == new_manifest
+        assert manifest != new_manifest
+        assert (
+            len(new_manifest._item_to_digest)
+            == len(manifest._item_to_digest) + 1
+        )
+        for shard, digest in manifest._item_to_digest.items():
+            assert new_manifest._item_to_digest[shard] == digest
 
     def _check_manifests_match_except_on_renamed_file(
         self,

tests/_serialization/testdata/file_shard/TestSerializer/empty_model_file

@@ -0,0 +1 @@
+.:0:0:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855

tests/_serialization/testdata/file_shard/TestSerializer/empty_model_file_small_shards

@@ -0,0 +1 @@
+.:0:0:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855

tests/_serialization/testdata/file_shard/TestSerializer/model_folder_with_empty_file

@@ -0,0 +1 @@
+empty_file:0:0:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855

tests/_serialization/testdata/file_shard/TestSerializer/model_folder_with_empty_file_small_shards

@@ -0,0 +1 @@
+empty_file:0:0:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855

tests/_signing/testdata/signing/TestPayload/empty_model_file_shard

@@ -4,7 +4,7 @@
     {
       "name": "empty_file_model",
       "digest": {
-        "sha256": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
+        "sha256": "5df6e0e2761359d30a8275058e299fcc0381534545f55cf43e41983f5d4c9456"
       }
     }
   ],
@@ -16,6 +16,12 @@
       "allow_symlinks": true,
       "shard_size": 1000000000.0
     },
-    "resources": []
+    "resources": [
+      {
+        "algorithm": "sha256-sharded-1000000000",
+        "digest": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
+        "name": ".:0:0"
+      }
+    ]
   }
 }

tests/_signing/testdata/signing/TestPayload/empty_model_file_small_shards

@@ -4,7 +4,7 @@
     {
       "name": "empty_file_model",
       "digest": {
-        "sha256": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
+        "sha256": "5df6e0e2761359d30a8275058e299fcc0381534545f55cf43e41983f5d4c9456"
       }
     }
   ],
@@ -16,6 +16,12 @@
       "allow_symlinks": true,
       "shard_size": 8.0
     },
-    "resources": []
+    "resources": [
+      {
+        "algorithm": "sha256-sharded-8",
+        "digest": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
+        "name": ".:0:0"
+      }
+    ]
   }
 }

tests/_signing/testdata/signing/TestPayload/model_folder_with_empty_file_shard

@@ -4,7 +4,7 @@
     {
       "name": "empty_file_inside_model",
       "digest": {
-        "sha256": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
+        "sha256": "5df6e0e2761359d30a8275058e299fcc0381534545f55cf43e41983f5d4c9456"
       }
     }
   ],
@@ -16,6 +16,12 @@
       "allow_symlinks": true,
       "shard_size": 1000000000.0
     },
-    "resources": []
+    "resources": [
+      {
+        "algorithm": "sha256-sharded-1000000000",
+        "digest": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
+        "name": "empty_file:0:0"
+      }
+    ]
   }
 }

tests/_signing/testdata/signing/TestPayload/model_folder_with_empty_file_small_shards

@@ -4,7 +4,7 @@
     {
       "name": "empty_file_inside_model",
       "digest": {
-        "sha256": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
+        "sha256": "5df6e0e2761359d30a8275058e299fcc0381534545f55cf43e41983f5d4c9456"
       }
     }
   ],
@@ -16,6 +16,12 @@
       "allow_symlinks": true,
       "shard_size": 8.0
     },
-    "resources": []
+    "resources": [
+      {
+        "algorithm": "sha256-sharded-8",
+        "digest": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
+        "name": "empty_file:0:0"
+      }
+    ]
   }
 }

tests/api_test.py

@@ -321,6 +321,44 @@ def test_reused_config_does_not_leak_ignore_paths(
         with pytest.raises(ValueError, match="Extra files"):
             config.verify(model_b, sig_b)
 
+    def test_sharded_signature_covers_empty_files(self, base_path, tmp_path):
+        os.chdir(base_path)
+
+        private_key = Path(TESTDATA / "keys/certificate/signing-key.pem")
+        public_key = Path(TESTDATA / "keys/certificate/signing-key-pub.pem")
+
+        model = tmp_path / "model"
+        model.mkdir()
+        (model / "weights").write_text("weights")
+        (model / "__init__.py").write_bytes(b"")
+        signature = tmp_path / "model.sig"
+
+        signing.Config().use_elliptic_key_signer(
+            private_key=private_key
+        ).set_hashing_config(
+            hashing.Config()
+            .set_ignored_paths(paths=[], ignore_git_paths=False)
+            .use_shard_serialization()
+        ).sign(model, signature)
+
+        assert get_signed_files(signature) == ["__init__.py:0:0", "weights:0:7"]
+
+        config = (
+            verifying.Config()
+            .use_elliptic_key_verifier(public_key=public_key)
+            .set_hashing_config(
+                hashing.Config()
+                .set_ignored_paths(paths=[], ignore_git_paths=False)
+                .use_shard_serialization()
+            )
+        )
+        config.verify(model, signature)
+
+        # Dropping the empty file changes the model, so it must be reported.
+        (model / "__init__.py").unlink()
+        with pytest.raises(ValueError, match="Missing files"):
+            config.verify(model, signature)
+
 
 class TestCertificateSigning:
     def test_sign_and_verify(self, base_path, populate_tmpdir):