From 381e3e1eb6dc23ff07caf5b15bcd8387a4a0ad5a Mon Sep 17 00:00:00 2001 From: Itecz Solution Date: Thu, 30 Jul 2026 17:30:42 +0530 Subject: [PATCH] record zero-length files in the shard serialization manifest _get_shards guarded its loop with `if path_size > 0`, so a zero-length file produced no shards and never became a manifest item. Shard serialization left every empty file in the model out of the signature, even though the file was there when the model was signed. The verifier re-serializes the model the same way, so the omission is symmetric and nothing reports a difference. Against a model signed with use_shard_serialization(), deleting a signed empty file verifies clean, adding new empty files verifies clean, and with ignore_unsigned_files an empty file can be filled with content and still verify clean. Empty marker files decide real loader behavior (__init__.py, py.typed, feature-flag files), so this is a gap in what the signature covers. Drop the guard so _endpoints(shard_size, 0) yields one (path, 0, 0) shard, and let ShardedFileHasher.set_shard accept end == start for it. File serialization already recorded empty files, which is why file_test.py asserts test_folder_model_empty_file_gets_included while the shard suite asserted the opposite and the shard goldens for both empty-file fixtures were zero-byte. The two serializers now agree. Shard signatures over models that contain empty files have to be regenerated. Signed-off-by: Itecz Solution --- CHANGELOG.md | 1 + src/model_signing/_hashing/io.py | 20 ++++---- .../_serialization/file_shard.py | 17 ++++--- tests/_hashing/io_hashing_test.py | 50 +++++++++---------- tests/_serialization/file_shard_test.py | 10 +++- .../TestSerializer/empty_model_file | 1 + .../empty_model_file_small_shards | 1 + .../model_folder_with_empty_file | 1 + .../model_folder_with_empty_file_small_shards | 1 + .../TestPayload/empty_model_file_shard | 10 +++- .../TestPayload/empty_model_file_small_shards | 10 +++- .../model_folder_with_empty_file_shard | 10 +++- .../model_folder_with_empty_file_small_shards | 10 +++- tests/api_test.py | 38 ++++++++++++++ 14 files changed, 130 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ff870d0..fb0ffd69 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 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)) diff --git a/src/model_signing/_hashing/io.py b/src/model_signing/_hashing/io.py index dd963e92..9155e551 100644 --- a/src/model_signing/_hashing/io.py +++ b/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: diff --git a/src/model_signing/_serialization/file_shard.py b/src/model_signing/_serialization/file_shard.py index fc55706e..689823dd 100644 --- a/src/model_signing/_serialization/file_shard.py +++ b/src/model_signing/_serialization/file_shard.py @@ -41,6 +41,8 @@ def _endpoints(step: int, end: int) -> Iterable[int]: [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`. @@ -191,14 +193,17 @@ def serialize( 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( diff --git a/tests/_hashing/io_hashing_test.py b/tests/_hashing/io_hashing_test.py index e445ba85..01a02860 100644 --- a/tests/_hashing/io_hashing_test.py +++ b/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( diff --git a/tests/_serialization/file_shard_test.py b/tests/_serialization/file_shard_test.py index c6a1f343..1eb436b4 100644 --- a/tests/_serialization/file_shard_test.py +++ b/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, diff --git a/tests/_serialization/testdata/file_shard/TestSerializer/empty_model_file b/tests/_serialization/testdata/file_shard/TestSerializer/empty_model_file index e69de29b..5f48b08e 100644 --- a/tests/_serialization/testdata/file_shard/TestSerializer/empty_model_file +++ b/tests/_serialization/testdata/file_shard/TestSerializer/empty_model_file @@ -0,0 +1 @@ +.:0:0:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 diff --git a/tests/_serialization/testdata/file_shard/TestSerializer/empty_model_file_small_shards b/tests/_serialization/testdata/file_shard/TestSerializer/empty_model_file_small_shards index e69de29b..5f48b08e 100644 --- a/tests/_serialization/testdata/file_shard/TestSerializer/empty_model_file_small_shards +++ b/tests/_serialization/testdata/file_shard/TestSerializer/empty_model_file_small_shards @@ -0,0 +1 @@ +.:0:0:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 diff --git a/tests/_serialization/testdata/file_shard/TestSerializer/model_folder_with_empty_file b/tests/_serialization/testdata/file_shard/TestSerializer/model_folder_with_empty_file index e69de29b..417be500 100644 --- a/tests/_serialization/testdata/file_shard/TestSerializer/model_folder_with_empty_file +++ b/tests/_serialization/testdata/file_shard/TestSerializer/model_folder_with_empty_file @@ -0,0 +1 @@ +empty_file:0:0:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 diff --git a/tests/_serialization/testdata/file_shard/TestSerializer/model_folder_with_empty_file_small_shards b/tests/_serialization/testdata/file_shard/TestSerializer/model_folder_with_empty_file_small_shards index e69de29b..417be500 100644 --- a/tests/_serialization/testdata/file_shard/TestSerializer/model_folder_with_empty_file_small_shards +++ b/tests/_serialization/testdata/file_shard/TestSerializer/model_folder_with_empty_file_small_shards @@ -0,0 +1 @@ +empty_file:0:0:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 diff --git a/tests/_signing/testdata/signing/TestPayload/empty_model_file_shard b/tests/_signing/testdata/signing/TestPayload/empty_model_file_shard index 4aaa3933..1d4eb361 100644 --- a/tests/_signing/testdata/signing/TestPayload/empty_model_file_shard +++ b/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" + } + ] } } diff --git a/tests/_signing/testdata/signing/TestPayload/empty_model_file_small_shards b/tests/_signing/testdata/signing/TestPayload/empty_model_file_small_shards index b1782b8b..ada451f6 100644 --- a/tests/_signing/testdata/signing/TestPayload/empty_model_file_small_shards +++ b/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" + } + ] } } diff --git a/tests/_signing/testdata/signing/TestPayload/model_folder_with_empty_file_shard b/tests/_signing/testdata/signing/TestPayload/model_folder_with_empty_file_shard index 9f73f3a2..0a90fb9b 100644 --- a/tests/_signing/testdata/signing/TestPayload/model_folder_with_empty_file_shard +++ b/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" + } + ] } } diff --git a/tests/_signing/testdata/signing/TestPayload/model_folder_with_empty_file_small_shards b/tests/_signing/testdata/signing/TestPayload/model_folder_with_empty_file_small_shards index 4b698954..28080f78 100644 --- a/tests/_signing/testdata/signing/TestPayload/model_folder_with_empty_file_small_shards +++ b/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" + } + ] } } diff --git a/tests/api_test.py b/tests/api_test.py index 7ca85ec9..4a14ceb1 100644 --- a/tests/api_test.py +++ b/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):