Redact SDK path in FlutterApp command log by jwren · Pull Request #9203 · flutter/flutter-intellij

jwren · 2026-11-03T06:31:11Z

Promote FlutterCommand.safeCommandLog to public static (using

PluginLogger.isFilePathLoggingEnabled and PathUtil.getFileName) and use

it when logging the command line in FlutterApp.start().

Coverage verified with Kover:

FlutterCommand.safeCommandLog: 100% line/branch coverage (3/3 lines)
FlutterApp.getCommandLog: 100% line coverage (2/2 lines)

Promote FlutterCommand.safeCommandLog to public static (using PluginLogger.isFilePathLoggingEnabled and PathUtil.getFileName) and use it when logging the command line in FlutterApp.start(). Coverage verified with Kover: - FlutterCommand.safeCommandLog: 100% line/branch coverage (3/3 lines) - FlutterApp.getCommandLog: 100% line coverage (2/2 lines) Fixes flutter#8566

gemini-code-assist

Code Review

This pull request implements redaction of the Flutter SDK path when logging command lines in FlutterApp if file path logging is disabled. It refactors FlutterCommand.safeCommandLog to be static and public, and integrates it into FlutterApp logging, accompanied by new unit tests. The review feedback suggests defensive handling of potential null values in commandLine.getExePath() to avoid NullPointerExceptions, and recommends removing a redundant static delegation method in FlutterApp to simplify the API.

gemini-code-assist · 2026-11-03T06:32:07Z


-    final Path path = Path.of(commandLine.getExePath());
-    return path.getFileName() + " " + commandLine.getParametersList();
+    return PathUtil.getFileName(commandLine.getExePath()) + " " + commandLine.getParametersList();


[CONCERN] To prevent potential NullPointerExceptions during logging, we should defensively handle the case where commandLine.getExePath() returns null. Logging utilities should be extremely robust and never throw exceptions that could disrupt the main application flow.

String exePath = commandLine.getExePath(); String exeName = exePath != null ? PathUtil.getFileName(exePath) : ""; return exeName + " " + commandLine.getParametersList();

References

Ensure robust and defensive code to prevent unexpected runtime exceptions. ^(link)

PathUtil.getFileName(@Nullable String) is already null-safe in the IntelliJ SDK (returning "" for null or empty strings), and GeneralCommandLine.getExePath() is annotated @NotNull, so an explicit null check here is redundant.

gemini-code-assist · 2026-11-03T06:32:07Z

+  /**
+   * Returns a log-safe string representation of the app's command line, redacting the SDK path
+   * when file path logging is disabled.
+   * <p>
+   * Exposed for testing.
+   */
+  @VisibleForTesting
+  @NotNull
+  public String getCommandLog() {
+    return getCommandLog(myCommand);
+  }
+
+  /**
+   * Returns a log-safe string representation of the given command line, redacting the SDK path
+   * when file path logging is disabled.
+   * <p>
+   * Exposed for testing.
+   */
+  @VisibleForTesting
+  @NotNull
+  public static String getCommandLog(@NotNull GeneralCommandLine command) {
+    return FlutterCommand.safeCommandLog(command);
+  }


[CONCERN] The static getCommandLog(GeneralCommandLine) method in FlutterApp is a redundant delegation to FlutterCommand.safeCommandLog(command). We can simplify the API surface of FlutterApp by removing this static method and having the instance getCommandLog() call FlutterCommand.safeCommandLog(myCommand) directly.

Suggested change

/**

* Returns a log-safe string representation of the app's command line, redacting the SDK path

* when file path logging is disabled.

* <p>

* Exposed for testing.

*/

@VisibleForTesting

@NotNull

public String getCommandLog() {

return getCommandLog(myCommand);

}

/**

* Returns a log-safe string representation of the given command line, redacting the SDK path

* when file path logging is disabled.

* <p>

* Exposed for testing.

*/

@VisibleForTesting

@NotNull

public static String getCommandLog(@NotNull GeneralCommandLine command) {

return FlutterCommand.safeCommandLog(command);

}

/**

* Returns a log-safe string representation of the app's command line, redacting the SDK path

* when file path logging is disabled.

* <p>

* Exposed for testing.

*/

@VisibleForTesting

@NotNull

public String getCommandLog() {

return FlutterCommand.safeCommandLog(myCommand);

}

References

Avoid redundant delegation methods to keep the codebase clean and maintainable. ^(link)

Done in 1590d7c. Removed the redundant static getCommandLog(GeneralCommandLine) helper and delegated getCommandLog() directly to FlutterCommand.safeCommandLog(myCommand).

gemini-code-assist · 2026-11-03T06:32:07Z

    throws ExecutionException {
    LOG.info(analyticsStart + " " + project.getName() + " (" + mode.mode() + ")");
-    LOG.info(command.toString());
+    LOG.info(getCommandLog(command));


[CONCERN] Since we can remove the redundant static getCommandLog method, we should call FlutterCommand.safeCommandLog(command) directly here.

Suggested change

LOG.info(getCommandLog(command));

LOG.info(FlutterCommand.safeCommandLog(command));

References

Avoid redundant delegation methods to keep the codebase clean and maintainable. ^(link)

Done in 1590d7c. Updated FlutterApp.start to call FlutterCommand.safeCommandLog(command) directly.

Inline FlutterApp.getCommandLog(GeneralCommandLine) so that the instance getCommandLog() and FlutterApp.start(...) delegate directly to FlutterCommand.safeCommandLog(...) per PR review feedback. Verified with FlutterAppTest and FlutterCommandTest. Issue: flutter#8566

pq · 2026-11-04T02:36:21Z

+   */
+  @VisibleForTesting
+  @NotNull
+  public String getCommandLog() {


LLM caught nit: now that safeCommandLog is public and tested directly in FlutterCommandTest, this test-only getter (and the FlutterAppTest cases) don't add much. They wouldn't catch start() going back to command.toString(). Drop them?

Done in 4aa8d4e — removed the test-only getCommandLog() getter from FlutterApp and the duplicate test cases in FlutterAppTest.

pq · 2026-11-04T02:36:21Z


 ### Changed
 - Centralizing file path logging into `PluginLogger`. (#8707)
+- Redacting the Flutter SDK path when logging the command line in `FlutterApp`. (#8566)


Nit: Could you update this to PR number rather than the issue?

Suggested change

- Redacting the Flutter SDK path when logging the command line in `FlutterApp`. (#8566)

- Redacting the Flutter SDK path when logging the command line in `FlutterApp`. (#9203)

Done in 4aa8d4e — updated to (#9203).

pq · 2026-11-04T02:36:21Z

+
+    assertEquals("flutter run lib/main.dart", command.getSanitizedDisplayCommand());
+  }
 }


Nit: while you're here, add a trailing newline?

Done in 4aa8d4e — added trailing newline.

pq

% nits, LGTM

Drop the redundant test-only getCommandLog() getter on FlutterApp and its duplicate test cases in FlutterAppTest, update CHANGELOG.md to reference PR flutter#9203, and add a trailing newline to FlutterCommandTest.

helin24

LGTM! Thanks for addressing the comments.

Note: Created by an AI agent (Gemini).

jwren requested review from helin24 and pq as code owners November 3, 2026 06:31

jwren marked this pull request as draft November 3, 2026 06:31

gemini-code-assist Bot reviewed Nov 3, 2026

View reviewed changes

jwren marked this pull request as ready for review November 3, 2026 18:24

pq approved these changes Nov 4, 2026

View reviewed changes

run: Remove test-only FlutterApp.getCommandLog and update CHANGELOG

4aa8d4e

Drop the redundant test-only getCommandLog() getter on FlutterApp and its duplicate test cases in FlutterAppTest, update CHANGELOG.md to reference PR flutter#9203, and add a trailing newline to FlutterCommandTest.

helin24 approved these changes Nov 4, 2026

View reviewed changes

jwren merged commit 75d53a9 into flutter:main Nov 4, 2026
13 checks passed

	LOG.info(getCommandLog(command));
	LOG.info(FlutterCommand.safeCommandLog(command));

	- Redacting the Flutter SDK path when logging the command line in `FlutterApp`. (#8566)
	- Redacting the Flutter SDK path when logging the command line in `FlutterApp`. (#9203)

Conversation

jwren commented Nov 3, 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 Nov 3, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Nov 3, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Nov 3, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pq left a comment

Choose a reason for hiding this comment

Uh oh!

helin24 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants