Skip to content

fix(mobile): a classic wallet's key can be revealed, behind the lock - #1114

Open
MrNova50 wants to merge 1 commit into
Miracle656:mainfrom
MrNova50:fix/1041-classic-secret-reveal
Open

MrNova50 wants to merge 1 commit into
Miracle656:mainfrom
MrNova50:fix/1041-classic-secret-reveal

Conversation

@MrNova50

Copy link
Copy Markdown

What happened before

A phone that cannot create a passkey — the reported Redmi Note 11 5G answers createCredentialAsync with no provider dependencies found — only ever gets the classic keypair wallet. Spending works for that wallet: spendAsset takes the classic branch when the stored address is not a C…, and requirePasskey() no-ops when there is a signer secret and no passkey. Recovery did not:

  • no screen anywhere could show or export the secret;
  • lib/backup.ts strips private key material on purpose ("the last line of defence"), so the encrypted backup cannot carry it;
  • the secret existed in exactly one keychain entry (veil_signer_secret).

So uninstalling the app, losing the phone, or a corrupted keychain destroyed the wallet silently, with no warning anywhere in the UI.

What happens now

Settings shows a Reveal secret key row when the stored address is a G-account. Tapping it walks a fixed order, and each step gates the next:

  1. identify the wallet and state its address in full;
  2. state the consequences — anyone who has this secret can move everything in this wallet; holding the key is holding the money; this is the only copy; Veil cannot recover it; anything you paste it into keeps a copy;
  3. require an explicit acknowledgement before the button enables;
  4. pass the device lock;
  5. only then display the secret — full, selectable, never truncated.

Leaving the screen, backgrounding the app, or tapping Hide it destroys what was displayed and returns to step 3.

Files: frontend/mobile/lib/secretExport.ts (new), frontend/mobile/app/settings/reveal-secret.tsx (new), frontend/mobile/app/(tabs)/settings.tsx (the row), two new test files, docs/MOBILE_RECOVERY.md.

Which lock, and why that one

requirePasskey() is useless here — it deliberately no-ops for exactly the wallets being served (keypair wallet, no passkey). The gate is expo-local-authentication, the same module app/lock.tsx and hooks/useInactivityLock.ts use, so "behind the app lock" means the device biometric or passcode the user already enrolled.

One deliberate divergence from app/lock.tsx. The idle lock lets a user with no screen lock through, which is right: an unanswerable lock is a door that will not open, and passing it loses nothing. A secret reveal is the opposite case — displaying a key behind a gate with nothing on the other side would tell the user their key was protected when it was never protected at all. So a device with nothing enrolled is a refusal, with the reason stated in the UI: set a device passcode first, otherwise anyone holding the phone would hold your money. That is UnlockOutcome = 'not-protected', and it is tested.

"Revealed once" — how I read it, and the alternative I rejected

I implemented one reveal at a time, not a durable "already exported" flag. A key is only useful if it can be written down, photographed, and re-entered after a reinstall; a permanent one-shot flag means one distraction, one failed screenshot, or one dead battery strands the funds with no way back — and the screen would then claim a recovery path it just destroyed. Re-showing costs a fresh acknowledgement and a fresh unlock every time, and nothing is displayed while either is outstanding. If the intent was a single lifetime reveal, say so and I will change it: it is a small edit, but the failure mode is irreversible, so I did not guess it in code.

The hazard that decided the design: signerSecret is polymorphic

lib/walletStore.ts's signerSecret slot holds the wallet's own key for a keypair wallet and the PRF-derived fee-payer for a smart wallet. A reveal that trusted the slot alone would dump a passkey wallet's fee-payer and call it a backup. So resolveSecretExport() requires both:

  • the stored address classifies as a keypair account (mirroring the C… check lib/spendAsset.ts makes), and
  • Keypair.fromSecret(storedSecret).publicKey() equals the stored address.

Everything else is a refusal with its own copy: smart-wallet, secret-mismatch (a malformed secret also lands here — it refuses rather than throwing at the screen), no-secret, unknown-address, no-wallet. A smart-wallet refusal never reads the fee-payer, and the returned shape is asserted whole in tests, so the key is unreachable from the value, not merely hidden by the UI. The second condition is also what makes the round trip true by construction: a revealed key is always the key that derives the address shown beside it.

Acceptance criteria

Criterion Where
A keypair wallet's secret can be revealed once, behind the lock reveal-secret.tsx; screen tests prove the button is disabled before acknowledgement, a dismissed prompt reveals nothing, an unenrolled device reveals nothing, and the revealed value equals the stored secret exactly
A smart wallet does not offer this the Settings row is gated on walletKind(address) === 'keypair'; the screen renders a refusal with no reveal button, and the fee-payer's secret is absent from the rendered text
The backup scrubber still rejects secrets, test intact lib/backup.ts and lib/__tests__/backup.test.ts are not in this diff. New tests assert assertNoSecretMaterial still rejects signerSecret / privateKey, and that a revealed wallet's own backup contains no key — encrypting metadata plus the secret throws BackupError
Export → reinstall → import restores the same address proven at the lib level (createTestnetWallet → unlock → resolve → clearWalletStore → importTestnetWallet(secret)) and end-to-end at the screen level, including a migration case where the revealed key is imported over leftover smart-wallet state and the stale fee-payer stays unreachable

Tests

cd frontend/mobile && npm test
  • lib/__tests__/secretExport.test.ts — 20 tests
  • __tests__/revealSecretScreen.test.tsx — 9 tests
  • full mobile suite: 96 suites, 1157 tests, all passing
  • npx tsc --noEmit -p tsconfig.typecheck.json clean; eslint 0 errors. No changeset needed — the mobile app is not a publishable package.

The reveal introduces no new storage keys and holds no key in component state before the lock is passed; one test scans every AsyncStorage key and value after a reveal and asserts nothing was written.

Mainnet safety

Nothing here touches registration, key derivation, or signing, and no contract call is involved: this reads a keypair secret already in the keychain and displays it. A wallet's address is unchanged by revealing it. The risk I did weigh is the opposite of a stranding bug — a revealed key is a secret placed in the user's hands, so the copy states the exposure plainly, warns about clipboard and screenshot retention, and re-conceals on background.

Not verified on this machine — please treat as unconfirmed

There is no Android/Gradle/device toolchain here, so the following were simulated in Jest, not run on hardware:

  • the real expo-local-authentication prompt on a Redmi Note 11 5G, including what getEnrolledLevelAsync reports on a Mi with only a screen PIN, and whether disableDeviceFallback: false behaves as intended on that OEM;
  • an actual reinstall and re-import on a device;
  • that the screen lays out cleanly at small widths with a full 56-character secret (styles follow app/settings/danger.tsx, but no screenshot exists).

Following docs/MOBILE_PRF_FINDINGS.md, I am reporting these as gaps rather than claiming them. This needs one device run before it is trusted.

Two related defects I deliberately did not touch

Both fall outside the acceptance criteria and would widen the diff, so they are observations, not changes:

  1. app/settings/danger.tsx tells a keypair-wallet user that "a backup lets you restore this wallet" before deleting the wallet. For a classic wallet the encrypted backup carries no key, so that sentence is false, and this screen is the only thing that makes it fixable.
  2. app/settings/recovery.tsx lists three recovery mechanisms and reports none of them as ready for a classic wallet — correct, but it reads like a broken setup. It should point at this screen.

Related issue

Closes #1041

Type of change

  • Bug fix
  • New feature
  • Refactor
  • Docs
  • Tests
  • CI / tooling

Component

  • Wallet frontend
  • SDK
  • Contracts
  • Agent

Checklist

  • I have read CONTRIBUTING.md
  • cargo test passes (contracts) — not affected by this diff
  • npm run typecheck passes (wallet / sdk / agent) — mobile tsconfig.typecheck.json is clean
  • npm run build passes (wallet / agent) — not run here, no device/Expo build toolchain; the mobile suite and typecheck are the substitute
  • I added or updated tests where relevant
  • I updated docs / README where relevant

Screenshots / test output

No screenshots — see "Not verified on this machine" above. Jest output:

PASS lib/__tests__/secretExport.test.ts
PASS __tests__/revealSecretScreen.test.tsx
Test Suites: 2 passed, 2 total
Tests:       29 passed, 29 total

Full workspace: Test Suites: 96 passed, 96 total
                Tests:       1157 passed, 1157 total

@MrNova50
MrNova50 requested a review from Miracle656 as a code owner October 17, 2026 01:39
@vercel

vercel Bot commented Oct 17, 2026

Copy link
Copy Markdown

@MrNova50 is attempting to deploy a commit to the miracle656's projects Team on Vercel.

A member of the Team first needs to authorize it.

@drips-wave

drips-wave Bot commented Oct 17, 2026

Copy link
Copy Markdown

@MrNova50 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! 🚀

Learn more about application limits

Some phones cannot make a passkey at all — a Redmi Note 11 5G answers
createCredentialAsync with "no provider dependencies found" — so those
users only ever get the classic keypair wallet. Spending works for it;
recovery does not. Its secret lives in exactly one keychain entry, the
encrypted backup strips key material by design, and there was no screen
anywhere that could put it in the user's hands. Uninstalling the app
destroyed the wallet silently.

Settings now offers "Reveal secret key" when the stored address is a
G-account, and a one-time reveal behind the device lock: identify the
wallet, state the consequences, make the user acknowledge them, then
authenticate. Leaving the screen or backgrounding the app destroys what
was shown, so showing it again costs another unlock.

The guard is address classification plus deriving the public key from
the stored secret and requiring it to equal the stored address. That
matters because lib/walletStore's signerSecret slot is polymorphic: for
a keypair wallet it is the wallet's own key, for a smart wallet it is
the PRF-derived fee-payer. A passkey wallet therefore gets a refusal and
its fee-payer is never read, and a corrupted pairing gets a refusal too.

lib/backup.ts and its test are untouched: the scrubber still rejects
secrets, which is what keeps a cloud-synced backup safe.
@MrNova50
MrNova50 force-pushed the fix/1041-classic-secret-reveal branch from 45a8de2 to d1e6c6f Compare October 17, 2026 08:58
@MrNova50

Copy link
Copy Markdown
Author

Rebased onto current main to pick up c01341c (fix(ci): regenerate the mobile lockfile so npm ci works again) — 45a8de2 → d1e6c6f.

The three red checks on the previous head (Mobile — typecheck & test, Maestro — Android emulator, Lockfiles) were not code failures: npm ci aborted in frontend/mobile before tests ran, because invisible-wallet-sdk@0.2.0 was missing from the then-current lockfile. That is exactly what c01341c fixed upstream. No code in this PR changed in the rebase — git diff between the two heads, limited to frontend/mobile, is the lockfile regeneration only.

Re-verified against the new base: full mobile suite green (96 suites, 1157 tests) on this branch's code, plus a targeted rerun of lib/__tests__/secretExport.test.ts, __tests__/revealSecretScreen.test.tsx and lib/__tests__/backup.test.ts — 54/54 — and tsc -p tsconfig.typecheck.json --noEmit clean.

The rerun workflows currently show action_required, so they need a maintainer's approval to execute. Nothing else to report.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A classic wallet's key cannot be backed up, so a lost phone is a lost wallet

1 participant