Skip to content

Fix Role and DelegatedRole hashing - #2973

Open
Sachith77 wants to merge 1 commit into
theupdateframework:developfrom
Sachith77:fix/role-hash-unhashable-types
Open

Fix Role and DelegatedRole hashing#2973
Sachith77 wants to merge 1 commit into
theupdateframework:developfrom
Sachith77:fix/role-hash-unhashable-types

Conversation

@Sachith77

Copy link
Copy Markdown

Description

While going through _payload.py, I noticed that Role.__hash__() fails because it tries to hash keyids (a list) and unrecognized_fields (a dict) directly.

This PR:

  • Converts keyids to a tuple before hashing.
  • Excludes unrecognized_fields since it may contain nested, unhashable JSON.
  • Fixes DelegatedRole.__hash__() using self.path instead of self.paths.
  • Converts paths and path_hash_prefixes to tuples before hashing.
  • Adds regression tests for hashing, equality, and set usage.

The full test suite passes locally with tox.

I noticed similar patterns in a few other metadata classes, but kept this PR focused on Role and DelegatedRole.

Signed-off-by: Sachith Reddy <sachith.24bcs10403@sst.scaler.com>
@Sachith77
Sachith77 requested a review from a team as a code ownerAugust 3, 2026 21:41
jku
jku approved these changes Aug 13, 2026

@jkujku left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it may have been a mistake to add hash() to begin with, I think it only happened because ruff complained about not having it and it seemed easy to do...

In any case these fixes seem reasonable to me, thanks

@Sachith77

Copy link
Copy Markdown
Author

Thanks for the review and the context on why hash() was added.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Sachith77@jku