Harden dev tooling scripts against command injection and log leaks by ishaq2321 · Pull Request #186076 · flutter/flutter

ishaq2321 · 2026-05-05T18:18:55Z

Description

This PR hardens several developer and CI scripts against command injection, URL-splitting, and information disclosure. These were flagged as high-severity vulnerabilities by static analysis.

Fixes applied:

Properly quoted $STAGING_DIR, ${log_file}, $WRAPPER_TEMP_DIR, and $WRAPPER_SRC_URL in Bash scripts to prevent arbitrary command injection.

flutter-dashboard · 2026-05-05T18:18:59Z

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

google-cla · 2026-05-05T18:19:06Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

gemini-code-assist

Code Review

This pull request updates shell scripts and Dart utility files to improve variable quoting, sanitize log output, and refine collection literals. A potential issue was identified in dev/bots/run_command.dart where setting ANDROID_NDK_PATH to an empty string might cause unexpected behavior in build tools; using a collection-if to conditionally include the environment variable was suggested instead.

gemini-code-assist · 2026-05-05T18:20:07Z

  final finalEnvironment = <String, String>{
    ...?environment,
-    'ANDROID_NDK_PATH': ?_discoverBestNdkPath(),
+    'ANDROID_NDK_PATH': _discoverBestNdkPath() ?? '',


Setting an environment variable to an empty string is semantically different from leaving it unset. Many build tools check for the presence of ANDROID_NDK_PATH, and an empty string might be interpreted as an invalid path, causing unexpected behavior or failures. It is better to conditionally include the key in the environment map only if a path is successfully discovered using a collection-if with a pattern match.

Suggested change

'ANDROID_NDK_PATH': _discoverBestNdkPath() ?? '',

if (_discoverBestNdkPath() case final String path) 'ANDROID_NDK_PATH': path,

eyebrowsoffire

Thanks for the fixes. The bash escaping looks like a good idea, but the logging changes and some other unrelated changes that crept in should probably be removed.

eyebrowsoffire · 2026-05-11T22:02:33Z

  }) async {
    if (dryRun) {
-      print('gsutil.py -- $args');
+      print('gsutil.py invoked with sanitized arguments');


This change is not necessary, for two reasons:

dryRun is never invoked in CI, it is just for locally testing that the gsutil command looks fine.

The args passed to gsutil are never sensitive. We don't pass tokens as command line arguments anywhere, and all of the bucket paths are public information.

eyebrowsoffire · 2026-05-11T22:03:29Z

  final finalEnvironment = <String, String>{
    ...?environment,
-    'ANDROID_NDK_PATH': ?_discoverBestNdkPath(),
+    if (_discoverBestNdkPath() != null) 'ANDROID_NDK_PATH': _discoverBestNdkPath()!,


This change also looks unnecessary?

eyebrowsoffire · 2026-05-11T22:03:36Z

    final allOutput = '${result.flattenedStdout}\n${result.flattenedStderr}';
    foundError(<String>[
-      ?failureMessage,
+      if (failureMessage != null) failureMessage,


Also unnecessary.

eyebrowsoffire · 2026-05-11T22:16:40Z

      return _processRunner.runProcess(command, workingDirectory: workingDirectory, failOk: failOk);
    } else {
-      print('Would run: ${command.join(' ')}');
+      print('Would run: ${command.take(2).join(' ')} ...');


This is also unnecessary for the same reasons listed in the above comment. !confirm is the equivalent of dry run, which is never run on CI, and we never pass sensitive arguments to gsutil.

eyebrowsoffire

LGTM

ishaq2321 · 2026-05-12T05:16:36Z

Thanks for the review and approval! I'd love to help out with more issues across the framework—whether related to this script hardening or completely different areas. Feel free to mention or assign me to anything in the backlog!

Also, since this is approved, could you please merge it or apply the autosubmit label when you have a moment?

auto-submit · 2026-05-12T14:29:25Z

autosubmit label was removed for flutter/flutter/186076, because This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a member of flutter-hackers before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

auto-submit · 2026-05-12T14:59:30Z

autosubmit label was removed for flutter/flutter/186076, because The base commit of the PR is older than 7 days and can not be merged. Please merge the latest changes from the main into this branch and resubmit the PR.

Fixes several high-severity XSS/CSRF findings by properly quoting Bash variables in command substitutions and truncating/sanitizing sensitive arguments (like gsutil tokens/paths) before printing to CI logs. Also patches CI orchestration syntax errors.

…11713) Manual roll requested by bmparr@google.com flutter/flutter@23f6f58...0541913 2026-05-14 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Windows] Propagate the enabled accessibility state (#184501)" (flutter/flutter#186492) 2026-05-13 srawlins@google.com [dev] Use super parameters in missed spots (flutter/flutter#186193) 2026-05-13 loic.peron@inetum.com [Windows] Propagate the enabled accessibility state (flutter/flutter#184501) 2026-05-13 matt.boetger@gmail.com [flutter_tool] filter out MotionEvent-JNI warning spam from logcat (#174783) (flutter/flutter#186079) 2026-05-13 engine-flutter-autoroll@skia.org Roll Packages from 93cbed6 to 2ec2236 (1 revision) (flutter/flutter#186464) 2026-05-13 mdebbar@google.com [web] Fix untriaged issues link label (flutter/flutter#186465) 2026-05-13 bdero@google.com [Impeller] Namespace user-supplied shaders to prevent entrypoint collisions (flutter/flutter#186332) 2026-05-13 1063596+reidbaker@users.noreply.github.com [flutter_tools] Migrate detectLowCompileSdkVersionOrNdkVersion to AGP task (flutter/flutter#184731) 2026-05-13 jason-simmons@users.noreply.github.com Update the Flutter Gallery web app template files to support running with Wasm (flutter/flutter#186268) 2026-05-13 jason-simmons@users.noreply.github.com [web] Use heap allocation for buffers that would consume too much space on the Wasm stack (flutter/flutter#186228) 2026-05-13 engine-flutter-autoroll@skia.org Roll Skia from 56ca5896c0d9 to 27f7bba22600 (3 revisions) (flutter/flutter#186444) 2026-05-13 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from z7ICmPtn4hspu02zk... to y6uQHA5xUN83IF395... (flutter/flutter#186442) 2026-05-13 engine-flutter-autoroll@skia.org Roll Skia from 6385958d2feb to 56ca5896c0d9 (1 revision) (flutter/flutter#186441) 2026-05-13 engine-flutter-autoroll@skia.org Roll Dart SDK from 9576691c37d8 to 8e30b88e4d5a (1 revision) (flutter/flutter#186429) 2026-05-13 engine-flutter-autoroll@skia.org Roll Skia from 77a21bc723dc to 6385958d2feb (9 revisions) (flutter/flutter#186428) 2026-05-13 164032450+AlexEduV@users.noreply.github.com Docs/improving docs for semantics UI lib (flutter/flutter#186125) 2026-05-12 jason-simmons@users.noreply.github.com [Tool] Support glob patterns when parsing workspaces in FlutterProject (flutter/flutter#185715) 2026-05-12 nico.reiab@gmail.com docs: fix overriden -> overridden in MediaQueryData dartdoc (flutter/flutter#186323) 2026-05-12 brackenavaron@gmail.com [Test cross imports] No material in `test/foundation`, `test/gestures`, `test/semantics`, `test/services` (flutter/flutter#186144) 2026-05-12 nico.reiab@gmail.com docs: fix "tha" -> "that" typo in widget_inspector_test comment (flutter/flutter#186322) 2026-05-12 nico.reiab@gmail.com docs: Fix doubled-word typos in framework dartdoc (flutter/flutter#186319) 2026-05-12 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#186418) 2026-05-12 30870216+gaaclarke@users.noreply.github.com Bumped required mediatek vender sdk version. (flutter/flutter#186405) 2026-05-12 magder@google.com Make DeepLinkJsonFromManifestTask Gradle task build cacheable (flutter/flutter#185903) 2026-05-12 66727653+ishaq2321@users.noreply.github.com Harden dev tooling scripts against command injection and log leaks (flutter/flutter#186076) 2026-05-12 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#186274) 2026-05-12 bdero@google.com [Flutter GPU] Allow allocating multi-mip textures and overwriting specific (mip, slice) levels (flutter/flutter#185890) 2026-05-12 zhongliu88889@gmail.com [web] Fix MenuAnchor dismiss when semantics enabled (flutter/flutter#183093) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC bmparr@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…lutter#11713) Manual roll requested by bmparr@google.com flutter/flutter@23f6f58...0541913 2026-05-14 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Windows] Propagate the enabled accessibility state (#184501)" (flutter/flutter#186492) 2026-05-13 srawlins@google.com [dev] Use super parameters in missed spots (flutter/flutter#186193) 2026-05-13 loic.peron@inetum.com [Windows] Propagate the enabled accessibility state (flutter/flutter#184501) 2026-05-13 matt.boetger@gmail.com [flutter_tool] filter out MotionEvent-JNI warning spam from logcat (#174783) (flutter/flutter#186079) 2026-05-13 engine-flutter-autoroll@skia.org Roll Packages from 93cbed6 to 2ec2236 (1 revision) (flutter/flutter#186464) 2026-05-13 mdebbar@google.com [web] Fix untriaged issues link label (flutter/flutter#186465) 2026-05-13 bdero@google.com [Impeller] Namespace user-supplied shaders to prevent entrypoint collisions (flutter/flutter#186332) 2026-05-13 1063596+reidbaker@users.noreply.github.com [flutter_tools] Migrate detectLowCompileSdkVersionOrNdkVersion to AGP task (flutter/flutter#184731) 2026-05-13 jason-simmons@users.noreply.github.com Update the Flutter Gallery web app template files to support running with Wasm (flutter/flutter#186268) 2026-05-13 jason-simmons@users.noreply.github.com [web] Use heap allocation for buffers that would consume too much space on the Wasm stack (flutter/flutter#186228) 2026-05-13 engine-flutter-autoroll@skia.org Roll Skia from 56ca5896c0d9 to 27f7bba22600 (3 revisions) (flutter/flutter#186444) 2026-05-13 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from z7ICmPtn4hspu02zk... to y6uQHA5xUN83IF395... (flutter/flutter#186442) 2026-05-13 engine-flutter-autoroll@skia.org Roll Skia from 6385958d2feb to 56ca5896c0d9 (1 revision) (flutter/flutter#186441) 2026-05-13 engine-flutter-autoroll@skia.org Roll Dart SDK from 9576691c37d8 to 8e30b88e4d5a (1 revision) (flutter/flutter#186429) 2026-05-13 engine-flutter-autoroll@skia.org Roll Skia from 77a21bc723dc to 6385958d2feb (9 revisions) (flutter/flutter#186428) 2026-05-13 164032450+AlexEduV@users.noreply.github.com Docs/improving docs for semantics UI lib (flutter/flutter#186125) 2026-05-12 jason-simmons@users.noreply.github.com [Tool] Support glob patterns when parsing workspaces in FlutterProject (flutter/flutter#185715) 2026-05-12 nico.reiab@gmail.com docs: fix overriden -> overridden in MediaQueryData dartdoc (flutter/flutter#186323) 2026-05-12 brackenavaron@gmail.com [Test cross imports] No material in `test/foundation`, `test/gestures`, `test/semantics`, `test/services` (flutter/flutter#186144) 2026-05-12 nico.reiab@gmail.com docs: fix "tha" -> "that" typo in widget_inspector_test comment (flutter/flutter#186322) 2026-05-12 nico.reiab@gmail.com docs: Fix doubled-word typos in framework dartdoc (flutter/flutter#186319) 2026-05-12 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#186418) 2026-05-12 30870216+gaaclarke@users.noreply.github.com Bumped required mediatek vender sdk version. (flutter/flutter#186405) 2026-05-12 magder@google.com Make DeepLinkJsonFromManifestTask Gradle task build cacheable (flutter/flutter#185903) 2026-05-12 66727653+ishaq2321@users.noreply.github.com Harden dev tooling scripts against command injection and log leaks (flutter/flutter#186076) 2026-05-12 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#186274) 2026-05-12 bdero@google.com [Flutter GPU] Allow allocating multi-mip textures and overwriting specific (mip, slice) levels (flutter/flutter#185890) 2026-05-12 zhongliu88889@gmail.com [web] Fix MenuAnchor dismiss when semantics enabled (flutter/flutter#183093) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC bmparr@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

gemini-code-assist Bot reviewed May 5, 2026

View reviewed changes

eyebrowsoffire requested changes May 11, 2026

View reviewed changes

eyebrowsoffire approved these changes May 12, 2026

View reviewed changes

eyebrowsoffire added the CICD Run CI/CD label May 12, 2026

eyebrowsoffire added the autosubmit Merge PR when tree becomes green via auto submit App label May 12, 2026

auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 12, 2026

jtmcdole approved these changes May 12, 2026

View reviewed changes

jtmcdole added the autosubmit Merge PR when tree becomes green via auto submit App label May 12, 2026

auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 12, 2026

This comment was marked as off-topic.

Sign in to view

ishaq2321 added 3 commits May 12, 2026 11:49

Fix ANDROID_NDK_PATH empty string bug using collection-if

996bcac

Revert unnecessary logging and Dart collection changes

92015a5

Piinks force-pushed the sec-harden-dev-scripts branch from d484b23 to 92015a5 Compare May 12, 2026 16:49

github-actions Bot removed the CICD Run CI/CD label May 12, 2026

Piinks added CICD Run CI/CD autosubmit Merge PR when tree becomes green via auto submit App labels May 12, 2026

auto-submit Bot added this pull request to the merge queue May 12, 2026

Merged via the queue into flutter:master with commit 583dec7 May 12, 2026
170 checks passed

flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 12, 2026

	'ANDROID_NDK_PATH': _discoverBestNdkPath() ?? '',
	if (_discoverBestNdkPath() case final String path) 'ANDROID_NDK_PATH': path,

Uh oh!

Conversation

ishaq2321 commented May 5, 2026 • edited by jtmcdole Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

flutter-dashboard Bot commented May 5, 2026

Uh oh!

google-cla Bot commented May 5, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 5, 2026

Choose a reason for hiding this comment

Uh oh!

eyebrowsoffire left a comment

Choose a reason for hiding this comment

Uh oh!

eyebrowsoffire May 11, 2026

Choose a reason for hiding this comment

Uh oh!

eyebrowsoffire May 11, 2026

Choose a reason for hiding this comment

Uh oh!

eyebrowsoffire May 11, 2026

Choose a reason for hiding this comment

Uh oh!

eyebrowsoffire May 11, 2026

Choose a reason for hiding this comment

Uh oh!

eyebrowsoffire left a comment

Choose a reason for hiding this comment

Uh oh!

ishaq2321 commented May 12, 2026

Uh oh!

auto-submit Bot commented May 12, 2026

Uh oh!

auto-submit Bot commented May 12, 2026

Uh oh!

This comment was marked as off-topic.

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ishaq2321 commented May 5, 2026 •

edited by jtmcdole

Loading