From d956c5b221f163c036d3f08df623736e5ce55150 Mon Sep 17 00:00:00 2001 From: Emily Hedlund Date: Thu, 22 Jan 2026 11:41:51 -0500 Subject: [PATCH] Sanitize command names and descriptions (#17228) --- .../src/services/FileCommandLoader.test.ts | 65 +++++++++++++++++++ .../cli/src/services/FileCommandLoader.ts | 19 ++++-- packages/cli/src/ui/components/Help.tsx | 7 +- .../src/ui/components/SuggestionsDisplay.tsx | 4 +- packages/cli/src/ui/utils/textUtils.test.ts | 25 +++++++ packages/cli/src/ui/utils/textUtils.ts | 21 ++++++ 6 files changed, 134 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/services/FileCommandLoader.test.ts b/packages/cli/src/services/FileCommandLoader.test.ts index cdfef1532e..077b8c45fe 100644 --- a/packages/cli/src/services/FileCommandLoader.test.ts +++ b/packages/cli/src/services/FileCommandLoader.test.ts @@ -1337,4 +1337,69 @@ describe('FileCommandLoader', () => { consoleErrorSpy.mockRestore(); }); }); + + describe('Sanitization', () => { + it('sanitizes command names from filenames containing control characters', async () => { + const userCommandsDir = Storage.getUserCommandsDir(); + mock({ + [userCommandsDir]: { + 'test\twith\nnewlines.toml': 'prompt = "Test prompt"', + }, + }); + + const loader = new FileCommandLoader(null); + const commands = await loader.loadCommands(signal); + expect(commands).toHaveLength(1); + // Non-alphanumeric characters (except - and .) become underscores + expect(commands[0].name).toBe('test_with_newlines'); + }); + + it('truncates excessively long filenames', async () => { + const userCommandsDir = Storage.getUserCommandsDir(); + const longName = 'a'.repeat(60) + '.toml'; + mock({ + [userCommandsDir]: { + [longName]: 'prompt = "Test prompt"', + }, + }); + + const loader = new FileCommandLoader(null); + const commands = await loader.loadCommands(signal); + expect(commands).toHaveLength(1); + expect(commands[0].name.length).toBe(50); + expect(commands[0].name).toBe('a'.repeat(47) + '...'); + }); + + it('sanitizes descriptions containing newlines and ANSI codes', async () => { + const userCommandsDir = Storage.getUserCommandsDir(); + mock({ + [userCommandsDir]: { + 'test.toml': + 'prompt = "Test"\ndescription = "Line 1\\nLine 2\\tTabbed\\r\\n\\u001B[31mRed text\\u001B[0m"', + }, + }); + + const loader = new FileCommandLoader(null); + const commands = await loader.loadCommands(signal); + expect(commands).toHaveLength(1); + // Newlines and tabs become spaces, ANSI is stripped + expect(commands[0].description).toBe('Line 1 Line 2 Tabbed Red text'); + }); + + it('truncates long descriptions', async () => { + const userCommandsDir = Storage.getUserCommandsDir(); + const longDesc = 'd'.repeat(150); + mock({ + [userCommandsDir]: { + 'test.toml': `prompt = "Test"\ndescription = "${longDesc}"`, + }, + }); + + const loader = new FileCommandLoader(null); + const commands = await loader.loadCommands(signal); + expect(commands).toHaveLength(1); + expect(commands[0].description.length).toBe(100); + expect(commands[0].description).toBe('d'.repeat(97) + '...'); + }); + }); }); diff --git a/packages/cli/src/services/FileCommandLoader.ts b/packages/cli/src/services/FileCommandLoader.ts index 688fb0ce0e..3d7711d6ea 100644 --- a/packages/cli/src/services/FileCommandLoader.ts +++ b/packages/cli/src/services/FileCommandLoader.ts @@ -33,6 +33,7 @@ import { ShellProcessor, } from './prompt-processors/shellProcessor.js'; import { AtFileProcessor } from './prompt-processors/atFileProcessor.js'; +import { sanitizeForListDisplay } from '../ui/utils/textUtils.js'; interface CommandDirectory { path: string; @@ -230,15 +231,25 @@ export class FileCommandLoader implements ICommandLoader { ); const baseCommandName = relativePath .split(path.sep) - // Sanitize each path segment to prevent ambiguity. Since ':' is our - // namespace separator, we replace any literal colons in filenames - // with underscores to avoid naming conflicts. - .map((segment) => segment.replaceAll(':', '_')) + // Sanitize each path segment to prevent ambiguity, replacing non-allowlisted characters with underscores. + // Since ':' is our namespace separator, this ensures that colons do not cause naming conflicts. + .map((segment) => { + let sanitized = segment.replace(/[^a-zA-Z0-9_\-.]/g, '_'); + + // Truncate excessively long segments to prevent UI overflow + if (sanitized.length > 50) { + sanitized = sanitized.substring(0, 47) + '...'; + } + return sanitized; + }) .join(':'); // Add extension name tag for extension commands const defaultDescription = `Custom command from ${path.basename(filePath)}`; let description = validDef.description || defaultDescription; + + description = sanitizeForListDisplay(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 8afee226a0..7dd01f1423 100644 --- a/packages/cli/src/ui/components/Help.tsx +++ b/packages/cli/src/ui/components/Help.tsx @@ -9,6 +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'; interface Help { commands: readonly SlashCommand[]; @@ -77,7 +78,8 @@ export const Help: React.FC = ({ commands }) => ( {command.kind === CommandKind.MCP_PROMPT && ( [MCP] )} - {command.description && ' - ' + command.description} + {command.description && + ' - ' + sanitizeForListDisplay(command.description, 100)} {command.subCommands && command.subCommands @@ -88,7 +90,8 @@ export const Help: React.FC = ({ commands }) => ( {' '} {subCommand.name} - {subCommand.description && ' - ' + subCommand.description} + {subCommand.description && + ' - ' + sanitizeForListDisplay(subCommand.description, 100)} ))} diff --git a/packages/cli/src/ui/components/SuggestionsDisplay.tsx b/packages/cli/src/ui/components/SuggestionsDisplay.tsx index eb08997e4f..7125ad0a0f 100644 --- a/packages/cli/src/ui/components/SuggestionsDisplay.tsx +++ b/packages/cli/src/ui/components/SuggestionsDisplay.tsx @@ -9,6 +9,8 @@ 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'; + export interface Suggestion { label: string; value: string; @@ -115,7 +117,7 @@ export function SuggestionsDisplay({ {suggestion.description && ( - {suggestion.description} + {sanitizeForListDisplay(suggestion.description, 100)} )} diff --git a/packages/cli/src/ui/utils/textUtils.test.ts b/packages/cli/src/ui/utils/textUtils.test.ts index e9bb0c196a..3a7a6ac1f3 100644 --- a/packages/cli/src/ui/utils/textUtils.test.ts +++ b/packages/cli/src/ui/utils/textUtils.test.ts @@ -13,9 +13,34 @@ import { escapeAnsiCtrlCodes, stripUnsafeCharacters, getCachedStringWidth, + sanitizeForListDisplay, } 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'); + }); + + 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'); + }); + + it('should truncate long strings', () => { + const longInput = 'a'.repeat(50); + expect(sanitizeForListDisplay(longInput, 20)).toBe( + 'a'.repeat(17) + '...', + ); + }); + + it('should handle empty or null input', () => { + expect(sanitizeForListDisplay('')).toBe(''); + expect(sanitizeForListDisplay(null as unknown as string)).toBe(''); + }); + }); + describe('getCachedStringWidth', () => { it('should handle unicode characters that crash string-width', () => { // U+0602 caused string-width to crash (see #16418) diff --git a/packages/cli/src/ui/utils/textUtils.ts b/packages/cli/src/ui/utils/textUtils.ts index 8da60a3821..65fbed1df9 100644 --- a/packages/cli/src/ui/utils/textUtils.ts +++ b/packages/cli/src/ui/utils/textUtils.ts @@ -123,6 +123,27 @@ export function stripUnsafeCharacters(str: string): string { .join(''); } +/** + * 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. + */ +export function sanitizeForListDisplay( + str: string, + maxLength?: number, +): string { + if (!str) { + return ''; + } + + let sanitized = stripAnsi(str).replace(/\s+/g, ' '); + + if (maxLength && sanitized.length > maxLength) { + sanitized = sanitized.substring(0, maxLength - 3) + '...'; + } + + return sanitized; +} + const stringWidthCache = new LRUCache( LRU_BUFFER_PERF_CACHE_LIMIT, );