flutter icon indicating copy to clipboard operation
flutter copied to clipboard

[Android] Harden the mechanism for setting Android engine flags

Open camsim99 opened this issue 2 months ago • 5 comments

[!NOTE]
This PR is based on conversation & feedback on go/flutter-android-harden-engine-shell-arguments.

Reworks the mechanism for setting Android engine flags such that:

  1. Flags CANNOT be set via Intent, which is exploitable by malicious actors, particularly in add-to-app scenarios. See go/flutter-fragment-flag-security-analysis for an example of such an exploitation.
  2. As a consequence of 1, flags that previously could be set dynamically upon the launch of an Intent can no longer directly be set dynamically. Instead, they must be set statically in the manifest or on the command line.
  3. If a flag is specified on the command line and in manifest metadata, the value specified on the command line will take precedence.

Additionally, this PR removes the exposure of--cache-sksl command line flag as per https://github.com/flutter/flutter/issues/140310#issuecomment-2708459007.

As the unit tests in this PR only cover setting flags via manifest in debug mode, I will follow up this PR with an integration test to test that flags are appropriately respected/ignored in release mode. See https://github.com/flutter/flutter/pull/178383 for my currently working but WIP draft.

Fixes https://github.com/flutter/flutter/issues/172553.

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

camsim99 avatar Oct 16 '25 20:10 camsim99

Flags that have no real need to be set statically in production apps (like --trace-skia) are now only settable via the command line.

Was that a requirement? It seems like there may be a valid case where someone would want to use a 3rd party profiler that launches an app itself, outside of flutters command-line tool. I don't think perfetto works that way, you have to launch yourself. RenderDoc works that way for gpu frame debugging.

gaaclarke avatar Oct 22 '25 21:10 gaaclarke

Flags that have no real need to be set statically in production apps (like --trace-skia) are now only settable via the command line.

Was that a requirement? It seems like there may be a valid case where someone would want to use a 3rd party profiler that launches an app itself, outside of flutters command-line tool. I don't think perfetto works that way, you have to launch yourself. RenderDoc works that way for gpu frame debugging.

Oh interesting case....no this isn't a requirement :) We could make all command line flags settable via the manifest. I just assumed that wouldn't be applicable for some flags.

camsim99 avatar Oct 22 '25 21:10 camsim99

CI had a failure that stopped further tests from running. We need to investigate to determine the root cause.

SHA at time of execution: f7c2bb479b3d8a83096e6ba75ef383341cc5b43a.

Possible causes:

  • Configuration Changes: The .ci.yaml file might have been modified between the creation of this pull request and the start of this test run. This can lead to ci yaml validation errors.
  • Infrastructure Issues: Problems with the CI environment itself (e.g., quota) could have caused the failure.

A blank commit, or merging to head, will be required to resume running CI for this PR.

Error Details:

GitHub Error: You have exceeded a secondary rate limit. Please wait a few minutes before you try again. For more on scraping GitHub and how it may affect your rights, please review our Terms of Service (https://docs.github.com/en/site-policy/github-terms/github-terms-of-service) If you reach out to GitHub Support for help, please include the request ID D684:1FA2BF:449952:12B8B8A:690A395C.

Stack trace:

#0      GitHub.handleStatusCode (package:github/src/common/github.dart:486:5)
#1      GitHub.request (package:github/src/common/github.dart:422:7)
<asynchronous suspension>
#2      GitHub.requestJson (package:github/src/common/github.dart:323:22)
<asynchronous suspension>
#3      RetryOptions.retry (package:retry/retry.dart:131:16)
<asynchronous suspension>
#4      LuciBuildService.scheduleTryBuilds (package:cocoon_service/src/service/luci_build_service.dart:246:24)
<asynchronous suspension>
#5      Scheduler._runCiTestingStage (package:cocoon_service/src/service/scheduler.dart:1253:9)
<asynchronous suspension>
#6      Scheduler.proceedToCiTestingStage (package:cocoon_service/src/service/scheduler.dart:1314:7)
<asynchronous suspension>
#7      Scheduler._closeSuccessfulEngineBuildStage (package:cocoon_service/src/service/scheduler.dart:1121:5)
<asynchronous suspension>
#8      Scheduler.processCheckRunCompleted (package:cocoon_service/src/service/scheduler.dart:1054:9)
<asynchronous suspension>
#9      PresubmitLuciSubscription.post (package:cocoon_service/src/request_handlers/presubmit_luci_subscription.dart:135:7)
<asynchronous suspension>
#10     RequestHandler.service (package:cocoon_service/src/request_handling/request_handler.dart:45:20)
<asynchronous suspension>
#11     SubscriptionHandler.service (package:cocoon_service/src/request_handling/subscription_handler.dart:140:5)
<asynchronous suspension>
#12     createServer.<anonymous closure> (package:cocoon_service/server.dart:339:7)
<asynchronous suspension>

flutter-dashboard[bot] avatar Nov 04 '25 17:11 flutter-dashboard[bot]

/gemini review

gaaclarke avatar Nov 12 '25 18:11 gaaclarke

@reidbaker This is ready for another look!

As an aside, looks like I will need an internal fix to land this for two files that reference FlutterShellArgs.fromIntent. Will look into how to proceed there.

camsim99 avatar Nov 26 '25 19:11 camsim99