fix(cli): resolve text sanitization data loss due to C1 control characters

[euxaristia]

Fixes #22616

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical bug in the CLI's text sanitization utility. Previously, the stripUnsafeCharacters function could lead to unintended data loss when encountering C1 control characters, as a subsequent sanitization step would misinterpret these characters. The changes introduce an early stripping mechanism for C1 control characters, ensuring robust and accurate text processing without compromising data integrity.

Highlights

• Text Sanitization Fix: Resolved an issue in the stripUnsafeCharacters function that caused data loss when processing text containing C1 control characters. The fix ensures these characters are stripped early to prevent stripVTControlCharacters from misinterpreting them and inadvertently removing valid text.

Changelog

• packages/cli/src/ui/utils/textUtils.ts
  • Introduced an early stripping step for C1 control characters (0x80-0x9F) in the stripUnsafeCharacters function.
  • Removed C1 control characters from the final regex pattern as they are now handled earlier.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request aims to fix a data loss issue caused by C1 control characters during text sanitization. While the intention to correctly handle ANSI sequences is valid, the proposed solution in the original comment for textUtils.ts (lines 121-126) suggests stripping all C1 and VT control characters after ANSI sequences. This contradicts repository Rule 5, which mandates a more lenient approach for list display contexts, where only ANSI codes and newlines/tabs should be stripped. I have modified the critical review comment to align with this rule, ensuring that only ANSI sequences are removed, thus preserving the desired lenient behavior while still addressing the original issue of incorrect ANSI stripping order.

[gemini-cli]

Hi there! Thank you for your interest in contributing to Gemini CLI.

To ensure we maintain high code quality and focus on our prioritized roadmap, we have updated our contribution policy (see Discussion #17383).

We only guarantee review and consideration of pull requests for issues that are explicitly labeled as 'help wanted'. All other community pull requests are subject to closure after 14 days if they do not align with our current focus areas. For this reason, we strongly recommend that contributors only submit pull requests against issues explicitly labeled as 'help-wanted'.

This pull request is being closed as it has been open for 14 days without a 'help wanted' designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding and for being part of our community!

[cocosheng-g]

@euxaristia, apologies for the bot closing this PR! We have reopened it. Please sync your branch to the latest main and we will have someone review it shortly.

[cocosheng-g]

Please add test

[euxaristia]

Please add test

Done!

[cocosheng-g]

/gemini review

[gemini-code-assist]

Code Review

This pull request modifies the stripUnsafeCharacters function to strip C1 control characters (0x80-0x9F) early, preventing stripVTControlCharacters from misinterpreting them as unterminated sequences and swallowing subsequent text. The review feedback suggests reordering this operation to occur after stripAnsi so that 8-bit ANSI sequences like CSI (0x9B) are fully removed rather than leaving behind trailing parameters as literal text. Additionally, it was recommended to update the new regression tests to reflect this improved stripping behavior.

[euxaristia]

/gemini review

[gemini-code-assist]

Code Review

This pull request addresses a data loss issue in stripUnsafeCharacters by stripping C1 control characters before processing VT sequences. I have provided feedback regarding the potential use of this function for list display, suggesting a more efficient regex-based approach and requesting documentation updates if a more lenient sanitization strategy is intended, as per project guidelines.

[gemini-code-assist]

While this change correctly fixes the data loss issue, please ensure that the implementation does not use stripUnsafeCharacters for list display, as the current lenient behavior of only stripping ANSI codes and replacing newlines/tabs is desired. If a more lenient approach is used, please document the rationale in the function documentation as per repository rules.

  const strippedAnsi = stripAnsi(str);

  // Strip C0, C1, and other unsafe characters via regex first.
  // This is more efficient than multiple replaces and crucially removes C1
  // characters (e.g., 0x90 DCS) before they can be misinterpreted by
  // stripVTControlCharacters, which could otherwise cause data loss.
  const strippedWithRegex = strippedAnsi.replace(
    // eslint-disable-next-line no-control-regex
    /[\x00-\x08\x0B\x0C\x0E-\x1F\x80-\x9F\u200E\u200F\u202A-\u202E\u2066-\u2069\u200B\uFEFF]/g,
    ''
  );

  // Finally, use stripVTControlCharacters for any remaining VT sequences
  // that the regex might not cover.
  return stripVTControlCharacters(strippedWithRegex);

References

1. Do not use stripUnsafeCharacters for sanitizing strings for list display; the current more lenient behavior of only stripping ANSI codes and replacing newlines/tabs is desired. If a more lenient behavior is used, explain the rationale in the function documentation.

[cocosheng-g]

Please address gemini comment

[euxaristia]

/gemini review

[gemini-code-assist]

Code Review

This pull request modifies the stripUnsafeCharacters function in textUtils.ts to prevent data loss when handling C1 control characters. The sanitization logic now applies a regex to strip C0 and C1 characters before invoking stripVTControlCharacters, ensuring that sequences like Device Control Strings (0x90) do not cause subsequent text to be swallowed. Corresponding regression tests were added to textUtils.test.ts to cover DCS and 8-bit CSI sequences. I have no feedback to provide.