Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
22 changes: 15 additions & 7 deletions src/model_signing/verifying.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
"""

from collections.abc import Iterable
import copy
import pathlib
import sys

Expand Down Expand Up @@ -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"],
)
Expand All @@ -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
)

Expand Down Expand Up @@ -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"],
Expand Down
46 changes: 46 additions & 0 deletions tests/api_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
Loading