diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx index bef187ea22..c3f686762f 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx @@ -8,6 +8,7 @@ import { describe, it, expect, vi } from 'vitest'; import { ToolConfirmationMessage } from './ToolConfirmationMessage.js'; import type { SerializableConfirmationDetails, + ToolCallConfirmationDetails, Config, } from '@google/gemini-cli-core'; import { renderWithProviders } from '../../../test-utils/render.js'; @@ -372,4 +373,35 @@ describe('ToolConfirmationMessage', () => { unmount(); }); }); + + it('should strip BiDi characters from MCP tool and server names', async () => { + const confirmationDetails: ToolCallConfirmationDetails = { + type: 'mcp', + title: 'Confirm MCP Tool', + serverName: 'test\u202Eserver', + toolName: 'test\u202Dtool', + toolDisplayName: 'Test Tool', + onConfirm: vi.fn(), + }; + + const { lastFrame, waitUntilReady, unmount } = renderWithProviders( + , + ); + await waitUntilReady(); + + const output = lastFrame(); + // BiDi characters \u202E and \u202D should be stripped + expect(output).toContain('MCP Server: testserver'); + expect(output).toContain('Tool: testtool'); + expect(output).toContain('Allow execution of MCP tool "testtool"'); + expect(output).toContain('from server "testserver"?'); + expect(output).toMatchSnapshot(); + unmount(); + }); }); diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx index 42642d66f9..84fb90d14f 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx @@ -21,7 +21,10 @@ import type { RadioSelectItem } from '../shared/RadioButtonSelect.js'; import { useToolActions } from '../../contexts/ToolActionsContext.js'; import { RadioButtonSelect } from '../shared/RadioButtonSelect.js'; import { MaxSizedBox, MINIMUM_MAX_HEIGHT } from '../shared/MaxSizedBox.js'; -import { sanitizeForDisplay } from '../../utils/textUtils.js'; +import { + sanitizeForDisplay, + stripUnsafeCharacters, +} from '../../utils/textUtils.js'; import { useKeypress } from '../../hooks/useKeypress.js'; import { theme } from '../../semantic-colors.js'; import { useSettings } from '../../contexts/SettingsContext.js'; @@ -324,15 +327,15 @@ export const ToolConfirmationMessage: React.FC< } else if (confirmationDetails.type === 'mcp') { // mcp tool confirmation const mcpProps = confirmationDetails; - question = `Allow execution of MCP tool "${mcpProps.toolName}" from server "${mcpProps.serverName}"?`; + question = `Allow execution of MCP tool "${sanitizeForDisplay(mcpProps.toolName)}" from server "${sanitizeForDisplay(mcpProps.serverName)}"?`; } if (confirmationDetails.type === 'edit') { if (!confirmationDetails.isModifying) { bodyContent = ( @@ -449,8 +452,12 @@ export const ToolConfirmationMessage: React.FC< bodyContent = ( - MCP Server: {mcpProps.serverName} - Tool: {mcpProps.toolName} + + MCP Server: {sanitizeForDisplay(mcpProps.serverName)} + + + Tool: {sanitizeForDisplay(mcpProps.toolName)} + ); } diff --git a/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessage.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessage.test.tsx.snap index 69574a60c6..72eda055d5 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessage.test.tsx.snap +++ b/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessage.test.tsx.snap @@ -35,6 +35,18 @@ Do you want to proceed? " `; +exports[`ToolConfirmationMessage > should strip BiDi characters from MCP tool and server names 1`] = ` +"MCP Server: testserver +Tool: testtool +Allow execution of MCP tool "testtool" from server "testserver"? + +● 1. Allow once + 2. Allow tool for this session + 3. Allow all server tools for this session + 4. No, suggest changes (esc) +" +`; + exports[`ToolConfirmationMessage > with folder trust > 'for edit confirmations' > should NOT show "allow always" when folder is untrusted 1`] = ` "╭──────────────────────────────────────────────────────────────────────────────╮ │ │ diff --git a/packages/cli/src/ui/utils/InlineMarkdownRenderer.tsx b/packages/cli/src/ui/utils/InlineMarkdownRenderer.tsx index 0418582919..430b27eeb3 100644 --- a/packages/cli/src/ui/utils/InlineMarkdownRenderer.tsx +++ b/packages/cli/src/ui/utils/InlineMarkdownRenderer.tsx @@ -8,6 +8,7 @@ import React from 'react'; import { Text } from 'ink'; import { theme } from '../semantic-colors.js'; import { debugLogger } from '@google/gemini-cli-core'; +import { stripUnsafeCharacters } from './textUtils.js'; // Constants for Markdown parsing const BOLD_MARKER_LENGTH = 2; // For "**" @@ -23,9 +24,10 @@ interface RenderInlineProps { } const RenderInlineInternal: React.FC = ({ - text, + text: rawText, defaultColor, }) => { + const text = stripUnsafeCharacters(rawText); const baseColor = defaultColor ?? theme.text.primary; // Early return for plain text without markdown or URLs if (!/[*_~`<[https?:]/.test(text)) { diff --git a/packages/cli/src/ui/utils/TableRenderer.tsx b/packages/cli/src/ui/utils/TableRenderer.tsx index fd19b51000..4689d461ff 100644 --- a/packages/cli/src/ui/utils/TableRenderer.tsx +++ b/packages/cli/src/ui/utils/TableRenderer.tsx @@ -17,6 +17,7 @@ import { } from 'ink'; import { theme } from '../semantic-colors.js'; import { RenderInline } from './InlineMarkdownRenderer.js'; +import { stripUnsafeCharacters } from './textUtils.js'; interface TableRendererProps { headers: string[]; @@ -60,12 +61,18 @@ export const TableRenderer: React.FC = ({ ); const styledHeaders = useMemo( - () => cleanedHeaders.map((header) => toStyledCharacters(header)), + () => + cleanedHeaders.map((header) => + toStyledCharacters(stripUnsafeCharacters(header)), + ), [cleanedHeaders], ); const styledRows = useMemo( - () => rows.map((row) => row.map((cell) => toStyledCharacters(cell))), + () => + rows.map((row) => + row.map((cell) => toStyledCharacters(stripUnsafeCharacters(cell))), + ), [rows], ); diff --git a/packages/cli/src/ui/utils/textUtils.test.ts b/packages/cli/src/ui/utils/textUtils.test.ts index be7f69d9f6..4927486d43 100644 --- a/packages/cli/src/ui/utils/textUtils.test.ts +++ b/packages/cli/src/ui/utils/textUtils.test.ts @@ -332,6 +332,35 @@ describe('textUtils', () => { }); }); + describe('BiDi and deceptive Unicode characters', () => { + it('should strip BiDi override characters', () => { + const input = 'safe\u202Etxt.sh'; + // When stripped, it should be 'safetxt.sh' + expect(stripUnsafeCharacters(input)).toBe('safetxt.sh'); + }); + + it('should strip all BiDi control characters (LRM, RLM, U+202A-U+202E, U+2066-U+2069)', () => { + const bidiChars = + '\u200E\u200F\u202A\u202B\u202C\u202D\u202E\u2066\u2067\u2068\u2069'; + expect(stripUnsafeCharacters('a' + bidiChars + 'b')).toBe('ab'); + }); + + it('should strip zero-width characters (U+200B, U+FEFF)', () => { + const zeroWidthChars = '\u200B\uFEFF'; + expect(stripUnsafeCharacters('a' + zeroWidthChars + 'b')).toBe('ab'); + }); + + it('should preserve ZWJ (U+200D) for complex emojis', () => { + const input = 'Family: 👨‍👩‍👧‍👦'; + expect(stripUnsafeCharacters(input)).toBe('Family: 👨‍👩‍👧‍👦'); + }); + + it('should preserve ZWNJ (U+200C)', () => { + const input = 'hello\u200Cworld'; + expect(stripUnsafeCharacters(input)).toBe('hello\u200Cworld'); + }); + }); + describe('performance: regex vs array-based', () => { it('should handle real-world terminal output with control chars', () => { // Simulate terminal output with various control sequences diff --git a/packages/cli/src/ui/utils/textUtils.ts b/packages/cli/src/ui/utils/textUtils.ts index d2ad40c148..bd7e2aca75 100644 --- a/packages/cli/src/ui/utils/textUtils.ts +++ b/packages/cli/src/ui/utils/textUtils.ts @@ -106,9 +106,13 @@ export function cpSlice(str: string, start: number, end?: number): string { * - VT control sequences (via Node.js util.stripVTControlCharacters) * - C0 control chars (0x00-0x1F) except TAB(0x09), LF(0x0A), CR(0x0D) * - C1 control chars (0x80-0x9F) that can cause display issues + * - BiDi control chars (U+200E, U+200F, U+202A-U+202E, U+2066-U+2069) + * - Zero-width chars (U+200B, U+FEFF) * * Characters preserved: * - All printable Unicode including emojis + * - ZWJ (U+200D) - needed for complex emoji sequences + * - ZWNJ (U+200C) - preserve zero-width non-joiner * - DEL (0x7F) - handled functionally by applyOperations, not a display issue * - CR/LF (0x0D/0x0A) - needed for line breaks * - TAB (0x09) - preserve tabs @@ -120,8 +124,13 @@ export function stripUnsafeCharacters(str: string): string { // Use a regex to strip remaining unsafe control characters // C0: 0x00-0x1F except 0x09 (TAB), 0x0A (LF), 0x0D (CR) // C1: 0x80-0x9F - // eslint-disable-next-line no-control-regex - return strippedVT.replace(/[\x00-\x08\x0B\x0C\x0E-\x1F\x80-\x9F]/g, ''); + // BiDi: U+200E (LRM), U+200F (RLM), U+202A-U+202E, U+2066-U+2069 + // Zero-width: U+200B (ZWSP), U+FEFF (BOM) + return strippedVT.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, + '', + ); } /**