Feat/cosmos enigma utils OK-50764 OK-50786 OK-50962#10456
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
ce2cf6b to
2f869e2
Compare
2f869e2 to
7649397
Compare
…etNetworkEncryption
2640845 to
1f0691f
Compare
| @providerApiMethod() | ||
| public async getEnigmaPubKey( | ||
| request: IJsBridgeMessagePayload, | ||
| params: { chainId: string }, | ||
| ): Promise<string> { | ||
| const utils = await this._getOrCreateEnigmaUtils(request, params.chainId); | ||
| const pubkey = await utils.getPubkey(); | ||
| return bytesToHex(pubkey); | ||
| } |
There was a problem hiding this comment.
🔴 Missing @permissionRequired() decorator on Enigma provider methods breaks security pattern
All existing Cosmos provider methods that perform sensitive cryptographic operations (signAmino, signDirect, sendTx, signArbitrary, verifyArbitrary) are decorated with @permissionRequired() to ensure the dApp has an approved connection before proceeding. The four new Enigma methods (getEnigmaPubKey, enigmaEncrypt, enigmaDecrypt, enigmaGetTxEncryptionKey) perform cryptographic operations using the user's private key-derived seed but lack this decorator. While _getOrCreateEnigmaUtils internally calls _getAccount → getAccountsInfo which may fail for unconnected dApps, the missing decorator breaks the defense-in-depth pattern established throughout the codebase. The CLAUDE.md rule states: "Crypto operations MUST follow established patterns" and "Transaction verification and risk detection MUST NOT be bypassed".
Prompt for agents
Add the @permissionRequired() decorator to all four Enigma provider methods in packages/kit-bg/src/providers/ProviderApiCosmos.ts to match the existing pattern used by signAmino, signDirect, sendTx, signArbitrary, and verifyArbitrary. Specifically, add @permissionRequired() before @providerApiMethod() on the following methods:
1. getEnigmaPubKey (line 773)
2. enigmaEncrypt (line 783)
3. enigmaDecrypt (line 793)
4. enigmaGetTxEncryptionKey (line 806)
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbf22ef6b2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!password) { | ||
| const result = (await this.backgroundApi.serviceDApp.openModal({ | ||
| request, | ||
| screens: [ | ||
| EModalRoutes.DAppConnectionModal, |
There was a problem hiding this comment.
Fail fast on unsupported wallets before password prompt
_getOrCreateEnigmaUtils asks for a password before checking whether the current keyring can derive an Enigma seed, but VaultCosmos.getEnigmaSeed throws for unsupported wallet types (e.g., hardware/watching). In that scenario, users are forced through an unlock modal and then still get a hard failure, which is a dead-end flow for those wallets; the support check should happen before opening the unlock modal.
Useful? React with 👍 / 👎.
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
| new PolyfillCryptoProvider(), | ||
| ); | ||
| const plaintext = new TextEncoder().encode( | ||
| contractCodeHash + JSON.stringify(msg), |
There was a problem hiding this comment.
🟡 JSON.stringify() used for cryptographic encryption plaintext, violating CLAUDE.md rule
CLAUDE.md explicitly states: "NEVER use JSON.stringify() for cryptographic operations → ALWAYS use stringUtils.stableStringify() for deterministic serialization". At SecretNetworkEncryption.ts:84, JSON.stringify(msg) is used to serialize the message object before AES-SIV encryption, which is a cryptographic operation. While the security impact is minimal (this is encryption plaintext, not a hash/signature input), and JSON.stringify matches the secret.js reference implementation for protocol compatibility, it is an explicit violation of the repository's mandatory rules.
Note on protocol compatibility
The Secret Network protocol and secret.js reference implementation use standard JSON.stringify. Switching to stableStringify could change key ordering and break compatibility with Secret Network contracts. If this is intentional for protocol compatibility, it should be documented with a comment explaining the exception.
| contractCodeHash + JSON.stringify(msg), | |
| // NOTE: Using JSON.stringify (not stableStringify) intentionally to match | |
| // the Secret Network / secret.js wire format for contract message encoding. | |
| contractCodeHash + JSON.stringify(msg), |
Was this helpful? React with 👍 or 👎 to provide feedback.
* feat: support cosmos enigma utils * feat: update CosmosEnigmaUnlockModal * feat: support unisat asset api * chore: update onekey inprovider version * fix: add type declarations for miscreant to fix tsgo build error * fix: resolve lint errors in miscreant types, ProviderApiBtc, and SecretNetworkEncryption --------- Co-authored-by: Leon <lixiao.dev@gmail.com>
OneKeyHQ/cross-inpage-provider#448
OneKeyHQ/cross-inpage-provider#449
OneKeyHQ/cross-inpage-provider#450