Skip to content

fix: hide transaction details from lock-screen notifications (L14) - #578

Closed
n13 wants to merge 2 commits into
mainfrom
fix/l14-private-notifications
Closed

fix: hide transaction details from lock-screen notifications (L14)#578
n13 wants to merge 2 commits into
mainfrom
fix/l14-private-notifications

Conversation

@n13

@n13 n13 commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

Local notifications (transfer received/sent, reversible-transaction reminders, balance alerts) include transaction amounts and addresses in the title/body, and were shown in full on the Android lock screen to anyone holding the locked phone.

This sets visibility: NotificationVisibility.private on the AndroidNotificationDetails in LocalNotificationsService._notificationDetails() — the single construction site used by both immediate (_showNotification) and scheduled (_scheduleNotification) notifications. On the lock screen, Android now shows the notification with sensitive content hidden; full content is visible only after unlock. Applied to all notifications from this service since all of them can disclose wallet/transaction information.

On iOS, lock-screen content visibility is a per-user system setting and cannot be controlled per-notification via flutter_local_notifications, so it is left as-is per common wallet practice.

Addresses finding L14 of the 2026-07-22 mobile wallet security audit.

@n13

n13 commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

🔍 Review — L14 (private lock-screen notifications)

Verdict: 🔴 Request changes

The intent, the single-construction-site targeting, and the .private-vs-.secret enum choice are all correct — but the change is a functional no-op. flutter_local_notifications applies visibility only to the notification builder, whose default is already VISIBILITY_PRIVATE, and it never sets the channel-level lock-screen visibility that actually governs redaction on Android 8.0+. The produced notification is byte-identical before and after, so it cannot change lock-screen behavior or remediate L14.

What it does

  • Adds visibility: NotificationVisibility.private to the AndroidNotificationDetails built in LocalNotificationsService._notificationDetails() (mobile-app/lib/services/local_notifications_service.dart:35–36, line 38 on the PR head ref fix/l14-private-notifications).
  • That single builder is shared by both _showNotification (local_notifications_service.dart:113) and _scheduleNotification (local_notifications_service.dart:136).
  • Leaves iOS DarwinNotificationDetails() unchanged.

Strengths

  • Single construction site is correct. _notificationDetails() (local_notifications_service.dart:28) is the only AndroidNotificationDetails(...) in the app. Grep of mobile-app/lib and mobile-app/android/app/src finds no other builder, no native notification code, and no second notification plugin (pubspec.yaml:50, only flutter_local_notifications: ^21.0.0). Both immediate and scheduled paths are covered.
  • Enum choice is right. For "show the notification but redact content on a secure lock screen, reveal after unlock," .private is correct; .secret (enums.dart:220, "Do not reveal any part…") would suppress it entirely and would be wrong.
  • FCM foreground leak is (incidentally) in scope. FirebaseMessagingService._setupForegroundMessageListener converts remote messages to NotificationData and calls notifier.addNotification(...) (firebase_messaging_service.dart:148–159), which for NotificationSource.push/remote routes to _localNotificationsService.showOrScheduleNotification(...) (notification_provider.dart:180–183) — i.e. through the same _notificationDetails(). So no separate Android builder is bypassed.

Findings

  1. [blocking] The change does not alter the notification and cannot hide anything — verified from source. local_notifications_service.dart:38

    • Before this PR, visibility was null. The plugin's setVisibility() returns early on null and never calls builder.setVisibility() (flutter_local_notifications-21.0.0/.../FlutterLocalNotificationsPlugin.java:911–913).
    • NotificationCompat.Builder's default is mVisibility = VISIBILITY_PRIVATE (androidx.core 1.17.0 NotificationCompat.java:1130), and it is applied to the platform notification unconditionally on API 21+ (NotificationCompatBuilder.java:173).
    • Therefore the produced notification already had VISIBILITY_PRIVATE; setting it explicitly to VISIBILITY_PRIVATE yields an identical notification. SystemUI reads only the final int and cannot distinguish "explicitly set" from "defaulted." Net effect on the lock screen: none. (Resolved version confirmed: pubspec.lockflutter_local_notifications 21.0.0.)
  2. [blocking] Root cause is channel-level, which this mechanism can't reach — so L14 is not actually remediated. local_notifications_service.dart:30–36

    • On Android 8.0+ (effectively all devices), lock-screen redaction is governed by the notification channel's setLockscreenVisibility(...). The plugin's setupNotificationChannel never calls it (FlutterLocalNotificationsPlugin.java:1208–1263), so local_channel_id is created with VISIBILITY_NO_OVERRIDE, and content visibility falls back to the device's global "lock screen → show all / hide sensitive" setting. On any device set to "Show all content," full tx amounts/addresses continue to appear — exactly the audit scenario — and this PR does nothing about it.
    • flutter_local_notifications 21.0.0 exposes no way to set channel lock-screen visibility: AndroidNotificationChannel has no such field (notification_channel.dart:11–24, only id/name/description/importance/sound/vibration/lights/badge). Remediation requires native Android code (create local_channel_id with setLockscreenVisibility(VISIBILITY_PRIVATE) before the plugin does, via MethodChannel/Application), and/or server-side FCM android.notification.visibility, and/or the broader FLAG_SECURE / recents-masking mitigation.
    • Verification gap: the fix should be tested on a real locked device before claiming L14 is closed. Prediction from the source analysis above: no change from baseline.
  3. [non-blocking] Channel immutability defeats even a corrected channel-level fix for existing installs. Notification channels are immutable after first creation; devices that already created local_channel_id will keep its current lock-screen visibility regardless of app changes. A real fix likely needs a new channel ID (with content migration) to take effect on upgrades — worth calling out in whatever the corrected PR does.

  4. [non-blocking / scope] Backgrounded/terminated FCM. The background handler only logs (firebase_messaging_service.dart:16–21, 162–164) and the design relies on "silent refresh on resume" (firebase_messaging_service.dart:145–147), implying data-only pushes that display nothing while locked — so no extra lock-screen leak today. But this is server-controlled: if the push backend (Senoti) ever sends a notification payload block, Android auto-renders it on the lock screen with visibility set by the payload/channel, entirely outside this client change. Fix belongs in the FCM payload (android.notification.visibility) / channel, not here. Non-blocking unless the push contract sends notification-payload messages.

  5. [non-blocking] iOS "left as-is" is reasonable but incomplete. iOS lock-screen previews are a per-user system setting not controllable per-notification via DarwinNotificationDetails, so leaving it is defensible; just note the iOS lock-screen preview leak is not addressed, matching common wallet practice.

  6. [nit] The inline comment overstates the guarantee. local_notifications_service.dart:34–36 — 'shows "sensitive content hidden"' only holds when the device's global lock-screen setting is "Hide sensitive content"; on "Show all content" devices content still shows in full. Given finding Rust Bindings #1/Feature/dilithium signatures #2, the comment (and PR body's "Android now shows the notification with sensitive content hidden") describes behavior the code does not actually produce.

Verification

Inadequate to support the PR's claim. The diff was not validated on a real locked device; the PR body asserts redacted lock-screen output, but source-level analysis of the exact resolved dependencies (flutter_local_notifications 21.0.0, androidx.core 1.17.0) shows the notification is unchanged and redaction is channel-governed. Before this can be considered a fix: (a) reproduce the L14 leak on a locked device, (b) apply a channel-level lock-screen-visibility mitigation (native / server / new channel id) or FLAG_SECURE, (c) re-test that content is redacted while locked and revealed after unlock. Confirmed non-issues: single construction site (grep of lib + android native), correct enum, and FCM-foreground coverage via the shared path.


🤖 AI-assisted review generated with Claude Code

@n13

n13 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

Superseded by #589 — uses NotificationVisibility.secret instead, which actually takes effect at the notification level regardless of the device's global lock-screen setting (the .private change here was a no-op, as the review showed).

@n13 n13 closed this Jul 28, 2026
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.

1 participant