Skip to content

Sanitize control/format characters in console logger output across all formatters#128741

Draft
Copilot wants to merge 2 commits into
mainfrom
copilot/sanitize-console-logger-control-characters
Draft

Sanitize control/format characters in console logger output across all formatters#128741
Copilot wants to merge 2 commits into
mainfrom
copilot/sanitize-console-logger-control-characters

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 29, 2026

Console logging currently writes untrusted control characters verbatim, allowing terminal escape/control effects and ambiguous output. This change sanitizes control/format characters across Simple, Systemd, and Json formatter paths, with an explicit opt-out for compatibility.

  • Behavioral change

    • Added ConsoleFormatterOptions.SanitizeControlCharacters (default true).
    • Escapes Unicode Cc/Cf characters as \uXXXX before writing log output.
  • Implementation

    • Added shared sanitizer: ConsoleControlCharacterSanitizer.
    • Applied sanitization to:
      • message text
      • exception text
      • category/scope text
      • JSON state/scopes keys and string values
      • JSON timestamp string payload when present
  • Public surface

    • Updated Microsoft.Extensions.Logging.Console ref API with SanitizeControlCharacters.
  • Tests

    • Added coverage for default sanitization behavior across formatter types.
    • Added coverage for opt-out behavior (SanitizeControlCharacters = false) on non-JSON formatters.
    • Updated existing expectations where newline-containing exception output is now escaped.
builder.Logging.AddSimpleConsole(o =>
{
    o.SanitizeControlCharacters = true; // default
});

logger.LogInformation("Hello {Name}", "prefix\u001B[31mRED\u001B[0m");
// output contains escaped control chars (e.g. \u001B), not raw terminal sequences

@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-extensions-logging
See info in area-owners.md if you want to be subscribed.

Co-authored-by: rosebyte <14963300+rosebyte@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot May 29, 2026 05:14
Copilot AI changed the title [WIP] Sanitize control characters in console logger log messages Sanitize control/format characters in console logger output across all formatters May 29, 2026
Copilot AI requested a review from rosebyte May 29, 2026 05:16
Copy link
Copy Markdown
Member

@tarekgh tarekgh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(superseded by the comment below)

Copy link
Copy Markdown
Member

@tarekgh tarekgh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of the control character sanitization changes

The security motivation here is solid. Preventing terminal escape injection (ANSI sequences, bidi overrides, etc.) is worth doing. But the current implementation has several problems that need to be addressed before merging.


\n, \r, and \t should not be escaped

The sanitizer uses UnicodeCategory.Control which catches every character in U+0000-U+001F, including \n, \r, and \t. These are not security threats. They are structural formatting characters that the formatters depend on.

Both SimpleConsoleFormatter and SystemdConsoleFormatter have explicit downstream logic that operates on real newlines:

  • SimpleConsoleFormatter.WriteMessage calls message.Replace(Environment.NewLine, _newLineWithMessagePadding) to add indentation padding after each newline in exception text.
  • SystemdConsoleFormatter.WriteReplacingNewLine calls message.Replace(Environment.NewLine, " ") to flatten multi-line messages into a single line (required by systemd/journald).

Because the sanitizer runs before these calls, it converts \n to the literal text \u000A. The downstream Replace calls then find no real newlines and become no-ops. This breaks multi-line exception formatting in Simple mode (no padding) and breaks the single-line guarantee in Systemd mode.

The fix should target only the actually dangerous characters: ESC (\x1B), BEL (\x07), backspace (\x08), bidi overrides (\u202E, \u202D), and similar. Not \n/\r/\t.


Double-escaping on the JSON path

Utf8JsonWriter with JavaScriptEncoder.Default already escapes all control characters (U+0000-U+001F) and all non-BasicLatin characters (including \u202E). Pre-sanitizing the strings is redundant and produces double-escaped output.

For example, ESC (\x1B) would normally appear as \u001B in the JSON output. With the sanitizer, it becomes \\u001B, which is a literal backslash followed by u001B. JSON consumers parsing these logs would see the text \u001B instead of the actual ESC character. The test changes in JsonConsoleFormatterTests.cs confirm this: they switched to expecting \\\\u000D\\\\u000A.

The sanitizer should be skipped entirely for the JsonConsoleFormatter path, or at minimum should not run when Utf8JsonWriter is handling the escaping.


Breaking change with default true

Setting SanitizeControlCharacters = true by default changes the output format for every existing application without any opt-in. Exception stack traces go from properly formatted multi-line output to a single blob containing \u000A literals. This will break log parsing tools and dashboards that expect the current format.

Consider either defaulting to false or narrowing the escape set so that \n/\r/\t pass through unchanged (which would make the default safe).


Minor issues

  • API review: adding a public property to ConsoleFormatterOptions requires going through the dotnet/runtime API review process.
  • Allocations: every exception log triggers a StringBuilder allocation since exception strings always contain \n. Consider string.Create or ValueStringBuilder for the hot path.
  • Test coverage: Log_ControlCharacters_SanitizationCanBeDisabled only tests Simple and Systemd formatters. JSON opt-out is not covered.
  • Existing test expectations modified: the changes to ConsoleLoggerTest.cs normalize broken formatting as the new expected output rather than preserving the original behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Console logger should sanitize control characters in log messages

3 participants