Refactor manifest extraction to use verification path

Replace Manifest.from_signature() with signing.manifest_from_signature()
that verifies signatures before extracting manifests. This addresses
reviewer feedback to reuse existing verification logic and adds security
to incremental signing by ensuring old signatures are verified before
their hashes are reused.

Changes:
- Add manifest_from_signature() to signing.py that calls Verifier.verify()
- Update sign_incremental() to require identity/oidc_issuer parameters
  for verification of old signatures
- Remove Manifest.from_signature() from manifest.py (eliminated code
  duplication)
- Update documentation examples in hashing.py
- Remove redundant tests (DSSE parsing already tested in signing_test.py)

This is a breaking change for incremental signing API, but improves
security by preventing tampering of old signatures.

Signed-off-by: Emrick Donadei <[email protected]>

Author: edonadei

src/model_signing/_signing/signing.py

@@ -166,6 +166,45 @@ def dsse_payload_to_manifest_compat(
     return manifest.Manifest(model_name, items, serialization)
 
 
+def manifest_from_signature(
+    signature_path: pathlib.Path,
+    *,
+    identity: str,
+    oidc_issuer: str,
+    use_staging: bool = False,
+) -> manifest.Manifest:
+    """Extracts and verifies a manifest from an existing signature file.
+
+    This function reads a signature file (Sigstore bundle), verifies the
+    cryptographic signature, and returns the manifest. This is essential when
+    reusing hashes from an old signature (e.g., in incremental signing) to
+    ensure the old signature hasn't been tampered with.
+
+    Args:
+        signature_path: Path to the signature file to read and verify.
+        identity: The expected identity that signed the model (e.g., email).
+        oidc_issuer: The expected OpenID Connect issuer that provided the
+            certificate used for the signature.
+        use_staging: Use staging configurations instead of production. This
+            should only be set to True when testing. Default is False.
+
+    Returns:
+        A Manifest object representing the signed model.
+
+    Raises:
+        ValueError: If signature verification fails or the signature file
+            cannot be parsed.
+        FileNotFoundError: If the signature file doesn't exist.
+    """
+    from model_signing._signing import sign_sigstore  # noqa: PLC0415
+
+    signature = sign_sigstore.Signature.read(signature_path)
+    verifier = sign_sigstore.Verifier(
+        identity=identity, oidc_issuer=oidc_issuer, use_staging=use_staging
+    )
+    return verifier.verify(signature)
+
+
 class Payload:
     """In-toto payload used to represent a model for signing.
 

src/model_signing/hashing.py

@@ -406,9 +406,13 @@ def use_incremental_serialization(
         You can override any of these by passing explicit values.
 
         Usage example:
-            # Extract manifest from previous signature
-            old_manifest = manifest.Manifest.from_signature(
-                pathlib.Path("model.sig.old")
+            from model_signing._signing import signing
+
+            # Extract and verify manifest from previous signature
+            old_manifest = signing.manifest_from_signature(
+                pathlib.Path("model.sig.old"),
+                identity="[email protected]",
+                oidc_issuer="https://github.com/login/oauth",
             )
 
             # Configure incremental hashing (auto-detects parameters)

src/model_signing/manifest.py

@@ -39,10 +39,8 @@
 """
 
 import abc
-import base64
 from collections.abc import Iterable, Iterator
 import dataclasses
-import json
 import pathlib
 import sys
 from typing import Any, Final
@@ -468,51 +466,3 @@ def serialization_type(self) -> dict[str, Any]:
         manifest so that signature verification can use the same method.
         """
         return self._serialization_type.serialization_parameters
-
-    @classmethod
-    def from_signature(cls, signature_path: pathlib.Path) -> Self:
-        """Extracts a manifest from an existing signature file.
-
-        This method reads a signature file (Sigstore bundle) and extracts the
-        manifest without performing cryptographic verification. This is useful
-        for incremental re-hashing where you need to know what files were
-        previously signed without verifying the signature.
-
-        Args:
-            signature_path: Path to the signature file to read.
-
-        Returns:
-            A Manifest object representing the signed model.
-
-        Raises:
-            ValueError: If the signature file cannot be parsed or doesn't
-                contain a valid manifest.
-            FileNotFoundError: If the signature file doesn't exist.
-        """
-        # Avoid circular import by importing here
-        from model_signing._signing import signing
-
-        # Read the signature file
-        content = signature_path.read_text(encoding="utf-8")
-        bundle_dict = json.loads(content)
-
-        # Extract the DSSE envelope payload
-        if "dsseEnvelope" in bundle_dict:
-            # This is a protobuf-based bundle
-            envelope = bundle_dict["dsseEnvelope"]
-        elif "dsse_envelope" in bundle_dict:
-            # Alternative snake_case naming
-            envelope = bundle_dict["dsse_envelope"]
-        else:
-            raise ValueError("Signature file does not contain a DSSE envelope")
-
-        # Decode the payload (it's base64 encoded)
-        payload_b64 = envelope.get("payload")
-        if not payload_b64:
-            raise ValueError("DSSE envelope does not contain a payload")
-
-        payload_bytes = base64.b64decode(payload_b64)
-        payload_dict = json.loads(payload_bytes)
-
-        # Use the existing function to convert DSSE payload to manifest
-        return signing.dsse_payload_to_manifest(payload_dict)

src/model_signing/signing.py

@@ -48,7 +48,6 @@
 from typing import Optional
 
 from model_signing import hashing
-from model_signing import manifest
 from model_signing._signing import sign_certificate as certificate
 from model_signing._signing import sign_ec_key as ec_key
 from model_signing._signing import sign_sigstore as sigstore
@@ -81,6 +80,9 @@ def sign_incremental(
     old_signature_path: hashing.PathLike,
     new_signature_path: hashing.PathLike,
     *,
+    identity: str,
+    oidc_issuer: str,
+    use_staging: bool = False,
     files_to_hash: Optional[Iterable[hashing.PathLike]] = None,
 ):
     """Signs a model incrementally, only re-hashing changed files.
@@ -91,6 +93,9 @@ def sign_incremental(
     digests from the previous signature for unchanged files and only hashes
     new or modified files.
 
+    The old signature is cryptographically verified before its hashes are
+    reused, ensuring the integrity of the incremental signing process.
+
     In this default configuration we sign using Sigstore.
 
     Usage example:
@@ -99,28 +104,40 @@ def sign_incremental(
             model_path="huge-model/",
             old_signature_path="model.sig.old",
             new_signature_path="model.sig.new",
+            identity="[email protected]",
+            oidc_issuer="https://github.com/login/oauth",
             files_to_hash=["huge-model/README.md"]
         )
 
     Args:
         model_path: The path to the model to sign.
         old_signature_path: The path to the previous signature. The manifest
-            from this signature will be extracted and used for incremental
+            from this signature will be verified and extracted for incremental
             hashing.
         new_signature_path: The path where the new signature will be written.
+        identity: The expected identity that signed the old signature
+            (e.g., email address).
+        oidc_issuer: The expected OpenID Connect issuer that provided the
+            certificate for the old signature.
+        use_staging: Use staging configurations for verification instead of
+            production. Should only be True when testing. Default is False.
         files_to_hash: Optional list of files that changed and need to be
             re-hashed. If None, only new files (not in old signature) will
             be hashed. Existing files will have their digests reused.
             To detect changed files, use git diff or similar tools.
 
     Raises:
         FileNotFoundError: If old_signature_path doesn't exist.
-        ValueError: If old_signature_path cannot be parsed.
+        ValueError: If old_signature_path cannot be parsed or verification
+            fails.
     """
     Config().sign_incremental(
         model_path,
         old_signature_path,
         new_signature_path,
+        identity=identity,
+        oidc_issuer=oidc_issuer,
+        use_staging=use_staging,
         files_to_hash=files_to_hash,
     )
 
@@ -165,30 +182,44 @@ def sign_incremental(
         old_signature_path: hashing.PathLike,
         new_signature_path: hashing.PathLike,
         *,
+        identity: str,
+        oidc_issuer: str,
+        use_staging: bool = False,
         files_to_hash: Optional[Iterable[hashing.PathLike]] = None,
     ):
         """Signs a model incrementally using the current configuration.
 
-        This method extracts the manifest from an existing signature and
-        configures incremental hashing to reuse digests for unchanged files.
-        Only new or modified files are re-hashed, providing significant
-        performance improvements for large models.
+        This method extracts and verifies the manifest from an existing
+        signature, then configures incremental hashing to reuse digests for
+        unchanged files. Only new or modified files are re-hashed, providing
+        significant performance improvements for large models.
 
         Args:
             model_path: The path to the model to sign.
             old_signature_path: The path to the previous signature.
             new_signature_path: The path where the new signature will be
                 written.
+            identity: The expected identity that signed the old signature
+                (e.g., email address).
+            oidc_issuer: The expected OpenID Connect issuer that provided
+                the certificate for the old signature.
+            use_staging: Use staging configurations for verification instead
+                of production. Should only be True when testing. Default is
+                False.
             files_to_hash: Optional list of files that changed and need to
                 be re-hashed. If None, only new files will be hashed.
 
         Raises:
             FileNotFoundError: If old_signature_path doesn't exist.
-            ValueError: If old_signature_path cannot be parsed.
+            ValueError: If old_signature_path cannot be parsed or verification
+                fails.
         """
-        # Extract manifest from old signature
-        old_manifest = manifest.Manifest.from_signature(
-            pathlib.Path(old_signature_path)
+        # Extract and verify manifest from old signature
+        old_manifest = signing.manifest_from_signature(
+            pathlib.Path(old_signature_path),
+            identity=identity,
+            oidc_issuer=oidc_issuer,
+            use_staging=use_staging,
         )
 
         # Configure incremental hashing
@@ -351,7 +382,9 @@ def use_pkcs11_signer(
             The new signing configuration.
         """
         try:
-            from model_signing._signing import sign_pkcs11 as pkcs11
+            from model_signing._signing import (  # noqa: PLC0415
+                sign_pkcs11 as pkcs11,
+            )
         except ImportError as e:
             raise RuntimeError(
                 "PKCS #11 functionality requires the 'pkcs11' extra. "
@@ -384,7 +417,9 @@ def use_pkcs11_certificate_signer(
             The new signing configuration.
         """
         try:
-            from model_signing._signing import sign_pkcs11 as pkcs11
+            from model_signing._signing import (  # noqa: PLC0415
+                sign_pkcs11 as pkcs11,
+            )
         except ImportError as e:
             raise RuntimeError(
                 "PKCS #11 functionality requires the 'pkcs11' extra. "