Repository: theupdateframework/python-tuf (python)

Issue:
repository: Tweak snapshot/timestamp triggers
Currently `Repository.do_snapshot()` and `Repository.do_timestamp()` decide whether the update is needed by looking at whether the contents are up-to-date.

This bypasses one case where timestamp and snapshot are needed: when the signing keys have changed. So I guess the two methods should also check if the current snapshot/timestamp metadata is verified by root. 

I did not do that originally since I was hoping the methods could be self contained and would not make assumptions about how the repository is generated/stored. This seems to be a good reason to peek at other metadata though: root should be assumed to exist and to be valid if you are calling do_snapshot/do_timestamp

Pull request:
addd_root_verification
<!--
Before submitting a pull request:
  * Run linter and tests locally: Use "tox"
  * Ensure your commits are signed-off-by: Use "commit --signoff"
  * Make sure new code has tests and is documented
  * For more info, see docs/CONTRIBUTING.rst

Once commits are signed off and tested, describe the purpose and contents
of the pull request below.
-->

**Description of the changes being introduced by the pull request**:
Add root verification step in do_snapshot and do_timestamp for catching changes in signing keys. 



Fixes #2438



Patch:
diff --git a/docs/CONTRIBUTING.rst b/docs/CONTRIBUTING.rst
index eff4180a..be6830fb 100644
--- a/docs/CONTRIBUTING.rst
+++ b/docs/CONTRIBUTING.rst
@@ -31,7 +31,7 @@ tox run.
 
 ::
 
-    $ tox
+     tox
 
 Below, you will see more details about each step managed by ``tox``, in case
 you need debug/run outside ``tox``.
@@ -44,16 +44,16 @@ the test aggregation script inside the *tests* subdirectory. ``tuf`` and its
 dependencies must already be installed.
 ::
 
-    $ cd tests/
-    $ python3 aggregate_tests.py
+     cd tests/
+     python3 aggregate_tests.py
 
 
 Individual tests can also be executed. Optional ``-v`` flags can be added to
 increase log level up to DEBUG (``-vvvv``).
 ::
 
-    $ cd tests/
-    $ python3 test_updater_ng.py -v
+     cd tests/
+     python3 test_updater_ng.py -v
 
 
 Coverage
@@ -64,8 +64,8 @@ invoked with the ``coverage`` tool (requires installation of ``coverage``, e.g.
 via PyPI).
 ::
 
-    $ cd tests/
-    $ coverage run aggregate_tests.py && coverage report
+     cd tests/
+     coverage run aggregate_tests.py && coverage report
 
 
 Auto-formatting
@@ -76,4 +76,4 @@ The linter in CI/CD will check that new TUF code is formatted with
 command line:
 ::
 
-    $ tox -e fix
+     tox -e fix
diff --git a/requirements/lint.txt b/requirements/lint.txt
index 1320b47e..bfd5f19d 100644
--- a/requirements/lint.txt
+++ b/requirements/lint.txt
@@ -6,5 +6,5 @@
 # Lint tools
 # (We are not so interested in the specific versions of the tools: the versions
 # are pinned to prevent unexpected linting failures when tools update)
-ruff==0.4.5
+ruff==0.4.7
 mypy==1.10.0
diff --git a/tuf/repository/_repository.py b/tuf/repository/_repository.py
index fc96b8f4..09306b82 100644
--- a/tuf/repository/_repository.py
+++ b/tuf/repository/_repository.py
@@ -9,6 +9,7 @@
 from copy import deepcopy
 from typing import Dict, Generator, Optional, Tuple
 
+from tuf.api.exceptions import UnsignedMetadataError
 from tuf.api.metadata import (
     Metadata,
     MetaFile,
@@ -188,6 +189,18 @@ def do_snapshot(
         update_version = force
         removed: Dict[str, MetaFile] = {}
 
+        root = self.root()
+        snapshot_md = self.open(Snapshot.type)
+
+        try:
+            root.verify_delegate(
+                Snapshot.type,
+                snapshot_md.signed_bytes,
+                snapshot_md.signatures,
+            )
+        except UnsignedMetadataError:
+            update_version = True
+
         with self.edit_snapshot() as snapshot:
             for keyname, new_meta in self.targets_infos.items():
                 if keyname not in snapshot.meta:
@@ -228,6 +241,19 @@ def do_timestamp(
         """
         update_version = force
         removed = None
+
+        root = self.root()
+        timestamp_md = self.open(Timestamp.type)
+
+        try:
+            root.verify_delegate(
+                Timestamp.type,
+                timestamp_md.signed_bytes,
+                timestamp_md.signatures,
+            )
+        except UnsignedMetadataError:
+            update_version = True
+
         with self.edit_timestamp() as timestamp:
             if self.snapshot_info.version < timestamp.snapshot_meta.version:
                 raise ValueError("snapshot version rollback")


Test patch:
diff --git a/requirements/test.txt b/requirements/test.txt
index ec15bb8b..ae6dda96 100644
--- a/requirements/test.txt
+++ b/requirements/test.txt
@@ -4,4 +4,4 @@
 -r pinned.txt
 
 # coverage measurement
-coverage==7.5.2
+coverage==7.5.3
diff --git a/tests/test_repository.py b/tests/test_repository.py
index 092df0ec..e1d228dc 100644
--- a/tests/test_repository.py
+++ b/tests/test_repository.py
@@ -186,7 +186,6 @@ def test_do_snapshot_after_new_targets_delegation(self) -> None:
         self.assertEqual(2, len(snapshot_versions))
         self.assertEqual(2, snapshot_versions[-1].signed.version)
 
-    @unittest.expectedFailure  # Issue 2438
     def test_do_snapshot_after_snapshot_key_change(self) -> None:
         # change snapshot signing keys
         with self.repo.edit_root() as root:
@@ -228,7 +227,6 @@ def test_do_timestamp_after_snapshot_change(self) -> None:
         self.assertEqual(2, len(timestamp_versions))
         self.assertEqual(2, timestamp_versions[-1].signed.version)
 
-    @unittest.expectedFailure  # Issue 2438
     def test_do_timestamp_after_timestamp_key_change(self) -> None:
         # change timestamp signing keys
         with self.repo.edit_root() as root: