feat: add Capacitor mobile biometric starter and encrypted seed phrase recovery flow#249
Conversation
|
@codegeni3 is attempting to deploy a commit to the miracle656's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@codegeni3 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Miracle656
left a comment
There was a problem hiding this comment.
Thanks for the thorough test snapshots and the factory on-curve validation — but there's a critical security issue in the contract change that blocks this, and the PR scope needs splitting.
🔴 Blocking: new raw-ECDSA auth path bypasses WebAuthn + uses unsafe transmute
The new parts.len() == 3 branch in __check_auth adds a second authentication path that verifies a raw P-256 signature over the payload directly, with no WebAuthn ceremony:
let payload_hash: soroban_sdk::crypto::Hash<32> =
unsafe { core::mem::transmute(signature_payload) };
env.crypto().secp256r1_verify(&public_key, &payload_hash, &sig_bytes);Two serious problems:
-
unsafe { core::mem::transmute(...) }in the wallet's auth path is not acceptable.BytesN<32>andHash<32>are distinct types with no layout guarantee — this is UB-adjacent and will break on any SDK internal change. There is a safe construction; follow the pattern the existing 5-part WebAuthn branch already uses to obtain the digest, rather than transmuting. -
It bypasses the entire WebAuthn security model. The 5-part path binds
authenticatorData(rpIdHash),clientDataJSON(origin + challenge), and the host-computed payload. This new path checks none of that — it accepts any raw signature over the payload from a registered key. For a passkey wallet whose private key is non-extractable in a secure enclave, a raw-signature path is both unusable in practice and a real attack-surface expansion if a key is ever exported. This is a fundamental change to the threat model and can't ride in on a mobile-starter PR.
🟠 Scope
This PR mixes (a) contract __check_auth changes, (b) factory validate_public_key hardening, (c) ~50 generated test_snapshots, (d) a binary mock_wallet.wasm, and (e) a Capacitor starter + "encrypted seed-phrase recovery." Please split:
- The factory on-curve check (
from_sec1_bytes) is a good standalone change — send it as its own small PR. Note it contradicts the existing comment about skipping validation for CPU cost, so update that comment. - The Capacitor starter + recovery UI can be its own PR with no contract changes.
- Drop the contract
__check_authchange entirely unless there's a concrete, reviewed design for non-WebAuthn signing.
Happy to fast-track the factory validation split.
Miracle656
left a comment
There was a problem hiding this comment.
Thanks for syncing with main, but the merge commit (48f4ff3d) doesn't address any of the review feedback — the blocking items are unchanged, and there's now a new conflict:
unsafe { core::mem::transmute(signature_payload) }is still in__check_auth. This is the hard blocker — please remove it; there's a safe way to obtain the digest (see how the existing WebAuthn branch does it).- The raw-ECDSA
parts.len() == 3branch still bypasses the WebAuthn ceremony (no origin / rpId / challenge binding). - New collision: since you merged main, the contract now has two
parts.len() == 3branches — #247 landed an ed25519 session-key branch ([key_id, ed25519_sig, nonce]) that's also 3 elements, and yours is[public_key, sig_bytes, nonce]. They're mutually exclusive; whichever matches first wins. The contract auth path can't have both.
As I noted before, the cleanest path is to drop the contract __check_auth change entirely and ship this as just the Capacitor starter + recovery UI (no contract changes). The factory validate_public_key on-curve check is a good standalone — send that as its own small PR. Right now the wallet auth changes can't be merged.
Happy to review the Capacitor-only version quickly.
…th new test snapshots and updated service worker configuration
Miracle656
left a comment
There was a problem hiding this comment.
Big improvement — the blocking security issues are resolved: the unsafe transmute is gone, the raw-ECDSA bypass branch is gone, and you're now building on the ed25519 session keys from #247 without weakening them. Thank you for that.
Two things still stand between this and merge, both mechanical:
🔴 1. Remove committed build artifacts
frontend/wallet/public/sw.js(the +1/−101 churn) — generated by the PWA buildcontracts/factory/test-fixtures/mock_wallet.wasm— binary blob
git rm --cached these and make sure they're gitignored; they shouldn't be in the diff.
🟠 2. Split the PR — it's 78 files / +10,834
Right now this bundles at least four separable things. Please break it up:
- A) Capacitor starter (
examples/capacitor/**) — its own PR, no contract changes - B) Recovery flow (
frontend/wallet/lib/recovery.ts+recover/settingspage changes) — its own PR - C) Factory
validate_public_keyon-curve check — tiny standalone PR (this one I'll merge fast) - Please also drop the re-indentation churn on
session_key.rs's tests — those just landed via #247 and don't need reformatting here.
Each of those is reviewable and mergeable on its own; as one 78-file PR it isn't. I've approved CI so the Rust build runs. Once it's split + artifacts removed, I'll merge the pieces quickly.
Miracle656
left a comment
There was a problem hiding this comment.
Merging — critical security items resolved (no transmute, no raw-ECDSA bypass; #247's ed25519 session keys intact) and CI is green (Rust build & test, SDK, frontend, agent all pass). Cleaning up the committed PWA build artifacts on main as a follow-up.
Overview
This PR implements mobile biometric Passkey integration via Capacitor and introduces an opt-in encrypted seed phrase recovery fallback for the smart wallet.
Changes
1. Capacitor Mobile Biometrics Starter
examples/capacitor/.examples/capacitor/src/shim.ts) that interceptsnavigator.credentialsand drives iOS & Android biometric prompt flows using@capgo/capacitor-native-biometric.examples/capacitor/src/App.tsx) illustrating registration, wallet contract deployment, and a full mock payment flow.2. Encrypted Seed Phrase Fallback (Recovery Flow)
frontend/wallet/lib/recovery.tssupporting BIP-39 mnemonic generation, key derivation, AES-GCM encryption/decryption, and IndexedDB storage.frontend/wallet/app/settings/page.tsx) enabling users to opt-in, write down their seed phrase, and encrypt it locally while registering the derived public key as a contract signer.frontend/wallet/app/recover/page.tsx) allowing users to restore their wallet using their 12-word recovery phrase.3. Contract Upgrades & Bugfixes
__check_authin the smart contracts to support direct P-256 signatures (parts.len() == 3branch) from the BIP-39 recovery key.Vec::from_array, and added low-S signature normalization across test fixtures to align with Soroban v22 validation constraints.