Skip to content

Harden clipboard + haptic#537

Merged
n13 merged 4 commits into
mainfrom
beast/haptic-and-harden-seed-copy
Jun 23, 2026
Merged

Harden clipboard + haptic#537
n13 merged 4 commits into
mainfrom
beast/haptic-and-harden-seed-copy

Conversation

@dewabisma

@dewabisma dewabisma commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Added haptic for input amount, so we get nice feedback when we tap the amount. We might be able to add more haptic like for when failed sending, or when success sending. Just improving user experience while not doing too much of it.
  • Harden clipboard when copying sensitive information like seed phrase, will have short TTL, currently 30 secs. in IOS it works great because we can call native clipboard so regardless of user in app or not it will clear the clipboard. But, for Android there is no way to get the same behavior hence, it will only clear if user come back to app after pasting it.

Note

Medium Risk
Changes how recovery phrases hit the system clipboard—a security-sensitive path—with stronger iOS behavior but weaker Android timer fallback if the app stays backgrounded.

Overview
Adds time-limited clipboard handling when users copy a recovery phrase, plus haptic feedback while editing amounts on send, POS, and multisig propose flows.

Recovery phrase copy now goes through SecureClipboardService and copySensitiveTextWithToaster instead of a plain clipboard write. Entries use a 30-second TTL; on iOS, AppDelegate bridges app.quantus/secure_clipboard to UIPasteboard with expirationDate so the OS clears the pasteboard even if the app is backgrounded or killed. On other platforms, a Dart Timer clears the clipboard on resume/foreground (weaker if the user stays away longer). Copy toasts in en and id now mention the auto-clear window.

Amount entry on input_amount_screen, pos_amount_screen, and propose_amount_screen triggers HapticFeedback.mediumImpact() on each amount change.

Reviewed by Cursor Bugbot for commit d15a4c9. Configure here.

@dewabisma dewabisma requested a review from n13 June 22, 2026 07:58

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit d15a4c9. Configure here.

Comment thread mobile-app/lib/services/secure_clipboard_service.dart
Comment thread mobile-app/lib/services/secure_clipboard_service.dart
@n13

n13 commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

I've reviewed PR #537 in a fresh worktree at /tmp/pr-reviews/quantus-pr-537-review (checked out at d15a4c9a, diffed against origin/main). Here's my assessment.

Overview

The PR does two unrelated things:

  1. Clipboard hardening — recovery-phrase copy now goes through a new SecureClipboardService with a 30s TTL (OS-enforced on iOS via UIPasteboard.expirationDate, Dart Timer fallback elsewhere).
  2. HapticsHapticFeedback.mediumImpact() on amount change in send/POS/propose screens.

The architecture is sound: a singleton service, a thin BuildContext extension mirroring the existing copyTextWithToaster, and a clean iOS method-channel bridge. Good doc comments and an honest acknowledgment of the Android limitation. The iOS registrar(forPlugin: "SecureClipboard") messenger idiom is correct and consistent with the existing didInitializeImplicitFlutterEngine usage.

Issues

1. Catch blocks bypass TelemetryService (should fix)

Both catch blocks in the new service log only via developer.log, which is stripped from production telemetry:

    } catch (e, s) {
      developer.log(
        'Native clipboard expiry unavailable, falling back to timer',
        name: 'secure_clipboard',
        level: 900,
        error: e,
        stackTrace: s,
      );
      return false;
    }

This is the one finding I'd treat as blocking. It contradicts the established convention across the service layer (wallet_initializer.dart, home_screen.dart, multisig_submission_service.dart, etc. all route to TelemetryService().sendError(...)), Bugbot's learned-rule comment, and the project rule that all failures must produce error logging. A clipboard-clear failure leaving a seed phrase resident is exactly the kind of silent failure that should surface. Add TelemetryService().sendError('secure_clipboard_clear_failed', error: e, stackTrace: s) (and an equivalent for the native fallback) alongside the existing logs.

2. Timer wipes unrelated clipboard content (Bugbot #1 — valid, but there's a cleaner fix)

On the non-iOS path, the timer unconditionally clears the clipboard, so anything the user copies in the 30s window gets wiped:

  Future<void> _clearClipboard() async {
    _clearTimer = null;

    try {
      await Clipboard.setData(const ClipboardData(text: ''));

The author's reply (comparing clipboard contents would trigger the OS read alert) is fair — but you can sidestep both problems without reading the clipboard: have the regular copyTextWithToaster call SecureClipboardService.instance.cancel() before writing. A subsequent normal copy then cancels the pending secure-clear, so it won't be wiped, and no paste prompt is shown. Right now copyTextWithToaster and copySensitiveTextWithToaster share showCopyToaster but the non-sensitive path doesn't know about the pending timer.

3. Haptic logic duplicated across three screens (DRY)

The exact same line is added to three near-identical _onAmountChanged handlers, all of which already funnel through the shared AmountInputLogic.onAmountChanged:

  void _onAmountChanged(String _) {
    HapticFeedback.mediumImpact();

    final isFlipped = widget.isPayMode ? false : ref.read(isCurrencyFlippedProvider);

Given the strict DRY guidance, consider centralizing the haptic (e.g. in the shared amount-input widget that owns the onChanged/keypad, or a single helper) so it lives in one place rather than being copy-pasted three times.

Minor / nits

  • Hardcoded "30s" in localization is decoupled from defaultTtl. The toast string 'Recovery phrase copied — clears in 30s' is a literal, while the TTL is Duration(seconds: 30) and copySensitiveTextWithToaster accepts a custom ttl. Change one and the message lies. Low risk today (only the default caller exists), but brittle.
  • mediumImpact() on every keystroke may feel heavy for per-character input; HapticFeedback.selectionClick() is the more idiomatic choice for incremental field edits. UX preference, not a bug.
  • PR/Cursor summary overstates Android behavior. It says the timer "clears on resume/foreground," but there's no AppLifecycleState observer — it's a plain Timer, so on Android a killed process leaves the phrase resident (the author's own description is the accurate one). No code change needed; just flagging the mismatch.
  • No test added despite cancel()'s doc noting it's "useful for tests." A small unit test (timer fires → clipboard cleared; new copy cancels prior timer) would lock in the TTL contract.

Verdict

Solid, well-documented change with a sensible cross-platform strategy. I'd ask for #1 (telemetry) before merge since it touches a security path and violates the project's error-logging rule, and I'd recommend #2 and #3 as quick follow-ups. Everything else is optional polish.

@n13

n13 commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

My review:

  1. Change to make it longer
    I think 30 seconds is too short please make it 3 minutes, which ensures the user gets to paste it wherever they want to paste it, but also wipes it so it doesn't sit around in the clipboard forever

The security change is pretty theoretical, an app would have to try and steal the clipboard, and the user would have to agree to this with the iOS "allow this app to paste" dialog.

That being said having the phrase in memory for a long time is not good

  1. Change the toast text back to what it was
    The copy clipboard text shouldn't say anything about expiry, that's confusing. We just make it long enough for reasonable use, we have no intention of rushing the user.

Haptics is fine

@n13

n13 commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

For android we could add this ?
image

Not sure though, it's a little unclear whether that's worth it? Maybe another PR

@n13 n13 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Longer time
  • No change in clipboard text

Nevermind about Android, not sure on this one - if we do this we can make a new PR

@n13 n13 merged commit a10aea1 into main Jun 23, 2026
1 check passed
@dewabisma dewabisma deleted the beast/haptic-and-harden-seed-copy branch June 23, 2026 06:27
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.

2 participants