Change formatting to prevent UI redressing attacks (#17611)

This commit is contained in:
Tommaso Sciortino
2026-01-27 08:56:01 -08:00
committed by GitHub
parent 6be42be575
commit 8b2b71c8ef
6 changed files with 26 additions and 24 deletions

View File

@@ -33,7 +33,7 @@ import {
ShellProcessor, ShellProcessor,
} from './prompt-processors/shellProcessor.js'; } from './prompt-processors/shellProcessor.js';
import { AtFileProcessor } from './prompt-processors/atFileProcessor.js'; import { AtFileProcessor } from './prompt-processors/atFileProcessor.js';
import { sanitizeForListDisplay } from '../ui/utils/textUtils.js'; import { sanitizeForDisplay } from '../ui/utils/textUtils.js';
interface CommandDirectory { interface CommandDirectory {
path: string; path: string;
@@ -248,7 +248,7 @@ export class FileCommandLoader implements ICommandLoader {
const defaultDescription = `Custom command from ${path.basename(filePath)}`; const defaultDescription = `Custom command from ${path.basename(filePath)}`;
let description = validDef.description || defaultDescription; let description = validDef.description || defaultDescription;
description = sanitizeForListDisplay(description, 100); description = sanitizeForDisplay(description, 100);
if (extensionName) { if (extensionName) {
description = `[${extensionName}] ${description}`; description = `[${extensionName}] ${description}`;

View File

@@ -9,7 +9,7 @@ import { Box, Text } from 'ink';
import { theme } from '../semantic-colors.js'; import { theme } from '../semantic-colors.js';
import { type SlashCommand, CommandKind } from '../commands/types.js'; import { type SlashCommand, CommandKind } from '../commands/types.js';
import { KEYBOARD_SHORTCUTS_URL } from '../constants.js'; import { KEYBOARD_SHORTCUTS_URL } from '../constants.js';
import { sanitizeForListDisplay } from '../utils/textUtils.js'; import { sanitizeForDisplay } from '../utils/textUtils.js';
interface Help { interface Help {
commands: readonly SlashCommand[]; commands: readonly SlashCommand[];
@@ -79,7 +79,7 @@ export const Help: React.FC<Help> = ({ commands }) => (
<Text color={theme.text.secondary}> [MCP]</Text> <Text color={theme.text.secondary}> [MCP]</Text>
)} )}
{command.description && {command.description &&
' - ' + sanitizeForListDisplay(command.description, 100)} ' - ' + sanitizeForDisplay(command.description, 100)}
</Text> </Text>
{command.subCommands && {command.subCommands &&
command.subCommands command.subCommands
@@ -91,7 +91,7 @@ export const Help: React.FC<Help> = ({ commands }) => (
{subCommand.name} {subCommand.name}
</Text> </Text>
{subCommand.description && {subCommand.description &&
' - ' + sanitizeForListDisplay(subCommand.description, 100)} ' - ' + sanitizeForDisplay(subCommand.description, 100)}
</Text> </Text>
))} ))}
</Box> </Box>

View File

@@ -9,7 +9,7 @@ import { theme } from '../semantic-colors.js';
import { ExpandableText, MAX_WIDTH } from './shared/ExpandableText.js'; import { ExpandableText, MAX_WIDTH } from './shared/ExpandableText.js';
import { CommandKind } from '../commands/types.js'; import { CommandKind } from '../commands/types.js';
import { Colors } from '../colors.js'; import { Colors } from '../colors.js';
import { sanitizeForListDisplay } from '../utils/textUtils.js'; import { sanitizeForDisplay } from '../utils/textUtils.js';
export interface Suggestion { export interface Suggestion {
label: string; label: string;
@@ -117,7 +117,7 @@ export function SuggestionsDisplay({
{suggestion.description && ( {suggestion.description && (
<Box flexGrow={1} paddingLeft={3}> <Box flexGrow={1} paddingLeft={3}>
<Text color={textColor} wrap="truncate"> <Text color={textColor} wrap="truncate">
{sanitizeForListDisplay(suggestion.description, 100)} {sanitizeForDisplay(suggestion.description, 100)}
</Text> </Text>
</Box> </Box>
)} )}

View File

@@ -21,6 +21,7 @@ import type { RadioSelectItem } from '../shared/RadioButtonSelect.js';
import { useToolActions } from '../../contexts/ToolActionsContext.js'; import { useToolActions } from '../../contexts/ToolActionsContext.js';
import { RadioButtonSelect } from '../shared/RadioButtonSelect.js'; import { RadioButtonSelect } from '../shared/RadioButtonSelect.js';
import { MaxSizedBox, MINIMUM_MAX_HEIGHT } from '../shared/MaxSizedBox.js'; import { MaxSizedBox, MINIMUM_MAX_HEIGHT } from '../shared/MaxSizedBox.js';
import { sanitizeForDisplay } from '../../utils/textUtils.js';
import { useKeypress } from '../../hooks/useKeypress.js'; import { useKeypress } from '../../hooks/useKeypress.js';
import { theme } from '../../semantic-colors.js'; import { theme } from '../../semantic-colors.js';
import { useSettings } from '../../contexts/SettingsContext.js'; import { useSettings } from '../../contexts/SettingsContext.js';
@@ -257,7 +258,7 @@ export const ToolConfirmationMessage: React.FC<
if (executionProps.commands && executionProps.commands.length > 1) { if (executionProps.commands && executionProps.commands.length > 1) {
question = `Allow execution of ${executionProps.commands.length} commands?`; question = `Allow execution of ${executionProps.commands.length} commands?`;
} else { } else {
question = `Allow execution of: '${executionProps.rootCommand}'?`; question = `Allow execution of: '${sanitizeForDisplay(executionProps.rootCommand)}'?`;
} }
} else if (confirmationDetails.type === 'info') { } else if (confirmationDetails.type === 'info') {
question = `Do you want to proceed?`; question = `Do you want to proceed?`;
@@ -346,7 +347,7 @@ export const ToolConfirmationMessage: React.FC<
<Box flexDirection="column"> <Box flexDirection="column">
{commandsToDisplay.map((cmd, idx) => ( {commandsToDisplay.map((cmd, idx) => (
<Text key={idx} color={theme.text.link}> <Text key={idx} color={theme.text.link}>
{cmd} {sanitizeForDisplay(cmd)}
</Text> </Text>
))} ))}
</Box> </Box>

View File

@@ -13,31 +13,34 @@ import {
escapeAnsiCtrlCodes, escapeAnsiCtrlCodes,
stripUnsafeCharacters, stripUnsafeCharacters,
getCachedStringWidth, getCachedStringWidth,
sanitizeForListDisplay, sanitizeForDisplay,
} from './textUtils.js'; } from './textUtils.js';
describe('textUtils', () => { describe('textUtils', () => {
describe('sanitizeForListDisplay', () => { describe('sanitizeForListDisplay', () => {
it('should strip ANSI codes and replace newlines/tabs with spaces', () => { it('should strip ANSI codes and replace newlines/tabs with spaces', () => {
const input = '\u001b[31mLine 1\nLine 2\tTabbed\r\nEnd\u001b[0m'; 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', () => { it('should collapse multiple consecutive whitespace characters into a single space', () => {
const input = 'Multiple \n\n newlines and \t\t tabs'; 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', () => { it('should truncate long strings', () => {
const longInput = 'a'.repeat(50); const longInput = 'a'.repeat(50);
expect(sanitizeForListDisplay(longInput, 20)).toBe( expect(sanitizeForDisplay(longInput, 20)).toBe('a'.repeat(17) + '...');
'a'.repeat(17) + '...',
);
}); });
it('should handle empty or null input', () => { it('should handle empty or null input', () => {
expect(sanitizeForListDisplay('')).toBe(''); expect(sanitizeForDisplay('')).toBe('');
expect(sanitizeForListDisplay(null as unknown as string)).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');
}); });
}); });

View File

@@ -124,18 +124,16 @@ export function stripUnsafeCharacters(str: string): string {
} }
/** /**
* Sanitize a string for display in list-like UI components (e.g. Help, Suggestions). * Sanitize a string for display in inline UI components (e.g. Help, Suggestions).
* Removes ANSI codes, collapses whitespace characters into a single space, and optionally truncates. * Removes ANSI codes, dangerous control characters, collapses whitespace
* characters into a single space, and optionally truncates.
*/ */
export function sanitizeForListDisplay( export function sanitizeForDisplay(str: string, maxLength?: number): string {
str: string,
maxLength?: number,
): string {
if (!str) { if (!str) {
return ''; return '';
} }
let sanitized = stripAnsi(str).replace(/\s+/g, ' '); let sanitized = stripUnsafeCharacters(str).replace(/\s+/g, ' ');
if (maxLength && sanitized.length > maxLength) { if (maxLength && sanitized.length > maxLength) {
sanitized = sanitized.substring(0, maxLength - 3) + '...'; sanitized = sanitized.substring(0, maxLength - 3) + '...';