From 8b2b71c8ef8f38a9ef286f18d90489396c01d188 Mon Sep 17 00:00:00 2001 From: Tommaso Sciortino Date: Tue, 27 Jan 2026 08:56:01 -0800 Subject: [PATCH] Change formatting to prevent UI redressing attacks (#17611) --- .../cli/src/services/FileCommandLoader.ts | 4 ++-- packages/cli/src/ui/components/Help.tsx | 6 +++--- .../src/ui/components/SuggestionsDisplay.tsx | 4 ++-- .../messages/ToolConfirmationMessage.tsx | 5 +++-- packages/cli/src/ui/utils/textUtils.test.ts | 19 +++++++++++-------- packages/cli/src/ui/utils/textUtils.ts | 12 +++++------- 6 files changed, 26 insertions(+), 24 deletions(-) diff --git a/packages/cli/src/services/FileCommandLoader.ts b/packages/cli/src/services/FileCommandLoader.ts index 3d7711d6ea..5bfbcd8996 100644 --- a/packages/cli/src/services/FileCommandLoader.ts +++ b/packages/cli/src/services/FileCommandLoader.ts @@ -33,7 +33,7 @@ import { ShellProcessor, } from './prompt-processors/shellProcessor.js'; import { AtFileProcessor } from './prompt-processors/atFileProcessor.js'; -import { sanitizeForListDisplay } from '../ui/utils/textUtils.js'; +import { sanitizeForDisplay } from '../ui/utils/textUtils.js'; interface CommandDirectory { path: string; @@ -248,7 +248,7 @@ export class FileCommandLoader implements ICommandLoader { const defaultDescription = `Custom command from ${path.basename(filePath)}`; let description = validDef.description || defaultDescription; - description = sanitizeForListDisplay(description, 100); + description = sanitizeForDisplay(description, 100); if (extensionName) { description = `[${extensionName}] ${description}`; diff --git a/packages/cli/src/ui/components/Help.tsx b/packages/cli/src/ui/components/Help.tsx index 7dd01f1423..762b8e9ff3 100644 --- a/packages/cli/src/ui/components/Help.tsx +++ b/packages/cli/src/ui/components/Help.tsx @@ -9,7 +9,7 @@ import { Box, Text } from 'ink'; import { theme } from '../semantic-colors.js'; import { type SlashCommand, CommandKind } from '../commands/types.js'; import { KEYBOARD_SHORTCUTS_URL } from '../constants.js'; -import { sanitizeForListDisplay } from '../utils/textUtils.js'; +import { sanitizeForDisplay } from '../utils/textUtils.js'; interface Help { commands: readonly SlashCommand[]; @@ -79,7 +79,7 @@ export const Help: React.FC = ({ commands }) => ( [MCP] )} {command.description && - ' - ' + sanitizeForListDisplay(command.description, 100)} + ' - ' + sanitizeForDisplay(command.description, 100)} {command.subCommands && command.subCommands @@ -91,7 +91,7 @@ export const Help: React.FC = ({ commands }) => ( {subCommand.name} {subCommand.description && - ' - ' + sanitizeForListDisplay(subCommand.description, 100)} + ' - ' + sanitizeForDisplay(subCommand.description, 100)} ))} diff --git a/packages/cli/src/ui/components/SuggestionsDisplay.tsx b/packages/cli/src/ui/components/SuggestionsDisplay.tsx index 7125ad0a0f..d9498e7a6b 100644 --- a/packages/cli/src/ui/components/SuggestionsDisplay.tsx +++ b/packages/cli/src/ui/components/SuggestionsDisplay.tsx @@ -9,7 +9,7 @@ import { theme } from '../semantic-colors.js'; import { ExpandableText, MAX_WIDTH } from './shared/ExpandableText.js'; import { CommandKind } from '../commands/types.js'; import { Colors } from '../colors.js'; -import { sanitizeForListDisplay } from '../utils/textUtils.js'; +import { sanitizeForDisplay } from '../utils/textUtils.js'; export interface Suggestion { label: string; @@ -117,7 +117,7 @@ export function SuggestionsDisplay({ {suggestion.description && ( - {sanitizeForListDisplay(suggestion.description, 100)} + {sanitizeForDisplay(suggestion.description, 100)} )} diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx index 90ecb0d8dd..3aef6bd529 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx @@ -21,6 +21,7 @@ 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 { useKeypress } from '../../hooks/useKeypress.js'; import { theme } from '../../semantic-colors.js'; import { useSettings } from '../../contexts/SettingsContext.js'; @@ -257,7 +258,7 @@ export const ToolConfirmationMessage: React.FC< if (executionProps.commands && executionProps.commands.length > 1) { question = `Allow execution of ${executionProps.commands.length} commands?`; } else { - question = `Allow execution of: '${executionProps.rootCommand}'?`; + question = `Allow execution of: '${sanitizeForDisplay(executionProps.rootCommand)}'?`; } } else if (confirmationDetails.type === 'info') { question = `Do you want to proceed?`; @@ -346,7 +347,7 @@ export const ToolConfirmationMessage: React.FC< {commandsToDisplay.map((cmd, idx) => ( - {cmd} + {sanitizeForDisplay(cmd)} ))} diff --git a/packages/cli/src/ui/utils/textUtils.test.ts b/packages/cli/src/ui/utils/textUtils.test.ts index 3a7a6ac1f3..62462dddf6 100644 --- a/packages/cli/src/ui/utils/textUtils.test.ts +++ b/packages/cli/src/ui/utils/textUtils.test.ts @@ -13,31 +13,34 @@ import { escapeAnsiCtrlCodes, stripUnsafeCharacters, getCachedStringWidth, - sanitizeForListDisplay, + sanitizeForDisplay, } from './textUtils.js'; describe('textUtils', () => { describe('sanitizeForListDisplay', () => { it('should strip ANSI codes and replace newlines/tabs with spaces', () => { const input = '\u001b[31mLine 1\nLine 2\tTabbed\r\nEnd\u001b[0m'; - expect(sanitizeForListDisplay(input)).toBe('Line 1 Line 2 Tabbed End'); + expect(sanitizeForDisplay(input)).toBe('Line 1 Line 2 Tabbed End'); }); it('should collapse multiple consecutive whitespace characters into a single space', () => { const input = 'Multiple \n\n newlines and \t\t tabs'; - expect(sanitizeForListDisplay(input)).toBe('Multiple newlines and tabs'); + expect(sanitizeForDisplay(input)).toBe('Multiple newlines and tabs'); }); it('should truncate long strings', () => { const longInput = 'a'.repeat(50); - expect(sanitizeForListDisplay(longInput, 20)).toBe( - 'a'.repeat(17) + '...', - ); + expect(sanitizeForDisplay(longInput, 20)).toBe('a'.repeat(17) + '...'); }); it('should handle empty or null input', () => { - expect(sanitizeForListDisplay('')).toBe(''); - expect(sanitizeForListDisplay(null as unknown as string)).toBe(''); + expect(sanitizeForDisplay('')).toBe(''); + expect(sanitizeForDisplay(null as unknown as string)).toBe(''); + }); + + it('should strip control characters like backspace', () => { + const input = 'Hello\x08 World'; + expect(sanitizeForDisplay(input)).toBe('Hello World'); }); }); diff --git a/packages/cli/src/ui/utils/textUtils.ts b/packages/cli/src/ui/utils/textUtils.ts index 65fbed1df9..e95977086d 100644 --- a/packages/cli/src/ui/utils/textUtils.ts +++ b/packages/cli/src/ui/utils/textUtils.ts @@ -124,18 +124,16 @@ export function stripUnsafeCharacters(str: string): string { } /** - * Sanitize a string for display in list-like UI components (e.g. Help, Suggestions). - * Removes ANSI codes, collapses whitespace characters into a single space, and optionally truncates. + * Sanitize a string for display in inline UI components (e.g. Help, Suggestions). + * Removes ANSI codes, dangerous control characters, collapses whitespace + * characters into a single space, and optionally truncates. */ -export function sanitizeForListDisplay( - str: string, - maxLength?: number, -): string { +export function sanitizeForDisplay(str: string, maxLength?: number): string { if (!str) { return ''; } - let sanitized = stripAnsi(str).replace(/\s+/g, ' '); + let sanitized = stripUnsafeCharacters(str).replace(/\s+/g, ' '); if (maxLength && sanitized.length > maxLength) { sanitized = sanitized.substring(0, maxLength - 3) + '...';