fix: move peer key validation into IKeyManager interface (WAPI-1116)#70
Open
chakra-guy wants to merge 9 commits intomainfrom
Open
fix: move peer key validation into IKeyManager interface (WAPI-1116)#70chakra-guy wants to merge 9 commits intomainfrom
chakra-guy wants to merge 9 commits intomainfrom
Conversation
Add validateSecp256k1PublicKey utility that checks for exactly 33 bytes and a 0x02/0x03 prefix. Applied at all public key ingestion points: wallet-client session creation, both dapp-client connection handlers, and SessionStore deserialization. Also guards against NaN in session expiry timestamps on both set and get.
5 tasks
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Tests used Uint8Array(33) filled with zeros (0x00 prefix) which is now rejected by the secp256k1 public key validation added in this PR. Updated to use 0x02 prefix for a valid compressed key format.
If validateSecp256k1PublicKey threw on a malformed peer key, the wallet client state would be stuck at CONNECTING with no cleanup, permanently bricking the instance. Moving _createSession into the try block ensures disconnect() runs on validation failure.
adonesky1
reviewed
Feb 25, 2026
The previous approach leaked secp256k1-specific validation into the core package, breaking its crypto-agnostic architecture. This moves validation behind the IKeyManager interface so each implementation validates keys according to its own crypto system. - Add validatePeerKey() to IKeyManager interface - Delete standalone validateSecp256k1PublicKey utility - Remove validation from SessionStore.get() (validated at ingestion) - Add validation in BaseClient.resume() for persisted sessions - Wire keymanager into dapp-client handler context - Use eciesjs PublicKey.fromHex() for curve-point validation
chakra-guy
changed the title
- Core changelog: categorize validatePeerKey as Changed (breaking interface addition), keep NaN guard under Fixed - Dapp/wallet-client changelogs: recategorize peer key validation entries as Fixed - Wallet-client: reset state to DISCONNECTED when _createSession throws before session is assigned, preventing permanent CONNECTING state
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds peer public key validation at handshake and session resume time by introducing a
validatePeerKey(key: Uint8Array): voidmethod on theIKeyManagerinterface. This keeps the core package crypto-agnostic while letting each implementation validate keys according to its own crypto system.IKeyManagergainsvalidatePeerKey()- validates peer keys at ingestion time_createSession(), dapp client validates in both connection handlersBaseClient.resume()validates persisted peer keys before reconnectingIConnectionHandlerContext(dapp-client) now exposeskeymanagerfor handler-level validationsetandget)INVALID_KEYerror code inErrorCodeenumeciesjs.PublicKey.fromHex()for full curve-point validation (no new dependencies)Breaking changes
IKeyManagerhas a new required method:validatePeerKey(key: Uint8Array): voidJira
Test plan
PublicKey.fromHex()rejects: wrong length, wrong prefix, not-on-curve, emptyNote
Medium Risk
Introduces a breaking API change (
IKeyManager.validatePeerKey) and enforces key validation during handshake/resume, which can surface new runtime failures if implementations are incomplete or overly strict. Session expiry parsing now treats non-numeric/NaNvalues as expired, potentially deleting previously-stored malformed sessions.Overview
Adds peer public-key validation as a first-class requirement.
IKeyManagernow includesvalidatePeerKey(key), and dapp/wallet flows call it when ingesting peer keys (wallet session creation, dapp handshake offer handling, andBaseClient.resume()for persisted sessions), with demo/tests updated to implement the method (ECIES curve-point validation in real managers; no-op in load tests).Hardens session persistence against invalid expiry timestamps.
SessionStorenow rejects saving sessions withNaNexpiry and treats non-numeric/NaNexpiresAtvalues as expired on read (auto-deleting them), with new tests covering these cases; wallet connect error handling also avoids leaving the client stuck inCONNECTINGwhen session creation fails.Written by Cursor Bugbot for commit 460249c. This will update automatically on new commits. Configure here.