Recovery export hardening: screen-capture failsafe + inline error surface + Solana Pay locale comma#30
Merged
epicexcelsior merged 6 commits intoMay 10, 2026
Conversation
… key ExportWalletModal previously fired preventScreenCaptureAsync inside a mounted-effect and forgot the result. The .catch(() => undefined) swallowed any rejection — older OS, sandbox issue, race with another capture-protected screen — and the modal still rendered the secret with its "SCREENSHOTS BLOCKED" hint while screenshots were in fact fully unblocked. The user could leak their base58 recovery key in a single screenshot with no UI signal that protection had failed. Track the prevention promise's resolved state across three values: pending, blocked, unavailable. On unavailable, refuse to render the secret entirely (replace KeyBox with an "Secure window unavailable" explainer panel) and gate authenticate() so the biometric prompt never fires. On AppState foreground, re-apply prevention because older Android versions can drop FLAG_SECURE while the app is backgrounded; if re-application fails, scrub any rendered secret immediately. KeyBox grows a captureReady prop and shows a "PREPARING SECURE WINDOW…" panel while pending so the user does not authenticate before the secure window is in place. Cleanup of the AppState listener and allowScreenCaptureAsync() runs on unmount as before.
WalletContext.exportPrivateKey returned `string | null` and folded
two distinct outcomes into the null case: user-cancelled biometric
(silent intent) and real system failure (keychain read failed,
decrypt failed, etc.). On a real failure it surfaced the reason
through a separate Alert.alert("Export failed", msg) popup that
appeared on top of the modal sheet, while the modal itself moved
to a generic "KEY UNAVAILABLE / Try again when ready" failed-state
card. The user got "try again" alongside an alert dialog the modal
was visually covering — so they retried, hit the same failure,
saw the alert again. Recovery is the highest-trust surface in the
app; that experience erodes confidence in the wallet itself.
Change the return to a tagged union: { ok: true; secretKey } for
success, { ok: false } for user-cancel (no message, modal stays in
initial state silently), { ok: false; message } for real failures.
The modal stores the message in failMessage state and KeyBox renders
it inline below the KEY UNAVAILABLE label, with a TAP TO RETRY
affordance on the same surface. Drop the Alert.alert call.
Solana Pay spec mandates "." as the decimal separator in the amount URI parameter. Locales that surface a decimal-pad keyboard with comma (de-DE, fr-FR, pt-BR, es-ES) emit "0,5" from the receive amount field. The previous regex rejected the comma and the URI was built with no amount= parameter at all, so any wallet scanning the QR (Phantom, Solflare, Backpack) prefilled with no amount and the user got a "no amount requested" experience even though they had explicitly entered one. Swap "," for "." inside normalizeAmount before regex validation. The output URI is always spec-compliant. Three locale-comma cases added to validate-tier0-services covering plain comma, comma plus trailing zeros, and the whitespace-padded comma shape that real TextInput delivers on keyboard dismiss.
PR anonmesh#29 removed the H_PAD constant when moving PendingCosigns into WalletScreen's grid (parent now owns horizontal padding) but left one reference at line 57. Result: upstream/v3 fails tsc on a fresh clone. Drop the stale paddingHorizontal entry from the wrap View style array and consolidate the two duplicate @expo/vector-icons imports while in the file.
Contributor
There was a problem hiding this comment.
Pull request overview
Hardens the recovery-key export modal by gating secret rendering on confirmed screen-capture blocking and by surfacing export failures inline, while also improving Solana Pay URI generation for comma-decimal locales and bundling a small v3 tsc hotfix.
Changes:
- Add capture-block state tracking and AppState re-apply logic for recovery export, plus inline failure messaging instead of a separate
Alert. - Normalize comma-decimal
amountinputs when building Solana Pay URIs and extend tier0 service validations. - Remove a stale padding reference and consolidate icon imports in
PendingCosigns.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| mobile_app/src/services/solanaPayUri.ts | Normalizes a single comma decimal separator to . before validating/encoding amount. |
| mobile_app/scripts/validate-tier0-services.mjs | Adds service-level assertions covering comma-decimal amount normalization. |
| mobile_app/context/WalletContext.tsx | Changes exportPrivateKey to return a tagged union with optional failure messaging. |
| mobile_app/components/settings/KeyBox.tsx | Adds capture-readiness and inline failure message rendering; improves retry UX on failure. |
| mobile_app/components/settings/ExportWalletModal.tsx | Implements capture-block state machine + foreground re-apply; gates auth; swaps to inline error surface. |
| mobile_app/components/nodes/PendingCosigns.tsx | Removes stale H_PAD padding usage and consolidates vector icon imports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
applyBlock() didn't reset captureBlock to 'pending' before re-applying prevention on foreground, so the in-flight window between resume and confirmed preventScreenCaptureAsync() could render the secret with stale 'blocked' state — screenshots possibly unblocked while UI claims otherwise. Three changes, belt-and-suspenders: - Pessimistically reset captureBlock to 'pending' at top of applyBlock so any in-flight reapply visibly gates render and auth. - Extend the secret-scrub effect to fire on any non-'blocked' state (was 'unavailable' only). Drops the in-memory secret on resume so a brief stale frame can't leak the key. - KeyBox renders 'preparing secure window' whenever captureReady is false, regardless of whether a stale secret prop survived a render. Also adds the multi-comma rejection assertion the PR body promised: '1,2,3' must not produce amount= in the Solana Pay URI.
Collaborator
Author
|
Update — pushed captureBlock race on AppState resume.
Multi-comma rejection assertion. PR body promised it; |
7 tasks
This was referenced May 8, 2026
Without this, the 1.4s "COPIED" badge survived a copy → background → resume cycle: capture-block dropped to pending, secret was scrubbed, but keyCopied state remained true. Next reveal flashed a stale COPIED badge on a freshly re-authenticated session. Also clear keyCopied at the start of authenticate() as belt-and-suspenders in case a user retries faster than the 1.4s timeout. Add a recipient-only URI test to make the no-amount path explicit, and reword the multi-separator test comment so it doesn't claim locale-grouped numbers (1.234,56) are universally invalid — they aren't, but the receive screen never feeds them.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
mobile_app/components/settings/ExportWalletModal.tsx:193
- The UI below KeyBox (hint text/ack checkbox/DONE disable) is gated only on
!!secretKey. IfcaptureBlockflips to'pending'on AppState resume, there can be a render wheresecretKeyis still set butcaptureReadyis false (KeyBox shows “PREPARING SECURE WINDOW…”), yet the hint still claims “SCREENSHOTS BLOCKED” anddismiss()is temporarily blocked bysecretKey && !copiedAck. Consider gating these secret-only elements oncaptureBlock === 'blocked'(orcaptureReady) as well to keep the UI consistent during the scrub window.
{!!secretKey && (
<>
<Text style={[S.hint, { color: colors.textTertiary, textAlign: 'center' }]}>
HOLD TO REVEAL · SCREENSHOTS BLOCKED · BASE58 ENCODED
</Text>
Comment on lines
192
to
+196
| } catch (err) { | ||
| const msg = err instanceof Error ? err.message : String(err); | ||
| if (msg === 'Authentication cancelled') return null; | ||
| if (msg === 'Authentication cancelled') return { ok: false }; | ||
| console.error('[wallet/exportPrivateKey] failed:', msg, err); | ||
| Alert.alert('Export failed', msg); | ||
| return null; | ||
| return { ok: false, message: msg }; |
| assert.equal( | ||
| buildSolanaPayUri({ recipient: "11111111111111111111111111111111" }), | ||
| "solana:11111111111111111111111111111111?label=AnonMesh&message=AnonMesh+receive", | ||
| "missing amount must produce a recipient-only URI", |
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
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
Recovery-export modal hardening + Solana Pay receive locale fix. Three security/UX items from the PR #28 review batch that were deliberately split off because they touch the highest-trust surface in the app (the recovery key reveal). One small bonus fix unblocks
tsconv3.What's in this PR
Recovery export hardening
ExportWalletModal.tsx,KeyBox.tsx) —preventScreenCaptureAsyncresolved-state is now tracked in a three-state machine (pending/blocked/unavailable). The recovery key refuses to render unless capture has been confirmed blocked. AnAppStateforeground listener re-applies prevention on resume, so backgrounding then foregrounding can't slip a screenshot. When prevention can't be applied,KeyBoxshows a "Secure window unavailable" panel instead of the secret.WalletContext.tsx,KeyBox.tsx) —exportPrivateKeynow returns a tagged-union{ ok: true, secretKey } | { ok: false, message? }instead ofstring | null. Real failures (system error, biometric hardware fault) surface inline inKeyBoxvia thefailedstate. User-cancel returns silently with no message. The secondaryAlert.alertpopup is dropped — one error surface, not two.Solana Pay receive locale comma
solanaPayUri.normalizeAmount(solanaPayUri.ts) — strips a single comma decimal separator before regex validation, so users in locales that render decimals as1,5get a valid URI without retyping. Three locale-comma cases added tovalidate-tier0-services(1,5,0,1, multi-comma rejection).Bundled v3 hotfix
PendingCosigns.tsxline 57 — last-minute fix bundled in because it's a one-line fix that unblockstsc --noEmitonv3. TheH_PADconstant was removed in PR V3 magic branch #29 (parentWalletScreengrid now owns horizontal padding) but one reference was missed. Drops the stalepaddingHorizontal: H_PADstyle entry and consolidates two duplicate@expo/vector-iconsimports while in the file. Functionally a no-op — would have shipped under bento padding-rework intent.Validation
npx tsc --noEmitclean (now green on v3 too)npm run lintclean (no warnings)npm run validate:tier0:servicespass (new locale-comma assertions exercised)node ./scripts/validate-tier0-config.mjspassDevice smoke (Solana Seeker, manual)
expo-screen-captureavailable, capture-block panel never shown. Background → foreground → key still rendered, prevention reapplied.1,5and0,1produce validsolana:URIs (Phantom on second device parses them).Known limitations (deliberate, not regressions)
validate-tier0-services(12-patternwalletDenialtaxonomy from PR Wallet tightening: adapter consumption, real SPL, T22 filter, partial-write durability #28); on-device hardware-fault test will run alongside iPhone validation.Rollback
Each commit independently revertable. The four commits don't touch shared state —
PendingCosignshotfix is independently revertable too.Relationship to PR #28
This branch was pushed off
upstream/v3@29b4825(post PR #28 + #29 merge). The three recovery-hardening commits were originally drafted alongside PR #28's wallet truth pass but split off for review-surface reasons (recovery export = highest-trust surface, separating sharpens review). One smallPendingCosignsline edit bundled in to unblocktscon v3.