From cd9a1dbd24e45ca2c16c9ca040c7c85297e2239e Mon Sep 17 00:00:00 2001 From: Itecz Solution Date: Sat, 18 Jul 2026 14:43:22 +0530 Subject: [PATCH] scope hashing config per verify call to stop ignore-path leak verifying.Config.verify carried hashing state across models when the config is reused (a pattern the module docstring recommends): - the guessed hashing config was cached on the instance, so a later model was hashed with an earlier model's serialization settings - each signed manifest's ignore paths were added to that shared config via add_ignored_paths and never reset, accumulating across calls - an extra unsigned file placed at a path some earlier model ignored was skipped instead of reported during a later model's verification, so a tampered model verified clean Build the hashing config per call (deepcopy a caller-supplied one, else a fresh guess) and apply the signed ignore paths to that local copy so no state persists across verify() calls. Signed-off-by: Itecz Solution --- CHANGELOG.md | 1 + src/model_signing/verifying.py | 22 ++++++++++------ tests/api_test.py | 46 ++++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b391983..7ff870d0 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 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)) - Fixed a bug where any PEM encoded key could be read during the key-based flows which resulted in a Python exception because the rest of the code only supported elliptic curve keys. ([#573](https://github.com/sigstore/model-transparency/pull/573)) diff --git a/src/model_signing/verifying.py b/src/model_signing/verifying.py index 87303490..be014f88 100644 --- a/src/model_signing/verifying.py +++ b/src/model_signing/verifying.py @@ -38,6 +38,7 @@ """ from collections.abc import Iterable +import copy import pathlib import sys @@ -97,10 +98,15 @@ def verify( expected_manifest = self._verifier.verify(signature) - if self._hashing_config is None: - self._guess_hashing_config(expected_manifest) + if self._hashing_config is not None: + # The signed manifest's ignore paths are applied below. Copy the + # config so they do not mutate the caller's config or accumulate + # into later verify() calls on a reused instance. + hashing_config = copy.deepcopy(self._hashing_config) + else: + hashing_config = self._guess_hashing_config(expected_manifest) if "ignore_paths" in expected_manifest.serialization_type: - self._hashing_config.add_ignored_paths( + hashing_config.add_ignored_paths( model_path=model_path, paths=expected_manifest.serialization_type["ignore_paths"], ) @@ -113,7 +119,7 @@ def verify( else: files_to_hash = None - actual_manifest = self._hashing_config.hash( + actual_manifest = hashing_config.hash( model_path, files_to_hash=files_to_hash ) @@ -189,19 +195,21 @@ def set_ignore_unsigned_files(self, ignore_unsigned_files: bool) -> Self: self._ignore_unsigned_files = ignore_unsigned_files return self - def _guess_hashing_config(self, source_manifest: manifest.Manifest) -> None: + def _guess_hashing_config( + self, source_manifest: manifest.Manifest + ) -> hashing.Config: """Attempts to guess the hashing config from a manifest.""" args = source_manifest.serialization_type method = args["method"] match method: case "files": - self._hashing_config = hashing.Config().use_file_serialization( + return hashing.Config().use_file_serialization( hashing_algorithm=args["hash_type"], allow_symlinks=args["allow_symlinks"], ignore_paths=args.get("ignore_paths", frozenset()), ) case "shards": - self._hashing_config = hashing.Config().use_shard_serialization( + return hashing.Config().use_shard_serialization( hashing_algorithm=args["hash_type"], shard_size=args["shard_size"], allow_symlinks=args["allow_symlinks"], diff --git a/tests/api_test.py b/tests/api_test.py index c65e7322..7ca85ec9 100644 --- a/tests/api_test.py +++ b/tests/api_test.py @@ -275,6 +275,52 @@ def test_sign_and_verify(self, base_path, populate_tmpdir): ) assert get_model_name(signature) == os.path.basename(model_path) + def test_reused_config_does_not_leak_ignore_paths( + 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 A is signed with its scratch file explicitly ignored. + model_a = tmp_path / "model_a" + model_a.mkdir() + (model_a / "weights").write_text("weights-a") + (model_a / "notes").write_text("scratch, excluded from signing") + sig_a = tmp_path / "a.sig" + signing.Config().use_elliptic_key_signer( + private_key=private_key + ).set_hashing_config( + hashing.Config().set_ignored_paths( + paths=[model_a / "notes"], ignore_git_paths=False + ) + ).sign(model_a, sig_a) + + # Model B is signed without ignoring anything. + model_b = tmp_path / "model_b" + model_b.mkdir() + (model_b / "weights").write_text("weights-b") + sig_b = tmp_path / "b.sig" + signing.Config().use_elliptic_key_signer( + private_key=private_key + ).set_hashing_config( + hashing.Config().set_ignored_paths(paths=[], ignore_git_paths=False) + ).sign(model_b, sig_b) + + # An unsigned file is planted in B at the path A happened to ignore. + (model_b / "notes").write_text("not covered by B's signature") + + config = verifying.Config().use_elliptic_key_verifier( + public_key=public_key + ) + config.verify(model_a, sig_a) + + # B's signature never excluded "notes", so the planted file must be + # reported rather than silently skipped using A's ignore paths. + with pytest.raises(ValueError, match="Extra files"): + config.verify(model_b, sig_b) + class TestCertificateSigning: def test_sign_and_verify(self, base_path, populate_tmpdir):