[Android] Verify SensitiveContent widget potential frame drops are not visible

We should build a sample app to verify that dropped frames caused by the MethodChannel communication that the SensitiveContent widget uses to communicate with the engine is largely unobservable, and to confirm that this should not be a blocking problem for feature release.

See #158473 (comment) for more context.

Not sure what the rubric is here, but I built a simple app and recorded some samples to show that we don't observe sensitive content before the screen becomes obscured. I tested with Google Meets. There is some delay because it's screen sharing on Meets; not sure if this is a concern but open to thoughts.

On the left, is the Google Meets screen share. On the right is the device.

Case 1: Navigating to new page with a sensitive `SensitiveContent` widget.

sc_proof_1.mov

Case 2: Launching an app with a `SensitiveContent` widget on the first screen

sc_proof_2.mov

Case 3: Clicking a button to switch a `SensitiveContent`'s sensitivity level from not sensitive to sensitive

sc_proof_3.mov

@reidbaker

@reidbaker @justinmc @chunhtai Can you take a look at the videos I recorded above? Proving dropped frames are not visible is required to land #158473, so hoping to get feedback from you all.

I think this should be fine, but I am not the expert of the threading in the engine. Based on the code, the messagechannel will be sent during build phase before the layer tree is sent to the engine. so I think toggling the setting should execute before the new frame is drawn.

@camsim99

@camsim99 this is exactly what I was hoping to see. With that app can I get you to test rotation? That would be the one final case that I would hate for us to miss.

oh btw, one more thing to try is to remove page transition duration entirely when pushing new route to see if it still work.

The requested test cases:

With that app can I get you to test rotation? That would be the one final case that I would hate for us to miss.

These cases seem to work :)

Rotating device after content has been marked sensitive

sc_proof_4.mov

Background app after content has been marked sensitive, open another app, rotate device, then resume app

sc_proof_5.mov

oh btw, one more thing to try is to remove page transition duration entirely when pushing new route to see if it still work.

This breaks it :(

Pushing a new route without page transition duration

(used PageRouteBuilder with transitionDuration: Duration.zero and reverseTransitionDuration: Duration.zero)

sc_proof_6.mov

What do we think about the navigation case? I would prefer for it not to block the PR because I assume it's not possible to fix without pursuing JNIgen. Maybe we can write a warning about the race condition with animations (I assume this applies to buttons, navigation)?

I can understand the case for not "releasing this feature" (whatever that means) without it being fixed, though the timeline would have to be extended for this project.

However, landing the PR would make our security strictly better, so that's where I'm coming from.

Lets ask the framework team for how to feature flag a widget.

Can you share your code for the page transition example?

@VisibleForTesting

I like the changes you made with @VisibleForTesting. Can you also include a dart doc comment pointing to this bug and saying the widget is not ready for consumption. Then I am comfortable landing the pr.

I assume you're talking about #158473 (comment)?

Or are you saying you also lean towards making the widget @VisibleForTesting if that's an option available to us?

My comment should have been on the pr. I will put it there.

Can you share your code for the page transition example?

Page transition code sample (not the exact one, but made it minimal)

import 'package:flutter/material.dart';
import 'package:flutter/services.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Sensitive Content Demo',
      theme: ThemeData(
        colorScheme: ColorScheme.fromSeed(seedColor: Colors.deepPurple),
        useMaterial3: true,
      ),
      home: FirstScreen(),
    );
  }
}

class _FirstScreenState extends State<FirstScreen> {
  @override
  Widget build(BuildContext context) {
    return Center(
      child: Column(
        children: <Widget>[
          SensitiveContent(
            sensitivityLevel: ContentSensitivity.notSensitive,
            child: ElevatedButton(
              child: Text(
                'Click here to navigate to red page with sensitive content!',
              ),
              onPressed: () {
                Navigator.of(context).pushReplacement(
                  PageRouteBuilder(
                    pageBuilder: (context, a1, a2) => SecondScreen(),
                    transitionDuration: Duration.zero,
                    reverseTransitionDuration: Duration.zero,
                  ),
                );
              },
            ),
          ),
        ],
      ),
    );
  }
}

class SecondScreen extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(backgroundColor: Colors.red, title: Text('Second screen')),
      body: Container(
        color: Colors.red,
        child: SizedBox.expand(
          child: SensitiveContent(
            sensitivityLevel: ContentSensitivity.sensitive,
            child: Text('Sensitive content!'),
          ),
        ),
      ),
    );
  }
}

Do we know precisely how we plan on fixing the leaking of information here?

Do we know if that solution will impact the API that we expose?

We plan to implement it with JNIgen instead of method channels to ensure that the call to obscure the screen is synchronous #160050. It shouldn't impact the API we expose though!

I think we've settled on a good plan here 👍 .

Mark SensitiveContent @visibleForTesting.
Don't export it from widgets.dart.
Explain this in SensitiveContent's dartdocs.

I've seen this sort of problem come up before, but never for a top-level widget/class. For example in the secure paste project, we turned the feature off by default using a parameter to EditableText until it was ready, but we can't exactly do that here. I think this is a good precedent for this situation though.

Left an in detail comment here: #158473 (comment), but new plan is to land #158473 exposing the widget and explain the vulnerability in the SensitiveContent docs for now. Thus, the feature will essentially be shipped in master.

Fixing this issue will accompany #160050. In the meantime, this issue can help track any known SensitiveContent widget vulnerabilities. I will reference this issue in the SensitiveContent docs.

[Android] Implement setting sensitive content, take 2 (#167815)

Dropping priority as this work will need to be re-done when a JNIgen implementation is pursued.

Additional context: making a synchronous call requires 2 things to happen. First we need the opt out thread merging to be removed. Second we need to go though a design review for how dart ui will use jnigen in our code base.

In the meantime page transition animations will prevent leakage and are on by default in most apps and that not landing the feature is strictly worse for protecting users.

[Android] Implement setting sensitive content, take 2 (flutter#167815)

Navigation Menu

Repository navigation

[Android] Verify `SensitiveContent` widget potential frame drops are not visible #164820

Case 1: Navigating to new page with a sensitive `SensitiveContent` widget.

Case 2: Launching an app with a `SensitiveContent` widget on the first screen

Case 3: Clicking a button to switch a `SensitiveContent`'s sensitivity level from not sensitive to sensitive

Rotating device after content has been marked sensitive

Background app after content has been marked sensitive, open another app, rotate device, then resume app

Pushing a new route without page transition duration

9 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[Android] Verify SensitiveContent widget potential frame drops are not visible #164820

Description

Activity

camsim99 commented on Mar 12, 2025

Case 1: Navigating to new page with a sensitive SensitiveContent widget.

Case 2: Launching an app with a SensitiveContent widget on the first screen

Case 3: Clicking a button to switch a SensitiveContent's sensitivity level from not sensitive to sensitive

camsim99 commented on Mar 12, 2025

chunhtai commented on Mar 12, 2025

reidbaker commented on Mar 12, 2025

chunhtai commented on Mar 12, 2025

camsim99 commented on Mar 12, 2025

Rotating device after content has been marked sensitive

Background app after content has been marked sensitive, open another app, rotate device, then resume app

Pushing a new route without page transition duration

camsim99 commented on Mar 12, 2025

camsim99 commented on Mar 12, 2025

reidbaker commented on Mar 13, 2025

reidbaker commented on Mar 13, 2025

9 remaining items

camsim99 commented on Mar 13, 2025

reidbaker commented on Mar 13, 2025

camsim99 commented on Mar 13, 2025

johnmccutchan commented on Mar 13, 2025

camsim99 commented on Mar 13, 2025

justinmc commented on Mar 14, 2025

camsim99 commented on Apr 16, 2025

camsim99 commented on May 8, 2025

reidbaker commented on May 8, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

[Android] Verify `SensitiveContent` widget potential frame drops are not visible #164820

Case 1: Navigating to new page with a sensitive `SensitiveContent` widget.

Case 2: Launching an app with a `SensitiveContent` widget on the first screen

Case 3: Clicking a button to switch a `SensitiveContent`'s sensitivity level from not sensitive to sensitive