Skip to content

fix: enforce derivation path role validation in wallet constructors (PM-20015)#1076

Open
m2ux wants to merge 4 commits intomainfrom
fix/PM-20015-derivation-path-role-validation
Open

fix: enforce derivation path role validation in wallet constructors (PM-20015)#1076
m2ux wants to merge 4 commits intomainfrom
fix/PM-20015-derivation-path-role-validation

Conversation

@m2ux
Copy link
Copy Markdown
Contributor

@m2ux m2ux commented Mar 25, 2026

Summary

Enforce derivation path role validation in wallet constructors to prevent silent key derivation under incorrect BIP-44 paths. Addresses Least Authority audit Issue AN (Low severity).

🎫 Ticket 📐 Engineering


Motivation

The Least Authority security audit (Issue AN, Low severity) identified that DustWallet::from_path() and ShieldedWallet::from_path() accept any DerivationPath regardless of its role field. A DustWallet constructed with a shielded-role path silently derives keys under the wrong hierarchical path (m/44'/2400'/0'/3/0 instead of m/44'/2400'/0'/2/0), producing unusable keys whose failure surfaces far from the misconfiguration site.


Changes

  • Role enum - Add PartialEq derive to enable idiomatic equality comparison
  • DustWallet - Add assert! guard clause in from_path() rejecting non-Role::Dust paths, plus 3 unit tests
  • ShieldedWallet - Add assert! guard clause in from_path() rejecting non-Role::Zswap paths, plus 3 unit tests
  • Change file - Release notes entry for the audit fix

📌 Submission Checklist

  • Changes are backward-compatible (or flagged if breaking)
  • Pull request description explains why the change is needed
  • Self-reviewed the diff
  • I have included a change file, or skipped for this reason: [reason]
  • If the changes introduce a new feature, I have bumped the node minor version
  • Update documentation (if relevant)
  • No new todos introduced

🔱 Fork Strategy

  • Node Runtime Update
  • Node Client Update
  • Other
  • N/A

🗹 TODO before merging

  • Ready for review

m2ux added 2 commits March 25, 2026 13:16
Enforce derivation path role validation in wallet constructors
(Least Authority audit finding Issue AN).
DustWallet::from_path() and ShieldedWallet::from_path() now assert that
the DerivationPath role matches the expected wallet type (Role::Dust and
Role::Zswap respectively). Mismatched roles produce a descriptive panic
consistent with DerivationPath::new() error handling convention.

Changes:
- Add PartialEq derive to Role enum (hd.rs)
- Add role validation assert in DustWallet::from_path() (dust.rs)
- Add role validation assert in ShieldedWallet::from_path() (shielded.rs)
- Add 6 unit tests covering valid/invalid role scenarios
- Add change file

Addresses Least Authority audit Issue AN (Low severity).

JIRA: PM-20015
@m2ux m2ux marked this pull request as ready for review March 25, 2026 14:47
@m2ux m2ux requested a review from a team as a code owner March 25, 2026 14:47
@m2ux m2ux self-assigned this Mar 26, 2026
Sign up for free to 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.

1 participant