Fix __hash__ crashing on unhashable dict attributes

tests/test_api.py

@@ -14,7 +14,7 @@
 from copy import copy, deepcopy
 from datetime import datetime, timedelta, timezone
 from pathlib import Path
-from typing import ClassVar
+from typing import TYPE_CHECKING, ClassVar
 
 from securesystemslib import exceptions as sslib_exceptions
 from securesystemslib.signer import (
@@ -48,9 +48,28 @@
 from tuf.api.serialization import DeserializationError, SerializationError
 from tuf.api.serialization.json import JSONSerializer
 
+if TYPE_CHECKING:
+    from collections.abc import Callable
+
 logger = logging.getLogger(__name__)
 
 
+def _sslib_hashable() -> bool:
+    """Return True if securesystemslib Key objects can be hashed.
+
+    Metadata containing Key or Signature objects is only hashable once
+    securesystemslib makes those hashable.
+    """
+    try:
+        hash(SSlibKey("kid", "ed25519", "ed25519", {"public": "aa"}))
+    except TypeError:
+        return False
+    return True
+
+
+SSLIB_HASHABLE = _sslib_hashable()
+
+
 class TestMetadata(unittest.TestCase):
     """Tests for public API of all classes in 'tuf/api/metadata.py'."""
 
@@ -1111,6 +1130,102 @@ def test_role_and_delegated_role_hash(self) -> None:
         role_set = {dr, dr2}
         self.assertEqual(len(role_set), 1)
 
+    def test_metadata_hash(self) -> None:
+        # Each of these __hash__ implementations passed a raw dict
+        # (unrecognized_fields, keys, roles, meta, targets, hashes,
+        # signatures) to hash(), which raises TypeError.
+        expires = datetime(2030, 1, 1, tzinfo=timezone.utc)
+        key = SSlibKey("kid1", "ed25519", "ed25519", {"public": "aa"})
+        delegated_role = DelegatedRole("r", ["kid1"], 1, False, ["*"], None)
+        signature = Signature("kid1", "abcd")
+
+        def root_with_key() -> Root:
+            root = Root(expires=expires)
+            root.add_key(key, "targets")
+            return root
+
+        factories: dict[str, Callable[[], object]] = {
+            "MetaFile": lambda: MetaFile(1, 10, {"sha256": "ab"}),
+            "TargetFile": lambda: TargetFile(10, {"sha256": "ab"}, "p"),
+            "Delegations": lambda: Delegations(
+                {"kid1": key}, {"r": delegated_role}
+            ),
+            "Root": root_with_key,
+            "Timestamp": lambda: Timestamp(expires=expires),
+            "Snapshot": lambda: Snapshot(expires=expires),
+            "Targets": lambda: Targets(expires=expires),
+            "Metadata": lambda: Metadata(
+                Snapshot(expires=expires), {"kid1": signature}
+            ),
+        }
+
+        # Metadata holding securesystemslib Key or Signature objects cannot be
+        # hashed until those are hashable in securesystemslib itself.
+        needs_sslib_hashing = {"Root", "Delegations", "Metadata"}
+
+        for name, factory in factories.items():
+            with self.subTest(name):
+                if name in needs_sslib_hashing and not SSLIB_HASHABLE:
+                    self.skipTest("securesystemslib Key/Signature not hashable")
+
+                obj, equal_obj = factory(), factory()
+
+                self.assertIsInstance(hash(obj), int)
+
+                # equal objects must produce equal hashes (Python data model)
+                self.assertEqual(obj, equal_obj)
+                self.assertEqual(hash(obj), hash(equal_obj))
+
+                # the object must work as a set member / dict key
+                self.assertEqual(len({obj, equal_obj}), 1)
+
+    def test_metadata_hash_covers_content(self) -> None:
+        # Objects that differ only in their contained collections must not
+        # collide: the file hashes identify a MetaFile/TargetFile, and meta
+        # and targets identify a Snapshot/Targets.
+        expires = datetime(2030, 1, 1, tzinfo=timezone.utc)
+
+        pairs = {
+            "MetaFile": (
+                MetaFile(1, 10, {"sha256": "aa"}),
+                MetaFile(1, 10, {"sha256": "bb"}),
+            ),
+            "TargetFile": (
+                TargetFile(10, {"sha256": "aa"}, "f"),
+                TargetFile(10, {"sha256": "bb"}, "f"),
+            ),
+            "Snapshot": (
+                Snapshot(expires=expires, meta={"a.json": MetaFile(1)}),
+                Snapshot(expires=expires, meta={"b.json": MetaFile(2)}),
+            ),
+            "Targets": (
+                Targets(
+                    expires=expires,
+                    targets={"a": TargetFile(1, {"sha256": "aa"}, "a")},
+                ),
+                Targets(
+                    expires=expires,
+                    targets={"b": TargetFile(2, {"sha256": "bb"}, "b")},
+                ),
+            ),
+        }
+
+        for name, (first, second) in pairs.items():
+            with self.subTest(name):
+                self.assertNotEqual(first, second)
+                self.assertNotEqual(hash(first), hash(second))
+
+    def test_metadata_hash_ignores_unrecognized_fields(self) -> None:
+        # unrecognized_fields holds arbitrary (possibly nested) JSON, so it is
+        # left out of __hash__. Objects differing only in unrecognized_fields
+        # are unequal but may share a hash, which the data model allows.
+        expires = datetime(2030, 1, 1, tzinfo=timezone.utc)
+        plain = Snapshot(expires=expires)
+        extra = Snapshot(expires=expires, unrecognized_fields={"a": ["b"]})
+
+        self.assertNotEqual(plain, extra)
+        self.assertIsInstance(hash(extra), int)
+
     def test_is_delegated_role_in_succinct_roles(self) -> None:
         succinct_roles = SuccinctRoles([], 1, 5, "bin")
         false_role_name_examples = [

tuf/api/_payload.py

@@ -188,7 +188,6 @@ def __hash__(self) -> int:
                 self.version,
                 self.spec_version,
                 self.expires,
-                self.unrecognized_fields,
             )
         )
 
@@ -565,10 +564,9 @@ def __hash__(self) -> int:
         return hash(
             (
                 super().__hash__(),
-                self.keys,
-                self.roles,
+                tuple(sorted(self.keys.items())),
+                tuple(sorted(self.roles.items())),
                 self.consistent_snapshot,
-                self.unrecognized_fields,
             )
         )
 
@@ -848,9 +846,12 @@ def __eq__(self, other: object) -> bool:
         )
 
     def __hash__(self) -> int:
-        return hash(
-            (self.version, self.length, self.hashes, self.unrecognized_fields)
+        hashes = (
+            tuple(sorted(self.hashes.items()))
+            if self.hashes is not None
+            else None
         )
+        return hash((self.version, self.length, hashes))
 
     @classmethod
     def from_dict(cls, meta_dict: dict[str, Any]) -> MetaFile:
@@ -1031,7 +1032,7 @@ def __eq__(self, other: object) -> bool:
         return super().__eq__(other) and self.meta == other.meta
 
     def __hash__(self) -> int:
-        return hash((super().__hash__(), self.meta))
+        return hash((super().__hash__(), tuple(sorted(self.meta.items()))))
 
     @classmethod
     def from_dict(cls, signed_dict: dict[str, Any]) -> Snapshot:
@@ -1463,10 +1464,10 @@ def __eq__(self, other: object) -> bool:
     def __hash__(self) -> int:
         return hash(
             (
-                self.keys,
-                self.roles,
+                tuple(sorted(self.keys.items())),
+                # Order of the delegated roles matters (see __eq__)
+                tuple(self.roles.items()) if self.roles is not None else None,
                 self.succinct_roles,
-                self.unrecognized_fields,
             )
         )
 
@@ -1593,7 +1594,7 @@ def __eq__(self, other: object) -> bool:
 
     def __hash__(self) -> int:
         return hash(
-            (self.length, self.hashes, self.path, self.unrecognized_fields)
+            (self.length, self.path, tuple(sorted(self.hashes.items())))
         )
 
     @classmethod
@@ -1740,7 +1741,13 @@ def __eq__(self, other: object) -> bool:
         )
 
     def __hash__(self) -> int:
-        return hash((super().__hash__(), self.targets, self.delegations))
+        return hash(
+            (
+                super().__hash__(),
+                tuple(sorted(self.targets.items())),
+                self.delegations,
+            )
+        )
 
     @classmethod
     def from_dict(cls, signed_dict: dict[str, Any]) -> Targets:

tuf/api/metadata.py

@@ -148,7 +148,7 @@ def __eq__(self, other: object) -> bool:
         )
 
     def __hash__(self) -> int:
-        return hash((self.signatures, self.signed, self.unrecognized_fields))
+        return hash((tuple(self.signatures.items()), self.signed))
 
     @property
     def signed_bytes(self) -> bytes: