[Android] Harden the mechanism for setting Android engine flags
[!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:
- 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. - As a consequence of 1, flags that previously could be set dynamically upon the launch of an
Intentcan no longer directly be set dynamically. Instead, they must be set statically in the manifest or on the command line. - 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
- [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
- [x] I read the Tree Hygiene wiki page, which explains my responsibilities.
- [x] I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
- [x] I signed the CLA.
- [x] I listed at least one issue that this PR fixes in the description above.
- [x] I updated/added relevant documentation (doc comments with
///). - [x] I added new tests to check the change I am making, or this PR is test-exempt.
- [x] I followed the breaking change policy and added Data Driven Fixes where supported.
- [x] All existing and new tests are passing.
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.
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.
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.
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.yamlfile 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>
/gemini review
@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.