Skip to content

Commit

Permalink
ngclient: Fail gracefully on missing role
Browse files Browse the repository at this point in the history
If role is delegated but missing from snapshot, we currently raise a
undocumented KeyError: a generic RepositoryError seems better as callers
are expected to handle it (and adding a more specific error seems
useless as this is a repository software bug, not just expired metadata or
something).

The same check is also done later in TrustedMetadataSet but I think
keeping the check in both is clearest.

Fixes #2195

Signed-off-by: Jussi Kukkonen <[email protected]>
  • Loading branch information
jku committed Nov 28, 2022
1 parent 6ce8bb8 commit 6450a3a
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 1 deletion.
14 changes: 14 additions & 0 deletions tests/test_updater_fetch_target.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from tests import utils
from tests.repository_simulator import RepositorySimulator
from tuf.api.exceptions import RepositoryError
from tuf.api.metadata import DelegatedRole, Delegations
from tuf.ngclient import Updater


Expand Down Expand Up @@ -209,6 +210,19 @@ def test_invalid_target_cache(self) -> None:
with open(path, "rb") as f:
self.assertEqual(f.read(), target.content)

def test_meta_missing_delegated_role(self) -> None:
"""Test a delegation where the role is not part of the snapshot"""

# Add new delegation, update snapshot. Do not add the actual role
role = DelegatedRole("role1", [], 1, True, ["*"])
self.sim.targets.delegations = Delegations({}, roles={role.name: role})
self.sim.update_snapshot()

# assert that RepositoryError is raised when role1 is needed
updater = self._init_updater()
with self.assertRaises(RepositoryError):
updater.get_targetinfo("")


if __name__ == "__main__":
if "--dump" in sys.argv:
Expand Down
9 changes: 8 additions & 1 deletion tuf/ngclient/updater.py
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,14 @@ def _load_targets(self, role: str, parent_role: str) -> Metadata[Targets]:
logger.debug("Failed to load local %s: %s", role, e)

assert self._trusted_set.snapshot is not None # nosec
metainfo = self._trusted_set.snapshot.signed.meta[f"{role}.json"]

snapshot = self._trusted_set.snapshot.signed
metainfo = snapshot.meta.get(f"{role}.json")
if metainfo is None:
raise exceptions.RepositoryError(
f"Role {role} was delegated but is not part of snapshot"
)

length = metainfo.length or self.config.targets_max_length
version = None
if self._trusted_set.root.signed.consistent_snapshot:
Expand Down

0 comments on commit 6450a3a

Please sign in to comment.