Fixed NEAR verifyMessage accessing undefined ed25519Key property#78
Fixed NEAR verifyMessage accessing undefined ed25519Key property#78SergeyG-Solicy wants to merge 1 commit intomainfrom
Conversation
Review Summary by QodoFix NEAR verifyMessage ed25519Key property access error
WalkthroughsDescription• Fixed incorrect property access in NEAR verifyMessage method • Removed unnecessary nested property access to ed25519Key • Simplified publicKey decoding logic for correct verification Diagramflowchart LR
A["publicKeyObject.ed25519Key.data<br/>(incorrect)"] -->|"simplify"| B["publicKeyObject.data<br/>(correct)"]
B -->|"create Uint8Array"| C["publicKeyDecoded"]
C -->|"use in verification"| D["nacl.sign.detached.verify"]
File Changes1. src/multichain/core/near.ts
|
Code Review by Qodo
1. Missing .beads/issues.jsonl update
|
WalkthroughModified the public key decoding logic in the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/multichain/core/near.ts (1)
269-276:⚠️ Potential issue | 🟡 MinorAdd explicit validation for Ed25519 key curve before verification.
The
verifyMessagefunction at line 270 directly passes the public key tonacl.sign.detached.verify(), which only supports Ed25519 signatures. While NEAR supports bothed25519andsecp256k1key curves (as evidenced by thecreateAccountmethod allowing both), this function will silently fail or throw an opaque error if a secp256k1 key is provided. Add an explicit check to reject unsupported key curves upfront with a clear error message.Suggested fix
const signatureDecoded = utils.serialize.base_decode(signature); const publicKeyObject = utils.key_pair.PublicKey.from(publicKey); + if (!publicKey.startsWith("ed25519:")) { + throw new Error( + `Unsupported NEAR public key curve for verifyMessage: expected ed25519, got ${publicKey.split(":")[0] || "unknown"}`, + ) + } const publicKeyDecoded = new Uint8Array(publicKeyObject.data); const messageBytes = decodeUTF8(message);This aligns with the coding guideline "Provide clear, actionable error messages" and prevents silent failures when non-Ed25519 keys are used.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/multichain/core/near.ts` around lines 269 - 276, In verifyMessage, ensure you explicitly validate the public key curve before attempting signature verification: inspect the utils.key_pair.PublicKey (or its string/enum form) to confirm the key curve is "ed25519" and throw a clear, descriptive error (e.g., "unsupported key curve: expected ed25519") if it's not; only then decode the PublicKey into publicKeyDecoded and call nacl.sign.detached.verify with messageBytes, signatureDecoded, publicKeyDecoded. This prevents passing secp256k1 keys (or others) into nacl.sign.detached.verify and yields an actionable error instead of an opaque failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/multichain/core/near.ts`:
- Around line 269-276: In verifyMessage, ensure you explicitly validate the
public key curve before attempting signature verification: inspect the
utils.key_pair.PublicKey (or its string/enum form) to confirm the key curve is
"ed25519" and throw a clear, descriptive error (e.g., "unsupported key curve:
expected ed25519") if it's not; only then decode the PublicKey into
publicKeyDecoded and call nacl.sign.detached.verify with messageBytes,
signatureDecoded, publicKeyDecoded. This prevents passing secp256k1 keys (or
others) into nacl.sign.detached.verify and yields an actionable error instead of
an opaque failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3f3635e6-b4f8-4b77-a4f7-cb55e758695a
📒 Files selected for processing (1)
src/multichain/core/near.ts
1 participant
Summary by CodeRabbit