feat(keyring-eth-money): validate serialized state with superstruct#474
feat(keyring-eth-money): validate serialized state with superstruct#474
Conversation
Add a superstruct schema to enforce that MoneyKeyring serialized state contains only a number[] mnemonic, a numberOfAccounts of 0 or 1, and the exact MONEY_DERIVATION_PATH. Both serialize and deserialize now assert against this schema. The MoneyKeyringSerializedState type is inferred from the struct via Infer.
Remove unnecessary casts, use `as const` for sample state, flatten test structure, and require full state in deserialize.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| mnemonic: array(number()), | ||
| numberOfAccounts: union([literal(0), literal(1)]), | ||
| hdPath: literal(MONEY_DERIVATION_PATH), | ||
| }); |
There was a problem hiding this comment.
Struct permits empty mnemonic that fails on deserialize
Low Severity
MoneyKeyringSerializedStateStruct allows mnemonic: [] (empty array), which is exactly what serialize() produces for a freshly constructed keyring. However, passing this state to deserialize() will throw, because the parent HdKeyring.deserialize treats [] as truthy (if (opts.mnemonic)) and calls #initFromMnemonic([]), which converts it to an empty string and fails BIP39 mnemonic validation. The struct validates a state that can't actually be round-tripped.
Additional Locations (1)
2 participants
The
MoneyKeyringserialized state was previously unvalidated.serializereturned the parent's full state anddeserializeaccepted a loosely typed partial object.This adds a superstruct schema (
MoneyKeyringSerializedStateStruct) that enforces the serialized state shape:mnemonic:number[]numberOfAccounts:0 | 1hdPath: the exactMONEY_DERIVATION_PATHliteralBoth
serializeanddeserializenow assert against this schema. TheMoneyKeyringSerializedStatetype is inferred from the struct viaInfer.Examples
N/A - this is an internal validation change with no external usage yet.
Note
Medium Risk
Tightens
MoneyKeyringserialize/deserializecontracts and will now reject previously accepted malformed state, which could break callers persisting older/loose formats. Risk is moderate but contained to a single keyring package and is covered by updated tests.Overview
MoneyKeyringnow validates its serialized state with asuperstructschema and exposes a newMoneyKeyringSerializedStatetype.serialize()asserts the parent state matches{ mnemonic: number[], numberOfAccounts: 0|1, hdPath: MONEY_DERIVATION_PATH }, anddeserialize()now requires and validates this exact shape (instead of coercing/overriding invalid inputs).Tests were updated to use byte-array mnemonics, cover
numberOfAccounts: 0, and verify invalidmnemonic/numberOfAccounts/hdPathare rejected; the package also exports the new type and adds@metamask/superstructas a dependency (with changelog updated).Written by Cursor Bugbot for commit c251ebd. This will update automatically on new commits. Configure here.